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

[Rob Herring <robh+dt@kernel.org>]

On Tue, Jun 11, 2019 at 4:02 PM dbasehore . <dbasehore@chromium.org> wrote:
>
> On Tue, Jun 11, 2019 at 8:25 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <dbasehore@chromium.org> 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@chromium.org>
> > > ---
> > >  .../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.

Even if the timing ctrlr handles it, don't you still need to know what
the native orientation is?

> > > +
> > > +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.

The point is your description is all about the kernel. This is a
binding which is not kernel specific.

> > > +
> > > +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.

Just remove the example or add what you want to the "boe,himax8279d8p"
binding doc. We are moving towards examples being compiled and
validated, so incomplete ones won't work.

Rob