| Message ID | 20231220123951.4076088-1-shuicheng.lin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/guc: Change wa and EU_PERF_CNTL registers to MCR type | expand |
On Wed, Dec 20, 2023 at 12:39:51PM +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@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 16 ++++++++-------- > 1 file changed, 8 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..61ff4c7e31a6 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > @@ -378,7 +378,7 @@ static int guc_mmio_regset_init(struct temp_regset *regset, > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN12_RCU_MODE, true); > > 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); I'd add some kind of a comment here, explaining that it's safe to always use the MCR form here, even though some of the workarounds are touching non-MCR registers. With a clarifying code comment added, Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Matt > > /* Be extra paranoid and include all whitelist registers. */ > for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) > @@ -394,13 +394,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 >
Best Regards Shuicheng > -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Thursday, December 21, 2023 7:46 AM > To: Lin, Shuicheng <shuicheng.lin@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Nerlige Ramappa, Umesh > <umesh.nerlige.ramappa@intel.com> > Subject: Re: [PATCH] drm/i915/guc: Change wa and EU_PERF_CNTL registers > to MCR type > > On Wed, Dec 20, 2023 at 12:39:51PM +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@intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > > --- > > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 16 ++++++++-------- > > 1 file changed, 8 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..61ff4c7e31a6 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > > @@ -378,7 +378,7 @@ static int guc_mmio_regset_init(struct temp_regset > *regset, > > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN12_RCU_MODE, > true); > > > > 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); > > I'd add some kind of a comment here, explaining that it's safe to always use > the MCR form here, even though some of the workarounds are touching non- > MCR registers. With a clarifying code comment added, > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > > Matt Hi Matt, thanks for the review. I have updated patch to add the code comment. Please let me know if you have any questions. Thanks. > > > > > /* Be extra paranoid and include all whitelist registers. */ > > for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) @@ -394,13 +394,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
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..61ff4c7e31a6 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c @@ -378,7 +378,7 @@ static int guc_mmio_regset_init(struct temp_regset *regset, ret |= GUC_MMIO_REG_ADD(gt, regset, GEN12_RCU_MODE, true); 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 +394,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;
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@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)