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

[Nickey Yang]

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