[igt-dev] [PATCH i-g-t v5 0/9] Add support to collecr code coverage data

[igt-dev] [PATCH i-g-t v5 0/9] Add support to collecr code coverage data

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

There are several IGT tests that require a DRM driver to unbind or unload.

However, depending on the hardware model, the audio driver (snd_hda_intel)
hooks into the DRM driver in order to work, and the audio PCI drivers and
ALSA core currently doesn't support unbind drivers that provide them access
to the hardware, without first unbinding the audio driver.

The real fix would be to improve the audio driver's bind/unbind logic, but
this will very likely require changes at the subsystem as well. There were
some discussions in the past at the ALSA subsystem:

	https://mailman.alsa-project.org/pipermail/alsa-devel/2021-September/190279.html

But this was not addressed yet.

It should be noticed that there are some daemons like alsactl, pulseaudio and
pipewire-pulse that constantly accesses the audio hardware. So, the only way to
unbind the audio driver is to first make such apps to stop using the audio
devices.

On other words, if, for instance, snd-hda-intel is bound into i915 driver,
those are the needed steps to unbind i915 driver:

- request pulseaudio or pipewire-pulse to stop using the audio devices;
- kill all other apps that are using /dev/snd devnodes;
- unbind/unload snd-hda-intel;
- unbind or unload i915 driver.

The patches on this series warrant that the above steps will be observed,
and unifies the logic which currently has two implementations: one at IGT
library and another one at core_hotunplug.

In the specific case of snd-hda and i915 driver, the actual binding between
them depends on the actual hardware architecture. In the past, only a few
CPU/GPU models would do such binding. As documented at sound/pci/hda/hda_intel,
it used to be:

	- Haswell;
	- Broadwell;
	- Baytrail;
	- Braswell.

But nowadays, on all devices with discrete graphics GPU, the snd_hda
driver needs to hook into the i915 driver, mostly for power management.
Also, newer integrated graphics also need to bind into i915, in order
to talk with the hardware registers.

As the binding between snd_hda and i915 depends on hardware model and
changes over time, the safest way is to know for sure when such binding
exists would be if the userspace tools would report, e. g.:

	$ lsmod|grep i915.*snd
	i915                 8167424  3 snd_hda_intel

	$ cat /proc/modules |grep i915.*snd
	i915 8167424 3 snd_hda_intel, Live 0x0000000000000000

With such knowledge, userspace can better decide if how to unbind the
DRM driver. The Kernel patches adding support for such feature can be
seen at:

	https://lore.kernel.org/intel-gfx/cover.1651348913.git.mchehab@kernel.org/

This series assumes that the above patch series (or a similar one) will
be there on Kernel 5.20 (this could be changed in the future if the patch
series take a longer time to be merged).

So, if Kernel is 5.20 and /proc/modules doesn't show any dependencies
between a snd driver and the DRM driver, IGT will simply unbind the DRM
driver directly, completely ignoring the audio drivers. If the dependency
exists, the logic shoud work for any audio and DRM driver - and even allow
recursive dependencies. So, it should work even for SOC/SOF audio drivers.

If Kernel is older, it will try to unload the audio first, but then the
logic is dependent on the drivers names. This series preserve the existing
dependency chain. e. g. it expects either snd_hda_intel or snd_hdmi_lpe_audio
driver, and assumes that those are likely bind to i915 driver.
Howver, as IGT doesn't know for sure if the such binding exists, it won't
fail the IGT test if the audio unload fails, as there's a chance that
the i915 unbind would work even when this fails.

---

v5:
 - Added an ancillary function to reallocate memory to allow
   calling strdup() multiple times without leaking data.
   
v4:
 - Added 3 extra patches to satisfy v3 review comments:
   - export kill_children() function;
   - don't leak "who" from module unload routines;
   - get rid of passing pipewire-pulse pid on functions.

v3:
 - patch 6 was improved to better handle pipewire-pulse case.
   Also added on its log the tests made on a RKL-S machine with
   Fedora 35 and wireplumber installed.
 - Minor changes on patches, addressing Andi's review from v2.

v2:
  - rebased on the top of today's origin/master, to make CI happy.
    no functional changes.

Mauro Carvalho Chehab (9):
  tests/core_hotunplug: properly finish processes using audio devices
  lib/igt_kmod: always fill who when unloading audio driver
  lib/igt_kmod: improve audio unbind logic
  lib/igt_kmod: don't leak who from module unload routines
  core_hotunplug: fix audio unbind logic
  lib/igt_kmod: make it less pedantic with audio driver removal
  lib/igt_core: export kill_children() function
  lib/igt_kmod: properly handle pipewire-pulse
  lib/igt_aux: get rid of passing pipewire-pulse pid on functions

 lib/igt_aux.c          | 288 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h          |   3 +
 lib/igt_core.c         |  12 +-
 lib/igt_core.h         |   1 +
 lib/igt_kmod.c         | 295 ++++++++++++++++++++++++++++++++++++++---
 lib/igt_kmod.h         |   4 +-
 tests/core_hotunplug.c |  55 +++-----
 tests/i915/perf_pmu.c  |   3 +-
 8 files changed, 595 insertions(+), 66 deletions(-)

-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 1/9] tests/core_hotunplug: properly finish processes using audio devices

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Before unloading or unbinding an audio driver, all processes
that are using it must be terminated. The current logic seeks
only for alsactl, but ignore other processes, including
pulseaudio.

Make the logic more general, extending it to any processes that
could have an open device under /dev/snd.

It should be noticed that some distros like Fedora and openSUSE
are now migrating from pulseaudio into pipewire-pulse. Right
now, there's no standard distribution-agnostic way to request
pipewire-pulse to stop using audio devices, but there's a new
patch upstream that will make things easier:

	https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/6ad6300ec657c88322a8cd6f3548261d3dc05359

Which should be available for pipewire-pulse versions 0.3.50 and
upper.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_aux.c          | 157 +++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h          |   1 +
 lib/igt_kmod.c         |  53 ++++++++------
 lib/igt_kmod.h         |   2 +
 tests/core_hotunplug.c |  11 +--
 5 files changed, 196 insertions(+), 28 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 03cc38c93e5c..f56b5a06f100 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -31,6 +31,7 @@
 #endif
 #include <stdio.h>
 #include <fcntl.h>
+#include <pwd.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <string.h>
@@ -1410,6 +1411,162 @@ igt_lsof(const char *dpath)
 	free(sanitized);
 }
 
