Re: [PATCH 6/7] drm/i915/icl: Handle rps interrupts without irq lock

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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