All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Only unmask required PM interrupts
@ 2013-08-14 20:34 Vinit Azad
  2013-08-14 20:46 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Vinit Azad @ 2013-08-14 20:34 UTC (permalink / raw)
  To: intel-gfx

Un-masking all PM interrupts causes hardware to generate
interrupts regardless of whether the interrupts are enabled
on the DE side. Since turbo only need up/down threshold and
rc6 timeout interrupt, mask all other interrupts bits to avoid
unnecessary overhead/wake up.

Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827
Signed-off-by: Vinit Azad <vinit.azad@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8e9ce07..17a0dae 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
 	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
-	/* unmask all PM interrupts */
-	I915_WRITE(GEN6_PMINTRMSK, 0);
+	/* only unmask PM interrupts we need. Mask all others. */
+	I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
 
 	rc6vids = 0;
 	ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
@@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
 	WARN_ON(dev_priv->rps.pm_iir != 0);
 	I915_WRITE(GEN6_PMIMR, 0);
 	spin_unlock_irq(&dev_priv->rps.lock);
-	/* enable all PM interrupts */
-	I915_WRITE(GEN6_PMINTRMSK, 0);
+	/* enable only the PM interrupts we need. Mask everything else */
+	I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
 
 	gen6_gt_force_wake_put(dev_priv);
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Only unmask required PM interrupts
  2013-08-14 20:34 [PATCH] drm/i915: Only unmask required PM interrupts Vinit Azad
@ 2013-08-14 20:46 ` Daniel Vetter
  2013-08-14 21:01   ` Azad, Vinit
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-08-14 20:46 UTC (permalink / raw)
  To: Vinit Azad; +Cc: intel-gfx

On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad <vinit.azad@intel.com> wrote:
> Un-masking all PM interrupts causes hardware to generate
> interrupts regardless of whether the interrupts are enabled
> on the DE side. Since turbo only need up/down threshold and
> rc6 timeout interrupt, mask all other interrupts bits to avoid
> unnecessary overhead/wake up.

Just to clarify since I can't really believe this yet: Even though we
disable all other interrupt sources in PMIER and mask them in PMIMR hw
still manages to fire off our interrupt handler? Do those interrupts
end up setting PMIIR?

Thanks, Daniel
>
> Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827
> Signed-off-by: Vinit Azad <vinit.azad@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8e9ce07..17a0dae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev)
>         I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
>         I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
>         spin_unlock_irq(&dev_priv->rps.lock);
> -       /* unmask all PM interrupts */
> -       I915_WRITE(GEN6_PMINTRMSK, 0);
> +       /* only unmask PM interrupts we need. Mask all others. */
> +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
>
>         rc6vids = 0;
>         ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
> @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
>         WARN_ON(dev_priv->rps.pm_iir != 0);
>         I915_WRITE(GEN6_PMIMR, 0);
>         spin_unlock_irq(&dev_priv->rps.lock);
> -       /* enable all PM interrupts */
> -       I915_WRITE(GEN6_PMINTRMSK, 0);
> +       /* enable only the PM interrupts we need. Mask everything else */
> +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
>
>         gen6_gt_force_wake_put(dev_priv);
>  }
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Only unmask required PM interrupts
  2013-08-14 20:46 ` Daniel Vetter
@ 2013-08-14 21:01   ` Azad, Vinit
  2013-08-14 21:07     ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Azad, Vinit @ 2013-08-14 21:01 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Our interrupt handler isn't being fired since we do set the IER bits properly (IIR bits aren't set). The overhead isn't because our driver is reacting to these interrupts, but because hardware keeps generating internal messages when PMINTRMSK doesn't mask out the up/down EI interrupts (which happen periodically).

Unfortunately, the default value of PMINTRMSK register is 0 instead of 0xFFFFFFFF as it is for other IMR, so we have to mask out the interrupts manually.

-----Original Message-----
From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, August 14, 2013 13:47
To: Azad, Vinit
Cc: intel-gfx
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts

On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad <vinit.azad@intel.com> wrote:
> Un-masking all PM interrupts causes hardware to generate interrupts 
> regardless of whether the interrupts are enabled on the DE side. Since 
> turbo only need up/down threshold and
> rc6 timeout interrupt, mask all other interrupts bits to avoid 
> unnecessary overhead/wake up.

Just to clarify since I can't really believe this yet: Even though we disable all other interrupt sources in PMIER and mask them in PMIMR hw still manages to fire off our interrupt handler? Do those interrupts end up setting PMIIR?

Thanks, Daniel
>
> Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827
> Signed-off-by: Vinit Azad <vinit.azad@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev)
>         I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
>         I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
>         spin_unlock_irq(&dev_priv->rps.lock);
> -       /* unmask all PM interrupts */
> -       I915_WRITE(GEN6_PMINTRMSK, 0);
> +       /* only unmask PM interrupts we need. Mask all others. */
> +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
>
>         rc6vids = 0;
>         ret = sandybridge_pcode_read(dev_priv, 
> GEN6_PCODE_READ_RC6VIDS, &rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
>         WARN_ON(dev_priv->rps.pm_iir != 0);
>         I915_WRITE(GEN6_PMIMR, 0);
>         spin_unlock_irq(&dev_priv->rps.lock);
> -       /* enable all PM interrupts */
> -       I915_WRITE(GEN6_PMINTRMSK, 0);
> +       /* enable only the PM interrupts we need. Mask everything else */
> +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
>
>         gen6_gt_force_wake_put(dev_priv);  }
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Only unmask required PM interrupts
  2013-08-14 21:01   ` Azad, Vinit
@ 2013-08-14 21:07     ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-08-14 21:07 UTC (permalink / raw)
  To: Azad, Vinit; +Cc: intel-gfx

On Wed, Aug 14, 2013 at 09:01:22PM +0000, Azad, Vinit wrote:
> Our interrupt handler isn't being fired since we do set the IER bits
> properly (IIR bits aren't set). The overhead isn't because our driver is
> reacting to these interrupts, but because hardware keeps generating
> internal messages when PMINTRMSK doesn't mask out the up/down EI
> interrupts (which happen periodically).

Ah, that makes more sense. Added to the commit message, please elaborate
more precisely for the next patch what exactly the effects are.

> Unfortunately, the default value of PMINTRMSK register is 0 instead of
> 0xFFFFFFFF as it is for other IMR, so we have to mask out the interrupts
> manually.
> 
> -----Original Message-----
> From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, August 14, 2013 13:47
> To: Azad, Vinit
> Cc: intel-gfx
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
> 
> On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad <vinit.azad@intel.com> wrote:
> > Un-masking all PM interrupts causes hardware to generate interrupts 
> > regardless of whether the interrupts are enabled on the DE side. Since 
> > turbo only need up/down threshold and
> > rc6 timeout interrupt, mask all other interrupts bits to avoid 
> > unnecessary overhead/wake up.
> 
> Just to clarify since I can't really believe this yet: Even though we
> disable all other interrupt sources in PMIER and mask them in PMIMR hw
> still manages to fire off our interrupt handler? Do those interrupts end
> up setting PMIIR?
> 
> Thanks, Daniel
> >
> > Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827
> > Signed-off-by: Vinit Azad <vinit.azad@intel.com>

Patch merged to dinq with a bit of manual fixup. Please base patches on
top of my drm-intel-nightly tree if possible, only really critical fixes
(for hangs, black screens, ...) should be pased on -fixes.

Thanks for the patch,
-Daniel

> > ---
> >  drivers/gpu/drm/i915/intel_pm.c |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev)
> >         I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
> >         I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
> >         spin_unlock_irq(&dev_priv->rps.lock);
> > -       /* unmask all PM interrupts */
> > -       I915_WRITE(GEN6_PMINTRMSK, 0);
> > +       /* only unmask PM interrupts we need. Mask all others. */
> > +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
> >
> >         rc6vids = 0;
> >         ret = sandybridge_pcode_read(dev_priv, 
> > GEN6_PCODE_READ_RC6VIDS, &rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >         WARN_ON(dev_priv->rps.pm_iir != 0);
> >         I915_WRITE(GEN6_PMIMR, 0);
> >         spin_unlock_irq(&dev_priv->rps.lock);
> > -       /* enable all PM interrupts */
> > -       I915_WRITE(GEN6_PMINTRMSK, 0);
> > +       /* enable only the PM interrupts we need. Mask everything else */
> > +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
> >
> >         gen6_gt_force_wake_put(dev_priv);  }
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-14 21:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-14 20:34 [PATCH] drm/i915: Only unmask required PM interrupts Vinit Azad
2013-08-14 20:46 ` Daniel Vetter
2013-08-14 21:01   ` Azad, Vinit
2013-08-14 21:07     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.