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

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

[Khandelwal, Rajat]

+Mika
@Malani, Prashant @bleung@google.com This is the patch which fixes the partner issue. Kindly escalate your thoughts. 

Thanks
Rajat

-----Original Message-----
From: Khandelwal, Rajat <rajat.khandelwal@intel.com> 
Sent: Wednesday, March 23, 2022 3:48 PM
To: mika.westerberg@linux.intel.com
Cc: Khandelwal, Rajat <rajat.khandelwal@intel.com>; bleung@google.com; jthies@google.com; Malani, Prashant <pmalani@google.com>; Rao, Abhijeet <abhijeet.rao@intel.com>; Regupathy, Rajaram <rajaram.regupathy@intel.com>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
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 <rajat.khandelwal@intel.com>
---
 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

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

[mika.westerberg]

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 @bleung@google.com This is the patch which fixes the partner issue. Kindly escalate your thoughts. 
> 
> Thanks
> Rajat
> 
> -----Original Message-----
> From: Khandelwal, Rajat <rajat.khandelwal@intel.com> 
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: mika.westerberg@linux.intel.com
> Cc: Khandelwal, Rajat <rajat.khandelwal@intel.com>; bleung@google.com; jthies@google.com; Malani, Prashant <pmalani@google.com>; Rao, Abhijeet <abhijeet.rao@intel.com>; Regupathy, Rajaram <rajaram.regupathy@intel.com>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <rajat.khandelwal@intel.com>
> ---
>  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

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

[Benson Leung]

Hi Rajat,

On Wed, Mar 23, 2022 at 3:26 AM Khandelwal, Rajat
<rajat.khandelwal@intel.com> wrote:
>
> +Mika
> @Malani, Prashant @bleung@google.com This is the patch which fixes the partner issue. Kindly escalate your thoughts.
>
> Thanks
> Rajat
>
> -----Original Message-----
> From: Khandelwal, Rajat <rajat.khandelwal@intel.com>
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: mika.westerberg@linux.intel.com
> Cc: Khandelwal, Rajat <rajat.khandelwal@intel.com>; bleung@google.com; jthies@google.com; Malani, Prashant <pmalani@google.com>; Rao, Abhijeet <abhijeet.rao@intel.com>; Regupathy, Rajaram <rajaram.regupathy@intel.com>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <rajat.khandelwal@intel.com>
> ---
>  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.
bleung@google.com
Chromium OS Project
bleung@chromium.org

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

[Khandelwal, Rajat]

@bleung@google.com
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: mika.westerberg@linux.intel.com <mika.westerberg@linux.intel.com> 
Sent: Wednesday, March 23, 2022 4:01 PM
To: Khandelwal, Rajat <rajat.khandelwal@intel.com>
Cc: bleung@google.com; Malani, Prashant <pmalani@google.com>; jthies@google.com; Rao, Abhijeet <abhijeet.rao@intel.com>; Regupathy, Rajaram <rajaram.regupathy@intel.com>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
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 @bleung@google.com This is the patch which fixes the partner issue. Kindly escalate your thoughts. 
> 
> Thanks
> Rajat
> 
> -----Original Message-----
> From: Khandelwal, Rajat <rajat.khandelwal@intel.com>
> Sent: Wednesday, March 23, 2022 3:48 PM
> To: mika.westerberg@linux.intel.com
> Cc: Khandelwal, Rajat <rajat.khandelwal@intel.com>; bleung@google.com; 
> jthies@google.com; Malani, Prashant <pmalani@google.com>; Rao, 
> Abhijeet <abhijeet.rao@intel.com>; Regupathy, Rajaram 
> <rajaram.regupathy@intel.com>; linux-usb@vger.kernel.org; 
> linux-kernel@vger.kernel.org
> 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 <rajat.khandelwal@intel.com>
> ---
>  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