On Thu, Jul 11, 2024 at 09:37:41AM GMT, Matt Roper wrote:
On Thu, Jul 11, 2024 at 10:43:39AM +0530, Uma Shankar wrote:
As per recommendation in the workarounds:
WA_14021987551, Wa_16023588340:
The first one here isn't a valid workaround lineage number, just a
per-platform ticket number. I think you meant Wa_22019338487, which is
the lineage number that can be used to track the applicability of the
workaround across all potentially relevant platform(s).
There is an issue with accessing Stolen memory pages due a
hardware limitation. Limit the usage of stolen memory for
fbdev for LNL+. Don't use BIOS FB from stolen on LNL+ and
From a quick skim of these workarounds I don't see a clear indication
that we need to avoid using stolen FB's? I thought these workarounds
were already being implemented separately (and aside from disabling FBC,
most of the necessary changes are on the GT side to adjust frequencies
and change caching behavior of LMEMBAR accesses). I.e.,
one part of Wa_22019338487 is what is implemented in
ggtt_update_access_counter(), because ggtt is in stolen. This can be
done for things under kernel-only control. For other things like the fb
we need to avoid them.
Also, while thinking about that... Did we check if we also need
something for DPT? AFAICS for LNL we will end up in
dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
ttm_bo_type_kernel,
XE_BO_FLAG_STOLEN |
XE_BO_FLAG_GGTT |
XE_BO_FLAG_PAGETABLE);
... and I don't see anything fencing the writes like we have in ggtt.
- Wa_16023588340: https://patchwork.freedesktop.org/series/135699/
- Wa_22019338487: https://patchwork.freedesktop.org/series/135460/
One other nitpick: we've been trying to avoid using "+" as shorthand
for "and beyond" lately since some of our IP names contain literal +'s
in their name (e.g., Xe_LPD+, Xe_LPM+, etc.). We don't want confusion
about whether "LNL+" refers to "LNL and beyond" (as you mean here) or
some other platform that's distinct from LNL.
But in general we usually don't treat workarounds as "forever" logic
unless they get written into the spec as new "official" programming. It
looks like these workarounds apply to LNL/BMG today, but we shouldn't
assume in advance that they'll automatically apply to platforms n+1,
n+2, etc. as well.
If we're making a concious decision to apply workaround programming to
more platforms than it's technically needed on (e.g., in cases where a
workaround doesn't have any negative impact, but applying it
unconditionally simplifies the driver logic), that should be called out
specifically in the commit message and comments to make it clear it
isn't a mistake. But I don't think that's the case here, since
otherwise we wouldn't be bothering with the DISPLAY_VER < 20 condition
either.
this is basically Wa_22019338487 and not exactly related with the
*display* version, hence my suggestion in the other reply to use XE_WA()
and tie it either to the platform or GRAPHICS_VERSION(2004)
Lucas De Marchi
Matt
assign the same from system memory.
Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
---
drivers/gpu/drm/xe/display/intel_fbdev_fb.c | 10 +++++++++-
drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 ++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
index 816ad13821a8..8fda8745ce0a 100644
--- a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
+++ b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
@@ -37,7 +37,14 @@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
size = PAGE_ALIGN(size);
obj = ERR_PTR(-ENODEV);
- if (!IS_DGFX(xe)) {
+ /*
+ * WA_14021987551, Wa_16023588340:
+ * There is an issue with accessing Stolen memory pages
+ * due a hardware limitation. Limit the usage of stolen
+ * memory for fbdev for LNL+. Don't use BIOS FB from
+ * stolen on LNL+ and assign the same from system memory
+ */
+ if (!IS_DGFX(xe) && (DISPLAY_VER(xe) < 20)) {
obj = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe),
NULL, size,
ttm_bo_type_kernel, XE_BO_FLAG_SCANOUT |
@@ -48,6 +55,7 @@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
else
drm_info(&xe->drm, "Allocated fbdev into stolen failed: %li\n", PTR_ERR(obj));
}
+
if (IS_ERR(obj)) {
obj = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL, size,
ttm_bo_type_kernel, XE_BO_FLAG_SCANOUT |
diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
index 5eccd6abb3ef..80dd6b64c921 100644
--- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
+++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
@@ -104,6 +104,16 @@ initial_plane_bo(struct xe_device *xe,
phys_base = base;
flags |= XE_BO_FLAG_STOLEN;
+ /*
+ * WA_14021987551, Wa_16023588340:
+ * There is an issue with accessing Stolen memory pages
+ * due a hardware limitation. Limit the usage of stolen
+ * memory for fbdev for LNL+. Don't use BIOS FB from
+ * stolen on LNL+ and assign the same from system memory
+ */
+ if (DISPLAY_VER(xe) >= 20)
+ return NULL;
+
/*
* If the FB is too big, just don't use it since fbdev is not very
* important and we should probably use that space with FBC or other
--
2.42.0
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
