Re: [alsa-devel] [PATCH] ASoC: imx-ssi: Fix mono playback
On Mon, Aug 06, 2012 at 04:36:25PM -0300, Fabio Estevam wrote:
Currently mono playback is played on a faster rate.
Adjust channels_min to 2 channels in order to play it back correctly.
This doesn't actually fix mono playback so much as remove support for it...
On Mon, Aug 6, 2012 at 4:54 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Mon, Aug 06, 2012 at 04:36:25PM -0300, Fabio Estevam wrote:
Currently mono playback is played on a faster rate.
Adjust channels_min to 2 channels in order to play it back correctly.
This doesn't actually fix mono playback so much as remove support for it...
Ok, understood.
Sascha,
Do you know if mono playback used to work previously in imx-ssi?
Regards,
Fabio Estevam
On Mon, Aug 6, 2012 at 4:58 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, understood.
Sascha,
Do you know if mono playback used to work previously in imx-ssi?
Ok, just tested a mono track on a mx6qsabrelite and it played well.
So it seems the issue is mc13783 related then.
Regards,
Fabio Estevam
On Mon, Aug 6, 2012 at 7:01 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Mon, Aug 06, 2012 at 06:49:04PM -0300, Fabio Estevam wrote:
So it seems the issue is mc13783 related then.
Is the device using I2S? I2S doesn't really support stero properly...
The format is:
#define FMT_SSI (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | \ SND_SOC_DAIFMT_CBM_CFM)
In wm1133-ev1.c you did:
/* TODO: The SSI driver should figure this out for us */ switch (channels) { case 2: snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffc, 0xffffffc, 2, 0); break; case 1: snd_soc_dai_set_tdm_slot(cpu_dai, 0xffffffe, 0xffffffe, 1, 0); break; default: return -EINVAL; }
Maybe I should do something similar in imx-mc13783?
Regards,
Fabio Estevam
On Mon, Aug 06, 2012 at 07:10:07PM -0300, Fabio Estevam wrote:
On Mon, Aug 6, 2012 at 7:01 PM, Mark Brown
Is the device using I2S? I2S doesn't really support stero properly...
The format is:
#define FMT_SSI (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | \ SND_SOC_DAIFMT_CBM_CFM)
No, then.
Maybe I should do something similar in imx-mc13783?
That's cut'n'paste since before my time, and the comment about it being something the SSI driver ought to do does apply. But it's worth trying.
On Mon, Aug 6, 2012 at 5:01 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
Is the device using I2S? I2S doesn't really support stero properly...
Every use case of I2S I've seen has been stereo only. What do you mean that it doesn't support stereo properly?
On Mon, Aug 06, 2012 at 11:28:37PM +0000, Tabi Timur-B04825 wrote:
On Mon, Aug 6, 2012 at 5:01 PM, Mark Brown
Is the device using I2S? I2S doesn't really support stero properly...
Every use case of I2S I've seen has been stereo only. What do you mean that it doesn't support stereo properly?
Sorry, I meant mono.
On Mon, Aug 6, 2012 at 6:49 PM, Fabio Estevam festevam@gmail.com wrote:
On Mon, Aug 6, 2012 at 4:58 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, understood.
Sascha,
Do you know if mono playback used to work previously in imx-ssi?
Ok, just tested a mono track on a mx6qsabrelite and it played well.
Ops, I was making a confusion:
mx6sabrelite runs a dt kernel and uses fsl_ssi.c driver.
mx31pdk runs a non-dt kernel and uses imx-ssi.c driver.
The fsl_ssi.c code has:
.playback = { /* The SSI does not support monaural audio. */ .channels_min = 2, .channels_max = 2, .rates = FSLSSI_I2S_RATES, .formats = FSLSSI_I2S_FORMATS, },
and if I do the same on imx-ssi.c, ie, change 'channels_min' to 2, then it also plays correctly.
So is my original patch correct? Maybe I should change the wording of the commit log?
Thanks,
Fabio Estevam
On Tue, Aug 07, 2012 at 12:22:41AM -0300, Fabio Estevam wrote:
So is my original patch correct? Maybe I should change the wording of the commit log?
Yeah, just say something about removing mono support since it doesn't work.
On Mon, Aug 6, 2012 at 2:58 PM, Fabio Estevam festevam@gmail.com wrote:
Do you know if mono playback used to work previously in imx-ssi?
I doubt it. I don't think the I2S standard allows for mono playback, so the SSI would need to send the same sample twice, and I don't see any support for that feature in the RM.
When I wrote fsl_ssi, I couldn't find any way to get mono playback to work.
BTW, I can't find the original patch in my inbox or the mailing list archives. When was it sent?
Hi Timur,
On Mon, Aug 6, 2012 at 8:33 PM, Tabi Timur-B04825 B04825@freescale.com wrote:
On Mon, Aug 6, 2012 at 2:58 PM, Fabio Estevam festevam@gmail.com wrote:
Do you know if mono playback used to work previously in imx-ssi?
I doubt it. I don't think the I2S standard allows for mono playback, so the SSI would need to send the same sample twice, and I don't see any support for that feature in the RM.
When I wrote fsl_ssi, I couldn't find any way to get mono playback to work.
BTW, I can't find the original patch in my inbox or the mailing list archives. When was it sent?
I sent the patch from FSL and it was blocked because I am not subscribed in alsa-devel list from my FSL account.
The original patch I sent was the following and it allowed me to play a mono track:
From bc46087110bd48a41f1f7dcb826853349662c766 Mon Sep 17 00:00:00 2001
From: Fabio Estevam fabio.estevam@freescale.com Date: Mon, 6 Aug 2012 16:30:51 -0300 Subject: [PATCH] ASoC: imx-ssi: Fix mono playback
Currently mono playback is played on a faster rate.
Adjust channels_min to 2 channels in order to play it back correctly.
Reported-by: Gaëtan Carlier gcembed@gmail.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- sound/soc/fsl/imx-ssi.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c index 28dd76c..c42df31 100644 --- a/sound/soc/fsl/imx-ssi.c +++ b/sound/soc/fsl/imx-ssi.c @@ -380,7 +380,7 @@ static int imx_ssi_dai_probe(struct snd_soc_dai *dai) static struct snd_soc_dai_driver imx_ssi_dai = { .probe = imx_ssi_dai_probe, .playback = { - .channels_min = 1, + .channels_min = 2, .channels_max = 2, .rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S16_LE,
It depends on the codec connected to the I2S. We have SSI that works in mono mode, using a modified driver. Our codec allows mono playback by only using the samples on the left channel.
There is the race issue in that playback and record must both match mono and stereo and ALSA doesn't handle substreams with constraints that depend on other substreams in a race free manner.
On Mon, Aug 6, 2012 at 7:33 PM, Tabi Timur-B04825 B04825@freescale.com wrote:
On Mon, Aug 6, 2012 at 2:58 PM, Fabio Estevam festevam@gmail.com wrote:
Do you know if mono playback used to work previously in imx-ssi?
I doubt it. I don't think the I2S standard allows for mono playback, so the SSI would need to send the same sample twice, and I don't see any support for that feature in the RM.
When I wrote fsl_ssi, I couldn't find any way to get mono playback to work.
BTW, I can't find the original patch in my inbox or the mailing list archives. When was it sent?
-- Timur Tabi Linux kernel developer at Freescale _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Trent Piepho wrote:
It depends on the codec connected to the I2S. We have SSI that works in mono mode, using a modified driver. Our codec allows mono playback by only using the samples on the left channel.
I would *love* to see that driver.
There is the race issue in that playback and record must both match mono and stereo and ALSA doesn't handle substreams with constraints that depend on other substreams in a race free manner.
Isn't that only a problem if both streams are opened literally simultaneously? I would assume that most apps will open one stream at a time.
On Tue, Aug 7, 2012 at 8:00 AM, Tabi Timur-B04825 B04825@freescale.com wrote:
There is the race issue in that playback and record must both match mono and stereo and ALSA doesn't handle substreams with constraints that depend on other substreams in a race free manner.
Isn't that only a problem if both streams are opened literally simultaneously? I would assume that most apps will open one stream at a time.
The problem comes up if another stream is opened before the hw_params are fixed on the other stream. So if the first stream is configured past the hw_params level before the next is opened there is no problem. But if not there is a race. There could be two different independent processes using the hardware which might be running at the same time.
I remember posting something about this race in more detail. One can mostly make it work by dynamically adding constraints in the open() method based on the configuration of the linked substream and then checking the hw_params again for correctness in the hw_params() method.
There end up being two problems. One is on the userspace side. An app could get a set constraints, choose hwparams that are valid according to those constraints, and then get told hwparams are not valid anyway. While an app could handle this, it has never been documented as something an app should handle in a certain way. So apps will most likely not handle it well.
The other problem is that if two substreams call hw_params at the same time there is a race internal to ALSA. I don't remember what the problem is exactly, just that there was one.
On Tue, Aug 07, 2012 at 01:39:26AM -0400, Trent Piepho wrote:
Don't top post!
It depends on the codec connected to the I2S. We have SSI that works in mono mode, using a modified driver. Our codec allows mono playback by only using the samples on the left channel.
This is extremely common.
Mark Brown wrote:
It depends on the codec connected to the I2S. We have SSI that works in mono mode, using a modified driver. Our codec allows mono playback by only using the samples on the left channel.
This is extremely common.
Wouldn't this require the I2S controller to inject dummy data into the right channel?
On Tue, Aug 07, 2012 at 09:26:34AM -0500, Timur Tabi wrote:
Mark Brown wrote:
It depends on the codec connected to the I2S. We have SSI that works in mono mode, using a modified driver. Our codec allows mono playback by only using the samples on the left channel.
This is extremely common.
Wouldn't this require the I2S controller to inject dummy data into the right channel?
Yes, generally the hardware just won't send anything on any empty bit clocks (or will ignore data that happens to be sent when it's not interested on the other end).
On Mon, Aug 06, 2012 at 04:58:38PM -0300, Fabio Estevam wrote:
On Mon, Aug 6, 2012 at 4:54 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Mon, Aug 06, 2012 at 04:36:25PM -0300, Fabio Estevam wrote:
Currently mono playback is played on a faster rate.
Adjust channels_min to 2 channels in order to play it back correctly.
This doesn't actually fix mono playback so much as remove support for it...
Ok, understood.
Sascha,
Do you know if mono playback used to work previously in imx-ssi?
AFAIK only stereo works with the mc13783.
Sascha
Sascha,
On Tue, Aug 7, 2012 at 3:26 AM, Sascha Hauer s.hauer@pengutronix.de wrote:
AFAIK only stereo works with the mc13783.
Ok, could you please let me know what do you think about my patch? It does the same thing as in fsl_ssi.c by setting channels_min to 2 and I am interested to know if this is an acceptable change.
Thanks,
Fabio Estevam
participants (6)
-
Fabio Estevam -
Mark Brown -
Sascha Hauer -
Tabi Timur-B04825 -
Timur Tabi -
Trent Piepho