Re: [PATCH 09/12] drm/modes: parse_cmdline: Add support for specifying panel_orientation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 11-11-2019 13:53, Maxime Ripard wrote:
Hi Hans,

Thanks for this series (and thanks for bouncing the mails too).

All the previous patches are
Acked-by: Maxime Ripard <mripard@xxxxxxxxxx>

Thank you for the review.

On Sun, Nov 10, 2019 at 04:40:58PM +0100, Hans de Goede wrote:
Sometimes we want to override a connector's panel_orientation from the
kernel commandline. Either for testing and for special cases, e.g. a kiosk
like setup which uses a TV mounted in portrait mode.

Users can already specify a "rotate" option through a video= kernel cmdline
option. But that only supports 0/180 degrees (see drm_client_modeset TODO)
and only works for in kernel modeset clients, not for userspace kms users.

The "panel-orientation" connector property OTOH does support 90/270 degrees
as it leaves dealing with the rotation up to userspace and this does work
for userspace kms clients (at least those which support this property).

BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
  Documentation/fb/modedb.rst                   |  3 ++
  drivers/gpu/drm/drm_modes.c                   | 32 +++++++++++++++++++
  .../gpu/drm/selftests/drm_cmdline_selftests.h |  1 +
  .../drm/selftests/test-drm_cmdline_parser.c   | 22 +++++++++++++
  include/drm/drm_connector.h                   |  8 +++++
  5 files changed, 66 insertions(+)

diff --git a/Documentation/fb/modedb.rst b/Documentation/fb/modedb.rst
index 9c4e3fd39e6d..624d08fd2856 100644
--- a/Documentation/fb/modedb.rst
+++ b/Documentation/fb/modedb.rst
@@ -65,6 +65,9 @@ Valid options are::
    - reflect_y (boolean): Perform an axial symmetry on the Y axis
    - rotate (integer): Rotate the initial framebuffer by x
      degrees. Valid values are 0, 90, 180 and 270.
+  - panel_orientation, one of "normal", "upside_down", "left_side_up", or
+    "right_side_up". For KMS drivers only, this sets the "panel orientation"
+    property on the kms connector as hint for kms users.

Even though the semantic is a bit different, I think we should remain
consistent and have the same argument than for rotate (ie, steps in
clockwise rotation in steps of 90 degrees).

Well the kernel kms defines for rotation also talk about 90  / 180 / 270":

#define DRM_MODE_ROTATE_0       (1<<0)
#define DRM_MODE_ROTATE_90      (1<<1)
#define DRM_MODE_ROTATE_180     (1<<2)
#define DRM_MODE_ROTATE_270     (1<<3)

Where as the panel orientation uses strings like right_side_up, which means
that the side of the panel which normally is the right side of the panel
is now pointing up as the panel is mounted 90 degrees rotated with its
original right side now pointing up. This IMHO is much clearer then
90 / 270 degrees which are ambiguous and perhaps more importantly this
matches the kernel defines for panel-orientation and matches the
String values enumerated by the enum type panel-orientation connector
property:

static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
        { DRM_MODE_PANEL_ORIENTATION_NORMAL,    "Normal"        },
        { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
        { DRM_MODE_PANEL_ORIENTATION_LEFT_UP,   "Left Side Up"  },
        { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,  "Right Side Up" },
};

So I would prefer to stick to the strings.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux