| Message ID | 1593233625-14961-11-git-send-email-spujar@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Tegra210 Audio | expand |
Hi Sameer > simple-audio-card,dai-link@xxx { > format = "i2s"; > bitclock-master=<&cpu1>; > frame-master=<&cpu1>; > > cpu1: cpu@0 { > ... > }; > > codec@0 { > ... > }; > > ... > }; > > In above case CPU is expected to be configured as a master and Codec as > a slave device. But both CPU/Codec are being configured as slave devices. > This happens because asoc_simple_parse_daifmt() uses Codec reference and > sets up the 'dai_link->dai_fmt' accordingly while parsing both CPU and > Codec. I'm sorry but I don't 100% understand about this case... asoc_simple_parse_daifmt() should work in this case The reason why it needs codec node is that SND_SOC_DAIFMT_CBx_CFx are "Codec" base Master/Slave. Thank you for your help !! Best regards --- Kuninori Morimoto
On 6/29/2020 6:26 AM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >> simple-audio-card,dai-link@xxx { >> format = "i2s"; >> bitclock-master=<&cpu1>; >> frame-master=<&cpu1>; >> >> cpu1: cpu@0 { >> ... >> }; >> >> codec@0 { >> ... >> }; >> >> ... >> }; >> >> In above case CPU is expected to be configured as a master and Codec as >> a slave device. But both CPU/Codec are being configured as slave devices. >> This happens because asoc_simple_parse_daifmt() uses Codec reference and >> sets up the 'dai_link->dai_fmt' accordingly while parsing both CPU and >> Codec. > I'm sorry but I don't 100% understand about this case... > asoc_simple_parse_daifmt() should work in this case > > The reason why it needs codec node is that > SND_SOC_DAIFMT_CBx_CFx are "Codec" base Master/Slave. Currently soc-core has following code snippet, /snd_soc_runtime_set_dai_fmt() {// // ...// // // if (cpu_dai->component->driver->non_legacy_dai_naming)// // fmt = inv_dai_fmt;// // // ...// // }/ Above flips polarity for 'cpu_dai' if 'non_legacy_dai_naming' flag is set. 1. Hence example mentioned in the commit message does not work if my 'cpu_dai' driver does not have this flag set. 2. While it is true that we consider reference of 'Codec' mode for simple CPU<->Codec DAI links, for DPCM this does not seem flexible. For DPCM links CPU and Codec are not directly connected (CPU<->Dummy or Dummy<->Codec). Please consider, for example, if the DAI link has multiple CPU/Codecs. Which 'Codec' reference needs to be considered? Isn't it better if we explicitly mention which DAI we want to operate as 'Master'? > > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
Hi Sameer > snd_soc_runtime_set_dai_fmt() { > ... > > if (cpu_dai->component->driver->non_legacy_dai_naming) > fmt = inv_dai_fmt; > > ... > } > > Above flips polarity for 'cpu_dai' if 'non_legacy_dai_naming' flag is set. > > 1. Hence example mentioned in the commit message does not work if my 'cpu_dai' > driver does not have this flag set. ? Do you want fo flip it ? or don't flip? It is for Codec <-> Codec connection. > 2. While it is true that we consider reference of 'Codec' mode for simple CPU<-> > Codec DAI links, for DPCM this does not seem flexible. For DPCM links CPU and > Codec are not directly connected (CPU<->Dummy or Dummy<->Codec). Please > consider, for example, if the DAI link has multiple CPU/Codecs. Which 'Codec' > reference needs to be considered? Isn't it better if we explicitly mention which > DAI we want to operate as 'Master'? I think Lars-Peter has (had ?) plan for this SND_SOC_DAIFMT_CBx_CFx flag flexibility ? Yes maybe it is needed for multi CPU/Codec system. Thank you for your help !! Best regards --- Kuninori Morimoto
On 6/30/2020 6:26 AM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >> snd_soc_runtime_set_dai_fmt() { >> ... >> >> if (cpu_dai->component->driver->non_legacy_dai_naming) >> fmt = inv_dai_fmt; >> >> ... >> } >> >> Above flips polarity for 'cpu_dai' if 'non_legacy_dai_naming' flag is set. >> >> 1. Hence example mentioned in the commit message does not work if my 'cpu_dai' >> driver does not have this flag set. > ? > Do you want fo flip it ? or don't flip? > It is for Codec <-> Codec connection. For DPCM links I don't want to flip based on one Codec reference. My goal was to make the binding work for multiple CPU/Codec link. Hence I thought it would be better to explicitly describe the 'Master' DAI. We can eventually get rid of 'codec' argument from simple_dai_link_of_dpcm(). >> 2. While it is true that we consider reference of 'Codec' mode for simple CPU<-> >> Codec DAI links, for DPCM this does not seem flexible. For DPCM links CPU and >> Codec are not directly connected (CPU<->Dummy or Dummy<->Codec). Please >> consider, for example, if the DAI link has multiple CPU/Codecs. Which 'Codec' >> reference needs to be considered? Isn't it better if we explicitly mention which >> DAI we want to operate as 'Master'? > I think Lars-Peter has (had ?) plan for this SND_SOC_DAIFMT_CBx_CFx > flag flexibility ? Yes maybe it is needed for multi CPU/Codec system. > > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
Hi Sameer > For DPCM links I don't want to flip based on one Codec reference. My > goal was to make the binding work for multiple CPU/Codec link. Hence I > thought it would be better to explicitly describe the 'Master' DAI. We > can eventually get rid of 'codec' argument from > simple_dai_link_of_dpcm(). Yes. 'codec' argument on current simple_dai_link_of_dpcm() is not good match for multi Codec support. Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 0f443c0..39cdc71 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -228,7 +228,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv, if (ret) goto out_put_node; - ret = asoc_simple_parse_daifmt(dev, node, codec, + ret = asoc_simple_parse_daifmt(dev, node, np, prefix, &dai_link->dai_fmt); if (ret < 0) goto out_put_node;
Consider following DPCM DAI link for example: simple-audio-card,dai-link@xxx { format = "i2s"; bitclock-master=<&cpu1>; frame-master=<&cpu1>; cpu1: cpu@0 { ... }; codec@0 { ... }; ... }; In above case CPU is expected to be configured as a master and Codec as a slave device. But both CPU/Codec are being configured as slave devices. This happens because asoc_simple_parse_daifmt() uses Codec reference and sets up the 'dai_link->dai_fmt' accordingly while parsing both CPU and Codec. Though populating 'non_legacy_dai_naming' flag as true for CPU component can address above issue in simple cases, but with multiple CPU/Codecs with DPCM DAI link it becomes tricky because right now the first Codec in the DAI link is used as reference. This is fixed by passing current DAI link child node reference to asoc_simple_parse_daifmt(). It parses a CPU/Codec node independently and sets daifmt as per 'bitcloclk/frame-master' property. Signed-off-by: Sameer Pujar <spujar@nvidia.com> --- sound/soc/generic/simple-card.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)