| Message ID | ce2048af-1044-4424-bca2-3799baefb9c2@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Wake on connect / wake on disconnect | expand |
Hi Mario, On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: > Mika, > > Recently there are some conversations about wake-up from connect/disconnect > happening and I wanted to get some background from you about the current > policy set in tb_switch_suspend(). > > Wake on connect and disconnect are only used for runtime, not for system > suspend. Would you be open to adding wake on connect as well for system > suspend? This should help enable use cases like plugging in a closed laptop > to a dock (which works on Windows). Don't we already have a similar for all usb4_portX devices? That does not work for you?
On 4/4/25 01:02, Mika Westerberg wrote: > Hi Mario, > > On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: >> Mika, >> >> Recently there are some conversations about wake-up from connect/disconnect >> happening and I wanted to get some background from you about the current >> policy set in tb_switch_suspend(). >> >> Wake on connect and disconnect are only used for runtime, not for system >> suspend. Would you be open to adding wake on connect as well for system >> suspend? This should help enable use cases like plugging in a closed laptop >> to a dock (which works on Windows). > > Don't we already have a similar for all usb4_portX devices? That does not > work for you? > I think that will functionally work - but I'll double check. However usb_portX power/wakeup defaults are 'disabled' so this would need a udev rule. Could we set the kernel default for those to 'enabled' instead?
On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: > On 4/4/25 01:02, Mika Westerberg wrote: > > Hi Mario, > > > > On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: > > > Mika, > > > > > > Recently there are some conversations about wake-up from connect/disconnect > > > happening and I wanted to get some background from you about the current > > > policy set in tb_switch_suspend(). > > > > > > Wake on connect and disconnect are only used for runtime, not for system > > > suspend. Would you be open to adding wake on connect as well for system > > > suspend? This should help enable use cases like plugging in a closed laptop > > > to a dock (which works on Windows). > > > > Don't we already have a similar for all usb4_portX devices? That does not > > work for you? > > > > I think that will functionally work - but I'll double check. > > However usb_portX power/wakeup defaults are 'disabled' so this would need a > udev rule. Could we set the kernel default for those to 'enabled' instead? No because that would trigger wakeup each time you unplug/plug and this is certainly not good if you close the lid, unplug from dock and throw the laptop to your backpack. Chrome uses this with "dark resume" so if this is supported by the userspace then it can also enable these.
On 4/4/2025 6:53 AM, Mika Westerberg wrote: > On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: >> On 4/4/25 01:02, Mika Westerberg wrote: >>> Hi Mario, >>> >>> On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: >>>> Mika, >>>> >>>> Recently there are some conversations about wake-up from connect/disconnect >>>> happening and I wanted to get some background from you about the current >>>> policy set in tb_switch_suspend(). >>>> >>>> Wake on connect and disconnect are only used for runtime, not for system >>>> suspend. Would you be open to adding wake on connect as well for system >>>> suspend? This should help enable use cases like plugging in a closed laptop >>>> to a dock (which works on Windows). >>> >>> Don't we already have a similar for all usb4_portX devices? That does not >>> work for you? >>> >> >> I think that will functionally work - but I'll double check. >> >> However usb_portX power/wakeup defaults are 'disabled' so this would need a >> udev rule. Could we set the kernel default for those to 'enabled' instead? > > No because that would trigger wakeup each time you unplug/plug and this is > certainly not good if you close the lid, unplug from dock and throw the > laptop to your backpack. Chrome uses this with "dark resume" so if this is > supported by the userspace then it can also enable these. Ahhh. I was thinking specifically with wake on connect that's not a problem, but the sysfs knob for the port changes both wake on connect and wake on disconnect. Is there actually a use case for chrome with wake on disconnect? IE if we didn't turn on wake on disconnect but defaulted to wake on connect would things be OK? Or made the sysfs knob control only wake on disconnect?
On Fri, Apr 04, 2025 at 10:03:18AM -0500, Mario Limonciello wrote: > On 4/4/2025 6:53 AM, Mika Westerberg wrote: > > On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: > > > On 4/4/25 01:02, Mika Westerberg wrote: > > > > Hi Mario, > > > > > > > > On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: > > > > > Mika, > > > > > > > > > > Recently there are some conversations about wake-up from connect/disconnect > > > > > happening and I wanted to get some background from you about the current > > > > > policy set in tb_switch_suspend(). > > > > > > > > > > Wake on connect and disconnect are only used for runtime, not for system > > > > > suspend. Would you be open to adding wake on connect as well for system > > > > > suspend? This should help enable use cases like plugging in a closed laptop > > > > > to a dock (which works on Windows). > > > > > > > > Don't we already have a similar for all usb4_portX devices? That does not > > > > work for you? > > > > > > > > > > I think that will functionally work - but I'll double check. > > > > > > However usb_portX power/wakeup defaults are 'disabled' so this would need a > > > udev rule. Could we set the kernel default for those to 'enabled' instead? > > > > No because that would trigger wakeup each time you unplug/plug and this is > > certainly not good if you close the lid, unplug from dock and throw the > > laptop to your backpack. Chrome uses this with "dark resume" so if this is > > supported by the userspace then it can also enable these. > > Ahhh. I was thinking specifically with wake on connect that's not a > problem, but the sysfs knob for the port changes both wake on connect and > wake on disconnect. > > Is there actually a use case for chrome with wake on disconnect? IE if we > didn't turn on wake on disconnect but defaulted to wake on connect would > things be OK? Or made the sysfs knob control only wake on disconnect? Good guestion - I don't know ;-) The Chrome folks wanted this so I suppose their usecase is specifically that dark resume and I think that's when you unplug a device so disconnect. Not so sure about wake on connect.
On 4/4/25 11:10, Mika Westerberg wrote: > On Fri, Apr 04, 2025 at 10:03:18AM -0500, Mario Limonciello wrote: >> On 4/4/2025 6:53 AM, Mika Westerberg wrote: >>> On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: >>>> On 4/4/25 01:02, Mika Westerberg wrote: >>>>> Hi Mario, >>>>> >>>>> On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: >>>>>> Mika, >>>>>> >>>>>> Recently there are some conversations about wake-up from connect/disconnect >>>>>> happening and I wanted to get some background from you about the current >>>>>> policy set in tb_switch_suspend(). >>>>>> >>>>>> Wake on connect and disconnect are only used for runtime, not for system >>>>>> suspend. Would you be open to adding wake on connect as well for system >>>>>> suspend? This should help enable use cases like plugging in a closed laptop >>>>>> to a dock (which works on Windows). >>>>> >>>>> Don't we already have a similar for all usb4_portX devices? That does not >>>>> work for you? >>>>> >>>> >>>> I think that will functionally work - but I'll double check. >>>> >>>> However usb_portX power/wakeup defaults are 'disabled' so this would need a >>>> udev rule. Could we set the kernel default for those to 'enabled' instead? >>> >>> No because that would trigger wakeup each time you unplug/plug and this is >>> certainly not good if you close the lid, unplug from dock and throw the >>> laptop to your backpack. Chrome uses this with "dark resume" so if this is >>> supported by the userspace then it can also enable these. >> >> Ahhh. I was thinking specifically with wake on connect that's not a >> problem, but the sysfs knob for the port changes both wake on connect and >> wake on disconnect. >> >> Is there actually a use case for chrome with wake on disconnect? IE if we >> didn't turn on wake on disconnect but defaulted to wake on connect would >> things be OK? Or made the sysfs knob control only wake on disconnect? > > Good guestion - I don't know ;-) The Chrome folks wanted this so I suppose > their usecase is specifically that dark resume and I think that's when you > unplug a device so disconnect. Not so sure about wake on connect. I found the original patch from Rajat [1]. Rajat, any comments? Could you loop in the right people from the Chrome side to ask? I think my "preference" would be that we make "wake on connect" always enabled and then let the sysfs knob control "wake on disconnect". [1] https://lore.kernel.org/linux-usb/20221101115042.248187-1-rajat.khandelwal@intel.com/
On 4/4/25 11:16, Mario Limonciello wrote: > > > On 4/4/25 11:10, Mika Westerberg wrote: >> On Fri, Apr 04, 2025 at 10:03:18AM -0500, Mario Limonciello wrote: >>> On 4/4/2025 6:53 AM, Mika Westerberg wrote: >>>> On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: >>>>> On 4/4/25 01:02, Mika Westerberg wrote: >>>>>> Hi Mario, >>>>>> >>>>>> On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: >>>>>>> Mika, >>>>>>> >>>>>>> Recently there are some conversations about wake-up from connect/ >>>>>>> disconnect >>>>>>> happening and I wanted to get some background from you about the >>>>>>> current >>>>>>> policy set in tb_switch_suspend(). >>>>>>> >>>>>>> Wake on connect and disconnect are only used for runtime, not for >>>>>>> system >>>>>>> suspend. Would you be open to adding wake on connect as well for >>>>>>> system >>>>>>> suspend? This should help enable use cases like plugging in a >>>>>>> closed laptop >>>>>>> to a dock (which works on Windows). >>>>>> >>>>>> Don't we already have a similar for all usb4_portX devices? That >>>>>> does not >>>>>> work for you? >>>>>> >>>>> >>>>> I think that will functionally work - but I'll double check. >>>>> >>>>> However usb_portX power/wakeup defaults are 'disabled' so this >>>>> would need a >>>>> udev rule. Could we set the kernel default for those to 'enabled' >>>>> instead? >>>> >>>> No because that would trigger wakeup each time you unplug/plug and >>>> this is >>>> certainly not good if you close the lid, unplug from dock and throw the >>>> laptop to your backpack. Chrome uses this with "dark resume" so if >>>> this is >>>> supported by the userspace then it can also enable these. >>> >>> Ahhh. I was thinking specifically with wake on connect that's not a >>> problem, but the sysfs knob for the port changes both wake on connect >>> and >>> wake on disconnect. >>> >>> Is there actually a use case for chrome with wake on disconnect? IE >>> if we >>> didn't turn on wake on disconnect but defaulted to wake on connect would >>> things be OK? Or made the sysfs knob control only wake on disconnect? >> >> Good guestion - I don't know ;-) The Chrome folks wanted this so I >> suppose >> their usecase is specifically that dark resume and I think that's when >> you >> unplug a device so disconnect. Not so sure about wake on connect. > > I found the original patch from Rajat [1]. > > Rajat, any comments? Could you loop in the right people from the Chrome > side to ask? I think my "preference" would be that we make "wake on > connect" always enabled and then let the sysfs knob control "wake on > disconnect". > > [1] https://lore.kernel.org/linux-usb/20221101115042.248187-1- > rajat.khandelwal@intel.com/ Oh Rajat's email bounced. The only person I know that I've worked on wakeup related stuff is Raul. I'll add him. Mika, Raul, Could you pull in current Chrome people from Intel and Google that could comment here?
On Fri, Apr 4, 2025 at 10:20 AM Mario Limonciello <superm1@kernel.org> wrote: > > > > On 4/4/25 11:16, Mario Limonciello wrote: > > > > > > On 4/4/25 11:10, Mika Westerberg wrote: > >> On Fri, Apr 04, 2025 at 10:03:18AM -0500, Mario Limonciello wrote: > >>> On 4/4/2025 6:53 AM, Mika Westerberg wrote: > >>>> On Fri, Apr 04, 2025 at 06:47:31AM -0500, Mario Limonciello wrote: > >>>>> On 4/4/25 01:02, Mika Westerberg wrote: > >>>>>> Hi Mario, > >>>>>> > >>>>>> On Thu, Apr 03, 2025 at 01:12:07PM -0500, Mario Limonciello wrote: > >>>>>>> Mika, > >>>>>>> > >>>>>>> Recently there are some conversations about wake-up from connect/ > >>>>>>> disconnect > >>>>>>> happening and I wanted to get some background from you about the > >>>>>>> current > >>>>>>> policy set in tb_switch_suspend(). > >>>>>>> > >>>>>>> Wake on connect and disconnect are only used for runtime, not for > >>>>>>> system > >>>>>>> suspend. Would you be open to adding wake on connect as well for > >>>>>>> system > >>>>>>> suspend? This should help enable use cases like plugging in a > >>>>>>> closed laptop > >>>>>>> to a dock (which works on Windows). > >>>>>> > >>>>>> Don't we already have a similar for all usb4_portX devices? That > >>>>>> does not > >>>>>> work for you? > >>>>>> > >>>>> > >>>>> I think that will functionally work - but I'll double check. > >>>>> > >>>>> However usb_portX power/wakeup defaults are 'disabled' so this > >>>>> would need a > >>>>> udev rule. Could we set the kernel default for those to 'enabled' > >>>>> instead? > >>>> > >>>> No because that would trigger wakeup each time you unplug/plug and > >>>> this is > >>>> certainly not good if you close the lid, unplug from dock and throw the > >>>> laptop to your backpack. Chrome uses this with "dark resume" so if > >>>> this is > >>>> supported by the userspace then it can also enable these. > >>> > >>> Ahhh. I was thinking specifically with wake on connect that's not a > >>> problem, but the sysfs knob for the port changes both wake on connect > >>> and > >>> wake on disconnect. > >>> > >>> Is there actually a use case for chrome with wake on disconnect? IE > >>> if we > >>> didn't turn on wake on disconnect but defaulted to wake on connect would > >>> things be OK? Or made the sysfs knob control only wake on disconnect? > >> > >> Good guestion - I don't know ;-) The Chrome folks wanted this so I > >> suppose > >> their usecase is specifically that dark resume and I think that's when > >> you > >> unplug a device so disconnect. Not so sure about wake on connect. > > > > I found the original patch from Rajat [1]. > > > > Rajat, any comments? Could you loop in the right people from the Chrome > > side to ask? I think my "preference" would be that we make "wake on > > connect" always enabled and then let the sysfs knob control "wake on > > disconnect". > > > > [1] https://lore.kernel.org/linux-usb/20221101115042.248187-1- > > rajat.khandelwal@intel.com/ > > Oh Rajat's email bounced. The only person I know that I've worked on > wakeup related stuff is Raul. I'll add him. > > Mika, Raul, > > Could you pull in current Chrome people from Intel and Google that could > comment here? + Opal who should be able to answer the question regarding wake on connect/disconnect.
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 6a2116cbb06f..f2f6a085a742 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -3599,6 +3599,7 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime) flags |= TB_WAKE_ON_USB4; flags |= TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE | TB_WAKE_ON_DP; } else if (device_may_wakeup(&sw->dev)) { + flags |= TB_WAKE_ON_CONNECT; flags |= TB_WAKE_ON_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE; }