| Message ID | 20230905093147.1960675-1-amadeuszx.slawinski@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 739c031110da9ba966b0189fa25a2a1c0d42263c |
| Headers | show |
| Series | ASoC: Intel: avs: Provide support for fallback topology | expand |
On 9/5/23 05:31, Amadeusz Sławiński wrote: > HDA and HDMI devices are simple enough that in case of user not having > topology tailored to their device, they can use fallback topology. > > Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com> > --- > sound/soc/intel/avs/pcm.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c > index 1fbb2c2fadb5..8565a530706d 100644 > --- a/sound/soc/intel/avs/pcm.c > +++ b/sound/soc/intel/avs/pcm.c > @@ -796,6 +796,28 @@ static int avs_component_probe(struct snd_soc_component *component) > > ret = avs_load_topology(component, filename); > kfree(filename); > + if (ret == -ENOENT && !strncmp(mach->tplg_filename, "hda-", 4)) { > + unsigned int vendor_id; > + > + if (sscanf(mach->tplg_filename, "hda-%08x-tplg.bin", &vendor_id) != 1) > + return ret; > + > + if (((vendor_id >> 16) & 0xFFFF) == 0x8086) > + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, > + "hda-8086-generic-tplg.bin"); it's very odd to test for 0x8086 in a driver that only supports Intel devices, isn't it? One of these two branches is always-true or there's a missing explanation on what this 0x8086 is used for? > + else > + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, > + "hda-generic-tplg.bin"); > + > + filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix, > + mach->tplg_filename); > + if (!filename) > + return -ENOMEM; > + > + dev_info(card->dev, "trying to load fallback topology %s\n", mach->tplg_filename); > + ret = avs_load_topology(component, filename); > + kfree(filename); > + } > if (ret < 0) > return ret; >
On 9/5/2023 2:42 PM, Pierre-Louis Bossart wrote: > > > On 9/5/23 05:31, Amadeusz Sławiński wrote: >> HDA and HDMI devices are simple enough that in case of user not having >> topology tailored to their device, they can use fallback topology. >> >> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com> >> --- >> sound/soc/intel/avs/pcm.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c >> index 1fbb2c2fadb5..8565a530706d 100644 >> --- a/sound/soc/intel/avs/pcm.c >> +++ b/sound/soc/intel/avs/pcm.c >> @@ -796,6 +796,28 @@ static int avs_component_probe(struct snd_soc_component *component) >> >> ret = avs_load_topology(component, filename); >> kfree(filename); >> + if (ret == -ENOENT && !strncmp(mach->tplg_filename, "hda-", 4)) { >> + unsigned int vendor_id; >> + >> + if (sscanf(mach->tplg_filename, "hda-%08x-tplg.bin", &vendor_id) != 1) >> + return ret; >> + >> + if (((vendor_id >> 16) & 0xFFFF) == 0x8086) >> + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, >> + "hda-8086-generic-tplg.bin"); > > it's very odd to test for 0x8086 in a driver that only supports Intel > devices, isn't it? > > One of these two branches is always-true or there's a missing > explanation on what this 0x8086 is used for? > Differentiating between generic codecs (https://github.com/thesofproject/avs-topology-xml/tree/main/hda) and hdmi ones (https://github.com/thesofproject/avs-topology-xml/tree/main/hdmi), as topology targets codec.
On 9/5/23 09:58, Amadeusz Sławiński wrote: > On 9/5/2023 2:42 PM, Pierre-Louis Bossart wrote: >> >> >> On 9/5/23 05:31, Amadeusz Sławiński wrote: >>> HDA and HDMI devices are simple enough that in case of user not having >>> topology tailored to their device, they can use fallback topology. >>> >>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com> >>> --- >>> sound/soc/intel/avs/pcm.c | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c >>> index 1fbb2c2fadb5..8565a530706d 100644 >>> --- a/sound/soc/intel/avs/pcm.c >>> +++ b/sound/soc/intel/avs/pcm.c >>> @@ -796,6 +796,28 @@ static int avs_component_probe(struct >>> snd_soc_component *component) >>> ret = avs_load_topology(component, filename); >>> kfree(filename); >>> + if (ret == -ENOENT && !strncmp(mach->tplg_filename, "hda-", 4)) { >>> + unsigned int vendor_id; >>> + >>> + if (sscanf(mach->tplg_filename, "hda-%08x-tplg.bin", >>> &vendor_id) != 1) >>> + return ret; >>> + >>> + if (((vendor_id >> 16) & 0xFFFF) == 0x8086) >>> + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, >>> + "hda-8086-generic-tplg.bin"); >> >> it's very odd to test for 0x8086 in a driver that only supports Intel >> devices, isn't it? >> >> One of these two branches is always-true or there's a missing >> explanation on what this 0x8086 is used for? >> > > Differentiating between generic codecs > (https://github.com/thesofproject/avs-topology-xml/tree/main/hda) and > hdmi ones > (https://github.com/thesofproject/avs-topology-xml/tree/main/hdmi), as > topology targets codec. Ah yes, 0x8086 for the codec vendor. I must admit I didn't click after a 4-day week-end... BTW your list of topologies helps with my assertion that we are missing a 'hardware layer' in the topology framework, it makes no sense to have a proliferation of topology files that all look the same. We really need the ability to tell which endpoints are active or not, and which hardware interface to use on a given platform. copy-pasting and using macros is going to lead us into a maintenance nightmare.
On Tue, 05 Sep 2023 11:31:47 +0200, Amadeusz Sławiński wrote: > HDA and HDMI devices are simple enough that in case of user not having > topology tailored to their device, they can use fallback topology. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: Intel: avs: Provide support for fallback topology commit: 739c031110da9ba966b0189fa25a2a1c0d42263c All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 1fbb2c2fadb5..8565a530706d 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -796,6 +796,28 @@ static int avs_component_probe(struct snd_soc_component *component) ret = avs_load_topology(component, filename); kfree(filename); + if (ret == -ENOENT && !strncmp(mach->tplg_filename, "hda-", 4)) { + unsigned int vendor_id; + + if (sscanf(mach->tplg_filename, "hda-%08x-tplg.bin", &vendor_id) != 1) + return ret; + + if (((vendor_id >> 16) & 0xFFFF) == 0x8086) + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, + "hda-8086-generic-tplg.bin"); + else + mach->tplg_filename = devm_kasprintf(adev->dev, GFP_KERNEL, + "hda-generic-tplg.bin"); + + filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix, + mach->tplg_filename); + if (!filename) + return -ENOMEM; + + dev_info(card->dev, "trying to load fallback topology %s\n", mach->tplg_filename); + ret = avs_load_topology(component, filename); + kfree(filename); + } if (ret < 0) return ret;
HDA and HDMI devices are simple enough that in case of user not having topology tailored to their device, they can use fallback topology. Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com> --- sound/soc/intel/avs/pcm.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)