[PATCH v6 00/12] VRAM Self Refresh

[PATCH v6 00/12] VRAM Self Refresh

[Badal Nilawar]

Changes in v6:
  - Addressed review comments from Rafael J. Wysocki.
  - Added now patch to refactor PM Sleep Ops

Changes in v5:
  - Added Co-developed by, whereever necessary
  - Addressed review comments 
  - Added new patch to handle vrsr in s2idle
    drm/xe/pm/s2idle: Don't evict user BOs for D3hot and D3cold-VRSR state  

Changes in v4:
  - Resolved build warnings

Changes in v3:
 PCIe ACPI Patches:
  - dropped the notifier block code and added patch to allow only one Aux
    power limit request per root port (Rafael J. Wysocki)
  - Addressed Review comments (Rafael J. Wysocki, Bjorn Helgaas)

 Xe Pacthes:
  - Addressed Review comments (Bjorn Helgaas)

Anshuman Gupta (6):
  PCI/ACPI: Add D3cold Aux Power Limit_DSM method
  PCI/ACPI: Add PERST# Assertion Delay _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 (6):
  drm/xe/vrsr: Introduce flag has_vrsr
  drm/xe/vrsr: Initialize VRSR feature
  drm/xe/vrsr: Enable VRSR on default VGA boot device
  drm/xe/pm: Refactor PM Sleep Ops
  drm/xe/pm/s2idle: Don't evict user BOs D3cold-VRSR state
  drm/xe/vrsr: Introduce a debugfs node named vrsr_capable

 drivers/gpu/drm/i915/display/intel_display.c |  22 ++
 drivers/gpu/drm/i915/display/intel_display.h |   1 +
 drivers/gpu/drm/xe/display/xe_display.c      |  11 +-
 drivers/gpu/drm/xe/display/xe_display.h      |   2 +
 drivers/gpu/drm/xe/regs/xe_regs.h            |   3 +
 drivers/gpu/drm/xe/xe_debugfs.c              |   2 +
 drivers/gpu/drm/xe/xe_device_types.h         |  12 +-
 drivers/gpu/drm/xe/xe_pci.c                  |  76 +++++-
 drivers/gpu/drm/xe/xe_pci_types.h            |   1 +
 drivers/gpu/drm/xe/xe_pcode_api.h            |   7 +
 drivers/gpu/drm/xe/xe_pm.c                   | 244 ++++++++++++++++---
 drivers/gpu/drm/xe/xe_pm.h                   |  14 +-
 drivers/pci/pci-acpi.c                       | 196 +++++++++++++++
 include/linux/pci-acpi.h                     |  18 +-
 14 files changed, 564 insertions(+), 45 deletions(-)

-- 
2.52.0

[PATCH v6 01/12] PCI/ACPI: Add D3cold Aux Power Limit_DSM method

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

Implement _DSM method 0Ah, as per PCI Firmware r3.3, sec 4.6.10,
to request auxiliary power required by the device when in D3cold state.

Implementation allows only a single device below the Downstream Port to
request for Aux Power Limit under a given Root Port/Downstream Port
because it does not track and aggregate requests from all child devices
below the Downstream Port as required by PCI Firmware r3.3, sec 4.6.10.

Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
V2(Bjorn/Rafael):
  - Call acpi_dsm_check() to find method 0Ah supported
  - Return retry interval to caller
V3(Kuppuswamy)
  - Add NULL check for retry interval
V4
  - Define enums for aux power request status (Rafael)
  - Add Co-developed-by and clean up Signed-off-by (Kappuswamy)
 (Bjorn)
  - Instead of root pci device pass the pci device of driver, traverse
    up the tree and discover _DSM
  - Allow only function 0 of device to request aux power
  - Allow retry_interval to be NULL
  - Refine commit message and function description
V5(Rafael)
  - Remove function 0 check and allow first caller of the given
    downstream port (with _DSM) to requst aux power including
    different function
  - Squash Patch v5.02 to this patch
  - In the logic, to allow single device to req power, use linked list
    instead of adding extra variables to acpi device structure
  - return positive code for no main power removal to distinguish from
    aux power request granted
---
 drivers/pci/pci-acpi.c   | 136 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |   9 +++
 2 files changed, 145 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 9369377725fa..645d3005ba50 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1418,6 +1418,142 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
 	ACPI_FREE(obj);
 }
 
+static LIST_HEAD(acpi_aux_pwr_list);
+static DEFINE_MUTEX(acpi_aux_pwr_lock);
+
+struct aux_pwr {
+	u32 aux_pwr_limit;		/* aux power limit granted by platform firmware */
+	struct device *dev;		/* device to which aux power is granted  */
+	struct acpi_device *adev;	/* root port/downstream port */
+	struct list_head list;
+};
+
+enum aux_pwr_req_status {
+	AUX_PWR_REQ_DENIED               = 0x0,
+	AUX_PWR_REQ_GRANTED              = 0x1,
+	AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL  = 0x2,
+	AUX_PWR_REQ_RETRY_INTERVAL_MIN   = 0x11,
+	AUX_PWR_REQ_RETRY_INTERVAL_MAX   = 0x1F
+};
+
+/**
+ * pci_acpi_request_d3cold_aux_power - Request aux power while device is in D3cold
+ * @dev: PCI device instance
+ * @requested_mw: Requested auxiliary power in milliwatts
+ * @retry_interval: Retry interval returned by platform to retry auxiliary
+ *                  power request
+ *
+ * Request auxilary power to platform firmware, via Root Port/Switch Downstream
+ * Port ACPI _DSM Function 0Ah, needed for the PCI device when it is in D3cold.
+ * Evaluate the _DSM and handle the response accordingly.
+ *
+ * For Multi-Function Devices, driver for Function 0 is required to report an
+ * aggregate power requirement covering all functions contained within the
+ * device.
+ *
+ * Return: 0 Aux power request granted
+ *	   1 No main power removal
+ *         errno on failure.
+ */
+int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
+				      u32 *retry_interval)
+{
+	union acpi_object in_obj = {
+		.integer.type = ACPI_TYPE_INTEGER,
+		.integer.value = requested_mw,
+	};
+
+	union acpi_object *out_obj;
+	int result;
+	struct pci_dev *bdev;
+	struct acpi_device *adev;
+	acpi_handle handle;
+	struct aux_pwr *apwr, *next;
+
+	if (!dev)
+		return -EINVAL;
+
+	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
+		handle = ACPI_HANDLE(&bdev->dev);
+		if (handle &&
+		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
+				   1 << DSM_PCI_D3COLD_AUX_POWER_LIMIT))
+			break;
+	}
+
+	if (!bdev)
+		return -ENODEV;
+
+	adev = ACPI_COMPANION(&bdev->dev);
+	if (!adev)
+		return -EINVAL;
+
+	guard(mutex)(&acpi_aux_pwr_lock);
+	/* Check if aux power already granted to different device */
+	list_for_each_entry_safe(apwr, next, &acpi_aux_pwr_list, list) {
+		if (apwr->adev == adev && apwr->dev != &dev->dev) {
+			pci_info(to_pci_dev(apwr->dev),
+				 "D3cold Aux Power request already granted: %u mW\n",
+				 apwr->aux_pwr_limit);
+			return -EALREADY;
+		}
+		if (apwr->adev == adev && apwr->dev == &dev->dev) {
+			list_del(&apwr->list);
+			kfree(apwr);
+			break;
+		}
+	}
+
+	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
+					  &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;
+	ACPI_FREE(out_obj);
+
+	if (retry_interval)
+		*retry_interval = 0;
+
+	switch (result) {
+	case AUX_PWR_REQ_DENIED:
+		pci_dbg(bdev, "D3cold Aux Power %u mW request denied\n",
+			requested_mw);
+		return -EINVAL;
+	case AUX_PWR_REQ_GRANTED:
+		pci_info(bdev, "D3cold Aux Power request granted: %u mW\n",
+			 requested_mw);
+		apwr = kzalloc(sizeof(*apwr), GFP_KERNEL);
+		if (apwr) {
+			apwr->aux_pwr_limit = requested_mw;
+			apwr->dev = &dev->dev;
+			apwr->adev = adev;
+			INIT_LIST_HEAD(&apwr->list);
+			list_add(&acpi_aux_pwr_list,
+				 &apwr->list);
+		}
+		return 0;
+	case AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL:
+		pci_info(bdev, "D3cold Aux Power: Main power won't be removed\n");
+		return 2;
+	case AUX_PWR_REQ_RETRY_INTERVAL_MIN ... AUX_PWR_REQ_RETRY_INTERVAL_MAX:
+		pci_info(bdev, "D3cold Aux Power request needs retry, interval: %u seconds\n",
+			 result & 0xF);
+		if (retry_interval) {
+			*retry_interval = result & 0xF;
+			return -EAGAIN;
+		}
+		return -EINVAL;
+	default:
+		pci_err(bdev, "D3cold Aux Power: Reserved or unsupported response: 0x%x\n",
+			result);
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);
+
 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..ba48e7ddb360 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -121,6 +121,7 @@ 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
 
 #ifdef CONFIG_PCIE_EDR
 void pci_acpi_add_edr_notifier(struct pci_dev *pdev);
@@ -132,10 +133,18 @@ 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_mw,
+				      u32 *retry_interval);
 
 #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) { }
+static inline int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev,
+						    u32 requested_mw,
+						    u32 *retry_interval)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_ACPI */
 
 #endif	/* _PCI_ACPI_H_ */
-- 
2.52.0

Re: [PATCH v6 01/12] PCI/ACPI: Add D3cold Aux Power Limit_DSM method

[Bjorn Helgaas]

On Tue, Jan 13, 2026 at 10:12:02PM +0530, Badal Nilawar wrote:
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Implement _DSM method 0Ah, as per PCI Firmware r3.3, sec 4.6.10,
> to request auxiliary power required by the device when in D3cold state.
> 
> Implementation allows only a single device below the Downstream Port to
> request for Aux Power Limit under a given Root Port/Downstream Port
> because it does not track and aggregate requests from all child devices
> below the Downstream Port as required by PCI Firmware r3.3, sec 4.6.10.
> 
> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> V2(Bjorn/Rafael):
>   - Call acpi_dsm_check() to find method 0Ah supported
>   - Return retry interval to caller
> V3(Kuppuswamy)
>   - Add NULL check for retry interval
> V4
>   - Define enums for aux power request status (Rafael)
>   - Add Co-developed-by and clean up Signed-off-by (Kappuswamy)
>  (Bjorn)
>   - Instead of root pci device pass the pci device of driver, traverse
>     up the tree and discover _DSM
>   - Allow only function 0 of device to request aux power
>   - Allow retry_interval to be NULL
>   - Refine commit message and function description
> V5(Rafael)
>   - Remove function 0 check and allow first caller of the given
>     downstream port (with _DSM) to requst aux power including
>     different function
>   - Squash Patch v5.02 to this patch
>   - In the logic, to allow single device to req power, use linked list
>     instead of adding extra variables to acpi device structure
>   - return positive code for no main power removal to distinguish from
>     aux power request granted
> ---
>  drivers/pci/pci-acpi.c   | 136 +++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |   9 +++
>  2 files changed, 145 insertions(+)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 9369377725fa..645d3005ba50 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1418,6 +1418,142 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
>  	ACPI_FREE(obj);
>  }
>  
> +static LIST_HEAD(acpi_aux_pwr_list);
> +static DEFINE_MUTEX(acpi_aux_pwr_lock);
> +
> +struct aux_pwr {
> +	u32 aux_pwr_limit;		/* aux power limit granted by platform firmware */
> +	struct device *dev;		/* device to which aux power is granted  */

Shorten these to fit in 80 columns like the rest of the file.

> +	struct acpi_device *adev;	/* root port/downstream port */
> +	struct list_head list;
> +};
> +
> +enum aux_pwr_req_status {
> +	AUX_PWR_REQ_DENIED               = 0x0,
> +	AUX_PWR_REQ_GRANTED              = 0x1,
> +	AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL  = 0x2,
> +	AUX_PWR_REQ_RETRY_INTERVAL_MIN   = 0x11,
> +	AUX_PWR_REQ_RETRY_INTERVAL_MAX   = 0x1F

Use lower-case hex ("0x1f") like the rest of the file.  Also below.

> +};
> +
> +/**
> + * pci_acpi_request_d3cold_aux_power - Request aux power while device is in D3cold

Shorten or wrap to fit in 80 columns.

> + * @dev: PCI device instance
> + * @requested_mw: Requested auxiliary power in milliwatts
> + * @retry_interval: Retry interval returned by platform to retry auxiliary
> + *                  power request
> + *
> + * Request auxilary power to platform firmware, via Root Port/Switch Downstream
> + * Port ACPI _DSM Function 0Ah, needed for the PCI device when it is in D3cold.
> + * Evaluate the _DSM and handle the response accordingly.

Drop this last sentence; I don't think it tells us anything new.

> + * For Multi-Function Devices, driver for Function 0 is required to report an
> + * aggregate power requirement covering all functions contained within the
> + * device.
> + *
> + * Return: 0 Aux power request granted
> + *	   1 No main power removal
> + *         errno on failure.
> + */
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
> +				      u32 *retry_interval)
> +{
> +	union acpi_object in_obj = {
> +		.integer.type = ACPI_TYPE_INTEGER,
> +		.integer.value = requested_mw,
> +	};
> +
> +	union acpi_object *out_obj;
> +	int result;
> +	struct pci_dev *bdev;
> +	struct acpi_device *adev;
> +	acpi_handle handle;
> +	struct aux_pwr *apwr, *next;
> +
> +	if (!dev)
> +		return -EINVAL;

We talked about only allowing this for function 0:
https://lore.kernel.org/all/20250904183046.GA1267851@bhelgaas/

> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {

I think bdev should start with pci_upstream_bridge(dev) as in the
other patch because this _DSM is only allowed in Downstream Ports.

> +		handle = ACPI_HANDLE(&bdev->dev);
> +		if (handle &&
> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
> +				   1 << DSM_PCI_D3COLD_AUX_POWER_LIMIT))
> +			break;
> +	}
> +
> +	if (!bdev)
> +		return -ENODEV;
> +
> +	adev = ACPI_COMPANION(&bdev->dev);
> +	if (!adev)
> +		return -EINVAL;
> +
> +	guard(mutex)(&acpi_aux_pwr_lock);
> +	/* Check if aux power already granted to different device */
> +	list_for_each_entry_safe(apwr, next, &acpi_aux_pwr_list, list) {
> +		if (apwr->adev == adev && apwr->dev != &dev->dev) {
> +			pci_info(to_pci_dev(apwr->dev),
> +				 "D3cold Aux Power request already granted: %u mW\n",
> +				 apwr->aux_pwr_limit);
> +			return -EALREADY;
> +		}
> +		if (apwr->adev == adev && apwr->dev == &dev->dev) {
> +			list_del(&apwr->list);
> +			kfree(apwr);
> +			break;
> +		}
> +	}
> +
> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
> +					  &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;
> +	ACPI_FREE(out_obj);
> +
> +	if (retry_interval)
> +		*retry_interval = 0;
> +
> +	switch (result) {
> +	case AUX_PWR_REQ_DENIED:
> +		pci_dbg(bdev, "D3cold Aux Power %u mW request denied\n",
> +			requested_mw);
> +		return -EINVAL;
> +	case AUX_PWR_REQ_GRANTED:
> +		pci_info(bdev, "D3cold Aux Power request granted: %u mW\n",
> +			 requested_mw);
> +		apwr = kzalloc(sizeof(*apwr), GFP_KERNEL);
> +		if (apwr) {
> +			apwr->aux_pwr_limit = requested_mw;
> +			apwr->dev = &dev->dev;
> +			apwr->adev = adev;
> +			INIT_LIST_HEAD(&apwr->list);
> +			list_add(&acpi_aux_pwr_list,
> +				 &apwr->list);
> +		}

I think we leak this allocation if the device is removed.  I think the
list idea is more complicated than aggregating would be.

I think we could:

  - add "aux_power_mw" in struct pci_dev

  - walk the tree below bdev, accumulating aux_power_mw
    (total_aux_power_mw += dev->aux_power_mw)

  - pass "total_aux_power_mw + requested_mw" to the _DSM

  - if successful, set dev->aux_power_mw = requested_mw

> +		return 0;
> +	case AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL:
> +		pci_info(bdev, "D3cold Aux Power: Main power won't be removed\n");
> +		return 2;

Kernel-doc says we return 1 for this case.

> +	case AUX_PWR_REQ_RETRY_INTERVAL_MIN ... AUX_PWR_REQ_RETRY_INTERVAL_MAX:
> +		pci_info(bdev, "D3cold Aux Power request needs retry, interval: %u seconds\n",
> +			 result & 0xF);

Lower-case hex.

> +		if (retry_interval) {
> +			*retry_interval = result & 0xF;
> +			return -EAGAIN;
> +		}
> +		return -EINVAL;

I think we should do:

  case AUX_PWR_REQ_RETRY_INTERVAL_MIN ...  AUX_PWR_REQ_RETRY_INTERVAL_MAX:
    result &= 0xf;
    pci_info(bdev, "... needs retry", result);
    if (retry_interval)
      *retry_interval = result;
    return -EAGAIN;

I don't think it's useful to return different errors based on whether
the caller supplied a "retry_interval" pointer.

> +	default:
> +		pci_err(bdev, "D3cold Aux Power: Reserved or unsupported response: 0x%x\n",
> +			result);
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);

Re: [PATCH v6 01/12] PCI/ACPI: Add D3cold Aux Power Limit_DSM method

[Nilawar, Badal]

On 15-01-2026 01:54, Bjorn Helgaas wrote:
> On Tue, Jan 13, 2026 at 10:12:02PM +0530, Badal Nilawar wrote:
>> From: Anshuman Gupta <anshuman.gupta@intel.com>
>>
>> Implement _DSM method 0Ah, as per PCI Firmware r3.3, sec 4.6.10,
>> to request auxiliary power required by the device when in D3cold state.
>>
>> Implementation allows only a single device below the Downstream Port to
>> request for Aux Power Limit under a given Root Port/Downstream Port
>> because it does not track and aggregate requests from all child devices
>> below the Downstream Port as required by PCI Firmware r3.3, sec 4.6.10.
>>
>> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>> V2(Bjorn/Rafael):
>>    - Call acpi_dsm_check() to find method 0Ah supported
>>    - Return retry interval to caller
>> V3(Kuppuswamy)
>>    - Add NULL check for retry interval
>> V4
>>    - Define enums for aux power request status (Rafael)
>>    - Add Co-developed-by and clean up Signed-off-by (Kappuswamy)
>>   (Bjorn)
>>    - Instead of root pci device pass the pci device of driver, traverse
>>      up the tree and discover _DSM
>>    - Allow only function 0 of device to request aux power
>>    - Allow retry_interval to be NULL
>>    - Refine commit message and function description
>> V5(Rafael)
>>    - Remove function 0 check and allow first caller of the given
>>      downstream port (with _DSM) to requst aux power including
>>      different function
>>    - Squash Patch v5.02 to this patch
>>    - In the logic, to allow single device to req power, use linked list
>>      instead of adding extra variables to acpi device structure
>>    - return positive code for no main power removal to distinguish from
>>      aux power request granted
>> ---
>>   drivers/pci/pci-acpi.c   | 136 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/pci-acpi.h |   9 +++
>>   2 files changed, 145 insertions(+)
>>
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index 9369377725fa..645d3005ba50 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -1418,6 +1418,142 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
>>   	ACPI_FREE(obj);
>>   }
>>   
>> +static LIST_HEAD(acpi_aux_pwr_list);
>> +static DEFINE_MUTEX(acpi_aux_pwr_lock);
>> +
>> +struct aux_pwr {
>> +	u32 aux_pwr_limit;		/* aux power limit granted by platform firmware */
>> +	struct device *dev;		/* device to which aux power is granted  */
> Shorten these to fit in 80 columns like the rest of the file.
Sure.
>
>> +	struct acpi_device *adev;	/* root port/downstream port */
>> +	struct list_head list;
>> +};
>> +
>> +enum aux_pwr_req_status {
>> +	AUX_PWR_REQ_DENIED               = 0x0,
>> +	AUX_PWR_REQ_GRANTED              = 0x1,
>> +	AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL  = 0x2,
>> +	AUX_PWR_REQ_RETRY_INTERVAL_MIN   = 0x11,
>> +	AUX_PWR_REQ_RETRY_INTERVAL_MAX   = 0x1F
> Use lower-case hex ("0x1f") like the rest of the file.  Also below.
Ok.
>
>> +};
>> +
>> +/**
>> + * pci_acpi_request_d3cold_aux_power - Request aux power while device is in D3cold
> Shorten or wrap to fit in 80 columns.
Ok
>
>> + * @dev: PCI device instance
>> + * @requested_mw: Requested auxiliary power in milliwatts
>> + * @retry_interval: Retry interval returned by platform to retry auxiliary
>> + *                  power request
>> + *
>> + * Request auxilary power to platform firmware, via Root Port/Switch Downstream
>> + * Port ACPI _DSM Function 0Ah, needed for the PCI device when it is in D3cold.
>> + * Evaluate the _DSM and handle the response accordingly.
> Drop this last sentence; I don't think it tells us anything new.
Ok
>
>> + * For Multi-Function Devices, driver for Function 0 is required to report an
>> + * aggregate power requirement covering all functions contained within the
>> + * device.
>> + *
>> + * Return: 0 Aux power request granted
>> + *	   1 No main power removal
>> + *         errno on failure.
>> + */
>> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>> +				      u32 *retry_interval)
>> +{
>> +	union acpi_object in_obj = {
>> +		.integer.type = ACPI_TYPE_INTEGER,
>> +		.integer.value = requested_mw,
>> +	};
>> +
>> +	union acpi_object *out_obj;
>> +	int result;
>> +	struct pci_dev *bdev;
>> +	struct acpi_device *adev;
>> +	acpi_handle handle;
>> +	struct aux_pwr *apwr, *next;
>> +
>> +	if (!dev)
>> +		return -EINVAL;
> We talked about only allowing this for function 0:
> https://lore.kernel.org/all/20250904183046.GA1267851@bhelgaas/

In rev5 there was suggestion from Rafael to remove function 0 check as 
synchronization among drivers will be
tricky. 
https://patchwork.freedesktop.org/patch/681119/?series=145342&rev=5#comment_1255325

>
>> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
> I think bdev should start with pci_upstream_bridge(dev) as in the
> other patch because this _DSM is only allowed in Downstream Ports.
Ok
>
>> +		handle = ACPI_HANDLE(&bdev->dev);
>> +		if (handle &&
>> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
>> +				   1 << DSM_PCI_D3COLD_AUX_POWER_LIMIT))
>> +			break;
>> +	}
>> +
>> +	if (!bdev)
>> +		return -ENODEV;
>> +
>> +	adev = ACPI_COMPANION(&bdev->dev);
>> +	if (!adev)
>> +		return -EINVAL;
>> +
>> +	guard(mutex)(&acpi_aux_pwr_lock);
>> +	/* Check if aux power already granted to different device */
>> +	list_for_each_entry_safe(apwr, next, &acpi_aux_pwr_list, list) {
>> +		if (apwr->adev == adev && apwr->dev != &dev->dev) {
>> +			pci_info(to_pci_dev(apwr->dev),
>> +				 "D3cold Aux Power request already granted: %u mW\n",
>> +				 apwr->aux_pwr_limit);
>> +			return -EALREADY;
>> +		}
>> +		if (apwr->adev == adev && apwr->dev == &dev->dev) {
>> +			list_del(&apwr->list);
>> +			kfree(apwr);
>> +			break;
>> +		}
>> +	}
>> +
>> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
>> +					  &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;
>> +	ACPI_FREE(out_obj);
>> +
>> +	if (retry_interval)
>> +		*retry_interval = 0;
>> +
>> +	switch (result) {
>> +	case AUX_PWR_REQ_DENIED:
>> +		pci_dbg(bdev, "D3cold Aux Power %u mW request denied\n",
>> +			requested_mw);
>> +		return -EINVAL;
>> +	case AUX_PWR_REQ_GRANTED:
>> +		pci_info(bdev, "D3cold Aux Power request granted: %u mW\n",
>> +			 requested_mw);
>> +		apwr = kzalloc(sizeof(*apwr), GFP_KERNEL);
>> +		if (apwr) {
>> +			apwr->aux_pwr_limit = requested_mw;
>> +			apwr->dev = &dev->dev;
>> +			apwr->adev = adev;
>> +			INIT_LIST_HEAD(&apwr->list);
>> +			list_add(&acpi_aux_pwr_list,
>> +				 &apwr->list);
>> +		}
> I think we leak this allocation if the device is removed.  I think the
> list idea is more complicated than aggregating would be.
>
> I think we could:
>
>    - add "aux_power_mw" in struct pci_dev
>
>    - walk the tree below bdev, accumulating aux_power_mw
>      (total_aux_power_mw += dev->aux_power_mw)
>
>    - pass "total_aux_power_mw + requested_mw" to the _DSM
>
>    - if successful, set dev->aux_power_mw = requested_mw

Introduced list based handling as in rev5 there was suggestion to avoid 
adding variables in acpi/acpi_power structures.
To handle allocation leak in function acpi_device_release() I will add 
code to scan list and delete the entry.

>> +		return 0;
>> +	case AUX_PWR_REQ_NO_MAIN_PWR_REMOVAL:
>> +		pci_info(bdev, "D3cold Aux Power: Main power won't be removed\n");
>> +		return 2;
> Kernel-doc says we return 1 for this case.
Will fix it.
>
>> +	case AUX_PWR_REQ_RETRY_INTERVAL_MIN ... AUX_PWR_REQ_RETRY_INTERVAL_MAX:
>> +		pci_info(bdev, "D3cold Aux Power request needs retry, interval: %u seconds\n",
>> +			 result & 0xF);
> Lower-case hex.
Ok
>
>> +		if (retry_interval) {
>> +			*retry_interval = result & 0xF;
>> +			return -EAGAIN;
>> +		}
>> +		return -EINVAL;
> I think we should do:
>
>    case AUX_PWR_REQ_RETRY_INTERVAL_MIN ...  AUX_PWR_REQ_RETRY_INTERVAL_MAX:
>      result &= 0xf;
>      pci_info(bdev, "... needs retry", result);
>      if (retry_interval)
>        *retry_interval = result;
>      return -EAGAIN;
>
> I don't think it's useful to return different errors based on whether
> the caller supplied a "retry_interval" pointer.

This is to indicate caller to retry after retry_interval returned by 
ACPI method.
It's up to caller whether to retry or just proceed.

Thanks,
Badal

>
>> +	default:
>> +		pci_err(bdev, "D3cold Aux Power: Reserved or unsupported response: 0x%x\n",
>> +			result);
>> +		return -EINVAL;
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);

Re: [PATCH v6 01/12] PCI/ACPI: Add D3cold Aux Power Limit_DSM method

[Bjorn Helgaas]

On Tue, Jan 20, 2026 at 07:33:17PM +0530, Nilawar, Badal wrote:
> On 15-01-2026 01:54, Bjorn Helgaas wrote:
> > On Tue, Jan 13, 2026 at 10:12:02PM +0530, Badal Nilawar wrote:
> > > From: Anshuman Gupta <anshuman.gupta@intel.com>
> > > 
> > > Implement _DSM method 0Ah, as per PCI Firmware r3.3, sec 4.6.10,
> > > to request auxiliary power required by the device when in D3cold state.
> ...

