Question about dapm setup

Question about dapm setup

[Kuninori Morimoto]

Hi ALSA ML

snd_soc_instantiate_card() setups dapm, but its timing seems
very randomly for me.
In my understanding, dapm setup timing is not so serious.
So, I think we can do it in one place,
but are there some reasons ?
For example, "xxxx should be called after yyyy"


static int snd_soc_instantiate_card(struct snd_soc_card *card)
{
	...

=>	snd_soc_dapm_debugfs_init(&card->dapm, card->debugfs_card_root);

	...

	if (card->dapm_widgets)
		snd_soc_dapm_new_controls(&card->dapm, card->dapm_widgets,
					  card->num_dapm_widgets);

	if (card->of_dapm_widgets)
		snd_soc_dapm_new_controls(&card->dapm, card->of_dapm_widgets,
					  card->num_of_dapm_widgets);

	...

	snd_soc_dapm_link_dai_widgets(card);
	snd_soc_dapm_connect_dai_link_widgets(card);

	if (card->controls)
		snd_soc_add_card_controls(card, card->controls,
					  card->num_controls);

	if (card->dapm_routes)
		snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
					card->num_dapm_routes);

	if (card->of_dapm_routes)
		snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
					card->num_of_dapm_routes);
	...

	snd_soc_dapm_new_widgets(card);

	...

	dapm_mark_endpoints_dirty(card);
	snd_soc_dapm_sync(&card->dapm);
	...
}




Thank you for your help !!
Best regards
---
Kuninori Morimoto

Re: Question about dapm setup

[Charles Keepax]

On Wed, May 29, 2019 at 01:50:07PM +0900, Kuninori Morimoto wrote:
> 
> Hi ALSA ML
> 
> snd_soc_instantiate_card() setups dapm, but its timing seems
> very randomly for me.
> In my understanding, dapm setup timing is not so serious.
> So, I think we can do it in one place,
> but are there some reasons ?
> For example, "xxxx should be called after yyyy"
> 

There are certainly reasons for some of it, but might be easier
to explain what you are thinking of moving, rather than trying to
list all dependencies.

> 
> static int snd_soc_instantiate_card(struct snd_soc_card *card)
> {
> 	...
> 
> =>	snd_soc_dapm_debugfs_init(&card->dapm, card->debugfs_card_root);
> 
> 	...
> 
> 	if (card->dapm_widgets)
> 		snd_soc_dapm_new_controls(&card->dapm, card->dapm_widgets,
> 					  card->num_dapm_widgets);
> 
> 	if (card->of_dapm_widgets)
> 		snd_soc_dapm_new_controls(&card->dapm, card->of_dapm_widgets,
> 					  card->num_of_dapm_widgets);
> 
> 	...
> 
> 	snd_soc_dapm_link_dai_widgets(card);
> 	snd_soc_dapm_connect_dai_link_widgets(card);

For example we need the calls to snd_soc_dapm_new_controls to
be before these two so that the widgets exist for linking them.

> 
> 	if (card->controls)
> 		snd_soc_add_card_controls(card, card->controls,
> 					  card->num_controls);
> 

This needs to be before the routes are added so that the routes
can find their associated controls.

> 	if (card->dapm_routes)
> 		snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
> 					card->num_dapm_routes);
> 
> 	if (card->of_dapm_routes)
> 		snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
> 					card->num_of_dapm_routes);
> 	...

And the routes also obviously need to be after the widgets are
added as well.

> 
> 	snd_soc_dapm_new_widgets(card);
> 
> 	...
> 
> 	dapm_mark_endpoints_dirty(card);
> 	snd_soc_dapm_sync(&card->dapm);
> 	...
> }
> 
> 

Hope that is roughly the sort of thing you were interested in.

Thanks,
Charles

Re: Question about dapm setup

[Kuninori Morimoto]

Hi Charles

> > snd_soc_instantiate_card() setups dapm, but its timing seems
> > very randomly for me.
> > In my understanding, dapm setup timing is not so serious.
> > So, I think we can do it in one place,
> > but are there some reasons ?
> > For example, "xxxx should be called after yyyy"
> > 
> 
> There are certainly reasons for some of it, but might be easier
> to explain what you are thinking of moving, rather than trying to
> list all dependencies.
(snip)
> For example we need the calls to snd_soc_dapm_new_controls to
> be before these two so that the widgets exist for linking them.
(snip)
> This needs to be before the routes are added so that the routes
> can find their associated controls.
(snip)
> And the routes also obviously need to be after the widgets are
> added as well.
(snip)
> Hope that is roughly the sort of thing you were interested in.

Thanks !! Nice to know !!
It is very clear for dapm setup timing.

