[RFC 0/6] VRAM Self Refresh

[RFC 0/6] VRAM Self Refresh

[Anshuman Gupta]

Enabling VRAM Self Refresh on Intel BMG GPU.
VRAM Self Refresh feature requires XeKMD
to request for D3Cold Aux Power Limit and PERST
Assertion Delay by calling _DSM 10 and _DSM 11 method.

Reference: PCI Firmware Specification
Section {4.6.10, 4.6.11}.

Anshuman Gupta (5):
  PCI/ACPI: Implement PCI FW _DSM method
  drm/xe/vrsr: Detect vrsr capability
  drm/xe/vrsr: Refactor d3cold.allowed to a enum
  drm/xe/pm: D3Cold target state
  drm/xe/vrsr: Enable VRSR

Badal Nilawar (1):
  drm/xe/vrsr: Apis to init and enable VRSR feature

 drivers/gpu/drm/xe/display/xe_display.c |  22 +++
 drivers/gpu/drm/xe/display/xe_display.h |   1 +
 drivers/gpu/drm/xe/regs/xe_regs.h       |   3 +
 drivers/gpu/drm/xe/xe_device_types.h    |  10 +-
 drivers/gpu/drm/xe/xe_pci.c             |   4 +-
 drivers/gpu/drm/xe/xe_pcode_api.h       |   8 +
 drivers/gpu/drm/xe/xe_pm.c              | 185 ++++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_pm.h              |   9 ++
 drivers/pci/pci-acpi.c                  | 123 ++++++++++++++++
 include/linux/pci-acpi.h                |  13 ++
 10 files changed, 358 insertions(+), 20 deletions(-)

-- 
2.34.1

[RFC 1/6] PCI/ACPI: Implement PCI FW _DSM method

[Anshuman Gupta]

Implement _DSM method 10 and _DSM Method 11 as per PCI firmware specs
section 4.6.10 and 4.6.11.

Current assumption is only one PCIe Endpoint driver (XeKMD for Battlemage GPU)
will request for Aux Power Limit under a given Root Port but
theoretically it is possible that other Non-Intel GPU or Non-GPU
PCIe Endpoint driver can also request for Aux Power Limit and request to
block the core power removal under same Root Port.
That will disrupt the Battlemage GPU VRAM Self Refresh.

One possible mitigation would be only allowing only first PCIe
Non-Bridge Endpoint Function 0 driver to call_DSM method 10.

Signed-off-by: Varun Gupta <varun.gupta@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/pci/pci-acpi.c   | 123 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |  13 +++++
 2 files changed, 136 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index af370628e583..806f6d19f46c 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1421,6 +1421,129 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
 	ACPI_FREE(obj);
 }
 
+/**
+ * pci_acpi_request_d3cold_aux_power - Request D3cold auxiliary power via ACPI DSM
+ * @dev: PCI device instance
+ * @requested_power: Requested auxiliary power in milliwatts
+ *
+ * This function sends a request to the host BIOS via ACPI _DSM Function 9 to grant
+ * the required D3Cold Auxiliary power for the specified PCI device.
+ * It evaluates the _DSM (Device Specific Method) to request the Auxiliary power and
+ * handles the response accordingly.
+ *
+ * This function shall be only called by 1st non-bridge Endpoint driver on Function 0.
+ * For a Multi-Function Device, the driver for Function 0 is required to report an
+ * aggregate power requirement covering all functions contained within the device.
+ *
+ * Return: Returns 0 on success and errno on failure.
+ */
+int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power)
+{
+	union acpi_object in_obj = {
+		.integer.type = ACPI_TYPE_INTEGER,
+		.integer.value = requested_power,
+	};
+
+	union acpi_object *out_obj;
+	acpi_handle handle;
+	int result, ret = -EINVAL;
+
+	if (!dev || !ACPI_HANDLE(&dev->dev))
+		return -EINVAL;
+
+	handle = ACPI_HANDLE(&dev->dev);
+
+	out_obj = acpi_evaluate_dsm_typed(handle,
+					  &pci_acpi_dsm_guid,
+					  4,
+					  DSM_PCI_D3COLD_AUX_POWER_LIMIT,
+					  &in_obj,
+					  ACPI_TYPE_INTEGER);
+	if (!out_obj)
+		return -EINVAL;
+
+	result = out_obj->integer.value;
+
+	switch (result) {
+	case 0x0:
+		dev_dbg(&dev->dev, "D3cold Aux Power request denied.\n");
+		break;
+	case 0x1:
+		dev_info(&dev->dev, "D3cold Aux Power request granted: %u mW.\n", requested_power);
+		ret = 0;
+		break;
+	case 0x2:
+		dev_info(&dev->dev, "D3cold Aux Power: Main power will not be removed.\n");
+		ret = -EBUSY;
+		break;
+	default:
+		if (result >= 0x11 && result <= 0x1F) {
+			int retry_interval = result & 0xF;
+
+			dev_warn(&dev->dev,
+				 "D3cold Aux Power request needs retry. Interval: %d seconds.\n", retry_interval);
+			msleep(retry_interval * 1000);
+			ret = -EAGAIN;
+		} else {
+			dev_err(&dev->dev, "D3cold Aux Power: Reserved or unsupported response: 0x%x.\n", result);
+		}
+		break;
+	}
+
+	ACPI_FREE(out_obj);
+	return ret;
+}
+EXPORT_SYMBOL(pci_acpi_request_d3cold_aux_power);
+
+/**
+ * pci_acpi_add_perst_assertion_delay - Request PERST delay via ACPI DSM
+ * @dev: PCI device instance
+ * @delay_us: Requested delay_us
+ *
+ * This function sends a request to the host BIOS via ACPI _DSM to grant the required
+ * PERST dealy for the specified PCI device. It evaluates the _DSM (Device
+ * Specific Method) to request the PERST delay and handles the response accordingly.
+ *
+ * Return: returns 0 on success and errno on failure.
+ */
+int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
+{
+	union acpi_object in_obj = {
+		.integer.type = ACPI_TYPE_INTEGER,
+		.integer.value = delay_us,
+	};
+
+	union acpi_object *out_obj;
+	acpi_handle handle;
+	int result, ret = -EINVAL;
+
+	if (!dev || !ACPI_HANDLE(&dev->dev))
+		return -EINVAL;
+
+	handle = ACPI_HANDLE(&dev->dev);
+
+	out_obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 4,
+					  DSM_PCI_PERST_ASSERTION_DELAY, &in_obj, ACPI_TYPE_INTEGER);
+	if (!out_obj)
+		return -EINVAL;
+
+	result = out_obj->integer.value;
+
+	if (result == delay_us) {
+		dev_info(&dev->dev, "PERST# Assertion Delay set to %u microseconds.\n", delay_us);
+		ret = 0;
+	} else if (result == 0) {
+		dev_warn(&dev->dev, "PERST# Assertion Delay request failed, no previous valid request.\n");
+	} else {
+		dev_warn(&dev->dev,
+			 "PERST# Assertion Delay request failed. Previous valid delay: %u microseconds.\n", result);
+	}
+
+	ACPI_FREE(out_obj);
+	return ret;
+}
+EXPORT_SYMBOL(pci_acpi_add_perst_assertion_delay);
+
 static void pci_acpi_set_external_facing(struct pci_dev *dev)
 {
 	u8 val;
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 078225b514d4..4b7373f91a9a 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -121,6 +121,8 @@ extern const guid_t pci_acpi_dsm_guid;
 #define DSM_PCI_DEVICE_NAME			0x07
 #define DSM_PCI_POWER_ON_RESET_DELAY		0x08
 #define DSM_PCI_DEVICE_READINESS_DURATIONS	0x09
+#define DSM_PCI_D3COLD_AUX_POWER_LIMIT		0x0A
+#define DSM_PCI_PERST_ASSERTION_DELAY		0x0B
 
 #ifdef CONFIG_PCIE_EDR
 void pci_acpi_add_edr_notifier(struct pci_dev *pdev);
@@ -132,10 +134,21 @@ static inline void pci_acpi_remove_edr_notifier(struct pci_dev *pdev) { }
 
 int pci_acpi_set_companion_lookup_hook(struct acpi_device *(*func)(struct pci_dev *));
 void pci_acpi_clear_companion_lookup_hook(void);
+int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power);
+int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us);
 
 #else	/* CONFIG_ACPI */
 static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
 static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
+int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power)
+{
+	return -EOPNOTSUPP;
+}
+
+int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_ACPI */
 
 #endif	/* _PCI_ACPI_H_ */
-- 
2.34.1

[RFC 2/6] drm/xe/vrsr: Detect vrsr capability

[Anshuman Gupta]

Detect VRAM Self Refresh(vrsr) Capability.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_regs.h    |  3 +++
 drivers/gpu/drm/xe/xe_device_types.h |  4 ++++
 drivers/gpu/drm/xe/xe_pm.c           | 27 +++++++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
index 6cf282618836..21563e9d958b 100644
--- a/drivers/gpu/drm/xe/regs/xe_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
@@ -57,6 +57,9 @@
 #define MTL_MPE_FREQUENCY			XE_REG(0x13802c)
 #define   MTL_RPE_MASK				REG_GENMASK(8, 0)
 
+#define VRAM_CAPABILITY                         XE_REG(0x138144)
+#define   VRAM_SUPPORTED                        REG_BIT(0)
+
 #define VF_CAP_REG				XE_REG(0x1901f8, XE_REG_OPTION_VF)
 #define   VF_CAP				REG_BIT(0)
 
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 4656305dd45a..c2ab2c91c968 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -490,6 +490,9 @@ struct xe_device {
 		/** @d3cold.allowed: Indicates if d3cold is a valid device state */
 		bool allowed;
 
+		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
+		bool vrsr_capable;
+
 		/**
 		 * @d3cold.vram_threshold:
 		 *
@@ -500,6 +503,7 @@ struct xe_device {
 		 * Default threshold value is 300mb.
 		 */
 		u32 vram_threshold;
+
 		/** @d3cold.lock: protect vram_threshold */
 		struct mutex lock;
 	} d3cold;
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 12200be7b43d..dead236355d8 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -17,12 +17,15 @@
 #include "xe_bo_evict.h"
 #include "xe_device.h"
 #include "xe_device_sysfs.h"
+#include "xe_force_wake.h"
 #include "xe_ggtt.h"
 #include "xe_gt.h"
 #include "xe_guc.h"
 #include "xe_irq.h"
+#include "xe_mmio.h"
 #include "xe_pcode.h"
 #include "xe_pxp.h"
+#include "regs/xe_regs.h"
 #include "xe_trace.h"
 #include "xe_wa.h"
 
