| Message ID | 20190430093746.26485-1-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | b3198c38f02d54a5e964258a2180d502abe6eaf0 |
| Headers | show |
| Series | drm/atomic-helper: Bump vblank timeout to 100 ms | expand |
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
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
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);
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(-)