- Alsa-devel - mailman.alsa-project.org

[PATCH v4] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload
by Peter Ujfalusi 03 May '22

03 May '22

It is possible to craft a topology where sof_get_control_data() would do out of bounds access because it expects that it is only called when the payload is bytes type. Confusingly it also handles other types of controls, but the payload parsing implementation is only valid for bytes. Fix the code to count the non bytes controls and instead of storing a pointer to sof_abi_hdr in sof_widget_data (which is only valid for bytes), store the pointer to the data itself and add a new member to save the size of the data. In case of non bytes controls we store the pointer to the chanv itself, which is just an array of values at the end. In case of bytes control, drop the wrong cdata->data (wdata[i].pdata) check against NULL since it is incorrect and invalid in this context. The data is pointing to the end of cdata struct, so it should never be null. Reported-by: Sergey Senozhatsky <senozhatsky(a)chromium.org> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- Hi, Changes since v3: - Move the comment about validating data along with the code which validates the data (checks the SOF_ABI_MAGIC) Changes since v2: - Drop the cdata->data check against NULL as it is not a valid test and since we are in sof_get_control_data() the memory has been already allocated changes since v1: - adjust the payload size for non bytes controls by subtracting the size of the sof_ipc_ctrl_data struct, plus add comment to note this Regards, Peter sound/soc/sof/ipc3-topology.c | 39 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index 572bcbfdb356..bc848f556b88 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -20,7 +20,8 @@ struct sof_widget_data { int ctrl_type; int ipc_cmd; - struct sof_abi_hdr *pdata; + void *pdata; + size_t pdata_size; struct snd_sof_control *control; }; @@ -784,16 +785,26 @@ static int sof_get_control_data(struct snd_soc_component *scomp, } cdata = wdata[i].control->ipc_control_data; - wdata[i].pdata = cdata->data; - if (!wdata[i].pdata) - return -EINVAL; - /* make sure data is valid - data can be updated at runtime */ - if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES && - wdata[i].pdata->magic != SOF_ABI_MAGIC) - return -EINVAL; + if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES) { + /* make sure data is valid - data can be updated at runtime */ + if (cdata->data->magic != SOF_ABI_MAGIC) + return -EINVAL; + + wdata[i].pdata = cdata->data->data; + wdata[i].pdata_size = cdata->data->size; + } else { + /* points to the control data union */ + wdata[i].pdata = cdata->chanv; + /* + * wdata[i].control->size is calculated with struct_size + * and includes the size of struct sof_ipc_ctrl_data + */ + wdata[i].pdata_size = wdata[i].control->size - + sizeof(struct sof_ipc_ctrl_data); + } - *size += wdata[i].pdata->size; + *size += wdata[i].pdata_size; /* get data type */ switch (cdata->cmd) { @@ -876,10 +887,12 @@ static int sof_process_load(struct snd_soc_component *scomp, */ if (ipc_data_size) { for (i = 0; i < widget->num_kcontrols; i++) { - memcpy(&process->data[offset], - wdata[i].pdata->data, - wdata[i].pdata->size); - offset += wdata[i].pdata->size; + if (!wdata[i].pdata_size) + continue; + + memcpy(&process->data[offset], wdata[i].pdata, + wdata[i].pdata_size); + offset += wdata[i].pdata_size; } } -- 2.36.0

3 2

[PATCH] ASoC: adau1761: Add ADAU1761-as-ADAU1361 compatibility mode
by Ricard Wanderlof 03 May '22

03 May '22

During probe, determine if the chip is in fact an ADAU1761 even though an ADAU1361 is specified, and perform additional operations to enable the ADAU1761 to behave as an ADAU1361, i.e. disregarding the DSP and setting up routing and PM transparently. This enables either chip to be mounted when an ADAU1361 is specified. Signed-off-by: Ricard Wanderlof <ricardw(a)axis.com> --- sound/soc/codecs/adau1761.c | 86 +++++++++++++++++++++++++++++++++++-- sound/soc/codecs/adau17x1.c | 20 +++++++-- sound/soc/codecs/adau17x1.h | 1 + 3 files changed, 99 insertions(+), 8 deletions(-) diff --git a/sound/soc/codecs/adau1761.c b/sound/soc/codecs/adau1761.c index fb006fc81653..8f887227981f 100644 --- a/sound/soc/codecs/adau1761.c +++ b/sound/soc/codecs/adau1761.c @@ -556,8 +556,6 @@ static const struct snd_soc_dapm_route adau1761_dapm_routes[] = { { "Left DAC", NULL, "Interpolator Resync Clock" }, { "Right DAC", NULL, "Interpolator Resync Clock" }, - { "DSP", NULL, "Digital Clock 0" }, - { "Slew Clock", NULL, "Digital Clock 0" }, { "Right Playback Mixer", NULL, "Slew Clock" }, { "Left Playback Mixer", NULL, "Slew Clock" }, @@ -569,6 +567,56 @@ static const struct snd_soc_dapm_route adau1761_dapm_routes[] = { { "Digital Clock 1", NULL, "SYSCLK" }, }; +static const struct snd_soc_dapm_route adau1761_dapm_dsp_routes[] = { + { "DSP", NULL, "Digital Clock 0" }, +}; + +static int adau1761_compatibility_probe(struct device *dev) +{ + struct adau *adau = dev_get_drvdata(dev); + struct regmap *regmap = adau->regmap; + int val, ret = 0; + + /* Only consider compatibility mode when ADAU1361 was specified. */ + if (adau->type != ADAU1361) + return 0; + + regcache_cache_bypass(regmap, true); + + /* + * This will enable the core clock and bypass the PLL, + * so that we can access the registers for probing purposes + * (without having to set up the PLL). + */ + regmap_write(regmap, ADAU17X1_CLOCK_CONTROL, + ADAU17X1_CLOCK_CONTROL_SYSCLK_EN); + + /* + * ADAU17X1_SERIAL_SAMPLING_RATE doesn't exist in non-DSP chips; + * reading it results in zero at all times, and write is a no-op. + * Use this register to probe for ADAU1761. + */ + regmap_write(regmap, ADAU17X1_SERIAL_SAMPLING_RATE, 1); + ret = regmap_read(regmap, ADAU17X1_SERIAL_SAMPLING_RATE, &val); + if (ret) + goto exit; + if (val != 1) + goto exit; + regmap_write(regmap, ADAU17X1_SERIAL_SAMPLING_RATE, 0); + ret = regmap_read(regmap, ADAU17X1_SERIAL_SAMPLING_RATE, &val); + if (ret) + goto exit; + if (val != 0) + goto exit; + + adau->type = ADAU1761_AS_1361; +exit: + /* Disable core clock after probing. */ + regmap_write(regmap, ADAU17X1_CLOCK_CONTROL, 0); + regcache_cache_bypass(regmap, false); + return ret; +} + static int adau1761_set_bias_level(struct snd_soc_component *component, enum snd_soc_bias_level level) { @@ -823,7 +871,11 @@ static int adau1761_component_probe(struct snd_soc_component *component) if (ret) return ret; - if (adau->type == ADAU1761) { + /* + * If we've got an ADAU1761, or an ADAU1761 operating as an + * ADAU1361, we need these non-DSP related DAPM widgets and routes. + */ + if (adau->type == ADAU1761 || adau->type == ADAU1761_AS_1361) { ret = snd_soc_dapm_new_controls(dapm, adau1761_dapm_widgets, ARRAY_SIZE(adau1761_dapm_widgets)); if (ret) @@ -834,7 +886,29 @@ static int adau1761_component_probe(struct snd_soc_component *component) if (ret) return ret; } - + /* + * These routes are DSP related and only used when we have a + * bona fide ADAU1761. + */ + if (adau->type == ADAU1761) { + ret = snd_soc_dapm_add_routes(dapm, adau1761_dapm_dsp_routes, + ARRAY_SIZE(adau1761_dapm_dsp_routes)); + if (ret) + return ret; + } + /* + * In the ADAU1761, by default, the AIF is routed to the DSP, whereas + * for the ADAU1361, the AIF is permanently routed to the ADC and DAC. + * Thus, if we have an ADAU1761 masquerading as an ADAU1361, + * we need to explicitly route the AIF to the ADC and DAC. + * For the ADAU1761, this is normally done by set_tdm_slot, but this + * function is not necessarily called during stream setup, so set up + * the compatible AIF routings here from the start. + */ + if (adau->type == ADAU1761_AS_1361) { + regmap_write(adau->regmap, ADAU17X1_SERIAL_INPUT_ROUTE, 0x01); + regmap_write(adau->regmap, ADAU17X1_SERIAL_OUTPUT_ROUTE, 0x01); + } ret = adau17x1_add_routes(component); if (ret < 0) return ret; @@ -919,6 +993,10 @@ int adau1761_probe(struct device *dev, struct regmap *regmap, if (ret) return ret; + ret = adau1761_compatibility_probe(dev); + if (ret) + return ret; + /* Enable cache only mode as we could miss writes before bias level * reaches standby and the core clock is enabled */ regcache_cache_only(regmap, true); diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index af05463af4ac..c0f44ecef606 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -334,6 +334,17 @@ static bool adau17x1_has_dsp(struct adau *adau) } } +/* Chip has a DSP but we're pretending it doesn't. */ +static bool adau17x1_has_disused_dsp(struct adau *adau) +{ + switch (adau->type) { + case ADAU1761_AS_1361: + return true; + default: + return false; + } +} + static bool adau17x1_has_safeload(struct adau *adau) { switch (adau->type) { @@ -516,10 +527,11 @@ static int adau17x1_hw_params(struct snd_pcm_substream *substream, regmap_update_bits(adau->regmap, ADAU17X1_CONVERTER0, ADAU17X1_CONVERTER0_CONVSR_MASK, div); - if (adau17x1_has_dsp(adau)) { + + if (adau17x1_has_dsp(adau) || adau17x1_has_disused_dsp(adau)) regmap_write(adau->regmap, ADAU17X1_SERIAL_SAMPLING_RATE, div); + if (adau17x1_has_dsp(adau)) regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, dsp_div); - } if (adau->sigmadsp) { ret = adau17x1_setup_firmware(component, params_rate(params)); @@ -663,7 +675,7 @@ static int adau17x1_set_dai_tdm_slot(struct snd_soc_dai *dai, switch (slot_width * slots) { case 32: - if (adau->type == ADAU1761) + if (adau->type == ADAU1761 || adau->type == ADAU1761_AS_1361) return -EINVAL; ser_ctrl1 = ADAU17X1_SERIAL_PORT1_BCLK32; @@ -738,7 +750,7 @@ static int adau17x1_set_dai_tdm_slot(struct snd_soc_dai *dai, regmap_update_bits(adau->regmap, ADAU17X1_SERIAL_PORT1, ADAU17X1_SERIAL_PORT1_BCLK_MASK, ser_ctrl1); - if (!adau17x1_has_dsp(adau)) + if (!adau17x1_has_dsp(adau) && !adau17x1_has_disused_dsp(adau)) return 0; if (adau->dsp_bypass[SNDRV_PCM_STREAM_PLAYBACK]) { diff --git a/sound/soc/codecs/adau17x1.h b/sound/soc/codecs/adau17x1.h index 98a3b6f5bc96..5e58abfffc3d 100644 --- a/sound/soc/codecs/adau17x1.h +++ b/sound/soc/codecs/adau17x1.h @@ -10,6 +10,7 @@ enum adau17x1_type { ADAU1361, ADAU1761, + ADAU1761_AS_1361, ADAU1381, ADAU1781, }; -- 2.20.1 -- Ricard Wolf Wanderlof ricardw(at)axis.com Axis Communications AB, Lund, Sweden www.axis.com Phone +46 46 272 2016 Fax +46 46 13 61 30

2 1

[PATCH] ASoC: dapm: Don't fold register value changes into notifications
by Mark Brown 03 May '22

03 May '22

DAPM tracks and reports the value presented to the user from DAPM controls separately to the register value, these may diverge during initialisation or when an autodisable control is in use. When writing DAPM controls we currently report that a change has occurred if either the DAPM value or the value stored in the register has changed, meaning that if the two are out of sync we may appear to report a spurious event to userspace. Since we use this folded in value for nothing other than the value reported to userspace simply drop the folding in of the register change. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- sound/soc/soc-dapm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index ca917a849c42..869c76506b66 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3437,7 +3437,6 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, update.val = val; card->update = &update; } - change |= reg_change; ret = soc_dapm_mixer_update_power(card, kcontrol, connect, rconnect); @@ -3539,7 +3538,6 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, update.val = val; card->update = &update; } - change |= reg_change; ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e); -- 2.30.2

1 1

[PATCH] ASoC: rt5514: Fix event generation for "DSP Voice Wake Up" control
by Mark Brown 03 May '22

03 May '22

The driver has a custom put function for "DSP Voice Wake Up" which does not generate event notifications on change, instead returning 0. Since we already exit early in the case that there is no change this can be fixed by unconditionally returning 1 at the end of the function. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- sound/soc/codecs/rt5514.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/rt5514.c b/sound/soc/codecs/rt5514.c index 1b9263460142..be8ece4630df 100644 --- a/sound/soc/codecs/rt5514.c +++ b/sound/soc/codecs/rt5514.c @@ -419,7 +419,7 @@ static int rt5514_dsp_voice_wake_up_put(struct snd_kcontrol *kcontrol, } } - return 0; + return 1; } static const struct snd_kcontrol_new rt5514_snd_controls[] = { -- 2.30.2

1 1

[PATCH 00/11] ASoC: SOF: Miscellaneous preparatory patches for IPC4
by Ranjani Sridharan 03 May '22

03 May '22

This series includes last few remaining miscellaneous patches to prepare for the introduction of new IPC version, IPC4, in the SOF driver. The changes include new IPC ops for topology parsing to set up the volume table, prepare the widgets for set up and free the routes. The remaining patches introduce new fields in the existing data structures for use in IPC4 and align the flows for widget/route set up so that they are common for both IPC3 and IPC4. Ranjani Sridharan (11): ASoC: SOF: Add a new op to set up volume table ASoC: SOF: sof-audio: reset route status before freeing widget ASoC: SOF: sof-audio: Set up widgets from source to sink ASoC: SOF: pcm: Move the call to snd_sof_pcm_platform_hw_params() ASoC: SOF: expose a couple of functions ASoC: SOF: Add a route_free op in struct sof_ipc_tplg_ops ASoC: SOF: Add two new fields to struct snd_sof_widget ASoC: SOF: pcm: remove unnecessary function declaration ASoC: SOF: topology: Skip parsing DAI link tokens if not needed ASoC: SOF: clarify use of widget complete flag ASoC: SOF: Add a prepare op to IPC topology widget ops sound/soc/sof/ipc3-control.c | 38 +++-- sound/soc/sof/pcm.c | 23 +-- sound/soc/sof/sof-audio.c | 302 +++++++++++++++++++++++++++++------ sound/soc/sof/sof-audio.h | 68 +++++++- sound/soc/sof/topology.c | 44 +++-- 5 files changed, 368 insertions(+), 107 deletions(-) -- 2.25.1

2 12

[PATCH] ASoC: max98090: Fix event generation for custom controls
by Mark Brown 03 May '22

03 May '22

The previous fix for event generation for custom controls compared the value already in the register with the value being written, missing the logic that only applies the value to the register when the control is already enabled. Fix this, compare the value cached in the driver data rather than the register. This should really be an autodisable control rather than open coded. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- sound/soc/codecs/max98090.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c index 5146837c0fce..48dcf071bb5a 100644 --- a/sound/soc/codecs/max98090.c +++ b/sound/soc/codecs/max98090.c @@ -396,6 +396,7 @@ static int max98090_put_enab_tlv(struct snd_kcontrol *kcontrol, unsigned int sel = ucontrol->value.integer.value[0]; unsigned int val = snd_soc_component_read(component, mc->reg); unsigned int *select; + int change; switch (mc->reg) { case M98090_REG_MIC1_INPUT_LEVEL: @@ -416,6 +417,7 @@ static int max98090_put_enab_tlv(struct snd_kcontrol *kcontrol, if (sel < 0 || sel > mc->max) return -EINVAL; + change = *select != sel; *select = sel; /* Setting a volume is only valid if it is already On */ @@ -430,7 +432,7 @@ static int max98090_put_enab_tlv(struct snd_kcontrol *kcontrol, mask << mc->shift, sel << mc->shift); - return *select != val; + return change; } static const char *max98090_perf_pwr_text[] = -- 2.30.2

1 1

[PATCH AUTOSEL 5.17 01/22] ASoC: soc-pcm: use GFP_KERNEL when the code is sleepable
by Sasha Levin 02 May '22

02 May '22

From: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> [ Upstream commit fb6d679fee95d272c0a94912c4e534146823ee89 ] At the kzalloc() call in dpcm_be_connect(), there is no spin lock involved. It's merely protected by card->pcm_mutex, instead. The spinlock is applied at the later call with snd_soc_pcm_stream_lock_irq() only for the list manipulations. (See it's *_irq(), not *_irqsave(); that means the context being sleepable at that point.) So, we can use GFP_KERNEL safely there. This patch revert commit d8a9c6e1f676 ("ASoC: soc-pcm: use GFP_ATOMIC for dpcm structure") which is no longer needed since commit b7898396f4bb ("ASoC: soc-pcm: Fix and cleanup DPCM locking"). Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> Link: https://lore.kernel.org/r/e740f1930843060e025e3c0f17ec1393cfdafb26.16487579… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 9a954680d492..11c9853e9e80 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1214,7 +1214,7 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, be_substream->pcm->nonatomic = 1; } - dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_ATOMIC); + dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_KERNEL); if (!dpcm) return -ENOMEM; -- 2.35.1

4 10

[PATCH v2 0/2] Let userspace know when snd-hda-intel needs i915
by Mauro Carvalho Chehab 01 May '22

01 May '22

Currently, kernel/module annotates module dependencies when request_symbol is used, but it doesn't cover more complex inter-driver dependencies that are subsystem and/or driver-specific. In the case of hdmi sound, depending on the CPU/GPU, sometimes the snd_hda_driver can talk directly with the hardware, but sometimes, it uses the i915 driver. When the snd_hda_driver uses i915, it should first be unbind/rmmod, as otherwise trying to unbind/rmmod the i915 driver cause driver issues, as as reported by CI tools with different GPU models: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6415/fi-tgl-1115g4/igt@core_ho… https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11495/bat-adlm-1/igt@i915_m… In the past, just a few CPUs were doing such bindings, but this issue now applies to all "modern" Intel CPUs that have onboard graphics, as well as to the newer discrete GPUs. With the discrete GPU case, the HDA controller is physically separate and requires i915 to power on the hardware for all hardware access. In this case, the issue is hit basicly 100% of the time. With on-board graphics, i915 driver is needed only when the display codec is accessed. If i915 is unbind during runtime suspend, while snd-hda-intel is still bound, nothing bad happens, but unbinding i915 on other situations may also cause issues. So, add support at kernel/modules to allow snd-hda drivers to properly annotate when a dependency on a DRM driver dependencies exists, and add a call to such new function at the snd-hda driver when it successfully binds into the DRM driver. This would allow userspace tools to check and properly remove the audio driver before trying to remove or unbind the GPU driver. It should be noticed that this series conveys the hidden module dependencies. Other changes are needed in order to allow removing or unbinding the i915 driver while keeping the snd-hda-intel driver loaded/bound. With that regards, there are some discussions on how to improve this at alsa-devel a while back: https://mailman.alsa-project.org/pipermail/alsa-devel/2021-September/190099… So, future improvements on both in i915 and the audio drivers could be made. E.g. with discrete GPUs, it's the only codec of the card, so it seems feasible to detach the ALSA card if i915 is bound (using infra made for VGA switcheroo), but, until these improvements are done and land in upstream, audio drivers needs to be unbound if i915 driver goes unbind. Yet, even if such fixes got merged, this series is still needed, as it makes such dependencies more explicit and easier to debug. PS.: This series was generated against next-20220428. --- v2: the dependencies are now handled directly at try_module_get(). Mauro Carvalho Chehab (2): module: update dependencies at try_module_get() ALSA: hda - identify when audio is provided by a video driver include/linux/module.h | 4 +++- kernel/module/main.c | 35 +++++++++++++++++++++++++++++++++-- sound/hda/hdac_component.c | 2 +- 3 files changed, 37 insertions(+), 4 deletions(-) -- 2.35.1

3 5

[PATCH v3 0/2] Let userspace know when snd-hda-intel needs i915
by Mauro Carvalho Chehab 30 Apr '22

30 Apr '22

Currently, kernel/module annotates module dependencies when request_symbol is used, but it doesn't cover more complex inter-driver dependencies that are subsystem and/or driver-specific. In the case of hdmi sound, depending on the CPU/GPU, sometimes the snd_hda_driver can talk directly with the hardware, but sometimes, it uses the i915 driver. When the snd_hda_driver uses i915, it should first be unbind/rmmod, as otherwise trying to unbind/rmmod the i915 driver cause driver issues, as as reported by CI tools with different GPU models: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6415/fi-tgl-1115g4/igt@core_ho… https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11495/bat-adlm-1/igt@i915_m… In the past, just a few CPUs were doing such bindings, but this issue now applies to all "modern" Intel CPUs that have onboard graphics, as well as to the newer discrete GPUs. With the discrete GPU case, the HDA controller is physically separate and requires i915 to power on the hardware for all hardware access. In this case, the issue is hit basicly 100% of the time. With on-board graphics, i915 driver is needed only when the display codec is accessed. If i915 is unbind during runtime suspend, while snd-hda-intel is still bound, nothing bad happens, but unbinding i915 on other situations may also cause issues. So, add support at kernel/modules to allow snd-hda drivers to properly annotate when a dependency on a DRM driver dependencies exists, and add a call to such new function at the snd-hda driver when it successfully binds into the DRM driver. This would allow userspace tools to check and properly remove the audio driver before trying to remove or unbind the GPU driver. It should be noticed that this series conveys the hidden module dependencies. Other changes are needed in order to allow removing or unbinding the i915 driver while keeping the snd-hda-intel driver loaded/bound. With that regards, there are some discussions on how to improve this at alsa-devel a while back: https://mailman.alsa-project.org/pipermail/alsa-devel/2021-September/190099… So, future improvements on both in i915 and the audio drivers could be made. E.g. with discrete GPUs, it's the only codec of the card, so it seems feasible to detach the ALSA card if i915 is bound (using infra made for VGA switcheroo), but, until these improvements are done and land in upstream, audio drivers needs to be unbound if i915 driver goes unbind. Yet, even if such fixes got merged, this series is still needed, as it makes such dependencies more explicit and easier to debug. PS.: This series was generated against next-20220428. --- v3: minor fixes: - fixed a checkpatch warning; - use a single line for the new function prototype. v2: - the dependencies are now handled directly at try_module_get(). Mauro Carvalho Chehab (2): module: update dependencies at try_module_get() ALSA: hda - identify when audio is provided by a video driver include/linux/module.h | 4 +++- kernel/module/main.c | 33 +++++++++++++++++++++++++++++++-- sound/hda/hdac_component.c | 2 +- 3 files changed, 35 insertions(+), 4 deletions(-) -- 2.35.1

3 5

[PATCH v4 0/2] Let userspace know when snd-hda-intel needs i915
by Mauro Carvalho Chehab 30 Apr '22

30 Apr '22

Currently, kernel/module annotates module dependencies when request_symbol is used, but it doesn't cover more complex inter-driver dependencies that are subsystem and/or driver-specific. In the case of hdmi sound, depending on the CPU/GPU, sometimes the snd_hda_driver can talk directly with the hardware, but sometimes, it uses the i915 driver. When the snd_hda_driver uses i915, it should first be unbind/rmmod, as otherwise trying to unbind/rmmod the i915 driver cause driver issues, as as reported by CI tools with different GPU models: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6415/fi-tgl-1115g4/igt@core_ho… https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11495/bat-adlm-1/igt@i915_m… In the past, just a few CPUs were doing such bindings, but this issue now applies to all "modern" Intel CPUs that have onboard graphics, as well as to the newer discrete GPUs. With the discrete GPU case, the HDA controller is physically separate and requires i915 to power on the hardware for all hardware access. In this case, the issue is hit basicly 100% of the time. With on-board graphics, i915 driver is needed only when the display codec is accessed. If i915 is unbind during runtime suspend, while snd-hda-intel is still bound, nothing bad happens, but unbinding i915 on other situations may also cause issues. So, add support at kernel/modules to allow snd-hda drivers to properly annotate when a dependency on a DRM driver dependencies exists, and add a call to such new function at the snd-hda driver when it successfully binds into the DRM driver. This would allow userspace tools to check and properly remove the audio driver before trying to remove or unbind the GPU driver. It should be noticed that this series conveys the hidden module dependencies. Other changes are needed in order to allow removing or unbinding the i915 driver while keeping the snd-hda-intel driver loaded/bound. With that regards, there are some discussions on how to improve this at alsa-devel a while back: https://mailman.alsa-project.org/pipermail/alsa-devel/2021-September/190099… So, future improvements on both in i915 and the audio drivers could be made. E.g. with discrete GPUs, it's the only codec of the card, so it seems feasible to detach the ALSA card if i915 is bound (using infra made for VGA switcheroo), but, until these improvements are done and land in upstream, audio drivers needs to be unbound if i915 driver goes unbind. Yet, even if such fixes got merged, this series is still needed, as it makes such dependencies more explicit and easier to debug. PS.: This series was generated against next-20220428. --- v4: - fix a compilation warning reported by Intel's Kernel robot when !CONFIG_MODULE_UNLOAD or !CONFIG_MODULE. v3: minor fixes: - fixed a checkpatch warning; - use a single line for the new function prototype. v2: - the dependencies are now handled directly at try_module_get(). Mauro Carvalho Chehab (2): module: update dependencies at try_module_get() ALSA: hda - identify when audio is provided by a video driver include/linux/module.h | 8 +++++--- kernel/module/main.c | 33 +++++++++++++++++++++++++++++++-- sound/hda/hdac_component.c | 2 +- 3 files changed, 37 insertions(+), 6 deletions(-) -- 2.35.1

1 2