[PATCH v4 3/5] PCI: Verify functions currently in D3cold have entered D0

Mario Limonciello posted 5 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v4 3/5] PCI: Verify functions currently in D3cold have entered D0
Posted by Mario Limonciello 1 year, 5 months ago
From: Mario Limonciello <mario.limonciello@amd.com>

It is reported that USB4 routers and downstream devices may behave
incorrectly if a dock cable is plugged in at approximately the time that
the autosuspend_delay is configured. In this situation the device has
attempted to enter D3cold, but didn't finish D3cold entry when the PCI
core tried to transition it back to D0.

Empirically measuring this situation an "aborted" D3cold exit takes
~60ms and a "normal" D3cold exit takes ~6ms.

The PCI-PM 1.2 spec specifies that the restore time for functions
in D3cold is either 'Full context restore or boot latency'.

As PCIe r6.0 sec 5.8 specifies that the device will have gone
through a conventional reset, it may take some time for the
device to be ready.

Wait up to 1 sec as specified in PCIe r6.0 sec 6.6.1 for a device
in D3cold to return to D0.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pci/pci.c | 11 +++++++++++
 drivers/pci/pci.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b7717155e2fd0..7e861b6923d0a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1425,6 +1425,17 @@ int pci_power_up(struct pci_dev *dev)
 	else if (state == PCI_D2)
 		udelay(PCI_PM_D2_DELAY);
 
+	/*
+	 * D3cold -> D0 will have gone through a conventional reset and may need
+	 * time to be ready.
+	 */
+	if (dev->current_state == PCI_D3cold) {
+		int ret;
+
+		ret = pci_dev_wait(dev, PCI_DEV_WAIT_D3COLD_D0, PCI_RESET_WAIT);
+		if (ret)
+			return ret;
+	}
 end:
 	dev->current_state = PCI_D0;
 	if (need_restore)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 477257e843952..a675f5d55f298 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -11,6 +11,7 @@ enum pci_reset_type {
 	PCI_DEV_WAIT_BUS_RESET,
 	PCI_DEV_WAIT_RESUME,
 	PCI_DEV_WAIT_DPC,
+	PCI_DEV_WAIT_D3COLD_D0,
 };
 
 /* Number of possible devfns: 0.0 to 1f.7 inclusive */
-- 
2.43.0

Re: [PATCH v4 3/5] PCI: Verify functions currently in D3cold have entered D0
Posted by Ilpo Järvinen 1 year, 5 months ago
On Thu, 22 Aug 2024, Mario Limonciello wrote:

> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> It is reported that USB4 routers and downstream devices may behave
> incorrectly if a dock cable is plugged in at approximately the time that
> the autosuspend_delay is configured. In this situation the device has
> attempted to enter D3cold, but didn't finish D3cold entry when the PCI
> core tried to transition it back to D0.
> 
> Empirically measuring this situation an "aborted" D3cold exit takes
> ~60ms and a "normal" D3cold exit takes ~6ms.
> 
> The PCI-PM 1.2 spec specifies that the restore time for functions
> in D3cold is either 'Full context restore or boot latency'.
> 
> As PCIe r6.0 sec 5.8 specifies that the device will have gone
> through a conventional reset, it may take some time for the
> device to be ready.
> 
> Wait up to 1 sec as specified in PCIe r6.0 sec 6.6.1 for a device
> in D3cold to return to D0.
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/pci/pci.c | 11 +++++++++++
>  drivers/pci/pci.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b7717155e2fd0..7e861b6923d0a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1425,6 +1425,17 @@ int pci_power_up(struct pci_dev *dev)
>  	else if (state == PCI_D2)
>  		udelay(PCI_PM_D2_DELAY);
>  
> +	/*
> +	 * D3cold -> D0 will have gone through a conventional reset and may need
> +	 * time to be ready.
> +	 */
> +	if (dev->current_state == PCI_D3cold) {
> +		int ret;
> +
> +		ret = pci_dev_wait(dev, PCI_DEV_WAIT_D3COLD_D0, PCI_RESET_WAIT);
> +		if (ret)
> +			return ret;
> +	}
>  end:
>  	dev->current_state = PCI_D0;
>  	if (need_restore)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 477257e843952..a675f5d55f298 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -11,6 +11,7 @@ enum pci_reset_type {
>  	PCI_DEV_WAIT_BUS_RESET,
>  	PCI_DEV_WAIT_RESUME,
>  	PCI_DEV_WAIT_DPC,
> +	PCI_DEV_WAIT_D3COLD_D0,

Don't you need to add a string for this too? :-/

I wonder if it would be prudent to add PCI_DEV_WAIT_MAX and 
use static_assert() for the sizeof the pci_reset_types[] in patch 1 to 
autodetect mismatch (though it won't help if something is added in the 
middle of the list).

-- 
 i.