[PATCH v9 0/5] Support system sleep with offloaded usb transfers

Guan-Yu Lin posted 5 patches 2 weeks, 5 days ago
drivers/usb/core/driver.c         | 131 +++++++++++++++++++++++++++++-
drivers/usb/core/endpoint.c       |   8 --
drivers/usb/core/usb.c            |   4 +
drivers/usb/dwc3/core.c           | 105 +++++++++++++++++++++++-
drivers/usb/dwc3/core.h           |   1 +
drivers/usb/host/xhci-plat.c      |  42 +++++++++-
drivers/usb/host/xhci-sideband.c  |  82 +++++++++++++++++++
include/linux/usb.h               |  27 +++++-
include/linux/usb/hcd.h           |   7 ++
include/linux/usb/xhci-sideband.h |   6 ++
sound/usb/qcom/qc_audio_offload.c |   3 +
11 files changed, 398 insertions(+), 18 deletions(-)
[PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Guan-Yu Lin 2 weeks, 5 days ago
Wesley Cheng and Mathias Nyman's USB offload design enables a co-processor
to handle some USB transfers, potentially allowing the main system to
sleep and save power. However, Linux's power management system halts the
USB host controller when the main system isn't managing any USB transfers.
To address this, the proposal modifies the system to recognize offloaded
USB transfers and manage power accordingly.

This involves two key steps:
1. Transfer Status Tracking: Propose xhci_sideband_get and
xhci_sideband_put to track USB transfers on the co-processor, ensuring the
system is aware of any ongoing activity.
2. Power Management Adjustment:  Modifications to the USB driver stack
(dwc3 controller driver, xhci host controller driver, and USB device
drivers) allow the system to sleep without disrupting co-processor managed
USB transfers. This involves adding conditional checks to bypass some
power management operations.

patches depends on series "Introduce QC USB SND audio offloading support" 
https://lore.kernel.org/lkml/20241213235403.4109199-1-quic_wcheng@quicinc.com/T/

changelog
----------
Changes in v9:
- Remove unnecessary white space change.

Changes in v8:
- Change the runtime pm api to correct the error handling flow.
- Not flushing endpoints of actively offloaded USB devices to avoid
  possible USB transfer conflicts.

Changes in v7:
- Remove counting mechanism in struct usb_hcd. The USB device's offload
  status will be solely recorded in each related struct usb_device.
- Utilizes `needs_remote_wakeup` attribute in struct usb_interface to
  control the suspend flow of USB interfaces and associated USB endpoints.
  This addresses the need to support interrupt transfers generated by
  offloaded USB devices while the system is suspended.
- Block any offload_usage change during USB device suspend period.

Changes in v6:
- Fix build errors when CONFIG_USB_XHCI_SIDEBAND is disabled.
- Explicitly specify the data structure of the drvdata refereced in
  dwc3_suspend(), dwc3_resume().
- Move the initialization of counters to the patches introducing them.

Changes in v5:
- Walk through the USB children in usb_sideband_check() to determine the
  sideband activity under the specific USB device. 
- Replace atomic_t by refcount_t.
- Reduce logs by using dev_dbg & remove __func__.

Changes in v4:
- Isolate the feature into USB driver stack.
- Integrate with series "Introduce QC USB SND audio offloading support"

Changes in v3:
- Integrate the feature with the pm core framework.

Changes in v2:
- Cosmetics changes on coding style.

[v3] PM / core: conditionally skip system pm in device/driver model
[v2] usb: host: enable suspend-to-RAM control in userspace
[v1] [RFC] usb: host: Allow userspace to control usb suspend flows
---

Guan-Yu Lin (5):
  usb: dwc3: separate dev_pm_ops for each pm_event
  usb: xhci-plat: separate dev_pm_ops for each pm_event
  usb: add apis for offload usage tracking
  xhci: sideband: add api to trace sideband usage
  usb: host: enable USB offload during system sleep

 drivers/usb/core/driver.c         | 131 +++++++++++++++++++++++++++++-
 drivers/usb/core/endpoint.c       |   8 --
 drivers/usb/core/usb.c            |   4 +
 drivers/usb/dwc3/core.c           | 105 +++++++++++++++++++++++-
 drivers/usb/dwc3/core.h           |   1 +
 drivers/usb/host/xhci-plat.c      |  42 +++++++++-
 drivers/usb/host/xhci-sideband.c  |  82 +++++++++++++++++++
 include/linux/usb.h               |  27 +++++-
 include/linux/usb/hcd.h           |   7 ++
 include/linux/usb/xhci-sideband.h |   6 ++
 sound/usb/qcom/qc_audio_offload.c |   3 +
 11 files changed, 398 insertions(+), 18 deletions(-)

-- 
2.48.0.rc2.279.g1de40edade-goog
Re: [PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Pierre-Louis Bossart 2 weeks, 5 days ago
On 1/17/25 8:48 AM, Guan-Yu Lin wrote:
> Wesley Cheng and Mathias Nyman's USB offload design enables a co-processor
> to handle some USB transfers, potentially allowing the main system to
> sleep and save power. However, Linux's power management system halts the
> USB host controller when the main system isn't managing any USB transfers.
> To address this, the proposal modifies the system to recognize offloaded
> USB transfers and manage power accordingly.

You probably want to expand on the problem statement and clarify quite a few ambiguous statements.

a) "allowing the main system to sleep and save power". What is the 'main system' and what sort of sleep are you referring to? 
Traditionally when playing audio the audio devices remain in D0. Support for playback during 'S0 idle' is more complicated, I have yet to see a working solution even without USB offload in the picture.

b) when referring to power management, you have to be specific on whether you mean 'runtime_pm' or regular power management. Not the same thing but there are related issues.

c) for audio offload the transactions that go through the DSP or co-processor are only for audio endpoints. Control transactions rely on endpoint0 and are NOT offloaded to the best of my knowledge. Which means that you would need to track control transactions as well, even if there is no audio streaming. Otherwise you would have a risk of the XHCI controller transitioning in and out of sleep states.
 
> This involves two key steps:
> 1. Transfer Status Tracking: Propose xhci_sideband_get and
> xhci_sideband_put to track USB transfers on the co-processor, ensuring the
> system is aware of any ongoing activity.


> 2. Power Management Adjustment:  Modifications to the USB driver stack
> (dwc3 controller driver, xhci host controller driver, and USB device
> drivers) allow the system to sleep without disrupting co-processor managed
> USB transfers. This involves adding conditional checks to bypass some
> power management operations.

This is even more confusing, initially the point was to prevent the controller from sleeping while there are offloaded transactions, but now the goal would be to allow the system to sleep while there are offloaded transactions. This isn't the same problem, is it?
 
> patches depends on series "Introduce QC USB SND audio offloading support" 
> https://lore.kernel.org/lkml/20241213235403.4109199-1-quic_wcheng@quicinc.com/T/
> 
> changelog
> ----------
> Changes in v9:
> - Remove unnecessary white space change.
> 
> Changes in v8:
> - Change the runtime pm api to correct the error handling flow.
> - Not flushing endpoints of actively offloaded USB devices to avoid
>   possible USB transfer conflicts.
> 
> Changes in v7:
> - Remove counting mechanism in struct usb_hcd. The USB device's offload
>   status will be solely recorded in each related struct usb_device.
> - Utilizes `needs_remote_wakeup` attribute in struct usb_interface to
>   control the suspend flow of USB interfaces and associated USB endpoints.
>   This addresses the need to support interrupt transfers generated by
>   offloaded USB devices while the system is suspended.
> - Block any offload_usage change during USB device suspend period.
> 
> Changes in v6:
> - Fix build errors when CONFIG_USB_XHCI_SIDEBAND is disabled.
> - Explicitly specify the data structure of the drvdata refereced in
>   dwc3_suspend(), dwc3_resume().
> - Move the initialization of counters to the patches introducing them.
> 
> Changes in v5:
> - Walk through the USB children in usb_sideband_check() to determine the
>   sideband activity under the specific USB device. 
> - Replace atomic_t by refcount_t.
> - Reduce logs by using dev_dbg & remove __func__.
> 
> Changes in v4:
> - Isolate the feature into USB driver stack.
> - Integrate with series "Introduce QC USB SND audio offloading support"
> 
> Changes in v3:
> - Integrate the feature with the pm core framework.
> 
> Changes in v2:
> - Cosmetics changes on coding style.
> 
> [v3] PM / core: conditionally skip system pm in device/driver model
> [v2] usb: host: enable suspend-to-RAM control in userspace
> [v1] [RFC] usb: host: Allow userspace to control usb suspend flows
> ---
> 
> Guan-Yu Lin (5):
>   usb: dwc3: separate dev_pm_ops for each pm_event
>   usb: xhci-plat: separate dev_pm_ops for each pm_event
>   usb: add apis for offload usage tracking
>   xhci: sideband: add api to trace sideband usage
>   usb: host: enable USB offload during system sleep
> 
>  drivers/usb/core/driver.c         | 131 +++++++++++++++++++++++++++++-
>  drivers/usb/core/endpoint.c       |   8 --
>  drivers/usb/core/usb.c            |   4 +
>  drivers/usb/dwc3/core.c           | 105 +++++++++++++++++++++++-
>  drivers/usb/dwc3/core.h           |   1 +
>  drivers/usb/host/xhci-plat.c      |  42 +++++++++-
>  drivers/usb/host/xhci-sideband.c  |  82 +++++++++++++++++++
>  include/linux/usb.h               |  27 +++++-
>  include/linux/usb/hcd.h           |   7 ++
>  include/linux/usb/xhci-sideband.h |   6 ++
>  sound/usb/qcom/qc_audio_offload.c |   3 +
>  11 files changed, 398 insertions(+), 18 deletions(-)
>
Re: [PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Guan-Yu Lin 2 weeks ago
On Sat, Jan 18, 2025 at 12:14 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.dev> wrote:
>
> On 1/17/25 8:48 AM, Guan-Yu Lin wrote:
> > Wesley Cheng and Mathias Nyman's USB offload design enables a co-processor
> > to handle some USB transfers, potentially allowing the main system to
> > sleep and save power. However, Linux's power management system halts the
> > USB host controller when the main system isn't managing any USB transfers.
> > To address this, the proposal modifies the system to recognize offloaded
> > USB transfers and manage power accordingly.
>
> You probably want to expand on the problem statement and clarify quite a few ambiguous statements.
>
> a) "allowing the main system to sleep and save power". What is the 'main system' and what sort of sleep are you referring to?
> Traditionally when playing audio the audio devices remain in D0. Support for playback during 'S0 idle' is more complicated, I have yet to see a working solution even without USB offload in the picture.
>
The main system refers to the device running the linux kernel. The
sleep refers to system suspend in the System Sleep model[1], which
should be S3 state (Suspend to RAM).

