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

[Matt Roper]

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 at 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