Re: [PATCH 2/2] xhci: Improve the XHCI resume time

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 21.4.2023 7.58, Basavaraj Natikar wrote:
> 
> On 4/20/2023 10:33 PM, Mark Hasemeyer wrote:
>>> It may be necessary to wait only for auto-resume cases.
>> I find this comment misleading as the patch assumes that it's only necessary to
>> wait for auto-resume cases. Are there any cases where the driver should wait
>> during system-resume?
> 
> Only in case of auto-resume (runtime resume).
> 
> Rewording the commit message as follows.

Thanks for fixing this extra system resume delay

Maybe some kind of big picture explanation could be added to the commit message,
such as:

Avoid extra 120ms delay during system resume.

xHC controller may signal wake up to 120ms before it shows which USB device
caused the wake on the xHC port registers.

The xhci driver therefore checks for port activity up to 120ms during resume,
making sure that the hub driver can see the port change, and won't immediately
runtime suspend back due to no port activity.

This is however only needed for runtime resume as system resume will resume
all child hubs and other child usb devices anyway.

> 
> Each XHCI controller while xhci_resumes by default takes 120 ms more if
> there is no activity on the ports or no ports connected. Therefore, if
> there are more USB controllers on the system, 120 ms more per controller
> will add delay to system resume from suspended states like s2idle, S3 or
> S4 states.
> 
> Once the XHCI controller is in runtime suspended state (D3 state), on USB
> device hotplug controller will runtime resume (D0 state) and check for
> pending port events if no events, wait for 120 ms to re-check for port
> activity to handle missed wake signal.
> 
> A delay of 120 ms more to re-check for port activity is needed only in
> auto-resume (runtime resume) cases. Hence, add a check only for runtime
> resume from runtime suspend (D3->D0) to avoid the 120ms more delay for
> other PM events (system resume from suspend states like s2idle, S3 or S4
> states) so that the system resume time can be improved.
> 
> Please let me know if any inputs.

I can only think of one minor side-effect that would be runtime suspending back
too early after system resume. This could happen when connecting the first
usb device to a roothub on a (system) suspended setup?

steps:
1. in system suspend, no usb devices connected, xhci in D3, can signal wake with PME#
2. connect first usb device, xHC signals PME# wake
3. system resumes, xhci resumes to D0, but no actity visible on xHC port registers
4. rootubs resumes, no other children on this bus.
5. roothubs sees no activity (due to 120ms max latency before visible on port registers)
6. roothubs runtime suspend
7. xhci runtime suspends
8. same device now causes xHC to PME# wake again,
9. runtime reusume xhci, do wait 120ms for port activity
10. see port activity, resume hub, enumerate device.

It might be that this really isn't an issue due to the the graceperiod fix:

33e321586e37 xhci: Add grace period after xHC start to prevent premature runtime suspend.

Thanks
-Mathias