[PATCH v5 0/3] drm/bridge: ti-sn65dsi83: Add reverse lvds lanes support

[PATCH v5 0/3] drm/bridge: ti-sn65dsi83: Add reverse lvds lanes support

[Wojciech Dubowik]

From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

Add support for reversed lvds output lanes. With an optional
data-lanes property one can support default layout <1 2 3 4>
or reversed layout <4 3 2 1>. The property is optional and when
not set it keeps the default output layout.

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
Changes in v5:
- Add an extra patch to simplify error handling in lvds DT parsing
  suggested by Luca
- Fix formatting and description in dt-bindings
- Make naming consistent for lane mappings constants
- Simplify parsing of data-lanes
Changes in v4:
- Fix data-lanes bindings to be under endpoint subnode and not in
  the node itself, discovered by shashiko-reviews
Changes in v3:
- Add optional data-lanes bindings for output nodes
Changes in v2:
- Parse existing data-lanes property instead of ading new DT
  bindings
---
Wojciech Dubowik (3):
  drm/bridge: ti-sn65dsi83: Simplify error condition logic
  dt-bindings: display: sn65dsi83: Add output data-lanes property
  drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

 .../bindings/display/bridge/ti,sn65dsi83.yaml | 44 ++++++++++++
 drivers/gpu/drm/bridge/ti-sn65dsi83.c         | 72 +++++++++++--------
 2 files changed, 87 insertions(+), 29 deletions(-)

-- 
2.47.3

[PATCH v5 1/3] drm/bridge: ti-sn65dsi83: Simplify error condition logic

[Wojciech Dubowik]

From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

Simplify error condition logic in lvds device tree property parsing. It
makes it easier to add extra properties without handling complex goto
entries.

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 43 ++++++++-------------------
 1 file changed, 13 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..f8a786953526 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -825,48 +825,35 @@ static int sn65dsi83_select_lvds_vod_swing(struct device *dev,
 static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
 {
 	struct device *dev = ctx->dev;
-	struct device_node *endpoint;
-	int endpoint_reg;
+	int endpoint_reg = (channel == CHANNEL_A) ? 2 : 3;
+	struct device_node *endpoint __free(device_node) =
+		of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
 	/* Set so the property can be freely selected if not defined */
 	u32 lvds_vod_swing_data[2] = { 0, 1000000 };
 	u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
 	/* Set default near end terminataion to 200 Ohm */
 	u32 lvds_term = 200;
 	int lvds_vod_swing_conf;
-	int ret = 0;
 	int ret_data;
 	int ret_clock;
 
-	if (channel == CHANNEL_A)
-		endpoint_reg = 2;
-	else
-		endpoint_reg = 3;
-
-	endpoint = of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
-
 	of_property_read_u32(endpoint, "ti,lvds-termination-ohms", &lvds_term);
 	if (lvds_term == 100)
 		ctx->lvds_term_conf[channel] = OHM_100;
 	else if (lvds_term == 200)
 		ctx->lvds_term_conf[channel] = OHM_200;
-	else {
-		ret = -EINVAL;
-		goto exit;
-	}
+	else
+		return -EINVAL;
 
 	ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
 					lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));
-	if (ret_data != 0 && ret_data != -EINVAL) {
-		ret = ret_data;
-		goto exit;
-	}
+	if (ret_data != 0 && ret_data != -EINVAL)
+		return ret_data;
 
 	ret_clock = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-clock-microvolt",
 					lvds_vod_swing_clk, ARRAY_SIZE(lvds_vod_swing_clk));
-	if (ret_clock != 0 && ret_clock != -EINVAL) {
-		ret = ret_clock;
-		goto exit;
-	}
+	if (ret_clock != 0 && ret_clock != -EINVAL)
+		return ret_clock;
 
 	/* Use default value if both properties are NOT defined. */
 	if (ret_data == -EINVAL && ret_clock == -EINVAL)
@@ -876,17 +863,13 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
 	if (!ret_data || !ret_clock) {
 		lvds_vod_swing_conf = sn65dsi83_select_lvds_vod_swing(dev, lvds_vod_swing_data,
 						lvds_vod_swing_clk, ctx->lvds_term_conf[channel]);
-		if (lvds_vod_swing_conf < 0) {
-			ret = lvds_vod_swing_conf;
-			goto exit;
-		}
+		if (lvds_vod_swing_conf < 0)
+			return lvds_vod_swing_conf;
 	}
 
 	ctx->lvds_vod_swing_conf[channel] = lvds_vod_swing_conf;
-	ret = 0;
-exit:
-	of_node_put(endpoint);
-	return ret;
+
+	return 0;
 }
 
 static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
-- 
2.47.3

[PATCH v5 2/3] dt-bindings: display: sn65dsi83: Add output data-lanes property

[Wojciech Dubowik]

From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

Add an optional output lvds data lanes property with two allowed values.
The array <1 2 3 4> for standard layout and <4 3 2 1> for reversed lvds
output lanes. The latter informs the driver that reverse lvds config
option has to be set in config register for the respective output channel.

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
 .../bindings/display/bridge/ti,sn65dsi83.yaml | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index e69b6343a8eb..4000bf0b1370 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -86,10 +86,54 @@ properties:
         description: Video port for LVDS Channel-A output (panel or bridge).
         $ref: '#/$defs/lvds-port'
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description:
+                  Array of physical LVDS data lane indexes
+                oneOf:
+                  - items:
+                      - const: 1
+                      - const: 2
+                      - const: 3
+                      - const: 4
+                  - items:
+                      - const: 4
+                      - const: 3
+                      - const: 2
+                      - const: 1
+
       port@3:
         description: Video port for LVDS Channel-B output (panel or bridge).
         $ref: '#/$defs/lvds-port'
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description:
+                  Array of physical LVDS data lane indexes
+                oneOf:
+                  - items:
+                      - const: 1
+                      - const: 2
+                      - const: 3
+                      - const: 4
+                  - items:
+                      - const: 4
+                      - const: 3
+                      - const: 2
+                      - const: 1
+
     required:
       - port@0
       - port@2
-- 
2.47.3

Re: [PATCH v5 2/3] dt-bindings: display: sn65dsi83: Add output data-lanes property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Redefining `endpoint` with `unevaluatedProperties: false` locally masks properties evaluated by the sibling `$ref: '#/$defs/lvds-port'`, causing valid TI-specific LVDS properties to be improperly rejected.
--

--- Patch [2]: [PATCH v5 2/3] dt-bindings: display: sn65dsi83: Add output data-lanes property ---
commit c64bd236e14a70bf7062c9fee49e33ef3475908b
Author: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

dt-bindings: display: sn65dsi83: Add output data-lanes property

Add an optional output LVDS data lanes property with two allowed values
for standard and reversed layouts. The reversed layout informs the driver
that the reverse LVDS config option must be set for the respective output
channel.

> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> index e69b6343a8ebb..4000bf0b13709 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -86,10 +86,54 @@ properties:
>          description: Video port for LVDS Channel-A output (panel or bridge).
>          $ref: '#/$defs/lvds-port'
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false

[Severity: Medium]
Will this unevaluatedProperties constraint cause valid properties inherited
from the sibling '$ref: '#/$defs/lvds-port'' reference to be improperly
rejected?

Since unevaluatedProperties is scoped only to this inline schema, it cannot
see annotations from the lvds-port definition. If a device tree file defines
port@2 with both data-lanes and properties like ti,lvds-termination-ohms,
could this result in a validation failure when running make dtbs_check?

[ ... ]

>        port@3:
>          description: Video port for LVDS Channel-B output (panel or bridge).
>          $ref: '#/$defs/lvds-port'
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false

[Severity: Medium]
Similar to port@2 above, could this locally scoped unevaluatedProperties: false
block reject valid properties inherited from the lvds-port definition on
port@3?

> +
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717115002.308035-1-wojciech.dubowik@mt.com?part=2

[PATCH v5 3/3] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

[Wojciech Dubowik]

From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

The chip supports output lvds lanes in two orders, the default <1 2 3 4>
and <4 3 2 1>. Add parsing of an optional output lvds data-lanes property
so we can inform chip that the lanes have been reversed.

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 31 +++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index f8a786953526..df44df2e872a 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -148,6 +148,18 @@ enum sn65dsi83_lvds_term {
 	OHM_200
 };
 
+enum {
+	LANE_MAPPING_NORMAL,
+	LANE_MAPPING_REVERSE,
+};
+
+#define DATA_LANES_COUNT	4
+
+static const u32 supported_data_lane_mapping[][DATA_LANES_COUNT] = {
+	[LANE_MAPPING_NORMAL] = { 1, 2, 3, 4 },
+	[LANE_MAPPING_REVERSE] = { 4, 3, 2, 1},
+};
+
 enum sn65dsi83_model {
 	MODEL_SN65DSI83,
 	MODEL_SN65DSI84,
@@ -163,6 +175,7 @@ struct sn65dsi83 {
 	struct regulator		*vcc;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
+	bool				lvds_reverse_lanes_conf[2];
 	int				lvds_vod_swing_conf[2];
 	int				lvds_term_conf[2];
 	int				irq;
@@ -644,6 +657,10 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 	regmap_write(ctx->regmap, REG_LVDS_LANE,
 		     (ctx->lvds_dual_link_even_odd_swap ?
 		      REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
+		     (ctx->lvds_reverse_lanes_conf[CHANNEL_A] ?
+		      REG_LVDS_LANE_CHA_REVERSE_LVDS : 0) |
+		     (ctx->lvds_reverse_lanes_conf[CHANNEL_B] ?
+		      REG_LVDS_LANE_CHB_REVERSE_LVDS : 0) |
 		     (ctx->lvds_term_conf[CHANNEL_A] ?
 			  REG_LVDS_LANE_CHA_LVDS_TERM : 0) |
 		     (ctx->lvds_term_conf[CHANNEL_B] ?
@@ -833,6 +850,7 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
 	u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
 	/* Set default near end terminataion to 200 Ohm */
 	u32 lvds_term = 200;
+	u32 data_lanes[DATA_LANES_COUNT];
 	int lvds_vod_swing_conf;
 	int ret_data;
 	int ret_clock;
@@ -845,6 +863,19 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
 	else
 		return -EINVAL;
 
+	ret_data = of_property_read_u32_array(endpoint, "data-lanes", data_lanes,
+			ARRAY_SIZE(data_lanes));
+	if (!ret_data) {
+		if (memcmp(data_lanes,
+				supported_data_lane_mapping[LANE_MAPPING_REVERSE],
+				ARRAY_SIZE(data_lanes)) == 0)
+			ctx->lvds_reverse_lanes_conf[channel] = true;
+		else if (memcmp(data_lanes,
+			   supported_data_lane_mapping[LANE_MAPPING_NORMAL],
+			   ARRAY_SIZE(data_lanes)) != 0)
+			return dev_err_probe(dev, -EINVAL, "invalid data lanes mapping\n");
+	}
+
 	ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
 					lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));
 	if (ret_data != 0 && ret_data != -EINVAL)
-- 
2.47.3

Re: [PATCH v5 3/3] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `memcmp()` is called with `ARRAY_SIZE(data_lanes)` instead of `sizeof(data_lanes)`, causing it to compare only the first element of the `u32` array.
--

--- Patch [3]: [PATCH v5 3/3] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support ---
commit 5b0901160799f83c03f127b68580169dd845ac90
Author: Wojciech Dubowik <Wojciech.Dubowik@mt.com>

drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

This commit adds support for parsing an optional "data-lanes" device tree
property. It allows the driver to configure the ti-sn65dsi83 chip for
reversed output LVDS lane order.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index f8a786953526d..df44df2e872af 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c

[ ... ]

> @@ -845,6 +863,19 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
>  	else
>  		return -EINVAL;
>  
> +	ret_data = of_property_read_u32_array(endpoint, "data-lanes", data_lanes,
> +			ARRAY_SIZE(data_lanes));
> +	if (!ret_data) {
> +		if (memcmp(data_lanes,
> +				supported_data_lane_mapping[LANE_MAPPING_REVERSE],
> +				ARRAY_SIZE(data_lanes)) == 0)

[Severity: Medium]
Does this code compare the entire array? memcmp() expects the length in
bytes, but ARRAY_SIZE(data_lanes) evaluates to 4 (the number of elements).

Because data_lanes is an array of u32, checking only 4 bytes means only
the first element is compared. Should this be using sizeof(data_lanes)
instead?

> +			ctx->lvds_reverse_lanes_conf[channel] = true;
> +		else if (memcmp(data_lanes,
> +			   supported_data_lane_mapping[LANE_MAPPING_NORMAL],
> +			   ARRAY_SIZE(data_lanes)) != 0)

[Severity: Medium]
The same question applies here. Because only the first element is verified,
would an invalid device tree configuration like <4 0 0 0> erroneously match
LANE_MAPPING_REVERSE and bypass the expected validation?

> +			return dev_err_probe(dev, -EINVAL, "invalid data lanes mapping\n");
> +	}
> +
>  	ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
>  					lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717115002.308035-1-wojciech.dubowik@mt.com?part=3