On Thu, Sep 07, 2017 at 02:59:11PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 07, 2017 at 01:29:22PM +0200, Maarten Lankhorst wrote:
> > Op 05-09-17 om 15:35 schreef Mika Kahola:
> > > It appears that we cannot trust scanline counters when MIPI/DSI display is
> > > connected. In CI system this appears as flickering errors that randomly
> > > appear in test cases. To avoid this flickering, let's just silence atomic
> > > update failure in case with DSI panel.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102403
> > > Signed-off-by: Mika Kahola <[email protected]>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sprite.c | 32 +++++++++++++++++---------------
> > >  1 file changed, 17 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > > b/drivers/gpu/drm/i915/intel_sprite.c
> > > index b0d6e3e..8511072 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -205,23 +205,25 @@ void intel_pipe_update_end(struct intel_crtc_state 
> > > *new_crtc_state)
> > >   if (intel_vgpu_active(dev_priv))
> > >           return;
> > >  
> > > - if (crtc->debug.start_vbl_count &&
> > > -     crtc->debug.start_vbl_count != end_vbl_count) {
> > > -         DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) 
> > > time %lld us, min %d, max %d, scanline start %d, end %d\n",
> > > -                   pipe_name(pipe), crtc->debug.start_vbl_count,
> > > -                   end_vbl_count,
> > > -                   ktime_us_delta(end_vbl_time, 
> > > crtc->debug.start_vbl_time),
> > > -                   crtc->debug.min_vbl, crtc->debug.max_vbl,
> > > -                   crtc->debug.scanline_start, scanline_end);
> > > - }
> > > + if (!intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI)) {
> > > +         if (crtc->debug.start_vbl_count &&
> > > +             crtc->debug.start_vbl_count != end_vbl_count) {
> > > +                 DRM_ERROR("Atomic update failure on pipe %c (start=%u 
> > > end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n",
> > > +                           pipe_name(pipe), crtc->debug.start_vbl_count,
> > > +                           end_vbl_count,
> > > +                           ktime_us_delta(end_vbl_time, 
> > > crtc->debug.start_vbl_time),
> > > +                           crtc->debug.min_vbl, crtc->debug.max_vbl,
> > > +                           crtc->debug.scanline_start, scanline_end);
> > > +         }
> > >  #ifdef CONFIG_DRM_I915_DEBUG_VBLANK_EVADE
> > > - else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) >
> > > -          VBLANK_EVASION_TIME_US)
> > > -         DRM_WARN("Atomic update on pipe (%c) took %lld us, max time 
> > > under evasion is %u us\n",
> > > -                  pipe_name(pipe),
> > > -                  ktime_us_delta(end_vbl_time, 
> > > crtc->debug.start_vbl_time),
> > > -                  VBLANK_EVASION_TIME_US);
> > > +         else if (ktime_us_delta(end_vbl_time, 
> > > crtc->debug.start_vbl_time) >
> > > +                  VBLANK_EVASION_TIME_US)
> > > +                 DRM_WARN("Atomic update on pipe (%c) took %lld us, max 
> > > time under evasion is %u us\n",
> > > +                          pipe_name(pipe),
> > > +                          ktime_us_delta(end_vbl_time, 
> > > crtc->debug.start_vbl_time),
> > > +                          VBLANK_EVASION_TIME_US);
> > >  #endif
> > > + }
> > >  }
> > >  
> > >  static void
> > 
> > I don't think this goes far enough. We should stop claiming accurate 
> > vblanks when MIPI/DSI is used.
> > intel_get_crtc_scanline will currently spin for 100 us to see if we can 
> > move from scanline offset = 0,
> > this means that we add an additional 100 us wait for MIPI/DSI always.
> > 
> > i915_get_crtc_scanoutpos should return false as well.
> 
> Oh and the bigger problem is that we can't actually guarantee atomic 
> updates without the vblank evade currently. I can't recall if BXT has
> the lock bit already somewhere. If it does we should probably start
> using it. Oh and we also have to make sure we sample the frame counter
> _after_ the lock has been released to make sure we do the necessary
> vblank waits and whatnot after the flip has been commited to the
> hardware.

I thought that even on gen9 we still need the evasion because there's a
bunch of stuff not under the lock bit? But yeah lock bit should be there,
it's gen9.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to