[alsa-devel] [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
When using table based DAPM setup there is no need to register DAPM elements for different sub-components separately. The widgets will be registered before the first sub-component is initialized, the routes are only added after the last sub-component has been initialized, meaning everything will be available when it is needed.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/omap/omap-abe-twl6040.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c index ebb1390..5011bfa 100644 --- a/sound/soc/omap/omap-abe-twl6040.c +++ b/sound/soc/omap/omap-abe-twl6040.c @@ -163,6 +163,10 @@ static const struct snd_soc_dapm_route audio_map[] = {
{"AFML", NULL, "Line In"}, {"AFMR", NULL, "Line In"}, + + /* DMIC routing */ + {"DMic", NULL, "Digital Mic"}, + {"Digital Mic", NULL, "Digital Mic1 Bias"}, };
static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd) @@ -196,20 +200,6 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static const struct snd_soc_dapm_route dmic_audio_map[] = { - {"DMic", NULL, "Digital Mic"}, - {"Digital Mic", NULL, "Digital Mic1 Bias"}, -}; - -static int omap_abe_dmic_init(struct snd_soc_pcm_runtime *rtd) -{ - struct snd_soc_codec *codec = rtd->codec; - struct snd_soc_dapm_context *dapm = &codec->dapm; - - return snd_soc_dapm_add_routes(dapm, dmic_audio_map, - ARRAY_SIZE(dmic_audio_map)); -} - /* Digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link abe_twl6040_dai_links[] = { { @@ -229,7 +219,6 @@ static struct snd_soc_dai_link abe_twl6040_dai_links[] = { .codec_dai_name = "dmic-hifi", .platform_name = "omap-pcm-audio", .codec_name = "dmic-codec", - .init = omap_abe_dmic_init, .ops = &omap_abe_dmic_ops, }, };
Use table based setup to register the controls and DAPM widgets and routes. This on one hand makes the code a bit shorter and cleaner and on the other hand the board level DAPM elements get registered in the card's DAPM context rather than in the CODEC's DAPM context.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/omap/rx51.c | 47 +++++++---------------------------------------- 1 file changed, 7 insertions(+), 40 deletions(-)
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 611179c..a15ffda 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -233,9 +233,6 @@ static const struct snd_soc_dapm_widget aic34_dapm_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", rx51_hp_event), SND_SOC_DAPM_MIC("HS Mic", NULL), SND_SOC_DAPM_LINE("FM Transmitter", NULL), -}; - -static const struct snd_soc_dapm_widget aic34_dapm_widgetsb[] = { SND_SOC_DAPM_SPK("Earphone", NULL), };
@@ -249,9 +246,7 @@ static const struct snd_soc_dapm_route audio_map[] = {
{"DMic Rate 64", NULL, "Mic Bias"}, {"Mic Bias", NULL, "DMic"}, -};
-static const struct snd_soc_dapm_route audio_mapb[] = { {"b LINE2R", NULL, "MONO_LOUT"}, {"Earphone", NULL, "b HPLOUT"},
@@ -277,9 +272,6 @@ static const struct snd_kcontrol_new aic34_rx51_controls[] = { SOC_ENUM_EXT("Jack Function", rx51_enum[2], rx51_get_jack, rx51_set_jack), SOC_DAPM_PIN_SWITCH("FM Transmitter"), -}; - -static const struct snd_kcontrol_new aic34_rx51_controlsb[] = { SOC_DAPM_PIN_SWITCH("Earphone"), };
@@ -294,19 +286,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "MIC3R"); snd_soc_dapm_nc_pin(dapm, "LINE1R");
- /* Add RX-51 specific controls */ - err = snd_soc_add_card_controls(rtd->card, aic34_rx51_controls, - ARRAY_SIZE(aic34_rx51_controls)); - if (err < 0) - return err; - - /* Add RX-51 specific widgets */ - snd_soc_dapm_new_controls(dapm, aic34_dapm_widgets, - ARRAY_SIZE(aic34_dapm_widgets)); - - /* Set up RX-51 specific audio path audio_map */ - snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map)); - err = tpa6130a2_add_controls(codec); if (err < 0) return err; @@ -329,24 +308,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) return err; }
-static int rx51_aic34b_init(struct snd_soc_dapm_context *dapm) -{ - int err; - - err = snd_soc_add_card_controls(dapm->card, aic34_rx51_controlsb, - ARRAY_SIZE(aic34_rx51_controlsb)); - if (err < 0) - return err; - - err = snd_soc_dapm_new_controls(dapm, aic34_dapm_widgetsb, - ARRAY_SIZE(aic34_dapm_widgetsb)); - if (err < 0) - return 0; - - return snd_soc_dapm_add_routes(dapm, audio_mapb, - ARRAY_SIZE(audio_mapb)); -} - /* Digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link rx51_dai[] = { { @@ -367,7 +328,6 @@ static struct snd_soc_aux_dev rx51_aux_dev[] = { { .name = "TLV320AIC34b", .codec_name = "tlv320aic3x-codec.2-0019", - .init = rx51_aic34b_init, }, };
@@ -388,6 +348,13 @@ static struct snd_soc_card rx51_sound_card = { .num_aux_devs = ARRAY_SIZE(rx51_aux_dev), .codec_conf = rx51_codec_conf, .num_configs = ARRAY_SIZE(rx51_codec_conf), + + .controls = aic34_rx51_controls, + .num_controls = ARRAY_SIZE(aic34_rx51_controls), + .dapm_widgets = aic34_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(aic34_dapm_widgets), + .dapm_routes = audio_map, + .num_dapm_routes = ARRAY_SIZE(audio_map), };
static struct platform_device *rx51_snd_device;
Use table based setup to register the controls and DAPM widgets and routes. This on one hand makes the code a bit shorter and cleaner and on the other hand the board level DAPM elements get registered in the card's DAPM context rather than in the CODEC's DAPM context.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/omap/omap3pandora.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/sound/soc/omap/omap3pandora.c b/sound/soc/omap/omap3pandora.c index cf604a2..02181bb 100644 --- a/sound/soc/omap/omap3pandora.c +++ b/sound/soc/omap/omap3pandora.c @@ -121,7 +121,7 @@ static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w, * |A| <~~clk~~+ * |P| <--- TWL4030 <--------- Line In and MICs */ -static const struct snd_soc_dapm_widget omap3pandora_out_dapm_widgets[] = { +static const struct snd_soc_dapm_widget omap3pandora_dapm_widgets[] = { SND_SOC_DAPM_DAC_E("PCM DAC", "HiFi Playback", SND_SOC_NOPM, 0, 0, omap3pandora_dac_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), @@ -130,22 +130,18 @@ static const struct snd_soc_dapm_widget omap3pandora_out_dapm_widgets[] = { SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_LINE("Line Out", NULL), -};
-static const struct snd_soc_dapm_widget omap3pandora_in_dapm_widgets[] = { SND_SOC_DAPM_MIC("Mic (internal)", NULL), SND_SOC_DAPM_MIC("Mic (external)", NULL), SND_SOC_DAPM_LINE("Line In", NULL), };
-static const struct snd_soc_dapm_route omap3pandora_out_map[] = { +static const struct snd_soc_dapm_route omap3pandora_map[] = { {"PCM DAC", NULL, "APLL Enable"}, {"Headphone Amplifier", NULL, "PCM DAC"}, {"Line Out", NULL, "PCM DAC"}, {"Headphone Jack", NULL, "Headphone Amplifier"}, -};
-static const struct snd_soc_dapm_route omap3pandora_in_map[] = { {"AUXL", NULL, "Line In"}, {"AUXR", NULL, "Line In"},
@@ -160,7 +156,6 @@ static int omap3pandora_out_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_codec *codec = rtd->codec; struct snd_soc_dapm_context *dapm = &codec->dapm; - int ret;
/* All TWL4030 output pins are floating */ snd_soc_dapm_nc_pin(dapm, "EARPIECE"); @@ -174,20 +169,13 @@ static int omap3pandora_out_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "HFR"); snd_soc_dapm_nc_pin(dapm, "VIBRA");
- ret = snd_soc_dapm_new_controls(dapm, omap3pandora_out_dapm_widgets, - ARRAY_SIZE(omap3pandora_out_dapm_widgets)); - if (ret < 0) - return ret; - - return snd_soc_dapm_add_routes(dapm, omap3pandora_out_map, - ARRAY_SIZE(omap3pandora_out_map)); + return 0; }
static int omap3pandora_in_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_codec *codec = rtd->codec; struct snd_soc_dapm_context *dapm = &codec->dapm; - int ret;
/* Not comnnected */ snd_soc_dapm_nc_pin(dapm, "HSMIC"); @@ -195,13 +183,7 @@ static int omap3pandora_in_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "DIGIMIC0"); snd_soc_dapm_nc_pin(dapm, "DIGIMIC1");
- ret = snd_soc_dapm_new_controls(dapm, omap3pandora_in_dapm_widgets, - ARRAY_SIZE(omap3pandora_in_dapm_widgets)); - if (ret < 0) - return ret; - - return snd_soc_dapm_add_routes(dapm, omap3pandora_in_map, - ARRAY_SIZE(omap3pandora_in_map)); + return 0; }
static struct snd_soc_ops omap3pandora_ops = { @@ -241,6 +223,11 @@ static struct snd_soc_card snd_soc_card_omap3pandora = { .owner = THIS_MODULE, .dai_link = omap3pandora_dai, .num_links = ARRAY_SIZE(omap3pandora_dai), + + .dapm_widgets = omap3pandora_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(omap3pandora_dapm_widgets), + .dapm_routes = omap3pandora_map, + .num_dapm_routes = ARRAY_SIZE(omap3pandora_map), };
static struct platform_device *omap3pandora_snd_device;
Hi,
On 03/08/2014 08:16 PM, Lars-Peter Clausen wrote:
When using table based DAPM setup there is no need to register DAPM elements for different sub-components separately. The widgets will be registered before the first sub-component is initialized, the routes are only added after the last sub-component has been initialized, meaning everything will be available when it is needed.
The reason why we add the DMIC routes in the way we do is that not all boards have DMIC in use. PandaBoards does not have DMIC while SDP/Blaze have. On PandaBoard we do not register the dmic dai link so the widgets are not going to be added and also the dmic DAI and codec will be not loaded on PandaBoards. I think this will cause some warning because of missing "DMic" widget?
On 03/10/2014 09:13 AM, Peter Ujfalusi wrote:
Hi,
On 03/08/2014 08:16 PM, Lars-Peter Clausen wrote:
When using table based DAPM setup there is no need to register DAPM elements for different sub-components separately. The widgets will be registered before the first sub-component is initialized, the routes are only added after the last sub-component has been initialized, meaning everything will be available when it is needed.
The reason why we add the DMIC routes in the way we do is that not all boards have DMIC in use. PandaBoards does not have DMIC while SDP/Blaze have. On PandaBoard we do not register the dmic dai link so the widgets are not going to be added and also the dmic DAI and codec will be not loaded on PandaBoards. I think this will cause some warning because of missing "DMic" widget?
Hm, ok, missed that part. Makes sense. I'll respin the patch to just use the card's DAPM context when registering the DMIC DAPM routes.
- Lars
On Mon, Mar 10, 2014 at 09:18:50AM +0100, Lars-Peter Clausen wrote:
Hm, ok, missed that part. Makes sense. I'll respin the patch to just use the card's DAPM context when registering the DMIC DAPM routes.
In general anything keying this stuff off DT is going to have that sort of thing going on - most if not all of the things that are registering in several chunks are doing so because some of it is conditional.
On 03/10/2014 10:12 AM, Mark Brown wrote:
On Mon, Mar 10, 2014 at 09:18:50AM +0100, Lars-Peter Clausen wrote:
Hm, ok, missed that part. Makes sense. I'll respin the patch to just use the card's DAPM context when registering the DMIC DAPM routes.
In general anything keying this stuff off DT is going to have that sort of thing going on - most if not all of the things that are registering in several chunks are doing so because some of it is conditional.
Before we had the support for card table based setup you'd had to register them in chunks, because the components became available one after another and you couldn't register DAPM elements for one component in the callback of another one. When using the table based setup the widgets are registered before all components are registered and the routes are registered after all the components have been registered.
On Mon, Mar 10, 2014 at 10:24:49AM +0100, Lars-Peter Clausen wrote:
On 03/10/2014 10:12 AM, Mark Brown wrote:
In general anything keying this stuff off DT is going to have that sort of thing going on - most if not all of the things that are registering in several chunks are doing so because some of it is conditional.
Before we had the support for card table based setup you'd had to register them in chunks, because the components became available one after another and you couldn't register DAPM elements for one component in the callback of another one. When using the table based setup the widgets are registered before all components are registered and the routes are registered after all the components have been registered.
No, that's not the case at all - what makes you think that's the case? All table based setup did was move things from code to data, I'm not sure what you mean by registering DAPM elements from one component in the callback for another. The only thing that should be registering things for other components is the card and the general idea was to register everything in late_probe().
On 03/10/2014 11:10 AM, Mark Brown wrote:
On Mon, Mar 10, 2014 at 10:24:49AM +0100, Lars-Peter Clausen wrote:
On 03/10/2014 10:12 AM, Mark Brown wrote:
In general anything keying this stuff off DT is going to have that sort of thing going on - most if not all of the things that are registering in several chunks are doing so because some of it is conditional.
Before we had the support for card table based setup you'd had to register them in chunks, because the components became available one after another and you couldn't register DAPM elements for one component in the callback of another one. When using the table based setup the widgets are registered before all components are registered and the routes are registered after all the components have been registered.
No, that's not the case at all - what makes you think that's the case?
The code ;)
All table based setup did was move things from code to data, I'm not sure what you mean by registering DAPM elements from one component in the callback for another.
I meant the card level DAPM elements that are related to a certain component. Not the DAPM elements of the component itself.
The only thing that should be registering things for other components is the card and the general idea was to register everything in late_probe().
What most machine drivers did before card table based setup is to have a per-component callback in which they did register the card DAPM elements associated with that component. E.g. routes from the CODEC output to a speaker, etc. You couldn't register the card level DAPM elements for one component in the init callback component of another one since the routes depend on the widgets being registered first. So those board drivers kept separate routes and widget tables for separate components. With table based setup for the card the routes are registered after all components have been registered, which means you can register all the routes via one table since all the dependencies are ready. Same is true if you use the card's late_probe callback to register the DAPM routes and widgets.
- Lars
On Mon, Mar 10, 2014 at 11:27:39AM +0100, Lars-Peter Clausen wrote:
On 03/10/2014 11:10 AM, Mark Brown wrote:
The only thing that should be registering things for other components is the card and the general idea was to register everything in late_probe().
What most machine drivers did before card table based setup is to have a per-component callback in which they did register the card DAPM elements associated with that component. E.g. routes from the CODEC output to a speaker, etc. You couldn't register the card level DAPM elements for one component in the init callback component of another one since the routes depend on the widgets being registered first. So those board drivers kept separate routes and widget tables for separate components. With table based setup for the card the
I'm sorry but this just isn't what was happening at all. Remember that most of the code that you're looking at only ever had one component so it really never made any difference which callback people happened to pick so long as it was one of the ones that ran after the CODEC was up.
The only driver that I can think of which did anything like what you describe was smdk-wm8580 and that was still only using one component, it's just that Jassi preferred to split the input and output paths since the DAIs were separate on the CODEC and he felt that was clearer. It didn't make any practical difference, it certainly wasn't due to startup ordering.
routes are registered after all components have been registered, which means you can register all the routes via one table since all the dependencies are ready. Same is true if you use the card's late_probe callback to register the DAPM routes and widgets.
Right, anything that actually cared would be using the late_probe() callback.
On 03/10/2014 12:05 PM, Mark Brown wrote:
On Mon, Mar 10, 2014 at 11:27:39AM +0100, Lars-Peter Clausen wrote:
On 03/10/2014 11:10 AM, Mark Brown wrote:
The only thing that should be registering things for other components is the card and the general idea was to register everything in late_probe().
What most machine drivers did before card table based setup is to have a per-component callback in which they did register the card DAPM elements associated with that component. E.g. routes from the CODEC output to a speaker, etc. You couldn't register the card level DAPM elements for one component in the init callback component of another one since the routes depend on the widgets being registered first. So those board drivers kept separate routes and widget tables for separate components. With table based setup for the card the
I'm sorry but this just isn't what was happening at all. Remember that most of the code that you're looking at only ever had one component so it really never made any difference which callback people happened to pick so long as it was one of the ones that ran after the CODEC was up.
The only driver that I can think of which did anything like what you describe was smdk-wm8580 and that was still only using one component, it's just that Jassi preferred to split the input and output paths since the DAIs were separate on the CODEC and he felt that was clearer. It didn't make any practical difference, it certainly wasn't due to startup ordering.
Take a look at e.g. omap/rx51.c it's doing what I'm describing and I presume it does this for the reasons I described.
But it doesn't really matter. The important thing about this series is that card level DAPM elements and controls should be registered with the card not the CODEC and I think we can agree on that one.
- Lars
On Mon, Mar 10, 2014 at 12:29:33PM +0100, Lars-Peter Clausen wrote:
On 03/10/2014 12:05 PM, Mark Brown wrote:
The only driver that I can think of which did anything like what you describe was smdk-wm8580 and that was still only using one component, it's just that Jassi preferred to split the input and output paths since the DAIs were separate on the CODEC and he felt that was clearer. It didn't make any practical difference, it certainly wasn't due to startup ordering.
Take a look at e.g. omap/rx51.c it's doing what I'm describing and I presume it does this for the reasons I described.
Oh, rx51 is just generally fun - I'd be a bit surprised if it still works. It's actually the out of ASoC probe stuff that's causing problems there, it was trying to do multi-component prior that being supported.
But it doesn't really matter. The important thing about this series is that card level DAPM elements and controls should be registered with the card not the CODEC and I think we can agree on that one.
Right, it's mostly just an alarm bell for review (I'd not looked at the series yet, I tend to defer things until the people working on the driver have had a chance to look).
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Peter Ujfalusi