| Message ID | 20190409161310.20382-6-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] drm/i915: Use dedicated rc6 enabling sequence for gen11 | expand |
Quoting Mika Kuoppala (2019-04-09 17:13:09) > Unlike previous gens, we already hold the irq_lock on > entering the rps handler so we can't use it as it is. > > Make a gen11 specific rps interrupt handler without > locking. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 6454ddc37f8b..619e6ab273e7 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1796,6 +1796,22 @@ static void i9xx_pipe_crc_irq_handler(struct drm_i915_private *dev_priv, > /* The RPS events need forcewake, so we add them to a work queue and mask their > * IMR bits until the work is done. Other interrupts can be processed without > * the work queue. */ > +static void gen11_rps_irq_handler(struct drm_i915_private *i915, u32 pm_iir) > +{ > + struct intel_rps *rps = &i915->gt_pm.rps; > + const u32 events = i915->pm_rps_events & pm_iir; > + > + lockdep_assert_held(&i915->irq_lock); > + > + if (events) { if (!events) return; ? Maybe you have reason for the indent later. > + gen6_mask_pm_irq(i915, events); > + if (rps->interrupts_enabled) { > + rps->pm_iir |= events; > + schedule_work(&rps->work); > + } > + } All I can say is that this is evidence that we've never had an rps interrupt! I guess this patch needs to be first just in case an interrupt is sent. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-04-09 17:13:09) >> Unlike previous gens, we already hold the irq_lock on >> entering the rps handler so we can't use it as it is. >> >> Make a gen11 specific rps interrupt handler without >> locking. >> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index 6454ddc37f8b..619e6ab273e7 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -1796,6 +1796,22 @@ static void i9xx_pipe_crc_irq_handler(struct drm_i915_private *dev_priv, >> /* The RPS events need forcewake, so we add them to a work queue and mask their >> * IMR bits until the work is done. Other interrupts can be processed without >> * the work queue. */ >> +static void gen11_rps_irq_handler(struct drm_i915_private *i915, u32 pm_iir) >> +{ >> + struct intel_rps *rps = &i915->gt_pm.rps; >> + const u32 events = i915->pm_rps_events & pm_iir; >> + >> + lockdep_assert_held(&i915->irq_lock); >> + >> + if (events) { > > if (!events) > return; > ? > Maybe you have reason for the indent later. No reasons. I am avid fan of early return. This was explained by copypasta from gen6 variant and then some interrupt event masked between my ears. will send v2 > >> + gen6_mask_pm_irq(i915, events); >> + if (rps->interrupts_enabled) { >> + rps->pm_iir |= events; >> + schedule_work(&rps->work); >> + } >> + } > > All I can say is that this is evidence that we've never had an rps > interrupt! > > I guess this patch needs to be first just in case an interrupt is > sent. Yeah, when got first ever sent, I witnessed spectacular fireworks. > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Ta, -Mika
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6454ddc37f8b..619e6ab273e7 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1796,6 +1796,22 @@ static void i9xx_pipe_crc_irq_handler(struct drm_i915_private *dev_priv, /* The RPS events need forcewake, so we add them to a work queue and mask their * IMR bits until the work is done. Other interrupts can be processed without * the work queue. */ +static void gen11_rps_irq_handler(struct drm_i915_private *i915, u32 pm_iir) +{ + struct intel_rps *rps = &i915->gt_pm.rps; + const u32 events = i915->pm_rps_events & pm_iir; + + lockdep_assert_held(&i915->irq_lock); + + if (events) { + gen6_mask_pm_irq(i915, events); + if (rps->interrupts_enabled) { + rps->pm_iir |= events; + schedule_work(&rps->work); + } + } +} + static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) { struct intel_rps *rps = &dev_priv->gt_pm.rps; @@ -2949,7 +2965,7 @@ gen11_other_irq_handler(struct drm_i915_private * const i915, const u8 instance, const u16 iir) { if (instance == OTHER_GTPM_INSTANCE) - return gen6_rps_irq_handler(i915, iir); + return gen11_rps_irq_handler(i915, iir); WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n", instance, iir);
Unlike previous gens, we already hold the irq_lock on entering the rps handler so we can't use it as it is. Make a gen11 specific rps interrupt handler without locking. Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)