Date: Wed, 26 Aug 2015 13:04:43 +0200
From: Thierry Reding <thierry.reding@gmail.com>
To: Xinliang Liu <xinliang.liu@linaro.org>, airlied@linux.ie,
        dri-devel@lists.freedesktop.org, liguozhu@hisilicon.com,
        linux-kernel@vger.kernel.org, benjamin.gaignard@linaro.org
Subject: Re: [PATCH] drm/crtc: Add a helper func to get a registered crtc
 from its index
Message-ID: <20150826110441.GA320@ulmo.nvidia.com>
References: <1440472431-50874-1-git-send-email-xinliang.liu@linaro.org>
 <20150825093618.GA20434@phenom.ffwll.local>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="wRRV7LY7NUeQGEoC"
Content-Disposition: inline
In-Reply-To: <20150825093618.GA20434@phenom.ffwll.local>
User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3821
Lines: 95


--wRRV7LY7NUeQGEoC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Aug 25, 2015 at 11:36:18AM +0200, Daniel Vetter wrote:
> On Tue, Aug 25, 2015 at 11:13:51AM +0800, Xinliang Liu wrote:
> > This patch add a helper func to get a registered crtc from its index.
> > In some case, where we know the crtc's index and we want to know the
> > crtc too.
> >=20
> > For example, the enable_vblank func of struct drm_driver:
> > In the implementation of this func, we know the index of the crtc but
> > we want to know the crtc. This helper func can get the crtc easily.
> > A sample impelmentation of enable_vblank is as shown as bellow:
> >=20
> > int hisi_drm_crtc_enable_vblank(struct drm_device *dev, int c)
> > {
> > 	struct drm_crtc *crtc =3D drm_get_crtc_from_index(dev, c);
> > 	struct hisi_crtc *hcrtc =3D to_hisi_crtc(crtc);
> > 	struct hisi_crtc_ops *ops =3D hcrtc->ops;
> > 	int ret =3D 0;
> >=20
> > 	if (ops->enable_vblank)
> > 		ret =3D ops->enable_vblank(hcrtc);
> >=20
> > 	return ret;
> > }
> >=20
> > Signed-off-by: Xinliang Liu <xinliang.liu@linaro.org>
>=20
> Yeah unfortunately drm_irq.c is still stick in the old pre-KMS days. I
> think we should go a bit further here though to allow new drivers to be
> completely free of int pipe:

Of course you meant to say /unsigned/ int pipe =3D)

> - add a new array pointer dev->mode_conifg.crtc_arr, which is
>   (re-)allocated in drm_crtc_init_with_planes. Then a pipe->crtc lookup
>   will be just
>=20
> 	crtc =3D dev->mode_config.crtc_arr[pipe];
>=20
> - add new hooks for vblank handling int drm_crtc_helper_funcs for
>   enable_vblanke, disable_vblank, get_vblank_timestamp and get_scanoutpos.
>   Ofc also anotate the docs for the existing hooks and make it clear new
>   drivers should use the new ones. Ofc these new hooks should directly
>   take a struct drm_crtc * instead of inte pipe.

I have a couple patches to address this partially, which came about as a
result of the int crtc/crtc_id/pipe/whatever -> unsigned int pipe
conversion work that I've been doing.

> - change the code in drm_irq.c to wrap all callbacks and first check
>   whether the new ones are there and only if that's not the case call the
>   old ones.
>=20
> With these changes drivers can be completely free of int pipe and use
> struct drm_crtc exclusivly I think, and the mess would be fully restricted
> to drm_irq.c.

I like the idea of moving the callbacks to drm_crtc_helper_funcs. That
allows us to introduce this step by step, without a flag date when every
driver needs to switch the drm_driver functions over.

Thierry

--wRRV7LY7NUeQGEoC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAABCAAGBQJV3Z1GAAoJEN0jrNd/PrOhgC4P/16BRyvS9Ao5KjHy/CqC3A4i
mYmuCmuHLudOsBNiuWqnAtZqzzXpTqCoZ6OwHBj4ARMKb0gklsRelCuNAE0U+pbH
Z2f6OmG6frLZf/NZT4uxKZepJlMK7a8764lFL+b1hcacNMcDwlpoC3UNeO5zmMeV
2Hh4pg6nh/zQ5zvdbuGlkKXtbp3jNSjj3gbtPRGcn6aqXFb7Fn5uwYlEXKMaxRef
6ilDPzpsUGhDL/mDzOhQCMATp1SEjXQKcTYhnKAPMtkGL2NX2otHedXxmjbDsyuS
JOZGx/5n3awekHGJGSnUvAw8qXDhw1s/6NnMEE52STJYRGQfkPEL1J8uONkB9+9d
3MDP9hxe9uxfGeVssxlSzwSq/BsBTzxJqvQl5UZuNdpsufvjgWES/5ZkpLlVb8g3
GNXqctmI3s5KT7guadk2PLAYi8+lNaOn1wjw2lSqt0YQjzUM05Nr3k12FVLlOIvf
CMnsHsDyEQyeLyOy91GwaVR+hStvkLgxdImgr9LEe/YKF33BLNpDN2xvU/2QyY+S
toULsHUVTU02hFSvRKooYfeX3vHIkTzL/bgxCu3cwSzN1ux+BcV3NVdvtLkMSyKa
awJAtATC9IdyZe93daFIfZ9TIm2swBdTCT3ihjX2pEM0fgh9F7YzH5NcZYzLv+iq
jlnUOAczuBHpB+Qjmjst
=wG/z
-----END PGP SIGNATURE-----

--wRRV7LY7NUeQGEoC--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/