Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

From: Maxime Ripard <maxime.ripard@bootlin.com>
To: "Noralf Trønnes" <noralf@tronnes.org>
Cc: eben@raspberrypi.org, David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	dri-devel@lists.freedesktop.org,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Sean Paul <seanpaul@chromium.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline
Date: Wed, 17 Apr 2019 16:30:29 +0200	[thread overview]
Message-ID: <20190417143029.qhalxycshbrtdwvn@flea> (raw)
In-Reply-To: <6785d207-bcfb-6723-22be-039c2b99edd4@tronnes.org>

[-- Attachment #1.1: Type: text/plain, Size: 2480 bytes --]

Hi Noralf,

On Tue, Apr 16, 2019 at 04:50:00PM +0200, Noralf Trønnes wrote:
> Den 11.04.2019 15.22, skrev Maxime Ripard:
> > Rotations and reflections setup are needed in some scenarios to initialise
> > properly the initial framebuffer. Some drivers already had a bunch of
> > quirks to deal with this, such as either a private kernel command line
> > parameter (omapdss) or on the device tree (various panels).
> >
> > In order to accomodate this, let's create a video mode parameter to deal
> > with the rotation and reflexion.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c |   4 +-
> >  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
> >  include/drm/drm_connector.h     |   1 +-
> >  3 files changed, 95 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index b3a5d79436ae..8781897559b2 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >  				    struct drm_connector *connector)
> >  {
> >  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> > +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
> >  	uint64_t valid_mask = 0;
> >  	int i, rotation;
> >
> > @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >  		rotation = DRM_MODE_ROTATE_0;
> >  	}
> >
> > +	if (mode->rotation != DRM_MODE_ROTATE_0)
> > +		fb_crtc->rotation = mode->rotation;
> > +
>
> We already have a property to describe initial display/panel rotation.
> If we can set connector->display_info.panel_orientation from the video=
> parameter, then there's no need to modify drm_fb_helper, it will just work.
>
> In that case, maybe 'orientation' is a better argument name with values
> mapped to the enum.

I wouldn't put it at the same level though. As far as I understand it,
the orientation is a hardware constraint: the hardware has been
designed that way, and should honor that orientation to make it look
with the top, well, on top.

However, the rotation is more of a user choice, and you could
definitely envision having a combination of a rotation and an
orientation constraint.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel