Re: [Intel-gfx] [PATCH 7/8] drm/i915: fixup the initial fb base on DG1

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 04, 2022 at 05:23:32PM +0000, Matthew Auld wrote:
> The offset we get looks to be the exact start of DSM, but the
> inital_plane_vma expects the address to be relative.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  .../drm/i915/display/intel_plane_initial.c    | 22 +++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index f797fcef18fc..b39d3a8dfe45 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -56,10 +56,24 @@ initial_plane_vma(struct drm_i915_private *i915,
>  	if (!mem || plane_config->size == 0)
>  		return NULL;
>  
> -	base = round_down(plane_config->base,
> -			  I915_GTT_MIN_ALIGNMENT);
> -	size = round_up(plane_config->base + plane_config->size,
> -			mem->min_page_size);
> +	base = plane_config->base;
> +	if (IS_DGFX(i915)) {
> +		/*
> +		 * On discrete the base address should be somewhere in LMEM, but
> +		 * depending on the size of LMEM the base address might
> +		 * intersect with the start of DSM, like on DG1, in which case
> +		 * we need the relative address. In such cases we might also
> +		 * need to choose between inital fb vs fbc, if space is limited.
> +		 *
> +		 * On future discrete HW, like DG2, we should be able to just
> +		 * allocate directly from LMEM, due to larger LMEM size.
> +		 */
> +		if (base >= i915->dsm.start)
> +			base -= i915->dsm.start;

Subsequent code expects the object to actually be inside stolen.
If that is not the case we should just give up.

The fact that we fail to confirm any of that on integrated
parts has always bugged me, but not enough to actually do
anything about it. Such a check would be somewhat more involved
since we'd have to look at the PTEs. But on discrete sounds like
we can get away with a trivial check.

> +	}
> +
> +	size = roundup(base + plane_config->size, mem->min_page_size);
> +	base = round_down(base, I915_GTT_MIN_ALIGNMENT);
>  	size -= base;
>  
>  	/*
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel