[PATCH 0/2] Handle RPM for xhci-pci

[PATCH 0/2] Handle RPM for xhci-pci

[Basavaraj Natikar]

This series includes fixes for PCI USB controllers that use RPM as their
default policy, including enabling RPM for controllers that support
low-power states.

Basavaraj Natikar (2):
  xhci: Loosen RPM as default policy to cover xHC 1.1 as well
  xhci: Enable RPM on controllers that support low-power states

 drivers/usb/host/xhci-pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well

[Basavaraj Natikar]

The USB host controller (1022:43f7) isn't going into PCI D3 by default
without anything connected. This is because the policy that was introduced
by commit a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all
xHC 1.2 or later devices") only covered 1.2 or later.

The 1.1 specification also has the same requirement as the 1.2
specification for D3 support. So expand the runtime PM as default policy
to 1.1 devices as well.

Fixes: a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all xHC 1.2 or later devices")
Link: https://composter.com.ua/documents/xHCI_Specification_for_USB.pdf
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/usb/host/xhci-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b9ae5c2a2527..c908a80ef436 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -533,7 +533,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	}
 
 	/* xHC spec requires PCI devices to support D3hot and D3cold */
-	if (xhci->hci_version >= 0x120)
+	if (xhci->hci_version >= 0x110)
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)
-- 
2.25.1

Re: [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well

[Mathias Nyman]

On 21.8.2023 9.57, Basavaraj Natikar wrote:
> The USB host controller (1022:43f7) isn't going into PCI D3 by default
> without anything connected. This is because the policy that was introduced
> by commit a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all
> xHC 1.2 or later devices") only covered 1.2 or later.
> 
> The 1.1 specification also has the same requirement as the 1.2
> specification for D3 support. So expand the runtime PM as default policy
> to 1.1 devices as well.
> 

I'm a bit hesitant to change the default policy for this many hosts in one go.
This affects all vendors.

I see a regression risk in this.
Many xhci issues have been related to runtime pm.

We do have a list of selected Intel xHCI 1.1 host that have enabled runtime pm
by default. Same could be done for 1022:43f7

Or if you are confident all AMD xHC 1.1 hosts can have this enabled, then that
works as well:

if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
	xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;

Thanks
Mathias

[PATCH 2/2] xhci: Enable RPM on controllers that support low-power states

[Basavaraj Natikar]

Use the low-power states of the underlying platform to enable runtime PM.
If the platform doesn't support runtime D3, then enabling default RPM will
result in the controller malfunctioning, as in the case of hotplug devices
not being detected because of a failed interrupt generation.

Cc: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/usb/host/xhci-pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c908a80ef436..1bb5b510c5ba 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -693,7 +693,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
 	pm_runtime_put_noidle(&dev->dev);
 
-	if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
+	if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
+		pm_runtime_forbid(&dev->dev);
+	else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
 		pm_runtime_allow(&dev->dev);
 
 	dma_set_max_seg_size(&dev->dev, UINT_MAX);
-- 
2.25.1

Re: [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states

[Mathias Nyman]

On 21.8.2023 9.57, Basavaraj Natikar wrote:
> Use the low-power states of the underlying platform to enable runtime PM.
> If the platform doesn't support runtime D3, then enabling default RPM will
> result in the controller malfunctioning, as in the case of hotplug devices
> not being detected because of a failed interrupt generation.
> 
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
>   drivers/usb/host/xhci-pci.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index c908a80ef436..1bb5b510c5ba 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -693,7 +693,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>   	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
>   	pm_runtime_put_noidle(&dev->dev);
>   
> -	if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
> +	if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
> +		pm_runtime_forbid(&dev->dev);
> +	else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
>   		pm_runtime_allow(&dev->dev);
>   
>   	dma_set_max_seg_size(&dev->dev, UINT_MAX);

Looks good to me

-Mathias

Re: [PATCH 0/2] Handle RPM for xhci-pci

[Mario Limonciello]

On 8/21/2023 01:57, Basavaraj Natikar wrote:
> This series includes fixes for PCI USB controllers that use RPM as their
> default policy, including enabling RPM for controllers that support
> low-power states.
> 
> Basavaraj Natikar (2):
>    xhci: Loosen RPM as default policy to cover xHC 1.1 as well
>    xhci: Enable RPM on controllers that support low-power states
> 
>   drivers/usb/host/xhci-pci.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 

Hi Matthias,

Can you take a look at this series?  It's been on the list about a month.

Thanks,

Re: [PATCH 0/2] Handle RPM for xhci-pci

[Mathias Nyman]

On 15.9.2023 7.38, Mario Limonciello wrote:
> On 8/21/2023 01:57, Basavaraj Natikar wrote:
>> This series includes fixes for PCI USB controllers that use RPM as their
>> default policy, including enabling RPM for controllers that support
>> low-power states.
>>
>> Basavaraj Natikar (2):
>>    xhci: Loosen RPM as default policy to cover xHC 1.1 as well
>>    xhci: Enable RPM on controllers that support low-power states
>>
>>   drivers/usb/host/xhci-pci.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
> 
> Hi Matthias,
> 
> Can you take a look at this series?  It's been on the list about a month.
> 
> Thanks,
> 

Yes, sorry about the delay

-Mathias