[PATCH v2 00/10] Add TOSHIBA TC358764 DSI/LVDS bridge driver

[PATCH v2 00/10] Add TOSHIBA TC358764 DSI/LVDS bridge driver

[Maciej Purski]

Hi all,

this patchset is a next attempt to add the tc358764 driver.
The previous one can be found here:

https://lists.freedesktop.org/archives/dri-devel/2014-February/053705.html

Back then, TC358764 was added as a panel driver.

The bridge is supposed to be a DSI peripheral. Currently exynos_dsi accepts only panels
as its peripherals. Therefore, some logic in exynos_dsi had to be ammended. That is implemented
in first 4 patches.

Apart from the driver this patchset adds support for BOE HV070WSA-100 panel, which is used by
TC358764 and dts nodes to exynos5250.dtsi and exynos5250-arndale.dtsi.

Best regards,

Maciej Purski

---

Changes in v2:
- fix commits authorship
- don't call pm_runtime_put_sync() in exyons_dsi_disable(), if pm_runtime_get_sync()
  has not been called
- squash dts commits
- merge some redundant regulators in tc358764 bindings and in DTS
- fix kbuild robot errors

Andrzej Hajda (6):
  panel/hv070wsa-100: add DT bindings
  drm/panel: add support for BOE HV070WSA-100 panel to simple-panel
  dt-bindings: tc358754: add DT bindings
  drm/bridge: tc358764: Add DSI to LVDS bridge driver
  ARM: dts: exynos5250: add DSI node
  ARM: dts: exynos5250-arndale: add DSI and panel nodes

Maciej Purski (4):
  drm/exynos: rename "bridge_node" to "mic_bridge_node"
  drm/exynos: move pm_runtime_get_sync() to exynos_dsi_init()
  drm/exynos: move connector creation to attach callback
  drm/exynos: add non-panel path to exynos_dsi_enable()

 .../bindings/display/bridge/toshiba,tc358764.txt   |  37 ++
 .../bindings/display/panel/boe,hv070wsa-100.txt    |   7 +
 arch/arm/boot/dts/exynos5250-arndale.dts           |  61 +++
 arch/arm/boot/dts/exynos5250.dtsi                  |  21 +
 drivers/gpu/drm/bridge/Kconfig                     |   9 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/tc358764.c                  | 547 +++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  88 ++--
 drivers/gpu/drm/panel/panel-simple.c               |  25 +
 9 files changed, 755 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
 create mode 100644 drivers/gpu/drm/bridge/tc358764.c

-- 
2.7.4

[PATCH v2 00/10] Add TOSHIBA TC358764 DSI/LVDS bridge driver

[Maciej Purski]

Hi all,

this patchset is a next attempt to add the tc358764 driver.
The previous one can be found here:

https://lists.freedesktop.org/archives/dri-devel/2014-February/053705.html

Back then, TC358764 was added as a panel driver.

The bridge is supposed to be a DSI peripheral. Currently exynos_dsi accepts only panels
as its peripherals. Therefore, some logic in exynos_dsi had to be ammended. That is implemented
in first 4 patches.

Apart from the driver this patchset adds support for BOE HV070WSA-100 panel, which is used by
TC358764 and dts nodes to exynos5250.dtsi and exynos5250-arndale.dtsi.

Best regards,

Maciej Purski

---

Changes in v2:
- fix commits authorship
- don't call pm_runtime_put_sync() in exyons_dsi_disable(), if pm_runtime_get_sync()
  has not been called
- squash dts commits
- merge some redundant regulators in tc358764 bindings and in DTS
- fix kbuild robot errors

Andrzej Hajda (6):
  panel/hv070wsa-100: add DT bindings
  drm/panel: add support for BOE HV070WSA-100 panel to simple-panel
  dt-bindings: tc358754: add DT bindings
  drm/bridge: tc358764: Add DSI to LVDS bridge driver
  ARM: dts: exynos5250: add DSI node
  ARM: dts: exynos5250-arndale: add DSI and panel nodes

Maciej Purski (4):
  drm/exynos: rename "bridge_node" to "mic_bridge_node"
  drm/exynos: move pm_runtime_get_sync() to exynos_dsi_init()
  drm/exynos: move connector creation to attach callback
  drm/exynos: add non-panel path to exynos_dsi_enable()

 .../bindings/display/bridge/toshiba,tc358764.txt   |  37 ++
 .../bindings/display/panel/boe,hv070wsa-100.txt    |   7 +
 arch/arm/boot/dts/exynos5250-arndale.dts           |  61 +++
 arch/arm/boot/dts/exynos5250.dtsi                  |  21 +
 drivers/gpu/drm/bridge/Kconfig                     |   9 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/tc358764.c                  | 547 +++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  88 ++--
 drivers/gpu/drm/panel/panel-simple.c               |  25 +
 9 files changed, 755 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
 create mode 100644 drivers/gpu/drm/bridge/tc358764.c

-- 
2.7.4

[PATCH v2 01/10] drm/exynos: rename "bridge_node" to "mic_bridge_node"

[Maciej Purski]

When adding support for peripheral out bridges, the "bridge" name
becomes imprecise as it refers to a different device than the
"out_bridge".

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index eae44fd..9599e6b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -279,7 +279,7 @@ struct exynos_dsi {
 	struct list_head transfer_list;
 
 	const struct exynos_dsi_driver_data *driver_data;
-	struct device_node *bridge_node;
+	struct device_node *mic_bridge_node;
 };
 
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
@@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;
 
-	dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
+	dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
 
 	return 0;
 }
@@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	struct drm_encoder *encoder = dev_get_drvdata(dev);
 	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
 	struct drm_device *drm_dev = data;
-	struct drm_bridge *bridge;
+	struct drm_bridge *mic_bridge;
 	int ret;
 
 	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
@@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	if (dsi->bridge_node) {
-		bridge = of_drm_find_bridge(dsi->bridge_node);
-		if (bridge)
-			drm_bridge_attach(encoder, bridge, NULL);
+	if (dsi->mic_bridge_node) {
+		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
+		if (mic_bridge)
+			drm_bridge_attach(encoder, mic_bridge, NULL);
 	}
 
 	return mipi_dsi_host_register(&dsi->dsi_host);
@@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev)
 {
 	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
 
-	of_node_put(dsi->bridge_node);
+	of_node_put(dsi->mic_bridge_node);
 
 	pm_runtime_disable(&pdev->dev);
 
-- 
2.7.4

[PATCH v2 01/10] drm/exynos: rename "bridge_node" to "mic_bridge_node"

[Maciej Purski]

When adding support for peripheral out bridges, the "bridge" name
becomes imprecise as it refers to a different device than the
"out_bridge".

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index eae44fd..9599e6b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -279,7 +279,7 @@ struct exynos_dsi {
 	struct list_head transfer_list;
 
 	const struct exynos_dsi_driver_data *driver_data;
-	struct device_node *bridge_node;
+	struct device_node *mic_bridge_node;
 };
 
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
@@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;
 
-	dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
+	dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
 
 	return 0;
 }
@@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	struct drm_encoder *encoder = dev_get_drvdata(dev);
 	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
 	struct drm_device *drm_dev = data;
-	struct drm_bridge *bridge;
+	struct drm_bridge *mic_bridge;
 	int ret;
 
 	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
@@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	if (dsi->bridge_node) {
-		bridge = of_drm_find_bridge(dsi->bridge_node);
-		if (bridge)
-			drm_bridge_attach(encoder, bridge, NULL);
+	if (dsi->mic_bridge_node) {
+		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
+		if (mic_bridge)
+			drm_bridge_attach(encoder, mic_bridge, NULL);
 	}
 
 	return mipi_dsi_host_register(&dsi->dsi_host);
@@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev)
 {
 	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
 
-	of_node_put(dsi->bridge_node);
+	of_node_put(dsi->mic_bridge_node);
 
 	pm_runtime_disable(&pdev->dev);
 
-- 
2.7.4

Re: [PATCH v2 01/10] drm/exynos: rename "bridge_node" to "mic_bridge_node"

[Inki Dae]

Hi,

I will start to review patch sets for -fixes first so I queued your four patches and you can check exynos-drm-next-todo branch for it.

Thanks,
Inki Dae

2018년 05월 30일 21:15에 Maciej Purski 이(가) 쓴 글:
> When adding support for peripheral out bridges, the "bridge" name
> becomes imprecise as it refers to a different device than the
> "out_bridge".
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index eae44fd..9599e6b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -279,7 +279,7 @@ struct exynos_dsi {
>  	struct list_head transfer_list;
>  
>  	const struct exynos_dsi_driver_data *driver_data;
> -	struct device_node *bridge_node;
> +	struct device_node *mic_bridge_node;
>  };
>  
>  #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
> @@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
>  	if (ret < 0)
>  		return ret;
>  
> -	dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
> +	dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
>  
>  	return 0;
>  }
> @@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  	struct drm_encoder *encoder = dev_get_drvdata(dev);
>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>  	struct drm_device *drm_dev = data;
> -	struct drm_bridge *bridge;
> +	struct drm_bridge *mic_bridge;
>  	int ret;
>  
>  	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
> @@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> -	if (dsi->bridge_node) {
> -		bridge = of_drm_find_bridge(dsi->bridge_node);
> -		if (bridge)
> -			drm_bridge_attach(encoder, bridge, NULL);
> +	if (dsi->mic_bridge_node) {
> +		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
> +		if (mic_bridge)
> +			drm_bridge_attach(encoder, mic_bridge, NULL);
>  	}
>  
>  	return mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev)
>  {
>  	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
>  
> -	of_node_put(dsi->bridge_node);
> +	of_node_put(dsi->mic_bridge_node);
>  
>  	pm_runtime_disable(&pdev->dev);
>  
> 

[PATCH v2 01/10] drm/exynos: rename "bridge_node" to "mic_bridge_node"

[Inki Dae]

Hi,

I will start to review patch sets for -fixes first so I queued your four patches and you can check exynos-drm-next-todo branch for it.

Thanks,
Inki Dae

2018? 05? 30? 21:15? Maciej Purski ?(?) ? ?:
> When adding support for peripheral out bridges, the "bridge" name
> becomes imprecise as it refers to a different device than the
> "out_bridge".
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index eae44fd..9599e6b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -279,7 +279,7 @@ struct exynos_dsi {
>  	struct list_head transfer_list;
>  
>  	const struct exynos_dsi_driver_data *driver_data;
> -	struct device_node *bridge_node;
> +	struct device_node *mic_bridge_node;
>  };
>  
>  #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
> @@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
>  	if (ret < 0)
>  		return ret;
>  
> -	dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
> +	dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
>  
>  	return 0;
>  }
> @@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  	struct drm_encoder *encoder = dev_get_drvdata(dev);
>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>  	struct drm_device *drm_dev = data;
> -	struct drm_bridge *bridge;
> +	struct drm_bridge *mic_bridge;
>  	int ret;
>  
>  	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
> @@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> -	if (dsi->bridge_node) {
> -		bridge = of_drm_find_bridge(dsi->bridge_node);
> -		if (bridge)
> -			drm_bridge_attach(encoder, bridge, NULL);
> +	if (dsi->mic_bridge_node) {
> +		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
> +		if (mic_bridge)
> +			drm_bridge_attach(encoder, mic_bridge, NULL);
>  	}
>  
>  	return mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev)
>  {
>  	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
>  
> -	of_node_put(dsi->bridge_node);
> +	of_node_put(dsi->mic_bridge_node);
>  
>  	pm_runtime_disable(&pdev->dev);
>  
> 

Re: [PATCH v2 01/10] drm/exynos: rename "bridge_node" to "mic_bridge_node"

[Inki Dae]

Hi,

I will start to review patch sets for -fixes first so I queued your four patches and you can check exynos-drm-next-todo branch for it.

Thanks,
Inki Dae

2018년 05월 30일 21:15에 Maciej Purski 이(가) 쓴 글:
> When adding support for peripheral out bridges, the "bridge" name
> becomes imprecise as it refers to a different device than the
> "out_bridge".
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index eae44fd..9599e6b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -279,7 +279,7 @@ struct exynos_dsi {
>  	struct list_head transfer_list;
>  
>  	const struct exynos_dsi_driver_data *driver_data;
> -	struct device_node *bridge_node;
> +	struct device_node *mic_bridge_node;
>  };
>  
>  #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
> @@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
>  	if (ret < 0)
>  		return ret;
>  
> -	dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
> +	dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);
>  
>  	return 0;
>  }
> @@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  	struct drm_encoder *encoder = dev_get_drvdata(dev);
>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>  	struct drm_device *drm_dev = data;
> -	struct drm_bridge *bridge;
> +	struct drm_bridge *mic_bridge;
>  	int ret;
>  
>  	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
> @@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> -	if (dsi->bridge_node) {
> -		bridge = of_drm_find_bridge(dsi->bridge_node);
> -		if (bridge)
> -			drm_bridge_attach(encoder, bridge, NULL);
> +	if (dsi->mic_bridge_node) {
> +		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
> +		if (mic_bridge)
> +			drm_bridge_attach(encoder, mic_bridge, NULL);
>  	}
>  
>  	return mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev)
>  {
>  	struct exynos_dsi *dsi = platform_get_drvdata(pdev);
>  
> -	of_node_put(dsi->bridge_node);
> +	of_node_put(dsi->mic_bridge_node);
>  
>  	pm_runtime_disable(&pdev->dev);
>  
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 02/10] drm/exynos: move pm_runtime_get_sync() to exynos_dsi_init()

[Maciej Purski]

In order to allow bridge drivers to use DSI transfers in their
pre_enable callbacks, pm_runtime_get_sync() should be performed before
exynos_dsi_enable(). DSIM_STATE_ENABLED flag now should not guard
from calling dsi_host_transfer() before enabling.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 9599e6b..85eb2262 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1312,6 +1312,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
 {
 	const struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 
+	pm_runtime_get_sync(dsi->dev);
 	exynos_dsi_reset(dsi);
 	exynos_dsi_enable_irq(dsi);
 
@@ -1388,7 +1389,6 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	ret = drm_panel_prepare(dsi->panel);
 	if (ret < 0) {
 		dsi->state &= ~DSIM_STATE_ENABLED;
-		pm_runtime_put_sync(dsi->dev);
 		return;
 	}
 
@@ -1400,7 +1400,6 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 		dsi->state &= ~DSIM_STATE_ENABLED;
 		exynos_dsi_set_display_enable(dsi, false);
 		drm_panel_unprepare(dsi->panel);
-		pm_runtime_put_sync(dsi->dev);
 		return;
 	}
 
@@ -1422,7 +1421,10 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
 
 	dsi->state &= ~DSIM_STATE_ENABLED;
 
-	pm_runtime_put_sync(dsi->dev);
+	if (dsi->state & DSIM_STATE_INITIALIZED) {
+		pm_runtime_put_sync(dsi->dev);
+		dsi->state &= ~DSIM_STATE_INITIALIZED;
+	}
 }
 
 static enum drm_connector_status
@@ -1566,9 +1568,6 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
 	struct exynos_dsi_transfer xfer;
 	int ret;
 
-	if (!(dsi->state & DSIM_STATE_ENABLED))
-		return -EINVAL;
-
 	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
 		ret = exynos_dsi_init(dsi);
 		if (ret)
-- 
2.7.4

[PATCH v2 02/10] drm/exynos: move pm_runtime_get_sync() to exynos_dsi_init()

[Maciej Purski]

In order to allow bridge drivers to use DSI transfers in their
pre_enable callbacks, pm_runtime_get_sync() should be performed before
exynos_dsi_enable(). DSIM_STATE_ENABLED flag now should not guard
from calling dsi_host_transfer() before enabling.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 9599e6b..85eb2262 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1312,6 +1312,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
 {
 	const struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 
+	pm_runtime_get_sync(dsi->dev);
 	exynos_dsi_reset(dsi);
 	exynos_dsi_enable_irq(dsi);
 
@@ -1388,7 +1389,6 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	ret = drm_panel_prepare(dsi->panel);
 	if (ret < 0) {
 		dsi->state &= ~DSIM_STATE_ENABLED;
-		pm_runtime_put_sync(dsi->dev);
 		return;
 	}
 
@@ -1400,7 +1400,6 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 		dsi->state &= ~DSIM_STATE_ENABLED;
 		exynos_dsi_set_display_enable(dsi, false);
 		drm_panel_unprepare(dsi->panel);
-		pm_runtime_put_sync(dsi->dev);
 		return;
 	}
 
@@ -1422,7 +1421,10 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
 
 	dsi->state &= ~DSIM_STATE_ENABLED;
 
-	pm_runtime_put_sync(dsi->dev);
+	if (dsi->state & DSIM_STATE_INITIALIZED) {
+		pm_runtime_put_sync(dsi->dev);
+		dsi->state &= ~DSIM_STATE_INITIALIZED;
+	}
 }
 
 static enum drm_connector_status
@@ -1566,9 +1568,6 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
 	struct exynos_dsi_transfer xfer;
 	int ret;
 
-	if (!(dsi->state & DSIM_STATE_ENABLED))
-		return -EINVAL;
-
 	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
 		ret = exynos_dsi_init(dsi);
 		if (ret)
-- 
2.7.4

[PATCH v2 03/10] drm/exynos: move connector creation to attach callback

[Maciej Purski]

The current implementation assumes that the only possible peripheral
device for DSIM is a panel. Using an output bridge should also be
possible.

If an output bridge in available, don't create a new connector.
Instead add bridge to DSIM encdoer in dsi_host_attach().

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 35 +++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 85eb2262..7b50bad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1501,7 +1501,28 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 				  struct mipi_dsi_device *device)
 {
 	struct exynos_dsi *dsi = host_to_dsi(host);
-	struct drm_device *drm = dsi->connector.dev;
+	struct drm_encoder *encoder = &dsi->encoder;
+	struct drm_device *drm = encoder->dev;
+	struct drm_bridge *out_bridge;
+
+	out_bridge  = of_drm_find_bridge(device->dev.of_node);
+	if (out_bridge) {
+		drm_bridge_attach(encoder, out_bridge, NULL);
+	} else {
+		int ret = exynos_dsi_create_connector(encoder);
+
+		if (ret) {
+			DRM_ERROR("failed to create connector ret = %d\n", ret);
+			drm_encoder_cleanup(encoder);
+			return ret;
+		}
+
+		dsi->panel = of_drm_find_panel(device->dev.of_node);
+		if (dsi->panel) {
+			drm_panel_attach(dsi->panel, &dsi->connector);
+			dsi->connector.status = connector_status_connected;
+		}
+	}
 
 	/*
 	 * This is a temporary solution and should be made by more generic way.
@@ -1520,11 +1541,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 	dsi->mode_flags = device->mode_flags;
-	dsi->panel = of_drm_find_panel(device->dev.of_node);
-	if (dsi->panel) {
-		drm_panel_attach(dsi->panel, &dsi->connector);
-		dsi->connector.status = connector_status_connected;
-	}
 	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
 			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
 
@@ -1653,13 +1669,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		return ret;
 
-	ret = exynos_dsi_create_connector(encoder);
-	if (ret) {
-		DRM_ERROR("failed to create connector ret = %d\n", ret);
-		drm_encoder_cleanup(encoder);
-		return ret;
-	}
-
 	if (dsi->mic_bridge_node) {
 		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
 		if (mic_bridge)
-- 
2.7.4

[PATCH v2 03/10] drm/exynos: move connector creation to attach callback

[Maciej Purski]

The current implementation assumes that the only possible peripheral
device for DSIM is a panel. Using an output bridge should also be
possible.

If an output bridge in available, don't create a new connector.
Instead add bridge to DSIM encdoer in dsi_host_attach().

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 35 +++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 85eb2262..7b50bad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1501,7 +1501,28 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 				  struct mipi_dsi_device *device)
 {
 	struct exynos_dsi *dsi = host_to_dsi(host);
-	struct drm_device *drm = dsi->connector.dev;
+	struct drm_encoder *encoder = &dsi->encoder;
+	struct drm_device *drm = encoder->dev;
+	struct drm_bridge *out_bridge;
+
+	out_bridge  = of_drm_find_bridge(device->dev.of_node);
+	if (out_bridge) {
+		drm_bridge_attach(encoder, out_bridge, NULL);
+	} else {
+		int ret = exynos_dsi_create_connector(encoder);
+
+		if (ret) {
+			DRM_ERROR("failed to create connector ret = %d\n", ret);
+			drm_encoder_cleanup(encoder);
+			return ret;
+		}
+
+		dsi->panel = of_drm_find_panel(device->dev.of_node);
+		if (dsi->panel) {
+			drm_panel_attach(dsi->panel, &dsi->connector);
+			dsi->connector.status = connector_status_connected;
+		}
+	}
 
 	/*
 	 * This is a temporary solution and should be made by more generic way.
@@ -1520,11 +1541,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 	dsi->mode_flags = device->mode_flags;
-	dsi->panel = of_drm_find_panel(device->dev.of_node);
-	if (dsi->panel) {
-		drm_panel_attach(dsi->panel, &dsi->connector);
-		dsi->connector.status = connector_status_connected;
-	}
 	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
 			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
 
@@ -1653,13 +1669,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		return ret;
 
-	ret = exynos_dsi_create_connector(encoder);
-	if (ret) {
-		DRM_ERROR("failed to create connector ret = %d\n", ret);
-		drm_encoder_cleanup(encoder);
-		return ret;
-	}
-
 	if (dsi->mic_bridge_node) {
 		mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
 		if (mic_bridge)
-- 
2.7.4

[PATCH v2 04/10] drm/exynos: add non-panel path to exynos_dsi_enable()

[Maciej Purski]

As DSIM can now have a bridge connected as a peripheral, it should be
possible to successfully enable exynos_dsi, when there is no panel
provided.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 7b50bad..7f6a0b1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1382,27 +1382,25 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return;
 
-	pm_runtime_get_sync(dsi->dev);
-
-	dsi->state |= DSIM_STATE_ENABLED;
-
-	ret = drm_panel_prepare(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_prepare(dsi->panel);
+		if (ret < 0)
+			return;
 	}
 
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
-	ret = drm_panel_enable(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		exynos_dsi_set_display_enable(dsi, false);
-		drm_panel_unprepare(dsi->panel);
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_enable(dsi->panel);
+		if (ret < 0) {
+			exynos_dsi_set_display_enable(dsi, false);
+			drm_panel_unprepare(dsi->panel);
+			return;
+		}
 	}
 
+	dsi->state |= DSIM_STATE_ENABLED;
 	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
-- 
2.7.4

[PATCH v2 04/10] drm/exynos: add non-panel path to exynos_dsi_enable()

[Maciej Purski]

As DSIM can now have a bridge connected as a peripheral, it should be
possible to successfully enable exynos_dsi, when there is no panel
provided.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 7b50bad..7f6a0b1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1382,27 +1382,25 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return;
 
-	pm_runtime_get_sync(dsi->dev);
-
-	dsi->state |= DSIM_STATE_ENABLED;
-
-	ret = drm_panel_prepare(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_prepare(dsi->panel);
+		if (ret < 0)
+			return;
 	}
 
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
-	ret = drm_panel_enable(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		exynos_dsi_set_display_enable(dsi, false);
-		drm_panel_unprepare(dsi->panel);
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_enable(dsi->panel);
+		if (ret < 0) {
+			exynos_dsi_set_display_enable(dsi, false);
+			drm_panel_unprepare(dsi->panel);
+			return;
+		}
 	}
 
+	dsi->state |= DSIM_STATE_ENABLED;
 	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
-- 
2.7.4

[PATCH v2 04/10] drm/exynos: add non-panel path to exynos_dsi_enable()

[Maciej Purski]

As DSIM can now have a bridge connected as a peripheral, it should be
possible to successfully enable exynos_dsi, when there is no panel
provided.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 7b50bad..7f6a0b1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1382,27 +1382,25 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return;
 
-	pm_runtime_get_sync(dsi->dev);
-
-	dsi->state |= DSIM_STATE_ENABLED;
-
-	ret = drm_panel_prepare(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_prepare(dsi->panel);
+		if (ret < 0)
+			return;
 	}
 
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
-	ret = drm_panel_enable(dsi->panel);
-	if (ret < 0) {
-		dsi->state &= ~DSIM_STATE_ENABLED;
-		exynos_dsi_set_display_enable(dsi, false);
-		drm_panel_unprepare(dsi->panel);
-		return;
+	if (dsi->panel) {
+		ret = drm_panel_enable(dsi->panel);
+		if (ret < 0) {
+			exynos_dsi_set_display_enable(dsi, false);
+			drm_panel_unprepare(dsi->panel);
+			return;
+		}
 	}
 
+	dsi->state |= DSIM_STATE_ENABLED;
 	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
-- 
2.7.4

[PATCH v2 05/10] panel/hv070wsa-100: add DT bindings

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bindings to BOE HV070-WSA WSVGA panel.
Bindings are compatible with simple panel bindings.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 .../devicetree/bindings/display/panel/boe,hv070wsa-100.txt         | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt

diff --git a/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
new file mode 100644
index 0000000..bfc20ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
@@ -0,0 +1,7 @@
+BOE HV070WSA-100 7.01" WSVGA TFT LCD panel
+
+Required properties:
+- compatible: should be "boe,hv070wsa-100"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
-- 
2.7.4

[PATCH v2 05/10] panel/hv070wsa-100: add DT bindings

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bindings to BOE HV070-WSA WSVGA panel.
Bindings are compatible with simple panel bindings.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 .../devicetree/bindings/display/panel/boe,hv070wsa-100.txt         | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt

diff --git a/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
new file mode 100644
index 0000000..bfc20ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
@@ -0,0 +1,7 @@
+BOE HV070WSA-100 7.01" WSVGA TFT LCD panel
+
+Required properties:
+- compatible: should be "boe,hv070wsa-100"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
-- 
2.7.4

Re: [PATCH v2 05/10] panel/hv070wsa-100: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:56PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>

"dt-bindings: display: ..." is preferred subject prefix.

> 
> The patch adds bindings to BOE HV070-WSA WSVGA panel.
> Bindings are compatible with simple panel bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../devicetree/bindings/display/panel/boe,hv070wsa-100.txt         | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> new file mode 100644
> index 0000000..bfc20ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> @@ -0,0 +1,7 @@
> +BOE HV070WSA-100 7.01" WSVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "boe,hv070wsa-100"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

You have to state exactly which properties apply. Does this panel have a 
backlight? 1 supply, 2 supplies, no supplies?

Rob

[PATCH v2 05/10] panel/hv070wsa-100: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:56PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>

"dt-bindings: display: ..." is preferred subject prefix.

> 
> The patch adds bindings to BOE HV070-WSA WSVGA panel.
> Bindings are compatible with simple panel bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../devicetree/bindings/display/panel/boe,hv070wsa-100.txt         | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> new file mode 100644
> index 0000000..bfc20ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> @@ -0,0 +1,7 @@
> +BOE HV070WSA-100 7.01" WSVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "boe,hv070wsa-100"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

You have to state exactly which properties apply. Does this panel have a 
backlight? 1 supply, 2 supplies, no supplies?

Rob

Re: [PATCH v2 05/10] panel/hv070wsa-100: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:56PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>

"dt-bindings: display: ..." is preferred subject prefix.

> 
> The patch adds bindings to BOE HV070-WSA WSVGA panel.
> Bindings are compatible with simple panel bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../devicetree/bindings/display/panel/boe,hv070wsa-100.txt         | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> new file mode 100644
> index 0000000..bfc20ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt
> @@ -0,0 +1,7 @@
> +BOE HV070WSA-100 7.01" WSVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "boe,hv070wsa-100"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

You have to state exactly which properties apply. Does this panel have a 
backlight? 1 supply, 2 supplies, no supplies?

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 06/10] drm/panel: add support for BOE HV070WSA-100 panel to simple-panel

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds support for BOE HV070WSA-100 WSVGA 7.01 inch panel
in panel-simple driver. The panel is used in Exynos5250-arndale boards.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index cbf1ab4..d5da58d 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -745,6 +745,28 @@ static const struct panel_desc avic_tm070ddh03 = {
 	},
 };
 
+static const struct drm_display_mode boe_hv070wsa_mode = {
+	.clock = 40800,
+	.hdisplay = 1024,
+	.hsync_start = 1024 + 90,
+	.hsync_end = 1024 + 90 + 90,
+	.htotal = 1024 + 90 + 90 + 90,
+	.vdisplay = 600,
+	.vsync_start = 600 + 3,
+	.vsync_end = 600 + 3 + 4,
+	.vtotal = 600 + 3 + 4 + 3,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc boe_hv070wsa = {
+	.modes = &boe_hv070wsa_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 154,
+		.height = 90,
+	},
+};
+
 static const struct drm_display_mode boe_nv101wxmn51_modes[] = {
 	{
 		.clock = 71900,
@@ -2113,6 +2135,9 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "avic,tm070ddh03",
 		.data = &avic_tm070ddh03,
 	}, {
+		.compatible = "boe,hv070wsa-100",
+		.data = &boe_hv070wsa
+	}, {
 		.compatible = "boe,nv101wxmn51",
 		.data = &boe_nv101wxmn51,
 	}, {
-- 
2.7.4

[PATCH v2 06/10] drm/panel: add support for BOE HV070WSA-100 panel to simple-panel

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds support for BOE HV070WSA-100 WSVGA 7.01 inch panel
in panel-simple driver. The panel is used in Exynos5250-arndale boards.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index cbf1ab4..d5da58d 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -745,6 +745,28 @@ static const struct panel_desc avic_tm070ddh03 = {
 	},
 };
 
+static const struct drm_display_mode boe_hv070wsa_mode = {
+	.clock = 40800,
+	.hdisplay = 1024,
+	.hsync_start = 1024 + 90,
+	.hsync_end = 1024 + 90 + 90,
+	.htotal = 1024 + 90 + 90 + 90,
+	.vdisplay = 600,
+	.vsync_start = 600 + 3,
+	.vsync_end = 600 + 3 + 4,
+	.vtotal = 600 + 3 + 4 + 3,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc boe_hv070wsa = {
+	.modes = &boe_hv070wsa_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 154,
+		.height = 90,
+	},
+};
+
 static const struct drm_display_mode boe_nv101wxmn51_modes[] = {
 	{
 		.clock = 71900,
@@ -2113,6 +2135,9 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "avic,tm070ddh03",
 		.data = &avic_tm070ddh03,
 	}, {
+		.compatible = "boe,hv070wsa-100",
+		.data = &boe_hv070wsa
+	}, {
 		.compatible = "boe,nv101wxmn51",
 		.data = &boe_nv101wxmn51,
 	}, {
-- 
2.7.4

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
Bindings describe power supplies, reset gpio and video interfaces.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
new file mode 100644
index 0000000..6eda14f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
@@ -0,0 +1,37 @@
+TC358764 MIPI-DSI to LVDS panel bridge
+
+Required properties:
+  - compatible: "toshiba,tc358764"
+  - reg: the virtual channel number of a DSI peripheral
+  - vddc-supply: core voltage supply, 1.2V
+  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
+  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
+  - reset-gpios: a GPIO spec for the reset pin
+
+The device node can contain zero to two 'port' child nodes, each with one
+child 'endpoint' node, according to the bindings defined in [1].
+The following are properties specific to those nodes.
+
+port:
+  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+	bridge@0 {
+		reg = <0>;
+		compatible = "toshiba,tc358764";
+		vddc-supply = <&vcc_1v2_reg>;
+		vddio-supply = <&vcc_1v8_reg>;
+		vddlvds-supply = <&vcc_3v3_reg>;
+		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@1 {
+			reg = <1>;
+			lvds_ep: endpoint {
+				remote-endpoint = <&panel_ep>;
+			};
+		};
+	};
-- 
2.7.4

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
Bindings describe power supplies, reset gpio and video interfaces.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
new file mode 100644
index 0000000..6eda14f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
@@ -0,0 +1,37 @@
+TC358764 MIPI-DSI to LVDS panel bridge
+
+Required properties:
+  - compatible: "toshiba,tc358764"
+  - reg: the virtual channel number of a DSI peripheral
+  - vddc-supply: core voltage supply, 1.2V
+  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
+  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
+  - reset-gpios: a GPIO spec for the reset pin
+
+The device node can contain zero to two 'port' child nodes, each with one
+child 'endpoint' node, according to the bindings defined in [1].
+The following are properties specific to those nodes.
+
+port:
+  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+	bridge at 0 {
+		reg = <0>;
+		compatible = "toshiba,tc358764";
+		vddc-supply = <&vcc_1v2_reg>;
+		vddio-supply = <&vcc_1v8_reg>;
+		vddlvds-supply = <&vcc_3v3_reg>;
+		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port at 1 {
+			reg = <1>;
+			lvds_ep: endpoint {
+				remote-endpoint = <&panel_ep>;
+			};
+		};
+	};
-- 
2.7.4

Re: [PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Laurent Pinchart]

Hi Maciej,

On Wednesday, 30 May 2018 15:15:58 EEST Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
> Bindings describe power supplies, reset gpio and video interfaces.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358764.txt   | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new
> file mode 100644
> index 0000000..6eda14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> @@ -0,0 +1,37 @@
> +TC358764 MIPI-DSI to LVDS panel bridge
> +
> +Required properties:
> +  - compatible: "toshiba,tc358764"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vddc-supply: core voltage supply, 1.2V
> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
> +  - reset-gpios: a GPIO spec for the reset pin
> +
> +The device node can contain zero to two 'port' child nodes, each with one
> +child 'endpoint' node, according to the bindings defined in [1].
> +The following are properties specific to those nodes.
> +
> +port:
> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

Could you please take the comments I made on v1 into account when you'll post 
v3 ?

> +Example:
> +
> +	bridge@0 {
> +		reg = <0>;
> +		compatible = "toshiba,tc358764";
> +		vddc-supply = <&vcc_1v2_reg>;
> +		vddio-supply = <&vcc_1v8_reg>;
> +		vddlvds-supply = <&vcc_3v3_reg>;
> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@1 {
> +			reg = <1>;
> +			lvds_ep: endpoint {
> +				remote-endpoint = <&panel_ep>;
> +			};
> +		};
> +	};


-- 
Regards,

Laurent Pinchart

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Laurent Pinchart]

Hi Maciej,

On Wednesday, 30 May 2018 15:15:58 EEST Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
> Bindings describe power supplies, reset gpio and video interfaces.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358764.txt   | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new
> file mode 100644
> index 0000000..6eda14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> @@ -0,0 +1,37 @@
> +TC358764 MIPI-DSI to LVDS panel bridge
> +
> +Required properties:
> +  - compatible: "toshiba,tc358764"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vddc-supply: core voltage supply, 1.2V
> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
> +  - reset-gpios: a GPIO spec for the reset pin
> +
> +The device node can contain zero to two 'port' child nodes, each with one
> +child 'endpoint' node, according to the bindings defined in [1].
> +The following are properties specific to those nodes.
> +
> +port:
> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

Could you please take the comments I made on v1 into account when you'll post 
v3 ?

> +Example:
> +
> +	bridge at 0 {
> +		reg = <0>;
> +		compatible = "toshiba,tc358764";
> +		vddc-supply = <&vcc_1v2_reg>;
> +		vddio-supply = <&vcc_1v8_reg>;
> +		vddlvds-supply = <&vcc_3v3_reg>;
> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port at 1 {
> +			reg = <1>;
> +			lvds_ep: endpoint {
> +				remote-endpoint = <&panel_ep>;
> +			};
> +		};
> +	};


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

On 05/30/2018 02:36 PM, Laurent Pinchart wrote:
> Hi Maciej,
> 
> On Wednesday, 30 May 2018 15:15:58 EEST Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
>> Bindings describe power supplies, reset gpio and video interfaces.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   .../bindings/display/bridge/toshiba,tc358764.txt   | 37 +++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new
>> file mode 100644
>> index 0000000..6eda14f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> @@ -0,0 +1,37 @@
>> +TC358764 MIPI-DSI to LVDS panel bridge
>> +
>> +Required properties:
>> +  - compatible: "toshiba,tc358764"
>> +  - reg: the virtual channel number of a DSI peripheral
>> +  - vddc-supply: core voltage supply, 1.2V
>> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
>> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
>> +  - reset-gpios: a GPIO spec for the reset pin
>> +
>> +The device node can contain zero to two 'port' child nodes, each with one
>> +child 'endpoint' node, according to the bindings defined in [1].
>> +The following are properties specific to those nodes.
>> +
>> +port:
>> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> 
> Could you please take the comments I made on v1 into account when you'll post
> v3 ?
> 

For now I'm going to stick with the old convention and make port 0 optional,
when the bridge is a DSI child and LVDS port 1 mandatory.

The current policy does not seem to have been changed. There's a quiet new binding,
which follows the old convention:
Documentation/devicetree/bindings/connector/usb-connector.txt

If you can find an example of new bindings, which follow your convention and you
convince Rob, then I'll be completely okay with it and I'll change it the way
you propose.


>> +Example:
>> +
>> +	bridge@0 {
>> +		reg = <0>;
>> +		compatible = "toshiba,tc358764";
>> +		vddc-supply = <&vcc_1v2_reg>;
>> +		vddio-supply = <&vcc_1v8_reg>;
>> +		vddlvds-supply = <&vcc_3v3_reg>;
>> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		port@1 {
>> +			reg = <1>;
>> +			lvds_ep: endpoint {
>> +				remote-endpoint = <&panel_ep>;
>> +			};
>> +		};
>> +	};
> 
> 

Best regards,

Maciej Purski

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

On 05/30/2018 02:36 PM, Laurent Pinchart wrote:
> Hi Maciej,
> 
> On Wednesday, 30 May 2018 15:15:58 EEST Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
>> Bindings describe power supplies, reset gpio and video interfaces.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   .../bindings/display/bridge/toshiba,tc358764.txt   | 37 +++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new
>> file mode 100644
>> index 0000000..6eda14f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> @@ -0,0 +1,37 @@
>> +TC358764 MIPI-DSI to LVDS panel bridge
>> +
>> +Required properties:
>> +  - compatible: "toshiba,tc358764"
>> +  - reg: the virtual channel number of a DSI peripheral
>> +  - vddc-supply: core voltage supply, 1.2V
>> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
>> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
>> +  - reset-gpios: a GPIO spec for the reset pin
>> +
>> +The device node can contain zero to two 'port' child nodes, each with one
>> +child 'endpoint' node, according to the bindings defined in [1].
>> +The following are properties specific to those nodes.
>> +
>> +port:
>> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> 
> Could you please take the comments I made on v1 into account when you'll post
> v3 ?
> 

For now I'm going to stick with the old convention and make port 0 optional,
when the bridge is a DSI child and LVDS port 1 mandatory.

The current policy does not seem to have been changed. There's a quiet new binding,
which follows the old convention:
Documentation/devicetree/bindings/connector/usb-connector.txt

If you can find an example of new bindings, which follow your convention and you
convince Rob, then I'll be completely okay with it and I'll change it the way
you propose.


>> +Example:
>> +
>> +	bridge at 0 {
>> +		reg = <0>;
>> +		compatible = "toshiba,tc358764";
>> +		vddc-supply = <&vcc_1v2_reg>;
>> +		vddio-supply = <&vcc_1v8_reg>;
>> +		vddlvds-supply = <&vcc_3v3_reg>;
>> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		port at 1 {
>> +			reg = <1>;
>> +			lvds_ep: endpoint {
>> +				remote-endpoint = <&panel_ep>;
>> +			};
>> +		};
>> +	};
> 
> 

Best regards,

Maciej Purski

Re: [PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
> Bindings describe power supplies, reset gpio and video interfaces.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> new file mode 100644
> index 0000000..6eda14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> @@ -0,0 +1,37 @@
> +TC358764 MIPI-DSI to LVDS panel bridge
> +
> +Required properties:
> +  - compatible: "toshiba,tc358764"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vddc-supply: core voltage supply, 1.2V
> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
> +  - reset-gpios: a GPIO spec for the reset pin
> +
> +The device node can contain zero to two 'port' child nodes, each with one

How would 0 ports be valid?

> +child 'endpoint' node, according to the bindings defined in [1].
> +The following are properties specific to those nodes.
> +
> +port:
> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +	bridge@0 {
> +		reg = <0>;
> +		compatible = "toshiba,tc358764";
> +		vddc-supply = <&vcc_1v2_reg>;
> +		vddio-supply = <&vcc_1v8_reg>;
> +		vddlvds-supply = <&vcc_3v3_reg>;
> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@1 {
> +			reg = <1>;
> +			lvds_ep: endpoint {
> +				remote-endpoint = <&panel_ep>;
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
> Bindings describe power supplies, reset gpio and video interfaces.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> new file mode 100644
> index 0000000..6eda14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> @@ -0,0 +1,37 @@
> +TC358764 MIPI-DSI to LVDS panel bridge
> +
> +Required properties:
> +  - compatible: "toshiba,tc358764"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vddc-supply: core voltage supply, 1.2V
> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
> +  - reset-gpios: a GPIO spec for the reset pin
> +
> +The device node can contain zero to two 'port' child nodes, each with one

How would 0 ports be valid?

> +child 'endpoint' node, according to the bindings defined in [1].
> +The following are properties specific to those nodes.
> +
> +port:
> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +	bridge at 0 {
> +		reg = <0>;
> +		compatible = "toshiba,tc358764";
> +		vddc-supply = <&vcc_1v2_reg>;
> +		vddio-supply = <&vcc_1v8_reg>;
> +		vddlvds-supply = <&vcc_3v3_reg>;
> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port at 1 {
> +			reg = <1>;
> +			lvds_ep: endpoint {
> +				remote-endpoint = <&panel_ep>;
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 

Re: [PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Rob Herring]

On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
> Bindings describe power supplies, reset gpio and video interfaces.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> new file mode 100644
> index 0000000..6eda14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> @@ -0,0 +1,37 @@
> +TC358764 MIPI-DSI to LVDS panel bridge
> +
> +Required properties:
> +  - compatible: "toshiba,tc358764"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vddc-supply: core voltage supply, 1.2V
> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
> +  - reset-gpios: a GPIO spec for the reset pin
> +
> +The device node can contain zero to two 'port' child nodes, each with one

How would 0 ports be valid?

> +child 'endpoint' node, according to the bindings defined in [1].
> +The following are properties specific to those nodes.
> +
> +port:
> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +	bridge@0 {
> +		reg = <0>;
> +		compatible = "toshiba,tc358764";
> +		vddc-supply = <&vcc_1v2_reg>;
> +		vddio-supply = <&vcc_1v8_reg>;
> +		vddlvds-supply = <&vcc_3v3_reg>;
> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@1 {
> +			reg = <1>;
> +			lvds_ep: endpoint {
> +				remote-endpoint = <&panel_ep>;
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

On 05/31/2018 06:02 AM, Rob Herring wrote:
> On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
>> Bindings describe power supplies, reset gpio and video interfaces.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> new file mode 100644
>> index 0000000..6eda14f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> @@ -0,0 +1,37 @@
>> +TC358764 MIPI-DSI to LVDS panel bridge
>> +
>> +Required properties:
>> +  - compatible: "toshiba,tc358764"
>> +  - reg: the virtual channel number of a DSI peripheral
>> +  - vddc-supply: core voltage supply, 1.2V
>> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
>> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
>> +  - reset-gpios: a GPIO spec for the reset pin
>> +
>> +The device node can contain zero to two 'port' child nodes, each with one
> 
> How would 0 ports be valid?
> 

I'll fix this. In my opinion the output LVDS port should be mandatory
and the input DSI port should be optional, as the bridge might be a DSI
child node. According to documentation, the bridge can also be I2C controlled.
In this case, it should contain a DSI input port and LVDS output port.


>> +child 'endpoint' node, according to the bindings defined in [1].
>> +The following are properties specific to those nodes.
>> +
>> +port:
>> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +Example:
>> +
>> +	bridge@0 {
>> +		reg = <0>;
>> +		compatible = "toshiba,tc358764";
>> +		vddc-supply = <&vcc_1v2_reg>;
>> +		vddio-supply = <&vcc_1v8_reg>;
>> +		vddlvds-supply = <&vcc_3v3_reg>;
>> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		port@1 {
>> +			reg = <1>;
>> +			lvds_ep: endpoint {
>> +				remote-endpoint = <&panel_ep>;
>> +			};
>> +		};
>> +	};
>> -- 
>> 2.7.4
>>
> 
> 
> 

[PATCH v2 07/10] dt-bindings: tc358754: add DT bindings

[Maciej Purski]

On 05/31/2018 06:02 AM, Rob Herring wrote:
> On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764.
>> Bindings describe power supplies, reset gpio and video interfaces.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   .../bindings/display/bridge/toshiba,tc358764.txt   | 37 ++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> new file mode 100644
>> index 0000000..6eda14f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
>> @@ -0,0 +1,37 @@
>> +TC358764 MIPI-DSI to LVDS panel bridge
>> +
>> +Required properties:
>> +  - compatible: "toshiba,tc358764"
>> +  - reg: the virtual channel number of a DSI peripheral
>> +  - vddc-supply: core voltage supply, 1.2V
>> +  - vddio-supply: I/O voltage supply, 1.8V or 3.3V
>> +  - vddlvds-supply: LVDS1/2 voltage supply, 3.3V
>> +  - reset-gpios: a GPIO spec for the reset pin
>> +
>> +The device node can contain zero to two 'port' child nodes, each with one
> 
> How would 0 ports be valid?
> 

I'll fix this. In my opinion the output LVDS port should be mandatory
and the input DSI port should be optional, as the bridge might be a DSI
child node. According to documentation, the bridge can also be I2C controlled.
In this case, it should contain a DSI input port and LVDS output port.


>> +child 'endpoint' node, according to the bindings defined in [1].
>> +The following are properties specific to those nodes.
>> +
>> +port:
>> +  - reg: (required) can be 0 for DSI port or 1 for LVDS port;
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +Example:
>> +
>> +	bridge at 0 {
>> +		reg = <0>;
>> +		compatible = "toshiba,tc358764";
>> +		vddc-supply = <&vcc_1v2_reg>;
>> +		vddio-supply = <&vcc_1v8_reg>;
>> +		vddlvds-supply = <&vcc_3v3_reg>;
>> +		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		port at 1 {
>> +			reg = <1>;
>> +			lvds_ep: endpoint {
>> +				remote-endpoint = <&panel_ep>;
>> +			};
>> +		};
>> +	};
>> -- 
>> 2.7.4
>>
> 
> 
> 

[PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/Kconfig    |   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 557 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/tc358764.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index fa2c799..9bd3eb8 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
 	---help---
 	  Thine THC63LVD1024 LVDS/parallel converter driver.
 
+config DRM_TOSHIBA_TC358764
+	tristate "TC358764 DSI/LVDS bridge"
+	depends on DRM && DRM_PANEL
+	depends on OF
+	select DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+	help
+	  Toshiba TC358764 DSI/LVDS bridge driver
+
 config DRM_TOSHIBA_TC358767
 	tristate "Toshiba TC358767 eDP bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 35f88d4..bf7c0ce 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_SII9234) += sii9234.o
 obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
+obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
new file mode 100644
index 0000000..3109eba
--- /dev/null
+++ b/drivers/gpu/drm/bridge/tc358764.c
@@ -0,0 +1,547 @@
+/*
+ * Copyright (C) 2018 Samsung Electronics Co., Ltd
+ *
+ * Authors:
+ *	Andrzej Hajda <a.hajda@samsung.com>
+ *	Maciej Purski <m.purski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ *
+ */
+
+#include <drm/drm_atomic_helper.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
+#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
+
+/* PPI layer registers */
+#define PPI_STARTPPI		0x0104 /* START control bit */
+#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
+#define PPI_LANEENABLE		0x0134 /* Enables each lane */
+#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
+#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
+#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
+#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
+#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
+#define PPI_START_FUNCTION	1
+
+/* DSI layer registers */
+#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
+#define DSI_LANEENABLE		0x0210 /* Enables each lane */
+#define DSI_RX_START		1
+
+/* Video path registers */
+#define VP_CTRL			0x0450 /* Video Path Control */
+#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
+#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
+#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
+#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
+#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
+#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
+#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
+#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
+#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
+#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
+#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
+#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
+#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
+#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
+#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
+#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
+#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
+#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
+#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
+#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
+#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
+
+/* LVDS registers */
+#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
+#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
+#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
+#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
+#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
+#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
+#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
+#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
+				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
+
+/* Input bit numbers used in mux registers */
+enum {
+	LVI_R0,
+	LVI_R1,
+	LVI_R2,
+	LVI_R3,
+	LVI_R4,
+	LVI_R5,
+	LVI_R6,
+	LVI_R7,
+	LVI_G0,
+	LVI_G1,
+	LVI_G2,
+	LVI_G3,
+	LVI_G4,
+	LVI_G5,
+	LVI_G6,
+	LVI_G7,
+	LVI_B0,
+	LVI_B1,
+	LVI_B2,
+	LVI_B3,
+	LVI_B4,
+	LVI_B5,
+	LVI_B6,
+	LVI_B7,
+	LVI_HS,
+	LVI_VS,
+	LVI_DE,
+	LVI_L0
+};
+
+#define LV_CFG			0x049C /* LVDS Configuration */
+#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
+#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
+#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
+#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
+#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
+
+/* System registers */
+#define SYS_RST			0x0504 /* System Reset */
+#define SYS_ID			0x0580 /* System ID */
+
+#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
+#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
+#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
+#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
+#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
+#define SYS_RST_REG		BIT(5) /* Reset Register module */
+
+#define LPX_PERIOD		2
+#define TTA_SURE		3
+#define TTA_GET			0x20000
+
+/* Lane enable PPI and DSI register bits */
+#define LANEENABLE_CLEN		BIT(0)
+#define LANEENABLE_L0EN		BIT(1)
+#define LANEENABLE_L1EN		BIT(2)
+#define LANEENABLE_L2EN		BIT(3)
+#define LANEENABLE_L3EN		BIT(4)
+
+/* LVCFG fields */
+#define LV_CFG_LVEN		BIT(0)
+#define LV_CFG_LVDLINK		BIT(1)
+#define LV_CFG_CLKPOL1		BIT(2)
+#define LV_CFG_CLKPOL2		BIT(3)
+
+static const char * const tc358764_supplies[] = {
+	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
+};
+
+struct tc358764 {
+	struct device *dev;
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
+	struct gpio_desc *gpio_reset;
+
+	struct drm_panel *panel;
+};
+
+static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+	struct mipi_dsi_msg msg = {
+		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
+		.channel = dsi->channel,
+		.flags = MIPI_DSI_MSG_USE_LPM,
+		.tx_buf = &addr,
+		.tx_len = 2,
+		.rx_buf = val,
+		.rx_len = 4
+	};
+	ssize_t ret;
+
+	if (!ops || !ops->transfer)
+		return -EINVAL;
+
+	cpu_to_le16s(&addr);
+
+	ret = ops->transfer(dsi->host, &msg);
+	if (ret >= 0)
+		le32_to_cpus(val);
+
+	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
+
+	return ret;
+}
+
+static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+	u8 data[6];
+	int ret;
+	struct mipi_dsi_msg msg = {
+		.type = MIPI_DSI_GENERIC_LONG_WRITE,
+		.channel = dsi->channel,
+		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
+		.tx_buf = data,
+		.tx_len = 6
+	};
+
+	if (!ops || !ops->transfer)
+		return -EINVAL;
+
+	data[0] = addr;
+	data[1] = addr >> 8;
+	data[2] = val;
+	data[3] = val >> 8;
+	data[4] = val >> 16;
+	data[5] = val >> 24;
+
+	ret = ops->transfer(dsi->host, &msg);
+
+	return ret;
+}
+
+static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct tc358764, bridge);
+}
+
+static inline
+struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
+{
+	return container_of(connector, struct tc358764, connector);
+}
+
+static int tc358764_init(struct tc358764 *ctx)
+{
+	u32 v = 0;
+
+	tc358764_read(ctx, SYS_ID, &v);
+	dev_info(ctx->dev, "ID: %#x\n", v);
+
+	/* configure PPI counters */
+	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
+	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
+
+	/* enable four data lanes and clock lane */
+	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
+		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
+	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
+		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
+
+	/* start */
+	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
+	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
+
+	/* configure video path */
+	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
+		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);
+
+	/* reset PHY */
+	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
+		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
+	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
+		       LV_PHY0_ND(6));
+
+	/* reset bridge */
+	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
+
+	/* set bit order */
+	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
+	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
+	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
+	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
+	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
+	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
+	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
+	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
+		       LV_CFG_LVEN);
+
+	return 0;
+}
+
+static void tc358764_reset(struct tc358764 *ctx)
+{
+	msleep(20);
+	gpiod_set_value(ctx->gpio_reset, 0);
+	msleep(20);
+	gpiod_set_value(ctx->gpio_reset, 1);
+	msleep(40);
+}
+
+static void tc358764_poweroff(struct tc358764 *ctx)
+{
+	int ret;
+
+	tc358764_reset(ctx);
+
+	drm_panel_disable(ctx->panel);
+	msleep(40);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
+}
+
+static int tc358764_get_modes(struct drm_connector *connector)
+{
+	struct tc358764 *ctx = connector_to_tc358764(connector);
+
+	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
+		return ctx->panel->funcs->get_modes(ctx->panel);
+
+	return 0;
+}
+
+static const
+struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
+	.get_modes = tc358764_get_modes,
+};
+
+static const struct drm_connector_funcs tc358764_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static void tc358764_disable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+
+	tc358764_poweroff(ctx);
+}
+
+static void tc358764_pre_enable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
+					ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
+
+	tc358764_reset(ctx);
+	tc358764_init(ctx);
+}
+
+static void tc358764_enable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	int ret;
+
+	drm_panel_prepare(ctx->panel);
+
+	ret = drm_panel_enable(ctx->panel);
+	if (ret < 0)
+		pr_err("panel enable failed\n");
+
+	msleep(40);
+}
+
+static int tc358764_attach(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	struct drm_device *drm = bridge->dev;
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Encoder not found\n");
+		return -ENODEV;
+	}
+
+	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	ret = drm_connector_init(drm, &ctx->connector,
+				 &tc358764_connector_funcs,
+				 DRM_MODE_CONNECTOR_LVDS);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		return ret;
+	}
+
+	drm_connector_helper_add(&ctx->connector,
+				 &tc358764_connector_helper_funcs);
+
+	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
+
+	if (ctx->panel)
+		drm_panel_attach(ctx->panel, &ctx->connector);
+
+	drm_atomic_helper_connector_reset(&ctx->connector);
+	drm_connector_register(&ctx->connector);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs tc358764_bridge_funcs = {
+	.disable = tc358764_disable,
+	.enable = tc358764_enable,
+	.pre_enable = tc358764_pre_enable,
+	.attach = tc358764_attach,
+};
+
+static struct device_node *tc358764_of_find_panel_node(struct device *dev)
+{
+	struct device_node *np, *ep;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
+	if (!ep) {
+		pr_err("faile to get endpoint\n");
+		return NULL;
+	}
+
+	np = of_graph_get_remote_port_parent(ep);
+
+	return np;
+}
+
+static int tc358764_parse_dt(struct tc358764 *ctx)
+{
+	struct device *dev = ctx->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *lvds;
+
+	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
+						      GPIOD_OUT_LOW,
+						      "tc358764-reset");
+	if (IS_ERR(ctx->gpio_reset)) {
+		dev_err(dev, "no reset GPIO pin provided\n");
+		return PTR_ERR(ctx->gpio_reset);
+	}
+
+	lvds = tc358764_of_find_panel_node(ctx->dev);
+	if (!lvds) {
+		dev_err(dev, "cannot find panel node\n");
+		return -EINVAL;
+	}
+
+	ctx->panel = of_drm_find_panel(lvds);
+	if (!ctx->panel) {
+		dev_err(dev, "panel not registered\n");
+		return -EPROBE_DEFER;
+	}
+
+	return 0;
+}
+
+static int tc358764_configure_regulators(struct tc358764 *ctx)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
+		ctx->supplies[i].supply = tc358764_supplies[i];
+
+	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
+
+	return ret;
+}
+
+static int tc358764_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct tc358764 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
+		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;
+
+	ret = tc358764_parse_dt(ctx);
+	if (ret < 0)
+		return ret;
+
+	ret = tc358764_configure_regulators(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->bridge.funcs = &tc358764_bridge_funcs;
+	ctx->bridge.of_node = dev->of_node;
+
+	drm_bridge_add(&ctx->bridge);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		drm_bridge_remove(&ctx->bridge);
+		dev_err(dev, "failed to attach dsi\n");
+	}
+
+	return ret;
+}
+
+static int tc358764_remove(struct mipi_dsi_device *dsi)
+{
+	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	tc358764_poweroff(ctx);
+
+	mipi_dsi_detach(dsi);
+	drm_bridge_remove(&ctx->bridge);
+
+	return 0;
+}
+
+static const struct of_device_id tc358764_of_match[] = {
+	{ .compatible = "toshiba,tc358764" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc358764_of_match);
+
+static struct mipi_dsi_driver tc358764_driver = {
+	.probe = tc358764_probe,
+	.remove = tc358764_remove,
+	.driver = {
+		.name = "tc358764",
+		.owner = THIS_MODULE,
+		.of_match_table = tc358764_of_match,
+	},
+};
+module_mipi_dsi_driver(tc358764_driver);
+
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
+MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/Kconfig    |   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 557 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/tc358764.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index fa2c799..9bd3eb8 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
 	---help---
 	  Thine THC63LVD1024 LVDS/parallel converter driver.
 
+config DRM_TOSHIBA_TC358764
+	tristate "TC358764 DSI/LVDS bridge"
+	depends on DRM && DRM_PANEL
+	depends on OF
+	select DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+	help
+	  Toshiba TC358764 DSI/LVDS bridge driver
+
 config DRM_TOSHIBA_TC358767
 	tristate "Toshiba TC358767 eDP bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 35f88d4..bf7c0ce 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_SII9234) += sii9234.o
 obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
+obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
new file mode 100644
index 0000000..3109eba
--- /dev/null
+++ b/drivers/gpu/drm/bridge/tc358764.c
@@ -0,0 +1,547 @@
+/*
+ * Copyright (C) 2018 Samsung Electronics Co., Ltd
+ *
+ * Authors:
+ *	Andrzej Hajda <a.hajda@samsung.com>
+ *	Maciej Purski <m.purski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ *
+ */
+
+#include <drm/drm_atomic_helper.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
+#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
+
+/* PPI layer registers */
+#define PPI_STARTPPI		0x0104 /* START control bit */
+#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
+#define PPI_LANEENABLE		0x0134 /* Enables each lane */
+#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
+#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
+#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
+#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
+#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
+#define PPI_START_FUNCTION	1
+
+/* DSI layer registers */
+#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
+#define DSI_LANEENABLE		0x0210 /* Enables each lane */
+#define DSI_RX_START		1
+
+/* Video path registers */
+#define VP_CTRL			0x0450 /* Video Path Control */
+#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
+#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
+#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
+#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
+#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
+#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
+#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
+#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
+#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
+#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
+#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
+#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
+#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
+#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
+#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
+#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
+#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
+#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
+#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
+#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
+#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
+
+/* LVDS registers */
+#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
+#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
+#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
+#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
+#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
+#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
+#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
+#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
+				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
+
+/* Input bit numbers used in mux registers */
+enum {
+	LVI_R0,
+	LVI_R1,
+	LVI_R2,
+	LVI_R3,
+	LVI_R4,
+	LVI_R5,
+	LVI_R6,
+	LVI_R7,
+	LVI_G0,
+	LVI_G1,
+	LVI_G2,
+	LVI_G3,
+	LVI_G4,
+	LVI_G5,
+	LVI_G6,
+	LVI_G7,
+	LVI_B0,
+	LVI_B1,
+	LVI_B2,
+	LVI_B3,
+	LVI_B4,
+	LVI_B5,
+	LVI_B6,
+	LVI_B7,
+	LVI_HS,
+	LVI_VS,
+	LVI_DE,
+	LVI_L0
+};
+
+#define LV_CFG			0x049C /* LVDS Configuration */
+#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
+#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
+#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
+#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
+#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
+
+/* System registers */
+#define SYS_RST			0x0504 /* System Reset */
+#define SYS_ID			0x0580 /* System ID */
+
+#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
+#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
+#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
+#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
+#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
+#define SYS_RST_REG		BIT(5) /* Reset Register module */
+
+#define LPX_PERIOD		2
+#define TTA_SURE		3
+#define TTA_GET			0x20000
+
+/* Lane enable PPI and DSI register bits */
+#define LANEENABLE_CLEN		BIT(0)
+#define LANEENABLE_L0EN		BIT(1)
+#define LANEENABLE_L1EN		BIT(2)
+#define LANEENABLE_L2EN		BIT(3)
+#define LANEENABLE_L3EN		BIT(4)
+
+/* LVCFG fields */
+#define LV_CFG_LVEN		BIT(0)
+#define LV_CFG_LVDLINK		BIT(1)
+#define LV_CFG_CLKPOL1		BIT(2)
+#define LV_CFG_CLKPOL2		BIT(3)
+
+static const char * const tc358764_supplies[] = {
+	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
+};
+
+struct tc358764 {
+	struct device *dev;
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
+	struct gpio_desc *gpio_reset;
+
+	struct drm_panel *panel;
+};
+
+static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+	struct mipi_dsi_msg msg = {
+		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
+		.channel = dsi->channel,
+		.flags = MIPI_DSI_MSG_USE_LPM,
+		.tx_buf = &addr,
+		.tx_len = 2,
+		.rx_buf = val,
+		.rx_len = 4
+	};
+	ssize_t ret;
+
+	if (!ops || !ops->transfer)
+		return -EINVAL;
+
+	cpu_to_le16s(&addr);
+
+	ret = ops->transfer(dsi->host, &msg);
+	if (ret >= 0)
+		le32_to_cpus(val);
+
+	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
+
+	return ret;
+}
+
+static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+	u8 data[6];
+	int ret;
+	struct mipi_dsi_msg msg = {
+		.type = MIPI_DSI_GENERIC_LONG_WRITE,
+		.channel = dsi->channel,
+		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
+		.tx_buf = data,
+		.tx_len = 6
+	};
+
+	if (!ops || !ops->transfer)
+		return -EINVAL;
+
+	data[0] = addr;
+	data[1] = addr >> 8;
+	data[2] = val;
+	data[3] = val >> 8;
+	data[4] = val >> 16;
+	data[5] = val >> 24;
+
+	ret = ops->transfer(dsi->host, &msg);
+
+	return ret;
+}
+
+static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct tc358764, bridge);
+}
+
+static inline
+struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
+{
+	return container_of(connector, struct tc358764, connector);
+}
+
+static int tc358764_init(struct tc358764 *ctx)
+{
+	u32 v = 0;
+
+	tc358764_read(ctx, SYS_ID, &v);
+	dev_info(ctx->dev, "ID: %#x\n", v);
+
+	/* configure PPI counters */
+	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
+	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
+	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
+
+	/* enable four data lanes and clock lane */
+	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
+		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
+	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
+		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
+
+	/* start */
+	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
+	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
+
+	/* configure video path */
+	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
+		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);
+
+	/* reset PHY */
+	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
+		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
+	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
+		       LV_PHY0_ND(6));
+
+	/* reset bridge */
+	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
+
+	/* set bit order */
+	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
+	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
+	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
+	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
+	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
+	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
+	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
+	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
+		       LV_CFG_LVEN);
+
+	return 0;
+}
+
+static void tc358764_reset(struct tc358764 *ctx)
+{
+	msleep(20);
+	gpiod_set_value(ctx->gpio_reset, 0);
+	msleep(20);
+	gpiod_set_value(ctx->gpio_reset, 1);
+	msleep(40);
+}
+
+static void tc358764_poweroff(struct tc358764 *ctx)
+{
+	int ret;
+
+	tc358764_reset(ctx);
+
+	drm_panel_disable(ctx->panel);
+	msleep(40);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
+}
+
+static int tc358764_get_modes(struct drm_connector *connector)
+{
+	struct tc358764 *ctx = connector_to_tc358764(connector);
+
+	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
+		return ctx->panel->funcs->get_modes(ctx->panel);
+
+	return 0;
+}
+
+static const
+struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
+	.get_modes = tc358764_get_modes,
+};
+
+static const struct drm_connector_funcs tc358764_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static void tc358764_disable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+
+	tc358764_poweroff(ctx);
+}
+
+static void tc358764_pre_enable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
+					ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
+
+	tc358764_reset(ctx);
+	tc358764_init(ctx);
+}
+
+static void tc358764_enable(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	int ret;
+
+	drm_panel_prepare(ctx->panel);
+
+	ret = drm_panel_enable(ctx->panel);
+	if (ret < 0)
+		pr_err("panel enable failed\n");
+
+	msleep(40);
+}
+
+static int tc358764_attach(struct drm_bridge *bridge)
+{
+	struct tc358764 *ctx = bridge_to_tc358764(bridge);
+	struct drm_device *drm = bridge->dev;
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Encoder not found\n");
+		return -ENODEV;
+	}
+
+	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	ret = drm_connector_init(drm, &ctx->connector,
+				 &tc358764_connector_funcs,
+				 DRM_MODE_CONNECTOR_LVDS);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		return ret;
+	}
+
+	drm_connector_helper_add(&ctx->connector,
+				 &tc358764_connector_helper_funcs);
+
+	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
+
+	if (ctx->panel)
+		drm_panel_attach(ctx->panel, &ctx->connector);
+
+	drm_atomic_helper_connector_reset(&ctx->connector);
+	drm_connector_register(&ctx->connector);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs tc358764_bridge_funcs = {
+	.disable = tc358764_disable,
+	.enable = tc358764_enable,
+	.pre_enable = tc358764_pre_enable,
+	.attach = tc358764_attach,
+};
+
+static struct device_node *tc358764_of_find_panel_node(struct device *dev)
+{
+	struct device_node *np, *ep;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
+	if (!ep) {
+		pr_err("faile to get endpoint\n");
+		return NULL;
+	}
+
+	np = of_graph_get_remote_port_parent(ep);
+
+	return np;
+}
+
+static int tc358764_parse_dt(struct tc358764 *ctx)
+{
+	struct device *dev = ctx->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *lvds;
+
+	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
+						      GPIOD_OUT_LOW,
+						      "tc358764-reset");
+	if (IS_ERR(ctx->gpio_reset)) {
+		dev_err(dev, "no reset GPIO pin provided\n");
+		return PTR_ERR(ctx->gpio_reset);
+	}
+
+	lvds = tc358764_of_find_panel_node(ctx->dev);
+	if (!lvds) {
+		dev_err(dev, "cannot find panel node\n");
+		return -EINVAL;
+	}
+
+	ctx->panel = of_drm_find_panel(lvds);
+	if (!ctx->panel) {
+		dev_err(dev, "panel not registered\n");
+		return -EPROBE_DEFER;
+	}
+
+	return 0;
+}
+
+static int tc358764_configure_regulators(struct tc358764 *ctx)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
+		ctx->supplies[i].supply = tc358764_supplies[i];
+
+	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
+
+	return ret;
+}
+
+static int tc358764_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct tc358764 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
+		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;
+
+	ret = tc358764_parse_dt(ctx);
+	if (ret < 0)
+		return ret;
+
+	ret = tc358764_configure_regulators(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->bridge.funcs = &tc358764_bridge_funcs;
+	ctx->bridge.of_node = dev->of_node;
+
+	drm_bridge_add(&ctx->bridge);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		drm_bridge_remove(&ctx->bridge);
+		dev_err(dev, "failed to attach dsi\n");
+	}
+
+	return ret;
+}
+
+static int tc358764_remove(struct mipi_dsi_device *dsi)
+{
+	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	tc358764_poweroff(ctx);
+
+	mipi_dsi_detach(dsi);
+	drm_bridge_remove(&ctx->bridge);
+
+	return 0;
+}
+
+static const struct of_device_id tc358764_of_match[] = {
+	{ .compatible = "toshiba,tc358764" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc358764_of_match);
+
+static struct mipi_dsi_driver tc358764_driver = {
+	.probe = tc358764_probe,
+	.remove = tc358764_remove,
+	.driver = {
+		.name = "tc358764",
+		.owner = THIS_MODULE,
+		.of_match_table = tc358764_of_match,
+	},
+};
+module_mipi_dsi_driver(tc358764_driver);
+
+MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
+MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
+MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Archit Taneja]

Hi,

On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>   drivers/gpu/drm/bridge/Kconfig    |   9 +
>   drivers/gpu/drm/bridge/Makefile   |   1 +
>   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 557 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/tc358764.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index fa2c799..9bd3eb8 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>   	---help---
>   	  Thine THC63LVD1024 LVDS/parallel converter driver.
>   
> +config DRM_TOSHIBA_TC358764
> +	tristate "TC358764 DSI/LVDS bridge"
> +	depends on DRM && DRM_PANEL
> +	depends on OF
> +	select DRM_MIPI_DSI
> +	select VIDEOMODE_HELPERS

I don't see videomode usage in the driver, can we drop this if it isn't
used?

> +	help
> +	  Toshiba TC358764 DSI/LVDS bridge driver
> +
>   config DRM_TOSHIBA_TC358767
>   	tristate "Toshiba TC358767 eDP bridge"
>   	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 35f88d4..bf7c0ce 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>   obj-$(CONFIG_DRM_SII9234) += sii9234.o
>   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> new file mode 100644
> index 0000000..3109eba
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358764.c
> @@ -0,0 +1,547 @@

We'd need a SPDX license here?
> +/*
> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
> + *
> + * Authors:
> + *	Andrzej Hajda <a.hajda@samsung.com>
> + *	Maciej Purski <m.purski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +
> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +
> +/* PPI layer registers */
> +#define PPI_STARTPPI		0x0104 /* START control bit */
> +#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
> +#define PPI_LANEENABLE		0x0134 /* Enables each lane */
> +#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
> +#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
> +#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
> +#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
> +#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
> +#define PPI_START_FUNCTION	1
> +
> +/* DSI layer registers */
> +#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
> +#define DSI_LANEENABLE		0x0210 /* Enables each lane */
> +#define DSI_RX_START		1
> +
> +/* Video path registers */
> +#define VP_CTRL			0x0450 /* Video Path Control */
> +#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
> +#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
> +#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
> +#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
> +#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
> +#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
> +#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
> +#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
> +#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
> +#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
> +#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
> +#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
> +#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
> +#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
> +#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
> +
> +/* LVDS registers */
> +#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
> +#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
> +#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
> +#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
> +#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
> +#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
> +#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
> +#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
> +				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
> +
> +/* Input bit numbers used in mux registers */
> +enum {
> +	LVI_R0,
> +	LVI_R1,
> +	LVI_R2,
> +	LVI_R3,
> +	LVI_R4,
> +	LVI_R5,
> +	LVI_R6,
> +	LVI_R7,
> +	LVI_G0,
> +	LVI_G1,
> +	LVI_G2,
> +	LVI_G3,
> +	LVI_G4,
> +	LVI_G5,
> +	LVI_G6,
> +	LVI_G7,
> +	LVI_B0,
> +	LVI_B1,
> +	LVI_B2,
> +	LVI_B3,
> +	LVI_B4,
> +	LVI_B5,
> +	LVI_B6,
> +	LVI_B7,
> +	LVI_HS,
> +	LVI_VS,
> +	LVI_DE,
> +	LVI_L0
> +};
> +
> +#define LV_CFG			0x049C /* LVDS Configuration */
> +#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
> +#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
> +#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
> +#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
> +#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
> +
> +/* System registers */
> +#define SYS_RST			0x0504 /* System Reset */
> +#define SYS_ID			0x0580 /* System ID */
> +
> +#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
> +#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
> +#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
> +#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
> +#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
> +#define SYS_RST_REG		BIT(5) /* Reset Register module */
> +
> +#define LPX_PERIOD		2
> +#define TTA_SURE		3
> +#define TTA_GET			0x20000
> +
> +/* Lane enable PPI and DSI register bits */
> +#define LANEENABLE_CLEN		BIT(0)
> +#define LANEENABLE_L0EN		BIT(1)
> +#define LANEENABLE_L1EN		BIT(2)
> +#define LANEENABLE_L2EN		BIT(3)
> +#define LANEENABLE_L3EN		BIT(4)
> +
> +/* LVCFG fields */
> +#define LV_CFG_LVEN		BIT(0)
> +#define LV_CFG_LVDLINK		BIT(1)
> +#define LV_CFG_CLKPOL1		BIT(2)
> +#define LV_CFG_CLKPOL2		BIT(3)
> +
> +static const char * const tc358764_supplies[] = {
> +	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
> +};
> +
> +struct tc358764 {
> +	struct device *dev;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
> +	struct gpio_desc *gpio_reset;
> +
> +	struct drm_panel *panel;
> +};
> +
> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM,
> +		.tx_buf = &addr,
> +		.tx_len = 2,
> +		.rx_buf = val,
> +		.rx_len = 4
> +	};
> +	ssize_t ret;
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	cpu_to_le16s(&addr);
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +	if (ret >= 0)
> +		le32_to_cpus(val); > +
> +	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
> +
> +	return ret;
> +}
> +
> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	u8 data[6];
> +	int ret;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_LONG_WRITE,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
> +		.tx_buf = data,
> +		.tx_len = 6
> +	};
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	data[0] = addr;
> +	data[1] = addr >> 8;
> +	data[2] = val;
> +	data[3] = val >> 8;
> +	data[4] = val >> 16;
> +	data[5] = val >> 24;
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +
> +	return ret;
> +}
> +
> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tc358764, bridge);
> +}
> +
> +static inline
> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct tc358764, connector);
> +}
> +
> +static int tc358764_init(struct tc358764 *ctx)
> +{
> +	u32 v = 0;
> +
> +	tc358764_read(ctx, SYS_ID, &v);
> +	dev_info(ctx->dev, "ID: %#x\n", v);
> +
> +	/* configure PPI counters */
> +	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> +	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
> +	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
> +
> +	/* enable four data lanes and clock lane */
> +	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +
> +	/* start */
> +	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
> +	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
> +
> +	/* configure video path */
> +	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
> +		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);

Could we specify somewhere that
> +
> +	/* reset PHY */
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
> +		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
> +		       LV_PHY0_ND(6));
> +
> +	/* reset bridge */
> +	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
> +
> +	/* set bit order */
> +	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
> +	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
> +	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
> +	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> +	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
> +		       LV_CFG_LVEN);
> +
> +	return 0;
> +}
> +
> +static void tc358764_reset(struct tc358764 *ctx)
> +{
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 0);
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	msleep(40);
> +}
> +
> +static void tc358764_poweroff(struct tc358764 *ctx)
> +{
> +	int ret;
> +
> +	tc358764_reset(ctx);
> +
> +	drm_panel_disable(ctx->panel);
> +	msleep(40);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
> +}
> +
> +static int tc358764_get_modes(struct drm_connector *connector)
> +{
> +	struct tc358764 *ctx = connector_to_tc358764(connector);
> +
> +	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
> +		return ctx->panel->funcs->get_modes(ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const
> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
> +	.get_modes = tc358764_get_modes,
> +};
> +
> +static const struct drm_connector_funcs tc358764_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void tc358764_disable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +
> +	tc358764_poweroff(ctx);
> +}
> +
> +static void tc358764_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> +					ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
> +
> +	tc358764_reset(ctx);
> +	tc358764_init(ctx);
> +}
> +
> +static void tc358764_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret;
> +
> +	drm_panel_prepare(ctx->panel);
> +
> +	ret = drm_panel_enable(ctx->panel);
> +	if (ret < 0)
> +		pr_err("panel enable failed\n");
> +
> +	msleep(40);
> +}
> +
> +static int tc358764_attach(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Encoder not found\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	ret = drm_connector_init(drm, &ctx->connector,
> +				 &tc358764_connector_funcs,
> +				 DRM_MODE_CONNECTOR_LVDS);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(&ctx->connector,
> +				 &tc358764_connector_helper_funcs);
> +
> +	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
> +
> +	if (ctx->panel)
> +		drm_panel_attach(ctx->panel, &ctx->connector);
> +
> +	drm_atomic_helper_connector_reset(&ctx->connector);
> +	drm_connector_register(&ctx->connector);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
> +	.disable = tc358764_disable,
> +	.enable = tc358764_enable,
> +	.pre_enable = tc358764_pre_enable,
> +	.attach = tc358764_attach,
> +};
> +
> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
> +{
> +	struct device_node *np, *ep;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
> +	if (!ep) {
> +		pr_err("faile to get endpoint\n");
> +		return NULL;
> +	}
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +
> +	return np;
> +}
> +
> +static int tc358764_parse_dt(struct tc358764 *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np = dev->of_node;
> +	struct device_node *lvds;
> +
> +	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
> +						      GPIOD_OUT_LOW,
> +						      "tc358764-reset");
> +	if (IS_ERR(ctx->gpio_reset)) {
> +		dev_err(dev, "no reset GPIO pin provided\n");
> +		return PTR_ERR(ctx->gpio_reset);
> +	}
> +
> +	lvds = tc358764_of_find_panel_node(ctx->dev);
> +	if (!lvds) {
> +		dev_err(dev, "cannot find panel node\n");
> +		return -EINVAL;
> +	}
> +
> +	ctx->panel = of_drm_find_panel(lvds);
> +	if (!ctx->panel) {
> +		dev_err(dev, "panel not registered\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tc358764_configure_regulators(struct tc358764 *ctx)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
> +		ctx->supplies[i].supply = tc358764_supplies[i];
> +
> +	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int tc358764_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct tc358764 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
> +		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;

if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
add the LPM flag here.

Looks good to me otherwise. Once the DT port issue is concluded:

Reviewed-by: Archit Taneja <architt@codeaurora.org>

> +
> +	ret = tc358764_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tc358764_configure_regulators(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->bridge.funcs = &tc358764_bridge_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +
> +	drm_bridge_add(&ctx->bridge);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		drm_bridge_remove(&ctx->bridge);
> +		dev_err(dev, "failed to attach dsi\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int tc358764_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	tc358764_poweroff(ctx);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_bridge_remove(&ctx->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc358764_of_match[] = {
> +	{ .compatible = "toshiba,tc358764" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
> +
> +static struct mipi_dsi_driver tc358764_driver = {
> +	.probe = tc358764_probe,
> +	.remove = tc358764_remove,
> +	.driver = {
> +		.name = "tc358764",
> +		.owner = THIS_MODULE,
> +		.of_match_table = tc358764_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(tc358764_driver);
> +
> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
> +MODULE_LICENSE("GPL v2");
> 

[PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Archit Taneja]

Hi,

On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>   drivers/gpu/drm/bridge/Kconfig    |   9 +
>   drivers/gpu/drm/bridge/Makefile   |   1 +
>   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 557 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/tc358764.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index fa2c799..9bd3eb8 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>   	---help---
>   	  Thine THC63LVD1024 LVDS/parallel converter driver.
>   
> +config DRM_TOSHIBA_TC358764
> +	tristate "TC358764 DSI/LVDS bridge"
> +	depends on DRM && DRM_PANEL
> +	depends on OF
> +	select DRM_MIPI_DSI
> +	select VIDEOMODE_HELPERS

I don't see videomode usage in the driver, can we drop this if it isn't
used?

> +	help
> +	  Toshiba TC358764 DSI/LVDS bridge driver
> +
>   config DRM_TOSHIBA_TC358767
>   	tristate "Toshiba TC358767 eDP bridge"
>   	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 35f88d4..bf7c0ce 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>   obj-$(CONFIG_DRM_SII9234) += sii9234.o
>   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> new file mode 100644
> index 0000000..3109eba
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358764.c
> @@ -0,0 +1,547 @@

We'd need a SPDX license here?
> +/*
> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
> + *
> + * Authors:
> + *	Andrzej Hajda <a.hajda@samsung.com>
> + *	Maciej Purski <m.purski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +
> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +
> +/* PPI layer registers */
> +#define PPI_STARTPPI		0x0104 /* START control bit */
> +#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
> +#define PPI_LANEENABLE		0x0134 /* Enables each lane */
> +#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
> +#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
> +#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
> +#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
> +#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
> +#define PPI_START_FUNCTION	1
> +
> +/* DSI layer registers */
> +#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
> +#define DSI_LANEENABLE		0x0210 /* Enables each lane */
> +#define DSI_RX_START		1
> +
> +/* Video path registers */
> +#define VP_CTRL			0x0450 /* Video Path Control */
> +#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
> +#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
> +#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
> +#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
> +#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
> +#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
> +#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
> +#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
> +#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
> +#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
> +#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
> +#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
> +#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
> +#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
> +#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
> +
> +/* LVDS registers */
> +#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
> +#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
> +#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
> +#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
> +#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
> +#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
> +#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
> +#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
> +				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
> +
> +/* Input bit numbers used in mux registers */
> +enum {
> +	LVI_R0,
> +	LVI_R1,
> +	LVI_R2,
> +	LVI_R3,
> +	LVI_R4,
> +	LVI_R5,
> +	LVI_R6,
> +	LVI_R7,
> +	LVI_G0,
> +	LVI_G1,
> +	LVI_G2,
> +	LVI_G3,
> +	LVI_G4,
> +	LVI_G5,
> +	LVI_G6,
> +	LVI_G7,
> +	LVI_B0,
> +	LVI_B1,
> +	LVI_B2,
> +	LVI_B3,
> +	LVI_B4,
> +	LVI_B5,
> +	LVI_B6,
> +	LVI_B7,
> +	LVI_HS,
> +	LVI_VS,
> +	LVI_DE,
> +	LVI_L0
> +};
> +
> +#define LV_CFG			0x049C /* LVDS Configuration */
> +#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
> +#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
> +#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
> +#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
> +#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
> +
> +/* System registers */
> +#define SYS_RST			0x0504 /* System Reset */
> +#define SYS_ID			0x0580 /* System ID */
> +
> +#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
> +#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
> +#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
> +#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
> +#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
> +#define SYS_RST_REG		BIT(5) /* Reset Register module */
> +
> +#define LPX_PERIOD		2
> +#define TTA_SURE		3
> +#define TTA_GET			0x20000
> +
> +/* Lane enable PPI and DSI register bits */
> +#define LANEENABLE_CLEN		BIT(0)
> +#define LANEENABLE_L0EN		BIT(1)
> +#define LANEENABLE_L1EN		BIT(2)
> +#define LANEENABLE_L2EN		BIT(3)
> +#define LANEENABLE_L3EN		BIT(4)
> +
> +/* LVCFG fields */
> +#define LV_CFG_LVEN		BIT(0)
> +#define LV_CFG_LVDLINK		BIT(1)
> +#define LV_CFG_CLKPOL1		BIT(2)
> +#define LV_CFG_CLKPOL2		BIT(3)
> +
> +static const char * const tc358764_supplies[] = {
> +	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
> +};
> +
> +struct tc358764 {
> +	struct device *dev;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
> +	struct gpio_desc *gpio_reset;
> +
> +	struct drm_panel *panel;
> +};
> +
> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM,
> +		.tx_buf = &addr,
> +		.tx_len = 2,
> +		.rx_buf = val,
> +		.rx_len = 4
> +	};
> +	ssize_t ret;
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	cpu_to_le16s(&addr);
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +	if (ret >= 0)
> +		le32_to_cpus(val); > +
> +	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
> +
> +	return ret;
> +}
> +
> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	u8 data[6];
> +	int ret;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_LONG_WRITE,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
> +		.tx_buf = data,
> +		.tx_len = 6
> +	};
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	data[0] = addr;
> +	data[1] = addr >> 8;
> +	data[2] = val;
> +	data[3] = val >> 8;
> +	data[4] = val >> 16;
> +	data[5] = val >> 24;
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +
> +	return ret;
> +}
> +
> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tc358764, bridge);
> +}
> +
> +static inline
> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct tc358764, connector);
> +}
> +
> +static int tc358764_init(struct tc358764 *ctx)
> +{
> +	u32 v = 0;
> +
> +	tc358764_read(ctx, SYS_ID, &v);
> +	dev_info(ctx->dev, "ID: %#x\n", v);
> +
> +	/* configure PPI counters */
> +	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> +	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
> +	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
> +
> +	/* enable four data lanes and clock lane */
> +	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +
> +	/* start */
> +	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
> +	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
> +
> +	/* configure video path */
> +	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
> +		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);

Could we specify somewhere that
> +
> +	/* reset PHY */
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
> +		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
> +		       LV_PHY0_ND(6));
> +
> +	/* reset bridge */
> +	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
> +
> +	/* set bit order */
> +	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
> +	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
> +	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
> +	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> +	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
> +		       LV_CFG_LVEN);
> +
> +	return 0;
> +}
> +
> +static void tc358764_reset(struct tc358764 *ctx)
> +{
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 0);
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	msleep(40);
> +}
> +
> +static void tc358764_poweroff(struct tc358764 *ctx)
> +{
> +	int ret;
> +
> +	tc358764_reset(ctx);
> +
> +	drm_panel_disable(ctx->panel);
> +	msleep(40);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
> +}
> +
> +static int tc358764_get_modes(struct drm_connector *connector)
> +{
> +	struct tc358764 *ctx = connector_to_tc358764(connector);
> +
> +	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
> +		return ctx->panel->funcs->get_modes(ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const
> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
> +	.get_modes = tc358764_get_modes,
> +};
> +
> +static const struct drm_connector_funcs tc358764_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void tc358764_disable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +
> +	tc358764_poweroff(ctx);
> +}
> +
> +static void tc358764_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> +					ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
> +
> +	tc358764_reset(ctx);
> +	tc358764_init(ctx);
> +}
> +
> +static void tc358764_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret;
> +
> +	drm_panel_prepare(ctx->panel);
> +
> +	ret = drm_panel_enable(ctx->panel);
> +	if (ret < 0)
> +		pr_err("panel enable failed\n");
> +
> +	msleep(40);
> +}
> +
> +static int tc358764_attach(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Encoder not found\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	ret = drm_connector_init(drm, &ctx->connector,
> +				 &tc358764_connector_funcs,
> +				 DRM_MODE_CONNECTOR_LVDS);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(&ctx->connector,
> +				 &tc358764_connector_helper_funcs);
> +
> +	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
> +
> +	if (ctx->panel)
> +		drm_panel_attach(ctx->panel, &ctx->connector);
> +
> +	drm_atomic_helper_connector_reset(&ctx->connector);
> +	drm_connector_register(&ctx->connector);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
> +	.disable = tc358764_disable,
> +	.enable = tc358764_enable,
> +	.pre_enable = tc358764_pre_enable,
> +	.attach = tc358764_attach,
> +};
> +
> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
> +{
> +	struct device_node *np, *ep;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
> +	if (!ep) {
> +		pr_err("faile to get endpoint\n");
> +		return NULL;
> +	}
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +
> +	return np;
> +}
> +
> +static int tc358764_parse_dt(struct tc358764 *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np = dev->of_node;
> +	struct device_node *lvds;
> +
> +	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
> +						      GPIOD_OUT_LOW,
> +						      "tc358764-reset");
> +	if (IS_ERR(ctx->gpio_reset)) {
> +		dev_err(dev, "no reset GPIO pin provided\n");
> +		return PTR_ERR(ctx->gpio_reset);
> +	}
> +
> +	lvds = tc358764_of_find_panel_node(ctx->dev);
> +	if (!lvds) {
> +		dev_err(dev, "cannot find panel node\n");
> +		return -EINVAL;
> +	}
> +
> +	ctx->panel = of_drm_find_panel(lvds);
> +	if (!ctx->panel) {
> +		dev_err(dev, "panel not registered\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tc358764_configure_regulators(struct tc358764 *ctx)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
> +		ctx->supplies[i].supply = tc358764_supplies[i];
> +
> +	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int tc358764_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct tc358764 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
> +		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;

if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
add the LPM flag here.

Looks good to me otherwise. Once the DT port issue is concluded:

Reviewed-by: Archit Taneja <architt@codeaurora.org>

> +
> +	ret = tc358764_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tc358764_configure_regulators(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->bridge.funcs = &tc358764_bridge_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +
> +	drm_bridge_add(&ctx->bridge);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		drm_bridge_remove(&ctx->bridge);
> +		dev_err(dev, "failed to attach dsi\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int tc358764_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	tc358764_poweroff(ctx);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_bridge_remove(&ctx->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc358764_of_match[] = {
> +	{ .compatible = "toshiba,tc358764" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
> +
> +static struct mipi_dsi_driver tc358764_driver = {
> +	.probe = tc358764_probe,
> +	.remove = tc358764_remove,
> +	.driver = {
> +		.name = "tc358764",
> +		.owner = THIS_MODULE,
> +		.of_match_table = tc358764_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(tc358764_driver);
> +
> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Archit Taneja]

Hi,

On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>   drivers/gpu/drm/bridge/Kconfig    |   9 +
>   drivers/gpu/drm/bridge/Makefile   |   1 +
>   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 557 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/tc358764.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index fa2c799..9bd3eb8 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>   	---help---
>   	  Thine THC63LVD1024 LVDS/parallel converter driver.
>   
> +config DRM_TOSHIBA_TC358764
> +	tristate "TC358764 DSI/LVDS bridge"
> +	depends on DRM && DRM_PANEL
> +	depends on OF
> +	select DRM_MIPI_DSI
> +	select VIDEOMODE_HELPERS

I don't see videomode usage in the driver, can we drop this if it isn't
used?

> +	help
> +	  Toshiba TC358764 DSI/LVDS bridge driver
> +
>   config DRM_TOSHIBA_TC358767
>   	tristate "Toshiba TC358767 eDP bridge"
>   	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 35f88d4..bf7c0ce 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>   obj-$(CONFIG_DRM_SII9234) += sii9234.o
>   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> new file mode 100644
> index 0000000..3109eba
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358764.c
> @@ -0,0 +1,547 @@

We'd need a SPDX license here?
> +/*
> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
> + *
> + * Authors:
> + *	Andrzej Hajda <a.hajda@samsung.com>
> + *	Maciej Purski <m.purski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +
> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +
> +/* PPI layer registers */
> +#define PPI_STARTPPI		0x0104 /* START control bit */
> +#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
> +#define PPI_LANEENABLE		0x0134 /* Enables each lane */
> +#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
> +#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
> +#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
> +#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
> +#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
> +#define PPI_START_FUNCTION	1
> +
> +/* DSI layer registers */
> +#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
> +#define DSI_LANEENABLE		0x0210 /* Enables each lane */
> +#define DSI_RX_START		1
> +
> +/* Video path registers */
> +#define VP_CTRL			0x0450 /* Video Path Control */
> +#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
> +#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
> +#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
> +#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
> +#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
> +#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
> +#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
> +#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
> +#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
> +#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
> +#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
> +#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
> +#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
> +#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
> +#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
> +
> +/* LVDS registers */
> +#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
> +#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
> +#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
> +#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
> +#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
> +#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
> +#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
> +#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
> +				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
> +
> +/* Input bit numbers used in mux registers */
> +enum {
> +	LVI_R0,
> +	LVI_R1,
> +	LVI_R2,
> +	LVI_R3,
> +	LVI_R4,
> +	LVI_R5,
> +	LVI_R6,
> +	LVI_R7,
> +	LVI_G0,
> +	LVI_G1,
> +	LVI_G2,
> +	LVI_G3,
> +	LVI_G4,
> +	LVI_G5,
> +	LVI_G6,
> +	LVI_G7,
> +	LVI_B0,
> +	LVI_B1,
> +	LVI_B2,
> +	LVI_B3,
> +	LVI_B4,
> +	LVI_B5,
> +	LVI_B6,
> +	LVI_B7,
> +	LVI_HS,
> +	LVI_VS,
> +	LVI_DE,
> +	LVI_L0
> +};
> +
> +#define LV_CFG			0x049C /* LVDS Configuration */
> +#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
> +#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
> +#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
> +#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
> +#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
> +
> +/* System registers */
> +#define SYS_RST			0x0504 /* System Reset */
> +#define SYS_ID			0x0580 /* System ID */
> +
> +#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
> +#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
> +#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
> +#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
> +#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
> +#define SYS_RST_REG		BIT(5) /* Reset Register module */
> +
> +#define LPX_PERIOD		2
> +#define TTA_SURE		3
> +#define TTA_GET			0x20000
> +
> +/* Lane enable PPI and DSI register bits */
> +#define LANEENABLE_CLEN		BIT(0)
> +#define LANEENABLE_L0EN		BIT(1)
> +#define LANEENABLE_L1EN		BIT(2)
> +#define LANEENABLE_L2EN		BIT(3)
> +#define LANEENABLE_L3EN		BIT(4)
> +
> +/* LVCFG fields */
> +#define LV_CFG_LVEN		BIT(0)
> +#define LV_CFG_LVDLINK		BIT(1)
> +#define LV_CFG_CLKPOL1		BIT(2)
> +#define LV_CFG_CLKPOL2		BIT(3)
> +
> +static const char * const tc358764_supplies[] = {
> +	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
> +};
> +
> +struct tc358764 {
> +	struct device *dev;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
> +	struct gpio_desc *gpio_reset;
> +
> +	struct drm_panel *panel;
> +};
> +
> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM,
> +		.tx_buf = &addr,
> +		.tx_len = 2,
> +		.rx_buf = val,
> +		.rx_len = 4
> +	};
> +	ssize_t ret;
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	cpu_to_le16s(&addr);
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +	if (ret >= 0)
> +		le32_to_cpus(val); > +
> +	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
> +
> +	return ret;
> +}
> +
> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	u8 data[6];
> +	int ret;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_LONG_WRITE,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
> +		.tx_buf = data,
> +		.tx_len = 6
> +	};
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	data[0] = addr;
> +	data[1] = addr >> 8;
> +	data[2] = val;
> +	data[3] = val >> 8;
> +	data[4] = val >> 16;
> +	data[5] = val >> 24;
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +
> +	return ret;
> +}
> +
> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tc358764, bridge);
> +}
> +
> +static inline
> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct tc358764, connector);
> +}
> +
> +static int tc358764_init(struct tc358764 *ctx)
> +{
> +	u32 v = 0;
> +
> +	tc358764_read(ctx, SYS_ID, &v);
> +	dev_info(ctx->dev, "ID: %#x\n", v);
> +
> +	/* configure PPI counters */
> +	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> +	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
> +	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
> +
> +	/* enable four data lanes and clock lane */
> +	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +
> +	/* start */
> +	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
> +	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
> +
> +	/* configure video path */
> +	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
> +		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);