> > > +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
> > > +				      u32 *retry_interval)
> > > +{
> > > +	union acpi_object in_obj = {
> > > +		.integer.type = ACPI_TYPE_INTEGER,
> > > +		.integer.value = requested_mw,
> > > +	};
> > > +
> > > +	union acpi_object *out_obj;
> > > +	int result;
> > > +	struct pci_dev *bdev;
> > > +	struct acpi_device *adev;
> > > +	acpi_handle handle;
> > > +	struct aux_pwr *apwr, *next;
> > > +
> > > +	if (!dev)
> > > +		return -EINVAL;
> >
> > We talked about only allowing this for function 0:
> > https://lore.kernel.org/all/20250904183046.GA1267851@bhelgaas/
> 
> In rev5 there was suggestion from Rafael to remove function 0 check as
> synchronization among drivers will be
> tricky. https://patchwork.freedesktop.org/patch/681119/?series=145342&rev=5#comment_1255325

> > > +	case AUX_PWR_REQ_GRANTED:
> > > +		pci_info(bdev, "D3cold Aux Power request granted: %u mW\n",
> > > +			 requested_mw);
> > > +		apwr = kzalloc(sizeof(*apwr), GFP_KERNEL);
> > > +		if (apwr) {
> > > +			apwr->aux_pwr_limit = requested_mw;
> > > +			apwr->dev = &dev->dev;
> > > +			apwr->adev = adev;
> > > +			INIT_LIST_HEAD(&apwr->list);
> > > +			list_add(&acpi_aux_pwr_list,
> > > +				 &apwr->list);
> > > +		}
> >
> > I think we leak this allocation if the device is removed.  I think the
> > list idea is more complicated than aggregating would be.
> > 
> > I think we could:
> > 
> >    - add "aux_power_mw" in struct pci_dev
> > 
> >    - walk the tree below bdev, accumulating aux_power_mw
> >      (total_aux_power_mw += dev->aux_power_mw)
> > 
> >    - pass "total_aux_power_mw + requested_mw" to the _DSM
> > 
> >    - if successful, set dev->aux_power_mw = requested_mw
> 
> Introduced list based handling as in rev5 there was suggestion to avoid
> adding variables in acpi/acpi_power structures.
> To handle allocation leak in function acpi_device_release() I will add code
> to scan list and delete the entry.

I think it's a mistake to add list handling for this sort of
non-dynamic situation because it adds complexity and lifetime issues.

I'm OK with adding this data to struct pci_dev.  That means we don't
have to deal with any extra list management or lifetime issues, and
it's also a way to handle the aggregation and avoid the problem of
synchronizing between drivers.

My proposal is to aggregate the power requirements of all the devices
below the bridge that implements this _DSM (bdev), and store the total
requirement in bdev->aux_power_mw.

We can do the aggregation piecemeal.  As each downstream driver calls
this function, its requirement can be added to aux_power_mw.

> > > +		if (retry_interval) {
> > > +			*retry_interval = result & 0xF;
> > > +			return -EAGAIN;
> > > +		}
> > > +		return -EINVAL;
> >
> > I think we should do:
> > 
> >    case AUX_PWR_REQ_RETRY_INTERVAL_MIN ...  AUX_PWR_REQ_RETRY_INTERVAL_MAX:
> >      result &= 0xf;
> >      pci_info(bdev, "... needs retry", result);
> >      if (retry_interval)
> >        *retry_interval = result;
> >      return -EAGAIN;
> > 
> > I don't think it's useful to return different errors based on whether
> > the caller supplied a "retry_interval" pointer.
> 
> This is to indicate caller to retry after retry_interval returned by ACPI
> method.
>
> It's up to caller whether to retry or just proceed.

I understand what the retry_interval is for.  Obviously the caller can
always decide whether to retry.

How does it help the caller if we return -EAGAIN if it supplied a
retry_interval pointer, but -EINVAL if it didn't?

When the _DSM returns an AUX_PWR_REQ_RETRY_INTERVAL status, I think we
should always return -EAGAIN.

Bjorn

[PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
fixed delay in timing between the time the PME_TO_Ack message is received
at the PCI Express Downstream Port that originated the PME_Turn_Off
message, and the time the platform asserts PERST# to the slot during the
corresponding Endpoint’s or PCI Express Upstream Port’s transition to
D3cold while the system is in an ACPI operational state.
Host platform supporting this feature ensures that device is observing
this delay in every applicable D3Cold transition.

Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
For VRSR feature with PERST# Assertion delay device will get enough time
to transition to auxiliary power before main power removal.
---
 drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |  9 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 645d3005ba50..73eaee20a270 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
 }
 EXPORT_SYMBOL_GPL(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
+ *
+ * Request PERST# Assertion Delay to platform firmware, via Root Port/
+ * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
+ * PCI device.
+ * Evaluate the _DSM and handle 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;
+	int result, ret = -EINVAL;
+	struct pci_dev *bdev;
+	acpi_handle handle;
+
+	if (!dev)
+		return -EINVAL;
+
+	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
+		handle = ACPI_HANDLE(&bdev->dev);
+		if (handle &&
+		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
+				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
+			break;
+	}
+
+	if (!bdev)
+		return -ENODEV;
+
+	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
+					  &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;
+	ACPI_FREE(out_obj);
+
+	if (result == delay_us) {
+		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
+		ret = 0;
+	} else {
+		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
+			 result);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(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 ba48e7ddb360..4d8b8cea019e 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -122,6 +122,7 @@ extern const guid_t pci_acpi_dsm_guid;
 #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);
@@ -135,7 +136,7 @@ int pci_acpi_set_companion_lookup_hook(struct acpi_device *(*func)(struct pci_de
 void pci_acpi_clear_companion_lookup_hook(void);
 int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
 				      u32 *retry_interval);
-
+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) { }
@@ -145,6 +146,12 @@ static inline int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev,
+						     u32 delay_us)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_ACPI */
 
 #endif	/* _PCI_ACPI_H_ */
-- 
2.52.0

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Manivannan Sadhasivam]

On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request

4.6.11

> fixed delay in timing between the time the PME_TO_Ack message is received
> at the PCI Express Downstream Port that originated the PME_Turn_Off
> message, and the time the platform asserts PERST# to the slot during the
> corresponding Endpoint’s or PCI Express Upstream Port’s transition to
> D3cold while the system is in an ACPI operational state.
> Host platform supporting this feature ensures that device is observing
> this delay in every applicable D3Cold transition.
> 
> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> For VRSR feature with PERST# Assertion delay device will get enough time
> to transition to auxiliary power before main power removal.
> ---
>  drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  9 +++++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 645d3005ba50..73eaee20a270 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>  }
>  EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);
>  
> +/**
> + * pci_acpi_add_perst_assertion_delay - Request PERST# Delay via ACPI DSM

I'd name the API as "pci_acpi_set_perst_assertion_delay", but the firmware spec
calls the _DSM as 'Add PERST# Assertion Delay", so I guess it is fine.

But the description should be changed to "Add PERST# assertion delay via ACPI DSM"

> + * @dev: PCI device instance
> + * @delay_us: Requested delay_us

"Delay to be added"?

> + *
> + * Request PERST# Assertion Delay to platform firmware, via Root Port/

Here also.

> + * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
> + * PCI device.
> + * Evaluate the _DSM and handle 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;
> +	int result, ret = -EINVAL;
> +	struct pci_dev *bdev;
> +	acpi_handle handle;
> +
> +	if (!dev)
> +		return -EINVAL;
> +
> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
> +		handle = ACPI_HANDLE(&bdev->dev);
> +		if (handle &&
> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
> +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
> +			break;
> +	}
> +
> +	if (!bdev)
> +		return -ENODEV;
> +
> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
> +					  &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;
> +	ACPI_FREE(out_obj);
> +
> +	if (result == delay_us) {
> +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
> +		ret = 0;
> +	} else {
> +		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
> +			 result);
> +	}

How about:

	if (result != delay_us) {
		pci_warn(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
			 result);
		return -EINVAL;
	}

	pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);

	return 0;

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Nilawar, Badal]

On 13-01-2026 22:34, Manivannan Sadhasivam wrote:
> On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
>> From: Anshuman Gupta <anshuman.gupta@intel.com>
>>
>> Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
> 4.6.11
I will fix this.
>
>> fixed delay in timing between the time the PME_TO_Ack message is received
>> at the PCI Express Downstream Port that originated the PME_Turn_Off
>> message, and the time the platform asserts PERST# to the slot during the
>> corresponding Endpoint’s or PCI Express Upstream Port’s transition to
>> D3cold while the system is in an ACPI operational state.
>> Host platform supporting this feature ensures that device is observing
>> this delay in every applicable D3Cold transition.
>>
>> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>> For VRSR feature with PERST# Assertion delay device will get enough time
>> to transition to auxiliary power before main power removal.
>> ---
>>   drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/pci-acpi.h |  9 +++++-
>>   2 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index 645d3005ba50..73eaee20a270 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>>   }
>>   EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);
>>   
>> +/**
>> + * pci_acpi_add_perst_assertion_delay - Request PERST# Delay via ACPI DSM
> I'd name the API as "pci_acpi_set_perst_assertion_delay", but the firmware spec
> calls the _DSM as 'Add PERST# Assertion Delay", so I guess it is fine.
>
> But the description should be changed to "Add PERST# assertion delay via ACPI DSM"
Sure.
>
>> + * @dev: PCI device instance
>> + * @delay_us: Requested delay_us
> "Delay to be added"?
Ok.
>
>> + *
>> + * Request PERST# Assertion Delay to platform firmware, via Root Port/
> Here also.
Ok.
>
>> + * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
>> + * PCI device.
>> + * Evaluate the _DSM and handle 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;
>> +	int result, ret = -EINVAL;
>> +	struct pci_dev *bdev;
>> +	acpi_handle handle;
>> +
>> +	if (!dev)
>> +		return -EINVAL;
>> +
>> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
>> +		handle = ACPI_HANDLE(&bdev->dev);
>> +		if (handle &&
>> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
>> +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
>> +			break;
>> +	}
>> +
>> +	if (!bdev)
>> +		return -ENODEV;
>> +
>> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
>> +					  &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;
>> +	ACPI_FREE(out_obj);
>> +
>> +	if (result == delay_us) {
>> +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
>> +		ret = 0;
>> +	} else {
>> +		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
>> +			 result);
>> +	}
> How about:
>
> 	if (result != delay_us) {
> 		pci_warn(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
> 			 result);

Kept pci_info as the delay request might not always be honored, and it's 
up to the caller to handle the return appropriately.
For instance, in the case of VRSR, if the delay request fails, VRSR 
won't be enabled.

Thanks,
Badal

> 		return -EINVAL;
> 	}
>
> 	pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
>
> 	return 0;
>
> - Mani
>

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Bjorn Helgaas]

On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
> fixed delay in timing between the time the PME_TO_Ack message is received
> at the PCI Express Downstream Port that originated the PME_Turn_Off
> message, and the time the platform asserts PERST# to the slot during the
> corresponding Endpoint’s or PCI Express Upstream Port’s transition to
> D3cold while the system is in an ACPI operational state.
> Host platform supporting this feature ensures that device is observing
> this delay in every applicable D3Cold transition.

Thanks for this work!

Add blank lines between paragraphs.

s/D3Cold/D3cold/ to match other uses in drivers/pci/

> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> For VRSR feature with PERST# Assertion delay device will get enough time
> to transition to auxiliary power before main power removal.
> ---
>  drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  9 +++++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 645d3005ba50..73eaee20a270 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>  }
>  EXPORT_SYMBOL_GPL(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
> + *
> + * Request PERST# Assertion Delay to platform firmware, via Root Port/
> + * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
> + * PCI device.
> + * Evaluate the _DSM and handle the response accordingly.

Add blank line between paragraphs.  Actually, you can just omit the
last sentence because it doesn't tell us anything useful.

> + * Return: returns 0 on success and errno on failure.

s/Return: returns 0/Return: 0/

> + */
> +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,
> +	};
> +

Spurious blank line.

> +	union acpi_object *out_obj;
> +	int result, ret = -EINVAL;

"ret" is unnecessary; see below.

> +	struct pci_dev *bdev;
> +	acpi_handle handle;
> +
> +	if (!dev)
> +		return -EINVAL;
> +
> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {

I think bdev should start with pci_upstream_bridge(dev).

IIUC you intend that "dev" is the Endpoint or Upstream Port for which
a driver wants PERST# assertion to be delayed.  Per sec 4.6.11, the
_DSM must be in a Downstream Port, not the device itself.

Sec 4.6.11 also says we should track this per Downstream Port and
request the maximum of delays requested by any child.  So I think we
need to:

  - add a perst_delay in struct pci_dev

  - when we find this _DSM, set
    bdev.perst_delay = max(bdev.perst_delay, delay_us)

  - pass bdev.perst_delay to the _DSM instead of delay_us

> +		handle = ACPI_HANDLE(&bdev->dev);
> +		if (handle &&
> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
> +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
> +			break;
> +	}
> +
> +	if (!bdev)
> +		return -ENODEV;
> +
> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
> +					  &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;
> +	ACPI_FREE(out_obj);
> +
> +	if (result == delay_us) {
> +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);

I think this message should use "bdev" instead of "dev".  I know the
request is for "dev", but the delay connected to bdev and applies to
all functions below the Downstream Port.

Wrap such that the string itself isn't broken across lines, but the
"delay_us" is one the next line (as in the "failed" message below).

> +		ret = 0;
> +	} else {
> +		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
> +			 result);
> +	}
> +
> +	return ret;

No need for the "else"; we can just return error early and unindent
the normal path:

  if (result != delay_us) {
    pci_info(bdev, "... failed");
    return -EINVAL;
  }

  pci_info(bdev, "... set");
  return 0;

> +}

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Bjorn Helgaas]

On Wed, Jan 14, 2026 at 01:55:17PM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
> > From: Anshuman Gupta <anshuman.gupta@intel.com>
> > 
> > Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
> > fixed delay in timing between the time the PME_TO_Ack message is received
> > at the PCI Express Downstream Port that originated the PME_Turn_Off
> > message, and the time the platform asserts PERST# to the slot during the
> > corresponding Endpoint’s or PCI Express Upstream Port’s transition to
> > D3cold while the system is in an ACPI operational state.
> > Host platform supporting this feature ensures that device is observing
> > this delay in every applicable D3Cold transition.
> ...

> Sec 4.6.11 also says we should track this per Downstream Port and
> request the maximum of delays requested by any child.  So I think we
> need to:
> 
>   - add a perst_delay in struct pci_dev

If you do this, name it "perst_delay_us" so we know the units.

>   - when we find this _DSM, set
>     bdev.perst_delay = max(bdev.perst_delay, delay_us)
> 
>   - pass bdev.perst_delay to the _DSM instead of delay_us

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Nilawar, Badal]

On 15-01-2026 01:25, Bjorn Helgaas wrote:
> On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
>> From: Anshuman Gupta <anshuman.gupta@intel.com>
>>
>> Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
>> fixed delay in timing between the time the PME_TO_Ack message is received
>> at the PCI Express Downstream Port that originated the PME_Turn_Off
>> message, and the time the platform asserts PERST# to the slot during the
>> corresponding Endpoint’s or PCI Express Upstream Port’s transition to
>> D3cold while the system is in an ACPI operational state.
>> Host platform supporting this feature ensures that device is observing
>> this delay in every applicable D3Cold transition.
> Thanks for this work!
>
> Add blank lines between paragraphs.
>
> s/D3Cold/D3cold/ to match other uses in drivers/pci/
Ok.
>
>> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>> For VRSR feature with PERST# Assertion delay device will get enough time
>> to transition to auxiliary power before main power removal.
>> ---
>>   drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/pci-acpi.h |  9 +++++-
>>   2 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index 645d3005ba50..73eaee20a270 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>>   }
>>   EXPORT_SYMBOL_GPL(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
>> + *
>> + * Request PERST# Assertion Delay to platform firmware, via Root Port/
>> + * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
>> + * PCI device.
>> + * Evaluate the _DSM and handle the response accordingly.
> Add blank line between paragraphs.  Actually, you can just omit the
> last sentence because it doesn't tell us anything useful.
Sure.
>
>> + * Return: returns 0 on success and errno on failure.
> s/Return: returns 0/Return: 0/
Ok
>
>> + */
>> +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,
>> +	};
>> +
> Spurious blank line.
Ok
>
>> +	union acpi_object *out_obj;
>> +	int result, ret = -EINVAL;
> "ret" is unnecessary; see below.
Will take care of this.
>
>> +	struct pci_dev *bdev;
>> +	acpi_handle handle;
>> +
>> +	if (!dev)
>> +		return -EINVAL;
>> +
>> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {
> I think bdev should start with pci_upstream_bridge(dev).
Yes.
>
> IIUC you intend that "dev" is the Endpoint or Upstream Port for which
> a driver wants PERST# assertion to be delayed.  Per sec 4.6.11, the
> _DSM must be in a Downstream Port, not the device itself.

Yes. _DSM is in Downstream port but not device itself. For VRSR use case 
_DSM
is in root port. From the description, my understanding is Downstream 
port (bridge/root port),
which has sent PME_Turn_Off message, will introduce delay after 
PME_TO_Ack and then initiate PERST.

>
> Sec 4.6.11 also says we should track this per Downstream Port and
> request the maximum of delays requested by any child.  So I think we
> need to:
>
>    - add a perst_delay in struct pci_dev
>
>    - when we find this _DSM, set
>      bdev.perst_delay = max(bdev.perst_delay, delay_us)
>
>    - pass bdev.perst_delay to the _DSM instead of delay_us

For vrsr use case delay requested is 10ms, which is maximum allowed by 
this _DSM.
I think we should take care of above suggestion when we implement aux 
power aggregation.
For now, I can create another list let's call it acpi_perst_delay_list, 
similar to acpi_aux_power_list, and
allow a only one device to request a PERST delay.

Thanks,
Badal

>> +		handle = ACPI_HANDLE(&bdev->dev);
>> +		if (handle &&
>> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
>> +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
>> +			break;
>> +	}
>> +
>> +	if (!bdev)
>> +		return -ENODEV;
>> +
>> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
>> +					  &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;
>> +	ACPI_FREE(out_obj);
>> +
>> +	if (result == delay_us) {
>> +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
> I think this message should use "bdev" instead of "dev".  I know the
> request is for "dev", but the delay connected to bdev and applies to
> all functions below the Downstream Port.
Since it is set for the device which as requested I kept dev.
>
> Wrap such that the string itself isn't broken across lines, but the
> "delay_us" is one the next line (as in the "failed" message below).
>
>> +		ret = 0;
>> +	} else {
>> +		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
>> +			 result);
>> +	}
>> +
>> +	return ret;
> No need for the "else"; we can just return error early and unindent
> the normal path:
Sure.
>
>    if (result != delay_us) {
>      pci_info(bdev, "... failed");
>      return -EINVAL;
>    }
>
>    pci_info(bdev, "... set");
>    return 0;
>
>> +}

Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Bjorn Helgaas]

On Tue, Jan 20, 2026 at 09:29:16PM +0530, Nilawar, Badal wrote:
> On 15-01-2026 01:25, Bjorn Helgaas wrote:
> > On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
> > > From: Anshuman Gupta <anshuman.gupta@intel.com>
> > > 
> > > Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
> > > fixed delay in timing between the time the PME_TO_Ack message is received
> > > at the PCI Express Downstream Port that originated the PME_Turn_Off
> > > message, and the time the platform asserts PERST# to the slot during the
> > > corresponding Endpoint’s or PCI Express Upstream Port’s transition to
> > > D3cold while the system is in an ACPI operational state.
> > > Host platform supporting this feature ensures that device is observing
> > > this delay in every applicable D3Cold transition.

