[Sound-open-firmware] [PATCH] cnl: gp-dma: fix gp-dma dead loop issue

[Wang, Rander]

Hi Liam,

	See my comments

Thank you！
Rander
> -----Original Message-----
> From: Liam Girdwood [mailto:liam.r.girdwood at linux.intel.com]
> Sent: Wednesday, February 28, 2018 5:23 PM
> To: Wang, Rander <rander.wang at intel.com>;
> sound-open-firmware at 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 at 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