ALSA: hda: Unify codec construction #3775
Conversation
crojewsk-intel
requested review from
libinyang,
bardliao,
lgirdwood,
plbossart,
ranj063,
kv2019i,
dbaluta,
ujfalusi,
ajitkupandey,
kuanhsuncheng and
yaochunhung
as code owners
|
Can one of the admins verify this patch?
|
|
For reference, v1 of the series can be found here. |
|
Which of these two GitHub handles should be added to the SOF project? |
If I'm not mistaken, @crojewsk already is. This account is here as Intel requests RW rights and I don't want to lose my Warcraft III stuff lol. |
|
Perhaps both if that's not the problem? |
crojewsk-intel
force-pushed
the
hda_single_ctor
branch
from
c6acbbb
to
d8dbc95
Compare
crojewsk-intel
force-pushed
the
hda_single_ctor
branch
from
d8dbc95
to
50f7054
Compare
|
Update. Raw delta remained 1:1 to previous version +113, -134. A lot of code shuffling between the first three patches though. The following has been done to make git bisect happy:
Dropped Mark's ack as the code changed too much. Last three patches remained basically unchanged - |
|
CI tests don't seem too good @crojewsk-intel ? see https://sof-ci.01.org/linuxpr/PR3775/build445/devicetest/ |
crojewsk-intel
force-pushed
the
hda_single_ctor
branch
from
50f7054
to
803dc60
Compare
Indeed! Kai helped me in understanding the logs. Update on its way. |
Preliminary step in making snd_hda_codec_device_init() the only constructor for struct hda_codec instances. To do that, existing usage of hdac_ext equivalents has to be dropped. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Preliminary step in making snd_hda_codec_device_init() the only constructor for struct hda_codec instances. To do that, existing usage of hdac_ext equivalents has to be dropped. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
To make snd_hda_codec_device_init() the only constructor for struct hda_codec instances remaining tasks are: 1) no struct may wrap struct hda_codec as its base type 2) bus drivers (skylake and sof) which are the current hdac_ext users need to be adjusted to make use of newly added codec init and exit routines instead 3) as bus drivers (skylake and sof) are to be responsible for creating codec device and assigning it to hdac_hda_priv->codec, hdac_hda_dev_probe() has to be freed of that job To keep git bisect happy, all of these in made in one-go. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
With all HDAudio drivers aligned to make use of the same constructor, have codec freed on the device release regardless of its type. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
There are no users for snd_hdac_ext_bus_device_init() and snd_hdac_ext_bus_device_exit(). While at it, remove hdac_to_hda_priv() too for the exact same reason. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
If early probe of HDAudio bus driver fails e.g.: due to missing firmware file, snd_hda_codec_shutdown() ends in manipulating uninitialized codec->pcm_list_head causing page fault. Iinitialization of HDAudio codec in ASoC is split in two: - snd_hda_codec_device_init() - snd_hda_codec_device_new() snd_hda_codec_device_init() is called during probe_codecs() by HDAudio bus driver while snd_hda_codec_device_new() is called by codec-component's ->probe(). The second call will not happen until all components required by related sound card are present within the ASoC framework. With firmware failing to load during the PCI's deferred initialization i.e.: probe_work(), no platform components are ever registered. HDAudio codec enumeration is done at that point though, so the codec components became registered to ASoC framework, calling snd_hda_codec_device_init() in the process. Now, during platform reboot snd_hda_codec_shutdown() is called for every codec found on the HDAudio bus causing oops if any of them has not completed both of their initialization steps. Relocating field initialization fixes the issue. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
|
Results look much better now - except for 2x timeout. Guess problem with a platform? Happens a lot in my farm too.. |
There was a problem hiding this comment.
feel free to add my tag
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Probably best to send in 2+ weeks, when the merge window closes, since it's a relatively big change impacting two maintainers.
|
Closing as the PR's purpose has been fulfilled. I'll be sending this to upstream next Friday or so. Thanks! |
A follow up to the recent HDAudio fixes series [1]. Given the recently reported regression [2], before the page fault occurring on codec shutdown can be fixed, codec construction procedure needs to be updated for skylake and sof-intel drivers. Drivers: pci-hda and avs need no changes - already making use of snd_hda_codec_device_init().
Changes in v2:
1: https://lore.kernel.org/alsa-devel/20220706120230.427296-7-cezary.rojewski@intel.com/
2: https://lore.kernel.org/alsa-devel/3c40df55-3aee-1e08-493b-7b30cd84dc00@linux.intel.com/