> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>
> Sent: Monday, July 15, 2024 6:53 PM
> To: Shankar, Uma <uma.shankar@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx;
> ville.syrjala@xxxxxxxxxxxxxxx; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@xxxxxxxxx>; Belgaumkar, Vinay
> <vinay.belgaumkar@xxxxxxxxx>; Roper, Matthew D
> <matthew.d.roper@xxxxxxxxx>
> 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@xxxxxxxxx>
> >---
> > 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
> >
