Re: [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.

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