[Intel-gfx] [PATCH v7 4/4] drm/i915/mtl: Skip MCR ops for ring fault register

[Matt Roper]

On Fri, Sep 29, 2023 at 12:14:37AM +0200, Andrzej Hajda wrote:
> On 28.09.2023 15:00, Nirmoy Das wrote:
> > On MTL GEN12_RING_FAULT_REG is not replicated so don't
> > do mcr based operation for this register.
> > 
> > v2: use MEDIA_VER() instead of GRAPHICS_VER()(Matt).
> > v3: s/"MEDIA_VER(i915) == 13"/"MEDIA_VER(i915) >= 13"(Matt)
> >      improve comment.
> > v4: improve the comment further(Andi)
> > 
> > Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
> > Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> > Reviewed-by: Andi Shyti <andi.shyti at linux.intel.com>
> > Reviewed-by: Andrzej Hajda <andrzej.hajda at intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_gt.c      | 13 ++++++++++++-
> >   drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
> >   drivers/gpu/drm/i915/i915_gpu_error.c   | 11 ++++++++++-
> >   3 files changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index 93062c35e072..dff8bba1f5d4 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -262,10 +262,21 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
> >   				   I915_MASTER_ERROR_INTERRUPT);
> >   	}
> > -	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> > +	/*
> > +	 * For the media GT, this ring fault register is not replicated,
> > +	 * so don't do multicast/replicated register read/write operation on it.
> > +	 */
> > +	if (MEDIA_VER(i915) >= 13 && gt->type == GT_MEDIA) {
> > +		intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
> > +				 RING_FAULT_VALID, 0);
> > +		intel_uncore_posting_read(uncore,
> > +					  XELPMP_RING_FAULT_REG);
> > +
> > +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> 
> WA 14017387313 suggests to "remove Semaphore acquisition steps for all GAM
> ranges" (XELPMP_RING_FAULT_REG is in GAM range), just FYI.

We've actually looked at that workaround before and decided that it
doesn't make sense to implement it on Linux.  The background for that
workaround is due to Windows driver design; their driver potentially
tries to access some MCR registers from within an interrupt handler,
which would cause problems if non-IRQ code grabs the semaphore, gets
interrupted, and then the interrupt handler deadlocks while also trying
to acquire it.  On Linux, we never access MCR registers from an
interrupt handler, so we're not susceptible to that issue.


Matt

> 
> Regards
> Andrzej
> 
> 
> >   		intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
> >   					   RING_FAULT_VALID, 0);
> >   		intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
> > +
> >   	} else if (GRAPHICS_VER(i915) >= 12) {
> >   		intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG, RING_FAULT_VALID, 0);
> >   		intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index cca4bac8f8b0..eecd0a87a647 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1084,6 +1084,7 @@
> >   #define GEN12_RING_FAULT_REG			_MMIO(0xcec4)
> >   #define XEHP_RING_FAULT_REG			MCR_REG(0xcec4)
> > +#define XELPMP_RING_FAULT_REG			_MMIO(0xcec4)
> >   #define   GEN8_RING_FAULT_ENGINE_ID(x)		(((x) >> 12) & 0x7)
> >   #define   RING_FAULT_GTTSEL_MASK		(1 << 11)
> >   #define   RING_FAULT_SRCID(x)			(((x) >> 3) & 0xff)
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index f4ebcfb70289..b4e31e59c799 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1234,7 +1234,16 @@ static void engine_record_registers(struct intel_engine_coredump *ee)
> >   	if (GRAPHICS_VER(i915) >= 6) {
> >   		ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
> > -		if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> > +		/*
> > +		 * For the media GT, this ring fault register is not replicated,
> > +		 * so don't do multicast/replicated register read/write
> > +		 * operation on it.
> > +		 */
> > +		if (MEDIA_VER(i915) >= 13 && engine->gt->type == GT_MEDIA)
> > +			ee->fault_reg = intel_uncore_read(engine->uncore,
> > +							  XELPMP_RING_FAULT_REG);
> > +
> > +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> >   			ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
> >   							      XEHP_RING_FAULT_REG);
> >   		else if (GRAPHICS_VER(i915) >= 12)
> 

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