But, it was my fault, the question was not clear.
I wanted to know was that there are many non dapm functions
are called between dapm setup.
pseudo code is..

	static int snd_soc_instantiate_card(struct snd_soc_card *card)
	{
		...

=>		snd_soc_dapm_debugfs_init()
=>		snd_soc_dapm_new_controls(...)

		card->probe(..)
		soc_probe_link_components(...)
		soc_probe_aux_device(...)
		soc_bind_dai_link(...)
		soc_probe_link_dais

=>		snd_soc_dapm_link_dai_widgets()
=>		snd_soc_dapm_connect_dai_link_widgets()
=>		snd_soc_add_card_controls()
=>		snd_soc_dapm_add_routes()

		snprintf(...)
		card->late_probe()

=>		snd_soc_dapm_new_widgets()
	
		snd_card_register()

=>		dapm_mark_endpoints_dirty()
=>		snd_soc_dapm_sync()
		...
	}

It looks very random. So my original question was
can we do like this (with keeping dapm order)?

	static int snd_soc_instantiate_card(struct snd_soc_card *card)
	{

=>		snd_soc_dapm_debugfs_init()
=>		snd_soc_dapm_new_controls(...)
=>		snd_soc_dapm_link_dai_widgets()
=>		snd_soc_dapm_connect_dai_link_widgets()
=>		snd_soc_add_card_controls()
=>		snd_soc_dapm_add_routes()
=>		snd_soc_dapm_new_widgets()
=>		dapm_mark_endpoints_dirty()
=>		snd_soc_dapm_sync()

		card->probe(..)
		soc_probe_link_components(...)
		soc_probe_aux_device(...)
		soc_bind_dai_link(...)
		soc_probe_link_dais
		snprintf(...)
		card->late_probe()
		snd_card_register()

=>		/* or dapm setup here instead ? */
	}

For example, snd_soc_dapm_xxx() should be called before/after
card->probe() etc, etc...


Thank you for your help !!
Best regards
---
Kuninori Morimoto

Re: Question about dapm setup

[Charles Keepax]

On Thu, May 30, 2019 at 09:12:10AM +0900, Kuninori Morimoto wrote:
> But, it was my fault, the question was not clear.
> I wanted to know was that there are many non dapm functions
> are called between dapm setup.
> pseudo code is..
> 
> It looks very random. So my original question was
> can we do like this (with keeping dapm order)?
> 

Ah ok I see, sorry I understand now.

> 	static int snd_soc_instantiate_card(struct snd_soc_card *card)
> 	{
> 
> =>		snd_soc_dapm_debugfs_init()
> =>		snd_soc_dapm_new_controls(...)
> =>		snd_soc_dapm_link_dai_widgets()
> =>		snd_soc_dapm_connect_dai_link_widgets()
> =>		snd_soc_add_card_controls()
> =>		snd_soc_dapm_add_routes()
> =>		snd_soc_dapm_new_widgets()
> =>		dapm_mark_endpoints_dirty()
> =>		snd_soc_dapm_sync()
> 
> 		card->probe(..)
> 		soc_probe_link_components(...)
> 		soc_probe_aux_device(...)
> 		soc_bind_dai_link(...)
> 		soc_probe_link_dais
> 		snprintf(...)
> 		card->late_probe()
> 		snd_card_register()
> 
> =>		/* or dapm setup here instead ? */
> 	}
> 
> For example, snd_soc_dapm_xxx() should be called before/after
> card->probe() etc, etc...
> 

There are definitely some dependencies for example component probes
will add widgets, controls and routes from those components so
those will need to be done before the card level routes are
added. The card level routes may link to widgets on individual
components.

Also the DAPM sync definitely needs to be after everything has
been setup.

I wouldn't be surprised if there are others as well, things like
creating the DAI link widgets are probably done through some
of these helpers and probably need to be at certain points in the
process.

Thanks,
Charles

Re: Question about dapm setup

[Kuninori Morimoto]

Hi Charles

Thank you for your help

> > 	static int snd_soc_instantiate_card(struct snd_soc_card *card)
> > 	{
> > 
> > =>		snd_soc_dapm_debugfs_init()
> > =>		snd_soc_dapm_new_controls(...)
> > =>		snd_soc_dapm_link_dai_widgets()
> > =>		snd_soc_dapm_connect_dai_link_widgets()
> > =>		snd_soc_add_card_controls()
> > =>		snd_soc_dapm_add_routes()
> > =>		snd_soc_dapm_new_widgets()
> > =>		dapm_mark_endpoints_dirty()
> > =>		snd_soc_dapm_sync()
> > 
> > 		card->probe(..)
> > 		soc_probe_link_components(...)
> > 		soc_probe_aux_device(...)
> > 		soc_bind_dai_link(...)
> > 		soc_probe_link_dais
> > 		snprintf(...)
> > 		card->late_probe()
> > 		snd_card_register()
> > 
> > =>		/* or dapm setup here instead ? */
> > 	}
(snip)
> There are definitely some dependencies for example component probes
> will add widgets, controls and routes from those components so
> those will need to be done before the card level routes are
> added. The card level routes may link to widgets on individual
> components.
> 
> Also the DAPM sync definitely needs to be after everything has
> been setup.
> 
> I wouldn't be surprised if there are others as well, things like
> creating the DAI link widgets are probably done through some
> of these helpers and probably need to be at certain points in the
> process.

Thank for your information !
I think we can do some cleanup there, but need more investigation.
I guess, the point is card->probe, and card->late_probe (?)

Thank you for your help !!
Best regards
---
Kuninori Morimoto