[alsa-devel] [PATCH 0/2] ASoC: DSP mode corrections for omap-mcbsp
Hello,
As a follow-up for the before easter mail thread on the omap DSP_A/_B mode support...
This series should fix the current DSP_B mode and also introduces support for DSP_A mode in omap-mcbsp.
Since omap's mcbsp module is quite flexible, one who is writing a board file for the omap platform has to check both the modes (DSP_A/_B) and the clock polarity settings against the codec requirements and pick the correct pair.
For example in osk5912 board file: To use DSP_B mode (as it is now, no changes needed): snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_IF | SND_SOC_DAIFMT_CBM_CFM);
To use the DSP_A mode for the same board, codec: snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM);
--- Peter Ujfalusi (2): ASoC: omap-mcbsp: Correct the DSP_B mode ASoC: omap-mcbsp: Add DSP_A support
sound/soc/omap/omap-mcbsp.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
Use correct DSP_B mode configuration for omap-mcbsp.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 9c09b94..4440bab 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); - regs->srgr1 |= FWID(wlen * channels - 2); + regs->srgr1 |= FWID(0); break; }
Add support for DSP_A in omap-mcbsp.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 4440bab..1576e70 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -281,6 +281,10 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, regs->srgr2 |= FPER(wlen * 2 - 1); regs->srgr1 |= FWID(wlen - 1); break; + case SND_SOC_DAIFMT_DSP_A: + regs->srgr2 |= FPER(wlen * channels - 1); + regs->srgr1 |= FWID(wlen * channels - 2); + break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); @@ -324,6 +328,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, regs->rcr2 |= RDATDLY(1); regs->xcr2 |= XDATDLY(1); break; + case SND_SOC_DAIFMT_DSP_A: case SND_SOC_DAIFMT_DSP_B: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0);
On Tue, 14 Apr 2009 09:45:30 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Use correct DSP_B mode configuration for omap-mcbsp.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
sound/soc/omap/omap-mcbsp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 9c09b94..4440bab 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1);
regs->srgr1 |= FWID(wlen * channels - 2);
break; }regs->srgr1 |= FWID(0);
Grr, I have some practical problems with my Beagle refusing to mount my SD card root but I give a test when I get it working.
But I fear this change makes it into DSP_B with inverted FS polarity. Register srgr1 defines the FS length and it's active low. I think this will be correct if you switch the polarity in omap_mcbsp_dai_set_dai_fmt.
At least defining pulse witdth with FWID(0) is more clear than FWID (wlen * channels - 2) :-)
Jarkko
On Tuesday 14 April 2009 11:20:16 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Tue, 14 Apr 2009 09:45:30 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Use correct DSP_B mode configuration for omap-mcbsp.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
sound/soc/omap/omap-mcbsp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 9c09b94..4440bab 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1);
regs->srgr1 |= FWID(wlen * channels - 2);
break; }regs->srgr1 |= FWID(0);
Grr, I have some practical problems with my Beagle refusing to mount my SD card root but I give a test when I get it working.
If you want to use the TWL codec to verify the DSP mode, than you will need to have the 4 channel support also. I'm planning to resend those as soon as this DSP_A/_B mode is settled.
But I fear this change makes it into DSP_B with inverted FS polarity. Register srgr1 defines the FS length and it's active low. I think this will be correct if you switch the polarity in omap_mcbsp_dai_set_dai_fmt.
As I written in the introduction mail, one has to verify and pick the correct pair of DSP_A/_B mode and FS/clock polarity in order to have correct config... I think it is acceptable to assume, that whoever writes such a board file should have at least this level of understanding of the FS/bitclock and how the data is driven and how the data is sampled.
Also I have provided sample configurations for the osk5912 board file - which is the only one at the moment using DSP mode on omap platform.
At least defining pulse witdth with FWID(0) is more clear than FWID (wlen * channels - 2) :-)
I think this is the only way to implement the DSP_B mode correctly in omap- mcbsp. Again, you need to pick the correct FS/clock polarity, but that is kind of simple.
Jarkko
On Tue, 14 Apr 2009 10:40:16 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
But I fear this change makes it into DSP_B with inverted FS polarity. Register srgr1 defines the FS length and it's active low. I think this will be correct if you switch the polarity in omap_mcbsp_dai_set_dai_fmt.
As I written in the introduction mail, one has to verify and pick the correct pair of DSP_A/_B mode and FS/clock polarity in order to have correct config... I think it is acceptable to assume, that whoever writes such a board file should have at least this level of understanding of the FS/bitclock and how the data is driven and how the data is sampled.
The pairs must mach. It's clearly a bug if another end is have to set different polarity than another. Those polarity defines are with respect to the mode, not e.g. what's the McBSP default.
Jarkko
On Tue, Apr 14, 2009 at 12:02:21PM +0300, Jarkko Nikula wrote:
The pairs must mach. It's clearly a bug if another end is have to set different polarity than another. Those polarity defines are with respect to the mode, not e.g. what's the McBSP default.
Indeed. However, the osk5912 board file that Peter pointed at as not needing any modifications for DSP_B already uses inverted frame clock for both CODEC and CPU. In that case either both are wrong or it's just that the driver is limited in what it supports which is not a problem.
On Tuesday 14 April 2009 12:31:33 ext Mark Brown wrote:
On Tue, Apr 14, 2009 at 12:02:21PM +0300, Jarkko Nikula wrote:
The pairs must mach. It's clearly a bug if another end is have to set different polarity than another. Those polarity defines are with respect to the mode, not e.g. what's the McBSP default.
Indeed. However, the osk5912 board file that Peter pointed at as not needing any modifications for DSP_B already uses inverted frame clock for both CODEC and CPU. In that case either both are wrong or it's just that the driver is limited in what it supports which is not a problem.
Well, I think the mcbsp module is quite - maybe too - flexible... To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
So: a) The proposal in the series DSP_B mode (the MSB is transmitted when the FS is high, the length for the pulse is 1): case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); break;
case SND_SOC_DAIFMT_DSP_B: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0); break;
case SND_SOC_DAIFMT_NB_IF: regs->pcr0 |= CLKXP | CLKRP; break;
DSP_A mode (the MSB is transmitted when the FS went low, the length for the pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles): case SND_SOC_DAIFMT_DSP_A: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(wlen * channels - 2); break;
case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0); break;
case SND_SOC_DAIFMT_IB_IF: break;
b) twist in DSP_A mode. The DSP_B mode is the same as it is in a)
DSP_A mode (the MSB is transmitted when the FS went low, the length for the pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles): case SND_SOC_DAIFMT_DSP_A: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); break;
case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(1); regs->xcr2 |= XDATDLY(1); break;
case SND_SOC_DAIFMT_NB_IF: regs->pcr0 |= CLKXP | CLKRP; break;
Both a) and b) would be OK for the DSP_A mode...
c) Remove the SND_SOC_DAIFMT_* (DSP_A, DSP_B, I2S and NB_NF, NB_IF, IB_NF, IB_IF) configuration from the omap_mcbsp_dai_set_dai_fmt, move it to the omap_mcbsp_dai_hw_params function and have something like these there:
switch (mcbsp_data->fmt & (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK)) { case SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF: ... break; case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF: ... break; case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_IF: ... break; case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF: ... break; case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_IF: ... break; case SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF: ... break; ...
}
and so on. I'm not sure if all of the combinations are valid, but it can be done like this also. It is another thing how actually the mcbsp would be configured for these modes - for DSP_B the FS polarity has to be inverted to get the MSB in a correct place, but it would be correct from the snd_soc_dai_set_fmt point of view, or something........
On Tue, Apr 14, 2009 at 01:34:33PM +0300, Peter Ujfalusi wrote:
To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
As Jarkko says the driver should be hiding all this from users - it should just set the port up as best it can, refusing to do anything that can't be supported by the hardware. The default polarity the hardware uses shouldn't be visible outside the driver.
a) The proposal in the series DSP_B mode (the MSB is transmitted when the FS is high, the length for the pulse is 1):
DSP_A mode (the MSB is transmitted when the FS went low, the length for the pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles):
The difference between the two modes shouldn't be edge of FS used, it should be a clock cycle, though with a 1 BCLK pulse on FS the effect will probably line up. The MSB data needs to be available for sampling on the appropriate rising edge of BCLK.
On Tue, 14 Apr 2009 12:34:33 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Well, I think the mcbsp module is quite - maybe too - flexible... To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)
Jarkko
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d6882be..c40ea21 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); - regs->srgr1 |= FWID(wlen * channels - 2); + regs->srgr1 |= FWID(0); break; }
@@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs; + unsigned int temp_fmt = fmt;
if (mcbsp_data->configured) return 0; @@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0); + /* Invert bit clock and FS polarity configuration */ + temp_fmt ^= SND_SOC_DAIFMT_IB_IF; break; default: /* Unsupported data format */ @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, }
/* Set bit clock (CLKX/CLKR) and FS polarities */ - switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: /* * Normal BCLK + FS.
Hi,
I m not able to use my osk5912 to test this. When i do an aplay with the latest kernel (linux-2.6.30-rc1) i am getting an me i/o error.
Thanks, Arun
On Tue, Apr 14, 2009 at 4:50 PM, Jarkko Nikula jarkko.nikula@nokia.com wrote:
On Tue, 14 Apr 2009 12:34:33 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Well, I think the mcbsp module is quite - maybe too - flexible... To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)
Jarkko
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d6882be..c40ea21 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1);
- regs->srgr1 |= FWID(wlen * channels - 2);
- regs->srgr1 |= FWID(0);
break; }
@@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
- unsigned int temp_fmt = fmt;
if (mcbsp_data->configured) return 0; @@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
- /* Invert bit clock and FS polarity configuration */
- temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
break; default: /* Unsupported data format */ @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, }
/* Set bit clock (CLKX/CLKR) and FS polarities */
- switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
- switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) {
case SND_SOC_DAIFMT_NB_NF: /* * Normal BCLK + FS. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Tue, 14 Apr 2009 13:32:45 +0200 ext Arun KS getarunks@gmail.com wrote:
Hi,
I m not able to use my osk5912 to test this. When i do an aplay with the latest kernel (linux-2.6.30-rc1) i am getting an me i/o error.
Are you using mainline or linux-omap? It seems that linux-omap is somewhat broken at the moment. I got Beagle working by going back into commit 823de921c923cc753a6746183e9c513a3452dec7 but not yet figured out one for N810...
Jarkko
On Tuesday 14 April 2009 14:20:04 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Tue, 14 Apr 2009 12:34:33 +0200
I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)
Jarkko
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d6882be..c40ea21 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1);
regs->srgr1 |= FWID(wlen * channels - 2);
break; }regs->srgr1 |= FWID(0);
@@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
unsigned int temp_fmt = fmt;
if (mcbsp_data->configured) return 0;
@@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
/* Invert bit clock and FS polarity configuration */
break; default: /* Unsupported data format */temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
@@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, }
/* Set bit clock (CLKX/CLKR) and FS polarities */
- switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
- switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: /*
- Normal BCLK + FS.
In case of the osk5912 board with tlv320aic32 codec, than you need to configure it like this?
snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF | /* invert bclk + nor frm */ SND_SOC_DAIFMT_CBM_CFM);
Btw, what does: inverted bclk normal bclk inverted frm normal frm actually means?
Is it so that it is: inverted bclk: sample is driven on the rising edge, sampled on the falling edge normal bclk: sample is driven on the falling edge, sampled on the rising edge inverted frm: FS is active high normal frm: FS is active low
On Tue, Apr 14, 2009 at 02:56:49PM +0300, Peter Ujfalusi wrote:
Is it so that it is: inverted bclk: sample is driven on the rising edge, sampled on the falling edge normal bclk: sample is driven on the falling edge, sampled on the rising edge
Yes.
inverted frm: FS is active high normal frm: FS is active low
Ish. For DSP mode normal frame means that things are synced of the rising edge of the frame clock so inverted frame should mean the falling edge. Frame clock is allowed to be longer than one BCLK cycle, though it is not normally and some things may need the low limit.
On Tuesday 14 April 2009 14:20:04 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Tue, 14 Apr 2009 12:34:33 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Well, I think the mcbsp module is quite - maybe too - flexible... To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)
Jarkko
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d6882be..c40ea21 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1);
regs->srgr1 |= FWID(wlen * channels - 2);
break; }regs->srgr1 |= FWID(0);
@@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
unsigned int temp_fmt = fmt;
if (mcbsp_data->configured) return 0;
@@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
/* Invert bit clock and FS polarity configuration */
temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
Should it be like this instead: + temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
We only need to invert the FS polarity and the bclk should stay as it is.. Also should than this modified fmt stored to mcbsp_data->fmt later?
break;
default: /* Unsupported data format */ @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, }
/* Set bit clock (CLKX/CLKR) and FS polarities */
- switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
- switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: /*
- Normal BCLK + FS.
On Tue, 14 Apr 2009 14:32:30 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Should it be like this instead:
- temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
Absolutely yes, good catch :-)
We only need to invert the FS polarity and the bclk should stay as it is.. Also should than this modified fmt stored to mcbsp_data->fmt later?
No since it's used only inside omap_mcbsp_dai_set_dai_fmt to tune polarity for the mode default. That's why there is this temp_fmt variable.
Jarkko
On Tuesday 14 April 2009 15:49:00 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Tue, 14 Apr 2009 14:32:30 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Should it be like this instead:
- temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
Absolutely yes, good catch :-)
We only need to invert the FS polarity and the bclk should stay as it is.. Also should than this modified fmt stored to mcbsp_data->fmt later?
No since it's used only inside omap_mcbsp_dai_set_dai_fmt to tune polarity for the mode default. That's why there is this temp_fmt variable.
Fair enough.
Could you resend the patch? I think this is a good starting point for getting this DSP_A/_B mode right, finally... I'll can than send the DSP_A patch on top of your DSP_B... Than I can resend the TWL4030 four channel patches on top of these ;)
Jarkko
participants (4)
-
Arun KS
-
Jarkko Nikula
-
Mark Brown
-
Peter Ujfalusi