From: "Noralf Trønnes" <noralf@tronnes.org> To: Maxime Ripard <maxime.ripard@bootlin.com> 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 v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline Date: Wed, 12 Jun 2019 15:21:19 +0200 [thread overview] Message-ID: <c73e9c29-e20b-65e0-553c-67b9c2cd349a@tronnes.org> (raw) In-Reply-To: <20190611132049.njlrgbtobzgyzyzh@flea> Den 11.06.2019 15.20, skrev Maxime Ripard: > Hi Noralf, > > On Fri, Apr 19, 2019 at 10:53:28AM +0200, Noralf Trønnes wrote: >> Den 18.04.2019 18.40, skrev Noralf Trønnes: >>> >>> >>> Den 18.04.2019 14.41, 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_client_modeset.c | 10 +++- >>>> drivers/gpu/drm/drm_modes.c | 110 ++++++++++++++++++++++------ >>>> include/drm/drm_connector.h | 9 ++- >>>> 3 files changed, 109 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c >>>> index f2869c82510c..15145d2c86d5 100644 >>>> --- a/drivers/gpu/drm/drm_client_modeset.c >>>> +++ b/drivers/gpu/drm/drm_client_modeset.c >>>> @@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe); >>>> bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation) >>>> { >>>> struct drm_connector *connector = modeset->connectors[0]; >>>> + struct drm_cmdline_mode *cmdline; >>>> struct drm_plane *plane = modeset->crtc->primary; >>>> u64 valid_mask = 0; >>>> unsigned int i; >>>> @@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotat >>>> *rotation = DRM_MODE_ROTATE_0; >>>> } >>>> >>>> + /** >>>> + * We want the rotation on the command line to overwrite >>>> + * whatever comes from the panel. >>>> + */ >>>> + cmdline = &connector->cmdline_mode; >>>> + if (cmdline->specified && >>>> + cmdline->rotation != DRM_MODE_ROTATE_0) >>> >>> I believe you need to drop that second check, otherwise rotate=0 will >>> not overwrite panel rotation. >>> >>>> + *rotation = cmdline->rotation; >> >> I remembered that you wanted this to propagate to DRM userspace. That's >> not happening here. > > It's propated to the userspace through the plane's rotation property, > I just checked. > Oh, so the rotation property stores its value in plane_state->rotation. And this is set in: drm_client_modeset_commit_atomic() -> drm_client_panel_rotation(): plane_state->rotation = rotation; >> The only way I see for that to happen, is to set >> ->panel_orientation. And to repeat myself, imo that makes >> 'orientation' a better name for this video= option. > > orientation and rotation are two different things to me. The > orientation of a panel for example is absolute, while the rotation is > a transformation. In this particular case, I think that both the > orientation and the rotation should be taken into account, with the > orientation being the default state, and the hardware / panel will > tell us that, while the rotation would be a transformation from that > default to whatever the user wants. > > More importantly, the orientation is a property of the hardware (ie, > how the display has been assembled), while the rotation is a software > construct. > > And if the property being used to expose that is the rotation, I guess > it would make sense to just use the same name and remain consistent. > Ok, I see. Based on this, I would assume that rotation would be relative to the orientation, but I see that in this patch rotation doesn't take orintation into account, it just overwrites it. I don't how userspace deals with rotation on top of orientation. Noralf. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Noralf Trønnes" <noralf@tronnes.org> To: Maxime Ripard <maxime.ripard@bootlin.com> Cc: eben@raspberrypi.org, David Airlie <airlied@linux.ie>, 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 v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline Date: Wed, 12 Jun 2019 15:21:19 +0200 [thread overview] Message-ID: <c73e9c29-e20b-65e0-553c-67b9c2cd349a@tronnes.org> (raw) In-Reply-To: <20190611132049.njlrgbtobzgyzyzh@flea> Den 11.06.2019 15.20, skrev Maxime Ripard: > Hi Noralf, > > On Fri, Apr 19, 2019 at 10:53:28AM +0200, Noralf Trønnes wrote: >> Den 18.04.2019 18.40, skrev Noralf Trønnes: >>> >>> >>> Den 18.04.2019 14.41, 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_client_modeset.c | 10 +++- >>>> drivers/gpu/drm/drm_modes.c | 110 ++++++++++++++++++++++------ >>>> include/drm/drm_connector.h | 9 ++- >>>> 3 files changed, 109 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c >>>> index f2869c82510c..15145d2c86d5 100644 >>>> --- a/drivers/gpu/drm/drm_client_modeset.c >>>> +++ b/drivers/gpu/drm/drm_client_modeset.c >>>> @@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe); >>>> bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation) >>>> { >>>> struct drm_connector *connector = modeset->connectors[0]; >>>> + struct drm_cmdline_mode *cmdline; >>>> struct drm_plane *plane = modeset->crtc->primary; >>>> u64 valid_mask = 0; >>>> unsigned int i; >>>> @@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotat >>>> *rotation = DRM_MODE_ROTATE_0; >>>> } >>>> >>>> + /** >>>> + * We want the rotation on the command line to overwrite >>>> + * whatever comes from the panel. >>>> + */ >>>> + cmdline = &connector->cmdline_mode; >>>> + if (cmdline->specified && >>>> + cmdline->rotation != DRM_MODE_ROTATE_0) >>> >>> I believe you need to drop that second check, otherwise rotate=0 will >>> not overwrite panel rotation. >>> >>>> + *rotation = cmdline->rotation; >> >> I remembered that you wanted this to propagate to DRM userspace. That's >> not happening here. > > It's propated to the userspace through the plane's rotation property, > I just checked. > Oh, so the rotation property stores its value in plane_state->rotation. And this is set in: drm_client_modeset_commit_atomic() -> drm_client_panel_rotation(): plane_state->rotation = rotation; >> The only way I see for that to happen, is to set >> ->panel_orientation. And to repeat myself, imo that makes >> 'orientation' a better name for this video= option. > > orientation and rotation are two different things to me. The > orientation of a panel for example is absolute, while the rotation is > a transformation. In this particular case, I think that both the > orientation and the rotation should be taken into account, with the > orientation being the default state, and the hardware / panel will > tell us that, while the rotation would be a transformation from that > default to whatever the user wants. > > More importantly, the orientation is a property of the hardware (ie, > how the display has been assembled), while the rotation is a software > construct. > > And if the property being used to expose that is the rotation, I guess > it would make sense to just use the same name and remain consistent. > Ok, I see. Based on this, I would assume that rotation would be relative to the orientation, but I see that in this patch rotation doesn't take orintation into account, it just overwrites it. I don't how userspace deals with rotation on top of orientation. Noralf. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-06-12 13:21 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-18 12:41 [PATCH v3 0/6] drm/vc4: Allow for more boot-time configuration Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 12:41 ` [PATCH v3 1/6] drm/modes: Rewrite the command line parser Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:11 ` Noralf Trønnes 2019-04-18 16:11 ` Noralf Trønnes 2019-04-18 12:41 ` [PATCH v3 2/6] drm/modes: Support modes names on the command line Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:22 ` Noralf Trønnes 2019-04-18 16:22 ` Noralf Trønnes 2019-04-18 12:41 ` [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:40 ` Noralf Trønnes 2019-04-18 16:40 ` Noralf Trønnes 2019-04-19 8:53 ` Noralf Trønnes 2019-04-19 8:53 ` Noralf Trønnes 2019-06-11 13:20 ` Maxime Ripard 2019-06-11 13:20 ` Maxime Ripard 2019-06-12 8:11 ` Jani Nikula 2019-06-12 8:11 ` Jani Nikula 2019-06-12 13:21 ` Noralf Trønnes [this message] 2019-06-12 13:21 ` Noralf Trønnes 2019-06-13 12:50 ` Maxime Ripard 2019-06-13 12:50 ` Maxime Ripard 2019-06-13 14:12 ` Noralf Trønnes 2019-06-13 14:12 ` Noralf Trønnes 2019-06-11 12:49 ` Maxime Ripard 2019-06-11 12:49 ` Maxime Ripard 2019-06-12 12:43 ` Noralf Trønnes 2019-06-12 12:43 ` Noralf Trønnes 2019-06-12 15:54 ` Maxime Ripard 2019-06-12 15:54 ` Maxime Ripard 2019-04-18 12:41 ` [PATCH v3 4/6] drm/modes: Parse overscan properties Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:50 ` Noralf Trønnes 2019-04-18 16:50 ` Noralf Trønnes 2019-04-19 9:05 ` Noralf Trønnes 2019-04-19 9:05 ` Noralf Trønnes 2019-04-18 12:41 ` [PATCH v3 5/6] drm/selftests: Add command line parser selftests Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:51 ` Noralf Trønnes 2019-04-18 16:51 ` Noralf Trønnes 2019-04-18 12:41 ` [PATCH v3 6/6] drm/vc4: hdmi: Set default state margin at reset Maxime Ripard 2019-04-18 12:41 ` Maxime Ripard 2019-04-18 16:59 ` Noralf Trønnes 2019-04-19 18:50 ` Noralf Trønnes 2019-04-19 18:50 ` Noralf Trønnes
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c73e9c29-e20b-65e0-553c-67b9c2cd349a@tronnes.org \ --to=noralf@tronnes.org \ --cc=airlied@linux.ie \ --cc=daniel.vetter@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=eben@raspberrypi.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=maarten.lankhorst@linux.intel.com \ --cc=maxime.ripard@bootlin.com \ --cc=paul.kocialkowski@bootlin.com \ --cc=seanpaul@chromium.org \ --cc=thomas.petazzoni@bootlin.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.