[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <s5hsh4o8nq1.wl-tiwai@suse.de>
Date: Thu, 12 Jul 2018 17:42:30 +0200
From: Takashi Iwai <tiwai@...e.de>
To: "Pierre-Louis Bossart" <pierre-louis.bossart@...ux.intel.com>
Cc: <alsa-devel@...a-project.org>, <lgirdwood@...il.com>,
<broonie@...nel.org>,
"Srinivas Kandagatla" <srinivas.kandagatla@...aro.org>,
<bgoswami@...eaurora.org>, <rohitkr@...eaurora.org>,
<vkoul@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [alsa-devel] [PATCH] ASoC: core: add support to card re-bind/unbind using component framework
On Thu, 12 Jul 2018 17:26:00 +0200,
Pierre-Louis Bossart wrote:
>
> On 7/11/18 3:43 AM, Srinivas Kandagatla wrote:
> > This patch aims at add achieving dynamic behaviour of audio card when
> > the dependent components disappear and reappear.
> >
> > With this patch the card is removed if any of the dependent component
> > is removed and card is added back if the dependent component comes back.
> > All this is done using component framework and matching based on
> > component name.
>
> Humm, no real comment on this patch proper but rather on how userspace
> would deal with this dynamic behavior?
> We had similar opens when we worked on the BYT/CHT HDMI stuff and
> ended-up with quite a few issues in userspace (PulseAudio mainly)
> related to dynamic behavior at the kernel level as a result of
> plug/unplug. Also not sure how an Android HAL would deal with a card
> disappearing temporarily if DSP resources become unavailable or
> unresponsive. Any thoughts or guidance you might think of?
Was the card-level register / unregister really problematic with PA?
It's basically similar as the hotplug like USB, so I thought it would
work as is.
Takashi
>
> >
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> > ---
> >
> > During discussion regarding card re-binding when components unregister
> > and register back at https://lkml.org/lkml/2018/7/9/785 it was suggested
> > that component framework can be added into core to provide this feature.
> >
> > With this new changes the card will re-bind once the dependent component
> > re-registers after unregistering. This works based on the match done
> > from component name using component framework.
> >
> > I have tested this patch with qdsp start-stop usecase for more than 10000
> > times in loop on Qcom platforms.
> >
> > I will send qdsp side cleanup patches once I get some feedback on this patch.
> >
> >
> > include/sound/soc.h | 5 ++++
> > sound/soc/soc-core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 70 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/sound/soc.h b/include/sound/soc.h
> > index 870ba6b64817..b94149d29c0d 100644
> > --- a/include/sound/soc.h
> > +++ b/include/sound/soc.h
> > @@ -17,6 +17,7 @@
> > #include <linux/workqueue.h>
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > +#include <linux/component.h>
> > #include <linux/regmap.h>
> > #include <linux/log2.h>
> > #include <sound/core.h>
> > @@ -1088,6 +1089,10 @@ struct snd_soc_card {
> > struct work_struct deferred_resume_work;
> > + /* component framework related */
> > + bool components_added;
> > + struct component_match *match;
> > +
> > /* lists of probed devices belonging to this card */
> > struct list_head component_dev_list;
> > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> > index 6d33634b934b..377ed8e67686 100644
> > --- a/sound/soc/soc-core.c
> > +++ b/sound/soc/soc-core.c
> > @@ -279,11 +279,31 @@ static inline void snd_soc_debugfs_exit(void)
> > #endif
> > +static int snd_soc_card_comp_compare(struct device *dev, void
> > *data)
> > +{
> > + struct snd_soc_component *component = NULL;
> > + struct snd_soc_component *t;
> > +
> > + lockdep_assert_held(&client_mutex);
> > + list_for_each_entry(t, &component_list, list) {
> > + if (dev == t->dev) {
> > + component = t;
> > + break;
> > + }
> > + }
> > +
> > + if (component && !strcmp(component->name, data))
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd,
> > struct snd_soc_component *component)
> > {
> > struct snd_soc_rtdcom_list *rtdcom;
> > struct snd_soc_rtdcom_list *new_rtdcom;
> > + char *cname;
> > for_each_rtdcom(rtd, rtdcom) {
> > /* already connected */
> > @@ -300,6 +320,13 @@ static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd,
> > list_add_tail(&new_rtdcom->list, &rtd->component_list);
> > + if (!rtd->card->components_added) {
> > + cname = devm_kasprintf(rtd->card->dev, GFP_KERNEL,
> > + "%s", component->name);
> > + component_match_add(rtd->card->dev, &rtd->card->match,
> > + snd_soc_card_comp_compare, cname);
> > + }
> > +
> > return 0;
> > }
> > @@ -835,6 +862,28 @@ static bool soc_is_dai_link_bound(struct
> > snd_soc_card *card,
> > return false;
> > }
> > +static int snd_soc_card_comp_bind(struct device *dev)
> > +{
> > + struct snd_soc_card *card = dev_get_drvdata(dev);
> > +
> > + if (card->instantiated)
> > + return 0;
> > +
> > + return snd_soc_register_card(card);
> > +}
> > +
> > +static void snd_soc_card_comp_unbind(struct device *dev)
> > +{
> > + struct snd_soc_card *card = dev_get_drvdata(dev);
> > +
> > + snd_soc_unregister_card(card);
> > +}
> > +
> > +static const struct component_master_ops snd_soc_card_comp_ops = {
> > + .bind = snd_soc_card_comp_bind,
> > + .unbind = snd_soc_card_comp_unbind,
> > +};
> > +
> > static int soc_bind_dai_link(struct snd_soc_card *card,
> > struct snd_soc_dai_link *dai_link)
> > {
> > @@ -2108,6 +2157,12 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
> > card->instantiated = 1;
> > snd_soc_dapm_sync(&card->dapm);
> > + if (!card->components_added) {
> > + component_master_add_with_match(card->dev,
> > + &snd_soc_card_comp_ops,
> > + card->match);
> > + card->components_added = true;
> > + }
> > mutex_unlock(&card->mutex);
> > mutex_unlock(&client_mutex);
> > @@ -3098,11 +3153,6 @@ static void
> > snd_soc_component_cleanup(struct snd_soc_component *component)
> > static void snd_soc_component_del_unlocked(struct
> > snd_soc_component *component)
> > {
> > - struct snd_soc_card *card = component->card;
> > -
> > - if (card)
> > - snd_soc_unregister_card(card);
> > -
> > list_del(&component->list);
> > }
> > @@ -3169,8 +3219,17 @@ int snd_soc_add_component(struct device
> > *dev,
> > snd_soc_component_add(component);
> > + ret = component_add(dev, NULL);
> > + if (ret < 0) {
> > + dev_err(dev, "ASoC: Failed to add Component: %d\n", ret);
> > + goto err_comp;
> > + }
> > +
> > return 0;
> > +err_comp:
> > + soc_remove_component(component);
> > + snd_soc_unregister_dais(component);
> > err_cleanup:
> > snd_soc_component_cleanup(component);
> > err_free:
> > @@ -3218,6 +3277,7 @@ static int __snd_soc_unregister_component(struct device *dev)
> > mutex_unlock(&client_mutex);
> > if (found) {
> > + component_del(dev, NULL);
> > snd_soc_component_cleanup(component);
> > }
> >
> >
>
>
Powered by blists - more mailing lists