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.bhad...@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