[alsa-devel] [PATCH] ASoC - Add support for upto 16 channels on OMAP MCBSP
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk --- sound/soc/omap/omap-mcbsp.c | 71 +++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 3341f49..290ef2f 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -49,6 +49,8 @@ struct omap_mcbsp_data { */ int active; int configured; + unsigned int in_freq; + int clk_div; };
#define to_mcbsp(priv) container_of((priv), struct omap_mcbsp_data, bus_id) @@ -257,7 +259,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, int dma, bus_id = mcbsp_data->bus_id, id = cpu_dai->id; int wlen, channels, wpf, sync_mode = OMAP_DMA_SYNC_ELEMENT; unsigned long port; - unsigned int format; + unsigned int format, frame_size, div;
if (cpu_class_is_omap1()) { dma = omap1_dma_reqs[bus_id][substream->stream]; @@ -294,27 +296,23 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params); - switch (channels) { - case 2: - if (format == SND_SOC_DAIFMT_I2S) { - /* Use dual-phase frames */ - regs->rcr2 |= RPHASE; - regs->xcr2 |= XPHASE; - /* Set 1 word per (McBSP) frame for phase1 and phase2 */ - wpf--; - regs->rcr2 |= RFRLEN2(wpf - 1); - regs->xcr2 |= XFRLEN2(wpf - 1); - } - case 1: - case 4: + if (channels == 2 && format == SND_SOC_DAIFMT_I2S) { + /* Use dual-phase frames */ + regs->rcr2 |= RPHASE; + regs->xcr2 |= XPHASE; + /* Set 1 word per (McBSP) frame for phase1 and phase2 */ + wpf--; + regs->rcr2 |= RFRLEN2(wpf - 1); + regs->xcr2 |= XFRLEN2(wpf - 1); /* Set word per (McBSP) frame for phase1 */ regs->rcr1 |= RFRLEN1(wpf - 1); regs->xcr1 |= XFRLEN1(wpf - 1); - break; - default: + } else if (channels > 0 && channels < 17) { + regs->rcr1 |= RFRLEN1(wpf - 1); + regs->xcr1 |= XFRLEN1(wpf - 1); + } else /* Unsupported number of channels */ return -EINVAL; - }
switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: @@ -330,6 +328,34 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
+ /* Default div to 1 if it wasn't set by machine driver, otherwise + * use set div as the maximum clock value + */ + div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1; + + /* calc best frame size for rate and clock divider */ + do { + frame_size = (mcbsp_data->in_freq / div) / params_rate(params); + pr_debug("freq %d, rate %d, frame size %d, div %d\n", + mcbsp_data->in_freq, params_rate(params), frame_size, div); + + if (frame_size > 256) + div++; + } while (frame_size > 256); + + /* Check we can fit the requested number of channels into our + * calculated frame size + */ + if ((channels * wlen) > frame_size) { + printk(KERN_ERR + "OMAP-MCBSP: cannot fit channels in frame size\n"); + return -EINVAL; + } + + /* Set the actual clkdiv to use for this samplerate */ + regs->srgr1 &= ~CLKGDV(0xFF); + regs->srgr1 |= CLKGDV(div - 1); + /* Set FS period and length in terms of bit clock periods */ switch (format) { case SND_SOC_DAIFMT_I2S: @@ -338,7 +364,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_A: case SND_SOC_DAIFMT_DSP_B: - regs->srgr2 |= FPER(wlen * channels - 1); + regs->srgr2 |= FPER(frame_size - 1); regs->srgr1 |= FWID(0); break; } @@ -449,12 +475,11 @@ static int omap_mcbsp_dai_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id, int div) { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); - struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
if (div_id != OMAP_MCBSP_CLKGDV) return -ENODEV;
- regs->srgr1 |= CLKGDV(div - 1); + mcbsp_data->clk_div = div;
return 0; } @@ -554,6 +579,8 @@ static int omap_mcbsp_dai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs; int err = 0;
+ mcbsp_data->in_freq = freq; + switch (clk_id) { case OMAP_MCBSP_SYSCLK_CLK: regs->srgr2 |= CLKSM; @@ -598,13 +625,13 @@ static struct snd_soc_dai_ops omap_mcbsp_dai_ops = { .id = (link_id), \ .playback = { \ .channels_min = 1, \ - .channels_max = 4, \ + .channels_max = 16, \ .rates = OMAP_MCBSP_RATES, \ .formats = SNDRV_PCM_FMTBIT_S16_LE, \ }, \ .capture = { \ .channels_min = 1, \ - .channels_max = 4, \ + .channels_max = 16, \ .rates = OMAP_MCBSP_RATES, \ .formats = SNDRV_PCM_FMTBIT_S16_LE, \ }, \
On Wed, Nov 04, 2009 at 05:53:55PM +0000, Liam Girdwood wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
I'm OK with this from an ASoC point of view (the provision of a default for existing machine drivers is good BTW), also adding Jarkko as well as Peter for the OMAP-specific review.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk
sound/soc/omap/omap-mcbsp.c | 71 +++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 3341f49..290ef2f 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -49,6 +49,8 @@ struct omap_mcbsp_data { */ int active; int configured;
- unsigned int in_freq;
- int clk_div;
};
#define to_mcbsp(priv) container_of((priv), struct omap_mcbsp_data, bus_id) @@ -257,7 +259,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, int dma, bus_id = mcbsp_data->bus_id, id = cpu_dai->id; int wlen, channels, wpf, sync_mode = OMAP_DMA_SYNC_ELEMENT; unsigned long port;
- unsigned int format;
unsigned int format, frame_size, div;
if (cpu_class_is_omap1()) { dma = omap1_dma_reqs[bus_id][substream->stream];
@@ -294,27 +296,23 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params);
- switch (channels) {
- case 2:
if (format == SND_SOC_DAIFMT_I2S) {/* Use dual-phase frames */regs->rcr2 |= RPHASE;regs->xcr2 |= XPHASE;/* Set 1 word per (McBSP) frame for phase1 and phase2 */wpf--;regs->rcr2 |= RFRLEN2(wpf - 1);regs->xcr2 |= XFRLEN2(wpf - 1);}- case 1:
- case 4:
- if (channels == 2 && format == SND_SOC_DAIFMT_I2S) {
/* Use dual-phase frames */regs->rcr2 |= RPHASE;regs->xcr2 |= XPHASE;/* Set 1 word per (McBSP) frame for phase1 and phase2 */wpf--;regs->rcr2 |= RFRLEN2(wpf - 1);regs->xcr2 |= XFRLEN2(wpf - 1);
break;- default:
- } else if (channels > 0 && channels < 17) {
regs->rcr1 |= RFRLEN1(wpf - 1);regs->xcr1 |= XFRLEN1(wpf - 1);- } else /* Unsupported number of channels */ return -EINVAL;
}
switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE:
@@ -330,6 +328,34 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- /* Default div to 1 if it wasn't set by machine driver, otherwise
* use set div as the maximum clock value*/- div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1;
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
- /* Check we can fit the requested number of channels into our
* calculated frame size*/- if ((channels * wlen) > frame_size) {
printk(KERN_ERR"OMAP-MCBSP: cannot fit channels in frame size\n");return -EINVAL;- }
- /* Set the actual clkdiv to use for this samplerate */
- regs->srgr1 &= ~CLKGDV(0xFF);
- regs->srgr1 |= CLKGDV(div - 1);
- /* Set FS period and length in terms of bit clock periods */ switch (format) { case SND_SOC_DAIFMT_I2S:
@@ -338,7 +364,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_DSP_A: case SND_SOC_DAIFMT_DSP_B:
regs->srgr2 |= FPER(wlen * channels - 1);
regs->srgr2 |= FPER(frame_size - 1);@@ -449,12 +475,11 @@ static int omap_mcbsp_dai_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id, int div) { struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
if (div_id != OMAP_MCBSP_CLKGDV) return -ENODEV;
regs->srgr1 |= CLKGDV(div - 1);
mcbsp_data->clk_div = div;
return 0;
} @@ -554,6 +579,8 @@ static int omap_mcbsp_dai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs; int err = 0;
- mcbsp_data->in_freq = freq;
- switch (clk_id) { case OMAP_MCBSP_SYSCLK_CLK: regs->srgr2 |= CLKSM;
@@ -598,13 +625,13 @@ static struct snd_soc_dai_ops omap_mcbsp_dai_ops = { .id = (link_id), \ .playback = { \ .channels_min = 1, \
.channels_max = 4, \
.channels_max = 16, \
.channels_max = 4, \
.channels_max = 16, \-- 1.6.3.3
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Wed, Nov 04, 2009 at 05:53:55PM +0000, Liam Girdwood wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk
- /* Default div to 1 if it wasn't set by machine driver, otherwise
* use set div as the maximum clock value*/- div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1;
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
- /* Check we can fit the requested number of channels into our
* calculated frame size*/- if ((channels * wlen) > frame_size) {
printk(KERN_ERR"OMAP-MCBSP: cannot fit channels in frame size\n");return -EINVAL;- }
- /* Set the actual clkdiv to use for this samplerate */
- regs->srgr1 &= ~CLKGDV(0xFF);
- regs->srgr1 |= CLKGDV(div - 1);
This chunk changes the semantics of other devices which I have never tested.
I also dont know how much damage it does if it does to slave mode. In fact I think it might break it in cases which are actually allowable as it uses the omap as its clock constraint and not the clock source.
Graeme
On Wed, 2009-11-04 at 18:55 +0000, Graeme Gregory wrote:
On Wed, Nov 04, 2009 at 05:53:55PM +0000, Liam Girdwood wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk
- /* Default div to 1 if it wasn't set by machine driver, otherwise
* use set div as the maximum clock value*/- div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1;
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
- /* Check we can fit the requested number of channels into our
* calculated frame size*/- if ((channels * wlen) > frame_size) {
printk(KERN_ERR"OMAP-MCBSP: cannot fit channels in frame size\n");return -EINVAL;- }
- /* Set the actual clkdiv to use for this samplerate */
- regs->srgr1 &= ~CLKGDV(0xFF);
- regs->srgr1 |= CLKGDV(div - 1);
This chunk changes the semantics of other devices which I have never tested.
I also dont know how much damage it does if it does to slave mode. In fact I think it might break it in cases which are actually allowable as it uses the omap as its clock constraint and not the clock source.
Btw, this has been reworked to avoid all I2S paths since you last worked on it.
So it looks like we need a conditional around this block for when the codec is mastering BCLK/FRM and MCBSP is slave (as you stated above). This will then use the original SRGR2 values for SND_SOC_DAIFMT_DSP_A/B in codec master mode.
V2 will follow.
Liam
On Wed, 04 Nov 2009 19:46:49 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
This would be better if it tries to calculate minimum frame size. Now the algorithm stops when the frame_size is 256 and leads to higher bit clock.
E.g. 4 * 16bits * 48 kHz and using 96 MHz internal clock:
Algorithm: div = 8, frame_size = 250 and bit clock = 12 MHz.
Possible dividers and frame_sizes:
25*80 (-> best, bit clock = 3.840 MHz) 20*100 16*125 10*200 8*250
This chunk changes the semantics of other devices which I have never tested.
I also dont know how much damage it does if it does to slave mode. In fact I think it might break it in cases which are actually allowable as it uses the omap as its clock constraint and not the clock source.
IRCC, the CLKGDV doesn't have effect while OMAP is slave but I can test that with Beagle.
Btw, this has been reworked to avoid all I2S paths since you last worked on it.
Probably that is not necessary if the algorithm above finds a frame size (½ for dual-phase frames) which doesn't exceed I2S word size? Uniform code is allways better :-)
On Thu, 2009-11-05 at 09:51 +0200, Jarkko Nikula wrote:
On Wed, 04 Nov 2009 19:46:49 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
This would be better if it tries to calculate minimum frame size. Now the algorithm stops when the frame_size is 256 and leads to higher bit clock.
E.g. 4 * 16bits * 48 kHz and using 96 MHz internal clock:
Algorithm: div = 8, frame_size = 250 and bit clock = 12 MHz.
Possible dividers and frame_sizes:
25*80 (-> best, bit clock = 3.840 MHz) 20*100 16*125 10*200 8*250
I've reworked this myself now. It does appear that the current FPER calculations assume BCLK scales with rate
i.e. BCLK = rate * channels * word len
This is fine for when McBSP is FRM/BCLK slave (all users except pandora) as FPER should be ignored internally.
However, when BCLK is constant (e.g. McBSP BCLK derived from constant source) and we run McBSP as FRM/BCLK master we currently break our sample rate generation.
Imho, it's better to generate FPER based upon BCLK and rate. e.g. we calculate the frame size required for the given BCLK and rate.
/* In McBSP master modes, FRAME (i.e. sample rate) is generated * by _counting_ BCLKs. Calculate frame size in BCLKs */ div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1; framesize = (mcbsp_data->in_freq / div) / params_rate(params);
if (framesize < wlen * channels) { printk(KERN_ERR "%s: not enough bandwidth for desired rate and channels\n", __func__); return -EINVAL; }
/* Set FS period and length in terms of bit clock periods */ switch (format) { case SND_SOC_DAIFMT_I2S: regs->srgr2 |= FPER(framesize - 1); regs->srgr1 |= FWID((framesize >> 1) - 1); break; case SND_SOC_DAIFMT_DSP_A: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(framesize - 1); regs->srgr1 |= FWID(0); break; }
I'm now slightly curious about how pandora handles different rates since it uses the McBSP in master mode too. I guess they can only handle a single sample rate ?
Liam
On Thu, 05 Nov 2009 14:55:30 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
I've reworked this myself now. It does appear that the current FPER calculations assume BCLK scales with rate
i.e. BCLK = rate * channels * word len
This is fine for when McBSP is FRM/BCLK slave (all users except pandora) as FPER should be ignored internally.
However, when BCLK is constant (e.g. McBSP BCLK derived from constant source) and we run McBSP as FRM/BCLK master we currently break our sample rate generation.
This is true, currently possibilities to get standard sample rates as accurately as possible when using McBSP as a master and when using fixed sample rate generator input clock (CLKSRG) are too much limited. So the period size adjustment is a good improvement.
CLKSRG can take its input from internal fixed 96 MHz, CLKS pin, CLKR pin, CLKX pin or internal interface clock (proprotional to CPU freq).
Imho, it's better to generate FPER based upon BCLK and rate. e.g. we calculate the frame size required for the given BCLK and rate.
Yep.
Actually BCLK doesn't have to be fixed as it is derived by dividing the CLKSRG with CLKGDV. This can allow to optimize the BCLK and frame size to be smaller when CLKSRG >> BCLK.
Would be nice if both the divider and frame size are calculated dynamically based on CLKSRG frequency and sample rate.
I'm now slightly curious about how pandora handles different rates since it uses the McBSP in master mode too. I guess they can only handle a single sample rate ?
IRCC correctly the external master clock used in Pandora was derived from the codec and that clock was following the sample rate.
On Thu, 2009-11-05 at 21:28 +0200, Jarkko Nikula wrote:
Actually BCLK doesn't have to be fixed as it is derived by dividing the CLKSRG with CLKGDV. This can allow to optimize the BCLK and frame size to be smaller when CLKSRG >> BCLK.
Would be nice if both the divider and frame size are calculated dynamically based on CLKSRG frequency and sample rate.
This does sound like a useful feature and should probably exist in soc-core for other platforms too.
I'm now slightly curious about how pandora handles different rates since it uses the McBSP in master mode too. I guess they can only handle a single sample rate ?
IRCC correctly the external master clock used in Pandora was derived from the codec and that clock was following the sample rate.
Makes sense now :)
Liam
On Thu, Nov 05, 2009 at 08:08:30PM +0000, Liam Girdwood wrote:
On Thu, 2009-11-05 at 21:28 +0200, Jarkko Nikula wrote:
Would be nice if both the divider and frame size are calculated dynamically based on CLKSRG frequency and sample rate.
This does sound like a useful feature and should probably exist in soc-core for other platforms too.
A lot of drivers would probably have trouble using this due to the number of different constraints in the dividers that can be set up that random hardware has - it'd probably take more effort to finesse things than it's worth - but a utility function for mostly unconstrained hardware would be good.
On Fri, 2009-11-06 at 13:20 +0000, Mark Brown wrote:
On Thu, Nov 05, 2009 at 08:08:30PM +0000, Liam Girdwood wrote:
On Thu, 2009-11-05 at 21:28 +0200, Jarkko Nikula wrote:
Would be nice if both the divider and frame size are calculated dynamically based on CLKSRG frequency and sample rate.
This does sound like a useful feature and should probably exist in soc-core for other platforms too.
A lot of drivers would probably have trouble using this due to the number of different constraints in the dividers that can be set up that random hardware has - it'd probably take more effort to finesse things than it's worth - but a utility function for mostly unconstrained hardware would be good.
I was thinking more generic here. e.g. additions to soc.h :-
static inline int snd_soc_get_framesize(int clock, struct snd_pcm_hw_params *params);
A caller could vary clock based upon it's divider options. Error would be returned if there was not enough bandwidth.
and
static inline int snd_soc_get_divider(int clock, struct snd_pcm_hw_params *params);
This would return the integer divider required or bandwidth error.
and
static inline int snd_soc_get_min_clock(struct snd_pcm_hw_params *params);
return min bit clock required for params.
Liam
On Fri, Nov 06, 2009 at 02:25:20PM +0000, Liam Girdwood wrote:
static inline int snd_soc_get_framesize(int clock, struct snd_pcm_hw_params *params);
That's roughly what I'm thinking of too - it doesn't even need to take the clock in since the frame size is unrelated to the clock in (and the calculation is also useful for devices which have FLLs they can use to vary the system clock to meet their bit clock requirements). We'd also want a versions that takes the TDM parameters in for use when configuring that.
I'll pull the relevant code out of some of my drivers sometime over the next few days.
Most of the devices I've seen that couldn't use this stuff at all actually need to maintain some sample rate ratio for their master clock and don't need to worry about BCLK since the fs restriction usually takes care of it.
static inline int snd_soc_get_divider(int clock, struct snd_pcm_hw_params *params);
This one I'm less sure about, partly due to naming but also because it tends to be where you end up with restrictions that'd need handling in drivers - I'm not sure that you'd save anything that wasn't already saved with the above two functions.
On Wednesday 04 November 2009 20:55:34 ext Graeme Gregory wrote:
This chunk changes the semantics of other devices which I have never tested.
I also dont know how much damage it does if it does to slave mode. In fact I think it might break it in cases which are actually allowable as it uses the omap as its clock constraint and not the clock source.
For the first look this is going to break the slave mode, when McBSP is slave the data->in_freq is going to be 0 (not configured). In McBSP slave mode the sample rate generator (SRG) is not is use.
Graeme
On Wednesday 04 November 2009 19:53:55 ext Liam Girdwood wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk
sound/soc/omap/omap-mcbsp.c | 71 +++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 3341f49..290ef2f 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -49,6 +49,8 @@ struct omap_mcbsp_data { */ int active; int configured;
- unsigned int in_freq;
- int clk_div;
};
#define to_mcbsp(priv) container_of((priv), struct omap_mcbsp_data, bus_id) @@ -257,7 +259,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, int dma, bus_id = mcbsp_data->bus_id, id = cpu_dai->id; int wlen, channels, wpf, sync_mode = OMAP_DMA_SYNC_ELEMENT; unsigned long port;
- unsigned int format;
unsigned int format, frame_size, div;
if (cpu_class_is_omap1()) { dma = omap1_dma_reqs[bus_id][substream->stream];
@@ -294,27 +296,23 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params);
- switch (channels) {
- case 2:
if (format == SND_SOC_DAIFMT_I2S) {/* Use dual-phase frames */regs->rcr2 |= RPHASE;regs->xcr2 |= XPHASE;/* Set 1 word per (McBSP) frame for phase1 and phase2 */wpf--;regs->rcr2 |= RFRLEN2(wpf - 1);regs->xcr2 |= XFRLEN2(wpf - 1);}- case 1:
- case 4:
- if (channels == 2 && format == SND_SOC_DAIFMT_I2S) {
/* Use dual-phase frames */regs->rcr2 |= RPHASE;regs->xcr2 |= XPHASE;/* Set 1 word per (McBSP) frame for phase1 and phase2 */wpf--;regs->rcr2 |= RFRLEN2(wpf - 1);regs->xcr2 |= XFRLEN2(wpf - 1);
break;- default:
- } else if (channels > 0 && channels < 17) {
regs->rcr1 |= RFRLEN1(wpf - 1);regs->xcr1 |= XFRLEN1(wpf - 1);- } else /* Unsupported number of channels */ return -EINVAL;
- }
I would have done this a bit differently:
... /* Check if the number of channels are valid */ if (channels < 1 || channels > 16) { /* Unsupported number of channels */ /* Probably would be good to have some error message about it? */ return -EINVAL; }
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params);
if (channels == 2 && format == SND_SOC_DAIFMT_I2S) { /* Use dual-phase frames */ regs->rcr2 |= RPHASE; regs->xcr2 |= XPHASE; /* Set 1 word per (McBSP) frame for phase1 and phase2 */ wpf--; regs->rcr2 |= RFRLEN2(wpf - 1); regs->xcr2 |= XFRLEN2(wpf - 1); } /* Set word per (McBSP) frame for phase1 */ regs->rcr1 |= RFRLEN1(wpf - 1); regs->xcr1 |= XFRLEN1(wpf - 1);
switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: @@ -330,6 +328,34 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- /* Default div to 1 if it wasn't set by machine driver, otherwise
* use set div as the maximum clock value*/- div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1;
- /* calc best frame size for rate and clock divider */
- do {
frame_size = (mcbsp_data->in_freq / div) / params_rate(params);
I think this as it is now will not work when McBSP is a slave. mcbsp_data->in_freq is going to be 0, since the sample rate generator is not in use.
pr_debug("freq %d, rate %d, frame size %d, div %d\n",mcbsp_data->in_freq, params_rate(params), frame_size, div);if (frame_size > 256)div++;- } while (frame_size > 256);
- /* Check we can fit the requested number of channels into our
* calculated frame size*/- if ((channels * wlen) > frame_size) {
printk(KERN_ERR"OMAP-MCBSP: cannot fit channels in frame size\n");return -EINVAL;- }
On Thu, 2009-11-05 at 10:26 +0200, Peter Ujfalusi wrote:
On Wednesday 04 November 2009 19:53:55 ext Liam Girdwood wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and was sponsored by Shotspotter inc.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk
sound/soc/omap/omap-mcbsp.c | 71 +++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 22 deletions(-)
snip
I would have done this a bit differently:
... /* Check if the number of channels are valid */ if (channels < 1 || channels > 16) { /* Unsupported number of channels */ /* Probably would be good to have some error message about it? */ return -EINVAL; }
Not really required as soc-core takes care of this for us :)
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params);
if (channels == 2 && format == SND_SOC_DAIFMT_I2S) { /* Use dual-phase frames */ regs->rcr2 |= RPHASE; regs->xcr2 |= XPHASE; /* Set 1 word per (McBSP) frame for phase1 and phase2 */ wpf--; regs->rcr2 |= RFRLEN2(wpf - 1); regs->xcr2 |= XFRLEN2(wpf - 1); } /* Set word per (McBSP) frame for phase1 */ regs->rcr1 |= RFRLEN1(wpf - 1); regs->xcr1 |= XFRLEN1(wpf - 1);
I prefer this too.
Liam
participants (5)
-
Graeme Gregory -
Jarkko Nikula -
Liam Girdwood -
Mark Brown -
Peter Ujfalusi