[PATCH v2] drm/xe: Apply workaround 14016747170

[Cavitt, Jonathan]

Hello,
To answer your questions:


  1.  Should it be FUSE4 instead of FUSE3?

I think my use of FUSE3 here was an artifact from when I was applying a different workaround to the same location that applied the same change.  I’d have to get external confirmation about that, though, and see which of the two would be more appropriate to use here.


  2.  Was there any particular reason for just adding an if(wa) and not using an if(wa)/else condition similar to i915 implementation?

The else condition was already covered during variable initialization.  Breaking the initialization stage into declaration and conditional initialization seemed unnecessary given such.


I hope this helps

-Jonathan Cavitt

From: Pottumuttu, Sai Teja <sai.teja.pottumuttu at intel.com>
Sent: Monday, September 2, 2024 4:16 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>; intel-xe at lists.freedesktop.org
Cc: Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>; Roper, Matthew D <matthew.d.roper at intel.com>; jani.nikula at linux.intel.com; Chauhan, Shekhar <shekhar.chauhan at intel.com>
Subject: Re: [PATCH v2] drm/xe: Apply workaround 14016747170



On 30-08-2024 21:41, Jonathan-Cavitt wrote:

Some revisions of MTL do not properly report the correct value from the

FUSE3_MBC_MEDIA register.  This results in the wrong value being

reported for the l3 mask.
Should it be FUSE4 instead of FUSE3?






Use the recommended replacement register in this case.



Signed-off-by: Jonathan-Cavitt <jonathan.cavitt at intel.com><mailto:jonathan.cavitt at intel.com>

CC: Matt Roper <matthew.d.roper at intel.com><mailto:matthew.d.roper at intel.com>

CC: Jani Nikula <jani.nikula at linux.intel.com><mailto:jani.nikula at linux.intel.com>

---



v2: Update includes ordering (Jani)



 drivers/gpu/drm/xe/regs/xe_gt_regs.h | 3 +++

 drivers/gpu/drm/xe/xe_gt_mcr.c       | 8 ++++++++

 drivers/gpu/drm/xe/xe_gt_topology.c  | 9 +++++++++

 drivers/gpu/drm/xe/xe_wa_oob.rules   | 1 +

 4 files changed, 21 insertions(+)



diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h

index 0d1a4a9f4e119..e0d735a5a7fa1 100644

--- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h

+++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h

@@ -522,6 +522,9 @@

 #define   FORCEWAKE_MT(bit)                   BIT(bit)

 #define   FORCEWAKE_MT_MASK(bit)              BIT((bit) + 16)



+#define MTL_GT_ACTIVITY_FACTOR           XE_REG(0x138010)

+#define   MTL_GT_L3_EXC_MASK             REG_GENMASK(5, 3)

+

 #define MTL_MEDIA_PERF_LIMIT_REASONS     XE_REG(0x138030)

 #define MTL_MEDIA_MC6                    XE_REG(0x138048)



diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c

index 7d7bd0be6233e..3253eaefb18b3 100644

--- a/drivers/gpu/drm/xe/xe_gt_mcr.c

+++ b/drivers/gpu/drm/xe/xe_gt_mcr.c

@@ -5,6 +5,8 @@



 #include "xe_gt_mcr.h"



+#include <generated/xe_wa_oob.h>

+

 #include "regs/xe_gt_regs.h"

 #include "xe_assert.h"

 #include "xe_gt.h"

@@ -14,6 +16,7 @@

 #include "xe_guc_hwconfig.h"

 #include "xe_mmio.h"

 #include "xe_sriov.h"

+#include "xe_wa.h"



 /**

  * DOC: GT Multicast/Replicated (MCR) Register Support

@@ -245,6 +248,11 @@ static void init_steering_l3bank(struct xe_gt *gt)

         u32 bank_mask = REG_FIELD_GET(GT_L3_EXC_MASK,

                                      xe_mmio_read32(gt, XEHP_FUSE4));



+        /* Wa_14016747170 */

+        if (XE_WA(gt, 14016747170))

+                bank_mask = REG_FIELD_GET(MTL_GT_L3_EXC_MASK,

+                                         xe_mmio_read32(gt, MTL_GT_ACTIVITY_FACTOR));

+

Was there any particular reason for just adding an if(wa) and not using an if(wa)/else condition similar to i915 implementation?

CC: Shekhar Chauhan <shekhar.chauhan at intel.com><mailto:shekhar.chauhan at intel.com>



         /*

          * Group selects mslice, instance selects bank within mslice.

          * Bank 0 is always valid _except_ when the bank mask is 010b.

diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c

index 0662f71c6ede7..f1946ce0d937b 100644

--- a/drivers/gpu/drm/xe/xe_gt_topology.c

+++ b/drivers/gpu/drm/xe/xe_gt_topology.c

@@ -8,10 +8,13 @@

 #include <linux/bitmap.h>

 #include <linux/compiler.h>



+#include <generated/xe_wa_oob.h>

+

 #include "regs/xe_gt_regs.h"

 #include "xe_assert.h"

 #include "xe_gt.h"

 #include "xe_mmio.h"

+#include "xe_wa.h"



 static void

 load_dss_mask(struct xe_gt *gt, xe_dss_mask_t mask, int numregs, ...)

@@ -144,6 +147,12 @@ load_l3_bank_mask(struct xe_gt *gt, xe_l3_bank_mask_t l3_bank_mask)

         u32 fuse4 = xe_mmio_read32(gt, XEHP_FUSE4);

         u32 bank_val = REG_FIELD_GET(GT_L3_EXC_MASK, fuse4);



+        /* Wa_14016747170 */

+        if (XE_WA(gt, 14016747170)) {

+                fuse4 = xe_mmio_read32(gt, MTL_GT_ACTIVITY_FACTOR);

+                bank_val = REG_FIELD_GET(MTL_GT_L3_EXC_MASK, fuse4);

+        }

+

         bitmap_set_value8(per_mask_bit, 0x3, 0);

         gen_l3_mask_from_pattern(xe, per_node, per_mask_bit, 2, bank_val);

         gen_l3_mask_from_pattern(xe, l3_bank_mask, per_node, 4,

diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules

index 920ca50601466..5bac4123b5db1 100644

--- a/drivers/gpu/drm/xe/xe_wa_oob.rules

+++ b/drivers/gpu/drm/xe/xe_wa_oob.rules

@@ -37,3 +37,4 @@

 16023588340     GRAPHICS_VERSION(2001)

 14019789679     GRAPHICS_VERSION(1255)

         GRAPHICS_VERSION_RANGE(1270, 2004)

+14016747170     GRAPHICS_VERSION_RANGE(1270, 1271), GRAPHICS_STEP(A0, B0)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20240903/c657c1c4/attachment-0001.htm>