[igt-dev] [PATCH i-g-t 0/9] D3Cold Tool & IGT

[igt-dev] [PATCH i-g-t 0/9] D3Cold Tool & IGT

[Anshuman Gupta]

This series adds support for D3Cold Tools and IGT.

Anshuman Gupta (9):
  test/i915_pm_rpm: Add placement to gem-{mmap-type, execbuf}
  lib/igt_device: Get gfx PCI card root port
  lib/igt_pm: D3Cold runtime pm infrastructure
  lib/intel_device_info: Add IS_DGFX() support
  tools: Add intel_pm_rpm tool
  i915_pm_rpm: Add D3Cold basic subtest
  i915_pm_rpm: Test D3Cold with gem_exec_stress
  i915_pm_rpm: Extend gem_execbuf test with D3Cold
  i915_pm_rpm: Extend gem-mmap-type test with D3Cold

 lib/igt_device.c         |  28 +++++
 lib/igt_device.h         |   1 +
 lib/igt_pm.c             | 226 +++++++++++++++++++++++++++++++++++++++
 lib/igt_pm.h             |  21 ++++
 lib/intel_chipset.h      |   2 +
 lib/intel_device_info.c  |   2 +
 tests/i915/i915_pm_rpm.c | 161 ++++++++++++++++++++++++++--
 tools/intel_pm_rpm.c     | 209 ++++++++++++++++++++++++++++++++++++
 tools/meson.build        |   1 +
 9 files changed, 643 insertions(+), 8 deletions(-)
 create mode 100644 tools/intel_pm_rpm.c

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/9] test/i915_pm_rpm: Add placement to gem-{mmap-type, execbuf}

[Anshuman Gupta]

Add memory region placement to gem-{mmap-type, execbuf} subtest.
This will be useful to extend these sub-test for D3Cold.

Cc: Chris Wilson <chris.p.wilson@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 461730e8a..d2bce5826 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -1069,7 +1069,8 @@ static void debugfs_forcewake_user_subtest(void)
 	igt_assert(wait_for_suspended());
 }
 
-static void gem_mmap_args(const struct mmap_offset *t)
+static void gem_mmap_args(const struct mmap_offset *t,
+			  struct drm_i915_gem_memory_class_instance *mem_regions)
 {
 	int i;
 	uint32_t handle;
@@ -1079,7 +1080,7 @@ static void gem_mmap_args(const struct mmap_offset *t)
 	/* Create, map and set data while the device is active. */
 	enable_one_screen_and_wait(&ms_data);
 
-	handle = gem_create(drm_fd, buf_size);
+	handle = gem_create_in_memory_region_list(drm_fd, buf_size, mem_regions, 1);
 
 	gem_buf = __gem_mmap_offset(drm_fd, handle, 0, buf_size,
 				    PROT_READ | PROT_WRITE, t->type);
@@ -1290,7 +1291,7 @@ static void submit_blt_cmd(uint32_t dst_handle, int dst_size,
 }
 
 /* Make sure we can submit a batch buffer and verify its result. */
-static void gem_execbuf_subtest(void)
+static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions)
 {
 	int x, y;
 	uint32_t handle;
@@ -1308,7 +1309,7 @@ static void gem_execbuf_subtest(void)
 	/* Create and set data while the device is active. */
 	enable_one_screen_and_wait(&ms_data);
 
-	handle = gem_create(drm_fd, dst_size);
+	handle = gem_create_in_memory_region_list(drm_fd, dst_size, mem_regions, 1);
 
 	cpu_buf = malloc(dst_size);
 	igt_assert(cpu_buf);
@@ -2083,15 +2084,21 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	/* GEM */
 	igt_subtest_with_dynamic("gem-mmap-type") {
 		for_each_mmap_offset_type(drm_fd, t) {
-			igt_dynamic_f("%s", t->name)
-				gem_mmap_args(t);
+			for_each_memory_region(r, drm_fd) {
+				igt_dynamic_f("%s-%s", t->name, r->name)
+				gem_mmap_args(t, &r->ci);
+			}
 		}
 	}
 
 	igt_subtest("gem-pread")
 		gem_pread_subtest();
-	igt_subtest("gem-execbuf")
-		gem_execbuf_subtest();
+	igt_subtest_with_dynamic("gem-execbuf") {
+		for_each_memory_region(r, drm_fd) {
+			igt_dynamic_f("%s", r->name)
+				gem_execbuf_subtest(&r->ci);
+		}
+	}
 	igt_subtest("gem-idle")
 		gem_idle_subtest();
 	igt_subtest("gem-evict-pwrite") {
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 1/9] test/i915_pm_rpm: Add placement to gem-{mmap-type, execbuf}

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:40PM +0530, Anshuman Gupta wrote:
> Add memory region placement to gem-{mmap-type, execbuf} subtest.
> This will be useful to extend these sub-test for D3Cold.
> 
> Cc: Chris Wilson <chris.p.wilson@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 461730e8a..d2bce5826 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1069,7 +1069,8 @@ static void debugfs_forcewake_user_subtest(void)
>  	igt_assert(wait_for_suspended());
>  }
>  
> -static void gem_mmap_args(const struct mmap_offset *t)
> +static void gem_mmap_args(const struct mmap_offset *t,
> +			  struct drm_i915_gem_memory_class_instance *mem_regions)
>  {
>  	int i;
>  	uint32_t handle;
> @@ -1079,7 +1080,7 @@ static void gem_mmap_args(const struct mmap_offset *t)
>  	/* Create, map and set data while the device is active. */
>  	enable_one_screen_and_wait(&ms_data);
>  
> -	handle = gem_create(drm_fd, buf_size);
> +	handle = gem_create_in_memory_region_list(drm_fd, buf_size, mem_regions, 1);
>  
>  	gem_buf = __gem_mmap_offset(drm_fd, handle, 0, buf_size,
>  				    PROT_READ | PROT_WRITE, t->type);
> @@ -1290,7 +1291,7 @@ static void submit_blt_cmd(uint32_t dst_handle, int dst_size,
>  }
>  
>  /* Make sure we can submit a batch buffer and verify its result. */
> -static void gem_execbuf_subtest(void)
> +static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions)
>  {
>  	int x, y;
>  	uint32_t handle;
> @@ -1308,7 +1309,7 @@ static void gem_execbuf_subtest(void)
>  	/* Create and set data while the device is active. */
>  	enable_one_screen_and_wait(&ms_data);
>  
> -	handle = gem_create(drm_fd, dst_size);
> +	handle = gem_create_in_memory_region_list(drm_fd, dst_size, mem_regions, 1);
>  
>  	cpu_buf = malloc(dst_size);
>  	igt_assert(cpu_buf);
> @@ -2083,15 +2084,21 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	/* GEM */
>  	igt_subtest_with_dynamic("gem-mmap-type") {
>  		for_each_mmap_offset_type(drm_fd, t) {
> -			igt_dynamic_f("%s", t->name)
> -				gem_mmap_args(t);
> +			for_each_memory_region(r, drm_fd) {
> +				igt_dynamic_f("%s-%s", t->name, r->name)
> +				gem_mmap_args(t, &r->ci);
> +			}
>  		}
>  	}
>  
>  	igt_subtest("gem-pread")
>  		gem_pread_subtest();
> -	igt_subtest("gem-execbuf")
> -		gem_execbuf_subtest();
> +	igt_subtest_with_dynamic("gem-execbuf") {
> +		for_each_memory_region(r, drm_fd) {
> +			igt_dynamic_f("%s", r->name)
> +				gem_execbuf_subtest(&r->ci);
> +		}
> +	}

I believe for D3Cold the only ones that matters are the lmem ones,
but I don't believe it hurts to try more.


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



>  	igt_subtest("gem-idle")
>  		gem_idle_subtest();
>  	igt_subtest("gem-evict-pwrite") {
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 2/9] lib/igt_device: Get gfx PCI card root port

[Anshuman Gupta]

Utility library function to get the Gfx Card PCI topology
root port pci device using libpaciaccess.
root port will be used to get the real ACPI D state.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 lib/igt_device.c | 28 ++++++++++++++++++++++++++++
 lib/igt_device.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/lib/igt_device.c b/lib/igt_device.c
index 9794a9928..fddfba72c 100644
--- a/lib/igt_device.c
+++ b/lib/igt_device.c
@@ -257,3 +257,31 @@ struct pci_device *igt_device_get_pci_device(int fd)
 
 	return pci_dev;
 }
+
+/**
+ * igt_device_get_pci_root_port:
+ * @fd: the device.
+ *
+ * Looks up the graphics pci device root port using libpciaccess.
+ *
+ * Returns:
+ * The root port pci_device.
+ */
+struct pci_device *
+igt_device_get_pci_root_port(int fd)
+{
+	struct pci_device *pci_dev, *prev;
+
+	pci_dev = __igt_device_get_pci_device(fd, 0);
+	igt_require(pci_dev);
+
+	while (pci_dev) {
+		prev = pci_dev;
+		pci_dev = pci_device_get_parent_bridge(pci_dev);
+	}
+
+	igt_debug("Root Port PCI device %04x:%02x:%02x.%01x\n",
+		  prev->domain, prev->bus, prev->dev, prev->func);
+
+	return prev;
+}
diff --git a/lib/igt_device.h b/lib/igt_device.h
index 1aaa840e2..800a0fcc3 100644
--- a/lib/igt_device.h
+++ b/lib/igt_device.h
@@ -34,5 +34,6 @@ void igt_device_drop_master(int fd);
 int igt_device_get_card_index(int fd);
 struct pci_device *igt_device_get_pci_device(int fd);
 struct pci_device *__igt_device_get_pci_device(int fd, unsigned int vf_id);
+struct pci_device *igt_device_get_pci_root_port(int fd);
 
 #endif /* __IGT_DEVICE_H__ */
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 2/9] lib/igt_device: Get gfx PCI card root port

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:41PM +0530, Anshuman Gupta wrote:
> Utility library function to get the Gfx Card PCI topology
> root port pci device using libpaciaccess.
> root port will be used to get the real ACPI D state.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  lib/igt_device.c | 28 ++++++++++++++++++++++++++++
>  lib/igt_device.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/lib/igt_device.c b/lib/igt_device.c
> index 9794a9928..fddfba72c 100644
> --- a/lib/igt_device.c
> +++ b/lib/igt_device.c
> @@ -257,3 +257,31 @@ struct pci_device *igt_device_get_pci_device(int fd)
>  
>  	return pci_dev;
>  }
> +
> +/**
> + * igt_device_get_pci_root_port:
> + * @fd: the device.
> + *
> + * Looks up the graphics pci device root port using libpciaccess.
> + *
> + * Returns:
> + * The root port pci_device.
> + */
> +struct pci_device *
> +igt_device_get_pci_root_port(int fd)
> +{
> +	struct pci_device *pci_dev, *prev;
> +
> +	pci_dev = __igt_device_get_pci_device(fd, 0);
> +	igt_require(pci_dev);
> +
> +	while (pci_dev) {
> +		prev = pci_dev;
> +		pci_dev = pci_device_get_parent_bridge(pci_dev);
> +	}

Neat!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> +
> +	igt_debug("Root Port PCI device %04x:%02x:%02x.%01x\n",
> +		  prev->domain, prev->bus, prev->dev, prev->func);
> +
> +	return prev;
> +}
> diff --git a/lib/igt_device.h b/lib/igt_device.h
> index 1aaa840e2..800a0fcc3 100644
> --- a/lib/igt_device.h
> +++ b/lib/igt_device.h
> @@ -34,5 +34,6 @@ void igt_device_drop_master(int fd);
>  int igt_device_get_card_index(int fd);
>  struct pci_device *igt_device_get_pci_device(int fd);
>  struct pci_device *__igt_device_get_pci_device(int fd, unsigned int vf_id);
> +struct pci_device *igt_device_get_pci_root_port(int fd);
>  
>  #endif /* __IGT_DEVICE_H__ */
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm infrastructure

[Anshuman Gupta]

Enable gfx card pci devices runtime pm for all pci devices
and bridge under the topology of Gfx Card root port.

Added a library function to get the PCI root port ACPI
D state and to print the pci card devices runtime pm
status.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 lib/igt_pm.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_pm.h |  21 +++++
 2 files changed, 247 insertions(+)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index b409ec463..8e8a330f8 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <stdio.h>
 #include <limits.h>
+#include <pciaccess.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
@@ -75,6 +76,7 @@ enum {
 #define MIN_POWER_STR		"min_power\n"
 /* Remember to fix this if adding longer strings */
 #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
+#define MAX_PCI_DEVICES		256
 int8_t *__sata_pm_policies;
 int __scsi_host_cnt;
 
@@ -856,3 +858,227 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
 
 	return strstr(buf, "LPSP: capable");
 }
+
+/**
+ * igt_pm_acpi_d3cold_suppoarted:
+ * @pci_dev: root port pci_dev.
+ * Check ACPI D3Cold support.
+ *
+ * Returns:
+ * True if ACPI D3Cold supported otherwise false.
+ */
+bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
+{
+	char name[PATH_MAX];
+	int dir, fd;
+
+	snprintf(name, PATH_MAX,
+		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	dir = open(name, O_RDONLY);
+	igt_require(dir > 0);
+
+	/* BIOS need to enable ACPI D3Cold Support.*/
+	fd = openat(dir, "real_power_state", O_RDONLY);
+	if (fd < 0 && errno == ENOENT)
+		return false;
+
+	igt_require(fd > 0);
+
+	return true;
+}
+
+/**
+ * igt_pm_get_acpi_real_d_state:
+ * @pci_dev: root port pci_dev.
+ * Get ACPI D state for a given root port.
+ *
+ * Returns:
+ * igt_acpi_d_state state.
+ */
+enum igt_acpi_d_state
+igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev)
+{
+	char name[PATH_MAX];
+	char acpi_d_state[64];
+	int fd, n_read;
+
+	snprintf(name, PATH_MAX,
+		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node/real_power_state",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return IGT_ACPI_UNKNOWN_STATE;
+
+	n_read = read(fd, acpi_d_state, sizeof(acpi_d_state) - 1);
+	igt_assert(n_read >= 0);
+	acpi_d_state[n_read] = '\0';
+	close(fd);
+
+	if (strncmp(acpi_d_state, "D0\n", n_read) == 0)
+		return IGT_ACPI_D0;
+	else if  (strncmp(acpi_d_state, "D1\n", n_read) == 0)
+		return IGT_ACPI_D1;
+	else if  (strncmp(acpi_d_state, "D2\n", n_read) == 0)
+		return IGT_ACPI_D2;
+	else if  (strncmp(acpi_d_state, "D3hot\n", n_read) == 0)
+		return IGT_ACPI_D3Hot;
+	else if  (strncmp(acpi_d_state, "D3cold\n", n_read) == 0)
+		return IGT_ACPI_D3Cold;
+	else
+		return IGT_ACPI_UNKNOWN_STATE;
+}
+
+static struct igt_pm_pci_dev_control  __igt_pm_pci_control[MAX_PCI_DEVICES];
+
+static void __igt_pm_pci_card_exit_handler(int sig)
+{
+	igt_pm_restore_pci_card_runtime_pm();
+}
+
+static void
+__igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev, int index)
+{
+	char name[PATH_MAX], buf[6];
+	int fd, control_size, size;
+	char *control;
+
+	igt_assert(index <  MAX_PCI_DEVICES);
+
+	control = __igt_pm_pci_control[index].control;
+	control_size =  sizeof(__igt_pm_pci_control[index].control);
+	__igt_pm_pci_control[index].pci_dev = pci_dev;
+
+	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	fd = open(name, O_RDWR);
+	igt_assert_f(fd >= 0, "Can't open control\n");
+
+	igt_assert(read(fd, control, control_size - 1) > 0);
+	strchomp(control);
+
+	igt_debug("Saved runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
+		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
+	igt_install_exit_handler(__igt_pm_pci_card_exit_handler);
+
+	size = write(fd, "auto\n", 5);
+	igt_assert(size == 5);
+
+	lseek(fd, 0, SEEK_SET);
+	size = read(fd, buf, ARRAY_SIZE(buf));
+	igt_assert(size == 5);
+	igt_assert(strncmp(buf, "auto\n", 5) == 0);
+
+	close(fd);
+}
+
+/**
+ * igt_pm_setup_pci_card_runtime_pm:
+ * @pci_dev: root port pci_dev.
+ * Setup runtime PM for all PCI endpoints devices for a given
+ * root port. It also save power control attribute for all PCI endpoints
+ * devices for a given root port.
+ */
+void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
+{
+	int primary, secondary, subordinate, ret;
+	struct pci_device_iterator *iter;
+	struct pci_device *dev;
+	int i = 0;
+
+	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
+	ret = pci_device_get_bridge_buses(pci_dev, &primary, &secondary, &subordinate);
+	igt_assert(!ret);
+
+	ret = pci_system_init();
+	igt_assert(!ret);
+
+	iter = pci_slot_match_iterator_create(NULL);
+	/* Setup runtime pm for PCI root port */
+	__igt_pm_setup_pci_card_runtime_pm(pci_dev, i++);
+	while ((dev = pci_device_next(iter)) != NULL) {
+		if (dev->bus >= secondary && dev->bus <= subordinate)
+			__igt_pm_setup_pci_card_runtime_pm(dev, i++);
+	}
+}
+
+static void igt_pm_restore_pci_dev_runtime_pm(struct pci_device *pci_dev, char *control, int len)
+{
+	char name[PATH_MAX];
+	int fd;
+
+	igt_debug("Restoring runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
+		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
+
+	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	fd = open(name, O_WRONLY);
+	igt_assert_f(fd >= 0, "Can't open control\n");
+
+	igt_assert(write(fd, control, len) == len);
+	close(fd);
+}
+
+/**
+ * igt_pm_restore_pci_card_runtime_pm:
+ * @pci_dev: root port pci_dev.
+ * Restore power control attribute for all PCI endpoints devices for a given
+ * root port.
+ */
+void igt_pm_restore_pci_card_runtime_pm(void)
+{
+	int i = 0;
+
+	for (i = 0; i < MAX_PCI_DEVICES; i++) {
+		if (!__igt_pm_pci_control[i].pci_dev)
+			break;
+
+		igt_pm_restore_pci_dev_runtime_pm(__igt_pm_pci_control[i].pci_dev,
+						  __igt_pm_pci_control[i].control,
+						  sizeof(__igt_pm_pci_control[i].control));
+	}
+
+	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
+	pci_system_cleanup();
+}
+
+static void igt_pm_print_pci_dev_runtime_status(struct pci_device *pci_dev)
+{
+	char name[PATH_MAX], runtime_status[64];
+	int fd, n_read;
+
+	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/runtime_status",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	fd = open(name, O_RDONLY);
+	igt_assert_f(fd >= 0, "Can't open runtime_status\n");
+
+	n_read = read(fd, runtime_status, sizeof(runtime_status) - 1);
+	igt_assert(n_read >= 0);
+	runtime_status[n_read] = '\0';
+	igt_info("runtime suspend status for PCI '%04x:%02x:%02x.%01x' %s\n",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, runtime_status);
+	close(fd);
+}
+
+/**
+ * igt_pm_print_pci_card_runtime_status:
+ * @pci_dev: root port pci_dev.
+ * Print runtime suspend status for all PCI endpoints devices for a given
+ * root port.
+ */
+void igt_pm_print_pci_card_runtime_status(void)
+{
+	int i = 0;
+
+	for (i = 0; i < MAX_PCI_DEVICES; i++) {
+		if (!__igt_pm_pci_control[i].pci_dev)
+			break;
+
+		igt_pm_print_pci_dev_runtime_status(__igt_pm_pci_control[i].pci_dev);
+	}
+}
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index 162d3ca3c..a5a9c4760 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -46,6 +46,21 @@ enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_UNKNOWN,
 };
 
+/* PCI ACPI firmware node real state */
+enum igt_acpi_d_state {
+	IGT_ACPI_D0,
+	IGT_ACPI_D1,
+	IGT_ACPI_D2,
+	IGT_ACPI_D3Hot,
+	IGT_ACPI_D3Cold,
+	IGT_ACPI_UNKNOWN_STATE,
+};
+
+struct	igt_pm_pci_dev_control {
+	struct pci_device *pci_dev;
+	char control[64];
+};
+
 bool igt_setup_runtime_pm(int device);
 void igt_disable_runtime_pm(void);
 void igt_restore_runtime_pm(void);
@@ -54,5 +69,11 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
 bool igt_pm_dmc_loaded(int debugfs);
 bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
 bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
+bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev);
+enum igt_acpi_d_state
+igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);
+void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev);
+void igt_pm_restore_pci_card_runtime_pm(void);
+void igt_pm_print_pci_card_runtime_status(void);
 
 #endif /* IGT_PM_H */
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm infrastructure

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:42PM +0530, Anshuman Gupta wrote:
> Enable gfx card pci devices runtime pm for all pci devices
> and bridge under the topology of Gfx Card root port.
> 
> Added a library function to get the PCI root port ACPI
> D state and to print the pci card devices runtime pm
> status.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  lib/igt_pm.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_pm.h |  21 +++++
>  2 files changed, 247 insertions(+)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index b409ec463..8e8a330f8 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -28,6 +28,7 @@
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <limits.h>
> +#include <pciaccess.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> @@ -75,6 +76,7 @@ enum {
>  #define MIN_POWER_STR		"min_power\n"
>  /* Remember to fix this if adding longer strings */
>  #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
> +#define MAX_PCI_DEVICES		256
>  int8_t *__sata_pm_policies;
>  int __scsi_host_cnt;
>  
> @@ -856,3 +858,227 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
>  
>  	return strstr(buf, "LPSP: capable");
>  }
> +
> +/**
> + * igt_pm_acpi_d3cold_suppoarted:

                              ^ typo

> + * @pci_dev: root port pci_dev.
> + * Check ACPI D3Cold support.
> + *
> + * Returns:
> + * True if ACPI D3Cold supported otherwise false.
> + */
> +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX];
> +	int dir, fd;
> +
> +	snprintf(name, PATH_MAX,
> +		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	dir = open(name, O_RDONLY);
> +	igt_require(dir > 0);
> +
> +	/* BIOS need to enable ACPI D3Cold Support.*/
> +	fd = openat(dir, "real_power_state", O_RDONLY);
> +	if (fd < 0 && errno == ENOENT)
> +		return false;
> +
> +	igt_require(fd > 0);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_pm_get_acpi_real_d_state:
> + * @pci_dev: root port pci_dev.
> + * Get ACPI D state for a given root port.
> + *
> + * Returns:
> + * igt_acpi_d_state state.
> + */
> +enum igt_acpi_d_state
> +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX];
> +	char acpi_d_state[64];
> +	int fd, n_read;
> +
> +	snprintf(name, PATH_MAX,
> +		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node/real_power_state",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDONLY);
> +	if (fd < 0)
> +		return IGT_ACPI_UNKNOWN_STATE;
> +
> +	n_read = read(fd, acpi_d_state, sizeof(acpi_d_state) - 1);
> +	igt_assert(n_read >= 0);
> +	acpi_d_state[n_read] = '\0';
> +	close(fd);
> +
> +	if (strncmp(acpi_d_state, "D0\n", n_read) == 0)
> +		return IGT_ACPI_D0;
> +	else if  (strncmp(acpi_d_state, "D1\n", n_read) == 0)
> +		return IGT_ACPI_D1;
> +	else if  (strncmp(acpi_d_state, "D2\n", n_read) == 0)
> +		return IGT_ACPI_D2;
> +	else if  (strncmp(acpi_d_state, "D3hot\n", n_read) == 0)
> +		return IGT_ACPI_D3Hot;
> +	else if  (strncmp(acpi_d_state, "D3cold\n", n_read) == 0)
> +		return IGT_ACPI_D3Cold;
> +	else
> +		return IGT_ACPI_UNKNOWN_STATE;
> +}
> +
> +static struct igt_pm_pci_dev_control  __igt_pm_pci_control[MAX_PCI_DEVICES];

the only thing that I'm not totally comfortable with this patch is this global
struct of 256 items in a lib file.

Although I liked the use of the exit handler to restore the status...

> +
> +static void __igt_pm_pci_card_exit_handler(int sig)
> +{
> +	igt_pm_restore_pci_card_runtime_pm();
> +}
> +
> +static void
> +__igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev, int index)
> +{
> +	char name[PATH_MAX], buf[6];
> +	int fd, control_size, size;
> +	char *control;
> +
> +	igt_assert(index <  MAX_PCI_DEVICES);
> +
> +	control = __igt_pm_pci_control[index].control;
> +	control_size =  sizeof(__igt_pm_pci_control[index].control);
> +	__igt_pm_pci_control[index].pci_dev = pci_dev;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDWR);
> +	igt_assert_f(fd >= 0, "Can't open control\n");
> +
> +	igt_assert(read(fd, control, control_size - 1) > 0);
> +	strchomp(control);
> +
> +	igt_debug("Saved runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
> +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
> +	igt_install_exit_handler(__igt_pm_pci_card_exit_handler);
> +
> +	size = write(fd, "auto\n", 5);
> +	igt_assert(size == 5);
> +
> +	lseek(fd, 0, SEEK_SET);
> +	size = read(fd, buf, ARRAY_SIZE(buf));
> +	igt_assert(size == 5);
> +	igt_assert(strncmp(buf, "auto\n", 5) == 0);
> +
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_setup_pci_card_runtime_pm:
> + * @pci_dev: root port pci_dev.
> + * Setup runtime PM for all PCI endpoints devices for a given
> + * root port. It also save power control attribute for all PCI endpoints
> + * devices for a given root port.
> + */
> +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
> +{
> +	int primary, secondary, subordinate, ret;
> +	struct pci_device_iterator *iter;
> +	struct pci_device *dev;
> +	int i = 0;
> +
> +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> +	ret = pci_device_get_bridge_buses(pci_dev, &primary, &secondary, &subordinate);
> +	igt_assert(!ret);
> +
> +	ret = pci_system_init();
> +	igt_assert(!ret);
> +
> +	iter = pci_slot_match_iterator_create(NULL);
> +	/* Setup runtime pm for PCI root port */
> +	__igt_pm_setup_pci_card_runtime_pm(pci_dev, i++);
> +	while ((dev = pci_device_next(iter)) != NULL) {
> +		if (dev->bus >= secondary && dev->bus <= subordinate)
> +			__igt_pm_setup_pci_card_runtime_pm(dev, i++);
> +	}
> +}
> +
> +static void igt_pm_restore_pci_dev_runtime_pm(struct pci_device *pci_dev, char *control, int len)
> +{
> +	char name[PATH_MAX];
> +	int fd;
> +
> +	igt_debug("Restoring runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
> +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_WRONLY);
> +	igt_assert_f(fd >= 0, "Can't open control\n");
> +
> +	igt_assert(write(fd, control, len) == len);
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_restore_pci_card_runtime_pm:
> + * @pci_dev: root port pci_dev.
> + * Restore power control attribute for all PCI endpoints devices for a given
> + * root port.
> + */
> +void igt_pm_restore_pci_card_runtime_pm(void)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> +		if (!__igt_pm_pci_control[i].pci_dev)
> +			break;
> +
> +		igt_pm_restore_pci_dev_runtime_pm(__igt_pm_pci_control[i].pci_dev,
> +						  __igt_pm_pci_control[i].control,
> +						  sizeof(__igt_pm_pci_control[i].control));
> +	}
> +
> +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> +	pci_system_cleanup();
> +}
> +
> +static void igt_pm_print_pci_dev_runtime_status(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX], runtime_status[64];
> +	int fd, n_read;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/runtime_status",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDONLY);
> +	igt_assert_f(fd >= 0, "Can't open runtime_status\n");
> +
> +	n_read = read(fd, runtime_status, sizeof(runtime_status) - 1);
> +	igt_assert(n_read >= 0);
> +	runtime_status[n_read] = '\0';
> +	igt_info("runtime suspend status for PCI '%04x:%02x:%02x.%01x' %s\n",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, runtime_status);
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_print_pci_card_runtime_status:
> + * @pci_dev: root port pci_dev.
> + * Print runtime suspend status for all PCI endpoints devices for a given
> + * root port.
> + */
> +void igt_pm_print_pci_card_runtime_status(void)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> +		if (!__igt_pm_pci_control[i].pci_dev)
> +			break;
> +
> +		igt_pm_print_pci_dev_runtime_status(__igt_pm_pci_control[i].pci_dev);
> +	}
> +}
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 162d3ca3c..a5a9c4760 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -46,6 +46,21 @@ enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_UNKNOWN,
>  };
>  
> +/* PCI ACPI firmware node real state */
> +enum igt_acpi_d_state {
> +	IGT_ACPI_D0,
> +	IGT_ACPI_D1,
> +	IGT_ACPI_D2,
> +	IGT_ACPI_D3Hot,
> +	IGT_ACPI_D3Cold,
> +	IGT_ACPI_UNKNOWN_STATE,
> +};
> +
> +struct	igt_pm_pci_dev_control {
> +	struct pci_device *pci_dev;
> +	char control[64];
> +};
> +
>  bool igt_setup_runtime_pm(int device);
>  void igt_disable_runtime_pm(void);
>  void igt_restore_runtime_pm(void);
> @@ -54,5 +69,11 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
>  bool igt_pm_dmc_loaded(int debugfs);
>  bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
>  bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
> +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev);
> +enum igt_acpi_d_state
> +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);
> +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev);
> +void igt_pm_restore_pci_card_runtime_pm(void);
> +void igt_pm_print_pci_card_runtime_status(void);
>  
>  #endif /* IGT_PM_H */
> -- 
> 2.26.2
> 

Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm infrastructure

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Friday, April 22, 2022 3:47 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal <badal.nilawar@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm
> infrastructure
> 
> On Mon, Apr 18, 2022 at 06:20:42PM +0530, Anshuman Gupta wrote:
> > Enable gfx card pci devices runtime pm for all pci devices and bridge
> > under the topology of Gfx Card root port.
> >
> > Added a library function to get the PCI root port ACPI D state and to
> > print the pci card devices runtime pm status.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  lib/igt_pm.c | 226
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_pm.h |  21 +++++
> >  2 files changed, 247 insertions(+)
> >
> > diff --git a/lib/igt_pm.c b/lib/igt_pm.c index b409ec463..8e8a330f8
> > 100644
> > --- a/lib/igt_pm.c
> > +++ b/lib/igt_pm.c
> > @@ -28,6 +28,7 @@
> >  #include <fcntl.h>
> >  #include <stdio.h>
> >  #include <limits.h>
> > +#include <pciaccess.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > @@ -75,6 +76,7 @@ enum {
> >  #define MIN_POWER_STR		"min_power\n"
> >  /* Remember to fix this if adding longer strings */
> >  #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
> > +#define MAX_PCI_DEVICES		256
> >  int8_t *__sata_pm_policies;
> >  int __scsi_host_cnt;
> >
> > @@ -856,3 +858,227 @@ bool i915_output_is_lpsp_capable(int drm_fd,
> > igt_output_t *output)
> >
> >  	return strstr(buf, "LPSP: capable");  }
> > +
> > +/**
> > + * igt_pm_acpi_d3cold_suppoarted:
> 
>                               ^ typo
> 
> > + * @pci_dev: root port pci_dev.
> > + * Check ACPI D3Cold support.
> > + *
> > + * Returns:
> > + * True if ACPI D3Cold supported otherwise false.
> > + */
> > +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev) {
> > +	char name[PATH_MAX];
> > +	int dir, fd;
> > +
> > +	snprintf(name, PATH_MAX,
> > +
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > +
> > +	dir = open(name, O_RDONLY);
> > +	igt_require(dir > 0);
> > +
> > +	/* BIOS need to enable ACPI D3Cold Support.*/
> > +	fd = openat(dir, "real_power_state", O_RDONLY);
> > +	if (fd < 0 && errno == ENOENT)
> > +		return false;
> > +
> > +	igt_require(fd > 0);
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * igt_pm_get_acpi_real_d_state:
> > + * @pci_dev: root port pci_dev.
> > + * Get ACPI D state for a given root port.
> > + *
> > + * Returns:
> > + * igt_acpi_d_state state.
> > + */
> > +enum igt_acpi_d_state
> > +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev) {
> > +	char name[PATH_MAX];
> > +	char acpi_d_state[64];
> > +	int fd, n_read;
> > +
> > +	snprintf(name, PATH_MAX,
> > +
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node/real_power_stat
> e",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > +
> > +	fd = open(name, O_RDONLY);
> > +	if (fd < 0)
> > +		return IGT_ACPI_UNKNOWN_STATE;
> > +
> > +	n_read = read(fd, acpi_d_state, sizeof(acpi_d_state) - 1);
> > +	igt_assert(n_read >= 0);
> > +	acpi_d_state[n_read] = '\0';
> > +	close(fd);
> > +
> > +	if (strncmp(acpi_d_state, "D0\n", n_read) == 0)
> > +		return IGT_ACPI_D0;
> > +	else if  (strncmp(acpi_d_state, "D1\n", n_read) == 0)
> > +		return IGT_ACPI_D1;
> > +	else if  (strncmp(acpi_d_state, "D2\n", n_read) == 0)
> > +		return IGT_ACPI_D2;
> > +	else if  (strncmp(acpi_d_state, "D3hot\n", n_read) == 0)
> > +		return IGT_ACPI_D3Hot;
> > +	else if  (strncmp(acpi_d_state, "D3cold\n", n_read) == 0)
> > +		return IGT_ACPI_D3Cold;
> > +	else
> > +		return IGT_ACPI_UNKNOWN_STATE;
> > +}
> > +
> > +static struct igt_pm_pci_dev_control
> > +__igt_pm_pci_control[MAX_PCI_DEVICES];
> 
> the only thing that I'm not totally comfortable with this patch is this global struct
> of 256 items in a lib file.
Thanks for comment, sole purpose of this array to restore the states.
Will a dynamic allocated buffer will address your concern ?
Which can be freed-up on restoring the attributes.
Thanks,
Anshuman Gupta.
> 
> Although I liked the use of the exit handler to restore the status...
> 
> > +
> > +static void __igt_pm_pci_card_exit_handler(int sig) {
> > +	igt_pm_restore_pci_card_runtime_pm();
> > +}
> > +
> > +static void
> > +__igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev, int
> > +index) {
> > +	char name[PATH_MAX], buf[6];
> > +	int fd, control_size, size;
> > +	char *control;
> > +
> > +	igt_assert(index <  MAX_PCI_DEVICES);
> > +
> > +	control = __igt_pm_pci_control[index].control;
> > +	control_size =  sizeof(__igt_pm_pci_control[index].control);
> > +	__igt_pm_pci_control[index].pci_dev = pci_dev;
> > +
> > +	snprintf(name, PATH_MAX,
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > +
> > +	fd = open(name, O_RDWR);
> > +	igt_assert_f(fd >= 0, "Can't open control\n");
> > +
> > +	igt_assert(read(fd, control, control_size - 1) > 0);
> > +	strchomp(control);
> > +
> > +	igt_debug("Saved runtime power management for PCI
> '%04x:%02x:%02x.%01x' '%s'\n",
> > +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> control);
> > +	igt_install_exit_handler(__igt_pm_pci_card_exit_handler);
> > +
> > +	size = write(fd, "auto\n", 5);
> > +	igt_assert(size == 5);
> > +
> > +	lseek(fd, 0, SEEK_SET);
> > +	size = read(fd, buf, ARRAY_SIZE(buf));
> > +	igt_assert(size == 5);
> > +	igt_assert(strncmp(buf, "auto\n", 5) == 0);
> > +
> > +	close(fd);
> > +}
> > +
> > +/**
> > + * igt_pm_setup_pci_card_runtime_pm:
> > + * @pci_dev: root port pci_dev.
> > + * Setup runtime PM for all PCI endpoints devices for a given
> > + * root port. It also save power control attribute for all PCI
> > +endpoints
> > + * devices for a given root port.
> > + */
> > +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev) {
> > +	int primary, secondary, subordinate, ret;
> > +	struct pci_device_iterator *iter;
> > +	struct pci_device *dev;
> > +	int i = 0;
> > +
> > +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> > +	ret = pci_device_get_bridge_buses(pci_dev, &primary, &secondary,
> &subordinate);
> > +	igt_assert(!ret);
> > +
> > +	ret = pci_system_init();
> > +	igt_assert(!ret);
> > +
> > +	iter = pci_slot_match_iterator_create(NULL);
> > +	/* Setup runtime pm for PCI root port */
> > +	__igt_pm_setup_pci_card_runtime_pm(pci_dev, i++);
> > +	while ((dev = pci_device_next(iter)) != NULL) {
> > +		if (dev->bus >= secondary && dev->bus <= subordinate)
> > +			__igt_pm_setup_pci_card_runtime_pm(dev, i++);
> > +	}
> > +}
> > +
> > +static void igt_pm_restore_pci_dev_runtime_pm(struct pci_device
> > +*pci_dev, char *control, int len) {
> > +	char name[PATH_MAX];
> > +	int fd;
> > +
> > +	igt_debug("Restoring runtime power management for PCI
> '%04x:%02x:%02x.%01x' '%s'\n",
> > +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> > +control);
> > +
> > +	snprintf(name, PATH_MAX,
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > +
> > +	fd = open(name, O_WRONLY);
> > +	igt_assert_f(fd >= 0, "Can't open control\n");
> > +
> > +	igt_assert(write(fd, control, len) == len);
> > +	close(fd);
> > +}
> > +
> > +/**
> > + * igt_pm_restore_pci_card_runtime_pm:
> > + * @pci_dev: root port pci_dev.
> > + * Restore power control attribute for all PCI endpoints devices for
> > +a given
> > + * root port.
> > + */
> > +void igt_pm_restore_pci_card_runtime_pm(void)
> > +{
> > +	int i = 0;
> > +
> > +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> > +		if (!__igt_pm_pci_control[i].pci_dev)
> > +			break;
> > +
> > +
> 	igt_pm_restore_pci_dev_runtime_pm(__igt_pm_pci_control[i].pci_dev,
> > +
> __igt_pm_pci_control[i].control,
> > +
> sizeof(__igt_pm_pci_control[i].control));
> > +	}
> > +
> > +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> > +	pci_system_cleanup();
> > +}
> > +
> > +static void igt_pm_print_pci_dev_runtime_status(struct pci_device
> > +*pci_dev) {
> > +	char name[PATH_MAX], runtime_status[64];
> > +	int fd, n_read;
> > +
> > +	snprintf(name, PATH_MAX,
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/runtime_status",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > +
> > +	fd = open(name, O_RDONLY);
> > +	igt_assert_f(fd >= 0, "Can't open runtime_status\n");
> > +
> > +	n_read = read(fd, runtime_status, sizeof(runtime_status) - 1);
> > +	igt_assert(n_read >= 0);
> > +	runtime_status[n_read] = '\0';
> > +	igt_info("runtime suspend status for PCI '%04x:%02x:%02x.%01x' %s\n",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> runtime_status);
> > +	close(fd);
> > +}
> > +
> > +/**
> > + * igt_pm_print_pci_card_runtime_status:
> > + * @pci_dev: root port pci_dev.
> > + * Print runtime suspend status for all PCI endpoints devices for a
> > +given
> > + * root port.
> > + */
> > +void igt_pm_print_pci_card_runtime_status(void)
> > +{
> > +	int i = 0;
> > +
> > +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> > +		if (!__igt_pm_pci_control[i].pci_dev)
> > +			break;
> > +
> > +
> 	igt_pm_print_pci_dev_runtime_status(__igt_pm_pci_control[i].pci_dev)
> ;
> > +	}
> > +}
> > diff --git a/lib/igt_pm.h b/lib/igt_pm.h index 162d3ca3c..a5a9c4760
> > 100644
> > --- a/lib/igt_pm.h
> > +++ b/lib/igt_pm.h
> > @@ -46,6 +46,21 @@ enum igt_runtime_pm_status {
> >  	IGT_RUNTIME_PM_STATUS_UNKNOWN,
> >  };
> >
> > +/* PCI ACPI firmware node real state */ enum igt_acpi_d_state {
> > +	IGT_ACPI_D0,
> > +	IGT_ACPI_D1,
> > +	IGT_ACPI_D2,
> > +	IGT_ACPI_D3Hot,
> > +	IGT_ACPI_D3Cold,
> > +	IGT_ACPI_UNKNOWN_STATE,
> > +};
> > +
> > +struct	igt_pm_pci_dev_control {
> > +	struct pci_device *pci_dev;
> > +	char control[64];
> > +};
> > +
> >  bool igt_setup_runtime_pm(int device);  void
> > igt_disable_runtime_pm(void);  void igt_restore_runtime_pm(void); @@
> > -54,5 +69,11 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status
> > status);  bool igt_pm_dmc_loaded(int debugfs);  bool
> > igt_pm_pc8_plus_residencies_enabled(int msr_fd);  bool
> > i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
> > +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev); enum
> > +igt_acpi_d_state igt_pm_get_acpi_real_d_state(struct pci_device
> > +*pci_dev); void igt_pm_setup_pci_card_runtime_pm(struct pci_device
> > +*pci_dev); void igt_pm_restore_pci_card_runtime_pm(void);
> > +void igt_pm_print_pci_card_runtime_status(void);
> >
> >  #endif /* IGT_PM_H */
> > --
> > 2.26.2
> >

Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm infrastructure

[Rodrigo Vivi]

On Tue, Apr 26, 2022 at 08:23:23AM -0400, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Sent: Friday, April 22, 2022 3:47 PM
> > To: Gupta, Anshuman <anshuman.gupta@intel.com>
> > Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal <badal.nilawar@intel.com>
> > Subject: Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm
> > infrastructure
> > 
> > On Mon, Apr 18, 2022 at 06:20:42PM +0530, Anshuman Gupta wrote:
> > > Enable gfx card pci devices runtime pm for all pci devices and bridge
> > > under the topology of Gfx Card root port.
> > >
> > > Added a library function to get the PCI root port ACPI D state and to
> > > print the pci card devices runtime pm status.
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  lib/igt_pm.c | 226
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/igt_pm.h |  21 +++++
> > >  2 files changed, 247 insertions(+)
> > >
> > > diff --git a/lib/igt_pm.c b/lib/igt_pm.c index b409ec463..8e8a330f8
> > > 100644
> > > --- a/lib/igt_pm.c
> > > +++ b/lib/igt_pm.c
> > > @@ -28,6 +28,7 @@
> > >  #include <fcntl.h>
> > >  #include <stdio.h>
> > >  #include <limits.h>
> > > +#include <pciaccess.h>
> > >  #include <stdlib.h>
> > >  #include <string.h>
> > >  #include <unistd.h>
> > > @@ -75,6 +76,7 @@ enum {
> > >  #define MIN_POWER_STR		"min_power\n"
> > >  /* Remember to fix this if adding longer strings */
> > >  #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
> > > +#define MAX_PCI_DEVICES		256
> > >  int8_t *__sata_pm_policies;
> > >  int __scsi_host_cnt;
> > >
> > > @@ -856,3 +858,227 @@ bool i915_output_is_lpsp_capable(int drm_fd,
> > > igt_output_t *output)
> > >
> > >  	return strstr(buf, "LPSP: capable");  }
> > > +
> > > +/**
> > > + * igt_pm_acpi_d3cold_suppoarted:
> > 
> >                               ^ typo
> > 
> > > + * @pci_dev: root port pci_dev.
> > > + * Check ACPI D3Cold support.
> > > + *
> > > + * Returns:
> > > + * True if ACPI D3Cold supported otherwise false.
> > > + */
> > > +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev) {
> > > +	char name[PATH_MAX];
> > > +	int dir, fd;
> > > +
> > > +	snprintf(name, PATH_MAX,
> > > +
> > "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > > +
> > > +	dir = open(name, O_RDONLY);
> > > +	igt_require(dir > 0);
> > > +
> > > +	/* BIOS need to enable ACPI D3Cold Support.*/
> > > +	fd = openat(dir, "real_power_state", O_RDONLY);
> > > +	if (fd < 0 && errno == ENOENT)
> > > +		return false;
> > > +
> > > +	igt_require(fd > 0);
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +/**
> > > + * igt_pm_get_acpi_real_d_state:
> > > + * @pci_dev: root port pci_dev.
> > > + * Get ACPI D state for a given root port.
> > > + *
> > > + * Returns:
> > > + * igt_acpi_d_state state.
> > > + */
> > > +enum igt_acpi_d_state
> > > +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev) {
> > > +	char name[PATH_MAX];
> > > +	char acpi_d_state[64];
> > > +	int fd, n_read;
> > > +
> > > +	snprintf(name, PATH_MAX,
> > > +
> > "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node/real_power_stat
> > e",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > > +
> > > +	fd = open(name, O_RDONLY);
> > > +	if (fd < 0)
> > > +		return IGT_ACPI_UNKNOWN_STATE;
> > > +
> > > +	n_read = read(fd, acpi_d_state, sizeof(acpi_d_state) - 1);
> > > +	igt_assert(n_read >= 0);
> > > +	acpi_d_state[n_read] = '\0';
> > > +	close(fd);
> > > +
> > > +	if (strncmp(acpi_d_state, "D0\n", n_read) == 0)
> > > +		return IGT_ACPI_D0;
> > > +	else if  (strncmp(acpi_d_state, "D1\n", n_read) == 0)
> > > +		return IGT_ACPI_D1;
> > > +	else if  (strncmp(acpi_d_state, "D2\n", n_read) == 0)
> > > +		return IGT_ACPI_D2;
> > > +	else if  (strncmp(acpi_d_state, "D3hot\n", n_read) == 0)
> > > +		return IGT_ACPI_D3Hot;
> > > +	else if  (strncmp(acpi_d_state, "D3cold\n", n_read) == 0)
> > > +		return IGT_ACPI_D3Cold;
> > > +	else
> > > +		return IGT_ACPI_UNKNOWN_STATE;
> > > +}
> > > +
> > > +static struct igt_pm_pci_dev_control
> > > +__igt_pm_pci_control[MAX_PCI_DEVICES];
> > 
> > the only thing that I'm not totally comfortable with this patch is this global struct
> > of 256 items in a lib file.
> Thanks for comment, sole purpose of this array to restore the states.
> Will a dynamic allocated buffer will address your concern ?
> Which can be freed-up on restoring the attributes.

that or some local definitions and the lib who only handles the pointers

> Thanks,
> Anshuman Gupta.
> > 
> > Although I liked the use of the exit handler to restore the status...
> > 
> > > +
> > > +static void __igt_pm_pci_card_exit_handler(int sig) {
> > > +	igt_pm_restore_pci_card_runtime_pm();
> > > +}
> > > +
> > > +static void
> > > +__igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev, int
> > > +index) {
> > > +	char name[PATH_MAX], buf[6];
> > > +	int fd, control_size, size;
> > > +	char *control;
> > > +
> > > +	igt_assert(index <  MAX_PCI_DEVICES);
> > > +
> > > +	control = __igt_pm_pci_control[index].control;
> > > +	control_size =  sizeof(__igt_pm_pci_control[index].control);
> > > +	__igt_pm_pci_control[index].pci_dev = pci_dev;
> > > +
> > > +	snprintf(name, PATH_MAX,
> > "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > > +
> > > +	fd = open(name, O_RDWR);
> > > +	igt_assert_f(fd >= 0, "Can't open control\n");
> > > +
> > > +	igt_assert(read(fd, control, control_size - 1) > 0);
> > > +	strchomp(control);
> > > +
> > > +	igt_debug("Saved runtime power management for PCI
> > '%04x:%02x:%02x.%01x' '%s'\n",
> > > +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> > control);
> > > +	igt_install_exit_handler(__igt_pm_pci_card_exit_handler);
> > > +
> > > +	size = write(fd, "auto\n", 5);
> > > +	igt_assert(size == 5);
> > > +
> > > +	lseek(fd, 0, SEEK_SET);
> > > +	size = read(fd, buf, ARRAY_SIZE(buf));
> > > +	igt_assert(size == 5);
> > > +	igt_assert(strncmp(buf, "auto\n", 5) == 0);
> > > +
> > > +	close(fd);
> > > +}
> > > +
> > > +/**
> > > + * igt_pm_setup_pci_card_runtime_pm:
> > > + * @pci_dev: root port pci_dev.
> > > + * Setup runtime PM for all PCI endpoints devices for a given
> > > + * root port. It also save power control attribute for all PCI
> > > +endpoints
> > > + * devices for a given root port.
> > > + */
> > > +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev) {
> > > +	int primary, secondary, subordinate, ret;
> > > +	struct pci_device_iterator *iter;
> > > +	struct pci_device *dev;
> > > +	int i = 0;
> > > +
> > > +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> > > +	ret = pci_device_get_bridge_buses(pci_dev, &primary, &secondary,
> > &subordinate);
> > > +	igt_assert(!ret);
> > > +
> > > +	ret = pci_system_init();
> > > +	igt_assert(!ret);
> > > +
> > > +	iter = pci_slot_match_iterator_create(NULL);
> > > +	/* Setup runtime pm for PCI root port */
> > > +	__igt_pm_setup_pci_card_runtime_pm(pci_dev, i++);
> > > +	while ((dev = pci_device_next(iter)) != NULL) {
> > > +		if (dev->bus >= secondary && dev->bus <= subordinate)
> > > +			__igt_pm_setup_pci_card_runtime_pm(dev, i++);
> > > +	}
> > > +}
> > > +
> > > +static void igt_pm_restore_pci_dev_runtime_pm(struct pci_device
> > > +*pci_dev, char *control, int len) {
> > > +	char name[PATH_MAX];
> > > +	int fd;
> > > +
> > > +	igt_debug("Restoring runtime power management for PCI
> > '%04x:%02x:%02x.%01x' '%s'\n",
> > > +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> > > +control);
> > > +
> > > +	snprintf(name, PATH_MAX,
> > "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > > +
> > > +	fd = open(name, O_WRONLY);
> > > +	igt_assert_f(fd >= 0, "Can't open control\n");
> > > +
> > > +	igt_assert(write(fd, control, len) == len);
> > > +	close(fd);
> > > +}
> > > +
> > > +/**
> > > + * igt_pm_restore_pci_card_runtime_pm:
> > > + * @pci_dev: root port pci_dev.
> > > + * Restore power control attribute for all PCI endpoints devices for
> > > +a given
> > > + * root port.
> > > + */
> > > +void igt_pm_restore_pci_card_runtime_pm(void)
> > > +{
> > > +	int i = 0;
> > > +
> > > +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> > > +		if (!__igt_pm_pci_control[i].pci_dev)
> > > +			break;
> > > +
> > > +
> > 	igt_pm_restore_pci_dev_runtime_pm(__igt_pm_pci_control[i].pci_dev,
> > > +
> > __igt_pm_pci_control[i].control,
> > > +
> > sizeof(__igt_pm_pci_control[i].control));
> > > +	}
> > > +
> > > +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> > > +	pci_system_cleanup();
> > > +}
> > > +
> > > +static void igt_pm_print_pci_dev_runtime_status(struct pci_device
> > > +*pci_dev) {
> > > +	char name[PATH_MAX], runtime_status[64];
> > > +	int fd, n_read;
> > > +
> > > +	snprintf(name, PATH_MAX,
> > "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/runtime_status",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> > > +
> > > +	fd = open(name, O_RDONLY);
> > > +	igt_assert_f(fd >= 0, "Can't open runtime_status\n");
> > > +
> > > +	n_read = read(fd, runtime_status, sizeof(runtime_status) - 1);
> > > +	igt_assert(n_read >= 0);
> > > +	runtime_status[n_read] = '\0';
> > > +	igt_info("runtime suspend status for PCI '%04x:%02x:%02x.%01x' %s\n",
> > > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func,
> > runtime_status);
> > > +	close(fd);
> > > +}
> > > +
> > > +/**
> > > + * igt_pm_print_pci_card_runtime_status:
> > > + * @pci_dev: root port pci_dev.
> > > + * Print runtime suspend status for all PCI endpoints devices for a
> > > +given
> > > + * root port.
> > > + */
> > > +void igt_pm_print_pci_card_runtime_status(void)
> > > +{
> > > +	int i = 0;
> > > +
> > > +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> > > +		if (!__igt_pm_pci_control[i].pci_dev)
> > > +			break;
> > > +
> > > +
> > 	igt_pm_print_pci_dev_runtime_status(__igt_pm_pci_control[i].pci_dev)
> > ;
> > > +	}
> > > +}
> > > diff --git a/lib/igt_pm.h b/lib/igt_pm.h index 162d3ca3c..a5a9c4760
> > > 100644
> > > --- a/lib/igt_pm.h
> > > +++ b/lib/igt_pm.h
> > > @@ -46,6 +46,21 @@ enum igt_runtime_pm_status {
> > >  	IGT_RUNTIME_PM_STATUS_UNKNOWN,
> > >  };
> > >
> > > +/* PCI ACPI firmware node real state */ enum igt_acpi_d_state {
> > > +	IGT_ACPI_D0,
> > > +	IGT_ACPI_D1,
> > > +	IGT_ACPI_D2,
> > > +	IGT_ACPI_D3Hot,
> > > +	IGT_ACPI_D3Cold,
> > > +	IGT_ACPI_UNKNOWN_STATE,
> > > +};
> > > +
> > > +struct	igt_pm_pci_dev_control {
> > > +	struct pci_device *pci_dev;
> > > +	char control[64];
> > > +};
> > > +
> > >  bool igt_setup_runtime_pm(int device);  void
> > > igt_disable_runtime_pm(void);  void igt_restore_runtime_pm(void); @@
> > > -54,5 +69,11 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status
> > > status);  bool igt_pm_dmc_loaded(int debugfs);  bool
> > > igt_pm_pc8_plus_residencies_enabled(int msr_fd);  bool
> > > i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
> > > +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev); enum
> > > +igt_acpi_d_state igt_pm_get_acpi_real_d_state(struct pci_device
> > > +*pci_dev); void igt_pm_setup_pci_card_runtime_pm(struct pci_device
> > > +*pci_dev); void igt_pm_restore_pci_card_runtime_pm(void);
> > > +void igt_pm_print_pci_card_runtime_status(void);
> > >
> > >  #endif /* IGT_PM_H */
> > > --
> > > 2.26.2
> > >

Re: [igt-dev] [PATCH i-g-t 3/9] lib/igt_pm: D3Cold runtime pm infrastructure

[Kamil Konieczny]

Hi Anshuman,

one small nit, see below.

On 2022-04-18 at 18:20:42 +0530, Anshuman Gupta wrote:
> Enable gfx card pci devices runtime pm for all pci devices
> and bridge under the topology of Gfx Card root port.
> 
> Added a library function to get the PCI root port ACPI
> D state and to print the pci card devices runtime pm
> status.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  lib/igt_pm.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_pm.h |  21 +++++
>  2 files changed, 247 insertions(+)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index b409ec463..8e8a330f8 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -28,6 +28,7 @@
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <limits.h>
> +#include <pciaccess.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> @@ -75,6 +76,7 @@ enum {
>  #define MIN_POWER_STR		"min_power\n"
>  /* Remember to fix this if adding longer strings */
>  #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
> +#define MAX_PCI_DEVICES		256
>  int8_t *__sata_pm_policies;
>  int __scsi_host_cnt;
>  
> @@ -856,3 +858,227 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
>  
>  	return strstr(buf, "LPSP: capable");
>  }
> +
> +/**
> + * igt_pm_acpi_d3cold_suppoarted:
> + * @pci_dev: root port pci_dev.
> + * Check ACPI D3Cold support.
> + *
> + * Returns:
> + * True if ACPI D3Cold supported otherwise false.
> + */
> +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX];
> +	int dir, fd;
> +
> +	snprintf(name, PATH_MAX,
> +		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	dir = open(name, O_RDONLY);
> +	igt_require(dir > 0);
> +
> +	/* BIOS need to enable ACPI D3Cold Support.*/
> +	fd = openat(dir, "real_power_state", O_RDONLY);
> +	if (fd < 0 && errno == ENOENT)
> +		return false;
> +
> +	igt_require(fd > 0);
> +
> +	return true;
> +}
> +
> +/**
> + * igt_pm_get_acpi_real_d_state:
> + * @pci_dev: root port pci_dev.
> + * Get ACPI D state for a given root port.
> + *
> + * Returns:
> + * igt_acpi_d_state state.
> + */
> +enum igt_acpi_d_state
> +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX];
> +	char acpi_d_state[64];
> +	int fd, n_read;
> +
> +	snprintf(name, PATH_MAX,
> +		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node/real_power_state",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDONLY);
> +	if (fd < 0)
> +		return IGT_ACPI_UNKNOWN_STATE;
> +
> +	n_read = read(fd, acpi_d_state, sizeof(acpi_d_state) - 1);
> +	igt_assert(n_read >= 0);
> +	acpi_d_state[n_read] = '\0';
> +	close(fd);
> +
> +	if (strncmp(acpi_d_state, "D0\n", n_read) == 0)
> +		return IGT_ACPI_D0;
> +	else if  (strncmp(acpi_d_state, "D1\n", n_read) == 0)
------- ^
s/else //

There is no point in using "else" after return. The same edit
may be done below.

> +		return IGT_ACPI_D1;
> +	else if  (strncmp(acpi_d_state, "D2\n", n_read) == 0)
------- ^
> +		return IGT_ACPI_D2;
> +	else if  (strncmp(acpi_d_state, "D3hot\n", n_read) == 0)
------- ^
> +		return IGT_ACPI_D3Hot;
> +	else if  (strncmp(acpi_d_state, "D3cold\n", n_read) == 0)
------- ^
> +		return IGT_ACPI_D3Cold;
> +	else
------- ^

You may use Linux kernel checkpatch to check patch condition.

Regards,
Kamil

> +		return IGT_ACPI_UNKNOWN_STATE;
> +}
> +
> +static struct igt_pm_pci_dev_control  __igt_pm_pci_control[MAX_PCI_DEVICES];
> +
> +static void __igt_pm_pci_card_exit_handler(int sig)
> +{
> +	igt_pm_restore_pci_card_runtime_pm();
> +}
> +
> +static void
> +__igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev, int index)
> +{
> +	char name[PATH_MAX], buf[6];
> +	int fd, control_size, size;
> +	char *control;
> +
> +	igt_assert(index <  MAX_PCI_DEVICES);
> +
> +	control = __igt_pm_pci_control[index].control;
> +	control_size =  sizeof(__igt_pm_pci_control[index].control);
> +	__igt_pm_pci_control[index].pci_dev = pci_dev;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDWR);
> +	igt_assert_f(fd >= 0, "Can't open control\n");
> +
> +	igt_assert(read(fd, control, control_size - 1) > 0);
> +	strchomp(control);
> +
> +	igt_debug("Saved runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
> +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
> +	igt_install_exit_handler(__igt_pm_pci_card_exit_handler);
> +
> +	size = write(fd, "auto\n", 5);
> +	igt_assert(size == 5);
> +
> +	lseek(fd, 0, SEEK_SET);
> +	size = read(fd, buf, ARRAY_SIZE(buf));
> +	igt_assert(size == 5);
> +	igt_assert(strncmp(buf, "auto\n", 5) == 0);
> +
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_setup_pci_card_runtime_pm:
> + * @pci_dev: root port pci_dev.
> + * Setup runtime PM for all PCI endpoints devices for a given
> + * root port. It also save power control attribute for all PCI endpoints
> + * devices for a given root port.
> + */
> +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
> +{
> +	int primary, secondary, subordinate, ret;
> +	struct pci_device_iterator *iter;
> +	struct pci_device *dev;
> +	int i = 0;
> +
> +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> +	ret = pci_device_get_bridge_buses(pci_dev, &primary, &secondary, &subordinate);
> +	igt_assert(!ret);
> +
> +	ret = pci_system_init();
> +	igt_assert(!ret);
> +
> +	iter = pci_slot_match_iterator_create(NULL);
> +	/* Setup runtime pm for PCI root port */
> +	__igt_pm_setup_pci_card_runtime_pm(pci_dev, i++);
> +	while ((dev = pci_device_next(iter)) != NULL) {
> +		if (dev->bus >= secondary && dev->bus <= subordinate)
> +			__igt_pm_setup_pci_card_runtime_pm(dev, i++);
> +	}
> +}
> +
> +static void igt_pm_restore_pci_dev_runtime_pm(struct pci_device *pci_dev, char *control, int len)
> +{
> +	char name[PATH_MAX];
> +	int fd;
> +
> +	igt_debug("Restoring runtime power management for PCI '%04x:%02x:%02x.%01x' '%s'\n",
> +		  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, control);
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/control",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_WRONLY);
> +	igt_assert_f(fd >= 0, "Can't open control\n");
> +
> +	igt_assert(write(fd, control, len) == len);
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_restore_pci_card_runtime_pm:
> + * @pci_dev: root port pci_dev.
> + * Restore power control attribute for all PCI endpoints devices for a given
> + * root port.
> + */
> +void igt_pm_restore_pci_card_runtime_pm(void)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> +		if (!__igt_pm_pci_control[i].pci_dev)
> +			break;
> +
> +		igt_pm_restore_pci_dev_runtime_pm(__igt_pm_pci_control[i].pci_dev,
> +						  __igt_pm_pci_control[i].control,
> +						  sizeof(__igt_pm_pci_control[i].control));
> +	}
> +
> +	memset(__igt_pm_pci_control, 0, sizeof(__igt_pm_pci_control));
> +	pci_system_cleanup();
> +}
> +
> +static void igt_pm_print_pci_dev_runtime_status(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX], runtime_status[64];
> +	int fd, n_read;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/power/runtime_status",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	fd = open(name, O_RDONLY);
> +	igt_assert_f(fd >= 0, "Can't open runtime_status\n");
> +
> +	n_read = read(fd, runtime_status, sizeof(runtime_status) - 1);
> +	igt_assert(n_read >= 0);
> +	runtime_status[n_read] = '\0';
> +	igt_info("runtime suspend status for PCI '%04x:%02x:%02x.%01x' %s\n",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, runtime_status);
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_print_pci_card_runtime_status:
> + * @pci_dev: root port pci_dev.
> + * Print runtime suspend status for all PCI endpoints devices for a given
> + * root port.
> + */
> +void igt_pm_print_pci_card_runtime_status(void)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < MAX_PCI_DEVICES; i++) {
> +		if (!__igt_pm_pci_control[i].pci_dev)
> +			break;
> +
> +		igt_pm_print_pci_dev_runtime_status(__igt_pm_pci_control[i].pci_dev);
> +	}
> +}
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 162d3ca3c..a5a9c4760 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -46,6 +46,21 @@ enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_UNKNOWN,
>  };
>  
> +/* PCI ACPI firmware node real state */
> +enum igt_acpi_d_state {
> +	IGT_ACPI_D0,
> +	IGT_ACPI_D1,
> +	IGT_ACPI_D2,
> +	IGT_ACPI_D3Hot,
> +	IGT_ACPI_D3Cold,
> +	IGT_ACPI_UNKNOWN_STATE,
> +};
> +
> +struct	igt_pm_pci_dev_control {
> +	struct pci_device *pci_dev;
> +	char control[64];
> +};
> +
>  bool igt_setup_runtime_pm(int device);
>  void igt_disable_runtime_pm(void);
>  void igt_restore_runtime_pm(void);
> @@ -54,5 +69,11 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
>  bool igt_pm_dmc_loaded(int debugfs);
>  bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
>  bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
> +bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev);
> +enum igt_acpi_d_state
> +igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);
> +void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev);
> +void igt_pm_restore_pci_card_runtime_pm(void);
> +void igt_pm_print_pci_card_runtime_status(void);
>  
>  #endif /* IGT_PM_H */
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 4/9] lib/intel_device_info: Add IS_DGFX() support

[Anshuman Gupta]

Currently IGT is lacking IS_DGFX() macro support.
There are some power features like D3Cold are only
supported on discrete card. So IGT test/tools specific
to D3Cold requires to consume IS_DGFX().
Adding a is_dgfx field in intel_device_info and initializing
it for DG1. All future discrete platform would require to
initialize this field.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 lib/intel_chipset.h     | 2 ++
 lib/intel_device_info.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
index db75a829f..07fdd213e 100644
--- a/lib/intel_chipset.h
+++ b/lib/intel_chipset.h
@@ -42,6 +42,7 @@ struct intel_device_info {
 	unsigned gt; /* 0 if unknown */
 	bool has_4tile : 1;
 	bool has_flatccs;
+	bool is_dgfx :1;
 	bool is_mobile : 1;
 	bool is_whitney : 1;
 	bool is_almador : 1;
@@ -208,6 +209,7 @@ void intel_check_pch(void);
 
 #define IS_MOBILE(devid)	(intel_get_device_info(devid)->is_mobile)
 #define IS_965(devid)		AT_LEAST_GEN(devid, 4)
+#define IS_DGFX(devid)	(intel_get_device_info(devid)->is_dgfx)
 
 #define HAS_BSD_RING(devid)	AT_LEAST_GEN(devid, 5)
 #define HAS_BLT_RING(devid)	AT_LEAST_GEN(devid, 6)
diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
index e55841df5..b735f16a6 100644
--- a/lib/intel_device_info.c
+++ b/lib/intel_device_info.c
@@ -386,6 +386,7 @@ static const struct intel_device_info intel_rocketlake_info = {
 static const struct intel_device_info intel_dg1_info = {
 	.graphics_ver = 12,
 	.display_ver = 12,
+	.is_dgfx = true,
 	.is_dg1 = true,
 	.codename = "dg1"
 };
@@ -394,6 +395,7 @@ static const struct intel_device_info intel_dg2_info = {
 	.graphics_ver = 12,
 	.display_ver = 13,
 	.has_4tile = true,
+	.is_dgfx = true,
 	.is_dg2 = true,
 	.codename = "dg2",
 	.has_flatccs = true,
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 4/9] lib/intel_device_info: Add IS_DGFX() support

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:43PM +0530, Anshuman Gupta wrote:
> Currently IGT is lacking IS_DGFX() macro support.
> There are some power features like D3Cold are only
> supported on discrete card. So IGT test/tools specific
> to D3Cold requires to consume IS_DGFX().
> Adding a is_dgfx field in intel_device_info and initializing
> it for DG1. All future discrete platform would require to
> initialize this field.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

we also need this one for other stuff, so probably better to split and already merge this...

> ---
>  lib/intel_chipset.h     | 2 ++
>  lib/intel_device_info.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
> index db75a829f..07fdd213e 100644
> --- a/lib/intel_chipset.h
> +++ b/lib/intel_chipset.h
> @@ -42,6 +42,7 @@ struct intel_device_info {
>  	unsigned gt; /* 0 if unknown */
>  	bool has_4tile : 1;
>  	bool has_flatccs;
> +	bool is_dgfx :1;
>  	bool is_mobile : 1;
>  	bool is_whitney : 1;
>  	bool is_almador : 1;
> @@ -208,6 +209,7 @@ void intel_check_pch(void);
>  
>  #define IS_MOBILE(devid)	(intel_get_device_info(devid)->is_mobile)
>  #define IS_965(devid)		AT_LEAST_GEN(devid, 4)
> +#define IS_DGFX(devid)	(intel_get_device_info(devid)->is_dgfx)
>  
>  #define HAS_BSD_RING(devid)	AT_LEAST_GEN(devid, 5)
>  #define HAS_BLT_RING(devid)	AT_LEAST_GEN(devid, 6)
> diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
> index e55841df5..b735f16a6 100644
> --- a/lib/intel_device_info.c
> +++ b/lib/intel_device_info.c
> @@ -386,6 +386,7 @@ static const struct intel_device_info intel_rocketlake_info = {
>  static const struct intel_device_info intel_dg1_info = {
>  	.graphics_ver = 12,
>  	.display_ver = 12,
> +	.is_dgfx = true,
>  	.is_dg1 = true,
>  	.codename = "dg1"
>  };
> @@ -394,6 +395,7 @@ static const struct intel_device_info intel_dg2_info = {
>  	.graphics_ver = 12,
>  	.display_ver = 13,
>  	.has_4tile = true,
> +	.is_dgfx = true,
>  	.is_dg2 = true,
>  	.codename = "dg2",
>  	.has_flatccs = true,
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Anshuman Gupta]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6584 bytes --]

intel_pm_rpm tool is a debug tool. It can be use to setup
and prepare the gfx card to go to D3Cold.
It also provide the debug option to disable all display and
prepare device to enter to runtime suspend.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tools/intel_pm_rpm.c | 209 +++++++++++++++++++++++++++++++++++++++++++
 tools/meson.build    |   1 +
 2 files changed, 210 insertions(+)
 create mode 100644 tools/intel_pm_rpm.c

diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c
new file mode 100644
index 000000000..df9cfa632
--- /dev/null
+++ b/tools/intel_pm_rpm.c
@@ -0,0 +1,209 @@
+/*
+ * Copyright © 2022 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <glib.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+#include "igt.h"
+#include "igt_device.h"
+#include "igt_device_scan.h"
+#include "igt_pm.h"
+
+typedef struct {
+	int drm_fd;
+	int debugfs_fd;
+	uint32_t devid;
+	drmModeResPtr res;
+	igt_display_t display;
+} data_t;
+
+const char *help_str =
+	"  --disable-display\t\tDisable all screen and try to go into runtime pm.\n"
+	"  --setup-d3cold\t\tPrepare dgfx gfx card to enter runtime D3Cold.\n"
+	"  --help\t\tProvide help. Provide card name with IGT_DEVICE=drm:/dev/dri/card*.";
+static struct option long_options[] = {
+	{"disable-display", 0, 0, 'd'},
+	{"setup-d3cold", 0, 0, 's'},
+	{"help", 0, 0, 'h'},
+	{ 0, 0, 0, 0 }
+};
+
+const char *optstr = "dsh";
+
+static void usage(const char *name)
+{
+	igt_info("Usage: %s [options]\n", name);
+	igt_info("%s\n", help_str);
+}
+
+static void disable_all_displays(data_t *data)
+{
+	igt_output_t *output;
+
+	for (int i = 0; i < data->display.n_outputs; i++) {
+		output = &data->display.outputs[i];
+		igt_output_set_pipe(output, PIPE_NONE);
+		igt_display_commit(&data->display);
+	}
+}
+
+static void setup_gfx_card_d3cold(data_t *data)
+{
+	struct pci_device *root;
+	int d_state;
+
+	/* igfx does not support d3cold */
+	if (!IS_DGFX(data->devid))
+		return;
+
+	root = igt_device_get_pci_root_port(data->drm_fd);
+
+	if (!igt_pm_acpi_d3cold_supported(root)) {
+		igt_info("D3Cold isn't supported on Root port %04x:%02x:%02x.%01x\n",
+			 root->domain, root->bus, root->dev, root->func);
+		return;
+	}
+
+	disable_all_displays(data);
+	igt_pm_setup_pci_card_runtime_pm(root);
+	sleep(1);
+	d_state = igt_pm_get_acpi_real_d_state(root);
+
+	if (d_state == IGT_ACPI_D3Cold) {
+		igt_info("D3Cold achieved for root port %04x:%02x:%02x.%01x\n",
+			 root->domain, root->bus, root->dev, root->func);
+	} else {
+		igt_pm_print_pci_card_runtime_status();
+		igt_info("D3Cold not achieved yet. Please monitor %04x:%02x:%02x.%01x real_power_state\n",
+			 root->domain, root->bus, root->dev, root->func);
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	bool disable_display = false, setup_d3cold = false;
+	struct igt_device_card card;
+	char *env_device = NULL;
+	int c, option_index = 0;
+	data_t data = {};
+	int ret = 0;
+
+	if (argc <= 1) {
+		usage(argv[0]);
+		return EXIT_SUCCESS;
+	}
+
+	env_device = getenv("IGT_DEVICE");
+	igt_devices_scan(false);
+
+	if (env_device) {
+		if (!igt_device_card_match(env_device, &card)) {
+			igt_warn("No device found for the env_device\n");
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+	} else {
+		if (!igt_device_find_first_i915_discrete_card(&card)) {
+			igt_warn("No discrete gpu found\n");
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+	}
+
+	while ((c = getopt_long(argc, argv, optstr,
+				long_options, &option_index)) != -1) {
+		switch (c) {
+		case 'd':
+			disable_display = true;
+			break;
+		case 's':
+			setup_d3cold = true;
+			break;
+		default:
+		case 'h':
+			usage(argv[0]);
+			ret = EXIT_SUCCESS;
+			goto exit;
+		}
+	}
+
+	data.drm_fd = igt_open_card(&card);
+	if (data.drm_fd  >= 0) {
+		igt_info("Device %s successfully opened\n", card.card);
+	} else {
+		igt_warn("Cannot open card %s device\n", card.card);
+		ret = EXIT_FAILURE;
+		goto exit;
+	}
+
+	data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+	data.devid = intel_get_drm_devid(data.drm_fd);
+	igt_setup_runtime_pm(data.drm_fd);
+
+	data.res = drmModeGetResources(data.drm_fd);
+	if (data.res) {
+		kmstest_set_vt_graphics_mode();
+		igt_display_require(&data.display, data.drm_fd);
+
+		if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
+			igt_warn("dmc fw is not loaded, no runtime pm\n");
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+	}
+
+	if (disable_display) {
+		disable_all_displays(&data);
+		if (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
+			__igt_debugfs_dump(data.drm_fd, "i915_runtime_pm_status", IGT_LOG_INFO);
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+
+		igt_info("Device runtime suspended, Useful for debugging.\n"
+			 "Hit CTRL-C to exit\n");
+		/* Don't return useful for debugging */
+		while (1)
+			sleep(600);
+	}
+
+	if (setup_d3cold)
+		setup_gfx_card_d3cold(&data);
+
+exit:
+	igt_restore_runtime_pm();
+
+	if (data.res)
+		igt_display_fini(&data.display);
+
+	close(data.debugfs_fd);
+	close(data.drm_fd);
+	igt_devices_free();
+
+	return ret;
+}
diff --git a/tools/meson.build b/tools/meson.build
index 771d0b9e3..24d0ea714 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -28,6 +28,7 @@ tools_progs = [
 	'intel_lid',
 	'intel_opregion_decode',
 	'intel_panel_fitter',
+	'intel_pm_rpm',
 	'intel_reg_checker',
 	'intel_residency',
 	'intel_stepping',
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> intel_pm_rpm tool is a debug tool. It can be use to setup
> and prepare the gfx card to go to D3Cold.
> It also provide the debug option to disable all display and
> prepare device to enter to runtime suspend.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tools/intel_pm_rpm.c | 209 +++++++++++++++++++++++++++++++++++++++++++
>  tools/meson.build    |   1 +
>  2 files changed, 210 insertions(+)
>  create mode 100644 tools/intel_pm_rpm.c
> 
> diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c
> new file mode 100644
> index 000000000..df9cfa632
> --- /dev/null
> +++ b/tools/intel_pm_rpm.c
> @@ -0,0 +1,209 @@
> +/*
> + * Copyright © 2022 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <errno.h>
> +#include <getopt.h>
> +#include <glib.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include "igt.h"
> +#include "igt_device.h"
> +#include "igt_device_scan.h"
> +#include "igt_pm.h"
> +
> +typedef struct {
> +	int drm_fd;
> +	int debugfs_fd;
> +	uint32_t devid;
> +	drmModeResPtr res;
> +	igt_display_t display;
> +} data_t;
> +
> +const char *help_str =
> +	"  --disable-display\t\tDisable all screen and try to go into runtime pm.\n"
> +	"  --setup-d3cold\t\tPrepare dgfx gfx card to enter runtime D3Cold.\n"
> +	"  --help\t\tProvide help. Provide card name with IGT_DEVICE=drm:/dev/dri/card*.";
> +static struct option long_options[] = {
> +	{"disable-display", 0, 0, 'd'},
> +	{"setup-d3cold", 0, 0, 's'},
> +	{"help", 0, 0, 'h'},
> +	{ 0, 0, 0, 0 }
> +};
> +
> +const char *optstr = "dsh";
> +
> +static void usage(const char *name)
> +{
> +	igt_info("Usage: %s [options]\n", name);
> +	igt_info("%s\n", help_str);
> +}
> +
> +static void disable_all_displays(data_t *data)
> +{
> +	igt_output_t *output;
> +
> +	for (int i = 0; i < data->display.n_outputs; i++) {
> +		output = &data->display.outputs[i];
> +		igt_output_set_pipe(output, PIPE_NONE);
> +		igt_display_commit(&data->display);
> +	}
> +}
> +
> +static void setup_gfx_card_d3cold(data_t *data)
> +{
> +	struct pci_device *root;
> +	int d_state;
> +
> +	/* igfx does not support d3cold */
> +	if (!IS_DGFX(data->devid))
> +		return;

I believe the if below will already take care of this since the real_power_state
won't show up. However, let's keep this check here to be really clear on the
expectations and to avoid navigating the tree in vain...

> +
> +	root = igt_device_get_pci_root_port(data->drm_fd);
> +
> +	if (!igt_pm_acpi_d3cold_supported(root)) {
> +		igt_info("D3Cold isn't supported on Root port %04x:%02x:%02x.%01x\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +		return;
> +	}
> +
> +	disable_all_displays(data);
> +	igt_pm_setup_pci_card_runtime_pm(root);
> +	sleep(1);
> +	d_state = igt_pm_get_acpi_real_d_state(root);
> +
> +	if (d_state == IGT_ACPI_D3Cold) {
> +		igt_info("D3Cold achieved for root port %04x:%02x:%02x.%01x\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +	} else {
> +		igt_pm_print_pci_card_runtime_status();
> +		igt_info("D3Cold not achieved yet. Please monitor %04x:%02x:%02x.%01x real_power_state\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +	}
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	bool disable_display = false, setup_d3cold = false;
> +	struct igt_device_card card;
> +	char *env_device = NULL;
> +	int c, option_index = 0;
> +	data_t data = {};
> +	int ret = 0;
> +
> +	if (argc <= 1) {
> +		usage(argv[0]);
> +		return EXIT_SUCCESS;
> +	}
> +
> +	env_device = getenv("IGT_DEVICE");
> +	igt_devices_scan(false);
> +
> +	if (env_device) {
> +		if (!igt_device_card_match(env_device, &card)) {
> +			igt_warn("No device found for the env_device\n");
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +	} else {
> +		if (!igt_device_find_first_i915_discrete_card(&card)) {
> +			igt_warn("No discrete gpu found\n");
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +	}
> +
> +	while ((c = getopt_long(argc, argv, optstr,
> +				long_options, &option_index)) != -1) {
> +		switch (c) {
> +		case 'd':
> +			disable_display = true;
> +			break;
> +		case 's':
> +			setup_d3cold = true;
> +			break;
> +		default:
> +		case 'h':
> +			usage(argv[0]);
> +			ret = EXIT_SUCCESS;
> +			goto exit;
> +		}
> +	}
> +
> +	data.drm_fd = igt_open_card(&card);
> +	if (data.drm_fd  >= 0) {
> +		igt_info("Device %s successfully opened\n", card.card);
> +	} else {
> +		igt_warn("Cannot open card %s device\n", card.card);
> +		ret = EXIT_FAILURE;
> +		goto exit;
> +	}
> +
> +	data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> +	data.devid = intel_get_drm_devid(data.drm_fd);
> +	igt_setup_runtime_pm(data.drm_fd);
> +
> +	data.res = drmModeGetResources(data.drm_fd);
> +	if (data.res) {
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_require(&data.display, data.drm_fd);
> +
> +		if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> +			igt_warn("dmc fw is not loaded, no runtime pm\n");
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +	}

Do we really need this? It is only for checking if DMC is there in case we have display right?
Should deserve a comment at least...

but already can use my:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

if explained with reply here or with comment if you see it fits

> +
> +	if (disable_display) {
> +		disable_all_displays(&data);
> +		if (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> +			__igt_debugfs_dump(data.drm_fd, "i915_runtime_pm_status", IGT_LOG_INFO);
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +
> +		igt_info("Device runtime suspended, Useful for debugging.\n"
> +			 "Hit CTRL-C to exit\n");
> +		/* Don't return useful for debugging */
> +		while (1)
> +			sleep(600);
> +	}
> +
> +	if (setup_d3cold)
> +		setup_gfx_card_d3cold(&data);
> +
> +exit:
> +	igt_restore_runtime_pm();
> +
> +	if (data.res)
> +		igt_display_fini(&data.display);
> +
> +	close(data.debugfs_fd);
> +	close(data.drm_fd);
> +	igt_devices_free();
> +
> +	return ret;
> +}
> diff --git a/tools/meson.build b/tools/meson.build
> index 771d0b9e3..24d0ea714 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -28,6 +28,7 @@ tools_progs = [
>  	'intel_lid',
>  	'intel_opregion_decode',
>  	'intel_panel_fitter',
> +	'intel_pm_rpm',
>  	'intel_reg_checker',
>  	'intel_residency',
>  	'intel_stepping',
> -- 
> 2.26.2
> 

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Rodrigo Vivi]

On Fri, Apr 22, 2022 at 06:22:28AM -0400, Rodrigo Vivi wrote:
> On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> > intel_pm_rpm tool is a debug tool. It can be use to setup
> > and prepare the gfx card to go to D3Cold.
> > It also provide the debug option to disable all display and
> > prepare device to enter to runtime suspend.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  tools/intel_pm_rpm.c | 209 +++++++++++++++++++++++++++++++++++++++++++
> >  tools/meson.build    |   1 +
> >  2 files changed, 210 insertions(+)
> >  create mode 100644 tools/intel_pm_rpm.c
> > 
> > diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c
> > new file mode 100644
> > index 000000000..df9cfa632
> > --- /dev/null
> > +++ b/tools/intel_pm_rpm.c
> > @@ -0,0 +1,209 @@
> > +/*
> > + * Copyright © 2022 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +
> > +#include <errno.h>
> > +#include <getopt.h>
> > +#include <glib.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include "igt.h"
> > +#include "igt_device.h"
> > +#include "igt_device_scan.h"
> > +#include "igt_pm.h"
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	uint32_t devid;
> > +	drmModeResPtr res;
> > +	igt_display_t display;
> > +} data_t;
> > +
> > +const char *help_str =
> > +	"  --disable-display\t\tDisable all screen and try to go into runtime pm.\n"
> > +	"  --setup-d3cold\t\tPrepare dgfx gfx card to enter runtime D3Cold.\n"
> > +	"  --help\t\tProvide help. Provide card name with IGT_DEVICE=drm:/dev/dri/card*.";
> > +static struct option long_options[] = {
> > +	{"disable-display", 0, 0, 'd'},
> > +	{"setup-d3cold", 0, 0, 's'},
> > +	{"help", 0, 0, 'h'},
> > +	{ 0, 0, 0, 0 }
> > +};
> > +
> > +const char *optstr = "dsh";
> > +
> > +static void usage(const char *name)
> > +{
> > +	igt_info("Usage: %s [options]\n", name);
> > +	igt_info("%s\n", help_str);
> > +}
> > +
> > +static void disable_all_displays(data_t *data)
> > +{
> > +	igt_output_t *output;
> > +
> > +	for (int i = 0; i < data->display.n_outputs; i++) {
> > +		output = &data->display.outputs[i];
> > +		igt_output_set_pipe(output, PIPE_NONE);
> > +		igt_display_commit(&data->display);
> > +	}
> > +}
> > +
> > +static void setup_gfx_card_d3cold(data_t *data)
> > +{
> > +	struct pci_device *root;
> > +	int d_state;
> > +
> > +	/* igfx does not support d3cold */
> > +	if (!IS_DGFX(data->devid))
> > +		return;
> 
> I believe the if below will already take care of this since the real_power_state
> won't show up. However, let's keep this check here to be really clear on the
> expectations and to avoid navigating the tree in vain...
> 
> > +
> > +	root = igt_device_get_pci_root_port(data->drm_fd);
> > +
> > +	if (!igt_pm_acpi_d3cold_supported(root)) {
> > +		igt_info("D3Cold isn't supported on Root port %04x:%02x:%02x.%01x\n",
> > +			 root->domain, root->bus, root->dev, root->func);
> > +		return;
> > +	}
> > +
> > +	disable_all_displays(data);
> > +	igt_pm_setup_pci_card_runtime_pm(root);
> > +	sleep(1);
> > +	d_state = igt_pm_get_acpi_real_d_state(root);
> > +
> > +	if (d_state == IGT_ACPI_D3Cold) {
> > +		igt_info("D3Cold achieved for root port %04x:%02x:%02x.%01x\n",
> > +			 root->domain, root->bus, root->dev, root->func);
> > +	} else {
> > +		igt_pm_print_pci_card_runtime_status();
> > +		igt_info("D3Cold not achieved yet. Please monitor %04x:%02x:%02x.%01x real_power_state\n",
> > +			 root->domain, root->bus, root->dev, root->func);


oh, I had forgot to send comment on this one...
I believe that with sleep(1) above you are always reaching to this msg here, no?!

Should we also change the autosuspend from everyone to 1 and maybe wait 2 here?

> > +	}
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	bool disable_display = false, setup_d3cold = false;
> > +	struct igt_device_card card;
> > +	char *env_device = NULL;
> > +	int c, option_index = 0;
> > +	data_t data = {};
> > +	int ret = 0;
> > +
> > +	if (argc <= 1) {
> > +		usage(argv[0]);
> > +		return EXIT_SUCCESS;
> > +	}
> > +
> > +	env_device = getenv("IGT_DEVICE");
> > +	igt_devices_scan(false);
> > +
> > +	if (env_device) {
> > +		if (!igt_device_card_match(env_device, &card)) {
> > +			igt_warn("No device found for the env_device\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	} else {
> > +		if (!igt_device_find_first_i915_discrete_card(&card)) {
> > +			igt_warn("No discrete gpu found\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	}
> > +
> > +	while ((c = getopt_long(argc, argv, optstr,
> > +				long_options, &option_index)) != -1) {
> > +		switch (c) {
> > +		case 'd':
> > +			disable_display = true;
> > +			break;
> > +		case 's':
> > +			setup_d3cold = true;
> > +			break;
> > +		default:
> > +		case 'h':
> > +			usage(argv[0]);
> > +			ret = EXIT_SUCCESS;
> > +			goto exit;
> > +		}
> > +	}
> > +
> > +	data.drm_fd = igt_open_card(&card);
> > +	if (data.drm_fd  >= 0) {
> > +		igt_info("Device %s successfully opened\n", card.card);
> > +	} else {
> > +		igt_warn("Cannot open card %s device\n", card.card);
> > +		ret = EXIT_FAILURE;
> > +		goto exit;
> > +	}
> > +
> > +	data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > +	data.devid = intel_get_drm_devid(data.drm_fd);
> > +	igt_setup_runtime_pm(data.drm_fd);
> > +
> > +	data.res = drmModeGetResources(data.drm_fd);
> > +	if (data.res) {
> > +		kmstest_set_vt_graphics_mode();
> > +		igt_display_require(&data.display, data.drm_fd);
> > +
> > +		if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> > +			igt_warn("dmc fw is not loaded, no runtime pm\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	}
> 
> Do we really need this? It is only for checking if DMC is there in case we have display right?
> Should deserve a comment at least...
> 
> but already can use my:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> if explained with reply here or with comment if you see it fits
> 
> > +
> > +	if (disable_display) {
> > +		disable_all_displays(&data);
> > +		if (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> > +			__igt_debugfs_dump(data.drm_fd, "i915_runtime_pm_status", IGT_LOG_INFO);
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +
> > +		igt_info("Device runtime suspended, Useful for debugging.\n"
> > +			 "Hit CTRL-C to exit\n");
> > +		/* Don't return useful for debugging */
> > +		while (1)
> > +			sleep(600);
> > +	}
> > +
> > +	if (setup_d3cold)
> > +		setup_gfx_card_d3cold(&data);
> > +
> > +exit:
> > +	igt_restore_runtime_pm();
> > +
> > +	if (data.res)
> > +		igt_display_fini(&data.display);
> > +
> > +	close(data.debugfs_fd);
> > +	close(data.drm_fd);
> > +	igt_devices_free();
> > +
> > +	return ret;
> > +}
> > diff --git a/tools/meson.build b/tools/meson.build
> > index 771d0b9e3..24d0ea714 100644
> > --- a/tools/meson.build
> > +++ b/tools/meson.build
> > @@ -28,6 +28,7 @@ tools_progs = [
> >  	'intel_lid',
> >  	'intel_opregion_decode',
> >  	'intel_panel_fitter',
> > +	'intel_pm_rpm',
> >  	'intel_reg_checker',
> >  	'intel_residency',
> >  	'intel_stepping',
> > -- 
> > 2.26.2
> > 

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Friday, April 22, 2022 5:38 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal <badal.nilawar@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool
> 
> On Fri, Apr 22, 2022 at 06:22:28AM -0400, Rodrigo Vivi wrote:
> > On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> > > intel_pm_rpm tool is a debug tool. It can be use to setup and
> > > prepare the gfx card to go to D3Cold.
> > > It also provide the debug option to disable all display and prepare
> > > device to enter to runtime suspend.
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  tools/intel_pm_rpm.c | 209
> +++++++++++++++++++++++++++++++++++++++++++
> > >  tools/meson.build    |   1 +
> > >  2 files changed, 210 insertions(+)
> > >  create mode 100644 tools/intel_pm_rpm.c
> > >
> > > diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c new file
> > > mode 100644 index 000000000..df9cfa632
> > > --- /dev/null
> > > +++ b/tools/intel_pm_rpm.c
> > > @@ -0,0 +1,209 @@
> > > +/*
> > > + * Copyright (c) 2022 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> > > +obtaining a
> > > + * copy of this software and associated documentation files (the
> > > +"Software"),
> > > + * to deal in the Software without restriction, including without
> > > +limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > +sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to
> > > +whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including
> > > +the next
> > > + * paragraph) shall be included in all copies or substantial
> > > +portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> > > +EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > +MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > > +EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > +DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> OTHERWISE,
> > > +ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> OR
> > > +OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + */
> > > +
> > > +#include <errno.h>
> > > +#include <getopt.h>
> > > +#include <glib.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <stdbool.h>
> > > +#include "igt.h"
> > > +#include "igt_device.h"
> > > +#include "igt_device_scan.h"
> > > +#include "igt_pm.h"
> > > +
> > > +typedef struct {
> > > +	int drm_fd;
> > > +	int debugfs_fd;
> > > +	uint32_t devid;
> > > +	drmModeResPtr res;
> > > +	igt_display_t display;
> > > +} data_t;
> > > +
> > > +const char *help_str =
> > > +	"  --disable-display\t\tDisable all screen and try to go into runtime
> pm.\n"
> > > +	"  --setup-d3cold\t\tPrepare dgfx gfx card to enter runtime D3Cold.\n"
> > > +	"  --help\t\tProvide help. Provide card name with
> > > +IGT_DEVICE=drm:/dev/dri/card*."; static struct option long_options[] = {
> > > +	{"disable-display", 0, 0, 'd'},
> > > +	{"setup-d3cold", 0, 0, 's'},
> > > +	{"help", 0, 0, 'h'},
> > > +	{ 0, 0, 0, 0 }
> > > +};
> > > +
> > > +const char *optstr = "dsh";
> > > +
> > > +static void usage(const char *name) {
> > > +	igt_info("Usage: %s [options]\n", name);
> > > +	igt_info("%s\n", help_str);
> > > +}
> > > +
> > > +static void disable_all_displays(data_t *data) {
> > > +	igt_output_t *output;
> > > +
> > > +	for (int i = 0; i < data->display.n_outputs; i++) {
> > > +		output = &data->display.outputs[i];
> > > +		igt_output_set_pipe(output, PIPE_NONE);
> > > +		igt_display_commit(&data->display);
> > > +	}
> > > +}
> > > +
> > > +static void setup_gfx_card_d3cold(data_t *data) {
> > > +	struct pci_device *root;
> > > +	int d_state;
> > > +
> > > +	/* igfx does not support d3cold */
> > > +	if (!IS_DGFX(data->devid))
> > > +		return;
> >
> > I believe the if below will already take care of this since the
> > real_power_state won't show up. However, let's keep this check here to
> > be really clear on the expectations and to avoid navigating the tree in vain...
> >
> > > +
> > > +	root = igt_device_get_pci_root_port(data->drm_fd);
> > > +
> > > +	if (!igt_pm_acpi_d3cold_supported(root)) {
> > > +		igt_info("D3Cold isn't supported on Root port
> %04x:%02x:%02x.%01x\n",
> > > +			 root->domain, root->bus, root->dev, root->func);
> > > +		return;
> > > +	}
> > > +
> > > +	disable_all_displays(data);
> > > +	igt_pm_setup_pci_card_runtime_pm(root);
> > > +	sleep(1);
> > > +	d_state = igt_pm_get_acpi_real_d_state(root);
> > > +
> > > +	if (d_state == IGT_ACPI_D3Cold) {
> > > +		igt_info("D3Cold achieved for root port
> %04x:%02x:%02x.%01x\n",
> > > +			 root->domain, root->bus, root->dev, root->func);
> > > +	} else {
> > > +		igt_pm_print_pci_card_runtime_status();
> > > +		igt_info("D3Cold not achieved yet. Please monitor
> %04x:%02x:%02x.%01x real_power_state\n",
> > > +			 root->domain, root->bus, root->dev, root->func);
> 
> 
> oh, I had forgot to send comment on this one...
> I believe that with sleep(1) above you are always reaching to this msg here, no?!
> 
> Should we also change the autosuspend from everyone to 1 and maybe wait 2
> here?
May be 0 should be ok to write all devices auto suspend delay.
The problem is here writing 1 will conflict with existing i915 runtime pm setup layer,
Which writes 0 to i915 auto suspend delay.

Another issue I had observed while writing auto suspend delay ,
Few pci devices doesn't support auto-suspend.
----------------------------------------------------------------------------------
ta@DUT2135-DG2MRB:~/drivers.gpu.i915.igt-gpu-tools$ sudo ./build/tools/intel_pm_rpm --s
Device /dev/dri/card1 successfully opened
DMC: fw loaded: yes
Test requirement not met in function __igt_pm_setup_pci_card_runtime_pm, file ../lib/igt_pm.c:969:
Test requirement: read(delay_fd, delay, delay_size - 1) > 0
PCI '0000:02:01.0' doesn't support auto_suspend
Last errno: 5, Input/output error
---------------------------------------------------------------------------------- 

Br,
Anshuman Gupta.
> 
> > > +	}
> > > +}
> > > +
> > > +int main(int argc, char *argv[])
> > > +{
> > > +	bool disable_display = false, setup_d3cold = false;
> > > +	struct igt_device_card card;
> > > +	char *env_device = NULL;
> > > +	int c, option_index = 0;
> > > +	data_t data = {};
> > > +	int ret = 0;
> > > +
> > > +	if (argc <= 1) {
> > > +		usage(argv[0]);
> > > +		return EXIT_SUCCESS;
> > > +	}
> > > +
> > > +	env_device = getenv("IGT_DEVICE");
> > > +	igt_devices_scan(false);
> > > +
> > > +	if (env_device) {
> > > +		if (!igt_device_card_match(env_device, &card)) {
> > > +			igt_warn("No device found for the env_device\n");
> > > +			ret = EXIT_FAILURE;
> > > +			goto exit;
> > > +		}
> > > +	} else {
> > > +		if (!igt_device_find_first_i915_discrete_card(&card)) {
> > > +			igt_warn("No discrete gpu found\n");
> > > +			ret = EXIT_FAILURE;
> > > +			goto exit;
> > > +		}
> > > +	}
> > > +
> > > +	while ((c = getopt_long(argc, argv, optstr,
> > > +				long_options, &option_index)) != -1) {
> > > +		switch (c) {
> > > +		case 'd':
> > > +			disable_display = true;
> > > +			break;
> > > +		case 's':
> > > +			setup_d3cold = true;
> > > +			break;
> > > +		default:
> > > +		case 'h':
> > > +			usage(argv[0]);
> > > +			ret = EXIT_SUCCESS;
> > > +			goto exit;
> > > +		}
> > > +	}
> > > +
> > > +	data.drm_fd = igt_open_card(&card);
> > > +	if (data.drm_fd  >= 0) {
> > > +		igt_info("Device %s successfully opened\n", card.card);
> > > +	} else {
> > > +		igt_warn("Cannot open card %s device\n", card.card);
> > > +		ret = EXIT_FAILURE;
> > > +		goto exit;
> > > +	}
> > > +
> > > +	data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > > +	data.devid = intel_get_drm_devid(data.drm_fd);
> > > +	igt_setup_runtime_pm(data.drm_fd);
> > > +
> > > +	data.res = drmModeGetResources(data.drm_fd);
> > > +	if (data.res) {
> > > +		kmstest_set_vt_graphics_mode();
> > > +		igt_display_require(&data.display, data.drm_fd);
> > > +
> > > +		if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> > > +			igt_warn("dmc fw is not loaded, no runtime pm\n");
> > > +			ret = EXIT_FAILURE;
> > > +			goto exit;
> > > +		}
> > > +	}
> >
> > Do we really need this? It is only for checking if DMC is there in case we have
> display right?
> > Should deserve a comment at least...
> >
> > but already can use my:
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > if explained with reply here or with comment if you see it fits
> >
> > > +
> > > +	if (disable_display) {
> > > +		disable_all_displays(&data);
> > > +		if
> (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> > > +			__igt_debugfs_dump(data.drm_fd,
> "i915_runtime_pm_status", IGT_LOG_INFO);
> > > +			ret = EXIT_FAILURE;
> > > +			goto exit;
> > > +		}
> > > +
> > > +		igt_info("Device runtime suspended, Useful for debugging.\n"
> > > +			 "Hit CTRL-C to exit\n");
> > > +		/* Don't return useful for debugging */
> > > +		while (1)
> > > +			sleep(600);
> > > +	}
> > > +
> > > +	if (setup_d3cold)
> > > +		setup_gfx_card_d3cold(&data);
> > > +
> > > +exit:
> > > +	igt_restore_runtime_pm();
> > > +
> > > +	if (data.res)
> > > +		igt_display_fini(&data.display);
> > > +
> > > +	close(data.debugfs_fd);
> > > +	close(data.drm_fd);
> > > +	igt_devices_free();
> > > +
> > > +	return ret;
> > > +}
> > > diff --git a/tools/meson.build b/tools/meson.build index
> > > 771d0b9e3..24d0ea714 100644
> > > --- a/tools/meson.build
> > > +++ b/tools/meson.build
> > > @@ -28,6 +28,7 @@ tools_progs = [
> > >  	'intel_lid',
> > >  	'intel_opregion_decode',
> > >  	'intel_panel_fitter',
> > > +	'intel_pm_rpm',
> > >  	'intel_reg_checker',
> > >  	'intel_residency',
> > >  	'intel_stepping',
> > > --
> > > 2.26.2
> > >

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Vivi, Rodrigo]

On Wed, 2022-04-27 at 23:59 -0700, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Sent: Friday, April 22, 2022 5:38 PM
> > To: Gupta, Anshuman <anshuman.gupta@intel.com>
> > Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal
> > <badal.nilawar@intel.com>
> > Subject: Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm
> > tool
> > 
> > On Fri, Apr 22, 2022 at 06:22:28AM -0400, Rodrigo Vivi wrote:
> > > On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> > > > intel_pm_rpm tool is a debug tool. It can be use to setup and
> > > > prepare the gfx card to go to D3Cold.
> > > > It also provide the debug option to disable all display and
> > > > prepare
> > > > device to enter to runtime suspend.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > >  tools/intel_pm_rpm.c | 209
> > +++++++++++++++++++++++++++++++++++++++++++
> > > >  tools/meson.build    |   1 +
> > > >  2 files changed, 210 insertions(+)
> > > >  create mode 100644 tools/intel_pm_rpm.c
> > > > 
> > > > diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c new
> > > > file
> > > > mode 100644 index 000000000..df9cfa632
> > > > --- /dev/null
> > > > +++ b/tools/intel_pm_rpm.c
> > > > @@ -0,0 +1,209 @@
> > > > +/*
> > > > + * Copyright (c) 2022 Intel Corporation
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > > > +obtaining a
> > > > + * copy of this software and associated documentation files
> > > > (the
> > > > +"Software"),
> > > > + * to deal in the Software without restriction, including
> > > > without
> > > > +limitation
> > > > + * the rights to use, copy, modify, merge, publish,
> > > > distribute,
> > > > +sublicense,
> > > > + * and/or sell copies of the Software, and to permit persons
> > > > to
> > > > +whom the
> > > > + * Software is furnished to do so, subject to the following
> > > > conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice
> > > > (including
> > > > +the next
> > > > + * paragraph) shall be included in all copies or substantial
> > > > +portions of the
> > > > + * Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > KIND,
> > > > +EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > +MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN
> > > > NO
> > > > +EVENT SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > > +DAMAGES OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> > OTHERWISE,
> > > > +ARISING
> > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> > OR
> > > > +OTHER DEALINGS
> > > > + * IN THE SOFTWARE.
> > > > + *
> > > > + */
> > > > +
> > > > +#include <errno.h>
> > > > +#include <getopt.h>
> > > > +#include <glib.h>
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > > > +#include <string.h>
> > > > +#include <stdbool.h>
> > > > +#include "igt.h"
> > > > +#include "igt_device.h"
> > > > +#include "igt_device_scan.h"
> > > > +#include "igt_pm.h"
> > > > +
> > > > +typedef struct {
> > > > +       int drm_fd;
> > > > +       int debugfs_fd;
> > > > +       uint32_t devid;
> > > > +       drmModeResPtr res;
> > > > +       igt_display_t display;
> > > > +} data_t;
> > > > +
> > > > +const char *help_str =
> > > > +       "  --disable-display\t\tDisable all screen and try to
> > > > go into runtime
> > pm.\n"
> > > > +       "  --setup-d3cold\t\tPrepare dgfx gfx card to enter
> > > > runtime D3Cold.\n"
> > > > +       "  --help\t\tProvide help. Provide card name with
> > > > +IGT_DEVICE=drm:/dev/dri/card*."; static struct option
> > > > long_options[] = {
> > > > +       {"disable-display", 0, 0, 'd'},
> > > > +       {"setup-d3cold", 0, 0, 's'},
> > > > +       {"help", 0, 0, 'h'},
> > > > +       { 0, 0, 0, 0 }
> > > > +};
> > > > +
> > > > +const char *optstr = "dsh";
> > > > +
> > > > +static void usage(const char *name) {
> > > > +       igt_info("Usage: %s [options]\n", name);
> > > > +       igt_info("%s\n", help_str);
> > > > +}
> > > > +
> > > > +static void disable_all_displays(data_t *data) {
> > > > +       igt_output_t *output;
> > > > +
> > > > +       for (int i = 0; i < data->display.n_outputs; i++) {
> > > > +               output = &data->display.outputs[i];
> > > > +               igt_output_set_pipe(output, PIPE_NONE);
> > > > +               igt_display_commit(&data->display);
> > > > +       }
> > > > +}
> > > > +
> > > > +static void setup_gfx_card_d3cold(data_t *data) {
> > > > +       struct pci_device *root;
> > > > +       int d_state;
> > > > +
> > > > +       /* igfx does not support d3cold */
> > > > +       if (!IS_DGFX(data->devid))
> > > > +               return;
> > > 
> > > I believe the if below will already take care of this since the
> > > real_power_state won't show up. However, let's keep this check
> > > here to
> > > be really clear on the expectations and to avoid navigating the
> > > tree in vain...
> > > 
> > > > +
> > > > +       root = igt_device_get_pci_root_port(data->drm_fd);
> > > > +
> > > > +       if (!igt_pm_acpi_d3cold_supported(root)) {
> > > > +               igt_info("D3Cold isn't supported on Root port
> > %04x:%02x:%02x.%01x\n",
> > > > +                        root->domain, root->bus, root->dev,
> > > > root->func);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       disable_all_displays(data);
> > > > +       igt_pm_setup_pci_card_runtime_pm(root);
> > > > +       sleep(1);
> > > > +       d_state = igt_pm_get_acpi_real_d_state(root);
> > > > +
> > > > +       if (d_state == IGT_ACPI_D3Cold) {
> > > > +               igt_info("D3Cold achieved for root port
> > %04x:%02x:%02x.%01x\n",
> > > > +                        root->domain, root->bus, root->dev,
> > > > root->func);
> > > > +       } else {
> > > > +               igt_pm_print_pci_card_runtime_status();
> > > > +               igt_info("D3Cold not achieved yet. Please
> > > > monitor
> > %04x:%02x:%02x.%01x real_power_state\n",
> > > > +                        root->domain, root->bus, root->dev,
> > > > root->func);
> > 
> > 
> > oh, I had forgot to send comment on this one...
> > I believe that with sleep(1) above you are always reaching to this
> > msg here, no?!
> > 
> > Should we also change the autosuspend from everyone to 1 and maybe
> > wait 2
> > here?
> May be 0 should be ok to write all devices auto suspend delay.
> The problem is here writing 1 will conflict with existing i915
> runtime pm setup layer,
> Which writes 0 to i915 auto suspend delay.

oh yeah, I keep forgetting we use 0.

> 
> Another issue I had observed while writing auto suspend delay ,
> Few pci devices doesn't support auto-suspend.
> ---------------------------------------------------------------------
> -------------
> ta@DUT2135-DG2MRB:~/drivers.gpu.i915.igt-gpu-tools$ sudo
> ./build/tools/intel_pm_rpm --s
> Device /dev/dri/card1 successfully opened
> DMC: fw loaded: yes
> Test requirement not met in function
> __igt_pm_setup_pci_card_runtime_pm, file ../lib/igt_pm.c:969:
> Test requirement: read(delay_fd, delay, delay_size - 1) > 0
> PCI '0000:02:01.0' doesn't support auto_suspend
> Last errno: 5, Input/output error
> ---------------------------------------------------------------------
> -------------

oh I see...
Let's forgot and move with what you already had in place than...
and you have my rv-b already.

> 
> Br,
> Anshuman Gupta.
> > 
> > > > +       }
> > > > +}
> > > > +
> > > > +int main(int argc, char *argv[])
> > > > +{
> > > > +       bool disable_display = false, setup_d3cold = false;
> > > > +       struct igt_device_card card;
> > > > +       char *env_device = NULL;
> > > > +       int c, option_index = 0;
> > > > +       data_t data = {};
> > > > +       int ret = 0;
> > > > +
> > > > +       if (argc <= 1) {
> > > > +               usage(argv[0]);
> > > > +               return EXIT_SUCCESS;
> > > > +       }
> > > > +
> > > > +       env_device = getenv("IGT_DEVICE");
> > > > +       igt_devices_scan(false);
> > > > +
> > > > +       if (env_device) {
> > > > +               if (!igt_device_card_match(env_device, &card))
> > > > {
> > > > +                       igt_warn("No device found for the
> > > > env_device\n");
> > > > +                       ret = EXIT_FAILURE;
> > > > +                       goto exit;
> > > > +               }
> > > > +       } else {
> > > > +               if
> > > > (!igt_device_find_first_i915_discrete_card(&card)) {
> > > > +                       igt_warn("No discrete gpu found\n");
> > > > +                       ret = EXIT_FAILURE;
> > > > +                       goto exit;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       while ((c = getopt_long(argc, argv, optstr,
> > > > +                               long_options, &option_index))
> > > > != -1) {
> > > > +               switch (c) {
> > > > +               case 'd':
> > > > +                       disable_display = true;
> > > > +                       break;
> > > > +               case 's':
> > > > +                       setup_d3cold = true;
> > > > +                       break;
> > > > +               default:
> > > > +               case 'h':
> > > > +                       usage(argv[0]);
> > > > +                       ret = EXIT_SUCCESS;
> > > > +                       goto exit;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       data.drm_fd = igt_open_card(&card);
> > > > +       if (data.drm_fd  >= 0) {
> > > > +               igt_info("Device %s successfully opened\n",
> > > > card.card);
> > > > +       } else {
> > > > +               igt_warn("Cannot open card %s device\n",
> > > > card.card);
> > > > +               ret = EXIT_FAILURE;
> > > > +               goto exit;
> > > > +       }
> > > > +
> > > > +       data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > > > +       data.devid = intel_get_drm_devid(data.drm_fd);
> > > > +       igt_setup_runtime_pm(data.drm_fd);
> > > > +
> > > > +       data.res = drmModeGetResources(data.drm_fd);
> > > > +       if (data.res) {
> > > > +               kmstest_set_vt_graphics_mode();
> > > > +               igt_display_require(&data.display,
> > > > data.drm_fd);
> > > > +
> > > > +               if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> > > > +                       igt_warn("dmc fw is not loaded, no
> > > > runtime pm\n");
> > > > +                       ret = EXIT_FAILURE;
> > > > +                       goto exit;
> > > > +               }
> > > > +       }
> > > 
> > > Do we really need this? It is only for checking if DMC is there
> > > in case we have
> > display right?
> > > Should deserve a comment at least...
> > > 
> > > but already can use my:
> > > 
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > if explained with reply here or with comment if you see it fits
> > > 
> > > > +
> > > > +       if (disable_display) {
> > > > +               disable_all_displays(&data);
> > > > +               if
> > (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> > > > +                       __igt_debugfs_dump(data.drm_fd,
> > "i915_runtime_pm_status", IGT_LOG_INFO);
> > > > +                       ret = EXIT_FAILURE;
> > > > +                       goto exit;
> > > > +               }
> > > > +
> > > > +               igt_info("Device runtime suspended, Useful for
> > > > debugging.\n"
> > > > +                        "Hit CTRL-C to exit\n");
> > > > +               /* Don't return useful for debugging */
> > > > +               while (1)
> > > > +                       sleep(600);
> > > > +       }
> > > > +
> > > > +       if (setup_d3cold)
> > > > +               setup_gfx_card_d3cold(&data);
> > > > +
> > > > +exit:
> > > > +       igt_restore_runtime_pm();
> > > > +
> > > > +       if (data.res)
> > > > +               igt_display_fini(&data.display);
> > > > +
> > > > +       close(data.debugfs_fd);
> > > > +       close(data.drm_fd);
> > > > +       igt_devices_free();
> > > > +
> > > > +       return ret;
> > > > +}
> > > > diff --git a/tools/meson.build b/tools/meson.build index
> > > > 771d0b9e3..24d0ea714 100644
> > > > --- a/tools/meson.build
> > > > +++ b/tools/meson.build
> > > > @@ -28,6 +28,7 @@ tools_progs = [
> > > >         'intel_lid',
> > > >         'intel_opregion_decode',
> > > >         'intel_panel_fitter',
> > > > +       'intel_pm_rpm',
> > > >         'intel_reg_checker',
> > > >         'intel_residency',
> > > >         'intel_stepping',
> > > > --
> > > > 2.26.2
> > > > 

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Thursday, April 28, 2022 2:09 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: Nilawar, Badal <badal.nilawar@intel.com>; igt-dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool
> 
> On Wed, 2022-04-27 at 23:59 -0700, Gupta, Anshuman wrote:
> >
> >
> > > -----Original Message-----
> > > From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > > Sent: Friday, April 22, 2022 5:38 PM
> > > To: Gupta, Anshuman <anshuman.gupta@intel.com>
> > > Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal
> > > <badal.nilawar@intel.com>
> > > Subject: Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm
> > > tool
> > >
> > > On Fri, Apr 22, 2022 at 06:22:28AM -0400, Rodrigo Vivi wrote:
> > > > On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> > > > > intel_pm_rpm tool is a debug tool. It can be use to setup and
> > > > > prepare the gfx card to go to D3Cold.
> > > > > It also provide the debug option to disable all display and
> > > > > prepare device to enter to runtime suspend.
> > > > >
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > > ---
> > > > >  tools/intel_pm_rpm.c | 209
> > > +++++++++++++++++++++++++++++++++++++++++++
> > > > >  tools/meson.build    |   1 +
> > > > >  2 files changed, 210 insertions(+)
> > > > >  create mode 100644 tools/intel_pm_rpm.c
> > > > >
> > > > > diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c new
> > > > > file mode 100644 index 000000000..df9cfa632
> > > > > --- /dev/null
> > > > > +++ b/tools/intel_pm_rpm.c
> > > > > @@ -0,0 +1,209 @@
> > > > > +/*
> > > > > + * Copyright (c) 2022 Intel Corporation
> > > > > + *
> > > > > + * Permission is hereby granted, free of charge, to any person
> > > > > +obtaining a
> > > > > + * copy of this software and associated documentation files
> > > > > (the
> > > > > +"Software"),
> > > > > + * to deal in the Software without restriction, including
> > > > > without
> > > > > +limitation
> > > > > + * the rights to use, copy, modify, merge, publish,
> > > > > distribute,
> > > > > +sublicense,
> > > > > + * and/or sell copies of the Software, and to permit persons
> > > > > to
> > > > > +whom the
> > > > > + * Software is furnished to do so, subject to the following
> > > > > conditions:
> > > > > + *
> > > > > + * The above copyright notice and this permission notice
> > > > > (including
> > > > > +the next
> > > > > + * paragraph) shall be included in all copies or substantial
> > > > > +portions of the
> > > > > + * Software.
> > > > > + *
> > > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > > KIND,
> > > > > +EXPRESS OR
> > > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > > +MERCHANTABILITY,
> > > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN
> > > > > NO
> > > > > +EVENT SHALL
> > > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > > > +DAMAGES OR OTHER
> > > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> > > OTHERWISE,
> > > > > +ARISING
> > > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> USE
> > > OR
> > > > > +OTHER DEALINGS
> > > > > + * IN THE SOFTWARE.
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#include <errno.h>
> > > > > +#include <getopt.h>
> > > > > +#include <glib.h>
> > > > > +#include <stdio.h>
> > > > > +#include <stdlib.h>
> > > > > +#include <string.h>
> > > > > +#include <stdbool.h>
> > > > > +#include "igt.h"
> > > > > +#include "igt_device.h"
> > > > > +#include "igt_device_scan.h"
> > > > > +#include "igt_pm.h"
> > > > > +
> > > > > +typedef struct {
> > > > > +       int drm_fd;
> > > > > +       int debugfs_fd;
> > > > > +       uint32_t devid;
> > > > > +       drmModeResPtr res;
> > > > > +       igt_display_t display;
> > > > > +} data_t;
> > > > > +
> > > > > +const char *help_str =
> > > > > +       "  --disable-display\t\tDisable all screen and try to
> > > > > go into runtime
> > > pm.\n"
> > > > > +       "  --setup-d3cold\t\tPrepare dgfx gfx card to enter
> > > > > runtime D3Cold.\n"
> > > > > +       "  --help\t\tProvide help. Provide card name with
> > > > > +IGT_DEVICE=drm:/dev/dri/card*."; static struct option
> > > > > long_options[] = {
> > > > > +       {"disable-display", 0, 0, 'd'},
> > > > > +       {"setup-d3cold", 0, 0, 's'},
> > > > > +       {"help", 0, 0, 'h'},
> > > > > +       { 0, 0, 0, 0 }
> > > > > +};
> > > > > +
> > > > > +const char *optstr = "dsh";
> > > > > +
> > > > > +static void usage(const char *name) {
> > > > > +       igt_info("Usage: %s [options]\n", name);
> > > > > +       igt_info("%s\n", help_str); }
> > > > > +
> > > > > +static void disable_all_displays(data_t *data) {
> > > > > +       igt_output_t *output;
> > > > > +
> > > > > +       for (int i = 0; i < data->display.n_outputs; i++) {
> > > > > +               output = &data->display.outputs[i];
> > > > > +               igt_output_set_pipe(output, PIPE_NONE);
> > > > > +               igt_display_commit(&data->display);
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +static void setup_gfx_card_d3cold(data_t *data) {
> > > > > +       struct pci_device *root;
> > > > > +       int d_state;
> > > > > +
> > > > > +       /* igfx does not support d3cold */
> > > > > +       if (!IS_DGFX(data->devid))
> > > > > +               return;
> > > >
> > > > I believe the if below will already take care of this since the
> > > > real_power_state won't show up. However, let's keep this check
> > > > here to be really clear on the expectations and to avoid
> > > > navigating the tree in vain...
> > > >
> > > > > +
> > > > > +       root = igt_device_get_pci_root_port(data->drm_fd);
> > > > > +
> > > > > +       if (!igt_pm_acpi_d3cold_supported(root)) {
> > > > > +               igt_info("D3Cold isn't supported on Root port
> > > %04x:%02x:%02x.%01x\n",
> > > > > +                        root->domain, root->bus, root->dev,
> > > > > root->func);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       disable_all_displays(data);
> > > > > +       igt_pm_setup_pci_card_runtime_pm(root);
> > > > > +       sleep(1);
> > > > > +       d_state = igt_pm_get_acpi_real_d_state(root);
> > > > > +
> > > > > +       if (d_state == IGT_ACPI_D3Cold) {
> > > > > +               igt_info("D3Cold achieved for root port
> > > %04x:%02x:%02x.%01x\n",
> > > > > +                        root->domain, root->bus, root->dev,
> > > > > root->func);
> > > > > +       } else {
> > > > > +               igt_pm_print_pci_card_runtime_status();
> > > > > +               igt_info("D3Cold not achieved yet. Please
> > > > > monitor
> > > %04x:%02x:%02x.%01x real_power_state\n",
> > > > > +                        root->domain, root->bus, root->dev,
> > > > > root->func);
> > >
> > >
> > > oh, I had forgot to send comment on this one...
> > > I believe that with sleep(1) above you are always reaching to this
> > > msg here, no?!
> > >
> > > Should we also change the autosuspend from everyone to 1 and maybe
> > > wait 2 here?
> > May be 0 should be ok to write all devices auto suspend delay.
> > The problem is here writing 1 will conflict with existing i915 runtime
> > pm setup layer, Which writes 0 to i915 auto suspend delay.
> 
> oh yeah, I keep forgetting we use 0.
> 
> >
> > Another issue I had observed while writing auto suspend delay , Few
> > pci devices doesn't support auto-suspend.
> > ---------------------------------------------------------------------
> > -------------
> > ta@DUT2135-DG2MRB:~/drivers.gpu.i915.igt-gpu-tools$ sudo
> > ./build/tools/intel_pm_rpm --s Device /dev/dri/card1 successfully
> > opened
> > DMC: fw loaded: yes
> > Test requirement not met in function
> > __igt_pm_setup_pci_card_runtime_pm, file ../lib/igt_pm.c:969:
> > Test requirement: read(delay_fd, delay, delay_size - 1) > 0 PCI
> > '0000:02:01.0' doesn't support auto_suspend Last errno: 5,
> > Input/output error
> > ---------------------------------------------------------------------
> > -------------
> 
> oh I see...
> Let's forgot and move with what you already had in place than...
> and you have my rv-b already.
V2 version setting up  the auto_suspend_delay to 0 ms and saving it only  for the devices which supports
autosuspend. Below there is debug output of this test.
(i915_pm_rpm:6477) DEBUG: Test requirement passed: setup_environment(false)
Starting subtest: d3cold-basic
(i915_pm_rpm:6477) igt_device-DEBUG: Test requirement passed: pci_dev
(i915_pm_rpm:6477) igt_device-DEBUG: Root Port PCI device 0000:00:01.0
(i915_pm_rpm:6477) igt_pm-DEBUG: Test requirement passed: dir > 0
(i915_pm_rpm:6477) igt_pm-DEBUG: Test requirement passed: fd > 0
(i915_pm_rpm:6477) DEBUG: Test requirement passed: igt_pm_acpi_d3cold_supported(root)
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:00:01.0' Saved 'control, autosuspend_delay_ms' as 'auto, 100'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:01:00.0' Saved 'control, autosuspend_delay_ms' as 'auto, 100'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:01.0' doesn't support auto_suspend
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:01.0' Saved 'control, autosuspend_delay_ms' as 'on, NA'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:04.0' doesn't support auto_suspend
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:04.0' Saved 'control, autosuspend_delay_ms' as 'on, NA'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:03:00.0' Saved 'control, autosuspend_delay_ms' as 'auto, 0'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:04:00.0' Saved 'control, autosuspend_delay_ms' as 'auto, 0'
(i915_pm_rpm:6477) igt_pm-DEBUG: (status = __igt_get_runtime_pm_status(fd)) == expected took 95ms
(i915_pm_rpm:6477) DEBUG: igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold took 0ms
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:00:01.0' Restoring control attr to 'auto'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:00:01.0' Restoring autosuspend_delay_ms attr to '100'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:01:00.0' Restoring control attr to 'auto'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:01:00.0' Restoring autosuspend_delay_ms attr to '100'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:01.0' Restoring control attr to 'on'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:02:04.0' Restoring control attr to 'on'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:03:00.0' Restoring control attr to 'auto'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:03:00.0' Restoring autosuspend_delay_ms attr to '0'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:04:00.0' Restoring control attr to 'auto'
(i915_pm_rpm:6477) igt_pm-DEBUG: PCI '0000:04:00.0' Restoring autosuspend_delay_ms attr to '0'
Subtest d3cold-basic: SUCCESS (1.754s)
(i915_pm_rpm:6477) igt_pm-DEBUG: Restoring runtime PM management to '0' and 'auto'
(i915_pm_rpm:6477) igt_pm-DEBUG: Restoring audio power management to '0' and 'auto'
(i915_pm_rpm:6477) igt_core-DEBUG: Exiting with status code 0
(i915_pm_rpm:6477) igt_kms-DEBUG: VT: original mode 0x0 restored
gta@DUT2135-DG2MRB:~/drivers.gpu.i915.igt-gpu-tools$

Br,
Anshuman Gupta.
> 
> >
> > Br,
> > Anshuman Gupta.
> > >
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +int main(int argc, char *argv[]) {
> > > > > +       bool disable_display = false, setup_d3cold = false;
> > > > > +       struct igt_device_card card;
> > > > > +       char *env_device = NULL;
> > > > > +       int c, option_index = 0;
> > > > > +       data_t data = {};
> > > > > +       int ret = 0;
> > > > > +
> > > > > +       if (argc <= 1) {
> > > > > +               usage(argv[0]);
> > > > > +               return EXIT_SUCCESS;
> > > > > +       }
> > > > > +
> > > > > +       env_device = getenv("IGT_DEVICE");
> > > > > +       igt_devices_scan(false);
> > > > > +
> > > > > +       if (env_device) {
> > > > > +               if (!igt_device_card_match(env_device, &card))
> > > > > {
> > > > > +                       igt_warn("No device found for the
> > > > > env_device\n");
> > > > > +                       ret = EXIT_FAILURE;
> > > > > +                       goto exit;
> > > > > +               }
> > > > > +       } else {
> > > > > +               if
> > > > > (!igt_device_find_first_i915_discrete_card(&card)) {
> > > > > +                       igt_warn("No discrete gpu found\n");
> > > > > +                       ret = EXIT_FAILURE;
> > > > > +                       goto exit;
> > > > > +               }
> > > > > +       }
> > > > > +
> > > > > +       while ((c = getopt_long(argc, argv, optstr,
> > > > > +                               long_options, &option_index))
> > > > > != -1) {
> > > > > +               switch (c) {
> > > > > +               case 'd':
> > > > > +                       disable_display = true;
> > > > > +                       break;
> > > > > +               case 's':
> > > > > +                       setup_d3cold = true;
> > > > > +                       break;
> > > > > +               default:
> > > > > +               case 'h':
> > > > > +                       usage(argv[0]);
> > > > > +                       ret = EXIT_SUCCESS;
> > > > > +                       goto exit;
> > > > > +               }
> > > > > +       }
> > > > > +
> > > > > +       data.drm_fd = igt_open_card(&card);
> > > > > +       if (data.drm_fd  >= 0) {
> > > > > +               igt_info("Device %s successfully opened\n",
> > > > > card.card);
> > > > > +       } else {
> > > > > +               igt_warn("Cannot open card %s device\n",
> > > > > card.card);
> > > > > +               ret = EXIT_FAILURE;
> > > > > +               goto exit;
> > > > > +       }
> > > > > +
> > > > > +       data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > > > > +       data.devid = intel_get_drm_devid(data.drm_fd);
> > > > > +       igt_setup_runtime_pm(data.drm_fd);
> > > > > +
> > > > > +       data.res = drmModeGetResources(data.drm_fd);
> > > > > +       if (data.res) {
> > > > > +               kmstest_set_vt_graphics_mode();
> > > > > +               igt_display_require(&data.display,
> > > > > data.drm_fd);
> > > > > +
> > > > > +               if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> > > > > +                       igt_warn("dmc fw is not loaded, no
> > > > > runtime pm\n");
> > > > > +                       ret = EXIT_FAILURE;
> > > > > +                       goto exit;
> > > > > +               }
> > > > > +       }
> > > >
> > > > Do we really need this? It is only for checking if DMC is there in
> > > > case we have
> > > display right?
> > > > Should deserve a comment at least...
> > > >
> > > > but already can use my:
> > > >
> > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > >
> > > > if explained with reply here or with comment if you see it fits
> > > >
> > > > > +
> > > > > +       if (disable_display) {
> > > > > +               disable_all_displays(&data);
> > > > > +               if
> > > (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> > > > > +                       __igt_debugfs_dump(data.drm_fd,
> > > "i915_runtime_pm_status", IGT_LOG_INFO);
> > > > > +                       ret = EXIT_FAILURE;
> > > > > +                       goto exit;
> > > > > +               }
> > > > > +
> > > > > +               igt_info("Device runtime suspended, Useful for
> > > > > debugging.\n"
> > > > > +                        "Hit CTRL-C to exit\n");
> > > > > +               /* Don't return useful for debugging */
> > > > > +               while (1)
> > > > > +                       sleep(600);
> > > > > +       }
> > > > > +
> > > > > +       if (setup_d3cold)
> > > > > +               setup_gfx_card_d3cold(&data);
> > > > > +
> > > > > +exit:
> > > > > +       igt_restore_runtime_pm();
> > > > > +
> > > > > +       if (data.res)
> > > > > +               igt_display_fini(&data.display);
> > > > > +
> > > > > +       close(data.debugfs_fd);
> > > > > +       close(data.drm_fd);
> > > > > +       igt_devices_free();
> > > > > +
> > > > > +       return ret;
> > > > > +}
> > > > > diff --git a/tools/meson.build b/tools/meson.build index
> > > > > 771d0b9e3..24d0ea714 100644
> > > > > --- a/tools/meson.build
> > > > > +++ b/tools/meson.build
> > > > > @@ -28,6 +28,7 @@ tools_progs = [
> > > > >         'intel_lid',
> > > > >         'intel_opregion_decode',
> > > > >         'intel_panel_fitter',
> > > > > +       'intel_pm_rpm',
> > > > >         'intel_reg_checker',
> > > > >         'intel_residency',
> > > > >         'intel_stepping',
> > > > > --
> > > > > 2.26.2
> > > > >

Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Friday, April 22, 2022 3:52 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Nilawar, Badal <badal.nilawar@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 5/9] tools: Add intel_pm_rpm tool
> 
> On Mon, Apr 18, 2022 at 06:20:44PM +0530, Anshuman Gupta wrote:
> > intel_pm_rpm tool is a debug tool. It can be use to setup and prepare
> > the gfx card to go to D3Cold.
> > It also provide the debug option to disable all display and prepare
> > device to enter to runtime suspend.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  tools/intel_pm_rpm.c | 209
> +++++++++++++++++++++++++++++++++++++++++++
> >  tools/meson.build    |   1 +
> >  2 files changed, 210 insertions(+)
> >  create mode 100644 tools/intel_pm_rpm.c
> >
> > diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c new file mode
> > 100644 index 000000000..df9cfa632
> > --- /dev/null
> > +++ b/tools/intel_pm_rpm.c
> > @@ -0,0 +1,209 @@
> > +/*
> > + * Copyright (c) 2022 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > +obtaining a
> > + * copy of this software and associated documentation files (the
> > +"Software"),
> > + * to deal in the Software without restriction, including without
> > +limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > +sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom
> > +the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > +the next
> > + * paragraph) shall be included in all copies or substantial portions
> > +of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > +EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > +MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT
> > +SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES
> > +OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > +ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > +OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +
> > +#include <errno.h>
> > +#include <getopt.h>
> > +#include <glib.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include "igt.h"
> > +#include "igt_device.h"
> > +#include "igt_device_scan.h"
> > +#include "igt_pm.h"
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	uint32_t devid;
> > +	drmModeResPtr res;
> > +	igt_display_t display;
> > +} data_t;
> > +
> > +const char *help_str =
> > +	"  --disable-display\t\tDisable all screen and try to go into runtime
> pm.\n"
> > +	"  --setup-d3cold\t\tPrepare dgfx gfx card to enter runtime D3Cold.\n"
> > +	"  --help\t\tProvide help. Provide card name with
> > +IGT_DEVICE=drm:/dev/dri/card*."; static struct option long_options[] = {
> > +	{"disable-display", 0, 0, 'd'},
> > +	{"setup-d3cold", 0, 0, 's'},
> > +	{"help", 0, 0, 'h'},
> > +	{ 0, 0, 0, 0 }
> > +};
> > +
> > +const char *optstr = "dsh";
> > +
> > +static void usage(const char *name)
> > +{
> > +	igt_info("Usage: %s [options]\n", name);
> > +	igt_info("%s\n", help_str);
> > +}
> > +
> > +static void disable_all_displays(data_t *data) {
> > +	igt_output_t *output;
> > +
> > +	for (int i = 0; i < data->display.n_outputs; i++) {
> > +		output = &data->display.outputs[i];
> > +		igt_output_set_pipe(output, PIPE_NONE);
> > +		igt_display_commit(&data->display);
> > +	}
> > +}
> > +
> > +static void setup_gfx_card_d3cold(data_t *data) {
> > +	struct pci_device *root;
> > +	int d_state;
> > +
> > +	/* igfx does not support d3cold */
> > +	if (!IS_DGFX(data->devid))
> > +		return;
> 
> I believe the if below will already take care of this since the real_power_state
> won't show up. However, let's keep this check here to be really clear on the
> expectations and to avoid navigating the tree in vain...
> 
> > +
> > +	root = igt_device_get_pci_root_port(data->drm_fd);
> > +
> > +	if (!igt_pm_acpi_d3cold_supported(root)) {
> > +		igt_info("D3Cold isn't supported on Root port
> %04x:%02x:%02x.%01x\n",
> > +			 root->domain, root->bus, root->dev, root->func);
> > +		return;
> > +	}
> > +
> > +	disable_all_displays(data);
> > +	igt_pm_setup_pci_card_runtime_pm(root);
> > +	sleep(1);
> > +	d_state = igt_pm_get_acpi_real_d_state(root);
> > +
> > +	if (d_state == IGT_ACPI_D3Cold) {
> > +		igt_info("D3Cold achieved for root port
> %04x:%02x:%02x.%01x\n",
> > +			 root->domain, root->bus, root->dev, root->func);
> > +	} else {
> > +		igt_pm_print_pci_card_runtime_status();
> > +		igt_info("D3Cold not achieved yet. Please monitor
> %04x:%02x:%02x.%01x real_power_state\n",
> > +			 root->domain, root->bus, root->dev, root->func);
> > +	}
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	bool disable_display = false, setup_d3cold = false;
> > +	struct igt_device_card card;
> > +	char *env_device = NULL;
> > +	int c, option_index = 0;
> > +	data_t data = {};
> > +	int ret = 0;
> > +
> > +	if (argc <= 1) {
> > +		usage(argv[0]);
> > +		return EXIT_SUCCESS;
> > +	}
> > +
> > +	env_device = getenv("IGT_DEVICE");
> > +	igt_devices_scan(false);
> > +
> > +	if (env_device) {
> > +		if (!igt_device_card_match(env_device, &card)) {
> > +			igt_warn("No device found for the env_device\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	} else {
> > +		if (!igt_device_find_first_i915_discrete_card(&card)) {
> > +			igt_warn("No discrete gpu found\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	}
> > +
> > +	while ((c = getopt_long(argc, argv, optstr,
> > +				long_options, &option_index)) != -1) {
> > +		switch (c) {
> > +		case 'd':
> > +			disable_display = true;
> > +			break;
> > +		case 's':
> > +			setup_d3cold = true;
> > +			break;
> > +		default:
> > +		case 'h':
> > +			usage(argv[0]);
> > +			ret = EXIT_SUCCESS;
> > +			goto exit;
> > +		}
> > +	}
> > +
> > +	data.drm_fd = igt_open_card(&card);
> > +	if (data.drm_fd  >= 0) {
> > +		igt_info("Device %s successfully opened\n", card.card);
> > +	} else {
> > +		igt_warn("Cannot open card %s device\n", card.card);
> > +		ret = EXIT_FAILURE;
> > +		goto exit;
> > +	}
> > +
> > +	data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > +	data.devid = intel_get_drm_devid(data.drm_fd);
> > +	igt_setup_runtime_pm(data.drm_fd);
> > +
> > +	data.res = drmModeGetResources(data.drm_fd);
> > +	if (data.res) {
> > +		kmstest_set_vt_graphics_mode();
> > +		igt_display_require(&data.display, data.drm_fd);
> > +
> > +		if (!igt_pm_dmc_loaded(data.debugfs_fd)) {
> > +			igt_warn("dmc fw is not loaded, no runtime pm\n");
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +	}
> 
> Do we really need this? It is only for checking if DMC is there in case we have
> display right?
> Should deserve a comment at least...
> 
> but already can use my:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Thanks for Review.
I will add below comment, with respect to https://cgit.freedesktop.org/drm-tip/tree/drivers/gpu/drm/i915/display/intel_dmc.c#n791.
/* i915 disables RPM in case DMC is not loaded on display supported cards */
Br,
Anshuman Gupta
> 
> if explained with reply here or with comment if you see it fits
> 
> > +
> > +	if (disable_display) {
> > +		disable_all_displays(&data);
> > +		if
> (!igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)) {
> > +			__igt_debugfs_dump(data.drm_fd,
> "i915_runtime_pm_status", IGT_LOG_INFO);
> > +			ret = EXIT_FAILURE;
> > +			goto exit;
> > +		}
> > +
> > +		igt_info("Device runtime suspended, Useful for debugging.\n"
> > +			 "Hit CTRL-C to exit\n");
> > +		/* Don't return useful for debugging */
> > +		while (1)
> > +			sleep(600);
> > +	}
> > +
> > +	if (setup_d3cold)
> > +		setup_gfx_card_d3cold(&data);
> > +
> > +exit:
> > +	igt_restore_runtime_pm();
> > +
> > +	if (data.res)
> > +		igt_display_fini(&data.display);
> > +
> > +	close(data.debugfs_fd);
> > +	close(data.drm_fd);
> > +	igt_devices_free();
> > +
> > +	return ret;
> > +}
> > diff --git a/tools/meson.build b/tools/meson.build index
> > 771d0b9e3..24d0ea714 100644
> > --- a/tools/meson.build
> > +++ b/tools/meson.build
> > @@ -28,6 +28,7 @@ tools_progs = [
> >  	'intel_lid',
> >  	'intel_opregion_decode',
> >  	'intel_panel_fitter',
> > +	'intel_pm_rpm',
> >  	'intel_reg_checker',
> >  	'intel_residency',
> >  	'intel_stepping',
> > --
> > 2.26.2
> >

[igt-dev] [PATCH i-g-t 6/9] i915_pm_rpm: Add D3Cold basic subtest

[Anshuman Gupta]

Add support for D3Cold basic subtest.
It setup and prepares the gfx PCI card for D3Cold
and checks the ACPI D3Cold state.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index d2bce5826..c30862dc4 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -1534,6 +1534,31 @@ __noreturn static void stay_subtest(void)
 		sleep(600);
 }
 
+static void d3cold_basic_subtest(void)
+{
+	struct pci_device *root;
+	bool result;
+
+	/* igfx does not support d3cold */
+	igt_require(IS_DGFX(ms_data.devid));
+
+	root = igt_device_get_pci_root_port(drm_fd);
+	disable_all_screens_and_wait(&ms_data);
+	igt_require(igt_pm_acpi_d3cold_supported(root));
+	igt_pm_setup_pci_card_runtime_pm(root);
+
+	result = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
+
+	if (!result) {
+		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
+			 root->domain, root->bus, root->dev, root->func);
+		igt_pm_print_pci_card_runtime_status();
+	}
+
+	igt_assert(result);
+	igt_pm_restore_pci_card_runtime_pm();
+}
+
 static void system_suspend_subtest(int state, int debug)
 {
 	disable_all_screens_and_wait(&ms_data);
@@ -2065,6 +2090,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		igt_subtest("stay")
 			stay_subtest();
 
+	igt_subtest("d3cold-basic")
+		d3cold_basic_subtest();
+
 	/* Essential things */
 	igt_subtest("drm-resources-equal")
 		drm_resources_equal_subtest();
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 6/9] i915_pm_rpm: Add D3Cold basic subtest

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:45PM +0530, Anshuman Gupta wrote:
> Add support for D3Cold basic subtest.
> It setup and prepares the gfx PCI card for D3Cold
> and checks the ACPI D3Cold state.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  tests/i915/i915_pm_rpm.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index d2bce5826..c30862dc4 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1534,6 +1534,31 @@ __noreturn static void stay_subtest(void)
>  		sleep(600);
>  }
>  
> +static void d3cold_basic_subtest(void)
> +{
> +	struct pci_device *root;
> +	bool result;
> +
> +	/* igfx does not support d3cold */
> +	igt_require(IS_DGFX(ms_data.devid));
> +
> +	root = igt_device_get_pci_root_port(drm_fd);
> +	disable_all_screens_and_wait(&ms_data);
> +	igt_require(igt_pm_acpi_d3cold_supported(root));
> +	igt_pm_setup_pci_card_runtime_pm(root);
> +
> +	result = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
> +
> +	if (!result) {
> +		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +		igt_pm_print_pci_card_runtime_status();
> +	}
> +
> +	igt_assert(result);
> +	igt_pm_restore_pci_card_runtime_pm();
> +}
> +
>  static void system_suspend_subtest(int state, int debug)
>  {
>  	disable_all_screens_and_wait(&ms_data);
> @@ -2065,6 +2090,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  		igt_subtest("stay")
>  			stay_subtest();
>  
> +	igt_subtest("d3cold-basic")
> +		d3cold_basic_subtest();
> +
>  	/* Essential things */
>  	igt_subtest("drm-resources-equal")
>  		drm_resources_equal_subtest();
> -- 
> 2.26.2
> 

Re: [igt-dev] [PATCH i-g-t 6/9] i915_pm_rpm: Add D3Cold basic subtest

[Kamil Konieczny]

Hi Anshuman,

On 2022-04-18 at 18:20:45 +0530, Anshuman Gupta wrote:
> Add support for D3Cold basic subtest.
> It setup and prepares the gfx PCI card for D3Cold
> and checks the ACPI D3Cold state.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index d2bce5826..c30862dc4 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1534,6 +1534,31 @@ __noreturn static void stay_subtest(void)
>  		sleep(600);
>  }
>  
> +static void d3cold_basic_subtest(void)
> +{
> +	struct pci_device *root;
> +	bool result;
> +
> +	/* igfx does not support d3cold */
> +	igt_require(IS_DGFX(ms_data.devid));
> +
> +	root = igt_device_get_pci_root_port(drm_fd);
> +	disable_all_screens_and_wait(&ms_data);
> +	igt_require(igt_pm_acpi_d3cold_supported(root));
> +	igt_pm_setup_pci_card_runtime_pm(root);
> +
> +	result = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
> +
> +	if (!result) {
> +		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +		igt_pm_print_pci_card_runtime_status();
> +	}
> +
> +	igt_assert(result);
> +	igt_pm_restore_pci_card_runtime_pm();
> +}
> +
>  static void system_suspend_subtest(int state, int debug)
>  {
>  	disable_all_screens_and_wait(&ms_data);
> @@ -2065,6 +2090,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  		igt_subtest("stay")
>  			stay_subtest();
>  

For each new added subtest add description before, like:

	igt_describe("Describe your subtest here");

Run you test with --describe option and see where you need to
add it (do this only to new subtests you are adding).

Regards,
Kamil

> +	igt_subtest("d3cold-basic")
> +		d3cold_basic_subtest();
> +
>  	/* Essential things */
>  	igt_subtest("drm-resources-equal")
>  		drm_resources_equal_subtest();
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 7/9] i915_pm_rpm: Test D3Cold with gem_exec_stress

[Anshuman Gupta]

Added d3cold dynamic subtest to gem_exec_stress with device class
memory region. It test both D3Cold-{VRAM_SR, Off} by using
d3cold_sr_lmem_threshold i915_params debugfs.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 95 +++++++++++++++++++++++++++++++++-------
 1 file changed, 78 insertions(+), 17 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index c30862dc4..8e9ff56ec 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -85,6 +85,7 @@ enum plane_type {
 #define WAIT_PC8_RES	2
 #define WAIT_EXTRA	4
 #define USE_DPMS	8
+#define WAIT_D3COLD	16
 
 int drm_fd, msr_fd, pc8_status_fd;
 int debugfs;
@@ -215,6 +216,21 @@ static bool wait_for_suspended(void)
 	}
 }
 
+static bool wait_for_d3cold(struct pci_device *root)
+{
+	bool d3colded;
+
+	d3colded = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
+
+	if (!d3colded) {
+		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
+			 root->domain, root->bus, root->dev, root->func);
+		igt_pm_print_pci_card_runtime_status();
+	}
+
+	return d3colded;
+}
+
 static bool wait_for_active(void)
 {
 	if (has_pc8 && !has_runtime_pm)
@@ -744,6 +760,30 @@ static void test_i2c(struct mode_set_data *data)
 			"There is an EDID mismatch between i2c and DRM!\n");
 }
 
+static int get_d3cold_sr_lmem_threshold(int dir)
+{
+	int param_dir, lmem_threshold, ret;
+
+	param_dir = openat(dir, "i915_params", O_RDONLY);
+	ret = igt_sysfs_scanf(param_dir, "d3cold_sr_lmem_threshold", "%d", &lmem_threshold);
+	igt_assert(ret >= 0);
+
+	close(param_dir);
+	return lmem_threshold;
+}
+
+static int set_d3cold_sr_lmem_threshold(int dir, int val)
+{
+	int param_dir, ret;
+
+	param_dir = openat(dir, "i915_params", O_RDONLY);
+	ret = igt_sysfs_printf(param_dir, "d3cold_sr_lmem_threshold", "%d", val);
+	igt_assert(ret > 0);
+
+	close(param_dir);
+	return ret;
+}
+
 static void setup_pc8(void)
 {
 	has_pc8 = false;
@@ -1069,6 +1109,19 @@ static void debugfs_forcewake_user_subtest(void)
 	igt_assert(wait_for_suspended());
 }
 
+static struct pci_device *setup_d3cold_and_get_root_port(void)
+{
+	struct pci_device *root;
+
+	/* igfx does not support d3cold */
+	igt_require(IS_DGFX(ms_data.devid));
+	root = igt_device_get_pci_root_port(drm_fd);
+	igt_require(igt_pm_acpi_d3cold_supported(root));
+	igt_pm_setup_pci_card_runtime_pm(root);
+
+	return root;
+}
+
 static void gem_mmap_args(const struct mmap_offset *t,
 			  struct drm_i915_gem_memory_class_instance *mem_regions)
 {
@@ -1392,6 +1445,7 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
 	int i;
 	int batch_size = 4 * sizeof(uint32_t);
 	uint32_t batch_buf[batch_size];
+	struct pci_device *root;
 	uint32_t handle;
 	struct drm_i915_gem_execbuffer2 execbuf = {};
 	struct drm_i915_gem_exec_object2 objs[1] = {{}};
@@ -1408,6 +1462,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
 	batch_buf[i++] = MI_NOOP;
 	igt_assert(i * sizeof(uint32_t) == batch_size);
 
+	if (wait_flags & WAIT_D3COLD)
+		root = setup_d3cold_and_get_root_port();
+
 	disable_all_screens_and_wait(&ms_data);
 
 	/* PC8 test is only applicable to igfx  */
@@ -1433,6 +1490,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
 			/* clean up idle work */
 			igt_drop_caches_set(drm_fd, DROP_IDLE);
 			igt_assert(wait_for_suspended());
+			if (wait_flags & WAIT_D3COLD)
+				igt_assert(wait_for_d3cold(root));
+
 		}
 		if (wait_flags & WAIT_PC8_RES)
 			igt_assert(pc8_plus_residency_changed(30));
@@ -1440,6 +1500,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
 			sleep(5);
 	}
 
+	if (wait_flags & WAIT_D3COLD)
+		igt_pm_restore_pci_card_runtime_pm();
+
 	gem_close(drm_fd, handle);
 }
 
@@ -1537,25 +1600,10 @@ __noreturn static void stay_subtest(void)
 static void d3cold_basic_subtest(void)
 {
 	struct pci_device *root;
-	bool result;
 
-	/* igfx does not support d3cold */
-	igt_require(IS_DGFX(ms_data.devid));
-
-	root = igt_device_get_pci_root_port(drm_fd);
+	root = setup_d3cold_and_get_root_port();
 	disable_all_screens_and_wait(&ms_data);
-	igt_require(igt_pm_acpi_d3cold_supported(root));
-	igt_pm_setup_pci_card_runtime_pm(root);
-
-	result = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
-
-	if (!result) {
-		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
-			 root->domain, root->bus, root->dev, root->func);
-		igt_pm_print_pci_card_runtime_status();
-	}
-
-	igt_assert(result);
+	igt_assert(wait_for_d3cold(root));
 	igt_pm_restore_pci_card_runtime_pm();
 }
 
@@ -2213,6 +2261,19 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 				gem_execbuf_stress_subtest(rounds, WAIT_STATUS, &r->ci);
 			igt_dynamic_f("%s-%s", "extra-wait", r->name)
 				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_EXTRA, &r->ci);
+			if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
+				continue;
+			igt_dynamic_f("%s-%s", "d3cold", r->name) {
+				int lmem_threshold;
+
+				lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
+				/* Test D3Cold-Off */
+				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_D3COLD, &r->ci);
+				/* Test D3Cold-VRAM_SR */
+				set_d3cold_sr_lmem_threshold(debugfs, 0);
+				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_D3COLD, &r->ci);
+				set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);
+			}
 		}
 	}
 
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 7/9] i915_pm_rpm: Test D3Cold with gem_exec_stress

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:46PM +0530, Anshuman Gupta wrote:
> Added d3cold dynamic subtest to gem_exec_stress with device class
> memory region. It test both D3Cold-{VRAM_SR, Off} by using
> d3cold_sr_lmem_threshold i915_params debugfs.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 95 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 78 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index c30862dc4..8e9ff56ec 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -85,6 +85,7 @@ enum plane_type {
>  #define WAIT_PC8_RES	2
>  #define WAIT_EXTRA	4
>  #define USE_DPMS	8
> +#define WAIT_D3COLD	16
>  
>  int drm_fd, msr_fd, pc8_status_fd;
>  int debugfs;
> @@ -215,6 +216,21 @@ static bool wait_for_suspended(void)
>  	}
>  }
>  
> +static bool wait_for_d3cold(struct pci_device *root)
> +{
> +	bool d3colded;
> +
> +	d3colded = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
> +
> +	if (!d3colded) {
> +		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
> +			 root->domain, root->bus, root->dev, root->func);
> +		igt_pm_print_pci_card_runtime_status();
> +	}
> +
> +	return d3colded;
> +}
> +
>  static bool wait_for_active(void)
>  {
>  	if (has_pc8 && !has_runtime_pm)
> @@ -744,6 +760,30 @@ static void test_i2c(struct mode_set_data *data)
>  			"There is an EDID mismatch between i2c and DRM!\n");
>  }
>  
> +static int get_d3cold_sr_lmem_threshold(int dir)
> +{
> +	int param_dir, lmem_threshold, ret;
> +
> +	param_dir = openat(dir, "i915_params", O_RDONLY);
> +	ret = igt_sysfs_scanf(param_dir, "d3cold_sr_lmem_threshold", "%d", &lmem_threshold);
> +	igt_assert(ret >= 0);
> +
> +	close(param_dir);
> +	return lmem_threshold;
> +}
> +
> +static int set_d3cold_sr_lmem_threshold(int dir, int val)
> +{
> +	int param_dir, ret;
> +
> +	param_dir = openat(dir, "i915_params", O_RDONLY);
> +	ret = igt_sysfs_printf(param_dir, "d3cold_sr_lmem_threshold", "%d", val);
> +	igt_assert(ret > 0);
> +
> +	close(param_dir);
> +	return ret;
> +}
> +
>  static void setup_pc8(void)
>  {
>  	has_pc8 = false;
> @@ -1069,6 +1109,19 @@ static void debugfs_forcewake_user_subtest(void)
>  	igt_assert(wait_for_suspended());
>  }
>  
> +static struct pci_device *setup_d3cold_and_get_root_port(void)
> +{
> +	struct pci_device *root;
> +
> +	/* igfx does not support d3cold */
> +	igt_require(IS_DGFX(ms_data.devid));
> +	root = igt_device_get_pci_root_port(drm_fd);
> +	igt_require(igt_pm_acpi_d3cold_supported(root));
> +	igt_pm_setup_pci_card_runtime_pm(root);
> +
> +	return root;
> +}
> +
>  static void gem_mmap_args(const struct mmap_offset *t,
>  			  struct drm_i915_gem_memory_class_instance *mem_regions)
>  {
> @@ -1392,6 +1445,7 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
>  	int i;
>  	int batch_size = 4 * sizeof(uint32_t);
>  	uint32_t batch_buf[batch_size];
> +	struct pci_device *root;
>  	uint32_t handle;
>  	struct drm_i915_gem_execbuffer2 execbuf = {};
>  	struct drm_i915_gem_exec_object2 objs[1] = {{}};
> @@ -1408,6 +1462,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
>  	batch_buf[i++] = MI_NOOP;
>  	igt_assert(i * sizeof(uint32_t) == batch_size);
>  
> +	if (wait_flags & WAIT_D3COLD)
> +		root = setup_d3cold_and_get_root_port();
> +
>  	disable_all_screens_and_wait(&ms_data);
>  
>  	/* PC8 test is only applicable to igfx  */
> @@ -1433,6 +1490,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
>  			/* clean up idle work */
>  			igt_drop_caches_set(drm_fd, DROP_IDLE);
>  			igt_assert(wait_for_suspended());
> +			if (wait_flags & WAIT_D3COLD)
> +				igt_assert(wait_for_d3cold(root));
> +
>  		}
>  		if (wait_flags & WAIT_PC8_RES)
>  			igt_assert(pc8_plus_residency_changed(30));
> @@ -1440,6 +1500,9 @@ gem_execbuf_stress_subtest(int rounds, int wait_flags,
>  			sleep(5);
>  	}
>  
> +	if (wait_flags & WAIT_D3COLD)
> +		igt_pm_restore_pci_card_runtime_pm();
> +
>  	gem_close(drm_fd, handle);
>  }
>  
> @@ -1537,25 +1600,10 @@ __noreturn static void stay_subtest(void)
>  static void d3cold_basic_subtest(void)
>  {
>  	struct pci_device *root;
> -	bool result;
>  
> -	/* igfx does not support d3cold */
> -	igt_require(IS_DGFX(ms_data.devid));
> -
> -	root = igt_device_get_pci_root_port(drm_fd);
> +	root = setup_d3cold_and_get_root_port();
>  	disable_all_screens_and_wait(&ms_data);
> -	igt_require(igt_pm_acpi_d3cold_supported(root));
> -	igt_pm_setup_pci_card_runtime_pm(root);
> -
> -	result = igt_wait(igt_pm_get_acpi_real_d_state(root) == IGT_ACPI_D3Cold, 10000, 500);
> -
> -	if (!result) {
> -		igt_info("D3Cold not achieved for root port %04x:%02x:%02x.%01x\n",
> -			 root->domain, root->bus, root->dev, root->func);
> -		igt_pm_print_pci_card_runtime_status();
> -	}
> -
> -	igt_assert(result);
> +	igt_assert(wait_for_d3cold(root));
>  	igt_pm_restore_pci_card_runtime_pm();
>  }
>  
> @@ -2213,6 +2261,19 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  				gem_execbuf_stress_subtest(rounds, WAIT_STATUS, &r->ci);
>  			igt_dynamic_f("%s-%s", "extra-wait", r->name)
>  				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_EXTRA, &r->ci);
> +			if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
> +				continue;
> +			igt_dynamic_f("%s-%s", "d3cold", r->name) {
> +				int lmem_threshold;
> +
> +				lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
> +				/* Test D3Cold-Off */
> +				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_D3COLD, &r->ci);
> +				/* Test D3Cold-VRAM_SR */

I believe it deserves 2 subtests:
igt_dynamic_f("%s-%s", "d3cold-off", r->name) ...
igt_dynamic_f("%s-%s", "d3cold-sr", r->name) ...

> +				set_d3cold_sr_lmem_threshold(debugfs, 0);
> +				gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_D3COLD, &r->ci);
> +				set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);
> +			}
>  		}
>  	}
>  
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 8/9] i915_pm_rpm: Extend gem_execbuf test with D3Cold

[Anshuman Gupta]

Added d3cold dynamic subtest to gem_execbuf with device class
memory region. It test both D3Cold-{VRAM_SR, Off} by using
d3cold_sr_lmem_threshold i915_params debugfs.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 8e9ff56ec..414797bc8 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -1344,10 +1344,11 @@ static void submit_blt_cmd(uint32_t dst_handle, int dst_size,
 }
 
 /* Make sure we can submit a batch buffer and verify its result. */
-static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions)
+static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions, bool d3cold)
 {
 	int x, y;
 	uint32_t handle;
+	struct pci_device *root;
 	int bpp = 4;
 	int pitch = 128 * bpp;
 	int dst_size = 128 * 128 * bpp; /* 128x128 square */
@@ -1369,6 +1370,9 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
 	memset(cpu_buf, 0, dst_size);
 	gem_write(drm_fd, handle, 0, cpu_buf, dst_size);
 
+	if (d3cold)
+		root = setup_d3cold_and_get_root_port();
+
 	/* Now suspend and try it. */
 	disable_all_screens_and_wait(&ms_data);
 
@@ -1376,9 +1380,14 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
 	submit_blt_cmd(handle, dst_size, sq_x, sq_y, sq_w, sq_h, pitch, color,
 		       &presumed_offset);
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
+
 	for (y = 0; y < 128; y++) {
 		for (x = 0; x < 128; x++) {
 			uint32_t px = cpu_buf[y * 128 + x];
@@ -1416,6 +1425,8 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
 		       &presumed_offset);
 
 	disable_all_screens_and_wait(&ms_data);
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	memset(cpu_buf, 0, dst_size);
 	gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
@@ -2172,7 +2183,18 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	igt_subtest_with_dynamic("gem-execbuf") {
 		for_each_memory_region(r, drm_fd) {
 			igt_dynamic_f("%s", r->name)
-				gem_execbuf_subtest(&r->ci);
+				gem_execbuf_subtest(&r->ci, false);
+			if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
+				continue;
+			igt_dynamic_f("%s-%s", "d3cold", r->name) {
+				int lmem_threshold;
+
+				lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
+				gem_execbuf_subtest(&r->ci, true);
+				set_d3cold_sr_lmem_threshold(debugfs, 0);
+				gem_execbuf_subtest(&r->ci, true);
+				set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);
+			}
 		}
 	}
 	igt_subtest("gem-idle")
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 8/9] i915_pm_rpm: Extend gem_execbuf test with D3Cold

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:47PM +0530, Anshuman Gupta wrote:
> Added d3cold dynamic subtest to gem_execbuf with device class
> memory region. It test both D3Cold-{VRAM_SR, Off} by using
> d3cold_sr_lmem_threshold i915_params debugfs.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 8e9ff56ec..414797bc8 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1344,10 +1344,11 @@ static void submit_blt_cmd(uint32_t dst_handle, int dst_size,
>  }
>  
>  /* Make sure we can submit a batch buffer and verify its result. */
> -static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions)
> +static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_regions, bool d3cold)
>  {
>  	int x, y;
>  	uint32_t handle;
> +	struct pci_device *root;
>  	int bpp = 4;
>  	int pitch = 128 * bpp;
>  	int dst_size = 128 * 128 * bpp; /* 128x128 square */
> @@ -1369,6 +1370,9 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
>  	memset(cpu_buf, 0, dst_size);
>  	gem_write(drm_fd, handle, 0, cpu_buf, dst_size);
>  
> +	if (d3cold)
> +		root = setup_d3cold_and_get_root_port();
> +
>  	/* Now suspend and try it. */
>  	disable_all_screens_and_wait(&ms_data);
>  
> @@ -1376,9 +1380,14 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
>  	submit_blt_cmd(handle, dst_size, sq_x, sq_y, sq_w, sq_h, pitch, color,
>  		       &presumed_offset);
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
> +
>  	for (y = 0; y < 128; y++) {
>  		for (x = 0; x < 128; x++) {
>  			uint32_t px = cpu_buf[y * 128 + x];
> @@ -1416,6 +1425,8 @@ static void gem_execbuf_subtest(struct drm_i915_gem_memory_class_instance *mem_r
>  		       &presumed_offset);
>  
>  	disable_all_screens_and_wait(&ms_data);
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	memset(cpu_buf, 0, dst_size);
>  	gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
> @@ -2172,7 +2183,18 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	igt_subtest_with_dynamic("gem-execbuf") {
>  		for_each_memory_region(r, drm_fd) {
>  			igt_dynamic_f("%s", r->name)
> -				gem_execbuf_subtest(&r->ci);
> +				gem_execbuf_subtest(&r->ci, false);
> +			if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
> +				continue;
> +			igt_dynamic_f("%s-%s", "d3cold", r->name) {
> +				int lmem_threshold;
> +
> +				lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
> +				gem_execbuf_subtest(&r->ci, true);
> +				set_d3cold_sr_lmem_threshold(debugfs, 0);
> +				gem_execbuf_subtest(&r->ci, true);

here as well... we should have 2 separated subtests

> +				set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);
> +			}
>  		}
>  	}
>  	igt_subtest("gem-idle")
> -- 
> 2.26.2
> 

[igt-dev] [PATCH i-g-t 9/9] i915_pm_rpm: Extend gem-mmap-type test with D3Cold

[Anshuman Gupta]

Added d3cold dynamic subtest to gem-mmap-type with device class
memory region. It test both D3Cold-{VRAM_SR, Off} by using
d3cold_sr_lmem_threshold i915_params debugfs.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 414797bc8..2ef8e53a8 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -1123,11 +1123,12 @@ static struct pci_device *setup_d3cold_and_get_root_port(void)
 }
 
 static void gem_mmap_args(const struct mmap_offset *t,
-			  struct drm_i915_gem_memory_class_instance *mem_regions)
+			  struct drm_i915_gem_memory_class_instance *mem_regions, bool d3cold)
 {
 	int i;
 	uint32_t handle;
 	int buf_size = 8192;
+	struct pci_device *root;
 	uint8_t *gem_buf;
 
 	/* Create, map and set data while the device is active. */
@@ -1145,16 +1146,25 @@ static void gem_mmap_args(const struct mmap_offset *t,
 	for (i = 0; i < buf_size; i++)
 		igt_assert(gem_buf[i] == (i & 0xFF));
 
+	if (d3cold)
+		root = setup_d3cold_and_get_root_port();
+
 	/* Now suspend, read and modify. */
 	disable_all_screens_and_wait(&ms_data);
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	for (i = 0; i < buf_size; i++)
 		igt_assert(gem_buf[i] == (i & 0xFF));
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	for (i = 0; i < buf_size; i++)
 		gem_buf[i] = (~i & 0xFF);
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	/* Now resume and see if it's still there. */
 	enable_one_screen_and_wait(&ms_data);
@@ -1166,12 +1176,16 @@ static void gem_mmap_args(const struct mmap_offset *t,
 	/* Now the opposite: suspend, and try to create the mmap while
 	 * suspended. */
 	disable_all_screens_and_wait(&ms_data);
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	gem_buf = __gem_mmap_offset(drm_fd, handle, 0, buf_size,
 				    PROT_READ | PROT_WRITE, t->type);
 	igt_require(gem_buf);
 
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	for (i = 0; i < buf_size; i++)
 		gem_buf[i] = i & 0xFF;
@@ -1180,6 +1194,8 @@ static void gem_mmap_args(const struct mmap_offset *t,
 		igt_assert(gem_buf[i] == (i & 0xFF));
 
 	igt_assert(wait_for_suspended());
+	if (d3cold)
+		igt_assert(wait_for_d3cold(root));
 
 	/* Resume and check if it's still there. */
 	enable_one_screen_and_wait(&ms_data);
@@ -2173,7 +2189,18 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		for_each_mmap_offset_type(drm_fd, t) {
 			for_each_memory_region(r, drm_fd) {
 				igt_dynamic_f("%s-%s", t->name, r->name)
-				gem_mmap_args(t, &r->ci);
+				gem_mmap_args(t, &r->ci, false);
+				if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
+					continue;
+				igt_dynamic_f("%s-%s-%s", t->name, "d3cold", r->name) {
+					int lmem_threshold;
+
+					lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
+					gem_mmap_args(t, &r->ci, true);
+					set_d3cold_sr_lmem_threshold(debugfs, 0);
+					gem_mmap_args(t, &r->ci, true);
+					set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);
+				}
 			}
 		}
 	}
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 9/9] i915_pm_rpm: Extend gem-mmap-type test with D3Cold

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 06:20:48PM +0530, Anshuman Gupta wrote:
> Added d3cold dynamic subtest to gem-mmap-type with device class
> memory region. It test both D3Cold-{VRAM_SR, Off} by using
> d3cold_sr_lmem_threshold i915_params debugfs.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 414797bc8..2ef8e53a8 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1123,11 +1123,12 @@ static struct pci_device *setup_d3cold_and_get_root_port(void)
>  }
>  
>  static void gem_mmap_args(const struct mmap_offset *t,
> -			  struct drm_i915_gem_memory_class_instance *mem_regions)
> +			  struct drm_i915_gem_memory_class_instance *mem_regions, bool d3cold)
>  {
>  	int i;
>  	uint32_t handle;
>  	int buf_size = 8192;
> +	struct pci_device *root;
>  	uint8_t *gem_buf;
>  
>  	/* Create, map and set data while the device is active. */
> @@ -1145,16 +1146,25 @@ static void gem_mmap_args(const struct mmap_offset *t,
>  	for (i = 0; i < buf_size; i++)
>  		igt_assert(gem_buf[i] == (i & 0xFF));
>  
> +	if (d3cold)
> +		root = setup_d3cold_and_get_root_port();
> +
>  	/* Now suspend, read and modify. */
>  	disable_all_screens_and_wait(&ms_data);
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	for (i = 0; i < buf_size; i++)
>  		igt_assert(gem_buf[i] == (i & 0xFF));
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	for (i = 0; i < buf_size; i++)
>  		gem_buf[i] = (~i & 0xFF);
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	/* Now resume and see if it's still there. */
>  	enable_one_screen_and_wait(&ms_data);
> @@ -1166,12 +1176,16 @@ static void gem_mmap_args(const struct mmap_offset *t,
>  	/* Now the opposite: suspend, and try to create the mmap while
>  	 * suspended. */
>  	disable_all_screens_and_wait(&ms_data);
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	gem_buf = __gem_mmap_offset(drm_fd, handle, 0, buf_size,
>  				    PROT_READ | PROT_WRITE, t->type);
>  	igt_require(gem_buf);
>  
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	for (i = 0; i < buf_size; i++)
>  		gem_buf[i] = i & 0xFF;
> @@ -1180,6 +1194,8 @@ static void gem_mmap_args(const struct mmap_offset *t,
>  		igt_assert(gem_buf[i] == (i & 0xFF));
>  
>  	igt_assert(wait_for_suspended());
> +	if (d3cold)
> +		igt_assert(wait_for_d3cold(root));
>  
>  	/* Resume and check if it's still there. */
>  	enable_one_screen_and_wait(&ms_data);
> @@ -2173,7 +2189,18 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  		for_each_mmap_offset_type(drm_fd, t) {
>  			for_each_memory_region(r, drm_fd) {
>  				igt_dynamic_f("%s-%s", t->name, r->name)
> -				gem_mmap_args(t, &r->ci);
> +				gem_mmap_args(t, &r->ci, false);
> +				if (r->ci.memory_class == I915_MEMORY_CLASS_SYSTEM)
> +					continue;
> +				igt_dynamic_f("%s-%s-%s", t->name, "d3cold", r->name) {
> +					int lmem_threshold;
> +
> +					lmem_threshold = get_d3cold_sr_lmem_threshold(debugfs);
> +					gem_mmap_args(t, &r->ci, true);
> +					set_d3cold_sr_lmem_threshold(debugfs, 0);
> +					gem_mmap_args(t, &r->ci, true);
> +					set_d3cold_sr_lmem_threshold(debugfs, lmem_threshold);

here as well... 2 subtests is probably better so if something fails we know which one..

> +				}
>  			}
>  		}
>  	}
> -- 
> 2.26.2
> 

[igt-dev] ✗ GitLab.Pipeline: warning for D3Cold Tool & IGT

[Patchwork]

== Series Details ==

Series: D3Cold Tool & IGT
URL   : https://patchwork.freedesktop.org/series/102780/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529 for the overview.

test:ninja-test-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/21360839):
  Ok:                   22
  Expected Fail:         3
  Fail:                289
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1650286681:step_script
  section_start:1650286681:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1728 matching files and directories   
  Uploading artifacts as "archive" to coordinator... 201 Created  id=21360839 responseStatus=201 Created token=iQxHfmmd
  section_end:1650286689:upload_artifacts_on_failure
  section_start:1650286689:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1650286690:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529

Re: [igt-dev] ✗ GitLab.Pipeline: warning for D3Cold Tool & IGT

[Rodrigo Vivi]

On Mon, Apr 18, 2022 at 01:01:05PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: D3Cold Tool & IGT
> URL   : https://patchwork.freedesktop.org/series/102780/
> State : warning
> 
> == Summary ==
> 
> Pipeline status: FAILED.
> 
> see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529 for the overview.
> 
> test:ninja-test-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/21360839):
>   Ok:                   22
>   Expected Fail:         3
>   Fail:                289

This is awful, but I couldn't find any current library change in this series
that would justify this amount of fail.

I already glanced through all the patches in this series and will continue
with the review in a hope that I find what is causing this.

I notice that igt_assert_f is used in patch 3, but not modified...

>   Unexpected Pass:       0
>   Skipped:               0
>   Timeout:               0
>   
>   Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
>   section_end:1650286681:step_script
>   section_start:1650286681:upload_artifacts_on_failure
>   Uploading artifacts for failed job
>   Uploading artifacts...
>   build: found 1728 matching files and directories   
>   Uploading artifacts as "archive" to coordinator... 201 Created  id=21360839 responseStatus=201 Created token=iQxHfmmd
>   section_end:1650286689:upload_artifacts_on_failure
>   section_start:1650286689:cleanup_file_variables
>   Cleaning up project directory and file based variables
>   section_end:1650286690:cleanup_file_variables
>   ERROR: Job failed: exit code 1
> 
> == Logs ==
> 
> For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529

Re: [igt-dev] ✗ GitLab.Pipeline: warning for D3Cold Tool & IGT

[Petri Latvala]

On Fri, Apr 22, 2022 at 05:35:49AM -0400, Rodrigo Vivi wrote:
> On Mon, Apr 18, 2022 at 01:01:05PM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: D3Cold Tool & IGT
> > URL   : https://patchwork.freedesktop.org/series/102780/
> > State : warning
> > 
> > == Summary ==
> > 
> > Pipeline status: FAILED.
> > 
> > see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529 for the overview.
> > 
> > test:ninja-test-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/21360839):
> >   Ok:                   22
> >   Expected Fail:         3
> >   Fail:                289
> 
> This is awful, but I couldn't find any current library change in this series
> that would justify this amount of fail.

That's all spurious, the runner failed to exec the test binaries
correctly. Can be ignored.


-- 
Petri Latvala


> 
> I already glanced through all the patches in this series and will continue
> with the review in a hope that I find what is causing this.
> 
> I notice that igt_assert_f is used in patch 3, but not modified...
> 
> >   Unexpected Pass:       0
> >   Skipped:               0
> >   Timeout:               0
> >   
> >   Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
> >   section_end:1650286681:step_script
> >   section_start:1650286681:upload_artifacts_on_failure
> >   Uploading artifacts for failed job
> >   Uploading artifacts...
> >   build: found 1728 matching files and directories   
> >   Uploading artifacts as "archive" to coordinator... 201 Created  id=21360839 responseStatus=201 Created token=iQxHfmmd
> >   section_end:1650286689:upload_artifacts_on_failure
> >   section_start:1650286689:cleanup_file_variables
> >   Cleaning up project directory and file based variables
> >   section_end:1650286690:cleanup_file_variables
> >   ERROR: Job failed: exit code 1
> > 
> > == Logs ==
> > 
> > For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/562529

[igt-dev] ✗ Fi.CI.BAT: failure for D3Cold Tool & IGT

[Patchwork]

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

== Series Details ==

Series: D3Cold Tool & IGT
URL   : https://patchwork.freedesktop.org/series/102780/
State : failure

== Summary ==

CI Bug Log - changes from IGT_6439 -> IGTPW_6939
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_6939 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_6939, 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_6939/index.html

Participating hosts (48 -> 47)
------------------------------

  Additional (1): bat-dg2-8 
  Missing    (2): fi-bsw-cyan fi-bdw-samus 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@slpc:
    - bat-dg1-5:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/bat-dg1-5/igt@i915_selftest@live@slpc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/bat-dg1-5/igt@i915_selftest@live@slpc.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][3] ([i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][5] ([i915#3282])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#3012])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@gt_engines:
    - bat-dg1-6:          [PASS][7] -> [INCOMPLETE][8] ([i915#4418])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/bat-dg1-6/igt@i915_selftest@live@gt_engines.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/bat-dg1-6/igt@i915_selftest@live@gt_engines.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][9] ([fdo#111827]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([i915#4070] / [i915#4103]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][11] ([fdo#109285] / [i915#4098])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-rkl-11600:       NOTRUN -> [SKIP][12] ([i915#4070] / [i915#533])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-rkl-11600:       NOTRUN -> [SKIP][13] ([i915#1072]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_psr@primary_mmap_gtt.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][14] ([i915#3555] / [i915#4098])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][15] ([i915#3301] / [i915#3708])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - fi-rkl-11600:       NOTRUN -> [SKIP][16] ([i915#3291] / [i915#3708]) +2 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@prime_vgem@basic-write.html

  * igt@runner@aborted:
    - bat-dg1-6:          NOTRUN -> [FAIL][17] ([i915#4312] / [i915#5257])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/bat-dg1-6/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - {bat-rpls-2}:       [DMESG-WARN][18] ([i915#4391]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-rkl-11600:       [INCOMPLETE][20] ([i915#5127]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@gt_lrc:
    - fi-bsw-n3050:       [DMESG-FAIL][22] ([i915#2373]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html

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

  * igt@kms_flip@basic-plain-flip@a-edp1:
    - fi-tgl-u2:          [DMESG-WARN][26] ([i915#402]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6439/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html

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

  [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#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [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#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4418]: https://gitlab.freedesktop.org/drm/intel/issues/4418
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5193]: https://gitlab.freedesktop.org/drm/intel/issues/5193
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5270]: https://gitlab.freedesktop.org/drm/intel/issues/5270
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5275]: https://gitlab.freedesktop.org/drm/intel/issues/5275
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5414]: https://gitlab.freedesktop.org/drm/intel/issues/5414
  [i915#5552]: https://gitlab.freedesktop.org/drm/intel/issues/5552


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

  * CI: CI-20190529 -> None
  * IGT: IGT_6439 -> IGTPW_6939

  CI-20190529: 20190529
  CI_DRM_11509: d97978df553d768e457cb68c637b2b0a6188b87c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6939: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6939/index.html
  IGT_6439: 0c6c92745d89c8244d6af8732a0dd03f45ac2030 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git



== Testlist changes ==

+igt@i915_pm_rpm@d3cold-basic

== Logs ==

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

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