[PATCH v2 0/2] media: i2c: ds90ub960: VC mapping improvements and TX1 support

[PATCH v2 0/2] media: i2c: ds90ub960: VC mapping improvements and TX1 support

[Rishikesh Donadkar]

This series improves virtual channel (VC) handling and adds TX1 port
support to the DS90UB960/UB9702 deserializer driver.

PATCH 01:    Enable CSI TX1 port by parsing active RX/TX ports from
             device tree and configuring RX_PORT_CTL BCC mapping
PATCH 02:    Replace hardcoded VC-to-port-number assignment with a
             proper per-port VC map built from source frame descriptors,
             fix loop termination bug in ub960_get_vc_maps() that caused
             ports after the first unconnected RX port to be skipped, and
             add UB9702-specific VC mapping (4 bits/VC, 2 VCs per port)
             instead of incorrectly reusing the UB960 layout

Tested streaming with the following configurations:
1. IMX219 + v3link on AM62A
2. ov2312 + v3link on AM62A
3. ov2312 + fusion2 on AM62A

Overlay and defconfig changes for the same can be found below:
https://github.com/RISHI27-dot/linux/commits/u/ds90ub960_mcs_v2/

---
Changes in v2:
- Squash PATCH 2/4, 3/4 and 4/4 into PATCH 2/2
- Update Jai's depricated email address
- Add Co-developed-by tag

Jai Luthra (1):
  media: i2c: ds90ub960: Support multi-channel sensors

Yemike Abhilash Chandra (1):
  media: i2c: ds90ub960: Enable CSI TX1 port

 drivers/media/i2c/ds90ub960.c | 240 ++++++++++++++++++++++------------
 1 file changed, 154 insertions(+), 86 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] media: i2c: ds90ub960: Enable CSI TX1 port

[Rishikesh Donadkar]

From: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>

The DS90UB960 chip has two CSI-2 transmit ports (TX0 and TX1), but the
current driver implementation only utilizes TX0. To enable TX1 as the
active output port, the I2C Bidirectional Control Channel (BCC) mapping in
the RX_PORT_CTL register must be configured to use I2C Slave Port 1.

This patch adds a new function, ub960_parse_active_ports(), which is
called during driver initialization to scan the device tree and identify
which RX and TX ports are enabled. The function creates bitmasks
representing the active ports and uses these masks to correctly configure
the RX_PORT_CTL register, ensuring proper port routing for whichever TX
port is in use.

DS90UB960 data sheet: https://www.ti.com/lit/ds/symlink/ds90ub960-q1.pdf
Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
Co-developed-by: Rishikesh Donadkar <r-donadkar@ti.com>
Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
---
 drivers/media/i2c/ds90ub960.c | 46 ++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 15a9797b47ac..653dc7a4eee5 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -26,6 +26,7 @@
  * - i2c-atr could be made embeddable instead of allocatable.
  */
 
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/cleanup.h>
 #include <linux/clk.h>
@@ -76,6 +77,8 @@
 
 #define UB960_NUM_BC_GPIOS		4
 
+#define UB960_CSI_TX0			BIT(4)
+
 /*
  * Register map
  *
@@ -114,6 +117,7 @@
 #define UB960_SR_SCL_HIGH_TIME			0x0a
 #define UB960_SR_SCL_LOW_TIME			0x0b
 #define UB960_SR_RX_PORT_CTL			0x0c
+#define UB960_SR_RX_PORT_CTL_BCC_MAP		GENMASK(7, 4)
 #define UB960_SR_IO_CTL				0x0d
 #define UB960_SR_GPIO_PIN_STS			0x0e
 #define UB960_SR_GPIO_INPUT_CTL			0x0f
@@ -589,6 +593,9 @@ struct ub960_data {
 	u32 tx_data_rate;		/* Nominal data rate (Gb/s) */
 	s64 tx_link_freq[1];
 
+	u8 rx_mask;
+	u8 tx_mask;
+
 	struct i2c_atr *atr;
 
 	struct {
@@ -2538,7 +2545,18 @@ static int ub960_init_rx_ports_ub960(struct ub960_data *priv)
 	struct device *dev = &priv->client->dev;
 	unsigned int port_lock_mask;
 	unsigned int port_mask;
-	int ret;
+	u8 enabled_rxports_mask;
+	u8 enabled_rxports;
+	int ret = 0;
+
+	/* Configure I2C interface for RX ports */
+	enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
+	enabled_rxports = (priv->tx_mask & UB960_CSI_TX0)  ? 0x00 : enabled_rxports_mask;
+
+	ret = ub960_update_bits(priv, UB960_SR_RX_PORT_CTL, enabled_rxports_mask,
+				enabled_rxports, &ret);
+	if (ret)
+		return ret;
 
 	for_each_active_rxport(priv, it) {
 		ret = ub960_init_rx_port_ub960(priv, it.rxport);
@@ -4789,6 +4807,30 @@ static int ub960_parse_dt_txports(struct ub960_data *priv)
 	return 0;
 }
 
+static void ub960_parse_active_ports(struct ub960_data *priv)
+{
+	struct device *dev = &priv->client->dev;
+	int nport;
+
+	priv->rx_mask = 0;
+	priv->tx_mask = 0;
+
+	for (nport = 0; nport < priv->hw_data->num_rxports + priv->hw_data->num_txports; nport++) {
+		struct fwnode_handle *ep_fwnode;
+
+		ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), nport, 0, 0);
+		if (!ep_fwnode)
+			continue;
+
+		if (nport < priv->hw_data->num_rxports)
+			priv->rx_mask |= BIT(nport);
+		else
+			priv->tx_mask |= BIT(nport);
+
+		fwnode_handle_put(ep_fwnode);
+	}
+}
+
 static int ub960_parse_dt(struct ub960_data *priv)
 {
 	int ret;
@@ -5162,6 +5204,8 @@ static int ub960_probe(struct i2c_client *client)
 	if (ret)
 		goto err_mutex_destroy;
 
+	ub960_parse_active_ports(priv);
+
 	ret = ub960_parse_dt(priv);
 	if (ret)
 		goto err_disable_core_hw;
-- 
2.34.1

Re: [PATCH v2 1/2] media: i2c: ds90ub960: Enable CSI TX1 port

[Tomi Valkeinen]

Hi,

On 20/07/2026 14:09, Rishikesh Donadkar wrote:
> From: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> 
> The DS90UB960 chip has two CSI-2 transmit ports (TX0 and TX1), but the
> current driver implementation only utilizes TX0. To enable TX1 as the
> active output port, the I2C Bidirectional Control Channel (BCC) mapping in
> the RX_PORT_CTL register must be configured to use I2C Slave Port 1.
I think you could expend the above a bit to explain how the HW handles 
the I2C routing.

> This patch adds a new function, ub960_parse_active_ports(), which is
> called during driver initialization to scan the device tree and identify
> which RX and TX ports are enabled. The function creates bitmasks
> representing the active ports and uses these masks to correctly configure
> the RX_PORT_CTL register, ensuring proper port routing for whichever TX
> port is in use.

This doesn't quite explain how the decisions about routing are done. 
What if two TX ports are enabled?

> DS90UB960 data sheet: https://www.ti.com/lit/ds/symlink/ds90ub960-q1.pdf

I don't think there's a need to link to the datasheet in the commit. 
Unless, say, you add support for a new device.

> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> Co-developed-by: Rishikesh Donadkar <r-donadkar@ti.com>
> Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
> ---
>   drivers/media/i2c/ds90ub960.c | 46 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index 15a9797b47ac..653dc7a4eee5 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -26,6 +26,7 @@
>    * - i2c-atr could be made embeddable instead of allocatable.
>    */
>   
> +#include <linux/bitfield.h>
>   #include <linux/bitops.h>
>   #include <linux/cleanup.h>
>   #include <linux/clk.h>
> @@ -76,6 +77,8 @@
>   
>   #define UB960_NUM_BC_GPIOS		4
>   
> +#define UB960_CSI_TX0			BIT(4)

This is a very strange looking define.

> +
>   /*
>    * Register map
>    *
> @@ -114,6 +117,7 @@
>   #define UB960_SR_SCL_HIGH_TIME			0x0a
>   #define UB960_SR_SCL_LOW_TIME			0x0b
>   #define UB960_SR_RX_PORT_CTL			0x0c
> +#define UB960_SR_RX_PORT_CTL_BCC_MAP		GENMASK(7, 4)
>   #define UB960_SR_IO_CTL				0x0d
>   #define UB960_SR_GPIO_PIN_STS			0x0e
>   #define UB960_SR_GPIO_INPUT_CTL			0x0f
> @@ -589,6 +593,9 @@ struct ub960_data {
>   	u32 tx_data_rate;		/* Nominal data rate (Gb/s) */
>   	s64 tx_link_freq[1];
>   
> +	u8 rx_mask;
> +	u8 tx_mask;
> +
>   	struct i2c_atr *atr;
>   
>   	struct {
> @@ -2538,7 +2545,18 @@ static int ub960_init_rx_ports_ub960(struct ub960_data *priv)
>   	struct device *dev = &priv->client->dev;
>   	unsigned int port_lock_mask;
>   	unsigned int port_mask;
> -	int ret;
> +	u8 enabled_rxports_mask;
> +	u8 enabled_rxports;
> +	int ret = 0;
> +
> +	/* Configure I2C interface for RX ports */
> +	enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
> +	enabled_rxports = (priv->tx_mask & UB960_CSI_TX0)  ? 0x00 : enabled_rxports_mask;

tx_mask should have bits 0 or 1 set, for TX0 and TX1.

This looks quite confusing. I think the comment could at least say 
what's the strategy here, how does it configure the ports.

Isn't this just a boolean switch? If TX0 is used, all ports are routed 
there? Else TX1 is used for all ports. Why do you even need tx_mask and 
rx_mask? And why the enabled_rxports_mask. Or maybe I just fail to read 
the code, thus a better comment or patch desc might help.

> +	ret = ub960_update_bits(priv, UB960_SR_RX_PORT_CTL, enabled_rxports_mask,
> +				enabled_rxports, &ret);
> +	if (ret)
> +		return ret;
>   
>   	for_each_active_rxport(priv, it) {
>   		ret = ub960_init_rx_port_ub960(priv, it.rxport);
> @@ -4789,6 +4807,30 @@ static int ub960_parse_dt_txports(struct ub960_data *priv)
>   	return 0;
>   }
>   
> +static void ub960_parse_active_ports(struct ub960_data *priv)
> +{
> +	struct device *dev = &priv->client->dev;
> +	int nport;
> +
> +	priv->rx_mask = 0;
> +	priv->tx_mask = 0;
> +
> +	for (nport = 0; nport < priv->hw_data->num_rxports + priv->hw_data->num_txports; nport++) {
> +		struct fwnode_handle *ep_fwnode;
> +
> +		ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), nport, 0, 0);
> +		if (!ep_fwnode)
> +			continue;
> +
> +		if (nport < priv->hw_data->num_rxports)
> +			priv->rx_mask |= BIT(nport);
> +		else
> +			priv->tx_mask |= BIT(nport);
> +
> +		fwnode_handle_put(ep_fwnode);

Does this work (or rather, the user of tx_mask) on UB954, with 2 rx ports?

> +	}
> +}
> +
>   static int ub960_parse_dt(struct ub960_data *priv)
>   {
>   	int ret;
> @@ -5162,6 +5204,8 @@ static int ub960_probe(struct i2c_client *client)
>   	if (ret)
>   		goto err_mutex_destroy;
>   
> +	ub960_parse_active_ports(priv);
> +

We already parse rx and tx ports in ub960_parse_dt(). Wouldn't that be a 
better place to collect the masks? The existing functions are doing 
almost exactly what you function above does, aren't they?

>   	ret = ub960_parse_dt(priv);
>   	if (ret)
>   		goto err_disable_core_hw;

[PATCH v2 2/2] media: i2c: ds90ub960: Support multi-channel sensors

[Rishikesh Donadkar]

From: Jai Luthra <j-luthra@ti.com>

Before this, the deserializer would ignore the virtual channel (VC) ID
set in the incoming CSI packets, and set the ID to the RX port number in
the outgoing packets. This was done to support multiple single-channel
cameras, all sending packets with the same default VC ID = 0.

Now we check which (and how many) channels are under use on any incoming
FPDLink port, and map it to available channels on the outgoing CSI port.

Add a dedicated virtual channel mapping for ub9702 deserializers which
require different VC configuration compared to ub960. Update the
configuration logic to select the appropriate map based on the
deserializer type.

Signed-off-by: Jai Luthra <j-luthra@ti.com>
Co-developed-by: Rishikesh Donadkar <r-donadkar@ti.com>
Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
---
 drivers/media/i2c/ds90ub960.c | 194 +++++++++++++++++++---------------
 1 file changed, 109 insertions(+), 85 deletions(-)

diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 653dc7a4eee5..9ce6d9e6e3e8 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -72,6 +72,7 @@
 #define UB960_MAX_RX_NPORTS	4
 #define UB960_MAX_TX_NPORTS	2
 #define UB960_MAX_NPORTS	(UB960_MAX_RX_NPORTS + UB960_MAX_TX_NPORTS)
+#define UB960_MAX_VC		4
 
 #define UB960_MAX_PORT_ALIASES	8
 
@@ -3471,40 +3472,100 @@ static int ub960_rxport_handle_events(struct ub960_data *priv, u8 nport)
  */
 
 /*
- * The current implementation only supports a simple VC mapping, where all VCs
- * from a one RX port will be mapped to the same VC. Also, the hardware
- * dictates that all streams from an RX port must go to a single TX port.
+ * Map incoming streams with different virtual channels from 1-4 sensors to
+ * unique VCs on CSI TX0. Sensors using multiple VCs will work, but due to
+ * limited total channels (4) this will reduce the total number of sensors that
+ * can work simultaneously.
  *
- * This function decides the target VC numbers for each RX port with a simple
- * algorithm, so that for each TX port, we get VC numbers starting from 0,
- * and counting up.
+ * The current implementation is limited to using a single CSI TX port
+ * at a time (either TX0 or TX1 and not both simultaneously),
+ * as that is the most common HW configuration found on boards with DS90UB960.
+ * For using both CSI TX0 & TX1 the below method will need significant changes.
  *
- * E.g. if all four RX ports are in use, of which the first two go to the
- * first TX port and the secont two go to the second TX port, we would get
- * the following VCs for the four RX ports: 0, 1, 0, 1.
+ * VC mapping differs between ub960 and ub9702 deserializers:
+ * For ub960:
+ *		- Each VC uses 2 bits in the mapping register
+ *		- Supports up to 4 virtual channels (VC0-VC3)
+ * For ub9702:
+ *		- Each VC uses 4 bits in the mapping register
+ *		- Currently uses only 2 virtual channels (VC0, VC1)
  *
- * TODO: implement a more sophisticated VC mapping. As the driver cannot know
- * what VCs the sinks expect (say, an FPGA with hardcoded VC routing), this
- * probably needs to be somehow configurable. Device tree?
+ * The mapping registers determine which output VC a given input VC
+ * will be mapped to when forwarding data from the deserializer.
  */
-static void ub960_get_vc_maps(struct ub960_data *priv,
-			      struct v4l2_subdev_state *state, u8 *vc)
+
+static void ub960_get_vc_maps(struct ub960_data *priv, u8 *vc_map)
 {
-	u8 cur_vc[UB960_MAX_TX_NPORTS] = {};
-	struct v4l2_subdev_route *route;
-	u8 handled_mask = 0;
+	struct device *dev = &priv->client->dev;
+	u8 nport, available_vc = 0;
 
-	for_each_active_route(&state->routing, route) {
-		unsigned int rx, tx;
+	for (nport = 0; nport < priv->hw_data->num_rxports; ++nport) {
+		struct v4l2_mbus_frame_desc source_fd;
+		bool used_vc[UB960_MAX_VC] = {false};
+		u8 vc, cur_vc = available_vc;
+		int j, ret;
+		u8 map;
 
-		rx = ub960_pad_to_port(priv, route->sink_pad);
-		if (BIT(rx) & handled_mask)
+		if (!priv->rxports[nport])
 			continue;
 
-		tx = ub960_pad_to_port(priv, route->source_pad);
+		ret = v4l2_subdev_call(priv->rxports[nport]->source.sd, pad,
+				       get_frame_desc,
+				       priv->rxports[nport]->source.pad,
+				       &source_fd);
+		/* Mark channels used in source in used_vc[] */
+		if (!ret) {
+			for (j = 0; j < source_fd.num_entries; ++j) {
+				u8 source_vc = source_fd.entry[j].bus.csi2.vc;
+
+				if (source_vc < UB960_MAX_VC)
+					used_vc[source_vc] = true;
+			}
+		} else if (ret == -ENOIOCTLCMD) {
+			/* assume VC=0 is used if sensor driver doesn't provide info */
+			used_vc[0] = true;
+		} else {
+			continue;
+		}
+
+		/* Start with all channels mapped to first free output */
 
-		vc[rx] = cur_vc[tx]++;
-		handled_mask |= BIT(rx);
+		if (priv->hw_data->chip_type == UB960) {
+			map = (cur_vc << 6) | (cur_vc << 4) | (cur_vc << 2) |
+				(cur_vc << 0);
+		} else {
+			map = (cur_vc << 4) | (cur_vc << 0);
+		}
+
+		/* Map actually used to channels to distinct free outputs */
+		for (vc = 0; vc < UB960_MAX_VC; ++vc) {
+			if (used_vc[vc]) {
+				if (priv->hw_data->chip_type == UB960) {
+					/* For ub960: 2 bits per VC */
+					map &= ~(0x03 << (2 * vc));
+					map |= (cur_vc << (2 * vc));
+				} else {
+					/* For ub9702: 4 bits per VC */
+					map &= ~(0x0f << (4 * vc));
+					map |= (cur_vc << (4 * vc));
+				}
+				++cur_vc;
+			}
+		}
+
+		/* Don't enable port if we ran out of available channels */
+		if (cur_vc > UB960_MAX_VC) {
+			dev_err(dev,
+				"No VCs available for RX port %d\n",
+				nport);
+			continue;
+		}
+
+		/* Enable port and update map */
+		vc_map[nport] = map;
+		available_vc = cur_vc;
+		dev_dbg(dev, "%s: VC map for port %d is 0x%02x",
+			__func__, nport, map);
 	}
 }
 
@@ -3552,45 +3613,6 @@ static int ub960_disable_rx_port(struct ub960_data *priv, unsigned int nport)
 				 UB960_SR_FWD_CTL1_PORT_DIS(nport), NULL);
 }
 
-/*
- * The driver only supports using a single VC for each source. This function
- * checks that each source only provides streams using a single VC.
- */
-static int ub960_validate_stream_vcs(struct ub960_data *priv)
-{
-	for_each_active_rxport(priv, it) {
-		struct v4l2_mbus_frame_desc desc;
-		int ret;
-		u8 vc;
-
-		ret = v4l2_subdev_call(it.rxport->source.sd, pad,
-				       get_frame_desc, it.rxport->source.pad,
-				       &desc);
-		if (ret)
-			return ret;
-
-		if (desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
-			continue;
-
-		if (desc.num_entries == 0)
-			continue;
-
-		vc = desc.entry[0].bus.csi2.vc;
-
-		for (unsigned int i = 1; i < desc.num_entries; i++) {
-			if (vc == desc.entry[i].bus.csi2.vc)
-				continue;
-
-			dev_err(&priv->client->dev,
-				"rx%u: source with multiple virtual-channels is not supported\n",
-				it.nport);
-			return -ENODEV;
-		}
-	}
-
-	return 0;
-}
-
 static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 					       struct v4l2_subdev_state *state)
 {
@@ -3606,11 +3628,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 	struct v4l2_subdev_route *route;
 	int ret;
 
-	ret = ub960_validate_stream_vcs(priv);
-	if (ret)
-		return ret;
-
-	ub960_get_vc_maps(priv, state, vc_map);
+	ub960_get_vc_maps(priv, vc_map);
 
 	for_each_active_route(&state->routing, route) {
 		struct ub960_rxport *rxport;
@@ -3676,16 +3694,14 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 	for_each_active_rxport(priv, it) {
 		unsigned long nport = it.nport;
 
-		u8 vc = vc_map[nport];
-
 		if (rx_data[nport].num_streams == 0)
 			continue;
 
 		switch (it.rxport->rx_mode) {
 		case RXPORT_MODE_RAW10:
 			ub960_rxport_write(priv, nport, UB960_RR_RAW10_ID,
-				rx_data[nport].pixel_dt | (vc << UB960_RR_RAW10_ID_VC_SHIFT),
-				&ret);
+				rx_data[nport].pixel_dt | (nport << UB960_RR_RAW10_ID_VC_SHIFT),
+					&ret);
 
 			ub960_rxport_write(priv, nport,
 				UB960_RR_RAW_EMBED_DTYPE,
@@ -3701,15 +3717,10 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 
 		case RXPORT_MODE_CSI2_SYNC:
 		case RXPORT_MODE_CSI2_NONSYNC:
-			if (priv->hw_data->chip_type == UB960 ||
-			    priv->hw_data->chip_type == UB954) {
-				/* Map all VCs from this port to the same VC */
-				ub960_rxport_write(priv, nport, UB960_RR_CSI_VC_MAP,
-						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(3)) |
-						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(2)) |
-						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(1)) |
-						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(0)),
-						   &ret);
+			if (priv->hw_data->chip_type == UB960) {
+				ub960_rxport_write(priv, nport,
+						   UB960_RR_CSI_VC_MAP,
+						   vc_map[nport], &ret);
 			} else {
 				unsigned int i;
 
@@ -3717,8 +3728,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 				for (i = 0; i < 8; i++)
 					ub960_rxport_write(priv, nport,
 							   UB9702_RR_VC_ID_MAP(i),
-							   (nport << 4) | nport,
-							   &ret);
+							   vc_map[nport], &ret);
 			}
 
 			break;
@@ -3965,6 +3975,11 @@ static int ub960_set_routing(struct v4l2_subdev *sd,
 	return _ub960_set_routing(sd, state, routing);
 }
 
+static inline u8 ub960_get_output_vc(u8 map, u8 input_vc)
+{
+	return (map >> (2 * input_vc)) & 0x03;
+}
+
 static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 				struct v4l2_mbus_frame_desc *fd)
 {
@@ -3982,7 +3997,7 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 
 	state = v4l2_subdev_lock_and_get_active_state(&priv->sd);
 
-	ub960_get_vc_maps(priv, state, vc_map);
+	ub960_get_vc_maps(priv, vc_map);
 
 	for_each_active_route(&state->routing, route) {
 		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
@@ -4025,7 +4040,16 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 		fd->entry[fd->num_entries].length = source_entry->length;
 		fd->entry[fd->num_entries].pixelcode = source_entry->pixelcode;
 
-		fd->entry[fd->num_entries].bus.csi2.vc = vc_map[nport];
+		if (priv->hw_data->chip_type == UB960)
+			fd->entry[fd->num_entries].bus.csi2.vc =
+				(vc_map[nport] >> (2 * source_entry->bus.csi2.vc)) & 0x03;
+		else
+			fd->entry[fd->num_entries].bus.csi2.vc =
+				(vc_map[nport] >> (4 * source_entry->bus.csi2.vc)) & 0x0f;
+
+		dev_dbg(dev, "Mapping sink %d/%d to output VC %d",
+			route->sink_pad, route->sink_stream,
+			fd->entry[fd->num_entries].bus.csi2.vc);
 
 		if (source_fd.type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
 			fd->entry[fd->num_entries].bus.csi2.dt =
-- 
2.34.1

Re: [PATCH v2 2/2] media: i2c: ds90ub960: Support multi-channel sensors

[Tomi Valkeinen]

Hi,

On 20/07/2026 14:09, Rishikesh Donadkar wrote:
> From: Jai Luthra <j-luthra@ti.com>
> 
> Before this, the deserializer would ignore the virtual channel (VC) ID
> set in the incoming CSI packets, and set the ID to the RX port number in
> the outgoing packets. This was done to support multiple single-channel
> cameras, all sending packets with the same default VC ID = 0.
> 
> Now we check which (and how many) channels are under use on any incoming
> FPDLink port, and map it to available channels on the outgoing CSI port.
> 
> Add a dedicated virtual channel mapping for ub9702 deserializers which
> require different VC configuration compared to ub960. Update the
> configuration logic to select the appropriate map based on the
> deserializer type.

So what's the channel routing here, then? I don't think the above really 
explains it. It also makes it sound like the routing for UB9702 is 
different compared to UB960. Does that refer only to the code, but the 
result is the same, or is the routing result different?

Reading the above, and with a quick glance to the code, makes me think 
you map an incoming stream to a specific output VC, thus you can support 
four streams with this (although comment in the code says UB9702 only 
supports VC0 and VC1?). But that's not what we want. If a sensor sends 
video and embedded data, both on VC0 but with different datatypes, 
allocating a separate VC for those two streams would allow us to only 
support two cameras.

But maybe that's not what the code does, so a better explanation would 
be nice.

Also, I think this breaks UB954. You even drop one UB954 check in the 
patch, and miss checking for it in multiple places.

> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> Co-developed-by: Rishikesh Donadkar <r-donadkar@ti.com>
> Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
> ---
>   drivers/media/i2c/ds90ub960.c | 194 +++++++++++++++++++---------------
>   1 file changed, 109 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index 653dc7a4eee5..9ce6d9e6e3e8 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -72,6 +72,7 @@
>   #define UB960_MAX_RX_NPORTS	4
>   #define UB960_MAX_TX_NPORTS	2
>   #define UB960_MAX_NPORTS	(UB960_MAX_RX_NPORTS + UB960_MAX_TX_NPORTS)
> +#define UB960_MAX_VC		4
>   
>   #define UB960_MAX_PORT_ALIASES	8
>   
> @@ -3471,40 +3472,100 @@ static int ub960_rxport_handle_events(struct ub960_data *priv, u8 nport)
>    */
>   
>   /*
> - * The current implementation only supports a simple VC mapping, where all VCs
> - * from a one RX port will be mapped to the same VC. Also, the hardware
> - * dictates that all streams from an RX port must go to a single TX port.
> + * Map incoming streams with different virtual channels from 1-4 sensors to
> + * unique VCs on CSI TX0. Sensors using multiple VCs will work, but due to
> + * limited total channels (4) this will reduce the total number of sensors that
> + * can work simultaneously.
>    *
> - * This function decides the target VC numbers for each RX port with a simple
> - * algorithm, so that for each TX port, we get VC numbers starting from 0,
> - * and counting up.
> + * The current implementation is limited to using a single CSI TX port
> + * at a time (either TX0 or TX1 and not both simultaneously),
> + * as that is the most common HW configuration found on boards with DS90UB960.
> + * For using both CSI TX0 & TX1 the below method will need significant changes.
>    *
> - * E.g. if all four RX ports are in use, of which the first two go to the
> - * first TX port and the secont two go to the second TX port, we would get
> - * the following VCs for the four RX ports: 0, 1, 0, 1.
> + * VC mapping differs between ub960 and ub9702 deserializers:
> + * For ub960:
> + *		- Each VC uses 2 bits in the mapping register
> + *		- Supports up to 4 virtual channels (VC0-VC3)
> + * For ub9702:
> + *		- Each VC uses 4 bits in the mapping register
> + *		- Currently uses only 2 virtual channels (VC0, VC1)
>    *
> - * TODO: implement a more sophisticated VC mapping. As the driver cannot know
> - * what VCs the sinks expect (say, an FPGA with hardcoded VC routing), this
> - * probably needs to be somehow configurable. Device tree?
> + * The mapping registers determine which output VC a given input VC
> + * will be mapped to when forwarding data from the deserializer.
>    */
> -static void ub960_get_vc_maps(struct ub960_data *priv,
> -			      struct v4l2_subdev_state *state, u8 *vc)
> +
> +static void ub960_get_vc_maps(struct ub960_data *priv, u8 *vc_map)
>   {
> -	u8 cur_vc[UB960_MAX_TX_NPORTS] = {};
> -	struct v4l2_subdev_route *route;
> -	u8 handled_mask = 0;
> +	struct device *dev = &priv->client->dev;
> +	u8 nport, available_vc = 0;
>   
> -	for_each_active_route(&state->routing, route) {
> -		unsigned int rx, tx;
> +	for (nport = 0; nport < priv->hw_data->num_rxports; ++nport) {
> +		struct v4l2_mbus_frame_desc source_fd;
> +		bool used_vc[UB960_MAX_VC] = {false};
> +		u8 vc, cur_vc = available_vc;
> +		int j, ret;
> +		u8 map;
>   
> -		rx = ub960_pad_to_port(priv, route->sink_pad);
> -		if (BIT(rx) & handled_mask)
> +		if (!priv->rxports[nport])
>   			continue;
>   
> -		tx = ub960_pad_to_port(priv, route->source_pad);
> +		ret = v4l2_subdev_call(priv->rxports[nport]->source.sd, pad,
> +				       get_frame_desc,
> +				       priv->rxports[nport]->source.pad,
> +				       &source_fd);
> +		/* Mark channels used in source in used_vc[] */
> +		if (!ret) {
> +			for (j = 0; j < source_fd.num_entries; ++j) {
> +				u8 source_vc = source_fd.entry[j].bus.csi2.vc;
> +
> +				if (source_vc < UB960_MAX_VC)
> +					used_vc[source_vc] = true;
> +			}
> +		} else if (ret == -ENOIOCTLCMD) {
> +			/* assume VC=0 is used if sensor driver doesn't provide info */
> +			used_vc[0] = true;
> +		} else {
> +			continue;
> +		}

This silently ignores errors. Any reason why we can't return error codes?

> +
> +		/* Start with all channels mapped to first free output */
>   
> -		vc[rx] = cur_vc[tx]++;
> -		handled_mask |= BIT(rx);
> +		if (priv->hw_data->chip_type == UB960) {
> +			map = (cur_vc << 6) | (cur_vc << 4) | (cur_vc << 2) |
> +				(cur_vc << 0);
> +		} else {
> +			map = (cur_vc << 4) | (cur_vc << 0);
> +		}
> +
> +		/* Map actually used to channels to distinct free outputs */
> +		for (vc = 0; vc < UB960_MAX_VC; ++vc) {
> +			if (used_vc[vc]) {
> +				if (priv->hw_data->chip_type == UB960) {
> +					/* For ub960: 2 bits per VC */
> +					map &= ~(0x03 << (2 * vc));
> +					map |= (cur_vc << (2 * vc));
> +				} else {
> +					/* For ub9702: 4 bits per VC */
> +					map &= ~(0x0f << (4 * vc));
> +					map |= (cur_vc << (4 * vc));
> +				}
> +				++cur_vc;
> +			}
> +		}
> +
> +		/* Don't enable port if we ran out of available channels */
> +		if (cur_vc > UB960_MAX_VC) {
> +			dev_err(dev,
> +				"No VCs available for RX port %d\n",
> +				nport);
> +			continue;
> +		}
> +
> +		/* Enable port and update map */
> +		vc_map[nport] = map;
> +		available_vc = cur_vc;
> +		dev_dbg(dev, "%s: VC map for port %d is 0x%02x",
> +			__func__, nport, map);
>   	}
>   }
>   
> @@ -3552,45 +3613,6 @@ static int ub960_disable_rx_port(struct ub960_data *priv, unsigned int nport)
>   				 UB960_SR_FWD_CTL1_PORT_DIS(nport), NULL);
>   }
>   
> -/*
> - * The driver only supports using a single VC for each source. This function
> - * checks that each source only provides streams using a single VC.
> - */
> -static int ub960_validate_stream_vcs(struct ub960_data *priv)
> -{
> -	for_each_active_rxport(priv, it) {
> -		struct v4l2_mbus_frame_desc desc;
> -		int ret;
> -		u8 vc;
> -
> -		ret = v4l2_subdev_call(it.rxport->source.sd, pad,
> -				       get_frame_desc, it.rxport->source.pad,
> -				       &desc);
> -		if (ret)
> -			return ret;
> -
> -		if (desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
> -			continue;
> -
> -		if (desc.num_entries == 0)
> -			continue;
> -
> -		vc = desc.entry[0].bus.csi2.vc;
> -
> -		for (unsigned int i = 1; i < desc.num_entries; i++) {
> -			if (vc == desc.entry[i].bus.csi2.vc)
> -				continue;
> -
> -			dev_err(&priv->client->dev,
> -				"rx%u: source with multiple virtual-channels is not supported\n",
> -				it.nport);
> -			return -ENODEV;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>   static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>   					       struct v4l2_subdev_state *state)
>   {
> @@ -3606,11 +3628,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>   	struct v4l2_subdev_route *route;
>   	int ret;
>   
> -	ret = ub960_validate_stream_vcs(priv);
> -	if (ret)
> -		return ret;
> -
> -	ub960_get_vc_maps(priv, state, vc_map);
> +	ub960_get_vc_maps(priv, vc_map);
>   
>   	for_each_active_route(&state->routing, route) {
>   		struct ub960_rxport *rxport;
> @@ -3676,16 +3694,14 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>   	for_each_active_rxport(priv, it) {
>   		unsigned long nport = it.nport;
>   
> -		u8 vc = vc_map[nport];
> -
>   		if (rx_data[nport].num_streams == 0)
>   			continue;
>   
>   		switch (it.rxport->rx_mode) {
>   		case RXPORT_MODE_RAW10:
>   			ub960_rxport_write(priv, nport, UB960_RR_RAW10_ID,
> -				rx_data[nport].pixel_dt | (vc << UB960_RR_RAW10_ID_VC_SHIFT),
> -				&ret);
> +				rx_data[nport].pixel_dt | (nport << UB960_RR_RAW10_ID_VC_SHIFT),
> +					&ret);

The above changes the VC for a RAW10 mode stream to nport... Why is 
that? Also the ub960_get_frame_desc doesn't reflect this change.

>   			ub960_rxport_write(priv, nport,
>   				UB960_RR_RAW_EMBED_DTYPE,
> @@ -3701,15 +3717,10 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>   
>   		case RXPORT_MODE_CSI2_SYNC:
>   		case RXPORT_MODE_CSI2_NONSYNC:
> -			if (priv->hw_data->chip_type == UB960 ||
> -			    priv->hw_data->chip_type == UB954) {
> -				/* Map all VCs from this port to the same VC */
> -				ub960_rxport_write(priv, nport, UB960_RR_CSI_VC_MAP,
> -						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(3)) |
> -						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(2)) |
> -						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(1)) |
> -						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(0)),
> -						   &ret);
> +			if (priv->hw_data->chip_type == UB960) {
> +				ub960_rxport_write(priv, nport,
> +						   UB960_RR_CSI_VC_MAP,
> +						   vc_map[nport], &ret);
>   			} else {
>   				unsigned int i;
>   
> @@ -3717,8 +3728,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>   				for (i = 0; i < 8; i++)
>   					ub960_rxport_write(priv, nport,
>   							   UB9702_RR_VC_ID_MAP(i),
> -							   (nport << 4) | nport,
> -							   &ret);
> +							   vc_map[nport], &ret);
>   			}
>   
>   			break;
> @@ -3965,6 +3975,11 @@ static int ub960_set_routing(struct v4l2_subdev *sd,
>   	return _ub960_set_routing(sd, state, routing);
>   }
>   
> +static inline u8 ub960_get_output_vc(u8 map, u8 input_vc)
> +{
> +	return (map >> (2 * input_vc)) & 0x03;
> +}

What's this about?

> +
>   static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>   				struct v4l2_mbus_frame_desc *fd)
>   {
> @@ -3982,7 +3997,7 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>   
>   	state = v4l2_subdev_lock_and_get_active_state(&priv->sd);
>   
> -	ub960_get_vc_maps(priv, state, vc_map);
> +	ub960_get_vc_maps(priv, vc_map);
>   
>   	for_each_active_route(&state->routing, route) {
>   		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
> @@ -4025,7 +4040,16 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>   		fd->entry[fd->num_entries].length = source_entry->length;
>   		fd->entry[fd->num_entries].pixelcode = source_entry->pixelcode;
>   
> -		fd->entry[fd->num_entries].bus.csi2.vc = vc_map[nport];
> +		if (priv->hw_data->chip_type == UB960)
> +			fd->entry[fd->num_entries].bus.csi2.vc =
> +				(vc_map[nport] >> (2 * source_entry->bus.csi2.vc)) & 0x03;
> +		else
> +			fd->entry[fd->num_entries].bus.csi2.vc =
> +				(vc_map[nport] >> (4 * source_entry->bus.csi2.vc)) & 0x0f;
> +
> +		dev_dbg(dev, "Mapping sink %d/%d to output VC %d",
> +			route->sink_pad, route->sink_stream,
> +			fd->entry[fd->num_entries].bus.csi2.vc);
>   
>   		if (source_fd.type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
>   			fd->entry[fd->num_entries].bus.csi2.dt =