intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
@ 2017-09-28  0:20 Rodrigo Vivi
  2017-09-28  0:42 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2017-09-28  0:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

One of the differences I spotted for GEN8+ platforms when
compared to older platforms is that spec for BDW+ includes
this sentence:

"The first CRC done indication after CRC is first enabled is
from only a partial frame, so it will not have the expected
CRC result."

This is an indication that on BDW+ platforms, by the time
we receive the interrupt the CRC is not accurate yet for
the full frame. That would be ok, because we are already
skipping the first CRC for all platforms. However the comment
on the code state that it is for some unknown reason. Also,
on CHV (gen8 lp) we were already discarding the second CRC
as well to make sure we have a reliable CRC on hand.

So based on all ou tests and bugs it seems that it is not
on CHV that needs to discard 2 first CRCs, but all BDW+
platforms.

Starting on SKL we have this CRC done bit (24), but the
experiments around the use of this bit wasn't that stable
as just discarding the second CRC. So, let's for now
just move with CHV solution for all gen8+ platforms and
make our CI a bit more stable.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0b7562135d1c..efd7827ff181 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1647,11 +1647,11 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 		 * bonkers. So let's just wait for the next vblank and read
 		 * out the buggy result.
 		 *
-		 * On CHV sometimes the second CRC is bonkers as well, so
+		 * On GEN8+ sometimes the second CRC is bonkers as well, so
 		 * don't trust that one either.
 		 */
 		if (pipe_crc->skipped == 0 ||
-		    (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped == 1)) {
+		    (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
 			pipe_crc->skipped++;
 			spin_unlock(&pipe_crc->lock);
 			return;
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Also discard second CRC on gen8+ platforms.
  2017-09-28  0:20 [PATCH] drm/i915: Also discard second CRC on gen8+ platforms Rodrigo Vivi
@ 2017-09-28  0:42 ` Patchwork
  2017-09-28  4:23 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-09-28  0:42 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Also discard second CRC on gen8+ platforms.
URL   : https://patchwork.freedesktop.org/series/31023/
State : success

== Summary ==

Series 31023v1 drm/i915: Also discard second CRC on gen8+ platforms.
https://patchwork.freedesktop.org/api/1.0/series/31023/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test drv_module_reload:
        Subgroup basic-reload:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:437s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:475s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:419s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:517s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:494s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:541s
fi-cnl-y         total:289  pass:259  dwarn:0   dfail:0   fail:3   skip:27  time:638s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:419s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:565s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:419s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:403s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:498s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:464s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:472s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:572s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:587s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:758s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:476s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:568s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:413s

7f93222785e4887c083c85b76fcbb391bb1991d9 drm-tip: 2017y-09m-27d-20h-05m-25s UTC integration manifest
75da1d53cf05 drm/i915: Also discard second CRC on gen8+ platforms.

== Logs ==

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Also discard second CRC on gen8+ platforms.
  2017-09-28  0:20 [PATCH] drm/i915: Also discard second CRC on gen8+ platforms Rodrigo Vivi
  2017-09-28  0:42 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-09-28  4:23 ` Patchwork
  2017-09-28  8:09 ` [PATCH] " Mika Kahola
  2017-09-28 11:28 ` Mika Kahola
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-09-28  4:23 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Also discard second CRC on gen8+ platforms.
URL   : https://patchwork.freedesktop.org/series/31023/
State : success

== Summary ==

Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252 +1
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

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

shard-hsw        total:2429 pass:1333 dwarn:4   dfail:0   fail:9   skip:1083 time:9977s

== Logs ==

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
  2017-09-28  0:20 [PATCH] drm/i915: Also discard second CRC on gen8+ platforms Rodrigo Vivi
  2017-09-28  0:42 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-09-28  4:23 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-09-28  8:09 ` Mika Kahola
  2017-09-28 16:55   ` Rodrigo Vivi
  2017-09-28 11:28 ` Mika Kahola
  3 siblings, 1 reply; 6+ messages in thread
From: Mika Kahola @ 2017-09-28  8:09 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx

This fixes my issue with GLK+MIPI/DSI when running IGT test

kms_frontbuffer_tracking --r basic

Tested-by: Mika Kahola <mika.kahola@intel.com>

On Wed, 2017-09-27 at 17:20 -0700, Rodrigo Vivi wrote:
> One of the differences I spotted for GEN8+ platforms when
> compared to older platforms is that spec for BDW+ includes
> this sentence:
> 
> "The first CRC done indication after CRC is first enabled is
> from only a partial frame, so it will not have the expected
> CRC result."
> 
> This is an indication that on BDW+ platforms, by the time
> we receive the interrupt the CRC is not accurate yet for
> the full frame. That would be ok, because we are already
> skipping the first CRC for all platforms. However the comment
> on the code state that it is for some unknown reason. Also,
> on CHV (gen8 lp) we were already discarding the second CRC
> as well to make sure we have a reliable CRC on hand.
> 
> So based on all ou tests and bugs it seems that it is not
> on CHV that needs to discard 2 first CRCs, but all BDW+
> platforms.
> 
> Starting on SKL we have this CRC done bit (24), but the
> experiments around the use of this bit wasn't that stable
> as just discarding the second CRC. So, let's for now
> just move with CHV solution for all gen8+ platforms and
> make our CI a bit more stable.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 0b7562135d1c..efd7827ff181 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,11 +1647,11 @@ static void
> display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>  		 * bonkers. So let's just wait for the next vblank
> and read
>  		 * out the buggy result.
>  		 *
> -		 * On CHV sometimes the second CRC is bonkers as
> well, so
> +		 * On GEN8+ sometimes the second CRC is bonkers as
> well, so
>  		 * don't trust that one either.
>  		 */
>  		if (pipe_crc->skipped == 0 ||
> -		    (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped ==
> 1)) {
> +		    (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped
> == 1)) {
>  			pipe_crc->skipped++;
>  			spin_unlock(&pipe_crc->lock);
>  			return;
-- 
Mika Kahola - Intel OTC

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
  2017-09-28  0:20 [PATCH] drm/i915: Also discard second CRC on gen8+ platforms Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2017-09-28  8:09 ` [PATCH] " Mika Kahola
@ 2017-09-28 11:28 ` Mika Kahola
  3 siblings, 0 replies; 6+ messages in thread
From: Mika Kahola @ 2017-09-28 11:28 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx

On Wed, 2017-09-27 at 17:20 -0700, Rodrigo Vivi wrote:
> One of the differences I spotted for GEN8+ platforms when
> compared to older platforms is that spec for BDW+ includes
> this sentence:
> 
> "The first CRC done indication after CRC is first enabled is
> from only a partial frame, so it will not have the expected
> CRC result."
> 
> This is an indication that on BDW+ platforms, by the time
> we receive the interrupt the CRC is not accurate yet for
> the full frame. That would be ok, because we are already
> skipping the first CRC for all platforms. However the comment
> on the code state that it is for some unknown reason. Also,
> on CHV (gen8 lp) we were already discarding the second CRC
> as well to make sure we have a reliable CRC on hand.
> 
> So based on all ou tests and bugs it seems that it is not
> on CHV that needs to discard 2 first CRCs, but all BDW+
> platforms.
> 
> Starting on SKL we have this CRC done bit (24), but the
> experiments around the use of this bit wasn't that stable
> as just discarding the second CRC. So, let's for now
> just move with CHV solution for all gen8+ platforms and
> make our CI a bit more stable.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 0b7562135d1c..efd7827ff181 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,11 +1647,11 @@ static void
> display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>  		 * bonkers. So let's just wait for the next vblank
> and read
>  		 * out the buggy result.
>  		 *
> -		 * On CHV sometimes the second CRC is bonkers as
> well, so
> +		 * On GEN8+ sometimes the second CRC is bonkers as
> well, so
>  		 * don't trust that one either.
>  		 */
>  		if (pipe_crc->skipped == 0 ||
> -		    (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped ==
> 1)) {
> +		    (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped
> == 1)) {
>  			pipe_crc->skipped++;
>  			spin_unlock(&pipe_crc->lock);
>  			return;
-- 
Mika Kahola - Intel OTC

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
  2017-09-28  8:09 ` [PATCH] " Mika Kahola
@ 2017-09-28 16:55   ` Rodrigo Vivi
  0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2017-09-28 16:55 UTC (permalink / raw)
  To: Mika Kahola; +Cc: intel-gfx

On Thu, Sep 28, 2017 at 08:09:43AM +0000, Mika Kahola wrote:
> This fixes my issue with GLK+MIPI/DSI when running IGT test
> 
> kms_frontbuffer_tracking --r basic
> 
> Tested-by: Mika Kahola <mika.kahola@intel.com>

Thanks for spotting the problem, for reviews and testings.
Patch merged to dinq.

> 
> On Wed, 2017-09-27 at 17:20 -0700, Rodrigo Vivi wrote:
> > One of the differences I spotted for GEN8+ platforms when
> > compared to older platforms is that spec for BDW+ includes
> > this sentence:
> > 
> > "The first CRC done indication after CRC is first enabled is
> > from only a partial frame, so it will not have the expected
> > CRC result."
> > 
> > This is an indication that on BDW+ platforms, by the time
> > we receive the interrupt the CRC is not accurate yet for
> > the full frame. That would be ok, because we are already
> > skipping the first CRC for all platforms. However the comment
> > on the code state that it is for some unknown reason. Also,
> > on CHV (gen8 lp) we were already discarding the second CRC
> > as well to make sure we have a reliable CRC on hand.
> > 
> > So based on all ou tests and bugs it seems that it is not
> > on CHV that needs to discard 2 first CRCs, but all BDW+
> > platforms.
> > 
> > Starting on SKL we have this CRC done bit (24), but the
> > experiments around the use of this bit wasn't that stable
> > as just discarding the second CRC. So, let's for now
> > just move with CHV solution for all gen8+ platforms and
> > make our CI a bit more stable.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 0b7562135d1c..efd7827ff181 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1647,11 +1647,11 @@ static void
> > display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
> >  		 * bonkers. So let's just wait for the next vblank
> > and read
> >  		 * out the buggy result.
> >  		 *
> > -		 * On CHV sometimes the second CRC is bonkers as
> > well, so
> > +		 * On GEN8+ sometimes the second CRC is bonkers as
> > well, so
> >  		 * don't trust that one either.
> >  		 */
> >  		if (pipe_crc->skipped == 0 ||
> > -		    (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped ==
> > 1)) {
> > +		    (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped
> > == 1)) {
> >  			pipe_crc->skipped++;
> >  			spin_unlock(&pipe_crc->lock);
> >  			return;
> -- 
> Mika Kahola - Intel OTC
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-28 16:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-28  0:20 [PATCH] drm/i915: Also discard second CRC on gen8+ platforms Rodrigo Vivi
2017-09-28  0:42 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-09-28  4:23 ` ✓ Fi.CI.IGT: " Patchwork
2017-09-28  8:09 ` [PATCH] " Mika Kahola
2017-09-28 16:55   ` Rodrigo Vivi
2017-09-28 11:28 ` Mika Kahola

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).