| Message ID | 20200305145314.32579-3-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a66f88394a78fec9a05fa6e517e9603e8eca8363 |
| Headers | show |
| Series | ASoC: Intel: Skylake: Fix HDaudio and Dmic | expand |
On 3/5/20 8:53 AM, Cezary Rojewski wrote: > With _reset_link removed from the probe sequence, codec_mask at the time > skl_find_hda_machine() is invoked will always be 0, so hda machine will > never be chosen. Rather than reorganizing boot flow, be permissive about > invalid mask. codec_mask will be set to proper value during probe_work - > before skl_codec_create() ever gets called. humm, what would happen e.g. if you have select the SKL driver but there is no ACPI information to select an I2S-based machine driver, and HDaudio/iDISP are disabled? You would have no error checks then? > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> > --- > sound/soc/intel/skylake/skl.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c > index d66231525356..4827fe6bc1cb 100644 > --- a/sound/soc/intel/skylake/skl.c > +++ b/sound/soc/intel/skylake/skl.c > @@ -481,13 +481,8 @@ static struct skl_ssp_clk skl_ssp_clks[] = { > static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl_dev *skl, > struct snd_soc_acpi_mach *machines) > { > - struct hdac_bus *bus = skl_to_bus(skl); > struct snd_soc_acpi_mach *mach; > > - /* check if we have any codecs detected on bus */ > - if (bus->codec_mask == 0) > - return NULL; > - > /* point to common table */ > mach = snd_soc_acpi_intel_hda_machines; > >
On 2020-03-06 21:57, Pierre-Louis Bossart wrote: > On 3/5/20 8:53 AM, Cezary Rojewski wrote: >> With _reset_link removed from the probe sequence, codec_mask at the time >> skl_find_hda_machine() is invoked will always be 0, so hda machine will >> never be chosen. Rather than reorganizing boot flow, be permissive about >> invalid mask. codec_mask will be set to proper value during probe_work - >> before skl_codec_create() ever gets called. > > humm, what would happen e.g. if you have select the SKL driver but there > is no ACPI information to select an I2S-based machine driver, and > HDaudio/iDISP are disabled? You would have no error checks then? > Laptops I've been testing this with have had Realtek + iDisp present onboard. Now, if you disable Realtek + HDMI/DP modules within legacy HDaudio Kconfig and HD audio support within Intel Skylake tree then you end up with no required modules for said configuration at all. Nothing will happen really: no warnings, no sound card either. Just run such kernel on my setup and results are quite obvious: - skl boots - no machines present - drv stays dormant Czarek
On 3/9/20 8:47 AM, Cezary Rojewski wrote: > On 2020-03-06 21:57, Pierre-Louis Bossart wrote: >> On 3/5/20 8:53 AM, Cezary Rojewski wrote: >>> With _reset_link removed from the probe sequence, codec_mask at the time >>> skl_find_hda_machine() is invoked will always be 0, so hda machine will >>> never be chosen. Rather than reorganizing boot flow, be permissive about >>> invalid mask. codec_mask will be set to proper value during probe_work - >>> before skl_codec_create() ever gets called. >> >> humm, what would happen e.g. if you have select the SKL driver but >> there is no ACPI information to select an I2S-based machine driver, >> and HDaudio/iDISP are disabled? You would have no error checks then? >> > > Laptops I've been testing this with have had Realtek + iDisp present > onboard. Now, if you disable Realtek + HDMI/DP modules within legacy > HDaudio Kconfig and HD audio support within Intel Skylake tree then you > end up with no required modules for said configuration at all. Nothing > will happen really: no warnings, no sound card either. I meant enable the HDaudio controller but disable HDaudio codecs/HDMI at the BIOS level. In that case the codec_mask will never be set. > Just run such kernel on my setup and results are quite obvious: > - skl boots > - no machines present > - drv stays dormant > > Czarek
On 2020-03-09 18:03, Pierre-Louis Bossart wrote: > On 3/9/20 8:47 AM, Cezary Rojewski wrote: >> On 2020-03-06 21:57, Pierre-Louis Bossart wrote: >>> On 3/5/20 8:53 AM, Cezary Rojewski wrote: >>>> With _reset_link removed from the probe sequence, codec_mask at the >>>> time >>>> skl_find_hda_machine() is invoked will always be 0, so hda machine will >>>> never be chosen. Rather than reorganizing boot flow, be permissive >>>> about >>>> invalid mask. codec_mask will be set to proper value during >>>> probe_work - >>>> before skl_codec_create() ever gets called. >>> >>> humm, what would happen e.g. if you have select the SKL driver but >>> there is no ACPI information to select an I2S-based machine driver, >>> and HDaudio/iDISP are disabled? You would have no error checks then? >>> >> >> Laptops I've been testing this with have had Realtek + iDisp present >> onboard. Now, if you disable Realtek + HDMI/DP modules within legacy >> HDaudio Kconfig and HD audio support within Intel Skylake tree then >> you end up with no required modules for said configuration at all. >> Nothing will happen really: no warnings, no sound card either. > > I meant enable the HDaudio controller but disable HDaudio codecs/HDMI at > the BIOS level. In that case the codec_mask will never be set. > Applying diff (better than BIOS options which are not even present on some prod machines): diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 7e7be8e4dcf9..b0e4579f26f6 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -421,7 +421,8 @@ int snd_hdac_bus_reset_link(struct hdac_bus *bus, bool full_reset) /* detect codecs */ if (!bus->codec_mask) { - bus->codec_mask = snd_hdac_chip_readw(bus, STATESTS); + //bus->codec_mask = snd_hdac_chip_readw(bus, STATESTS); + bus->codec_mask = 0; dev_dbg(bus->dev, "codec_mask = 0x%lx\n", bus->codec_mask); } --- Results: - skl boots - no machines present - drv stays dormant Dumping boot log below (notice the '[ 21.569291] snd_soc_skl 0000:00:1f.3: codec_mask = 0x0' message): [ 21.462507] snd_soc_core:snd_soc_register_dai: snd-soc-dummy snd-soc-dummy: ASoC: dynamically register DAI snd-soc-dummy [ 21.462541] snd_soc_core:snd_soc_register_dai: snd-soc-dummy snd-soc-dummy: ASoC: Registered DAI 'snd-soc-dummy-dai' [ 21.536668] snd_soc_skl 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if info 0x040380 [ 21.542406] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Capability version: 0x0 [ 21.542412] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: HDA capability ID: 0x2 [ 21.542416] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Found ML capability [ 21.542421] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Capability version: 0x0 [ 21.542425] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: HDA capability ID: 0x3 [ 21.542429] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Found PP capability offset=800 [ 21.542434] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Capability version: 0x0 [ 21.542438] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: HDA capability ID: 0x1 [ 21.542443] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Found GTS capability offset=500 [ 21.542448] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Capability version: 0x0 [ 21.542451] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: HDA capability ID: 0x5 [ 21.542455] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Found DRSM capability [ 21.542460] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Capability version: 0x0 [ 21.542465] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: HDA capability ID: 0x4 [ 21.542468] snd_hda_core:snd_hdac_bus_parse_capabilities: snd_soc_skl 0000:00:1f.3: Found SPB capability [ 21.542672] snd_soc_skl:skl_first_init: snd_soc_skl 0000:00:1f.3: chipset global capabilities = 0x9701 [ 21.556416] snd_soc_skl:skl_nhlt_update_topology_bin: snd_soc_skl 0000:00:1f.3: oem_id LENOVO, oem_table_id TP-N23 oem_revision 4880 [ 21.557053] snd_soc_skl:skl_find_machine: snd_soc_skl 0000:00:1f.3: No matching I2S machine driver found [ 21.557650] snd_soc_skl:skl_init_dsp: snd_soc_skl 0000:00:1f.3: dsp registration status=0 [ 21.557658] snd_hda_ext_core:snd_hdac_ext_bus_get_ml_capabilities: snd_soc_skl 0000:00:1f.3: In snd_hdac_ext_bus_get_ml_capabilities Link count: 2 [ 21.558254] snd_soc_skl 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915]) [ 21.558259] snd_hda_core:snd_hdac_display_power: snd_soc_skl 0000:00:1f.3: display power enable [ 21.558272] snd_hda_core:snd_hdac_set_codec_wakeup: snd_soc_skl 0000:00:1f.3: enable codec wakeup [ 21.561088] snd_hda_core:snd_hdac_set_codec_wakeup: snd_soc_skl 0000:00:1f.3: disable codec wakeup [ 21.562606] snd_soc_skl:skl_init_pci: snd_soc_skl 0000:00:1f.3: Clearing TCSEL [ 21.562617] snd_hda_core:snd_hdac_set_codec_wakeup: snd_soc_skl 0000:00:1f.3: enable codec wakeup [ 21.569291] snd_soc_skl 0000:00:1f.3: codec_mask = 0x0 [ 21.569306] snd_hda_core:snd_hdac_set_codec_wakeup: snd_soc_skl 0000:00:1f.3: disable codec wakeup [ 21.570856] snd_soc_skl 0000:00:1f.3: no hda codecs found! [ 21.570901] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.570923] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP0 Pin' [ 21.570927] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.570947] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP1 Pin' [ 21.570951] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.570971] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP2 Pin' [ 21.570976] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.570996] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP3 Pin' [ 21.571000] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571020] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP4 Pin' [ 21.571024] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571044] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'SSP5 Pin' [ 21.571048] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571067] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'iDisp1 Pin' [ 21.571077] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571099] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'iDisp2 Pin' [ 21.571103] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571125] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'iDisp3 Pin' [ 21.571129] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571148] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'DMIC01 Pin' [ 21.571153] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571172] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'DMIC16k Pin' [ 21.571176] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571209] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'Analog CPU DAI' [ 21.571213] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571230] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'Alt Analog CPU DAI' [ 21.571233] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: dynamically register DAI 0000:00:1f.3 [ 21.571250] snd_soc_core:snd_soc_register_dai: snd_soc_skl 0000:00:1f.3: ASoC: Registered DAI 'Digital CPU DAI' [ 21.571566] snd_hda_core:snd_hdac_display_power: snd_soc_skl 0000:00:1f.3: display power disable [ 21.571585] snd_soc_skl:skl_runtime_suspend: snd_soc_skl 0000:00:1f.3: in skl_runtime_suspend
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index d66231525356..4827fe6bc1cb 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -481,13 +481,8 @@ static struct skl_ssp_clk skl_ssp_clks[] = { static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl_dev *skl, struct snd_soc_acpi_mach *machines) { - struct hdac_bus *bus = skl_to_bus(skl); struct snd_soc_acpi_mach *mach; - /* check if we have any codecs detected on bus */ - if (bus->codec_mask == 0) - return NULL; - /* point to common table */ mach = snd_soc_acpi_intel_hda_machines;
With _reset_link removed from the probe sequence, codec_mask at the time skl_find_hda_machine() is invoked will always be 0, so hda machine will never be chosen. Rather than reorganizing boot flow, be permissive about invalid mask. codec_mask will be set to proper value during probe_work - before skl_codec_create() ever gets called. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> --- sound/soc/intel/skylake/skl.c | 5 ----- 1 file changed, 5 deletions(-)