diff mbox series

[v3,10/14] drm/i915/guc: Handle save/restore of MCR registers explicitly

Message ID 20221014230239.1023689-11-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series Explicit MCR handling and MTL steering | expand

Commit Message

Matt Roper Oct. 14, 2022, 11:02 p.m. UTC
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@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +++++++++++++---------
 1 file changed, 34 insertions(+), 21 deletions(-)

Comments

Vivekanandan, Balasubramani Oct. 17, 2022, 4:54 p.m. UTC | #1
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@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 55 +++++++++++++---------
>  1 file changed, 34 insertions(+), 21 deletions(-)

Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

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
>
diff mbox series

Patch

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);