| Message ID | 20190719152314.7706-2-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/vblank: Document and fix vblank count barrier semantics | expand |
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? Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> 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 >
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
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 =
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 ... 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(-)