Re: [PATCH v3 5/6] dt-bindings: add the rockchip, dual-channel for dw-mipi-dsi

From: Nickey Yang
Date: Thu Nov 30 2017 - 12:33:13 EST


Hi Archit,


On 2017å10æ26æ 12:53, Archit Taneja wrote:


On 10/25/2017 09:21 AM, Nickey Yang wrote:
Configure dsi slave channel when driving a panel
which needs 2 DSI links.

Signed-off-by: Nickey Yang <nickey.yang@xxxxxxxxxxxxxx>
---
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
index 6bb59ab..a2bea22 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
@@ -19,6 +19,8 @@ Optional properties:
 - power-domains: a phandle to mipi dsi power domain node.
 - resets: list of phandle + reset specifier pairs, as described in [3].
 - reset-names: string reset name, must be "apb".
+- rockchip,dual-channel: phandle to a 2nd DSI channel, useful as a slave
+channel when driving a panel which needs 2 DSI links.
The example below is how dual DSI bindings could look like. Let me know what
you think of it.

If both DSI outputs drive the same device (i.e, point to the same panel DT
node), then I think it's reasonable enough to assume that the DSIs are
operating in a 'dual-channel' mode. That being said, we still need DT to
describe which of the DSIs generates the clock for both the channels. This
is done with the 'clock-master' DT binding.

Thanks,
Archit

mipi_dsi: mipi@ff960000 {
ÂÂÂÂ...
ÂÂÂÂ...

ÂÂÂÂclock-master;ÂÂÂ /* implies that this DSI instance drivers the clock
ÂÂÂÂÂÂÂÂÂÂÂÂ * for both the DSIs.
ÂÂÂÂÂÂÂÂÂÂÂÂ */

ÂÂÂÂports {
ÂÂÂÂÂÂÂ mipi_in: port {
ÂÂÂÂÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂ };

ÂÂÂÂÂÂÂ /* add extra output ports for both DSIs */
ÂÂÂÂÂÂÂ mipi_out: port {
ÂÂÂÂÂÂÂÂÂÂÂ mipi_panel_out: endpoint {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ remote-endpoint = <&panel_in_channel0>;
ÂÂÂÂÂÂÂÂÂÂÂ };
ÂÂÂÂÂÂÂ };
ÂÂÂÂ};

ÂÂÂÂpanel {
ÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂ /*
ÂÂÂÂÂÂÂÂ * panel node can describe its input ports, if both the DSIs output
ÂÂÂÂÂÂÂÂ * ports are connected to the same device (i.e, the same DSI panel),
ÂÂÂÂÂÂÂÂ * we can assume that the DSIs need to operate in dual DSI mode
ÂÂÂÂÂÂÂÂ */
ÂÂÂÂÂÂÂ ports {
ÂÂÂÂÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂÂÂÂÂ port@0 {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ panel_in_channel0: endpoint {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ remote-endpoint = <&mipi_panel_out>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ };
ÂÂÂÂÂÂÂÂÂÂÂ };

ÂÂÂÂÂÂÂÂÂÂÂ port@1 {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ panel_in_channel1: endpoint {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ remote-endpoint = <&mipi1_panel_out>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ };

ÂÂÂÂÂÂÂÂÂÂÂ };
ÂÂÂÂÂÂÂ };
ÂÂÂÂ};
};


mipi_dsi1: mipi@ff968000 {
ÂÂÂÂ...
ÂÂÂÂ...

ÂÂÂÂports {
ÂÂÂÂÂÂÂ mipi1_in: port {
ÂÂÂÂÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂÂÂÂÂ ...
ÂÂÂÂÂÂÂ };

ÂÂÂÂÂÂÂ mipi1_out: port {
ÂÂÂÂÂÂÂÂÂÂÂ mipi1_panel_out: endpoint {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ remote-endpoint = <&panel_in_channel1>;
ÂÂÂÂÂÂÂÂÂÂÂ };
ÂÂÂÂÂÂÂ };
ÂÂÂÂ};
};

I try to follow as you suggested,use

mipi_dsi: mipi@ff960000 {
ÂÂÂ ...
ÂÂÂ ...
ÂÂÂ clock-master;ÂÂÂ /* implies that this DSI instance drivers the clock
ÂÂÂ ÂÂÂ ÂÂÂ Â* for both the DSIs.
ÂÂÂ ÂÂÂ ÂÂÂ Â*/
ÂÂÂ ports {
ÂÂÂ ÂÂÂ mipi_in: port {
ÂÂÂ ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ /* add extra output ports for both DSIs */
ÂÂÂ ÂÂÂ mipi_out: port {
ÂÂÂ ÂÂÂ ÂÂÂ mipi_panel_out: endpoint {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ remote-endpoint = <&panel_in_channel0>;
ÂÂÂ ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ };
ÂÂÂ };
ÂÂÂ panel {
ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ /*
ÂÂÂ ÂÂÂ Â* panel node can describe its input ports, if both the DSIs output
ÂÂÂ ÂÂÂ Â* ports are connected to the same device (i.e, the same DSI panel),
ÂÂÂ ÂÂÂ Â* we can assume that the DSIs need to operate in dual DSI mode
ÂÂÂ ÂÂÂ Â*/
ÂÂÂ ÂÂÂ ports {
ÂÂÂ ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ ÂÂÂ port@0 {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ panel_in_channel0: endpoint {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ remote-endpoint = <&mipi_panel_out>;
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ ÂÂÂ port@1 {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ panel_in_channel1: endpoint {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ remote-endpoint = <&mipi1_panel_out>;
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ };

ÂÂÂ ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ };
ÂÂÂ };
};

mipi_dsi1: mipi@ff968000 {
ÂÂÂ ...
ÂÂÂ ...
ÂÂÂ ports {
ÂÂÂ ÂÂÂ mipi1_in: port {
ÂÂÂ ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ ÂÂÂ ...
ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ mipi1_out: port {
ÂÂÂ ÂÂÂ ÂÂÂ mipi1_panel_out: endpoint {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ remote-endpoint = <&panel_in_channel1>;
ÂÂÂ ÂÂÂ ÂÂÂ };
ÂÂÂ ÂÂÂ };
ÂÂÂ };
}

But it seems we can not use of_drm_find_panel(like below)

/*
ÂÂÂÂÂÂÂ port = of_graph_get_port_by_id(dev->of_node, 1);
ÂÂÂÂÂÂÂ if (port) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ endpoint = of_get_child_by_name(port, "endpoint");
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ of_node_put(port);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!endpoint) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev_err(dev, "no output endpoint found\n");
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ panel_node = of_graph_get_remote_port_parent(endpoint);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ of_node_put(endpoint);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!panel_node) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev_err(dev, "no output node found\n");
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ panel = of_drm_find_panel(panel_node);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ of_node_put(panel_node);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!panel)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EPROBE_DEFER;
ÂÂÂÂÂÂÂ }
*/
to get DSI1 outputs,because of_drm_find_panel need compare

if (panel->dev->of_node == np)

in dsi_panel driver innolux->base.dev = &innolux->link->dev;
dsi->dev

struct innolux_panel {
ÂÂÂÂÂÂÂ struct drm_panel base;
ÂÂÂÂÂÂÂ struct mipi_dsi_device *link;
};
It means one panel can only be found in his dsi node,(like dsi0 above).

I'm doubting about it, Or may we follow tegra_dsi_ganged_probe
(drivers/gpu/drm/tergra/dsi.c) method.


Thanks,
Nickey

  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
 [2] Documentation/devicetree/bindings/media/video-interfaces.txt