[igt-dev] [PATCH i-g-t v2 0/3] Add d3 mmap test

[igt-dev] [PATCH i-g-t v2 0/3] Add d3 mmap test

[Anshuman Gupta]

Adding d3 mmap test and adding forcewake igt exit handler to 
vram-d3cold-threshold as well.

Anshuman Gupta (3):
  tests/xe_pm: Add d3-mmap IGT test
  test/xe_pm: Use fw_handle exit handler for vram-d3cold-threshold
  HAX: Add d3-mmap to xe-fast-feedback

 tests/intel-ci/xe-fast-feedback.testlist |   1 +
 tests/intel/xe_pm.c                      | 114 +++++++++++++++++++++--
 2 files changed, 106 insertions(+), 9 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t v2 1/3] tests/xe_pm: Add d3-mmap IGT test

[Anshuman Gupta]

Adding a test to validate mmap memory mappings along with runtime
suspend and resume for both xe device and it's pci parent bridge
in device hierarchy.

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

diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index fd28d5630..5495da154 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -39,6 +39,7 @@ typedef struct {
 } device_t;
 
 uint64_t orig_threshold;
+int fw_handle = -1;
 
 /* runtime_usage is only available if kernel build CONFIG_PM_ADVANCED_DEBUG */
 static bool runtime_usage_available(struct pci_device *pci)
@@ -166,6 +167,14 @@ static bool out_of_d3(device_t device, enum igt_acpi_d_state state)
 	return true;
 }
 
+static void close_fw_handle(int sig)
+{
+	if (fw_handle < 0)
+		return;
+
+	close(fw_handle);
+}
+
 /**
  * SUBTEST: %s-basic
  * Description: set GPU state to %arg[1] and test suspend/autoresume
@@ -437,6 +446,68 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
 	igt_assert(in_d3(device, IGT_ACPI_D3Cold));
 }
 
+/**
+ * SUBTEST: d3-mmap-%s
+ * Description:
+ *	Validate mmap memory mapping with d3 state, for %arg[1] region,
+ *	if supported by device.
+ * arg[1]:
+ *
+ * @vram:	vram region
+ * @system:	system region
+ *
+ * Functionality: pm-d3
+ * Run type: FULL
+ */
+static void test_mmap(device_t device, uint32_t flags)
+{
+	size_t bo_size = 8192;
+	uint8_t *map = NULL;
+	uint32_t bo;
+	int i;
+
+	igt_require_f(flags, "Device doesn't support such memory region\n");
+
+	bo = xe_bo_create_flags(device.fd_xe, 0, bo_size, flags);
+	map = xe_bo_map(device.fd_xe, bo, bo_size);
+	igt_assert(map);
+
+	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
+	igt_assert(fw_handle >= 0);
+	igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
+
+	for (i = 0; i < bo_size; i++)
+		map[i] = i & 0xFF;
+
+	for (i = 0; i < bo_size; i++)
+		igt_assert(map[i] == (i & 0xFF));
+
+	/* Runtime suspend and validate the pattern and clear the pattern */
+	close(fw_handle);
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+
+	for (i = 0; i < bo_size; i++)
+		igt_assert(map[i] == (i & 0xFF));
+
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+
+	for (i = 0; i < bo_size; i++)
+		map[i] = (~i & 0xFF);
+
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+
+	/* Runtime resume and check the pattern */
+	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
+	igt_assert(fw_handle >= 0);
+	igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
+	for (i = 0; i < bo_size; i++)
+		igt_assert(map[i] == (~i & 0xFF));
+
+	igt_assert(munmap(map, bo_size) == 0);
+	gem_close(device.fd_xe, bo);
+	close(fw_handle);
+}
+
 igt_main
 {
 	struct drm_xe_engine_class_instance *hwe;
@@ -542,6 +613,29 @@ igt_main
 		test_vram_d3cold_threshold(device, sysfs_fd);
 	}
 
+	igt_subtest_group {
+		igt_fixture {
+			igt_install_exit_handler(close_fw_handle);
+		}
+
+		igt_describe("Validate mmap memory mappings with system region,"
+			     "when device along with parent bridge in d3");
+		igt_subtest("d3-mmap-system") {
+			test_mmap(device, system_memory(device.fd_xe));
+		}
+
+		igt_describe("Validate mmap memory mappings with vram region,"
+			     "when device along with parent bridge in d3");
+		igt_subtest("d3-mmap-vram") {
+			if (device.pci_root != device.pci_xe) {
+				igt_pm_enable_pci_card_runtime_pm(device.pci_root, NULL);
+				igt_pm_set_d3cold_allowed(device.pci_slot_name, 1);
+			}
+
+			test_mmap(device, visible_vram_memory(device.fd_xe, 0));
+		}
+	}
+
 	igt_fixture {
 		close(sysfs_fd);
 		igt_pm_set_d3cold_allowed(device.pci_slot_name, d3cold_allowed);
-- 
2.25.1

[igt-dev] [PATCH i-g-t v2 2/3] test/xe_pm: Use fw_handle exit handler for vram-d3cold-threshold

[Anshuman Gupta]

Scaling forcewake close exit handler for vram-d3cold-threshold
subtest, while doing so add the missing subtest Functionality
as well.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/intel/xe_pm.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index 5495da154..0c15f8d15 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -378,6 +378,7 @@ NULL));
  * Description:
  *	Validate whether card is limited to d3hot while vram used
  *	is greater than vram_d3cold_threshold.
+ * Functionality: pm-d3cold
  */
 static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
 {
@@ -390,7 +391,7 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
 	};
 	uint64_t vram_used_mb = 0, vram_total_mb = 0, threshold;
 	uint32_t bo, flags;
-	int handle, i;
+	int i;
 	bool active;
 	void *map;
 
@@ -436,10 +437,10 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
 	 * the device from runtime suspend.
 	 * Therefore open and close fw handle to wake the device.
 	 */
-	handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
-	igt_assert(handle >= 0);
+	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
+	igt_assert(fw_handle >= 0);
 	active = igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE;
-	close(handle);
+	close(fw_handle);
 	igt_assert(active);
 
 	/* Test D3Cold again after freeing up the Xe BO */
@@ -606,18 +607,19 @@ igt_main
 		}
 	}
 
-	igt_describe("Validate whether card is limited to d3hot, if vram used > vram threshold");
-	igt_subtest("vram-d3cold-threshold") {
-		orig_threshold = get_vram_d3cold_threshold(sysfs_fd);
-		igt_install_exit_handler(vram_d3cold_threshold_restore);
-		test_vram_d3cold_threshold(device, sysfs_fd);
-	}
-
 	igt_subtest_group {
 		igt_fixture {
 			igt_install_exit_handler(close_fw_handle);
 		}
 
+		igt_describe("Validate whether card is limited to d3hot,"
+			     "if vram used > vram threshold");
+		igt_subtest("vram-d3cold-threshold") {
+			orig_threshold = get_vram_d3cold_threshold(sysfs_fd);
+			igt_install_exit_handler(vram_d3cold_threshold_restore);
+			test_vram_d3cold_threshold(device, sysfs_fd);
+		}
+
 		igt_describe("Validate mmap memory mappings with system region,"
 			     "when device along with parent bridge in d3");
 		igt_subtest("d3-mmap-system") {
-- 
2.25.1

[igt-dev] [PATCH i-g-t v2 3/3] HAX: Add d3-mmap to xe-fast-feedback

[Anshuman Gupta]

Adding d3-mmap to xe-fast-feedback.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/intel-ci/xe-fast-feedback.testlist | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/intel-ci/xe-fast-feedback.testlist b/tests/intel-ci/xe-fast-feedback.testlist
index 610cc958c..8357208ab 100644
--- a/tests/intel-ci/xe-fast-feedback.testlist
+++ b/tests/intel-ci/xe-fast-feedback.testlist
@@ -144,6 +144,7 @@ igt@xe_mmap@vram-system
 igt@xe_mmio@mmio-timestamp
 igt@xe_mmio@mmio-invalid
 igt@xe_pm_residency@gt-c6-on-idle
+igt@xe_pm@d3-mmap
 igt@xe_prime_self_import@basic-with_one_bo
 igt@xe_prime_self_import@basic-with_fd_dup
 #igt@xe_prime_self_import@basic-llseek-size
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t v2 1/3] tests/xe_pm: Add d3-mmap IGT test

[Rodrigo Vivi]

On Fri, Sep 22, 2023 at 06:53:59PM +0530, Anshuman Gupta wrote:
> Adding a test to validate mmap memory mappings along with runtime
> suspend and resume for both xe device and it's pci parent bridge
> in device hierarchy.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/intel/xe_pm.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index fd28d5630..5495da154 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -39,6 +39,7 @@ typedef struct {
>  } device_t;
>  
>  uint64_t orig_threshold;
> +int fw_handle = -1;
>  
>  /* runtime_usage is only available if kernel build CONFIG_PM_ADVANCED_DEBUG */
>  static bool runtime_usage_available(struct pci_device *pci)
> @@ -166,6 +167,14 @@ static bool out_of_d3(device_t device, enum igt_acpi_d_state state)
>  	return true;
>  }
>  
> +static void close_fw_handle(int sig)
> +{
> +	if (fw_handle < 0)
> +		return;
> +
> +	close(fw_handle);
> +}

it took me a while to understand why global fw_handle here
and the exit handler install at the end and then the conversion
of the second patch. It makes total sense to ensure that we don't
leave the fw awake after a test issue.
But could you please do this in a separated patch before the
addition of this new test?

> +
>  /**
>   * SUBTEST: %s-basic
>   * Description: set GPU state to %arg[1] and test suspend/autoresume
> @@ -437,6 +446,68 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
>  	igt_assert(in_d3(device, IGT_ACPI_D3Cold));
>  }
>  
> +/**
> + * SUBTEST: d3-mmap-%s
> + * Description:
> + *	Validate mmap memory mapping with d3 state, for %arg[1] region,
> + *	if supported by device.
> + * arg[1]:
> + *
> + * @vram:	vram region
> + * @system:	system region
> + *
> + * Functionality: pm-d3
> + * Run type: FULL
> + */
> +static void test_mmap(device_t device, uint32_t flags)
> +{
> +	size_t bo_size = 8192;
> +	uint8_t *map = NULL;
> +	uint32_t bo;
> +	int i;
> +
> +	igt_require_f(flags, "Device doesn't support such memory region\n");
> +
> +	bo = xe_bo_create_flags(device.fd_xe, 0, bo_size, flags);
> +	map = xe_bo_map(device.fd_xe, bo, bo_size);
> +	igt_assert(map);
> +
> +	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
> +	igt_assert(fw_handle >= 0);
> +	igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
> +
> +	for (i = 0; i < bo_size; i++)
> +		map[i] = i & 0xFF;
> +
> +	for (i = 0; i < bo_size; i++)
> +		igt_assert(map[i] == (i & 0xFF));
> +
> +	/* Runtime suspend and validate the pattern and clear the pattern */
> +	close(fw_handle);
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +
> +	for (i = 0; i < bo_size; i++)
> +		igt_assert(map[i] == (i & 0xFF));
> +
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +
> +	for (i = 0; i < bo_size; i++)
> +		map[i] = (~i & 0xFF);
> +
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +
> +	/* Runtime resume and check the pattern */
> +	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
> +	igt_assert(fw_handle >= 0);
> +	igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);

You are using the forcewake to wake the gpu up, but  I wonder if what we want
here is to also give a time to get to the suspend, then assert SUSPENDED,
and then do the read to the mapped area and ensure that we are ACTIVE because
of the read?
I mean, with this we would be really testing the Badal's case of exit
on page fault, right?

> +	for (i = 0; i < bo_size; i++)
> +		igt_assert(map[i] == (~i & 0xFF));

should we have some kind of 0xc0ffee check instead?
I'm afraid of some cases that we might read garbage 1s
and be okay with it.

> +
> +	igt_assert(munmap(map, bo_size) == 0);
> +	gem_close(device.fd_xe, bo);
> +	close(fw_handle);
> +}
> +
>  igt_main
>  {
>  	struct drm_xe_engine_class_instance *hwe;
> @@ -542,6 +613,29 @@ igt_main
>  		test_vram_d3cold_threshold(device, sysfs_fd);
>  	}
>  
> +	igt_subtest_group {
> +		igt_fixture {
> +			igt_install_exit_handler(close_fw_handle);
> +		}
> +
> +		igt_describe("Validate mmap memory mappings with system region,"
> +			     "when device along with parent bridge in d3");
> +		igt_subtest("d3-mmap-system") {
> +			test_mmap(device, system_memory(device.fd_xe));
> +		}
> +
> +		igt_describe("Validate mmap memory mappings with vram region,"
> +			     "when device along with parent bridge in d3");
> +		igt_subtest("d3-mmap-vram") {
> +			if (device.pci_root != device.pci_xe) {
> +				igt_pm_enable_pci_card_runtime_pm(device.pci_root, NULL);
> +				igt_pm_set_d3cold_allowed(device.pci_slot_name, 1);
> +			}
> +
> +			test_mmap(device, visible_vram_memory(device.fd_xe, 0));
> +		}
> +	}
> +
>  	igt_fixture {
>  		close(sysfs_fd);
>  		igt_pm_set_d3cold_allowed(device.pci_slot_name, d3cold_allowed);
> -- 
> 2.25.1
> 

[igt-dev] ✗ Fi.CI.BAT: failure for Add d3 mmap test

[Patchwork]

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

== Series Details ==

Series: Add d3 mmap test
URL   : https://patchwork.freedesktop.org/series/124115/
State : failure

== Summary ==

CI Bug Log - changes from IGT_7498 -> IGTPW_9851
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_9851 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9851, please notify your bug team (lgci.bug.filing@intel.com) 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_9851/index.html

Participating hosts (39 -> 37)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (3): fi-hsw-4770 bat-dg2-9 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@perf:
    - fi-kbl-soraka:      NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@i915_selftest@live@perf.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][4] ([i915#5334] / [i915#7872])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][5] ([i915#1886] / [i915#7913])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@kms_dsc@dsc-basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][6] ([fdo#109271]) +9 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/fi-kbl-soraka/igt@kms_dsc@dsc-basic.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - bat-adlp-9:         [INCOMPLETE][7] ([i915#4983] / [i915#7913]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7498/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7952]: https://gitlab.freedesktop.org/drm/intel/issues/7952


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7498 -> IGTPW_9851

  CI-20190529: 20190529
  CI_DRM_13669: d89ee1549402313f7362e8f214dba7b8a5261d81 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9851: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/index.html
  IGT_7498: 05d14fd260a3cf9dc00ed24733d5589eee32ec08 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@xe_pm@d3-mmap-system
+igt@xe_pm@d3-mmap-vram

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t v2 1/3] tests/xe_pm: Add d3-mmap IGT test

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Friday, September 22, 2023 7:53 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Dixit, Ashutosh
> <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
> Tauro, Riana <riana.tauro@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>
> Subject: Re: [PATCH i-g-t v2 1/3] tests/xe_pm: Add d3-mmap IGT test
> 
> On Fri, Sep 22, 2023 at 06:53:59PM +0530, Anshuman Gupta wrote:
> > Adding a test to validate mmap memory mappings along with runtime
> > suspend and resume for both xe device and it's pci parent bridge in
> > device hierarchy.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  tests/intel/xe_pm.c | 94
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 94 insertions(+)
> >
> > diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c index
> > fd28d5630..5495da154 100644
> > --- a/tests/intel/xe_pm.c
> > +++ b/tests/intel/xe_pm.c
> > @@ -39,6 +39,7 @@ typedef struct {
> >  } device_t;
> >
> >  uint64_t orig_threshold;
> > +int fw_handle = -1;
> >
> >  /* runtime_usage is only available if kernel build
> > CONFIG_PM_ADVANCED_DEBUG */  static bool
> > runtime_usage_available(struct pci_device *pci) @@ -166,6 +167,14 @@
> static bool out_of_d3(device_t device, enum igt_acpi_d_state state)
> >  	return true;
> >  }
> >
> > +static void close_fw_handle(int sig)
> > +{
> > +	if (fw_handle < 0)
> > +		return;
> > +
> > +	close(fw_handle);
> > +}
> 
> it took me a while to understand why global fw_handle here and the exit
> handler install at the end and then the conversion of the second patch. It
> makes total sense to ensure that we don't leave the fw awake after a test
> issue.
> But could you please do this in a separated patch before the addition of this
> new test?
Sure I will do that.
> 
> > +
> >  /**
> >   * SUBTEST: %s-basic
> >   * Description: set GPU state to %arg[1] and test suspend/autoresume
> > @@ -437,6 +446,68 @@ static void test_vram_d3cold_threshold(device_t
> device, int sysfs_fd)
> >  	igt_assert(in_d3(device, IGT_ACPI_D3Cold));  }
> >
> > +/**
> > + * SUBTEST: d3-mmap-%s
> > + * Description:
> > + *	Validate mmap memory mapping with d3 state, for %arg[1] region,
> > + *	if supported by device.
> > + * arg[1]:
> > + *
> > + * @vram:	vram region
> > + * @system:	system region
> > + *
> > + * Functionality: pm-d3
> > + * Run type: FULL
> > + */
> > +static void test_mmap(device_t device, uint32_t flags) {
> > +	size_t bo_size = 8192;
> > +	uint8_t *map = NULL;
> > +	uint32_t bo;
> > +	int i;
> > +
> > +	igt_require_f(flags, "Device doesn't support such memory region\n");
> > +
> > +	bo = xe_bo_create_flags(device.fd_xe, 0, bo_size, flags);
> > +	map = xe_bo_map(device.fd_xe, bo, bo_size);
> > +	igt_assert(map);
> > +
> > +	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all",
> O_RDONLY);
> > +	igt_assert(fw_handle >= 0);
> > +	igt_assert(igt_get_runtime_pm_status() ==
> > +IGT_RUNTIME_PM_STATUS_ACTIVE);
> > +
> > +	for (i = 0; i < bo_size; i++)
> > +		map[i] = i & 0xFF;
> > +
> > +	for (i = 0; i < bo_size; i++)
> > +		igt_assert(map[i] == (i & 0xFF));
> > +
> > +	/* Runtime suspend and validate the pattern and clear the pattern */
> > +	close(fw_handle);
> > +
> 	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSP
> ENDED));
> > +
> > +	for (i = 0; i < bo_size; i++)
> > +		igt_assert(map[i] == (i & 0xFF));
> > +
> > +
> 	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSP
> ENDED));
> > +
> > +	for (i = 0; i < bo_size; i++)
> > +		map[i] = (~i & 0xFF);
> > +
> > +
> 	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSP
> ENDED));
> > +
> > +	/* Runtime resume and check the pattern */
> > +	fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all",
> O_RDONLY);
> > +	igt_assert(fw_handle >= 0);
> > +	igt_assert(igt_get_runtime_pm_status() ==
> > +IGT_RUNTIME_PM_STATUS_ACTIVE);
> 
> You are using the forcewake to wake the gpu up, but  I wonder if what we
> want here is to also give a time to get to the suspend, then assert SUSPENDED,
> and then do the read to the mapped area and ensure that we are ACTIVE
> because of the read?
> I mean, with this we would be really testing the Badal's case of exit on page
> fault, right?
This test is just validating the pattern in the bo during runtime suspend and resume, this test is agnostic for vram region and system region.
Bo. 
To validate the page fault case, we would need a test specific to vram region. I will do that change. 

Thanks,
Anshuman.
  
> 
> > +	for (i = 0; i < bo_size; i++)
> > +		igt_assert(map[i] == (~i & 0xFF));
> 
> should we have some kind of 0xc0ffee check instead?
> I'm afraid of some cases that we might read garbage 1s and be okay with it.
> 
> > +
> > +	igt_assert(munmap(map, bo_size) == 0);
> > +	gem_close(device.fd_xe, bo);
> > +	close(fw_handle);
> > +}
> > +
> >  igt_main
> >  {
> >  	struct drm_xe_engine_class_instance *hwe; @@ -542,6 +613,29 @@
> > igt_main
> >  		test_vram_d3cold_threshold(device, sysfs_fd);
> >  	}
> >
> > +	igt_subtest_group {
> > +		igt_fixture {
> > +			igt_install_exit_handler(close_fw_handle);
> > +		}
> > +
> > +		igt_describe("Validate mmap memory mappings with system
> region,"
> > +			     "when device along with parent bridge in d3");
> > +		igt_subtest("d3-mmap-system") {
> > +			test_mmap(device, system_memory(device.fd_xe));
> > +		}
> > +
> > +		igt_describe("Validate mmap memory mappings with vram
> region,"
> > +			     "when device along with parent bridge in d3");
> > +		igt_subtest("d3-mmap-vram") {
> > +			if (device.pci_root != device.pci_xe) {
> > +
> 	igt_pm_enable_pci_card_runtime_pm(device.pci_root, NULL);
> > +
> 	igt_pm_set_d3cold_allowed(device.pci_slot_name, 1);
> > +			}
> > +
> > +			test_mmap(device,
> visible_vram_memory(device.fd_xe, 0));
> > +		}
> > +	}
> > +
> >  	igt_fixture {
> >  		close(sysfs_fd);
> >  		igt_pm_set_d3cold_allowed(device.pci_slot_name,
> d3cold_allowed);
> > --
> > 2.25.1
> >

[igt-dev] ✓ CI.xeBAT: success for Add d3 mmap test

[Patchwork]

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

== Series Details ==

Series: Add d3 mmap test
URL   : https://patchwork.freedesktop.org/series/124115/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7498_BAT -> XEIGTPW_9851_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@xe_pm@d3-mmap}:
    - bat-atsm-2:         NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-atsm-2/igt@xe_pm@d3-mmap.html
    - bat-adlp-7:         NOTRUN -> [SKIP][2]
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-adlp-7/igt@xe_pm@d3-mmap.html
    - bat-pvc-2:          NOTRUN -> [SKIP][3]
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-pvc-2/igt@xe_pm@d3-mmap.html
    - bat-dg2-oem2:       NOTRUN -> [SKIP][4]
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@xe_pm@d3-mmap.html

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - bat-dg2-oem2:       [INCOMPLETE][5] ([Intel XE#729]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@core_hotunplug@unbind-rebind.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_flip@basic-flip-vs-modeset@b-dp3:
    - bat-dg2-oem2:       [FAIL][7] ([Intel XE#708]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-modeset@b-dp3.html
   [8]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-modeset@b-dp3.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3:
    - bat-dg2-oem2:       [FAIL][9] ([Intel XE#554]) -> [PASS][10] +10 other tests pass
   [9]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html
   [10]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-a-dp-3:
    - bat-dg2-oem2:       [DMESG-WARN][11] ([Intel XE#547]) -> [PASS][12] +25 other tests pass
   [11]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-a-dp-3.html
   [12]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-a-dp-3.html

  * igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-dp-3:
    - bat-dg2-oem2:       [DMESG-WARN][13] ([Intel XE#712]) -> [PASS][14] +2 other tests pass
   [13]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-dp-3.html
   [14]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_pipe_crc_basic@hang-read-crc@pipe-c-dp-3.html

  * igt@kms_pipe_crc_basic@hang-read-crc@pipe-d-dp-3:
    - bat-dg2-oem2:       [DMESG-FAIL][15] ([Intel XE#712]) -> [PASS][16] +2 other tests pass
   [15]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_pipe_crc_basic@hang-read-crc@pipe-d-dp-3.html
   [16]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_pipe_crc_basic@hang-read-crc@pipe-d-dp-3.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@basic:
    - bat-dg2-oem2:       [DMESG-WARN][17] ([Intel XE#712]) -> [FAIL][18] ([Intel XE#608])
   [17]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_frontbuffer_tracking@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12:
    - bat-dg2-oem2:       [DMESG-WARN][19] ([Intel XE#547]) -> [FAIL][20] ([Intel XE#400]) +2 other tests fail
   [19]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7498/bat-dg2-oem2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12.html
   [20]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/bat-dg2-oem2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12.html

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

  [Intel XE#400]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/400
  [Intel XE#547]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/547
  [Intel XE#554]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/554
  [Intel XE#608]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/608
  [Intel XE#708]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/708
  [Intel XE#712]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/712
  [Intel XE#729]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/729


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

  * IGT: IGT_7498 -> IGTPW_9851

  IGTPW_9851: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9851/index.html
  IGT_7498: 05d14fd260a3cf9dc00ed24733d5589eee32ec08 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-390-6149acb947f2f8b65ee1a058982a5d6fce3124ec: 6149acb947f2f8b65ee1a058982a5d6fce3124ec

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9851/index.html

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