| Message ID | 20240711051339.3289552-1-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/xe/fbdev: Limit the usage of stolen for LNL+ | expand |
On Thu, Jul 11, 2024 at 10:43:39AM GMT, Uma Shankar wrote: >As per recommendation in the workarounds: >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. > >Signed-off-by: Uma Shankar <uma.shankar@intel.com> >--- > 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: not the proper way to handle WAs in xe. Please use XE_WA() >+ * 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 I wonder if we can't simply set to 0 the available stolen space after the places that really need it already had their allocation done. >+ */ >+ if (!IS_DGFX(xe) && (DISPLAY_VER(xe) < 20)) { extra parenthesis not needed and I think the rule to be added in xe_wa_oob.rules would be about GRAPHICS_VERSION(2004) or PLATFORM(LUNARLAKE) to avoid this propagating to future platforms. > 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; same as above Lucas De Marchi >+ > /* > * 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 >
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., - 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. Matt > assign the same from system memory. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > 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 >
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@intel.com> >> --- >> 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
On Thu, Jul 11, 2024 at 11:31:54AM -0500, Lucas De Marchi wrote: > On Thu, Jul 11, 2024 at 10:43:39AM GMT, Uma Shankar wrote: > >As per recommendation in the workarounds: > >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. > > > >Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >--- > > 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: > > not the proper way to handle WAs in xe. Please use XE_WA() > > > >+ * 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 > > I wonder if we can't simply set to 0 the available stolen space after the > places that really need it already had their allocation done. FBC needs stolen.
> -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Thursday, July 11, 2024 10:02 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; > ville.syrjala@linux.intel.com; Ceraolo Spurio, Daniele > <daniele.ceraolospurio@intel.com>; Belgaumkar, Vinay > <vinay.belgaumkar@intel.com> > Subject: Re: [PATCH] drm/xe/fbdev: Limit the usage of stolen for LNL+ > > On Thu, Jul 11, 2024 at 10:43:39AM GMT, Uma Shankar wrote: > >As per recommendation in the workarounds: > >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. > > > >Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >--- > > 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: > > not the proper way to handle WAs in xe. Please use XE_WA() Sure Lucas, will update the patch. > > >+ * 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 > > I wonder if we can't simply set to 0 the available stolen space after the places > that really need it already had their allocation done. We will need it for FBC, so making it all 0 will not be good. > >+ */ > >+ if (!IS_DGFX(xe) && (DISPLAY_VER(xe) < 20)) { > > extra parenthesis not needed and I think the rule to be added in xe_wa_oob.rules > would be about GRAPHICS_VERSION(2004) or > PLATFORM(LUNARLAKE) to avoid this propagating to future platforms. > Yeah got it, will change to GRPAHICS_VERSION and restrict it to LNL. > > 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; > > same as above > > Lucas De Marchi > > >+ > > /* > > * 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 > >
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Thursday, July 11, 2024 10:08 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; > ville.syrjala@linux.intel.com; Ceraolo Spurio, Daniele > <daniele.ceraolospurio@intel.com>; Belgaumkar, Vinay > <vinay.belgaumkar@intel.com> > 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@intel.com> > > --- > > 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
> -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Thursday, July 11, 2024 10:37 PM > To: Roper, Matthew D <matthew.d.roper@intel.com> > Cc: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; > intel-xe@lists.freedesktop.org; ville.syrjala@linux.intel.com; Ceraolo Spurio, > Daniele <daniele.ceraolospurio@intel.com>; Belgaumkar, Vinay > <vinay.belgaumkar@intel.com> > Subject: Re: [PATCH] drm/xe/fbdev: Limit the usage of stolen for LNL+ > > 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. Yeah right. > > 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. For DPT as per some discussions with windows team, it seems this should not harm as CPU accesses will be limited. Will move this out if we encounter any corner case. So far this seems to be ok as per the CI and local testing. Regards, Uma Shankar > > > > > - 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@intel.com> > >> --- > >> 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
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
As per recommendation in the workarounds: 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. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- 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(-)