On 25 August 2015 at 17:36, Daniel Vetter <daniel@xxxxxxxx> wrote:
Hi Daniel,
Thank you very much for reply. Sorry, I just come back from vacation.
Very happy that you have a good idea to solve the mess.
Looking forward to see your patch soon!
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.
>
> 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:
>
> int hisi_drm_crtc_enable_vblank(struct drm_device *dev, int c)
> {
> struct drm_crtc *crtc = drm_get_crtc_from_index(dev, c);
> struct hisi_crtc *hcrtc = to_hisi_crtc(crtc);
> struct hisi_crtc_ops *ops = hcrtc->ops;
> int ret = 0;
>
> if (ops->enable_vblank)
> ret = ops->enable_vblank(hcrtc);
>
> return ret;
> }
>
> Signed-off-by: Xinliang Liu <xinliang.liu@xxxxxxxxxx>
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:
- 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
crtc = dev->mode_config.crtc_arr[pipe];
- 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 agree too. It seems that vblank is a part of crtc.
- 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.
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.
For the drm_irq_install(struct drm_device *dev, int irq) function, I suggest to add one param such as "void *data" to pass crtc
so that in the irq_handler we can find crtc easily.
Cheers, Daniel
> _______________________________________________
> ---
> drivers/gpu/drm/drm_crtc.c | 25 +++++++++++++++++++++++++
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b9ba061..8764765 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -747,6 +747,31 @@ unsigned int drm_crtc_index(struct drm_crtc *crtc)
> }
> EXPORT_SYMBOL(drm_crtc_index);
>
> +/**
> + * drm_get_crtc_from_index - find a registered CRTC from the index
> + * @dev: DRM device
> + * @index: index of a registered CRTC
> + *
> + * Given a index, return the registered CRTC within a DRM
> + * device's list of CRTCs.
> + */
> +struct drm_crtc *drm_get_crtc_from_index(struct drm_device *dev,
> + unsigned int index)
> +{
> + unsigned int index_tmp = 0;
> + struct drm_crtc *crtc;
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + if (index_tmp == index)
> + return crtc;
> +
> + index_tmp++;
> + }
> +
> + BUG();
> +}
> +EXPORT_SYMBOL(drm_get_crtc_from_index);
> +
> /*
> * drm_mode_remove - remove and free a mode
> * @connector: connector list to modify
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 57ca8cc..3a46d39d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1225,6 +1225,8 @@ extern int drm_crtc_init_with_planes(struct drm_device *dev,
> const struct drm_crtc_funcs *funcs);
> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
> extern unsigned int drm_crtc_index(struct drm_crtc *crtc);
> +extern struct drm_crtc *drm_get_crtc_from_index(struct drm_device *dev,
> + unsigned int index);
>
> /**
> * drm_crtc_mask - find the mask of a registered CRTC
> --
> 1.9.1
>
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
