[Intel-gfx] [PATCH] drm/i915/perf: workaround register corruption in OATAILPTR

[Umesh Nerlige Ramappa]

On Tue, Nov 17, 2020 at 03:01:24PM +0200, Lionel Landwerlin wrote:
>After having written the entire OA buffer with reports, the HW will
>write again at the beginning of the OA buffer. It'll indicate it by
>setting the WRAP bits in the OASTATUS register.
>
>When a wrap happens and that at the end of the read vfunc we write the
>OASTATUS register back to clear the REPORT_LOST bit, we sometimes see
>that the OATAILPTR register is reset to a previous position on Gen8/9
>(apparently not the case on Gen11+). This leads the next call to the
>read vfunc to process reports we've already read. Because we've marked
>those as read by clearing the reason & timestamp dwords, they're
>discarded and a "Skipping spurious, invalid OA report" message is
>emitted.
>
>The workaround to avoid this OATAILPTR value reset seems to be to set
>the wrap bits when writing back OASTATUS.
>
>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+")
>---
> drivers/gpu/drm/i915/i915_perf.c | 9 +++++++--
> drivers/gpu/drm/i915/i915_reg.h  | 2 ++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>index e94976976571..3640d0e229d2 100644
>--- a/drivers/gpu/drm/i915/i915_perf.c
>+++ b/drivers/gpu/drm/i915/i915_perf.c
>@@ -909,8 +909,13 @@ static int gen8_oa_read(struct i915_perf_stream *stream,
> 				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
> 		if (ret)
> 			return ret;
>-		intel_uncore_write(uncore, oastatus_reg,
>-				   oastatus & ~GEN8_OASTATUS_REPORT_LOST);
>+
>+		intel_uncore_rmw(uncore, oastatus_reg,
>+				 GEN8_OASTATUS_COUNTER_OVERFLOW |
>+				 GEN8_OASTATUS_REPORT_LOST,
>+				 IS_GEN_RANGE(uncore->i915, 8, 10) ?
>+				 (GEN8_OASTATUS_HEAD_POINTER_WRAP |
>+				  GEN8_OASTATUS_TAIL_POINTER_WRAP) : 0);

Great find. Thanks for resolving this.

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>

Umesh
> 	}
>
> 	return gen8_append_oa_reports(stream, buf, count, offset);
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 7ea70b7ffcc6..5375b219cc3b 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -679,6 +679,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define  GEN7_OASTATUS2_MEM_SELECT_GGTT     (1 << 0) /* 0: PPGTT, 1: GGTT */
>
> #define GEN8_OASTATUS _MMIO(0x2b08)
>+#define  GEN8_OASTATUS_TAIL_POINTER_WRAP    (1 << 17)
>+#define  GEN8_OASTATUS_HEAD_POINTER_WRAP    (1 << 16)
> #define  GEN8_OASTATUS_OVERRUN_STATUS	    (1 << 3)
> #define  GEN8_OASTATUS_COUNTER_OVERFLOW     (1 << 2)
> #define  GEN8_OASTATUS_OABUFFER_OVERFLOW    (1 << 1)
>-- 
>2.29.2
>