2022-03-23 20:31:19

by Rajat Khandelwal

[permalink] [raw]
Subject: RE: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

+Mika
@Malani, Prashant @[email protected] This is the patch which fixes the partner issue. Kindly escalate your thoughts.

Thanks
Rajat

-----Original Message-----
From: Khandelwal, Rajat <[email protected]>
Sent: Wednesday, March 23, 2022 3:48 PM
To: [email protected]
Cc: Khandelwal, Rajat <[email protected]>; [email protected]; [email protected]; Malani, Prashant <[email protected]>; Rao, Abhijeet <[email protected]>; Regupathy, Rajaram <[email protected]>; [email protected]; [email protected]
Subject: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

Device routers don't wake up during S0ix when something is plugged in/out or if a DP monitor gets connected. This causes the linux device to not wake up during S0ix cycling as the host router didn't wake up because the device router didn't. This patch adds a new functionality to linux.

Signed-off-by: Rajat-Khandelwal <[email protected]>
---
drivers/thunderbolt/switch.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index d026e305fe5d..4f8056724aa4 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3067,13 +3067,11 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime)
tb_switch_suspend(port->remote->sw, runtime);
}

- if (runtime) {
+ if (runtime || device_may_wakeup(&sw->dev)) {
/* Trigger wake when something is plugged in/out */
flags |= TB_WAKE_ON_CONNECT | TB_WAKE_ON_DISCONNECT;
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_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE;
}

tb_switch_set_wake(sw, flags);
--
2.17.1


2022-03-23 23:40:59

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

Hi Rajat,

On Wed, Mar 23, 2022 at 3:26 AM Khandelwal, Rajat
<[email protected]> wrote:
>
> +Mika
> @Malani, Prashant @[email protected] This is the patch which fixes the partner issue. Kindly escalate your thoughts.
>
> Thanks
> Rajat
>
> -----Original Message-----
> From: Khandelwal, Rajat <[email protected]>
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: [email protected]
> Cc: Khandelwal, Rajat <[email protected]>; [email protected]; [email protected]; Malani, Prashant <[email protected]>; Rao, Abhijeet <[email protected]>; Regupathy, Rajaram <[email protected]>; [email protected]; [email protected]
> Subject: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in
>
> Device routers don't wake up during S0ix when something is plugged in/out or if a DP monitor gets connected. This causes the linux device to not wake up during S0ix cycling as the host router didn't wake up because the device router didn't. This patch adds a new functionality to linux.
>
> Signed-off-by: Rajat-Khandelwal <[email protected]>
> ---
> drivers/thunderbolt/switch.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index d026e305fe5d..4f8056724aa4 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -3067,13 +3067,11 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime)
> tb_switch_suspend(port->remote->sw, runtime);
> }
>
> - if (runtime) {
> + if (runtime || device_may_wakeup(&sw->dev)) {
> /* Trigger wake when something is plugged in/out */
> flags |= TB_WAKE_ON_CONNECT | TB_WAKE_ON_DISCONNECT;
> 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_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE;
> }
>
> tb_switch_set_wake(sw, flags);
> --
> 2.17.1
>


Can you please help double check your email configuration for sending
patches and responses to the mailing list?
I've checked the linux-usb mailing list archives and they don't
capture your original patch email or your forward.
https://marc.info/?l=linux-usb&r=1&b=202203&w=2

I did find Mika's response to your forward, though.

Let's get this figured out so that folks can have an easier time
applying your patches for evaluation with b4 or
similar tools.

Thanks,

Benson

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]

2022-03-24 21:06:18

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

Hi,

If you want it to be woken up when something is plugged to the USB4 port
then I suggest to add this to the "wakeup" capability of the usb4_port
itself (see drivers/thunderbolt/usb4_port.c). And make it disabled by
default too.

On Wed, Mar 23, 2022 at 10:26:37AM +0000, Khandelwal, Rajat wrote:
> +Mika
> @Malani, Prashant @[email protected] This is the patch which fixes the partner issue. Kindly escalate your thoughts.
>
> Thanks
> Rajat
>
> -----Original Message-----
> From: Khandelwal, Rajat <[email protected]>
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: [email protected]
> Cc: Khandelwal, Rajat <[email protected]>; [email protected]; [email protected]; Malani, Prashant <[email protected]>; Rao, Abhijeet <[email protected]>; Regupathy, Rajaram <[email protected]>; [email protected]; [email protected]
> Subject: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in
>
> Device routers don't wake up during S0ix when something is plugged in/out or if a DP monitor gets connected. This causes the linux device to not wake up during S0ix cycling as the host router didn't wake up because the device router didn't. This patch adds a new functionality to linux.
>
> Signed-off-by: Rajat-Khandelwal <[email protected]>
> ---
> drivers/thunderbolt/switch.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index d026e305fe5d..4f8056724aa4 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -3067,13 +3067,11 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime)
> tb_switch_suspend(port->remote->sw, runtime);
> }
>
> - if (runtime) {
> + if (runtime || device_may_wakeup(&sw->dev)) {
> /* Trigger wake when something is plugged in/out */
> flags |= TB_WAKE_ON_CONNECT | TB_WAKE_ON_DISCONNECT;
> 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_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE;
> }
>
> tb_switch_set_wake(sw, flags);
> --
> 2.17.1

2022-03-24 22:24:44

by Rajat Khandelwal

[permalink] [raw]
Subject: RE: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

@[email protected]
The original bug only targets DP plugins. Should we keep connect/disconnect flows also in loop? As Mika once told me, if the lid of the DUT is closed, and the USB4/TBT device is plugged out/in, the DUT shouldn't wake up as it will cause unnecessary consumption of energy.
Please confirm if only DP hotplugs in S0ix for waking are required.

Thanks
Rajat

-----Original Message-----
From: [email protected] <[email protected]>
Sent: Wednesday, March 23, 2022 4:01 PM
To: Khandelwal, Rajat <[email protected]>
Cc: [email protected]; Malani, Prashant <[email protected]>; [email protected]; Rao, Abhijeet <[email protected]>; Regupathy, Rajaram <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH] USB4/TBT device routers should wake up during S0ix when something gets connected/disconnected or a DP monitor gets plugged in

Hi,

If you want it to be woken up when something is plugged to the USB4 port then I suggest to add this to the "wakeup" capability of the usb4_port itself (see drivers/thunderbolt/usb4_port.c). And make it disabled by default too.

On Wed, Mar 23, 2022 at 10:26:37AM +0000, Khandelwal, Rajat wrote:
> +Mika
> @Malani, Prashant @[email protected] This is the patch which fixes the partner issue. Kindly escalate your thoughts.
>
> Thanks
> Rajat
>
> -----Original Message-----
> From: Khandelwal, Rajat <[email protected]>
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: [email protected]
> Cc: Khandelwal, Rajat <[email protected]>; [email protected];
> [email protected]; Malani, Prashant <[email protected]>; Rao,
> Abhijeet <[email protected]>; Regupathy, Rajaram
> <[email protected]>; [email protected];
> [email protected]
> Subject: [PATCH] USB4/TBT device routers should wake up during S0ix
> when something gets connected/disconnected or a DP monitor gets
> plugged in
>
> Device routers don't wake up during S0ix when something is plugged in/out or if a DP monitor gets connected. This causes the linux device to not wake up during S0ix cycling as the host router didn't wake up because the device router didn't. This patch adds a new functionality to linux.
>
> Signed-off-by: Rajat-Khandelwal <[email protected]>
> ---
> drivers/thunderbolt/switch.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thunderbolt/switch.c
> b/drivers/thunderbolt/switch.c index d026e305fe5d..4f8056724aa4 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -3067,13 +3067,11 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime)
> tb_switch_suspend(port->remote->sw, runtime);
> }
>
> - if (runtime) {
> + if (runtime || device_may_wakeup(&sw->dev)) {
> /* Trigger wake when something is plugged in/out */
> flags |= TB_WAKE_ON_CONNECT | TB_WAKE_ON_DISCONNECT;
> 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_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE;
> }
>
> tb_switch_set_wake(sw, flags);
> --
> 2.17.1