| Message ID | 20181211020557.61783-1-tony@atomide.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Graph fixes for using multiple endpoints per port | expand |
Hi Tony > Here are two fixes that allow me to have multiple endpoints defined in > the dts file for audio-graph-card. To do that, we need to fix up few > issues as the graph binding Documentation/devicetree/bindings/graph.txt > allows multiple endpoints per port. This allows configuring TDM codecs > for I2S for example. > > Kuninori-san, can you please check if this makes sense to you and > compare against the graph binding? This looks a little bit strange for me. Can you show me your DT for it ? Best regards --- Kuninori Morimoto
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 03:31]: > > Hi Tony > > > Here are two fixes that allow me to have multiple endpoints defined in > > the dts file for audio-graph-card. To do that, we need to fix up few > > issues as the graph binding Documentation/devicetree/bindings/graph.txt > > allows multiple endpoints per port. This allows configuring TDM codecs > > for I2S for example. > > > > Kuninori-san, can you please check if this makes sense to you and > > compare against the graph binding? > > This looks a little bit strange for me. > Can you show me your DT for it ? Sure, adding also Sebastian to Cc. Here's what I currently have for droid 4 dts with two codecs on I2S. Please just ignore the GNSS parts there.. The TDM configuration is all done in the cpcap_audio_codec via set_tdm_slot(). The modem voice call codec is a serdev driver :) I'll need some more time to be able to post patches but it's basically working for voice calls. Regards, Tony 8< --------- diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts --- a/arch/arm/boot/dts/omap4-droid4-xt894.dts +++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts @@ -653,6 +653,28 @@ pinctrl-0 = <&uart1_pins>; interrupts-extended = <&wakeupgen GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH &omap4_pmx_core 0xfc>; + + modem { + compatible = "motorola,mapphone-mdm6600-serdev"; + phys = <&fsusb1_phy>; + phy-names = "usb"; + + mot_mdm6600_audio: audio-codec { + #address-cells = <1>; + #size-cells = <0>; + #sound-dai-cells = <1>; + + port@0 { + mot_mdm6600_audio_codec0: endpoint { + remote-endpoint = <&cpu_dai_mdm>; + }; + }; + }; + + gnss { + compatible = "motorola,mapphone-mdm6600-gnss"; + }; + }; }; &uart3 { @@ -746,12 +768,18 @@ status = "okay"; mcbsp3_port: port { - cpu_dai3: endpoint { + cpu_dai3: endpoint@0 { dai-format = "dsp_a"; frame-master = <&cpcap_audio_codec1>; bitclock-master = <&cpcap_audio_codec1>; remote-endpoint = <&cpcap_audio_codec1>; }; + cpu_dai_mdm: endpoint@1 { + dai-format = "dsp_a"; + frame-master = <&cpcap_audio_codec1>; + bitclock-master = <&cpcap_audio_codec1>; + remote-endpoint = <&mot_mdm6600_audio_codec0>; + }; }; };
Hi Tony > > This looks a little bit strange for me. > > Can you show me your DT for it ? > > Sure, adding also Sebastian to Cc. Here's what I currently have for droid 4 > dts with two codecs on I2S. Please just ignore the GNSS parts there.. > > The TDM configuration is all done in the cpcap_audio_codec via set_tdm_slot(). > The modem voice call codec is a serdev driver :) I'll need some more time to > be able to post patches but it's basically working for voice calls. Hmm... it seems strange... I guess you are using "ti,omap4-mcbsp" driver (= linux/sound/soc/omap/omap-mcbsp.c) Is this correct ? If so, your driver is registering component as ret = devm_snd_soc_register_component(&pdev->dev, &omap_mcbsp_component, &omap_mcbsp_dai, 1); Your driver has only 1 DAI. > mcbsp3_port: port { > - cpu_dai3: endpoint { > + cpu_dai3: endpoint@0 { > dai-format = "dsp_a"; > frame-master = <&cpcap_audio_codec1>; > bitclock-master = <&cpcap_audio_codec1>; > remote-endpoint = <&cpcap_audio_codec1>; > }; > + cpu_dai_mdm: endpoint@1 { > + dai-format = "dsp_a"; > + frame-master = <&cpcap_audio_codec1>; > + bitclock-master = <&cpcap_audio_codec1>; > + remote-endpoint = <&mot_mdm6600_audio_codec0>; > + }; And here, you have 1 port, 2 endpoint. Then, asoc_simple_card_get_dai_id() should return 1. And, your [2/2] patch, I guess you are misunderstanding about "port" vs "endpoint", or omap-mcbsp driver side need to update ? Best regards --- Kuninori Morimoto
Hi Tony, again > > mcbsp3_port: port { > > - cpu_dai3: endpoint { > > + cpu_dai3: endpoint@0 { > > dai-format = "dsp_a"; > > frame-master = <&cpcap_audio_codec1>; > > bitclock-master = <&cpcap_audio_codec1>; > > remote-endpoint = <&cpcap_audio_codec1>; > > }; > > + cpu_dai_mdm: endpoint@1 { > > + dai-format = "dsp_a"; > > + frame-master = <&cpcap_audio_codec1>; > > + bitclock-master = <&cpcap_audio_codec1>; > > + remote-endpoint = <&mot_mdm6600_audio_codec0>; > > + }; audio-graph-scu-card and my posting merged audio-graph-card are assuming multi-endpoint will be used for DPCM purpose. But, above endpoint connection seems you want to is not DPCM (?), I'm not sure. Best regards --- Kuninori Morimoto
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 05:16]: > > Hi Tony > > > > This looks a little bit strange for me. > > > Can you show me your DT for it ? > > > > Sure, adding also Sebastian to Cc. Here's what I currently have for droid 4 > > dts with two codecs on I2S. Please just ignore the GNSS parts there.. > > > > The TDM configuration is all done in the cpcap_audio_codec via set_tdm_slot(). > > The modem voice call codec is a serdev driver :) I'll need some more time to > > be able to post patches but it's basically working for voice calls. > > Hmm... it seems strange... > I guess you are using "ti,omap4-mcbsp" driver (= linux/sound/soc/omap/omap-mcbsp.c) > Is this correct ? Yes. > If so, your driver is registering component as > > ret = devm_snd_soc_register_component(&pdev->dev, > &omap_mcbsp_component, > &omap_mcbsp_dai, 1); > > Your driver has only 1 DAI. Yes I have a patch for omap-mcbsp.c too :) > > mcbsp3_port: port { > > - cpu_dai3: endpoint { > > + cpu_dai3: endpoint@0 { > > dai-format = "dsp_a"; > > frame-master = <&cpcap_audio_codec1>; > > bitclock-master = <&cpcap_audio_codec1>; > > remote-endpoint = <&cpcap_audio_codec1>; > > }; > > + cpu_dai_mdm: endpoint@1 { > > + dai-format = "dsp_a"; > > + frame-master = <&cpcap_audio_codec1>; > > + bitclock-master = <&cpcap_audio_codec1>; > > + remote-endpoint = <&mot_mdm6600_audio_codec0>; > > + }; > > And here, you have 1 port, 2 endpoint. > Then, asoc_simple_card_get_dai_id() should return 1. > > And, your [2/2] patch, > I guess you are misunderstanding about "port" vs "endpoint", > or omap-mcbsp driver side need to update ? Yes omap-mcbsp driver needs to be updated for multiple endpoints. Adding Jarkko and Peter also to Cc, below is the WIP patch that I'm currently using for omap-mcbsp to add more DAIs. So far nothing else to do in the omap-mcbsp as it's the cpcap hardware that configures the TDM timeslots. And I'm currently assuming the first instance is the master, I guess that should be parsed from the the frame-master dts property instead. Regards, Tony 8< ---------------------- diff --git a/sound/soc/omap/omap-mcbsp-priv.h b/sound/soc/omap/omap-mcbsp-priv.h --- a/sound/soc/omap/omap-mcbsp-priv.h +++ b/sound/soc/omap/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/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -28,6 +28,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> @@ -1318,23 +1319,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", @@ -1423,18 +1454,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;
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 05:30]: > > Hi Tony, again > > > > mcbsp3_port: port { > > > - cpu_dai3: endpoint { > > > + cpu_dai3: endpoint@0 { > > > dai-format = "dsp_a"; > > > frame-master = <&cpcap_audio_codec1>; > > > bitclock-master = <&cpcap_audio_codec1>; > > > remote-endpoint = <&cpcap_audio_codec1>; > > > }; > > > + cpu_dai_mdm: endpoint@1 { > > > + dai-format = "dsp_a"; > > > + frame-master = <&cpcap_audio_codec1>; > > > + bitclock-master = <&cpcap_audio_codec1>; > > > + remote-endpoint = <&mot_mdm6600_audio_codec0>; > > > + }; > > audio-graph-scu-card and my posting merged audio-graph-card > are assuming multi-endpoint will be used for DPCM purpose. > > But, above endpoint connection seems you want to is > not DPCM (?), I'm not sure. Yes DPCM should work nicely here :) So to recap, Sebastian already added the cpcap codec a while back, please see arch/arm/boot/dts/omap4-droid4-xt894.dts for the soundcard node. Then see the link below for an earlier email describing how the various components are wired for TDM [0]. Hope that clarifies things a bit more, Tony [0] https://lkml.org/lkml/2018/3/28/405
Hi Tony > > And, your [2/2] patch, > > I guess you are misunderstanding about "port" vs "endpoint", > > or omap-mcbsp driver side need to update ? > > Yes omap-mcbsp driver needs to be updated for multiple endpoints. > > Adding Jarkko and Peter also to Cc, below is the WIP patch that I'm > currently using for omap-mcbsp to add more DAIs. > > So far nothing else to do in the omap-mcbsp as it's the cpcap hardware > that configures the TDM timeslots. And I'm currently assuming the > first instance is the master, I guess that should be parsed from the > the frame-master dts property instead. (snip) > + if (np) > + mcbsp->dai_count = of_graph_get_endpoint_count(np); OK, you have multi DAI. Then, you need to count is "port", not "endpoint". So, your DT will be below. You want to have is *1 for asoc_simple_card_get_dai_id(). You want to double check is *2 (maybe). ports { mcbsp3_port0: port@0 { *1 reg = <0>; cpu_dai3: endpoint { dai-format = "dsp_a"; frame-master = <&cpcap_audio_codec1>; bitclock-master = <&cpcap_audio_codec1>; remote-endpoint = <&cpcap_audio_codec1>; }; }; mcbsp3_port1: port@1 { *1 reg = <1>; cpu_dai_mdm: endpoint { dai-format = "dsp_a"; *2 frame-master = <&cpcap_audio_codec1>; *2 bitclock-master = <&cpcap_audio_codec1>; remote-endpoint = <&mot_mdm6600_audio_codec0>; }; }; }; Best regards --- Kuninori Morimoto
Hi, * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 06:14]: > > Hi Tony > > > > And, your [2/2] patch, > > > I guess you are misunderstanding about "port" vs "endpoint", > > > or omap-mcbsp driver side need to update ? > > > > Yes omap-mcbsp driver needs to be updated for multiple endpoints. > > > > Adding Jarkko and Peter also to Cc, below is the WIP patch that I'm > > currently using for omap-mcbsp to add more DAIs. > > > > So far nothing else to do in the omap-mcbsp as it's the cpcap hardware > > that configures the TDM timeslots. And I'm currently assuming the > > first instance is the master, I guess that should be parsed from the > > the frame-master dts property instead. > (snip) > > + if (np) > > + mcbsp->dai_count = of_graph_get_endpoint_count(np); > > OK, you have multi DAI. > Then, you need to count is "port", not "endpoint". The issue I have with that it does not then follow the binding doc :) See this part in Documentation/devicetree/bindings/graph.txt: "If a single port is connected to more than one remote device, an 'endpoint' child node must be provided for each link." Isn't the I2C TDM case the same as "single port connecected to more than one remote device" rather than multiple ports? To me it seems we're currently only handling the multiple ports case, and not multiple endpoints for a port. Other than fixing that, things should work just as earlier with my two patches. That is unless I accidentally broke something. So just trying to correct the binding usage. Or am I missing something? Regards, Tony
Hi Tony > The issue I have with that it does not then follow the binding doc :) > > See this part in Documentation/devicetree/bindings/graph.txt: > > "If a single port is connected to more than one remote device, an > 'endpoint' child node must be provided for each link." > > Isn't the I2C TDM case the same as "single port connecected to > more than one remote device" rather than multiple ports? > > To me it seems we're currently only handling the multiple ports > case, and not multiple endpoints for a port. Other than fixing > that, things should work just as earlier with my two patches. > That is unless I accidentally broke something. > > So just trying to correct the binding usage. Or am I missing > something? I'm not 100% sure your "I2C TDM case", but you can check multi-endpoint sample on "Example: Multi DAI with DPCM" below. "pcm3168a" is using multi-endpoint. Does this help you ? https://patchwork.kernel.org/patch/10712877/ Best regards --- Kuninori Morimoto
Hi Tony, again > > The issue I have with that it does not then follow the binding doc :) > > > > See this part in Documentation/devicetree/bindings/graph.txt: > > > > "If a single port is connected to more than one remote device, an > > 'endpoint' child node must be provided for each link." My understanding is that 1 "port" is for 1 "physical interface". In sound case, it is 1 "DAI". And, 1 "endpoint" is for 1 "connection". "If a single port is connected to more than one remote device, an 'endpoint' child node must be provided for each link." This meanns, "If 1 DAI (*1) is connected to multiple remote DAIs(*2), this connection is indicated by multiple "endpoint"" or something like that. (*2) DAIA-endpoint---endpoint--\ DAIB-endpoint---endpoint-----DAI (*1) DAIC-endpoint---endpoint--/ > > Isn't the I2C TDM case the same as "single port connecected to > > more than one remote device" rather than multiple ports? I re-checked https://lkml.org/lkml/2018/3/28/405. Are MDM6600 / OMAP4 CPU portion, and are CPCAP / WL1285 Codec portion ? Then, it is not yet supported (on ALSA SoC level?). If my memory was correct, Lars-Peter had some idea for Mux, But, not yet implemented I think. audio-graph[-scu] / simple-card[-scu] are supporting DPCM, but it is for multiple CPU - single Codec. Best regards --- Kuninori Morimoto
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 23:16]: > > Hi Tony > > > The issue I have with that it does not then follow the binding doc :) > > > > See this part in Documentation/devicetree/bindings/graph.txt: > > > > "If a single port is connected to more than one remote device, an > > 'endpoint' child node must be provided for each link." > > > > Isn't the I2C TDM case the same as "single port connecected to > > more than one remote device" rather than multiple ports? > > > > To me it seems we're currently only handling the multiple ports > > case, and not multiple endpoints for a port. Other than fixing > > that, things should work just as earlier with my two patches. > > That is unless I accidentally broke something. > > > > So just trying to correct the binding usage. Or am I missing > > something? > > I'm not 100% sure your "I2C TDM case", but you can check > multi-endpoint sample on "Example: Multi DAI with DPCM" below. > "pcm3168a" is using multi-endpoint. > Does this help you ? > > https://patchwork.kernel.org/patch/10712877/ Hmm, so do you have multiple separate ports at the "&sound" node hardware? If so then yeah multiple ports make sense. But if you only a single physical (I2S?) port at the "&sound" node hardware, then IMO you should only have one port and multiple endpoints there according to the graph.txt binding doc. In my McBSP case there is only a single physical I2S port that can be TDM split into timeslots. Regards, Tony
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181212 00:12]: > > Hi Tony, again > > > > The issue I have with that it does not then follow the binding doc :) > > > > > > See this part in Documentation/devicetree/bindings/graph.txt: > > > > > > "If a single port is connected to more than one remote device, an > > > 'endpoint' child node must be provided for each link." > > My understanding is that 1 "port" is for 1 "physical interface". Yes I agree. > In sound case, it is 1 "DAI". > And, 1 "endpoint" is for 1 "connection". Yes. So I have 1 physical port (mcbsp) TDM split between two codecs (cpcap and mdm). > "If a single port is connected to more than one remote device, an > 'endpoint' child node must be provided for each link." > > This meanns, "If 1 DAI (*1) is connected to multiple remote DAIs(*2), > this connection is indicated by multiple "endpoint"" or something like that. > > (*2) > DAIA-endpoint---endpoint--\ > DAIB-endpoint---endpoint-----DAI (*1) > DAIC-endpoint---endpoint--/ Yeah. So the only thing missing is parsing multiple endpoints at the DAI end :) And that's why the two patches I posted. > > > Isn't the I2C TDM case the same as "single port connecected to > > > more than one remote device" rather than multiple ports? > > I re-checked https://lkml.org/lkml/2018/3/28/405. > Are MDM6600 / OMAP4 CPU portion, and are CPCAP / WL1285 Codec portion ? MDM6600 is a Qualcomm modem running Motorola custom firmware CPCAP is a Motorola custom PMIC for omap4430 SoC WL128 is a WLAN and Bluetooth chip So following your drawing above: There are two separate McBSP instances, here's the one in question: MDM6600 modem-----endpoint--\ CPCAP PMIC codec2-endpoint----McBSP3 on SoC WL1285 Bluetooth--endpoint--/ The other McBSP instance is dedicated for SoC audio: CPCAP PMIC codec1-endpoint---McBSP3 on SoC > Then, it is not yet supported (on ALSA SoC level?). Only the cpcap codec is in the mainline currently, the mdm codec driver has not yet been posted as it depends on some ts 27.010 serdev patches. > If my memory was correct, Lars-Peter had some idea for Mux, > But, not yet implemented I think. Hmm well I don't think much else is needed currently, we already have everything needed at the ASoC level. See yet another WIP patch configuring the TDM for mdm codec voice call by the existing cpcap codec driver below just by implementing .set_tdm_slot function. > audio-graph[-scu] / simple-card[-scu] are supporting DPCM, > but it is for multiple CPU - single Codec. Well with my patches I certainly have the above configuration working just fine with two audio-graph-card instances connected to a single physical McBSP port. Regards, Tony 8< -------------------- diff --git a/sound/soc/codecs/cpcap.c b/sound/soc/codecs/cpcap.c --- a/sound/soc/codecs/cpcap.c +++ b/sound/soc/codecs/cpcap.c @@ -16,6 +16,14 @@ #include <sound/soc.h> #include <sound/tlv.h> +/* Register 512 CPCAP_REG_VAUDIOC --- Audio Regulator and Bias Voltage */ +#define CPCAP_BIT_AUDIO_LOW_PWR 6 +#define CPCAP_BIT_AUD_LOWPWR_SPEED 5 +#define CPCAP_BIT_VAUDIOPRISTBY 4 +#define CPCAP_BIT_VAUDIO_MODE1 2 +#define CPCAP_BIT_VAUDIO_MODE0 1 +#define CPCAP_BIT_V_AUDIO_EN 0 + /* Register 513 CPCAP_REG_CC --- CODEC */ #define CPCAP_BIT_CDC_CLK2 15 #define CPCAP_BIT_CDC_CLK1 14 @@ -251,6 +259,8 @@ struct cpcap_audio { int codec_clk_id; int codec_freq; int codec_format; + + unsigned int voice_call:1; }; static int cpcap_st_workaround(struct snd_soc_dapm_widget *w, @@ -1370,6 +1380,114 @@ static int cpcap_voice_set_dai_fmt(struct snd_soc_dai *codec_dai, return 0; } +/* + * Configure voice call if cpcap->voice_call is set. + * + * We can configure most with snd_soc_dai_set_sysclk(), snd_soc_dai_set_fmt() + * and snd_soc_dai_set_tdm_slot(). This function configures the rest of the + * cpcap related hardware piceses as CPU is not involved in the voice call. + */ +static int cpcap_voice_call(struct snd_soc_dai *dai) +{ + struct snd_soc_component *component = dai->component; + struct cpcap_audio *cpcap = snd_soc_component_get_drvdata(component); + int mask, err; + + /* Maybe enable modem to codec VAUDIO_MODE1? */ + mask = BIT(CPCAP_BIT_VAUDIO_MODE1); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_VAUDIOC, + mask, cpcap->voice_call ? mask : 0); + if (err) + return err; + + /* Maybe clear MIC1_MUX? */ + mask = BIT(CPCAP_BIT_MIC1_MUX); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_TXI, + mask, cpcap->voice_call ? 0 : mask); + if (err) + return err; + + /* Maybe set MIC2_MUX? */ + mask = BIT(CPCAP_BIT_MB_ON1L) | BIT(CPCAP_BIT_MB_ON1R) | + BIT(CPCAP_BIT_MIC2_MUX) | BIT(CPCAP_BIT_MIC2_PGA_EN); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_TXI, + mask, cpcap->voice_call ? mask : 0); + if (err) + return err; + + /* Maybe enable LDSP? */ + mask = BIT(CPCAP_BIT_A2_LDSP_L_EN) | BIT(CPCAP_BIT_A2_LDSP_R_EN); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_RXOA, + mask, cpcap->voice_call ? mask : 0); + if (err) + return err; + + /* Maybe enable CPCAP_BIT_PGA_CDC_EN for call? */ + mask = BIT(CPCAP_BIT_PGA_CDC_EN); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_RXCOA, + mask, cpcap->voice_call ? mask : 0); + if (err) + return err; + + /* Maybe unmute voice? */ + err = snd_soc_dai_digital_mute(dai, !cpcap->voice_call, + SNDRV_PCM_STREAM_PLAYBACK); + if (err) + return err; + + /* Maybe enable modem to codec mic CDC and HPF? */ + mask = BIT(CPCAP_BIT_MIC2_CDC_EN) | BIT(CPCAP_BIT_CDC_EN_RX) | + BIT(CPCAP_BIT_AUDOHPF_1) | BIT(CPCAP_BIT_AUDOHPF_0) | + BIT(CPCAP_BIT_AUDIHPF_1) | BIT(CPCAP_BIT_AUDIHPF_0); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_CC, + mask, cpcap->voice_call ? mask : 0); + if (err) + return err; + + /* Maybe enable modem to codec CDC? */ + mask = BIT(CPCAP_BIT_CDC_CLK_EN); + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_CDI, + mask, cpcap->voice_call ? mask : 0); + + return err; +} + +static int cpcap_voice_set_tdm_slot(struct snd_soc_dai *dai, + unsigned int tx_mask, unsigned int rx_mask, + int slots, int slot_width) +{ + struct snd_soc_component *component = dai->component; + struct cpcap_audio *cpcap = snd_soc_component_get_drvdata(component); + int err, ts_mask, mask; + + /* Modem to codec audio with no CPU involved? */ + if (tx_mask == 0 && rx_mask == 1 && slot_width == 8) + cpcap->voice_call = true; + else + cpcap->voice_call = false; + + ts_mask = 0x7 << CPCAP_BIT_MIC2_TIMESLOT0; + ts_mask |= 0x7 << CPCAP_BIT_MIC1_RX_TIMESLOT0; + + mask = (tx_mask & 0x7) << CPCAP_BIT_MIC2_TIMESLOT0; + mask |= (rx_mask & 0x7) << CPCAP_BIT_MIC1_RX_TIMESLOT0; + + err = regmap_update_bits(cpcap->regmap, CPCAP_REG_CDI, + ts_mask, mask); + if (err) + return err; + + err = cpcap_set_samprate(cpcap, CPCAP_DAI_VOICE, slot_width * 1000); + if (err) + return err; + + err = cpcap_voice_call(dai); + if (err) + return err; + + return 0; +} + static int cpcap_voice_set_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_component *component = dai->component; @@ -1391,6 +1509,7 @@ static const struct snd_soc_dai_ops cpcap_dai_voice_ops = { .hw_params = cpcap_voice_hw_params, .set_sysclk = cpcap_voice_set_dai_sysclk, .set_fmt = cpcap_voice_set_dai_fmt, + .set_tdm_slot = cpcap_voice_set_tdm_slot, .digital_mute = cpcap_voice_set_mute, };
* Tony Lindgren <tony@atomide.com> [181212 00:43]: > The other McBSP instance is dedicated for SoC audio: > > CPCAP PMIC codec1-endpoint---McBSP3 on SoC Sorry this should be McBSP2, not McBSP3 above for the SoC dedicated audio port. Tony
Hi Tony > > https://patchwork.kernel.org/patch/10712877/ > > Hmm, so do you have multiple separate ports at the "&sound" node > hardware? If so then yeah multiple ports make sense. > > But if you only a single physical (I2S?) port at the > "&sound" node hardware, then IMO you should only have one > port and multiple endpoints there according to the graph.txt > binding doc. > > In my McBSP case there is only a single physical I2S port > that can be TDM split into timeslots. Mine has 4 DAIs. Each DAI can output 2ch. These will be merged and wil be 8ch TDM and goes to Codec. But hmm.. it is 4 DAIs, but 1 "physical" interface... So, your patch seems correct, but will breaks DPCM... I will confirm it. Best regards --- Kuninori Morimoto
Hi Tony, again > > > https://patchwork.kernel.org/patch/10712877/ > > > > Hmm, so do you have multiple separate ports at the "&sound" node > > hardware? If so then yeah multiple ports make sense. > > > > But if you only a single physical (I2S?) port at the > > "&sound" node hardware, then IMO you should only have one > > port and multiple endpoints there according to the graph.txt > > binding doc. > > > > In my McBSP case there is only a single physical I2S port > > that can be TDM split into timeslots. > > Mine has 4 DAIs. Each DAI can output 2ch. > These will be merged and wil be 8ch TDM and goes to Codec. > But hmm.. it is 4 DAIs, but 1 "physical" interface... > > So, your patch seems correct, but will breaks DPCM... > I will confirm it. I thought "port" = "DAI", but yeah, "port" = "physical interface". Then, my issue is that we can't judge DAI size from DT. For example, MIXer case, 2 CPU DAIs are connected to 1 Codec. DAI0 --- CPU --- Codec DAI1 / In this case, CPU side needs 2 DAIs, Codec side needs 1 DAI only. For both CPU/Codec case, OF graph will be like below, and we can't judge DAIs size from this. port { ep0: endpint@0 { remote-endpoint = <xxx>; }; ep1: endpint@1 { remote-endpoint = <xxx>; }; } To solve this issue, we need to use "reg" for it. Then, we can get correct DAI ID. /* CPU has 2 DAIs */ CPU { port { ep0: endpint@0 { reg = <0>; remote-endpoint = <xxx>; }; ep1: endpint@1 { reg = <1>; remote-endpoint = <xxx>; }; }; } /* Codec has 1 DAI */ Codec { port { reg = <0>; ep0: endpint@0 { remote-endpoint = <xxx>; }; ep1: endpint@1 { remote-endpoint = <xxx>; }; }; } Can you agree this ? we need extra patch, but it can solve your / my problem. Now I'm posting patches to merging "audio-graph-card" and "DPCM ver audio-graph-card". If you are OK, I will include above solution patch to this patch-set. Current audio-graph doesn't expect your use-case, and I want to avoid conflict. So, "merged" audio-graph should solve your use-case. If you can agree about this, I will post patch-set. Best regards --- Kuninori Morimoto
On 11/12/2018 7.35, Tony Lindgren wrote: > * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 05:16]: >> >> Hi Tony >> >>>> This looks a little bit strange for me. >>>> Can you show me your DT for it ? >>> >>> Sure, adding also Sebastian to Cc. Here's what I currently have for droid 4 >>> dts with two codecs on I2S. Please just ignore the GNSS parts there.. >>> >>> The TDM configuration is all done in the cpcap_audio_codec via set_tdm_slot(). >>> The modem voice call codec is a serdev driver :) I'll need some more time to >>> be able to post patches but it's basically working for voice calls. >> >> Hmm... it seems strange... >> I guess you are using "ti,omap4-mcbsp" driver (= linux/sound/soc/omap/omap-mcbsp.c) >> Is this correct ? > > Yes. > >> If so, your driver is registering component as >> >> ret = devm_snd_soc_register_component(&pdev->dev, >> &omap_mcbsp_component, >> &omap_mcbsp_dai, 1); >> >> Your driver has only 1 DAI. > > Yes I have a patch for omap-mcbsp.c too :) > >>> mcbsp3_port: port { >>> - cpu_dai3: endpoint { >>> + cpu_dai3: endpoint@0 { >>> dai-format = "dsp_a"; >>> frame-master = <&cpcap_audio_codec1>; >>> bitclock-master = <&cpcap_audio_codec1>; >>> remote-endpoint = <&cpcap_audio_codec1>; >>> }; >>> + cpu_dai_mdm: endpoint@1 { >>> + dai-format = "dsp_a"; >>> + frame-master = <&cpcap_audio_codec1>; >>> + bitclock-master = <&cpcap_audio_codec1>; >>> + remote-endpoint = <&mot_mdm6600_audio_codec0>; >>> + }; >> >> And here, you have 1 port, 2 endpoint. >> Then, asoc_simple_card_get_dai_id() should return 1. >> >> And, your [2/2] patch, >> I guess you are misunderstanding about "port" vs "endpoint", >> or omap-mcbsp driver side need to update ? > > Yes omap-mcbsp driver needs to be updated for multiple endpoints. > > Adding Jarkko and Peter also to Cc, below is the WIP patch that I'm > currently using for omap-mcbsp to add more DAIs. Hrm. McBSP have single DX and single DR pin. Iow each McBSP have single DAI. > > So far nothing else to do in the omap-mcbsp as it's the cpcap hardware > that configures the TDM timeslots. And I'm currently assuming the > first instance is the master, I guess that should be parsed from the > the frame-master dts property instead. > > Regards, > > Tony > > 8< ---------------------- > diff --git a/sound/soc/omap/omap-mcbsp-priv.h b/sound/soc/omap/omap-mcbsp-priv.h > --- a/sound/soc/omap/omap-mcbsp-priv.h > +++ b/sound/soc/omap/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/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c > --- a/sound/soc/omap/omap-mcbsp.c > +++ b/sound/soc/omap/omap-mcbsp.c > @@ -28,6 +28,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> > @@ -1318,23 +1319,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; I prefer to have the 'if (mcbsp->pdata->reg_size == 2)' grouped for playback and capture in one place. > + } > + > + return 0; > +} > > static const struct snd_soc_component_driver omap_mcbsp_component = { > .name = "omap-mcbsp", > @@ -1423,18 +1454,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 Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 12/12/2018 2.19, Tony Lindgren wrote: > * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181211 23:16]: >> >> Hi Tony >> >>> The issue I have with that it does not then follow the binding doc :) >>> >>> See this part in Documentation/devicetree/bindings/graph.txt: >>> >>> "If a single port is connected to more than one remote device, an >>> 'endpoint' child node must be provided for each link." >>> >>> Isn't the I2C TDM case the same as "single port connecected to >>> more than one remote device" rather than multiple ports? >>> >>> To me it seems we're currently only handling the multiple ports >>> case, and not multiple endpoints for a port. Other than fixing >>> that, things should work just as earlier with my two patches. >>> That is unless I accidentally broke something. >>> >>> So just trying to correct the binding usage. Or am I missing >>> something? >> >> I'm not 100% sure your "I2C TDM case", but you can check >> multi-endpoint sample on "Example: Multi DAI with DPCM" below. >> "pcm3168a" is using multi-endpoint. >> Does this help you ? >> >> https://patchwork.kernel.org/patch/10712877/ > > Hmm, so do you have multiple separate ports at the "&sound" node > hardware? If so then yeah multiple ports make sense. > > But if you only a single physical (I2S?) port at the > "&sound" node hardware, then IMO you should only have one > port and multiple endpoints there according to the graph.txt > binding doc. > > In my McBSP case there is only a single physical I2S port > that can be TDM split into timeslots. So what is missing from the McBSP driver is to configure the TDM. We never had a hardware which would require it so it is _not_ implemented. imho the 'only' thing is to implement the set_tdm_slot callback for the McBSP DAI. In DT you would have single card with two dai_link section and each section would set different tdm slots to use for the codecs listening on different slots. There is one issue for sure with this setup: the two PCM can not be used at the same time. But we have one DMA channel so if you would open both the PCM stream need to be set up in a way to match with the HW or create a asound.conf file to do some mapping. > > Regards, > > Tony > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Peter Ujfalusi <peter.ujfalusi@ti.com> [181212 13:03]: > On 12/12/2018 2.19, Tony Lindgren wrote: > > In my McBSP case there is only a single physical I2S port > > that can be TDM split into timeslots. > > So what is missing from the McBSP driver is to configure the TDM. We > never had a hardware which would require it so it is _not_ implemented. Curiously.. Nothing needs to be done in the McBSP driver for the droid 4 TDM configuration AFAIK. The CPCAP PMIC is the clock master, and only the PMIC registers need to be configured in this case for the timeslot to switch between codecs connected to McBSP3. > imho the 'only' thing is to implement the set_tdm_slot callback for the > McBSP DAI. In DT you would have single card with two dai_link section > and each section would set different tdm slots to use for the codecs > listening on different slots. > > There is one issue for sure with this setup: the two PCM can not be used > at the same time. But we have one DMA channel so if you would open both > the PCM stream need to be set up in a way to match with the HW or create > a asound.conf file to do some mapping. Yes in the droid 4 TDM case only one device can be used at a time and all that configuration is done in the PMIC codec .set_tdm_slot function. I think it's possible to do more complex configurations where McBSP is the master and would implement a .set_tdm_slot function. But I don't know anything about that and I'm not aware of any such use cases in the mainline kernel. Regards, Tony
* Peter Ujfalusi <peter.ujfalusi@ti.com> [181212 12:47]:
> McBSP have single DX and single DR pin. Iow each McBSP have single DAI.
Yeah it's single physical port for sure. But at least currently we need
additional secondary DAIs for each TDM channel in addition to the primary
DAI managing the McBSP registers.
Currently there's nothing to do for the secondary DAIs, but if McBSP was
also the clock master then the secondary DAI functions would need to take
care of the multiplexing McBSP hardware resources. But that can be done
later if we actually ever see some McBSP being TDM master use case..
Regards,
Tony
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181212 06:52]: > > Hi Tony, again > > > > > https://patchwork.kernel.org/patch/10712877/ > > > > > > Hmm, so do you have multiple separate ports at the "&sound" node > > > hardware? If so then yeah multiple ports make sense. > > > > > > But if you only a single physical (I2S?) port at the > > > "&sound" node hardware, then IMO you should only have one > > > port and multiple endpoints there according to the graph.txt > > > binding doc. > > > > > > In my McBSP case there is only a single physical I2S port > > > that can be TDM split into timeslots. > > > > Mine has 4 DAIs. Each DAI can output 2ch. > > These will be merged and wil be 8ch TDM and goes to Codec. > > But hmm.. it is 4 DAIs, but 1 "physical" interface... > > > > So, your patch seems correct, but will breaks DPCM... > > I will confirm it. > > I thought "port" = "DAI", but yeah, "port" = "physical interface". OK good to hear :) > Then, my issue is that we can't judge DAI size from DT. > For example, MIXer case, 2 CPU DAIs are connected to 1 Codec. > > DAI0 --- CPU --- Codec > DAI1 / > > In this case, CPU side needs 2 DAIs, > Codec side needs 1 DAI only. Oh so the other way around compared to my use case. Hmm. > For both CPU/Codec case, OF graph will be like below, > and we can't judge DAIs size from this. > > port { > ep0: endpint@0 { > remote-endpoint = <xxx>; > }; > ep1: endpint@1 { > remote-endpoint = <xxx>; > }; > } Hmm I too need to add secondary DAIs for McBSP in addition to the primary DAI controlling the McBSP hardware resources. > To solve this issue, we need to use "reg" for it. > Then, we can get correct DAI ID. Hmm yeah maybe. Just to think of other options, maybe also the #size-cells could be used? As the binding allows adding #address-cells and #size-cells to the port node.. Usually if you refer a subnode of a device you just use #address=cells = <2> where the second field would be for the offset. So maybe this could be used for 1 DAI this way: /* Codec has 1 DAI */ Codec { port { #address-cells = <2>; #size-cells = <1>; ep: endpoint { remote-endpoint = <xxx>; }; }; } Where this codec would then need to be referenced with just an additional instance number: foo = <&ep 0>; bar = <&ep 1>; ... And then for a codec with 2 DAIs the usual #size-cells = <1> would be used with numbered endpoints for each DAI the way you already described: port { ep0: endpint@0 { remote-endpoint = <xxx>; }; ep1: endpint@1 { remote-endpoint = <xxx>; }; } Do you think that would work? > Can you agree this ? we need extra patch, > but it can solve your / my problem. Yes it's starting to make sense :) > Now I'm posting patches to merging > "audio-graph-card" and "DPCM ver audio-graph-card". > If you are OK, I will include above solution patch > to this patch-set. Sure, maybe please first check if the #size-cells = <2> option would work though? > Current audio-graph doesn't expect your use-case, > and I want to avoid conflict. > > So, "merged" audio-graph should solve your use-case. > If you can agree about this, I will post patch-set. Yeah I agree, just still wondering what might be the best way to represent 1 DAI vs 2 DAIs. Regards, Tony
Hi Tony > /* Codec has 1 DAI */ > Codec { > port { > #address-cells = <2>; > #size-cells = <1>; > > ep: endpoint { > remote-endpoint = <xxx>; > }; > }; > } (snip) > foo = <&ep 0>; > bar = <&ep 1>; Ahh, it looks nice idea ! but hmm..., it seems dtc doesn't allow us to use #address-cells = <2> for "remote-endpoint". --- ${LINUX}/scripts/dtc/checks.c --- static struct node *get_remote_endpoint(struct check *c, struct dt_info *dti, struct node *endpoint) { ... prop = get_property(endpoint, "remote-endpoint"); if (!prop) return NULL; => phandle = propval_cell(prop); /* Give up if this is an overlay with external references */ ... } I'm not good at dtc, but propval_cell() is assuming it is single address cell. We will have many assert warning on remote-endpoint = <&xxx 0>; Can you please double check it ? And, I'm wonder remote-endpoint need to cross pointing, right ? one way looks nice by address-cells <2>, but other way is ? port { cpu-ep0: endpoint@0 { remote-endpoint = <&codec-ep 0>; }; cpu-ep1: endpoint@1 { remote-endpoint = <&codec-ep 1>; }; }; port { #address-cells = <2>; #size-cells = <1>; codec-ep: endpoint { (*1) remote-endpoint = <&cpu-ep0>; }; }; This case, cpu-epX -> codec-ep are OK. But, codec-ep -> cpu-epX case can point only 1 endpoint (*1). I'm not sure, but maybe this is not a OF graph policy (?) > > Can you agree this ? we need extra patch, > > but it can solve your / my problem. > > Yes it's starting to make sense :) Thanks > > Now I'm posting patches to merging > > "audio-graph-card" and "DPCM ver audio-graph-card". > > If you are OK, I will include above solution patch > > to this patch-set. > > Sure, maybe please first check if the #size-cells = <2> > option would work though? maybe #address-cells, instead of #size-cells ;P But, yeah, please double check it too. > > Current audio-graph doesn't expect your use-case, > > and I want to avoid conflict. > > > > So, "merged" audio-graph should solve your use-case. > > If you can agree about this, I will post patch-set. > > Yeah I agree, just still wondering what might be the best > way to represent 1 DAI vs 2 DAIs. According to OF graph maintainer, he said that counting port / endpoint are not guaranteed, and we need to use "reg". (It is the reason of b6f3fc005a2c8b425d7a0973b43bef05890bf479) If you could double checked, and we could agreed that "reg" is the solution for us. I will post patches. Best regards --- Kuninori Morimoto
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181213 00:24]: > (snip) > > foo = <&ep 0>; > > bar = <&ep 1>; > > Ahh, it looks nice idea ! > but hmm..., it seems dtc doesn't allow us to use #address-cells = <2> > for "remote-endpoint". OK. Yeah let's scrap that then, it does not suit for endpoints it seems. Thanks for checking. > According to OF graph maintainer, he said that > counting port / endpoint are not guaranteed, > and we need to use "reg". > (It is the reason of b6f3fc005a2c8b425d7a0973b43bef05890bf479) > > If you could double checked, and we could agreed > that "reg" is the solution for us. > I will post patches. OK yes your "reg" proposal works for me. Regards, Tony
Hi Tony > > According to OF graph maintainer, he said that > > counting port / endpoint are not guaranteed, > > and we need to use "reg". > > (It is the reason of b6f3fc005a2c8b425d7a0973b43bef05890bf479) > > > > If you could double checked, and we could agreed > > that "reg" is the solution for us. > > I will post patches. > > OK yes your "reg" proposal works for me. OK, thanks. I will post patch-set, soon. Best regards --- Kuninori Morimoto
* Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [181213 01:06]: > > Hi Tony > > > > According to OF graph maintainer, he said that > > > counting port / endpoint are not guaranteed, > > > and we need to use "reg". > > > (It is the reason of b6f3fc005a2c8b425d7a0973b43bef05890bf479) > > > > > > If you could double checked, and we could agreed > > > that "reg" is the solution for us. > > > I will post patches. > > > > OK yes your "reg" proposal works for me. > > OK, thanks. > I will post patch-set, soon. OK sounds good to me. Thanks, Tony
On 12/12/2018 16.50, Tony Lindgren wrote: > * Peter Ujfalusi <peter.ujfalusi@ti.com> [181212 13:03]: >> On 12/12/2018 2.19, Tony Lindgren wrote: >>> In my McBSP case there is only a single physical I2S port >>> that can be TDM split into timeslots. >> >> So what is missing from the McBSP driver is to configure the TDM. We >> never had a hardware which would require it so it is _not_ implemented. > > Curiously.. Nothing needs to be done in the McBSP driver for the droid > 4 TDM configuration AFAIK. So you always have 4 timeslot and that's it? > The CPCAP PMIC is the clock master, and only the PMIC registers need to > be configured in this case for the timeslot to switch between codecs > connected to McBSP3. The McBSP TDM configuration is not master only. You basically tell McBSP on which timeslot to transmit/receive. Let's say you have two codecs connected to a single McBSP. codec1 is configured to listen for timeslot 0/1 codec2 is configured to listen for timeslot 2/3 If you open a stereo stream to codec1 then you tell McBSP to send/receive the data under timeslot 0/1 and ignore any other timeslots. If you open a stereo stream to codec2 then you tell McBSP to send/receive the data under timeslot 2/3 and ignore any other timeslots. For codec1 you don't really need anything regarding to TDM configuration as McBSP will send/receive right after the start condition on the FS, but for codec2 you need to configure the TDM mode of McBSP to ignore timeslot 0/1 >> imho the 'only' thing is to implement the set_tdm_slot callback for the >> McBSP DAI. In DT you would have single card with two dai_link section >> and each section would set different tdm slots to use for the codecs >> listening on different slots. >> >> There is one issue for sure with this setup: the two PCM can not be used >> at the same time. But we have one DMA channel so if you would open both >> the PCM stream need to be set up in a way to match with the HW or create >> a asound.conf file to do some mapping. > > Yes in the droid 4 TDM case only one device can be used at a time > and all that configuration is done in the PMIC codec .set_tdm_slot > function. Hrm, do you have two DAIs on the PMIC side or different timeslots from the TDM stream is routed to different outputs, similarly to twl6040 where timeslot 0/1 is Headset, timeslot 2/3 is Handsfree and timeslot 5 is to drive a vibra? > I think it's possible to do more complex configurations where McBSP > is the master and would implement a .set_tdm_slot function. But I > don't know anything about that and I'm not aware of any such use > cases in the mainline kernel. No, the set_tdm_slot is applicable for both master and slave mode of McBSP. > > Regards, > > Tony > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Peter Ujfalusi <peter.ujfalusi@ti.com> [181213 06:52]: > On 12/12/2018 16.50, Tony Lindgren wrote: > > * Peter Ujfalusi <peter.ujfalusi@ti.com> [181212 13:03]: > >> On 12/12/2018 2.19, Tony Lindgren wrote: > >>> In my McBSP case there is only a single physical I2S port > >>> that can be TDM split into timeslots. > >> > >> So what is missing from the McBSP driver is to configure the TDM. We > >> never had a hardware which would require it so it is _not_ implemented. > > > > Curiously.. Nothing needs to be done in the McBSP driver for the droid > > 4 TDM configuration AFAIK. > > So you always have 4 timeslot and that's it? No.. "droid 4", not "4 timeslot" above :) See the McBSP3 parts of the diagram at [0]. The PMIC has register bits for timeslots 0 to 2: $ git grep CPCAP | grep TIMESLOT > > The CPCAP PMIC is the clock master, and only the PMIC registers need to > > be configured in this case for the timeslot to switch between codecs > > connected to McBSP3. > > The McBSP TDM configuration is not master only. > You basically tell McBSP on which timeslot to transmit/receive. > Let's say you have two codecs connected to a single McBSP. > codec1 is configured to listen for timeslot 0/1 > codec2 is configured to listen for timeslot 2/3 OK. Right now I'm only configuring things at the PMIC end as the clocking is different compared to using CPCAP PMIC voice codec and when routing data from the MDM voice call codec to the CPCAP PMIC. > If you open a stereo stream to codec1 then you tell McBSP to > send/receive the data under timeslot 0/1 and ignore any other timeslots. > > If you open a stereo stream to codec2 then you tell McBSP to > send/receive the data under timeslot 2/3 and ignore any other timeslots. > > For codec1 you don't really need anything regarding to TDM configuration > as McBSP will send/receive right after the start condition on the FS, > but for codec2 you need to configure the TDM mode of McBSP to ignore > timeslot 0/1 OK. Sounds like what you're describing could be used to route the MDM voice codec audio for capture to a file via McBSP etc. > >> imho the 'only' thing is to implement the set_tdm_slot callback for the > >> McBSP DAI. In DT you would have single card with two dai_link section > >> and each section would set different tdm slots to use for the codecs > >> listening on different slots. > >> > >> There is one issue for sure with this setup: the two PCM can not be used > >> at the same time. But we have one DMA channel so if you would open both > >> the PCM stream need to be set up in a way to match with the HW or create > >> a asound.conf file to do some mapping. > > > > Yes in the droid 4 TDM case only one device can be used at a time > > and all that configuration is done in the PMIC codec .set_tdm_slot > > function. > > Hrm, do you have two DAIs on the PMIC side or different timeslots from > the TDM stream is routed to different outputs, similarly to twl6040 > where timeslot 0/1 is Headset, timeslot 2/3 is Handsfree and timeslot 5 > is to drive a vibra? Two DAIs on the PMIC, one at McBSP2 and one shared with MDM codec at McBSP3. > > I think it's possible to do more complex configurations where McBSP > > is the master and would implement a .set_tdm_slot function. But I > > don't know anything about that and I'm not aware of any such use > > cases in the mainline kernel. > > No, the set_tdm_slot is applicable for both master and slave mode of McBSP. OK sure. Regards, Tony [0] https://lkml.org/lkml/2018/3/28/405