[Intel-gfx] [PATCH] drm/i915: Respect ring_mask and num_pipes when install IRQ

[Wang, Elaine]

> On Thu, Dec 08, 2016 at 06:46:49PM +0800, Wang Elaine wrote:
> > From: Elaine Wang <elaine.wang at intel.com>
> >
> > Some platforms only have VCS ring in VDBox. To avoid accessing the
> > non-existent rings or display registers, check the ring_mask and
> > num_pipes in gen8 IRQ install and reset functions.
> >
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Signed-off-by: Elaine Wang <elaine.wang at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++------
> >  1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c index 0b119b9..3b3ed22 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2990,8 +2990,10 @@ static void gen8_irq_reset(struct drm_device
> *dev)
> >
> POWER_DOMAIN_PIPE(pipe)))
> >  			GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> >
> > -	GEN5_IRQ_RESET(GEN8_DE_PORT_);
> > -	GEN5_IRQ_RESET(GEN8_DE_MISC_);
> > +	if (INTEL_INFO(dev_priv)->num_pipes) {
> > +		GEN5_IRQ_RESET(GEN8_DE_PORT_);
> > +		GEN5_IRQ_RESET(GEN8_DE_MISC_);
> > +	}
> >  	GEN5_IRQ_RESET(GEN8_PCU_);
> >
> >  	if (HAS_PCH_SPLIT(dev_priv))
> > @@ -3351,14 +3353,20 @@ static void gen8_gt_irq_postinstall(struct
> > drm_i915_private *dev_priv)
> >
> >  	dev_priv->pm_ier = 0x0;
> >  	dev_priv->pm_imr = ~dev_priv->pm_ier;
> > -	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> > -	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> > +
> > +	if (HAS_ENGINE(dev_priv, RCS) || HAS_ENGINE(dev_priv, BCS))
> > +		GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0],
> gt_interrupts[0]);
> > +
> > +	if (HAS_ENGINE(dev_priv, VCS))
> > +		GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1],
> gt_interrupts[1]);
> 
> Hmm. None of these registers should live in the ring register space, so I'm
> not sure why we would have to do this. Would the non-existing ring
> somehow falsely report interrupts?
> 
> It might make sense to only set/clear the bits when the engine exits purely
> from a consistency POV, but in that case the RCS||BCS thing you have going
> here doesn't really make sense.
You're right. For the platforms that don't have RCS, BCS or VECS, the interrupt registers
still exist.  No harm to program to program them. I'll revise the patch.
> 
> >  	/*
> >  	 * RPS interrupts will get enabled/disabled on demand when RPS
> itself
> >  	 * is enabled/disabled. Same wil be the case for GuC interrupts.
> >  	 */
> >  	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
> > -	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
> > +
> > +	if (HAS_ENGINE(dev_priv, VECS))
> > +		GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3],
> gt_interrupts[3]);
> >  }
> >
> >  static void gen8_de_irq_postinstall(struct drm_i915_private
> > *dev_priv) @@ -3414,7 +3422,9 @@ static int gen8_irq_postinstall(struct
> drm_device *dev)
> >  		ibx_irq_pre_postinstall(dev);
> >
> >  	gen8_gt_irq_postinstall(dev_priv);
> > -	gen8_de_irq_postinstall(dev_priv);
> > +
> > +	if (INTEL_INFO(dev_priv)->num_pipes)
> > +		gen8_de_irq_postinstall(dev_priv);
> >
> >  	if (HAS_PCH_SPLIT(dev_priv))
> >  		ibx_irq_postinstall(dev);
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC