Re: [PATCH v2 1/1] drm/i915: skip the second CRC even for GEN 7 GPUs

Chegondi, Harish
2019-10-23T22:15:21Z

Hi,

Even though I tried to link this patch with it's first version by
specifying --in-reply-to=<message-id>, it wasn't successful. So here is
the link to the first version of the patch and the discussion.

https://patchwork.freedesktop.org/patch/305153/?series=60697&rev=1

The first version of this patch has been "Acked-by" but wasn't
"Reviewed-by" as the patch adds another workaround on top of an already
existing workaround. The patch doesn't fix the cause of invalid CRCs
being generated which still needs to be investigated and fixed. I am
rebasing and resending the patch to seek feedback on how to move
further with this patch.

Thank You
Harish

On Wed, 2019-10-23 at 00:24 -0700, Harish Chegondi 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.
>
> v2: Rebase
>
> Cc: Jani Saarinen <jani.saarinen@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>
> Acked-by: Jani Nikula <jani.nikula@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 572a5c37cc61..312ca9d5292a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1565,11 +1565,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;