Re: OMAP: DSS2: Common IRQ handler for all OMAPs

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Mon, 2011-02-14 at 08:30 -0600, Balbi, Felipe wrote:
> Hi,
> 
> On Mon, Feb 14, 2011 at 04:21:47PM +0200, Tomi Valkeinen wrote:
> > On Wed, 2011-02-02 at 08:56 +0000, archit taneja wrote:
> > > OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI
> > > on omap2.
> > > OMAP3 has a common irq line for DISPC and DSI interrupts.
> > > OMAP4 has seperate irq lines for DISPC and DSI Interrupts.
> > > 
> > > Use dss_features to have a common DSS irq handler for all OMAP revisions.
> > > 
> > > Also, use a member of the global dss structure to store the irq number
> > > as it is used in 2 functions.
> > 
> > It's good to remove the cpu_is_xxxx() calls, but I'm not quite sure
> > about this patch...
> > 
> > Could we use shared interrupt handlers here, so that dss.c would handle
> > only DISPC interrupts (or should it be even in dispc.c?) and dsi.c would
> > handle DSI interrupts?
> > 
> > On OMAP3 both dss.c and dsi.c would register to the same interrupt, and
> > they would need to check if the interrupt was really for them. On OMAP4
> > the code could be the same, even though the check is unnecessary.
> > 
> > Also, as I mentioned in the email I sent some minutes ago, this patch
> > fixes the free_irq call in dss_exit. Things like that should never be
> > fixed silently, even if it's trivial like in this case.
> 
> does it make sense to install an irq_chip for that ? I mean, can you
> mask/unmask dss and or dsi IRQs ? If you can, then it might make sense
> to take a look into GENIRQ and install an irq_chip for that. Then both
> dsi and dss would be able to use standard request_irq() API.
> 
> Take a look at drivers/mfd/twl*irq.c and drivers/cbus/retu.c (the last
> one on linux-omap only) there are examples of how that should be
> implemented. Actually twl*irq.c is wrong, as it bypasses the threaded
> IRQ stuff. I have some patches for those, but I'm not sure they are
> working correctly yet, needs more testing. My twl4030-irq patches are
> available at [1] for reference.

I'm not familiar with genirq/irq_chip. But yes, as Archit said, we can
mask/unmask DSS interrupts. I mean, there's only one interrupt line, but
DSS has irqstatus and irqenable registers that can be used.

If I understood correctly, irq_chip could be used to manage DSS and DSI
interrupts, but I don't know right away what that would mean in
practice, and would it make things easier or not. But this could be
something that needs more study, as there's a custom isr handling system
in DSS, and it would be nice if that could be replaced with genirq.

But I guess the main problem still remains with genirqs also: we have
one interrupt line on omap3, used by two separate modules. And one irq
on omap2, used by one module, and two irqs on omap4, used by two
modules.

 Tomi