[alsa-devel] [PATCH] ASoC: tlv320aic3x: Fix mono
The TLV320AIC3x DAI supports only stereo. This patch fixes mono support for this codec by allowing only stereo at codec level.
Tested with imx-ssi and TLV320AIC3104.
Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: alsa-devel@alsa-project.org Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- sound/soc/codecs/tlv320aic3x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 5708a97..4989143 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1210,13 +1210,13 @@ static struct snd_soc_dai_driver aic3x_dai = { .name = "tlv320aic3x-hifi", .playback = { .stream_name = "Playback", - .channels_min = 1, + .channels_min = 2, .channels_max = 2, .rates = AIC3X_RATES, .formats = AIC3X_FORMATS,}, .capture = { .stream_name = "Capture", - .channels_min = 1, + .channels_min = 2, .channels_max = 2, .rates = AIC3X_RATES, .formats = AIC3X_FORMATS,},
On Fri, Jan 25, 2013 at 06:19:20PM +0100, Benoît Thébaudeau wrote:
The TLV320AIC3x DAI supports only stereo. This patch fixes mono support for this codec by allowing only stereo at codec level.
So, this doesn't actually fix mono - it removes support for it. It's a bit surprising that this is a requirement actually, the device seems to support DSP modes which usually do mono totally happily (I2S style modes have problems due to the clocking). I'd like to see some confirmation that there's not some other issue in your system...
On Saturday, January 26, 2013 9:55:14 AM, Mark Brown wrote:
On Fri, Jan 25, 2013 at 06:19:20PM +0100, Benoît Thébaudeau wrote:
The TLV320AIC3x DAI supports only stereo. This patch fixes mono support for this codec by allowing only stereo at codec level.
So, this doesn't actually fix mono - it removes support for it.
Yes, for this driver, but this fixes mono for the whole system, meaning that e.g. it fixes mono playback support with players like MPlayer. I will reword if you prefer.
It's a bit surprising that this is a requirement actually, the device seems to support DSP modes which usually do mono totally happily (I2S style modes have problems due to the clocking). I'd like to see some confirmation that there's not some other issue in your system...
I've only tested with I²S (imx-ssi). But for all the available DAI modes, the data sheet gives the encoding for both stereo channels (from p. 21), and nowhere it says that mono is an option for DAI: http://www.ti.com/lit/ds/symlink/tlv320aic3104.pdf
I will retest with DSP modes if you prefer.
The idea was the same as for this commit: http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h... Even though the MC13783 only supports I²S and MSB justified modes, and there may be an issue specific to it: http://cache.freescale.com/files/rf_if/doc/data_sheet/MC13783.pdf?pspll=1
Best regards, Benoît
On Sat, Jan 26, 2013 at 03:17:01PM +0100, Benoît Thébaudeau wrote:
On Saturday, January 26, 2013 9:55:14 AM, Mark Brown wrote:
So, this doesn't actually fix mono - it removes support for it.
Yes, for this driver, but this fixes mono for the whole system, meaning that e.g. it fixes mono playback support with players like MPlayer. I will reword if you prefer.
Yes, please.
I've only tested with I²S (imx-ssi). But for all the available DAI modes, the data sheet gives the encoding for both stereo channels (from p. 21), and nowhere it says that mono is an option for DAI:
I2S isn't really relevant here, it's not possible to clock I2S as a mono interface.
I will retest with DSP modes if you prefer.
Yes, please.
On Sunday, January 27, 2013 3:58:47 AM, Mark Brown wrote:
On Sat, Jan 26, 2013 at 03:17:01PM +0100, Benoît Thébaudeau wrote:
On Saturday, January 26, 2013 9:55:14 AM, Mark Brown wrote: I've only tested with I²S (imx-ssi). But for all the available DAI modes, the data sheet gives the encoding for both stereo channels (from p. 21), and nowhere it says that mono is an option for DAI:
I2S isn't really relevant here, it's not possible to clock I2S as a mono interface.
I will retest with DSP modes if you prefer.
Yes, please.
I have now tested all the DAI formats compatible with both imx-ssi and tlv320aic3x, i.e.: - I2S | NB_NF | CBM_CFM, - DSP_A | IB_NF | CBM_CFM, - DSP_B | IB_NF | CBM_CFM, - LEFT_J | NB_NF | CBM_CFM.
In all cases, stereo works just fine.
As to mono, setting .channels_min to 2 also always works fine (as expected, but not interesting here).
For mono with .channels_min = 1 (i.e. without this patch), I have tried with:
1) snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0); -> all DAI formats: wrong sample rate for L/R, playing twice too fast
2) snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0); -> all DAI formats: nothing on R, wrong sample rate for L, playing twice too fast, plus loud noise superimposed for LEFT_J
So, I don't see how to avoid this patch.
I will send a reworded update.
Best regards, Benoît
On Mon, Jan 28, 2013 at 10:03:40PM +0100, Benoît Thébaudeau wrote:
- snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
-> all DAI formats: wrong sample rate for L/R, playing twice too fast
- snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0);
-> all DAI formats: nothing on R, wrong sample rate for L, playing twice too fast, plus loud noise superimposed for LEFT_J
Why are you having to call this in the first place?
Anyway, I guess we do have a hardware limiation here so I'll apply the patch when you resend it.
On Tuesday, January 29, 2013 3:01:32 AM, Mark Brown wrote:
On Mon, Jan 28, 2013 at 10:03:40PM +0100, Benoît Thébaudeau wrote:
- snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0);
-> all DAI formats: wrong sample rate for L/R, playing twice too fast
- snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0);
-> all DAI formats: nothing on R, wrong sample rate for L, playing twice too fast, plus loud noise superimposed for LEFT_J
Why are you having to call this in the first place?
For I²S, because the reference manual of the i.MX25 says that SSI.STCCR.DC should be set to 1 (i.e. 2 channels) in this mode. This also allowed to test several settings for mono.
For DSP modes, because stereo doesn't work without this call (right channel missing).
Anyway, I guess we do have a hardware limiation here so I'll apply the patch when you resend it.
OK. I have just resent it.
Best regards, Benoît
On Tue, Jan 29, 2013 at 09:40:18PM +0100, Benoît Thébaudeau wrote:
On Tuesday, January 29, 2013 3:01:32 AM, Mark Brown wrote:
Why are you having to call this in the first place?
For I²S, because the reference manual of the i.MX25 says that SSI.STCCR.DC should be set to 1 (i.e. 2 channels) in this mode. This also allowed to test several settings for mono.
For DSP modes, because stereo doesn't work without this call (right channel missing).
So why does the driver not just do this automatically?
On Wednesday, January 30, 2013 3:11:45 AM, Mark Brown wrote:
On Tue, Jan 29, 2013 at 09:40:18PM +0100, Benoît Thébaudeau wrote:
On Tuesday, January 29, 2013 3:01:32 AM, Mark Brown wrote:
Why are you having to call this in the first place?
For I²S, because the reference manual of the i.MX25 says that SSI.STCCR.DC should be set to 1 (i.e. 2 channels) in this mode. This also allowed to test several settings for mono.
For DSP modes, because stereo doesn't work without this call (right channel missing).
So why does the driver not just do this automatically?
I don't know. I add Fabio and Sascha to Cc in case they know.
I think that it's either to let the users of imx-ssi do weird things if they have to (but implementing a default setup would not prevent users to change it by calling snd_soc_dai_set_tdm_slot(), so...), or rather because nobody had enough time or all the test configurations to work on this so far. It's marked as TODO in sound/soc/fsl/wm1133-ev1.c.
Best regards, Benoît
Playing a mono stream on the TLV320AIC3x results in too fast playback rate.
Remove mono support so that mono streams can be played correctly on this codec.
Tested with imx-ssi (i.MX25) and TLV320AIC3104.
Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: alsa-devel@alsa-project.org Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com --- Changes for v2: - Reword patch description as mono removal rather than as mono fix. --- sound/soc/codecs/tlv320aic3x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 5708a97..4989143 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1210,13 +1210,13 @@ static struct snd_soc_dai_driver aic3x_dai = { .name = "tlv320aic3x-hifi", .playback = { .stream_name = "Playback", - .channels_min = 1, + .channels_min = 2, .channels_max = 2, .rates = AIC3X_RATES, .formats = AIC3X_FORMATS,}, .capture = { .stream_name = "Capture", - .channels_min = 1, + .channels_min = 2, .channels_max = 2, .rates = AIC3X_RATES, .formats = AIC3X_FORMATS,},
On Tue, Jan 29, 2013 at 09:31:48PM +0100, Benoît Thébaudeau wrote:
Playing a mono stream on the TLV320AIC3x results in too fast playback rate.
Remove mono support so that mono streams can be played correctly on this codec.
Applied, thanks.
participants (2)
-
Benoît Thébaudeau -
Mark Brown