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 <[email protected]>
---
.../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
+
+The panel driver may apply the rotation at the TCON level, which will
+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.
+
+Example:
+ panel: panel@0 {
+ compatible = "boe,himax8279d8p";
+ reg = <0>;
+ enable-gpios = <&pio 45 0>;
+ pp33-gpios = <&pio 35 0>;
+ pp18-gpios = <&pio 36 0>;
+ 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
On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <[email protected]> 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 <[email protected]>
> ---
> .../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.
> +
> +The panel driver may apply the rotation at the TCON level, which will
What's TCON? Something Mediatek specific IIRC.
> +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.
> +
> +Example:
This file is a collection of common properties. It shouldn't have an
example especially as this example is mostly non-common properties.
> + 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.
> + 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
>
On Tue, Jun 11, 2019 at 8:25 AM Rob Herring <[email protected]> wrote:
>
> On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <[email protected]> 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 <[email protected]>
> > ---
> > .../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
> >
On Tue, Jun 11, 2019 at 4:02 PM dbasehore . <[email protected]> wrote:
>
> On Tue, Jun 11, 2019 at 8:25 AM Rob Herring <[email protected]> wrote:
> >
> > On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <[email protected]> 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 <[email protected]>
> > > ---
> > > .../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
On Thu, Jun 13, 2019 at 5:52 AM Rob Herring <[email protected]> wrote:
>
> On Tue, Jun 11, 2019 at 4:02 PM dbasehore . <[email protected]> wrote:
> >
> > On Tue, Jun 11, 2019 at 8:25 AM Rob Herring <[email protected]> wrote:
> > >
> > > On Mon, Jun 10, 2019 at 10:03 PM Derek Basehore <[email protected]> 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 <[email protected]>
> > > > ---
> > > > .../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?
Not really. For all intents and purposes, the orientation of the panel
has changed.
>
> > > > +
> > > > +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.
Ah, I see. I thought you were saying what the implementation should be.
>
> > > > +
> > > > +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.
Ok, will do.
>
> Rob
Thanks for the quick reviews.