From: Tony Lindgren <[email protected]>
We can have multiple connections on a single McBSP instance configured
with audio graph card when using TDM (Time Division Multiplexing). Let's
allow that by configuring dais dynamically.
See Documentation/devicetree/bindings/sound/audio-graph-card.txt and
Documentation/devicetree/bindings/graph.txt for more details for
multiple endpoints.
I've tested this with droid4 where cpcap pmic and modem voice are both
both wired to mcbsp3. I've also tested this on droid4 both with and
without the pending modem audio codec driver that is waiting for n_gsm
serdev dependencies to clear.
Cc: Aaro Koskinen <[email protected]>
Cc: Arthur D. <[email protected]>
Cc: Jarkko Nikula <[email protected]>
Cc: Merlijn Wajer <[email protected]>
Cc: Peter Ujfalusi <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
Signed-off-by: Pavel Machek <[email protected]>
---
sound/soc/ti/omap-mcbsp-priv.h | 2 ++
sound/soc/ti/omap-mcbsp.c | 76 +++++++++++++++++++++++++++++-------------
2 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
index 7865cda4bf0a..9464f5d35822 100644
--- a/sound/soc/ti/omap-mcbsp-priv.h
+++ b/sound/soc/ti/omap-mcbsp-priv.h
@@ -262,6 +262,8 @@ struct omap_mcbsp {
struct omap_mcbsp_platform_data *pdata;
struct omap_mcbsp_st_data *st_data;
struct omap_mcbsp_reg_cfg cfg_regs;
+ struct snd_soc_dai_driver *dais;
+ int dai_count;
struct snd_dmaengine_dai_dma_data dma_data[2];
unsigned int dma_req[2];
int dma_op_mode;
diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
index 6025b30bbe77..189a6461b671 100644
--- a/sound/soc/ti/omap-mcbsp.c
+++ b/sound/soc/ti/omap-mcbsp.c
@@ -14,6 +14,7 @@
#include <linux/pm_runtime.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_graph.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
@@ -1299,23 +1300,53 @@ static int omap_mcbsp_remove(struct snd_soc_dai *dai)
return 0;
}
-static struct snd_soc_dai_driver omap_mcbsp_dai = {
- .probe = omap_mcbsp_probe,
- .remove = omap_mcbsp_remove,
- .playback = {
- .channels_min = 1,
- .channels_max = 16,
- .rates = OMAP_MCBSP_RATES,
- .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
- },
- .capture = {
- .channels_min = 1,
- .channels_max = 16,
- .rates = OMAP_MCBSP_RATES,
- .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
- },
- .ops = &mcbsp_dai_ops,
-};
+static int omap_mcbsp_init_dais(struct omap_mcbsp *mcbsp)
+{
+ struct device_node *np = mcbsp->dev->of_node;
+ int i;
+
+ if (np)
+ mcbsp->dai_count = of_graph_get_endpoint_count(np);
+
+ if (!mcbsp->dai_count)
+ mcbsp->dai_count = 1;
+
+ mcbsp->dais = devm_kcalloc(mcbsp->dev, mcbsp->dai_count,
+ sizeof(*mcbsp->dais), GFP_KERNEL);
+ if (!mcbsp->dais)
+ return -ENOMEM;
+
+ for (i = 0; i < mcbsp->dai_count; i++) {
+ struct snd_soc_dai_driver *dai = &mcbsp->dais[i];
+
+ dai->name = devm_kasprintf(mcbsp->dev, GFP_KERNEL, "%s-dai%i",
+ dev_name(mcbsp->dev), i);
+
+ if (i == 0) {
+ dai->probe = omap_mcbsp_probe;
+ dai->remove = omap_mcbsp_remove;
+ dai->ops = &mcbsp_dai_ops;
+ }
+ dai->playback.channels_min = 1;
+ dai->playback.channels_max = 16;
+ dai->playback.rates = OMAP_MCBSP_RATES;
+ if (mcbsp->pdata->reg_size == 2)
+ dai->playback.formats = SNDRV_PCM_FMTBIT_S16_LE;
+ else
+ dai->playback.formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S32_LE;
+ dai->capture.channels_min = 1;
+ dai->capture.channels_max = 16;
+ dai->capture.rates = OMAP_MCBSP_RATES;
+ if (mcbsp->pdata->reg_size == 2)
+ dai->capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
+ else
+ dai->capture.formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S32_LE;
+ }
+
+ return 0;
+}
static const struct snd_soc_component_driver omap_mcbsp_component = {
.name = "omap-mcbsp",
@@ -1404,18 +1435,17 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
mcbsp->dev = &pdev->dev;
platform_set_drvdata(pdev, mcbsp);
- ret = omap_mcbsp_init(pdev);
+ ret = omap_mcbsp_init_dais(mcbsp);
if (ret)
return ret;
- if (mcbsp->pdata->reg_size == 2) {
- omap_mcbsp_dai.playback.formats = SNDRV_PCM_FMTBIT_S16_LE;
- omap_mcbsp_dai.capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
- }
+ ret = omap_mcbsp_init(pdev);
+ if (ret)
+ return ret;
ret = devm_snd_soc_register_component(&pdev->dev,
&omap_mcbsp_component,
- &omap_mcbsp_dai, 1);
+ mcbsp->dais, mcbsp->dai_count);
if (ret)
return ret;
--
2.11.0
--
http://www.livejournal.com/~pavelmachek
Hi Pavel,
On 1/24/21 11:27 AM, Pavel Machek wrote:
> From: Tony Lindgren <[email protected]>
>
> We can have multiple connections on a single McBSP instance configured
> with audio graph card when using TDM (Time Division Multiplexing). Let's
> allow that by configuring dais dynamically.
Still we have _one_ DAI per McBSP. TDM slots are not DAIs.
> See Documentation/devicetree/bindings/sound/audio-graph-card.txt and
> Documentation/devicetree/bindings/graph.txt for more details for
> multiple endpoints.
Documentation/devicetree/bindings/sound/audio-graph-card.yaml
Documentation/devicetree/bindings/sound/audio-graph.yaml
However the schemas does not provide too much insight (the txts were a
bit more verbose).
> I've tested this with droid4 where cpcap pmic and modem voice are both
> both wired to mcbsp3. I've also tested this on droid4 both with and
> without the pending modem audio codec driver that is waiting for n_gsm
> serdev dependencies to clear.
>
> Cc: Aaro Koskinen <[email protected]>
> Cc: Arthur D. <[email protected]>
> Cc: Jarkko Nikula <[email protected]>
> Cc: Merlijn Wajer <[email protected]>
> Cc: Peter Ujfalusi <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> Signed-off-by: Pavel Machek <[email protected]>
>
> ---
> sound/soc/ti/omap-mcbsp-priv.h | 2 ++
> sound/soc/ti/omap-mcbsp.c | 76 +++++++++++++++++++++++++++++-------------
> 2 files changed, 55 insertions(+), 23 deletions(-)
>
> diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
> index 7865cda4bf0a..9464f5d35822 100644
> --- a/sound/soc/ti/omap-mcbsp-priv.h
> +++ b/sound/soc/ti/omap-mcbsp-priv.h
> @@ -262,6 +262,8 @@ struct omap_mcbsp {
> struct omap_mcbsp_platform_data *pdata;
> struct omap_mcbsp_st_data *st_data;
> struct omap_mcbsp_reg_cfg cfg_regs;
> + struct snd_soc_dai_driver *dais;
> + int dai_count;
> struct snd_dmaengine_dai_dma_data dma_data[2];
> unsigned int dma_req[2];
> int dma_op_mode;
> diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
> index 6025b30bbe77..189a6461b671 100644
> --- a/sound/soc/ti/omap-mcbsp.c
> +++ b/sound/soc/ti/omap-mcbsp.c
> @@ -14,6 +14,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_graph.h>
> #include <sound/core.h>
> #include <sound/pcm.h>
> #include <sound/pcm_params.h>
> @@ -1299,23 +1300,53 @@ static int omap_mcbsp_remove(struct snd_soc_dai *dai)
> return 0;
> }
>
> -static struct snd_soc_dai_driver omap_mcbsp_dai = {
> - .probe = omap_mcbsp_probe,
> - .remove = omap_mcbsp_remove,
> - .playback = {
> - .channels_min = 1,
> - .channels_max = 16,
> - .rates = OMAP_MCBSP_RATES,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
> - },
> - .capture = {
> - .channels_min = 1,
> - .channels_max = 16,
> - .rates = OMAP_MCBSP_RATES,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
> - },
> - .ops = &mcbsp_dai_ops,
> -};
> +static int omap_mcbsp_init_dais(struct omap_mcbsp *mcbsp)
> +{
> + struct device_node *np = mcbsp->dev->of_node;
> + int i;
> +
> + if (np)
> + mcbsp->dai_count = of_graph_get_endpoint_count(np);
> +
> + if (!mcbsp->dai_count)
> + mcbsp->dai_count = 1;
> +
> + mcbsp->dais = devm_kcalloc(mcbsp->dev, mcbsp->dai_count,
> + sizeof(*mcbsp->dais), GFP_KERNEL);
> + if (!mcbsp->dais)
> + return -ENOMEM;
> +
> + for (i = 0; i < mcbsp->dai_count; i++) {
> + struct snd_soc_dai_driver *dai = &mcbsp->dais[i];
> +
> + dai->name = devm_kasprintf(mcbsp->dev, GFP_KERNEL, "%s-dai%i",
> + dev_name(mcbsp->dev), i);
> +
> + if (i == 0) {
> + dai->probe = omap_mcbsp_probe;
> + dai->remove = omap_mcbsp_remove;
> + dai->ops = &mcbsp_dai_ops;
> + }
You are effectively creating extra dummy-dais (no ops), but naming them
as McBSP.
The dummy-dais will only work if the real dai is in use and the dai link
which contains the real dai must be configured (TDM slots, format,
channels) to accomodate the link which contains the dummy-dai.
The idea did not aged well for me ;)
It still looks and sounds like a hack to model the TDM slots on a single
DAI as multiple DAIs just to satisfy a generic binding which is not
aimed to satisfy the droid4 setup (which is far from anything simple).
> + dai->playback.channels_min = 1;
> + dai->playback.channels_max = 16;
> + dai->playback.rates = OMAP_MCBSP_RATES;
> + if (mcbsp->pdata->reg_size == 2)
> + dai->playback.formats = SNDRV_PCM_FMTBIT_S16_LE;
> + else
> + dai->playback.formats = SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S32_LE;
> + dai->capture.channels_min = 1;
> + dai->capture.channels_max = 16;
> + dai->capture.rates = OMAP_MCBSP_RATES;
> + if (mcbsp->pdata->reg_size == 2)
> + dai->capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
> + else
> + dai->capture.formats = SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S32_LE;
> + }
> +
> + return 0;
> +}
>
> static const struct snd_soc_component_driver omap_mcbsp_component = {
> .name = "omap-mcbsp",
> @@ -1404,18 +1435,17 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
> mcbsp->dev = &pdev->dev;
> platform_set_drvdata(pdev, mcbsp);
>
> - ret = omap_mcbsp_init(pdev);
> + ret = omap_mcbsp_init_dais(mcbsp);
> if (ret)
> return ret;
>
> - if (mcbsp->pdata->reg_size == 2) {
> - omap_mcbsp_dai.playback.formats = SNDRV_PCM_FMTBIT_S16_LE;
> - omap_mcbsp_dai.capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
> - }
> + ret = omap_mcbsp_init(pdev);
> + if (ret)
> + return ret;
>
> ret = devm_snd_soc_register_component(&pdev->dev,
> &omap_mcbsp_component,
> - &omap_mcbsp_dai, 1);
> + mcbsp->dais, mcbsp->dai_count);
> if (ret)
> return ret;
>
>
--
P?ter
* Péter Ujfalusi <[email protected]> [210126 06:00]:
> On 1/24/21 11:27 AM, Pavel Machek wrote:
> > From: Tony Lindgren <[email protected]>
> > + for (i = 0; i < mcbsp->dai_count; i++) {
> > + struct snd_soc_dai_driver *dai = &mcbsp->dais[i];
> > +
> > + dai->name = devm_kasprintf(mcbsp->dev, GFP_KERNEL, "%s-dai%i",
> > + dev_name(mcbsp->dev), i);
> > +
> > + if (i == 0) {
> > + dai->probe = omap_mcbsp_probe;
> > + dai->remove = omap_mcbsp_remove;
> > + dai->ops = &mcbsp_dai_ops;
> > + }
>
> You are effectively creating extra dummy-dais (no ops), but naming them as
> McBSP.
> The dummy-dais will only work if the real dai is in use and the dai link
> which contains the real dai must be configured (TDM slots, format, channels)
> to accomodate the link which contains the dummy-dai.
>
> The idea did not aged well for me ;)
> It still looks and sounds like a hack to model the TDM slots on a single DAI
> as multiple DAIs just to satisfy a generic binding which is not aimed to
> satisfy the droid4 setup (which is far from anything simple).
After thinking about this a bit more, I think the voice call dai should be
created by the voice dai. When we have an active voice call, the data
transfer is completely out of control of the kernel mcbsp driver. It needs
to be only configured on the pmic.
So I'm suggesting tha we create the modem voice call dai as a child for
sound/soc/codecs/cpcap.c, does that sound OK to you guys?
I think from snd-soc-audio-graph-card binding point of view, we'd just move
the cpu_dai_mdm endpoint out of the mcbsp in the device tree and drop the
$subject patch. Then the dts for the pmic voice codec becomes something
like this (untested):
cpcap_audio: audio-codec {
#sound-dai-cells = <1>;
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
cpcap_audio_codec0: endpoint {
};
};
port@1 {
reg = <1>;
cpcap_audio_codec1: endpoint@0 {
};
cpu_dai_mdm: endpoint@1 {
reg = <1>;
dai-format = "dsp_a";
frame-master = <&cpcap_audio_codec1>;
bitclock-master = <&cpcap_audio_codec1>;
remote-endpoint = <&mot_mdm6600_audio_codec0>;
};
};
};
For things like recording a voice call, I think mcbsp could be configured
to also listen on the traffic and dump it out. I guess that could also
happen directly with calls from the sound/soc/codecs/cpcap.c driver to
the mcbsp driver since we havethe audio graph mapping. Anyways, that's a
separate series of patches, still something to consider.
Regards,
Tony