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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1560519599.9328.40.camel@nxp.com>
Date:   Fri, 14 Jun 2019 13:40:00 +0000
From:   Robert Chiras <robert.chiras@....com>
To:     "festevam@...il.com" <festevam@...il.com>
CC:     dl-linux-imx <linux-imx@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "sam@...nborg.org" <sam@...nborg.org>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "airlied@...ux.ie" <airlied@...ux.ie>
Subject: Re: [EXT] Re: [PATCH 1/2] dt-bindings: display: panel: Add support
 for Raydium RM67191 panel

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 <robert.chiras@....com>
> wrote:
> > 
> > 
> > Add dt-bindings documentation for Raydium RM67191 DSI panel.
> > 
> > Signed-off-by: Robert Chiras <robert.chiras@....com>
> > ---
> >  .../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 0000000..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 = <&pinctrl_mipi_dsi_0_1_en>;
> You should also pass pinctrl-names = "default"; if you use pinctrl-0.
Thanks. Will do that
> 
> > 
> > +               reset-gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> Should be active low.
But, the GPIO is active high.