Re: [Intel-gfx] [PATCH v4 2/2] drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads

2018-03-26 Thread Tvrtko Ursulin 


On 26/03/2018 17:12, Yunwei Zhang wrote:

L3Bank could be fused off in hardware for debug purpose, and it
is possible that subslice is enabled while its corresponding L3Bank pairs
are disabled. In such case, if MCR packet control register(0xFDC) is
programed to point to a disabled bank pair, a MMIO read into L3Bank range
will return 0 instead of correct values.

However, this is not going to be the case in any production silicon.
Therefore, we only check at initialization and issue a warning should
this really happen.

v2:
  - use fls instead of find_last_bit (Chris)
  - use is_power_of_2() instead of counting bit set (Chris)
v3:
  - rebase on latest tip
v4:
  - Added referecens (Mika)

References: HSDES#1405586840

Signed-off-by: Yunwei Zhang 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
---
  drivers/gpu/drm/i915/i915_reg.h|  4 
  drivers/gpu/drm/i915/intel_engine_cs.c | 18 ++
  2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1bca695f..4f2f5e1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2691,6 +2691,10 @@ enum i915_power_well_id {
  #define   GEN10_F2_SS_DIS_SHIFT   18
  #define   GEN10_F2_SS_DIS_MASK(0xf << GEN10_F2_SS_DIS_SHIFT)
  
+#define	GEN10_MIRROR_FUSE3		_MMIO(0x9118)

+#define GEN10_L3BANK_PAIR_COUNT 4
+#define GEN10_L3BANK_MASK   0x0F
+
  #define GEN8_EU_DISABLE0  _MMIO(0x9134)
  #define   GEN8_EU_DIS0_S0_MASK0xff
  #define   GEN8_EU_DIS0_S1_SHIFT   24
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index cc19e0a..3eb763c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -811,7 +811,25 @@ static u32 calculate_mcr(u32 mcr, struct drm_i915_private 
*dev_priv)
  static void wa_init_mcr(struct drm_i915_private *dev_priv)
  {
u32 mcr;
+   u32 fuse3;
+   const struct sseu_dev_info *sseu = &(INTEL_SSEU(dev_priv));
+   u32 slice;


fuse3 and slice can be moved into the is_power_of_2 block below.

  
+	/* If more than one slice are enabled, L3Banks should be all enabled */

+   if (is_power_of_2(sseu->slice_mask)) {
+   /*
+* WaProgramMgsrForL3BankSpecificMmioReads:
+* read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches
+* enabled subslice, no need to redirect MCR packet
+*/
+   slice = fls(sseu->slice_mask);
+   fuse3 = I915_READ(GEN10_MIRROR_FUSE3);
+   if (WARN_ON(!((fuse3 & GEN10_L3BANK_MASK)
+  & ((sseu->subslice_mask[slice]
+  | 
sseu->subslice_mask[slice]>>GEN10_L3BANK_PAIR_COUNT)


Spaces around operators to satisfy checkpatch.


+  & GEN10_L3BANK_MASK


Whole conditional is a bit hard to read (maybe it is just me!) so maybe 
store sseu->subslice_mask[slice] to a local? Not sure if that would make 
it better.



+   DRM_WARN("Production silicon should have matched L3Bank and 
subslice enabled\n");


Aren't both WARN_ON and DRM_WARN an overkill? One should be enough I hope.


+   }
mcr = I915_READ(GEN8_MCR_SELECTOR);
mcr = calculate_mcr(mcr, dev_priv);
I915_WRITE(GEN8_MCR_SELECTOR, mcr);



Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v4 2/2] drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads

2018-03-26 Thread Yunwei Zhang 
L3Bank could be fused off in hardware for debug purpose, and it
is possible that subslice is enabled while its corresponding L3Bank pairs
are disabled. In such case, if MCR packet control register(0xFDC) is
programed to point to a disabled bank pair, a MMIO read into L3Bank range
will return 0 instead of correct values.

However, this is not going to be the case in any production silicon.
Therefore, we only check at initialization and issue a warning should
this really happen.

v2:
 - use fls instead of find_last_bit (Chris)
 - use is_power_of_2() instead of counting bit set (Chris)
v3:
 - rebase on latest tip
v4:
 - Added referecens (Mika)

References: HSDES#1405586840

Signed-off-by: Yunwei Zhang 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_reg.h|  4 
 drivers/gpu/drm/i915/intel_engine_cs.c | 18 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1bca695f..4f2f5e1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2691,6 +2691,10 @@ enum i915_power_well_id {
 #define   GEN10_F2_SS_DIS_SHIFT18
 #define   GEN10_F2_SS_DIS_MASK (0xf << GEN10_F2_SS_DIS_SHIFT)
 
+#defineGEN10_MIRROR_FUSE3  _MMIO(0x9118)
+#define GEN10_L3BANK_PAIR_COUNT 4
+#define GEN10_L3BANK_MASK   0x0F
+
 #define GEN8_EU_DISABLE0   _MMIO(0x9134)
 #define   GEN8_EU_DIS0_S0_MASK 0xff
 #define   GEN8_EU_DIS0_S1_SHIFT24
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index cc19e0a..3eb763c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -811,7 +811,25 @@ static u32 calculate_mcr(u32 mcr, struct drm_i915_private 
*dev_priv)
 static void wa_init_mcr(struct drm_i915_private *dev_priv)
 {
u32 mcr;
+   u32 fuse3;
+   const struct sseu_dev_info *sseu = &(INTEL_SSEU(dev_priv));
+   u32 slice;
 
+   /* If more than one slice are enabled, L3Banks should be all enabled */
+   if (is_power_of_2(sseu->slice_mask)) {
+   /*
+* WaProgramMgsrForL3BankSpecificMmioReads:
+* read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches
+* enabled subslice, no need to redirect MCR packet
+*/
+   slice = fls(sseu->slice_mask);
+   fuse3 = I915_READ(GEN10_MIRROR_FUSE3);
+   if (WARN_ON(!((fuse3 & GEN10_L3BANK_MASK)
+  & ((sseu->subslice_mask[slice]
+  | 
sseu->subslice_mask[slice]>>GEN10_L3BANK_PAIR_COUNT)
+  & GEN10_L3BANK_MASK
+   DRM_WARN("Production silicon should have matched L3Bank 
and subslice enabled\n");
+   }
mcr = I915_READ(GEN8_MCR_SELECTOR);
mcr = calculate_mcr(mcr, dev_priv);
I915_WRITE(GEN8_MCR_SELECTOR, mcr);
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx