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

["Rojewski, Cezary" <cezary.rojewski@intel.com>]

On 2020-11-18 8:49 AM, Takashi Iwai wrote:
> On Tue, 17 Nov 2020 23:13:13 +0100, Rojewski, Cezary wrote:

...

>>
>> 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.
> 

I agree, module param is ultimately easier to handle than denylist. The
reason I had mentioned that is: if user is capable of changing value for
module param, then we might as well assume he or she is the experienced
one and playing with denylist isn't a problem either.

And hopefully we don't reach a point in time where again 3 flavours for
atom-based products are available : )

>> 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.
>>

...

>> 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.
> 

Perhaps I misworded my previous statement. What I meant is: you don't
have access to all the stuff we - Intel employees - have like specs,
firmware documentation, hardware specifics and yet you see the problem.
And this tells me there's still a lot to be done.

>> 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.

Well, the disadvantage is: weight of maintenance of newly added code.
All in all, as mentioned in other reply, we could settle with selection
mechanism for atom while leaving hsw/bdw out of it.

Czarek