On 2020-11-13 6:06 PM, Hans de Goede wrote:
> On 11/13/20 5:49 PM, Mark Brown wrote:
>> On Fri, Nov 13, 2020 at 01:06:48PM +0000, Rojewski, Cezary wrote:
>>
>>> For a very long time upstream was filled with "flavors" of drivers for
>>> Intel solutions. Having three available for BYT is a very good example
>>> of that. The division of what goes where wasn't exactly explicit either.
>>> This all leads to confusion - while community and users may feel
>>> confused about what's recommended and what they should actually be
>>> using, surprisingly (unsurprisingly?) developers were too.
>>
>> ...
>>
>>> Patchset presented here goes directly against that goal. We, Intel
>>> developers, are tasked with providing reliable, working solutions
>>> exposing best possible experience for our customers when dealing with
>>> our products. And thus solutions provided are called recommended. We
>>> don't deal with flavors and try-it-out-on-your-own-risk.
>>
>> My feeling here was that this is helping with this goal in that it's not
>> changing the defaults but is rather pushing the decision making process
>> from build time to runtime. This means that distributions are able to
>> ship the preferred implementations for all the platforms without causing
>> any issues for the hopefully small set of users who need or want to work
>> on a different firmware, if they've been doing something like sticking
>> with an alternative firmware for old users since things were working
>> they'll be able to smoothly transition over to the current recommended
>> default, eg leaving old users on the old firmware by default. That's a
>> bit of a niche use case but then hopefully all use cases for selecting a
>> non-default firmware are niche.
>>
>> It also means that people don't have to think about this so much at
>> build time, they can just turn everything on and not worry they'll cause
>> problems for people using the binary and still get the recommended
>> runtime behaviour by default unless the user actively does something
Thanks for your input, Mark.
The ultimate goal is OK but the execution is not. Take a look at the
following:
+static inline bool snd_soc_acpi_sof_parent(struct device *dev)
+{
+ return dev->parent && dev->parent->driver && dev->parent->driver->name &&
+ !strcmp(dev->parent->driver->name, "sof-audio-acpi");
+}
+
-and-
+ /* set pm ops */
+ if (sof_parent)
+ pdev->dev.driver->pm = &snd_soc_pm_ops;
+
-and-
+ /* set card and driver name */
+ if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+ bdw_rt5650_card.name = SOF_CARD_NAME;
+ bdw_rt5650_card.driver_name = SOF_DRIVER_NAME;
+ } else {
+ bdw_rt5650_card.name = CARD_NAME;
+ bdw_rt5650_card.driver_name = DRIVER_NAME;
+ }
+
pieces that are appearing several times throughout the series.
ASoC is a framework on its own. It is by all means an extension to an
older, more general ALSA framework. With every passing month SOF code
found in /sound/soc/sof gets closer to becoming yet another framework,
one that is placed on top of ASoC. Samples taken from this series
augment this statement. If ASoC framework is missing means for its child
drivers to do specific things, it's better to update the framework than
creating yet another one.
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?
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.
>
> Exactly. As Pierre-Louis mentions the Intel team does not have most
> of the affected hardware. Since I've been working on making BYT and CHT
> based devices work better with Linux as a side project for the last
> couple of years I have been buying these kinda devices 2nd hand when ever
...
> As Pierre-Louis mentioned I've already been working with him on getting
> ready to move everything BYT/CHT related over to SOF. I've already been
> testing SOF on various devices with mostly ok results so far.
> But this is a process not a switch which we can simply throw.
Hans, thanks for sharing your concerns.
I'm afraid it's basically impossible to be fully prepared as you and
Pierre pointed out. Even when speaking about catpt and BDW, we too
didn't have all the available production stuff.
And thus I don't believe there will ever be a "good moment" to switch.
Once internal validation confirms driver is stable it's better to switch
entirely to the new solution with CI and devs on standby - ready to
address any upcoming reports. Don't believe /atom/ has clean slide
anyway given the patches and issues being posted from time to time
related to said solution.
I understand you're here for atom-based products yet the patchset also
touches on catpt aka hsw/bdw. While to my knowledge old /atom/ has no
active CI running - so the switch is desirable - the same cannot be said
about catpt. Because of that, I don't see any reason for appending any
catpt-related changes here. Leave no place for confusion. One solution
for one architecture that's recommended and maintained.
Czarek
