[Intel-gfx] [PATCH v3 3/5] drm/i915/mtl: make IRQ reset and postinstall multi-gt aware

[Sripada, Radhakrishna]

+Daniele,

Hi Matt,

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Monday, March 6, 2023 2:55 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; De Marchi, Lucas
> <lucas.demarchi at intel.com>; Zanoni, Paulo R <paulo.r.zanoni at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v3 3/5] drm/i915/mtl: make IRQ reset and
> postinstall multi-gt aware
> 
> On Wed, Mar 01, 2023 at 12:10:51PM -0800, Radhakrishna Sripada wrote:
> > Irq reset and post install are to be made multi-gt aware for the
> > interrupts to work for the media tile on Meteorlake. Iterate through
> > all the gts to process irq reset for each gt.
> 
> I think I mentioned on the previous version, but this isn't right.  MTL
> does not have separate interrupt registers for each GT the way
> multi-tile platforms like PVC do.  The GT interrupt registers you're
> handling here are in the sgunit so there's only a single copy of them;
> iterating over them multiple times in a row doesn't accomplish anything.
> 
> The media engine bits are still on the same registers as the primary GT
> and the GSC and media GuC are new additional bits that need to be
> handled.  The necessary handling for the GSC and media GuC should have
> already landed in a187f13d51fa ("drm/i915/guc: handle interrupts from
> media GuC") and c07ee636901d ("drm/i915/mtl: add GSC CS interrupt
> support"), but if there's another bit that was missed somewhere (or if
> we were doing something like looking at the wrong intel_gt's engine mask
> somewhere), that would need to be addressed directly rather than just
> looping over the same IRQ registers multiple times.

This patch is needed to handle media interrupts. Without this patch we observed
GuC not loading/communication errors on media gt.

My understanding is that Sgunit is embedded into the SAMedia block and hence need
To be iterated over separately.

Daniele,
Can you confirm if that is accurate.

Thanks,
RK
> 
> 
> Matt
> 
> >
> > Based on original version by Paulo and Tvrtko
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 30 ++++++++++++++++++------------
> >  1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 417c981e4968..9377f59c1ac2 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2759,16 +2759,19 @@ static void gen11_irq_reset(struct
> drm_i915_private *dev_priv)
> >
> >  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;
> > +	struct intel_gt *gt;
> > +	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_);
> > +		GEN3_IRQ_RESET(gt->uncore, GEN11_GU_MISC_);
> > +		GEN3_IRQ_RESET(gt->uncore, GEN8_PCU_);
> > +	}
> > +
> > +	gen11_display_irq_reset(dev_priv);
> >  }
> >
> >  void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> > @@ -3422,13 +3425,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);
> > @@ -3437,8 +3443,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.34.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation