Re: [Intel-gfx] [PATCH] drm/i915/mtl: Fix MTL stolen memory GGTT mapping

["Das, Nirmoy" <nirmoy.das@linux.intel.com>]

On 3/28/2023 3:24 AM, Daniele Ceraolo Spurio wrote:
> The PTEs expect the offset from the base of the fake LMEM region (i.e.
> the base of stolen) and not from the base of the DSM. Quoting the specs:
> "Driver will set the Device Memory bit = 1 in the PTE when pointing to a
> page in DSM and program the PTE with offset from LMEM_BAR. Device Memory
> Offset from LMEM_BAR is same as offset from BGSM."
>
> DSM starts 8MBs from BGSM, so we set dsm_base = 8MB.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@intel.com>
> Cc: Fei Yang <fei.yang@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>
> I've omitted the fixes tag from the commit message since MTL is still
> under force_probe, so there isn't really any need to propagate the fixes,
> but here it is for reference:
>
> Fixes: dbb2ffbfd708 ("drm/i915/mtl: enable local stolen memory")
>
>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index d8e06e783e30..8ac376c24aa2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -890,8 +890,9 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
>   	if (HAS_LMEMBAR_SMEM_STOLEN(i915)) {
>   		/*
>   		 * MTL dsm size is in GGC register.
> -		 * Also MTL uses offset to DSMBASE in ptes, so i915
> -		 * uses dsm_base = 0 to setup stolen region.
> +		 * Also MTL uses offset to GSMBASE in ptes, so i915
> +		 * uses dsm_base = 8MBs to setup stolen region, since
> +		 * DSMBASE = GSMBASE + 8MB.
>   		 */
>   		ret = mtl_get_gms_size(uncore);
>   		if (ret < 0) {
> @@ -899,11 +900,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
>   			return ERR_PTR(ret);
>   		}
>   
> -		dsm_base = 0;
> +		dsm_base = SZ_8M;
>   		dsm_size = (resource_size_t)(ret * SZ_1M);
>   
>   		GEM_BUG_ON(pci_resource_len(pdev, GEN12_LMEM_BAR) != SZ_256M);
> -		GEM_BUG_ON((dsm_size + SZ_8M) > lmem_size);
> +		GEM_BUG_ON((dsm_base + dsm_size) > lmem_size);
>   	} else {
>   		/* Use DSM base address instead for stolen memory */
>   		dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
> @@ -912,14 +913,12 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
>   		dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
>   	}
>   
> -	io_size = dsm_size;
> -	if (HAS_LMEMBAR_SMEM_STOLEN(i915)) {
> -		io_start = pci_resource_start(pdev, GEN12_LMEM_BAR) + SZ_8M;
> -	} else if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
> +	if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
>   		io_start = 0;
>   		io_size = 0;
>   	} else {
>   		io_start = pci_resource_start(pdev, GEN12_LMEM_BAR) + dsm_base;
> +		io_size = dsm_size;
>   	}
>   
>   	min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :