[Intel-xe] [PATCH v2 3/3] drm/xe: Add Wa_14019821291

Gustavo Sousa gustavo.sousa at intel.com
Thu Oct 26 14:10:14 UTC 2023


Quoting Lucas De Marchi (2023-10-24 19:07:39-03:00)
>From: Matt Roper <matthew.d.roper at intel.com>
>
>This workaround is primarily implemented by the BIOS.  However if the
>BIOS applies the workaround it will reserve a small piece of our DSM
>(which should be at the top, right below the WOPCM); we just need to
>keep that region reserved so that nothing else attempts to re-use it.
>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>---
> drivers/gpu/drm/xe/Makefile            |  2 +-
> drivers/gpu/drm/xe/regs/xe_gt_regs.h   |  2 ++
> drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_wa_oob.rules     |  1 +
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index cee57681732d..ef9d954f5a75 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -37,7 +37,7 @@ quiet_cmd_wa_oob = GEN     $(notdir $(generated_oob))
> $(generated_oob) &: $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
>         $(call cmd,wa_oob)
> 
>-$(obj)/xe_guc.o $(obj)/xe_migrate.o $(obj)/xe_ring_ops.o $(obj)/xe_vm.o $(obj)/xe_wa.o: $(generated_oob)
>+$(obj)/xe_guc.o $(obj)/xe_migrate.o $(obj)/xe_ring_ops.o $(obj)/xe_vm.o $(obj)/xe_wa.o $(obj)/xe_ttm_stolen_mgr.o: $(generated_oob)

Nitpick: It seems the list of targets is sorted. Maybe we should keep it
that way.

On an unrelated note, I wonder if there is a way to have the dependency
on the header automatically detected.

> 
> # Please keep these build lists sorted!
> 
>diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>index 55ceadfc30b0..d542a4e2e6fa 100644
>--- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>@@ -155,6 +155,8 @@
> #define XEHP_SQCM                                XE_REG_MCR(0x8724)
> #define   EN_32B_ACCESS                                REG_BIT(30)
> 
>+#define GSCPSMI_BASE                                XE_REG(0x880c)
>+
> #define        MIRROR_FUSE3                                XE_REG(0x9118)
> #define   XE2_NODE_ENABLE_MASK                        REG_GENMASK(31, 16)
> #define   L3BANK_PAIR_COUNT                        4
>diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>index 79fbd74a3944..b23827534ab9 100644
>--- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>+++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>@@ -11,6 +11,8 @@
> #include <drm/ttm/ttm_placement.h>
> #include <drm/ttm/ttm_range_manager.h>
> 
>+#include "generated/xe_wa_oob.h"
>+#include "regs/xe_gt_regs.h"
> #include "regs/xe_regs.h"
> #include "xe_bo.h"
> #include "xe_device.h"
>@@ -19,6 +21,7 @@
> #include "xe_res_cursor.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_vram_mgr.h"
>+#include "xe_wa.h"
> 
> struct xe_ttm_stolen_mgr {
>         struct xe_ttm_vram_mgr base;
>@@ -112,6 +115,7 @@ static u32 get_wopcm_size(struct xe_device *xe)
> static u32 detect_bar2_integrated(struct xe_device *xe, struct xe_ttm_stolen_mgr *mgr)
> {
>         struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>+        struct xe_gt *media_gt = xe_device_get_root_tile(xe)->media_gt;
>         u32 stolen_size, wopcm_size;
>         u32 ggc, gms;
> 
>@@ -154,6 +158,26 @@ static u32 detect_bar2_integrated(struct xe_device *xe, struct xe_ttm_stolen_mgr
> 
>         stolen_size -= wopcm_size;
> 
>+        if (XE_WA(media_gt, 14019821291)) {

media_gt could be NULL here.

>+                u64 gscpsmi_base = xe_mmio_read64_2x32(media_gt, GSCPSMI_BASE);

By looking at the description for this register in the spec, it seems
that only bits [5:0] contain bits that are not really part of the
address. Shouldn't we take this into consideration?

In that case, I'm not sure how we should interpret the bits [31:6]. Are
they the really lower part of the address (requiring "gscpsmi_base > 6")?
Or do they map directly to the bits of the address and we should
consider the lower [5:0] as zeros (requiring "gscpsmi_base & ~0x3f")?

The description for the BIOS workaround makes me think that it is the
latter.

--
Gustavo Sousa

>+
>+                /*
>+                 * This workaround is primarily implemented by the BIOS.  We
>+                 * just need to figure out whether the BIOS has applied the
>+                 * workaround (meaning the programmed address falls within
>+                 * the DSM) and, if so, reserve that part of the DSM to
>+                 * prevent accidental reuse.  The DSM location should be just
>+                 * below the WOPCM.
>+                 */
>+                if (gscpsmi_base >= mgr->io_base &&
>+                    gscpsmi_base < mgr->io_base + stolen_size) {
>+                        xe_gt_dbg(media_gt,
>+                                  "Reserving %llu bytes of DSM for Wa_14019821291\n",
>+                                  mgr->io_base + stolen_size - gscpsmi_base);
>+                        stolen_size = gscpsmi_base - mgr->io_base;
>+                }
>+        }
>+
>         if (drm_WARN_ON(&xe->drm, stolen_size + SZ_8M > pci_resource_len(pdev, 2)))
>                 return 0;
> 
>diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
>index f3ff774dc4aa..752842d734be 100644
>--- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>+++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>@@ -19,3 +19,4 @@
>                 SUBPLATFORM(DG2, G12)
> 16017236439        PLATFORM(PVC)
> 22010954014        PLATFORM(DG2)
>+14019821291        MEDIA_VERSION_RANGE(1300, 2000)
>-- 
>2.40.1
>


More information about the Intel-xe mailing list