| Message ID | 1593233625-14961-16-git-send-email-spujar@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Tegra210 Audio | expand |
Hi Sameer > PCM devices are created for dai links with 'no-pcm' flag as '0'. > Such DAI links have CPU component which implement pcm_construct() > and pcm_destruct() callbacks. Based on this, current patch exposes > a helper function to identify such components and populate 'no_pcm' > flag for DPCM DAI link. (snip) > +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc) > +{ > + struct snd_soc_component *component; > + struct snd_soc_dai *dai; > + > + for_each_component(component) { > + if (!component->driver) > + continue; > + > + for_each_component_dais(component, dai) { > + if (!dai->name || !dlc->dai_name) > + continue; > + > + if (strcmp(dai->name, dlc->dai_name)) > + continue; We can/should NULL poinster check for "dlc->dai_name" on top of this function instead of inside loop ? And then, we can remove "dai->name" check because next strcmp() automatically fail if dai->name was NULL. Thank you for your help !! Best regards --- Kuninori Morimoto
On 6/29/2020 7:08 AM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >> PCM devices are created for dai links with 'no-pcm' flag as '0'. >> Such DAI links have CPU component which implement pcm_construct() >> and pcm_destruct() callbacks. Based on this, current patch exposes >> a helper function to identify such components and populate 'no_pcm' >> flag for DPCM DAI link. > (snip) >> +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc) >> +{ >> + struct snd_soc_component *component; >> + struct snd_soc_dai *dai; >> + >> + for_each_component(component) { >> + if (!component->driver) >> + continue; >> + >> + for_each_component_dais(component, dai) { >> + if (!dai->name || !dlc->dai_name) >> + continue; >> + >> + if (strcmp(dai->name, dlc->dai_name)) >> + continue; > > We can/should NULL poinster check for "dlc->dai_name" on top of > this function instead of inside loop ? > And then, we can remove "dai->name" check because next strcmp() > automatically fail if dai->name was NULL. Sounds good, will update. > > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
Hi Sameer > PCM devices are created for dai links with 'no-pcm' flag as '0'. > Such DAI links have CPU component which implement pcm_construct() > and pcm_destruct() callbacks. Based on this, current patch exposes > a helper function to identify such components and populate 'no_pcm' > flag for DPCM DAI link. > > This helps to have BE<->BE component links where PCM devices need > not be created for CPU components involved in the links. > > Signed-off-by: Sameer Pujar <spujar@nvidia.com> > --- (snip) > +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc) > +{ > + struct snd_soc_component *component; > + struct snd_soc_dai *dai; > + > + for_each_component(component) { > + if (!component->driver) > + continue; > + > + for_each_component_dais(component, dai) { > + if (!dai->name || !dlc->dai_name) > + continue; > + > + if (strcmp(dai->name, dlc->dai_name)) > + continue; > + > + if (component->driver->pcm_construct) > + return true; > + } > + } > + > + return false; > +} At least my CPU driver doesn't use component:pcm_construct but is using DAI:pcm_new for some reasons. I'm not sure checking DAI:pcm here is enough, or not... Thank you for your help !! Best regards --- Kuninori Morimoto
On 6/30/2020 11:37 AM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >> PCM devices are created for dai links with 'no-pcm' flag as '0'. >> Such DAI links have CPU component which implement pcm_construct() >> and pcm_destruct() callbacks. Based on this, current patch exposes >> a helper function to identify such components and populate 'no_pcm' >> flag for DPCM DAI link. >> >> This helps to have BE<->BE component links where PCM devices need >> not be created for CPU components involved in the links. >> >> Signed-off-by: Sameer Pujar <spujar@nvidia.com> >> --- > (snip) >> +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc) >> +{ >> + struct snd_soc_component *component; >> + struct snd_soc_dai *dai; >> + >> + for_each_component(component) { >> + if (!component->driver) >> + continue; >> + >> + for_each_component_dais(component, dai) { >> + if (!dai->name || !dlc->dai_name) >> + continue; >> + >> + if (strcmp(dai->name, dlc->dai_name)) >> + continue; >> + >> + if (component->driver->pcm_construct) >> + return true; >> + } >> + } >> + >> + return false; >> +} > At least my CPU driver doesn't use component:pcm_construct > but is using DAI:pcm_new for some reasons. > I'm not sure checking DAI:pcm here is enough, or not... OK. If adding DAI:pcm_new above here is not sufficient, then a flag can be used to describe FE component? or is there a better alternative? > > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
Hi Sameer > > At least my CPU driver doesn't use component:pcm_construct > > but is using DAI:pcm_new for some reasons. > > I'm not sure checking DAI:pcm here is enough, or not... > > OK. If adding DAI:pcm_new above here is not sufficient, then a flag > can be used to describe FE component? or is there a better > alternative? soc_component_is_pcm() is called from simple_dai_link_of_dpcm :: "FE" side. I wonder component->driver->non_legacy_dai_naming can't work for you ? Thank you for your help !! Best regards --- Kuninori Morimoto
On 7/2/2020 6:22 AM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >>> At least my CPU driver doesn't use component:pcm_construct >>> but is using DAI:pcm_new for some reasons. >>> I'm not sure checking DAI:pcm here is enough, or not... >> OK. If adding DAI:pcm_new above here is not sufficient, then a flag >> can be used to describe FE component? or is there a better >> alternative? > soc_component_is_pcm() is called from simple_dai_link_of_dpcm :: "FE" side. Yes I had to use this on "FE" side only because I wanted to find a real "FE" in FE<->BE and BE<->BE links. Please have a look at patch [23/23] for the sound DT binding I am using. Basically I want to connect multiple components in a chained fashion (FE <-> BE1 <-> BE2 ... <BEx>). Some of these BEs can be SoC components like resampler/mixer/mux/de-mux etc., the HW I am using has a cross bar which allows me to select/connect BEs in any order and I am trying to have the same flexibility here. Hence I only want to create PCM devices for real "FE" and trying to use simple-card as much as possible. More details about the HW and problems were discussed in [0]. [0] https://lkml.org/lkml/2020/4/30/519 > I wonder component->driver->non_legacy_dai_naming can't work for you ? I see currently in simple-card driver that, BE<->BE link would be treated as CODEC<->CODEC link if 'non_legacy_dai_naming' flag is set at both ends of BE. Do we need to set this flag for all BE? However I am not sure how this will work out for a BE<->BE DPCM DAI link considering the fact that I want to use chain of components and I guess routing map would get complicated. Also going by the flag name it was not meant to differentiate between a FE and BE? > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
Hi Sameer > > I wonder component->driver->non_legacy_dai_naming can't work for you ? > > I see currently in simple-card driver that, BE<->BE link would be > treated as CODEC<->CODEC link if 'non_legacy_dai_naming' flag is set > at both ends of BE. Do we need to set this flag for all BE? > However I am not sure how this will work out for a BE<->BE DPCM DAI > link considering the fact that I want to use chain of components and I > guess routing map would get complicated. Also going by the flag name > it was not meant to differentiate between a FE and BE? OK, non_legacy_dai_naming was just my quick idea. Maybe your soc_component_is_pcm() idea can work, but it seems a littl bit hackish for me. So, can you please 1) Add soc_component_is_pcm() on simple-card, not soc-core ? Maybe we can move it to soc-core later, but want to keep it under simple-card, so far. 2) Use it with data->component_chaining, and some comment ? non component_chaining user doesn't get damage in worst case, and easy to understand. /* * This is for BE<->BE connection. * It needs to ... * It is assumng ... * Note is ... */ if (data->component_chaining && !soc_component_is_pcm(cpus)) dai_link->no_pcm = 1; 3) maybe you can reuse snd_soc_find_dai() for soc_component_is_pcm() ? dai = snd_soc_find_dai(dlc); if (dai && (dai->pcm_new || dai->component->driver->pcm_construct)) return xxx Thank you for your help !! Best regards --- Kuninori Morimoto
On 7/2/2020 2:20 PM, Kuninori Morimoto wrote: > External email: Use caution opening links or attachments > > > Hi Sameer > >>> I wonder component->driver->non_legacy_dai_naming can't work for you ? >> I see currently in simple-card driver that, BE<->BE link would be >> treated as CODEC<->CODEC link if 'non_legacy_dai_naming' flag is set >> at both ends of BE. Do we need to set this flag for all BE? >> However I am not sure how this will work out for a BE<->BE DPCM DAI >> link considering the fact that I want to use chain of components and I >> guess routing map would get complicated. Also going by the flag name >> it was not meant to differentiate between a FE and BE? > OK, non_legacy_dai_naming was just my quick idea. > > Maybe your soc_component_is_pcm() idea can work, > but it seems a littl bit hackish for me. > So, can you please > > 1) Add soc_component_is_pcm() on simple-card, not soc-core ? > Maybe we can move it to soc-core later, > but want to keep it under simple-card, so far. > > 2) Use it with data->component_chaining, and some comment ? > non component_chaining user doesn't get damage in worst case, > and easy to understand. > > /* > * This is for BE<->BE connection. > * It needs to ... > * It is assumng ... > * Note is ... > */ > if (data->component_chaining && > !soc_component_is_pcm(cpus)) > dai_link->no_pcm = 1; > > 3) maybe you can reuse snd_soc_find_dai() for soc_component_is_pcm() ? > > dai = snd_soc_find_dai(dlc); > if (dai && > (dai->pcm_new || dai->component->driver->pcm_construct)) > return xxx Sounds fine, I can make changes as per above points. Thanks. > > Thank you for your help !! > > Best regards > --- > Kuninori Morimoto
diff --git a/include/sound/soc.h b/include/sound/soc.h index 33acead..1e0376f 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -397,6 +397,7 @@ struct snd_soc_dai_driver; struct snd_soc_dai_link; struct snd_soc_component; struct snd_soc_component_driver; +struct snd_soc_dai_link_component; struct soc_enum; struct snd_soc_jack; struct snd_soc_jack_zone; @@ -437,6 +438,7 @@ int snd_soc_add_component(struct device *dev, const struct snd_soc_component_driver *component_driver, struct snd_soc_dai_driver *dai_drv, int num_dai); +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc); int snd_soc_register_component(struct device *dev, const struct snd_soc_component_driver *component_driver, struct snd_soc_dai_driver *dai_drv, int num_dai); diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 62f2978..f19030b 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -188,6 +188,9 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv, if (ret) goto out_put_node; + if (!soc_component_is_pcm(cpus)) + dai_link->no_pcm = 1; + ret = asoc_simple_parse_clk_cpu(dev, np, dai_link, dai); if (ret < 0) goto out_put_node; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 9041a03..d2a47c3 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -259,6 +259,31 @@ static inline void snd_soc_debugfs_exit(void) #endif +bool soc_component_is_pcm(struct snd_soc_dai_link_component *dlc) +{ + struct snd_soc_component *component; + struct snd_soc_dai *dai; + + for_each_component(component) { + if (!component->driver) + continue; + + for_each_component_dais(component, dai) { + if (!dai->name || !dlc->dai_name) + continue; + + if (strcmp(dai->name, dlc->dai_name)) + continue; + + if (component->driver->pcm_construct) + return true; + } + } + + return false; +} +EXPORT_SYMBOL_GPL(soc_component_is_pcm); + static int snd_soc_rtd_add_component(struct snd_soc_pcm_runtime *rtd, struct snd_soc_component *component) {
PCM devices are created for dai links with 'no-pcm' flag as '0'. Such DAI links have CPU component which implement pcm_construct() and pcm_destruct() callbacks. Based on this, current patch exposes a helper function to identify such components and populate 'no_pcm' flag for DPCM DAI link. This helps to have BE<->BE component links where PCM devices need not be created for CPU components involved in the links. Signed-off-by: Sameer Pujar <spujar@nvidia.com> --- include/sound/soc.h | 2 ++ sound/soc/generic/simple-card.c | 3 +++ sound/soc/soc-core.c | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+)