[PATCH] drm/atomic-helper: Bump vblank timeout to 100 ms

[PATCH] drm/atomic-helper: Bump vblank timeout to 100 ms

[Linus Walleij]

The 50 ms default timeout waiting for vblanks is too small
for the first vblank from the ST-Ericsson MCDE display
controller over DSI. Presumably this is because the DSI
display is command-mode only and the state machine will
take some time setting up its state for the first display
update, and we hit a timeout. 100 ms makes it pass without
problems.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
After a quite prolonged hunting for the cause of missed
vblanks in the MCDE driver I finally realized it timed
out because it was simply taking some time on the first
vblank. 50 ms makes sense on 60Hz monitors for sure,
but an embedded DSI state machine can be slow, as it
turns out.

Tell me if this should be a per-driver variable and I
will think of something.
---
 drivers/gpu/drm/drm_atomic_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 40ac19848034..f0aa7b195d79 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1424,7 +1424,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		ret = wait_event_timeout(dev->vblank[i].queue,
 				old_state->crtcs[i].last_vblank_count !=
 					drm_crtc_vblank_count(crtc),
-				msecs_to_jiffies(50));
+				msecs_to_jiffies(100));
 
 		WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n",
 		     crtc->base.id, crtc->name);
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: Bump vblank timeout to 100 ms

[Ville Syrjälä]

On Tue, Apr 30, 2019 at 11:37:46AM +0200, Linus Walleij wrote:
> The 50 ms default timeout waiting for vblanks is too small
> for the first vblank from the ST-Ericsson MCDE display
> controller over DSI. Presumably this is because the DSI
> display is command-mode only and the state machine will
> take some time setting up its state for the first display
> update, and we hit a timeout. 100 ms makes it pass without
> problems.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> After a quite prolonged hunting for the cause of missed
> vblanks in the MCDE driver I finally realized it timed
> out because it was simply taking some time on the first
> vblank. 50 ms makes sense on 60Hz monitors for sure,
> but an embedded DSI state machine can be slow, as it
> turns out.
> 
> Tell me if this should be a per-driver variable and I
> will think of something.
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 40ac19848034..f0aa7b195d79 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1424,7 +1424,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>  		ret = wait_event_timeout(dev->vblank[i].queue,
>  				old_state->crtcs[i].last_vblank_count !=
>  					drm_crtc_vblank_count(crtc),
> -				msecs_to_jiffies(50));
> +				msecs_to_jiffies(100));

50ms is pretty tight for 24Hz as well. I suppose we could make this
depend on the expected frame/field duration, but it's generally
indicative of a programming error of some sort, so as long as it's
long enough I think we're good.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  		WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n",
>  		     crtc->base.id, crtc->name);
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: Bump vblank timeout to 100 ms

[Daniel Vetter]

On Tue, Apr 30, 2019 at 01:44:19PM +0300, Ville Syrjälä wrote:
> On Tue, Apr 30, 2019 at 11:37:46AM +0200, Linus Walleij wrote:
> > The 50 ms default timeout waiting for vblanks is too small
> > for the first vblank from the ST-Ericsson MCDE display
> > controller over DSI. Presumably this is because the DSI
> > display is command-mode only and the state machine will
> > take some time setting up its state for the first display
> > update, and we hit a timeout. 100 ms makes it pass without
> > problems.
> > 
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > After a quite prolonged hunting for the cause of missed
> > vblanks in the MCDE driver I finally realized it timed
> > out because it was simply taking some time on the first
> > vblank. 50 ms makes sense on 60Hz monitors for sure,
> > but an embedded DSI state machine can be slow, as it
> > turns out.
> > 
> > Tell me if this should be a per-driver variable and I
> > will think of something.
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 40ac19848034..f0aa7b195d79 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1424,7 +1424,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
> >  		ret = wait_event_timeout(dev->vblank[i].queue,
> >  				old_state->crtcs[i].last_vblank_count !=
> >  					drm_crtc_vblank_count(crtc),
> > -				msecs_to_jiffies(50));
> > +				msecs_to_jiffies(100));
> 
> 50ms is pretty tight for 24Hz as well. I suppose we could make this
> depend on the expected frame/field duration, but it's generally
> indicative of a programming error of some sort, so as long as it's
> long enough I think we're good.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yeah makes sense to bump this a bit. An uber-fancy version would look at
the programmed refresh rate and maybe give the driver extra slack for the
first frame after a modeset. But this is good enough.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> >  
> >  		WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n",
> >  		     crtc->base.id, crtc->name);
> > -- 
> > 2.20.1
> 
> -- 
> Ville Syrjälä
> Intel

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