On 14/12/16 13:48, Laurent Pinchart wrote:
> Hi Tomi,
>
> On Wednesday 14 Dec 2016 12:22:29 Tomi Valkeinen wrote:
>> On 14/12/16 02:27, Laurent Pinchart wrote:
>>> As the FIFO underflow IRQ handler just prints an error message to the
>>> kernel log, simplify the code by not registering one IRQ handler per
>>> plane but print the messages directly from the main IRQ handler.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>>> ---
>>>
>>> +static void omap_irq_fifo_underflow(struct omap_drm_private *priv,
>>> + u32 irqstatus)
>>> +{
>>> + static DEFINE_RATELIMIT_STATE(_rs, DEFAULT_RATELIMIT_INTERVAL,
>>> + DEFAULT_RATELIMIT_BURST);
>>> + static const struct {
>>> + const char *name;
>>> + u32 mask;
>>> + } sources[] = {
>>> + { "gfx", DISPC_IRQ_GFX_FIFO_UNDERFLOW },
>>> + { "vid1", DISPC_IRQ_VID1_FIFO_UNDERFLOW },
>>> + { "vid2", DISPC_IRQ_VID2_FIFO_UNDERFLOW },
>>> + { "vid3", DISPC_IRQ_VID3_FIFO_UNDERFLOW },
>>> + };
>>> +
>>> + const u32 mask = DISPC_IRQ_GFX_FIFO_UNDERFLOW
>>> + | DISPC_IRQ_VID1_FIFO_UNDERFLOW
>>> + | DISPC_IRQ_VID2_FIFO_UNDERFLOW
>>> + | DISPC_IRQ_VID3_FIFO_UNDERFLOW;
>>> + unsigned int i;
>>> +
>>> + spin_lock(&list_lock);
>>> + irqstatus &= priv->irq_mask & mask;
>>> + spin_unlock(&list_lock);
>>> +
>>> + if (!irqstatus)
>>> + return;
>>
>> This is called every time we get any DSS interrupt, so I think it would
>> be good to have a fast-path here without the lock: irqstatus & mask.
>>
>> Or maybe store the enabled underflow irq bits separately from irq_mask,
>> as the underflow bits are never changed after the initial setup, and
>> then there's no need for locking.
>
> I'd prefer going for the former, but I'm a bit concerned that an IRQ bit
> defined as FIFO overflow on one platform could be defined as something else on
> another platform and be mistaken.
>
> Given that we already take the same lock in the IRQ handler to call the wait
> handlers, do you think this is really an issue ?
Yep, I think it's fine for the time being.
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