@@ -236,6 +239,28 @@ static bool xe_pm_pci_d3cold_capable(struct xe_device *xe)
 	return true;
 }
 
+static bool xe_pm_vrsr_capable(struct xe_device *xe)
+{
+	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
+	unsigned int fw_ref;
+	struct xe_gt *gt;
+	u32 val;
+
+	gt = xe_root_mmio_gt(xe);
+
+	if (!xe->info.probe_display)
+		return false;
+
+	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+	if (!fw_ref)
+		return false;
+
+	val = xe_mmio_read32(mmio, VRAM_CAPABILITY);
+	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+
+	return val & VRAM_SUPPORTED;
+}
+
 static void xe_pm_runtime_init(struct xe_device *xe)
 {
 	struct device *dev = xe->drm.dev;
@@ -303,6 +328,8 @@ int xe_pm_init(struct xe_device *xe)
 		err = xe_pm_set_vram_threshold(xe, DEFAULT_VRAM_THRESHOLD);
 		if (err)
 			return err;
+
+		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
 	}
 
 	xe_pm_runtime_init(xe);
-- 
2.34.1

[RFC 3/6] drm/xe/vrsr: Apis to init and enable VRSR feature

[Anshuman Gupta]

From: Badal Nilawar <badal.nilawar@intel.com>

APIs to enable and initialize VRSR feature.

Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/xe/xe_device_types.h |  1 +
 drivers/gpu/drm/xe/xe_pcode_api.h    |  8 +++
 drivers/gpu/drm/xe/xe_pm.c           | 91 ++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_pm.h           |  3 +
 4 files changed, 103 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index c2ab2c91c968..da7946b75cd5 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -7,6 +7,7 @@
 #define _XE_DEVICE_TYPES_H_
 
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
index 2bae9afdbd35..17a90b2c6737 100644
--- a/drivers/gpu/drm/xe/xe_pcode_api.h
+++ b/drivers/gpu/drm/xe/xe_pcode_api.h
@@ -42,6 +42,14 @@
 #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
 #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)
 
+#define	  PCODE_D3_VRAM_SELF_REFRESH	0x71
+#define	    PCODE_D3_VRSR_SC_DISABLE	0x0
+#define	    PCODE_D3_VRSR_SC_ENABLE	0x1
+#define     PCODE_D3_VRSR_SC_AUX_PL_AND_PERST_DELAY	0x2
+#define     PCODE_D3_VRSR_PERST_SHIFT	16
+#define	    POWER_D3_VRSR_PSERST_MASK	REG_GENMASK(31, 16)
+#define	    POWER_D3_VRSR_AUX_PL_MASK	REG_GENMASK(15, 0)
+
 #define   PCODE_FREQUENCY_CONFIG		0x6e
 /* Frequency Config Sub Commands (param1) */
 #define     PCODE_MBOX_FC_SC_READ_FUSED_P0	0x0
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index dead236355d8..32583651988f 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -23,6 +23,7 @@
 #include "xe_guc.h"
 #include "xe_irq.h"
 #include "xe_mmio.h"
+#include "xe_pcode_api.h"
 #include "xe_pcode.h"
 #include "xe_pxp.h"
 #include "regs/xe_regs.h"
@@ -85,6 +86,92 @@ static struct lockdep_map xe_pm_runtime_nod3cold_map = {
 };
 #endif
 
+/**
+ * xe_pm_init_vrsr - Initialize VRAM self refresh
+ * @xe: The xe device
+ *
+ * This function reads the AUX power and PERST assertion delay from pcode.
+ * Then request host BIOS via ACPI _DSM to grant required AUX power and PERST
+ * assertion delay.
+ *
+ * Return: returns 0 on success and errno on failure
+ */
+int xe_pm_init_vrsr(struct xe_device *xe)
+{
+	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+	struct pci_dev *root_pdev;
+	int ret;
+	u32 uval;
+	u32 aux_pwr_limit;
+	u32 perst_delay;
+
+	root_pdev = pcie_find_root_port(pdev);
+	if (!root_pdev)
+		return -EINVAL;
+
+	/* Avoid Illegal Subcommand error */
+	if (xe->info.platform != XE_BATTLEMAGE)
+		return -ENXIO;
+
+	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
+			    PCODE_D3_VRSR_SC_AUX_PL_AND_PERST_DELAY, 0),
+			    &uval, NULL);
+
+	if (ret)
+		return ret;
+
+	aux_pwr_limit = REG_FIELD_GET(POWER_D3_VRSR_AUX_PL_MASK, uval);
+	perst_delay = REG_FIELD_GET(POWER_D3_VRSR_PSERST_MASK, uval);
+
+	drm_dbg(&xe->drm, "AUX POWER LIMIT =%d\n", aux_pwr_limit);
+	drm_dbg(&xe->drm, "PERST Assertion delay =%d\n", perst_delay);
+
+	ret = pci_acpi_request_d3cold_aux_power(root_pdev, aux_pwr_limit);
+	if (ret)
+		goto vrsr;
+
+	ret = pci_acpi_add_perst_assertion_delay(root_pdev, perst_delay);
+	if (ret)
+		goto vrsr;
+
+	return ret;
+
+vrsr:
+	drm_err(&xe->drm, "ACPI DSM failed, VRSR is not capable\n");
+	xe->d3cold.vrsr_capable = false;
+	return ret;
+}
+
+/**
+ * xe_pm_enable_vrsr - Enable VRAM self refresh
+ * @xe: The xe device.
+ * @enable: true: Enable, false: Disable
+ *
+ * This function enables the VRSR feature in D3Cold path.
+ *
+ * Return: It returns 0 on success and errno on failure.
+ */
+int xe_pm_enable_vrsr(struct xe_device *xe, bool enable)
+{
+	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+	int ret;
+	u32 uval = 0;
+
+	/* Avoid Illegal Subcommand error */
+	if (xe->info.platform != XE_BATTLEMAGE)
+		return -ENXIO;
+
+	if (enable)
+		ret = xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
+				     PCODE_D3_VRSR_SC_ENABLE, 0), uval);
+	else
+		ret = xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
+				     PCODE_D3_VRSR_SC_DISABLE, 0), uval);
+
+	return ret;
+}
+
 /**
  * xe_rpm_reclaim_safe() - Whether runtime resume can be done from reclaim context
  * @xe: The xe device.
@@ -330,6 +417,10 @@ int xe_pm_init(struct xe_device *xe)
 			return err;
 
 		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
+		if (xe->d3cold.vrsr_capable) {
+			drm_dbg(&xe->drm, "vram sr capable\n");
+			xe_pm_init_vrsr(xe);
+		}
 	}
 
 	xe_pm_runtime_init(xe);
diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
index 998d1ed64556..c9f176912b46 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -35,4 +35,7 @@ bool xe_rpm_reclaim_safe(const struct xe_device *xe);
 struct task_struct *xe_pm_read_callback_task(struct xe_device *xe);
 int xe_pm_module_init(void);
 
+int xe_pm_init_vrsr(struct xe_device *xe);
+int xe_pm_enable_vrsr(struct xe_device *xe, bool enable);
+
 #endif
-- 
2.34.1

[RFC 4/6] drm/xe/vrsr: Refactor d3cold.allowed to a enum

[Anshuman Gupta]

Add xe_d3_state enum to add support for VRAM Self Refresh
d3cold state.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/xe/xe_device_types.h | 5 +++--
 drivers/gpu/drm/xe/xe_pm.c           | 6 +++---
 drivers/gpu/drm/xe/xe_pm.h           | 6 ++++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index da7946b75cd5..937461939ecc 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -19,6 +19,7 @@
 #include "xe_memirq_types.h"
 #include "xe_oa_types.h"
 #include "xe_platform_types.h"
+#include "xe_pm.h"
 #include "xe_pmu_types.h"
 #include "xe_pt_types.h"
 #include "xe_sriov_types.h"
@@ -488,8 +489,8 @@ struct xe_device {
 		/** @d3cold.capable: Indicates if root port is d3cold capable */
 		bool capable;
 
-		/** @d3cold.allowed: Indicates if d3cold is a valid device state */
-		bool allowed;
+		/** @d3cold.allowed: Indicates d3cold target state */
+		enum xe_d3_state allowed;
 
 		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
 		bool vrsr_capable;
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 32583651988f..81e67b5693dc 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -859,7 +859,7 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
 	int i;
 
 	if (!xe->d3cold.capable) {
-		xe->d3cold.allowed = false;
+		xe->d3cold.allowed = XE_D3HOT;
 		return;
 	}
 
@@ -874,9 +874,9 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
 	mutex_lock(&xe->d3cold.lock);
 
 	if (total_vram_used_mb < xe->d3cold.vram_threshold)
-		xe->d3cold.allowed = true;
+		xe->d3cold.allowed = XE_D3COLD_OFF;
 	else
-		xe->d3cold.allowed = false;
+		xe->d3cold.allowed = XE_D3HOT;
 
 	mutex_unlock(&xe->d3cold.lock);
 }
diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
index c9f176912b46..b7b74757ce21 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -12,6 +12,12 @@
 
 struct xe_device;
 
+enum xe_d3_state {
+	XE_D3HOT = 0,
+	XE_D3COLD_OFF,
+	XE_D3COLD_VRSR,
+};
+
 int xe_pm_suspend(struct xe_device *xe);
 int xe_pm_resume(struct xe_device *xe);
 
-- 
2.34.1

[RFC 5/6] drm/xe/pm: D3Cold target state

[Anshuman Gupta]

Trade-off D3Cold target state based upon current vram usages.
if vram usages is greater then vram_d3cold_threshold and GPU
has display connected then target D3Cold state is D3Cold-VRSR
otherwise target state is D3COLD-Off.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/xe/display/xe_display.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/xe/display/xe_display.h |  1 +
 drivers/gpu/drm/xe/xe_pm.c              | 12 ++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 02a413a07382..140a43d6b1b6 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -548,3 +548,25 @@ int xe_display_probe(struct xe_device *xe)
 	unset_display_features(xe);
 	return 0;
 }
