RE: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

RE: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

[Khandelwal, Rajat]

+Abhijeet
@Benson Leung @Malani, Prashant Please provide your comments. 

-----Original Message-----
From: Mika Westerberg <mika.westerberg@linux.intel.com> 
Sent: Wednesday, August 3, 2022 11:07 AM
To: Khandelwal, Rajat <rajat.khandelwal@intel.com>
Cc: andreas.noever@gmail.com; Jamet, Michael <michael.jamet@intel.com>; YehezkelShB@gmail.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Malani, Prashant <pmalani@google.com>; bleung@google.com
Subject: Re: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

Hi,

On Wed, Aug 03, 2022 at 06:31:39AM +0530, Rajat Khandelwal wrote:
> Current implementation doesn't allow user to go into system suspend
> and simultaneously project the screen on an external display connected
> to a TBT/USB4 device. This patch enables this feature.
> 
> Let me know what you think about this.

Please prefix patches to follow the sybsystem style. For Thunderbolt it
should be "thunderbolt: ....".

This will cause any monitor plug to wake up the system and I'm not sure
we want that, at least to be default behaviour. If you have a regular
Type-C (non-USB4/TBT) dock and you plug in a monitor to that, does it
wake up?

IIRC I already suggested you to do this for the USB4 ports themselves
(struct usb4_port) so that userspace can make them wake up the system on
any event by writing "enabled > .../wakeup" which is the normal way.

RE: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

[Khandelwal, Rajat]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]



-----Original Message-----
From: Khandelwal, Rajat 
Sent: Wednesday, August 3, 2022 1:00 PM
To: Mika Westerberg <mika.westerberg@linux.intel.com>; Malani, Prashant <pmalani@google.com>; bleung@google.com
Cc: andreas.noever@gmail.com; Jamet, Michael <michael.jamet@intel.com>; YehezkelShB@gmail.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Rao, Abhijeet <abhijeet.rao@intel.com>
Subject: RE: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

+Abhijeet
@Benson Leung @Malani, Prashant Please provide your comments. 

-----Original Message-----
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Sent: Wednesday, August 3, 2022 11:07 AM
To: Khandelwal, Rajat <rajat.khandelwal@intel.com>
Cc: andreas.noever@gmail.com; Jamet, Michael <michael.jamet@intel.com>; YehezkelShB@gmail.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Malani, Prashant <pmalani@google.com>; bleung@google.com
Subject: Re: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

Hi,

On Wed, Aug 03, 2022 at 06:31:39AM +0530, Rajat Khandelwal wrote:
> Current implementation doesn't allow user to go into system suspend 
> and simultaneously project the screen on an external display connected 
> to a TBT/USB4 device. This patch enables this feature.
> 
> Let me know what you think about this.

Please prefix patches to follow the sybsystem style. For Thunderbolt it should be "thunderbolt: ....".

This will cause any monitor plug to wake up the system and I'm not sure we want that, at least to be default behaviour. If you have a regular Type-C (non-USB4/TBT) dock and you plug in a monitor to that, does it wake up?

IIRC I already suggested you to do this for the USB4 ports themselves (struct usb4_port) so that userspace can make them wake up the system on any event by writing "enabled > .../wakeup" which is the normal way.

[-- Attachment #2: 0001-Supporting-wake-on-DP-hot-events-behind-TBT-USB4-rou.patch --]
[-- Type: application/octet-stream, Size: 2629 bytes --]

From 76ba76b9959e89cdfab082ef807a65c0b7066ed7 Mon Sep 17 00:00:00 2001
From: Rajat Khandelwal <rajat.khandelwal@intel.com>
Date: Wed, 3 Aug 2022 06:10:29 +0530
Subject: [PATCH] Supporting wake on DP hot events behind TBT/USB4 routers

Current implementation doesn't allow user to go into system suspend
and simultaneously project the screen on an external display connected
to a TBT/USB4 device. This patch enables this feature.

Let me know what you think about this.

Signed-off-by: Rajat Khandelwal <rajat.khandelwal@intel.com>
---
 drivers/thunderbolt/switch.c  | 3 ++-
 drivers/thunderbolt/tb_regs.h | 1 +
 drivers/thunderbolt/usb4.c    | 7 ++++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 561e1d77240e..3a9f17fcbdfe 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3152,7 +3152,8 @@ 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_USB4 | TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE;
+		flags |= TB_WAKE_ON_USB4;
+		flags |= TB_WAKE_ON_USB3 | TB_WAKE_ON_PCIE | TB_WAKE_ON_DP;
 	}
 
 	tb_switch_set_wake(sw, flags);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 6a16f61a72a1..f03fa6f26d5c 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -206,6 +206,7 @@ struct tb_regs_switch_header {
 #define ROUTER_CS_6_TNS				BIT(1)
 #define ROUTER_CS_6_WOPS			BIT(2)
 #define ROUTER_CS_6_WOUS			BIT(3)
+#define ROUTER_CS_6_WODS			BIT(4)
 #define ROUTER_CS_6_HCI				BIT(18)
 #define ROUTER_CS_6_CR				BIT(25)
 #define ROUTER_CS_7				0x07
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 3a2e7126db9d..227367ce2d36 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -166,11 +166,12 @@ static void usb4_switch_check_wakes(struct tb_switch *sw)
 		if (tb_sw_read(sw, &val, TB_CFG_SWITCH, ROUTER_CS_6, 1))
 			return;
 
-		tb_sw_dbg(sw, "PCIe wake: %s, USB3 wake: %s\n",
+		tb_sw_dbg(sw, "PCIe wake: %s, USB3 wake: %s, DP wake: %s\n",
 			  (val & ROUTER_CS_6_WOPS) ? "yes" : "no",
-			  (val & ROUTER_CS_6_WOUS) ? "yes" : "no");
+			  (val & ROUTER_CS_6_WOUS) ? "yes" : "no",
+			  (val & ROUTER_CS_6_WODS) ? "yes" : "no");
 
-		wakeup = val & (ROUTER_CS_6_WOPS | ROUTER_CS_6_WOUS);
+		wakeup = val & (ROUTER_CS_6_WOPS | ROUTER_CS_6_WOUS | ROUTER_CS_6_WODS);
 	}
 
 	/* Check for any connected downstream ports for USB4 wake */
-- 
2.17.1

Re: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?


http://daringfireball.net/2007/07/on_top

On Wed, Aug 03, 2022 at 07:29:36AM +0000, Khandelwal, Rajat wrote:
> +Abhijeet
> @Benson Leung @Malani, Prashant Please provide your comments. 

Re: [PATCH] thunderbolt: Supporting wake on DP hot events behind TBT/USB4 routers

[Greg KH]

On Wed, Aug 03, 2022 at 07:31:30AM +0000, Khandelwal, Rajat wrote:
> 

What did you write here?

confused,

greg k-h