On 14.10.2022 16:02, Matt Roper wrote: > MCR registers can be placed on the GuC's save/restore list, but at the > moment they are always handled in a multicast manner (i.e., the GuC > reads one instance to save the value and then does a multicast write to > restore that single value to all instances). In the future the GuC will > probably give us an alternate interface to do unicast per-instance > save/restore operations, so we should be very clear about which > registers on the list are MCR registers (and in the future which > save/restore behavior we want for them). > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +++++++++++++--------- > 1 file changed, 34 insertions(+), 21 deletions(-) Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@xxxxxxxxx> Regards, Bala > > 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 cc357fa0c270..de923fb82301 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > @@ -278,24 +278,16 @@ __mmio_reg_add(struct temp_regset *regset, struct guc_mmio_reg *reg) > return slot; > } > > -#define GUC_REGSET_STEERING(group, instance) ( \ > - FIELD_PREP(GUC_REGSET_STEERING_GROUP, (group)) | \ > - FIELD_PREP(GUC_REGSET_STEERING_INSTANCE, (instance)) | \ > - GUC_REGSET_NEEDS_STEERING \ > -) > - > static long __must_check guc_mmio_reg_add(struct intel_gt *gt, > struct temp_regset *regset, > - i915_reg_t reg, u32 flags) > + u32 offset, u32 flags) > { > u32 count = regset->storage_used - (regset->registers - regset->storage); > - u32 offset = i915_mmio_reg_offset(reg); > struct guc_mmio_reg entry = { > .offset = offset, > .flags = flags, > }; > struct guc_mmio_reg *slot; > - u8 group, inst; > > /* > * The mmio list is built using separate lists within the driver. > @@ -307,17 +299,6 @@ static long __must_check guc_mmio_reg_add(struct intel_gt *gt, > sizeof(entry), guc_mmio_reg_cmp)) > return 0; > > - /* > - * The GuC doesn't have a default steering, so we need to explicitly > - * steer all registers that need steering. However, we do not keep track > - * of all the steering ranges, only of those that have a chance of using > - * a non-default steering from the i915 pov. Instead of adding such > - * tracking, it is easier to just program the default steering for all > - * regs that don't need a non-default one. > - */ > - intel_gt_mcr_get_nonterminated_steering(gt, reg, &group, &inst); > - entry.flags |= GUC_REGSET_STEERING(group, inst); > - > slot = __mmio_reg_add(regset, &entry); > if (IS_ERR(slot)) > return PTR_ERR(slot); > @@ -335,6 +316,38 @@ static long __must_check guc_mmio_reg_add(struct intel_gt *gt, > > #define GUC_MMIO_REG_ADD(gt, regset, reg, masked) \ > guc_mmio_reg_add(gt, \ > + regset, \ > + i915_mmio_reg_offset(reg), \ > + (masked) ? GUC_REGSET_MASKED : 0) > + > +#define GUC_REGSET_STEERING(group, instance) ( \ > + FIELD_PREP(GUC_REGSET_STEERING_GROUP, (group)) | \ > + FIELD_PREP(GUC_REGSET_STEERING_INSTANCE, (instance)) | \ > + GUC_REGSET_NEEDS_STEERING \ > +) > + > +static long __must_check guc_mcr_reg_add(struct intel_gt *gt, > + struct temp_regset *regset, > + i915_reg_t reg, u32 flags) > +{ > + u8 group, inst; > + > + /* > + * The GuC doesn't have a default steering, so we need to explicitly > + * steer all registers that need steering. However, we do not keep track > + * of all the steering ranges, only of those that have a chance of using > + * a non-default steering from the i915 pov. Instead of adding such > + * tracking, it is easier to just program the default steering for all > + * regs that don't need a non-default one. > + */ > + intel_gt_mcr_get_nonterminated_steering(gt, reg, &group, &inst); > + flags |= GUC_REGSET_STEERING(group, inst); > + > + return guc_mmio_reg_add(gt, regset, i915_mmio_reg_offset(reg), flags); > +} > + > +#define GUC_MCR_REG_ADD(gt, regset, reg, masked) \ > + guc_mcr_reg_add(gt, \ > regset, \ > (reg), \ > (masked) ? GUC_REGSET_MASKED : 0) > @@ -375,7 +388,7 @@ static int guc_mmio_regset_init(struct temp_regset *regset, > /* add in local MOCS registers */ > for (i = 0; i < LNCFCMOCS_REG_COUNT; i++) > if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) > - ret |= GUC_MMIO_REG_ADD(gt, regset, XEHP_LNCFCMOCS(i), false); > + ret |= GUC_MCR_REG_ADD(gt, regset, XEHP_LNCFCMOCS(i), false); > else > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN9_LNCFCMOCS(i), false); > > -- > 2.37.3 >