[Sound-open-firmware] [PATCH] cnl: gp-dma: fix gp-dma dead loop issue
In irq function, two dma would be checked which one is the interrupted one, so get it after check. Signed-off-by: Rander Wang <rander.wang@intel.com> --- src/drivers/dw-dma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index be19e12..6fd46e0 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -1034,7 +1034,7 @@ static int dw_dma_probe(struct dma *dma) static void dw_dma_irq_handler(void *data) { struct dma *dma = (struct dma *)data; - struct dma_pdata *p = dma_get_drvdata(dma); + struct dma_pdata *p; struct dma_sg_elem next; uint32_t status_tfr = 0; uint32_t status_block = 0; @@ -1058,6 +1058,7 @@ static void dw_dma_irq_handler(void *data) } tracev_dma("DIr"); + p = dma_get_drvdata(dma); /* get the source of our IRQ. */ status_block = dw_read(dma, DW_STATUS_BLOCK); -- 2.14.1
On Wed, 2018-02-28 at 16:52 +0800, Rander Wang wrote:
In irq function, two dma would be checked which one is the interrupted one, so get it after check.
Signed-off-by: Rander Wang <rander.wang@intel.com> --- src/drivers/dw-dma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index be19e12..6fd46e0 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -1034,7 +1034,7 @@ static int dw_dma_probe(struct dma *dma) static void dw_dma_irq_handler(void *data) { struct dma *dma = (struct dma *)data; - struct dma_pdata *p = dma_get_drvdata(dma); + struct dma_pdata *p; struct dma_sg_elem next; uint32_t status_tfr = 0; uint32_t status_block = 0; @@ -1058,6 +1058,7 @@ static void dw_dma_irq_handler(void *data) }
tracev_dma("DIr"); + p = dma_get_drvdata(dma);
/* get the source of our IRQ. */ status_block = dw_read(dma, DW_STATUS_BLOCK);
Iiuc, the IRQ for both DMACs on CNL is shared ? and we determine which DMAC has the IRQ by reading each status register above ? If so, this fails when both DMACs have IRQs at the same time. Please do something like :- dw_dma_irq_handler_cnl() { dma = dmac0; /* read status for DMAC0 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma); dma = dmac1; /* read status for DMAC1 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma); } Liam
Hi Liam, See my comments Thank you！ Rander
-----Original Message----- From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com] Sent: Wednesday, February 28, 2018 5:23 PM To: Wang, Rander <rander.wang@intel.com>; sound-open-firmware@alsa-project.org Subject: Re: [Sound-open-firmware] [PATCH] cnl: gp-dma: fix gp-dma dead loop issue
On Wed, 2018-02-28 at 16:52 +0800, Rander Wang wrote:
In irq function, two dma would be checked which one is the interrupted one, so get it after check.
Signed-off-by: Rander Wang <rander.wang@intel.com> --- src/drivers/dw-dma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index be19e12..6fd46e0 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -1034,7 +1034,7 @@ static int dw_dma_probe(struct dma *dma) static void dw_dma_irq_handler(void *data) { struct dma *dma = (struct dma *)data; - struct dma_pdata *p = dma_get_drvdata(dma); + struct dma_pdata *p; struct dma_sg_elem next; uint32_t status_tfr = 0; uint32_t status_block = 0; @@ -1058,6 +1058,7 @@ static void dw_dma_irq_handler(void *data) }
tracev_dma("DIr"); + p = dma_get_drvdata(dma);
/* get the source of our IRQ. */ status_block = dw_read(dma, DW_STATUS_BLOCK);
Iiuc, the IRQ for both DMACs on CNL is shared ? and we determine which DMAC has the IRQ by reading each status register above ?
If so, this fails when both DMACs have IRQs at the same time.
If only one core is enabled, the interrupt with the same irq level would trigger Irq function two times? And if multicore is enabled, if two core received the interrupt at the same time, Maybe something wrong?
Please do something like :-
dw_dma_irq_handler_cnl() { dma = dmac0;
/* read status for DMAC0 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma);
dma = dmac1;
/* read status for DMAC1 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma); }
Liam
On Thu, 2018-03-01 at 08:59 +0000, Wang, Rander wrote:
Hi Liam,
See my comments
Thank you！ Rander
-----Original Message----- From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com] Sent: Wednesday, February 28, 2018 5:23 PM To: Wang, Rander <rander.wang@intel.com>; sound-open-firmware@alsa-project.org Subject: Re: [Sound-open-firmware] [PATCH] cnl: gp-dma: fix gp-dma dead loop issue
On Wed, 2018-02-28 at 16:52 +0800, Rander Wang wrote:
In irq function, two dma would be checked which one is the interrupted one, so get it after check.
Signed-off-by: Rander Wang <rander.wang@intel.com> --- src/drivers/dw-dma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index be19e12..6fd46e0 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -1034,7 +1034,7 @@ static int dw_dma_probe(struct dma *dma)
static
void dw_dma_irq_handler(void *data) { struct dma *dma = (struct dma *)data; - struct dma_pdata *p = dma_get_drvdata(dma); + struct dma_pdata *p; struct dma_sg_elem next; uint32_t status_tfr = 0; uint32_t status_block = 0; @@ -1058,6 +1058,7 @@ static void dw_dma_irq_handler(void *data) }
tracev_dma("DIr"); + p = dma_get_drvdata(dma);
/* get the source of our IRQ. */ status_block = dw_read(dma, DW_STATUS_BLOCK);
Iiuc, the IRQ for both DMACs on CNL is shared ? and we determine which DMAC has the IRQ by reading each status register above ?
If so, this fails when both DMACs have IRQs at the same time.
If only one core is enabled, the interrupt with the same irq level would trigger Irq function two times?
No, the IRQ line is ORed between both DMACs. Either or both will cause and IRQ. Both status register must be read though.
And if multicore is enabled, if two core received the interrupt at the same time, Maybe something wrong?
That is possible, but the IRQ should be masked in other cores. Btw, this is wrong too:- static int dw_dma_probe(struct dma *dma) { struct dma_pdata *dw_pdata; struct dma *dmac = dma; int i; #ifdef CONFIG_CANNONLAKE int j; for (j = 0; j < MAX_GPDMA_COUNT; j++) #endif { We dont need this for loop. dw_dma_probe () will be called in platform.c for each DMAC.
Please do something like :-
dw_dma_irq_handler_cnl() { dma = dmac0;
/* read status for DMAC0 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma);
dma = dmac1;
/* read status for DMAC1 */ status_intr = dw_read(dma, DW_INTR_STATUS); if (status_intr) dw_dma_irq_handler(dma); }
Liam
participants (3)
-
Liam Girdwood -
Rander Wang -
Wang, Rander