[alsa-devel] ASoC: Merging at91 and avr32 dirs - test report
I tested the patches and got everything working with minor changes.
However, there seems to be some problem with PM, occasionally ASoC is stuck with Standby mode (looking at dapm_widgets) when I start the playback. Restarting the player brings the sound back. I'll carry out some further investigation and get back to this.
My kernel is 2.6.27-rc3, and machine driver is adapted from playpaq_wm8510.c to run on at91rm9200 board.
The SSC peripheral clock was disabled more often than it was enabled, and that catched the bug trap in arch/arm/mach-at91/clock.c. I'm not sure if this was the real problem, but it's the way I managed to solve it.
-Jukka
diff -uprN a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c --- a/sound/soc/atmel/atmel_ssc_dai.c 2008-10-10 17:23:46.000000000 +0300 +++ b/sound/soc/atmel/atmel_ssc_dai.c 2008-10-10 17:26:06.000000000 +0300 @@ -261,11 +261,11 @@ static void atmel_ssc_shutdown(struct sn spin_lock_irq(&ssc_p->lock); ssc_p->dir_mask &= ~dir_mask; if (!ssc_p->dir_mask) { - /* Shutdown the SSC clock. */ - pr_debug("atmel_ssc_dau: Stopping clock\n"); - clk_disable(ssc_p->ssc->clk); - if (ssc_p->initialized) { + /* Shutdown the SSC clock. */ + pr_debug("atmel_ssc_dai: Stopping clock\n"); + clk_disable(ssc_p->ssc->clk); + free_irq(ssc_p->ssc->irq, ssc_p); ssc_p->initialized = 0; }
On Fri, Oct 10, 2008 at 06:15:17PM +0300, ext-jukka.hynninen@vaisala.com wrote:
However, there seems to be some problem with PM, occasionally ASoC is stuck with Standby mode (looking at dapm_widgets) when I start the playback. Restarting the player brings the sound back. I'll carry out some further investigation and get back to this.
How does this compare with the behaviour of the original at32 machine support - did that have similar issues?
The SSC peripheral clock was disabled more often than it was enabled, and that catched the bug trap in arch/arm/mach-at91/clock.c. I'm not sure if this was the real problem, but it's the way I managed to solve it.
Looks reasonable to me.
diff -uprN a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c --- a/sound/soc/atmel/atmel_ssc_dai.c 2008-10-10 17:23:46.000000000 +0300 +++ b/sound/soc/atmel/atmel_ssc_dai.c 2008-10-10 17:26:06.000000000 +0300 @@ -261,11 +261,11 @@ static void atmel_ssc_shutdown(struct sn spin_lock_irq(&ssc_p->lock); ssc_p->dir_mask &= ~dir_mask; if (!ssc_p->dir_mask) {
/* Shutdown the SSC clock. */pr_debug("atmel_ssc_dau: Stopping clock\n");clk_disable(ssc_p->ssc->clk);if (ssc_p->initialized) {
/* Shutdown the SSC clock. */pr_debug("atmel_ssc_dai: Stopping clock\n");clk_disable(ssc_p->ssc->clk);free_irq(ssc_p->ssc->irq, ssc_p); ssc_p->initialized = 0; }
Quoting Mark Brown:
However, there seems to be some problem with PM, occasionally ASoC is stuck with Standby mode (looking at dapm_widgets) when I start the playback. Restarting the player brings the sound back. I'll carry out some further investigation and get back to this.
How does this compare with the behaviour of the original at32 machine support - did that have similar issues?
Actually I was previously using at91 support. Nevertheless, didn't notice such problems.
I'll gather more info and try to figure out what's going wrong.
-Jukka
However, there seems to be some problem with PM, occasionally ASoC
is
stuck with Standby mode (looking at dapm_widgets) when I start the playback. Restarting the player brings the sound back. I'll carry
out
some further investigation and get back to this.
How does this compare with the behaviour of the original at32 machine support - did that have similar issues?
Actually I was previously using at91 support. Nevertheless, didn't
notice
such problems.
This is propably a false alarm, exactly the same happens with at91-ssc.c
The above described behaviour was due to mpg123, which (for some reason) does not wake up system from D3 state. Furthermore, running mpg123 confuses DAPM to think the codec is powered up properly, and waiting for delayed shutdown. Running aplay within anti-pop timeout will result in no-sound situation.
Using mpg123, the soc_pcm_prepare() never gets called, which makes me suspect that the real problem might lie in mpg123 code. A similar issue has been
reported here in the past, but I couldn't find a solution:
http://thread.gmane.org/gmane.linux.alsa.devel/47908
-Jukka
On Tue, Oct 14, 2008 at 03:38:22PM +0300, ext-jukka.hynninen@vaisala.com wrote:
This is propably a false alarm, exactly the same happens with at91-ssc.c
OK, so you're happy that the new and old AT91 drivers perform equivalently?
Using mpg123, the soc_pcm_prepare() never gets called, which makes me suspect that the real problem might lie in mpg123 code. A similar issue has been
The drivers should be robust against buggy applications, though.
reported here in the past, but I couldn't find a solution:
Yes, I think it's the same issue - looks like a core bug which is still present in current ASoC. The patch below is *entirely* untested but may help. It needs more thought and (obviously) testing to make sure it's safe before it can be merged.
[BTW, your mailer is doing some *really* strange line-wrapping.]
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index ad38113..725b30b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -429,51 +429,42 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream) } }
- /* we only want to start a DAPM playback stream if we are not waiting - * on an existing one stopping */ - if (codec_dai->pop_wait) { - /* we are waiting for the delayed work to start */ - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - snd_soc_dapm_stream_event(socdev->codec, - codec_dai->capture.stream_name, - SND_SOC_DAPM_STREAM_START); - else { - codec_dai->pop_wait = 0; - cancel_delayed_work(&socdev->delayed_work); - snd_soc_dai_digital_mute(codec_dai, 0); - } - } else { - /* no delayed work - do we need to power up codec */ - if (codec->bias_level != SND_SOC_BIAS_ON) { + /* cancel any delayed stream shutdown that is pending */ + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && + codec_dai->pop_wait) { + codec_dai->pop_wait = 0; + cancel_delayed_work(&socdev->delayed_work); + }
- snd_soc_dapm_set_bias_level(socdev, - SND_SOC_BIAS_PREPARE); + /* do we need to power up codec */ + if (codec->bias_level != SND_SOC_BIAS_ON) { + snd_soc_dapm_set_bias_level(socdev, + SND_SOC_BIAS_PREPARE);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - snd_soc_dapm_stream_event(codec, + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + snd_soc_dapm_stream_event(codec, codec_dai->playback.stream_name, SND_SOC_DAPM_STREAM_START); - else - snd_soc_dapm_stream_event(codec, + else + snd_soc_dapm_stream_event(codec, codec_dai->capture.stream_name, SND_SOC_DAPM_STREAM_START);
- snd_soc_dapm_set_bias_level(socdev, SND_SOC_BIAS_ON); - snd_soc_dai_digital_mute(codec_dai, 0); + snd_soc_dapm_set_bias_level(socdev, SND_SOC_BIAS_ON); + snd_soc_dai_digital_mute(codec_dai, 0);
- } else { - /* codec already powered - power on widgets */ - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - snd_soc_dapm_stream_event(codec, + } else { + /* codec already powered - power on widgets */ + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + snd_soc_dapm_stream_event(codec, codec_dai->playback.stream_name, SND_SOC_DAPM_STREAM_START); - else - snd_soc_dapm_stream_event(codec, + else + snd_soc_dapm_stream_event(codec, codec_dai->capture.stream_name, SND_SOC_DAPM_STREAM_START);
- snd_soc_dai_digital_mute(codec_dai, 0); - } + snd_soc_dai_digital_mute(codec_dai, 0); }
out:
Quoting Mark Brown:
This is propably a false alarm, exactly the same happens with
at91-ssc.c
OK, so you're happy that the new and old AT91 drivers perform equivalently?
Life's full of small joys
present in current ASoC. The patch below is *entirely* untested but
may
help. It needs more thought and (obviously) testing to make sure it's safe before it can be merged.
Thanks, works like a charm. I'll put the patch on a test.
-Jukka
Hi, ext-jukka.hynninen@vaisala.com a écrit :
Quoting Mark Brown:
This is propably a false alarm, exactly the same happens with
at91-ssc.c
OK, so you're happy that the new and old AT91 drivers perform equivalently?
Life's full of small joys
present in current ASoC. The patch below is *entirely* untested but
may
help. It needs more thought and (obviously) testing to make sure it's safe before it can be merged.
Thanks, works like a charm. I'll put the patch on a test.
-Jukka _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
I am writing to know what is the current status about the at91/avr32 merging? Is there any progress concerning the testing? What is required for the patch to be accepted?
Regards, Sedji
On Mon, Oct 27, 2008 at 10:57:43AM +0100, Sedji Gaouaou wrote:
I am writing to know what is the current status about the at91/avr32 merging? Is there any progress concerning the testing? What is required for the patch to be accepted?
Jukka seemed happy but we've had no testing at all on AVR32 so far as far as I can see - Geoffrey had been going to test it but his hardware broke. I'd say the best thing now -rc1 is out is to merge the patches as-is and then fix up any further issues that are identified later.
The playpaq support will need a small refresh for the current machine driver, though.
Quoting Mark Brown:
Is there any progress concerning the testing? What is required for the patch to be accepted?
Jukka seemed happy but we've had no testing at all on AVR32 so far as far as I can see - Geoffrey had been going to test it but his
Affirmative, no promlems whatsoever encountered during several days stress testing. At91 & WM8510 runs smoothly.
Just wanted to remind on the change that was needed to avoid the SSC peripheral clock problem. I hope this will also be included.
-Jukka
On Mon, Oct 27, 2008 at 01:13:06PM +0200, ext-jukka.hynninen@vaisala.com wrote:
Just wanted to remind on the change that was needed to avoid the SSC peripheral clock problem. I hope this will also be included.
Which change was this? I suspect we need another round of patches from Sedji and then a final check with that...
Quoting Mark Brown:
Just wanted to remind on the change that was needed to avoid the SSC peripheral clock problem. I hope this will also be included.
Which change was this? I suspect we need another round of patches
from
Sedji and then a final check with that...
http://article.gmane.org/gmane.linux.alsa.devel/56742/
-Jukka
Hi,
ext-jukka.hynninen@vaisala.com a écrit :
I tested the patches and got everything working with minor changes.
However, there seems to be some problem with PM, occasionally ASoC is stuck with Standby mode (looking at dapm_widgets) when I start the playback. Restarting the player brings the sound back. I'll carry out some further investigation and get back to this.
Thank you for your help!
I've done more tests using the oss emulation in ALSA, and I have encountered a problem. In a previous mail Mark you mentioned this:
+/*
- Record SSC clock dividers for use in hw_params().
- */
+static int atmel_ssc_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
- int div_id, int div)
+{
- struct atmel_ssc_info *ssc_p = &ssc_info[cpu_dai->id];
- switch (div_id) {
- case AT91SSC_CMR_DIV:
/** The same master clock divider is used for both* transmit and receive, so if a value has already* been set, it must match this value.*/if (ssc_p->cmr_div == 0)ssc_p->cmr_div = div;elseif (div != ssc_p->cmr_div)return -EBUSY;break;
What happens if the user wants to change the master clock divider at runtime - for example, when changing sample rates?
I had actually a crash because of this, so I will send a patch to correct it(I simply removed it since I don't thonk it is needed).
Regards, Sedji
On Mon, Oct 13, 2008 at 04:22:06PM +0200, Sedji Gaouaou wrote:
ext-jukka.hynninen@vaisala.com a ?crit :
- switch (div_id) {
- case AT91SSC_CMR_DIV:
/** The same master clock divider is used for both* transmit and receive, so if a value has already* been set, it must match this value.*/if (ssc_p->cmr_div == 0)ssc_p->cmr_div = div;elseif (div != ssc_p->cmr_div)return -EBUSY;break;
What happens if the user wants to change the master clock divider at runtime - for example, when changing sample rates?
Usually you'd end up using constraints to prevent user space selecting a different sample rate if it wasn't possible to change safely.
participants (3)
-
ext-jukka.hynninen＠vaisala.com -
Mark Brown -
Sedji Gaouaou