+
+bool xe_display_connected(struct xe_device *xe)
+{
+	struct drm_connector *list_connector;
+	struct drm_connector_list_iter iter;
+	bool ret = false;
+
+	mutex_lock(&xe->drm.mode_config.mutex);
+	drm_connector_list_iter_begin(&xe->drm, &iter);
+
+	drm_for_each_connector_iter(list_connector, &iter) {
+		if (list_connector->status == connector_status_connected) {
+			ret = true;
+			break;
+		}
+	}
+
+	drm_connector_list_iter_end(&iter);
+	mutex_unlock(&xe->drm.mode_config.mutex);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
index 685dc74402fb..c6bc54323084 100644
--- a/drivers/gpu/drm/xe/display/xe_display.h
+++ b/drivers/gpu/drm/xe/display/xe_display.h
@@ -40,6 +40,7 @@ void xe_display_pm_resume(struct xe_device *xe);
 void xe_display_pm_runtime_suspend(struct xe_device *xe);
 void xe_display_pm_runtime_suspend_late(struct xe_device *xe);
 void xe_display_pm_runtime_resume(struct xe_device *xe);
+bool xe_display_connected(struct xe_device *xe);
 
 #else
 
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 81e67b5693dc..6d28aedcb062 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -198,6 +198,14 @@ static void xe_rpm_lockmap_release(const struct xe_device *xe)
 			 &xe_pm_runtime_d3cold_map);
 }
 
+static void xe_pm_suspend_prepare(struct xe_device *xe)
+{
+	if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
+		xe_pm_d3cold_allowed_toggle(xe);
+	else
+		xe->d3cold.allowed = XE_D3COLD_OFF;
+}
+
 /**
  * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
  * @xe: xe device instance
@@ -213,6 +221,8 @@ int xe_pm_suspend(struct xe_device *xe)
 	drm_dbg(&xe->drm, "Suspending device\n");
 	trace_xe_pm_suspend(xe, __builtin_return_address(0));
 
+	xe_pm_suspend_prepare(xe);
+
 	err = xe_pxp_pm_suspend(xe->pxp);
 	if (err)
 		goto err;
@@ -875,6 +885,8 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
 
 	if (total_vram_used_mb < xe->d3cold.vram_threshold)
 		xe->d3cold.allowed = XE_D3COLD_OFF;
+	else if (xe->d3cold.vrsr_capable && xe_display_connected(xe))
+		xe->d3cold.allowed = XE_D3COLD_VRSR;
 	else
 		xe->d3cold.allowed = XE_D3HOT;
 
-- 
2.34.1

[RFC 6/6] drm/xe/vrsr: Enable VRSR

[Anshuman Gupta]

Enabling VRSR in runtime suspend and also in System wide suspend.
Also fix couple of typo in xe_pm.c.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c |  4 +--
 drivers/gpu/drm/xe/xe_pm.c  | 49 +++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 70b697fde5b9..55b42b3a10d2 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -967,7 +967,7 @@ static int xe_pci_suspend(struct device *dev)
 
 	/*
 	 * Enabling D3Cold is needed for S2Idle/S0ix.
-	 * It is save to allow here since xe_pm_suspend has evicted
+	 * It is safe to allow here since xe_pm_suspend has evicted
 	 * the local memory and the direct complete optimization is disabled.
 	 */
 	d3cold_toggle(pdev, D3COLD_ENABLE);
@@ -983,7 +983,7 @@ static int xe_pci_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	int err;
 
-	/* Give back the D3Cold decision to the runtime P M*/
+	/* Give back the D3Cold decision to the runtime PM */
 	d3cold_toggle(pdev, D3COLD_DISABLE);
 
 	err = pci_set_power_state(pdev, PCI_D0);
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 6d28aedcb062..5c96f8629a87 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -232,10 +232,12 @@ int xe_pm_suspend(struct xe_device *xe)
 
 	xe_display_pm_suspend(xe);
 
-	/* FIXME: Super racey... */
-	err = xe_bo_evict_all(xe);
-	if (err)
-		goto err_pxp;
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
+		/* FIXME: Super racey... */
+		err = xe_bo_evict_all(xe);
+		if (err)
+			goto err_pxp;
+	}
 
 	for_each_gt(gt, xe, id) {
 		err = xe_gt_suspend(gt);
@@ -247,6 +249,12 @@ int xe_pm_suspend(struct xe_device *xe)
 
 	xe_display_pm_suspend_late(xe);
 
+	if (xe->d3cold.allowed == XE_D3COLD_VRSR) {
+		err = xe_pm_enable_vrsr(xe, true);
+			if (err)
+				goto err_display;
+	}
+
 	drm_dbg(&xe->drm, "Device suspended\n");
 	return 0;
 
@@ -288,9 +296,11 @@ int xe_pm_resume(struct xe_device *xe)
 	 * This only restores pinned memory which is the memory required for the
 	 * GT(s) to resume.
 	 */
-	err = xe_bo_restore_kernel(xe);
-	if (err)
-		goto err;
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
+		err = xe_bo_restore_kernel(xe);
+		if (err)
+			goto err;
+	}
 
 	xe_irq_resume(xe);
 
@@ -299,9 +309,11 @@ int xe_pm_resume(struct xe_device *xe)
 
 	xe_display_pm_resume(xe);
 
-	err = xe_bo_restore_user(xe);
-	if (err)
-		goto err;
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
+		err = xe_bo_restore_user(xe);
+		if (err)
+			goto err;
+	}
 
 	xe_pxp_pm_resume(xe->pxp);
 
@@ -543,7 +555,7 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
 
 	xe_display_pm_runtime_suspend(xe);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
 		err = xe_bo_evict_all(xe);
 		if (err)
 			goto out_resume;
@@ -559,6 +571,14 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
 
 	xe_display_pm_runtime_suspend_late(xe);
 
+	if (xe->d3cold.allowed == XE_D3COLD_VRSR) {
+		err = xe_pm_enable_vrsr(xe, true);
+			if (err) {
+				drm_err(&xe->drm, "Failed to enable VRSR: %d\n", err);
+				goto out_resume;
+			}
+	}
+
 	xe_rpm_lockmap_release(xe);
 	xe_pm_write_callback_task(xe, NULL);
 	return 0;
@@ -590,7 +610,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 
 	xe_rpm_lockmap_acquire(xe);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
 		err = xe_pcode_ready(xe, true);
 		if (err)
 			goto out;
@@ -606,6 +626,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 			goto out;
 	}
 
+	if (xe->d3cold.allowed == XE_D3COLD_VRSR)
+		xe_display_pm_resume_early(xe);
+
 	xe_irq_resume(xe);
 
 	for_each_gt(gt, xe, id)
@@ -613,7 +636,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 
 	xe_display_pm_runtime_resume(xe);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
 		err = xe_bo_restore_user(xe);
 		if (err)
 			goto out;
-- 
2.34.1

Re: [RFC 1/6] PCI/ACPI: Implement PCI FW _DSM method

[Bjorn Helgaas]

On Mon, Feb 24, 2025 at 10:18:44PM +0530, Anshuman Gupta wrote:
> Implement _DSM method 10 and _DSM Method 11 as per PCI firmware specs
> section 4.6.10 and 4.6.11.

Please split into two patches, one for each _DSM.  Include spec
citations, e.g., PCI Firmware r3.3, sec 4.6.10.  Section numbers are
not guaranteed to stay consistent across spec revisions, so we need
both the revision and section number.

Include some descriptive words about the DSM in each subject line,
e.g., "D3cold Aux Power Limit", "PERST# Assertion Delay".

> Current assumption is only one PCIe Endpoint driver (XeKMD for Battlemage GPU)
> will request for Aux Power Limit under a given Root Port but
> theoretically it is possible that other Non-Intel GPU or Non-GPU
> PCIe Endpoint driver can also request for Aux Power Limit and request to
> block the core power removal under same Root Port.
> That will disrupt the Battlemage GPU VRAM Self Refresh.

I guess this is sort of an acknowledgement of the r3.3, sec 4.6.10
spec text about system software being responsible for tracking and
aggregating requests when there are multiple functions below the
Downstream Port?

If so, remove the Battlemage-specific language and just say something
about the fact that this implementation doesn't do any of that
tracking and aggregation.

> One possible mitigation would be only allowing only first PCIe
> Non-Bridge Endpoint Function 0 driver to call_DSM method 10.

Wrap to fill 75 columns in commit logs.  Add blank lines between
paragraphs.

> Signed-off-by: Varun Gupta <varun.gupta@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/pci/pci-acpi.c   | 123 +++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  13 +++++
>  2 files changed, 136 insertions(+)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index af370628e583..806f6d19f46c 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1421,6 +1421,129 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
>  	ACPI_FREE(obj);
>  }
>  
> +/**
> + * pci_acpi_request_d3cold_aux_power - Request D3cold auxiliary power via ACPI DSM
> + * @dev: PCI device instance
> + * @requested_power: Requested auxiliary power in milliwatts
> + *
> + * This function sends a request to the host BIOS via ACPI _DSM Function 9 to grant
> + * the required D3Cold Auxiliary power for the specified PCI device.
> + * It evaluates the _DSM (Device Specific Method) to request the Auxiliary power and
> + * handles the response accordingly.
> + *
> + * This function shall be only called by 1st non-bridge Endpoint driver on Function 0.
> + * For a Multi-Function Device, the driver for Function 0 is required to report an
> + * aggregate power requirement covering all functions contained within the device.

Wrap to fit in 80 columns like the rest of the file.

> + * Return: Returns 0 on success and errno on failure.
> + */
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power)
> +{
> +	union acpi_object in_obj = {
> +		.integer.type = ACPI_TYPE_INTEGER,
> +		.integer.value = requested_power,
> +	};
> +
> +	union acpi_object *out_obj;
> +	acpi_handle handle;
> +	int result, ret = -EINVAL;
> +
> +	if (!dev || !ACPI_HANDLE(&dev->dev))
> +		return -EINVAL;
> +
> +	handle = ACPI_HANDLE(&dev->dev);
> +
> +	out_obj = acpi_evaluate_dsm_typed(handle,
> +					  &pci_acpi_dsm_guid,
> +					  4,
> +					  DSM_PCI_D3COLD_AUX_POWER_LIMIT,
> +					  &in_obj,
> +					  ACPI_TYPE_INTEGER);

Wrap to fill 78-80 columns.

> +	if (!out_obj)
> +		return -EINVAL;
> +
> +	result = out_obj->integer.value;
> +
> +	switch (result) {
> +	case 0x0:
> +		dev_dbg(&dev->dev, "D3cold Aux Power request denied.\n");

Include requested_power here too, for debugging purposes.

> +		break;
> +	case 0x1:
> +		dev_info(&dev->dev, "D3cold Aux Power request granted: %u mW.\n", requested_power);
> +		ret = 0;
> +		break;
> +	case 0x2:
> +		dev_info(&dev->dev, "D3cold Aux Power: Main power will not be removed.\n");

No periods at end of messages.

> +		ret = -EBUSY;
> +		break;
> +	default:
> +		if (result >= 0x11 && result <= 0x1F) {
> +			int retry_interval = result & 0xF;
> +
> +			dev_warn(&dev->dev,
> +				 "D3cold Aux Power request needs retry. Interval: %d seconds.\n", retry_interval);
> +			msleep(retry_interval * 1000);
> +			ret = -EAGAIN;
> +		} else {
> +			dev_err(&dev->dev, "D3cold Aux Power: Reserved or unsupported response: 0x%x.\n", result);
> +		}
> +		break;
> +	}
> +
> +	ACPI_FREE(out_obj);
> +	return ret;
> +}
> +EXPORT_SYMBOL(pci_acpi_request_d3cold_aux_power);
> +
> +/**
> + * pci_acpi_add_perst_assertion_delay - Request PERST delay via ACPI DSM
> + * @dev: PCI device instance
> + * @delay_us: Requested delay_us
> + *
> + * This function sends a request to the host BIOS via ACPI _DSM to grant the required
> + * PERST dealy for the specified PCI device. It evaluates the _DSM (Device
> + * Specific Method) to request the PERST delay and handles the response accordingly.

s/PERST/PERST#/
s/dealy/delay/

> + * Return: returns 0 on success and errno on failure.
> + */
> +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
> +{
> +	union acpi_object in_obj = {
> +		.integer.type = ACPI_TYPE_INTEGER,
> +		.integer.value = delay_us,
> +	};
> +
> +	union acpi_object *out_obj;
> +	acpi_handle handle;
> +	int result, ret = -EINVAL;
> +
> +	if (!dev || !ACPI_HANDLE(&dev->dev))
> +		return -EINVAL;
> +
> +	handle = ACPI_HANDLE(&dev->dev);
> +
> +	out_obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 4,
> +					  DSM_PCI_PERST_ASSERTION_DELAY, &in_obj, ACPI_TYPE_INTEGER);

Wrap to fit in 78-80 columns.

> +	if (!out_obj)
> +		return -EINVAL;
> +
> +	result = out_obj->integer.value;
> +
> +	if (result == delay_us) {
> +		dev_info(&dev->dev, "PERST# Assertion Delay set to %u microseconds.\n", delay_us);
> +		ret = 0;
> +	} else if (result == 0) {
> +		dev_warn(&dev->dev, "PERST# Assertion Delay request failed, no previous valid request.\n");
> +	} else {
> +		dev_warn(&dev->dev,
> +			 "PERST# Assertion Delay request failed. Previous valid delay: %u microseconds.\n", result);
> +	}
> +
> +	ACPI_FREE(out_obj);
> +	return ret;
> +}
> +EXPORT_SYMBOL(pci_acpi_add_perst_assertion_delay);
> +
>  static void pci_acpi_set_external_facing(struct pci_dev *dev)
>  {
>  	u8 val;
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 078225b514d4..4b7373f91a9a 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -121,6 +121,8 @@ extern const guid_t pci_acpi_dsm_guid;
>  #define DSM_PCI_DEVICE_NAME			0x07
>  #define DSM_PCI_POWER_ON_RESET_DELAY		0x08
>  #define DSM_PCI_DEVICE_READINESS_DURATIONS	0x09
> +#define DSM_PCI_D3COLD_AUX_POWER_LIMIT		0x0A
> +#define DSM_PCI_PERST_ASSERTION_DELAY		0x0B
>  
>  #ifdef CONFIG_PCIE_EDR
>  void pci_acpi_add_edr_notifier(struct pci_dev *pdev);
> @@ -132,10 +134,21 @@ static inline void pci_acpi_remove_edr_notifier(struct pci_dev *pdev) { }
>  
>  int pci_acpi_set_companion_lookup_hook(struct acpi_device *(*func)(struct pci_dev *));
>  void pci_acpi_clear_companion_lookup_hook(void);
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power);
> +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us);
>  
>  #else	/* CONFIG_ACPI */
>  static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
>  static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif	/* CONFIG_ACPI */
>  
>  #endif	/* _PCI_ACPI_H_ */
> -- 
> 2.34.1
> 

Re: [RFC 3/6] drm/xe/vrsr: Apis to init and enable VRSR feature

[Bjorn Helgaas]

s/Apis/APIs/ in subject to match common usage and use below.

Also perhaps s/Detect vrsr/Detect VRSR/ in previous patch subject to
match this one.

On Mon, Feb 24, 2025 at 10:18:46PM +0530, Anshuman Gupta wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
> 
> APIs to enable and initialize VRSR feature.

I always think it's nice when the commit log includes the actual names
of the APIs being added so we don't have to grub that out of the
patch.

Re: [RFC 5/6] drm/xe/pm: D3Cold target state

[Bjorn Helgaas]

Could the subject line say something about what this patch does?

On Mon, Feb 24, 2025 at 10:18:48PM +0530, Anshuman Gupta wrote:
> Trade-off D3Cold target state based upon current vram usages.
> if vram usages is greater then vram_d3cold_threshold and GPU
> has display connected then target D3Cold state is D3Cold-VRSR
> otherwise target state is D3COLD-Off.

s/if vram usages/If vram usage/ (capitalize sentence and fix "usages")

I dunno what "D3Cold-VRSR" means.  I suppose GPU folks do.

Re: [RFC 5/6] drm/xe/pm: D3Cold target state

[Ville Syrjälä]

On Mon, Feb 24, 2025 at 10:18:48PM +0530, Anshuman Gupta wrote:
> Trade-off D3Cold target state based upon current vram usages.
> if vram usages is greater then vram_d3cold_threshold and GPU
> has display connected 

Why would anyone care about displays being connected or not?

> then target D3Cold state is D3Cold-VRSR
> otherwise target state is D3COLD-Off.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/display/xe_display.h |  1 +
>  drivers/gpu/drm/xe/xe_pm.c              | 12 ++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 02a413a07382..140a43d6b1b6 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -548,3 +548,25 @@ int xe_display_probe(struct xe_device *xe)
>  	unset_display_features(xe);
>  	return 0;
>  }
> +
> +bool xe_display_connected(struct xe_device *xe)
> +{
> +	struct drm_connector *list_connector;
> +	struct drm_connector_list_iter iter;
> +	bool ret = false;
> +
> +	mutex_lock(&xe->drm.mode_config.mutex);
> +	drm_connector_list_iter_begin(&xe->drm, &iter);
> +
> +	drm_for_each_connector_iter(list_connector, &iter) {
> +		if (list_connector->status == connector_status_connected) {
> +			ret = true;
> +			break;
> +		}
> +	}
> +
> +	drm_connector_list_iter_end(&iter);
> +	mutex_unlock(&xe->drm.mode_config.mutex);
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> index 685dc74402fb..c6bc54323084 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.h
> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> @@ -40,6 +40,7 @@ void xe_display_pm_resume(struct xe_device *xe);
>  void xe_display_pm_runtime_suspend(struct xe_device *xe);
>  void xe_display_pm_runtime_suspend_late(struct xe_device *xe);
>  void xe_display_pm_runtime_resume(struct xe_device *xe);
> +bool xe_display_connected(struct xe_device *xe);
>  
>  #else
>  
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 81e67b5693dc..6d28aedcb062 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -198,6 +198,14 @@ static void xe_rpm_lockmap_release(const struct xe_device *xe)
>  			 &xe_pm_runtime_d3cold_map);
>  }
>  
> +static void xe_pm_suspend_prepare(struct xe_device *xe)
> +{
> +	if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
> +		xe_pm_d3cold_allowed_toggle(xe);
> +	else
> +		xe->d3cold.allowed = XE_D3COLD_OFF;
> +}
> +
>  /**
>   * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
>   * @xe: xe device instance
> @@ -213,6 +221,8 @@ int xe_pm_suspend(struct xe_device *xe)
>  	drm_dbg(&xe->drm, "Suspending device\n");
>  	trace_xe_pm_suspend(xe, __builtin_return_address(0));
>  
> +	xe_pm_suspend_prepare(xe);
> +
>  	err = xe_pxp_pm_suspend(xe->pxp);
>  	if (err)
>  		goto err;
> @@ -875,6 +885,8 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>  
>  	if (total_vram_used_mb < xe->d3cold.vram_threshold)
>  		xe->d3cold.allowed = XE_D3COLD_OFF;
> +	else if (xe->d3cold.vrsr_capable && xe_display_connected(xe))
> +		xe->d3cold.allowed = XE_D3COLD_VRSR;
>  	else
>  		xe->d3cold.allowed = XE_D3HOT;
>  
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

RE: [RFC 5/6] drm/xe/pm: D3Cold target state

[Gupta, Anshuman]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, February 25, 2025 11:20 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
> pci@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> bhelgaas@google.com; ilpo.jarvinen@linux.intel.com; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nilawar,
> Badal <badal.nilawar@intel.com>; Nasim, Kam <kam.nasim@intel.com>
> Subject: Re: [RFC 5/6] drm/xe/pm: D3Cold target state
> 
> On Mon, Feb 24, 2025 at 10:18:48PM +0530, Anshuman Gupta wrote:
> > Trade-off D3Cold target state based upon current vram usages.
> > if vram usages is greater than vram_d3cold_threshold and GPU has
> > display connected
> 
> Why would anyone care about displays being connected or not?
As per specs we got to enable vrsr only when there is display connected,
We can check that in probe but a drm connector status can change after completion of probe. That is the reason to put a check for display connected in idle callback.
Thanks,
Anshuman
> 
> > then target D3Cold state is D3Cold-VRSR otherwise target state is
> > D3COLD-Off.
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/xe/display/xe_display.c | 22 ++++++++++++++++++++++
> > drivers/gpu/drm/xe/display/xe_display.h |  1 +
> >  drivers/gpu/drm/xe/xe_pm.c              | 12 ++++++++++++
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > b/drivers/gpu/drm/xe/display/xe_display.c
> > index 02a413a07382..140a43d6b1b6 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -548,3 +548,25 @@ int xe_display_probe(struct xe_device *xe)
> >  	unset_display_features(xe);
> >  	return 0;
> >  }
> > +
> > +bool xe_display_connected(struct xe_device *xe) {
> > +	struct drm_connector *list_connector;
> > +	struct drm_connector_list_iter iter;
> > +	bool ret = false;
> > +
> > +	mutex_lock(&xe->drm.mode_config.mutex);
> > +	drm_connector_list_iter_begin(&xe->drm, &iter);
> > +
> > +	drm_for_each_connector_iter(list_connector, &iter) {
> > +		if (list_connector->status == connector_status_connected) {
> > +			ret = true;
> > +			break;
> > +		}
> > +	}
> > +
> > +	drm_connector_list_iter_end(&iter);
> > +	mutex_unlock(&xe->drm.mode_config.mutex);
> > +
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.h
> > b/drivers/gpu/drm/xe/display/xe_display.h
> > index 685dc74402fb..c6bc54323084 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > @@ -40,6 +40,7 @@ void xe_display_pm_resume(struct xe_device *xe);
> > void xe_display_pm_runtime_suspend(struct xe_device *xe);  void
> > xe_display_pm_runtime_suspend_late(struct xe_device *xe);  void
> > xe_display_pm_runtime_resume(struct xe_device *xe);
> > +bool xe_display_connected(struct xe_device *xe);
> >
> >  #else
> >
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 81e67b5693dc..6d28aedcb062 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -198,6 +198,14 @@ static void xe_rpm_lockmap_release(const struct
> xe_device *xe)
> >  			 &xe_pm_runtime_d3cold_map);
> >  }
> >
> > +static void xe_pm_suspend_prepare(struct xe_device *xe) {
> > +	if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
> > +		xe_pm_d3cold_allowed_toggle(xe);
> > +	else
> > +		xe->d3cold.allowed = XE_D3COLD_OFF; }
> > +
> >  /**
> >   * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
> >   * @xe: xe device instance
> > @@ -213,6 +221,8 @@ int xe_pm_suspend(struct xe_device *xe)
> >  	drm_dbg(&xe->drm, "Suspending device\n");
> >  	trace_xe_pm_suspend(xe, __builtin_return_address(0));
> >
> > +	xe_pm_suspend_prepare(xe);
> > +
> >  	err = xe_pxp_pm_suspend(xe->pxp);
> >  	if (err)
> >  		goto err;
> > @@ -875,6 +885,8 @@ void xe_pm_d3cold_allowed_toggle(struct
> xe_device
> > *xe)
> >
> >  	if (total_vram_used_mb < xe->d3cold.vram_threshold)
> >  		xe->d3cold.allowed = XE_D3COLD_OFF;
> > +	else if (xe->d3cold.vrsr_capable && xe_display_connected(xe))
> > +		xe->d3cold.allowed = XE_D3COLD_VRSR;
> >  	else
> >  		xe->d3cold.allowed = XE_D3HOT;
> >
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel

RE: [RFC 1/6] PCI/ACPI: Implement PCI FW _DSM method

[Gupta, Anshuman]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, February 25, 2025 1:11 AM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
> pci@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> bhelgaas@google.com; ilpo.jarvinen@linux.intel.com; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nilawar,
> Badal <badal.nilawar@intel.com>; Nasim, Kam <kam.nasim@intel.com>;
> Gupta, Varun <varun.gupta@intel.com>
> Subject: Re: [RFC 1/6] PCI/ACPI: Implement PCI FW _DSM method
> 
> On Mon, Feb 24, 2025 at 10:18:44PM +0530, Anshuman Gupta wrote:
> > Implement _DSM method 10 and _DSM Method 11 as per PCI firmware
> specs
> > section 4.6.10 and 4.6.11.
> 
> Please split into two patches, one for each _DSM.  Include spec citations, e.g.,
> PCI Firmware r3.3, sec 4.6.10.  Section numbers are not guaranteed to stay
> consistent across spec revisions, so we need both the revision and section
> number.
> 
> Include some descriptive words about the DSM in each subject line, e.g.,
> "D3cold Aux Power Limit", "PERST# Assertion Delay".
> 
> > Current assumption is only one PCIe Endpoint driver (XeKMD for
> > Battlemage GPU) will request for Aux Power Limit under a given Root
> > Port but theoretically it is possible that other Non-Intel GPU or
> > Non-GPU PCIe Endpoint driver can also request for Aux Power Limit and
> > request to block the core power removal under same Root Port.
> > That will disrupt the Battlemage GPU VRAM Self Refresh.
> 
> I guess this is sort of an acknowledgement of the r3.3, sec 4.6.10 spec text
> about system software being responsible for tracking and aggregating
> requests when there are multiple functions below the Downstream Port?
Thanks for review comment.
AFAIU apart from multiple function below the Downstream Port (from same PCIe Card),
there can be possibility of another PCie card connected via a switch to same root port
like below topology.


			                 |-> PCIe PCIe Downstream Port -> End Point Device 	
Root Port -> PCIe Upstream Port   |-> PCIe PCIe Downstream Port -> End Point Device	
			                 |-> PCIe PCIe Downstream Port -> PCIe Upstream Port ->  PCIe Downstream Port -> *EndPoint Device 	

*Endpoint Device from different PCIe card can also request to block the core power removal under same Root Port ?
 How to document such limitation ?

Thanks,
Anshuman
> 
> If so, remove the Battlemage-specific language and just say something about
> the fact that this implementation doesn't do any of that tracking and
> aggregation.
> 
> > One possible mitigation would be only allowing only first PCIe
> > Non-Bridge Endpoint Function 0 driver to call_DSM method 10.
> 
> Wrap to fill 75 columns in commit logs.  Add blank lines between paragraphs.
> 
> > Signed-off-by: Varun Gupta <varun.gupta@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/pci/pci-acpi.c   | 123
> +++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci-acpi.h |  13 +++++
> >  2 files changed, 136 insertions(+)
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> > af370628e583..806f6d19f46c 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -1421,6 +1421,129 @@ static void pci_acpi_optimize_delay(struct
> pci_dev *pdev,
> >  	ACPI_FREE(obj);
> >  }
> >
> > +/**
> > + * pci_acpi_request_d3cold_aux_power - Request D3cold auxiliary power
> > +via ACPI DSM
> > + * @dev: PCI device instance
> > + * @requested_power: Requested auxiliary power in milliwatts
> > + *
> > + * This function sends a request to the host BIOS via ACPI _DSM
> > +Function 9 to grant
> > + * the required D3Cold Auxiliary power for the specified PCI device.
> > + * It evaluates the _DSM (Device Specific Method) to request the
> > +Auxiliary power and
> > + * handles the response accordingly.
> > + *
> > + * This function shall be only called by 1st non-bridge Endpoint driver on
> Function 0.
> > + * For a Multi-Function Device, the driver for Function 0 is required
> > +to report an
> > + * aggregate power requirement covering all functions contained within the
> device.
> 
> Wrap to fit in 80 columns like the rest of the file.
> 
> > + * Return: Returns 0 on success and errno on failure.
> > + */
> > +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32
> > +requested_power) {
> > +	union acpi_object in_obj = {
> > +		.integer.type = ACPI_TYPE_INTEGER,
> > +		.integer.value = requested_power,
> > +	};
> > +
> > +	union acpi_object *out_obj;
> > +	acpi_handle handle;
> > +	int result, ret = -EINVAL;
> > +
> > +	if (!dev || !ACPI_HANDLE(&dev->dev))
> > +		return -EINVAL;
> > +
> > +	handle = ACPI_HANDLE(&dev->dev);
> > +
> > +	out_obj = acpi_evaluate_dsm_typed(handle,
> > +					  &pci_acpi_dsm_guid,
> > +					  4,
> > +
> DSM_PCI_D3COLD_AUX_POWER_LIMIT,
> > +					  &in_obj,
> > +					  ACPI_TYPE_INTEGER);
> 
> Wrap to fill 78-80 columns.
> 
> > +	if (!out_obj)
> > +		return -EINVAL;
> > +
> > +	result = out_obj->integer.value;
> > +
> > +	switch (result) {
> > +	case 0x0:
> > +		dev_dbg(&dev->dev, "D3cold Aux Power request denied.\n");
> 
> Include requested_power here too, for debugging purposes.
> 
> > +		break;
> > +	case 0x1:
> > +		dev_info(&dev->dev, "D3cold Aux Power request granted: %u
> mW.\n", requested_power);
> > +		ret = 0;
> > +		break;
> > +	case 0x2:
> > +		dev_info(&dev->dev, "D3cold Aux Power: Main power will not
> be
> > +removed.\n");
> 
> No periods at end of messages.
> 
> > +		ret = -EBUSY;
> > +		break;
> > +	default:
> > +		if (result >= 0x11 && result <= 0x1F) {
> > +			int retry_interval = result & 0xF;
> > +
> > +			dev_warn(&dev->dev,
> > +				 "D3cold Aux Power request needs retry.
> Interval: %d seconds.\n", retry_interval);
> > +			msleep(retry_interval * 1000);
> > +			ret = -EAGAIN;
> > +		} else {
> > +			dev_err(&dev->dev, "D3cold Aux Power: Reserved or
> unsupported response: 0x%x.\n", result);
> > +		}
> > +		break;
> > +	}
> > +
> > +	ACPI_FREE(out_obj);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(pci_acpi_request_d3cold_aux_power);
> > +
> > +/**
> > + * pci_acpi_add_perst_assertion_delay - Request PERST delay via ACPI
> > +DSM
> > + * @dev: PCI device instance
> > + * @delay_us: Requested delay_us
> > + *
> > + * This function sends a request to the host BIOS via ACPI _DSM to
> > +grant the required
> > + * PERST dealy for the specified PCI device. It evaluates the _DSM
> > +(Device
> > + * Specific Method) to request the PERST delay and handles the response
> accordingly.
> 
> s/PERST/PERST#/
> s/dealy/delay/
> 
> > + * Return: returns 0 on success and errno on failure.
> > + */
> > +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32
> > +delay_us) {
> > +	union acpi_object in_obj = {
> > +		.integer.type = ACPI_TYPE_INTEGER,
> > +		.integer.value = delay_us,
> > +	};
> > +
> > +	union acpi_object *out_obj;
> > +	acpi_handle handle;
> > +	int result, ret = -EINVAL;
> > +
> > +	if (!dev || !ACPI_HANDLE(&dev->dev))
> > +		return -EINVAL;
> > +
> > +	handle = ACPI_HANDLE(&dev->dev);
> > +
> > +	out_obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 4,
> > +					  DSM_PCI_PERST_ASSERTION_DELAY,
> &in_obj, ACPI_TYPE_INTEGER);
> 
> Wrap to fit in 78-80 columns.
> 
> > +	if (!out_obj)
> > +		return -EINVAL;
> > +
> > +	result = out_obj->integer.value;
> > +
> > +	if (result == delay_us) {
> > +		dev_info(&dev->dev, "PERST# Assertion Delay set to %u
> microseconds.\n", delay_us);
> > +		ret = 0;
> > +	} else if (result == 0) {
> > +		dev_warn(&dev->dev, "PERST# Assertion Delay request failed,
> no previous valid request.\n");
> > +	} else {
> > +		dev_warn(&dev->dev,
> > +			 "PERST# Assertion Delay request failed. Previous valid
> delay: %u microseconds.\n", result);
> > +	}
> > +
> > +	ACPI_FREE(out_obj);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(pci_acpi_add_perst_assertion_delay);
> > +
> >  static void pci_acpi_set_external_facing(struct pci_dev *dev)  {
> >  	u8 val;
> > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h index
> > 078225b514d4..4b7373f91a9a 100644
> > --- a/include/linux/pci-acpi.h
> > +++ b/include/linux/pci-acpi.h
> > @@ -121,6 +121,8 @@ extern const guid_t pci_acpi_dsm_guid;
> >  #define DSM_PCI_DEVICE_NAME			0x07
> >  #define DSM_PCI_POWER_ON_RESET_DELAY		0x08
> >  #define DSM_PCI_DEVICE_READINESS_DURATIONS	0x09
> > +#define DSM_PCI_D3COLD_AUX_POWER_LIMIT		0x0A
> > +#define DSM_PCI_PERST_ASSERTION_DELAY		0x0B
> >
> >  #ifdef CONFIG_PCIE_EDR
> >  void pci_acpi_add_edr_notifier(struct pci_dev *pdev); @@ -132,10
> > +134,21 @@ static inline void pci_acpi_remove_edr_notifier(struct
> > pci_dev *pdev) { }
> >
> >  int pci_acpi_set_companion_lookup_hook(struct acpi_device
> > *(*func)(struct pci_dev *));  void
> > pci_acpi_clear_companion_lookup_hook(void);
> > +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32
> > +requested_power); int pci_acpi_add_perst_assertion_delay(struct
> > +pci_dev *dev, u32 delay_us);
> >
> >  #else	/* CONFIG_ACPI */
> >  static inline void acpi_pci_add_bus(struct pci_bus *bus) { }  static
> > inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
> > +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32
> > +requested_power) {
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32
> > +delay_us) {
> > +	return -EOPNOTSUPP;
> > +}
> >  #endif	/* CONFIG_ACPI */
> >
> >  #endif	/* _PCI_ACPI_H_ */
> > --
> > 2.34.1
> >

Re: [RFC 5/6] drm/xe/pm: D3Cold target state

[Ville Syrjälä]

On Tue, Feb 25, 2025 at 06:00:04PM +0000, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, February 25, 2025 11:20 PM
> > To: Gupta, Anshuman <anshuman.gupta@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
> > pci@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> > bhelgaas@google.com; ilpo.jarvinen@linux.intel.com; De Marchi, Lucas
> > <lucas.demarchi@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nilawar,
> > Badal <badal.nilawar@intel.com>; Nasim, Kam <kam.nasim@intel.com>
> > Subject: Re: [RFC 5/6] drm/xe/pm: D3Cold target state
> > 
> > On Mon, Feb 24, 2025 at 10:18:48PM +0530, Anshuman Gupta wrote:
> > > Trade-off D3Cold target state based upon current vram usages.
> > > if vram usages is greater than vram_d3cold_threshold and GPU has
> > > display connected
> > 
> > Why would anyone care about displays being connected or not?
> As per specs we got to enable vrsr only when there is display connected,

What specs? And why do they say connected displays should be
a factor here?

I think the only thing that makes any sense for this kind of stuff
would be sysfs power/ knobs that allow the system administrator to
tune the behaviour for their specific use case. And if no such knobs
exist currently then perhaps they should be added.

> We can check that in probe but a drm connector status can change after completion of probe. That is the reason to put a check for display connected in idle callback.
> Thanks,
> Anshuman
> > 
> > > then target D3Cold state is D3Cold-VRSR otherwise target state is
> > > D3COLD-Off.
> > >
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/display/xe_display.c | 22 ++++++++++++++++++++++
> > > drivers/gpu/drm/xe/display/xe_display.h |  1 +
> > >  drivers/gpu/drm/xe/xe_pm.c              | 12 ++++++++++++
> > >  3 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > > b/drivers/gpu/drm/xe/display/xe_display.c
> > > index 02a413a07382..140a43d6b1b6 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > > @@ -548,3 +548,25 @@ int xe_display_probe(struct xe_device *xe)
> > >  	unset_display_features(xe);
> > >  	return 0;
> > >  }
> > > +
> > > +bool xe_display_connected(struct xe_device *xe) {
> > > +	struct drm_connector *list_connector;
> > > +	struct drm_connector_list_iter iter;
> > > +	bool ret = false;
> > > +
> > > +	mutex_lock(&xe->drm.mode_config.mutex);
> > > +	drm_connector_list_iter_begin(&xe->drm, &iter);
> > > +
> > > +	drm_for_each_connector_iter(list_connector, &iter) {
> > > +		if (list_connector->status == connector_status_connected) {
> > > +			ret = true;
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	drm_connector_list_iter_end(&iter);
> > > +	mutex_unlock(&xe->drm.mode_config.mutex);
> > > +
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.h
> > > b/drivers/gpu/drm/xe/display/xe_display.h
> > > index 685dc74402fb..c6bc54323084 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > > @@ -40,6 +40,7 @@ void xe_display_pm_resume(struct xe_device *xe);
> > > void xe_display_pm_runtime_suspend(struct xe_device *xe);  void
> > > xe_display_pm_runtime_suspend_late(struct xe_device *xe);  void
> > > xe_display_pm_runtime_resume(struct xe_device *xe);
> > > +bool xe_display_connected(struct xe_device *xe);
> > >
> > >  #else
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > > index 81e67b5693dc..6d28aedcb062 100644
> > > --- a/drivers/gpu/drm/xe/xe_pm.c
> > > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > > @@ -198,6 +198,14 @@ static void xe_rpm_lockmap_release(const struct
> > xe_device *xe)
> > >  			 &xe_pm_runtime_d3cold_map);
> > >  }
> > >
> > > +static void xe_pm_suspend_prepare(struct xe_device *xe) {
> > > +	if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
> > > +		xe_pm_d3cold_allowed_toggle(xe);
> > > +	else
> > > +		xe->d3cold.allowed = XE_D3COLD_OFF; }
> > > +
> > >  /**
> > >   * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
> > >   * @xe: xe device instance
> > > @@ -213,6 +221,8 @@ int xe_pm_suspend(struct xe_device *xe)
> > >  	drm_dbg(&xe->drm, "Suspending device\n");
> > >  	trace_xe_pm_suspend(xe, __builtin_return_address(0));
> > >
> > > +	xe_pm_suspend_prepare(xe);
> > > +
> > >  	err = xe_pxp_pm_suspend(xe->pxp);
> > >  	if (err)
> > >  		goto err;
> > > @@ -875,6 +885,8 @@ void xe_pm_d3cold_allowed_toggle(struct
> > xe_device
> > > *xe)
> > >
> > >  	if (total_vram_used_mb < xe->d3cold.vram_threshold)
> > >  		xe->d3cold.allowed = XE_D3COLD_OFF;
> > > +	else if (xe->d3cold.vrsr_capable && xe_display_connected(xe))
> > > +		xe->d3cold.allowed = XE_D3COLD_VRSR;
> > >  	else
> > >  		xe->d3cold.allowed = XE_D3HOT;
> > >
> > > --
> > > 2.34.1
> > 
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

Re: [RFC 1/6] PCI/ACPI: Implement PCI FW _DSM method

[Bjorn Helgaas]

On Tue, Feb 25, 2025 at 06:25:52PM +0000, Gupta, Anshuman wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> ... redundant headers snipped

> > On Mon, Feb 24, 2025 at 10:18:44PM +0530, Anshuman Gupta wrote:
> > > Implement _DSM method 10 and _DSM Method 11 as per PCI firmware
> > specs
> > > section 4.6.10 and 4.6.11.
> > 
> > Please split into two patches, one for each _DSM.  Include spec
> > citations, e.g., PCI Firmware r3.3, sec 4.6.10.  Section numbers
> > are not guaranteed to stay consistent across spec revisions, so we
> > need both the revision and section number.
> > 
> > Include some descriptive words about the DSM in each subject line,
> > e.g., "D3cold Aux Power Limit", "PERST# Assertion Delay".
> > 
> > > Current assumption is only one PCIe Endpoint driver (XeKMD for
> > > Battlemage GPU) will request for Aux Power Limit under a given Root
> > > Port but theoretically it is possible that other Non-Intel GPU or
> > > Non-GPU PCIe Endpoint driver can also request for Aux Power Limit and
> > > request to block the core power removal under same Root Port.
> > > That will disrupt the Battlemage GPU VRAM Self Refresh.
> > 
> > I guess this is sort of an acknowledgement of the r3.3, sec 4.6.10 spec text
> > about system software being responsible for tracking and aggregating
> > requests when there are multiple functions below the Downstream Port?

> AFAIU apart from multiple function below the Downstream Port (from
> same PCIe Card), there can be possibility of another PCie card
> connected via a switch to same root port like below topology.
> 
> 			                 |-> PCIe PCIe Downstream Port -> End Point Device 	
> Root Port -> PCIe Upstream Port   |-> PCIe PCIe Downstream Port -> End Point Device	
> 			                 |-> PCIe PCIe Downstream Port -> PCIe Upstream Port ->  PCIe Downstream Port -> *EndPoint Device 	
> 
> *Endpoint Device from different PCIe card can also request to block the core power removal under same Root Port ?

Of course.

>  How to document such limitation ?

> > If so, remove the Battlemage-specific language and just say something about
> > the fact that this implementation doesn't do any of that tracking and
> > aggregation.

^^ Here's a hint about how to document this.  My point is that this
has nothing to do with Battlemage in particular, so the text about
Battlemage-specific things is a distraction from the real point, which
IIUC is this:

  Note that this implementation assumes only a single device below the
  Downstream Port because it does not track and aggregate requests
  from all child devices below the Downstream Port as required by sec
  4.6.10.

> > > One possible mitigation would be only allowing only first PCIe
> > > Non-Bridge Endpoint Function 0 driver to call_DSM method 10.

Re: [RFC 2/6] drm/xe/vrsr: Detect vrsr capability

[Rodrigo Vivi]

On Mon, Feb 24, 2025 at 10:18:45PM +0530, Anshuman Gupta wrote:
> Detect VRAM Self Refresh(vrsr) Capability.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_regs.h    |  3 +++
>  drivers/gpu/drm/xe/xe_device_types.h |  4 ++++
>  drivers/gpu/drm/xe/xe_pm.c           | 27 +++++++++++++++++++++++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
> index 6cf282618836..21563e9d958b 100644
> --- a/drivers/gpu/drm/xe/regs/xe_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_regs.h
> @@ -57,6 +57,9 @@
>  #define MTL_MPE_FREQUENCY			XE_REG(0x13802c)
>  #define   MTL_RPE_MASK				REG_GENMASK(8, 0)
>  
> +#define VRAM_CAPABILITY                         XE_REG(0x138144)
> +#define   VRAM_SUPPORTED                        REG_BIT(0)

I'm missing a 'SR' mention here.

I know the register name is VRAM_CAPABILITY what looks horrible, but let's live with
it, but we could then use same or similar terminology from BSPec:

VRAM_SR_CAP

or VRAM_SR_CAP_SUPPORTED

or VRAM_SR_SUPPORTED at least?

with some mention to SR here:

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

> +
>  #define VF_CAP_REG				XE_REG(0x1901f8, XE_REG_OPTION_VF)
>  #define   VF_CAP				REG_BIT(0)
>  
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 4656305dd45a..c2ab2c91c968 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -490,6 +490,9 @@ struct xe_device {
>  		/** @d3cold.allowed: Indicates if d3cold is a valid device state */
>  		bool allowed;
>  
> +		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
> +		bool vrsr_capable;
> +
>  		/**
>  		 * @d3cold.vram_threshold:
>  		 *
> @@ -500,6 +503,7 @@ struct xe_device {
>  		 * Default threshold value is 300mb.
>  		 */
>  		u32 vram_threshold;
> +
>  		/** @d3cold.lock: protect vram_threshold */
>  		struct mutex lock;
>  	} d3cold;
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 12200be7b43d..dead236355d8 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -17,12 +17,15 @@
>  #include "xe_bo_evict.h"
>  #include "xe_device.h"
>  #include "xe_device_sysfs.h"
> +#include "xe_force_wake.h"
>  #include "xe_ggtt.h"
>  #include "xe_gt.h"
>  #include "xe_guc.h"
>  #include "xe_irq.h"
> +#include "xe_mmio.h"
>  #include "xe_pcode.h"
>  #include "xe_pxp.h"
> +#include "regs/xe_regs.h"
>  #include "xe_trace.h"
>  #include "xe_wa.h"
>  
> @@ -236,6 +239,28 @@ static bool xe_pm_pci_d3cold_capable(struct xe_device *xe)
>  	return true;
>  }
>  
> +static bool xe_pm_vrsr_capable(struct xe_device *xe)
> +{
> +	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> +	unsigned int fw_ref;
> +	struct xe_gt *gt;
> +	u32 val;
> +
> +	gt = xe_root_mmio_gt(xe);
> +
> +	if (!xe->info.probe_display)
> +		return false;
> +
> +	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	if (!fw_ref)
> +		return false;
> +
> +	val = xe_mmio_read32(mmio, VRAM_CAPABILITY);
> +	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +
> +	return val & VRAM_SUPPORTED;
> +}
> +
>  static void xe_pm_runtime_init(struct xe_device *xe)
>  {
>  	struct device *dev = xe->drm.dev;
> @@ -303,6 +328,8 @@ int xe_pm_init(struct xe_device *xe)
>  		err = xe_pm_set_vram_threshold(xe, DEFAULT_VRAM_THRESHOLD);
>  		if (err)
>  			return err;
> +
> +		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
>  	}
>  
>  	xe_pm_runtime_init(xe);
> -- 
> 2.34.1
> 

Re: [RFC 3/6] drm/xe/vrsr: Apis to init and enable VRSR feature

[Rodrigo Vivi]

On Mon, Feb 24, 2025 at 10:18:46PM +0530, Anshuman Gupta wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
> 
> APIs to enable and initialize VRSR feature.
> 
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h |  1 +
>  drivers/gpu/drm/xe/xe_pcode_api.h    |  8 +++
>  drivers/gpu/drm/xe/xe_pm.c           | 91 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pm.h           |  3 +
>  4 files changed, 103 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index c2ab2c91c968..da7946b75cd5 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -7,6 +7,7 @@
>  #define _XE_DEVICE_TYPES_H_
>  
>  #include <linux/pci.h>
> +#include <linux/pci-acpi.h>
>  
>  #include <drm/drm_device.h>
>  #include <drm/drm_file.h>
> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
> index 2bae9afdbd35..17a90b2c6737 100644
> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
> @@ -42,6 +42,14 @@
>  #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
>  #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)
>  
> +#define	  PCODE_D3_VRAM_SELF_REFRESH	0x71
> +#define	    PCODE_D3_VRSR_SC_DISABLE	0x0
> +#define	    PCODE_D3_VRSR_SC_ENABLE	0x1
> +#define     PCODE_D3_VRSR_SC_AUX_PL_AND_PERST_DELAY	0x2
> +#define     PCODE_D3_VRSR_PERST_SHIFT	16
> +#define	    POWER_D3_VRSR_PSERST_MASK	REG_GENMASK(31, 16)
> +#define	    POWER_D3_VRSR_AUX_PL_MASK	REG_GENMASK(15, 0)
> +
>  #define   PCODE_FREQUENCY_CONFIG		0x6e
>  /* Frequency Config Sub Commands (param1) */
>  #define     PCODE_MBOX_FC_SC_READ_FUSED_P0	0x0
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index dead236355d8..32583651988f 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -23,6 +23,7 @@
>  #include "xe_guc.h"
>  #include "xe_irq.h"
>  #include "xe_mmio.h"
> +#include "xe_pcode_api.h"
>  #include "xe_pcode.h"
>  #include "xe_pxp.h"
>  #include "regs/xe_regs.h"
> @@ -85,6 +86,92 @@ static struct lockdep_map xe_pm_runtime_nod3cold_map = {
>  };
>  #endif
>  
> +/**
> + * xe_pm_init_vrsr - Initialize VRAM self refresh
> + * @xe: The xe device
> + *
> + * This function reads the AUX power and PERST assertion delay from pcode.
> + * Then request host BIOS via ACPI _DSM to grant required AUX power and PERST
> + * assertion delay.
> + *
> + * Return: returns 0 on success and errno on failure
> + */
> +int

why not void?
we are not checking it anyway...

> xe_pm_init_vrsr(struct xe_device *xe)

perhaps xe_pm_d3cold_vrsr_init()

> +{
> +	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	struct pci_dev *root_pdev;
> +	int ret;
> +	u32 uval;
> +	u32 aux_pwr_limit;
> +	u32 perst_delay;
> +
> +	root_pdev = pcie_find_root_port(pdev);
> +	if (!root_pdev)
> +		return -EINVAL;
> +
> +	/* Avoid Illegal Subcommand error */
> +	if (xe->info.platform != XE_BATTLEMAGE)
> +		return -ENXIO;

I wonder if we should do a has_d3cold_vrsr flag for this, or at least
move this check earlier.

> +
> +	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
> +			    PCODE_D3_VRSR_SC_AUX_PL_AND_PERST_DELAY, 0),
> +			    &uval, NULL);
> +
> +	if (ret)
> +		return ret;
> +
> +	aux_pwr_limit = REG_FIELD_GET(POWER_D3_VRSR_AUX_PL_MASK, uval);
> +	perst_delay = REG_FIELD_GET(POWER_D3_VRSR_PSERST_MASK, uval);
> +
> +	drm_dbg(&xe->drm, "AUX POWER LIMIT =%d\n", aux_pwr_limit);
> +	drm_dbg(&xe->drm, "PERST Assertion delay =%d\n", perst_delay);
> +
> +	ret = pci_acpi_request_d3cold_aux_power(root_pdev, aux_pwr_limit);
> +	if (ret)
> +		goto vrsr;
> +
> +	ret = pci_acpi_add_perst_assertion_delay(root_pdev, perst_delay);
> +	if (ret)
> +		goto vrsr;
> +
> +	return ret;
> +
> +vrsr:
> +	drm_err(&xe->drm, "ACPI DSM failed, VRSR is not capable\n");
> +	xe->d3cold.vrsr_capable = false;
> +	return ret;
> +}
> +
> +/**
> + * xe_pm_enable_vrsr - Enable VRAM self refresh
> + * @xe: The xe device.
> + * @enable: true: Enable, false: Disable
> + *
> + * This function enables the VRSR feature in D3Cold path.
> + *
> + * Return: It returns 0 on success and errno on failure.
> + */
> +int xe_pm_enable_vrsr(struct xe_device *xe, bool enable)
> +{
> +	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
> +	int ret;
> +	u32 uval = 0;
> +
> +	/* Avoid Illegal Subcommand error */
> +	if (xe->info.platform != XE_BATTLEMAGE)
> +		return -ENXIO;
> +
> +	if (enable)
> +		ret = xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
> +				     PCODE_D3_VRSR_SC_ENABLE, 0), uval);
> +	else
> +		ret = xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
> +				     PCODE_D3_VRSR_SC_DISABLE, 0), uval);
> +
> +	return ret;
> +}
> +
>  /**
>   * xe_rpm_reclaim_safe() - Whether runtime resume can be done from reclaim context
>   * @xe: The xe device.
> @@ -330,6 +417,10 @@ int xe_pm_init(struct xe_device *xe)
>  			return err;
>  
>  		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
> +		if (xe->d3cold.vrsr_capable) {
> +			drm_dbg(&xe->drm, "vram sr capable\n");
> +			xe_pm_init_vrsr(xe);
> +		}

perhaps move this piece of code entirely to the init function itself?

>  	}
>  
>  	xe_pm_runtime_init(xe);
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 998d1ed64556..c9f176912b46 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -35,4 +35,7 @@ bool xe_rpm_reclaim_safe(const struct xe_device *xe);
>  struct task_struct *xe_pm_read_callback_task(struct xe_device *xe);
>  int xe_pm_module_init(void);
>  
> +int xe_pm_init_vrsr(struct xe_device *xe);
> +int xe_pm_enable_vrsr(struct xe_device *xe, bool enable);
> +
>  #endif
> -- 
> 2.34.1
> 

Re: [RFC 4/6] drm/xe/vrsr: Refactor d3cold.allowed to a enum

[Rodrigo Vivi]

On Mon, Feb 24, 2025 at 10:18:47PM +0530, Anshuman Gupta wrote:
> Add xe_d3_state enum to add support for VRAM Self Refresh
> d3cold state.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h | 5 +++--
>  drivers/gpu/drm/xe/xe_pm.c           | 6 +++---
>  drivers/gpu/drm/xe/xe_pm.h           | 6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index da7946b75cd5..937461939ecc 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -19,6 +19,7 @@
>  #include "xe_memirq_types.h"
>  #include "xe_oa_types.h"
>  #include "xe_platform_types.h"
> +#include "xe_pm.h"
>  #include "xe_pmu_types.h"
>  #include "xe_pt_types.h"
>  #include "xe_sriov_types.h"
> @@ -488,8 +489,8 @@ struct xe_device {
>  		/** @d3cold.capable: Indicates if root port is d3cold capable */
>  		bool capable;
>  
> -		/** @d3cold.allowed: Indicates if d3cold is a valid device state */
> -		bool allowed;
> +		/** @d3cold.allowed: Indicates d3cold target state */
> +		enum xe_d3_state allowed;

