Hi Andrzej,
On Wednesday, 30 May 2018 12:59:12 EEST Andrzej Hajda wrote:
> On 28.05.2018 12:18, Laurent Pinchart wrote:
> > On Monday, 28 May 2018 12:47:11 EEST Maciej Purski wrote:
> >> 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.ha...@samsung.com>
> >> Signed-off-by: Maciej Purski <m.pur...@samsung.com>
> >> ---
> >>
> >> .../bindings/display/bridge/toshiba,tc358764.txt | 42 ++++++++++++++++
> >> 1 file changed, 42 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..d09bdc2
> >> --- /dev/null
> >> +++
> >> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt
> >> @@ -0,0 +1,42 @@
> >> +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
> >> + - vddio-supply: I/O voltage supply
> >> + - vddmipi-supply: MIPI voltage supply
> >> + - vddlvds133-supply: LVDS1 3.3V voltage supply
> >> + - vddlvds112-supply: LVDS1 1.2V voltage supply
> >
> > That's a lot of power supplies. Could some of them be merged together ?
> > See https://patchwork.freedesktop.org/patch/216058/ for an earlier
> > discussion on the same subject.
>
> Specs says about 3 supply voltage values:
> - 1.2V - digital core, DSI-RX PHY
> - 1.8-3.3V - digital I/O
> - 3.3V - LVDS-TX PHY
>
> So I guess it should be minimal number of supplies. Natural candidates:
>
> - 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
>
> I have changed name of the latest supply to be more consistent with
> other supplies, and changed 1.8-3.3 (which incorrectly suggest voltage
> range), to more precise voltage alternative.
This looks fine to me.
> >> + - 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;
> >
> > This seems pretty vague to me. It could be read as meaning that ports are
> > completely optional, and that the port number you list can be used, but
> > that something else could be used to.
> >
> > Let's make the port nodes mandatory. I propose the following.
> >
> > Required nodes:
> >
> > The TC358764 has DSI and LVDS ports whose connections are described using
> > the OF graph bindings defined in
> > Documentation/devicetree/bindings/graph.txt. The device node must contain
> > one 'port' child node per DSI and LVDS port. The port nodes are numbered
> > as follows.
> >
> > Port Number
> > -------------------------------------------------------------------
> > DSI Input 0
> > LVDS Output 1
> >
> > Each port node must contain endpoint nodes describing the hardware
> > connections.
>
> Since the bridge is controlled via DSI bus, DSI input port is not necessary.
I don't agree with this. Regardless of how the bridge is controlled, I think
we should always use ports to describe the data connections. Otherwise it
would get more complicated for display controller drivers to use different
types of bridges.
> >> +[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>;
> >> + vddmipi-supply = <&vcc_1v2_reg>;
> >> + vddlvds133-supply = <&vcc_3v3_reg>;
> >> + vddlvds112-supply = <&vcc_1v2_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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
