<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 26 August 2015 at 19:04, Thierry Reding <span dir="ltr"><<a href="mailto:thierry.reding@gmail.com" target="_blank">thierry.reding@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, Aug 25, 2015 at 11:36:18AM +0200, Daniel Vetter wrote:<br>
> On Tue, Aug 25, 2015 at 11:13:51AM +0800, Xinliang Liu wrote:<br>
> > This patch add a helper func to get a registered crtc from its index.<br>
> > In some case, where we know the crtc's index and we want to know the<br>
> > crtc too.<br>
> ><br>
> > For example, the enable_vblank func of struct drm_driver:<br>
> > In the implementation of this func, we know the index of the crtc but<br>
> > we want to know the crtc. This helper func can get the crtc easily.<br>
> > A sample impelmentation of enable_vblank is as shown as bellow:<br>
> ><br>
> > int hisi_drm_crtc_enable_vblank(struct drm_device *dev, int c)<br>
> > {<br>
> > struct drm_crtc *crtc = drm_get_crtc_from_index(dev, c);<br>
> > struct hisi_crtc *hcrtc = to_hisi_crtc(crtc);<br>
> > struct hisi_crtc_ops *ops = hcrtc->ops;<br>
> > int ret = 0;<br>
> ><br>
> > if (ops->enable_vblank)<br>
> > ret = ops->enable_vblank(hcrtc);<br>
> ><br>
> > return ret;<br>
> > }<br>
> ><br>
> > Signed-off-by: Xinliang Liu <<a href="mailto:xinliang.liu@linaro.org">xinliang.liu@linaro.org</a>><br>
><br>
> Yeah unfortunately drm_irq.c is still stick in the old pre-KMS days. I<br>
> think we should go a bit further here though to allow new drivers to be<br>
> completely free of int pipe:<br>
<br>
</span>Of course you meant to say /unsigned/ int pipe =)<br>
<span class=""><br>
> - add a new array pointer dev->mode_conifg.crtc_arr, which is<br>
> (re-)allocated in drm_crtc_init_with_planes. Then a pipe->crtc lookup<br>
> will be just<br>
><br>
> crtc = dev->mode_config.crtc_arr[pipe];<br>
><br>
> - add new hooks for vblank handling int drm_crtc_helper_funcs for<br>
> enable_vblanke, disable_vblank, get_vblank_timestamp and get_scanoutpos.<br>
> Ofc also anotate the docs for the existing hooks and make it clear new<br>
> drivers should use the new ones. Ofc these new hooks should directly<br>
> take a struct drm_crtc * instead of inte pipe.<br>
<br>
</span>I have a couple patches to address this partially, which came about as a<br>
result of the int crtc/crtc_id/pipe/whatever -> unsigned int pipe<br>
conversion work that I've been doing.<br></blockquote><div>Hi Thierry, I wonder what your patches look like. Could you share them to me.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> - change the code in drm_irq.c to wrap all callbacks and first check<br>
> whether the new ones are there and only if that's not the case call the<br>
> old ones.<br>
><br>
> With these changes drivers can be completely free of int pipe and use<br>
> struct drm_crtc exclusivly I think, and the mess would be fully restricted<br>
> to drm_irq.c.<br>
<br>
</span>I like the idea of moving the callbacks to drm_crtc_helper_funcs. That<br>
allows us to introduce this step by step, without a flag date when every<br>
driver needs to switch the drm_driver functions over.<br>
<span class="HOEnZb"><font color="#888888"><br>
Thierry<br>
</font></span></blockquote></div><br></div></div>