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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





+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;
+

The legacy Baytrail/Cherrytrail driver uses its own power management flow instead of the ASoC one. This patch does not cause the problem, it recognizes instead that this legacy driver cannot use the same pm ops.

I wish we didn't have to do this but there's just no alternative.

Dynamically assigning the .pm ops is not illegal and has been done in other drivers.

-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;
+	}
+

That is also intentional. We modified the card names based on Jaroslav's feedback, and this code change is just the mirror of what happened before with build-time code changes. Should we have changed the legacy card names? Maybe, but that might have added issues for users so we left the names untouched.

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.

There are no ASoC devices or drivers, we use platform devices/drivers. I don't see any need for an ASoC extension here.

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.

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.

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.

You refer to tests made by Intel when we are talking about community based tests here. We precisely do not have 'CI and devs on standby', the transition will be made by distributions themselves.

Besides, CI cannot test jack detection and all the flavors of microphone/speaker placements, which are the source of 99% of the issues.

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.

There is no confusion, the wording is explicit

"
SOF does not fully support Broadwell and has limitations related to
+	  DMA and suspend-resume, this is not a recommended option for
+	  distributions.
"

But there are niche users who prefer it for their own experiments, so what's the issue in making their life simpler?



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux