[alsa-devel] [PATCH] ASoC: core: Add support for platform and CODEC drivers on same device
Currently DAI playback and capture widgets are created during soc_probe_codec and soc_probe_platform, using the device associated with the DAI to check which widgets should be created. If a device registers both a CODEC and platform driver this leads the CODEC playback and capture widgets being overwritten by the widgets created by the platform probe.
It is more sensible to retain the CODEC widgets as the most common use case for registering both a CODEC and platform driver on the same chip is a CODEC which contains a DSP for compressed playback. In this situation it is more sensible to attach the routing information to the CODEC and add a thin platform driver interface to link into the compressed API.
So this patch will check for existing widgets during soc_probe_platform and only create new widgets if no existing ones exist.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/soc-core.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 04af2a6..53efe1d 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1159,7 +1159,8 @@ static int soc_probe_platform(struct snd_soc_card *card,
/* Create DAPM widgets for each DAI stream */ list_for_each_entry(dai, &dai_list, list) { - if (dai->dev != platform->dev) + if (dai->dev != platform->dev || + dai->playback_widget || dai->capture_widget) continue;
snd_soc_dapm_new_dai_widgets(&platform->dapm, dai);
On Thu, Jan 24, 2013 at 09:49:11AM +0000, Charles Keepax wrote:
So this patch will check for existing widgets during soc_probe_platform and only create new widgets if no existing ones exist.
Why not do these extra checks in new_dai_widgets() itself? That'd be cover more cases and generally seems more robust.
On Thu, Jan 24, 2013 at 09:49:11AM +0000, Charles Keepax wrote:
So this patch will check for existing widgets during soc_probe_platform and only create new widgets if no existing ones exist.
...or as I was sending that it occurred to me that it'd be even neater to share the DAPM context, though that's much more refactoring.
On Sat, Jan 26, 2013 at 05:20:21PM +0800, Mark Brown wrote:
On Thu, Jan 24, 2013 at 09:49:11AM +0000, Charles Keepax wrote:
So this patch will check for existing widgets during soc_probe_platform and only create new widgets if no existing ones exist.
...or as I was sending that it occurred to me that it'd be even neater to share the DAPM context, though that's much more refactoring.
Looking at this in more detail sharing the DAPM context doesn't fix the issue. The problem is related to overwriting the widgets on the DAI which means any routes added to the old widgets are no longer considered when DAPM processes the DAI. So I will sent in a patch which does the check in snd_soc_dapm_new_dai_widgets as that is indeed a more robust and general solution.
However, that said there is some argument for sharing the context anyway as there is no need for the one device to have two contexts associated with it, however I am not sure it is worth it. The most sensible way I can see to do so is to replace the dapm structs in snd_soc_platform and snd_soc_codec with pointers and dynamically allocate the dapm contexts. However this is a fair amount of dynamic allocation and leaves a lot of user code that needs updating (albiet trivially), all just to avoid a pointlessly duplicated context that does no harm. I do have some patches moving down this direction whilst I was investigating it so let me know if this is something we might be interested in seeing and I will look to find some time to get them into a state they could be pushed out for comments.
Charles
Currently DAI playback and capture widgets are created during soc_probe_codec and soc_probe_platform using the device associated with the platform/codec to locate the DAI upon which the widgets should be created. If a device registers both a CODEC and platform driver this leads the widgets created during CODEC probe being overwritten by the widgets created during the platform probe.
In general we want to retain the existing playback and capture widgets because routes may have already been added to them. This patch will check for existing widgets during snd_soc_dapm_new_dai_widgets and only create new widgets if none exist.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/soc-dapm.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 33acd8b..6959304 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3351,7 +3351,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, memset(&template, 0, sizeof(template)); template.reg = SND_SOC_NOPM;
- if (dai->driver->playback.stream_name) { + if (dai->driver->playback.stream_name && !dai->playback_widget) { template.id = snd_soc_dapm_dai; template.name = dai->driver->playback.stream_name; template.sname = dai->driver->playback.stream_name; @@ -3369,7 +3369,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, dai->playback_widget = w; }
- if (dai->driver->capture.stream_name) { + if (dai->driver->capture.stream_name && !dai->capture_widget) { template.id = snd_soc_dapm_dai; template.name = dai->driver->capture.stream_name; template.sname = dai->driver->capture.stream_name;
On Fri, Mar 15, 2013 at 03:38:21PM +0000, Charles Keepax wrote:
Currently DAI playback and capture widgets are created during soc_probe_codec and soc_probe_platform using the device associated
Don't bury patches in the middle of threads, especially if it's several months after the fact - it just makes it more likely that the patch will be missed or ignored.
Anyway now I remember the full context...
with the platform/codec to locate the DAI upon which the widgets should be created. If a device registers both a CODEC and platform driver this leads the widgets created during CODEC probe being overwritten by the widgets created during the platform probe.
In general we want to retain the existing playback and capture widgets because routes may have already been added to them. This patch will check for existing widgets during snd_soc_dapm_new_dai_widgets and only create new widgets if none exist.
This still seems like it's bodging things in the wrong place. The issue is that we're trying to instantiate the DAI multiple times because we've not got a firm idea of the chip as a whole. Adding checks like this to the code is just going to make everything more and more complicated to understand over time without addressing the source of complexity.
This is especially true now that Morimoto-san has added component objects which exist to provide that sort of grouping, we should be addressing things like this by pulling them up to the component level so that we know we're only doing them once per chip. I think the first step here is to add the ability to create DAIs based on components, then to let components sit where CODECs sit in terms of setting up DAIs so we can start moving CODEC drivers over into components.
On Fri, Mar 15, 2013 at 03:35:40PM +0000, Charles Keepax wrote:
On Sat, Jan 26, 2013 at 05:20:21PM +0800, Mark Brown wrote:
On Thu, Jan 24, 2013 at 09:49:11AM +0000, Charles Keepax wrote:
So this patch will check for existing widgets during soc_probe_platform and only create new widgets if no existing ones exist.
...or as I was sending that it occurred to me that it'd be even neater to share the DAPM context, though that's much more refactoring.
Looking at this in more detail sharing the DAPM context doesn't fix the issue. The problem is related to overwriting the widgets on the DAI which means any routes added to the old widgets are no longer considered when DAPM processes the DAI. So I will sent
I don't think you've fully understood my suggestion, or what's involved in much more refactoring. I can't fully recall the context since it's been several months since the discussion came up but I suspect the thing here is that the widgets are mostly a function of the DAPM context so if we share the context we share the widgets and don't end up creating multiple copies.
However, that said there is some argument for sharing the context anyway as there is no need for the one device to have two contexts associated with it, however I am not sure it is worth it. The most sensible way I can see to do so is to replace the dapm structs in snd_soc_platform and snd_soc_codec with pointers and dynamically allocate the dapm contexts. However this is a
Why would we need to dynamically allocate anything here? The DAIs should all be owned by something (the CODEC or, now, the component).
participants (2)
-
Charles Keepax -
Mark Brown