> b) when referring to power management, you have to be specific on whether you mean 'runtime_pm' or regular power management. Not the same thing but there are related issues.
>
Thanks for pointing it out. By power management, I mean the System
Sleep model[1].

> c) for audio offload the transactions that go through the DSP or co-processor are only for audio endpoints. Control transactions rely on endpoint0 and are NOT offloaded to the best of my knowledge. Which means that you would need to track control transactions as well, even if there is no audio streaming. Otherwise you would have a risk of the XHCI controller transitioning in and out of sleep states.
>
To my knowledge, the system would not issue control transfer after the
system goes to sleep. If there are any abnormalities in the XHCI
controller that requires the system to handle, the controller would
issue an interrupt to the system. We could allow this interrupt to
wake up the system, and the system could then issue corresponding
control transfers to handle it.

> > This involves two key steps:
> > 1. Transfer Status Tracking: Propose xhci_sideband_get and
> > xhci_sideband_put to track USB transfers on the co-processor, ensuring the
> > system is aware of any ongoing activity.
>
>
> > 2. Power Management Adjustment:  Modifications to the USB driver stack
> > (dwc3 controller driver, xhci host controller driver, and USB device
> > drivers) allow the system to sleep without disrupting co-processor managed
> > USB transfers. This involves adding conditional checks to bypass some
> > power management operations.
>
> This is even more confusing, initially the point was to prevent the controller from sleeping while there are offloaded transactions, but now the goal would be to allow the system to sleep while there are offloaded transactions. This isn't the same problem, is it?
>

The purpose of this series is to allow offloaded usb transfers happen
during system sleep. In order to achieve this, we need to prevent the
controller from sleeping when there's offloaded usb transfer ongoing,
specifically when the system is sleeping.
Without this series, the system could still allow offloaded usb
traffic when the system is active, but the system would put the
controller to sleep when the system is going to sleep, thus we're not
able to suspend the system when we have offloaded usb transfers in the
current system.

[1] https://www.kernel.org/doc/html/v4.12/driver-api/pm/devices.html

Regards,
Guan-Yu
Re: [PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Pierre-Louis Bossart 1 week, 1 day ago
>>> 2. Power Management Adjustment:  Modifications to the USB driver stack
>>> (dwc3 controller driver, xhci host controller driver, and USB device
>>> drivers) allow the system to sleep without disrupting co-processor managed
>>> USB transfers. This involves adding conditional checks to bypass some
>>> power management operations.
>>
>> This is even more confusing, initially the point was to prevent the controller from sleeping while there are offloaded transactions, but now the goal would be to allow the system to sleep while there are offloaded transactions. This isn't the same problem, is it?
>>
> 
> The purpose of this series is to allow offloaded usb transfers happen
> during system sleep. In order to achieve this, we need to prevent the
> controller from sleeping when there's offloaded usb transfer ongoing,
> specifically when the system is sleeping.
> Without this series, the system could still allow offloaded usb
> traffic when the system is active, but the system would put the
> controller to sleep when the system is going to sleep, thus we're not
> able to suspend the system when we have offloaded usb transfers in the
> current system.

I am not following, sorry.

Is the desired outcome to 

a) prevent the system from entering S3 if there is an active USB audio offloaded stream?