Could we specify somewhere that
> +
> +	/* reset PHY */
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
> +		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
> +		       LV_PHY0_ND(6));
> +
> +	/* reset bridge */
> +	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
> +
> +	/* set bit order */
> +	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
> +	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
> +	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
> +	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> +	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
> +		       LV_CFG_LVEN);
> +
> +	return 0;
> +}
> +
> +static void tc358764_reset(struct tc358764 *ctx)
> +{
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 0);
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	msleep(40);
> +}
> +
> +static void tc358764_poweroff(struct tc358764 *ctx)
> +{
> +	int ret;
> +
> +	tc358764_reset(ctx);
> +
> +	drm_panel_disable(ctx->panel);
> +	msleep(40);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
> +}
> +
> +static int tc358764_get_modes(struct drm_connector *connector)
> +{
> +	struct tc358764 *ctx = connector_to_tc358764(connector);
> +
> +	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
> +		return ctx->panel->funcs->get_modes(ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const
> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
> +	.get_modes = tc358764_get_modes,
> +};
> +
> +static const struct drm_connector_funcs tc358764_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void tc358764_disable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +
> +	tc358764_poweroff(ctx);
> +}
> +
> +static void tc358764_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> +					ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
> +
> +	tc358764_reset(ctx);
> +	tc358764_init(ctx);
> +}
> +
> +static void tc358764_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret;
> +
> +	drm_panel_prepare(ctx->panel);
> +
> +	ret = drm_panel_enable(ctx->panel);
> +	if (ret < 0)
> +		pr_err("panel enable failed\n");
> +
> +	msleep(40);
> +}
> +
> +static int tc358764_attach(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Encoder not found\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	ret = drm_connector_init(drm, &ctx->connector,
> +				 &tc358764_connector_funcs,
> +				 DRM_MODE_CONNECTOR_LVDS);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(&ctx->connector,
> +				 &tc358764_connector_helper_funcs);
> +
> +	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
> +
> +	if (ctx->panel)
> +		drm_panel_attach(ctx->panel, &ctx->connector);
> +
> +	drm_atomic_helper_connector_reset(&ctx->connector);
> +	drm_connector_register(&ctx->connector);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
> +	.disable = tc358764_disable,
> +	.enable = tc358764_enable,
> +	.pre_enable = tc358764_pre_enable,
> +	.attach = tc358764_attach,
> +};
> +
> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
> +{
> +	struct device_node *np, *ep;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
> +	if (!ep) {
> +		pr_err("faile to get endpoint\n");
> +		return NULL;
> +	}
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +
> +	return np;
> +}
> +
> +static int tc358764_parse_dt(struct tc358764 *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np = dev->of_node;
> +	struct device_node *lvds;
> +
> +	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
> +						      GPIOD_OUT_LOW,
> +						      "tc358764-reset");
> +	if (IS_ERR(ctx->gpio_reset)) {
> +		dev_err(dev, "no reset GPIO pin provided\n");
> +		return PTR_ERR(ctx->gpio_reset);
> +	}
> +
> +	lvds = tc358764_of_find_panel_node(ctx->dev);
> +	if (!lvds) {
> +		dev_err(dev, "cannot find panel node\n");
> +		return -EINVAL;
> +	}
> +
> +	ctx->panel = of_drm_find_panel(lvds);
> +	if (!ctx->panel) {
> +		dev_err(dev, "panel not registered\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tc358764_configure_regulators(struct tc358764 *ctx)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
> +		ctx->supplies[i].supply = tc358764_supplies[i];
> +
> +	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int tc358764_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct tc358764 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
> +		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;

if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
add the LPM flag here.

Looks good to me otherwise. Once the DT port issue is concluded:

Reviewed-by: Archit Taneja <architt@codeaurora.org>

> +
> +	ret = tc358764_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tc358764_configure_regulators(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->bridge.funcs = &tc358764_bridge_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +
> +	drm_bridge_add(&ctx->bridge);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		drm_bridge_remove(&ctx->bridge);
> +		dev_err(dev, "failed to attach dsi\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int tc358764_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	tc358764_poweroff(ctx);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_bridge_remove(&ctx->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc358764_of_match[] = {
> +	{ .compatible = "toshiba,tc358764" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
> +
> +static struct mipi_dsi_driver tc358764_driver = {
> +	.probe = tc358764_probe,
> +	.remove = tc358764_remove,
> +	.driver = {
> +		.name = "tc358764",
> +		.owner = THIS_MODULE,
> +		.of_match_table = tc358764_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(tc358764_driver);
> +
> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
> +MODULE_LICENSE("GPL v2");
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Maciej Purski]

On 05/31/2018 08:51 AM, Archit Taneja wrote:
> Hi,
> 
> On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>>   drivers/gpu/drm/bridge/Kconfig    |   9 +
>>   drivers/gpu/drm/bridge/Makefile   |   1 +
>>   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 557 insertions(+)
>>   create mode 100644 drivers/gpu/drm/bridge/tc358764.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index fa2c799..9bd3eb8 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>>       ---help---
>>         Thine THC63LVD1024 LVDS/parallel converter driver.
>> +config DRM_TOSHIBA_TC358764
>> +    tristate "TC358764 DSI/LVDS bridge"
>> +    depends on DRM && DRM_PANEL
>> +    depends on OF
>> +    select DRM_MIPI_DSI
>> +    select VIDEOMODE_HELPERS
> 
> I don't see videomode usage in the driver, can we drop this if it isn't
> used?
> 

It seems that those are some remains of old versions. It is not required now.

>> +    help
>> +      Toshiba TC358764 DSI/LVDS bridge driver
>> +
>>   config DRM_TOSHIBA_TC358767
>>       tristate "Toshiba TC358767 eDP bridge"
>>       depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index 35f88d4..bf7c0ce 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>>   obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
>> new file mode 100644
>> index 0000000..3109eba
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/tc358764.c
>> @@ -0,0 +1,547 @@
> 
> We'd need a SPDX license here?
>> +/*
>> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
>> + *
>> + * Authors:
>> + *    Andrzej Hajda <a.hajda@samsung.com>
>> + *    Maciej Purski <m.purski@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.
>> + *
>> + */
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_panel.h>
>> +
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#include <video/mipi_display.h>
>> +#include <video/of_videomode.h>
>> +#include <video/videomode.h>
>> +
>> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
>> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
>> +
>> +/* PPI layer registers */
>> +#define PPI_STARTPPI        0x0104 /* START control bit */
>> +#define PPI_LPTXTIMECNT        0x0114 /* LPTX timing signal */
>> +#define PPI_LANEENABLE        0x0134 /* Enables each lane */
>> +#define PPI_TX_RX_TA        0x013C /* BTA timing parameters */
>> +#define PPI_D0S_CLRSIPOCOUNT    0x0164 /* Assertion timer for Lane 0 */
>> +#define PPI_D1S_CLRSIPOCOUNT    0x0168 /* Assertion timer for Lane 1 */
>> +#define PPI_D2S_CLRSIPOCOUNT    0x016C /* Assertion timer for Lane 2 */
>> +#define PPI_D3S_CLRSIPOCOUNT    0x0170 /* Assertion timer for Lane 3 */
>> +#define PPI_START_FUNCTION    1
>> +
>> +/* DSI layer registers */
>> +#define DSI_STARTDSI        0x0204 /* START control bit of DSI-TX */
>> +#define DSI_LANEENABLE        0x0210 /* Enables each lane */
>> +#define DSI_RX_START        1
>> +
>> +/* Video path registers */
>> +#define VP_CTRL            0x0450 /* Video Path Control */
>> +#define VP_CTRL_MSF(v)        FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
>> +#define VP_CTRL_VTGEN(v)    FLD_VAL(v, 4, 4) /* Use chip clock for timing */
>> +#define VP_CTRL_EVTMODE(v)    FLD_VAL(v, 5, 5) /* Event mode */
>> +#define VP_CTRL_RGB888(v)    FLD_VAL(v, 8, 8) /* RGB888 mode */
>> +#define VP_CTRL_VSDELAY(v)    FLD_VAL(v, 31, 20) /* VSYNC delay */
>> +#define VP_CTRL_HSPOL        BIT(17) /* Polarity of HSYNC signal */
>> +#define VP_CTRL_DEPOL        BIT(18) /* Polarity of DE signal */
>> +#define VP_CTRL_VSPOL        BIT(19) /* Polarity of VSYNC signal */
>> +#define VP_HTIM1        0x0454 /* Horizontal Timing Control 1 */
>> +#define VP_HTIM1_HBP(v)        FLD_VAL(v, 24, 16)
>> +#define VP_HTIM1_HSYNC(v)    FLD_VAL(v, 8, 0)
>> +#define VP_HTIM2        0x0458 /* Horizontal Timing Control 2 */
>> +#define VP_HTIM2_HFP(v)        FLD_VAL(v, 24, 16)
>> +#define VP_HTIM2_HACT(v)    FLD_VAL(v, 10, 0)
>> +#define VP_VTIM1        0x045C /* Vertical Timing Control 1 */
>> +#define VP_VTIM1_VBP(v)        FLD_VAL(v, 23, 16)
>> +#define VP_VTIM1_VSYNC(v)    FLD_VAL(v, 7, 0)
>> +#define VP_VTIM2        0x0460 /* Vertical Timing Control 2 */
>> +#define VP_VTIM2_VFP(v)        FLD_VAL(v, 23, 16)
>> +#define VP_VTIM2_VACT(v)    FLD_VAL(v, 10, 0)
>> +#define VP_VFUEN        0x0464 /* Video Frame Timing Update Enable */
>> +
>> +/* LVDS registers */
>> +#define LV_MX0003        0x0480 /* Mux input bit 0 to 3 */
>> +#define LV_MX0407        0x0484 /* Mux input bit 4 to 7 */
>> +#define LV_MX0811        0x0488 /* Mux input bit 8 to 11 */
>> +#define LV_MX1215        0x048C /* Mux input bit 12 to 15 */
>> +#define LV_MX1619        0x0490 /* Mux input bit 16 to 19 */
>> +#define LV_MX2023        0x0494 /* Mux input bit 20 to 23 */
>> +#define LV_MX2427        0x0498 /* Mux input bit 24 to 27 */
>> +#define LV_MX(b0, b1, b2, b3)    (FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
>> +                FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
>> +
>> +/* Input bit numbers used in mux registers */
>> +enum {
>> +    LVI_R0,
>> +    LVI_R1,
>> +    LVI_R2,
>> +    LVI_R3,
>> +    LVI_R4,
>> +    LVI_R5,
>> +    LVI_R6,
>> +    LVI_R7,
>> +    LVI_G0,
>> +    LVI_G1,
>> +    LVI_G2,
>> +    LVI_G3,
>> +    LVI_G4,
>> +    LVI_G5,
>> +    LVI_G6,
>> +    LVI_G7,
>> +    LVI_B0,
>> +    LVI_B1,
>> +    LVI_B2,
>> +    LVI_B3,
>> +    LVI_B4,
>> +    LVI_B5,
>> +    LVI_B6,
>> +    LVI_B7,
>> +    LVI_HS,
>> +    LVI_VS,
>> +    LVI_DE,
>> +    LVI_L0
>> +};
>> +
>> +#define LV_CFG            0x049C /* LVDS Configuration */
>> +#define LV_PHY0            0x04A0 /* LVDS PHY 0 */
>> +#define LV_PHY0_RST(v)        FLD_VAL(v, 22, 22) /* PHY reset */
>> +#define LV_PHY0_IS(v)        FLD_VAL(v, 15, 14)
>> +#define LV_PHY0_ND(v)        FLD_VAL(v, 4, 0) /* Frequency range select */
>> +#define LV_PHY0_PRBS_ON(v)    FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
>> +
>> +/* System registers */
>> +#define SYS_RST            0x0504 /* System Reset */
>> +#define SYS_ID            0x0580 /* System ID */
>> +
>> +#define SYS_RST_I2CS        BIT(0) /* Reset I2C-Slave controller */
>> +#define SYS_RST_I2CM        BIT(1) /* Reset I2C-Master controller */
>> +#define SYS_RST_LCD        BIT(2) /* Reset LCD controller */
>> +#define SYS_RST_BM        BIT(3) /* Reset Bus Management controller */
>> +#define SYS_RST_DSIRX        BIT(4) /* Reset DSI-RX and App controller */
>> +#define SYS_RST_REG        BIT(5) /* Reset Register module */
>> +
>> +#define LPX_PERIOD        2
>> +#define TTA_SURE        3
>> +#define TTA_GET            0x20000
>> +
>> +/* Lane enable PPI and DSI register bits */
>> +#define LANEENABLE_CLEN        BIT(0)
>> +#define LANEENABLE_L0EN        BIT(1)
>> +#define LANEENABLE_L1EN        BIT(2)
>> +#define LANEENABLE_L2EN        BIT(3)
>> +#define LANEENABLE_L3EN        BIT(4)
>> +
>> +/* LVCFG fields */
>> +#define LV_CFG_LVEN        BIT(0)
>> +#define LV_CFG_LVDLINK        BIT(1)
>> +#define LV_CFG_CLKPOL1        BIT(2)
>> +#define LV_CFG_CLKPOL2        BIT(3)
>> +
>> +static const char * const tc358764_supplies[] = {
>> +    "vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
>> +};
>> +
>> +struct tc358764 {
>> +    struct device *dev;
>> +    struct drm_bridge bridge;
>> +    struct drm_connector connector;
>> +    struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
>> +    struct gpio_desc *gpio_reset;
>> +
>> +    struct drm_panel *panel;
>> +};
>> +
>> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
>> +{
>> +    struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> +    const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>> +    struct mipi_dsi_msg msg = {
>> +        .type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
>> +        .channel = dsi->channel,
>> +        .flags = MIPI_DSI_MSG_USE_LPM,
>> +        .tx_buf = &addr,
>> +        .tx_len = 2,
>> +        .rx_buf = val,
>> +        .rx_len = 4
>> +    };
>> +    ssize_t ret;
>> +
>> +    if (!ops || !ops->transfer)
>> +        return -EINVAL;
>> +
>> +    cpu_to_le16s(&addr);
>> +
>> +    ret = ops->transfer(dsi->host, &msg);
>> +    if (ret >= 0)
>> +        le32_to_cpus(val); > +
>> +    dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
>> +
>> +    return ret;
>> +}
>> +
>> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
>> +{
>> +    struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> +    const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>> +    u8 data[6];
>> +    int ret;
>> +    struct mipi_dsi_msg msg = {
>> +        .type = MIPI_DSI_GENERIC_LONG_WRITE,
>> +        .channel = dsi->channel,
>> +        .flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
>> +        .tx_buf = data,
>> +        .tx_len = 6
>> +    };
>> +
>> +    if (!ops || !ops->transfer)
>> +        return -EINVAL;
>> +
>> +    data[0] = addr;
>> +    data[1] = addr >> 8;
>> +    data[2] = val;
>> +    data[3] = val >> 8;
>> +    data[4] = val >> 16;
>> +    data[5] = val >> 24;
>> +
>> +    ret = ops->transfer(dsi->host, &msg);
>> +
>> +    return ret;
>> +}
>> +
>> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
>> +{
>> +    return container_of(bridge, struct tc358764, bridge);
>> +}
>> +
>> +static inline
>> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
>> +{
>> +    return container_of(connector, struct tc358764, connector);
>> +}
>> +
>> +static int tc358764_init(struct tc358764 *ctx)
>> +{
>> +    u32 v = 0;
>> +
>> +    tc358764_read(ctx, SYS_ID, &v);
>> +    dev_info(ctx->dev, "ID: %#x\n", v);
>> +
>> +    /* configure PPI counters */
>> +    tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
>> +    tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
>> +    tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
>> +    tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
>> +    tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
>> +    tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
>> +
>> +    /* enable four data lanes and clock lane */
>> +    tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
>> +               LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
>> +    tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
>> +               LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
>> +
>> +    /* start */
>> +    tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
>> +    tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
>> +
>> +    /* configure video path */
>> +    tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
>> +               VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);
> 
> Could we specify somewhere that

Could you clarify, what do you mean here?

>> +
>> +    /* reset PHY */
>> +    tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
>> +               LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
>> +    tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
>> +               LV_PHY0_ND(6));
>> +
>> +    /* reset bridge */
>> +    tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
>> +
>> +    /* set bit order */
>> +    tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
>> +    tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
>> +    tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
>> +    tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
>> +    tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
>> +    tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
>> +    tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
>> +    tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
>> +               LV_CFG_LVEN);
>> +
>> +    return 0;
>> +}
>> +
>> +static void tc358764_reset(struct tc358764 *ctx)
>> +{
>> +    msleep(20);
>> +    gpiod_set_value(ctx->gpio_reset, 0);
>> +    msleep(20);
>> +    gpiod_set_value(ctx->gpio_reset, 1);
>> +    msleep(40);
>> +}
>> +
>> +static void tc358764_poweroff(struct tc358764 *ctx)
>> +{
>> +    int ret;
>> +
>> +    tc358764_reset(ctx);
>> +
>> +    drm_panel_disable(ctx->panel);
>> +    msleep(40);
>> +
>> +    ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +    if (ret < 0)
>> +        dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
>> +}
>> +
>> +static int tc358764_get_modes(struct drm_connector *connector)
>> +{
>> +    struct tc358764 *ctx = connector_to_tc358764(connector);
>> +
>> +    if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
>> +        return ctx->panel->funcs->get_modes(ctx->panel);
>> +
>> +    return 0;
>> +}
>> +
>> +static const
>> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
>> +    .get_modes = tc358764_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs tc358764_connector_funcs = {
>> +    .fill_modes = drm_helper_probe_single_connector_modes,
>> +    .destroy = drm_connector_cleanup,
>> +    .reset = drm_atomic_helper_connector_reset,
>> +    .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +    .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static void tc358764_disable(struct drm_bridge *bridge)
>> +{
>> +    struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +
>> +    tc358764_poweroff(ctx);
>> +}
>> +
>> +static void tc358764_pre_enable(struct drm_bridge *bridge)
>> +{
>> +    struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +    int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
>> +                    ctx->supplies);
>> +    if (ret < 0)
>> +        dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
>> +
>> +    tc358764_reset(ctx);
>> +    tc358764_init(ctx);
>> +}
>> +
>> +static void tc358764_enable(struct drm_bridge *bridge)
>> +{
>> +    struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +    int ret;
>> +
>> +    drm_panel_prepare(ctx->panel);
>> +
>> +    ret = drm_panel_enable(ctx->panel);
>> +    if (ret < 0)
>> +        pr_err("panel enable failed\n");
>> +
>> +    msleep(40);
>> +}
>> +
>> +static int tc358764_attach(struct drm_bridge *bridge)
>> +{
>> +    struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +    struct drm_device *drm = bridge->dev;
>> +    int ret;
>> +
>> +    if (!bridge->encoder) {
>> +        DRM_ERROR("Encoder not found\n");
>> +        return -ENODEV;
>> +    }
>> +
>> +    ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
>> +    ret = drm_connector_init(drm, &ctx->connector,
>> +                 &tc358764_connector_funcs,
>> +                 DRM_MODE_CONNECTOR_LVDS);
>> +    if (ret) {
>> +        DRM_ERROR("Failed to initialize connector\n");
>> +        return ret;
>> +    }
>> +
>> +    drm_connector_helper_add(&ctx->connector,
>> +                 &tc358764_connector_helper_funcs);
>> +
>> +    drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
>> +
>> +    if (ctx->panel)
>> +        drm_panel_attach(ctx->panel, &ctx->connector);
>> +
>> +    drm_atomic_helper_connector_reset(&ctx->connector);
>> +    drm_connector_register(&ctx->connector);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
>> +    .disable = tc358764_disable,
>> +    .enable = tc358764_enable,
>> +    .pre_enable = tc358764_pre_enable,
>> +    .attach = tc358764_attach,
>> +};
>> +
>> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
>> +{
>> +    struct device_node *np, *ep;
>> +
>> +    ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
>> +    if (!ep) {
>> +        pr_err("faile to get endpoint\n");
>> +        return NULL;
>> +    }
>> +
>> +    np = of_graph_get_remote_port_parent(ep);
>> +
>> +    return np;
>> +}
>> +
>> +static int tc358764_parse_dt(struct tc358764 *ctx)
>> +{
>> +    struct device *dev = ctx->dev;
>> +    struct device_node *np = dev->of_node;
>> +    struct device_node *lvds;
>> +
>> +    ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
>> +                              GPIOD_OUT_LOW,
>> +                              "tc358764-reset");
>> +    if (IS_ERR(ctx->gpio_reset)) {
>> +        dev_err(dev, "no reset GPIO pin provided\n");
>> +        return PTR_ERR(ctx->gpio_reset);
>> +    }
>> +
>> +    lvds = tc358764_of_find_panel_node(ctx->dev);
>> +    if (!lvds) {
>> +        dev_err(dev, "cannot find panel node\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    ctx->panel = of_drm_find_panel(lvds);
>> +    if (!ctx->panel) {
>> +        dev_err(dev, "panel not registered\n");
>> +        return -EPROBE_DEFER;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int tc358764_configure_regulators(struct tc358764 *ctx)
>> +{
>> +    int i, ret;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
>> +        ctx->supplies[i].supply = tc358764_supplies[i];
>> +
>> +    ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
>> +                      ctx->supplies);
>> +    if (ret < 0)
>> +        dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
>> +
>> +    return ret;
>> +}
>> +
>> +static int tc358764_probe(struct mipi_dsi_device *dsi)
>> +{
>> +    struct device *dev = &dsi->dev;
>> +    struct tc358764 *ctx;
>> +    int ret;
>> +
>> +    ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
>> +    if (!ctx)
>> +        return -ENOMEM;
>> +
>> +    mipi_dsi_set_drvdata(dsi, ctx);
>> +
>> +    ctx->dev = dev;
>> +
>> +    dsi->lanes = 4;
>> +    dsi->format = MIPI_DSI_FMT_RGB888;
>> +    dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
>> +        | MIPI_DSI_MODE_VIDEO_AUTO_VERT;
> 
> if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
> add the LPM flag here.
> 

I'll check, if we can use the helper instead of calling ops->transfer() directly.

Thanks,

Maciej Purski

> Looks good to me otherwise. Once the DT port issue is concluded:
> 
> Reviewed-by: Archit Taneja <architt@codeaurora.org>
> 
>> +
>> +    ret = tc358764_parse_dt(ctx);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ret = tc358764_configure_regulators(ctx);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ctx->bridge.funcs = &tc358764_bridge_funcs;
>> +    ctx->bridge.of_node = dev->of_node;
>> +
>> +    drm_bridge_add(&ctx->bridge);
>> +
>> +    ret = mipi_dsi_attach(dsi);
>> +    if (ret < 0) {
>> +        drm_bridge_remove(&ctx->bridge);
>> +        dev_err(dev, "failed to attach dsi\n");
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int tc358764_remove(struct mipi_dsi_device *dsi)
>> +{
>> +    struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
>> +
>> +    tc358764_poweroff(ctx);
>> +
>> +    mipi_dsi_detach(dsi);
>> +    drm_bridge_remove(&ctx->bridge);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct of_device_id tc358764_of_match[] = {
>> +    { .compatible = "toshiba,tc358764" },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
>> +
>> +static struct mipi_dsi_driver tc358764_driver = {
>> +    .probe = tc358764_probe,
>> +    .remove = tc358764_remove,
>> +    .driver = {
>> +        .name = "tc358764",
>> +        .owner = THIS_MODULE,
>> +        .of_match_table = tc358764_of_match,
>> +    },
>> +};
>> +module_mipi_dsi_driver(tc358764_driver);
>> +
>> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
>> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
>> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
>> +MODULE_LICENSE("GPL v2");
>>
> 
> 
> 

[PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver

[Maciej Purski]

On 05/31/2018 08:51 AM, Archit Taneja wrote:
> Hi,
> 
> On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>> ? drivers/gpu/drm/bridge/Kconfig??? |?? 9 +
>> ? drivers/gpu/drm/bridge/Makefile?? |?? 1 +
>> ? drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>> ? 3 files changed, 557 insertions(+)
>> ? create mode 100644 drivers/gpu/drm/bridge/tc358764.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index fa2c799..9bd3eb8 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>> ????? ---help---
>> ??????? Thine THC63LVD1024 LVDS/parallel converter driver.
>> +config DRM_TOSHIBA_TC358764
>> +??? tristate "TC358764 DSI/LVDS bridge"
>> +??? depends on DRM && DRM_PANEL
>> +??? depends on OF
>> +??? select DRM_MIPI_DSI
>> +??? select VIDEOMODE_HELPERS
> 
> I don't see videomode usage in the driver, can we drop this if it isn't
> used?
> 

It seems that those are some remains of old versions. It is not required now.

>> +??? help
>> +????? Toshiba TC358764 DSI/LVDS bridge driver
>> +
>> ? config DRM_TOSHIBA_TC358767
>> ????? tristate "Toshiba TC358767 eDP bridge"
>> ????? depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index 35f88d4..bf7c0ce 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>> ? obj-$(CONFIG_DRM_SII902X) += sii902x.o
>> ? obj-$(CONFIG_DRM_SII9234) += sii9234.o
>> ? obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>> ? obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>> ? obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> ? obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
>> new file mode 100644
>> index 0000000..3109eba
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/tc358764.c
>> @@ -0,0 +1,547 @@
> 
> We'd need a SPDX license here?
>> +/*
>> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
>> + *
>> + * Authors:
>> + *??? Andrzej Hajda <a.hajda@samsung.com>
>> + *??? Maciej Purski <m.purski@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.
>> + *
>> + */
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_panel.h>
>> +
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#include <video/mipi_display.h>
>> +#include <video/of_videomode.h>
>> +#include <video/videomode.h>
>> +
>> +#define FLD_MASK(start, end)??? (((1 << ((start) - (end) + 1)) - 1) << (end))
>> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
>> +
>> +/* PPI layer registers */
>> +#define PPI_STARTPPI??????? 0x0104 /* START control bit */
>> +#define PPI_LPTXTIMECNT??????? 0x0114 /* LPTX timing signal */
>> +#define PPI_LANEENABLE??????? 0x0134 /* Enables each lane */
>> +#define PPI_TX_RX_TA??????? 0x013C /* BTA timing parameters */
>> +#define PPI_D0S_CLRSIPOCOUNT??? 0x0164 /* Assertion timer for Lane 0 */
>> +#define PPI_D1S_CLRSIPOCOUNT??? 0x0168 /* Assertion timer for Lane 1 */
>> +#define PPI_D2S_CLRSIPOCOUNT??? 0x016C /* Assertion timer for Lane 2 */
>> +#define PPI_D3S_CLRSIPOCOUNT??? 0x0170 /* Assertion timer for Lane 3 */
>> +#define PPI_START_FUNCTION??? 1
>> +
>> +/* DSI layer registers */
>> +#define DSI_STARTDSI??????? 0x0204 /* START control bit of DSI-TX */
>> +#define DSI_LANEENABLE??????? 0x0210 /* Enables each lane */
>> +#define DSI_RX_START??????? 1
>> +
>> +/* Video path registers */
>> +#define VP_CTRL??????????? 0x0450 /* Video Path Control */
>> +#define VP_CTRL_MSF(v)??????? FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
>> +#define VP_CTRL_VTGEN(v)??? FLD_VAL(v, 4, 4) /* Use chip clock for timing */
>> +#define VP_CTRL_EVTMODE(v)??? FLD_VAL(v, 5, 5) /* Event mode */
>> +#define VP_CTRL_RGB888(v)??? FLD_VAL(v, 8, 8) /* RGB888 mode */
>> +#define VP_CTRL_VSDELAY(v)??? FLD_VAL(v, 31, 20) /* VSYNC delay */
>> +#define VP_CTRL_HSPOL??????? BIT(17) /* Polarity of HSYNC signal */
>> +#define VP_CTRL_DEPOL??????? BIT(18) /* Polarity of DE signal */
>> +#define VP_CTRL_VSPOL??????? BIT(19) /* Polarity of VSYNC signal */
>> +#define VP_HTIM1??????? 0x0454 /* Horizontal Timing Control 1 */
>> +#define VP_HTIM1_HBP(v)??????? FLD_VAL(v, 24, 16)
>> +#define VP_HTIM1_HSYNC(v)??? FLD_VAL(v, 8, 0)
>> +#define VP_HTIM2??????? 0x0458 /* Horizontal Timing Control 2 */
>> +#define VP_HTIM2_HFP(v)??????? FLD_VAL(v, 24, 16)
>> +#define VP_HTIM2_HACT(v)??? FLD_VAL(v, 10, 0)
>> +#define VP_VTIM1??????? 0x045C /* Vertical Timing Control 1 */
>> +#define VP_VTIM1_VBP(v)??????? FLD_VAL(v, 23, 16)
>> +#define VP_VTIM1_VSYNC(v)??? FLD_VAL(v, 7, 0)
>> +#define VP_VTIM2??????? 0x0460 /* Vertical Timing Control 2 */
>> +#define VP_VTIM2_VFP(v)??????? FLD_VAL(v, 23, 16)
>> +#define VP_VTIM2_VACT(v)??? FLD_VAL(v, 10, 0)
>> +#define VP_VFUEN??????? 0x0464 /* Video Frame Timing Update Enable */
>> +
>> +/* LVDS registers */
>> +#define LV_MX0003??????? 0x0480 /* Mux input bit 0 to 3 */
>> +#define LV_MX0407??????? 0x0484 /* Mux input bit 4 to 7 */
>> +#define LV_MX0811??????? 0x0488 /* Mux input bit 8 to 11 */
>> +#define LV_MX1215??????? 0x048C /* Mux input bit 12 to 15 */
>> +#define LV_MX1619??????? 0x0490 /* Mux input bit 16 to 19 */
>> +#define LV_MX2023??????? 0x0494 /* Mux input bit 20 to 23 */
>> +#define LV_MX2427??????? 0x0498 /* Mux input bit 24 to 27 */
>> +#define LV_MX(b0, b1, b2, b3)??? (FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
>> +??????????????? FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
>> +
>> +/* Input bit numbers used in mux registers */
>> +enum {
>> +??? LVI_R0,
>> +??? LVI_R1,
>> +??? LVI_R2,
>> +??? LVI_R3,
>> +??? LVI_R4,
>> +??? LVI_R5,
>> +??? LVI_R6,
>> +??? LVI_R7,
>> +??? LVI_G0,
>> +??? LVI_G1,
>> +??? LVI_G2,
>> +??? LVI_G3,
>> +??? LVI_G4,
>> +??? LVI_G5,
>> +??? LVI_G6,
>> +??? LVI_G7,
>> +??? LVI_B0,
>> +??? LVI_B1,
>> +??? LVI_B2,
>> +??? LVI_B3,
>> +??? LVI_B4,
>> +??? LVI_B5,
>> +??? LVI_B6,
>> +??? LVI_B7,
>> +??? LVI_HS,
>> +??? LVI_VS,
>> +??? LVI_DE,
>> +??? LVI_L0
>> +};
>> +
>> +#define LV_CFG??????????? 0x049C /* LVDS Configuration */
>> +#define LV_PHY0??????????? 0x04A0 /* LVDS PHY 0 */
>> +#define LV_PHY0_RST(v)??????? FLD_VAL(v, 22, 22) /* PHY reset */
>> +#define LV_PHY0_IS(v)??????? FLD_VAL(v, 15, 14)
>> +#define LV_PHY0_ND(v)??????? FLD_VAL(v, 4, 0) /* Frequency range select */
>> +#define LV_PHY0_PRBS_ON(v)??? FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
>> +
>> +/* System registers */
>> +#define SYS_RST??????????? 0x0504 /* System Reset */
>> +#define SYS_ID??????????? 0x0580 /* System ID */
>> +
>> +#define SYS_RST_I2CS??????? BIT(0) /* Reset I2C-Slave controller */
>> +#define SYS_RST_I2CM??????? BIT(1) /* Reset I2C-Master controller */
>> +#define SYS_RST_LCD??????? BIT(2) /* Reset LCD controller */
>> +#define SYS_RST_BM??????? BIT(3) /* Reset Bus Management controller */
>> +#define SYS_RST_DSIRX??????? BIT(4) /* Reset DSI-RX and App controller */
>> +#define SYS_RST_REG??????? BIT(5) /* Reset Register module */
>> +
>> +#define LPX_PERIOD??????? 2
>> +#define TTA_SURE??????? 3
>> +#define TTA_GET??????????? 0x20000
>> +
>> +/* Lane enable PPI and DSI register bits */
>> +#define LANEENABLE_CLEN??????? BIT(0)
>> +#define LANEENABLE_L0EN??????? BIT(1)
>> +#define LANEENABLE_L1EN??????? BIT(2)
>> +#define LANEENABLE_L2EN??????? BIT(3)
>> +#define LANEENABLE_L3EN??????? BIT(4)
>> +
>> +/* LVCFG fields */
>> +#define LV_CFG_LVEN??????? BIT(0)
>> +#define LV_CFG_LVDLINK??????? BIT(1)
>> +#define LV_CFG_CLKPOL1??????? BIT(2)
>> +#define LV_CFG_CLKPOL2??????? BIT(3)
>> +
>> +static const char * const tc358764_supplies[] = {
>> +??? "vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
>> +};
>> +
>> +struct tc358764 {
>> +??? struct device *dev;
>> +??? struct drm_bridge bridge;
>> +??? struct drm_connector connector;
>> +??? struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
>> +??? struct gpio_desc *gpio_reset;
>> +
>> +??? struct drm_panel *panel;
>> +};
>> +
>> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
>> +{
>> +??? struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> +??? const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>> +??? struct mipi_dsi_msg msg = {
>> +??????? .type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
>> +??????? .channel = dsi->channel,
>> +??????? .flags = MIPI_DSI_MSG_USE_LPM,
>> +??????? .tx_buf = &addr,
>> +??????? .tx_len = 2,
>> +??????? .rx_buf = val,
>> +??????? .rx_len = 4
>> +??? };
>> +??? ssize_t ret;
>> +
>> +??? if (!ops || !ops->transfer)
>> +??????? return -EINVAL;
>> +
>> +??? cpu_to_le16s(&addr);
>> +
>> +??? ret = ops->transfer(dsi->host, &msg);
>> +??? if (ret >= 0)
>> +??????? le32_to_cpus(val); > +
>> +??? dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
>> +
>> +??? return ret;
>> +}
>> +
>> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
>> +{
>> +??? struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> +??? const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>> +??? u8 data[6];
>> +??? int ret;
>> +??? struct mipi_dsi_msg msg = {
>> +??????? .type = MIPI_DSI_GENERIC_LONG_WRITE,
>> +??????? .channel = dsi->channel,
>> +??????? .flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
>> +??????? .tx_buf = data,
>> +??????? .tx_len = 6
>> +??? };
>> +
>> +??? if (!ops || !ops->transfer)
>> +??????? return -EINVAL;
>> +
>> +??? data[0] = addr;
>> +??? data[1] = addr >> 8;
>> +??? data[2] = val;
>> +??? data[3] = val >> 8;
>> +??? data[4] = val >> 16;
>> +??? data[5] = val >> 24;
>> +
>> +??? ret = ops->transfer(dsi->host, &msg);
>> +
>> +??? return ret;
>> +}
>> +
>> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
>> +{
>> +??? return container_of(bridge, struct tc358764, bridge);
>> +}
>> +
>> +static inline
>> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
>> +{
>> +??? return container_of(connector, struct tc358764, connector);
>> +}
>> +
>> +static int tc358764_init(struct tc358764 *ctx)
>> +{
>> +??? u32 v = 0;
>> +
>> +??? tc358764_read(ctx, SYS_ID, &v);
>> +??? dev_info(ctx->dev, "ID: %#x\n", v);
>> +
>> +??? /* configure PPI counters */
>> +??? tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
>> +??? tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
>> +??? tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
>> +??? tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
>> +??? tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
>> +??? tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
>> +
>> +??? /* enable four data lanes and clock lane */
>> +??? tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
>> +?????????????? LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
>> +??? tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
>> +?????????????? LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
>> +
>> +??? /* start */
>> +??? tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
>> +??? tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
>> +
>> +??? /* configure video path */
>> +??? tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
>> +?????????????? VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);
> 
> Could we specify somewhere that

Could you clarify, what do you mean here?

>> +
>> +??? /* reset PHY */
>> +??? tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
>> +?????????????? LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
>> +??? tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
>> +?????????????? LV_PHY0_ND(6));
>> +
>> +??? /* reset bridge */
>> +??? tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
>> +
>> +??? /* set bit order */
>> +??? tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
>> +??? tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
>> +??? tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
>> +??? tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
>> +??? tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
>> +??? tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
>> +??? tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
>> +??? tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
>> +?????????????? LV_CFG_LVEN);
>> +
>> +??? return 0;
>> +}
>> +
>> +static void tc358764_reset(struct tc358764 *ctx)
>> +{
>> +??? msleep(20);
>> +??? gpiod_set_value(ctx->gpio_reset, 0);
>> +??? msleep(20);
>> +??? gpiod_set_value(ctx->gpio_reset, 1);
>> +??? msleep(40);
>> +}
>> +
>> +static void tc358764_poweroff(struct tc358764 *ctx)
>> +{
>> +??? int ret;
>> +
>> +??? tc358764_reset(ctx);
>> +
>> +??? drm_panel_disable(ctx->panel);
>> +??? msleep(40);
>> +
>> +??? ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +??? if (ret < 0)
>> +??????? dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
>> +}
>> +
>> +static int tc358764_get_modes(struct drm_connector *connector)
>> +{
>> +??? struct tc358764 *ctx = connector_to_tc358764(connector);
>> +
>> +??? if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
>> +??????? return ctx->panel->funcs->get_modes(ctx->panel);
>> +
>> +??? return 0;
>> +}
>> +
>> +static const
>> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
>> +??? .get_modes = tc358764_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs tc358764_connector_funcs = {
>> +??? .fill_modes = drm_helper_probe_single_connector_modes,
>> +??? .destroy = drm_connector_cleanup,
>> +??? .reset = drm_atomic_helper_connector_reset,
>> +??? .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +??? .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static void tc358764_disable(struct drm_bridge *bridge)
>> +{
>> +??? struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +
>> +??? tc358764_poweroff(ctx);
>> +}
>> +
>> +static void tc358764_pre_enable(struct drm_bridge *bridge)
>> +{
>> +??? struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +??? int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
>> +??????????????????? ctx->supplies);
>> +??? if (ret < 0)
>> +??????? dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
>> +
>> +??? tc358764_reset(ctx);
>> +??? tc358764_init(ctx);
>> +}
>> +
>> +static void tc358764_enable(struct drm_bridge *bridge)
>> +{
>> +??? struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +??? int ret;
>> +
>> +??? drm_panel_prepare(ctx->panel);
>> +
>> +??? ret = drm_panel_enable(ctx->panel);
>> +??? if (ret < 0)
>> +??????? pr_err("panel enable failed\n");
>> +
>> +??? msleep(40);
>> +}
>> +
>> +static int tc358764_attach(struct drm_bridge *bridge)
>> +{
>> +??? struct tc358764 *ctx = bridge_to_tc358764(bridge);
>> +??? struct drm_device *drm = bridge->dev;
>> +??? int ret;
>> +
>> +??? if (!bridge->encoder) {
>> +??????? DRM_ERROR("Encoder not found\n");
>> +??????? return -ENODEV;
>> +??? }
>> +
>> +??? ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
>> +??? ret = drm_connector_init(drm, &ctx->connector,
>> +???????????????? &tc358764_connector_funcs,
>> +???????????????? DRM_MODE_CONNECTOR_LVDS);
>> +??? if (ret) {
>> +??????? DRM_ERROR("Failed to initialize connector\n");
>> +??????? return ret;
>> +??? }
>> +
>> +??? drm_connector_helper_add(&ctx->connector,
>> +???????????????? &tc358764_connector_helper_funcs);
>> +
>> +??? drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
>> +
>> +??? if (ctx->panel)
>> +??????? drm_panel_attach(ctx->panel, &ctx->connector);
>> +
>> +??? drm_atomic_helper_connector_reset(&ctx->connector);
>> +??? drm_connector_register(&ctx->connector);
>> +
>> +??? return 0;
>> +}
>> +
>> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
>> +??? .disable = tc358764_disable,
>> +??? .enable = tc358764_enable,
>> +??? .pre_enable = tc358764_pre_enable,
>> +??? .attach = tc358764_attach,
>> +};
>> +
>> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
>> +{
>> +??? struct device_node *np, *ep;
>> +
>> +??? ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
>> +??? if (!ep) {
>> +??????? pr_err("faile to get endpoint\n");
>> +??????? return NULL;
>> +??? }
>> +
>> +??? np = of_graph_get_remote_port_parent(ep);
>> +
>> +??? return np;
>> +}
>> +
>> +static int tc358764_parse_dt(struct tc358764 *ctx)
>> +{
>> +??? struct device *dev = ctx->dev;
>> +??? struct device_node *np = dev->of_node;
>> +??? struct device_node *lvds;
>> +
>> +??? ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
>> +????????????????????????????? GPIOD_OUT_LOW,
>> +????????????????????????????? "tc358764-reset");
>> +??? if (IS_ERR(ctx->gpio_reset)) {
>> +??????? dev_err(dev, "no reset GPIO pin provided\n");
>> +??????? return PTR_ERR(ctx->gpio_reset);
>> +??? }
>> +
>> +??? lvds = tc358764_of_find_panel_node(ctx->dev);
>> +??? if (!lvds) {
>> +??????? dev_err(dev, "cannot find panel node\n");
>> +??????? return -EINVAL;
>> +??? }
>> +
>> +??? ctx->panel = of_drm_find_panel(lvds);
>> +??? if (!ctx->panel) {
>> +??????? dev_err(dev, "panel not registered\n");
>> +??????? return -EPROBE_DEFER;
>> +??? }
>> +
>> +??? return 0;
>> +}
>> +
>> +static int tc358764_configure_regulators(struct tc358764 *ctx)
>> +{
>> +??? int i, ret;
>> +
>> +??? for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
>> +??????? ctx->supplies[i].supply = tc358764_supplies[i];
>> +
>> +??? ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
>> +????????????????????? ctx->supplies);
>> +??? if (ret < 0)
>> +??????? dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
>> +
>> +??? return ret;
>> +}
>> +
>> +static int tc358764_probe(struct mipi_dsi_device *dsi)
>> +{
>> +??? struct device *dev = &dsi->dev;
>> +??? struct tc358764 *ctx;
>> +??? int ret;
>> +
>> +??? ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
>> +??? if (!ctx)
>> +??????? return -ENOMEM;
>> +
>> +??? mipi_dsi_set_drvdata(dsi, ctx);
>> +
>> +??? ctx->dev = dev;
>> +
>> +??? dsi->lanes = 4;
>> +??? dsi->format = MIPI_DSI_FMT_RGB888;
>> +??? dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
>> +??????? | MIPI_DSI_MODE_VIDEO_AUTO_VERT;
> 
> if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
> add the LPM flag here.
> 

I'll check, if we can use the helper instead of calling ops->transfer() directly.

Thanks,

Maciej Purski

> Looks good to me otherwise. Once the DT port issue is concluded:
> 
> Reviewed-by: Archit Taneja <architt@codeaurora.org>
> 
>> +
>> +??? ret = tc358764_parse_dt(ctx);
>> +??? if (ret < 0)
>> +??????? return ret;
>> +
>> +??? ret = tc358764_configure_regulators(ctx);
>> +??? if (ret < 0)
>> +??????? return ret;
>> +
>> +??? ctx->bridge.funcs = &tc358764_bridge_funcs;
>> +??? ctx->bridge.of_node = dev->of_node;
>> +
>> +??? drm_bridge_add(&ctx->bridge);
>> +
>> +??? ret = mipi_dsi_attach(dsi);
>> +??? if (ret < 0) {
>> +??????? drm_bridge_remove(&ctx->bridge);
>> +??????? dev_err(dev, "failed to attach dsi\n");
>> +??? }
>> +
>> +??? return ret;
>> +}
>> +
>> +static int tc358764_remove(struct mipi_dsi_device *dsi)
>> +{
>> +??? struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
>> +
>> +??? tc358764_poweroff(ctx);
>> +
>> +??? mipi_dsi_detach(dsi);
>> +??? drm_bridge_remove(&ctx->bridge);
>> +
>> +??? return 0;
>> +}
>> +
>> +static const struct of_device_id tc358764_of_match[] = {
>> +??? { .compatible = "toshiba,tc358764" },
>> +??? { }
>> +};
>> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
>> +
>> +static struct mipi_dsi_driver tc358764_driver = {
>> +??? .probe = tc358764_probe,
>> +??? .remove = tc358764_remove,
>> +??? .driver = {
>> +??????? .name = "tc358764",
>> +??????? .owner = THIS_MODULE,
>> +??????? .of_match_table = tc358764_of_match,
>> +??? },
>> +};
>> +module_mipi_dsi_driver(tc358764_driver);
>> +
>> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
>> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
>> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
>> +MODULE_LICENSE("GPL v2");
>>
> 
> 
> 

[PATCH v2 09/10] ARM: dts: exynos5250: add DSI node

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds common part of DSI node for Exynos5250 platforms
and a required mipi-phy node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 2daf505..9965eca 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -733,6 +733,27 @@
 			#phy-cells = <0>;
 		};
 
+		mipi_phy: video-phy@10040710 {
+			compatible = "samsung,s5pv210-mipi-video-phy";
+			reg = <0x10040710 0x100>;
+			#phy-cells = <1>;
+			syscon = <&pmu_system_controller>;
+		};
+
+		dsi_0: dsi@14500000 {
+			compatible = "samsung,exynos4210-mipi-dsi";
+			reg = <0x14500000 0x10000>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			samsung,power-domain = <&pd_disp1>;
+			phys = <&mipi_phy 3>;
+			phy-names = "dsim";
+			clocks = <&clock CLK_DSIM0>, <&clock CLK_SCLK_MIPI1>;
+			clock-names = "bus_clk", "sclk_mipi";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		adc: adc@12d10000 {
 			compatible = "samsung,exynos-adc-v1";
 			reg = <0x12D10000 0x100>;
-- 
2.7.4

[PATCH v2 09/10] ARM: dts: exynos5250: add DSI node

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds common part of DSI node for Exynos5250 platforms
and a required mipi-phy node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 2daf505..9965eca 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -733,6 +733,27 @@
 			#phy-cells = <0>;
 		};
 
+		mipi_phy: video-phy at 10040710 {
+			compatible = "samsung,s5pv210-mipi-video-phy";
+			reg = <0x10040710 0x100>;
+			#phy-cells = <1>;
+			syscon = <&pmu_system_controller>;
+		};
+
+		dsi_0: dsi at 14500000 {
+			compatible = "samsung,exynos4210-mipi-dsi";
+			reg = <0x14500000 0x10000>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			samsung,power-domain = <&pd_disp1>;
+			phys = <&mipi_phy 3>;
+			phy-names = "dsim";
+			clocks = <&clock CLK_DSIM0>, <&clock CLK_SCLK_MIPI1>;
+			clock-names = "bus_clk", "sclk_mipi";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		adc: adc at 12d10000 {
 			compatible = "samsung,exynos-adc-v1";
 			reg = <0x12D10000 0x100>;
-- 
2.7.4

[PATCH v2 09/10] ARM: dts: exynos5250: add DSI node

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds common part of DSI node for Exynos5250 platforms
and a required mipi-phy node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 2daf505..9965eca 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -733,6 +733,27 @@
 			#phy-cells = <0>;
 		};
 
+		mipi_phy: video-phy@10040710 {
+			compatible = "samsung,s5pv210-mipi-video-phy";
+			reg = <0x10040710 0x100>;
+			#phy-cells = <1>;
+			syscon = <&pmu_system_controller>;
+		};
+
+		dsi_0: dsi@14500000 {
+			compatible = "samsung,exynos4210-mipi-dsi";
+			reg = <0x14500000 0x10000>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			samsung,power-domain = <&pd_disp1>;
+			phys = <&mipi_phy 3>;
+			phy-names = "dsim";
+			clocks = <&clock CLK_DSIM0>, <&clock CLK_SCLK_MIPI1>;
+			clock-names = "bus_clk", "sclk_mipi";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		adc: adc@12d10000 {
 			compatible = "samsung,exynos-adc-v1";
 			reg = <0x12D10000 0x100>;
-- 
2.7.4

Re: [PATCH v2 09/10] ARM: dts: exynos5250: add DSI node

[Krzysztof Kozlowski]

On Wed, May 30, 2018 at 2:16 PM, Maciej Purski <m.purski@samsung.com> wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
>
> The patch adds common part of DSI node for Exynos5250 platforms
> and a required mipi-phy node.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

Thanks for changes. I'll take both DTS patches after bindings get
accepted and after upcomming merge window.

Best regards,
Krzysztof

[PATCH v2 09/10] ARM: dts: exynos5250: add DSI node

[Krzysztof Kozlowski]

On Wed, May 30, 2018 at 2:16 PM, Maciej Purski <m.purski@samsung.com> wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
>
> The patch adds common part of DSI node for Exynos5250 platforms
> and a required mipi-phy node.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

Thanks for changes. I'll take both DTS patches after bindings get
accepted and after upcomming merge window.

Best regards,
Krzysztof

[PATCH v2 10/10] ARM: dts: exynos5250-arndale: add DSI and panel nodes

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bridge and panel nodes.
It adds also DSI properties specific for arndale board and
regulators required by the bridge.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 arch/arm/boot/dts/exynos5250-arndale.dts | 61 ++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 7a8a5c5..816d89d 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -71,6 +71,17 @@
 		};
 	};
 
+	panel: panel {
+		compatible = "boe,hv070wsa-100";
+		power-supply = <&vcc_3v3_reg>;
+		enable-gpios = <&gpd1 3 GPIO_ACTIVE_HIGH>;
+		port {
+			panel_ep: endpoint {
+				remote-endpoint = <&bridge_out_ep>;
+			};
+		};
+	};
+
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -97,6 +108,30 @@
 			reg = <2>;
 			regulator-name = "hdmi-en";
 		};
+
+		vcc_1v2_reg: regulator@3 {
+			compatible = "regulator-fixed";
+			reg = <3>;
+			regulator-name = "VCC_1V2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+
+		vcc_1v8_reg: regulator@4 {
+			compatible = "regulator-fixed";
+			reg = <4>;
+			regulator-name = "VCC_1V8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vcc_3v3_reg: regulator@5 {
+			compatible = "regulator-fixed";
+			reg = <5>;
+			regulator-name = "VCC_3V3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
 	};
 
 	fixed-rate-clocks {
@@ -119,6 +154,32 @@
 	cpu0-supply = <&buck2_reg>;
 };
 
+&dsi_0 {
+	vddcore-supply = <&ldo8_reg>;
+	vddio-supply = <&ldo10_reg>;
+	samsung,pll-clock-frequency = <24000000>;
+	samsung,burst-clock-frequency = <320000000>;
+	samsung,esc-clock-frequency = <10000000>;
+	status = "okay";
+
+	bridge@0 {
+		reg = <0>;
+		compatible = "toshiba,tc358764";
+		vddc-supply = <&vcc_1v2_reg>;
+		vddio-supply = <&vcc_1v8_reg>;
+		vddlvds-supply = <&vcc_3v3_reg>;
+		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@1 {
+			reg = <1>;
+			bridge_out_ep: endpoint {
+				remote-endpoint = <&panel_ep>;
+			};
+		};
+	};
+};
+
 &dp {
 	status = "okay";
 	samsung,color-space = <0>;
-- 
2.7.4

[PATCH v2 10/10] ARM: dts: exynos5250-arndale: add DSI and panel nodes

[Maciej Purski]

From: Andrzej Hajda <a.hajda@samsung.com>

The patch adds bridge and panel nodes.
It adds also DSI properties specific for arndale board and
regulators required by the bridge.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 arch/arm/boot/dts/exynos5250-arndale.dts | 61 ++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 7a8a5c5..816d89d 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -71,6 +71,17 @@
 		};
 	};
 
+	panel: panel {
+		compatible = "boe,hv070wsa-100";
+		power-supply = <&vcc_3v3_reg>;
+		enable-gpios = <&gpd1 3 GPIO_ACTIVE_HIGH>;
+		port {
+			panel_ep: endpoint {
+				remote-endpoint = <&bridge_out_ep>;
+			};
+		};
+	};
+
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -97,6 +108,30 @@
 			reg = <2>;
 			regulator-name = "hdmi-en";
 		};
+
+		vcc_1v2_reg: regulator at 3 {
+			compatible = "regulator-fixed";
+			reg = <3>;
+			regulator-name = "VCC_1V2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+
+		vcc_1v8_reg: regulator at 4 {
+			compatible = "regulator-fixed";
+			reg = <4>;
+			regulator-name = "VCC_1V8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vcc_3v3_reg: regulator at 5 {
+			compatible = "regulator-fixed";
+			reg = <5>;
+			regulator-name = "VCC_3V3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
 	};
 
 	fixed-rate-clocks {
@@ -119,6 +154,32 @@
 	cpu0-supply = <&buck2_reg>;
 };
 
+&dsi_0 {
+	vddcore-supply = <&ldo8_reg>;
+	vddio-supply = <&ldo10_reg>;
+	samsung,pll-clock-frequency = <24000000>;
+	samsung,burst-clock-frequency = <320000000>;
+	samsung,esc-clock-frequency = <10000000>;
+	status = "okay";
+
+	bridge at 0 {
+		reg = <0>;
+		compatible = "toshiba,tc358764";
+		vddc-supply = <&vcc_1v2_reg>;
+		vddio-supply = <&vcc_1v8_reg>;
+		vddlvds-supply = <&vcc_3v3_reg>;
+		reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port at 1 {
+			reg = <1>;
+			bridge_out_ep: endpoint {
+				remote-endpoint = <&panel_ep>;
+			};
+		};
+	};
+};
+
 &dp {
 	status = "okay";
 	samsung,color-space = <0>;
-- 
2.7.4