[PATCH i-g-t 2/2] lib/igt_kmod: Do not explicitly unload intel mei driver

[Lucas De Marchi]

On Wed, Dec 18, 2024 at 03:38:30PM -0800, Daniele Ceraolo Spurio wrote:
>The mei driver is a child of i915 on discrete, but not on integrated.
>This means we want to unplug it before removing i915 in the former case
>but not in the latter. Therefore, instead of manually unloading the mei
>driver, we can unbind i915 and leave it to the kernel to unplug all the
>dependencies. This also means that we don't need to explicitly disable
>the vtcon anymore.
>
>The other 2 dependencies are left untouched:
>- intel_ips uses symbol_get, so the automatic dependency detection won't
>  unload it.

and it shouldn't - that's a hack and from the linux-modules perspective
we'd like to drop symbol_get() users. Btw, symbol_get/symbol_put only
guarantees the module can't be removed (since there's code in the kernel
that can still call into them). It doesn't mean the hardware can't go
away (hotplugged or unbound from the driver). The module should be
prepared for that. At the very least, it should handle errors from the
function and release the symbols, but it's probably a candidate for a
component driver.

>- the hotunplug test mentions that the sound driver can throw errors on
>  i915 unbind, so we need to keep the explicit unload.
>
>Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>

the bigger hammer is blocked on audio driver not getting the refcount
right: https://patchwork.freedesktop.org/series/141532/

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>


Lucas De Marchi

>---
> lib/igt_kmod.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>index a8b0339b9..fa9541d72 100644
>--- a/lib/igt_kmod.c
>+++ b/lib/igt_kmod.c
>@@ -620,16 +620,9 @@ int __igt_intel_driver_unload(char **who, const char *driver)
> 	const char *aux[] = {
> 		/* gen5: ips uses symbol_get() so only a soft module dependency */
> 		"intel_ips",
>-		/* mei_gsc uses an i915 aux dev and the other mei mods depend on it */
>-		"mei_pxp",
>-		"mei_hdcp",
>-		"mei_gsc",
> 		NULL,
> 	};
>
>-	/* unbind vt */
>-	bind_fbcon(false);
>-
> 	ret = igt_audio_driver_unload(who);
> 	if (ret)
> 		return ret;
>@@ -648,6 +641,8 @@ int __igt_intel_driver_unload(char **who, const char *driver)
> 	}
>
> 	if (igt_kmod_is_loaded(driver)) {
>+		igt_kmod_unbind(driver);
>+
> 		ret = igt_kmod_unload(driver);
> 		if (ret) {
> 			if (who)
>-- 
>2.43.0
>