- Sound-open-firmware - mailman.alsa-project.org

Getting equalizer to work on T14 Intel Gen 3 ( Alder Lake )
by Sylvain Munaut 08 Aug '23

08 Aug '23

Hi, So I have an alder lake laptop using ALC3287 codec. Things "work" in the sense that I get sound output from the speaker but it's of a very underwhelming quality. My understanding is that under windows there is quite a bit of EQ applied to try and get a better response from the speakers. So I'm trying to get that working under linux, but I have no idea where to even _start_. I have seen the "runtime adjustment' page on the website, but when looking at the controls I get : $ amixer -Dhw:0 controls | grep EQ numid=43,iface=MIXER,name='EQIIR10.0 eqiir_coef_10' numid=45,iface=MIXER,name='EQIIR11.0 eqiir_coef_11' numid=37,iface=MIXER,name='EQIIR2.0 eqiir_bytes_2' I get 3 IIR and not FIR. And I also have no idea what the difference is between those ... I also tried loading some eq_bass_boost settings in all of them using sof-ctl and although it does work, it doesn't change the sound output in any way as far as I can tell. So what am I doing wrong ? How can I get eq working ? I tried looking at the source code but I don't even see anything related to ALC3287 codec in there (there are others), so I'm not even sure what ends up running in my case. This is what get printed during boot : [ 17.427412] sof-audio-pci-intel-tgl 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if info 0x040380 [ 17.428323] sof-audio-pci-intel-tgl 0000:00:1f.3: Digital mics found on Skylake+ platform, using SOF driver [ 17.434104] sof-audio-pci-intel-tgl 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380 [ 18.738194] sof-audio-pci-intel-tgl 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915]) [ 18.828369] sof-audio-pci-intel-tgl 0000:00:1f.3: use msi interrupt mode [ 18.874040] sof-audio-pci-intel-tgl 0000:00:1f.3: hda codecs found, mask 5 [ 18.874043] sof-audio-pci-intel-tgl 0000:00:1f.3: using HDA machine driver skl_hda_dsp_generic now [ 18.874046] sof-audio-pci-intel-tgl 0000:00:1f.3: DMICs detected in NHLT tables: 2 [ 18.874057] Loading firmware: intel/sof/sof-adl.ri [ 18.876222] sof-audio-pci-intel-tgl 0000:00:1f.3: Firmware info: version 2:2:0-57864 [ 18.876223] sof-audio-pci-intel-tgl 0000:00:1f.3: Firmware: ABI 3:22:1 Kernel ABI 3:23:0 [ 18.876226] sof-audio-pci-intel-tgl 0000:00:1f.3: unknown sof_ext_man header type 3 size 0x30 [ 18.989668] sof-audio-pci-intel-tgl 0000:00:1f.3: Firmware info: version 2:2:0-57864 [ 18.989671] sof-audio-pci-intel-tgl 0000:00:1f.3: Firmware: ABI 3:22:1 Kernel ABI 3:23:0 [ 19.019641] Loading firmware: intel/sof-tplg/sof-hda-generic-2ch.tplg [ 19.020671] sof-audio-pci-intel-tgl 0000:00:1f.3: Topology: ABI 3:22:1 Kernel ABI 3:23:0 [ 19.133888] input: sof-hda-dsp Mic as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input18 [ 19.133947] input: sof-hda-dsp Headphone as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input19 [ 19.133973] input: sof-hda-dsp HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input20 [ 19.134032] input: sof-hda-dsp HDMI/DP,pcm=4 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input21 [ 19.134050] input: sof-hda-dsp HDMI/DP,pcm=5 as /devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input22 Cheers, Sylvain

2 2

Regression fix not progressing towards mainline/stable
by Thorsten Leemhuis 08 Aug '23

08 Aug '23

Hi everyone, I noticed a regression fix that afaics lingers in the thesofproject's git repo instead of progressing towards the stable trees and wondered what's up here. I'm talking about about the fix "ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop" for this ticket: https://github.com/thesofproject/linux/issues/4455 Two more tickets about it: https://github.com/thesofproject/linux/issues/4482 https://bugzilla.kernel.org/show_bug.cgi?id=217673 It seems the fix is ready and reviewed for two weeks now, but not even in -next by now. That's not how it should be for regression fixes, as per https://docs.kernel.org/process/handling-regressions.html it ideally should be in -stable soon or already. Is this a mistake, am I missing something, or is there a good reason for this? Ciao, Thorsten

2 1

[PATCH] ASoC: SOF: mediatek: mt8186 modify dram type as non-cache
by Trevor Wu 04 Aug '23

04 Aug '23

To prevent incorrect access between the host and DSP sides, we need to modify DRAM as a non-cache memory type. Additionally, we can retrieve the size of shared DMA from the device tree. Signed-off-by: Trevor Wu <trevor.wu(a)mediatek.com> Reviewed-by: Yaochun Hung <yc.hung(a)mediatek.com> Reviewed-by: Kuan-Hsun Cheng <Allen-KH.Cheng(a)mediatek.com> --- sound/soc/sof/mediatek/mt8186/mt8186.c | 40 +++++++++++++++----------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index 3e0ea0e109e2..f587edf9e0a7 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -111,6 +111,14 @@ static int platform_parse_resource(struct platform_device *pdev, void *data) dev_dbg(dev, "DMA %pR\n", &res); + adsp->pa_shared_dram = (phys_addr_t)res.start; + adsp->shared_size = resource_size(&res); + if (adsp->pa_shared_dram & DRAM_REMAP_MASK) { + dev_err(dev, "adsp shared dma memory(%#x) is not 4K-aligned\n", + (u32)adsp->pa_shared_dram); + return -EINVAL; + } + ret = of_reserved_mem_device_init(dev); if (ret) { dev_err(dev, "of_reserved_mem_device_init failed\n"); @@ -244,23 +252,18 @@ static int adsp_shared_base_ioremap(struct platform_device *pdev, void *data) { struct device *dev = &pdev->dev; struct mtk_adsp_chip_info *adsp = data; - u32 shared_size; /* remap shared-dram base to be non-cachable */ - shared_size = TOTAL_SIZE_SHARED_DRAM_FROM_TAIL; - adsp->pa_shared_dram = adsp->pa_dram + adsp->dramsize - shared_size; - if (adsp->va_dram) { - adsp->shared_dram = adsp->va_dram + DSP_DRAM_SIZE - shared_size; - } else { - adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, - shared_size); - if (!adsp->shared_dram) { - dev_err(dev, "ioremap failed for shared DRAM\n"); - return -ENOMEM; - } + adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, + adsp->shared_size); + if (!adsp->shared_dram) { + dev_err(dev, "failed to ioremap base %pa size %#x\n", + adsp->shared_dram, adsp->shared_size); + return -ENOMEM; } - dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", - adsp->shared_dram, &adsp->pa_shared_dram, shared_size); + + dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", + adsp->shared_dram, &adsp->pa_shared_dram, adsp->shared_size); return 0; } @@ -307,9 +310,12 @@ static int mt8186_dsp_probe(struct snd_sof_dev *sdev) return -ENOMEM; } - sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, - priv->adsp->pa_dram, - priv->adsp->dramsize); + priv->adsp->va_sram = sdev->bar[SOF_FW_BLK_TYPE_IRAM]; + + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap(sdev->dev, + priv->adsp->pa_dram, + priv->adsp->dramsize); + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) { dev_err(sdev->dev, "failed to ioremap base %pa size %#x\n", &priv->adsp->pa_dram, priv->adsp->dramsize); -- 2.18.0

4 3

Re: [PATCH] ASoC: SOF: Intel: Move binding to display driver outside of deferred probe
by Maarten Lankhorst 04 Aug '23

04 Aug '23

Hey, Den 2023-08-04 kl. 13:59, skrev Mark Brown: > On Fri, Aug 04, 2023 at 12:47:54PM +0200, Maarten Lankhorst wrote: >> On 2023-08-01 18:32, Pierre-Louis Bossart wrote: >> This mail can be applied with git am -c. >> ------8<--------- >> Now that we can use -EPROBE_DEFER, it's no longer required to spin off > Don't do this, it breaks my automation and means I very nearly just > skipped the patch entirely since it looked like the middle of some x86 > discussion. Yeah, it's replacing the patch from earlier. I can resend, but means having to add all acks, r-b'd, etc. :) If you have scripts that do that, all the better. Cheers, ~Maarten

1 0

Re: [PATCH v2 0/9] sound: Use -EPROBE_DEFER instead of i915 module loading.
by Takashi Iwai 01 Aug '23

01 Aug '23

On Wed, 19 Jul 2023 18:41:32 +0200, Maarten Lankhorst wrote: > > Explicitly loading i915 becomes a problem when upstreaming the new intel driver > for Tiger Lake and higher graphics (xe). By loading i915, it doesn't wait for > driver load of xe, and will fail completely before it loads. > > -EPROBE_DEFER has to be returned before any device is created in probe(), > otherwise the removal of the device will cause EPROBE_DEFER to try again > in an infinite loop. > > The conversion is done in gradual steps. First I add an argument to > snd_hdac_i915_init to allow for -EPROBE_DEFER so I can convert each driver > separately. Then I convert each driver to move snd_hdac_i915_init out of the > workqueue. Finally I drop the ability to choose modprobe behavior after the > last user is converted. > > I suspect the avs and skylake drivers used snd_hdac_i915_init purely for the > modprobe, but I don't have the hardware to test if it can be safely removed. > It can still be done easily in a followup patch to simplify probing. > > --- > New since first version: > > - snd_hda_core.gpu_bind is added as a mechanism to force gpu binding, > for testing. snd_hda_core.gpu_bind=0 forces waiting for GPU bind to > off, snd_hda_core.gpu_bind=1 forces waiting for gpu bind. Default > setting depends on whether kernel booted with nomodeset. > - Incorporated all feedback review. Maarten, are you working on v3 patch set? Or, for moving forward, should we merge v2 now and fix the rest based on that later? thanks, Takashi

3 5

Re: [PATCH v2 2/9] ALSA: hda/i915: Allow override of gpu binding.
by Péter Ujfalusi 25 Jul '23

25 Jul '23

On 19/07/2023 19:41, Maarten Lankhorst wrote: > Selecting CONFIG_DRM selects CONFIG_VIDEO_NOMODESET, which exports > video_firmware_drivers_only(). This can be used as a first > approximation on whether i915 will be available. It's safe to use as > this is only built when CONFIG_SND_HDA_I915 is selected by CONFIG_I915. > > It's not completely fool proof, as you can boot with "nomodeset > i915.modeset=1" to make i915 load regardless, or use > "i915.force_probe=!*" to never load i915, but the common case of > booting with nomodeset to disable all GPU drivers this will work as > intended. > > Because of this, we add an extra module parameter, > snd_hda_core.gpu_bind that can be used to signal users intent. > -1 follows nomodeset, 0 disables binding, 1 forces wait/-EPROBE_DEFER > on binding. Reviewed-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> > --- > sound/hda/hdac_i915.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c > index 161a9711cd63e..c32709fa4115f 100644 > --- a/sound/hda/hdac_i915.c > +++ b/sound/hda/hdac_i915.c > @@ -11,6 +11,13 @@ > #include <sound/hda_i915.h> > #include <sound/hda_register.h> > > +#include <video/nomodeset.h> > + > +static int gpu_bind = -1; > +module_param(gpu_bind, int, 0644); > +MODULE_PARM_DESC(gpu_bind, "Whether to bind sound component to GPU " > + "(1=always, 0=never, -1=on nomodeset(default))"); > + > #define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \ > ((pci)->device == 0x0c0c) || \ > ((pci)->device == 0x0d0c) || \ > @@ -121,6 +128,9 @@ static int i915_gfx_present(struct pci_dev *hdac_pci) > { > struct pci_dev *display_dev = NULL; > > + if (!gpu_bind || (gpu_bind < 0 && video_firmware_drivers_only())) > + return false; > + > for_each_pci_dev(display_dev) { > if (display_dev->vendor == PCI_VENDOR_ID_INTEL && > (display_dev->class >> 16) == PCI_BASE_CLASS_DISPLAY && -- Péter

4 4

Re: [PATCH v2 8/9] ASoC: SOF: Intel: Remove deferred probe for SOF
by Pierre-Louis Bossart 25 Jul '23

25 Jul '23

On 7/19/23 18:41, Maarten Lankhorst wrote: > This was only used to allow modprobing i915, by converting to the > -EPROBE_DEFER mechanism, it can be completely removed, and is in > fact counterproductive since -EPROBE_DEFER otherwise won't be > handled correctly. I personally remember only that the request_module("i915") was the main motivation for the use of the workqueue, but when it comes to the HDaudio codec management we don't even know what we don't know. I am a bit worried that the snd-hda-intel driver keeps the workqueue for HDaudio codec initialization, and this patch removes the workqueue completely for SOF. That doesn't seem right. Either both drivers need a workqueue or none need a workqueue. Maybe what we need is to move the i915/xe initialization out of the workqueue, and see in a second pass if that workqueue can be safely removed from the SOF driver? > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> > Acked-by: Matthew Auld <matthew.auld(a)intel.com> > Acked-by: Mark Brown <broonie(a)kernel.org> > --- > sound/soc/sof/Kconfig | 19 ----------------- > sound/soc/sof/core.c | 38 ++------------------------------- > sound/soc/sof/intel/Kconfig | 1 - > sound/soc/sof/intel/hda-codec.c | 2 +- > sound/soc/sof/intel/hda.c | 32 ++++++++++++++++----------- > sound/soc/sof/sof-pci-dev.c | 3 +-- > sound/soc/sof/sof-priv.h | 5 ----- > 7 files changed, 23 insertions(+), 77 deletions(-) > > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig > index 80361139a49ad..8ee39e5558062 100644 > --- a/sound/soc/sof/Kconfig > +++ b/sound/soc/sof/Kconfig > @@ -82,17 +82,6 @@ config SND_SOC_SOF_DEVELOPER_SUPPORT > > if SND_SOC_SOF_DEVELOPER_SUPPORT > > -config SND_SOC_SOF_FORCE_PROBE_WORKQUEUE > - bool "SOF force probe workqueue" > - select SND_SOC_SOF_PROBE_WORK_QUEUE > - help > - This option forces the use of a probe workqueue, which is only used > - when HDaudio is enabled due to module dependencies. Forcing this > - option is intended for debug only, but this should not add any > - functional issues in nominal cases. > - Say Y if you are involved in SOF development and need this option. > - If not, select N. > - > config SND_SOC_SOF_NOCODEC > tristate > > @@ -271,14 +260,6 @@ config SND_SOC_SOF > module dependencies but since the module or built-in type is decided > at the top level it doesn't matter. > > -config SND_SOC_SOF_PROBE_WORK_QUEUE > - bool > - help > - This option is not user-selectable but automagically handled by > - 'select' statements at a higher level. > - When selected, the probe is handled in two steps, for example to > - avoid lockdeps if request_module is used in the probe. > - > # Supported IPC versions > config SND_SOC_SOF_IPC3 > bool > diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c > index 30db685cc5f4b..cdf86dc4a8a87 100644 > --- a/sound/soc/sof/core.c > +++ b/sound/soc/sof/core.c > @@ -191,7 +191,8 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > /* probe the DSP hardware */ > ret = snd_sof_probe(sdev); > if (ret < 0) { > - dev_err(sdev->dev, "error: failed to probe DSP %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(sdev->dev, "error: failed to probe DSP %d\n", ret); > goto probe_err; > } > > @@ -309,8 +310,6 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > if (plat_data->sof_probe_complete) > plat_data->sof_probe_complete(sdev->dev); > > - sdev->probe_completed = true; > - > return 0; > > sof_machine_err: > @@ -336,19 +335,6 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > return ret; > } > > -static void sof_probe_work(struct work_struct *work) > -{ > - struct snd_sof_dev *sdev = > - container_of(work, struct snd_sof_dev, probe_work); > - int ret; > - > - ret = sof_probe_continue(sdev); > - if (ret < 0) { > - /* errors cannot be propagated, log */ > - dev_err(sdev->dev, "error: %s failed err: %d\n", __func__, ret); > - } > -} > - > int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data) > { > struct snd_sof_dev *sdev; > @@ -436,33 +422,16 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data) > > sof_set_fw_state(sdev, SOF_FW_BOOT_NOT_STARTED); > > - if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) { > - INIT_WORK(&sdev->probe_work, sof_probe_work); > - schedule_work(&sdev->probe_work); > - return 0; > - } > - > return sof_probe_continue(sdev); > } > EXPORT_SYMBOL(snd_sof_device_probe); > > -bool snd_sof_device_probe_completed(struct device *dev) > -{ > - struct snd_sof_dev *sdev = dev_get_drvdata(dev); > - > - return sdev->probe_completed; > -} > -EXPORT_SYMBOL(snd_sof_device_probe_completed); > - > int snd_sof_device_remove(struct device *dev) > { > struct snd_sof_dev *sdev = dev_get_drvdata(dev); > struct snd_sof_pdata *pdata = sdev->pdata; > int ret; > > - if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) > - cancel_work_sync(&sdev->probe_work); > - > /* > * Unregister any registered client device first before IPC and debugfs > * to allow client drivers to be removed cleanly > @@ -501,9 +470,6 @@ int snd_sof_device_shutdown(struct device *dev) > { > struct snd_sof_dev *sdev = dev_get_drvdata(dev); > > - if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) > - cancel_work_sync(&sdev->probe_work); > - > if (sdev->fw_state == SOF_FW_BOOT_COMPLETE) { > sof_fw_trace_free(sdev); > return snd_sof_shutdown(sdev); > diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig > index 69c1a370d3b61..d9e87a91670a3 100644 > --- a/sound/soc/sof/intel/Kconfig > +++ b/sound/soc/sof/intel/Kconfig > @@ -293,7 +293,6 @@ config SND_SOC_SOF_HDA_LINK > config SND_SOC_SOF_HDA_AUDIO_CODEC > bool "SOF support for HDAudio codecs" > depends on SND_SOC_SOF_HDA_LINK > - select SND_SOC_SOF_PROBE_WORK_QUEUE > help > This adds support for HDAudio codecs with Sound Open Firmware > for Intel(R) platforms. > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c > index f1fd5b44aaac9..344b61576c0e3 100644 > --- a/sound/soc/sof/intel/hda-codec.c > +++ b/sound/soc/sof/intel/hda-codec.c > @@ -415,7 +415,7 @@ int hda_codec_i915_init(struct snd_sof_dev *sdev) > return 0; > > /* i915 exposes a HDA codec for HDMI audio */ > - ret = snd_hdac_i915_init(bus, true); > + ret = snd_hdac_i915_init(bus, false); > if (ret < 0) > return ret; > > diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c > index 64bebe1a72bbc..a8b7a68142c05 100644 > --- a/sound/soc/sof/intel/hda.c > +++ b/sound/soc/sof/intel/hda.c > @@ -801,8 +801,11 @@ static int hda_init(struct snd_sof_dev *sdev) > > /* init i915 and HDMI codecs */ > ret = hda_codec_i915_init(sdev); > - if (ret < 0) > - dev_warn(sdev->dev, "init of i915 and HDMI codec failed\n"); > + if (ret < 0) { > + if (ret != -EPROBE_DEFER) > + dev_warn(sdev->dev, "init of i915 and HDMI codec failed: %i\n", ret); > + return ret; > + } > > /* get controller capabilities */ > ret = hda_dsp_ctrl_get_caps(sdev); > @@ -1115,14 +1118,6 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) > sdev->pdata->hw_pdata = hdev; > hdev->desc = chip; > > - hdev->dmic_dev = platform_device_register_data(sdev->dev, "dmic-codec", > - PLATFORM_DEVID_NONE, > - NULL, 0); > - if (IS_ERR(hdev->dmic_dev)) { > - dev_err(sdev->dev, "error: failed to create DMIC device\n"); > - return PTR_ERR(hdev->dmic_dev); > - } > - > /* > * use position update IPC if either it is forced > * or we don't have other choice > @@ -1142,6 +1137,15 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) > if (ret < 0) > goto hdac_bus_unmap; > > + hdev->dmic_dev = platform_device_register_data(sdev->dev, "dmic-codec", > + PLATFORM_DEVID_NONE, > + NULL, 0); > + if (IS_ERR(hdev->dmic_dev)) { > + dev_err(sdev->dev, "error: failed to create DMIC device\n"); > + ret = PTR_ERR(hdev->dmic_dev); > + goto hdac_exit; > + } > + I am not following why we have to move dmic-related code, can we try to move this in a separate patch? > if (sdev->dspless_mode_selected) > goto skip_dsp_setup; > > @@ -1150,7 +1154,7 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) > if (!sdev->bar[HDA_DSP_BAR]) { > dev_err(sdev->dev, "error: ioremap error\n"); > ret = -ENXIO; > - goto hdac_bus_unmap; > + goto platform_unreg; > } > > sdev->mmio_bar = HDA_DSP_BAR; > @@ -1248,10 +1252,12 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) > /* dsp_unmap: not currently used */ > if (!sdev->dspless_mode_selected) > iounmap(sdev->bar[HDA_DSP_BAR]); > -hdac_bus_unmap: > +platform_unreg: > platform_device_unregister(hdev->dmic_dev); > - iounmap(bus->remap_addr); > +hdac_exit: > hda_codec_i915_exit(sdev); > +hdac_bus_unmap: > + iounmap(bus->remap_addr); > err: > return ret; > } > diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c > index f5ece43d0ec24..0fa424613082e 100644 > --- a/sound/soc/sof/sof-pci-dev.c > +++ b/sound/soc/sof/sof-pci-dev.c > @@ -339,8 +339,7 @@ void sof_pci_remove(struct pci_dev *pci) > snd_sof_device_remove(&pci->dev); > > /* follow recommendation in pci-driver.c to increment usage counter */ > - if (snd_sof_device_probe_completed(&pci->dev) && > - !(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME)) > + if (!(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME)) > pm_runtime_get_noresume(&pci->dev); > > /* release pci regions and disable device */ > diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h > index d4f6702e93dcb..71db636cfdccc 100644 > --- a/sound/soc/sof/sof-priv.h > +++ b/sound/soc/sof/sof-priv.h > @@ -564,10 +564,6 @@ struct snd_sof_dev { > enum sof_fw_state fw_state; > bool first_boot; > > - /* work queue in case the probe is implemented in two steps */ > - struct work_struct probe_work; > - bool probe_completed; > - > /* DSP HW differentiation */ > struct snd_sof_pdata *pdata; > > @@ -675,7 +671,6 @@ struct snd_sof_dev { > int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data); > int snd_sof_device_remove(struct device *dev); > int snd_sof_device_shutdown(struct device *dev); > -bool snd_sof_device_probe_completed(struct device *dev); > > int snd_sof_runtime_suspend(struct device *dev); > int snd_sof_runtime_resume(struct device *dev);

3 2

Re: [PATCH v2 7/9] ALSA: hda/intel: Move snd_hdac_i915_init to before probe_work.
by Pierre-Louis Bossart 25 Jul '23

25 Jul '23

On 7/19/23 18:41, Maarten Lankhorst wrote: > Now that we can use -EPROBE_DEFER, it's no longer required to spin off > the snd_hdac_i915_init into a workqueue. > > Use the -EPROBE_DEFER mechanism instead, which must be returned in the > probe function. > > Changes since v1: > - Use dev_err_probe() > - Don't move probed_devs bitmap unnecessarily. (tiwai) > - Move snd_hdac_i915_init slightly upward, to ensure > it's always initialised before vga-switcheroo is called. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> > --- > sound/pci/hda/hda_intel.c | 59 ++++++++++++++++++++------------------- > 1 file changed, 30 insertions(+), 29 deletions(-) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index 11cf9907f039f..e3128d7d742e7 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -2147,6 +2147,36 @@ static int azx_probe(struct pci_dev *pci, > > pci_set_drvdata(pci, card); > > +#ifdef CONFIG_SND_HDA_I915 > + /* bind with i915 if needed */ > + if (chip->driver_caps & AZX_DCAPS_I915_COMPONENT) { > + err = snd_hdac_i915_init(azx_bus(chip), false); > + if (err < 0) { > + /* if the controller is bound only with HDMI/DP > + * (for HSW and BDW), we need to abort the probe; > + * for other chips, still continue probing as other > + * codecs can be on the same link. > + */ > + if (CONTROLLER_IN_GPU(pci)) { > + dev_err_probe(card->dev, err, > + "HSW/BDW HD-audio HDMI/DP requires binding with gfx driver\n"); > + > + goto out_free; > + } else { > + /* don't bother any longer */ > + chip->driver_caps &= ~AZX_DCAPS_I915_COMPONENT; > + } > + } > + > + /* HSW/BDW controllers need this power */ > + if (CONTROLLER_IN_GPU(pci)) > + hda->need_i915_power = true; > + } > +#else > + if (CONTROLLER_IN_GPU(pci)) > + dev_err(card->dev, "Haswell/Broadwell HDMI/DP must build in CONFIG_SND_HDA_I915\n"); > +#endif Is it intentional that the display_power() is left in the probe workqueue? this piece of code follows the stuff above in the existing code? /* Request display power well for the HDA controller or codec. For * Haswell/Broadwell, both the display HDA controller and codec need * this power. For other platforms, like Baytrail/Braswell, only the * display codec needs the power and it can be released after probe. */ display_power(chip, true); > + > err = register_vga_switcheroo(chip); > if (err < 0) { > dev_err(card->dev, "Error registering vga_switcheroo client\n"); > @@ -2174,11 +2204,6 @@ static int azx_probe(struct pci_dev *pci, > } > #endif /* CONFIG_SND_HDA_PATCH_LOADER */ > > -#ifndef CONFIG_SND_HDA_I915 > - if (CONTROLLER_IN_GPU(pci)) > - dev_err(card->dev, "Haswell/Broadwell HDMI/DP must build in CONFIG_SND_HDA_I915\n"); > -#endif > - > if (schedule_probe) > schedule_delayed_work(&hda->probe_work, 0); > > @@ -2275,30 +2300,6 @@ static int azx_probe_continue(struct azx *chip) > to_hda_bus(bus)->bus_probing = 1; > hda->probe_continued = 1; > > - /* bind with i915 if needed */ > - if (chip->driver_caps & AZX_DCAPS_I915_COMPONENT) { > - err = snd_hdac_i915_init(bus, true); > - if (err < 0) { > - /* if the controller is bound only with HDMI/DP > - * (for HSW and BDW), we need to abort the probe; > - * for other chips, still continue probing as other > - * codecs can be on the same link. > - */ > - if (CONTROLLER_IN_GPU(pci)) { > - dev_err(chip->card->dev, > - "HSW/BDW HD-audio HDMI/DP requires binding with gfx driver\n"); > - goto out_free; > - } else { > - /* don't bother any longer */ > - chip->driver_caps &= ~AZX_DCAPS_I915_COMPONENT; > - } > - } > - > - /* HSW/BDW controllers need this power */ > - if (CONTROLLER_IN_GPU(pci)) > - hda->need_i915_power = true; > - } > - > /* Request display power well for the HDA controller or codec. For > * Haswell/Broadwell, both the display HDA controller and codec need > * this power. For other platforms, like Baytrail/Braswell, only the

2 1

Re: [PATCH v2 4/9] ALSA: hda/i915: Allow xe as match for i915_component_master_match
by Pierre-Louis Bossart 25 Jul '23

25 Jul '23

On 7/19/23 18:41, Maarten Lankhorst wrote: > xe is a new driver for intel GPU's that shares the sound related code > with i915. > > Don't allow it to be modprobed though; the module is not upstream yet > and we should exclusively use the EPROBE_DEFER mechanism. I can't figure out what this comment means. how would the -EPROBE_DEFER mechanism help if the driver that will trigger a new probe is not upstream? Not following at all what you intended to explain. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> > --- > sound/hda/hdac_i915.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c > index 961fcd3397f40..12c1f8d93499f 100644 > --- a/sound/hda/hdac_i915.c > +++ b/sound/hda/hdac_i915.c > @@ -115,7 +115,8 @@ static int i915_component_master_match(struct device *dev, int subcomponent, > hdac_pci = to_pci_dev(bus->dev); > i915_pci = to_pci_dev(dev); > > - if (!strcmp(dev->driver->name, "i915") && > + if ((!strcmp(dev->driver->name, "i915") || > + !strcmp(dev->driver->name, "xe")) && > subcomponent == I915_COMPONENT_AUDIO && > connectivity_check(i915_pci, hdac_pci)) > return 1;

2 1

Re: [PATCH v2 1/9] ALSA: hda/intel: Fix error handling in azx_probe()
by Péter Ujfalusi 24 Jul '23

24 Jul '23

On 19/07/2023 19:41, Maarten Lankhorst wrote: > Add missing pci_set_drv to NULL call on error. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> > --- > sound/pci/hda/hda_intel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index ef831770ca7da..0d2d6bc6c75ef 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -2188,6 +2188,7 @@ static int azx_probe(struct pci_dev *pci, > return 0; > > out_free: > + pci_set_drvdata(pci, NULL); The original patch added this: f4c482a4d0b3 ("ALSA: hda - Fix yet another race of vga_switcheroo registration") but got removed later by: 20a24225d8f9 ("ALSA: PCI: Remove superfluous pci_set_drvdata(pci, NULL) at remove") and partially added back (to azx_remove) by: e81478bbe7a1 ("ALSA: hda: fix general protection fault in azx_runtime_idle") I guess, it should do not harm to add it back... > snd_card_free(card); > return err; > } Reviewed-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com>

2 1