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@[email protected]
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11495/bat-adlm-1/igt@[email protected]
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.html
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
On some devices, the hda driver needs to hook into a video driver,
in order to be able to properly access the audio hardware and/or
the power management function.
That's the case of several snd_hda_intel devices that depends on
i915 driver.
Ensure that a proper reference between the snd-hda driver needing
such binding is shown at /proc/modules, in order to allow userspace
to know about such binding.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
See [PATCH v3 0/2] at: https://lore.kernel.org/all/[email protected]/
sound/hda/hdac_component.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/hda/hdac_component.c b/sound/hda/hdac_component.c
index bb37e7e0bd79..30e130457272 100644
--- a/sound/hda/hdac_component.c
+++ b/sound/hda/hdac_component.c
@@ -199,7 +199,7 @@ static int hdac_component_master_bind(struct device *dev)
}
/* pin the module to avoid dynamic unbinding, but only if given */
- if (!try_module_get(acomp->ops->owner)) {
+ if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
ret = -ENODEV;
goto out_unbind;
}
--
2.35.1
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on linus/master v5.18-rc4 next-20220429]
[cannot apply to tiwai-sound/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next
config: riscv-buildonly-randconfig-r003-20220428 (https://download.01.org/0day-ci/archive/20220501/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 400775649969b9baf3bc2a510266e7912bb16ae9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
git checkout 32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/hda/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> sound/hda/hdac_component.c:202:7: error: call to undeclared function '__try_module_get'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
^
sound/hda/hdac_component.c:202:7: note: did you mean 'try_module_get'?
include/linux/module.h:759:20: note: 'try_module_get' declared here
static inline bool try_module_get(struct module *module)
^
1 error generated.
vim +/__try_module_get +202 sound/hda/hdac_component.c
183
184 static int hdac_component_master_bind(struct device *dev)
185 {
186 struct drm_audio_component *acomp = hdac_get_acomp(dev);
187 int ret;
188
189 if (WARN_ON(!acomp))
190 return -EINVAL;
191
192 ret = component_bind_all(dev, acomp);
193 if (ret < 0)
194 return ret;
195
196 if (WARN_ON(!(acomp->dev && acomp->ops))) {
197 ret = -EINVAL;
198 goto out_unbind;
199 }
200
201 /* pin the module to avoid dynamic unbinding, but only if given */
> 202 if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
203 ret = -ENODEV;
204 goto out_unbind;
205 }
206
207 if (acomp->audio_ops && acomp->audio_ops->master_bind) {
208 ret = acomp->audio_ops->master_bind(dev, acomp);
209 if (ret < 0)
210 goto module_put;
211 }
212
213 complete_all(&acomp->master_bind_complete);
214 return 0;
215
216 module_put:
217 module_put(acomp->ops->owner);
218 out_unbind:
219 component_unbind_all(dev, acomp);
220 complete_all(&acomp->master_bind_complete);
221
222 return ret;
223 }
224
--
0-DAY CI Kernel Test Service
https://01.org/lkp