> > > +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
> > > +{
> ...

> > > +	if (!dev)
> > > +		return -EINVAL;

Calling this with "dev == NULL" is a bug in the caller, so I don't
think we should check for this.  That way we will take the NULL
pointer dereference, cause an oops, and can fix the bug.  If we check
for NULL, the bug is likely to be unnoticed.

> > Sec 4.6.11 also says we should track this per Downstream Port and
> > request the maximum of delays requested by any child.  So I think we
> > need to:
> > 
> >    - add a perst_delay in struct pci_dev
> > 
> >    - when we find this _DSM, set
> >      bdev.perst_delay = max(bdev.perst_delay, delay_us)
> > 
> >    - pass bdev.perst_delay to the _DSM instead of delay_us
> 
> For vrsr use case delay requested is 10ms, which is maximum allowed by this
> _DSM.
> I think we should take care of above suggestion when we implement aux power
> aggregation.
> For now, I can create another list let's call it acpi_perst_delay_list,
> similar to acpi_aux_power_list, and
> allow a only one device to request a PERST delay.

I think lists will be a maintenance headache.

> > > +		handle = ACPI_HANDLE(&bdev->dev);
> > > +		if (handle &&
> > > +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
> > > +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
> > > +			break;
> > > +	}
> > > +
> > > +	if (!bdev)
> > > +		return -ENODEV;
> > > +
> > > +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
> > > +					  &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;
> > > +	ACPI_FREE(out_obj);
> > > +
> > > +	if (result == delay_us) {
> > > +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);
> >
> > I think this message should use "bdev" instead of "dev".  I know the
> > request is for "dev", but the delay connected to bdev and applies to
> > all functions below the Downstream Port.
>
> Since it is set for the device which as requested I kept dev.

It's set for *all* devices below bdev, so I think the message should
give a clue about that.  If we only include "dev", there is no
indication that other devices are affected.  Maybe we could include
both dev and bdev?

  pci_info(bdev, "PERST# assertion delay %u microseconds set as requested by %s\n",
           delay_us, pci_name(bdev));

[PATCH v6 03/12] drm/xe/vrsr: Introduce flag has_vrsr

[Badal Nilawar]

Introduce flag has_vrsr to determine if platform supports VRSR feature

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Karthik Poosa <karthik.poosa@intel.com>
---
V2: Rebased
---
 drivers/gpu/drm/xe/xe_device_types.h | 2 ++
 drivers/gpu/drm/xe/xe_pci.c          | 2 ++
 drivers/gpu/drm/xe/xe_pci_types.h    | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index f689766adcb1..a7e9d981618c 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -363,6 +363,8 @@ struct xe_device {
 		u8 has_sriov:1;
 		/** @info.has_usm: Device has unified shared memory support */
 		u8 has_usm:1;
+		/** @info.has_vrsr: Has capability to enter into VRAM self refresh */
+		u8 has_vrsr:1;
 		/** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
 		u8 has_64bit_timestamp:1;
 		/** @info.is_dgfx: is discrete device */
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 5c705124270e..5c14e1978a72 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -375,6 +375,7 @@ static const struct xe_device_desc bmg_desc = {
 	.has_soc_remapper_telem = true,
 	.has_sriov = true,
 	.has_mem_copy_instr = true,
+	.has_vrsr = true,
 	.max_gt_per_tile = 2,
 	.needs_scratch = true,
 	.subplatforms = (const struct xe_subplatform_desc[]) {
@@ -706,6 +707,7 @@ static int xe_info_init_early(struct xe_device *xe,
 	xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
 		desc->has_sriov;
 	xe->info.has_mem_copy_instr = desc->has_mem_copy_instr;
+	xe->info.has_vrsr = desc->has_vrsr;
 	xe->info.skip_guc_pc = desc->skip_guc_pc;
 	xe->info.skip_mtcfg = desc->skip_mtcfg;
 	xe->info.skip_pcode = desc->skip_pcode;
diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
index 20acc5349ee6..d9c490a68043 100644
--- a/drivers/gpu/drm/xe/xe_pci_types.h
+++ b/drivers/gpu/drm/xe/xe_pci_types.h
@@ -57,6 +57,7 @@ struct xe_device_desc {
 	u8 has_soc_remapper_sysctrl:1;
 	u8 has_soc_remapper_telem:1;
 	u8 has_sriov:1;
+	u8 has_vrsr:1;
 	u8 needs_scratch:1;
 	u8 skip_guc_pc:1;
 	u8 skip_mtcfg:1;
-- 
2.52.0

[PATCH v6 04/12] drm/xe/vrsr: Detect VRSR Capability

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

Detect VRAM Self Refresh(vrsr) Capability.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
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           | 26 ++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
index ad93c57edd17..153c0a3c98d2 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_SR_CAPABILITY                         XE_REG(0x138144)
+#define   VRAM_SR_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 a7e9d981618c..17a577772794 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -576,6 +576,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:
 		 *
@@ -586,6 +589,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 fa607be299a7..18a44b395559 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -13,15 +13,18 @@
 #include <drm/ttm/ttm_placement.h>
 
 #include "display/xe_display.h"
+#include "regs/xe_regs.h"
 #include "xe_bo.h"
 #include "xe_bo_evict.h"
 #include "xe_device.h"
+#include "xe_force_wake.h"
 #include "xe_ggtt.h"
 #include "xe_gt.h"
 #include "xe_gt_idle.h"
 #include "xe_i2c.h"
 #include "xe_irq.h"
 #include "xe_late_bind_fw.h"
+#include "xe_mmio.h"
 #include "xe_pcode.h"
 #include "xe_pxp.h"
 #include "xe_sriov_vf_ccs.h"
@@ -318,6 +321,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_SR_CAPABILITY);
+	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+
+	return val & VRAM_SR_SUPPORTED;
+}
+
 static void xe_pm_runtime_init(struct xe_device *xe)
 {
 	struct device *dev = xe->drm.dev;
@@ -462,6 +487,7 @@ int xe_pm_init(struct xe_device *xe)
 		err = xe_pm_set_vram_threshold(xe, vram_threshold);
 		if (err)
 			goto err_unregister;
+		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
 	}
 
 	xe_pm_runtime_init(xe);
-- 
2.52.0

[PATCH v6 05/12] drm/xe/vrsr: Initialize VRSR feature

[Badal Nilawar]

Add the API xe_pm_vrsr_enable to initialize the VRSR feature,
requesting AUX power limit and PERST# assertion delay.

V2: Add retry mechanism while requesting AUX power limit
V3: Split xe_pm_vrsr_enable() into separate enable and
    disable functions

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    |   7 ++
 drivers/gpu/drm/xe/xe_pm.c           | 110 ++++++++++++++++++++++++++-
 drivers/gpu/drm/xe/xe_pm.h           |   2 +
 4 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 17a577772794..3b42400f8563 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 85cc7478b787..15687bb57c94 100644
--- a/drivers/gpu/drm/xe/xe_pcode_api.h
+++ b/drivers/gpu/drm/xe/xe_pcode_api.h
@@ -72,6 +72,13 @@
 #define       FAN_TABLE				1
 #define       VR_CONFIG				2
 
+#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	    POWER_D3_VRSR_PERST_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 18a44b395559..3fe673f0f87d 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -5,6 +5,7 @@
 
 #include "xe_pm.h"
 
+#include <linux/delay.h>
 #include <linux/fault-inject.h>
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
@@ -25,6 +26,7 @@
 #include "xe_irq.h"
 #include "xe_late_bind_fw.h"
 #include "xe_mmio.h"
+#include "xe_pcode_api.h"
 #include "xe_pcode.h"
 #include "xe_pxp.h"
 #include "xe_sriov_vf_ccs.h"
@@ -343,6 +345,112 @@ static bool xe_pm_vrsr_capable(struct xe_device *xe)
 	return val & VRAM_SR_SUPPORTED;
 }
 
+static int pci_acpi_aux_power_setup(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);
+	int ret;
+	u32 uval;
+	u32 aux_pwr_limit;
+	u32 retry_interval;
+	u32 perst_delay;
+
+	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_PERST_MASK, uval);
+
+	drm_dbg(&xe->drm, "Aux Power limit = %d mW\n", aux_pwr_limit);
+	drm_dbg(&xe->drm, "PERST# Assertion delay = %d microseconds\n", perst_delay);
+
+retry:
+	ret = pci_acpi_request_d3cold_aux_power(pdev, aux_pwr_limit, &retry_interval);
+
+	if (ret == -EAGAIN) {
+		drm_warn(&xe->drm, "D3cold Aux Power request needs retry interval: %d seconds\n",
+			 retry_interval);
+		msleep(retry_interval * 1000);
+		goto retry;
+	}
+
+	if (ret)
+		return ret;
+
+	ret = pci_acpi_add_perst_assertion_delay(pdev, perst_delay);
+
+	return ret;
+}
+
+static void xe_pm_vrsr_init(struct xe_device *xe)
+{
+	int ret;
+
+	if (!xe->info.has_vrsr)
+		return;
+
+	if (!xe_pm_vrsr_capable(xe))
+		return;
+
+	/*
+	 * If the VRSR initialization fails, the device will proceed with the regular
+	 * D3cold flow
+	 */
+	ret = pci_acpi_aux_power_setup(xe);
+	if (ret) {
+		drm_info(&xe->drm, "VRSR capable: No\n");
+		return;
+	}
+
+	xe->d3cold.vrsr_capable = true;
+	drm_info(&xe->drm, "VRSR capable: Yes\n");
+}
+
+/**
+ * xe_pm_vrsr_enable - Enable VRAM self refresh
+ * @xe: The xe device.
+ *
+ * Enable VRSR feature in D3cold path.
+ *
+ * Return: It returns 0 on success and errno on failure.
+ */
+int xe_pm_vrsr_enable(struct xe_device *xe)
+{
+	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+	int ret;
+
+	if (!xe->d3cold.vrsr_capable)
+		return -ENXIO;
+
+	drm_dbg(&xe->drm, "Enabling VRSR\n");
+
+	ret = xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
+						   PCODE_D3_VRSR_SC_ENABLE, 0), 0);
+	return ret;
+}
+
+/**
+ * xe_pm_vrsr_disable - Disable VRAM self refresh
+ * @xe: The xe device.
+ *
+ * Disable VRSR feature in D3cold path.
+ */
+void xe_pm_vrsr_disable(struct xe_device *xe)
+{
+	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+
+	if (!xe->d3cold.vrsr_capable)
+		return;
+
+	drm_dbg(&xe->drm, "Disabling VRSR\n");
+
+	xe_pcode_write(root_tile, PCODE_MBOX(PCODE_D3_VRAM_SELF_REFRESH,
+					     PCODE_D3_VRSR_SC_DISABLE, 0), 0);
+}
+
 static void xe_pm_runtime_init(struct xe_device *xe)
 {
 	struct device *dev = xe->drm.dev;
@@ -487,7 +595,7 @@ int xe_pm_init(struct xe_device *xe)
 		err = xe_pm_set_vram_threshold(xe, vram_threshold);
 		if (err)
 			goto err_unregister;
-		xe->d3cold.vrsr_capable = xe_pm_vrsr_capable(xe);
+		xe_pm_vrsr_init(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 6b27039e7b2d..8c7fe8e4241d 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -54,4 +54,6 @@ DEFINE_GUARD_COND(xe_pm_runtime, _ioctl, xe_pm_runtime_get_ioctl(_T), _RET >= 0)
 DEFINE_GUARD(xe_pm_runtime_release_only, struct xe_device *,
 	     __xe_pm_runtime_noop(_T), xe_pm_runtime_put(_T));
 
+int xe_pm_vrsr_enable(struct xe_device *xe);
+void xe_pm_vrsr_disable(struct xe_device *xe);
 #endif
-- 
2.52.0

[PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Badal Nilawar]

The VRSR feature is to enhance the display screen refresh experience
when the device exits from the D3cold state. Therefore, apply the VRSR
feature to the default VGA boot device and when a display is connected.

v2: Move generic display logic to i915/display (Jani)

Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.h |  1 +
 drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
 drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
 drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
 5 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 81b3a6692ca2..97c74272fb19 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct intel_display *display)
 {
 	return IS_DISPLAY_VER(display, 6, 11) && intel_display_vtd_active(display);
 }
+
+bool intel_display_connected(struct intel_display *display)
+{
+	struct drm_connector *list_connector;
+	struct drm_connector_list_iter iter;
+	bool ret = false;
+
+	mutex_lock(&display->drm->mode_config.mutex);
+	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index f8e6e4e82722..20690aa59324 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum port port);
 
 bool intel_scanout_needs_vtd_wa(struct intel_display *display);
 int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
+bool intel_display_connected(struct intel_display *display);
 
 #endif
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index f8a831b5dc7d..54ed39b257ad 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
 	return intel_display_driver_probe_defer(pdev);
 }
 
+bool xe_display_connected(struct xe_device *xe)
+{
+	return intel_display_connected(xe->display);
+}
+
 /**
  * xe_display_driver_set_hooks - Add driver flags and hooks for display
  * @driver: DRM device driver
diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
index 76db95c25f7e..11d4b09808e5 100644
--- a/drivers/gpu/drm/xe/display/xe_display.h
+++ b/drivers/gpu/drm/xe/display/xe_display.h
@@ -37,6 +37,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
 
@@ -67,5 +68,6 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
 static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
 static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
 
+static inline bool xe_display_connected(struct xe_device *xe) { return false; }
 #endif /* CONFIG_DRM_XE_DISPLAY */
 #endif /* _XE_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 3fe673f0f87d..e7aa876ce9e0 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -9,6 +9,7 @@
 #include <linux/fault-inject.h>
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
+#include <linux/vgaarb.h>
 
 #include <drm/drm_managed.h>
 #include <drm/ttm/ttm_placement.h>
@@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct xe_device *xe)
 
 static void xe_pm_vrsr_init(struct xe_device *xe)
 {
+	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
 	int ret;
 
 	if (!xe->info.has_vrsr)
@@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
 	if (!xe_pm_vrsr_capable(xe))
 		return;
 
+	if (pdev != vga_default_device() || !xe_display_connected(xe))
+		return;
+
 	/*
 	 * If the VRSR initialization fails, the device will proceed with the regular
 	 * D3cold flow
-- 
2.52.0

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Jani Nikula]

On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
> The VRSR feature is to enhance the display screen refresh experience
> when the device exits from the D3cold state. Therefore, apply the VRSR
> feature to the default VGA boot device and when a display is connected.

I don't understand how you get from the 1st sentence "therefore" the 2nd
sentence. Please elaborate what you're trying to do here, and why.

So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
xe_display_connected() -> intel_display_connected(), and that's the only
path and point in time to check whether displays are connected. If not,
the decision is "not VRSR capable", which is just a weird concusion to
make. The *capability* does not depend on displays, does it?

If you boot a device without a display, and then plug in a display, no
VRSR for you?

More comments inline.

> v2: Move generic display logic to i915/display (Jani)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  1 +
>  drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
>  drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
>  drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
>  5 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 81b3a6692ca2..97c74272fb19 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct intel_display *display)
>  {
>  	return IS_DISPLAY_VER(display, 6, 11) && intel_display_vtd_active(display);
>  }
> +
> +bool intel_display_connected(struct intel_display *display)
> +{
> +	struct drm_connector *list_connector;
> +	struct drm_connector_list_iter iter;
> +	bool ret = false;
> +
> +	mutex_lock(&display->drm->mode_config.mutex);
> +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index f8e6e4e82722..20690aa59324 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum port port);
>  
>  bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>  int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
> +bool intel_display_connected(struct intel_display *display);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index f8a831b5dc7d..54ed39b257ad 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
>  	return intel_display_driver_probe_defer(pdev);
>  }
>  
> +bool xe_display_connected(struct xe_device *xe)
> +{
> +	return intel_display_connected(xe->display);
> +}
> +
>  /**
>   * xe_display_driver_set_hooks - Add driver flags and hooks for display
>   * @driver: DRM device driver
> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> index 76db95c25f7e..11d4b09808e5 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.h
> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> @@ -37,6 +37,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
>  
> @@ -67,5 +68,6 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>  static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
>  static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
>  
> +static inline bool xe_display_connected(struct xe_device *xe) { return false; }

There was a blank line before #endif. Please keep it. Ditto throughout
the series.

>  #endif /* CONFIG_DRM_XE_DISPLAY */
>  #endif /* _XE_DISPLAY_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 3fe673f0f87d..e7aa876ce9e0 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -9,6 +9,7 @@
>  #include <linux/fault-inject.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/suspend.h>
> +#include <linux/vgaarb.h>
>  
>  #include <drm/drm_managed.h>
>  #include <drm/ttm/ttm_placement.h>
> @@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct xe_device *xe)
>  
>  static void xe_pm_vrsr_init(struct xe_device *xe)
>  {
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  	int ret;
>  
>  	if (!xe->info.has_vrsr)
> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
>  	if (!xe_pm_vrsr_capable(xe))
>  		return;
>  
> +	if (pdev != vga_default_device() || !xe_display_connected(xe))

Simply considering the places in the kernel that call
vga_default_device(), this just doesn't feel right.


BR,
Jani.


> +		return;
> +
>  	/*
>  	 * If the VRSR initialization fails, the device will proceed with the regular
>  	 * D3cold flow

-- 
Jani Nikula, Intel

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Rodrigo Vivi]

On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
> > The VRSR feature is to enhance the display screen refresh experience
> > when the device exits from the D3cold state. Therefore, apply the VRSR
> > feature to the default VGA boot device and when a display is connected.
> 
> I don't understand how you get from the 1st sentence "therefore" the 2nd
> sentence. Please elaborate what you're trying to do here, and why.

On a scenario with multiple GPU, only one can use the aux power and the
feature itself was mainly designed for the display case - to bring up
display faster after the d3cold.

But yes, the right explanation for the why needs to be here.

Also, although unlikely, we never know what users can do out there, and
perhaps we will have someone with multiple cards and display plugged in
more than one?! We probably also need a global counter/flag to avoid
a second one to quick in.

But we definitely need to prioritize the first one with display connected.

> 
> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
> xe_display_connected() -> intel_display_connected(), and that's the only
> path and point in time to check whether displays are connected. If not,
> the decision is "not VRSR capable", which is just a weird concusion to
> make. The *capability* does not depend on displays, does it?
> 
> If you boot a device without a display, and then plug in a display, no
> VRSR for you?

yeap, it looks like the check is in the wrong place. It needs to be
checked when going to d3cold...

> 
> More comments inline.
> 
> > v2: Move generic display logic to i915/display (Jani)
> >
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_display.h |  1 +
> >  drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
> >  drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
> >  drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
> >  5 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 81b3a6692ca2..97c74272fb19 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct intel_display *display)
> >  {
> >  	return IS_DISPLAY_VER(display, 6, 11) && intel_display_vtd_active(display);
> >  }
> > +
> > +bool intel_display_connected(struct intel_display *display)
> > +{
> > +	struct drm_connector *list_connector;
> > +	struct drm_connector_list_iter iter;
> > +	bool ret = false;
> > +
> > +	mutex_lock(&display->drm->mode_config.mutex);
> > +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
> > +
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index f8e6e4e82722..20690aa59324 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum port port);
> >  
> >  bool intel_scanout_needs_vtd_wa(struct intel_display *display);
> >  int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
> > +bool intel_display_connected(struct intel_display *display);
> >  
> >  #endif
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> > index f8a831b5dc7d..54ed39b257ad 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
> >  	return intel_display_driver_probe_defer(pdev);
> >  }
> >  
> > +bool xe_display_connected(struct xe_device *xe)
> > +{
> > +	return intel_display_connected(xe->display);
> > +}
> > +
> >  /**
> >   * xe_display_driver_set_hooks - Add driver flags and hooks for display
> >   * @driver: DRM device driver
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> > index 76db95c25f7e..11d4b09808e5 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > @@ -37,6 +37,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
> >  
> > @@ -67,5 +68,6 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> >  static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
> >  static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
> >  
> > +static inline bool xe_display_connected(struct xe_device *xe) { return false; }
> 
> There was a blank line before #endif. Please keep it. Ditto throughout
> the series.
> 
> >  #endif /* CONFIG_DRM_XE_DISPLAY */
> >  #endif /* _XE_DISPLAY_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 3fe673f0f87d..e7aa876ce9e0 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/fault-inject.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/suspend.h>
> > +#include <linux/vgaarb.h>
> >  
> >  #include <drm/drm_managed.h>
> >  #include <drm/ttm/ttm_placement.h>
> > @@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct xe_device *xe)
> >  
> >  static void xe_pm_vrsr_init(struct xe_device *xe)
> >  {
> > +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> >  	int ret;
> >  
> >  	if (!xe->info.has_vrsr)
> > @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
> >  	if (!xe_pm_vrsr_capable(xe))
> >  		return;
> >  
> > +	if (pdev != vga_default_device() || !xe_display_connected(xe))
> 
> Simply considering the places in the kernel that call
> vga_default_device(), this just doesn't feel right.

I also don't understand why to check this vga default device...

> 
> 
> BR,
> Jani.
> 
> 
> > +		return;
> > +
> >  	/*
> >  	 * If the VRSR initialization fails, the device will proceed with the regular
> >  	 * D3cold flow
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Nilawar, Badal]

On 15-01-2026 20:55, Rodrigo Vivi wrote:
> On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
>> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
>>> The VRSR feature is to enhance the display screen refresh experience
>>> when the device exits from the D3cold state. Therefore, apply the VRSR
>>> feature to the default VGA boot device and when a display is connected.
>> I don't understand how you get from the 1st sentence "therefore" the 2nd
>> sentence. Please elaborate what you're trying to do here, and why.
> On a scenario with multiple GPU, only one can use the aux power and the
> feature itself was mainly designed for the display case - to bring up
> display faster after the d3cold.
This is to enhance screen refresh experience of primary display.
>
> But yes, the right explanation for the why needs to be here.
I will rephrase the explanation.
>
> Also, although unlikely, we never know what users can do out there, and
> perhaps we will have someone with multiple cards and display plugged in
> more than one?! We probably also need a global counter/flag to avoid
> a second one to quick in.
>
> But we definitely need to prioritize the first one with display connected.
At present there is no way to know which one is primary display that's 
why check is against default_vga_device.
>
>> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
>> xe_display_connected() -> intel_display_connected(), and that's the only
>> path and point in time to check whether displays are connected. If not,
>> the decision is "not VRSR capable", which is just a weird concusion to
>> make. The *capability* does not depend on displays, does it?
>>
>> If you boot a device without a display, and then plug in a display, no
>> VRSR for you?
> yeap, it looks like the check is in the wrong place. It needs to be
> checked when going to d3cold...

Yes, VRSR will not be enabled if display is not connected at boot.
*capability* does not depend on display but VRSR use case is.

>> More comments inline.
>>
>>> v2: Move generic display logic to i915/display (Jani)
>>>
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>>>   drivers/gpu/drm/i915/display/intel_display.h |  1 +
>>>   drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
>>>   drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
>>>   drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
>>>   5 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 81b3a6692ca2..97c74272fb19 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct intel_display *display)
>>>   {
>>>   	return IS_DISPLAY_VER(display, 6, 11) && intel_display_vtd_active(display);
>>>   }
>>> +
>>> +bool intel_display_connected(struct intel_display *display)
>>> +{
>>> +	struct drm_connector *list_connector;
>>> +	struct drm_connector_list_iter iter;
>>> +	bool ret = false;
>>> +
>>> +	mutex_lock(&display->drm->mode_config.mutex);
>>> +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
>>> +
>>> +	return ret;
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>>> index f8e6e4e82722..20690aa59324 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>>> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum port port);
>>>   
>>>   bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>>>   int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
>>> +bool intel_display_connected(struct intel_display *display);
>>>   
>>>   #endif
>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>>> index f8a831b5dc7d..54ed39b257ad 100644
>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
>>>   	return intel_display_driver_probe_defer(pdev);
>>>   }
>>>   
>>> +bool xe_display_connected(struct xe_device *xe)
>>> +{
>>> +	return intel_display_connected(xe->display);
>>> +}
>>> +
>>>   /**
>>>    * xe_display_driver_set_hooks - Add driver flags and hooks for display
>>>    * @driver: DRM device driver
>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
>>> index 76db95c25f7e..11d4b09808e5 100644
>>> --- a/drivers/gpu/drm/xe/display/xe_display.h
>>> +++ b/drivers/gpu/drm/xe/display/xe_display.h
>>> @@ -37,6 +37,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
>>>   
>>> @@ -67,5 +68,6 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>>>   static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
>>>   static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
>>>   
>>> +static inline bool xe_display_connected(struct xe_device *xe) { return false; }
>> There was a blank line before #endif. Please keep it. Ditto throughout
>> the series.
>>
>>>   #endif /* CONFIG_DRM_XE_DISPLAY */
>>>   #endif /* _XE_DISPLAY_H_ */
>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>>> index 3fe673f0f87d..e7aa876ce9e0 100644
>>> --- a/drivers/gpu/drm/xe/xe_pm.c
>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>>> @@ -9,6 +9,7 @@
>>>   #include <linux/fault-inject.h>
>>>   #include <linux/pm_runtime.h>
>>>   #include <linux/suspend.h>
>>> +#include <linux/vgaarb.h>
>>>   
>>>   #include <drm/drm_managed.h>
>>>   #include <drm/ttm/ttm_placement.h>
>>> @@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct xe_device *xe)
>>>   
>>>   static void xe_pm_vrsr_init(struct xe_device *xe)
>>>   {
>>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>   	int ret;
>>>   
>>>   	if (!xe->info.has_vrsr)
>>> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
>>>   	if (!xe_pm_vrsr_capable(xe))
>>>   		return;
>>>   
>>> +	if (pdev != vga_default_device() || !xe_display_connected(xe))
>> Simply considering the places in the kernel that call
>> vga_default_device(), this just doesn't feel right.
> I also don't understand why to check this vga default device...

As previously mentioned, a check for the default VGA device was added to 
determine if this is the primary display.

Thanks,
Badal

>
>>
>> BR,
>> Jani.
>>
>>
>>> +		return;
>>> +
>>>   	/*
>>>   	 * If the VRSR initialization fails, the device will proceed with the regular
>>>   	 * D3cold flow
>> -- 
>> Jani Nikula, Intel

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Jani Nikula]

On Tue, 20 Jan 2026, "Nilawar, Badal" <badal.nilawar@intel.com> wrote:
> On 15-01-2026 20:55, Rodrigo Vivi wrote:
>> On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
>>> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
>>>> The VRSR feature is to enhance the display screen refresh experience
>>>> when the device exits from the D3cold state. Therefore, apply the VRSR
>>>> feature to the default VGA boot device and when a display is connected.
>>> I don't understand how you get from the 1st sentence "therefore" the 2nd
>>> sentence. Please elaborate what you're trying to do here, and why.
>> On a scenario with multiple GPU, only one can use the aux power and the
>> feature itself was mainly designed for the display case - to bring up
>> display faster after the d3cold.
> This is to enhance screen refresh experience of primary display.

The way it's being added, it's just really oddly specific.

>>
>> But yes, the right explanation for the why needs to be here.
> I will rephrase the explanation.
>>
>> Also, although unlikely, we never know what users can do out there, and
>> perhaps we will have someone with multiple cards and display plugged in
>> more than one?! We probably also need a global counter/flag to avoid
>> a second one to quick in.
>>
>> But we definitely need to prioritize the first one with display connected.
> At present there is no way to know which one is primary display that's 
> why check is against default_vga_device.
>>
>>> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
>>> xe_display_connected() -> intel_display_connected(), and that's the only
>>> path and point in time to check whether displays are connected. If not,
>>> the decision is "not VRSR capable", which is just a weird concusion to
>>> make. The *capability* does not depend on displays, does it?
>>>
>>> If you boot a device without a display, and then plug in a display, no
>>> VRSR for you?
>> yeap, it looks like the check is in the wrong place. It needs to be
>> checked when going to d3cold...
>
> Yes, VRSR will not be enabled if display is not connected at boot.

Why? And this needs to be properly explained in the commit message. The
current one isn't enough.

> *capability* does not depend on display but VRSR use case is.

Please at least don't conflate the two. Don't determine capability based
on whether the conditions on the use case exist.

Contrast with, I don't know, FBC. The platform will still have FBC
capability even if the conditions for enabling it aren't met.

BR,
Jani.


>
>>> More comments inline.
>>>
>>>> v2: Move generic display logic to i915/display (Jani)
>>>>
>>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>>>>   drivers/gpu/drm/i915/display/intel_display.h |  1 +
>>>>   drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
>>>>   drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
>>>>   drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
>>>>   5 files changed, 35 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>>> index 81b3a6692ca2..97c74272fb19 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct intel_display *display)
>>>>   {
>>>>   	return IS_DISPLAY_VER(display, 6, 11) && intel_display_vtd_active(display);
>>>>   }
>>>> +
>>>> +bool intel_display_connected(struct intel_display *display)
>>>> +{
>>>> +	struct drm_connector *list_connector;
>>>> +	struct drm_connector_list_iter iter;
>>>> +	bool ret = false;
>>>> +
>>>> +	mutex_lock(&display->drm->mode_config.mutex);
>>>> +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>>>> index f8e6e4e82722..20690aa59324 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>>>> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum port port);
>>>>   
>>>>   bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>>>>   int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
>>>> +bool intel_display_connected(struct intel_display *display);
>>>>   
>>>>   #endif
>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>>>> index f8a831b5dc7d..54ed39b257ad 100644
>>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>>> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
>>>>   	return intel_display_driver_probe_defer(pdev);
>>>>   }
>>>>   
>>>> +bool xe_display_connected(struct xe_device *xe)
>>>> +{
>>>> +	return intel_display_connected(xe->display);
>>>> +}
>>>> +
>>>>   /**
>>>>    * xe_display_driver_set_hooks - Add driver flags and hooks for display
>>>>    * @driver: DRM device driver
>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
>>>> index 76db95c25f7e..11d4b09808e5 100644
>>>> --- a/drivers/gpu/drm/xe/display/xe_display.h
>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.h
>>>> @@ -37,6 +37,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
>>>>   
>>>> @@ -67,5 +68,6 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>>>>   static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
>>>>   static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
>>>>   
>>>> +static inline bool xe_display_connected(struct xe_device *xe) { return false; }
>>> There was a blank line before #endif. Please keep it. Ditto throughout
>>> the series.
>>>
>>>>   #endif /* CONFIG_DRM_XE_DISPLAY */
>>>>   #endif /* _XE_DISPLAY_H_ */
>>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>>>> index 3fe673f0f87d..e7aa876ce9e0 100644
>>>> --- a/drivers/gpu/drm/xe/xe_pm.c
>>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>>>> @@ -9,6 +9,7 @@
>>>>   #include <linux/fault-inject.h>
>>>>   #include <linux/pm_runtime.h>
>>>>   #include <linux/suspend.h>
>>>> +#include <linux/vgaarb.h>
>>>>   
>>>>   #include <drm/drm_managed.h>
>>>>   #include <drm/ttm/ttm_placement.h>
>>>> @@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct xe_device *xe)
>>>>   
>>>>   static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>   {
>>>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>>   	int ret;
>>>>   
>>>>   	if (!xe->info.has_vrsr)
>>>> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>   	if (!xe_pm_vrsr_capable(xe))
>>>>   		return;
>>>>   
>>>> +	if (pdev != vga_default_device() || !xe_display_connected(xe))
>>> Simply considering the places in the kernel that call
>>> vga_default_device(), this just doesn't feel right.
>> I also don't understand why to check this vga default device...
>
> As previously mentioned, a check for the default VGA device was added to 
> determine if this is the primary display.
>
> Thanks,
> Badal
>
>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>>> +		return;
>>>> +
>>>>   	/*
>>>>   	 * If the VRSR initialization fails, the device will proceed with the regular
>>>>   	 * D3cold flow
>>> -- 
>>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

RE: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Shankar, Uma]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Tuesday, January 20, 2026 7:14 PM
> To: Nilawar, Badal <badal.nilawar@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>
> Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
> pci@vger.kernel.org; Gupta, Anshuman <anshuman.gupta@intel.com>;
> rafael@kernel.org; lenb@kernel.org; bhelgaas@google.com;
> ilpo.jarvinen@linux.intel.com; Gupta, Varun <varun.gupta@intel.com>;
> ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Poosa,
> Karthik <karthik.poosa@intel.com>; Auld, Matthew <matthew.auld@intel.com>;
> Anirban, Sk <sk.anirban@intel.com>; Jadav, Raag <raag.jadav@intel.com>
> Subject: Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot
> device
> 
> On Tue, 20 Jan 2026, "Nilawar, Badal" <badal.nilawar@intel.com> wrote:
> > On 15-01-2026 20:55, Rodrigo Vivi wrote:
> >> On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
> >>> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
> >>>> The VRSR feature is to enhance the display screen refresh
> >>>> experience when the device exits from the D3cold state. Therefore,
> >>>> apply the VRSR feature to the default VGA boot device and when a display
> is connected.
> >>> I don't understand how you get from the 1st sentence "therefore" the
> >>> 2nd sentence. Please elaborate what you're trying to do here, and why.
> >> On a scenario with multiple GPU, only one can use the aux power and
> >> the feature itself was mainly designed for the display case - to
> >> bring up display faster after the d3cold.
> > This is to enhance screen refresh experience of primary display.
> 
> The way it's being added, it's just really oddly specific.
> 
> >>
> >> But yes, the right explanation for the why needs to be here.
> > I will rephrase the explanation.
> >>
> >> Also, although unlikely, we never know what users can do out there,
> >> and perhaps we will have someone with multiple cards and display
> >> plugged in more than one?! We probably also need a global
> >> counter/flag to avoid a second one to quick in.
> >>
> >> But we definitely need to prioritize the first one with display connected.
> > At present there is no way to know which one is primary display that's
> > why check is against default_vga_device.
> >>
> >>> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
> >>> xe_display_connected() -> intel_display_connected(), and that's the
> >>> only path and point in time to check whether displays are connected.
> >>> If not, the decision is "not VRSR capable", which is just a weird
> >>> concusion to make. The *capability* does not depend on displays, does it?
> >>>
> >>> If you boot a device without a display, and then plug in a display,
> >>> no VRSR for you?
> >> yeap, it looks like the check is in the wrong place. It needs to be
> >> checked when going to d3cold...
> >
> > Yes, VRSR will not be enabled if display is not connected at boot.
> 
> Why? And this needs to be properly explained in the commit message. The
> current one isn't enough.
> 
> > *capability* does not depend on display but VRSR use case is.
> 
> Please at least don't conflate the two. Don't determine capability based on whether
> the conditions on the use case exist.
> 
> Contrast with, I don't know, FBC. The platform will still have FBC capability even if
> the conditions for enabling it aren't met.

Yes right, I think display can be plugged later after boot as well. In this case also VRSR should be
enabled.  This can be handled through the display hotplug path and VRSR can be enabled accordingly.

Also elaborate the reasoning so the assumptions, limitations and design choices are clear and
why certain trade offs are made are clarified.

Regards,
Uma Shankar

> BR,
> Jani.
> 
> 
> >
> >>> More comments inline.
> >>>
> >>>> v2: Move generic display logic to i915/display (Jani)
> >>>>
> >>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> >>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >>>> ---
> >>>>   drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
> >>>>   drivers/gpu/drm/i915/display/intel_display.h |  1 +
> >>>>   drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
> >>>>   drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
> >>>>   drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
> >>>>   5 files changed, 35 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >>>> b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> index 81b3a6692ca2..97c74272fb19 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct
> intel_display *display)
> >>>>   {
> >>>>   	return IS_DISPLAY_VER(display, 6, 11) &&
> intel_display_vtd_active(display);
> >>>>   }
> >>>> +
> >>>> +bool intel_display_connected(struct intel_display *display) {
> >>>> +	struct drm_connector *list_connector;
> >>>> +	struct drm_connector_list_iter iter;
> >>>> +	bool ret = false;
> >>>> +
> >>>> +	mutex_lock(&display->drm->mode_config.mutex);
> >>>> +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
> >>>> +
> >>>> +	return ret;
> >>>> +}
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> >>>> b/drivers/gpu/drm/i915/display/intel_display.h
> >>>> index f8e6e4e82722..20690aa59324 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> >>>> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display
> >>>> *display, enum port port);
> >>>>
> >>>>   bool intel_scanout_needs_vtd_wa(struct intel_display *display);
> >>>>   int intel_crtc_num_joined_pipes(const struct intel_crtc_state
> >>>> *crtc_state);
> >>>> +bool intel_display_connected(struct intel_display *display);
> >>>>
> >>>>   #endif
> >>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> >>>> b/drivers/gpu/drm/xe/display/xe_display.c
> >>>> index f8a831b5dc7d..54ed39b257ad 100644
> >>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
> >>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> >>>> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev
> *pdev)
> >>>>   	return intel_display_driver_probe_defer(pdev);
> >>>>   }
> >>>>
> >>>> +bool xe_display_connected(struct xe_device *xe) {
> >>>> +	return intel_display_connected(xe->display);
> >>>> +}
> >>>> +
> >>>>   /**
> >>>>    * xe_display_driver_set_hooks - Add driver flags and hooks for display
> >>>>    * @driver: DRM device driver
> >>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.h
> >>>> b/drivers/gpu/drm/xe/display/xe_display.h
> >>>> index 76db95c25f7e..11d4b09808e5 100644
> >>>> --- a/drivers/gpu/drm/xe/display/xe_display.h
> >>>> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> >>>> @@ -37,6 +37,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
> >>>>
> >>>> @@ -67,5 +68,6 @@ static inline void
> xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> >>>>   static inline void xe_display_pm_runtime_suspend_late(struct xe_device
> *xe) {}
> >>>>   static inline void xe_display_pm_runtime_resume(struct xe_device
> >>>> *xe) {}
> >>>>
> >>>> +static inline bool xe_display_connected(struct xe_device *xe) {
> >>>> +return false; }
> >>> There was a blank line before #endif. Please keep it. Ditto
> >>> throughout the series.
> >>>
> >>>>   #endif /* CONFIG_DRM_XE_DISPLAY */
> >>>>   #endif /* _XE_DISPLAY_H_ */
> >>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c
> >>>> b/drivers/gpu/drm/xe/xe_pm.c index 3fe673f0f87d..e7aa876ce9e0
> >>>> 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_pm.c
> >>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
> >>>> @@ -9,6 +9,7 @@
> >>>>   #include <linux/fault-inject.h>
> >>>>   #include <linux/pm_runtime.h>
> >>>>   #include <linux/suspend.h>
> >>>> +#include <linux/vgaarb.h>
> >>>>
> >>>>   #include <drm/drm_managed.h>
> >>>>   #include <drm/ttm/ttm_placement.h> @@ -387,6 +388,7 @@ static int
> >>>> pci_acpi_aux_power_setup(struct xe_device *xe)
> >>>>
> >>>>   static void xe_pm_vrsr_init(struct xe_device *xe)
> >>>>   {
> >>>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> >>>>   	int ret;
> >>>>
> >>>>   	if (!xe->info.has_vrsr)
> >>>> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
> >>>>   	if (!xe_pm_vrsr_capable(xe))
> >>>>   		return;
> >>>>
> >>>> +	if (pdev != vga_default_device() || !xe_display_connected(xe))
> >>> Simply considering the places in the kernel that call
> >>> vga_default_device(), this just doesn't feel right.
> >> I also don't understand why to check this vga default device...
> >
> > As previously mentioned, a check for the default VGA device was added
> > to determine if this is the primary display.
> >
> > Thanks,
> > Badal
> >
> >>
> >>>
> >>> BR,
> >>> Jani.
> >>>
> >>>
> >>>> +		return;
> >>>> +
> >>>>   	/*
> >>>>   	 * If the VRSR initialization fails, the device will proceed with the regular
> >>>>   	 * D3cold flow
> >>> --
> >>> Jani Nikula, Intel
> 
> --
> Jani Nikula, Intel

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Nilawar, Badal]

On 20-01-2026 20:12, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: Tuesday, January 20, 2026 7:14 PM
>> To: Nilawar, Badal <badal.nilawar@intel.com>; Vivi, Rodrigo
>> <rodrigo.vivi@intel.com>
>> Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
>> pci@vger.kernel.org; Gupta, Anshuman <anshuman.gupta@intel.com>;
>> rafael@kernel.org; lenb@kernel.org; bhelgaas@google.com;
>> ilpo.jarvinen@linux.intel.com; Gupta, Varun <varun.gupta@intel.com>;
>> ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Poosa,
>> Karthik <karthik.poosa@intel.com>; Auld, Matthew <matthew.auld@intel.com>;
>> Anirban, Sk <sk.anirban@intel.com>; Jadav, Raag <raag.jadav@intel.com>
>> Subject: Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot
>> device
>>
>> On Tue, 20 Jan 2026, "Nilawar, Badal" <badal.nilawar@intel.com> wrote:
>>> On 15-01-2026 20:55, Rodrigo Vivi wrote:
>>>> On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
>>>>> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
>>>>>> The VRSR feature is to enhance the display screen refresh
>>>>>> experience when the device exits from the D3cold state. Therefore,
>>>>>> apply the VRSR feature to the default VGA boot device and when a display
>> is connected.
>>>>> I don't understand how you get from the 1st sentence "therefore" the
>>>>> 2nd sentence. Please elaborate what you're trying to do here, and why.
>>>> On a scenario with multiple GPU, only one can use the aux power and
>>>> the feature itself was mainly designed for the display case - to
>>>> bring up display faster after the d3cold.
>>> This is to enhance screen refresh experience of primary display.
>> The way it's being added, it's just really oddly specific.
>>
>>>> But yes, the right explanation for the why needs to be here.
>>> I will rephrase the explanation.
>>>> Also, although unlikely, we never know what users can do out there,
>>>> and perhaps we will have someone with multiple cards and display
>>>> plugged in more than one?! We probably also need a global
>>>> counter/flag to avoid a second one to quick in.
>>>>
>>>> But we definitely need to prioritize the first one with display connected.
>>> At present there is no way to know which one is primary display that's
>>> why check is against default_vga_device.
>>>>> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
>>>>> xe_display_connected() -> intel_display_connected(), and that's the
>>>>> only path and point in time to check whether displays are connected.
>>>>> If not, the decision is "not VRSR capable", which is just a weird
>>>>> concusion to make. The *capability* does not depend on displays, does it?
>>>>>
>>>>> If you boot a device without a display, and then plug in a display,
>>>>> no VRSR for you?
>>>> yeap, it looks like the check is in the wrong place. It needs to be
>>>> checked when going to d3cold...
>>> Yes, VRSR will not be enabled if display is not connected at boot.
>> Why? And this needs to be properly explained in the commit message. The
>> current one isn't enough.
>>
>>> *capability* does not depend on display but VRSR use case is.
>> Please at least don't conflate the two. Don't determine capability based on whether
>> the conditions on the use case exist.
>>
>> Contrast with, I don't know, FBC. The platform will still have FBC capability even if
>> the conditions for enabling it aren't met.
> Yes right, I think display can be plugged later after boot as well. In this case also VRSR should be
> enabled.  This can be handled through the display hotplug path and VRSR can be enabled accordingly.
Sure will add handling in display hotplug path.
>
> Also elaborate the reasoning so the assumptions, limitations and design choices are clear and
> why certain trade offs are made are clarified.

Sure.

Thanks,
Badal

>
> Regards,
> Uma Shankar
>
>> BR,
>> Jani.
>>
>>
>>>>> More comments inline.
>>>>>
>>>>>> v2: Move generic display logic to i915/display (Jani)
>>>>>>
>>>>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>>>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>>>>>>    drivers/gpu/drm/i915/display/intel_display.h |  1 +
>>>>>>    drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
>>>>>>    drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
>>>>>>    drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
>>>>>>    5 files changed, 35 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> index 81b3a6692ca2..97c74272fb19 100644
>>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct
>> intel_display *display)
>>>>>>    {
>>>>>>    	return IS_DISPLAY_VER(display, 6, 11) &&
>> intel_display_vtd_active(display);
>>>>>>    }
>>>>>> +
>>>>>> +bool intel_display_connected(struct intel_display *display) {
>>>>>> +	struct drm_connector *list_connector;
>>>>>> +	struct drm_connector_list_iter iter;
>>>>>> +	bool ret = false;
>>>>>> +
>>>>>> +	mutex_lock(&display->drm->mode_config.mutex);
>>>>>> +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
>>>>>> +
>>>>>> +	return ret;
>>>>>> +}
>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> b/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> index f8e6e4e82722..20690aa59324 100644
>>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display
>>>>>> *display, enum port port);
>>>>>>
>>>>>>    bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>>>>>>    int intel_crtc_num_joined_pipes(const struct intel_crtc_state
>>>>>> *crtc_state);
>>>>>> +bool intel_display_connected(struct intel_display *display);
>>>>>>
>>>>>>    #endif
>>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> b/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> index f8a831b5dc7d..54ed39b257ad 100644
>>>>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev
>> *pdev)
>>>>>>    	return intel_display_driver_probe_defer(pdev);
>>>>>>    }
>>>>>>
>>>>>> +bool xe_display_connected(struct xe_device *xe) {
>>>>>> +	return intel_display_connected(xe->display);
>>>>>> +}
>>>>>> +
>>>>>>    /**
>>>>>>     * xe_display_driver_set_hooks - Add driver flags and hooks for display
>>>>>>     * @driver: DRM device driver
>>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> b/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> index 76db95c25f7e..11d4b09808e5 100644
>>>>>> --- a/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> @@ -37,6 +37,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
>>>>>>
>>>>>> @@ -67,5 +68,6 @@ static inline void
>> xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>>>>>>    static inline void xe_display_pm_runtime_suspend_late(struct xe_device
>> *xe) {}
>>>>>>    static inline void xe_display_pm_runtime_resume(struct xe_device
>>>>>> *xe) {}
>>>>>>
>>>>>> +static inline bool xe_display_connected(struct xe_device *xe) {
>>>>>> +return false; }
>>>>> There was a blank line before #endif. Please keep it. Ditto
>>>>> throughout the series.
>>>>>
>>>>>>    #endif /* CONFIG_DRM_XE_DISPLAY */
>>>>>>    #endif /* _XE_DISPLAY_H_ */
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c
>>>>>> b/drivers/gpu/drm/xe/xe_pm.c index 3fe673f0f87d..e7aa876ce9e0
>>>>>> 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_pm.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>>>>>> @@ -9,6 +9,7 @@
>>>>>>    #include <linux/fault-inject.h>
>>>>>>    #include <linux/pm_runtime.h>
>>>>>>    #include <linux/suspend.h>
>>>>>> +#include <linux/vgaarb.h>
>>>>>>
>>>>>>    #include <drm/drm_managed.h>
>>>>>>    #include <drm/ttm/ttm_placement.h> @@ -387,6 +388,7 @@ static int
>>>>>> pci_acpi_aux_power_setup(struct xe_device *xe)
>>>>>>
>>>>>>    static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>>>    {
>>>>>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>>>>    	int ret;
>>>>>>
>>>>>>    	if (!xe->info.has_vrsr)
>>>>>> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>>>    	if (!xe_pm_vrsr_capable(xe))
>>>>>>    		return;
>>>>>>
>>>>>> +	if (pdev != vga_default_device() || !xe_display_connected(xe))
>>>>> Simply considering the places in the kernel that call
>>>>> vga_default_device(), this just doesn't feel right.
>>>> I also don't understand why to check this vga default device...
>>> As previously mentioned, a check for the default VGA device was added
>>> to determine if this is the primary display.
>>>
>>> Thanks,
>>> Badal
>>>
>>>>> BR,
>>>>> Jani.
>>>>>
>>>>>
>>>>>> +		return;
>>>>>> +
>>>>>>    	/*
>>>>>>    	 * If the VRSR initialization fails, the device will proceed with the regular
>>>>>>    	 * D3cold flow
>>>>> --
>>>>> Jani Nikula, Intel
>> --
>> Jani Nikula, Intel

Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

[Vivi, Rodrigo]

On Tue, 2026-01-20 at 18:58 +0530, Nilawar, Badal wrote:
> 
> On 15-01-2026 20:55, Rodrigo Vivi wrote:
> > On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
> > > On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com>
> > > wrote:
> > > > The VRSR feature is to enhance the display screen refresh
> > > > experience
> > > > when the device exits from the D3cold state. Therefore, apply
> > > > the VRSR
> > > > feature to the default VGA boot device and when a display is
> > > > connected.
> > > I don't understand how you get from the 1st sentence "therefore"
> > > the 2nd
> > > sentence. Please elaborate what you're trying to do here, and
> > > why.
> > On a scenario with multiple GPU, only one can use the aux power and
> > the
> > feature itself was mainly designed for the display case - to bring
> > up
> > display faster after the d3cold.
> This is to enhance screen refresh experience of primary display.
> > 
> > But yes, the right explanation for the why needs to be here.
> I will rephrase the explanation.
> > 
> > Also, although unlikely, we never know what users can do out there,
> > and
> > perhaps we will have someone with multiple cards and display
> > plugged in
> > more than one?! We probably also need a global counter/flag to
> > avoid
> > a second one to quick in.
> > 
> > But we definitely need to prioritize the first one with display
> > connected.
> At present there is no way to know which one is primary display
> that's 
> why check is against default_vga_device.

This is not my point.

In case we have 2 different devices with display connected on both
devices, we cannot enable VRSR on both of them... But this approach
here is enabling on both of them. And this will break things.

Although we cannot know which one is the primary display, we should
be able to only enable VRSR in a single device or never. Hence you
do need to have a global flag.

> > 
> > > So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
> > > xe_display_connected() -> intel_display_connected(), and that's
> > > the only
> > > path and point in time to check whether displays are connected.
> > > If not,
> > > the decision is "not VRSR capable", which is just a weird
> > > concusion to
> > > make. The *capability* does not depend on displays, does it?
> > > 
> > > If you boot a device without a display, and then plug in a
> > > display, no
> > > VRSR for you?
> > yeap, it looks like the check is in the wrong place. It needs to be
> > checked when going to d3cold...
> 
> Yes, VRSR will not be enabled if display is not connected at boot.
> *capability* does not depend on display but VRSR use case is.

This looks wrong design. I agree with Jani and Uma.

> 
> > > More comments inline.
> > > 
> > > > v2: Move generic display logic to i915/display (Jani)
> > > > 
> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/display/intel_display.c | 22
> > > > ++++++++++++++++++++
> > > >   drivers/gpu/drm/i915/display/intel_display.h |  1 +
> > > >   drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
> > > >   drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
> > > >   drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
> > > >   5 files changed, 35 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 81b3a6692ca2..97c74272fb19 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct
> > > > intel_display *display)
> > > >   {
> > > >   	return IS_DISPLAY_VER(display, 6, 11) &&
> > > > intel_display_vtd_active(display);
> > > >   }
> > > > +
> > > > +bool intel_display_connected(struct intel_display *display)
> > > > +{
> > > > +	struct drm_connector *list_connector;
> > > > +	struct drm_connector_list_iter iter;
> > > > +	bool ret = false;
> > > > +
> > > > +	mutex_lock(&display->drm->mode_config.mutex);
> > > > +	drm_connector_list_iter_begin(display->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(&display->drm->mode_config.mutex);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> > > > b/drivers/gpu/drm/i915/display/intel_display.h
> > > > index f8e6e4e82722..20690aa59324 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > > @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display
> > > > *display, enum port port);
> > > >   
> > > >   bool intel_scanout_needs_vtd_wa(struct intel_display
> > > > *display);
> > > >   int intel_crtc_num_joined_pipes(const struct intel_crtc_state
> > > > *crtc_state);
> > > > +bool intel_display_connected(struct intel_display *display);
> > > >   
> > > >   #endif
> > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > > > b/drivers/gpu/drm/xe/display/xe_display.c
> > > > index f8a831b5dc7d..54ed39b257ad 100644
> > > > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > > > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > > > @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct
> > > > pci_dev *pdev)
> > > >   	return intel_display_driver_probe_defer(pdev);
> > > >   }
> > > >   
> > > > +bool xe_display_connected(struct xe_device *xe)
> > > > +{
> > > > +	return intel_display_connected(xe->display);
> > > > +}
> > > > +
> > > >   /**
> > > >    * xe_display_driver_set_hooks - Add driver flags and hooks
> > > > for display
> > > >    * @driver: DRM device driver
> > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.h
> > > > b/drivers/gpu/drm/xe/display/xe_display.h
> > > > index 76db95c25f7e..11d4b09808e5 100644
> > > > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > > > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > > > @@ -37,6 +37,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
> > > >   
> > > > @@ -67,5 +68,6 @@ static inline void
> > > > xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> > > >   static inline void xe_display_pm_runtime_suspend_late(struct
> > > > xe_device *xe) {}
> > > >   static inline void xe_display_pm_runtime_resume(struct
> > > > xe_device *xe) {}
> > > >   
> > > > +static inline bool xe_display_connected(struct xe_device *xe)
> > > > { return false; }
> > > There was a blank line before #endif. Please keep it. Ditto
> > > throughout
> > > the series.
> > > 
> > > >   #endif /* CONFIG_DRM_XE_DISPLAY */
> > > >   #endif /* _XE_DISPLAY_H_ */
> > > > diff --git a/drivers/gpu/drm/xe/xe_pm.c
> > > > b/drivers/gpu/drm/xe/xe_pm.c
> > > > index 3fe673f0f87d..e7aa876ce9e0 100644
> > > > --- a/drivers/gpu/drm/xe/xe_pm.c
> > > > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > > > @@ -9,6 +9,7 @@
> > > >   #include <linux/fault-inject.h>
> > > >   #include <linux/pm_runtime.h>
> > > >   #include <linux/suspend.h>
> > > > +#include <linux/vgaarb.h>
> > > >   
> > > >   #include <drm/drm_managed.h>
> > > >   #include <drm/ttm/ttm_placement.h>
> > > > @@ -387,6 +388,7 @@ static int pci_acpi_aux_power_setup(struct
> > > > xe_device *xe)
> > > >   
> > > >   static void xe_pm_vrsr_init(struct xe_device *xe)
> > > >   {
> > > > +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> > > >   	int ret;
> > > >   
> > > >   	if (!xe->info.has_vrsr)
> > > > @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct
> > > > xe_device *xe)
> > > >   	if (!xe_pm_vrsr_capable(xe))
> > > >   		return;
> > > >   
> > > > +	if (pdev != vga_default_device() ||
> > > > !xe_display_connected(xe))
> > > Simply considering the places in the kernel that call
> > > vga_default_device(), this just doesn't feel right.
> > I also don't understand why to check this vga default device...
> 
> As previously mentioned, a check for the default VGA device was added
> to 
> determine if this is the primary display.

I believe this choice is fragile at best.

> 
> Thanks,
> Badal
> 
> > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > 
> > > > +		return;
> > > > +
> > > >   	/*
> > > >   	 * If the VRSR initialization fails, the device will
> > > > proceed with the regular
> > > >   	 * D3cold flow
> > > -- 
> > > Jani Nikula, Intel

[PATCH v6 07/12] drm/xe/vrsr: Refactor d3cold.allowed to a enum

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

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

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Reviewed-by: Karthik Poosa <karthik.poosa@intel.com>
---
 drivers/gpu/drm/xe/display/xe_display.c |  6 +++---
 drivers/gpu/drm/xe/xe_device_types.h    |  5 +++--
 drivers/gpu/drm/xe/xe_pci.c             |  6 +++---
 drivers/gpu/drm/xe/xe_pm.c              | 24 ++++++++++++------------
 drivers/gpu/drm/xe/xe_pm.h              |  8 +++++++-
 5 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 54ed39b257ad..b664bf58028a 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -401,7 +401,7 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe)
 	if (!xe->info.probe_display)
 		return;
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		xe_display_enable_d3cold(xe);
 		return;
 	}
@@ -427,7 +427,7 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
 	if (!xe->info.probe_display)
 		return;
 
-	if (xe->d3cold.allowed)
+	if (xe->d3cold.target_state)
 		xe_display_pm_suspend_late(xe);
 
 	/*
@@ -507,7 +507,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe)
 	if (!xe->info.probe_display)
 		return;
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		xe_display_disable_d3cold(xe);
 		return;
 	}
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 3b42400f8563..7dc8c0e15aaa 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -22,6 +22,7 @@
 #include "xe_oa_types.h"
 #include "xe_pagefault_types.h"
 #include "xe_platform_types.h"
+#include "xe_pm.h"
 #include "xe_pmu_types.h"
 #include "xe_pt_types.h"
 #include "xe_sriov_pf_types.h"
@@ -574,8 +575,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.target_state: Indicates d3cold target state */
+		enum xe_d3_state target_state;
 
 		/** @d3cold.vrsr_capable: Indicates if d3cold VRAM Self Refresh is supported */
 		bool vrsr_capable;
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 5c14e1978a72..e1de6b337f1f 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1201,7 +1201,7 @@ static int xe_pci_runtime_suspend(struct device *dev)
 
 	pci_save_state(pdev);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		d3cold_toggle(pdev, D3COLD_ENABLE);
 		pci_disable_device(pdev);
 		pci_ignore_hotplug(pdev);
@@ -1226,7 +1226,7 @@ static int xe_pci_runtime_resume(struct device *dev)
 
 	pci_restore_state(pdev);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		err = pci_enable_device(pdev);
 		if (err)
 			return err;
@@ -1242,7 +1242,7 @@ static int xe_pci_runtime_idle(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct xe_device *xe = pdev_to_xe_device(pdev);
 
-	xe_pm_d3cold_allowed_toggle(xe);
+	xe_pm_d3cold_target_state_toggle(xe);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index e7aa876ce9e0..ef7a686eb014 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -728,14 +728,14 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
 
 	xe_display_pm_runtime_suspend(xe);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		err = xe_bo_evict_all(xe);
 		if (err)
 			goto out_resume;
 	}
 
 	for_each_gt(gt, xe, id) {
-		err = xe->d3cold.allowed ? xe_gt_suspend(gt) : xe_gt_runtime_suspend(gt);
+		err = xe->d3cold.target_state ? xe_gt_suspend(gt) : xe_gt_runtime_suspend(gt);
 		if (err)
 			goto out_resume;
 	}
@@ -777,7 +777,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 
 	xe_rpm_lockmap_acquire(xe);
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		for_each_gt(gt, xe, id)
 			xe_gt_idle_disable_c6(gt);
 
@@ -796,12 +796,12 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 			goto out;
 	}
 
-	xe_i2c_pm_resume(xe, xe->d3cold.allowed);
+	xe_i2c_pm_resume(xe, xe->d3cold.target_state != XE_D3HOT);
 
 	xe_irq_resume(xe);
 
 	for_each_gt(gt, xe, id) {
-		err = xe->d3cold.allowed ? xe_gt_resume(gt) : xe_gt_runtime_resume(gt);
+		err = xe->d3cold.target_state ? xe_gt_resume(gt) : xe_gt_runtime_resume(gt);
 		if (err)
 			break;
 	}
@@ -814,7 +814,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 	if (err)
 		goto out;
 
-	if (xe->d3cold.allowed) {
+	if (xe->d3cold.target_state) {
 		err = xe_bo_restore_late(xe);
 		if (err)
 			goto out;
@@ -825,7 +825,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 	if (IS_VF_CCS_READY(xe))
 		xe_sriov_vf_ccs_register_context(xe);
 
-	if (xe->d3cold.allowed)
+	if (xe->d3cold.target_state != XE_D3HOT)
 		xe_late_bind_fw_load(&xe->late_bind);
 
 out:
@@ -1085,13 +1085,13 @@ int xe_pm_set_vram_threshold(struct xe_device *xe, u32 threshold)
 }
 
 /**
- * xe_pm_d3cold_allowed_toggle - Check conditions to toggle d3cold.allowed
+ * xe_pm_d3cold_target_state_toggle - Check conditions to toggle target_state
  * @xe: xe device instance
  *
  * To be called during runtime_pm idle callback.
  * Check for all the D3Cold conditions ahead of runtime suspend.
  */
-void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
+void xe_pm_d3cold_target_state_toggle(struct xe_device *xe)
 {
 	struct ttm_resource_manager *man;
 	u32 total_vram_used_mb = 0;
@@ -1099,7 +1099,7 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
 	int i;
 
 	if (!xe->d3cold.capable) {
-		xe->d3cold.allowed = false;
+		xe->d3cold.target_state = XE_D3HOT;
 		return;
 	}
 
@@ -1114,9 +1114,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.target_state = XE_D3COLD_OFF;
 	else
-		xe->d3cold.allowed = false;
+		xe->d3cold.target_state = 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 8c7fe8e4241d..cd7b4a9b2c05 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -13,6 +13,12 @@
 
 struct xe_device;
 
+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);
 
@@ -31,7 +37,7 @@ void xe_pm_runtime_get_noresume(struct xe_device *xe);
 bool xe_pm_runtime_resume_and_get(struct xe_device *xe);
 void xe_pm_assert_unbounded_bridge(struct xe_device *xe);
 int xe_pm_set_vram_threshold(struct xe_device *xe, u32 threshold);
-void xe_pm_d3cold_allowed_toggle(struct xe_device *xe);
+void xe_pm_d3cold_target_state_toggle(struct xe_device *xe);
 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_block_on_suspend(struct xe_device *xe);
-- 
2.52.0

[PATCH v6 08/12] drm/xe/pm: D3cold target state

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

Trade-off D3cold target state based upon current vram usage.
If vram usage is greater than vram_d3cold_threshold and GPU
is VRSR capable target D3cold state is D3cold-VRSR
otherwise target state is D3cold-Off.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Reviewed-by: Karthik Poosa <karthik.poosa@intel.com>
---
 drivers/gpu/drm/xe/xe_pm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index ef7a686eb014..1a1edbfcf240 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -167,6 +167,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_target_state_toggle(xe);
+	else
+		xe->d3cold.target_state = XE_D3COLD_OFF;
+}
+
 /**
  * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
  * @xe: xe device instance
@@ -183,6 +191,8 @@ int xe_pm_suspend(struct xe_device *xe)
 	xe_pm_block_begin_signalling();
 	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;
@@ -1115,10 +1125,14 @@ void xe_pm_d3cold_target_state_toggle(struct xe_device *xe)
 
 	if (total_vram_used_mb < xe->d3cold.vram_threshold)
 		xe->d3cold.target_state = XE_D3COLD_OFF;
+	else if (xe->d3cold.vrsr_capable)
+		xe->d3cold.target_state = XE_D3COLD_VRSR;
 	else
 		xe->d3cold.target_state = XE_D3HOT;
 
 	mutex_unlock(&xe->d3cold.lock);
+
+	drm_dbg(&xe->drm, "Xe D3cold target state %d\n", xe->d3cold.target_state);
 }
 
 /**
-- 
2.52.0

[PATCH v6 09/12] drm/xe/pm: Refactor PM Sleep Ops

[Badal Nilawar]

Refactor PM Sleep OPS to indicate xe_pm_suspend/resume is called
during hibernation (S4) or suspend (S3/S2idle).

Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c | 64 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_pm.c  | 10 +++---
 drivers/gpu/drm/xe/xe_pm.h  |  4 +--
 3 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index e1de6b337f1f..f1ec6aa26faa 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1135,7 +1135,7 @@ static int xe_pci_suspend(struct device *dev)
 	if (xe_survivability_mode_is_boot_enabled(xe))
 		return -EBUSY;
 
-	err = xe_pm_suspend(xe);
+	err = xe_pm_suspend(xe, false);
 	if (err)
 		return err;
 
@@ -1173,13 +1173,66 @@ static int xe_pci_resume(struct device *dev)
 
 	pci_set_master(pdev);
 
-	err = xe_pm_resume(pdev_to_xe_device(pdev));
+	err = xe_pm_resume(pdev_to_xe_device(pdev), false);
 	if (err)
 		return err;
 
 	return 0;
 }
 
+static int xe_pci_freeze(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct xe_device *xe = pdev_to_xe_device(pdev);
+	int err;
+
+	if (xe_survivability_mode_is_boot_enabled(xe))
+		return -EBUSY;
+
+	err = xe_pm_suspend(xe, true);
+	if (err)
+		return err;
+
+	/*
+	 * Enabling D3Cold is needed for S2Idle/S0ix.
+	 * It is save to allow here since xe_pm_suspend has evicted
+	 * the local memory and the direct complete optimization is disabled.
+	 */
+	d3cold_toggle(pdev, D3COLD_ENABLE);
+
+	pci_save_state(pdev);
+	pci_disable_device(pdev);
+	pci_set_power_state(pdev, PCI_D3cold);
+
+	return 0;
+}
+
+static int xe_pci_thaw(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int err;
+
+	/* Give back the D3Cold decision to the runtime P M*/
+	d3cold_toggle(pdev, D3COLD_DISABLE);
+
+	err = pci_set_power_state(pdev, PCI_D0);
+	if (err)
+		return err;
+
+	pci_restore_state(pdev);
+
+	err = pci_enable_device(pdev);
+	if (err)
+		return err;
+
+	pci_set_master(pdev);
+
+	err = xe_pm_resume(pdev_to_xe_device(pdev), true);
+	if (err)
+		return err;
+
+	return 0;
+}
 static int xe_pci_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -1248,7 +1301,12 @@ static int xe_pci_runtime_idle(struct device *dev)
 }
 
 static const struct dev_pm_ops xe_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(xe_pci_suspend, xe_pci_resume)
