| Message ID | 55619dbe3b3399292d86420725a0138ced488cca.1557985315.git.harish.chegondi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reg: igt@kms_pipe_crc_basic@* CRC mismatch test failures | expand |
On Wed, 15 May 2019, Harish Chegondi <harish.chegondi@intel.com> wrote: > display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the > second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which > is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > Cc: Petri Latvala <petri.latvala@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 s/References:/Bugzilla:/ Acked-by: Jani Nikula <jani.nikula@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 233211fde0ea..3809e9f7fae2 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so > + * On GEN7+ sometimes the second CRC is bonkers as well, so > * don't trust that one either. > */ > if (pipe_crc->skipped <= 0 || > - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { > + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { > pipe_crc->skipped++; > spin_unlock(&pipe_crc->lock); > return;
Op 16-05-2019 om 07:58 schreef Harish Chegondi: > display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the > second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which > is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > Cc: Petri Latvala <petri.latvala@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 > --- > 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 233211fde0ea..3809e9f7fae2 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so > + * On GEN7+ sometimes the second CRC is bonkers as well, so > * don't trust that one either. > */ > if (pipe_crc->skipped <= 0 || > - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { > + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { > pipe_crc->skipped++; > spin_unlock(&pipe_crc->lock); > return; I would be interested in the results, haswell is different from VLV. Has it ever been observed on that platform?
On Thu, 16 May 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > Op 16-05-2019 om 07:58 schreef Harish Chegondi: >> display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the >> second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which >> is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. >> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> >> Cc: Petri Latvala <petri.latvala@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 >> --- >> 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 233211fde0ea..3809e9f7fae2 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so >> + * On GEN7+ sometimes the second CRC is bonkers as well, so >> * don't trust that one either. >> */ >> if (pipe_crc->skipped <= 0 || >> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { >> + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { >> pipe_crc->skipped++; >> spin_unlock(&pipe_crc->lock); >> return; > > I would be interested in the results, haswell is different from > VLV. Has it ever been observed on that platform? Good point. I looked at [1] which I presumed was on VLV, but it says nothing about HSW. BR, Jani. [1] https://bugs.freedesktop.org/show_bug.cgi?id=103191#c34 >
On Thu, May 16, 2019 at 03:55:25PM +0300, Jani Nikula wrote: > On Thu, 16 May 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > > Op 16-05-2019 om 07:58 schreef Harish Chegondi: > >> display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the > >> second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which > >> is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. > >> > >> Cc: Jani Nikula <jani.nikula@intel.com> > >> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > >> Cc: Petri Latvala <petri.latvala@intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> > >> References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 > >> --- > >> 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 233211fde0ea..3809e9f7fae2 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so > >> + * On GEN7+ sometimes the second CRC is bonkers as well, so > >> * don't trust that one either. > >> */ > >> if (pipe_crc->skipped <= 0 || > >> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { > >> + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { > >> pipe_crc->skipped++; > >> spin_unlock(&pipe_crc->lock); > >> return; > > > > I would be interested in the results, haswell is different from > > VLV. Has it ever been observed on that platform? > > Good point. I looked at [1] which I presumed was on VLV, but it says > nothing about HSW. This whole thing is just a pile of duct tape anyway. The reason(s) for these funky crcs has never been properly analysed.
On Thu, 16 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, May 16, 2019 at 03:55:25PM +0300, Jani Nikula wrote: >> On Thu, 16 May 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: >> > Op 16-05-2019 om 07:58 schreef Harish Chegondi: >> >> display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the >> >> second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which >> >> is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. >> >> >> >> Cc: Jani Nikula <jani.nikula@intel.com> >> >> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> >> >> Cc: Petri Latvala <petri.latvala@intel.com> >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> >> Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 >> >> --- >> >> 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 233211fde0ea..3809e9f7fae2 100644 >> >> --- a/drivers/gpu/drm/i915/i915_irq.c >> >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> >> @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so >> >> + * On GEN7+ sometimes the second CRC is bonkers as well, so >> >> * don't trust that one either. >> >> */ >> >> if (pipe_crc->skipped <= 0 || >> >> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { >> >> + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { >> >> pipe_crc->skipped++; >> >> spin_unlock(&pipe_crc->lock); >> >> return; >> > >> > I would be interested in the results, haswell is different from >> > VLV. Has it ever been observed on that platform? >> >> Good point. I looked at [1] which I presumed was on VLV, but it says >> nothing about HSW. > > This whole thing is just a pile of duct tape anyway. The reason(s) > for these funky crcs has never been properly analysed. I don't disagree. And this is partially why I was so eager to ack the patch at hand. Another strip of duct tape does no harm when you already have so much, and we can't hold this particular issue hostage to root cause the issues. BR, Jani.
On Thu, 2019-05-16 at 15:55 +0300, Jani Nikula wrote: > On Thu, 16 May 2019, Maarten Lankhorst < > maarten.lankhorst@linux.intel.com> wrote: > > Op 16-05-2019 om 07:58 schreef Harish Chegondi: > > > display_pipe_crc_irq_handler() skips the first CRC for all GPUs > > > and the > > > second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT > > > which > > > is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 > > > --- > > > 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 233211fde0ea..3809e9f7fae2 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so > > > + * On GEN7+ sometimes the second CRC is bonkers as well, so > > > * don't trust that one either. > > > */ > > > if (pipe_crc->skipped <= 0 || > > > - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { > > > + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { > > > pipe_crc->skipped++; > > > spin_unlock(&pipe_crc->lock); > > > return; > > > > I would be interested in the results, haswell is different from > > VLV. Has it ever been observed on that platform? > > Good point. I looked at [1] which I presumed was on VLV, but it says > nothing about HSW. In fdo # 103191, CRC mismatch failures in igt@kms_pipe_crc_basic@* tests have not been observed on HSW. These tests have been very consistently failing on CI fi-byt-squawks system which is a chromebook, sporadically failing on CI fi-byt-clapper system which is also a chromebook. However the tests are passing on other CI BYT systems like fi-byt-n2820 and fi-byt-j1900 which are not chromebooks and the display is not eDP. I haven't seen these failures happening on other GEN7 GPUs. Instead of skipping the second CRC just for BYT, I thought it may be a better idea to skip the second CRC on all GEN7 GPUs as the current code is already skipping the second CRC on all GEN8+ GPUs. Thanks! Harish. > > BR, > Jani. > > > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=103191#c34 > >
On Thu, 2019-05-16 at 16:30 +0300, Ville Syrjälä wrote: > On Thu, May 16, 2019 at 03:55:25PM +0300, Jani Nikula wrote: > > On Thu, 16 May 2019, Maarten Lankhorst < > > maarten.lankhorst@linux.intel.com> wrote: > > > Op 16-05-2019 om 07:58 schreef Harish Chegondi: > > > > display_pipe_crc_irq_handler() skips the first CRC for all GPUs > > > > and the > > > > second CRC for GEN8+ GPUs. The second CRC is invalid even for > > > > BYT which > > > > is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 > > > > --- > > > > 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 233211fde0ea..3809e9f7fae2 100644 > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as > > > > well, so > > > > + * On GEN7+ sometimes the second CRC is bonkers as > > > > well, so > > > > * don't trust that one either. > > > > */ > > > > if (pipe_crc->skipped <= 0 || > > > > - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == > > > > 1)) { > > > > + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == > > > > 1)) { > > > > pipe_crc->skipped++; > > > > spin_unlock(&pipe_crc->lock); > > > > return; > > > > > > I would be interested in the results, haswell is different from > > > VLV. Has it ever been observed on that platform? > > > > Good point. I looked at [1] which I presumed was on VLV, but it > > says > > nothing about HSW. > > This whole thing is just a pile of duct tape anyway. The reason(s) > for these funky crcs has never been properly analysed. Ville, Are the patches in your branch : git://github.com/vsyrjala/linux.git g4x_fixes_4 related to fixing these invalid CRCs ? Thanks! Harish
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 233211fde0ea..3809e9f7fae2 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1775,11 +1775,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 GEN8+ sometimes the second CRC is bonkers as well, so + * On GEN7+ sometimes the second CRC is bonkers as well, so * don't trust that one either. */ if (pipe_crc->skipped <= 0 || - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) { + (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) { pipe_crc->skipped++; spin_unlock(&pipe_crc->lock); return;
display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs. Cc: Jani Nikula <jani.nikula@intel.com> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> Cc: Petri Latvala <petri.latvala@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Harish Chegondi <harish.chegondi@intel.com> References: https://bugs.freedesktop.org/show_bug.cgi?id=103191 --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)