RE: [PATCH] OMAP: DSS2: Have separate irq handlers for DISPC and DSI

Valkeinen, Tomi
2011-02-18T09:10:41Z

Hi,

On Thu, 2011-02-17 at 08:25 -0600, Taneja, Archit wrote:
> Currently, the core DSS platform device requests for an irq line for OMAP2 and
> OMAP3. Make DISPC and DSI platform devices request for a shared IRQ line.
>
> On OMAP3, the logical OR of DSI and DISPC interrupt lines goes to the MPU. There
> is a register DSS_IRQSTATUS which tells if the interrupt came from DISPC or DSI.
>
> On OMAP2, there is no DSI, only DISPC interrupts goto the MPU. There is no
> DSS_IRQSTATUS register.
>
> Hence, it makes more sense to have separate irq handlers corresponding to the
> DSS sub modules instead of having a common handler.
>
> Since on OMAP3 the logical OR of the lines goes to MPU, the irq line is shared
> among the IRQ handlers.
>
> The hwmod irq info has been removed for DSS to DISPC and DSI for OMAP2 and OMAP3
> hwmod databases. The Probes of DISPC and DSI now request for irq handlers. A dss
> feature is also added to tell whether the irq lines is shared between DISPC and
> DSI or not.

Yes, I think this looks much cleaner.

However, I'm not sure if it's necessary to check the DSS_IRQSTATUS. It
depends a bit on how DSS_IRQSTATUS works. If it just mirrors the
DISPC/DSI_IRQSTATUS (ie, if there's any bit up in DSI_IRQSTATUS, the DSI
bit in DSS_IRQSTATUS is up), we can do without it.

For example let's say we haven't enabled any interrupts in DSI, so
preferably we want to spend as little time in the dsi irq handler as
possible.

Now, if DSI_IRQSTATUS is all zeroes, and thus DSS_IRQSTATUS.DSI is also
zero, we can skip the DSI handler by checking DSS_IRQSTATUS.DSI as you
do in this patch. But we could as well check DSI_IRQSTATUS, and exit if
it's all zeroes.

If, on the other hand, DSI_IRQSTATUS has any bit up, then also
DSS_IRQSTATUS.DSI is up, and we have to check the DSI interrupts anyway.

So in both cases the end result is the same, and we can do with less
code by not using DSS_IRQSTATUS.

However, I'm not 100% sure DSS_IRQSTATUS works like that, but it would
sound logical and I don't know how could it work otherwise.

Tomi