| Message ID | 20180629111722.20299-6-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 29, 2018 at 01:17:18PM +0200, Boris Brezillon wrote: > Now that we have a way to fake VBLANK events when requested by the CRTC > hook it up to the generic commit_tail() helpers. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index ca586993c2a2..1a088462bc42 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > + drm_atomic_helper_fake_vblank(old_state); > + > drm_atomic_helper_commit_hw_done(old_state); > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) > drm_atomic_helper_commit_planes(dev, old_state, > DRM_PLANE_COMMIT_ACTIVE_ONLY); > > + drm_atomic_helper_fake_vblank(old_state); > + > drm_atomic_helper_commit_hw_done(old_state); > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > -- > 2.14.1 >
On Fri, Jun 29, 2018 at 01:17:18PM +0200, Boris Brezillon wrote: > Now that we have a way to fake VBLANK events when requested by the CRTC > hook it up to the generic commit_tail() helpers. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> I really don't like this. We've had epic amounts of bugs with atomic drivers failing to send out vblank events when they should, and I added a bunch of debug checks to make sure that doesn't happen anymore. Now there's no way anymore for drivers to spot this until they have misrenderings on wayland and no idea why. Imo this should only be used by specific drivers, with a comment why exactly they need it. -Daniel > --- > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index ca586993c2a2..1a088462bc42 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > + drm_atomic_helper_fake_vblank(old_state); > + > drm_atomic_helper_commit_hw_done(old_state); > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) > drm_atomic_helper_commit_planes(dev, old_state, > DRM_PLANE_COMMIT_ACTIVE_ONLY); > > + drm_atomic_helper_fake_vblank(old_state); > + > drm_atomic_helper_commit_hw_done(old_state); > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > -- > 2.14.1 >
On Mon, Jul 02, 2018 at 09:54:32AM +0200, Daniel Vetter wrote: > On Fri, Jun 29, 2018 at 01:17:18PM +0200, Boris Brezillon wrote: > > Now that we have a way to fake VBLANK events when requested by the CRTC > > hook it up to the generic commit_tail() helpers. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > I really don't like this. We've had epic amounts of bugs with atomic > drivers failing to send out vblank events when they should, and I added a > bunch of debug checks to make sure that doesn't happen anymore. > > Now there's no way anymore for drivers to spot this until they have > misrenderings on wayland and no idea why. Imo this should only be used by > specific drivers, with a comment why exactly they need it. Meh I retract, you have the special no_vblank state flag to control this. Looks all good to me. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > -Daniel > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index ca586993c2a2..1a088462bc42 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) > > > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > > > + drm_atomic_helper_fake_vblank(old_state); > > + > > drm_atomic_helper_commit_hw_done(old_state); > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) > > drm_atomic_helper_commit_planes(dev, old_state, > > DRM_PLANE_COMMIT_ACTIVE_ONLY); > > > > + drm_atomic_helper_fake_vblank(old_state); > > + > > drm_atomic_helper_commit_hw_done(old_state); > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > -- > > 2.14.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Mon, 2 Jul 2018 09:54:32 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Jun 29, 2018 at 01:17:18PM +0200, Boris Brezillon wrote: > > Now that we have a way to fake VBLANK events when requested by the CRTC > > hook it up to the generic commit_tail() helpers. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > I really don't like this. We've had epic amounts of bugs with atomic > drivers failing to send out vblank events when they should, and I added a > bunch of debug checks to make sure that doesn't happen anymore. > > Now there's no way anymore for drivers to spot this until they have > misrenderings on wayland and no idea why. Imo this should only be used by > specific drivers, with a comment why exactly they need it. Hm, that's not entirely true. Drivers have to explicitly set ->no_vblank to true for drm_atomic_helper_fake_vblank() to actually do something, and I seriously hope drivers won't set that field randomly. So, on all existing drivers but VC4 after the TXP patch, the behavior will be unchanged, and the core will spot misbehaving drivers just like before. > -Daniel > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index ca586993c2a2..1a088462bc42 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) > > > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > > > + drm_atomic_helper_fake_vblank(old_state); > > + > > drm_atomic_helper_commit_hw_done(old_state); > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) > > drm_atomic_helper_commit_planes(dev, old_state, > > DRM_PLANE_COMMIT_ACTIVE_ONLY); > > > > + drm_atomic_helper_fake_vblank(old_state); > > + > > drm_atomic_helper_commit_hw_done(old_state); > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > -- > > 2.14.1 > > >
On Mon, 2 Jul 2018 09:57:30 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Jul 02, 2018 at 09:54:32AM +0200, Daniel Vetter wrote: > > On Fri, Jun 29, 2018 at 01:17:18PM +0200, Boris Brezillon wrote: > > > Now that we have a way to fake VBLANK events when requested by the CRTC > > > hook it up to the generic commit_tail() helpers. > > > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > > > I really don't like this. We've had epic amounts of bugs with atomic > > drivers failing to send out vblank events when they should, and I added a > > bunch of debug checks to make sure that doesn't happen anymore. > > > > Now there's no way anymore for drivers to spot this until they have > > misrenderings on wayland and no idea why. Imo this should only be used by > > specific drivers, with a comment why exactly they need it. > > Meh I retract, you have the special no_vblank state flag to control this. > Looks all good to me. Hehe, just replied to your other email :-). > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > -Daniel > > > > > --- > > > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > > index ca586993c2a2..1a088462bc42 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) > > > > > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > > > > > + drm_atomic_helper_fake_vblank(old_state); > > > + > > > drm_atomic_helper_commit_hw_done(old_state); > > > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > > @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) > > > drm_atomic_helper_commit_planes(dev, old_state, > > > DRM_PLANE_COMMIT_ACTIVE_ONLY); > > > > > > + drm_atomic_helper_fake_vblank(old_state); > > > + > > > drm_atomic_helper_commit_hw_done(old_state); > > > > > > drm_atomic_helper_wait_for_vblanks(dev, old_state); > > > -- > > > 2.14.1 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch >
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ca586993c2a2..1a088462bc42 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1448,6 +1448,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) drm_atomic_helper_commit_modeset_enables(dev, old_state); + drm_atomic_helper_fake_vblank(old_state); + drm_atomic_helper_commit_hw_done(old_state); drm_atomic_helper_wait_for_vblanks(dev, old_state); @@ -1477,6 +1479,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) drm_atomic_helper_commit_planes(dev, old_state, DRM_PLANE_COMMIT_ACTIVE_ONLY); + drm_atomic_helper_fake_vblank(old_state); + drm_atomic_helper_commit_hw_done(old_state); drm_atomic_helper_wait_for_vblanks(dev, old_state);
Now that we have a way to fake VBLANK events when requested by the CRTC hook it up to the generic commit_tail() helpers. Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ 1 file changed, 4 insertions(+)