Re: [Intel-gfx] [PATCH] drm/i915/perf: Enable application triggered OA reports

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Tue, Mar 31, 2020 at 02:46:46PM +0300, Lionel Landwerlin wrote:
>Gen12 brought an important redesign of the OA unit, splitting it in 2
>with a per context part (OAR) and a global part (OAG).
>
>OAR deals with per context counters and implements the
>MI_REPORT_PERF_COUNT command.
>
>OAG deals with global counters and the OA buffer.
>
>Unfortunately some of the counters available in OAG are not available
>in OAR, for instance counters that would report global caches
>utilization.
>
>Since applications making use of this want to access those additional
>OAG counters we can enable them to generate a report from their
>command buffer into the OA buffer. This is somewhat equivalent to
>having them doing their own MI_REPORT_PERF_COUNT. The application then
>parse the OA buffer as they were doing previously, only looking for a
>begin/end OA report with the appropriate reason field in the OA buffer
>instead of using MI_REPORT_PERF_COUNT generated reports for begin/end.
>
>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_perf.c            | 10 +++++++---
> drivers/gpu/drm/i915/i915_reg.h             |  2 ++
> 3 files changed, 27 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>index e96cc7fa0936..552eadaa6f9a 100644
>--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>@@ -1127,6 +1127,10 @@ static void gen9_whitelist_build(struct i915_wa_list *w)
>
> 	/* WaSendPushConstantsFromMMIO:skl,bxt */
> 	whitelist_reg(w, COMMON_SLICE_CHICKEN2);
>+
>+	/* Allow userspace trigger OA report generation in OA buffer. */
>+	whitelist_reg(w, OAREPORTTRIG2);
>+	whitelist_reg(w, OAREPORTTRIG6);
> }
>
> static void skl_whitelist_build(struct intel_engine_cs *engine)
>@@ -1208,6 +1212,10 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine)
>
> 	/* WaEnablePreemptionGranularityControlByUMD:cnl */
> 	whitelist_reg(w, GEN8_CS_CHICKEN1);
>+
>+	/* Allow userspace trigger OA report generation in OA buffer. */
>+	whitelist_reg(w, OAREPORTTRIG2);
>+	whitelist_reg(w, OAREPORTTRIG6);
> }
>
> static void icl_whitelist_build(struct intel_engine_cs *engine)
>@@ -1237,6 +1245,12 @@ static void icl_whitelist_build(struct intel_engine_cs *engine)
> 		whitelist_reg_ext(w, PS_INVOCATION_COUNT,
> 				  RING_FORCE_TO_NONPRIV_ACCESS_RD |
> 				  RING_FORCE_TO_NONPRIV_RANGE_4);
>+
>+		/*
>+		 * Allow userspace trigger OA report generation in OA buffer.
>+		 */
>+		whitelist_reg(w, OAREPORTTRIG2);
>+		whitelist_reg(w, OAREPORTTRIG6);
> 		break;
>
> 	case VIDEO_DECODE_CLASS:
>@@ -1281,6 +1295,10 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine)
>
> 		/* Wa_1806527549:tgl */
> 		whitelist_reg(w, HIZ_CHICKEN);
>+
>+		/* Allow userspace trigger OA report generation in OA buffer. */
>+		whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2);
>+		whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6);
> 		break;
> 	default:
> 		break;
>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>index 28e3d76fa2e6..ae935b1b1ae3 100644
>--- a/drivers/gpu/drm/i915/i915_perf.c
>+++ b/drivers/gpu/drm/i915/i915_perf.c
>@@ -1450,7 +1450,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream)
> 	 *  bit."
> 	 */
> 	intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset |
>-		   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT);
>+		   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT |
>+		   GEN8_OABUFFER_EDGE_TRIGGER);
> 	intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK);
>
> 	/* Mark that we need updated tail pointers to read from... */
>@@ -1503,7 +1504,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
> 	 *  bit."
> 	 */
> 	intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset |
>-			   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT);
>+			   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT |
>+			   GEN12_OAG_OABUFFER_EDGE_TRIGGER);
> 	intel_uncore_write(uncore, GEN12_OAG_OATAILPTR,
> 			   gtt_offset & GEN12_OAG_OATAILPTR_MASK);
>
>@@ -4481,8 +4483,10 @@ int i915_perf_ioctl_version(void)
> 	 *
> 	 * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the
> 	 *    interval for the hrtimer used to check for OA data.
>+	 *
>+	 * 6. Add edge trigger report generation support.
> 	 */
>-	return 5;
>+	return 6;

Do you think we should be adding a comment in uapi for revision 6?

If not, this patch looks good and is:

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

Thanks,
Umesh

> }
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 17484345cb80..813ae77ffeb7 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -661,6 +661,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define GEN8_OABUFFER_UDW _MMIO(0x23b4)
> #define GEN8_OABUFFER _MMIO(0x2b14)
>+#define  GEN8_OABUFFER_EDGE_TRIGGER         (1 << 2)
> #define  GEN8_OABUFFER_MEM_SELECT_GGTT      (1 << 0)  /* 0: PPGTT, 1: GGTT */
>
> #define GEN7_OASTATUS1 _MMIO(0x2364)
>@@ -712,6 +713,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define GEN12_OAG_OABUFFER  _MMIO(0xdb08)
> #define  GEN12_OAG_OABUFFER_BUFFER_SIZE_MASK  (0x7)
> #define  GEN12_OAG_OABUFFER_BUFFER_SIZE_SHIFT (3)
>+#define  GEN12_OAG_OABUFFER_EDGE_TRIGGER      (1 << 2)
> #define  GEN12_OAG_OABUFFER_MEMORY_SELECT     (1 << 0) /* 0: PPGTT, 1: GGTT */
>
> #define GEN12_OAG_OAGLBCTXCTRL _MMIO(0x2b28)
>-- 
>2.26.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx