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

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

[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
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dsi: Silence atomic update failure with DSI panel

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Silence atomic update failure with DSI panel
URL   : https://patchwork.freedesktop.org/series/29828/
State : success

== Summary ==

Series 29828v1 drm/i915/dsi: Silence atomic update failure with DSI panel
https://patchwork.freedesktop.org/api/1.0/series/29828/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:460s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:450s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:364s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:552s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:257s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:522s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:525s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:513s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:439s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:615s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:443s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:424s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:475s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:513s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:601s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:602s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:527s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:532s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:515s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:437s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:483s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:553s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:408s

229aae71d40f142bc0fbc011d6610ac4ddbd7cd6 drm-tip: 2017y-09m-05d-11h-05m-56s UTC integration manifest
f95f83c31d9a drm/i915/dsi: Silence atomic update failure with DSI panel

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5582/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915/dsi: Silence atomic update failure with DSI panel

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Silence atomic update failure with DSI panel
URL   : https://patchwork.freedesktop.org/series/29828/
State : warning

== Summary ==

Test kms_flip:
        Subgroup wf_vblank-vs-modeset:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup wf_vblank-vs-dpms-interruptible:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2255 pass:1223 dwarn:2   dfail:0   fail:15  skip:1014 time:9456s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5582/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Tue, Sep 05, 2017 at 04:35:04PM +0300, Mika Kahola wrote:
> 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>

This just changes a loud atomic failure to a silent atomic failure. What
we instead need to do is actually fix the bug, not hide it.

This means DSI is atm blacklisted almost entirely, but well it's broken,
so no harm in that.

Please no polishing of just results in CI, it needs to give us honest
results.
-Daniel
> ---
>  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
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Mika Kahola]

On Tue, 2017-09-05 at 18:11 +0200, Daniel Vetter wrote:
> On Tue, Sep 05, 2017 at 04:35:04PM +0300, Mika Kahola wrote:
> > 
> > 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>
> This just changes a loud atomic failure to a silent atomic failure.
> What
> we instead need to do is actually fix the bug, not hide it.
> 
BSpec has a notion that (PIPE_SCANLINE) that is "Not supported with
MIPI DSI."

That's why I thought it might be ok to silence the error as the
computation that we try to accomplish wouldn't work anyway. Maybe this
way we could remove DSI from being blackllisted.

> This means DSI is atm blacklisted almost entirely, but well it's
> broken,
> so no harm in that.
> 
> Please no polishing of just results in CI, it needs to give us honest
> results.
> -Daniel
> > 
> > ---
> >  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
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Martin Peres]

On 06/09/17 13:09, Mika Kahola wrote:
> On Tue, 2017-09-05 at 18:11 +0200, Daniel Vetter wrote:
>> On Tue, Sep 05, 2017 at 04:35:04PM +0300, Mika Kahola wrote:
>>>
>>> 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>
>> This just changes a loud atomic failure to a silent atomic failure.
>> What
>> we instead need to do is actually fix the bug, not hide it.
>>
> BSpec has a notion that (PIPE_SCANLINE) that is "Not supported with
> MIPI DSI."
> 
> That's why I thought it might be ok to silence the error as the
> computation that we try to accomplish wouldn't work anyway. Maybe this
> way we could remove DSI from being blackllisted.

I agree. If the HW can't do it, so what can we do here?

As long as this is well documented and the userspace knows about this 
issue (if anything relies on this feature), then what else can we do?

With the relevant BSpec quotes added above the changes, I can give my:
Acked-by: Martin Peres <martin.peres@linux.intel.com>

I would however like to know if this breaks any feature the userspace 
relies on.

Martin
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Maarten Lankhorst]

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.

Does this mean need_vlv_dsi_wa in intel_pipe_update_start is now a noop? Should we perhaps only apply this
for gen9+ MIPI/DSI?

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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.
> 
> Does this mean need_vlv_dsi_wa in intel_pipe_update_start is now a noop? Should we perhaps only apply this
> for gen9+ MIPI/DSI?

VLV/CHV scanline counter (more or less) works with DSI. It's only BXT+
that's totally broken.

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

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

[Mika Kahola]

On Thu, 2017-09-07 at 13:29 +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.
> 
Definitely there's room for optimization here. We could get rid of
those delays.

> i915_get_crtc_scanoutpos should return false as well.
> 
> Does this mean need_vlv_dsi_wa in intel_pipe_update_start is now a
> noop? Should we perhaps only apply this
> for gen9+ MIPI/DSI?
We should limit this to gen9+ MIPI/DSI. The BSpec says that MIPI DSI is
not supported from Broxton onwards.
> 
> ~Maarten
> 
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

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

[Daniel Vetter]

