<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On 26 August 2015 at 19:04, Thierry Reding <<a href="mailto:thierry.reding@gmail.com" target="_blank">thierry.reding@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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. > > > > 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 <<a href="mailto:xinliang.liu@linaro.org">xinliang.liu@linaro.org</a>> > > 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 =) > - 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 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. </blockquote><div>Hi Thierry, I wonder what your patches look like. Could you share them to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > - 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. 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 </blockquote></div> </div></div>