Re: [Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
On Thu, Sep 28, 2017 at 08:09:43AM +, Mika Kahola wrote:
> This fixes my issue with GLK+MIPI/DSI when running IGT test
>
> kms_frontbuffer_tracking --r basic
>
> Tested-by: Mika Kahola
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
> > Signed-off-by: Rodrigo Vivi
> > ---
> > 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
Re: [Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
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
> Signed-off-by: Rodrigo Vivi
Reviewed-by: Mika Kahola
> ---
> 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
Re: [Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
This fixes my issue with GLK+MIPI/DSI when running IGT test
kms_frontbuffer_tracking --r basic
Tested-by: Mika Kahola
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
> Signed-off-by: Rodrigo Vivi
> ---
> 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
[Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
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
Signed-off-by: Rodrigo Vivi
---
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
