Re: [PATCH i-g-t 5/9] tests/core_hotunplug: Stop unloading audio

[Matt Roper <matthew.d.roper@intel.com>]

On Mon, Nov 04, 2024 at 10:18:41PM -0800, Lucas De Marchi wrote:
> Not needed anymore and test should succeed when unbinding the device
> from the driver. On a DG2 system which has pipewire running,
> igt@core_hotunplug@unbind-rebind passes without this patch, but takes
> ~26sec and a dmesg full of warnings related to audio. After this patch
> it took 4.7sec and apparently 1 unrelated warning about
> "Unclaimed read from register 0xc4000" when rebinding.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  tests/core_hotunplug.c | 46 ------------------------------------------
>  1 file changed, 46 deletions(-)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 7f17f4423..07a8dea37 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -96,7 +96,6 @@ struct hotunplug {
>  	const char *failure;
>  	bool need_healthcheck;
>  	bool has_intel_perf;
> - 	char *snd_driver;
>  	int chipset;
>  };
>  
> @@ -187,21 +186,6 @@ static void prepare(struct hotunplug *priv)
>  static void driver_unbind(struct hotunplug *priv, const char *prefix,
>  			  int timeout)
>  {
> -	/*
> -	 * FIXME: on some devices, the audio driver (snd_hda_intel)
> -	 * binds into the i915 driver. On such hardware, kernel warnings
> -	 * and errors may happen if i915 is unbind/removed before removing
> -	 * first the audio driver.
> -	 * So, add a logic that unloads the audio driver before trying to
> -	 * unbind i915 driver, reloading it when binding again.
> -	 */
> -	if (igt_audio_driver_unload(&priv->snd_driver)) {
> -		igt_skip("Audio driver %s in use, skipping test\n",
> -			 priv->snd_driver);
> -	} else if (priv->snd_driver) {
> -		igt_info("Unloaded audio driver %s\n", priv->snd_driver);
> -	}
> -
>  	local_debug("%sunbinding the driver from the device\n", prefix);
>  	priv->failure = "Driver unbind failure!";
>  
> @@ -230,13 +214,6 @@ static void driver_bind(struct hotunplug *priv, int timeout)
>  	igt_fail_on_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr,
>  				F_OK, 0),
>  		      "Rebound device not present (%s)!\n", priv->dev_bus_addr);
> -
> -	if (priv->snd_driver) {
> -		igt_info("Realoading %s\n", priv->snd_driver);
> -		igt_kmod_load(priv->snd_driver, NULL);
> -		free(priv->snd_driver);
> -		priv->snd_driver = NULL;
> -	}
>  }
>  
>  /* Remove (virtually unplug) the device from its bus */
> @@ -245,21 +222,6 @@ static void device_unplug(struct hotunplug *priv, const char *prefix,
>  {
>  	igt_require(priv->fd.sysfs_dev == -1);
>  
> -	/*
> -	 * FIXME: on some devices, the audio driver (snd_hda_intel)
> -	 * binds into the i915 driver. On such hardware, kernel warnings
> -	 * and errors may happen if i915 is unbind/removed before removing
> -	 * first the audio driver.
> -	 * So, add a logic that unloads the audio driver before trying to
> -	 * unbind i915 driver, reloading it when binding again.
> -	 */
> -	if (igt_audio_driver_unload(&priv->snd_driver)) {
> -		igt_skip("Audio driver %s in use, skipping test\n",
> -			 priv->snd_driver);
> -	} else if (priv->snd_driver) {
> -		igt_info("Unloaded audio driver %s\n", priv->snd_driver);
> -	}
> -
>  	priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
>  				    O_DIRECTORY);
>  	igt_assert_fd(priv->fd.sysfs_dev);
> @@ -293,13 +255,6 @@ static void bus_rescan(struct hotunplug *priv, int timeout)
>  	igt_fail_on_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr,
>  				F_OK, 0),
>  		      "Fakely unplugged device not rediscovered (%s)!\n", priv->dev_bus_addr);
> -
> -	if (priv->snd_driver) {
> -		igt_info("Realoading %s\n", priv->snd_driver);
> -		igt_kmod_load(priv->snd_driver, NULL);
> -		free(priv->snd_driver);
> -		priv->snd_driver = NULL;
> -	}
>  }
>  
>  static void cleanup(struct hotunplug *priv)
> @@ -679,7 +634,6 @@ igt_main
>  		.failure	= NULL,
>  		.need_healthcheck = true,
>  		.has_intel_perf = false,
> -		.snd_driver	= NULL,
>  		.chipset	= DRIVER_ANY,
>  	};
>  
> -- 
> 2.47.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation