[Intel-gfx] [PATCH v2 2/2] drm/i915/gt: Extract function to apply media fuses

[Lucas De Marchi]

On Mon, Sep 12, 2022 at 10:56:16AM +0200, Andrzej Hajda wrote:
>On 10.09.2022 01:18, Lucas De Marchi wrote:
>>Just like is done for compute and copy engines, extract a function to
>>handle media engines. While at it, be consistent on using or not the
>>uncore/gt/info variable aliases.
>>
>>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>index b6602439224d..814f83b5fe59 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>@@ -663,6 +663,74 @@ bool gen11_vdbox_has_sfc(struct intel_gt *gt,
>>  	return false;
>>  }
>>+static void engine_mask_apply_media_fuses(struct intel_gt *gt)
>>+{
>>+	struct drm_i915_private *i915 = gt->i915;
>>+	unsigned int logical_vdbox = 0;
>>+	unsigned int i;
>>+	u32 media_fuse, fuse1;
>>+	u16 vdbox_mask;
>>+	u16 vebox_mask;
>>+
>>+	if (MEDIA_VER(gt->i915) < 11)
>>+		return;
>>+
>>+	/*
>>+	 * On newer platforms the fusing register is called 'enable' and has
>>+	 * enable semantics, while on older platforms it is called 'disable'
>>+	 * and bits have disable semantices.
>>+	 */
>>+	media_fuse = intel_uncore_read(gt->uncore, GEN11_GT_VEBOX_VDBOX_DISABLE);
>>+	if (MEDIA_VER_FULL(i915) < IP_VER(12, 50))
>>+		media_fuse = ~media_fuse;
>>+
>>+	vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
>>+	vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
>>+		      GEN11_GT_VEBOX_DISABLE_SHIFT;
>>+
>>+	if (MEDIA_VER_FULL(i915) >= IP_VER(12, 50)) {
>>+		fuse1 = intel_uncore_read(gt->uncore, HSW_PAVP_FUSE1);
>>+		gt->info.sfc_mask = REG_FIELD_GET(XEHP_SFC_ENABLE_MASK, fuse1);
>>+	} else {
>>+		gt->info.sfc_mask = ~0;
>>+	}
>>+
>>+	for (i = 0; i < I915_MAX_VCS; i++) {
>>+		if (!HAS_ENGINE(gt, _VCS(i))) {
>>+			vdbox_mask &= ~BIT(i);
>>+			continue;
>>+		}
>>+
>>+		if (!(BIT(i) & vdbox_mask)) {
>>+			gt->info.engine_mask &= ~BIT(_VCS(i));
>>+			drm_dbg(&i915->drm, "vcs%u fused off\n", i);
>>+			continue;
>>+		}
>>+
>>+		if (gen11_vdbox_has_sfc(gt, i, logical_vdbox, vdbox_mask))
>>+			gt->info.vdbox_sfc_access |= BIT(i);
>>+		logical_vdbox++;
>>+	}
>>+	drm_dbg(&i915->drm, "vdbox enable: %04x, instances: %04lx\n",
>>+		vdbox_mask, VDBOX_MASK(gt));
>>+	GEM_BUG_ON(vdbox_mask != VDBOX_MASK(gt));
>>+
>>+	for (i = 0; i < I915_MAX_VECS; i++) {
>>+		if (!HAS_ENGINE(gt, _VECS(i))) {
>>+			vebox_mask &= ~BIT(i);
>>+			continue;
>>+		}
>>+
>>+		if (!(BIT(i) & vebox_mask)) {
>>+			gt->info.engine_mask &= ~BIT(_VECS(i));
>>+			drm_dbg(&i915->drm, "vecs%u fused off\n", i);
>>+		}
>>+	}
>>+	drm_dbg(&i915->drm, "vebox enable: %04x, instances: %04lx\n",
>>+		vebox_mask, VEBOX_MASK(gt));
>>+	GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt));
>>+}
>>+
>>  static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
>>  {
>>  	struct drm_i915_private *i915 = gt->i915;
>>@@ -671,6 +739,9 @@ static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
>>  	unsigned long ccs_mask;
>>  	unsigned int i;
>>+	if (GRAPHICS_VER(i915) < 11)
>>+		return;
>>+
>
>Why there is no similar sentinel in case of engine_mask_apply_copy_fuses?
>Beside this:
>Reviewed-by: Andrzej Hajda <andrzej.hajda at intel.com>

I noticed it too while doing these patches. I have a pending one for
that, but it seems I failed to send it on Friday.

drm/i915: Skip applying copy engine fuses

Support for reading the fuses to check what are the Link Copy engines
was added in commit ad5f74f34201 ("drm/i915/pvc: read fuses for link
copy engines"). However they were added unconditionally because the
FUSE3 register is present since graphics version 10.

However the bitfield with meml3 fuses only exists since graphics version
12. Moreover, Link Copy engines are currently only available in PVC.
Tying additional copy engines to the meml3 fuses is not correct for
other platforms.

Make sure there is a check for  `12.60 <= ver < 12.70`. Later platforms
may extend this function later if it's needed to fuse off copy engines.

Currently it's harmless as the Link Copy engines are still not exported:
info->engine_mask only has BCS0 set and the register is only read for
platforms that do have it.

Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---
  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6e0122b3dca2..ac5ff17888cd 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -694,6 +694,10 @@ static void engine_mask_apply_copy_fuses(struct intel_gt *gt)
  	unsigned long meml3_mask;
  	unsigned long quad;
  
+	if (!(GRAPHICS_VER_FULL(i915) >= IP_VER(12, 60) &&
+	      GRAPHICS_VER_FULL(i915) < IP_VER(12, 70)))
+		return;
+
  	meml3_mask = intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3);
  	meml3_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, meml3_mask);