[alsa-devel] [topic/asoc][RFC 0/1] ASoC: OMAP: Add support for mono link configuration to McBSP DAI
Hi
I'm trying to cover also pure mono codecs like bluetooth audio with McBSP DAI driver. Protocol part is trivial like patch here but interfacing with machine driver is somewhat question mark.
With this patch, machine driver needs to "hack" with cpu_dai in its init function:
cpu_dai->playback.channels_min = 1; cpu_dai->playback.channels_max = 1; cpu_dai->capture.channels_min = 1; cpu_dai->capture.channels_max = 1;
Another idea is to introduce new omap_mcbsp_dai_mono[] array in omap-mcbsp.c and initialize those with modified builder macro, e.g:
-#define OMAP_MCBSP_DAI_BUILDER(link_id) \ +#define OMAP_MCBSP_DAI_BUILDER(link_id, min_ch, max_ch) \ { \ .name = "omap-mcbsp-dai-"#link_id, \ .id = (link_id), \ .type = SND_SOC_DAI_I2S, \ .playback = { \ - .channels_min = 2, \ - .channels_max = 2, \ + .channels_min = (min_ch), \ + .channels_max = (max_ch), \ ...
struct snd_soc_dai omap_mcbsp_dai[] = { - OMAP_MCBSP_DAI_BUILDER(0), - OMAP_MCBSP_DAI_BUILDER(1), + OMAP_MCBSP_DAI_BUILDER(0, 2, 2), + OMAP_MCBSP_DAI_BUILDER(1, 2, 2), #if NUM_LINKS >= 3 ...
and
+struct snd_soc_dai omap_mcbsp_dai_mono[] = { + OMAP_MCBSP_DAI_BUILDER(0, 1, 1), + OMAP_MCBSP_DAI_BUILDER(1, 1, 1), +#if NUM_LINKS >= 3 ...
but this looks a bit overkill if typical configuration has only one such a mono link and might confuse if a machine driver still needs to reference those arrays using sequential array index like:
.cpu_dai = &omap_mcbsp_dai[0], and .cpu_dai = &omap_mcbsp_dai_mono[1],
sequential since if I like that cpu_dai->id is also sequential for links.
Opinnions or better ideas :-)
Patch adds support for mono audio link configuration to function omap_mcbsp_dai_hw_params but doesn't expose it in omap_mcbsp_dai table by default.
Main idea behind this is to allow operate with real mono codecs but not break compatibility with existing codecs which can expose their channels_min as 1. Machine driver can use functionality provided by this patch by re-configuring runtime DAI's channels_min and channel_max to 1 for a link having mono codec.
For mono link, the I2S signalling remains the same but only first frame (left channel) is transmitting audio data and second frame is having a null data. In DSP_A, only first frame is transmitted.
Signed-off-by: Jarkko Nikula jarkko.nikula@nokia.com --- sound/soc/omap/omap-mcbsp.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 2eeb135..bb05c76 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -203,7 +203,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs; int dma, bus_id = mcbsp_data->bus_id, id = cpu_dai->id; - int wlen; + int wlen, channels; unsigned long port;
if (cpu_class_is_omap1()) { @@ -232,12 +232,17 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return 0; }
- switch (params_channels(params)) { + channels = params_channels(params); + switch (channels) { case 2: - /* Set 1 word per (McBPSP) frame and use dual-phase frames */ - regs->rcr2 |= RFRLEN2(1 - 1) | RPHASE; + /* Use dual-phase frames */ + regs->rcr2 |= RPHASE; + regs->xcr2 |= XPHASE; + case 1: + /* Set 1 word per (McBSP) frame */ + regs->rcr2 |= RFRLEN2(1 - 1); regs->rcr1 |= RFRLEN1(1 - 1); - regs->xcr2 |= XFRLEN2(1 - 1) | XPHASE; + regs->xcr2 |= XFRLEN2(1 - 1); regs->xcr1 |= XFRLEN1(1 - 1); break; default: @@ -266,8 +271,8 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, regs->srgr1 |= FWID(wlen - 1); break; case SND_SOC_DAIFMT_DSP_A: - regs->srgr2 |= FPER(wlen * 2 - 1); - regs->srgr1 |= FWID(wlen * 2 - 2); + regs->srgr2 |= FPER(wlen * channels - 1); + regs->srgr1 |= FWID(wlen * channels - 2); break; }
On Mon, Nov 24, 2008 at 03:09:23PM +0200, Jarkko Nikula wrote:
I'm trying to cover also pure mono codecs like bluetooth audio with McBSP DAI driver. Protocol part is trivial like patch here but interfacing with machine driver is somewhat question mark.
struct snd_soc_dai omap_mcbsp_dai[] = {
- OMAP_MCBSP_DAI_BUILDER(0, 2, 2),
- OMAP_MCBSP_DAI_BUILDER(1, 2, 2),
...
+struct snd_soc_dai omap_mcbsp_dai_mono[] = {
- OMAP_MCBSP_DAI_BUILDER(0, 1, 1),
- OMAP_MCBSP_DAI_BUILDER(1, 1, 1),
Is it not possible to have the DAI switch between mono and stereo at runtime rather than having a DAI for each? That's the normal approach. The core should constrain things based on the capabilities of the connected device.
On Mon, 24 Nov 2008 13:28:57 +0000 "ext Mark Brown" broonie@sirena.org.uk wrote:
+struct snd_soc_dai omap_mcbsp_dai_mono[] = {
- OMAP_MCBSP_DAI_BUILDER(0, 1, 1),
- OMAP_MCBSP_DAI_BUILDER(1, 1, 1),
Is it not possible to have the DAI switch between mono and stereo at runtime rather than having a DAI for each? That's the normal approach. The core should constrain things based on the capabilities of the connected device.
OMAP McBSP DAI can do almost anything :-)
My problem was this kind of switching correct to do in machine driver?
cpu_dai->playback.channels_min = 1; cpu_dai->playback.channels_max = 1; cpu_dai->capture.channels_min = 1; cpu_dai->capture.channels_max = 1;
Defining channels_min = 1 in omap_mcbsp_dai initializer would cause that link is then switching between mono/stereo depending what user space is asking.
Like "aplay -c 1 /dev/urandom" causes that all machines having codec with "channels_min = 1" and "channels_max = 2" would cause incorrect mono link configuration where alsalib should do the mono->stereo conversion instead and vice versa for pure mono link.
Jarkko
On Mon, Nov 24, 2008 at 03:45:33PM +0200, Jarkko Nikula wrote:
My problem was this kind of switching correct to do in machine driver?
cpu_dai->playback.channels_min = 1; cpu_dai->playback.channels_max = 1; cpu_dai->capture.channels_min = 1; cpu_dai->capture.channels_max = 1;
No, these should be constant. Any dynamic constraints should be applied with the constraints API.
Defining channels_min = 1 in omap_mcbsp_dai initializer would cause that link is then switching between mono/stereo depending what user space is asking.
Yes.
Like "aplay -c 1 /dev/urandom" causes that all machines having codec with "channels_min = 1" and "channels_max = 2" would cause incorrect mono link configuration where alsalib should do the mono->stereo conversion instead and vice versa for pure mono link.
So, this is a bit messy at the moment since we don't really have any DAPM-style routing on the digital side yet. What most of these codecs should probably be doing is declaring a 2 channel minimuim - normally that's what they require as a wire format even if they only actually use the data from one of those channels.
This will be less efficient when these devices are used in a mono configuration but is safer until we can map out the digital side better in software; it's roughly what you're trying to do here on the CPU side.
On Mon, 24 Nov 2008 14:33:04 +0000 "ext Mark Brown" broonie@sirena.org.uk wrote:
On Mon, Nov 24, 2008 at 03:45:33PM +0200, Jarkko Nikula wrote:
My problem was this kind of switching correct to do in machine driver?
cpu_dai->playback.channels_min = 1; cpu_dai->playback.channels_max = 1; cpu_dai->capture.channels_min = 1; cpu_dai->capture.channels_max = 1;
No, these should be constant. Any dynamic constraints should be applied with the constraints API.
Oh my, now feeling like not done my homework. Thanks Mark and sorry about the line noise :-)
Will send soon patchset adding snd_pcm_hw_constraint_minmax(..., SNDRV_PCM_HW_PARAM_CHANNELS to existing OMAP machine drivers + updated version of this patch.
So, this is a bit messy at the moment since we don't really have any DAPM-style routing on the digital side yet. What most of these codecs should probably be doing is declaring a 2 channel minimuim - normally that's what they require as a wire format even if they only actually use the data from one of those channels.
I think it's not worth to change existing codecs now. Keeping 1 channel minimum on codec side allow fascinating hackish DAI configurations like using only one channel from stereo codec or to operate with some intelligent CPU DAI able to do mono->stereo and stereo->mono mixing internally. Yet another line noise from me...
Jarkko
participants (2)
-
Jarkko Nikula -
Mark Brown