[PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Tony Lindgren]

The tc358765 is similar to tc358775 except for the stdby-gpios.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../display/bridge/toshiba,tc358775.yaml        | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Vinay Simha BN <simhavcs@gmail.com>
 
 description: |
-  This binding supports DSI to LVDS bridge TC358775
+  This binding supports DSI to LVDS bridges TC358765 and TC358775
 
   MIPI DSI-RX Data 4-lane, CLK 1-lane with data rates up to 800 Mbps/lane.
   Video frame size:
@@ -21,7 +21,9 @@ description: |
 
 properties:
   compatible:
-    const: toshiba,tc358775
+    enum:
+      - toshiba,tc358765
+      - toshiba,tc358775
 
   reg:
     maxItems: 1
@@ -70,12 +72,21 @@ required:
   - reg
   - vdd-supply
   - vddio-supply
-  - stby-gpios
   - reset-gpios
   - ports
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: toshiba,tc358775
+    then:
+      required:
+        - stby-gpios
+
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
-- 
2.42.1

[PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Tony Lindgren]

The tc358765 is similar to tc358775 except for the stdby-gpios.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../display/bridge/toshiba,tc358775.yaml        | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Vinay Simha BN <simhavcs@gmail.com>
 
 description: |
-  This binding supports DSI to LVDS bridge TC358775
+  This binding supports DSI to LVDS bridges TC358765 and TC358775
 
   MIPI DSI-RX Data 4-lane, CLK 1-lane with data rates up to 800 Mbps/lane.
   Video frame size:
@@ -21,7 +21,9 @@ description: |
 
 properties:
   compatible:
-    const: toshiba,tc358775
+    enum:
+      - toshiba,tc358765
+      - toshiba,tc358775
 
   reg:
     maxItems: 1
@@ -70,12 +72,21 @@ required:
   - reg
   - vdd-supply
   - vddio-supply
-  - stby-gpios
   - reset-gpios
   - ports
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: toshiba,tc358775
+    then:
+      required:
+        - stby-gpios
+
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
-- 
2.42.1

[PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

The current code assume hardcoded dsi host endpoint 1, which may not
be the case. Let's fix that and simplify the code by getting the dsi
endpoint with of_graph_get_remote_endpoint() that does not assume any
endpoint numbering.

Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -528,25 +528,17 @@ tc_mode_valid(struct drm_bridge *bridge,
 static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
 {
 	struct device_node *endpoint;
-	struct device_node *parent;
 	struct device_node *remote;
 	int dsi_lanes = -1;
 
-	/*
-	 * To get the data-lanes of dsi, we need to access the dsi0_out of port1
-	 *  of dsi0 endpoint from bridge port0 of d2l_in
-	 */
 	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
 						 TC358775_DSI_IN, -1);
 	if (endpoint) {
-		/* dsi0_out node */
-		parent = of_graph_get_remote_port_parent(endpoint);
+		/* Get the configured data lanes on the dsi host side */
+		remote = of_graph_get_remote_endpoint(endpoint);
 		of_node_put(endpoint);
-		if (parent) {
-			/* dsi0 port 1 */
-			dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, -1, 1, 4);
-			of_node_put(parent);
-		}
+		dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
+		of_node_put(remote);
 	}
 
 	if (dsi_lanes < 0)
-- 
2.42.1

[PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

The current code assume hardcoded dsi host endpoint 1, which may not
be the case. Let's fix that and simplify the code by getting the dsi
endpoint with of_graph_get_remote_endpoint() that does not assume any
endpoint numbering.

Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -528,25 +528,17 @@ tc_mode_valid(struct drm_bridge *bridge,
 static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
 {
 	struct device_node *endpoint;
-	struct device_node *parent;
 	struct device_node *remote;
 	int dsi_lanes = -1;
 
-	/*
-	 * To get the data-lanes of dsi, we need to access the dsi0_out of port1
-	 *  of dsi0 endpoint from bridge port0 of d2l_in
-	 */
 	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
 						 TC358775_DSI_IN, -1);
 	if (endpoint) {
-		/* dsi0_out node */
-		parent = of_graph_get_remote_port_parent(endpoint);
+		/* Get the configured data lanes on the dsi host side */
+		remote = of_graph_get_remote_endpoint(endpoint);
 		of_node_put(endpoint);
-		if (parent) {
-			/* dsi0 port 1 */
-			dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, -1, 1, 4);
-			of_node_put(parent);
-		}
+		dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
+		of_node_put(remote);
 	}
 
 	if (dsi_lanes < 0)
-- 
2.42.1

[PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support

[Tony Lindgren]

The jeida-24 register values are the default hardware settings, but they
not listed in the driver. Let's add them.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -458,8 +458,18 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 	 * Default hardware register settings of tc358775 configured
 	 * with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
 	 */
-	if (connector->display_info.bus_formats[0] ==
-		MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
+	switch (connector->display_info.bus_formats[0]) {
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+		/* JEIDA-24 */
+		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
+		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
+		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
+		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
+		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
+		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
+		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
 		/* VESA-24 */
 		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
 		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
@@ -468,7 +478,9 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
 		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
 		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
-	} else { /*  MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+		/* JEIDA-18 */
 		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
 		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
 		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
@@ -476,6 +488,9 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
 		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
 		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
+		break;
+	default:
+		break;
 	}
 
 	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
-- 
2.42.1

[PATCH 4/6] drm/bridge: tc358775: Add burst and low-power modes

[Tony Lindgren]

Burst and low-power modes are supported both for tc358765 and tc358775.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -632,7 +632,8 @@ static int tc_attach_host(struct tc_data *tc)
 
 	dsi->lanes = tc->num_dsi_lanes;
 	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		MIPI_DSI_MODE_LPM;
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0) {
-- 
2.42.1

[PATCH 5/6] drm/bridge: tc358775: Add support for tc358765

[Tony Lindgren]

Looks like tc358775 is pin compatible with earlier tc358765 according to
the tc358774xbg_datasheet_en_20190118.pdf documentation. On tc358775,
there is an extra GPIO for STBY pin added, and few other improvments.

The tc358765 has a register bit for video event mode that we must set for
the LCD output to work. On tc358775, this bit no longer exists.

Looks like the registers seem to match otherwise based on a quick glance
comparing the defines to the earlier Android kernel tc358765 driver.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 55 +++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/media-bus-format.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -107,6 +108,7 @@
 #define RDPKTLN         0x0404  /* Command Read Packet Length */
 
 #define VPCTRL          0x0450  /* Video Path Control */
+#define EVTMODE		BIT(5)  /* Video event mode enable, tc35876x only */
 #define HTIM1           0x0454  /* Horizontal Timing Control 1 */
 #define HTIM2           0x0458  /* Horizontal Timing Control 2 */
 #define VTIM1           0x045C  /* Vertical Timing Control 1 */
@@ -254,6 +256,11 @@ enum tc358775_ports {
 	TC358775_LVDS_OUT1,
 };
 
+enum tc3587x5_type {
+	TC358765,
+	TC358775,
+};
+
 struct tc_data {
 	struct i2c_client	*i2c;
 	struct device		*dev;
@@ -271,6 +278,8 @@ struct tc_data {
 	struct gpio_desc	*stby_gpio;
 	u8			lvds_link; /* single-link or dual-link */
 	u8			bpc;
+
+	enum tc3587x5_type	type;
 };
 
 static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
@@ -294,8 +303,10 @@ static void tc_bridge_pre_enable(struct drm_bridge *bridge)
 		dev_err(dev, "regulator vdd enable failed, %d\n", ret);
 	usleep_range(10000, 11000);
 
-	gpiod_set_value(tc->stby_gpio, 0);
-	usleep_range(10000, 11000);
+	if (!IS_ERR(tc->stby_gpio)) {
+		gpiod_set_value(tc->stby_gpio, 0);
+		usleep_range(10000, 11000);
+	}
 
 	gpiod_set_value(tc->reset_gpio, 0);
 	usleep_range(10, 20);
@@ -310,8 +321,10 @@ static void tc_bridge_post_disable(struct drm_bridge *bridge)
 	gpiod_set_value(tc->reset_gpio, 1);
 	usleep_range(10, 20);
 
-	gpiod_set_value(tc->stby_gpio, 1);
-	usleep_range(10000, 11000);
+	if (!IS_ERR(tc->stby_gpio)) {
+		gpiod_set_value(tc->stby_gpio, 1);
+		usleep_range(10000, 11000);
+	}
 
 	ret = regulator_disable(tc->vdd);
 	if (ret < 0)
@@ -424,10 +437,16 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 	d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
 	d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
 
+	/* Video event mode, this bit does not exist for tc358775 */
+	if (tc->type == TC358765)
+		val = EVTMODE;
+	else
+		val = 0;
+
 	if (tc->bpc == 8)
-		val = TC358775_VPCTRL_OPXLFMT(1);
+		val |= TC358775_VPCTRL_OPXLFMT(1);
 	else /* bpc = 6; */
-		val = TC358775_VPCTRL_MSF(1);
+		val |= TC358775_VPCTRL_MSF(1);
 
 	dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
 	clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 : DIVIDE_BY_3);
@@ -656,6 +675,7 @@ static int tc_probe(struct i2c_client *client)
 
 	tc->dev = dev;
 	tc->i2c = client;
+	tc->type = (enum tc3587x5_type)of_device_get_match_data(dev);
 
 	tc->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
 						  TC358775_LVDS_OUT0, 0);
@@ -680,11 +700,15 @@ static int tc_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	tc->stby_gpio = devm_gpiod_get(dev, "stby", GPIOD_OUT_HIGH);
-	if (IS_ERR(tc->stby_gpio)) {
-		ret = PTR_ERR(tc->stby_gpio);
-		dev_err(dev, "cannot get stby-gpio %d\n", ret);
-		return ret;
+	if (tc->type == TC358775) {
+		tc->stby_gpio = devm_gpiod_get(dev, "stby", GPIOD_OUT_HIGH);
+		if (IS_ERR(tc->stby_gpio)) {
+			ret = PTR_ERR(tc->stby_gpio);
+			dev_err(dev, "cannot get stby-gpio %d\n", ret);
+			return ret;
+		}
+	} else {
+		tc->stby_gpio = ERR_PTR(-ENODEV);
 	}
 
 	tc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
@@ -696,6 +720,7 @@ static int tc_probe(struct i2c_client *client)
 
 	tc->bridge.funcs = &tc_bridge_funcs;
 	tc->bridge.of_node = dev->of_node;
+	tc->bridge.pre_enable_prev_first = true;
 	drm_bridge_add(&tc->bridge);
 
 	i2c_set_clientdata(client, tc);
@@ -715,17 +740,21 @@ static void tc_remove(struct i2c_client *client)
 {
 	struct tc_data *tc = i2c_get_clientdata(client);
 
+	mipi_dsi_detach(tc->dsi);
 	drm_bridge_remove(&tc->bridge);
+	tc->bridge.funcs = NULL;
 }
 
 static const struct i2c_device_id tc358775_i2c_ids[] = {
-	{ "tc358775", 0 },
+	{ "tc358765", TC358765, },
+	{ "tc358775", TC358775, },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
 
 static const struct of_device_id tc358775_of_ids[] = {
-	{ .compatible = "toshiba,tc358775", },
+	{ .compatible = "toshiba,tc358765", .data = (void *)TC358765, },
+	{ .compatible = "toshiba,tc358775", .data = (void *)TC358775, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tc358775_of_ids);
-- 
2.42.1

[PATCH 6/6] drm/bridge: tc358775: Configure hs_rate and lp_rate

[Tony Lindgren]

The hs_rate and lp_rate may be used by the dsi host for timing
calculations. The tc358775 has a maximum bit rate of 1 Gbps/lane,
tc358765 has maximurate of 800 Mbps per lane.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/bridge/tc358775.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -653,6 +653,11 @@ static int tc_attach_host(struct tc_data *tc)
 	dsi->format = MIPI_DSI_FMT_RGB888;
 	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
 		MIPI_DSI_MODE_LPM;
+	if (tc->type == TC358765)
+		dsi->hs_rate = 800000000;
+	else
+		dsi->hs_rate = 1000000000;
+	dsi->lp_rate = 10000000;
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0) {
-- 
2.42.1

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Krzysztof Kozlowski]

On 26/11/2023 17:32, Tony Lindgren wrote:
> The tc358765 is similar to tc358775 except for the stdby-gpios.
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Krzysztof Kozlowski]

On 26/11/2023 17:32, Tony Lindgren wrote:
> The tc358765 is similar to tc358775 except for the stdby-gpios.
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Michael Walle]

Hi,

> The tc358765 is similar to tc358775 except for the stdby-gpios.

Bad timing (for me). I'm about to send a bigger patch series for the
tc358775 which fixes the (completely) broken initialialization. And also
contains some of your fixes.

That being said, I intend to make the standby gpio optional also for the
tc358755, because it might just be hardwired on the board.

But second, I'm really curious if this bridge is working for you correctly
as it is at the moment. One particular thing I've noticed is that you must
release the reset while both the clock and the data lanes are in LP11 mode.
Otherwise, the bridge won't work properly (i.e. horizontally shifted
picture, or no picture at all).

What DSI host controller are you using?

-michael

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Michael Walle]

Hi,

> The tc358765 is similar to tc358775 except for the stdby-gpios.

Bad timing (for me). I'm about to send a bigger patch series for the
tc358775 which fixes the (completely) broken initialialization. And also
contains some of your fixes.

That being said, I intend to make the standby gpio optional also for the
tc358755, because it might just be hardwired on the board.

But second, I'm really curious if this bridge is working for you correctly
as it is at the moment. One particular thing I've noticed is that you must
release the reset while both the clock and the data lanes are in LP11 mode.
Otherwise, the bridge won't work properly (i.e. horizontally shifted
picture, or no picture at all).

What DSI host controller are you using?

-michael

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 12:44]:
> Hi,
> 
> > The tc358765 is similar to tc358775 except for the stdby-gpios.
> 
> Bad timing (for me). I'm about to send a bigger patch series for the
> tc358775 which fixes the (completely) broken initialialization. And also
> contains some of your fixes.

OK cool, let's merge patches as needed then. Maybe ack the patches that
can already be merged, and put together some merged set for the rest of
the patches? Or let me know what you prefer, I'll be glad to test and
tweak as needed for tc358765.

> That being said, I intend to make the standby gpio optional also for the
> tc358755, because it might just be hardwired on the board.

OK that sounds good to me.

> But second, I'm really curious if this bridge is working for you correctly
> as it is at the moment. One particular thing I've noticed is that you must
> release the reset while both the clock and the data lanes are in LP11 mode.
> Otherwise, the bridge won't work properly (i.e. horizontally shifted
> picture, or no picture at all).

Yes things are working for me. But then again the bridge is initialized
by the bootloader, and then Android kernel, and then I kexec to the
mainline kernel. And this is with a tc358765 if that might make a
difference.

So I have not seen your reset issue. Andreas may be seeing it on another
tc bridge variant though, so adding Andreas to the loop also.

> What DSI host controller are you using?

This is an old motorola mz617 tablet with a tc358765 bridge, so omapdrm.
I do have one pending patch for omapdrm dsi to change the init order a
bit so tc358765 probes reliably. But seems that's a separate issue from
your reset issue, I'll post that to the lists too to discuss.

What's the dsi host you have? Do you also have some separate i2c eeprom
that's needed for initializing something or is that lcd panel specific?
No idea what the eeprom is really doing here..

Regards,

Tony

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 12:44]:
> Hi,
> 
> > The tc358765 is similar to tc358775 except for the stdby-gpios.
> 
> Bad timing (for me). I'm about to send a bigger patch series for the
> tc358775 which fixes the (completely) broken initialialization. And also
> contains some of your fixes.

OK cool, let's merge patches as needed then. Maybe ack the patches that
can already be merged, and put together some merged set for the rest of
the patches? Or let me know what you prefer, I'll be glad to test and
tweak as needed for tc358765.

> That being said, I intend to make the standby gpio optional also for the
> tc358755, because it might just be hardwired on the board.

OK that sounds good to me.

> But second, I'm really curious if this bridge is working for you correctly
> as it is at the moment. One particular thing I've noticed is that you must
> release the reset while both the clock and the data lanes are in LP11 mode.
> Otherwise, the bridge won't work properly (i.e. horizontally shifted
> picture, or no picture at all).

Yes things are working for me. But then again the bridge is initialized
by the bootloader, and then Android kernel, and then I kexec to the
mainline kernel. And this is with a tc358765 if that might make a
difference.

So I have not seen your reset issue. Andreas may be seeing it on another
tc bridge variant though, so adding Andreas to the loop also.

> What DSI host controller are you using?

This is an old motorola mz617 tablet with a tc358765 bridge, so omapdrm.
I do have one pending patch for omapdrm dsi to change the init order a
bit so tc358765 probes reliably. But seems that's a separate issue from
your reset issue, I'll post that to the lists too to discuss.

What's the dsi host you have? Do you also have some separate i2c eeprom
that's needed for initializing something or is that lcd panel specific?
No idea what the eeprom is really doing here..

Regards,

Tony

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Michael Walle]

> The current code assume hardcoded dsi host endpoint 1, which may not
> be the case. Let's fix that and simplify the code by getting the dsi
> endpoint with of_graph_get_remote_endpoint() that does not assume any
> endpoint numbering.
> 
> Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -528,25 +528,17 @@ tc_mode_valid(struct drm_bridge *bridge,
>  static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
>  {
>  	struct device_node *endpoint;
> -	struct device_node *parent;
>  	struct device_node *remote;
>  	int dsi_lanes = -1;
>  
> -	/*
> -	 * To get the data-lanes of dsi, we need to access the dsi0_out of port1
> -	 *  of dsi0 endpoint from bridge port0 of d2l_in
> -	 */
>  	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
>  						 TC358775_DSI_IN, -1);
>  	if (endpoint) {
> -		/* dsi0_out node */
> -		parent = of_graph_get_remote_port_parent(endpoint);
> +		/* Get the configured data lanes on the dsi host side */
> +		remote = of_graph_get_remote_endpoint(endpoint);
>  		of_node_put(endpoint);
> -		if (parent) {
> -			/* dsi0 port 1 */
> -			dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, -1, 1, 4);
> -			of_node_put(parent);
> -		}
> +		dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);

I actually have the same fix, but with one additional detail, which I'm
unsure about though: This looks at the data-lanes property of the *remote*
endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b,
anx7625) look at the local endpoint and I'm not sure what is correct.

-michael

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Michael Walle]

> The current code assume hardcoded dsi host endpoint 1, which may not
> be the case. Let's fix that and simplify the code by getting the dsi
> endpoint with of_graph_get_remote_endpoint() that does not assume any
> endpoint numbering.
> 
> Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -528,25 +528,17 @@ tc_mode_valid(struct drm_bridge *bridge,
>  static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
>  {
>  	struct device_node *endpoint;
> -	struct device_node *parent;
>  	struct device_node *remote;
>  	int dsi_lanes = -1;
>  
> -	/*
> -	 * To get the data-lanes of dsi, we need to access the dsi0_out of port1
> -	 *  of dsi0 endpoint from bridge port0 of d2l_in
> -	 */
>  	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
>  						 TC358775_DSI_IN, -1);
>  	if (endpoint) {
> -		/* dsi0_out node */
> -		parent = of_graph_get_remote_port_parent(endpoint);
> +		/* Get the configured data lanes on the dsi host side */
> +		remote = of_graph_get_remote_endpoint(endpoint);
>  		of_node_put(endpoint);
> -		if (parent) {
> -			/* dsi0 port 1 */
> -			dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, -1, 1, 4);
> -			of_node_put(parent);
> -		}
> +		dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);

I actually have the same fix, but with one additional detail, which I'm
unsure about though: This looks at the data-lanes property of the *remote*
endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b,
anx7625) look at the local endpoint and I'm not sure what is correct.

-michael

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 13:10]:
> I actually have the same fix, but with one additional detail, which I'm
> unsure about though: This looks at the data-lanes property of the *remote*
> endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b,
> anx7625) look at the local endpoint and I'm not sure what is correct.

Yes I've been wondering about that too. Let's just move it over to the
bridge node? We could produce a warning if the dsi host node has the
data-lanes property.. No current in kernel users AFAIK.

FYI, for omapdrm, we already have a legacy dt property "lanes" for the
wiring that tells number of lanes used and the order of the lanes.

Regards,

Tony

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 13:10]:
> I actually have the same fix, but with one additional detail, which I'm
> unsure about though: This looks at the data-lanes property of the *remote*
> endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b,
> anx7625) look at the local endpoint and I'm not sure what is correct.

Yes I've been wondering about that too. Let's just move it over to the
bridge node? We could produce a warning if the dsi host node has the
data-lanes property.. No current in kernel users AFAIK.

FYI, for omapdrm, we already have a legacy dt property "lanes" for the
wiring that tells number of lanes used and the order of the lanes.

Regards,

Tony

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Alexander Stein]

Hi,

Am Montag, 27. November 2023, 13:44:30 CET schrieb Michael Walle:
> Hi,
> 
> > The tc358765 is similar to tc358775 except for the stdby-gpios.
> 
> Bad timing (for me). I'm about to send a bigger patch series for the
> tc358775 which fixes the (completely) broken initialialization. And also
> contains some of your fixes.
> 
> That being said, I intend to make the standby gpio optional also for the
> tc358755, because it might just be hardwired on the board.
> 
> But second, I'm really curious if this bridge is working for you correctly
> as it is at the moment. One particular thing I've noticed is that you must
> release the reset while both the clock and the data lanes are in LP11 mode.
> Otherwise, the bridge won't work properly (i.e. horizontally shifted
> picture, or no picture at all).

Apparently this seems to be true for all Toshiba DSI bridges. The power on 
sequence for TC9595 (TC358767) also requires LP-11 before releasing the reset 
signal RESX. Additionally LP-11 is requires for using the DP AUX channel.
This also relates to the patch set from Dmitry [1].

Best regards,
Alexander

[1] https://lore.kernel.org/dri-devel/20231016165355.1327217-1-dmitry.baryshkov@linaro.org/

> What DSI host controller are you using?
> 
> -michael


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765

[Alexander Stein]

Hi,

Am Montag, 27. November 2023, 13:44:30 CET schrieb Michael Walle:
> Hi,
> 
> > The tc358765 is similar to tc358775 except for the stdby-gpios.
> 
> Bad timing (for me). I'm about to send a bigger patch series for the
> tc358775 which fixes the (completely) broken initialialization. And also
> contains some of your fixes.
> 
> That being said, I intend to make the standby gpio optional also for the
> tc358755, because it might just be hardwired on the board.
> 
> But second, I'm really curious if this bridge is working for you correctly
> as it is at the moment. One particular thing I've noticed is that you must
> release the reset while both the clock and the data lanes are in LP11 mode.
> Otherwise, the bridge won't work properly (i.e. horizontally shifted
> picture, or no picture at all).

Apparently this seems to be true for all Toshiba DSI bridges. The power on 
sequence for TC9595 (TC358767) also requires LP-11 before releasing the reset 
signal RESX. Additionally LP-11 is requires for using the DP AUX channel.
This also relates to the patch set from Dmitry [1].

Best regards,
Alexander

[1] https://lore.kernel.org/dri-devel/20231016165355.1327217-1-dmitry.baryshkov@linaro.org/

> What DSI host controller are you using?
> 
> -michael


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support

[Michael Walle]

> The jeida-24 register values are the default hardware settings, but they
> not listed in the driver. Let's add them.

jeida-24 and jeida-18 should have the same mapping, jeida-18 is broken in
this driver. could you test this patch:

--snip--

From 46da1d76d4908e5879ed746cce1faeacd69c432e Mon Sep 17 00:00:00 2001
From: Michael Walle <mwalle@kernel.org>
Date: Wed, 4 Oct 2023 13:52:57 +0200
Subject: [PATCH] drm/bridge: tc358775: fix support for jeida-18 and jeida-24

The bridge always uses 24bpp internally. Therefore, for jeida-18
mapping we need to discard the lowest two bits for each channel and thus
starting with LV_[RGB]2. jeida-24 has the same mapping but uses four
lanes instead of three, with the forth pair transmitting the lowest two
bits of each channel. Thus, the mapping between jeida-18 and jeida-24
is actually the same, except that one channel is turned off (by
selecting the RGB666 format in VPCTRL).

While at it, remove the bogus comment about the hardware default because
the default is overwritten in any case.

Tested with a jeida-18 display (Evervision VGG644804).

Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 90a89d70d832..592c69c2aedc 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 	dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
 		connector->display_info.bus_formats[0],
 		tc->bpc);
-	/*
-	 * Default hardware register settings of tc358775 configured
-	 * with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
-	 */
 	if (connector->display_info.bus_formats[0] ==
 		MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
 		/* VESA-24 */
@@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
 		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
 		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
-	} else { /*  MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
-		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
-		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
-		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
-		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
-		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
-		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
-		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
+	} else {
+		/* JEIDA-18 and JEIDA-24 */
+		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
+		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
+		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
+		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
+		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
+		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
+		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
 	}
 
 	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
-- 
2.39.2

Re: [PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support

[Michael Walle]

> The jeida-24 register values are the default hardware settings, but they
> not listed in the driver. Let's add them.

jeida-24 and jeida-18 should have the same mapping, jeida-18 is broken in
this driver. could you test this patch:

--snip--

From 46da1d76d4908e5879ed746cce1faeacd69c432e Mon Sep 17 00:00:00 2001
From: Michael Walle <mwalle@kernel.org>
Date: Wed, 4 Oct 2023 13:52:57 +0200
Subject: [PATCH] drm/bridge: tc358775: fix support for jeida-18 and jeida-24

The bridge always uses 24bpp internally. Therefore, for jeida-18
mapping we need to discard the lowest two bits for each channel and thus
starting with LV_[RGB]2. jeida-24 has the same mapping but uses four
lanes instead of three, with the forth pair transmitting the lowest two
bits of each channel. Thus, the mapping between jeida-18 and jeida-24
is actually the same, except that one channel is turned off (by
selecting the RGB666 format in VPCTRL).

While at it, remove the bogus comment about the hardware default because
the default is overwritten in any case.

Tested with a jeida-18 display (Evervision VGG644804).

Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
index 90a89d70d832..592c69c2aedc 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 	dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
 		connector->display_info.bus_formats[0],
 		tc->bpc);
-	/*
-	 * Default hardware register settings of tc358775 configured
-	 * with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
-	 */
 	if (connector->display_info.bus_formats[0] ==
 		MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
 		/* VESA-24 */
@@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
 		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
 		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
-	} else { /*  MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
-		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
-		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
-		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
-		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
-		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
-		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
-		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
+	} else {
+		/* JEIDA-18 and JEIDA-24 */
+		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
+		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
+		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
+		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
+		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
+		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
+		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
 	}
 
 	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
-- 
2.39.2

Re: [PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 13:25]:
> > The jeida-24 register values are the default hardware settings, but they
> > not listed in the driver. Let's add them.
> 
> jeida-24 and jeida-18 should have the same mapping, jeida-18 is broken in
> this driver. could you test this patch:

Yes great works for me with the tc358765 patches:

Tested-by: Tony Lindgren <tony@atomide.com>

> --snip--
> 
> From 46da1d76d4908e5879ed746cce1faeacd69c432e Mon Sep 17 00:00:00 2001
> From: Michael Walle <mwalle@kernel.org>
> Date: Wed, 4 Oct 2023 13:52:57 +0200
> Subject: [PATCH] drm/bridge: tc358775: fix support for jeida-18 and jeida-24
> 
> The bridge always uses 24bpp internally. Therefore, for jeida-18
> mapping we need to discard the lowest two bits for each channel and thus
> starting with LV_[RGB]2. jeida-24 has the same mapping but uses four
> lanes instead of three, with the forth pair transmitting the lowest two
> bits of each channel. Thus, the mapping between jeida-18 and jeida-24
> is actually the same, except that one channel is turned off (by
> selecting the RGB666 format in VPCTRL).
> 
> While at it, remove the bogus comment about the hardware default because
> the default is overwritten in any case.
> 
> Tested with a jeida-18 display (Evervision VGG644804).
> 
> Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> index 90a89d70d832..592c69c2aedc 100644
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>  	dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
>  		connector->display_info.bus_formats[0],
>  		tc->bpc);
> -	/*
> -	 * Default hardware register settings of tc358775 configured
> -	 * with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
> -	 */
>  	if (connector->display_info.bus_formats[0] ==
>  		MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
>  		/* VESA-24 */
> @@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>  		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
>  		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
>  		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> -	} else { /*  MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
> -		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> -		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
> -		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
> -		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> -		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
> -		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> -		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
> +	} else {
> +		/* JEIDA-18 and JEIDA-24 */
> +		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
> +		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
> +		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
> +		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
> +		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
> +		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
> +		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
>  	}
>  
>  	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
> -- 
> 2.39.2
> 

Re: [PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 13:25]:
> > The jeida-24 register values are the default hardware settings, but they
> > not listed in the driver. Let's add them.
> 
> jeida-24 and jeida-18 should have the same mapping, jeida-18 is broken in
> this driver. could you test this patch:

Yes great works for me with the tc358765 patches:

Tested-by: Tony Lindgren <tony@atomide.com>

> --snip--
> 
> From 46da1d76d4908e5879ed746cce1faeacd69c432e Mon Sep 17 00:00:00 2001
> From: Michael Walle <mwalle@kernel.org>
> Date: Wed, 4 Oct 2023 13:52:57 +0200
> Subject: [PATCH] drm/bridge: tc358775: fix support for jeida-18 and jeida-24
> 
> The bridge always uses 24bpp internally. Therefore, for jeida-18
> mapping we need to discard the lowest two bits for each channel and thus
> starting with LV_[RGB]2. jeida-24 has the same mapping but uses four
> lanes instead of three, with the forth pair transmitting the lowest two
> bits of each channel. Thus, the mapping between jeida-18 and jeida-24
> is actually the same, except that one channel is turned off (by
> selecting the RGB666 format in VPCTRL).
> 
> While at it, remove the bogus comment about the hardware default because
> the default is overwritten in any case.
> 
> Tested with a jeida-18 display (Evervision VGG644804).
> 
> Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> index 90a89d70d832..592c69c2aedc 100644
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>  	dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
>  		connector->display_info.bus_formats[0],
>  		tc->bpc);
> -	/*
> -	 * Default hardware register settings of tc358775 configured
> -	 * with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
> -	 */
>  	if (connector->display_info.bus_formats[0] ==
>  		MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
>  		/* VESA-24 */
> @@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>  		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
>  		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
>  		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> -	} else { /*  MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
> -		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> -		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
> -		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
> -		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> -		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
> -		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> -		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
> +	} else {
> +		/* JEIDA-18 and JEIDA-24 */
> +		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5));
> +		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2));
> +		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1));
> +		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2));
> +		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4));
> +		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0));
> +		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0));
>  	}
>  
>  	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
> -- 
> 2.39.2
> 

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Michael Walle]

