From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Ripard Subject: Re: [PATCH 09/12] drm/modes: parse_cmdline: Add support for specifying panel_orientation Date: Wed, 13 Nov 2019 17:12:23 +0100 Message-ID: <20191113161223.GJ4345@gilmour.lan> References: <20191110154101.26486-1-hdegoede@redhat.com> <20191110154101.26486-10-hdegoede@redhat.com> <20191111125358.GP4345@gilmour.lan> <7f0bf3c5-50cb-108a-fbb0-d3933b12643c@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0100438364==" Return-path: Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by gabe.freedesktop.org (Postfix) with ESMTPS id 307E96ED84 for ; Wed, 13 Nov 2019 16:12:27 +0000 (UTC) In-Reply-To: <7f0bf3c5-50cb-108a-fbb0-d3933b12643c@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hans de Goede Cc: David Airlie , dri-devel@lists.freedesktop.org, Sean Paul , Daniel Vetter , Mathieu =?iso-8859-1?Q?Alexandre-T=E9treault?= List-Id: dri-devel@lists.freedesktop.org --===============0100438364== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WetXT0Y5FME7H56p" Content-Disposition: inline --WetXT0Y5FME7H56p Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > 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 > > > --- > > > 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 --WetXT0Y5FME7H56p Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCXcwrZwAKCRDj7w1vZxhR xeidAP9TPZm/34px5rLfdL7cKtjqnWKthWcABmlvG6XXUlXpowEArHP27xtVnbq0 iRkBhr5mR1P/jXyTtEq/+zLfgRpP9As= =LGxq -----END PGP SIGNATURE----- --WetXT0Y5FME7H56p-- --===============0100438364== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============0100438364==--