> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > Sent: Thursday, July 11, 2024 10:08 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> > Subject: Re: [PATCH] drm/xe/fbdev: Limit the usage of stolen for LNL+ > > 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). It was showing up as the WA with wa_status moved to pending_dem_decision so used that number. But sure, will update the same to 22019338487 to get it correct. > > > > 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., > > - Wa_16023588340: https://patchwork.freedesktop.org/series/135699/ > - Wa_22019338487: https://patchwork.freedesktop.org/series/135460/ There is a restriction of accessing stolen with CPU also be able to access it. So adding the change to avoid this situation. > 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. Thanks for clarifying Matt, will update the change and restrict it to GRAPHICS_VERSION 2004. > > 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
