[alsa-devel] [PATCH 0/4] ASoC: sst-haswell-pcm: Move controls and DAPM elements to component
The sst-haswell-pcm driver is the only one that registers controls and DAPM elements for a snd_soc_platform_driver. Moving them over to the component that is registered by the same driver make it possible to remove the steal_silbling_dai_hack in the core as well as the extra indirection that is used to register the controls and DAPM elements.
Vinod, Subhransu, can you take a look at this and make sure it works as expected and also keep this in mind for the new DPCM drivers?
Thanks, - Lars
Lars-Peter Clausen (4): ASoC: Add snd_soc_component_{get,set}_drvdata() ASoC: sst-haswell-pcm: Alloc state struct in driver probe() ASoC: sst-haswell-pcm: Move controls and DAPM elements to component ASoC: Remove table based DAPM/control setup support from snd_soc_platform_driver
include/sound/soc.h | 28 ++++++++++--------- sound/soc/intel/sst-haswell-pcm.c | 56 ++++++++++++++++++------------------- sound/soc/soc-core.c | 58 ++++----------------------------------- 3 files changed, 49 insertions(+), 93 deletions(-)
Add Add snd_soc_component_{get,set}_drvdata() similar to snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata(). Also update them to use the new functions internally.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index e43fbb6..e6440d8 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1299,26 +1299,37 @@ static inline void *snd_soc_card_get_drvdata(struct snd_soc_card *card) return card->drvdata; }
+static inline void snd_soc_component_set_drvdata(struct snd_soc_component *c, + void *data) +{ + dev_set_drvdata(c->dev, data); +} + +static inline void *snd_soc_component_get_drvdata(struct snd_soc_component *c) +{ + return dev_get_drvdata(c->dev); +} + static inline void snd_soc_codec_set_drvdata(struct snd_soc_codec *codec, void *data) { - dev_set_drvdata(codec->dev, data); + snd_soc_component_set_drvdata(&codec->component, data); }
static inline void *snd_soc_codec_get_drvdata(struct snd_soc_codec *codec) { - return dev_get_drvdata(codec->dev); + return snd_soc_component_get_drvdata(&codec->component); }
static inline void snd_soc_platform_set_drvdata(struct snd_soc_platform *platform, void *data) { - dev_set_drvdata(platform->dev, data); + snd_soc_component_set_drvdata(&platform->component, data); }
static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platform) { - return dev_get_drvdata(platform->dev); + return snd_soc_component_get_drvdata(&platform->component); }
static inline void snd_soc_pcm_set_drvdata(struct snd_soc_pcm_runtime *rtd,
On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
Add Add snd_soc_component_{get,set}_drvdata() similar to snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata(). Also update them to use the new functions internally.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Reviewed-by: Vinod Koul vinod.koul@intel.com
On 09/03/2014 02:22 PM, Vinod Koul wrote:
On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
Add Add snd_soc_component_{get,set}_drvdata() similar to snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata(). Also update them to use the new functions internally.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Reviewed-by: Vinod Koul vinod.koul@intel.com
Is this for the first patch or for the whole series?
Thanks, - Lars
On Wed, Sep 03, 2014 at 09:06:49PM +0200, Lars-Peter Clausen wrote:
On 09/03/2014 02:22 PM, Vinod Koul wrote:
On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
Add Add snd_soc_component_{get,set}_drvdata() similar to snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata(). Also update them to use the new functions internally.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Reviewed-by: Vinod Koul vinod.koul@intel.com
Is this for the first patch or for the whole series?
Patch only. The HSW driver changes, I think Liam should ack
Resource allocations should happen in driver probe callback rather than in snd_soc_platform probe functions. Especially if the resource is device managed. The snd_soc_* probe/remove functions are mainly intended to be used for things that require the component to be already bound to a card.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/intel/sst-haswell-pcm.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/sst-haswell-pcm.c b/sound/soc/intel/sst-haswell-pcm.c index 61bf6da..1de0958 100644 --- a/sound/soc/intel/sst-haswell-pcm.c +++ b/sound/soc/intel/sst-haswell-pcm.c @@ -778,20 +778,11 @@ static const struct snd_soc_dapm_route graph[] = {
static int hsw_pcm_probe(struct snd_soc_platform *platform) { + struct hsw_priv_data *priv_data = snd_soc_platform_get_drvdata(platform); struct sst_pdata *pdata = dev_get_platdata(platform->dev); - struct hsw_priv_data *priv_data; - struct device *dma_dev; + struct device *dma_dev = pdata->dma_dev; int i, ret = 0;
- if (!pdata) - return -ENODEV; - - dma_dev = pdata->dma_dev; - - priv_data = devm_kzalloc(platform->dev, sizeof(*priv_data), GFP_KERNEL); - priv_data->hsw = pdata->dsp; - snd_soc_platform_set_drvdata(platform, priv_data); - /* allocate DSP buffer page tables */ for (i = 0; i < ARRAY_SIZE(hsw_dais); i++) {
@@ -863,12 +854,23 @@ static const struct snd_soc_component_driver hsw_dai_component = { static int hsw_pcm_dev_probe(struct platform_device *pdev) { struct sst_pdata *sst_pdata = dev_get_platdata(&pdev->dev); + struct hsw_priv_data *priv_data; int ret;
+ if (!sst_pdata) + return -EINVAL; + + priv_data = devm_kzalloc(&pdev->dev, sizeof(*priv_data), GFP_KERNEL); + if (!priv_data) + return -ENOMEM; + ret = sst_hsw_dsp_init(&pdev->dev, sst_pdata); if (ret < 0) return -ENODEV;
+ priv_data->hsw = sst_pdata->dsp; + platform_set_drvdata(pdev, priv_data); + ret = snd_soc_register_platform(&pdev->dev, &hsw_soc_platform); if (ret < 0) goto err_plat;
The sst-haswell-pcm driver registers both a snd_soc_component and a snd_soc_platform and expects that the DAPM widgets for the DAIs registered by component are added to the DAPM context of the platform. This requires us to have a hack in the ASoC core which does so. Moving the DAPM elements over to the component allows us to remove this hack.
While we are at it also move the controls over to the component. The controls don't need the platform for anything other than snd_soc_platform_get_drvdata(), this can easily be replaced by snd_soc_component_get_drvdata(). As the long term goal is to register only a single component this is a step in the right direction.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/intel/sst-haswell-pcm.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/sound/soc/intel/sst-haswell-pcm.c b/sound/soc/intel/sst-haswell-pcm.c index 1de0958..33fc5c3 100644 --- a/sound/soc/intel/sst-haswell-pcm.c +++ b/sound/soc/intel/sst-haswell-pcm.c @@ -138,11 +138,10 @@ static inline unsigned int hsw_ipc_to_mixer(u32 value) static int hsw_stream_volume_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); + struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol); + struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - struct hsw_priv_data *pdata = - snd_soc_platform_get_drvdata(platform); struct hsw_pcm_data *pcm_data = &pdata->pcm[mc->reg]; struct sst_hsw *hsw = pdata->hsw; u32 volume; @@ -176,11 +175,10 @@ static int hsw_stream_volume_put(struct snd_kcontrol *kcontrol, static int hsw_stream_volume_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); + struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol); + struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - struct hsw_priv_data *pdata = - snd_soc_platform_get_drvdata(platform); struct hsw_pcm_data *pcm_data = &pdata->pcm[mc->reg]; struct sst_hsw *hsw = pdata->hsw; u32 volume; @@ -208,8 +206,8 @@ static int hsw_stream_volume_get(struct snd_kcontrol *kcontrol, static int hsw_volume_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); - struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); + struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol); + struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt); struct sst_hsw *hsw = pdata->hsw; u32 volume;
@@ -233,8 +231,8 @@ static int hsw_volume_put(struct snd_kcontrol *kcontrol, static int hsw_volume_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); - struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); + struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol); + struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt); struct sst_hsw *hsw = pdata->hsw; unsigned int volume = 0;
@@ -839,16 +837,16 @@ static struct snd_soc_platform_driver hsw_soc_platform = { .ops = &hsw_pcm_ops, .pcm_new = hsw_pcm_new, .pcm_free = hsw_pcm_free, - .controls = hsw_volume_controls, - .num_controls = ARRAY_SIZE(hsw_volume_controls), - .dapm_widgets = widgets, - .num_dapm_widgets = ARRAY_SIZE(widgets), - .dapm_routes = graph, - .num_dapm_routes = ARRAY_SIZE(graph), };
static const struct snd_soc_component_driver hsw_dai_component = { - .name = "haswell-dai", + .name = "haswell-dai", + .controls = hsw_volume_controls, + .num_controls = ARRAY_SIZE(hsw_volume_controls), + .dapm_widgets = widgets, + .num_dapm_widgets = ARRAY_SIZE(widgets), + .dapm_routes = graph, + .num_dapm_routes = ARRAY_SIZE(graph), };
static int hsw_pcm_dev_probe(struct platform_device *pdev)
There are no users left and new users should rather use the component_driver struct embedded in the snd_soc_platform_driver struct to do this. E.g.:
static const struct snd_soc_platform_driver foobar_driver = { .component_driver = { .dapm_widgets = ..., .num_dapm_widgets = ..., ..., }, ... };
instead of
static const struct snd_soc_platform_driver foobar_driver = { .dapm_widgets = ..., .num_dapm_widgets = ..., ... };
This also allows us to remove the steal_sibling_dai_widgets hack.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc.h | 9 -------- sound/soc/soc-core.c | 58 ++++++---------------------------------------------- 2 files changed, 6 insertions(+), 61 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index e6440d8..3f07a1e 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -764,7 +764,6 @@ struct snd_soc_component { unsigned int num_dapm_widgets; const struct snd_soc_dapm_route *dapm_routes; unsigned int num_dapm_routes; - bool steal_sibling_dai_widgets; struct snd_soc_codec *codec;
int (*probe)(struct snd_soc_component *); @@ -869,14 +868,6 @@ struct snd_soc_platform_driver { int (*pcm_new)(struct snd_soc_pcm_runtime *); void (*pcm_free)(struct snd_pcm *);
- /* Default control and setup, added after probe() is run */ - const struct snd_kcontrol_new *controls; - int num_controls; - const struct snd_soc_dapm_widget *dapm_widgets; - int num_dapm_widgets; - const struct snd_soc_dapm_route *dapm_routes; - int num_dapm_routes; - /* * For platform caused delay reporting. * Optional. diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e982cb0..c3a63a1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1089,7 +1089,6 @@ static int soc_probe_component(struct snd_soc_card *card, struct snd_soc_component *component) { struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); - struct snd_soc_component *dai_component, *component2; struct snd_soc_dai *dai; int ret;
@@ -1116,44 +1115,12 @@ static int soc_probe_component(struct snd_soc_card *card, } }
- /* - * This is rather ugly, but certain platforms expect that the DAPM - * widgets for the DAIs for components with the same parent device are - * created in the platforms DAPM context. Until that is fixed we need to - * keep this. - */ - if (component->steal_sibling_dai_widgets) { - dai_component = NULL; - list_for_each_entry(component2, &component_list, list) { - if (component == component2) - continue; - - if (component2->dev == component->dev && - !list_empty(&component2->dai_list)) { - dai_component = component2; - break; - } - } - } else { - dai_component = component; - list_for_each_entry(component2, &component_list, list) { - if (component2->dev == component->dev && - component2->steal_sibling_dai_widgets) { - dai_component = NULL; - break; - } - } - } - - if (dai_component) { - list_for_each_entry(dai, &dai_component->dai_list, list) { - snd_soc_dapm_new_dai_widgets(dapm, dai); - if (ret != 0) { - dev_err(component->dev, - "Failed to create DAI widgets %d\n", - ret); - goto err_probe; - } + list_for_each_entry(dai, &component->dai_list, list) { + ret = snd_soc_dapm_new_dai_widgets(dapm, dai); + if (ret != 0) { + dev_err(component->dev, + "Failed to create DAI widgets %d\n", ret); + goto err_probe; } }
@@ -4165,19 +4132,6 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform,
platform->dev = dev; platform->driver = platform_drv; - if (platform_drv->controls) { - platform->component.controls = platform_drv->controls; - platform->component.num_controls = platform_drv->num_controls; - } - if (platform_drv->dapm_widgets) { - platform->component.dapm_widgets = platform_drv->dapm_widgets; - platform->component.num_dapm_widgets = platform_drv->num_dapm_widgets; - platform->component.steal_sibling_dai_widgets = true; - } - if (platform_drv->dapm_routes) { - platform->component.dapm_routes = platform_drv->dapm_routes; - platform->component.num_dapm_routes = platform_drv->num_dapm_routes; - }
if (platform_drv->probe) platform->component.probe = snd_soc_platform_drv_probe;
On Wed, Aug 20, 2014 at 01:08:45PM +0200, Lars-Peter Clausen wrote:
The sst-haswell-pcm driver is the only one that registers controls and DAPM elements for a snd_soc_platform_driver. Moving them over to the component that is registered by the same driver make it possible to remove the steal_silbling_dai_hack in the core as well as the extra indirection that is used to register the controls and DAPM elements.
Applied all, thanks.
participants (3)
-
Lars-Peter Clausen
-
Mark Brown
-
Vinod Koul