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

[Hans de Goede]

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.

Regards,

Hans