+	.suspend = xe_pci_suspend,
+	.resume = xe_pci_resume,
+	.freeze = xe_pci_freeze,
+	.thaw = xe_pci_thaw,
+	.poweroff = xe_pci_freeze,
+	.restore = xe_pci_thaw,
 	SET_RUNTIME_PM_OPS(xe_pci_runtime_suspend, xe_pci_runtime_resume, xe_pci_runtime_idle)
 };
 #endif
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 1a1edbfcf240..2cbcb9de7586 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -178,16 +178,17 @@ static void xe_pm_suspend_prepare(struct xe_device *xe)
 /**
  * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
  * @xe: xe device instance
+ * @hibernation: whether the suspend is for hibernation
  *
  * Return: 0 on success
  */
-int xe_pm_suspend(struct xe_device *xe)
+int xe_pm_suspend(struct xe_device *xe, bool hibernation)
 {
 	struct xe_gt *gt;
 	u8 id;
 	int err;
 
-	drm_dbg(&xe->drm, "Suspending device\n");
+	drm_dbg(&xe->drm, "Suspending device for %s\n", hibernation ? "hibernation" : "S3/S2idle");
 	xe_pm_block_begin_signalling();
 	trace_xe_pm_suspend(xe, __builtin_return_address(0));
 
@@ -238,10 +239,11 @@ int xe_pm_suspend(struct xe_device *xe)
 /**
  * xe_pm_resume - Helper for System resume S3->S0 / S2idle->S0
  * @xe: xe device instance
+ * @hibernation: whether the resume is from hibernation
  *
  * Return: 0 on success
  */
-int xe_pm_resume(struct xe_device *xe)
+int xe_pm_resume(struct xe_device *xe, bool hibernation)
 {
 	struct xe_tile *tile;
 	struct xe_gt *gt;
@@ -249,7 +251,7 @@ int xe_pm_resume(struct xe_device *xe)
 	int err;
 
 	xe_pm_block_begin_signalling();
-	drm_dbg(&xe->drm, "Resuming device\n");
+	drm_dbg(&xe->drm, "Resuming device from %s\n", hibernation ? "hibernation" : "S3/S2idle");
 	trace_xe_pm_resume(xe, __builtin_return_address(0));
 
 	for_each_gt(gt, xe, id)
diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
index cd7b4a9b2c05..973475f2f7dd 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -19,8 +19,8 @@ enum xe_d3_state {
 	XE_D3COLD_OFF,
 };
 
-int xe_pm_suspend(struct xe_device *xe);
-int xe_pm_resume(struct xe_device *xe);
+int xe_pm_suspend(struct xe_device *xe, bool hibernation);
+int xe_pm_resume(struct xe_device *xe, bool hibernation);
 
 int xe_pm_init_early(struct xe_device *xe);
 int xe_pm_init(struct xe_device *xe);
-- 
2.52.0

Re: [PATCH v6 09/12] drm/xe/pm: Refactor PM Sleep Ops

[Bjorn Helgaas]

On Tue, Jan 13, 2026 at 10:12:10PM +0530, Badal Nilawar wrote:
> Refactor PM Sleep OPS to indicate xe_pm_suspend/resume is called
> during hibernation (S4) or suspend (S3/S2idle).

> +	 * Enabling D3Cold is needed for S2Idle/S0ix.
> +	 * It is save to allow here since xe_pm_suspend has evicted
> +	 * the local memory and the direct complete optimization is disabled.

s/save/safe/ ?

> +	/* Give back the D3Cold decision to the runtime P M*/

s/P M/PM/

Re: [PATCH v6 09/12] drm/xe/pm: Refactor PM Sleep Ops

[Nilawar, Badal]

On 14-01-2026 23:30, Bjorn Helgaas wrote:
> On Tue, Jan 13, 2026 at 10:12:10PM +0530, Badal Nilawar wrote:
>> Refactor PM Sleep OPS to indicate xe_pm_suspend/resume is called
>> during hibernation (S4) or suspend (S3/S2idle).
>> +	 * Enabling D3Cold is needed for S2Idle/S0ix.
>> +	 * It is save to allow here since xe_pm_suspend has evicted
>> +	 * the local memory and the direct complete optimization is disabled.
> s/save/safe/ ?
>
>> +	/* Give back the D3Cold decision to the runtime P M*/
> s/P M/PM/
Sure I will fix above comments.

Thanks,
Badal

[PATCH v6 10/12] drm/xe/vrsr: Enable VRSR

[Badal Nilawar]

From: Anshuman Gupta <anshuman.gupta@intel.com>

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

V2: Disable VRSR in runtime/system resume path
V3: Handle BO eviction

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Acked-by: Karthik Poosa <karthik.poosa@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c |  4 +--
 drivers/gpu/drm/xe/xe_pm.c  | 52 ++++++++++++++++++++++++++++---------
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f1ec6aa26faa..8fc650b2de02 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1141,7 +1141,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);
@@ -1158,7 +1158,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 2cbcb9de7586..1dd8e2c0f51e 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -205,10 +205,12 @@ int xe_pm_suspend(struct xe_device *xe, bool hibernation)
 
 	xe_display_pm_suspend(xe);
 
-	/* FIXME: Super racey... */
-	err = xe_bo_evict_all(xe);
-	if (err)
-		goto err_display;
+	if (hibernation || xe->d3cold.target_state != XE_D3COLD_VRSR) {
+		/* FIXME: Super racey... */
+		err = xe_bo_evict_all(xe);
+		if (err)
+			goto err;
+	}
 
 	for_each_gt(gt, xe, id) {
 		err = xe_gt_suspend(gt);
@@ -222,6 +224,12 @@ int xe_pm_suspend(struct xe_device *xe, bool hibernation)
 
 	xe_i2c_pm_suspend(xe);
 
+	if (!hibernation && xe->d3cold.target_state == XE_D3COLD_VRSR) {
+		err = xe_pm_vrsr_enable(xe);
+		if (err)
+			goto err_display;
+	}
+
 	drm_dbg(&xe->drm, "Device suspended\n");
 	xe_pm_block_end_signalling();
 
@@ -264,15 +272,20 @@ int xe_pm_resume(struct xe_device *xe, bool hibernation)
 	if (err)
 		return err;
 
+	if (!hibernation && xe->d3cold.target_state == XE_D3COLD_VRSR)
+		xe_pm_vrsr_disable(xe);
+
 	xe_display_pm_resume_early(xe);
 
 	/*
 	 * This only restores pinned memory which is the memory required for the
 	 * GT(s) to resume.
 	 */
-	err = xe_bo_restore_early(xe);
-	if (err)
-		goto err;
+	if (hibernation || xe->d3cold.target_state != XE_D3COLD_VRSR) {
+		err = xe_bo_restore_early(xe);
+		if (err)
+			goto err;
+	}
 
 	xe_i2c_pm_resume(xe, true);
 
@@ -292,9 +305,11 @@ int xe_pm_resume(struct xe_device *xe, bool hibernation)
 	if (err)
 		goto err;
 
-	err = xe_bo_restore_late(xe);
-	if (err)
-		goto err;
+	if (hibernation || xe->d3cold.target_state != XE_D3COLD_VRSR) {
+		err = xe_bo_restore_late(xe);
+		if (err)
+			goto err;
+	}
 
 	xe_pxp_pm_resume(xe->pxp);
 
@@ -740,7 +755,7 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
 
 	xe_display_pm_runtime_suspend(xe);
 
-	if (xe->d3cold.target_state) {
+	if (xe->d3cold.target_state == XE_D3COLD_OFF) {
 		err = xe_bo_evict_all(xe);
 		if (err)
 			goto out_resume;
@@ -758,6 +773,14 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
 
 	xe_i2c_pm_suspend(xe);
 
+	if (xe->d3cold.target_state == XE_D3COLD_VRSR) {
+		err = xe_pm_vrsr_enable(xe);
+		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;
@@ -789,6 +812,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 
 	xe_rpm_lockmap_acquire(xe);
 
+	if (xe->d3cold.target_state == XE_D3COLD_VRSR)
+		xe_pm_vrsr_disable(xe);
+
 	if (xe->d3cold.target_state) {
 		for_each_gt(gt, xe, id)
 			xe_gt_idle_disable_c6(gt);
@@ -798,7 +824,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 			goto out;
 
 		xe_display_pm_resume_early(xe);
+	}
 
+	if (xe->d3cold.target_state == XE_D3COLD_OFF) {
 		/*
 		 * This only restores pinned memory which is the memory
 		 * required for the GT(s) to resume.
@@ -826,7 +854,7 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 	if (err)
 		goto out;
 
-	if (xe->d3cold.target_state) {
+	if (xe->d3cold.target_state == XE_D3COLD_OFF) {
 		err = xe_bo_restore_late(xe);
 		if (err)
 			goto out;
-- 
2.52.0

Re: [PATCH v6 10/12] drm/xe/vrsr: Enable VRSR

[Bjorn Helgaas]

On Tue, Jan 13, 2026 at 10:12:11PM +0530, Badal Nilawar wrote:
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Enable VRSR in runtime suspend and also in System wide suspend.
> Also fix couple of typo in xe_pm.c.

>  	 * 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);
> @@ -1158,7 +1158,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 */

Hahahaha, just change the previous patch so these typos are never
introduced :)

[PATCH v6 11/12] drm/xe/pm/s2idle: Don't evict user BOs D3cold-VRSR state

[Badal Nilawar]

Since VRAM stays active in D3cold-VRSR, evicting user BOs is unnecessary.

Cc: Matthew Auld <matthew.auld@intel.com>
Co-developed-by: Sk Anirban <sk.anirban@intel.com>
Signed-off-by: Sk Anirban <sk.anirban@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
V1: Removed D3hot handling since, on discrete cards, we do not block
    D3cold during system suspend
---
 drivers/gpu/drm/xe/xe_pm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 1dd8e2c0f51e..3c7983afca9c 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -169,7 +169,7 @@ static void xe_rpm_lockmap_release(const struct xe_device *xe)
 
 static void xe_pm_suspend_prepare(struct xe_device *xe)
 {
-	if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
+	if (pm_suspend_default_s2idle())
 		xe_pm_d3cold_target_state_toggle(xe);
 	else
 		xe->d3cold.target_state = XE_D3COLD_OFF;
@@ -192,8 +192,6 @@ int xe_pm_suspend(struct xe_device *xe, bool hibernation)
 	xe_pm_block_begin_signalling();
 	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;
@@ -554,8 +552,12 @@ static int xe_pm_notifier_callback(struct notifier_block *nb,
 	int err = 0;
 
 	switch (action) {
-	case PM_HIBERNATION_PREPARE:
 	case PM_SUSPEND_PREPARE:
+		xe_pm_suspend_prepare(xe);
+		if (xe->d3cold.target_state == XE_D3COLD_VRSR)
+			break;
+		fallthrough;
+	case PM_HIBERNATION_PREPARE:
 	{
 		struct xe_validation_ctx ctx;
 
@@ -580,8 +582,11 @@ static int xe_pm_notifier_callback(struct notifier_block *nb,
 		xe_pm_block_end_signalling();
 		break;
 	}
-	case PM_POST_HIBERNATION:
 	case PM_POST_SUSPEND:
+		if (xe->d3cold.target_state == XE_D3COLD_VRSR)
+			break;
+		fallthrough;
+	case PM_POST_HIBERNATION:
 		complete_all(&xe->pm_block);
 		xe_pm_wake_rebind_workers(xe);
 		xe_bo_notifier_unprepare_all_pinned(xe);
-- 
2.52.0

[PATCH v6 12/12] drm/xe/vrsr: Introduce a debugfs node named vrsr_capable

[Badal Nilawar]

Add a debugfs node named vrsr_capable to check if the device
supports VRSR.

V2: Use debugfs_create_bool for vrsr_capable node (Karthik)

Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/xe/xe_debugfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index 844cfafe1ec7..60b78d3f91da 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -550,6 +550,8 @@ void xe_debugfs_register(struct xe_device *xe)
 	debugfs_create_file("disable_late_binding", 0600, root, xe,
 			    &disable_late_binding_fops);
 
+	debugfs_create_bool("vrsr_capable", 0400, root,
+			    &xe->d3cold.vrsr_capable);
 	/*
 	 * Don't expose page reclaim configuration file if not supported by the
 	 * hardware initially.
-- 
2.52.0