let's also rename the variable name to 'target_state'

or xe->d3cold.allowed = XE_D3HOT //becomes strange

xe->d3.target_state = XE_D3HOT sounds better imho

>  
>  		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
>  		bool vrsr_capable;
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 32583651988f..81e67b5693dc 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -859,7 +859,7 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>  	int i;
>  
>  	if (!xe->d3cold.capable) {
> -		xe->d3cold.allowed = false;
> +		xe->d3cold.allowed = XE_D3HOT;
>  		return;
>  	}
>  
> @@ -874,9 +874,9 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>  	mutex_lock(&xe->d3cold.lock);
>  
>  	if (total_vram_used_mb < xe->d3cold.vram_threshold)
> -		xe->d3cold.allowed = true;
> +		xe->d3cold.allowed = XE_D3COLD_OFF;
>  	else
> -		xe->d3cold.allowed = false;
> +		xe->d3cold.allowed = XE_D3HOT;
>  
>  	mutex_unlock(&xe->d3cold.lock);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index c9f176912b46..b7b74757ce21 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -12,6 +12,12 @@
>  
>  struct xe_device;
>  
> +enum xe_d3_state {
> +	XE_D3HOT = 0,
> +	XE_D3COLD_OFF,
> +	XE_D3COLD_VRSR,
> +};
> +
>  int xe_pm_suspend(struct xe_device *xe);
>  int xe_pm_resume(struct xe_device *xe);
>  
> -- 
> 2.34.1
> 

