| Message ID | 20190611040350.90064-3-dbasehore@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Panel rotation patches | expand |
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. > + > +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 <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. > > > + > > +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 . <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
On Thu, Jun 13, 2019 at 5:52 AM Rob Herring <robh+dt@kernel.org> wrote: > > 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? 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.
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>; + }; + }; + };
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(+)