On Tue, Jan 02, 2024 at 01:02:31AM +0000, Shuicheng Lin wrote: > Some of the wa registers are MCR register, and EU_PERF_CNTL registers > are MCR register. > MCR register needs extra process for read/write. > As normal MMIO register also could work with the MCR register process, > change all wa registers to MCR type for code simplicity. > > Signed-off-by: Shuicheng Lin <shuicheng.lin@xxxxxxxxx> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Tweaked the comment to conform to Linux kernel coding standards, and applied to drm-intel-gt-next. Thanks for the patch. At some point we'll need to rework the OA/perf code to use proper register types for a bunch of their registers (so that we don't need ugly casts like this), but we can take care of that separately. Matt > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > index 63724e17829a..5cbcbe9a4e93 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > @@ -377,8 +377,12 @@ static int guc_mmio_regset_init(struct temp_regset *regset, > CCS_MASK(engine->gt)) > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN12_RCU_MODE, true); > > + /* some of the WA registers are MCR registers. As it is safe to > + * use MCR form for non-MCR registers, for code simplicity, all > + * WA registers are added with MCR form. > + */ > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) > - ret |= GUC_MMIO_REG_ADD(gt, regset, wa->reg, wa->masked_reg); > + ret |= GUC_MCR_REG_ADD(gt, regset, wa->mcr_reg, wa->masked_reg); > > /* Be extra paranoid and include all whitelist registers. */ > for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) > @@ -394,13 +398,13 @@ static int guc_mmio_regset_init(struct temp_regset *regset, > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN9_LNCFCMOCS(i), false); > > if (GRAPHICS_VER(engine->i915) >= 12) { > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL0, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL1, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL2, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL3, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL4, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL5, false); > - ret |= GUC_MMIO_REG_ADD(gt, regset, EU_PERF_CNTL6, false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL0)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL1)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL2)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL3)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL4)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL5)), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, MCR_REG(i915_mmio_reg_offset(EU_PERF_CNTL6)), false); > } > > return ret ? -1 : 0; > -- > 2.25.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation