[PATCH v10 0/2] Panel rotation patches

[PATCH v10 0/2] Panel rotation patches

[Derek Basehore]

This adds the plumbing for reading panel rotation from the devicetree
and sets up adding a panel property for the panel orientation on
Mediatek SoCs when a rotation is present.

v10 changes:
-Adapted to drm_panel_attach changes, so panel orientation is now set
 for the connector in get_modes
-Dropped patch that was already accepted

v9 changes:
-Changed to copying display properties from another copy of
 drm_display_info in drm_panel_attach.

v8 changes:
-added reviewed-by tags
-fixed conflict with i915 patch that recently landed
-Added additional documentation

v7 changes:
-forgot to add static inline

v6 changes:
-added enum declaration to drm_panel.h header

v5 changes:
-rebased

v4 changes:
-fixed some changes made to the i915 driver
-clarified comments on of orientation helper

v3 changes:
-changed from attach/detach callbacks to directly setting fixed panel
 values in drm_panel_attach
-removed update to Documentation
-added separate function for quirked panel orientation property init

v2 changes:
fixed build errors in i915

Derek Basehore (4):
  drm/panel: Add helper for reading DT rotation
  drm/panel: set display info in panel attach
  drm/connector: Split out orientation quirk detection
  drm/mtk: add panel orientation property

 drivers/gpu/drm/drm_connector.c    | 45 ++++++++++++++-----
 drivers/gpu/drm/drm_panel.c        | 70 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c    |  4 +-
 drivers/gpu/drm/i915/vlv_dsi.c     |  5 +--
 drivers/gpu/drm/mediatek/mtk_dsi.c |  8 ++++
 include/drm/drm_connector.h        |  2 +
 include/drm/drm_panel.h            | 21 +++++++++
 7 files changed, 138 insertions(+), 17 deletions(-)

-- 
2.22.0.410.gd8fdbe21b5-goog

[PATCH v10 1/2] drm/panel: Add helper for reading DT rotation

[Derek Basehore]

This adds a helper function for reading the rotation (panel
orientation) from the device tree.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/drm_panel.c | 43 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_panel.h     |  9 ++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 8c7bac85a793..5557c75301f1 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -300,6 +300,49 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np)
 	return ERR_PTR(-EPROBE_DEFER);
 }
 EXPORT_SYMBOL(of_drm_find_panel);
+
+/**
+ * of_drm_get_panel_orientation - look up the orientation of the panel through
+ * the "rotation" binding from a device tree node
+ * @np: device tree node of the panel
+ * @orientation: orientation enum to be filled in
+ *
+ * Looks up the rotation of a panel in the device tree. The orientation of the
+ * panel is expressed as a property name "rotation" in the device tree. The
+ * rotation in the device tree is counter clockwise.
+ *
+ * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
+ * rotation property doesn't exist. -EERROR otherwise.
+ */
+int of_drm_get_panel_orientation(const struct device_node *np,
+				 enum drm_panel_orientation *orientation)
+{
+	int rotation, ret;
+
+	ret = of_property_read_u32(np, "rotation", &rotation);
+	if (ret == -EINVAL) {
+		/* Don't return an error if there's no rotation property. */
+		*orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+		return 0;
+	}
+
+	if (ret < 0)
+		return ret;
+
+	if (rotation == 0)
+		*orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+	else if (rotation == 90)
+		*orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+	else if (rotation == 180)
+		*orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+	else if (rotation == 270)
+		*orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL(of_drm_get_panel_orientation);
 #endif
 
 #if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 6193cb555acc..781c735f0f9b 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -35,6 +35,8 @@ struct drm_device;
 struct drm_panel;
 struct display_timing;
 
+enum drm_panel_orientation;
+
 /**
  * struct drm_panel_funcs - perform operations on a given panel
  *
@@ -191,11 +193,18 @@ int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector *connector
 
 #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
 struct drm_panel *of_drm_find_panel(const struct device_node *np);
+int of_drm_get_panel_orientation(const struct device_node *np,
+				 enum drm_panel_orientation *orientation);
 #else
 static inline struct drm_panel *of_drm_find_panel(const struct device_node *np)
 {
 	return ERR_PTR(-ENODEV);
 }
+static inline int of_drm_get_panel_orientation(const struct device_node *np,
+		enum drm_panel_orientation *orientation)
+{
+	return -ENODEV;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
-- 
2.25.0.265.gbab2e86ba0-goog

[PATCH v10 2/2] drm/panel: read panel orientation for BOE tv101wum-nl6

[Derek Basehore]

This reads the DT setting for the panel rotation to set the panel
orientation in the get_modes callback.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
---
 drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 48a164257d18..ee2d96413900 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -11,6 +11,7 @@
 #include <linux/of_device.h>
 #include <linux/regulator/consumer.h>
 
+#include <drm/drm_connector.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
@@ -43,6 +44,7 @@ struct boe_panel {
 
 	const struct panel_desc *desc;
 
+	enum drm_panel_orientation orientation;
 	struct regulator *pp1800;
 	struct regulator *avee;
 	struct regulator *avdd;
@@ -717,6 +719,7 @@ static int boe_panel_get_modes(struct drm_panel *panel,
 	connector->display_info.width_mm = boe->desc->size.width_mm;
 	connector->display_info.height_mm = boe->desc->size.height_mm;
 	connector->display_info.bpc = boe->desc->bpc;
+	drm_connector_set_panel_orientation(connector, boe->orientation);
 
 	return 1;
 }
@@ -756,6 +759,9 @@ static int boe_panel_add(struct boe_panel *boe)
 
 	drm_panel_init(&boe->base, dev, &boe_panel_funcs,
 		       DRM_MODE_CONNECTOR_DSI);
+	err = of_drm_get_panel_orientation(dev->of_node, &boe->orientation);
+	if (err < 0)
+		return err;
 
 	err = drm_panel_of_backlight(&boe->base);
 	if (err)
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v10 0/2] Panel rotation patches

[Dmitry Osipenko]

06.03.2020 03:21, Derek Basehore пишет:
> This adds the plumbing for reading panel rotation from the devicetree
> and sets up adding a panel property for the panel orientation on
> Mediatek SoCs when a rotation is present.

Hello Derek and everyone,

I'm looking at adding display rotation support to NVIDIA Tegra DRM
driver because some devices have display panel physically mounted
upside-down, and thus, display controller's scan-out needs to be rotated
by 180° in this case.

Derek, yours panel-rotation patches add support for assigning panel's
orientation to the connector, but then only primary display plane
receives rotation value in [1], while rotation needs to be applied to
all available overlay/cursor planes and this should happen in other
places than [1] as well.

[1] drm_client_modeset_commit_atomic()

Please also note that in a case of the scan-out rotation, plane's
coordinates need to be changed in accordance to the display's rotation.

I looked briefly through the DRM code and my understanding that the DRM
core currently doesn't support use-case where scan-out needs to rotated
based on a panel's orientation, correct? Is it the use-case you're
working on for the Mediatek driver?

Re: [PATCH v10 0/2] Panel rotation patches

[dbasehore .]

Hi Dmitry, sorry for the late reply.

On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 06.03.2020 03:21, Derek Basehore пишет:
> > This adds the plumbing for reading panel rotation from the devicetree
> > and sets up adding a panel property for the panel orientation on
> > Mediatek SoCs when a rotation is present.
>
> Hello Derek and everyone,
>
> I'm looking at adding display rotation support to NVIDIA Tegra DRM
> driver because some devices have display panel physically mounted
> upside-down, and thus, display controller's scan-out needs to be rotated
> by 180° in this case.
>
> Derek, yours panel-rotation patches add support for assigning panel's
> orientation to the connector, but then only primary display plane
> receives rotation value in [1], while rotation needs to be applied to
> all available overlay/cursor planes and this should happen in other
> places than [1] as well.

This is intended. We don't correct the output in the kernel. We
instead rely on notifying userspace that the panel is rotated, then we
handle it there.

>
> [1] drm_client_modeset_commit_atomic()
>
> Please also note that in a case of the scan-out rotation, plane's
> coordinates need to be changed in accordance to the display's rotation.
>
> I looked briefly through the DRM code and my understanding that the DRM
> core currently doesn't support use-case where scan-out needs to rotated
> based on a panel's orientation, correct? Is it the use-case you're
> working on for the Mediatek driver?

Yes, we rely on userspace to rotate the output. The major reason for
this is because there may not be a "free" hardware rotation that can
be applied to the overlay. Sean Paul and others also preferred that
userspace control what is output to the screen instead of the kernel
taking care of it. This code just adds the drm property to the panel.

Re: [PATCH v10 0/2] Panel rotation patches

[Dmitry Osipenko]

14.04.2020 22:32, dbasehore . пишет:
> Hi Dmitry, sorry for the late reply.
> 
> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 06.03.2020 03:21, Derek Basehore пишет:
>>> This adds the plumbing for reading panel rotation from the devicetree
>>> and sets up adding a panel property for the panel orientation on
>>> Mediatek SoCs when a rotation is present.
>>
>> Hello Derek and everyone,
>>
>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
>> driver because some devices have display panel physically mounted
>> upside-down, and thus, display controller's scan-out needs to be rotated
>> by 180° in this case.
>>
>> Derek, yours panel-rotation patches add support for assigning panel's
>> orientation to the connector, but then only primary display plane
>> receives rotation value in [1], while rotation needs to be applied to
>> all available overlay/cursor planes and this should happen in other
>> places than [1] as well.
> 
> This is intended. We don't correct the output in the kernel. We
> instead rely on notifying userspace that the panel is rotated, then we
> handle it there.
> 
>>
>> [1] drm_client_modeset_commit_atomic()
>>
>> Please also note that in a case of the scan-out rotation, plane's
>> coordinates need to be changed in accordance to the display's rotation.
>>
>> I looked briefly through the DRM code and my understanding that the DRM
>> core currently doesn't support use-case where scan-out needs to rotated
>> based on a panel's orientation, correct? Is it the use-case you're
>> working on for the Mediatek driver?
> 
> Yes, we rely on userspace to rotate the output. The major reason for
> this is because there may not be a "free" hardware rotation that can
> be applied to the overlay. Sean Paul and others also preferred that
> userspace control what is output to the screen instead of the kernel
> taking care of it. This code just adds the drm property to the panel.
> 

Could you please explain what that userspace is?

AFAIK, things like Xorg modesetting don't support that orientation property.

Re: [PATCH v10 0/2] Panel rotation patches

[dbasehore .]

On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 14.04.2020 22:32, dbasehore . пишет:
> > Hi Dmitry, sorry for the late reply.
> >
> > On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> >>
> >> 06.03.2020 03:21, Derek Basehore пишет:
> >>> This adds the plumbing for reading panel rotation from the devicetree
> >>> and sets up adding a panel property for the panel orientation on
> >>> Mediatek SoCs when a rotation is present.
> >>
> >> Hello Derek and everyone,
> >>
> >> I'm looking at adding display rotation support to NVIDIA Tegra DRM
> >> driver because some devices have display panel physically mounted
> >> upside-down, and thus, display controller's scan-out needs to be rotated
> >> by 180° in this case.
> >>
> >> Derek, yours panel-rotation patches add support for assigning panel's
> >> orientation to the connector, but then only primary display plane
> >> receives rotation value in [1], while rotation needs to be applied to
> >> all available overlay/cursor planes and this should happen in other
> >> places than [1] as well.
> >
> > This is intended. We don't correct the output in the kernel. We
> > instead rely on notifying userspace that the panel is rotated, then we
> > handle it there.
> >
> >>
> >> [1] drm_client_modeset_commit_atomic()
> >>
> >> Please also note that in a case of the scan-out rotation, plane's
> >> coordinates need to be changed in accordance to the display's rotation.
> >>
> >> I looked briefly through the DRM code and my understanding that the DRM
> >> core currently doesn't support use-case where scan-out needs to rotated
> >> based on a panel's orientation, correct? Is it the use-case you're
> >> working on for the Mediatek driver?
> >
> > Yes, we rely on userspace to rotate the output. The major reason for
> > this is because there may not be a "free" hardware rotation that can
> > be applied to the overlay. Sean Paul and others also preferred that
> > userspace control what is output to the screen instead of the kernel
> > taking care of it. This code just adds the drm property to the panel.
> >
>
> Could you please explain what that userspace is?

This was added for Chrome OS, which uses its own graphics stack,
Ozone, instead of Xorg.

>
> AFAIK, things like Xorg modesetting don't support that orientation property.

Re: [PATCH v10 0/2] Panel rotation patches

[Dmitry Osipenko]

15.04.2020 00:32, dbasehore . пишет:
> On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 14.04.2020 22:32, dbasehore . пишет:
>>> Hi Dmitry, sorry for the late reply.
>>>
>>> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> 06.03.2020 03:21, Derek Basehore пишет:
>>>>> This adds the plumbing for reading panel rotation from the devicetree
>>>>> and sets up adding a panel property for the panel orientation on
>>>>> Mediatek SoCs when a rotation is present.
>>>>
>>>> Hello Derek and everyone,
>>>>
>>>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
>>>> driver because some devices have display panel physically mounted
>>>> upside-down, and thus, display controller's scan-out needs to be rotated
>>>> by 180° in this case.
>>>>
>>>> Derek, yours panel-rotation patches add support for assigning panel's
>>>> orientation to the connector, but then only primary display plane
>>>> receives rotation value in [1], while rotation needs to be applied to
>>>> all available overlay/cursor planes and this should happen in other
>>>> places than [1] as well.
>>>
>>> This is intended. We don't correct the output in the kernel. We
>>> instead rely on notifying userspace that the panel is rotated, then we
>>> handle it there.
>>>
>>>>
>>>> [1] drm_client_modeset_commit_atomic()
>>>>
>>>> Please also note that in a case of the scan-out rotation, plane's
>>>> coordinates need to be changed in accordance to the display's rotation.
>>>>
>>>> I looked briefly through the DRM code and my understanding that the DRM
>>>> core currently doesn't support use-case where scan-out needs to rotated
>>>> based on a panel's orientation, correct? Is it the use-case you're
>>>> working on for the Mediatek driver?
>>>
>>> Yes, we rely on userspace to rotate the output. The major reason for
>>> this is because there may not be a "free" hardware rotation that can
>>> be applied to the overlay. Sean Paul and others also preferred that
>>> userspace control what is output to the screen instead of the kernel
>>> taking care of it. This code just adds the drm property to the panel.
>>>
>>
>> Could you please explain what that userspace is?
> 
> This was added for Chrome OS, which uses its own graphics stack,
> Ozone, instead of Xorg.
> 

Thank you very much for the clarification.

It's probably not a big problem for something monolithic and customized
like ChromeOS to issue a software update in order to take into account
all specifics of a particular device, but this doesn't work nicely for a
generic software, like a usual Linux distro.

>> AFAIK, things like Xorg modesetting don't support that orientation property.

In my case it's not only the display panel which is upside-down, but
also the touchscreen. Hence both display output and touchscreen input
need to be rotated at once, otherwise you'll end up with either display
or input being upside-down.

The 180° rotation should be free on NVIDIA Tegra. There are no known
limitations for the planes and BSP kernel video driver handles the
plane's coordinates/framebuffer rotation within the driver.

The kernel's input subsystem allows us to transparently (for userspace)
remap the touchscreen input (by specifying generic touchscreen
device-tree properties), while this is not the case for the DRM subsystem.

@Thierry, @Sean, @Daniel, could you please help me to understand how a
coordinated display / input rotation could be implemented, making the
rotation transparent to the user (i.e. avoiding xorg.conf hacking and
etc)? It should be nice if display's output could be flipped within the
DRM driver, hiding this fact from userspace.

Will it be okay if we'll add a transparent-rotation support specifically
to the Tegra DRM driver? For example if device-tree contains
nvidia,display-flip-y property, then the Tegra DRM driver will take care
of rotating coordinates/framebuffer of the display planes.

Re: [PATCH v10 0/2] Panel rotation patches

[Sean Paul]

On Thu, Apr 16, 2020 at 7:03 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 15.04.2020 00:32, dbasehore . пишет:
> > On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> >>
> >> 14.04.2020 22:32, dbasehore . пишет:
> >>> Hi Dmitry, sorry for the late reply.
> >>>
> >>> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> >>>>
> >>>> 06.03.2020 03:21, Derek Basehore пишет:
> >>>>> This adds the plumbing for reading panel rotation from the devicetree
> >>>>> and sets up adding a panel property for the panel orientation on
> >>>>> Mediatek SoCs when a rotation is present.
> >>>>
> >>>> Hello Derek and everyone,
> >>>>
> >>>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
> >>>> driver because some devices have display panel physically mounted
> >>>> upside-down, and thus, display controller's scan-out needs to be rotated
> >>>> by 180° in this case.
> >>>>
> >>>> Derek, yours panel-rotation patches add support for assigning panel's
> >>>> orientation to the connector, but then only primary display plane
> >>>> receives rotation value in [1], while rotation needs to be applied to
> >>>> all available overlay/cursor planes and this should happen in other
> >>>> places than [1] as well.
> >>>
> >>> This is intended. We don't correct the output in the kernel. We
> >>> instead rely on notifying userspace that the panel is rotated, then we
> >>> handle it there.
> >>>
> >>>>
> >>>> [1] drm_client_modeset_commit_atomic()
> >>>>
> >>>> Please also note that in a case of the scan-out rotation, plane's
> >>>> coordinates need to be changed in accordance to the display's rotation.
> >>>>
> >>>> I looked briefly through the DRM code and my understanding that the DRM
> >>>> core currently doesn't support use-case where scan-out needs to rotated
> >>>> based on a panel's orientation, correct? Is it the use-case you're
> >>>> working on for the Mediatek driver?
> >>>
> >>> Yes, we rely on userspace to rotate the output. The major reason for
> >>> this is because there may not be a "free" hardware rotation that can
> >>> be applied to the overlay. Sean Paul and others also preferred that
> >>> userspace control what is output to the screen instead of the kernel
> >>> taking care of it. This code just adds the drm property to the panel.
> >>>
> >>
> >> Could you please explain what that userspace is?
> >
> > This was added for Chrome OS, which uses its own graphics stack,
> > Ozone, instead of Xorg.
> >
>
> Thank you very much for the clarification.
>
> It's probably not a big problem for something monolithic and customized
> like ChromeOS to issue a software update in order to take into account
> all specifics of a particular device, but this doesn't work nicely for a
> generic software, like a usual Linux distro.
>
> >> AFAIK, things like Xorg modesetting don't support that orientation property.
>
> In my case it's not only the display panel which is upside-down, but
> also the touchscreen. Hence both display output and touchscreen input
> need to be rotated at once, otherwise you'll end up with either display
> or input being upside-down.
>
> The 180° rotation should be free on NVIDIA Tegra. There are no known
> limitations for the planes and BSP kernel video driver handles the
> plane's coordinates/framebuffer rotation within the driver.
>
> The kernel's input subsystem allows us to transparently (for userspace)
> remap the touchscreen input (by specifying generic touchscreen
> device-tree properties), while this is not the case for the DRM subsystem.
>
> @Thierry, @Sean, @Daniel, could you please help me to understand how a
> coordinated display / input rotation could be implemented, making the
> rotation transparent to the user (i.e. avoiding xorg.conf hacking and
> etc)? It should be nice if display's output could be flipped within the
> DRM driver, hiding this fact from userspace.

I think the right thing to do is to fix userspace to respect this
property, since that has the most communal benefit.

However(!!) if you don't want to do that, how about inspecting the
info->panel_orientation value after drm_panel_attach in tegra driver
and then adjusting rotation values in the driver. Of course, you
wouldn't want to expose the panel orientation property since you don't
want userspaces to be double-rotating on you, but it's optional so
you'd be fine.

>
> Will it be okay if we'll add a transparent-rotation support specifically
> to the Tegra DRM driver? For example if device-tree contains
> nvidia,display-flip-y property, then the Tegra DRM driver will take care
> of rotating coordinates/framebuffer of the display planes.

I don't think this is necessary, but it also wouldn't really be
appropriate to put software attributes into devicetree anyways.

Sean

Re: [PATCH v10 0/2] Panel rotation patches

[Dmitry Osipenko]

12.05.2020 23:59, Sean Paul пишет:
> On Thu, Apr 16, 2020 at 7:03 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 15.04.2020 00:32, dbasehore . пишет:
>>> On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> 14.04.2020 22:32, dbasehore . пишет:
>>>>> Hi Dmitry, sorry for the late reply.
>>>>>
>>>>> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>>>
>>>>>> 06.03.2020 03:21, Derek Basehore пишет:
>>>>>>> This adds the plumbing for reading panel rotation from the devicetree
>>>>>>> and sets up adding a panel property for the panel orientation on
>>>>>>> Mediatek SoCs when a rotation is present.
>>>>>>
>>>>>> Hello Derek and everyone,
>>>>>>
>>>>>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
>>>>>> driver because some devices have display panel physically mounted
>>>>>> upside-down, and thus, display controller's scan-out needs to be rotated
>>>>>> by 180° in this case.
>>>>>>
>>>>>> Derek, yours panel-rotation patches add support for assigning panel's
>>>>>> orientation to the connector, but then only primary display plane
>>>>>> receives rotation value in [1], while rotation needs to be applied to
>>>>>> all available overlay/cursor planes and this should happen in other
>>>>>> places than [1] as well.
>>>>>
>>>>> This is intended. We don't correct the output in the kernel. We
>>>>> instead rely on notifying userspace that the panel is rotated, then we
>>>>> handle it there.
>>>>>
>>>>>>
>>>>>> [1] drm_client_modeset_commit_atomic()
>>>>>>
>>>>>> Please also note that in a case of the scan-out rotation, plane's
>>>>>> coordinates need to be changed in accordance to the display's rotation.
>>>>>>
>>>>>> I looked briefly through the DRM code and my understanding that the DRM
>>>>>> core currently doesn't support use-case where scan-out needs to rotated
>>>>>> based on a panel's orientation, correct? Is it the use-case you're
>>>>>> working on for the Mediatek driver?
>>>>>
>>>>> Yes, we rely on userspace to rotate the output. The major reason for
>>>>> this is because there may not be a "free" hardware rotation that can
>>>>> be applied to the overlay. Sean Paul and others also preferred that
>>>>> userspace control what is output to the screen instead of the kernel
>>>>> taking care of it. This code just adds the drm property to the panel.
>>>>>
>>>>
>>>> Could you please explain what that userspace is?
>>>
>>> This was added for Chrome OS, which uses its own graphics stack,
>>> Ozone, instead of Xorg.
>>>
>>
>> Thank you very much for the clarification.
>>
>> It's probably not a big problem for something monolithic and customized
>> like ChromeOS to issue a software update in order to take into account
>> all specifics of a particular device, but this doesn't work nicely for a
>> generic software, like a usual Linux distro.
>>
>>>> AFAIK, things like Xorg modesetting don't support that orientation property.
>>
>> In my case it's not only the display panel which is upside-down, but
>> also the touchscreen. Hence both display output and touchscreen input
>> need to be rotated at once, otherwise you'll end up with either display
>> or input being upside-down.
>>
>> The 180° rotation should be free on NVIDIA Tegra. There are no known
>> limitations for the planes and BSP kernel video driver handles the
>> plane's coordinates/framebuffer rotation within the driver.
>>
>> The kernel's input subsystem allows us to transparently (for userspace)
>> remap the touchscreen input (by specifying generic touchscreen
>> device-tree properties), while this is not the case for the DRM subsystem.
>>
>> @Thierry, @Sean, @Daniel, could you please help me to understand how a
>> coordinated display / input rotation could be implemented, making the
>> rotation transparent to the user (i.e. avoiding xorg.conf hacking and
>> etc)? It should be nice if display's output could be flipped within the
>> DRM driver, hiding this fact from userspace.
> 
> I think the right thing to do is to fix userspace to respect this
> property, since that has the most communal benefit.

Hello Sean,

This will be ideal, but it's difficult to achieve in a loosely
controlled userspace environment.

> However(!!) if you don't want to do that, how about inspecting the
> info->panel_orientation value after drm_panel_attach in tegra driver
> and then adjusting rotation values in the driver. Of course, you
> wouldn't want to expose the panel orientation property since you don't
> want userspaces to be double-rotating on you, but it's optional so
> you'd be fine.

Thank you very much for the suggestion, I'll be trying it out soon.

>>
>> Will it be okay if we'll add a transparent-rotation support specifically
>> to the Tegra DRM driver? For example if device-tree contains
>> nvidia,display-flip-y property, then the Tegra DRM driver will take care
>> of rotating coordinates/framebuffer of the display planes.
> 
> I don't think this is necessary, but it also wouldn't really be
> appropriate to put software attributes into devicetree anyways.

Yes, I'm also not feeling very excited about this variant.

Re: [PATCH v10 0/2] Panel rotation patches

[Dmitry Osipenko]

18.05.2020 10:36, Dmitry Osipenko пишет:
> 12.05.2020 23:59, Sean Paul пишет:
>> On Thu, Apr 16, 2020 at 7:03 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>
>>> 15.04.2020 00:32, dbasehore . пишет:
>>>> On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>>
>>>>> 14.04.2020 22:32, dbasehore . пишет:
>>>>>> Hi Dmitry, sorry for the late reply.
>>>>>>
>>>>>> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>>>>
>>>>>>> 06.03.2020 03:21, Derek Basehore пишет:
>>>>>>>> This adds the plumbing for reading panel rotation from the devicetree
>>>>>>>> and sets up adding a panel property for the panel orientation on
>>>>>>>> Mediatek SoCs when a rotation is present.
>>>>>>>
>>>>>>> Hello Derek and everyone,
>>>>>>>
>>>>>>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
>>>>>>> driver because some devices have display panel physically mounted
>>>>>>> upside-down, and thus, display controller's scan-out needs to be rotated
>>>>>>> by 180° in this case.
>>>>>>>
>>>>>>> Derek, yours panel-rotation patches add support for assigning panel's
>>>>>>> orientation to the connector, but then only primary display plane
>>>>>>> receives rotation value in [1], while rotation needs to be applied to
>>>>>>> all available overlay/cursor planes and this should happen in other
>>>>>>> places than [1] as well.
>>>>>>
>>>>>> This is intended. We don't correct the output in the kernel. We
>>>>>> instead rely on notifying userspace that the panel is rotated, then we
>>>>>> handle it there.
>>>>>>
>>>>>>>
>>>>>>> [1] drm_client_modeset_commit_atomic()
>>>>>>>
>>>>>>> Please also note that in a case of the scan-out rotation, plane's
>>>>>>> coordinates need to be changed in accordance to the display's rotation.
>>>>>>>
>>>>>>> I looked briefly through the DRM code and my understanding that the DRM
>>>>>>> core currently doesn't support use-case where scan-out needs to rotated
>>>>>>> based on a panel's orientation, correct? Is it the use-case you're
>>>>>>> working on for the Mediatek driver?
>>>>>>
>>>>>> Yes, we rely on userspace to rotate the output. The major reason for
>>>>>> this is because there may not be a "free" hardware rotation that can
>>>>>> be applied to the overlay. Sean Paul and others also preferred that
>>>>>> userspace control what is output to the screen instead of the kernel
>>>>>> taking care of it. This code just adds the drm property to the panel.
>>>>>>
>>>>>
>>>>> Could you please explain what that userspace is?
>>>>
>>>> This was added for Chrome OS, which uses its own graphics stack,
>>>> Ozone, instead of Xorg.
>>>>
>>>
>>> Thank you very much for the clarification.
>>>
>>> It's probably not a big problem for something monolithic and customized
>>> like ChromeOS to issue a software update in order to take into account
>>> all specifics of a particular device, but this doesn't work nicely for a
>>> generic software, like a usual Linux distro.
>>>
>>>>> AFAIK, things like Xorg modesetting don't support that orientation property.
>>>
>>> In my case it's not only the display panel which is upside-down, but
>>> also the touchscreen. Hence both display output and touchscreen input
>>> need to be rotated at once, otherwise you'll end up with either display
>>> or input being upside-down.
>>>
>>> The 180° rotation should be free on NVIDIA Tegra. There are no known
>>> limitations for the planes and BSP kernel video driver handles the
>>> plane's coordinates/framebuffer rotation within the driver.
>>>
>>> The kernel's input subsystem allows us to transparently (for userspace)
>>> remap the touchscreen input (by specifying generic touchscreen
>>> device-tree properties), while this is not the case for the DRM subsystem.
>>>
>>> @Thierry, @Sean, @Daniel, could you please help me to understand how a
>>> coordinated display / input rotation could be implemented, making the
>>> rotation transparent to the user (i.e. avoiding xorg.conf hacking and
>>> etc)? It should be nice if display's output could be flipped within the
>>> DRM driver, hiding this fact from userspace.
>>
>> I think the right thing to do is to fix userspace to respect this
>> property, since that has the most communal benefit.
> 
> Hello Sean,
> 
> This will be ideal, but it's difficult to achieve in a loosely
> controlled userspace environment.
> 
>> However(!!) if you don't want to do that, how about inspecting the
>> info->panel_orientation value after drm_panel_attach in tegra driver
>> and then adjusting rotation values in the driver. Of course, you
>> wouldn't want to expose the panel orientation property since you don't
>> want userspaces to be double-rotating on you, but it's optional so
>> you'd be fine.
> 
> Thank you very much for the suggestion, I'll be trying it out soon.
> 
>>>
>>> Will it be okay if we'll add a transparent-rotation support specifically
>>> to the Tegra DRM driver? For example if device-tree contains
>>> nvidia,display-flip-y property, then the Tegra DRM driver will take care
>>> of rotating coordinates/framebuffer of the display planes.
>>
>> I don't think this is necessary, but it also wouldn't really be
>> appropriate to put software attributes into devicetree anyways.
> 
> Yes, I'm also not feeling very excited about this variant.
> 

After some consideration, I decided that it will be better to start easy
by supporting the minimum needed for the rotation property to work on
Tegra, i.e. having userspace to take care of the rotation. It will be
possible to change it later on if will be necessary.

@dbasehore, I'll prepare Tegra DRM patchset around Monday and will
include yours two patches that add DT reading helper and set the display
info, since these patches haven't been applied yet.