Re: [PATCH 6/7] drm/i915: Enable pipeDMC fault interrupts on tgl+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jun 06, 2024 at 02:43:12PM +0300, Jani Nikula wrote:
> On Wed, 05 Jun 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > PipeDMC has its own fault interrupt. Enable that so that
> > we can know if things are failing.
> >
> > While at it, define the other pipeDMC interrupt as well, even
> > though we're not currently using it.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_irq.c | 15 +++++++++++++--
> >  drivers/gpu/drm/i915/i915_reg.h                  |  2 ++
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > index 219f7693207d..6eb14ffc3c1b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > @@ -836,13 +836,24 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
> >  static u32 gen8_de_pipe_fault_mask(struct drm_i915_private *dev_priv)
> >  {
> >  	if (DISPLAY_VER(dev_priv) >= 13 || HAS_D12_PLANE_MINIMIZATION(dev_priv))
> > -		return GEN9_PIPE_CURSOR_FAULT |
> > +		return GEN12_PIPEDMC_FAULT |
> > +			GEN9_PIPE_CURSOR_FAULT |
> >  			GEN11_PIPE_PLANE5_FAULT |
> >  			GEN9_PIPE_PLANE4_FAULT |
> >  			GEN9_PIPE_PLANE3_FAULT |
> >  			GEN9_PIPE_PLANE2_FAULT |
> >  			GEN9_PIPE_PLANE1_FAULT;
> > -	else if (DISPLAY_VER(dev_priv) >= 11)
> > +	else if (DISPLAY_VER(dev_priv) == 12)
> > +		return GEN12_PIPEDMC_FAULT |
> > +			GEN9_PIPE_CURSOR_FAULT |
> > +			GEN11_PIPE_PLANE7_FAULT |
> > +			GEN11_PIPE_PLANE6_FAULT |
> > +			GEN11_PIPE_PLANE5_FAULT |
> > +			GEN9_PIPE_PLANE4_FAULT |
> > +			GEN9_PIPE_PLANE3_FAULT |
> > +			GEN9_PIPE_PLANE2_FAULT |
> > +			GEN9_PIPE_PLANE1_FAULT;
> > +	else if (DISPLAY_VER(dev_priv) == 11)
> >  		return GEN9_PIPE_CURSOR_FAULT |
> >  			GEN11_PIPE_PLANE7_FAULT |
> >  			GEN11_PIPE_PLANE6_FAULT |
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index b85e12bb4781..8cd5abc52a2d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2625,6 +2625,8 @@
> >  #define  GEN8_PIPE_FIFO_UNDERRUN	REG_BIT(31)
> >  #define  GEN8_PIPE_CDCLK_CRC_ERROR	REG_BIT(29)
> >  #define  GEN8_PIPE_CDCLK_CRC_DONE	REG_BIT(28)
> > +#define  GEN12_PIPEDMC_INTERRUPT	REG_BIT(26) /* tgl+ */
> > +#define  GEN12_PIPEDMC_FAULT		REG_BIT(25) /* tgl+ */
> 
> Marked as pipedmc gtt fault in the spec.

I did ponder about adding _GTT to all the fault bits, but it looks
like at least the plane fault bits are supposed to be set for both
GTT and ATS faults (despite still being named foo_gtt_fault in the
spec). It's not clear whether that same behaviour should hold for
other hardware units as well. So I decided to stick with the current
"no _GTT in the name" for all the non-ATS fault bits.

> 
> There's also pipe dmc error at bit 17... but the description says
> "simple flp queue done interrupt"... *sigh*

Yeah, that'll need more digging at some point.

-- 
Ville Syrjälä
Intel