Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tuesday 15 February 2011 01:15 PM, Valkeinen, Tomi wrote: snip 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 We actually have 4 interrupt lines on OMAP4, 1 for dispc, 2 for dsi1 and dsi2 and 1 for hdmi. Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tue, Feb 15, 2011 at 09:58:24AM +0530, archit taneja wrote: 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. We could disable dsi IRQs by masking all the possible interrupt events in DSI_IRQSTATUS. The same goes for dispc. Is this what you meant by masking/unmasking irqs? yes it is. Then it makes sense to have an irq_chip for those two irqs, I think. /proc/interrupt will reflect how the hardware works (DSI and DISPC IRQs being handled by DSS), both dsi and dispc can use normal request_irq() without setting IRQF_SHARED, etc etc. All you need to do is: static struct irq_chip dss_irq_chip = { .name = DSS, .irq_bus_lock = dss_bus_lock, .irq_bus_sync_unlock= dss_bus_sync_unlock, .irq_mask = dss_irq_mask, .irq_unmask = dss_irq_unmask, .irq_ack= dss_irq_ack, }; then, somewhere during probe() you have to: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, IRQF_VALID) #else set_irq_noprobe(irq); #endif set_irq_data(irq, dss_device_structure_pointer); set_irq_chip_and_handler(irq, dss_irq_chip, handle_simple_irq); } and on exit() you just need to cleanup: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, 0) #endif set_irq_chip_and_handler(irq, NULL, NULL); set_irq_data(irq, NULL); } -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tuesday 15 February 2011 01:35 PM, Balbi, Felipe wrote: Hi, On Tue, Feb 15, 2011 at 09:58:24AM +0530, archit taneja wrote: 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. We could disable dsi IRQs by masking all the possible interrupt events in DSI_IRQSTATUS. The same goes for dispc. Is this what you meant by masking/unmasking irqs? yes it is. Then it makes sense to have an irq_chip for those two irqs, I think. /proc/interrupt will reflect how the hardware works (DSI and DISPC IRQs being handled by DSS), both dsi and dispc can use normal request_irq() without setting IRQF_SHARED, etc etc. All you need to do is: static struct irq_chip dss_irq_chip = { .name = DSS, .irq_bus_lock = dss_bus_lock, .irq_bus_sync_unlock= dss_bus_sync_unlock, .irq_mask = dss_irq_mask, .irq_unmask = dss_irq_unmask, .irq_ack= dss_irq_ack, }; then, somewhere during probe() you have to: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, IRQF_VALID) #else set_irq_noprobe(irq); #endif set_irq_data(irq, dss_device_structure_pointer); set_irq_chip_and_handler(irq,dss_irq_chip, handle_simple_irq); } and on exit() you just need to cleanup: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, 0) #endif set_irq_chip_and_handler(irq, NULL, NULL); set_irq_data(irq, NULL); } Thanks for the info, but this seems to be suitable for the case when there are multiple irq events coming from the same interrupt line. On OMAP4 we have 4 different IRQ lines going to ARM, i.e 4 lines defined in irqs-44xx.h. We are looking for a common irq handler located somewhere centrally, and each module can hook their irq line to this handler. Does irq_chip do this? what does irq_base and irq_end signify? Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tue, Feb 15, 2011 at 01:50:49PM +0530, archit taneja wrote: yes it is. Then it makes sense to have an irq_chip for those two irqs, I think. /proc/interrupt will reflect how the hardware works (DSI and DISPC IRQs being handled by DSS), both dsi and dispc can use normal request_irq() without setting IRQF_SHARED, etc etc. All you need to do is: static struct irq_chip dss_irq_chip = { .name = DSS, .irq_bus_lock = dss_bus_lock, .irq_bus_sync_unlock= dss_bus_sync_unlock, .irq_mask = dss_irq_mask, .irq_unmask = dss_irq_unmask, .irq_ack= dss_irq_ack, }; then, somewhere during probe() you have to: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, IRQF_VALID) #else set_irq_noprobe(irq); #endif set_irq_data(irq, dss_device_structure_pointer); set_irq_chip_and_handler(irq,dss_irq_chip, handle_simple_irq); } and on exit() you just need to cleanup: for (irq = irq_base; irq irq_end; irq++) { #ifdef CONFIG_ARM set_irq_flags(irq, 0) #endif set_irq_chip_and_handler(irq, NULL, NULL); set_irq_data(irq, NULL); } Thanks for the info, but this seems to be suitable for the case when there are multiple irq events coming from the same interrupt line. On OMAP4 we have 4 different IRQ lines going to ARM, i.e 4 lines defined in irqs-44xx.h. do you have 4 lines for 4 different modules ? If so, what's the problem here ? The irq_chip you're using is INTC We are looking for a common irq handler located somewhere centrally, and each module can hook their irq line to this handler. Does irq_chip do this? what does irq_base and irq_end signify? I guess it can't do that, no. irq_base is the starting number for this irq_chip's IRQ and irq_end is irq_base + number_of_irqs. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tuesday 15 February 2011 02:07 PM, Valkeinen, Tomi wrote: On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? Okay, so for OMAP3 we should populate dsi.pdev and dispc.pdev in such a way that they return the single irq line number for DSS. And for OMAP4, the separate line numbers will go for the modules anyway. How do differentiate with the common handler now? It will be dirty if we have checks on the irq_line. Could we pass the pdev as the arg to differentiate the source? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Copying Benoit, On Tuesday 15 February 2011 02:07 PM, Valkeinen, Tomi wrote: On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi Benoit, Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line. Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi Archit, On 2/15/2011 10:25 AM, Taneja, Archit wrote: Copying Benoit, On Tuesday 15 February 2011 02:07 PM, Valkeinen, Tomi wrote: On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi Benoit, Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities. So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change. The driver has to evolve to handle properly the new HW capabilities while keeping the old stuff working. We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line. Then you need to hack your driver but not the hwmod data:-( Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi Benoit, On Tue, Feb 15, 2011 at 3:53 PM, Cousson, Benoit b-cous...@ti.com wrote: Hi Archit, On 2/15/2011 10:25 AM, Taneja, Archit wrote: Copying Benoit, On Tuesday 15 February 2011 02:07 PM, Valkeinen, Tomi wrote: On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi Benoit, Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities. So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change. I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like mentioned in Tomi's sample code above. How is hwmod data for these cases handled today? [shared IRQ, different platform devices?] Best regards, ~Sumit. The driver has to evolve to handle properly the new HW capabilities while keeping the old stuff working. We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line. Then you need to hack your driver but not the hwmod data:-( Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi Sumit, On 2/15/2011 11:28 AM, Semwal, Sumit wrote: Hi Benoit, On Tue, Feb 15, 2011 at 3:53 PM, Cousson, Benoitb-cous...@ti.com wrote: Hi Archit, On 2/15/2011 10:25 AM, Taneja, Archit wrote: Copying Benoit, On Tuesday 15 February 2011 02:07 PM, Valkeinen, Tomi wrote: On Tue, 2011-02-15 at 02:30 -0600, Taneja, Archit wrote: Hi, On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote: snip I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. Tomi Benoit, Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities. So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change. I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like mentioned in Tomi's sample code above. OK, thanks for the clarification, actually I missed a little bit the point :-( So in fact the 2 modules share that same IRQ today, and you just want to populate both hwmod with the same input. If this is a real OR between the two IRQ lines, meaning the dispc cannot mask the dsi IRQ or the opposite, then having the same IRQ number in the two different hwmods is a correct representation of the HW. Then both devices with get the same IRQ number during the platform_get_irq, so if the driver is aware of that it is fine. How is hwmod data for these cases handled today? [shared IRQ, different platform devices?] I don't think we have such case today at least on OMAP4. Or maybe it is just not properly documented, so only one hwmod is mapped to the IRQ line. Regards, Benoit Best regards, ~Sumit. The driver has to evolve to handle properly the new HW capabilities while keeping the old stuff working. We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line. Then you need to hack your driver but not the hwmod data:-( Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tue, Feb 15, 2011 at 10:37:37AM +0200, Tomi Valkeinen wrote: This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? IMO, for omap3 it would be better to have irq_chip there. Then you can keep e.g. DISPC IRQ disabled until dispc.c calls request_irq(). What happens today if you have IRQ enabled but dispc isn't ready to act on those ? On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. What if another HW requests the wrong IRQ number and it ends up being your dispc IRQ line ? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
On Tue, 2011-02-15 at 04:57 -0600, Balbi, Felipe wrote: Hi, On Tue, Feb 15, 2011 at 10:37:37AM +0200, Tomi Valkeinen wrote: This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? IMO, for omap3 it would be better to have irq_chip there. Then you can keep e.g. DISPC IRQ disabled until dispc.c calls request_irq(). What happens today if you have IRQ enabled but dispc isn't ready to act on those ? Currently we have a single interrupt handler, which then calls either dispc and/or dsi handler. Dispc and dsi are always ready to handle those. I don't think it would be a good solution if irq_chip would be used only for omap3. Then we'd have totally different solutions for different omap versions. But if irq_chip can be easily used for all omap versions, then perhaps. But then again, using IRQF_SHARED should (I think) solve the problem quite easily without big changes to the code. irq_chip sounds like a bigger change. On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. What if another HW requests the wrong IRQ number and it ends up being your dispc IRQ line ? Are you asking what happens if we have a bug in kernel code? Anything can happen =). But I don't see that as a reason not to use IRQF_SHARED. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Tue, Feb 15, 2011 at 01:25:34PM +0200, Tomi Valkeinen wrote: On Tue, Feb 15, 2011 at 10:37:37AM +0200, Tomi Valkeinen wrote: This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers? That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean? IMO, for omap3 it would be better to have irq_chip there. Then you can keep e.g. DISPC IRQ disabled until dispc.c calls request_irq(). What happens today if you have IRQ enabled but dispc isn't ready to act on those ? Currently we have a single interrupt handler, which then calls either dispc and/or dsi handler. Dispc and dsi are always ready to handle those. Exactly the kind of thing irq_chip would help you :-) I don't think it would be a good solution if irq_chip would be used only for omap3. Then we'd have totally different solutions for different omap versions. But if irq_chip can be easily used for all omap versions, then perhaps. But the HW is different anyway. On OMAP3 you're connect to a DSS irq demuxer, on OMAP4 you have dedicated lines straight from OMAP's INTC. On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4. What if another HW requests the wrong IRQ number and it ends up being your dispc IRQ line ? Are you asking what happens if we have a bug in kernel code? Anything can happen =). But I don't see that as a reason not to use IRQF_SHARED. ok, I have to agree. :-) But you will be allowing for that to happen. If you don't IRQF_SHARED, request_irq() will fail on the second call with -EBUSY I believe. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, snip Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities. So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change. I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like mentioned in Tomi's sample code above. OK, thanks for the clarification, actually I missed a little bit the point :-( So in fact the 2 modules share that same IRQ today, and you just want to populate both hwmod with the same input. If this is a real OR between the two IRQ lines, meaning the dispc cannot mask the dsi IRQ or the opposite, then having the same IRQ number in the two different hwmods is a correct representation of the HW. There is a real OR between the 2 irq lines in OMAP3, as there is no DSS_IRQENABLE, but there is a DSS_IRQSTATUS. You can mask one of DISPC or DSI by zeroing all the bits in DISPC_IRQENABLE or DSI_IRQENABLE respectively. But there is no higher level register to mask them. snip Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
On 2/15/2011 1:43 PM, Taneja, Archit wrote: Hi, snip Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines. Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities. So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change. I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like mentioned in Tomi's sample code above. OK, thanks for the clarification, actually I missed a little bit the point :-( So in fact the 2 modules share that same IRQ today, and you just want to populate both hwmod with the same input. If this is a real OR between the two IRQ lines, meaning the dispc cannot mask the dsi IRQ or the opposite, then having the same IRQ number in the two different hwmods is a correct representation of the HW. There is a real OR between the 2 irq lines in OMAP3, as there is no DSS_IRQENABLE, but there is a DSS_IRQSTATUS. You can mask one of DISPC or DSI by zeroing all the bits in DISPC_IRQENABLE or DSI_IRQENABLE respectively. But there is no higher level register to mask them. That's perfect then, and it deserves the duplication of this irq number for both hwmods. Regards, Benoit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Wed, 2011-02-02 at 08:56 +, 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_() 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. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Mon, Feb 14, 2011 at 04:21:47PM +0200, Tomi Valkeinen wrote: On Wed, 2011-02-02 at 08:56 +, 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_() 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. [1] http://gitorious.org/usb/usb/commits/twlirq -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
Hi, On Monday 14 February 2011 08:00 PM, 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 +, 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_() 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? Could you elaborate this a bit more? 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. The code can't be exactly the same. The DSS_IRQSTATUS register used on OMAP3 doesn't exist on OMAP4. A read to this register on OMAP4 would cause a hang/crash. 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. Will take care of this from now on. 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. We could disable dsi IRQs by masking all the possible interrupt events in DSI_IRQSTATUS. The same goes for dispc. Is this what you meant by masking/unmasking irqs? snip Regards, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
On Mon, 2011-02-14 at 22:28 -0600, Taneja, Archit wrote: Hi, On Monday 14 February 2011 08:00 PM, 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 +, 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_() 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? Could you elaborate this a bit more? I meant something like this: dispc.c: dispc_init() { /* did we have a pdev for dispc? if not, this needs to be dss.pdev */ request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, dispc irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } dsi.c: dsi_init() { request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, dsi irq, foo); } irq_handler() { if (irq_can_be_shared) { check if the irq is for us. exit if not; } handle; } 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. The code can't be exactly the same. The DSS_IRQSTATUS register used on OMAP3 doesn't exist on OMAP4. A read to this register on OMAP4 would cause a hang/crash. Ok, we need a dss_feature bit for this then. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP: DSS2: Common IRQ handler for all OMAPs
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 +, 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_() 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] OMAP: DSS2: Common IRQ handler for all OMAPs
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. Signed-off-by: Archit Taneja arc...@ti.com --- Note: Applies over a) v10 of OMAP2,3 DSS2 HWMOD b)v3 of DSS2: Generalize clock names and c) v3 of DSS2: OMAP4 DSS HWMOD : https://patchwork.kernel.org/patch/500191/ https://patchwork.kernel.org/patch/520191/ https://patchwork.kernel.org/patch/511211/ drivers/video/omap2/dss/dss.c | 46 +-- drivers/video/omap2/dss/dss_features.c |5 ++- drivers/video/omap2/dss/dss_features.h | 17 ++- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c index c7cdbea..24d6f98 100644 --- a/drivers/video/omap2/dss/dss.c +++ b/drivers/video/omap2/dss/dss.c @@ -33,6 +33,7 @@ #include plat/display.h #include plat/clock.h #include dss.h +#include dss_features.h #define DSS_SZ_REGSSZ_512 @@ -61,6 +62,7 @@ static struct { struct platform_device *pdev; void __iomem*base; int ctx_id; + int irq; struct clk *dpll4_m4_ck; struct clk *dss_ick; @@ -494,28 +496,22 @@ found: return 0; } - - -static irqreturn_t dss_irq_handler_omap2(int irq, void *arg) -{ - dispc_irq_handler(); - - return IRQ_HANDLED; -} - -static irqreturn_t dss_irq_handler_omap3(int irq, void *arg) +static irqreturn_t dss_irq_handler(int irq, void *arg) { - u32 irqstatus; + if (dss_has_feature(FEAT_COMMON_IRQ_DISPC_DSI)) { + u32 irqstatus; - irqstatus = dss_read_reg(DSS_IRQSTATUS); + irqstatus = dss_read_reg(DSS_IRQSTATUS); - if (irqstatus (10)) /* DISPC_IRQ */ - dispc_irq_handler(); + if (irqstatus (10)) /* DISPC_IRQ */ + dispc_irq_handler(); #ifdef CONFIG_OMAP2_DSS_DSI - if (irqstatus (11)) /* DSI_IRQ */ - dsi_irq_handler(); + if (irqstatus (11)) /* DSI_IRQ */ + dsi_irq_handler(); #endif - + } else { + dispc_irq_handler(); + } return IRQ_HANDLED; } @@ -563,7 +559,7 @@ void dss_set_dac_pwrdn_bgz(bool enable) static int dss_init(bool skip_init) { - int r, dss_irq; + int r; u32 rev; struct resource *dss_mem; @@ -609,18 +605,14 @@ static int dss_init(bool skip_init) REG_FLD_MOD(DSS_CONTROL, 0, 2, 2); /* venc clock mode = normal */ #endif - dss_irq = platform_get_irq(dss.pdev, 0); - if (dss_irq 0) { + dss.irq = platform_get_irq(dss.pdev, 0); + if (dss.irq 0) { DSSERR(omap2 dss: platform_get_irq failed\n); r = -ENODEV; goto fail1; } - r = request_irq(dss_irq, - cpu_is_omap24xx() - ? dss_irq_handler_omap2 - : dss_irq_handler_omap3, - 0, OMAP DSS, NULL); + r = request_irq(dss.irq, dss_irq_handler, 0, OMAP DSS, NULL); if (r 0) { DSSERR(omap2 dss: request_irq failed\n); @@ -648,7 +640,7 @@ static int dss_init(bool skip_init) return 0; fail2: - free_irq(dss_irq, NULL); + free_irq(dss.irq, NULL); fail1: iounmap(dss.base); fail0: @@ -660,7 +652,7 @@ static void dss_exit(void) if (cpu_is_omap34xx()) clk_put(dss.dpll4_m4_ck); - free_irq(INT_24XX_DSS_IRQ, NULL); + free_irq(dss.irq, NULL); iounmap(dss.base); } diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c index cf3ef69..f3ef929 100644 --- a/drivers/video/omap2/dss/dss_features.c +++ b/drivers/video/omap2/dss/dss_features.c @@ -157,7 +157,7 @@ static struct omap_dss_features omap3430_dss_features = { .has_feature= FEAT_GLOBAL_ALPHA | FEAT_LCDENABLEPOL | FEAT_LCDENABLESIGNAL | FEAT_PCKFREEENABLE | - FEAT_FUNCGATED, + FEAT_FUNCGATED | FEAT_COMMON_IRQ_DISPC_DSI, .num_mgrs = 2, .num_ovls = 3, @@ -172,7 +172,8 @@ static struct omap_dss_features omap3630_dss_features = { .has_feature= FEAT_GLOBAL_ALPHA | FEAT_LCDENABLEPOL | FEAT_LCDENABLESIGNAL | FEAT_PCKFREEENABLE | - FEAT_PRE_MULT_ALPHA | FEAT_FUNCGATED, + FEAT_PRE_MULT_ALPHA | FEAT_FUNCGATED | + FEAT_COMMON_IRQ_DISPC_DSI, .num_mgrs = 2, .num_ovls = 3, diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h index b9c70be..1c93a49