Re: [RFC,6/6] drm/xe/vrsr: Enable VRSR

[Poosa, Karthik]

On 24-02-2025 22:18, Anshuman Gupta wrote:
> Enabling VRSR in runtime suspend and also in System wide suspend.
> Also fix couple of typo in xe_pm.c.
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_pci.c |  4 +--
>   drivers/gpu/drm/xe/xe_pm.c  | 49 +++++++++++++++++++++++++++----------
>   2 files changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 70b697fde5b9..55b42b3a10d2 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -967,7 +967,7 @@ static int xe_pci_suspend(struct device *dev)
>   
>   	/*
>   	 * Enabling D3Cold is needed for S2Idle/S0ix.
> -	 * It is save to allow here since xe_pm_suspend has evicted
> +	 * It is safe to allow here since xe_pm_suspend has evicted
>   	 * the local memory and the direct complete optimization is disabled.
>   	 */
>   	d3cold_toggle(pdev, D3COLD_ENABLE);
> @@ -983,7 +983,7 @@ static int xe_pci_resume(struct device *dev)
>   	struct pci_dev *pdev = to_pci_dev(dev);
>   	int err;
>   
> -	/* Give back the D3Cold decision to the runtime P M*/
> +	/* Give back the D3Cold decision to the runtime PM */
>   	d3cold_toggle(pdev, D3COLD_DISABLE);
>   
>   	err = pci_set_power_state(pdev, PCI_D0);
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 6d28aedcb062..5c96f8629a87 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -232,10 +232,12 @@ int xe_pm_suspend(struct xe_device *xe)
>   
>   	xe_display_pm_suspend(xe);
>   
> -	/* FIXME: Super racey... */
> -	err = xe_bo_evict_all(xe);
> -	if (err)
> -		goto err_pxp;
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
> +		/* FIXME: Super racey... */
> +		err = xe_bo_evict_all(xe);
> +		if (err)
> +			goto err_pxp;
> +	}
>   
>   	for_each_gt(gt, xe, id) {
>   		err = xe_gt_suspend(gt);
> @@ -247,6 +249,12 @@ int xe_pm_suspend(struct xe_device *xe)
>   
>   	xe_display_pm_suspend_late(xe);
>   
> +	if (xe->d3cold.allowed == XE_D3COLD_VRSR) {
> +		err = xe_pm_enable_vrsr(xe, true);
> +			if (err)
> +				goto err_display;
> +	}
> +
>   	drm_dbg(&xe->drm, "Device suspended\n");
>   	return 0;
>   
> @@ -288,9 +296,11 @@ int xe_pm_resume(struct xe_device *xe)
>   	 * This only restores pinned memory which is the memory required for the
>   	 * GT(s) to resume.
>   	 */
> -	err = xe_bo_restore_kernel(xe);
> -	if (err)
> -		goto err;
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
> +		err = xe_bo_restore_kernel(xe);
> +		if (err)
> +			goto err;
> +	}
>   
>   	xe_irq_resume(xe);
>   
> @@ -299,9 +309,11 @@ int xe_pm_resume(struct xe_device *xe)
>   
>   	xe_display_pm_resume(xe);
>   
> -	err = xe_bo_restore_user(xe);
> -	if (err)
> -		goto err;
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
> +		err = xe_bo_restore_user(xe);
> +		if (err)
> +			goto err;
> +	}
>   
>   	xe_pxp_pm_resume(xe->pxp);
>   
> @@ -543,7 +555,7 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>   
>   	xe_display_pm_runtime_suspend(xe);
>   
> -	if (xe->d3cold.allowed) {
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
>   		err = xe_bo_evict_all(xe);
>   		if (err)
>   			goto out_resume;
> @@ -559,6 +571,14 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>   
>   	xe_display_pm_runtime_suspend_late(xe);
>   
> +	if (xe->d3cold.allowed == XE_D3COLD_VRSR) {
> +		err = xe_pm_enable_vrsr(xe, true);
> +			if (err) {
> +				drm_err(&xe->drm, "Failed to enable VRSR: %d\n", err);
> +				goto out_resume;
> +			}
> +	}
> +
>   	xe_rpm_lockmap_release(xe);
>   	xe_pm_write_callback_task(xe, NULL);
>   	return 0;
> @@ -590,7 +610,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
>   
>   	xe_rpm_lockmap_acquire(xe);
>   
> -	if (xe->d3cold.allowed) {
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
>   		err = xe_pcode_ready(xe, true);
>   		if (err)
>   			goto out;
> @@ -606,6 +626,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
>   			goto out;
>   	}
>   
> +	if (xe->d3cold.allowed == XE_D3COLD_VRSR)
> +		xe_display_pm_resume_early(xe);
> +
>   	xe_irq_resume(xe);
>   
>   	for_each_gt(gt, xe, id)
> @@ -613,7 +636,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
>   
>   	xe_display_pm_runtime_resume(xe);
>   
> -	if (xe->d3cold.allowed) {
> +	if (xe->d3cold.allowed == XE_D3COLD_OFF) {
>   		err = xe_bo_restore_user(xe);
>   		if (err)
>   			goto out;

LGTM.

Acked-by: Karthik Poosa <karthik.poosa@intel.com>

Re: [RFC 4/6] drm/xe/vrsr: Refactor d3cold.allowed to a enum

[Poosa, Karthik]

On 10-03-2025 22:58, Rodrigo Vivi wrote:
> On Mon, Feb 24, 2025 at 10:18:47PM +0530, Anshuman Gupta wrote:
>> Add xe_d3_state enum to add support for VRAM Self Refresh
>> d3cold state.
>>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_device_types.h | 5 +++--
>>   drivers/gpu/drm/xe/xe_pm.c           | 6 +++---
>>   drivers/gpu/drm/xe/xe_pm.h           | 6 ++++++
>>   3 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index da7946b75cd5..937461939ecc 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -19,6 +19,7 @@
>>   #include "xe_memirq_types.h"
>>   #include "xe_oa_types.h"
>>   #include "xe_platform_types.h"
>> +#include "xe_pm.h"
>>   #include "xe_pmu_types.h"
>>   #include "xe_pt_types.h"
>>   #include "xe_sriov_types.h"
>> @@ -488,8 +489,8 @@ struct xe_device {
>>   		/** @d3cold.capable: Indicates if root port is d3cold capable */
>>   		bool capable;
>>   
>> -		/** @d3cold.allowed: Indicates if d3cold is a valid device state */
>> -		bool allowed;
>> +		/** @d3cold.allowed: Indicates d3cold target state */
>> +		enum xe_d3_state allowed;
> let's also rename the variable name to 'target_state'
>
> or xe->d3cold.allowed = XE_D3HOT //becomes strange
>
> xe->d3.target_state = XE_D3HOT sounds better imho
>
>>   
>>   		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
>>   		bool vrsr_capable;
>> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>> index 32583651988f..81e67b5693dc 100644
>> --- a/drivers/gpu/drm/xe/xe_pm.c
>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>> @@ -859,7 +859,7 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>>   	int i;
>>   
>>   	if (!xe->d3cold.capable) {
>> -		xe->d3cold.allowed = false;
>> +		xe->d3cold.allowed = XE_D3HOT;
>>   		return;
>>   	}
>>   
>> @@ -874,9 +874,9 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>>   	mutex_lock(&xe->d3cold.lock);
>>   
>>   	if (total_vram_used_mb < xe->d3cold.vram_threshold)
>> -		xe->d3cold.allowed = true;
>> +		xe->d3cold.allowed = XE_D3COLD_OFF;
>>   	else
>> -		xe->d3cold.allowed = false;
>> +		xe->d3cold.allowed = XE_D3HOT;
>>   
>>   	mutex_unlock(&xe->d3cold.lock);
>>   }
>> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
>> index c9f176912b46..b7b74757ce21 100644
>> --- a/drivers/gpu/drm/xe/xe_pm.h
>> +++ b/drivers/gpu/drm/xe/xe_pm.h
>> @@ -12,6 +12,12 @@
>>   
>>   struct xe_device;
>>   
>> +enum xe_d3_state {
>> +	XE_D3HOT = 0,
>> +	XE_D3COLD_OFF,
>> +	XE_D3COLD_VRSR,
>> +};
Could you please reorder the enum based on the level of power 
consumption as shown below ?

+enum xe_d3_state {
+    XE_D3HOT = 0,
+    XE_D3COLD_VRSR,
+    XE_D3COLD_OFF,
+};

>> +
>>   int xe_pm_suspend(struct xe_device *xe);
>>   int xe_pm_resume(struct xe_device *xe);
>>   
>> -- 
>> 2.34.1
>>