Linux-USB Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Handle RPM for xhci-pci
@ 2023-08-21  6:57 Basavaraj Natikar
  2023-08-21  6:57 ` [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well Basavaraj Natikar
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Basavaraj Natikar @ 2023-08-21  6:57 UTC (permalink / raw
  To: mathias.nyman, gregkh, mika.westerberg, mario.limonciello,
	linux-usb
  Cc: 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well
  2023-08-21  6:57 [PATCH 0/2] Handle RPM for xhci-pci Basavaraj Natikar
@ 2023-08-21  6:57 ` Basavaraj Natikar
  2023-09-15 13:20   ` Mathias Nyman
  2023-08-21  6:57 ` [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states Basavaraj Natikar
  2023-09-15  4:38 ` [PATCH 0/2] Handle RPM for xhci-pci Mario Limonciello
  2 siblings, 1 reply; 7+ messages in thread
From: Basavaraj Natikar @ 2023-08-21  6:57 UTC (permalink / raw
  To: mathias.nyman, gregkh, mika.westerberg, mario.limonciello,
	linux-usb
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states
  2023-08-21  6:57 [PATCH 0/2] Handle RPM for xhci-pci Basavaraj Natikar
  2023-08-21  6:57 ` [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well Basavaraj Natikar
@ 2023-08-21  6:57 ` Basavaraj Natikar
  2023-09-15 13:21   ` Mathias Nyman
  2023-09-15  4:38 ` [PATCH 0/2] Handle RPM for xhci-pci Mario Limonciello
  2 siblings, 1 reply; 7+ messages in thread
From: Basavaraj Natikar @ 2023-08-21  6:57 UTC (permalink / raw
  To: mathias.nyman, gregkh, mika.westerberg, mario.limonciello,
	linux-usb
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Handle RPM for xhci-pci
  2023-08-21  6:57 [PATCH 0/2] Handle RPM for xhci-pci Basavaraj Natikar
  2023-08-21  6:57 ` [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well Basavaraj Natikar
  2023-08-21  6:57 ` [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states Basavaraj Natikar
@ 2023-09-15  4:38 ` Mario Limonciello
  2023-09-15 12:55   ` Mathias Nyman
  2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2023-09-15  4:38 UTC (permalink / raw
  To: mathias.nyman; +Cc: Basavaraj Natikar, gregkh, mika.westerberg, linux-usb

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,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Handle RPM for xhci-pci
  2023-09-15  4:38 ` [PATCH 0/2] Handle RPM for xhci-pci Mario Limonciello
@ 2023-09-15 12:55   ` Mathias Nyman
  0 siblings, 0 replies; 7+ messages in thread
From: Mathias Nyman @ 2023-09-15 12:55 UTC (permalink / raw
  To: Mario Limonciello, mathias.nyman
  Cc: Basavaraj Natikar, gregkh, mika.westerberg, linux-usb

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well
  2023-08-21  6:57 ` [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well Basavaraj Natikar
@ 2023-09-15 13:20   ` Mathias Nyman
  0 siblings, 0 replies; 7+ messages in thread
From: Mathias Nyman @ 2023-09-15 13:20 UTC (permalink / raw
  To: Basavaraj Natikar, mathias.nyman, gregkh, mika.westerberg,
	mario.limonciello, linux-usb

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states
  2023-08-21  6:57 ` [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states Basavaraj Natikar
@ 2023-09-15 13:21   ` Mathias Nyman
  0 siblings, 0 replies; 7+ messages in thread
From: Mathias Nyman @ 2023-09-15 13:21 UTC (permalink / raw
  To: Basavaraj Natikar, mathias.nyman, gregkh, mika.westerberg,
	mario.limonciello, linux-usb

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-15 13:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-21  6:57 [PATCH 0/2] Handle RPM for xhci-pci Basavaraj Natikar
2023-08-21  6:57 ` [PATCH 1/2] xhci: Loosen RPM as default policy to cover xHC 1.1 as well Basavaraj Natikar
2023-09-15 13:20   ` Mathias Nyman
2023-08-21  6:57 ` [PATCH 2/2] xhci: Enable RPM on controllers that support low-power states Basavaraj Natikar
2023-09-15 13:21   ` Mathias Nyman
2023-09-15  4:38 ` [PATCH 0/2] Handle RPM for xhci-pci Mario Limonciello
2023-09-15 12:55   ` Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).