Re: [alsa-devel] [PATCH 1/4] ASoC: pxa-ssp: enhance I2S and add Left_J support
Hi Daniel,
This is Paul Shen , I will substitute boshen9@gmail.com for bshen9@marvell.com to answer open source mails.
From: Daniel Mack daniel@caiaq.de Date: Wed, Jun 3, 2009 at 9:18 PM Subject: Re: [alsa-devel] [PATCH 1/4] ASoC: pxa-ssp: enhance I2S and add Left_J support To: Eric Miao eric.y.miao@gmail.com Cc: alsa-devel@alsa-project.org, linux-arm-kernel linux-arm-kernel@lists.arm.linux.org.uk, Mark Brown broonie@sirena.org.uk
On Wed, Jun 03, 2009 at 08:33:42PM +0800, Eric Miao wrote:
Make the pxa I2S configuration generic, add support for Left_J, add support for variable frame width like 32fs, 48fs, 64fs and 96fs
Signed-off-by: Paul Shen bshen9@marvell.com Signed-off-by: Eric Miao eric.miao@marvell.com Cc: Daniel Mack daniel@caiaq.de
arch/arm/mach-pxa/include/mach/regs-ssp.h | 14 +++--- sound/soc/pxa/pxa-ssp.c | 62 ++++++++++++++-------------- sound/soc/pxa/pxa-ssp.h | 9 ++++ 3 files changed, 47 insertions(+), 38 deletions(-)
Ok, I tried that code on my board and this is what I had to change there:
The tdm time slot configuration needs to be set again in my board support code just like in your example: snd_soc_set_tdm_slot(cpu_dai, 3, 2). And the PXA_SSP_DIV_SCR value needed to be doubled from 4 to 8.
I tested with below codes to set the cpu_dai :
format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS | PXA_SSP_FRM_64FS;
snd_soc_dai_set_fmt(cpu_dai, format); snd_soc_dai_set_tdm_slot(cpu_dai, 3, 2);
snd_soc_dai_set_sysclk(cpu_dai, PXA_SSP_CLK_EXT, clk, 0); snd_soc_dai_set_clkdiv(cpu_dai, PXA_SSP_DIV_SCR, 4);
It is no need to "snd_soc_dai_set_pll(cpu_dai, 0, 0, clk)" , for your case ,no ssp-on-chip pll configuration is needed.
On my littleton platform,it is ok, and I did not modify the PXA_SSP_DIV_SCR from 4 to 8.
I dumped the ssp register: SSCR0 0xa10003ff SSCR1 0x00e01d80 SSTO 0x00000000 SSPSP 0x31a00084 SSSR 0x0000f0fc SSACD 0x00000000 SSACDD 0x00000000
How about your registers ?
With that changes, LRCLK is 44100Hz when configured to 44100Hz. But the bitclk is not 64fs anymore but 32fs only (1.41Mhz). Is there any implementation details I miss? What does your codec clock config look like?
Do you mean it only down to 32fs when LRCLK is 44100HZ ? And when LRCLK is 48000HZ, all is OK ?
Another small thing:
CC sound/soc/pxa/pxa-ssp.o sound/soc/pxa/pxa-ssp.c:186: warning: 'ssp_get_scr' defined but not used
Daniel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Paul,
sorry for the delay.
On Thu, Jun 04, 2009 at 05:44:55PM +0800, Paul Shen wrote:
The tdm time slot configuration needs to be set again in my board support code just like in your example: snd_soc_set_tdm_slot(cpu_dai, 3, 2). And the PXA_SSP_DIV_SCR value needed to be doubled from 4 to 8.
I tested with below codes to set the cpu_dai :
format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS | PXA_SSP_FRM_64FS;
Ah - the PXA_SSP_FRM_64FS is the detail I missed. With that option set, my system works well, so I'm generally fine with the changes.
However, I proposed a generic way (not pxa/ssp specific) to let the format flags carry the information about such frame format details some months ago and the approach was rejected because there are already too many ways to let the DAI know about this. In particular, the DIV_SCR (serial clock rate divider) is used to propagate the base frequency ratio and together with other options, you can find out the desired frame format.
In my implementation, the '32 bit over 64fs case' was detected with this condition in pxa_ssp_hw_params():
(scr == 4) && (snd_pcm_format_physical_width(params_format(params)) == 16)
Now, SCR isn't used at all any more but instead PXA_SSP_FRM_WIDTH() handles the case. Eventually this might be a matter of taste, but if we go this way, why shouldn't that definitions be in ASoC's generic headers?
Anyway, the code looks much cleaner now :)
Thanks, Daniel
On Fri, Jun 05, 2009 at 07:26:30PM +0200, Daniel Mack wrote:
Ah - the PXA_SSP_FRM_64FS is the detail I missed. With that option set, my system works well, so I'm generally fine with the changes.
However, I proposed a generic way (not pxa/ssp specific) to let the format flags carry the information about such frame format details some months ago and the approach was rejected because there are already too many ways to let the DAI know about this.
Indeed, and there's some potential lack of clarity with the interaction of settings in the DAI format with these other settings.
In particular, the DIV_SCR(serial clock rate divider) is used to propagate the base frequency ratio and together with other options, you can find out the desired frame format.
There's that, and there's also the TDM settings which provide essentially the same information - an overclocked bit clock is equivalent to a TDM mode where the first slot is in use. It's not immediately clear to me what a combination of the two options ought to do.
It's also a logically different bit of configuration - for most devices it only makes a difference to the device that's generating the clocks and wouldn't need to be specified for the other devices since they'd just ignore the extra clocks. The DAI format specifies the format for the clocks, primarily the frame clocks, but does not currently handle their rates.
handles the case. Eventually this might be a matter of taste, but if we go this way, why shouldn't that definitions be in ASoC's generic headers?
Definitely.
participants (3)
-
Daniel Mack -
Mark Brown -
Paul Shen