Re: [PATCH 2/3] drm/vkms: Use wait_for_flip_done

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 03, 2019 at 08:49:06AM -0400, Rodrigo Siqueira wrote:
> On 07/19, Daniel Vetter wrote:
> > It's the recommended version, wait_for_vblanks is a bit a hacky
> > interim thing that predates all the flip_done tracking. It's
> > unfortunately still the default ...
> 
> Just one question, is it safe to replace drm_atomic_helper_wait_for_vblanks by
> drm_atomic_helper_wait_for_flip_done? I noticed that only six drivers use these
> functions; they are:
> 
> * atmel-hlcdc
> * mediatek
> * msm
> * tegra
> * tilcdc
> * virtio
> 
> If we change these drivers, can we drop the helper
> drm_atomic_helper_wait_for_vblanks?

Yes, but there might be a tiny behaviour change, that's why I haven't just
made it the default.

Also note that wait_for_vblanks is still the default in the
atomic_commit_tail (seee drm_atomic_helper_commit_tail), so there's a pile
more drivers using this implicitly.

But yeah would be really great to fix that all up, since I think
wait_for_flip_done is the better function. Maybe a todo.rst? Or perhaps we
should at least do it for atomic helpers, and just see what breaks? As a
start for this conversion.

> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>

Thanks, Daniel

> 
> Thanks
> 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > ---
> >  drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 44ab9f8ef8be..80524a22412a 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -83,7 +83,7 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
> >  
> >  	drm_atomic_helper_commit_hw_done(old_state);
> >  
> > -	drm_atomic_helper_wait_for_vblanks(dev, old_state);
> > +	drm_atomic_helper_wait_for_flip_done(dev, old_state);
> >  
> >  	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> >  		struct vkms_crtc_state *vkms_state =
> > -- 
> > 2.22.0
> > 
> 
> -- 
> Rodrigo Siqueira
> Software Engineer, Advanced Micro Devices (AMD)
> https://siqueira.tech



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx