[PATCH 0/6] TI SN65DSI83 Features

[PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi,

the purpose of this small series is to enable the support for the reverse
lane feature and to add support for reset controllers which can drive
the enable pin.

Regards,
  Marco

Marco Felsch (6):
  drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable
  dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse
    lvds lanes
  drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes
  drm/bridge: ti-sn65dsi83: make use of dev_err_probe
  dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller
    documentation
  drm/bridge: ti-sn65dsi83: add support for a external reset controller

 .../bindings/display/bridge/ti,sn65dsi83.yaml | 64 ++++++++++++-
 drivers/gpu/drm/bridge/ti-sn65dsi83.c         | 89 +++++++++++++++++--
 2 files changed, 145 insertions(+), 8 deletions(-)

-- 
2.30.2

[PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi,

the purpose of this small series is to enable the support for the reverse
lane feature and to add support for reset controllers which can drive
the enable pin.

Regards,
  Marco

Marco Felsch (6):
  drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable
  dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse
    lvds lanes
  drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes
  drm/bridge: ti-sn65dsi83: make use of dev_err_probe
  dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller
    documentation
  drm/bridge: ti-sn65dsi83: add support for a external reset controller

 .../bindings/display/bridge/ti,sn65dsi83.yaml | 64 ++++++++++++-
 drivers/gpu/drm/bridge/ti-sn65dsi83.c         | 89 +++++++++++++++++--
 2 files changed, 145 insertions(+), 8 deletions(-)

-- 
2.30.2

[PATCH 1/6] drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable

[Marco Felsch]

No functional change. Just reuse the already existing val variable to
setup the register. This is in preparation for adding the new feature to
reverse the CHA/CHB lane orders. Without this change this call gets very
unreadable.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 2831f0813c3a..112fea004c8e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -437,11 +437,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	regmap_write(ctx->regmap, REG_LVDS_FMT, val);
 	regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
-	regmap_write(ctx->regmap, REG_LVDS_LANE,
-		     (ctx->lvds_dual_link_even_odd_swap ?
-		      REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
-		     REG_LVDS_LANE_CHA_LVDS_TERM |
-		     REG_LVDS_LANE_CHB_LVDS_TERM);
+
+	val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM;
+	if (ctx->lvds_dual_link_even_odd_swap)
+		val |= REG_LVDS_LANE_EVEN_ODD_SWAP;
+
+	regmap_write(ctx->regmap, REG_LVDS_LANE, val);
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
 
 	le16val = cpu_to_le16(mode->hdisplay);
-- 
2.30.2

[PATCH 1/6] drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable

[Marco Felsch]

No functional change. Just reuse the already existing val variable to
setup the register. This is in preparation for adding the new feature to
reverse the CHA/CHB lane orders. Without this change this call gets very
unreadable.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 2831f0813c3a..112fea004c8e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -437,11 +437,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	regmap_write(ctx->regmap, REG_LVDS_FMT, val);
 	regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
-	regmap_write(ctx->regmap, REG_LVDS_LANE,
-		     (ctx->lvds_dual_link_even_odd_swap ?
-		      REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
-		     REG_LVDS_LANE_CHA_LVDS_TERM |
-		     REG_LVDS_LANE_CHB_LVDS_TERM);
+
+	val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM;
+	if (ctx->lvds_dual_link_even_odd_swap)
+		val |= REG_LVDS_LANE_EVEN_ODD_SWAP;
+
+	regmap_write(ctx->regmap, REG_LVDS_LANE, val);
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
 
 	le16val = cpu_to_le16(mode->hdisplay);
-- 
2.30.2

[PATCH 2/6] dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes

[Marco Felsch]

The TI converter chip can swap the LVDS data lanes in a pre-defined
manner. This can be useful to improve the layout characteristic.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index 48a97bb3e2e0..7306b9874dc3 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -80,13 +80,67 @@ properties:
                   - const: 4
 
       port@2:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: Video port for LVDS Channel-A output (panel or bridge).
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                description: |
+                  Array of physical LVDS data lane indexes in reverse or normal
+                  order. Specify it in reverse order to enable the bridge CHA
+                  reverse ordering. If not specified normal order is used.
+                items:
+                  oneOf:
+                    # reverse order
+                    - items:
+                        - const: 4
+                        - const: 3
+                        - const: 2
+                        - const: 1
+                    # normal order
+                    - items:
+                        - const: 1
+                        - const: 2
+                        - const: 3
+                        - const: 4
+
       port@3:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: Video port for LVDS Channel-B output (panel or bridge).
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                description: |
+                  Array of physical LVDS data lane indexes in reverse or normal
+                  order. Specify it in reverse order to enable the bridge CHB
+                  reverse ordering. If not specified normal order is used.
+                items:
+                  oneOf:
+                    # reverse order
+                    - items:
+                        - const: 4
+                        - const: 3
+                        - const: 2
+                        - const: 1
+                    # normal order
+                    - items:
+                        - const: 1
+                        - const: 2
+                        - const: 3
+                        - const: 4
+
     required:
       - port@0
       - port@2
-- 
2.30.2

[PATCH 2/6] dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes

[Marco Felsch]

The TI converter chip can swap the LVDS data lanes in a pre-defined
manner. This can be useful to improve the layout characteristic.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index 48a97bb3e2e0..7306b9874dc3 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -80,13 +80,67 @@ properties:
                   - const: 4
 
       port@2:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: Video port for LVDS Channel-A output (panel or bridge).
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                description: |
+                  Array of physical LVDS data lane indexes in reverse or normal
+                  order. Specify it in reverse order to enable the bridge CHA
+                  reverse ordering. If not specified normal order is used.
+                items:
+                  oneOf:
+                    # reverse order
+                    - items:
+                        - const: 4
+                        - const: 3
+                        - const: 2
+                        - const: 1
+                    # normal order
+                    - items:
+                        - const: 1
+                        - const: 2
+                        - const: 3
+                        - const: 4
+
       port@3:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: Video port for LVDS Channel-B output (panel or bridge).
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                description: |
+                  Array of physical LVDS data lane indexes in reverse or normal
+                  order. Specify it in reverse order to enable the bridge CHB
+                  reverse ordering. If not specified normal order is used.
+                items:
+                  oneOf:
+                    # reverse order
+                    - items:
+                        - const: 4
+                        - const: 3
+                        - const: 2
+                        - const: 1
+                    # normal order
+                    - items:
+                        - const: 1
+                        - const: 2
+                        - const: 3
+                        - const: 4
+
     required:
       - port@0
       - port@2
-- 
2.30.2

Re: [PATCH 2/6] dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes

[Rob Herring]

On Mon, May 30, 2022 at 05:05:45PM +0200, Marco Felsch wrote:
> The TI converter chip can swap the LVDS data lanes in a pre-defined
> manner. This can be useful to improve the layout characteristic.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++-
>  1 file changed, 56 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/6] dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes

[Rob Herring]

On Mon, May 30, 2022 at 05:05:45PM +0200, Marco Felsch wrote:
> The TI converter chip can swap the LVDS data lanes in a pre-defined
> manner. This can be useful to improve the layout characteristic.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++-
>  1 file changed, 56 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 3/6] drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes

[Marco Felsch]

The chip can swap the LVDS channel A/B data lanes e.g. to improve the
layout characteristic. This commit adds the feature so the system
integrator can specify it within the device-tree.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 64 +++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 112fea004c8e..baf94b2b78a1 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -32,6 +32,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -148,6 +149,8 @@ struct sn65dsi83 {
 	int				dsi_lanes;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
+	bool				lvds_reverse_cha;
+	bool				lvds_reverse_chb;
 };
 
 static const struct regmap_range sn65dsi83_readable_ranges[] = {
@@ -441,6 +444,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 	val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM;
 	if (ctx->lvds_dual_link_even_odd_swap)
 		val |= REG_LVDS_LANE_EVEN_ODD_SWAP;
+	if (ctx->lvds_reverse_cha)
+		val |= REG_LVDS_LANE_CHA_REVERSE_LVDS;
+	if (ctx->lvds_reverse_chb)
+		val |= REG_LVDS_LANE_CHB_REVERSE_LVDS;
 
 	regmap_write(ctx->regmap, REG_LVDS_LANE, val);
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
@@ -566,6 +573,47 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
 	.atomic_get_input_bus_fmts = sn65dsi83_atomic_get_input_bus_fmts,
 };
 
+static int sn65dsi83_parse_lvds_lane_order(struct sn65dsi83 *ctx, unsigned char port)
+{
+	struct device *dev = ctx->dev;
+	struct device_node *ep;
+	int lvds_lanes;
+	int ret = 0;
+
+	if (port < 2 || port > 3)
+		return -EINVAL;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, port, 0);
+	lvds_lanes = of_property_count_u32_elems(ep, "data-lanes");
+	if (lvds_lanes == 4) {
+		u32 lane_assignments[] = { 1, 2, 3, 4 };
+
+		of_property_read_u32_array(ep, "data-lanes", lane_assignments,
+					   lvds_lanes);
+		if (lane_assignments[0] == 4 &&
+		    lane_assignments[1] == 3 &&
+		    lane_assignments[2] == 2 &&
+		    lane_assignments[3] == 1) {
+			if (port == 2)
+				ctx->lvds_reverse_cha = true;
+			else
+				ctx->lvds_reverse_chb = true;
+		} else if (lane_assignments[0] != 1 ||
+			   lane_assignments[1] != 2 ||
+			   lane_assignments[2] != 3 ||
+			   lane_assignments[3] != 4) {
+			dev_err(dev, "Unsupported LVDS lane order\n");
+			ret = -EINVAL;
+		}
+	} else if (lvds_lanes > 0) {
+		dev_err(dev, "All 4 LVDS data-lanes must be specified\n");
+		ret = -EINVAL;
+	}
+	of_node_put(ep);
+
+	return ret;
+}
+
 static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 {
 	struct drm_bridge *panel_bridge;
@@ -610,6 +658,22 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		}
 	}
 
+	/*
+	 * Todo:
+	 * Check if the reverse lane setup is working in dual-link as well.
+	 */
+	if (!ctx->lvds_dual_link) {
+		ret = sn65dsi83_parse_lvds_lane_order(ctx, 2);
+		if (ret)
+			return ret;
+
+		if (model != MODEL_SN65DSI83) {
+			ret = sn65dsi83_parse_lvds_lane_order(ctx, 3);
+			if (ret)
+				return ret;
+		}
+	}
+
 	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
 	if (IS_ERR(panel_bridge)) {
 		ret = PTR_ERR(panel_bridge);
-- 
2.30.2

[PATCH 3/6] drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes

[Marco Felsch]

The chip can swap the LVDS channel A/B data lanes e.g. to improve the
layout characteristic. This commit adds the feature so the system
integrator can specify it within the device-tree.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 64 +++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 112fea004c8e..baf94b2b78a1 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -32,6 +32,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -148,6 +149,8 @@ struct sn65dsi83 {
 	int				dsi_lanes;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
+	bool				lvds_reverse_cha;
+	bool				lvds_reverse_chb;
 };
 
 static const struct regmap_range sn65dsi83_readable_ranges[] = {
@@ -441,6 +444,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 	val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM;
 	if (ctx->lvds_dual_link_even_odd_swap)
 		val |= REG_LVDS_LANE_EVEN_ODD_SWAP;
+	if (ctx->lvds_reverse_cha)
+		val |= REG_LVDS_LANE_CHA_REVERSE_LVDS;
+	if (ctx->lvds_reverse_chb)
+		val |= REG_LVDS_LANE_CHB_REVERSE_LVDS;
 
 	regmap_write(ctx->regmap, REG_LVDS_LANE, val);
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
@@ -566,6 +573,47 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
 	.atomic_get_input_bus_fmts = sn65dsi83_atomic_get_input_bus_fmts,
 };
 
+static int sn65dsi83_parse_lvds_lane_order(struct sn65dsi83 *ctx, unsigned char port)
+{
+	struct device *dev = ctx->dev;
+	struct device_node *ep;
+	int lvds_lanes;
+	int ret = 0;
+
+	if (port < 2 || port > 3)
+		return -EINVAL;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, port, 0);
+	lvds_lanes = of_property_count_u32_elems(ep, "data-lanes");
+	if (lvds_lanes == 4) {
+		u32 lane_assignments[] = { 1, 2, 3, 4 };
+
+		of_property_read_u32_array(ep, "data-lanes", lane_assignments,
+					   lvds_lanes);
+		if (lane_assignments[0] == 4 &&
+		    lane_assignments[1] == 3 &&
+		    lane_assignments[2] == 2 &&
+		    lane_assignments[3] == 1) {
+			if (port == 2)
+				ctx->lvds_reverse_cha = true;
+			else
+				ctx->lvds_reverse_chb = true;
+		} else if (lane_assignments[0] != 1 ||
+			   lane_assignments[1] != 2 ||
+			   lane_assignments[2] != 3 ||
+			   lane_assignments[3] != 4) {
+			dev_err(dev, "Unsupported LVDS lane order\n");
+			ret = -EINVAL;
+		}
+	} else if (lvds_lanes > 0) {
+		dev_err(dev, "All 4 LVDS data-lanes must be specified\n");
+		ret = -EINVAL;
+	}
+	of_node_put(ep);
+
+	return ret;
+}
+
 static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 {
 	struct drm_bridge *panel_bridge;
@@ -610,6 +658,22 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		}
 	}
 
+	/*
+	 * Todo:
+	 * Check if the reverse lane setup is working in dual-link as well.
+	 */
+	if (!ctx->lvds_dual_link) {
+		ret = sn65dsi83_parse_lvds_lane_order(ctx, 2);
+		if (ret)
+			return ret;
+
+		if (model != MODEL_SN65DSI83) {
+			ret = sn65dsi83_parse_lvds_lane_order(ctx, 3);
+			if (ret)
+				return ret;
+		}
+	}
+
 	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
 	if (IS_ERR(panel_bridge)) {
 		ret = PTR_ERR(panel_bridge);
-- 
2.30.2

[PATCH 4/6] drm/bridge: ti-sn65dsi83: make use of dev_err_probe

[Marco Felsch]

Use the new helper to improve the debug capabilities.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index baf94b2b78a1..3c1dc16985b5 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -757,7 +757,8 @@ static int sn65dsi83_probe(struct i2c_client *client,
 	ctx->enable_gpio = devm_gpiod_get_optional(ctx->dev, "enable",
 						   GPIOD_OUT_LOW);
 	if (IS_ERR(ctx->enable_gpio))
-		return PTR_ERR(ctx->enable_gpio);
+		return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio),
+				     "Failed to get GPIO\n");
 
 	usleep_range(10000, 11000);
 
-- 
2.30.2

[PATCH 4/6] drm/bridge: ti-sn65dsi83: make use of dev_err_probe

[Marco Felsch]

Use the new helper to improve the debug capabilities.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index baf94b2b78a1..3c1dc16985b5 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -757,7 +757,8 @@ static int sn65dsi83_probe(struct i2c_client *client,
 	ctx->enable_gpio = devm_gpiod_get_optional(ctx->dev, "enable",
 						   GPIOD_OUT_LOW);
 	if (IS_ERR(ctx->enable_gpio))
-		return PTR_ERR(ctx->enable_gpio);
+		return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio),
+				     "Failed to get GPIO\n");
 
 	usleep_range(10000, 11000);
 
-- 
2.30.2

[PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Marco Felsch]

The bridge device can now also be enabled/disabled by an external reset
controller. So the device now supports either enable/disable by simple
GPIO or by an Reset-Controller.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index 7306b9874dc3..eff8360c184e 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -35,6 +35,12 @@ properties:
   vcc-supply:
     description: A 1.8V power supply (see regulator/regulator.yaml).
 
+  resets:
+    maxItems: 1
+    description: |
+      Reset specifier for bridge_en pin. This is required only if the brdige_en
+      pin is connected to a reset controller.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.30.2

[PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Marco Felsch]

The bridge device can now also be enabled/disabled by an external reset
controller. So the device now supports either enable/disable by simple
GPIO or by an Reset-Controller.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index 7306b9874dc3..eff8360c184e 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -35,6 +35,12 @@ properties:
   vcc-supply:
     description: A 1.8V power supply (see regulator/regulator.yaml).
 
+  resets:
+    maxItems: 1
+    description: |
+      Reset specifier for bridge_en pin. This is required only if the brdige_en
+      pin is connected to a reset controller.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.30.2

Re: (EXT) [PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Alexander Stein]

Hi Marco,

Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
> The bridge device can now also be enabled/disabled by an external reset
> controller. So the device now supports either enable/disable by simple
> GPIO or by an Reset-Controller.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index
> 7306b9874dc3..eff8360c184e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -35,6 +35,12 @@ properties:
>    vcc-supply:
>      description: A 1.8V power supply (see regulator/regulator.yaml).
> 
> +  resets:
> +    maxItems: 1
> +    description: |
> +      Reset specifier for bridge_en pin. This is required only if the
> brdige_en +      pin is connected to a reset controller.
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports

Maybe it should be added here, that 'resets' is an alternative to 'enable-
gpios' as both are essentially controlling the same pin from the bridge.

Best regards
Alexander

Re: (EXT) [PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Alexander Stein]

Hi Marco,

Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
> The bridge device can now also be enabled/disabled by an external reset
> controller. So the device now supports either enable/disable by simple
> GPIO or by an Reset-Controller.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index
> 7306b9874dc3..eff8360c184e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -35,6 +35,12 @@ properties:
>    vcc-supply:
>      description: A 1.8V power supply (see regulator/regulator.yaml).
> 
> +  resets:
> +    maxItems: 1
> +    description: |
> +      Reset specifier for bridge_en pin. This is required only if the
> brdige_en +      pin is connected to a reset controller.
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports

Maybe it should be added here, that 'resets' is an alternative to 'enable-
gpios' as both are essentially controlling the same pin from the bridge.

Best regards
Alexander

Re: (EXT) [PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Marco Felsch]

Hi Alexander,

On 22-05-31, Alexander Stein wrote:
> Hi Marco,
> 
> Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
> > The bridge device can now also be enabled/disabled by an external reset
> > controller. So the device now supports either enable/disable by simple
> > GPIO or by an Reset-Controller.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index
> > 7306b9874dc3..eff8360c184e 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > @@ -35,6 +35,12 @@ properties:
> >    vcc-supply:
> >      description: A 1.8V power supply (see regulator/regulator.yaml).
> > 
> > +  resets:
> > +    maxItems: 1
> > +    description: |
> > +      Reset specifier for bridge_en pin. This is required only if the
> > brdige_en +      pin is connected to a reset controller.
> > +
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> 
> Maybe it should be added here, that 'resets' is an alternative to 'enable-
> gpios' as both are essentially controlling the same pin from the bridge.

I mention the bridge_en pin two times :) But I could also add that this
is an alternative to the enable-gpios. Maybe it even more clear than.

Regards,
  Marco

> 
> Best regards
> Alexander
> 
> 
> 

Re: (EXT) [PATCH 5/6] dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation

[Marco Felsch]

Hi Alexander,

On 22-05-31, Alexander Stein wrote:
> Hi Marco,
> 
> Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
> > The bridge device can now also be enabled/disabled by an external reset
> > controller. So the device now supports either enable/disable by simple
> > GPIO or by an Reset-Controller.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml    | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index
> > 7306b9874dc3..eff8360c184e 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> > @@ -35,6 +35,12 @@ properties:
> >    vcc-supply:
> >      description: A 1.8V power supply (see regulator/regulator.yaml).
> > 
> > +  resets:
> > +    maxItems: 1
> > +    description: |
> > +      Reset specifier for bridge_en pin. This is required only if the
> > brdige_en +      pin is connected to a reset controller.
> > +
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> 
> Maybe it should be added here, that 'resets' is an alternative to 'enable-
> gpios' as both are essentially controlling the same pin from the bridge.

I mention the bridge_en pin two times :) But I could also add that this
is an alternative to the enable-gpios. Maybe it even more clear than.

Regards,
  Marco

> 
> Best regards
> Alexander
> 
> 
> 

[PATCH 6/6] drm/bridge: ti-sn65dsi83: add support for a external reset controller

[Marco Felsch]

The bridge chip has an enable gpio which of course can enable/disable
the bridge. Most the time this gpio is connected directly to the host
but sometimes it is connected to a reset controller chip and the host
controlls the reset controller chip instead. This commit adds the
support to handle that.

Therefore we need either the reset controller or a gpio to be present
and valid. The behaviour is changed in that way that a gpio or a reset
controller have to be successfully requested else the driver probe
fails, like the current behaviour.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 3c1dc16985b5..7b232a4f8bcb 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -35,6 +35,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
@@ -146,6 +147,7 @@ struct sn65dsi83 {
 	struct drm_bridge		*panel_bridge;
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
+	struct reset_control		*reset;
 	int				dsi_lanes;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
@@ -350,6 +352,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	/* Deassert reset */
 	gpiod_set_value(ctx->enable_gpio, 1);
+	reset_control_deassert(ctx->reset);
 	usleep_range(1000, 1100);
 
 	/* Get the LVDS format from the bridge state. */
@@ -511,6 +514,7 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
 
 	/* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */
 	gpiod_set_value(ctx->enable_gpio, 0);
+	reset_control_assert(ctx->reset);
 	usleep_range(10000, 11000);
 
 	ret = regulator_disable(ctx->vcc);
@@ -760,6 +764,13 @@ static int sn65dsi83_probe(struct i2c_client *client,
 		return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio),
 				     "Failed to get GPIO\n");
 
+	/* Or use a external reset chip to do so */
+	ctx->reset = devm_reset_control_get_optional(ctx->dev, NULL);
+	if (IS_ERR(ctx->reset))
+		return dev_err_probe(ctx->dev, PTR_ERR(ctx->reset),
+				     "Failed to get reset\n");
+	reset_control_assert(ctx->reset);
+
 	usleep_range(10000, 11000);
 
 	ret = sn65dsi83_parse_dt(ctx, model);
-- 
2.30.2

[PATCH 6/6] drm/bridge: ti-sn65dsi83: add support for a external reset controller

[Marco Felsch]

The bridge chip has an enable gpio which of course can enable/disable
the bridge. Most the time this gpio is connected directly to the host
but sometimes it is connected to a reset controller chip and the host
controlls the reset controller chip instead. This commit adds the
support to handle that.

Therefore we need either the reset controller or a gpio to be present
and valid. The behaviour is changed in that way that a gpio or a reset
controller have to be successfully requested else the driver probe
fails, like the current behaviour.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 3c1dc16985b5..7b232a4f8bcb 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -35,6 +35,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
@@ -146,6 +147,7 @@ struct sn65dsi83 {
 	struct drm_bridge		*panel_bridge;
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
+	struct reset_control		*reset;
 	int				dsi_lanes;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
@@ -350,6 +352,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	/* Deassert reset */
 	gpiod_set_value(ctx->enable_gpio, 1);
+	reset_control_deassert(ctx->reset);
 	usleep_range(1000, 1100);
 
 	/* Get the LVDS format from the bridge state. */
@@ -511,6 +514,7 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
 
 	/* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */
 	gpiod_set_value(ctx->enable_gpio, 0);
+	reset_control_assert(ctx->reset);
 	usleep_range(10000, 11000);
 
 	ret = regulator_disable(ctx->vcc);
@@ -760,6 +764,13 @@ static int sn65dsi83_probe(struct i2c_client *client,
 		return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio),
 				     "Failed to get GPIO\n");
 
+	/* Or use a external reset chip to do so */
+	ctx->reset = devm_reset_control_get_optional(ctx->dev, NULL);
+	if (IS_ERR(ctx->reset))
+		return dev_err_probe(ctx->dev, PTR_ERR(ctx->reset),
+				     "Failed to get reset\n");
+	reset_control_assert(ctx->reset);
+
 	usleep_range(10000, 11000);
 
 	ret = sn65dsi83_parse_dt(ctx, model);
-- 
2.30.2

Re: [PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi all,

gentle ping on this series since I didn't received much feedback for the
code.

Regards,
  Marco

On 22-05-30, Marco Felsch wrote:
> Hi,
> 
> the purpose of this small series is to enable the support for the reverse
> lane feature and to add support for reset controllers which can drive
> the enable pin.
> 
> Regards,
>   Marco
> 
> Marco Felsch (6):
>   drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable
>   dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse
>     lvds lanes
>   drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes
>   drm/bridge: ti-sn65dsi83: make use of dev_err_probe
>   dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller
>     documentation
>   drm/bridge: ti-sn65dsi83: add support for a external reset controller
> 
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 64 ++++++++++++-
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c         | 89 +++++++++++++++++--
>  2 files changed, 145 insertions(+), 8 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

Re: [PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi all,

gentle ping on this series since I didn't received much feedback for the
code.

Regards,
  Marco

On 22-05-30, Marco Felsch wrote:
> Hi,
> 
> the purpose of this small series is to enable the support for the reverse
> lane feature and to add support for reset controllers which can drive
> the enable pin.
> 
> Regards,
>   Marco
> 
> Marco Felsch (6):
>   drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable
>   dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse
>     lvds lanes
>   drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes
>   drm/bridge: ti-sn65dsi83: make use of dev_err_probe
>   dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller
>     documentation
>   drm/bridge: ti-sn65dsi83: add support for a external reset controller
> 
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 64 ++++++++++++-
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c         | 89 +++++++++++++++++--
>  2 files changed, 145 insertions(+), 8 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

Re: [PATCH 0/6] TI SN65DSI83 Features

[Robert Foss]

Hey Marco,

Sorry about the slow reply.

This series no longer applies on drm-misc-next. Can you rebase this
series and send it out a v2?


Rob.

Re: [PATCH 0/6] TI SN65DSI83 Features

[Robert Foss]

Hey Marco,

Sorry about the slow reply.

This series no longer applies on drm-misc-next. Can you rebase this
series and send it out a v2?


Rob.

Re: [PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi Robert,

On 22-08-29, Robert Foss wrote:
> Hey Marco,
> 
> Sorry about the slow reply.
> 
> This series no longer applies on drm-misc-next. Can you rebase this
> series and send it out a v2?

I can but I would rather prefer to get some feedback first. Right now I
just got a minor documentation issue feedback from Alexander.

Regards,
  Marco

Re: [PATCH 0/6] TI SN65DSI83 Features

[Marco Felsch]

Hi Robert,

On 22-08-29, Robert Foss wrote:
> Hey Marco,
> 
> Sorry about the slow reply.
> 
> This series no longer applies on drm-misc-next. Can you rebase this
> series and send it out a v2?

I can but I would rather prefer to get some feedback first. Right now I
just got a minor documentation issue feedback from Alexander.

Regards,
  Marco