Re: [Intel-gfx] [PATCH v3 10/14] drm/i915/guc: Handle save/restore of MCR registers explicitly
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
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +-
> 1 file changed, 34 insertions(+), 21 deletions(-)
Reviewed-by: Balasubramani Vivekanandan
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, , );
> - entry.flags |= GUC_REGSET_STEERING(group, inst);
> -
> slot = __mmio_reg_add(regset, );
> 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, , );
> + 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,
[Intel-gfx] [PATCH v3 10/14] drm/i915/guc: Handle save/restore of MCR registers explicitly
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
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +-
1 file changed, 34 insertions(+), 21 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 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, , );
- entry.flags |= GUC_REGSET_STEERING(group, inst);
-
slot = __mmio_reg_add(regset, );
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, , );
+ 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
