Re: [RFC][PATCH 3/3 v2] ASoC: OMAP: Enhance OMAP1510 DMA progress software counter

[Janusz Krzysztofik]

Hi Jarkko,

Tuesday 11 August 2009 08:42:48 Jarkko Nikula wrote:
> On Tue, 11 Aug 2009 01:07:10 +0200
> Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx> wrote:
> > Enhance period index accuracy, particularly just before buffer rewind, by
> > making use of DMA interrupt status flags in addition to simply counting
> > up interrupts.
> >
> > Corrections since v1:
> > 1. Fix buggy DMA interrupt handling in case of multiple status flags set
> > in parallel.
> > 2. Enable OMAP_DMA_LAST_IRQ only for playback on OMAP1510.
> >
> > -	}
> > +	} else if (stat == OMAP_DMA_LAST_IRQ)
> > +		return;
>
> Is this test needed? This interrupt is set only for playback on
> omap1510 so this looks null-op.

You're right, I have put it here before limiting the flag request to playback 
on OMAP1510 only. So it can be omitted, but only if we assume we are always 
called exactly as requested. It is not needed unless the flag is ever 
requested (by mistake?) by other arch/arm/plat-omap/dma.c bits and passed on 
by omap1_dma_handle_ch() that passes all flags without checking if requested. 
But then, shouldn't we start from always checking for OMAP_DMA_FRAME_IRQ set?

> > -	omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ);
> > +	if ((cpu_is_omap1510()) &&
> > +			(substream->stream == SNDRV_PCM_STREAM_PLAYBACK))
> > +		omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ |
> > +							OMAP_DMA_LAST_IRQ);
>
> Indent OMAP_DMA_LAST_IRQ with tab(s) and spaces to the same column
> than OMAP_DMA_FRAME_IRQ. Looks nicer then.

OK, will fix it.

> Should the 
> OMAP_DMA_BLOCK_IRQ to be set since it is handled in omap_pcm_dma_irq?

This one is already requested from inside omap_request_dma() and used inside 
omap1_dma_handle_ch() in addition to passing it to us.

Cheers,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/