Re: [PATCH] davinci: edma: clear interrupt status for interrupt enabled channels only
Nori, Sekhar nsek...@ti.com writes: On Fri, Mar 12, 2010 at 23:33:10, Kevin Hilman wrote: Sekhar Nori nsek...@ti.com writes: From: Anuj Aggarwal anuj.aggar...@ti.com Currently, the ISR in the EDMA driver clears the pending interrupt for all channels without regard to whether that channel has a registered callback or not. This causes problems for devices like DM355/DM365 where the multimedia accelerator uses EDMA by polling on the interrupt pending bits of some of the EDMA channels. Since these channels are actually allocated through the Linux EDMA driver (by an out-of-kernel module), the same shadow region is used by Linux and accelerator. There a race between the Linux ISR and the polling code running on the accelerator on the IPR (interrupt pending register). This patch fixes the issue by making the ISR clear the interrupts only for those channels which have interrupt enabled. The channels which are allocated for the purpose of being polled on by the accelerator will not have a callback function provided and so will not have IER (interrupt enable register) bits set. Dumb question: why cat the MM accelerator use the normal ISR + callback? There is some OS-less code running on the accelerator. I believe (at least some parts of) the accelerator cannot even see the DDR so having an OS implemented there seems far fetched. Without an OS in place, it is easier to poll on EDMA IPR bits. Having Linux handle the interrupt brings in IPC mechanisms which again needs an OS running on the other side and is also seen as detrimental to performance. OK, thanks for the clarification. applying, and queueing in davinci-next for 2.6.35 Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH] davinci: edma: clear interrupt status for interrupt enabled channels only
On Fri, Mar 12, 2010 at 23:33:10, Kevin Hilman wrote: Sekhar Nori nsek...@ti.com writes: From: Anuj Aggarwal anuj.aggar...@ti.com Currently, the ISR in the EDMA driver clears the pending interrupt for all channels without regard to whether that channel has a registered callback or not. This causes problems for devices like DM355/DM365 where the multimedia accelerator uses EDMA by polling on the interrupt pending bits of some of the EDMA channels. Since these channels are actually allocated through the Linux EDMA driver (by an out-of-kernel module), the same shadow region is used by Linux and accelerator. There a race between the Linux ISR and the polling code running on the accelerator on the IPR (interrupt pending register). This patch fixes the issue by making the ISR clear the interrupts only for those channels which have interrupt enabled. The channels which are allocated for the purpose of being polled on by the accelerator will not have a callback function provided and so will not have IER (interrupt enable register) bits set. Dumb question: why cat the MM accelerator use the normal ISR + callback? There is some OS-less code running on the accelerator. I believe (at least some parts of) the accelerator cannot even see the DDR so having an OS implemented there seems far fetched. Without an OS in place, it is easier to poll on EDMA IPR bits. Having Linux handle the interrupt brings in IPC mechanisms which again needs an OS running on the other side and is also seen as detrimental to performance. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] davinci: edma: clear interrupt status for interrupt enabled channels only
Sekhar Nori nsek...@ti.com writes:
From: Anuj Aggarwal anuj.aggar...@ti.com
Currently, the ISR in the EDMA driver clears the pending interrupt for all
channels without regard to whether that channel has a registered callback
or not.
This causes problems for devices like DM355/DM365 where the multimedia
accelerator uses EDMA by polling on the interrupt pending bits of some of the
EDMA channels. Since these channels are actually allocated through the Linux
EDMA driver (by an out-of-kernel module), the same shadow region is used by
Linux and accelerator. There a race between the Linux ISR and the polling code
running on the accelerator on the IPR (interrupt pending register).
This patch fixes the issue by making the ISR clear the interrupts only for
those channels which have interrupt enabled. The channels which are allocated
for the purpose of being polled on by the accelerator will not have a callback
function provided and so will not have IER (interrupt enable register) bits
set.
Dumb question: why cat the MM accelerator use the normal ISR + callback?
Kevin
Tested on DM365 and OMAP-L137/L138 with audio and MMC/SD (as EDMA users).
Signed-off-by: Anuj Aggarwal anuj.aggar...@ti.com
Signed-off-by: Sekhar Nori nsek...@ti.com
CC: Archith John Bency arch...@ti.com
---
arch/arm/mach-davinci/dma.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index 15dd886..ea74a90 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -358,9 +358,11 @@ static irqreturn_t dma_irq_handler(int irq, void *data)
while (1) {
int j;
- if (edma_shadow0_read_array(ctlr, SH_IPR, 0))
+ if (edma_shadow0_read_array(ctlr, SH_IPR, 0)
+ edma_shadow0_read_array(ctlr, SH_IER, 0))
j = 0;
- else if (edma_shadow0_read_array(ctlr, SH_IPR, 1))
+ else if (edma_shadow0_read_array(ctlr, SH_IPR, 1)
+ edma_shadow0_read_array(ctlr, SH_IER, 1))
j = 1;
else
break;
@@ -368,8 +370,9 @@ static irqreturn_t dma_irq_handler(int irq, void *data)
edma_shadow0_read_array(ctlr, SH_IPR, j));
for (i = 0; i 32; i++) {
int k = (j 5) + i;
- if (edma_shadow0_read_array(ctlr, SH_IPR, j)
- (1 i)) {
+ if ((edma_shadow0_read_array(ctlr, SH_IPR, j) BIT(i))
+ (edma_shadow0_read_array(ctlr,
+ SH_IER, j) BIT(i))) {
/* Clear the corresponding IPR bits */
edma_shadow0_write_array(ctlr, SH_ICR, j,
(1 i));
--
1.6.2.4
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] davinci: edma: clear interrupt status for interrupt enabled channels only
From: Anuj Aggarwal anuj.aggar...@ti.com
Currently, the ISR in the EDMA driver clears the pending interrupt for all
channels without regard to whether that channel has a registered callback
or not.
This causes problems for devices like DM355/DM365 where the multimedia
accelerator uses EDMA by polling on the interrupt pending bits of some of the
EDMA channels. Since these channels are actually allocated through the Linux
EDMA driver (by an out-of-kernel module), the same shadow region is used by
Linux and accelerator. There a race between the Linux ISR and the polling code
running on the accelerator on the IPR (interrupt pending register).
This patch fixes the issue by making the ISR clear the interrupts only for
those channels which have interrupt enabled. The channels which are allocated
for the purpose of being polled on by the accelerator will not have a callback
function provided and so will not have IER (interrupt enable register) bits set.
Tested on DM365 and OMAP-L137/L138 with audio and MMC/SD (as EDMA users).
Signed-off-by: Anuj Aggarwal anuj.aggar...@ti.com
Signed-off-by: Sekhar Nori nsek...@ti.com
CC: Archith John Bency arch...@ti.com
---
arch/arm/mach-davinci/dma.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index 15dd886..ea74a90 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -358,9 +358,11 @@ static irqreturn_t dma_irq_handler(int irq, void *data)
while (1) {
int j;
- if (edma_shadow0_read_array(ctlr, SH_IPR, 0))
+ if (edma_shadow0_read_array(ctlr, SH_IPR, 0)
+ edma_shadow0_read_array(ctlr, SH_IER, 0))
j = 0;
- else if (edma_shadow0_read_array(ctlr, SH_IPR, 1))
+ else if (edma_shadow0_read_array(ctlr, SH_IPR, 1)
+ edma_shadow0_read_array(ctlr, SH_IER, 1))
j = 1;
else
break;
@@ -368,8 +370,9 @@ static irqreturn_t dma_irq_handler(int irq, void *data)
edma_shadow0_read_array(ctlr, SH_IPR, j));
for (i = 0; i 32; i++) {
int k = (j 5) + i;
- if (edma_shadow0_read_array(ctlr, SH_IPR, j)
- (1 i)) {
+ if ((edma_shadow0_read_array(ctlr, SH_IPR, j) BIT(i))
+(edma_shadow0_read_array(ctlr,
+ SH_IER, j) BIT(i))) {
/* Clear the corresponding IPR bits */
edma_shadow0_write_array(ctlr, SH_ICR, j,
(1 i));
--
1.6.2.4
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
