drm/i915/perf: Configure OAR controls for specific context

Commit Message

Umesh Nerlige Ramappa Nov. 7, 2019, 11:22 p.m. UTC

It turns out that the OAR CONTROL register is not getting configured
correctly in conjunction with the context save/restore bit. When
measuring work for a single context, the OAR counters do not increment.

Configure OAR format and enable OAR counters at the same time as
enabling context save/restore for OAR counters.

Signed-off-by: Umesh Nerlige Ramappa <[e-mail]>
---
 drivers/gpu/drm/i915/i915_perf.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Comments

Chris Wilson Nov. 7, 2019, 11:30 p.m. UTC | #1

Quoting Umesh Nerlige Ramappa (2019-11-07 23:22:34)
> It turns out that the OAR CONTROL register is not getting configured
> correctly in conjunction with the context save/restore bit. When
> measuring work for a single context, the OAR counters do not increment.
> 
> Configure OAR format and enable OAR counters at the same time as
> enabling context save/restore for OAR counters.

Did you write a test case to capture this bug for posterity? If so,
please include a Testcase: link. If not, please do so :)
-Chris
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Umesh Nerlige Ramappa Nov. 7, 2019, 11:41 p.m. UTC | #2

On Thu, Nov 07, 2019 at 11:30:51PM +0000, Chris Wilson wrote:
>Quoting Umesh Nerlige Ramappa (2019-11-07 23:22:34)
>> It turns out that the OAR CONTROL register is not getting configured
>> correctly in conjunction with the context save/restore bit. When
>> measuring work for a single context, the OAR counters do not increment.
>>
>> Configure OAR format and enable OAR counters at the same time as
>> enabling context save/restore for OAR counters.
>
>Did you write a test case to capture this bug for posterity? If so,
>please include a Testcase: link. If not, please do so :)

I did, although the test is also up for review here in an igt series - 
https://patchwork.freedesktop.org/patch/339514/

Thanks,
Umesh

>-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 2c380aba1ce9..527a16637689 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2219,26 +2219,33 @@  static int gen8_configure_context(struct i915_gem_context *ctx,
 	return err;
 }
 
-static int gen12_emit_oar_config(struct intel_context *ce, bool enable)
+static int gen12_emit_oar_config(struct i915_perf_stream *stream, bool enable)
 {
 	struct i915_request *rq;
+	struct intel_context *ce = stream->pinned_ctx;
 	u32 *cs;
+	u32 format = stream->oa_buffer.format;
 	int err = 0;
 
 	rq = i915_request_create(ce);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-	cs = intel_ring_begin(rq, 4);
+	cs = intel_ring_begin(rq, 6);
 	if (IS_ERR(cs)) {
 		err = PTR_ERR(cs);
 		goto out;
 	}
 
-	*cs++ = MI_LOAD_REGISTER_IMM(1);
+	*cs++ = MI_LOAD_REGISTER_IMM(2);
+	/* Enable context save/restore of OAR counters */
 	*cs++ = i915_mmio_reg_offset(RING_CONTEXT_CONTROL(ce->engine->mmio_base));
 	*cs++ = _MASKED_FIELD(GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE,
 			      enable ? GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE : 0);
+	/* Enable OAR counters */
+	*cs++ = i915_mmio_reg_offset(GEN12_OAR_OACONTROL);
+	*cs++ = (format << GEN12_OAR_OACONTROL_COUNTER_FORMAT_SHIFT) |
+		(enable ? GEN12_OAR_OACONTROL_COUNTER_ENABLE : 0);
 	*cs++ = MI_NOOP;
 
 	intel_ring_advance(rq, cs);
@@ -2474,8 +2481,7 @@  static int gen12_enable_metric_set(struct i915_perf_stream *stream)
 	 * requested this.
 	 */
 	if (stream->ctx) {
-		ret = gen12_emit_oar_config(stream->pinned_ctx,
-					    oa_config != NULL);
+		ret = gen12_emit_oar_config(stream, oa_config != NULL);
 		if (ret)
 			return ret;
 	}
@@ -2513,7 +2519,7 @@  static void gen12_disable_metric_set(struct i915_perf_stream *stream)
 
 	/* disable the context save/restore or OAR counters */
 	if (stream->ctx)
-		gen12_emit_oar_config(stream->pinned_ctx, false);
+		gen12_emit_oar_config(stream, false);
 
 	/* Make sure we disable noa to save power. */
 	intel_uncore_rmw(uncore, RPM_CONFIG1, GEN10_GT_NOA_ENABLE, 0);
@@ -2713,7 +2719,8 @@  static int i915_oa_stream_init(struct i915_perf_stream *stream,
 		return -EINVAL;
 	}
 
-	if (!(props->sample_flags & SAMPLE_OA_REPORT)) {
+	if (!(props->sample_flags & SAMPLE_OA_REPORT) &&
+	    (INTEL_GEN(perf->i915) < 12 || !stream->ctx)) {
 		DRM_DEBUG("Only OA report sampling supported\n");
 		return -EINVAL;
 	}
@@ -2745,7 +2752,7 @@  static int i915_oa_stream_init(struct i915_perf_stream *stream,
 
 	format_size = perf->oa_formats[props->oa_format].size;
 
-	stream->sample_flags |= SAMPLE_OA_REPORT;
+	stream->sample_flags = props->sample_flags;
 	stream->sample_size += format_size;
 
 	stream->oa_buffer.format_size = format_size;