On Sun, Mar 19, 2023 at 08:56:00PM -0700, Dixit, Ashutosh wrote:
On Fri, 17 Mar 2023 16:16:41 -0700, Umesh Nerlige Ramappa wrote:
Hi Umesh,
Please read from bottom to top. Mostly nits but let's see what you think.
OAM does not work with media C6 enabled on some steppings of MTL.
Disable OAM if we detect that media C6 was enabled in bios.
v2: (Ashutosh)
- Remove drm_notice from the driver load path
- Log a drm_err when opening an OAM stream on affected steppings
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_perf.c | 41 ++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 18afa76653b7..823379d63caf 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -209,6 +209,7 @@
#include "gt/intel_gt_regs.h"
#include "gt/intel_lrc.h"
#include "gt/intel_lrc_reg.h"
+#include "gt/intel_rc6.h"
#include "gt/intel_ring.h"
#include "gt/uc/intel_guc_slpc.h"
@@ -4216,6 +4217,20 @@ static int read_properties_unlocked(struct i915_perf *perf,
return -EINVAL;
}
+ /*
+ * Wa_14017512683: mtl[a0..c0): Use of OAM must be preceded with Media
+ * C6 disable in BIOS. Fail if Media C6 is enabled on steppings where OAM
+ * does not work as expected.
+ */
+ if (IS_MTL_MEDIA_STEP(props->engine->i915, STEP_A0, STEP_C0) &&
+ props->engine->gt->type == GT_MEDIA &&
Instead of gt type I think it's better to check perf_group->type. That is
why as I said below maybe better to have a valid perf_group even in this
case.
+ intel_check_bios_c6_setup(&props->engine->gt->rc6)) {
+ drm_dbg(&perf->i915->drm,
+ "OAM requires media C6 to be disabled in BIOS\n");
+ return -EINVAL;
+ }
So now we can change this check to something like:
if (engine->perf_group->type == OAM && i915_perf->mtl_bios_mc6_enabled)
sure, that will work too.
+
+
if (!engine_supports_oa(props->engine)) {
drm_dbg(&perf->i915->drm,
"Engine not supported by OA %d:%d\n",
@@ -4897,6 +4912,15 @@ static u32 num_perf_groups_per_gt(struct intel_gt *gt)
static u32 __oam_engine_group(struct intel_engine_cs *engine)
{
+ /*
+ * Wa_14017512683: mtl[a0..c0): Use of OAM must be preceded with Media
+ * C6 disable in BIOS. To disable use of OAM with media engines, set the
+ * oa_group to PERF_GROUP_INVALID.
+ */
+ if (IS_MTL_MEDIA_STEP(engine->i915, STEP_A0, STEP_C0) &&
+ intel_check_bios_c6_setup(&engine->gt->rc6))
+ return PERF_GROUP_INVALID;
I think we should just remove this. Let the perf group be valid in this
case too since we cannot support OA for a different reason. Then we can use
the OAM perf group above.
Sure, I will drop this
Though if we drop this we have only 2 instances of the checks instead of 3
so maybe ok to not have i915_perf->mtl_bios_mc6_enabled?
yes, since it's only 2 places, I would prefer to leave it as is.
Thanks,
Umesh
