All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] thunderbolt: Fix typo in comment
@ 2022-04-11 13:00 Mika Westerberg
  2022-04-11 13:00 ` [PATCH 2/4] thunderbolt: Use decimal number with port numbers Mika Westerberg
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-11 13:00 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Brad Campbell

Should be 'in' instead of 'bin'. Fix it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/nhi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 4a582183f675..6221ca4ea287 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -1207,7 +1207,7 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	nhi->pdev = pdev;
 	nhi->ops = (const struct tb_nhi_ops *)id->driver_data;
-	/* cannot fail - table is allocated bin pcim_iomap_regions */
+	/* cannot fail - table is allocated in pcim_iomap_regions */
 	nhi->iobase = pcim_iomap_table(pdev)[0];
 	nhi->hop_count = ioread32(nhi->iobase + REG_HOP_COUNT) & 0x3ff;
 	dev_dbg(&pdev->dev, "total paths: %d\n", nhi->hop_count);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/4] thunderbolt: Use decimal number with port numbers
  2022-04-11 13:00 [PATCH 1/4] thunderbolt: Fix typo in comment Mika Westerberg
@ 2022-04-11 13:00 ` Mika Westerberg
  2022-04-11 13:00 ` [PATCH 3/4] thunderbolt: Dump path config space entries during discovery Mika Westerberg
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-11 13:00 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Brad Campbell

This makes it consistent with the other logging functions.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/tb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index b6fcd8d45324..ad025ff142ba 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -674,7 +674,7 @@ static inline int tb_port_write(struct tb_port *port, const void *buffer,
 #define __TB_PORT_PRINT(level, _port, fmt, arg...)                      \
 	do {                                                            \
 		const struct tb_port *__port = (_port);                 \
-		level(__port->sw->tb, "%llx:%x: " fmt,                  \
+		level(__port->sw->tb, "%llx:%u: " fmt,                  \
 		      tb_route(__port->sw), __port->port, ## arg);      \
 	} while (0)
 #define tb_port_WARN(port, fmt, arg...) \
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/4] thunderbolt: Dump path config space entries during discovery
  2022-04-11 13:00 [PATCH 1/4] thunderbolt: Fix typo in comment Mika Westerberg
  2022-04-11 13:00 ` [PATCH 2/4] thunderbolt: Use decimal number with port numbers Mika Westerberg
@ 2022-04-11 13:00 ` Mika Westerberg
  2022-04-11 13:00 ` [PATCH 4/4] thunderbolt: Use different lane for second DisplayPort tunnel Mika Westerberg
  2022-04-12  0:38 ` [PATCH 1/4] thunderbolt: Fix typo in comment Brad Campbell
  3 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-11 13:00 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Brad Campbell

This is useful when debugging possible issues during tunnel discovery.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/path.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index 299712accfe9..ee03fd75a472 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -166,6 +166,9 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
 		return NULL;
 	}
 
+	tb_dbg(path->tb, "discovering %s path starting from %llx:%u\n",
+	       path->name, tb_route(src->sw), src->port);
+
 	p = src;
 	h = src_hopid;
 
@@ -198,10 +201,13 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
 		path->hops[i].out_port = out_port;
 		path->hops[i].next_hop_index = next_hop;
 
+		tb_dump_hop(&path->hops[i], &hop);
+
 		h = next_hop;
 		p = out_port->remote;
 	}
 
+	tb_dbg(path->tb, "path discovery complete\n");
 	return path;
 
 err:
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 4/4] thunderbolt: Use different lane for second DisplayPort tunnel
  2022-04-11 13:00 [PATCH 1/4] thunderbolt: Fix typo in comment Mika Westerberg
  2022-04-11 13:00 ` [PATCH 2/4] thunderbolt: Use decimal number with port numbers Mika Westerberg
  2022-04-11 13:00 ` [PATCH 3/4] thunderbolt: Dump path config space entries during discovery Mika Westerberg
@ 2022-04-11 13:00 ` Mika Westerberg
  2022-04-12  0:38 ` [PATCH 1/4] thunderbolt: Fix typo in comment Brad Campbell
  3 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-11 13:00 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Brad Campbell

Brad reported that on Apple hardware with Light Ridge or Falcon Ridge
controller, plugging in a chain of Thunderbolt displays (Light Ridge
based controllers) causes all kinds of tearing and flickering. The
reason for this is that on Thunderbolt 1 hardware there is no lane
bonding so we have two independent 10 Gb/s lanes, and currently Linux
tunnels both displays through the lane 1. This makes the displays to
share the 10 Gb/s bandwidth which may not be enough for higher
resolutions.

For this reason make the second tunnel go through the lane 0 instead.
This seems to match what the macOS connection manager is also doing.

Reported-by: Brad Campbell <lists2009@fnarfbargle.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/tb.c     | 19 +++++++++++++++++--
 drivers/thunderbolt/test.c   | 16 ++++++++--------
 drivers/thunderbolt/tunnel.c | 11 ++++++-----
 drivers/thunderbolt/tunnel.h |  4 ++--
 4 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 9beb47b31c75..44d04b651a8b 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -867,7 +867,7 @@ static struct tb_port *tb_find_dp_out(struct tb *tb, struct tb_port *in)
 
 static void tb_tunnel_dp(struct tb *tb)
 {
-	int available_up, available_down, ret;
+	int available_up, available_down, ret, link_nr;
 	struct tb_cm *tcm = tb_priv(tb);
 	struct tb_port *port, *in, *out;
 	struct tb_tunnel *tunnel;
@@ -912,6 +912,20 @@ static void tb_tunnel_dp(struct tb *tb)
 		return;
 	}
 
+	/*
+	 * This is only applicable to links that are not bonded (so
+	 * when Thunderbolt 1 hardware is involved somewhere in the
+	 * topology). For these try to share the DP bandwidth between
+	 * the two lanes.
+	 */
+	link_nr = 1;
+	list_for_each_entry(tunnel, &tcm->tunnel_list, list) {
+		if (tb_tunnel_is_dp(tunnel)) {
+			link_nr = 0;
+			break;
+		}
+	}
+
 	/*
 	 * DP stream needs the domain to be active so runtime resume
 	 * both ends of the tunnel.
@@ -943,7 +957,8 @@ static void tb_tunnel_dp(struct tb *tb)
 	tb_dbg(tb, "available bandwidth for new DP tunnel %u/%u Mb/s\n",
 	       available_up, available_down);
 
-	tunnel = tb_tunnel_alloc_dp(tb, in, out, available_up, available_down);
+	tunnel = tb_tunnel_alloc_dp(tb, in, out, link_nr, available_up,
+				    available_down);
 	if (!tunnel) {
 		tb_port_dbg(out, "could not allocate DP tunnel\n");
 		goto err_reclaim;
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f69bab236ee..66b6e665e96f 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -1348,7 +1348,7 @@ static void tb_test_tunnel_dp(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, tunnel != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1394,7 +1394,7 @@ static void tb_test_tunnel_dp_chain(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev4->ports[14];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, tunnel != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1444,7 +1444,7 @@ static void tb_test_tunnel_dp_tree(struct kunit *test)
 	in = &dev2->ports[13];
 	out = &dev5->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, tunnel != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1509,7 +1509,7 @@ static void tb_test_tunnel_dp_max_length(struct kunit *test)
 	in = &dev6->ports[13];
 	out = &dev12->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, tunnel != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1627,7 +1627,7 @@ static void tb_test_tunnel_port_on_path(struct kunit *test)
 	in = &dev2->ports[13];
 	out = &dev5->ports[13];
 
-	dp_tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	dp_tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, dp_tunnel != NULL);
 
 	KUNIT_EXPECT_TRUE(test, tb_tunnel_port_on_path(dp_tunnel, in));
@@ -2009,7 +2009,7 @@ static void tb_test_credit_alloc_dp(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev->ports[14];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, tunnel != NULL);
 	KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
 
@@ -2245,7 +2245,7 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL1(struct kunit *test,
 
 	in = &host->ports[5];
 	out = &dev->ports[13];
-	dp_tunnel1 = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	dp_tunnel1 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, dp_tunnel1 != NULL);
 	KUNIT_ASSERT_EQ(test, dp_tunnel1->npaths, (size_t)3);
 
@@ -2282,7 +2282,7 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL2(struct kunit *test,
 
 	in = &host->ports[6];
 	out = &dev->ports[14];
-	dp_tunnel2 = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
+	dp_tunnel2 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0);
 	KUNIT_ASSERT_TRUE(test, dp_tunnel2 != NULL);
 	KUNIT_ASSERT_EQ(test, dp_tunnel2->npaths, (size_t)3);
 
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 118742ec93ed..8ccd70920b6a 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -858,6 +858,7 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
  * @tb: Pointer to the domain structure
  * @in: DP in adapter port
  * @out: DP out adapter port
+ * @link_nr: Preferred lane adapter when the link is not bonded
  * @max_up: Maximum available upstream bandwidth for the DP tunnel (%0
  *	    if not limited)
  * @max_down: Maximum available downstream bandwidth for the DP tunnel
@@ -869,8 +870,8 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
  * Return: Returns a tb_tunnel on success or NULL on failure.
  */
 struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
-				     struct tb_port *out, int max_up,
-				     int max_down)
+				     struct tb_port *out, int link_nr,
+				     int max_up, int max_down)
 {
 	struct tb_tunnel *tunnel;
 	struct tb_path **paths;
@@ -894,21 +895,21 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 	paths = tunnel->paths;
 
 	path = tb_path_alloc(tb, in, TB_DP_VIDEO_HOPID, out, TB_DP_VIDEO_HOPID,
-			     1, "Video");
+			     link_nr, "Video");
 	if (!path)
 		goto err_free;
 	tb_dp_init_video_path(path);
 	paths[TB_DP_VIDEO_PATH_OUT] = path;
 
 	path = tb_path_alloc(tb, in, TB_DP_AUX_TX_HOPID, out,
-			     TB_DP_AUX_TX_HOPID, 1, "AUX TX");
+			     TB_DP_AUX_TX_HOPID, link_nr, "AUX TX");
 	if (!path)
 		goto err_free;
 	tb_dp_init_aux_path(path);
 	paths[TB_DP_AUX_PATH_OUT] = path;
 
 	path = tb_path_alloc(tb, out, TB_DP_AUX_RX_HOPID, in,
-			     TB_DP_AUX_RX_HOPID, 1, "AUX RX");
+			     TB_DP_AUX_RX_HOPID, link_nr, "AUX RX");
 	if (!path)
 		goto err_free;
 	tb_dp_init_aux_path(path);
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 03e56076b5bc..bb4d1f1d6d0b 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -71,8 +71,8 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
 struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 					bool alloc_hopid);
 struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
-				     struct tb_port *out, int max_up,
-				     int max_down);
+				     struct tb_port *out, int link_nr,
+				     int max_up, int max_down);
 struct tb_tunnel *tb_tunnel_alloc_dma(struct tb *tb, struct tb_port *nhi,
 				      struct tb_port *dst, int transmit_path,
 				      int transmit_ring, int receive_path,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] thunderbolt: Fix typo in comment
  2022-04-11 13:00 [PATCH 1/4] thunderbolt: Fix typo in comment Mika Westerberg
                   ` (2 preceding siblings ...)
  2022-04-11 13:00 ` [PATCH 4/4] thunderbolt: Use different lane for second DisplayPort tunnel Mika Westerberg
@ 2022-04-12  0:38 ` Brad Campbell
  2022-04-19  7:55   ` Mika Westerberg
  3 siblings, 1 reply; 6+ messages in thread
From: Brad Campbell @ 2022-04-12  0:38 UTC (permalink / raw)
  To: Mika Westerberg, linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner

On 11/4/22 21:00, Mika Westerberg wrote:
> Should be 'in' instead of 'bin'. Fix it.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/thunderbolt/nhi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 4a582183f675..6221ca4ea287 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -1207,7 +1207,7 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	nhi->pdev = pdev;
>  	nhi->ops = (const struct tb_nhi_ops *)id->driver_data;
> -	/* cannot fail - table is allocated bin pcim_iomap_regions */
> +	/* cannot fail - table is allocated in pcim_iomap_regions */
>  	nhi->iobase = pcim_iomap_table(pdev)[0];
>  	nhi->hop_count = ioread32(nhi->iobase + REG_HOP_COUNT) & 0x3ff;
>  	dev_dbg(&pdev->dev, "total paths: %d\n", nhi->hop_count);

You can add 

Tested-by: Brad Campbell <lists2009@fnarfbargle.com> 

to the whole series.

Regards,
Brad

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] thunderbolt: Fix typo in comment
  2022-04-12  0:38 ` [PATCH 1/4] thunderbolt: Fix typo in comment Brad Campbell
@ 2022-04-19  7:55   ` Mika Westerberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-19  7:55 UTC (permalink / raw)
  To: Brad Campbell
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner

On Tue, Apr 12, 2022 at 08:38:29AM +0800, Brad Campbell wrote:
> On 11/4/22 21:00, Mika Westerberg wrote:
> > Should be 'in' instead of 'bin'. Fix it.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/thunderbolt/nhi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> > index 4a582183f675..6221ca4ea287 100644
> > --- a/drivers/thunderbolt/nhi.c
> > +++ b/drivers/thunderbolt/nhi.c
> > @@ -1207,7 +1207,7 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  
> >  	nhi->pdev = pdev;
> >  	nhi->ops = (const struct tb_nhi_ops *)id->driver_data;
> > -	/* cannot fail - table is allocated bin pcim_iomap_regions */
> > +	/* cannot fail - table is allocated in pcim_iomap_regions */
> >  	nhi->iobase = pcim_iomap_table(pdev)[0];
> >  	nhi->hop_count = ioread32(nhi->iobase + REG_HOP_COUNT) & 0x3ff;
> >  	dev_dbg(&pdev->dev, "total paths: %d\n", nhi->hop_count);
> 
> You can add 
> 
> Tested-by: Brad Campbell <lists2009@fnarfbargle.com> 
> 
> to the whole series.

Thanks!

Applied now to thunderbolt.git/next.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-19  7:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 13:00 [PATCH 1/4] thunderbolt: Fix typo in comment Mika Westerberg
2022-04-11 13:00 ` [PATCH 2/4] thunderbolt: Use decimal number with port numbers Mika Westerberg
2022-04-11 13:00 ` [PATCH 3/4] thunderbolt: Dump path config space entries during discovery Mika Westerberg
2022-04-11 13:00 ` [PATCH 4/4] thunderbolt: Use different lane for second DisplayPort tunnel Mika Westerberg
2022-04-12  0:38 ` [PATCH 1/4] thunderbolt: Fix typo in comment Brad Campbell
2022-04-19  7:55   ` Mika Westerberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.