On Mon, Mar 20, 2023 at 12:16:17PM +0200, Jani Nikula wrote:
On Fri, 17 Mar 2023, Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> wrote:
From: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
If we fail to adjust the GuC run-control on opening the perf stream,
make sure we unwind the wakeref just taken.
v2: Retain old goto label names (Ashutosh)
Fixes: 01e742746785 ("drm/i915/guc: Support OA when Wa_16011777198 is enabled")
Signed-off-by: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_perf.c | 14 +++++++++-----
drivers/gpu/drm/i915/i915_perf_types.h | 6 ++++++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 824a34ec0b83..283a4a3c6862 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1592,9 +1592,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
/*
* Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
*/
- if (intel_uc_uses_guc_rc(>->uc) &&
- (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
- IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))
+ if (stream->override_gucrc)
drm_WARN_ON(>->i915->drm,
intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc));
@@ -3305,8 +3303,10 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
if (ret) {
drm_dbg(&stream->perf->i915->drm,
"Unable to override gucrc mode\n");
- goto err_config;
+ goto err_gucrc;
}
+
+ stream->override_gucrc = true;
}
ret = alloc_oa_buffer(stream);
@@ -3345,11 +3345,15 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
free_oa_buffer(stream);
err_oa_buf_alloc:
- free_oa_configs(stream);
+ if (stream->override_gucrc)
+ intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc);
+err_gucrc:
intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
intel_engine_pm_put(stream->engine);
+ free_oa_configs(stream);
+
err_config:
free_noa_wait(stream);
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index ca150b7af3f2..e36f046fe2b6 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -316,6 +316,12 @@ struct i915_perf_stream {
* buffer should be checked for available data.
*/
u64 poll_oa_period;
+
+ /**
+ * @override_gucrc: GuC RC has been overridden for the perf stream,
+ * and we need to restore the default configuration on release.
+ */
+ bool override_gucrc:1;
What do you gain by making this a bitfield? It's already a big struct,
and there already are other bool flags.
Noticed it now. I guess this is not portable as it's compiler dependent.
I would just remove the bitfield. I do see a few occurrences of bitfield
bools in i915 though, so any specific guidelines on when to use bool vs
bitfields?
Thanks,
Umesh
BR,
Jani.
};
/**
--
Jani Nikula, Intel Open Source Graphics Center
