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@xxxxxxxx> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.