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 <[email protected]>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +++++++++++++---------
>  1 file changed, 34 insertions(+), 21 deletions(-)

Reviewed-by: Balasubramani Vivekanandan <[email protected]>

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
> 

Reply via email to