+ dt maintainers

>> I actually have the same fix, but with one additional detail, which 
>> I'm
>> unsure about though: This looks at the data-lanes property of the 
>> *remote*
>> endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, 
>> lt8912b,
>> anx7625) look at the local endpoint and I'm not sure what is correct.
> 
> Yes I've been wondering about that too. Let's just move it over to the
> bridge node? We could produce a warning if the dsi host node has the
> data-lanes property.. No current in kernel users AFAIK.

I haven't found any in-tree users either. In my patch, I first try the 
remote
end and then the local end. But thinking more about it I don't think
this is correct. Maybe we can do it the other way around, first try
data-lanes of the local endpoint and if not found, then try the remote
one. That way, we would at least be backwards compatible in the driver.
And for the dt-bindings, make it mandatory to have a local data-lanes.

-michael

> FYI, for omapdrm, we already have a legacy dt property "lanes" for the
> wiring that tells number of lanes used and the order of the lanes.
> 
> Regards,
> 
> Tony

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Michael Walle]

+ dt maintainers

>> I actually have the same fix, but with one additional detail, which 
>> I'm
>> unsure about though: This looks at the data-lanes property of the 
>> *remote*
>> endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, 
>> lt8912b,
>> anx7625) look at the local endpoint and I'm not sure what is correct.
> 
> Yes I've been wondering about that too. Let's just move it over to the
> bridge node? We could produce a warning if the dsi host node has the
> data-lanes property.. No current in kernel users AFAIK.

I haven't found any in-tree users either. In my patch, I first try the 
remote
end and then the local end. But thinking more about it I don't think
this is correct. Maybe we can do it the other way around, first try
data-lanes of the local endpoint and if not found, then try the remote
one. That way, we would at least be backwards compatible in the driver.
And for the dt-bindings, make it mandatory to have a local data-lanes.

-michael

> FYI, for omapdrm, we already have a legacy dt property "lanes" for the
> wiring that tells number of lanes used and the order of the lanes.
> 
> Regards,
> 
> Tony

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 14:31]:
> + dt maintainers
> 
> > > I actually have the same fix, but with one additional detail, which
> > > I'm
> > > unsure about though: This looks at the data-lanes property of the
> > > *remote*
> > > endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83,
> > > lt8912b,
> > > anx7625) look at the local endpoint and I'm not sure what is correct.
> > 
> > Yes I've been wondering about that too. Let's just move it over to the
> > bridge node? We could produce a warning if the dsi host node has the
> > data-lanes property.. No current in kernel users AFAIK.
> 
> I haven't found any in-tree users either. In my patch, I first try the
> remote
> end and then the local end. But thinking more about it I don't think
> this is correct. Maybe we can do it the other way around, first try
> data-lanes of the local endpoint and if not found, then try the remote
> one. That way, we would at least be backwards compatible in the driver.
> And for the dt-bindings, make it mandatory to have a local data-lanes.

OK sounds good to me.

Tony

Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes

[Tony Lindgren]

* Michael Walle <mwalle@kernel.org> [231127 14:31]:
> + dt maintainers
> 
> > > I actually have the same fix, but with one additional detail, which
> > > I'm
> > > unsure about though: This looks at the data-lanes property of the
> > > *remote*
> > > endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83,
> > > lt8912b,
> > > anx7625) look at the local endpoint and I'm not sure what is correct.
> > 
> > Yes I've been wondering about that too. Let's just move it over to the
> > bridge node? We could produce a warning if the dsi host node has the
> > data-lanes property.. No current in kernel users AFAIK.
> 
> I haven't found any in-tree users either. In my patch, I first try the
> remote
> end and then the local end. But thinking more about it I don't think
> this is correct. Maybe we can do it the other way around, first try
> data-lanes of the local endpoint and if not found, then try the remote
> one. That way, we would at least be backwards compatible in the driver.
> And for the dt-bindings, make it mandatory to have a local data-lanes.

OK sounds good to me.

Tony