[alsa-devel] [PATCH] ASoC: samsung - Don't setup i2s if already active.
If the dai is already running when setup is called, skip setup. Setting up again caused set_sysclk to set rclk_srcrate to other->rclk_srcrate, other would often be set to the default of 44.1. Playing a 48k stream then adding a 48k capture stream would lead to both streams set to 44.1 (The value of other->rclk_srcrate).
Similarly in shutdown, if either playback or capture is still active, return instead of turning off the clocks.
Signed-off-by: Dylan Reid dgreid@chromium.org --- sound/soc/samsung/i2s.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 40b00a1..b9935dd 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -637,6 +637,10 @@ static int i2s_startup(struct snd_pcm_substream *substream, struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai; unsigned long flags;
+ /* Check not already running. */ + if (dai->playback_active || dai->capture_active) + return 0; + spin_lock_irqsave(&lock, flags);
i2s->mode |= DAI_OPENED; @@ -661,6 +665,10 @@ static void i2s_shutdown(struct snd_pcm_substream *substream, struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai; unsigned long flags;
+ /* Check not still running. */ + if (dai->playback_active || dai->capture_active) + return; + spin_lock_irqsave(&lock, flags);
i2s->mode &= ~DAI_OPENED;
On Thu, Nov 08, 2012 at 02:46:56PM -0800, Dylan Reid wrote:
If the dai is already running when setup is called, skip setup. Setting up again caused set_sysclk to set rclk_srcrate to other->rclk_srcrate, other would often be set to the default of 44.1. Playing a 48k stream then adding a 48k capture stream would lead to both streams set to 44.1 (The value of other->rclk_srcrate).
This doesn't seem the obvious fix here - surely if the setup comes out with the wrong answer the configuration it was asked for doesn't match the configuration the device currently has and therefore we should return an error as the new stream will have an incorrect setup?
Really this driver should probably be being redone in terms of the DPCM code, it predates it and is a very simple case of it but it's in the same ballpark feature wise.
Similarly in shutdown, if either playback or capture is still active, return instead of turning off the clocks.
This seems sensible; another option is to do clk_enable() for both streams then let the clock framework refcount for us.
On Fri, Nov 9, 2012 at 8:08 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, Nov 08, 2012 at 02:46:56PM -0800, Dylan Reid wrote:
If the dai is already running when setup is called, skip setup. Setting up again caused set_sysclk to set rclk_srcrate to other->rclk_srcrate, other would often be set to the default of 44.1. Playing a 48k stream then adding a 48k capture stream would lead to both streams set to 44.1 (The value of other->rclk_srcrate).
This doesn't seem the obvious fix here - surely if the setup comes out with the wrong answer the configuration it was asked for doesn't match the configuration the device currently has and therefore we should return an error as the new stream will have an incorrect setup?
Thanks Mark, sorry I didn't explain this better. It is getting the correct answer. However if the same format is requested for another stream, startup will clear rclk_srcrate. rclk_srcrate being cleared causes i2s_set_fmt to call i2s_set_sysclk. i2s_set_sysclk assumes it should use the "other" setting for rclk_srcrate (because something is active).
for example:
aplay -Dhw:0 some_48k_file.wav followed by arecord -Dhw:0 -f dat /tmp/foo.wav
both 48k streams, but when the arecord is started, the aplay will have its rate set to what ever the unused secondary DAI is using (initialized to 44.1).
Really this driver should probably be being redone in terms of the DPCM code, it predates it and is a very simple case of it but it's in the same ballpark feature wise.
That looks promising, not only for this but for a few other issues. I'll take investigate some more, thanks for the suggestion.
Similarly in shutdown, if either playback or capture is still active, return instead of turning off the clocks.
This seems sensible; another option is to do clk_enable() for both streams then let the clock framework refcount for us.
participants (2)
-
Dylan Reid -
Mark Brown