Re: [EXT] Re: [PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

2019-06-14 Thread Robert Chiras
Hi Fabio,

On Vi, 2019-06-14 at 09:59 -0300, Fabio Estevam wrote:
> On Fri, Jun 14, 2019 at 8:53 AM Robert Chiras 
> wrote:
> > 
> > 
> > Add dt-bindings documentation for Raydium RM67191 DSI panel.
> > 
> > Signed-off-by: Robert Chiras 
> > ---
> >  .../bindings/display/panel/raydium,rm67191.txt | 42
> > ++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > new file mode 100644
> > index 000..5a6268d
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t
> > xt
> > @@ -0,0 +1,42 @@
> > +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
> > +
> > +Required properties:
> > +- compatible:  "raydium,rm67191"
> > +- reg: virtual channel for MIPI-DSI protocol
> > +   must be <0>
> > +- dsi-lanes:   number of DSI lanes to be used
> > +   must be <3> or <4>
> > +- port:input port node with endpoint definition as
> > +   defined in
> > Documentation/devicetree/bindings/graph.txt;
> > +   the input port should be connected to a
> > MIPI-DSI device
> > +   driver
> > +
> > +Optional properties:
> > +- reset-gpio:  a GPIO spec for the RST_B GPIO pin
> reset-gpios (with the s in the end) is the recommendation.
> 
> > 
> > +- display-timings: timings for the connected panel according
> > to [1]
> This is not needed.
Well, I know that the panel timings are already hard-coded into the
driver, but on 850D, we have two display controllers: eLCDDIF and DCSS.
While eLCDIF works just fine with the display-timings received (and
undocumented) from panel vendor, with DCSS we had some issues and we
had to tweak the display-timings. This is why I added this property,
for a special case where we have to use different timings without
changing the driver (just a different dtb file). Do you think this is a
bad practice? If yes, then what mechanism of doing that do you
recommend?
> 
> > 
> > +- video-mode:  0 - burst-mode
> > +   1 - non-burst with sync event
> > +   2 - non-burst with sync pulse
> > +
> > +[1]: Documentation/devicetree/bindings/display/display-timing.txt
> This path does not exist.
Right. Will update the path.
> 
> Also, could you try to align these bindings with the one from
> raydium,rm68200?
> 
> There are power-supply and backlight optional properties there, which
> seem useful.
This panel is OLED, while the rm68200 is LCD (from what I've noticed).
Meaning this panel backligth is also controlled by the DSI controller,
not by a separate backlight LED driver.
I will consider, instead, adding support for a power-supply (if
possible).
> 
> > 
> > +
> > +Example:
> > +
> > +   panel@0 {
> > +   compatible = "raydium,rm67191";
> > +   reg = <0>;
> > +   pinctrl-0 = <_mipi_dsi_0_1_en>;
> You should also pass pinctrl-names = "default"; if you use pinctrl-0.
Thanks. Will do that
> 
> > 
> > +   reset-gpio = < 7 GPIO_ACTIVE_HIGH>;
> Should be active low.
But, the GPIO is active high.

Re: [PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

2019-06-14 Thread Fabio Estevam
On Fri, Jun 14, 2019 at 8:53 AM Robert Chiras  wrote:
>
> Add dt-bindings documentation for Raydium RM67191 DSI panel.
>
> Signed-off-by: Robert Chiras 
> ---
>  .../bindings/display/panel/raydium,rm67191.txt | 42 
> ++
>  1 file changed, 42 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt 
> b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> new file mode 100644
> index 000..5a6268d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
> @@ -0,0 +1,42 @@
> +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
> +
> +Required properties:
> +- compatible:  "raydium,rm67191"
> +- reg: virtual channel for MIPI-DSI protocol
> +   must be <0>
> +- dsi-lanes:   number of DSI lanes to be used
> +   must be <3> or <4>
> +- port:input port node with endpoint definition as
> +   defined in 
> Documentation/devicetree/bindings/graph.txt;
> +   the input port should be connected to a MIPI-DSI 
> device
> +   driver
> +
> +Optional properties:
> +- reset-gpio:  a GPIO spec for the RST_B GPIO pin

reset-gpios (with the s in the end) is the recommendation.

> +- display-timings: timings for the connected panel according to [1]

This is not needed.

> +- video-mode:  0 - burst-mode
> +   1 - non-burst with sync event
> +   2 - non-burst with sync pulse
> +
> +[1]: Documentation/devicetree/bindings/display/display-timing.txt

This path does not exist.

Also, could you try to align these bindings with the one from raydium,rm68200?

There are power-supply and backlight optional properties there, which
seem useful.

> +
> +Example:
> +
> +   panel@0 {
> +   compatible = "raydium,rm67191";
> +   reg = <0>;
> +   pinctrl-0 = <_mipi_dsi_0_1_en>;

You should also pass pinctrl-names = "default"; if you use pinctrl-0.

> +   reset-gpio = < 7 GPIO_ACTIVE_HIGH>;

Should be active low.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel

2019-06-14 Thread Robert Chiras
Add dt-bindings documentation for Raydium RM67191 DSI panel.

Signed-off-by: Robert Chiras 
---
 .../bindings/display/panel/raydium,rm67191.txt | 42 ++
 1 file changed, 42 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt 
b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
new file mode 100644
index 000..5a6268d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt
@@ -0,0 +1,42 @@
+Raydium RM67171 OLED LCD panel with MIPI-DSI protocol
+
+Required properties:
+- compatible:  "raydium,rm67191"
+- reg: virtual channel for MIPI-DSI protocol
+   must be <0>
+- dsi-lanes:   number of DSI lanes to be used
+   must be <3> or <4>
+- port:input port node with endpoint definition as
+   defined in Documentation/devicetree/bindings/graph.txt;
+   the input port should be connected to a MIPI-DSI device
+   driver
+
+Optional properties:
+- reset-gpio:  a GPIO spec for the RST_B GPIO pin
+- pinctrl-0phandle to the pin settings for the reset pin
+- width-mm:physical panel width [mm]
+- height-mm:   physical panel height [mm]
+- display-timings: timings for the connected panel according to [1]
+- video-mode:  0 - burst-mode
+   1 - non-burst with sync event
+   2 - non-burst with sync pulse
+
+[1]: Documentation/devicetree/bindings/display/display-timing.txt
+
+Example:
+
+   panel@0 {
+   compatible = "raydium,rm67191";
+   reg = <0>;
+   pinctrl-0 = <_mipi_dsi_0_1_en>;
+   reset-gpio = < 7 GPIO_ACTIVE_HIGH>;
+   dsi-lanes = <4>;
+   width-mm = <68>;
+   height-mm = <121>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
-- 
2.7.4