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

[Maxime Ripard]

On Mon, Nov 11, 2019 at 06:25:33PM +0100, Hans de Goede wrote:
> 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 at kernel.org>
>
> 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 at redhat.com>
> > > ---
> > >   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.

Ok, that works for me then

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191113/27f527af/attachment.sig>