Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai <tiwai@suse.de>]

On Tue, 17 Nov 2020 23:13:13 +0100,
Rojewski, Cezary wrote:
> 
> On 2020-11-17 3:04 PM, Takashi Iwai wrote:
> > On Mon, 16 Nov 2020 18:47:22 +0100,
> > Pierre-Louis Bossart wrote:
> >>
> >>> Explicit 'ifs' asking whether we're dealing with SOF or other solution
> >>> is at best a code smell. I believe this is unnecessary complexity added
> >>> to the code especially once you realize user needs to play with module
> >>> parameters to switch between solutions. If we assume user is able to
> >>> play with module parameters then why not simply make use of blacklist
> >>> mechanism?
> >>
> >> Been there, done that. We don't want to use either denylist of kernel
> >> parameters.
> >>
> >> denylists create confusion for users, it's an endless stream of false
> >> errors and time lost in bug reports.
> >>
> >> The use of the kernel parameter is ONLY for expert users who want to
> >> tinker with the system or debug issues, the average user should not
> >> have to play with either denylists or kernel parameters.
> > 
> > I guess Cezary mean the modprobe blacklist?  This was used in the
> > early stage of ASoC Skylake driver development, but in the end, it's
> > more cumbersome because user needs to change multiple places.  The
> > single module parameter was easier to handle.
> > 
> 
> Thanks for joining the discussion, Takashi.
> 
> If the switch of solution for atom-based products is imminent, why add
> code which becomes redundant soon after?
> 
> Yes, indeed I meant the modprobe blacklisting as it solves the problem
> without addition of any code. Doubt alsa-driver entries are scattered in
> /etc/modprobe.d/ so switching between one solution to another via
> blacklist becomes as easy as changing 'options intel-dsp-config
> <param>==<value>' entry.

Ideally blacklist would work well, but practically it can be more
problematic.  When you *switch* between multiple drivers via
blacklist, you'll have to mask one of them while keeping another
untouched, so either:
  blacklist A
or
  blacklist B

Now, imagine that distro sets "blacklist A" to choose B as the
default.  What user has to do?  They have to modify "blacklist A"
line with "blacklist B".  But it can't be done with an additional
modprobe.d/*.config file; otherwise this blacklist remains.  It means
they have to scratch the system configuration file itself -- which
might be again overridden by a package update or whatever.

This will be more complex if there are more than three choices, of
course.

Admittedly, the situation with the system config file be same for
module option if distro sets the option in modprobe.d/*, too.  But,
there is another difference: the default option value can be set in
the kernel code, while the blacklist approach is to let all open and
choose via blacklist.  IOW, devs have some control for choosing the
default value for the module option but for blacklist they are all
done by user-space side.


> In regard to catpt, solution is even simpler: just remove
> sound/soc/sof/intel/bdw.c as that code is not valid & recommended
> anyway and linux kernel is not place for such. There shouldn't be really
> any options for not recommended stuff. Leave the selection explicit.
> 
> >>> And last but not least: intel-dsp-config is (as already stated) a mean
> >>> for solving the HDA-runtime-driver-selection problem. Mixing it with
> >>> ACPI devices is another layer of confusion.
> >>
> >> Why? Who said it was PCI only? We already take care of DMIC,
> >> SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
> >> one API to be used when more than one driver can register to the same
> >> device.
> > 
> > Well, currently intel-dsp-config sits in sound/hda, which isn't really
> > intuitive.  Though, Intel driver file paths are already fairly
> > scattered, so it doesn't matter too much :)
> > 
> > I don't mind to move it to another directory, but which one...?
> > x86 might match, but shuffling the place won't help for maintenance.
> > 
> > I personally find this move good, at least it makes things easier for
> > distros.  There are small details like the above, but technically
> > seen, I see no big problem.
> 
> Well, if non-Intel guys see the localization of code counter-intuitive
> then how about those who play with it daily..

I play it and maintain it daily, that's why I find unintuitive :)
I guess most users don't notice the file path, as the module loading
or option is done only by the module name.

> The new "sof-parent" checks won't make maintaining any easier and I
> believe there are easier solutions as written above.

If you find a good way to overcome the disadvantage, that's great.
Let's see.


thanks,

Takashi