diff mbox series

drm/i915: Make IRQ reset and postinstall multi-gt aware

Message ID 20230321232009.541585-1-andi.shyti@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Make IRQ reset and postinstall multi-gt aware | expand

Commit Message

Andi Shyti March 21, 2023, 11:20 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

In multitile systems IRQ need to be reset and enabled per GT.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Comments

Matt Roper March 22, 2023, 12:10 a.m. UTC | #1
On Wed, Mar 22, 2023 at 12:20:09AM +0100, Andi Shyti wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> In multitile systems IRQ need to be reset and enabled per GT.

At the moment we're not enabling multi-tile support on any platforms
yet.  Xe_HP SDV has pretty much already served its purpose as an early
Xe_HP test platform, and most PVC effort is refocusing on the Xe KMD
right now.

Note that we don't want/need changes like this on non-tile multi-gt
platforms like MTL.  The interrupt registers you're accessing here are
sgunit registers so there's only ever a single copy of the register on
such platforms; looping around and processing the same register two
times in a row doesn't accomplish anything that just processing them a
single time doesn't.


Matt

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 31271c30a8cf4..ee4530ec14de3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2762,14 +2762,19 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_gt *gt = to_gt(dev_priv);
>  	struct intel_uncore *uncore = gt->uncore;
> +	unsigned int i;
>  
>  	dg1_master_intr_disable(dev_priv->uncore.regs);
>  
> -	gen11_gt_irq_reset(gt);
> -	gen11_display_irq_reset(dev_priv);
> +	for_each_gt(gt, dev_priv, i) {
> +		gen11_gt_irq_reset(gt);
>  
> -	GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> -	GEN3_IRQ_RESET(uncore, GEN8_PCU_);
> +		uncore = gt->uncore;
> +		GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> +		GEN3_IRQ_RESET(uncore, GEN8_PCU_);
> +	}
> +
> +	gen11_display_irq_reset(dev_priv);
>  }
>  
>  void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> @@ -3423,13 +3428,16 @@ static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
>  
>  static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_gt *gt = to_gt(dev_priv);
> -	struct intel_uncore *uncore = gt->uncore;
>  	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
> +	struct intel_gt *gt;
> +	unsigned int i;
>  
> -	gen11_gt_irq_postinstall(gt);
> +	for_each_gt(gt, dev_priv, i) {
> +		gen11_gt_irq_postinstall(gt);
>  
> -	GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
> +		GEN3_IRQ_INIT(gt->uncore, GEN11_GU_MISC_, ~gu_misc_masked,
> +			      gu_misc_masked);
> +	}
>  
>  	if (HAS_DISPLAY(dev_priv)) {
>  		icp_irq_postinstall(dev_priv);
> @@ -3438,8 +3446,8 @@ static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
>  				   GEN11_DISPLAY_IRQ_ENABLE);
>  	}
>  
> -	dg1_master_intr_enable(uncore->regs);
> -	intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
> +	dg1_master_intr_enable(to_gt(dev_priv)->uncore->regs);
> +	intel_uncore_posting_read(to_gt(dev_priv)->uncore, DG1_MSTR_TILE_INTR);
>  }
>  
>  static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
> -- 
> 2.39.2
>
Andi Shyti March 22, 2023, 12:27 a.m. UTC | #2
Hi Matt,

On Tue, Mar 21, 2023 at 05:10:51PM -0700, Matt Roper wrote:
> On Wed, Mar 22, 2023 at 12:20:09AM +0100, Andi Shyti wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > In multitile systems IRQ need to be reset and enabled per GT.
> 
> At the moment we're not enabling multi-tile support on any platforms
> yet.  Xe_HP SDV has pretty much already served its purpose as an early
> Xe_HP test platform, and most PVC effort is refocusing on the Xe KMD
> right now.
> 
> Note that we don't want/need changes like this on non-tile multi-gt
> platforms like MTL.  The interrupt registers you're accessing here are
> sgunit registers so there's only ever a single copy of the register on
> such platforms; looping around and processing the same register two
> times in a row doesn't accomplish anything that just processing them a
> single time doesn't.

Right... irq's registers in MTL are in the root tile.

However, In a multi-gt point of view all the "gt" functions need
to be iterated over all the GT's... maybe to have things cleaner
we might need some dedicated mtl_irq_reset and
mtl_irq_postinstall wrappers.

Thanks! (again :-))

Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 31271c30a8cf4..ee4530ec14de3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2762,14 +2762,19 @@  static void dg1_irq_reset(struct drm_i915_private *dev_priv)
 {
 	struct intel_gt *gt = to_gt(dev_priv);
 	struct intel_uncore *uncore = gt->uncore;
+	unsigned int i;
 
 	dg1_master_intr_disable(dev_priv->uncore.regs);
 
-	gen11_gt_irq_reset(gt);
-	gen11_display_irq_reset(dev_priv);
+	for_each_gt(gt, dev_priv, i) {
+		gen11_gt_irq_reset(gt);
 
-	GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
-	GEN3_IRQ_RESET(uncore, GEN8_PCU_);
+		uncore = gt->uncore;
+		GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
+		GEN3_IRQ_RESET(uncore, GEN8_PCU_);
+	}
+
+	gen11_display_irq_reset(dev_priv);
 }
 
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
@@ -3423,13 +3428,16 @@  static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
 
 static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct intel_gt *gt = to_gt(dev_priv);
-	struct intel_uncore *uncore = gt->uncore;
 	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
+	struct intel_gt *gt;
+	unsigned int i;
 
-	gen11_gt_irq_postinstall(gt);
+	for_each_gt(gt, dev_priv, i) {
+		gen11_gt_irq_postinstall(gt);
 
-	GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
+		GEN3_IRQ_INIT(gt->uncore, GEN11_GU_MISC_, ~gu_misc_masked,
+			      gu_misc_masked);
+	}
 
 	if (HAS_DISPLAY(dev_priv)) {
 		icp_irq_postinstall(dev_priv);
@@ -3438,8 +3446,8 @@  static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
 				   GEN11_DISPLAY_IRQ_ENABLE);
 	}
 
-	dg1_master_intr_enable(uncore->regs);
-	intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
+	dg1_master_intr_enable(to_gt(dev_priv)->uncore->regs);
+	intel_uncore_posting_read(to_gt(dev_priv)->uncore, DG1_MSTR_TILE_INTR);
 }
 
 static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)