Re: [PATCH] drm/i915/dsi: Silence atomic update failure with DSI panel

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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 <mika.kahola@intel.com>
> > ---
> >  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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx