[igt-dev] [PATCH i-g-t 0/2] Disable d3cold_allowed for basic-s2idle-without-i915

[igt-dev] [PATCH i-g-t 0/2] Disable d3cold_allowed for basic-s2idle-without-i915

[Riana Tauro]

When module is unloaded and s2idle is triggered,
PCI core leaves the endpoint in D0 and the bridge in D3 state causing
PCIE spec violation. The config space is read as 0xFF

Keep the bridge in D0 before module unload to prevent
this issue

Riana Tauro (2):
  lib/igt_pm : Add pm functions for d3cold_allowed
  tests/i915/i915_suspend: Disable d3cold_allowed for
    basic-s2idle-without-i915

 lib/igt_pm.c              | 46 +++++++++++++++++++++++++++++++++++++++
 lib/igt_pm.h              |  4 ++++
 tests/i915/i915_suspend.c | 28 ++++++++++++++++++++++++
 3 files changed, 78 insertions(+)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/2] lib/igt_pm : Add pm functions for d3cold_allowed

[Riana Tauro]

Add functions to set/get d3cold_allowed attribute of
the pci_device

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 lib/igt_pm.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_pm.h |  4 ++++
 2 files changed, 50 insertions(+)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 99251b40..ce34a0a4 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -1122,6 +1122,52 @@ void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
 	igt_pm_setup_pci_card_power_attrs(pci_dev, true, 0);
 }
 
+/**
+ * igt_pm_get_d3cold_allowed:
+ * @igt_device_card: device card
+ * @val: value to be read into
+ *
+ * Reads the value of d3cold_allowed attribute
+ * of the pci device
+ */
+void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char *val)
+{
+	char name[PATH_MAX];
+	int fd;
+
+	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
+		 card->pci_slot_name);
+
+	fd = open(name, O_RDONLY);
+	igt_assert_f(fd >= 0, "Can't open %s\n", name);
+
+	igt_sysfs_read(fd, "d3cold_allowed", val, sizeof(val));
+
+	close(fd);
+}
+
+/**
+ * igt_pm_get_d3cold_allowed:
+ * @igt_device_card: device card
+ * @val: value to be written
+ *
+ * writes the value to d3cold_allowed attribute of pci device
+ */
+void igt_pm_set_d3cold_allowed(struct igt_device_card *card, const char *val)
+{
+	char name[PATH_MAX];
+	int fd;
+
+	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
+		 card->pci_slot_name);
+
+	fd = open(name, O_RDONLY);
+	igt_assert_f(fd >= 0, "Can't open %s\n", name);
+
+	igt_sysfs_write(fd, "d3cold_allowed", val, sizeof(val));
+	close(fd);
+}
+
 static void
 igt_pm_restore_power_attr(struct pci_device *pci_dev, const char *attr, char *val, int len)
 {
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index cbbde12b..d9db8100 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -24,6 +24,8 @@
 #ifndef IGT_PM_H
 #define IGT_PM_H
 
+#include "igt_device_scan.h"
+
 void igt_pm_enable_audio_runtime_pm(void);
 void igt_pm_enable_sata_link_power_management(void);
 void igt_pm_restore_sata_link_power_management(void);
@@ -76,6 +78,8 @@ enum igt_acpi_d_state
 igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);
 void igt_pm_enable_pci_card_runtime_pm(struct pci_device *root,
 				       struct pci_device *i915);
+void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char *val);
+void igt_pm_set_d3cold_allowed(struct igt_device_card *card, const char *val);
 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);
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/2] tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915

[Riana Tauro]

When module is unloaded and s2idle is triggered,
PCI core leaves the endpoint in D0 and the bridge in D3 state causing
PCIE spec violation. The config space is read as 0xFF

Keep the bridge in D0 before module unload to prevent
this issue

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 tests/i915/i915_suspend.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
index 94935595..a6769d64 100644
--- a/tests/i915/i915_suspend.c
+++ b/tests/i915/i915_suspend.c
@@ -43,6 +43,7 @@
 #include "igt.h"
 #include "igt_kmod.h"
 #include "igt_device.h"
+#include "igt_device_scan.h"
 
 #define OBJECT_SIZE (16*1024*1024)
 
@@ -206,13 +207,40 @@ test_forcewake(int fd, bool hibernate)
 static void
 test_suspend_without_i915(int state)
 {
+	struct igt_device_card card;
+	char d3cold_allowed[2];
+	int fd;
+
+	fd = __drm_open_driver(DRIVER_INTEL);
+	igt_devices_scan(false);
+
+	/*
+	 * When module is unloaded and s2idle is triggered,
+	 * PCI core leaves the endpoint in D0 and the bridge in
+	 * D3 state causing PCIE spec violation.
+	 *
+	 * Keep the bridge in D0 before module unload to prevent
+	 * this issue
+	 **/
+	if (state == SUSPEND_STATE_FREEZE &&
+	    igt_device_find_first_i915_discrete_card(&card)) {
+		igt_pm_get_d3cold_allowed(&card, d3cold_allowed);
+		igt_pm_set_d3cold_allowed(&card, "0\n");
+	}
+	close(fd);
+
 	igt_kmsg(KMSG_INFO "Unloading i915\n");
 	igt_assert_eq(igt_i915_driver_unload(),0);
 
 	igt_system_suspend_autoresume(state, SUSPEND_TEST_NONE);
 
+	if (state == SUSPEND_STATE_FREEZE && strlen(card.card))
+		igt_pm_set_d3cold_allowed(&card, d3cold_allowed);
+
 	igt_kmsg(KMSG_INFO "Re-loading i915 \n");
 	igt_assert_eq(igt_i915_driver_load(NULL), 0);
+
+	igt_devices_free();
 }
 
 int fd;
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_pm : Add pm functions for d3cold_allowed

[Gupta, Anshuman]

> -----Original Message-----
> From: Tauro, Riana <riana.tauro@intel.com>
> Sent: Thursday, September 8, 2022 8:16 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: [PATCH i-g-t 1/2] lib/igt_pm : Add pm functions for d3cold_allowed
> 
> Add functions to set/get d3cold_allowed attribute of the pci_device
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  lib/igt_pm.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_pm.h |  4 ++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 99251b40..ce34a0a4 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -1122,6 +1122,52 @@ void igt_pm_setup_pci_card_runtime_pm(struct
> pci_device *pci_dev)
>  	igt_pm_setup_pci_card_power_attrs(pci_dev, true, 0);  }
> 
> +/**
> + * igt_pm_get_d3cold_allowed:
> + * @igt_device_card: device card
> + * @val: value to be read into
> + *
> + * Reads the value of d3cold_allowed attribute
> + * of the pci device
> + */
> +void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char *val)
> +{
> +	char name[PATH_MAX];
> +	int fd;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
> +		 card->pci_slot_name);
> +
> +	fd = open(name, O_RDONLY);
> +	igt_assert_f(fd >= 0, "Can't open %s\n", name);
> +
> +	igt_sysfs_read(fd, "d3cold_allowed", val, sizeof(val));
> +
> +	close(fd);
> +}
> +
> +/**
> + * igt_pm_get_d3cold_allowed:
> + * @igt_device_card: device card
> + * @val: value to be written
> + *
> + * writes the value to d3cold_allowed attribute of pci device  */ void
> +igt_pm_set_d3cold_allowed(struct igt_device_card *card, const char
> +*val) {
> +	char name[PATH_MAX];
> +	int fd;
> +
> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
> +		 card->pci_slot_name);
> +
> +	fd = open(name, O_RDONLY);
> +	igt_assert_f(fd >= 0, "Can't open %s\n", name);
> +
> +	igt_sysfs_write(fd, "d3cold_allowed", val, sizeof(val));
> +	close(fd);
> +}
> +
>  static void
>  igt_pm_restore_power_attr(struct pci_device *pci_dev, const char *attr, char
> *val, int len)  { diff --git a/lib/igt_pm.h b/lib/igt_pm.h index cbbde12b..d9db8100
> 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -24,6 +24,8 @@
>  #ifndef IGT_PM_H
>  #define IGT_PM_H
> 
> +#include "igt_device_scan.h"
Please include this from c file, where it is needed.
Thanks,
Anshuman.
> +
>  void igt_pm_enable_audio_runtime_pm(void);
>  void igt_pm_enable_sata_link_power_management(void);
>  void igt_pm_restore_sata_link_power_management(void);
> @@ -76,6 +78,8 @@ enum igt_acpi_d_state
>  igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);  void
> igt_pm_enable_pci_card_runtime_pm(struct pci_device *root,
>  				       struct pci_device *i915);
> +void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char
> +*val); void igt_pm_set_d3cold_allowed(struct igt_device_card *card,
> +const char *val);
>  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);
> --
> 2.25.1

[igt-dev] ✗ Fi.CI.BAT: failure for Disable d3cold_allowed for basic-s2idle-without-i915

[Patchwork]

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

== Series Details ==

Series: Disable d3cold_allowed for basic-s2idle-without-i915
URL   : https://patchwork.freedesktop.org/series/108305/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12097 -> IGTPW_7757
====================================================

Summary
-------

  **FAILURE**

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

Participating hosts (41 -> 39)
------------------------------

  Missing    (2): fi-bdw-samus fi-pnv-d510 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-dg1-5:          NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/bat-dg1-5/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Suppressed ####

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

  * igt@gem_exec_suspend@basic-s0@lmem0:
    - {bat-dg2-11}:       NOTRUN -> [DMESG-WARN][2] +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/bat-dg2-11/igt@gem_exec_suspend@basic-s0@lmem0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - fi-cfl-8109u:       NOTRUN -> [INCOMPLETE][3] ([i915#6114])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-cfl-8109u/igt@i915_selftest@live.html

  * igt@i915_selftest@live@execlists:
    - fi-bdw-gvtdvm:      [PASS][4] -> [INCOMPLETE][5] ([i915#2940])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/fi-bdw-gvtdvm/igt@i915_selftest@live@execlists.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-bdw-gvtdvm/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gem:
    - fi-blb-e6850:       [PASS][6] -> [DMESG-FAIL][7] ([i915#4528])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/fi-blb-e6850/igt@i915_selftest@live@gem.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-blb-e6850/igt@i915_selftest@live@gem.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][8] ([fdo#109271] / [fdo#111827])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-bdw-5557u/igt@kms_chamelium@common-hpd-after-suspend.html
    - bat-dg1-5:          NOTRUN -> [SKIP][9] ([fdo#111827])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/bat-dg1-5/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cfl-8109u:       [PASS][10] -> [DMESG-FAIL][11] ([i915#62]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-cfl-8109u:       [PASS][12] -> [DMESG-WARN][13] ([i915#62]) +10 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/fi-cfl-8109u/igt@prime_vgem@basic-fence-flip.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-cfl-8109u/igt@prime_vgem@basic-fence-flip.html

  * igt@runner@aborted:
    - fi-cfl-8109u:       NOTRUN -> [FAIL][14] ([i915#4312] / [i915#6599])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-cfl-8109u/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][15] ([fdo#109271] / [i915#2403] / [i915#4312])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-blb-e6850/igt@runner@aborted.html
    - fi-bdw-gvtdvm:      NOTRUN -> [FAIL][16] ([i915#4312])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-bdw-gvtdvm/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-dg1-5:          [INCOMPLETE][17] ([i915#6011]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/bat-dg1-5/igt@i915_suspend@basic-s2idle-without-i915.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/bat-dg1-5/igt@i915_suspend@basic-s2idle-without-i915.html
    - {bat-dg2-11}:       [INCOMPLETE][19] ([i915#4817] / [i915#6011]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/bat-dg2-11/igt@i915_suspend@basic-s2idle-without-i915.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/bat-dg2-11/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [FAIL][21] ([i915#6298]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12097/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#6011]: https://gitlab.freedesktop.org/drm/intel/issues/6011
  [i915#6114]: https://gitlab.freedesktop.org/drm/intel/issues/6114
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6599]: https://gitlab.freedesktop.org/drm/intel/issues/6599


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

  * CI: CI-20190529 -> None
  * IGT: IGT_6648 -> IGTPW_7757

  CI-20190529: 20190529
  CI_DRM_12097: 1fb88df487169c0b6d8682967b497ab6006a728f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_7757: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7757/index.html
  IGT_6648: 3c9079c0b97445fbfc903b9c5a1d69707b80af80 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t 2/2] tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915

[Gupta, Anshuman]

> -----Original Message-----
> From: Tauro, Riana <riana.tauro@intel.com>
> Sent: Thursday, September 8, 2022 8:16 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: [PATCH i-g-t 2/2] tests/i915/i915_suspend: Disable d3cold_allowed for
> basic-s2idle-without-i915
> 
> When module is unloaded and s2idle is triggered, PCI core leaves the endpoint in
> D0 and the bridge in D3 state causing PCIE spec violation. The config space is
> read as 0xFF
> 
> Keep the bridge in D0 before module unload to prevent this issue
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  tests/i915/i915_suspend.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c index
> 94935595..a6769d64 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -43,6 +43,7 @@
>  #include "igt.h"
>  #include "igt_kmod.h"
>  #include "igt_device.h"
> +#include "igt_device_scan.h"
> 
>  #define OBJECT_SIZE (16*1024*1024)
> 
> @@ -206,13 +207,40 @@ test_forcewake(int fd, bool hibernate)  static void
> test_suspend_without_i915(int state)  {
> +	struct igt_device_card card;
> +	char d3cold_allowed[2];
> +	int fd;
Why do we need fd ?
> +
> +	fd = __drm_open_driver(DRIVER_INTEL);
> +	igt_devices_scan(false);
> +
> +	/*
> +	 * When module is unloaded and s2idle is triggered,
> +	 * PCI core leaves the endpoint in D0 and the bridge in
> +	 * D3 state causing PCIE spec violation.
> +	 *
> +	 * Keep the bridge in D0 before module unload to prevent
> +	 * this issue
> +	 **/
Comment style need to change.
Anshuman.
> +	if (state == SUSPEND_STATE_FREEZE &&
> +	    igt_device_find_first_i915_discrete_card(&card)) {
> +		igt_pm_get_d3cold_allowed(&card, d3cold_allowed);
> +		igt_pm_set_d3cold_allowed(&card, "0\n");
> +	}
> +	close(fd);
> +
>  	igt_kmsg(KMSG_INFO "Unloading i915\n");
>  	igt_assert_eq(igt_i915_driver_unload(),0);
> 
>  	igt_system_suspend_autoresume(state, SUSPEND_TEST_NONE);
> 
> +	if (state == SUSPEND_STATE_FREEZE && strlen(card.card))
> +		igt_pm_set_d3cold_allowed(&card, d3cold_allowed);
> +
>  	igt_kmsg(KMSG_INFO "Re-loading i915 \n");
>  	igt_assert_eq(igt_i915_driver_load(NULL), 0);
> +
> +	igt_devices_free();
>  }
> 
>  int fd;
> --
> 2.25.1

Re: [igt-dev] [PATCH i-g-t 2/2] tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915

[Tauro, Riana]

On 9/8/2022 8:58 PM, Gupta, Anshuman wrote:
> 
> 
>> -----Original Message-----
>> From: Tauro, Riana <riana.tauro@intel.com>
>> Sent: Thursday, September 8, 2022 8:16 PM
>> To: igt-dev@lists.freedesktop.org
>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: [PATCH i-g-t 2/2] tests/i915/i915_suspend: Disable d3cold_allowed for
>> basic-s2idle-without-i915
>>
>> When module is unloaded and s2idle is triggered, PCI core leaves the endpoint in
>> D0 and the bridge in D3 state causing PCIE spec violation. The config space is
>> read as 0xFF
>>
>> Keep the bridge in D0 before module unload to prevent this issue
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>>   tests/i915/i915_suspend.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c index
>> 94935595..a6769d64 100644
>> --- a/tests/i915/i915_suspend.c
>> +++ b/tests/i915/i915_suspend.c
>> @@ -43,6 +43,7 @@
>>   #include "igt.h"
>>   #include "igt_kmod.h"
>>   #include "igt_device.h"
>> +#include "igt_device_scan.h"
>>
>>   #define OBJECT_SIZE (16*1024*1024)
>>
>> @@ -206,13 +207,40 @@ test_forcewake(int fd, bool hibernate)  static void
>> test_suspend_without_i915(int state)  {
>> +	struct igt_device_card card;
>> +	char d3cold_allowed[2];
>> +	int fd;
> Why do we need fd ?

igt_devices_scan uses udev lib to scan the drm devices.
This requires the i915 to be loaded.

Should i use igt_i915_driver_load(NULL) instead?

>> +
>> +	fd = __drm_open_driver(DRIVER_INTEL);
>> +	igt_devices_scan(false);
>> +
>> +	/*
>> +	 * When module is unloaded and s2idle is triggered,
>> +	 * PCI core leaves the endpoint in D0 and the bridge in
>> +	 * D3 state causing PCIE spec violation.
>> +	 *
>> +	 * Keep the bridge in D0 before module unload to prevent
>> +	 * this issue
>> +	 **/
> Comment style need to change.
> Anshuman.
>> +	if (state == SUSPEND_STATE_FREEZE &&
>> +	    igt_device_find_first_i915_discrete_card(&card)) {
>> +		igt_pm_get_d3cold_allowed(&card, d3cold_allowed);
>> +		igt_pm_set_d3cold_allowed(&card, "0\n");
>> +	}
>> +	close(fd);
>> +
>>   	igt_kmsg(KMSG_INFO "Unloading i915\n");
>>   	igt_assert_eq(igt_i915_driver_unload(),0);
>>
>>   	igt_system_suspend_autoresume(state, SUSPEND_TEST_NONE);
>>
>> +	if (state == SUSPEND_STATE_FREEZE && strlen(card.card))
>> +		igt_pm_set_d3cold_allowed(&card, d3cold_allowed);
>> +
>>   	igt_kmsg(KMSG_INFO "Re-loading i915 \n");
>>   	igt_assert_eq(igt_i915_driver_load(NULL), 0);
>> +
>> +	igt_devices_free();
>>   }
>>
>>   int fd;
>> --
>> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_pm : Add pm functions for d3cold_allowed

[Tauro, Riana]

On 9/8/2022 8:39 PM, Gupta, Anshuman wrote:
> 
> 
>> -----Original Message-----
>> From: Tauro, Riana <riana.tauro@intel.com>
>> Sent: Thursday, September 8, 2022 8:16 PM
>> To: igt-dev@lists.freedesktop.org
>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: [PATCH i-g-t 1/2] lib/igt_pm : Add pm functions for d3cold_allowed
>>
>> Add functions to set/get d3cold_allowed attribute of the pci_device
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>>   lib/igt_pm.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_pm.h |  4 ++++
>>   2 files changed, 50 insertions(+)
>>
>> diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 99251b40..ce34a0a4 100644
>> --- a/lib/igt_pm.c
>> +++ b/lib/igt_pm.c
>> @@ -1122,6 +1122,52 @@ void igt_pm_setup_pci_card_runtime_pm(struct
>> pci_device *pci_dev)
>>   	igt_pm_setup_pci_card_power_attrs(pci_dev, true, 0);  }
>>
>> +/**
>> + * igt_pm_get_d3cold_allowed:
>> + * @igt_device_card: device card
>> + * @val: value to be read into
>> + *
>> + * Reads the value of d3cold_allowed attribute
>> + * of the pci device
>> + */
>> +void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char *val)
>> +{
>> +	char name[PATH_MAX];
>> +	int fd;
>> +
>> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
>> +		 card->pci_slot_name);
>> +
>> +	fd = open(name, O_RDONLY);
>> +	igt_assert_f(fd >= 0, "Can't open %s\n", name);
>> +
>> +	igt_sysfs_read(fd, "d3cold_allowed", val, sizeof(val));
>> +
>> +	close(fd);
>> +}
>> +
>> +/**
>> + * igt_pm_get_d3cold_allowed:
>> + * @igt_device_card: device card
>> + * @val: value to be written
>> + *
>> + * writes the value to d3cold_allowed attribute of pci device  */ void
>> +igt_pm_set_d3cold_allowed(struct igt_device_card *card, const char
>> +*val) {
>> +	char name[PATH_MAX];
>> +	int fd;
>> +
>> +	snprintf(name, PATH_MAX, "/sys/bus/pci/devices/%s",
>> +		 card->pci_slot_name);
>> +
>> +	fd = open(name, O_RDONLY);
>> +	igt_assert_f(fd >= 0, "Can't open %s\n", name);
>> +
>> +	igt_sysfs_write(fd, "d3cold_allowed", val, sizeof(val));
>> +	close(fd);
>> +}
>> +
>>   static void
>>   igt_pm_restore_power_attr(struct pci_device *pci_dev, const char *attr, char
>> *val, int len)  { diff --git a/lib/igt_pm.h b/lib/igt_pm.h index cbbde12b..d9db8100
>> 100644
>> --- a/lib/igt_pm.h
>> +++ b/lib/igt_pm.h
>> @@ -24,6 +24,8 @@
>>   #ifndef IGT_PM_H
>>   #define IGT_PM_H
>>
>> +#include "igt_device_scan.h"
> Please include this from c file, where it is needed.
> Thanks,
> Anshuman.
It is needed since igt_device_card is used in function declaration.
>> +
>>   void igt_pm_enable_audio_runtime_pm(void);
>>   void igt_pm_enable_sata_link_power_management(void);
>>   void igt_pm_restore_sata_link_power_management(void);
>> @@ -76,6 +78,8 @@ enum igt_acpi_d_state
>>   igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);  void
>> igt_pm_enable_pci_card_runtime_pm(struct pci_device *root,
>>   				       struct pci_device *i915);
>> +void igt_pm_get_d3cold_allowed(struct igt_device_card *card, char
>> +*val); void igt_pm_set_d3cold_allowed(struct igt_device_card *card,
>> +const char *val);
>>   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);
>> --
>> 2.25.1
>