* [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
@ 2014-08-27 8:43 Daniel Vetter
2014-08-27 8:51 ` Chris Wilson
2014-08-27 23:00 ` Jesse Barnes
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-08-27 8:43 UTC (permalink / raw
To: Intel Graphics Development; +Cc: Daniel Vetter, Paulo Zanoni
Now that vlv has runtime pm we kinda should check for that like on the
pch split platforms. Looks like this was simply lost in the vlv rpm
enabling.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_irq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9eb303c1b621..76bc4d0de5a4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
assert_spin_locked(&dev_priv->irq_lock);
+ WARN_ON(!intel_irqs_enabled(dev_priv));
if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
status_mask & ~PIPESTAT_INT_STATUS_MASK,
@@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
assert_spin_locked(&dev_priv->irq_lock);
+ WARN_ON(!intel_irqs_enabled(dev_priv));
if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
status_mask & ~PIPESTAT_INT_STATUS_MASK,
--
2.0.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 8:43 [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat Daniel Vetter
@ 2014-08-27 8:51 ` Chris Wilson
2014-08-27 10:23 ` Daniel Vetter
2014-08-27 23:00 ` Jesse Barnes
1 sibling, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2014-08-27 8:51 UTC (permalink / raw
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote:
> Now that vlv has runtime pm we kinda should check for that like on the
> pch split platforms. Looks like this was simply lost in the vlv rpm
> enabling.
Is there a reason why setting up the pipestat prior to enabling
interrupts is verboten, or is it just not done today?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 8:51 ` Chris Wilson
@ 2014-08-27 10:23 ` Daniel Vetter
2014-08-27 11:28 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-08-27 10:23 UTC (permalink / raw
To: Chris Wilson, Daniel Vetter, Intel Graphics Development,
Paulo Zanoni
On Wed, Aug 27, 2014 at 09:51:58AM +0100, Chris Wilson wrote:
> On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote:
> > Now that vlv has runtime pm we kinda should check for that like on the
> > pch split platforms. Looks like this was simply lost in the vlv rpm
> > enabling.
>
> Is there a reason why setting up the pipestat prior to enabling
> interrupts is verboten, or is it just not done today?
The similar warning on pch platforms was added to catch runtime pm bugs
where someone enabled some interrupt source, but didn't hold a runtime pm
reference. That's just not going to work. So motivation for this patch is
vlv/chv runtime pm really.
But I also think that this check provides value for driver
load/resume/suspend since if we change interrupt settings after the
interrupts have been disabled or before enabling something is at least
fishy. At least insofar as that our irq install hooks nowadays overwrite
all of them with default values.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 10:23 ` Daniel Vetter
@ 2014-08-27 11:28 ` Chris Wilson
2014-08-27 13:29 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2014-08-27 11:28 UTC (permalink / raw
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni
On Wed, Aug 27, 2014 at 12:23:47PM +0200, Daniel Vetter wrote:
> On Wed, Aug 27, 2014 at 09:51:58AM +0100, Chris Wilson wrote:
> > On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote:
> > > Now that vlv has runtime pm we kinda should check for that like on the
> > > pch split platforms. Looks like this was simply lost in the vlv rpm
> > > enabling.
> >
> > Is there a reason why setting up the pipestat prior to enabling
> > interrupts is verboten, or is it just not done today?
>
> The similar warning on pch platforms was added to catch runtime pm bugs
> where someone enabled some interrupt source, but didn't hold a runtime pm
> reference. That's just not going to work. So motivation for this patch is
> vlv/chv runtime pm really.
>
> But I also think that this check provides value for driver
> load/resume/suspend since if we change interrupt settings after the
> interrupts have been disabled or before enabling something is at least
> fishy. At least insofar as that our irq install hooks nowadays overwrite
> all of them with default values.
That's a better blurp. But doesn't it sound a little overly generic just
to keep whether or not pipestat_enable is called after uninstall or
before install? Since we track the expected register value, could we
just semi-randomly check that the register matches that value?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 11:28 ` Chris Wilson
@ 2014-08-27 13:29 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-08-27 13:29 UTC (permalink / raw
To: Chris Wilson, Daniel Vetter, Daniel Vetter,
Intel Graphics Development, Paulo Zanoni
On Wed, Aug 27, 2014 at 1:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Aug 27, 2014 at 12:23:47PM +0200, Daniel Vetter wrote:
>> On Wed, Aug 27, 2014 at 09:51:58AM +0100, Chris Wilson wrote:
>> > On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote:
>> > > Now that vlv has runtime pm we kinda should check for that like on the
>> > > pch split platforms. Looks like this was simply lost in the vlv rpm
>> > > enabling.
>> >
>> > Is there a reason why setting up the pipestat prior to enabling
>> > interrupts is verboten, or is it just not done today?
>>
>> The similar warning on pch platforms was added to catch runtime pm bugs
>> where someone enabled some interrupt source, but didn't hold a runtime pm
>> reference. That's just not going to work. So motivation for this patch is
>> vlv/chv runtime pm really.
>>
>> But I also think that this check provides value for driver
>> load/resume/suspend since if we change interrupt settings after the
>> interrupts have been disabled or before enabling something is at least
>> fishy. At least insofar as that our irq install hooks nowadays overwrite
>> all of them with default values.
>
> That's a better blurp. But doesn't it sound a little overly generic just
> to keep whether or not pipestat_enable is called after uninstall or
> before install? Since we track the expected register value, could we
> just semi-randomly check that the register matches that value?
I guess we could do both, but I don't really see a good place. Maybe
in the enable/disable functions, but I'm not even sure we track the
expected state for everyone really.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 8:43 [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat Daniel Vetter
2014-08-27 8:51 ` Chris Wilson
@ 2014-08-27 23:00 ` Jesse Barnes
2014-09-04 15:59 ` Daniel Vetter
2014-09-08 8:18 ` Daniel Vetter
1 sibling, 2 replies; 11+ messages in thread
From: Jesse Barnes @ 2014-08-27 23:00 UTC (permalink / raw
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
On Wed, 27 Aug 2014 10:43:37 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Now that vlv has runtime pm we kinda should check for that like on the
> pch split platforms. Looks like this was simply lost in the vlv rpm
> enabling.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9eb303c1b621..76bc4d0de5a4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>
> assert_spin_locked(&dev_priv->irq_lock);
> + WARN_ON(!intel_irqs_enabled(dev_priv));
>
> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> status_mask & ~PIPESTAT_INT_STATUS_MASK,
> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>
> assert_spin_locked(&dev_priv->irq_lock);
> + WARN_ON(!intel_irqs_enabled(dev_priv));
>
> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> status_mask & ~PIPESTAT_INT_STATUS_MASK,
Yeah looks good, wonder if it'll trigger any new warnings.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 23:00 ` Jesse Barnes
@ 2014-09-04 15:59 ` Daniel Vetter
2014-09-04 16:24 ` Jesse Barnes
2014-09-08 8:18 ` Daniel Vetter
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-09-04 15:59 UTC (permalink / raw
To: Jesse Barnes; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, Aug 28, 2014 at 1:00 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 9eb303c1b621..76bc4d0de5a4 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>>
>> assert_spin_locked(&dev_priv->irq_lock);
>> + WARN_ON(!intel_irqs_enabled(dev_priv));
>>
>> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
>> status_mask & ~PIPESTAT_INT_STATUS_MASK,
>> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>>
>> assert_spin_locked(&dev_priv->irq_lock);
>> + WARN_ON(!intel_irqs_enabled(dev_priv));
>>
>> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
>> status_mask & ~PIPESTAT_INT_STATUS_MASK,
>
> Yeah looks good, wonder if it'll trigger any new warnings.
It will blow up in a bunch of postinstall hooks, just like the one for
ilk. At least without my patch to shuffle the pm._irqs_disabled
assignment around.
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
... so does that count as an implicit r-b on my other patch?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-09-04 15:59 ` Daniel Vetter
@ 2014-09-04 16:24 ` Jesse Barnes
2014-09-04 16:59 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Jesse Barnes @ 2014-09-04 16:24 UTC (permalink / raw
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, 4 Sep 2014 17:59:18 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Aug 28, 2014 at 1:00 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index 9eb303c1b621..76bc4d0de5a4 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >>
> >> assert_spin_locked(&dev_priv->irq_lock);
> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
> >>
> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
> >> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >>
> >> assert_spin_locked(&dev_priv->irq_lock);
> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
> >>
> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
> >
> > Yeah looks good, wonder if it'll trigger any new warnings.
>
> It will blow up in a bunch of postinstall hooks, just like the one for
> ilk. At least without my patch to shuffle the pm._irqs_disabled
> assignment around.
>
> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> ... so does that count as an implicit r-b on my other patch?
Sure, though didn't Jani find some issues with it?
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-09-04 16:24 ` Jesse Barnes
@ 2014-09-04 16:59 ` Daniel Vetter
2014-09-04 17:36 ` Jesse Barnes
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-09-04 16:59 UTC (permalink / raw
To: Jesse Barnes; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, Sep 4, 2014 at 6:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Thu, 4 Sep 2014 17:59:18 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
>> On Thu, Aug 28, 2014 at 1:00 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> >> index 9eb303c1b621..76bc4d0de5a4 100644
>> >> --- a/drivers/gpu/drm/i915/i915_irq.c
>> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> >> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>> >>
>> >> assert_spin_locked(&dev_priv->irq_lock);
>> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
>> >>
>> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
>> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
>> >> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
>> >>
>> >> assert_spin_locked(&dev_priv->irq_lock);
>> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
>> >>
>> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
>> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
>> >
>> > Yeah looks good, wonder if it'll trigger any new warnings.
>>
>> It will blow up in a bunch of postinstall hooks, just like the one for
>> ilk. At least without my patch to shuffle the pm._irqs_disabled
>> assignment around.
>>
>> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>>
>> ... so does that count as an implicit r-b on my other patch?
>
> Sure, though didn't Jani find some issues with it?
QA claims that both my and your patch break the display on hsw, bdw
and snb or something like that. Which either means I'm blind (since
either patch should only affect ilk in a functional way) or they're
doing something really strange. I have no idea what's actually going
on there, but we have a regular stream of reports of this one here ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-09-04 16:59 ` Daniel Vetter
@ 2014-09-04 17:36 ` Jesse Barnes
0 siblings, 0 replies; 11+ messages in thread
From: Jesse Barnes @ 2014-09-04 17:36 UTC (permalink / raw
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, 4 Sep 2014 18:59:55 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Sep 4, 2014 at 6:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Thu, 4 Sep 2014 17:59:18 +0200
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> >> On Thu, Aug 28, 2014 at 1:00 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> >> index 9eb303c1b621..76bc4d0de5a4 100644
> >> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> >> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> >> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >> >>
> >> >> assert_spin_locked(&dev_priv->irq_lock);
> >> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
> >> >>
> >> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> >> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
> >> >> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> >> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >> >>
> >> >> assert_spin_locked(&dev_priv->irq_lock);
> >> >> + WARN_ON(!intel_irqs_enabled(dev_priv));
> >> >>
> >> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> >> >> status_mask & ~PIPESTAT_INT_STATUS_MASK,
> >> >
> >> > Yeah looks good, wonder if it'll trigger any new warnings.
> >>
> >> It will blow up in a bunch of postinstall hooks, just like the one for
> >> ilk. At least without my patch to shuffle the pm._irqs_disabled
> >> assignment around.
> >>
> >> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >>
> >> ... so does that count as an implicit r-b on my other patch?
> >
> > Sure, though didn't Jani find some issues with it?
>
> QA claims that both my and your patch break the display on hsw, bdw
> and snb or something like that. Which either means I'm blind (since
> either patch should only affect ilk in a functional way) or they're
> doing something really strange. I have no idea what's actually going
> on there, but we have a regular stream of reports of this one here ...
Yeah I agree the results are weird; don't see how we'd affect BDW
display, esp with my patch, so I'd say go ahead with either. Though if
you do mine I guess you'd drop the above.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat
2014-08-27 23:00 ` Jesse Barnes
2014-09-04 15:59 ` Daniel Vetter
@ 2014-09-08 8:18 ` Daniel Vetter
1 sibling, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-09-08 8:18 UTC (permalink / raw
To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni
On Wed, Aug 27, 2014 at 04:00:22PM -0700, Jesse Barnes wrote:
> On Wed, 27 Aug 2014 10:43:37 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > Now that vlv has runtime pm we kinda should check for that like on the
> > pch split platforms. Looks like this was simply lost in the vlv rpm
> > enabling.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 9eb303c1b621..76bc4d0de5a4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> > u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >
> > assert_spin_locked(&dev_priv->irq_lock);
> > + WARN_ON(!intel_irqs_enabled(dev_priv));
> >
> > if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> > status_mask & ~PIPESTAT_INT_STATUS_MASK,
> > @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> > u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
> >
> > assert_spin_locked(&dev_priv->irq_lock);
> > + WARN_ON(!intel_irqs_enabled(dev_priv));
> >
> > if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> > status_mask & ~PIPESTAT_INT_STATUS_MASK,
>
> Yeah looks good, wonder if it'll trigger any new warnings.
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Since Jani just merged the driver load ordering patch I've pulled this one
into dinq to maximise testing coverage. Let's see what happens ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-08 8:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-27 8:43 [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat Daniel Vetter
2014-08-27 8:51 ` Chris Wilson
2014-08-27 10:23 ` Daniel Vetter
2014-08-27 11:28 ` Chris Wilson
2014-08-27 13:29 ` Daniel Vetter
2014-08-27 23:00 ` Jesse Barnes
2014-09-04 15:59 ` Daniel Vetter
2014-09-04 16:24 ` Jesse Barnes
2014-09-04 16:59 ` Daniel Vetter
2014-09-04 17:36 ` Jesse Barnes
2014-09-08 8:18 ` 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.