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

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

[Wang Elaine]

From: Elaine Wang <elaine.wang@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@linux.intel.com>
Signed-off-by: Elaine Wang <elaine.wang@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]);
 	/*
 	 * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Respect ring_mask and num_pipes when install IRQ

[Joonas Lahtinen]

On to, 2016-12-08 at 18:46 +0800, Wang Elaine wrote:
> > From: Elaine Wang <elaine.wang@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@linux.intel.com>
> Signed-off-by: Elaine Wang <elaine.wang@intel.com>

Looks like a sensible thing to me, not able to test myself. Could use
an R-b from Ville.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Respect ring_mask and num_pipes when install IRQ

[Patchwork]

== Series Details ==

Series: drm/i915: Respect ring_mask and num_pipes when install IRQ
URL   : https://patchwork.freedesktop.org/series/16547/
State : success

== Summary ==

Series 16547v1 drm/i915: Respect ring_mask and num_pipes when install IRQ
https://patchwork.freedesktop.org/api/1.0/series/16547/revisions/1/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-t5700     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

24cc1f39920c0caf747c6bda267ca19b99f21786 drm-tip: 2016y-12m-08d-12h-31m-59s UTC integration manifest
248ffa7 drm/i915: Respect ring_mask and num_pipes when install IRQ

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3236/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Respect ring_mask and num_pipes when install IRQ

[Ville Syrjälä]

On Thu, Dec 08, 2016 at 06:46:49PM +0800, Wang Elaine wrote:
> From: Elaine Wang <elaine.wang@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@linux.intel.com>
> Signed-off-by: Elaine Wang <elaine.wang@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.

>  	/*
>  	 * 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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@linux.intel.com>
> > Signed-off-by: Elaine Wang <elaine.wang@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx