diff mbox series

[v7,4/4] drm/i915/mtl: Skip MCR ops for ring fault register

Message ID 20230928130015.6758-4-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series [v7,1/4] drm/i915: Introduce intel_gt_mcr_lock_sanitize() | expand

Commit Message

Das, Nirmoy Sept. 28, 2023, 1 p.m. UTC
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@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@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(-)

Comments

Andrzej Hajda Sept. 28, 2023, 10:14 p.m. UTC | #1
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@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@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.

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 Sept. 28, 2023, 10:33 p.m. UTC | #2
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@intel.com>
> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@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)
>
diff mbox series

Patch

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)) {
 		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)