[Intel-gfx] [PATCH] drm/i915: Some polish for the new pipestat_irq_handler

[Daniel Vetter]

On Wed, Feb 12, 2014 at 5:52 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>>  drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++++++-----
>>  drivers/gpu/drm/i915/i915_reg.h |  4 ----
>>  2 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 386a640b7c92..bbd65809742b 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1559,25 +1559,40 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
>>       spin_lock(&dev_priv->irq_lock);
>>       for_each_pipe(pipe) {
>>               int reg;
>> -             u32 mask;
>> +             u32 mask, iir_bit;
>>
>> -             if (!dev_priv->pipestat_irq_mask[pipe] &&
>> -                 !__cpu_fifo_underrun_reporting_enabled(dev, pipe))
>> +             if (!dev_priv->pipestat_irq_mask[pipe])
>>                       continue;
>
> Underrun reporting doesn't have an enable bit, so if we don't check it
> here we'd fail to detect underruns when no PIPESTAT interrupts are
> enabled. Currently that probably wouldn't happen since we always enable
> some display interrupts, but I'd keep the check nonetheless.

Oh right, I've misunderstood the code. Still I'm not too happy about
the duplication of logic we have here with two places computing
whether we're interested in the pipestat bits for this pipe.

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