+static void pulseaudio_unload_module(proc_t *proc_info)
+{
+	char xdg_dir[PATH_MAX];
+	const char *homedir;
+	struct passwd *pw;
+
+	igt_fork(child, 1) {
+		pw = getpwuid(proc_info->euid);
+		homedir = pw->pw_dir;
+		snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
+
+		igt_info("Ask pulseaudio to stop using audio device\n");
+
+		setgid(proc_info->egid);
+		setuid(proc_info->euid);
+		clearenv();
+		setenv("HOME", homedir, 1);
+		setenv("XDG_RUNTIME_DIR",xdg_dir, 1);
+
+		system("for i in $(pacmd list-sources|grep module:|cut -d : -f 2); do pactl unload-module $i; done");
+	}
+	igt_waitchildren();
+}
+
+/**
+ * __igt_lsof_audio_and_kill_proc() - check if a given process is using an
+ *	audio device. If so, stop or prevent them to use such devices.
+ *
+ * @proc_info: process struct, as returned by readproc()
+ * @proc_path: path of the process under procfs
+ *
+ * No processes can be using an audio device by the time it gets removed.
+ * This function checks if a process is using an audio device from /dev/snd.
+ * If so, it will check:
+ * 	- if the process is pulseaudio, it can't be killed, as systemd will
+ * 	  respawn it. So, instead, send a request for it to stop bind the
+ * 	  audio devices.
+ * If the check fails, it means that the process can simply be killed.
+ */
+static int
+__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
+{
+	const char *audio_dev = "/dev/snd/";
+	char path[PATH_MAX * 2];
+	struct dirent *d;
+	struct stat st;
+	char *fd_lnk;
+	int fail = 0;
+	ssize_t read;
+
+	DIR *dp = opendir(proc_path);
+	igt_assert(dp);
+
+	while ((d = readdir(dp))) {
+		if (*d->d_name == '.')
+			continue;
+
+		memset(path, 0, sizeof(path));
+		snprintf(path, sizeof(path), "%s/%s", proc_path, d->d_name);
+
+		if (lstat(path, &st) == -1)
+			continue;
+
+		fd_lnk = malloc(st.st_size + 1);
+
+		igt_assert((read = readlink(path, fd_lnk, st.st_size + 1)));
+		fd_lnk[read] = '\0';
+
+		if (strncmp(audio_dev, fd_lnk, strlen(audio_dev))) {
+			free(fd_lnk);
+			continue;
+		}
+
+		free(fd_lnk);
+
+		/*
+		 * In order to avoid racing against pa/systemd, ensure that
+		 * pulseaudio will close all audio files. This should be
+		 * enough to unbind audio modules and won't cause race issues
+		 * with systemd trying to reload it.
+		 */
+		if (!strcmp(proc_info->cmd, "pulseaudio")) {
+			pulseaudio_unload_module(proc_info);
+			break;
+		}
+
+		/*
+		 * FIXME: terminating pipewire-pulse is not that easy, as
+		 * pipewire there's no standard way up to pipewire version
+		 * 0.3.49. Just trying to kill pipewire will start a race
+		 * between IGT and systemd. If IGT wins, the audio driver
+		 * will be unloaded before systemd tries to reload it, but
+		 * if systemd wins, the audio device will be re-opened and
+		 * used before IGT has a chance to remove the audio driver.
+		 * Pipewire version 0.3.50 should bring a standard way:
+		 *
+		 * 1) start a thread running:
+		 *	 pw-reserve -n Audio0 -r
+		 * 2) unload/unbind the the audio driver(s);
+		 * 3) stop the pw-reserve thread.
+		 *
+		 * We should add support for it once distros start shipping it.
+		 */
+
+		/* For all other processes, just kill them */
+		igt_info("process %d (%s) is using audio device. Should be terminated.\n",
+				proc_info->tid, proc_info->cmd);
+
+		if (kill(proc_info->tid, SIGTERM) < 0) {
+			igt_info("Fail to terminate %s (pid: %d) with SIGTERM\n",
+				proc_info->cmd, proc_info->tid);
+			if (kill(proc_info->tid, SIGABRT) < 0) {
+				fail++;
+				igt_info("Fail to terminate %s (pid: %d) with SIGABRT\n",
+					proc_info->cmd, proc_info->tid);
+			}
+		}
+
+		break;
+	}
+
+	closedir(dp);
+	return fail;
+}
+
+/*
+ * This function identifies each process running on the machine that is
+ * opening an audio device and tries to stop it.
+ *
+ * Special care should be taken with pipewire and pipewire-pulse, as those
+ * daemons are respanned if they got killed.
+ */
+int
+igt_lsof_kill_audio_processes(void)
+{
+	char path[PATH_MAX];
+	proc_t *proc_info;
+	PROCTAB *proc;
+	int fail = 0;
+
+	proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
+	igt_assert(proc != NULL);
+
+	while ((proc_info = readproc(proc, NULL))) {
+		if (snprintf(path, sizeof(path), "/proc/%d/fd", proc_info->tid) < 1)
+			fail++;
+		else
+			fail += __igt_lsof_audio_and_kill_proc(proc_info, path);
+
+		freeproc(proc_info);
+	}
+	closeproc(proc);
+
+	return fail;
+}
+
 static struct igt_siglatency {
 	timer_t timer;
 	struct timespec target;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 9f2588aeca90..bb96d1afb777 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -306,6 +306,7 @@ bool igt_allow_unlimited_files(void);
 int igt_is_process_running(const char *comm);
 int igt_terminate_process(int sig, const char *comm);
 void igt_lsof(const char *dpath);
+int igt_lsof_kill_audio_processes(void);
 
 #define igt_hweight(x) \
 	__builtin_choose_expr(sizeof(x) == 8, \
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index f252535d5a3a..133d19048a9b 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -389,7 +389,7 @@ igt_i915_driver_load(const char *opts)
 	return 0;
 }
 
-int __igt_i915_driver_unload(const char **who)
+int igt_audio_driver_unload(const char **who)
 {
 	int ret;
 	const char *sound[] = {
@@ -398,6 +398,27 @@ int __igt_i915_driver_unload(const char **who)
 		NULL,
 	};
 
+	for (const char **m = sound; *m; m++) {
+		if (igt_kmod_is_loaded(*m)) {
+			if (igt_lsof_kill_audio_processes())
+				return EACCES;
+
+			kick_snd_hda_intel();
+			ret = igt_kmod_unload(*m, 0);
+			if (ret) {
+				if (who)
+					*who = *m;
+				return ret;
+			}
+		}
+	}
+	return 0;
+}
+
+int __igt_i915_driver_unload(const char **who)
+{
+	int ret;
+
 	const char *aux[] = {
 		/* gen5: ips uses symbol_get() so only a soft module dependency */
 		"intel_ips",
@@ -411,27 +432,19 @@ int __igt_i915_driver_unload(const char **who)
 	/* unbind vt */
 	bind_fbcon(false);
 
-	for (const char **m = sound; *m; m++) {
-		if (igt_kmod_is_loaded(*m)) {
-			igt_terminate_process(SIGTERM, "alsactl");
-			kick_snd_hda_intel();
-			ret = igt_kmod_unload(*m, 0);
-			if (ret) {
-				if (who)
-					*who = *m;
-				return ret;
-			}
-		}
-	}
+	ret = igt_audio_driver_unload(who);
+	if (ret)
+		return ret;
 
 	for (const char **m = aux; *m; m++) {
-		if (igt_kmod_is_loaded(*m)) {
-			ret = igt_kmod_unload(*m, 0);
-			if (ret) {
-				if (who)
-					*who = *m;
-				return ret;
-			}
+		if (!igt_kmod_is_loaded(*m))
+			continue;
+
+		ret = igt_kmod_unload(*m, 0);
+		if (ret) {
+			if (who)
+				*who = *m;
+			return ret;
 		}
 	}
 
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index 0898122b3efe..15f0be31e8e4 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -36,6 +36,8 @@ bool igt_kmod_has_param(const char *mod_name, const char *param);
 int igt_kmod_load(const char *mod_name, const char *opts);
 int igt_kmod_unload(const char *mod_name, unsigned int flags);
 
+int igt_audio_driver_unload(const char **whom);
+
 int igt_i915_driver_load(const char *opts);
 int igt_i915_driver_unload(void);
 int __igt_i915_driver_unload(const char **whom);
diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 02eae19e1e16..18e37ec8feba 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -152,19 +152,14 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix,
 	 * safest and easiest way out.
 	 */
 	if (priv->snd_unload) {
-		igt_terminate_process(SIGTERM, "alsactl");
-
-		/* unbind snd_hda_intel */
-		kick_snd_hda_intel();
-
-		if (igt_kmod_unload("snd_hda_intel", 0)) {
+		if (igt_audio_driver_unload(NULL)) {
 			priv->snd_unload = false;
-			igt_warn("Could not unload snd_hda_intel\n");
+			igt_warn("Could not unload audio driver\n");
 			igt_kmod_list_loaded();
 			igt_lsof("/dev/snd");
 			igt_skip("Audio is in use, skipping\n");
 		} else {
-			igt_info("Preventively unloaded snd_hda_intel\n");
+			igt_info("Preventively unloaded audio driver\n");
 		}
 	}
 
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 2/9] lib/igt_kmod: always fill who when unloading audio driver

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

As we'll use this information at core_hotunplug to announce
when an audio module is unloaded, fill it even if return code
is zero.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_aux.c  | 2 +-
 lib/igt_kmod.c | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f56b5a06f100..0d90ebb5b6ec 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -1422,7 +1422,7 @@ static void pulseaudio_unload_module(proc_t *proc_info)
 		homedir = pw->pw_dir;
 		snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
 
-		igt_info("Ask pulseaudio to stop using audio device\n");
+		igt_info("Request pulseaudio to stop using audio device\n");
 
 		setgid(proc_info->egid);
 		setuid(proc_info->euid);
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 133d19048a9b..d282be70f102 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -400,16 +400,15 @@ int igt_audio_driver_unload(const char **who)
 
 	for (const char **m = sound; *m; m++) {
 		if (igt_kmod_is_loaded(*m)) {
+			if (who)
+				*who = *m;
 			if (igt_lsof_kill_audio_processes())
 				return EACCES;
 
 			kick_snd_hda_intel();
 			ret = igt_kmod_unload(*m, 0);
-			if (ret) {
-				if (who)
-					*who = *m;
+			if (ret)
 				return ret;
-			}
 		}
 	}
 	return 0;
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 3/9] lib/igt_kmod: improve audio unbind logic

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The dependencies between audio and DRM drivers are not trivial.

On several CPUs, the logic inside snd-hda-intel, for instance, tries to
hook into i915 driver, via drm_audio_component logic. That also happens
when there's no runtime PM.

When the audio driver is bound into i915, removing or unbinding i915
without first removing the audio driver produce Kernel errors.

So, the audio driver(s) should be removed first, and this can only
happen after pulseaudio, pipewire-pulse, audioctl and any other
userspace program stops using it.

This is more prune to failures. So, the best is to only try to stop
the audio driver when it is known to have dependencies on the video
driver.

Before an upcoming Kernel patch, there's no way to detect if the
audio driver required a DRM one. So, the safest way is to always
remove the audio drivers that are known to cause issues.

After the new Kernel, the logic can be more selective, only removing
the audio driver if /proc/modules shows dependencies with the DRM
driver.

Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 191 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index d282be70f102..ca74c602ce66 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -24,6 +24,7 @@
 #include <ctype.h>
 #include <signal.h>
 #include <errno.h>
+#include <sys/utsname.h>
 
 #include "igt_aux.h"
 #include "igt_core.h"
@@ -389,7 +390,7 @@ igt_i915_driver_load(const char *opts)
 	return 0;
 }
 
-int igt_audio_driver_unload(const char **who)
+static int igt_always_unload_audio_driver(const char **who)
 {
 	int ret;
 	const char *sound[] = {
@@ -414,6 +415,195 @@ int igt_audio_driver_unload(const char **who)
 	return 0;
 }
 
+struct module_ref {
+	char *name;
+	unsigned long mem;
+	unsigned int ref_count;
+	unsigned int num_required;
+	unsigned int *required_by;
+};
+
+static struct module_ref *read_module_dependencies(unsigned int *num_mod)
+{
+	FILE *fp = fopen("/proc/modules", "r");
+	struct module_ref *mod = NULL;
+	size_t line_buf_size = 0;
+	char *required_by, *p;
+	unsigned n_mod = 0;
+	unsigned num_req;
+	char *line = NULL;
+	int len = 0;
+	int i, ret;
+
+	igt_assert(fp);
+
+	while ((len = getline(&line, &line_buf_size, fp)) > 0) {
+		mod = realloc(mod, (n_mod + 1) * sizeof(*mod));
+		igt_assert(mod);
+		mod[n_mod].required_by = NULL;
+
+		p = strtok(line, " ");
+		mod[n_mod].name = strdup(p);
+
+		p = strtok(NULL, " ");
+		ret = sscanf(p, "%lu", &mod[n_mod].mem);
+		igt_assert(ret == 1);
+
+		p = strtok(NULL, " ");
+		ret = sscanf(p, "%u", &mod[n_mod].ref_count);
+		igt_assert(ret == 1);
+
+		num_req = 0;
+		required_by = strtok(NULL, " ");
+		if (strcmp(required_by, "-")) {
+			p = strtok(required_by, ",");
+			while (p) {
+				for (i = 0; i < n_mod; i++)
+					if (!strcmp(p, mod[i].name))
+						break;
+
+				igt_assert(i < n_mod);
+
+				mod[n_mod].required_by = realloc(mod[n_mod].required_by,
+								 (num_req + 1) * sizeof(unsigned int));
+				mod[n_mod].required_by[num_req] = i;
+				num_req++;
+				p = strtok(NULL, ",");
+			}
+		}
+		mod[n_mod].num_required = num_req;
+		n_mod++;
+	}
+	free(line);
+	fclose(fp);
+
+	*num_mod = n_mod;
+	return mod;
+}
+
+static void free_module_ref(struct module_ref *mod, unsigned int num_mod)
+{
+	int i;
+
+	for (i = 0; i < num_mod; i++) {
+		free(mod[i].name);
+		free(mod[i].required_by);
+	}
+	free(mod);
+}
+
+static int igt_unload_driver(struct module_ref *mod, unsigned int num_mod,
+			     unsigned int pos)
+{
+	int ret, i;
+
+	/* Recursively remove depending modules, if any */
+	for (i = 0; i < mod[pos].num_required; i++) {
+		ret = igt_unload_driver(mod, num_mod,
+					mod[num_mod].required_by[i]);
+		if (ret)
+			return ret;
+	}
+
+	return igt_kmod_unload(mod[pos].name, 0);
+}
+
+#define LINUX_VERSION(major, minor, patch)			\
+		     ((major) << 16 | (minor) << 8 | (patch))
+
+static int linux_kernel_version(void)
+{
+	struct utsname utsname;
+	int ver[3] = { 0 };
+	int i = 0;
+	int n = 0;
+	char *p;
+
+	if (uname(&utsname))
+		return 0;
+
+	p = utsname.release;
+
+	while (*p && i < 3) {
+		if (isdigit(*p)) {
+			n = n * 10 + (*p - '0');
+			p++;
+			continue;
+		}
+		if (n > 255)
+			n = 255;
+		ver[i] = n;
+		i++;
+		n = 0;
+
+		if (*p != '.')
+			break;
+		p++;
+	}
+
+	return LINUX_VERSION(ver[0], ver[1], ver[2]);
+}
+
+int igt_audio_driver_unload(const char **who)
+{
+	const char *drm_driver = "i915";
+	unsigned int num_mod, i, j;
+	struct module_ref *mod;
+	int pos = -1;
+	int ret = 0;
+
+	/*
+	 * On older Kernels, there's no way to check if the audio driver
+	 * binds into the DRM one. So, always remove audio drivers that
+	 * might be binding.
+	 */
+	if (linux_kernel_version() < LINUX_VERSION(5, 20, 0))
+		return igt_always_unload_audio_driver(who);
+
+	/*
+	 * Newer Kernels gained support for showing the dependencies between
+	 * audio and DRM drivers via /proc/modules and lsmod. Use it to
+	 * detect if removing the audio driver is needed, properly unloading
+	 * any audio processes that could be using the audio driver and
+	 * handling module dependencies. Such solution should be generic
+	 * enough to work with newer SOC/SOF drivers if ever needed.
+	 */
+
+	mod = read_module_dependencies(&num_mod);
+
+	for (i = 0; i < num_mod; i++)
+		if (!strcmp(mod[i].name, drm_driver))
+			break;
+
+	if (i == num_mod) {
+		ret = 0;
+		goto ret;
+	}
+
+	/* Recursively remove all drivers that depend on drm_driver */
+	for (j = 0; j < mod[i].num_required; j++) {
+		pos = mod[i].required_by[j];
+		if (who)
+			*who = strdup(mod[pos].name);
+		/*
+		 * If a sound driver depends on drm_driver, kill audio processes
+		 * first, in order to make it possible to unload the driver
+		 */
+		if (strstr(mod[pos].name, "snd")) {
+			if (igt_lsof_kill_audio_processes()) {
+				ret = EACCES;
+				goto ret;
+			}
+		}
+		ret = igt_unload_driver(mod, num_mod, pos);
+	}
+
+ret:
+	free_module_ref(mod, num_mod);
+
+	return ret;
+}
+
 int __igt_i915_driver_unload(const char **who)
 {
 	int ret;
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 4/9] lib/igt_kmod: don't leak who from module unload routines

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Add code to free allocated strings at the module unload routines
from igt_kmod.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c        | 31 +++++++++++++++++++++++--------
 lib/igt_kmod.h        |  4 ++--
 tests/i915/perf_pmu.c |  3 ++-
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index ca74c602ce66..9bb2fc705eb8 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -363,6 +363,15 @@ igt_kmod_list_loaded(void)
 	kmod_module_unref_list(list);
 }
 
+static void *strdup_realloc(char *origptr, const char *strdata)
+{
+	size_t nbytes = strlen(strdata) + 1;
+	char *newptr = realloc(origptr, nbytes);
+
+	memcpy(newptr, strdata, nbytes);
+	return newptr;
+}
+
 /**
  * igt_i915_driver_load:
  * @opts: options to pass to i915 driver
@@ -390,7 +399,7 @@ igt_i915_driver_load(const char *opts)
 	return 0;
 }
 
-static int igt_always_unload_audio_driver(const char **who)
+static int igt_always_unload_audio_driver(char **who)
 {
 	int ret;
 	const char *sound[] = {
@@ -402,7 +411,8 @@ static int igt_always_unload_audio_driver(const char **who)
 	for (const char **m = sound; *m; m++) {
 		if (igt_kmod_is_loaded(*m)) {
 			if (who)
-				*who = *m;
+				*who = strdup_realloc(*who, *m);
+
 			if (igt_lsof_kill_audio_processes())
 				return EACCES;
 
@@ -544,7 +554,7 @@ static int linux_kernel_version(void)
 	return LINUX_VERSION(ver[0], ver[1], ver[2]);
 }
 
-int igt_audio_driver_unload(const char **who)
+int igt_audio_driver_unload(char **who)
 {
 	const char *drm_driver = "i915";
 	unsigned int num_mod, i, j;
@@ -584,7 +594,8 @@ int igt_audio_driver_unload(const char **who)
 	for (j = 0; j < mod[i].num_required; j++) {
 		pos = mod[i].required_by[j];
 		if (who)
-			*who = strdup(mod[pos].name);
+			*who = strdup_realloc(*who, mod[pos].name);
+
 		/*
 		 * If a sound driver depends on drm_driver, kill audio processes
 		 * first, in order to make it possible to unload the driver
@@ -604,7 +615,7 @@ ret:
 	return ret;
 }
 
-int __igt_i915_driver_unload(const char **who)
+int __igt_i915_driver_unload(char **who)
 {
 	int ret;
 
@@ -632,7 +643,8 @@ int __igt_i915_driver_unload(const char **who)
 		ret = igt_kmod_unload(*m, 0);
 		if (ret) {
 			if (who)
-				*who = *m;
+				*who = strdup_realloc(*who, *m);
+
 			return ret;
 		}
 	}
@@ -641,7 +653,8 @@ int __igt_i915_driver_unload(const char **who)
 		ret = igt_kmod_unload("i915", 0);
 		if (ret) {
 			if (who)
-				*who = "i915";
+				*who = strdup_realloc(*who, "i915");
+
 			return ret;
 		}
 	}
@@ -658,7 +671,7 @@ int __igt_i915_driver_unload(const char **who)
 int
 igt_i915_driver_unload(void)
 {
-	const char *who;
+	char *who = NULL;
 	int ret;
 
 	ret = __igt_i915_driver_unload(&who);
@@ -667,8 +680,10 @@ igt_i915_driver_unload(void)
 		igt_kmod_list_loaded();
 		igt_lsof("/dev/dri");
 		igt_lsof("/dev/snd");
+		free(who);
 		return ret;
 	}
+	free(who);
 
 	if (igt_kmod_is_loaded("intel-gtt"))
 		igt_kmod_unload("intel-gtt", 0);
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index 15f0be31e8e4..f98dd29fb175 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -36,11 +36,11 @@ bool igt_kmod_has_param(const char *mod_name, const char *param);
 int igt_kmod_load(const char *mod_name, const char *opts);
 int igt_kmod_unload(const char *mod_name, unsigned int flags);
 
-int igt_audio_driver_unload(const char **whom);
+int igt_audio_driver_unload(char **whom);
 
 int igt_i915_driver_load(const char *opts);
 int igt_i915_driver_unload(void);
-int __igt_i915_driver_unload(const char **whom);
+int __igt_i915_driver_unload(char **whom);
 
 int igt_amdgpu_driver_load(const char *opts);
 int igt_amdgpu_driver_unload(void);
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index c48cc07df0d2..1e385bebc77d 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -2047,7 +2047,7 @@ static void test_unload(unsigned int num_engines)
 		int fd[4 + num_engines * 3], i;
 		uint64_t *buf;
 		int count = 0, ret;
-		const char *who;
+		char *who = NULL;
 		int i915;
 
 		i915 = __drm_open_driver(DRIVER_INTEL);
@@ -2104,6 +2104,7 @@ static void test_unload(unsigned int num_engines)
 		pmu_read_multi(fd[0], count, buf);
 		ret = __igt_i915_driver_unload(&who);
 		igt_assert(ret != 0 && !strcmp(who, "i915"));
+		free(who);
 		pmu_read_multi(fd[0], count, buf);
 
 		igt_debug("Close perf\n");
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 5/9] core_hotunplug: fix audio unbind logic

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The current audio unbind logic is wrong: it expects the audio
driver to depend on i915 only on Haswell, Broadwell and DG1.

That doesn't match the Kernel driver, where snd-hda-audio binds
on i915 also on Skylake, Braswell and whenever it needs to use
pm runtime from the DRM driver.

Now that lib/igt-kmod has gained improved support for audio
unbind, update core_hotunplug to benefit from the newer logic.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 tests/core_hotunplug.c | 53 ++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 18e37ec8feba..7c68b336e67b 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -54,7 +54,7 @@ struct hotunplug {
 	const char *failure;
 	bool need_healthcheck;
 	bool has_intel_perf;
-	bool snd_unload;
+ 	const char *snd_driver;
 };
 
 /* Helpers */
@@ -140,27 +140,22 @@ static void prepare(struct hotunplug *priv)
 static void driver_unbind(struct hotunplug *priv, const char *prefix,
 			  int timeout)
 {
-	/**
-	 * FIXME: Unbinding the i915 driver on affected platforms with
-	 * audio results in a kernel WARN on "i915 raw-wakerefs=1
-	 * wakelocks=1 on cleanup". The below CI friendly user level
-	 * workaround to unload and de-couple audio from IGT testing,
-	 * prevents the warning from appearing. Drop this hack as soon
-	 * as this is fixed in the kernel. unbind/re-bind validation
-	 * on audio side is not robust and we could have potential
-	 * failures blocking display CI, currently this seems to the
-	 * safest and easiest way out.
+	/*
+	 * 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 (priv->snd_unload) {
-		if (igt_audio_driver_unload(NULL)) {
-			priv->snd_unload = false;
-			igt_warn("Could not unload audio driver\n");
-			igt_kmod_list_loaded();
-			igt_lsof("/dev/snd");
-			igt_skip("Audio is in use, skipping\n");
-		} else {
-			igt_info("Preventively unloaded audio driver\n");
-		}
+	if (igt_audio_driver_unload(&priv->snd_driver)) {
+		if (priv->snd_driver)
+			igt_warn("Could not unload audio driver %s\n", priv->snd_driver);
+		igt_kmod_list_loaded();
+		igt_lsof("/dev/snd");
+		igt_skip("Audio is in use, skipping\n");
+	} 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);
@@ -192,8 +187,11 @@ static void driver_bind(struct hotunplug *priv, int timeout)
 				F_OK, 0),
 		      "Rebound device not present (%s)!\n", priv->dev_bus_addr);
 
-	if (priv->snd_unload)
-		igt_kmod_load("snd_hda_intel", NULL);
+	if (priv->snd_driver) {
+		igt_info("Realoading %s\n", priv->snd_driver);
+		igt_kmod_load(priv->snd_driver, NULL);
+		priv->snd_driver = NULL;
+	}
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -606,7 +604,7 @@ igt_main
 		.failure	= NULL,
 		.need_healthcheck = true,
 		.has_intel_perf = false,
-		.snd_unload	= false,
+		.snd_driver	= NULL,
 	};
 
 	igt_fixture {
@@ -616,13 +614,6 @@ igt_main
 		igt_skip_on_f(fd_drm < 0, "No known DRM device found\n");
 
 		if (is_i915_device(fd_drm)) {
-			/*
-			 * Audio driver does not support hot unplug via DRM
-			 * audio component framework, so driver must be unloaded
-			 * explicitly for i915 unbind tests.
-			 */
-			priv.snd_unload = true;
-
 			gem_quiescent_gpu(fd_drm);
 			igt_require_gem(fd_drm);
 
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 6/9] lib/igt_kmod: make it less pedantic with audio driver removal

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Current Linux Kernel don't report if the audio driver binds
into the DRM driver. As this is CPU specific, allow audio
driver unload fail without skipping the IGT tests on legacy
Kernels, as this may not be mandatory.

On new kernels where lsmod will properly display the dependency
between the audio and DRM drivers, skip the core hotunplug
test if it fails to unload the audio driver, as this is
unrelated to the DRM driver - and it could simply because there
are some userspace code using the audio device while the IGT
test is running.

Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c         | 31 +++++++++++++++++++++++++++----
 tests/core_hotunplug.c |  7 ++-----
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 9bb2fc705eb8..5d358c899cfb 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -408,18 +408,34 @@ static int igt_always_unload_audio_driver(char **who)
 		NULL,
 	};
 
+	/*
+	 * With old Kernels, the dependencies between audio and DRM drivers
+	 * are not shown. So, it may not be mandatory to remove the audio
+	 * driver before unload/unbind the DRM one. So, let's print warnings,
+	 * but return 0 on errors, as, if the dependency is mandatory, this
+	 * will be detected later when trying to unbind/unload the DRM driver.
+	 */
 	for (const char **m = sound; *m; m++) {
 		if (igt_kmod_is_loaded(*m)) {
 			if (who)
 				*who = strdup_realloc(*who, *m);
 
-			if (igt_lsof_kill_audio_processes())
-				return EACCES;
+			ret = igt_lsof_kill_audio_processes();
+			if (ret) {
+				igt_warn("Could not stop %d audio process(es)\n", ret);
+				igt_kmod_list_loaded();
+				igt_lsof("/dev/snd");
+				return 0;
+			}
 
 			kick_snd_hda_intel();
 			ret = igt_kmod_unload(*m, 0);
-			if (ret)
-				return ret;
+			if (ret) {
+				igt_warn("Could not unload audio driver %s\n", *m);
+				igt_kmod_list_loaded();
+				igt_lsof("/dev/snd");
+				return 0;
+			}
 		}
 	}
 	return 0;
@@ -610,6 +626,13 @@ int igt_audio_driver_unload(char **who)
 	}
 
 ret:
+	if (ret) {
+		igt_warn("Couldn't unload %s, which is using the %s driver\n",
+			 mod[pos].name, drm_driver);
+		igt_kmod_list_loaded();
+		igt_lsof("/dev/snd");
+	}
+
 	free_module_ref(mod, num_mod);
 
 	return ret;
diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7c68b336e67b..3888c660d4ac 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -149,11 +149,8 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix,
 	 * unbind i915 driver, reloading it when binding again.
 	 */
 	if (igt_audio_driver_unload(&priv->snd_driver)) {
-		if (priv->snd_driver)
-			igt_warn("Could not unload audio driver %s\n", priv->snd_driver);
-		igt_kmod_list_loaded();
-		igt_lsof("/dev/snd");
-		igt_skip("Audio is in use, skipping\n");
+		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);
 	}
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 7/9] lib/igt_core: export kill_children() function

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

This function is needed outside igt_core. So, make it exportable,
and allow passing different signs to the children.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_core.c | 12 ++++++------
 lib/igt_core.h |  1 +
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 6dad3c84858f..b6bd67ab400f 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2028,11 +2028,11 @@ void __igt_fail_assert(const char *domain, const char *file, const int line,
 	igt_fail(IGT_EXIT_FAILURE);
 }
 
-static void kill_children(void)
+void igt_kill_children(int signal)
 {
 	for (int c = 0; c < num_test_children; c++) {
 		if (test_children[c] > 0)
-			kill(test_children[c], SIGKILL);
+			kill(test_children[c], signal);
 	}
 }
 
@@ -2060,7 +2060,7 @@ void __igt_abort(const char *domain, const char *file, const int line,
 	}
 
 	/* just try our best, we are aborting the execution anyway */
-	kill_children();
+	igt_kill_children(SIGKILL);
 
 	print_backtrace();
 
@@ -2124,7 +2124,7 @@ void igt_exit(void)
 			 command_str, igt_exitcode);
 	igt_debug("Exiting with status code %d\n", igt_exitcode);
 
-	kill_children();
+	igt_kill_children(SIGKILL);
 	assert(!num_test_children);
 
 	assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
@@ -2390,7 +2390,7 @@ int __igt_waitchildren(void)
 				err = 256;
 			}
 
-			kill_children();
+			igt_kill_children(SIGKILL);
 		}
 
 		count++;
@@ -2424,7 +2424,7 @@ static void igt_alarm_killchildren(int signal)
 {
 	igt_info("Timed out waiting for children\n");
 
-	kill_children();
+	igt_kill_children(SIGKILL);
 }
 
 /**
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 78dc6202ced4..526282faff24 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -1103,6 +1103,7 @@ bool __igt_fork(void);
 int __igt_waitchildren(void);
 void igt_waitchildren(void);
 void igt_waitchildren_timeout(int seconds, const char *reason);
+void igt_kill_children(int signal);
 
 /**
  * igt_helper_process:
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 8/9] lib/igt_kmod: properly handle pipewire-pulse

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Newer distributions like Fedora and openSUSE thumbweed are now
coming with pipewire-pulse instead of pulseaudio - either as
default or as an optional audio stack.

That adds a new requirement when unloading sound drivers,
which, in turn, is needed when the DRM driver is bound into
it.

Add the needed logic to work properly in case pipewire-pulse
is detected.

Tested on ADL-N with Fedora 35 and wireplumber:

	IGT-Version: 1.26-g982672f3 (x86_64) (Linux: 5.18.0-rc7-drm-ad75b5b819c9+ x86_64)
	Starting subtest: unbind-rebind
	process 585 (alsactl) is using audio device. Should be terminated.
	process 11932 (pipewire) is using audio device. Should be terminated.
	process 11937 (pipewire-pulse) is using audio device. Should be requested to stop using them.
	Preventing pipewire-pulse to use the audio drivers
	Device Audio0 can not be acquired: Success
	reserve acquired
	Unloaded audio driver snd_hda_intel
	Realoading snd_hda_intel
	Subtest unbind-rebind: SUCCESS (2.603s)

Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_aux.c  | 175 ++++++++++++++++++++++++++++++++++++++++++-------
 lib/igt_aux.h  |   4 +-
 lib/igt_kmod.c |  17 ++++-
 3 files changed, 171 insertions(+), 25 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 0d90ebb5b6ec..ff0d948361fd 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -1435,12 +1435,120 @@ static void pulseaudio_unload_module(proc_t *proc_info)
 	igt_waitchildren();
 }
 
+static void pipewire_reserve_wait(int pipewire_pulse_pid)
+{
+	char xdg_dir[PATH_MAX];
+	const char *homedir;
+	struct passwd *pw;
+	proc_t *proc_info;
+	PROCTAB *proc;
+
+	igt_fork(child, 1) {
+		igt_info("Preventing pipewire-pulse to use the audio drivers\n");
+
+		proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
+		igt_assert(proc != NULL);
+
+		while ((proc_info = readproc(proc, NULL))) {
+			if (pipewire_pulse_pid == proc_info->tid)
+				break;
+			freeproc(proc_info);
+		}
+		closeproc(proc);
+
+		/* Sanity check: if it can't find the process, it means it has gone */
+		if (pipewire_pulse_pid != proc_info->tid)
+			exit(0);
+
+		pw = getpwuid(proc_info->euid);
+		homedir = pw->pw_dir;
+		snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
+		setgid(proc_info->egid);
+		setuid(proc_info->euid);
+		clearenv();
+		setenv("HOME", homedir, 1);
+		setenv("XDG_RUNTIME_DIR",xdg_dir, 1);
+		freeproc(proc_info);
+
+		/*
+		 * pw-reserve will run in background. It will only exit when
+		 * igt_kill_children() is called later on. So, it shouldn't
+		 * call igt_waitchildren(). Instead, just exit with the return
+		 * code from pw-reserve.
+		 */
+		exit(system("pw-reserve -n Audio0 -r"));
+	}
+}
+
+static int pipewire_pw_reserve_pid = 0;
+
+/* Maximum time waiting for pw-reserve to start running */
+#define PIPEWIRE_RESERVE_MAX_TIME 1000 /* milisseconds */
+
+int pipewire_pulse_start_reserve(int pipewire_pulse_pid)
+{
+	bool is_pw_reserve_running = false;
+	proc_t *proc_info;
+	int attempts = 0;
+	PROCTAB *proc;
+
+	if (!pipewire_pulse_pid)
+		return 0;
+
+	pipewire_reserve_wait(pipewire_pulse_pid);
+
+	/*
+	 * Note: using pw-reserve to stop using audio only works with
+	 * pipewire version 0.3.50 or upper.
+	 */
+	for (attempts = 0; attempts < PIPEWIRE_RESERVE_MAX_TIME; attempts++) {
+		usleep(1000);
+		proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
+		igt_assert(proc != NULL);
+
+		while ((proc_info = readproc(proc, NULL))) {
+			if (!strcmp(proc_info->cmd, "pw-reserve")) {
+				is_pw_reserve_running = true;
+				pipewire_pw_reserve_pid = proc_info->tid;
+				freeproc(proc_info);
+				break;
+			}
+			freeproc(proc_info);
+		}
+		closeproc(proc);
+		if (is_pw_reserve_running)
+			break;
+	}
+	if (!is_pw_reserve_running) {
+		igt_warn("Failed to remove audio drivers from pipewire\n");
+		return 1;
+	}
+	/* Let's grant some time for pw_reserve to notify pipewire via D-BUS */
+	usleep(50000);
+
+	/*
+	 * pw-reserve is running, and should have stopped using the audio
+	 * drivers. We can now remove the driver.
+	 */
+
+	return 0;
+}
+
+void pipewire_pulse_stop_reserve(int pipewire_pulse_pid)
+{
+	if (!pipewire_pulse_pid)
+		return;
+
+	igt_kill_children(SIGTERM);
+}
+
 /**
  * __igt_lsof_audio_and_kill_proc() - check if a given process is using an
  *	audio device. If so, stop or prevent them to use such devices.
  *
  * @proc_info: process struct, as returned by readproc()
  * @proc_path: path of the process under procfs
+ * @pipewire_pulse_pid: PID of pipewire-pulse process
  *
  * No processes can be using an audio device by the time it gets removed.
  * This function checks if a process is using an audio device from /dev/snd.
@@ -1448,10 +1556,15 @@ static void pulseaudio_unload_module(proc_t *proc_info)
  * 	- if the process is pulseaudio, it can't be killed, as systemd will
  * 	  respawn it. So, instead, send a request for it to stop bind the
  * 	  audio devices.
+ *	- if the process is pipewire-pulse, it can't be killed, as systemd will
+ *	  respawn it. So, instead, the caller should call pw-reserve, remove
+ *	  the kernel driver and then stop pw-reserve. On such case, this
+ *	  function returns the PID of pipewire-pulse, but won't touch it.
  * If the check fails, it means that the process can simply be killed.
  */
 static int
-__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
+__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path,
+			       int *pipewire_pulse_pid)
 {
 	const char *audio_dev = "/dev/snd/";
 	char path[PATH_MAX * 2];
@@ -1460,8 +1573,43 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
 	char *fd_lnk;
 	int fail = 0;
 	ssize_t read;
+	DIR *dp;
 
-	DIR *dp = opendir(proc_path);
+	/*
+	 * Terminating pipewire-pulse require an special procedure, which
+	 * is only available at version 0.3.50 and upper. Just trying to
+	 * kill pipewire will start a race between IGT and systemd. If IGT
+	 * wins, the audio driver will be unloaded before systemd tries to
+	 * reload it, but if systemd wins, the audio device will be re-opened
+	 * and used before IGT has a chance to remove the audio driver.
+	 * Pipewire version 0.3.50 should bring a standard way:
+	 *
+	 * 1) start a thread running:
+	 *	 pw-reserve -n Audio0 -r
+	 * 2) unload/unbind the the audio driver(s);
+	 * 3) stop the pw-reserve thread.
+	 */
+	if (!strcmp(proc_info->cmd, "pipewire-pulse")) {
+		igt_info("process %d (%s) is using audio device. Should be requested to stop using them.\n",
+			 proc_info->tid, proc_info->cmd);
+		*pipewire_pulse_pid = proc_info->tid;
+		return 0;
+	}
+	/*
+	 * pipewire-pulse itself doesn't hook into a /dev/snd device. Instead,
+	 * the actual binding happens at the Pipewire Session Manager, e.g.
+	 * either wireplumber or pipewire media-session.
+	 *
+	 * Just killing such processes won't produce any effect, as systemd
+	 * will respawn them. So, just ignore here, they'll honor pw-reserve,
+	 * when the time comes.
+	 */
+	if (!strcmp(proc_info->cmd, "pipewire-media-session"))
+		return 0;
+	if (!strcmp(proc_info->cmd, "wireplumber"))
+		return 0;
+
+	dp = opendir(proc_path);
 	igt_assert(dp);
 
 	while ((d = readdir(dp))) {
@@ -1497,24 +1645,6 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
 			break;
 		}
 
-		/*
-		 * FIXME: terminating pipewire-pulse is not that easy, as
-		 * pipewire there's no standard way up to pipewire version
-		 * 0.3.49. Just trying to kill pipewire will start a race
-		 * between IGT and systemd. If IGT wins, the audio driver
-		 * will be unloaded before systemd tries to reload it, but
-		 * if systemd wins, the audio device will be re-opened and
-		 * used before IGT has a chance to remove the audio driver.
-		 * Pipewire version 0.3.50 should bring a standard way:
-		 *
-		 * 1) start a thread running:
-		 *	 pw-reserve -n Audio0 -r
-		 * 2) unload/unbind the the audio driver(s);
-		 * 3) stop the pw-reserve thread.
-		 *
-		 * We should add support for it once distros start shipping it.
-		 */
-
 		/* For all other processes, just kill them */
 		igt_info("process %d (%s) is using audio device. Should be terminated.\n",
 				proc_info->tid, proc_info->cmd);
@@ -1544,7 +1674,7 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
  * daemons are respanned if they got killed.
  */
 int
-igt_lsof_kill_audio_processes(void)
+igt_lsof_kill_audio_processes(int *pipewire_pulse_pid)
 {
 	char path[PATH_MAX];
 	proc_t *proc_info;
@@ -1553,12 +1683,13 @@ igt_lsof_kill_audio_processes(void)
 
 	proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
 	igt_assert(proc != NULL);
+	*pipewire_pulse_pid = 0;
 
 	while ((proc_info = readproc(proc, NULL))) {
 		if (snprintf(path, sizeof(path), "/proc/%d/fd", proc_info->tid) < 1)
 			fail++;
 		else
-			fail += __igt_lsof_audio_and_kill_proc(proc_info, path);
+			fail += __igt_lsof_audio_and_kill_proc(proc_info, path, pipewire_pulse_pid);
 
 		freeproc(proc_info);
 	}
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index bb96d1afb777..d8b05088f0ba 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -306,7 +306,9 @@ bool igt_allow_unlimited_files(void);
 int igt_is_process_running(const char *comm);
 int igt_terminate_process(int sig, const char *comm);
 void igt_lsof(const char *dpath);
-int igt_lsof_kill_audio_processes(void);
+int igt_lsof_kill_audio_processes(int *pipewire_pulse_pid);
+int pipewire_pulse_start_reserve(int pipewire_pulse_pid);
+void pipewire_pulse_stop_reserve(int pipewire_pulse_pid);
 
 #define igt_hweight(x) \
 	__builtin_choose_expr(sizeof(x) == 8, \
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 5d358c899cfb..fe2b792b306f 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -401,6 +401,7 @@ igt_i915_driver_load(const char *opts)
 
 static int igt_always_unload_audio_driver(char **who)
 {
+	int pipewire_pulse_pid;
 	int ret;
 	const char *sound[] = {
 		"snd_hda_intel",
@@ -420,7 +421,7 @@ static int igt_always_unload_audio_driver(char **who)
 			if (who)
 				*who = strdup_realloc(*who, *m);
 
-			ret = igt_lsof_kill_audio_processes();
+			ret = igt_lsof_kill_audio_processes(&pipewire_pulse_pid);
 			if (ret) {
 				igt_warn("Could not stop %d audio process(es)\n", ret);
 				igt_kmod_list_loaded();
@@ -428,8 +429,12 @@ static int igt_always_unload_audio_driver(char **who)
 				return 0;
 			}
 
+			ret = pipewire_pulse_start_reserve(pipewire_pulse_pid);
+			if (ret)
+				igt_warn("Failed to notify pipewire_pulse\n");
 			kick_snd_hda_intel();
 			ret = igt_kmod_unload(*m, 0);
+			pipewire_pulse_stop_reserve(pipewire_pulse_pid);
 			if (ret) {
 				igt_warn("Could not unload audio driver %s\n", *m);
 				igt_kmod_list_loaded();
@@ -574,6 +579,7 @@ int igt_audio_driver_unload(char **who)
 {
 	const char *drm_driver = "i915";
 	unsigned int num_mod, i, j;
+	int pipewire_pulse_pid = 0;
 	struct module_ref *mod;
 	int pos = -1;
 	int ret = 0;
@@ -617,12 +623,19 @@ int igt_audio_driver_unload(char **who)
 		 * first, in order to make it possible to unload the driver
 		 */
 		if (strstr(mod[pos].name, "snd")) {
-			if (igt_lsof_kill_audio_processes()) {
+			if (igt_lsof_kill_audio_processes(&pipewire_pulse_pid)) {
 				ret = EACCES;
 				goto ret;
 			}
 		}
+
+		ret = pipewire_pulse_start_reserve(pipewire_pulse_pid);
+		if (ret)
+			igt_warn("Failed to notify pipewire_pulse\n");
 		ret = igt_unload_driver(mod, num_mod, pos);
+		pipewire_pulse_stop_reserve(pipewire_pulse_pid);
+		if (ret)
+			break;
 	}
 
 ret:
-- 
2.36.1

[igt-dev] [PATCH i-g-t v5 9/9] lib/igt_aux: get rid of passing pipewire-pulse pid on functions

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The logic already stores pw-reserve PID on a static var. Store
also the pipewire-pulse pid, in order to keep passing the arguments
on all functions.

Suggested-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_aux.c  | 24 ++++++++++++------------
 lib/igt_aux.h  |  6 +++---
 lib/igt_kmod.c | 14 ++++++--------
 3 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ff0d948361fd..755bffbc6d56 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -1435,7 +1435,10 @@ static void pulseaudio_unload_module(proc_t *proc_info)
 	igt_waitchildren();
 }
 
-static void pipewire_reserve_wait(int pipewire_pulse_pid)
+static int pipewire_pulse_pid = 0;
+static int pipewire_pw_reserve_pid = 0;
+
+static void pipewire_reserve_wait(void)
 {
 	char xdg_dir[PATH_MAX];
 	const char *homedir;
@@ -1480,12 +1483,10 @@ static void pipewire_reserve_wait(int pipewire_pulse_pid)
 	}
 }
 
-static int pipewire_pw_reserve_pid = 0;
-
 /* Maximum time waiting for pw-reserve to start running */
 #define PIPEWIRE_RESERVE_MAX_TIME 1000 /* milisseconds */
 
-int pipewire_pulse_start_reserve(int pipewire_pulse_pid)
+int pipewire_pulse_start_reserve(void)
 {
 	bool is_pw_reserve_running = false;
 	proc_t *proc_info;
@@ -1495,7 +1496,7 @@ int pipewire_pulse_start_reserve(int pipewire_pulse_pid)
 	if (!pipewire_pulse_pid)
 		return 0;
 
-	pipewire_reserve_wait(pipewire_pulse_pid);
+	pipewire_reserve_wait();
 
 	/*
 	 * Note: using pw-reserve to stop using audio only works with
@@ -1534,7 +1535,7 @@ int pipewire_pulse_start_reserve(int pipewire_pulse_pid)
 	return 0;
 }
 
-void pipewire_pulse_stop_reserve(int pipewire_pulse_pid)
+void pipewire_pulse_stop_reserve(void)
 {
 	if (!pipewire_pulse_pid)
 		return;
@@ -1563,8 +1564,7 @@ void pipewire_pulse_stop_reserve(int pipewire_pulse_pid)
  * If the check fails, it means that the process can simply be killed.
  */
 static int
-__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path,
-			       int *pipewire_pulse_pid)
+__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
 {
 	const char *audio_dev = "/dev/snd/";
 	char path[PATH_MAX * 2];
@@ -1592,7 +1592,7 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path,
 	if (!strcmp(proc_info->cmd, "pipewire-pulse")) {
 		igt_info("process %d (%s) is using audio device. Should be requested to stop using them.\n",
 			 proc_info->tid, proc_info->cmd);
-		*pipewire_pulse_pid = proc_info->tid;
+		pipewire_pulse_pid = proc_info->tid;
 		return 0;
 	}
 	/*
@@ -1674,7 +1674,7 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path,
  * daemons are respanned if they got killed.
  */
 int
-igt_lsof_kill_audio_processes(int *pipewire_pulse_pid)
+igt_lsof_kill_audio_processes(void)
 {
 	char path[PATH_MAX];
 	proc_t *proc_info;
@@ -1683,13 +1683,13 @@ igt_lsof_kill_audio_processes(int *pipewire_pulse_pid)
 
 	proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
 	igt_assert(proc != NULL);
-	*pipewire_pulse_pid = 0;
+	pipewire_pulse_pid = 0;
 
 	while ((proc_info = readproc(proc, NULL))) {
 		if (snprintf(path, sizeof(path), "/proc/%d/fd", proc_info->tid) < 1)
 			fail++;
 		else
-			fail += __igt_lsof_audio_and_kill_proc(proc_info, path, pipewire_pulse_pid);
+			fail += __igt_lsof_audio_and_kill_proc(proc_info, path);
 
 		freeproc(proc_info);
 	}
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index d8b05088f0ba..16a9785cd6df 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -306,9 +306,9 @@ bool igt_allow_unlimited_files(void);
 int igt_is_process_running(const char *comm);
 int igt_terminate_process(int sig, const char *comm);
 void igt_lsof(const char *dpath);
-int igt_lsof_kill_audio_processes(int *pipewire_pulse_pid);
-int pipewire_pulse_start_reserve(int pipewire_pulse_pid);
-void pipewire_pulse_stop_reserve(int pipewire_pulse_pid);
+int igt_lsof_kill_audio_processes(void);
+int pipewire_pulse_start_reserve(void);
+void pipewire_pulse_stop_reserve(void);
 
 #define igt_hweight(x) \
 	__builtin_choose_expr(sizeof(x) == 8, \
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index fe2b792b306f..dfdcfcc546b0 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -401,7 +401,6 @@ igt_i915_driver_load(const char *opts)
 
 static int igt_always_unload_audio_driver(char **who)
 {
-	int pipewire_pulse_pid;
 	int ret;
 	const char *sound[] = {
 		"snd_hda_intel",
@@ -421,7 +420,7 @@ static int igt_always_unload_audio_driver(char **who)
 			if (who)
 				*who = strdup_realloc(*who, *m);
 
-			ret = igt_lsof_kill_audio_processes(&pipewire_pulse_pid);
+			ret = igt_lsof_kill_audio_processes();
 			if (ret) {
 				igt_warn("Could not stop %d audio process(es)\n", ret);
 				igt_kmod_list_loaded();
@@ -429,12 +428,12 @@ static int igt_always_unload_audio_driver(char **who)
 				return 0;
 			}
 
-			ret = pipewire_pulse_start_reserve(pipewire_pulse_pid);
+			ret = pipewire_pulse_start_reserve();
 			if (ret)
 				igt_warn("Failed to notify pipewire_pulse\n");
 			kick_snd_hda_intel();
 			ret = igt_kmod_unload(*m, 0);
-			pipewire_pulse_stop_reserve(pipewire_pulse_pid);
+			pipewire_pulse_stop_reserve();
 			if (ret) {
 				igt_warn("Could not unload audio driver %s\n", *m);
 				igt_kmod_list_loaded();
@@ -579,7 +578,6 @@ int igt_audio_driver_unload(char **who)
 {
 	const char *drm_driver = "i915";
 	unsigned int num_mod, i, j;
-	int pipewire_pulse_pid = 0;
 	struct module_ref *mod;
 	int pos = -1;
 	int ret = 0;
@@ -623,17 +621,17 @@ int igt_audio_driver_unload(char **who)
 		 * first, in order to make it possible to unload the driver
 		 */
 		if (strstr(mod[pos].name, "snd")) {
-			if (igt_lsof_kill_audio_processes(&pipewire_pulse_pid)) {
+			if (igt_lsof_kill_audio_processes()) {
 				ret = EACCES;
 				goto ret;
 			}
 		}
 
-		ret = pipewire_pulse_start_reserve(pipewire_pulse_pid);
+		ret = pipewire_pulse_start_reserve();
 		if (ret)
 			igt_warn("Failed to notify pipewire_pulse\n");
 		ret = igt_unload_driver(mod, num_mod, pos);
-		pipewire_pulse_stop_reserve(pipewire_pulse_pid);
+		pipewire_pulse_stop_reserve();
 		if (ret)
 			break;
 	}
-- 
2.36.1

[igt-dev] ✗ Fi.CI.BAT: failure for Add support to collecr code coverage data (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7567 bytes --]

== Series Details ==

Series: Add support to collecr code coverage data (rev3)
URL   : https://patchwork.freedesktop.org/series/100629/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11664 -> IGTPW_7121
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_7121 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_7121, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/index.html

Participating hosts (41 -> 42)
------------------------------

  Additional (2): bat-dg2-8 bat-adlm-1 
  Missing    (1): fi-hsw-4770 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_7121:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_lrc:
    - fi-bsw-n3050:       [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html

  
Known issues
------------

  Here are the changes found in IGTPW_7121 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_auth@basic-auth:
    - fi-kbl-soraka:      [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/fi-kbl-soraka/igt@core_auth@basic-auth.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-kbl-soraka/igt@core_auth@basic-auth.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-pnv-d510:        NOTRUN -> [SKIP][5] ([fdo#109271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-pnv-d510/igt@kms_chamelium@common-hpd-after-suspend.html
    - fi-rkl-guc:         NOTRUN -> [SKIP][6] ([fdo#111827])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-rkl-guc/igt@kms_chamelium@common-hpd-after-suspend.html
    - fi-snb-2600:        NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#111827])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-snb-2600/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - bat-dg1-6:          [PASS][8] -> [SKIP][9] ([i915#4078])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/bat-dg1-6/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/bat-dg1-6/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html

  * igt@runner@aborted:
    - fi-bsw-n3050:       NOTRUN -> [FAIL][10] ([fdo#109271] / [i915#3428] / [i915#4312])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-bsw-n3050/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - fi-rkl-guc:         [INCOMPLETE][11] ([i915#4983]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [INCOMPLETE][13] ([i915#3921]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - fi-pnv-d510:        [DMESG-FAIL][15] ([i915#4528]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@workarounds:
    - {bat-dg2-9}:        [DMESG-WARN][17] ([i915#5763]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11664/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/bat-dg2-9/igt@i915_selftest@live@workarounds.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763
  [i915#5801]: https://gitlab.freedesktop.org/drm/intel/issues/5801
  [i915#5879]: https://gitlab.freedesktop.org/drm/intel/issues/5879
  [i915#5885]: https://gitlab.freedesktop.org/drm/intel/issues/5885
  [i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6476 -> IGTPW_7121

  CI-20190529: 20190529
  CI_DRM_11664: ff658f5f8e7e427556a8e6f1adc0e4519527ca95 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_7121: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/index.html
  IGT_6476: 08aa9296163b94cf4c529fc890ae3e90e21c3cdb @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7121/index.html

[-- Attachment #2: Type: text/html, Size: 6729 bytes --]

Re: [igt-dev] [PATCH i-g-t v5 4/9] lib/igt_kmod: don't leak who from module unload routines

[Andi Shyti]

Hi Mauro,

On Tue, May 17, 2022 at 02:44:42PM +0200, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Add code to free allocated strings at the module unload routines
> from igt_kmod.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [igt-dev] [PATCH i-g-t v5 3/9] lib/igt_kmod: improve audio unbind logic

[Andi Shyti]

Hi Mauro,

On Tue, May 17, 2022 at 02:44:41PM +0200, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> The dependencies between audio and DRM drivers are not trivial.
> 
> On several CPUs, the logic inside snd-hda-intel, for instance, tries to
> hook into i915 driver, via drm_audio_component logic. That also happens
> when there's no runtime PM.
> 
> When the audio driver is bound into i915, removing or unbinding i915
> without first removing the audio driver produce Kernel errors.
> 
> So, the audio driver(s) should be removed first, and this can only
> happen after pulseaudio, pipewire-pulse, audioctl and any other
> userspace program stops using it.
> 
> This is more prune to failures. So, the best is to only try to stop
> the audio driver when it is known to have dependencies on the video
> driver.
> 
> Before an upcoming Kernel patch, there's no way to detect if the
> audio driver required a DRM one. So, the safest way is to always
> remove the audio drivers that are known to cause issues.
> 
> After the new Kernel, the logic can be more selective, only removing
> the audio driver if /proc/modules shows dependencies with the DRM
> driver.
> 
> Reviewed-by Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Andi