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

[Jaroslav Kysela]

Dne 09. 04. 21 v 11:09 Amadeusz Sławiński napsal(a):
> 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.

If the driver exports some extra information via sysfs, it can be already used
in the current UCM configs. I welcome any of this activity. It may ease
things. But it's a complement to my proposal to allow modify the sound card
identification in udev (not only for UCM). I believe that we can support
multiple ways and it's up to the maintainer / user of the specific distro to
select any.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.