On 2022-05-06 4:56 PM, Pierre-Louis Bossart wrote:
On 5/6/22 08:39, Cezary Rojewski wrote:
...
Sorry for the late response, did not realize there is an unanswered
comment here.
So, the rough list goes as:
- hdac_hda.c hardcodes codec capabilities rather than aligning with what
sound/pci/hda/ code does
- merges HDMI (i.e. Intel i915 audio component) and HDA DAIs together
whereas these are two separate devices
- because of above, implements custom search/matching mechanism for PCM/DAI
- cont. because of above, its header hosts private data struct,
unnecessary complication
- follows HDA_DEV_ASOC convention rather than HDA_DEV_LEGACY causing
misalignments between sound/pci/hda and sound/soc/ behaviour
- has basic PM runtime support and does not survive scenarios where
resume/suspend + denylist + rmmod/modprobe are mixed together or invoked
in unordered fashion between this module and several others in the audio
stack
My suggestion is different: have all HD-Audio ASoC users switch to this
implementation when possible and remove the existing code along with
skylake-driver.
I am not against change and will agree that HDaudio support is far from
perfect, but it's been released for multiple generations from dozens of
OEMs and mostly works. All the issues reported to us are related to
codec configurations that also don't work with the legacy HDaudio
driver, DMIC configurations, CSME authentication or system hangs that
have not been root-caused [1]. HDaudio/ASoC interfaces are not on our
radar as problematic.
That's why aligning with sound/pci/hda behavior is better for both, ALSA
and ASoC users -> one place to fix the problems, both clients happy.
Disrupting basic HDaudio support to do things better has to be handled
with extreme caution and a ton of testing involving distro maintainers
and community members, so we are talking about an opt-in transition, not
an immediate switch. We've done a similar transition in the past to stop
using a dedicated hdac_hdmi.c codec, see all references to the
'use_common_hdmi' parameter in the SOF code. That transition seems to go
exactly against your second point above on HDMI and HDA being different
devices, so this could be an interesting debate.
Changes to the HDAudio/ASoC support would need to be handled with a
separate patchset anyways, and the SOF side changes done after we are
finished with the IPC4 and MeteorLake upstreaming. No one in our team
has any bandwidth to help with reviews or tests on this topic at the moment.
Agree. This won't be forced on anyone and that's why separate
implementation needed to be provided. There is too much to cover if we
were to refactor hdac_hda.c
I will also re-state that the removal of the skylake driver can only
happen after a long period of deprecation, when firmware and topologies
have been picked by distributions and all users are known to have
switched, so it's very likely that any alignment between "all HD-Audio
ASoC users" mentioned above does include the Skylake driver, doesn't it?
Nah, I don't believe we need to be saving skylake-driver here. By "all
HD-Audio ASoC users" I meant sof-driver as it isn't going anywhere -
what cannot be said about the skylake-driver :)
So to circle back: is there anything preventing the use of the existing
hdac_hda.c codec in this "ASoC: Intel: avs: Machine boards and HDA codec
support" series and can the HDaudio codec change be done "later" in a
more organized way?
Yeah, all the pm scenarios will fail when paired with the avs-driver.
The expectations are different. We'd have to fix probe() and remove()
(and related) sequences for the hdac_hda.c, and given that its users did
not notice prompts further problems with the refactor. This is very
similar to the skylake-driver vs avs-driver case. We could have applied
~300 patches we had internally that prepare skylake-driver to be
re-modeled and then apply patchsets which look more or less like the
avs-driver series instead of providing a parallel driver.
But the reality shows that such approach applies too much pressure on
the reviewers and leaves no fallback option for the end users if
anything fails along the way.
Regards,
Czarek
