[v2] drm/xe/fbdev: Limit the usage of stolen for LNL+

Shankar, Uma uma.shankar at intel.com
Mon Jul 15 18:47:33 UTC 2024



> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> Sent: Monday, July 15, 2024 6:53 PM
> To: Shankar, Uma <uma.shankar at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org;
> ville.syrjala at linux.intel.com; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio at intel.com>; Belgaumkar, Vinay
> <vinay.belgaumkar at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Subject: Re: [v2] drm/xe/fbdev: Limit the usage of stolen for LNL+
> 
> On Mon, Jul 15, 2024 at 02:26:59AM GMT, Uma Shankar wrote:
> >As per recommendation in the workarounds:
> >WA_22019338487
> >
> >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.
> >
> >v2: Corrected the WA Number, limited WA to LNL and
> >    Adopted XE_WA framework as suggested by Lucas and Matt.
> >
> >Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> >---
> > drivers/gpu/drm/xe/display/intel_fbdev_fb.c   | 20 ++++++++++++++++++-
> > drivers/gpu/drm/xe/display/xe_plane_initial.c | 12 +++++++++++
> > drivers/gpu/drm/xe/xe_wa_oob.rules            |  1 +
> > 3 files changed, 32 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..9c70c9158108 100644
> >--- a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> >+++ b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> >@@ -10,6 +10,8 @@
> > #include "xe_bo.h"
> > #include "xe_gt.h"
> > #include "xe_ttm_stolen_mgr.h"
> >+#include "xe_wa.h"
> 
> missing newline
> 
> >+#include <generated/xe_wa_oob.h>
> >
> > struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > 					       struct drm_fb_helper_surface_size
> *sizes) @@ -20,6 +22,9
> >@@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper
> *helper,
> > 	struct drm_mode_fb_cmd2 mode_cmd = {};
> > 	struct drm_i915_gem_object *obj;
> > 	int size;
> >+	bool wa_22019338487 = false;
> >+	struct xe_gt *gt;
> >+	u8 id;
> >
> > 	/* we don't do packed 24bpp */
> > 	if (sizes->surface_bpp == 24)
> >@@ -37,7 +42,19 @@ 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_22019338487:
> >+	 * 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
> >+	 */
> >+	for_each_gt(gt, xe, id) {
> 
> why do you loop here, but in the other path you use main_gt of tile0?
> 
> I think at this point it's pretty safe to just do:
> 
> 	if (XE_WA(xe_root_mmio_gt(xe), 22019338487))

Yeah this sound a better choice here. Will drop the for loop and use this.
Thanks for pointing out.

> Also, no need for the comment above, the commit message and WA
> documentation is sufficient.

Sure, will do.
 
> >+		if (XE_WA(gt, 22019338487))
> >+			wa_22019338487 = true;
> >+	}
> >+
> >+	if (!IS_DGFX(xe) && !wa_22019338487) {
> > 		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 +65,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..7e93ddad6df8 100644
> >--- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
> >+++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> >@@ -18,6 +18,8 @@
> > #include "intel_frontbuffer.h"
> > #include "intel_plane_initial.h"
> > #include "xe_bo.h"
> >+#include "xe_wa.h"
> >+#include <generated/xe_wa_oob.h>
> >
> > static bool
> > intel_reuse_initial_plane_obj(struct intel_crtc *this, @@ -104,6
> >+106,16 @@ initial_plane_bo(struct xe_device *xe,
> > 		phys_base = base;
> > 		flags |= XE_BO_FLAG_STOLEN;
> >
> >+		/*
> >+		 * WA_22019338487:
> >+		 * 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 (XE_WA(tile0->primary_gt, 22019338487))
> 
> just use the same xe_root_mmio_gt() as suggested above.
>
> >+			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
> >diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules
> >b/drivers/gpu/drm/xe/xe_wa_oob.rules
> >index 08f7336881e3..9b08fedbf85c 100644
> >--- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> >+++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> >@@ -29,4 +29,5 @@
> > 13011645652	GRAPHICS_VERSION(2004)
> > 22019338487	MEDIA_VERSION(2000)
> > 		GRAPHICS_VERSION(2001)
> >+		GRAPHICS_VERSION(2004)
> 
> This will end up matching the graphics GT in LNL for other paths it was not
> previously taking. Looking at the code, main change will be:
> 
> drivers/gpu/drm/xe/xe_guc_pc.c:pc_max_freq_cap()
> drivers/gpu/drm/xe/xe_gt.c:xe_gt_sanitize_freq()
> 
> about the freq handling for the GT. And the change will be wrong
> 
> I think we could just make this a new entry with:
> 
> 	22019338487_display GRAPHICS_VERSION(2024) or
> 	22019338487_display PLATFORM(LUNARLAKE)
> 
> I like the second more as then it doesn't matter what gt you use in the code. Matt
> Roper, thoughts?

Thanks Lucas, for pointing this out. Yes indeed it can cause that. I will adopt your 2nd option
and send a new version.

Thanks for all the valuable feedback and suggestions.

Regards,
Uma Shankar
 
> Lucas De Marchi
> 
> > 16023588340	GRAPHICS_VERSION(2001)
> >--
> >2.42.0
> >


More information about the Intel-gfx mailing list