Re: [PATCH 2/5] dt-bindings: display/panel: Expand rotation documentation

[dbasehore .]

On Tue, Jun 11, 2019 at 8:25 AM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
>
> On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <dbasehore@xxxxxxxxxxxx> wrote:
> >
> > This adds to the rotation documentation to explain how drivers should
> > use the property and gives an example of the property in a devicetree
> > node.
> >
> > Signed-off-by: Derek Basehore <dbasehore@xxxxxxxxxxxx>
> > ---
> >  .../bindings/display/panel/panel.txt          | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel.txt b/Documentation/devicetree/bindings/display/panel/panel.txt
> > index e2e6867852b8..f35d62d933fc 100644
> > --- a/Documentation/devicetree/bindings/display/panel/panel.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/panel.txt
> > @@ -2,3 +2,35 @@ Common display properties
> >  -------------------------
> >
> >  - rotation:    Display rotation in degrees counter clockwise (0,90,180,270)
> > +
> > +Property read from the device tree using of of_drm_get_panel_orientation
>
> Don't put kernel specifics into bindings.

Will remove that. I'll clean up the documentation to indicate that
this binding creates a panel orientation property unless the rotation
is handled in the Timing Controller on the panel if that sounds fine.

>
> > +
> > +The panel driver may apply the rotation at the TCON level, which will
>
> What's TCON? Something Mediatek specific IIRC.

The TCON is the Timing controller, which is on the panel. Every panel
has one. I'll add to the doc that the TCON is in the panel, etc.

>
> > +make the panel look like it isn't rotated to the kernel and any other
> > +software.
> > +
> > +If not, a panel orientation property should be added through the SoC
> > +vendor DRM code using the drm_connector_init_panel_orientation_property
> > +function.
>
> The 'rotation' property should be defined purely based on how the
> panel is mounted relative to a device's orientation. If the display
> pipeline has some ability to handle rotation, that's a feature of the
> display pipeline and not the panel.

This is how the panel orientation property is already handled in the
kernel. See drivers/gpu/drm/i915/vlv_dsi.c for more details.

>
> > +
> > +Example:
>
> This file is a collection of common properties. It shouldn't have an
> example especially as this example is mostly non-common properties.

Just copied one of our DTS entries that uses the property. I'll remove
everything under compatible except for rotation and status.

>
> > +       panel: panel@0 {
> > +               compatible = "boe,himax8279d8p";
> > +               reg = <0>;
> > +               enable-gpios = <&pio 45 0>;
>
> > +               pp33-gpios = <&pio 35 0>;
> > +               pp18-gpios = <&pio 36 0>;
>
> BTW, are these upstream because they look like GPIO controlled
> supplies which we model with gpio-regulator binding typically.

The boe,himax8279 driver was sent upstream, but it doesn't appear to
be merged. I'll look into it on that thread.

>
> > +               pinctrl-names = "default", "state_3300mv", "state_1800mv";
> > +               pinctrl-0 = <&panel_pins_default>;
> > +               pinctrl-1 = <&panel_pins_3300mv>;
> > +               pinctrl-2 = <&panel_pins_1800mv>;
> > +               backlight = <&backlight_lcd0>;
> > +               rotation = <180>;
> > +               status = "okay";
> > +
> > +               port {
> > +                       panel_in: endpoint {
> > +                               remote-endpoint = <&dsi_out>;
> > +                       };
> > +               };
> > +       };
> > --
> > 2.22.0.rc2.383.gf4fbbf30c2-goog
> >