or b) allow offloaded transactions even when the system is in S3?


which is it?

a) would be rather interesting, but currently we don't have any such behavior supported. When the system enters S3 all audio stops. The stream will resume when the system goes back to S0. Do we really want the battery to drain in S3?

b) seems rather complicated, once the on-going DMA transfers complete then who's going to refill buffers for the USB offloaded streams? Allowing the lowest level to operate even in S3 is only a small part of the puzzle, someone's got to provide data at some point. Unless the data is generated also by a side DSP having access to mass storage or wireless interfaces?
Re: [PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Guan-Yu Lin 2 days, 16 hours ago
On Tue, Jan 28, 2025 at 11:22 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.dev> wrote:
>
> I am not following, sorry.
>
> Is the desired outcome to
>
> a) prevent the system from entering S3 if there is an active USB audio offloaded stream?
>
> or b) allow offloaded transactions even when the system is in S3?
>
>
> which is it?
>
> a) would be rather interesting, but currently we don't have any such behavior supported. When the system enters S3 all audio stops. The stream will resume when the system goes back to S0. Do we really want the battery to drain in S3?
>
> b) seems rather complicated, once the on-going DMA transfers complete then who's going to refill buffers for the USB offloaded streams? Allowing the lowest level to operate even in S3 is only a small part of the puzzle, someone's got to provide data at some point. Unless the data is generated also by a side DSP having access to mass storage or wireless interfaces?

Thanks for the question, the intent of our proposal should be (b), to
allow offloaded transactions even when the system is in S3.
In our design, the DSP wakes the system before the buffers are fully
drained. This patchset enables the USB controller for offloaded
transfers during system suspend (S3). To be precise, this patchset
focuses solely on enabling the USB controller in S3 and does not
include other necessary components for continuous offloaded USB
transfers. I'll revise the commit message/cover letter to reflect
this.Thanks for highlighting the potential ambiguity.

Regards,
Guan-Yu
Re: [PATCH v9 0/5] Support system sleep with offloaded usb transfers
Posted by Pierre-Louis Bossart 1 day, 19 hours ago
On 2/2/25 20:57, Guan-Yu Lin wrote:
> On Tue, Jan 28, 2025 at 11:22 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.dev> wrote:
>>
>> I am not following, sorry.
>>
>> Is the desired outcome to
>>
>> a) prevent the system from entering S3 if there is an active USB audio offloaded stream?
>>
>> or b) allow offloaded transactions even when the system is in S3?
>>
>>
>> which is it?
>>
>> a) would be rather interesting, but currently we don't have any such behavior supported. When the system enters S3 all audio stops. The stream will resume when the system goes back to S0. Do we really want the battery to drain in S3?
>>
>> b) seems rather complicated, once the on-going DMA transfers complete then who's going to refill buffers for the USB offloaded streams? Allowing the lowest level to operate even in S3 is only a small part of the puzzle, someone's got to provide data at some point. Unless the data is generated also by a side DSP having access to mass storage or wireless interfaces?
> 
> Thanks for the question, the intent of our proposal should be (b), to
> allow offloaded transactions even when the system is in S3.
> In our design, the DSP wakes the system before the buffers are fully
> drained. This patchset enables the USB controller for offloaded
> transfers during system suspend (S3). To be precise, this patchset
> focuses solely on enabling the USB controller in S3 and does not
> include other necessary components for continuous offloaded USB
> transfers. I'll revise the commit message/cover letter to reflect
> this.Thanks for highlighting the potential ambiguity.

Thanks for the precision.

It was my understanding that anything above S1 could incur a hardware/software latency of two seconds or more to go back to S0. That would imply a buffering scheme that's significantly larger than usual offloaded solutions. In "typical" offload implementations it's rare to go beyond 100s of ms, since at some point you run into user-experience issues when applying volume changes or when changing tracks. It's usually a no-go if the user has to wait for a perceivable amount of time while the buffers drain.

Not to mention that quite a few platforms no longer support S3, since 'Modern Standby' aka "S0 Low Power Idle" or 's2idle' was introduced in the Windows 10 days S3 became largely a legacy feature gradually dropped since no one really uses it.