[Intel-xe] [PATCH v2 3/3] drm/xe: Add Wa_14019821291
Matt Roper
matthew.d.roper at intel.com
Thu Oct 26 16:29:09 UTC 2023
On Thu, Oct 26, 2023 at 11:10:14AM -0300, Gustavo Sousa wrote:
> 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")?
This is a common pattern in the hardware --- when an address is
guaranteed to be page-aligned, they'll make reg[31:6] = addr[31:6] and
then potentially utilize the lowest 6 bits (which must be zero in the
address) to hold flags. Usually the bspec has a "Format:" label on most
registers which makes it more clear when this is happening, but that
seems to be missing on this bspec page.
In this case, bits 5:1 of the register are reserved and guaranteed to
be 0 on all current platforms. Bit 0 is the register's lock bit, which
the BIOS is supposed to leave at 0 if they're implementing this
workaround. But it still wouldn't be a bad idea to mask off those lower
6 bits just to be safe, in case this workaround winds up carrying
forward to future platforms where the situation might be different.
Matt
>
> 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
> >
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list