On Wed, Sep 06, 2017 at 07:48:33PM +0300, Martin Peres wrote:
> On 06/09/17 13:09, Mika Kahola wrote:
> > On Tue, 2017-09-05 at 18:11 +0200, Daniel Vetter wrote:
> > > On Tue, Sep 05, 2017 at 04:35:04PM +0300, Mika Kahola wrote:
> > > > 
> > > > 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>
> > > This just changes a loud atomic failure to a silent atomic failure.
> > > What
> > > we instead need to do is actually fix the bug, not hide it.
> > > 
> > BSpec has a notion that (PIPE_SCANLINE) that is "Not supported with
> > MIPI DSI."
> > 
> > That's why I thought it might be ok to silence the error as the
> > computation that we try to accomplish wouldn't work anyway. Maybe this
> > way we could remove DSI from being blackllisted.
> 
> I agree. If the HW can't do it, so what can we do here?
> 
> As long as this is well documented and the userspace knows about this issue
> (if anything relies on this feature), then what else can we do?
> 
> With the relevant BSpec quotes added above the changes, I can give my:
> Acked-by: Martin Peres <martin.peres@linux.intel.com>
> 
> I would however like to know if this breaks any feature the userspace relies
> on.

Catching up on this, for the record:

Lack of this breaks atomic. So yeah, not really an optional feature, and
definitely not a failure mode we should flush out to hide it.

And if the scanline trick doesn't work, then we need a different way to
achieve the same. Just because the one way we use everywhere else doesn't
work, doesn't mean atomic on dsi is going to be impossible. It would be
real bad if that's the case.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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 <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.

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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]

Hi Mika,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.13 next-20170908]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mika-Kahola/drm-i915-dsi-Silence-atomic-update-failure-with-DSI-panel/20170908-160933
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x001-201736 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_pipe_update_end':
>> drivers/gpu/drm/i915/intel_sprite.c:209:27: error: 'new_crtc_state' undeclared (first use in this function)
     if (!intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI)) {
                              ^~~~~~~~~~~~~~
   drivers/gpu/drm/i915/intel_sprite.c:209:27: note: each undeclared identifier is reported only once for each function it appears in

vim +/new_crtc_state +209 drivers/gpu/drm/i915/intel_sprite.c

   170	
   171	/**
   172	 * intel_pipe_update_end() - end update of a set of display registers
   173	 * @crtc: the crtc of which the registers were updated
   174	 * @start_vbl_count: start vblank counter (used for error checking)
   175	 *
   176	 * Mark the end of an update started with intel_pipe_update_start(). This
   177	 * re-enables interrupts and verifies the update was actually completed
   178	 * before a vblank using the value of @start_vbl_count.
   179	 */
   180	void intel_pipe_update_end(struct intel_crtc *crtc)
   181	{
   182		enum pipe pipe = crtc->pipe;
   183		int scanline_end = intel_get_crtc_scanline(crtc);
   184		u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
   185		ktime_t end_vbl_time = ktime_get();
   186		struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
   187	
   188		trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
   189	
   190		/* We're still in the vblank-evade critical section, this can't race.
   191		 * Would be slightly nice to just grab the vblank count and arm the
   192		 * event outside of the critical section - the spinlock might spin for a
   193		 * while ... */
   194		if (crtc->base.state->event) {
   195			WARN_ON(drm_crtc_vblank_get(&crtc->base) != 0);
   196	
   197			spin_lock(&crtc->base.dev->event_lock);
   198			drm_crtc_arm_vblank_event(&crtc->base, crtc->base.state->event);
   199			spin_unlock(&crtc->base.dev->event_lock);
   200	
   201			crtc->base.state->event = NULL;
   202		}
   203	
   204		local_irq_enable();
   205	
   206		if (intel_vgpu_active(dev_priv))
   207			return;
   208	
 > 209		if (!intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI)) {
   210			if (crtc->debug.start_vbl_count &&
   211			    crtc->debug.start_vbl_count != end_vbl_count) {
   212				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",
   213					  pipe_name(pipe), crtc->debug.start_vbl_count,
   214					  end_vbl_count,
   215					  ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
   216					  crtc->debug.min_vbl, crtc->debug.max_vbl,
   217					  crtc->debug.scanline_start, scanline_end);
   218			}
   219	#ifdef CONFIG_DRM_I915_DEBUG_VBLANK_EVADE
   220			else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) >
   221				 VBLANK_EVASION_TIME_US)
   222				DRM_WARN("Atomic update on pipe (%c) took %lld us, max time under evasion is %u us\n",
   223					 pipe_name(pipe),
   224					 ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
   225					 VBLANK_EVASION_TIME_US);
   226	#endif
   227		}
   228	}
   229	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23087 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Shankar, Uma]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Daniel Vetter
>Sent: Friday, September 8, 2017 12:30 PM
>To: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsi: Silence atomic update failure with
>DSI panel
>
>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 <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.
>

Yes, we definitely need the evasion even for DSI. Pipe Scanline register will
not work for Gen9 DSI (it works on BYT/CHT though). The timings for DSI are
driven from port unlike DDI. We can however use Pipe frame timestamp and
current timestamp registers to logically calculate that. We have submitted a
patch for the same. Please have a look https://patchwork.kernel.org/patch/9944249/.

Regards,
Uma Shankar

>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
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx