[PATCH] ALSA: core - add more card sysfs entries

[Amadeusz Sławiński]

On 4/9/2021 10:34 AM, Jaroslav Kysela wrote:
> Dne 09. 04. 21 v 9:39 Takashi Iwai napsal(a):
>> On Thu, 08 Apr 2021 20:51:41 +0200,
>> Pierre-Louis Bossart wrote:
>>>
>>>
>>>
>>>>>> When we have a common standard layer for the plug-and-play handling (udev), we
>>>>>> should concentrate to allow changing / refining of this information there.
>>>>>> Those strings are not used for anything else than the user space. So from my
>>>>>> view, there's no reason to create another mechanism to handle the overrides.
>>>>>> It should be a safe, fast, flexible and_optional_  solution. The udev can
>>>>>> alter the sysfs attributes directly without any hassle with the file
>>>>>> modifications or looking for another way to pass / store this information
>>>>>> somewhere.
>>>>>
>>>>> There's one part where I am lost.
>>>>>
>>>>> The initial idea of udev what to modify kernel parameters to pick a
>>>>> different path for firmware/topology before probing the PCI driver. At
>>>>
>>>> This may be a problematic point. The kernel cmdline cannot be modified from
>>>> udev (as far as I know). The module parameters can be set using modprobe's
>>>> config files or when loaded with sysfs attributes (/sys/module/*/parameters).
>>>> Eventually, you can call the modprobe command with custom module parameters
>>>> when the appropriate MODALIAS is probed.
>>>>
>>>> Perhaps, I'm missing something here, too. Some example udev rules may help.
>>>
>>> see the example shared by Curtis
>>>
>>> SUBSYSTEM=="pci", ATTR{vendor}=="0x8086", ATTR{device}=="0xa0c8",
>>> ATTR{class}=="0x040100", ATTRS{[dmi/id]board_name}=="Eldrid",
>>> RUN+="/sbin/modprobe snd_sof_pci tplg_path=intel/sof-tplg/pdm1"
>>>
>>> Those 'path' parameters would have to be set prior to creating the
>>> card, making them writable via sysfs would not work, the firmware and
>>> topology are already loaded and changing the paths would have no
>>> effect.
>>
>> Couldn't the driver probe the firmware files with some device-specific
>> string suffix at first?  e.g. the driver can issue request_firmware()
>> with $base_file-$dmi_board at first, then falls back to the generic
>> $base_file.  A similar method was already used in Broadcom WiFi
>> driver.
>>
>> Also, the driver may do request_firmware() with a fixed path for the
>> custom firmware at first (e.g. "intel/sof-tplg-custom"); then a system
>> integrator may set up a specific configuration even that doesn't match
>> with DMI or whatever identifier.
> 
> And when we have two firmware files which differs just by functionality
> requested by user? Although your method will work, I won't close the
> possibility to configure everything in udev rather using a hard coded fw load
> scheme only.
> 
> 						Jaroslav
> 

I've slept on it and now I think I see what you are trying to do.

1. Load FW dependent on platform/user settings
2. Load appropriate topology for FW
3. Have UCM for the FEs and controls exposed by driver


As for 1. I would say that FW should be loaded from one location
if there is some platform that requires special FW just add quirks, like 
it is done with every other driver, and if someone wants to build their 
own special FW, they just replace it. We can't possibly support hundreds 
of possible FW modifications if users want them by putting them in 
separate files. Alternatively allow override via kernel parameters.
Overriding FW files via udev would only make sense to me if it was 
possible to upload new FW at runtime.

I would say that same applies for 2.

This leaves number 3. which would require kernel exposing some kind of 
information about loaded topology, so user space can use proper UCM.
In topology manifest there are few reserved fields 
(https://elixir.bootlin.com/linux/latest/source/include/uapi/sound/asoc.h#L382), 
so we can add some information there which should be unique per topology 
and then expose it in userspace on topology load, it can be the name of 
UCM file topology wants to be loaded for example.

For example do something along those lines:

struct snd_soc_tplg_manifest {
	__le32 size;		/* in bytes of this structure */
	__le32 control_elems;	/* number of control elements */
	__le32 widget_elems;	/* number of widget elements */
	__le32 graph_elems;	/* number of graph elements */
	__le32 pcm_elems;	/* number of PCM elements */
	__le32 dai_link_elems;	/* number of DAI link elements */
	__le32 dai_elems;	/* number of physical DAI elements */
	__le32 ucm_files;	/* UCM files to use with topology */
	__le32 reserved[19];	/* reserved for new ABI element types */
	struct snd_soc_tplg_private priv;
} __attribute__((packed));

struct snd_soc_tplg_ucm_files {
	struct snd_soc_tplg_ctl_hdr hdr;
	__le32 size;	/* size of struct in bytes */
	__le32 count;	/* UCM entries */
	char ucms[SNDRV_CTL_ELEM_ID_NAME_MAXLEN][];
}

And then expose it somewhere under sysfs after parsing topology.