[Intel-gfx] [PATCH v2] drm/i915/mtl: Add Wa_14019821291

[Matt Roper]

On Fri, Oct 20, 2023 at 02:29:09PM +0530, Dnyaneshwar Bhadane wrote:
> This workaround is primarily implemented by the BIOS.  However if the
> BIOS applies the workaround it will reserve a small piece of our DSM
> (which should be at the top, right below the WOPCM); we just need to
> keep that region reserved so that nothing else attempts to re-use it.
> 
> v2: Declare regs in gt/intel_gt_regs.h file
> 
> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane at intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h    |  3 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 1a766d8e7cce..bb2639d1a824 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -409,6 +409,23 @@ static void icl_get_stolen_reserved(struct drm_i915_private *i915,
>  		*base -= *size;
>  	else
>  		*base = reg_val & GEN11_STOLEN_RESERVED_ADDR_MASK;
> +
> +	/* Wa_14019821291 */
> +	if (MEDIA_VER_FULL(i915) == IP_VER(13, 0)) {
> +		/*
> +		 * This workaround is primarily implemented by the BIOS.  We
> +		 * just need to figure out whether the BIOS has applied the
> +		 * workaround (meaning the programmed address falls within
> +		 * the DSM) and, if so, reserve that part of the DSM to
> +		 * prevent accidental reuse.  The DSM location should be just
> +		 * below the WOPCM.
> +		 */
> +		u64 gscpsmi_base = intel_uncore_read64_2x32(uncore,
> +							    MTL_GSCPSMI_BASEADDR_LSB,
> +							    MTL_GSCPSMI_BASEADDR_MSB);
> +		if (gscpsmi_base >= *base && gscpsmi_base < *base + *size)
> +			*size = gscpsmi_base - *base;
> +	}

Right now it looks like you re-calculate the size of the reserved region
to include the gscpsmi workaround space, but you don't update *base,
which is reserved_base in the caller.  That will cause the
i915->dsm.reserved resource to get initialized with the old base but the
new size (i.e., it will effectively grow in the wrong direction if you
don't change the base too).

I think the simplest thing to do is just move this workaround above the
if/else that comes right before it.  Since the affected platforms here
always take the 'if' branch, that will ensure that *base gets adjusted
downward to account for the larger *size value.


Matt

>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index eecd0a87a647..9de41703fae5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -537,6 +537,9 @@
>  #define XEHP_SQCM				MCR_REG(0x8724)
>  #define   EN_32B_ACCESS				REG_BIT(30)
>  
> +#define MTL_GSCPSMI_BASEADDR_LSB		_MMIO(0x880c)
> +#define MTL_GSCPSMI_BASEADDR_MSB		_MMIO(0x8810)
> +
>  #define HSW_IDICR				_MMIO(0x9008)
>  #define   IDIHASHMSK(x)				(((x) & 0x3f) << 16)
>  
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation