Re: [Intel-xe] [PATCH] drm/xe: Fix MTL+ stolen memory mapping

["Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>]

On 7/25/2023 9:43 AM, Lucas De Marchi wrote:
> Based on commit 8d8d062be6b9 ("drm/i915/mtl: Fix MTL stolen memory GGTT
> mapping"). For stolen on MTL and beyond, the address in the PTE is the
> offset from DSM base. While at it, update the comments explaining each
> part of the calculation.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

> ---
>
> Untested as stolen in MTL is supposedly to be disabled. Need to find a
> proper way to test it.

Is this disabling a recent change? I did try to use stolen on a tree 
from a couple of weeks back (for GSC loading) and there were no errors 
due to stolen being disabled, although things didn't initially work due 
to missing changes like this one. From what I was able to check/test, 
there is at least 2 more things that are required in addition to this patch:

1) update resource_is_stolen_vram to return true for MTL-style stolen 
LMEM, so that the LM bit is correctly set in the PTEs
2) implement support for removing the stolen reserved area from the 
allocator range (this seems to be missing for all platforms, not just MTL).

Daniele

>
>   drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> index 21ecc734f10a..271b3fba4129 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> @@ -94,11 +94,22 @@ static u32 detect_bar2_integrated(struct xe_device *xe, struct xe_ttm_stolen_mgr
>   
>   	ggc = xe_mmio_read32(xe_root_mmio_gt(xe), GGC);
>   
> -	/* check GGMS, should be fixed 0x3 (8MB) */
> +	/*
> +	 * Check GGMS: it should be fixed 0x3 (8MB), which corresponds to the
> +	 * GTT size
> +	 */
>   	if (drm_WARN_ON(&xe->drm, (ggc & GGMS_MASK) != GGMS_MASK))
>   		return 0;
>   
> -	mgr->stolen_base = mgr->io_base = pci_resource_start(pdev, 2) + SZ_8M;
> +	/*
> +	 * Graphics >= 1270 uses the offset to the GSMBASE as address in the
> +	 * PTEs, together with the DM flag being set. Previously there was no
> +	 * such flag so the address was the io_base.
> +	 *
> +	 * DSMBASE = GSMBASE + 8MB
> +	 */
> +	mgr->stolen_base = SZ_8M;
> +	mgr->io_base = pci_resource_start(pdev, 2) + mgr->stolen_base;
>   
>   	/* return valid GMS value, -EIO if invalid */
>   	gms = REG_FIELD_GET(GMS_MASK, ggc);