[alsa-devel] [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Ambrose, Martin]

On Fri, Sep 24, 2010 at 14:14:32, Troy Kisky wrote:

> On 9/24/2010 9:48 AM, Ambrose, Martin wrote:
> > On Sat, Jul 04, 2009 at 21:29:59, Troy Kisky wrote:
> > 
> >> If the codec is master then prepare should call
> >> mcbsp_start, not trigger.
> > 
> > For me this change induces audio artifacts when starting or stopping a gstreamer pipeline.
> > Take, e.g., the pipeline
> > 	gst-launch -v filesrc location=file.mp3 ! mad ! alsasink
> > 
> > In the setup, before the file is opened, gstreamer places the pipeline into the PAUSE state.
> > 	... posting state-changed READY to PAUSED
> > 
> > This results in a call to the davinci_i2s_prepare function which, when the codec is master,
> > stops then starts the mcbsp driver. However at this point in the pipeline no valid data has
> > been provided to the DMA layer (davinci-pcm). Consequently there is random noise of somewhat
> > random length played out the DAC at the start of file playback. Moreover when the stream changes
> > from the PAUSE to the PLAYING state another mcbsp_stop/mcbsp_start sequence is performed which
> > seems inefficient. Similar behavior, and noise, occurs when the pipeline is shutdown.
> > 
> > The aplay utility has a simpler startup/shutdown procedure which doesn't produce any noise.
> > 
> > There doesn't seem to be any state context, e.g. pause vs start, provided to the prepare
> > function so I can't see how to handle such cases in the current driver while preserving
> > the use of this patch. If I revert this patch then I don't get any noise but I don't 
> > know what the consequences are. 
> > 
> > Comments/suggestions welcome.
> > 
> 
> I don't see the problem because my codec waits until trigger to activate its interface.

Thanks for the feedback. I'm using the tlv320aic3x codec driver. Which codec are you using?

> So the question is, who should be responsible for the final turn on?
> 
> My thought was that the device (master) which starts the external wires to wiggling should be last.

Fair enough. I suppose that is the reasoning for the addition of the prepare function.
The assumption being that a cpu_dai callback to the prepare function will always proceed
a call to the codec_dai trigger function. In this way the serial port can be configured and enabled.
Then the codec can turn on the bit/frame clocks which will start the flow of data. 
Although it would seem the codec is unmuted a bit prematurely in this scenario since it happens
before the clocks are enabled in the core prepare function -- at least I think this is the case.

However there would still seem to be spurious, or at least superfluous, calls
to mcbsp_stop/start when just requesting the device to enter the pause state.

I guess the call tree is then different in the case where the cpu, or machine, is the 
clock master. This has pros/cons obviously.

> If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop
> or noise.

I still a newbie when it comes to the ALSA architecture.
Is the proper signal indicating everything is ready, including valid data, a trigger call with cmd=START?

> It would be nice if such rules could be documented, are they???

Not that I know of. 

Regards,
Martin