* [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation @ 2017-04-23 16:11 Hans de Goede 2017-04-23 16:11 ` [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation Hans de Goede ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Hans de Goede @ 2017-04-23 16:11 UTC (permalink / raw) To: Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: Hans de Goede, intel-gfx, dri-devel, Bastien Nocera Hi All, So I recently bought a (second-hand) Bay Trail tablet which has its LCD mounted upside-down. As such I've ported Ville Syrjala's patches to deal with this to current mainline and I'm hereby posting them upstream for merging. These patches fix the kernel-console as well as the boot-splash (at leats plymouth does not reset the rotation) being upside down as soon as a native kms driver such as the i915 driver is loaded. This fixes the orientation of the displayed image from boot till the Xserver or a Wayland compositor takes over. I've a patch for iio-sensor-proxy which fixes the rotation under Xorg / Wayland when using a desktop environment which honors iio-sensor-proxy's rotation detection: https://github.com/hadess/iio-sensor-proxy/pull/162 Regards, Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation 2017-04-23 16:11 [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Hans de Goede @ 2017-04-23 16:11 ` Hans de Goede 2017-04-26 12:13 ` Bastien Nocera 2017-04-23 16:11 ` [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config Hans de Goede 2017-04-24 12:48 ` [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Ville Syrjälä 2 siblings, 1 reply; 14+ messages in thread From: Hans de Goede @ 2017-04-23 16:11 UTC (permalink / raw) To: Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: Hans de Goede, intel-gfx, dri-devel, Bastien Nocera From: Ville Syrjala <ville.syrjala@linux.intel.com> If a connector added through drm_fb_helper_add_one_connector() has a crtc attached and that crtc has a rotation configured make the fbdev inherit the crtc's rotation. This is useful on e.g. some tablets which have their lcd panel mounted upside down, which before this commit would result in the kernel boot messages switching from being shown the right way up in efifb to being shown upside down as soon as a native kms driver loads. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894 Cc: Ville Syrjala <ville.syrjala@linux.intel.com> [hdegoede@redhat.com: Split the drm/fb-helper bits out of Ville's "drm/fb-helper: Inherit rotation wip" patch] Tested-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/drm_fb_helper.c | 51 ++++++++++++++++++++++++++++++++++++++--- include/drm/drm_fb_helper.h | 2 ++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 324a688..c97e00ab 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ { struct drm_fb_helper_connector **temp; struct drm_fb_helper_connector *fb_helper_connector; + struct drm_crtc *crtc = connector->encoder ? + connector->encoder->crtc : NULL; if (!drm_fbdev_emulation) return 0; @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ drm_connector_reference(connector); fb_helper_connector->connector = connector; + if (crtc && crtc->primary->state) + fb_helper_connector->rotation = crtc->primary->state->rotation; + if (!fb_helper_connector->rotation) + fb_helper_connector->rotation = DRM_ROTATE_0; + fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector; return 0; } @@ -333,6 +340,35 @@ int drm_fb_helper_debug_leave(struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_debug_leave); +static int fbdev_plane_index(struct drm_fb_helper *fb_helper, + struct drm_plane *plane) +{ + int i; + + if (plane->type != DRM_PLANE_TYPE_PRIMARY) + return -ENODEV; + + for (i = 0; i < fb_helper->crtc_count; i++) { + struct drm_crtc *crtc = fb_helper->crtc_info[i].mode_set.crtc; + + if (crtc && crtc->primary == plane) + return i; + } + + return -ENODEV; +} + +static unsigned int fbdev_plane_rotation(struct drm_fb_helper *fb_helper, + struct drm_plane *plane) +{ + int i = fbdev_plane_index(fb_helper, plane); + + if (i < 0) + return DRM_ROTATE_0; + else + return fb_helper->crtc_info[i].rotation; +} + static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; @@ -357,7 +393,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper) goto fail; } - plane_state->rotation = DRM_ROTATE_0; + plane_state->rotation = fbdev_plane_rotation(fb_helper, plane); plane->old_fb = plane->fb; plane_mask |= 1 << drm_plane_index(plane); @@ -414,8 +450,8 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) if (plane->rotation_property) drm_mode_plane_set_obj_prop(plane, - plane->rotation_property, - DRM_ROTATE_0); + plane->rotation_property, + fbdev_plane_rotation(fb_helper, plane)); } for (i = 0; i < fb_helper->crtc_count; i++) { @@ -760,6 +796,7 @@ int drm_fb_helper_init(struct drm_device *dev, if (!fb_helper->crtc_info[i].mode_set.connectors) goto out_free; fb_helper->crtc_info[i].mode_set.num_connectors = 0; + fb_helper->crtc_info[i].rotation = DRM_ROTATE_0; } i = 0; @@ -2090,8 +2127,14 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, if (!drm_mode_equal(modes[o], modes[n])) continue; + + if (crtc->rotation && + crtc->rotation != fb_helper_conn->rotation) + continue; } + crtc->rotation = fb_helper_conn->rotation; + crtcs[n] = crtc; memcpy(crtcs, best_crtcs, n * sizeof(struct drm_fb_helper_crtc *)); score = my_score + drm_pick_crtcs(fb_helper, crtcs, modes, n + 1, @@ -2182,6 +2225,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; + fb_crtc->rotation = + fb_helper->connector_info[i]->rotation; modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); drm_connector_reference(connector); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 6f5aceb..19fc313 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -48,6 +48,7 @@ struct drm_fb_helper_crtc { struct drm_mode_set mode_set; struct drm_display_mode *desired_mode; int x, y; + uint8_t rotation; }; /** @@ -159,6 +160,7 @@ struct drm_fb_helper_funcs { struct drm_fb_helper_connector { struct drm_connector *connector; + uint8_t rotation; }; /** -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation 2017-04-23 16:11 ` [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation Hans de Goede @ 2017-04-26 12:13 ` Bastien Nocera 2017-04-30 19:22 ` Hans de Goede 0 siblings, 1 reply; 14+ messages in thread From: Bastien Nocera @ 2017-04-26 12:13 UTC (permalink / raw) To: Hans de Goede, Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: intel-gfx, dri-devel On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote: > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > If a connector added through drm_fb_helper_add_one_connector() has > a crtc attached and that crtc has a rotation configured make the > fbdev inherit the crtc's rotation. > > This is useful on e.g. some tablets which have their lcd panel > mounted > upside down, which before this commit would result in the kernel boot > messages switching from being shown the right way up in efifb to > being > shown upside down as soon as a native kms driver loads. > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894 > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > [hdegoede@redhat.com: Split the drm/fb-helper bits out of Ville's > "drm/fb-helper: Inherit rotation wip" patch] > Tested-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 51 > ++++++++++++++++++++++++++++++++++++++--- > include/drm/drm_fb_helper.h | 2 ++ > 2 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > b/drivers/gpu/drm/drm_fb_helper.c > index 324a688..c97e00ab 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct > drm_fb_helper *fb_helper, struct drm_ > { > struct drm_fb_helper_connector **temp; > struct drm_fb_helper_connector *fb_helper_connector; > + struct drm_crtc *crtc = connector->encoder ? > + connector->encoder->crtc : NULL; > > if (!drm_fbdev_emulation) > return 0; > @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct > drm_fb_helper *fb_helper, struct drm_ > > drm_connector_reference(connector); > fb_helper_connector->connector = connector; > + if (crtc && crtc->primary->state) > + fb_helper_connector->rotation = crtc->primary- > >state->rotation; > + if (!fb_helper_connector->rotation) > + fb_helper_connector->rotation = DRM_ROTATE_0; That's equivalent to: if (fb_helper_connector->rotation = DRM_ROTATE_0) fb_helper_connector->rotation = DRM_ROTATE_0; Maybe: if (crtc && crtc->primary->state) ... else ...->rotation = DRM_ROTATE_0; would be clearer? Or simply omit it if the connector is zero'ed. <snip> > diff --git a/include/drm/drm_fb_helper.h > b/include/drm/drm_fb_helper.h > index 6f5aceb..19fc313 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -48,6 +48,7 @@ struct drm_fb_helper_crtc { > struct drm_mode_set mode_set; > struct drm_display_mode *desired_mode; > int x, y; > + uint8_t rotation; > }; > > /** > @@ -159,6 +160,7 @@ struct drm_fb_helper_funcs { > > struct drm_fb_helper_connector { > struct drm_connector *connector; > + uint8_t rotation; In both those cases, I'd add a mention of the type of enum/mask etc. for the rotation, for example: uint8_t rotation; /* DRM_ROTATE_* */ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation 2017-04-26 12:13 ` Bastien Nocera @ 2017-04-30 19:22 ` Hans de Goede 2017-05-06 9:20 ` Bastien Nocera 0 siblings, 1 reply; 14+ messages in thread From: Hans de Goede @ 2017-04-30 19:22 UTC (permalink / raw) To: Bastien Nocera, Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: intel-gfx, dri-devel Hi, On 26-04-17 14:13, Bastien Nocera wrote: > On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote: >> From: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> If a connector added through drm_fb_helper_add_one_connector() has >> a crtc attached and that crtc has a rotation configured make the >> fbdev inherit the crtc's rotation. >> >> This is useful on e.g. some tablets which have their lcd panel >> mounted >> upside down, which before this commit would result in the kernel boot >> messages switching from being shown the right way up in efifb to >> being >> shown upside down as soon as a native kms driver loads. >> >> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894 >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> [hdegoede@redhat.com: Split the drm/fb-helper bits out of Ville's >> "drm/fb-helper: Inherit rotation wip" patch] >> Tested-by: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/gpu/drm/drm_fb_helper.c | 51 >> ++++++++++++++++++++++++++++++++++++++--- >> include/drm/drm_fb_helper.h | 2 ++ >> 2 files changed, 50 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c >> b/drivers/gpu/drm/drm_fb_helper.c >> index 324a688..c97e00ab 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct >> drm_fb_helper *fb_helper, struct drm_ >> { >> struct drm_fb_helper_connector **temp; >> struct drm_fb_helper_connector *fb_helper_connector; >> + struct drm_crtc *crtc = connector->encoder ? >> + connector->encoder->crtc : NULL; >> >> if (!drm_fbdev_emulation) >> return 0; >> @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct >> drm_fb_helper *fb_helper, struct drm_ >> >> drm_connector_reference(connector); >> fb_helper_connector->connector = connector; >> + if (crtc && crtc->primary->state) >> + fb_helper_connector->rotation = crtc->primary- >>> state->rotation; >> + if (!fb_helper_connector->rotation) >> + fb_helper_connector->rotation = DRM_ROTATE_0; > > That's equivalent to: > if (fb_helper_connector->rotation = DRM_ROTATE_0) > fb_helper_connector->rotation = DRM_ROTATE_0; No it is not equivalent: include/drm/drm_blend.h 40:#define DRM_ROTATE_0 BIT(0) and BIT(0) is (1 << 0) Regards, Hans > > Maybe: > if (crtc && crtc->primary->state) > ... > else > ...->rotation = DRM_ROTATE_0; > would be clearer? Or simply omit it if the connector is zero'ed. > > <snip> >> diff --git a/include/drm/drm_fb_helper.h >> b/include/drm/drm_fb_helper.h >> index 6f5aceb..19fc313 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -48,6 +48,7 @@ struct drm_fb_helper_crtc { >> struct drm_mode_set mode_set; >> struct drm_display_mode *desired_mode; >> int x, y; >> + uint8_t rotation; >> }; >> >> /** >> @@ -159,6 +160,7 @@ struct drm_fb_helper_funcs { >> >> struct drm_fb_helper_connector { >> struct drm_connector *connector; >> + uint8_t rotation; > > In both those cases, I'd add a mention of the type of enum/mask etc. > for the rotation, for example: > uint8_t rotation; /* DRM_ROTATE_* */ > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation 2017-04-30 19:22 ` Hans de Goede @ 2017-05-06 9:20 ` Bastien Nocera 0 siblings, 0 replies; 14+ messages in thread From: Bastien Nocera @ 2017-05-06 9:20 UTC (permalink / raw) To: Hans de Goede, Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: intel-gfx, dri-devel On Sun, 2017-04-30 at 21:22 +0200, Hans de Goede wrote: > Hi, > > On 26-04-17 14:13, Bastien Nocera wrote: > > On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote: > > > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > > > > > If a connector added through drm_fb_helper_add_one_connector() > > > has > > > a crtc attached and that crtc has a rotation configured make the > > > fbdev inherit the crtc's rotation. > > > > > > This is useful on e.g. some tablets which have their lcd panel > > > mounted > > > upside down, which before this commit would result in the kernel > > > boot > > > messages switching from being shown the right way up in efifb to > > > being > > > shown upside down as soon as a native kms driver loads. > > > > > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894 > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > [hdegoede@redhat.com: Split the drm/fb-helper bits out of Ville's > > > "drm/fb-helper: Inherit rotation wip" patch] > > > Tested-by: Hans de Goede <hdegoede@redhat.com> > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > --- > > > drivers/gpu/drm/drm_fb_helper.c | 51 > > > ++++++++++++++++++++++++++++++++++++++--- > > > include/drm/drm_fb_helper.h | 2 ++ > > > 2 files changed, 50 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > > b/drivers/gpu/drm/drm_fb_helper.c > > > index 324a688..c97e00ab 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct > > > drm_fb_helper *fb_helper, struct drm_ > > > { > > > struct drm_fb_helper_connector **temp; > > > struct drm_fb_helper_connector *fb_helper_connector; > > > + struct drm_crtc *crtc = connector->encoder ? > > > + connector->encoder->crtc : NULL; > > > > > > if (!drm_fbdev_emulation) > > > return 0; > > > @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct > > > drm_fb_helper *fb_helper, struct drm_ > > > > > > drm_connector_reference(connector); > > > fb_helper_connector->connector = connector; > > > + if (crtc && crtc->primary->state) > > > + fb_helper_connector->rotation = crtc->primary- > > > > state->rotation; > > > > > > + if (!fb_helper_connector->rotation) > > > + fb_helper_connector->rotation = DRM_ROTATE_0; > > > > That's equivalent to: > > if (fb_helper_connector->rotation = DRM_ROTATE_0) > > fb_helper_connector->rotation = DRM_ROTATE_0; > > No it is not equivalent: > > include/drm/drm_blend.h > 40:#define DRM_ROTATE_0 BIT(0) > > and BIT(0) is (1 << 0) Urgh. I'd have a "DRM_ROTATE_UNSET = 0" added either to the public header, or to this file and compare to it instead of "!rotation". Because this really wasn't clear or obvious. Cheers _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config 2017-04-23 16:11 [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Hans de Goede 2017-04-23 16:11 ` [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation Hans de Goede @ 2017-04-23 16:11 ` Hans de Goede 2017-04-26 12:14 ` Bastien Nocera 2017-04-24 12:48 ` [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Ville Syrjälä 2 siblings, 1 reply; 14+ messages in thread From: Hans de Goede @ 2017-04-23 16:11 UTC (permalink / raw) To: Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: Hans de Goede, intel-gfx, dri-devel, Bastien Nocera From: Ville Syrjala <ville.syrjala@linux.intel.com> When retrieving the initial settings / mode from the hardware also retrieve the initial rotation config. Together with "drm/fb-helper: Make fbdev inherit the crtc's rotation" this will make the fbdev inherit the initial rotation. This is useful on e.g. some tablets which have their lcd panel mounted upside down, which before this commit would result in the kernel boot messages switching from being shown the right way up in efifb to being shown upside down as soon as a native kms driver loads. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894 Cc: Ville Syrjala <ville.syrjala@linux.intel.com> [hdegoede@redhat.com: Split the intel_display bits out of Ville's "drm/fb-helper: Inherit rotation wip" patch] Tested-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ed1f4f2..5c9f504 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2821,6 +2821,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, plane_state->crtc_w = fb->width; plane_state->crtc_h = fb->height; + plane_state->rotation = plane_config->rotation; + intel_state->base.src = drm_plane_state_src(plane_state); intel_state->base.dst = drm_plane_state_dest(plane_state); @@ -8733,6 +8735,9 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, plane_config->tiling = I915_TILING_X; fb->modifier = I915_FORMAT_MOD_X_TILED; } + + if (val & DISPPLANE_ROTATE_180) + plane_config->rotation = DRM_ROTATE_180; } pixel_format = val & DISPPLANE_PIXFORMAT_MASK; @@ -9784,6 +9789,24 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, goto error; } + /* + * DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr + * while i915 HW rotation is clockwise, thats why this swapping. + */ + switch (val & PLANE_CTL_ROTATE_MASK) { + case PLANE_CTL_ROTATE_0: + break; + case PLANE_CTL_ROTATE_90: + plane_config->rotation = DRM_ROTATE_270; + break; + case PLANE_CTL_ROTATE_180: + plane_config->rotation = DRM_ROTATE_180; + break; + case PLANE_CTL_ROTATE_270: + plane_config->rotation = DRM_ROTATE_90; + break; + } + base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000; plane_config->base = base; @@ -9872,6 +9895,9 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, plane_config->tiling = I915_TILING_X; fb->modifier = I915_FORMAT_MOD_X_TILED; } + + if (val & DISPPLANE_ROTATE_180) + plane_config->rotation = DRM_ROTATE_180; } pixel_format = val & DISPPLANE_PIXFORMAT_MASK; @@ -16713,7 +16739,9 @@ int intel_modeset_init(struct drm_device *dev) drm_modeset_unlock_all(dev); for_each_intel_crtc(dev, crtc) { - struct intel_initial_plane_config plane_config = {}; + struct intel_initial_plane_config plane_config = { + .rotation = DRM_ROTATE_0 + }; if (!crtc->active) continue; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 344f238..63623dd 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -418,6 +418,7 @@ struct intel_initial_plane_config { unsigned int tiling; int size; u32 base; + uint8_t rotation; }; #define SKL_MIN_SRC_W 8 -- 2.9.3 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config 2017-04-23 16:11 ` [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config Hans de Goede @ 2017-04-26 12:14 ` Bastien Nocera 0 siblings, 0 replies; 14+ messages in thread From: Bastien Nocera @ 2017-04-26 12:14 UTC (permalink / raw) To: Hans de Goede, Daniel Vetter, Jani Nikula, Ville Syrjälä Cc: intel-gfx, dri-devel On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote: > From: Ville Syrjala <ville.syrjala@linux.intel.com> <snip> > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 344f238..63623dd 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -418,6 +418,7 @@ struct intel_initial_plane_config { > unsigned int tiling; > int size; > u32 base; > + uint8_t rotation; Mentioning what this is (DRM_ROTATE_* or something else) is even more important here. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-23 16:11 [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Hans de Goede 2017-04-23 16:11 ` [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation Hans de Goede 2017-04-23 16:11 ` [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config Hans de Goede @ 2017-04-24 12:48 ` Ville Syrjälä 2017-04-26 12:28 ` Bastien Nocera 2 siblings, 1 reply; 14+ messages in thread From: Ville Syrjälä @ 2017-04-24 12:48 UTC (permalink / raw) To: Hans de Goede; +Cc: dri-devel, Daniel Vetter, intel-gfx, Bastien Nocera On Sun, Apr 23, 2017 at 06:11:04PM +0200, Hans de Goede wrote: > Hi All, > > So I recently bought a (second-hand) Bay Trail tablet which has its LCD > mounted upside-down. As such I've ported Ville Syrjala's patches to deal > with this to current mainline and I'm hereby posting them upstream > for merging. > > These patches fix the kernel-console as well as the boot-splash > (at leats plymouth does not reset the rotation) being upside down as > soon as a native kms driver such as the i915 driver is loaded. > > This fixes the orientation of the displayed image from boot till the > Xserver or a Wayland compositor takes over. It doesn't work for X? Using -modesetting perhaps? IIRC it worked just fine with -intel way back when. > > I've a patch for iio-sensor-proxy which fixes the rotation under Xorg / > Wayland when using a desktop environment which honors iio-sensor-proxy's > rotation detection: > https://github.com/hadess/iio-sensor-proxy/pull/162 Or is it just this thing that clobbers what the DDX inherited from the kernel as the initial rotation? -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-24 12:48 ` [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Ville Syrjälä @ 2017-04-26 12:28 ` Bastien Nocera 2017-04-27 16:24 ` Ville Syrjälä 0 siblings, 1 reply; 14+ messages in thread From: Bastien Nocera @ 2017-04-26 12:28 UTC (permalink / raw) To: Ville Syrjälä, Hans de Goede Cc: Daniel Vetter, intel-gfx, dri-devel On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote: > <snip> > > I've a patch for iio-sensor-proxy which fixes the rotation under > > Xorg / > > Wayland when using a desktop environment which honors iio-sensor- > > proxy's > > rotation detection: > > https://github.com/hadess/iio-sensor-proxy/pull/162 > > Or is it just this thing that clobbers what the DDX inherited from > the > kernel as the initial rotation? I think it's mostly got to do with the compositor (or X) not knowing what "normal" or "0 degrees rotation" corresponds to. I would expect the baseline test for this wouldn't involve iio-sensor- proxy at all, and boot to the desktop with the expected orientation showing up in the desktop env/XRandR as "not rotated". Eg. for the GPD Win which does have this problem, this should appear as being rotated (image 1): https://c.slashgear.com/wp-content/uploads/2016/11/gpd-win-book-2.jpg But not this (image 2): https://c.slashgear.com/wp-content/uploads/2016/11/gpd-win-0-800x420.jpg After that, the problem is whether the accelerometer is mounted the same way as the "non-rotated screen" (option 2), or the "non-rotated screen" (option 1), which would show what quirking is needed. Cheers _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-26 12:28 ` Bastien Nocera @ 2017-04-27 16:24 ` Ville Syrjälä 2017-04-27 16:39 ` Bastien Nocera 0 siblings, 1 reply; 14+ messages in thread From: Ville Syrjälä @ 2017-04-27 16:24 UTC (permalink / raw) To: Bastien Nocera; +Cc: Hans de Goede, intel-gfx, dri-devel, Daniel Vetter On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote: > On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote: > > > <snip> > > > > I've a patch for iio-sensor-proxy which fixes the rotation under > > > Xorg / > > > Wayland when using a desktop environment which honors iio-sensor- > > > proxy's > > > rotation detection: > > > https://github.com/hadess/iio-sensor-proxy/pull/162 > > > > Or is it just this thing that clobbers what the DDX inherited from > > the > > kernel as the initial rotation? > > I think it's mostly got to do with the compositor (or X) not knowing > what "normal" or "0 degrees rotation" corresponds to. Well, there are really two cases to consider: 1. BIOS/whatever configures display hardware rotation in a way that matches the orientation of the physical display 2. BIOS didn't do that. Either the hardware can't do what would be required, or the BIOS just chose not to do it. Case 1 should work with these patches as long as the DDX will set up the initial randr rotation to match what it read out from the kms rotation property of the primary plane. Case 2 can't work without some mechanism to query the orientation of the display from the firmware/etc. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-27 16:24 ` Ville Syrjälä @ 2017-04-27 16:39 ` Bastien Nocera 2017-04-30 19:34 ` Hans de Goede 0 siblings, 1 reply; 14+ messages in thread From: Bastien Nocera @ 2017-04-27 16:39 UTC (permalink / raw) To: Ville Syrjälä Cc: Hans de Goede, intel-gfx, dri-devel, Daniel Vetter On Thu, 2017-04-27 at 19:24 +0300, Ville Syrjälä wrote: > On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote: > > On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote: > > > > > > > <snip> > > > > > > I've a patch for iio-sensor-proxy which fixes the rotation > > > > under > > > > Xorg / > > > > Wayland when using a desktop environment which honors iio- > > > > sensor- > > > > proxy's > > > > rotation detection: > > > > https://github.com/hadess/iio-sensor-proxy/pull/162 > > > > > > Or is it just this thing that clobbers what the DDX inherited > > > from > > > the > > > kernel as the initial rotation? > > > > I think it's mostly got to do with the compositor (or X) not > > knowing > > what "normal" or "0 degrees rotation" corresponds to. > > Well, there are really two cases to consider: > > 1. BIOS/whatever configures display hardware rotation in a way > that matches the orientation of the physical display > 2. BIOS didn't do that. Either the hardware can't do what > would be required, or the BIOS just chose not to do it. > > Case 1 should work with these patches as long as the DDX will set up > the > initial randr rotation to match what it read out from the kms > rotation > property of the primary plane. Yes. My problem was that instead of fixing the DDX to behave properly, reusing the same orientation as already configured, we were using iio- sensor-proxy to trigger the initial rotation. This doesn't work if there's no accelerometer, or orientation is locked, which is counter- intuitive. > Case 2 can't work without some mechanism to query the orientation > of the display from the firmware/etc. Yes. I'm not sure where we'd be exporting this quirk though, as we need it available early enough so that it can be used by boot splashes. DMI matches in the graphics driver? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-27 16:39 ` Bastien Nocera @ 2017-04-30 19:34 ` Hans de Goede 2017-05-06 9:22 ` Bastien Nocera 2017-05-07 9:05 ` Hans de Goede 0 siblings, 2 replies; 14+ messages in thread From: Hans de Goede @ 2017-04-30 19:34 UTC (permalink / raw) To: Bastien Nocera, Ville Syrjälä Cc: Daniel Vetter, intel-gfx, dri-devel Hi, On 27-04-17 18:39, Bastien Nocera wrote: > On Thu, 2017-04-27 at 19:24 +0300, Ville Syrjälä wrote: >> On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote: >>> On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote: >>>> >>> >>> <snip> >>> >>>>> I've a patch for iio-sensor-proxy which fixes the rotation >>>>> under >>>>> Xorg / >>>>> Wayland when using a desktop environment which honors iio- >>>>> sensor- >>>>> proxy's >>>>> rotation detection: >>>>> https://github.com/hadess/iio-sensor-proxy/pull/162 >>>> >>>> Or is it just this thing that clobbers what the DDX inherited >>>> from >>>> the >>>> kernel as the initial rotation? >>> >>> I think it's mostly got to do with the compositor (or X) not >>> knowing >>> what "normal" or "0 degrees rotation" corresponds to. >> >> Well, there are really two cases to consider: >> >> 1. BIOS/whatever configures display hardware rotation in a way >> that matches the orientation of the physical display >> 2. BIOS didn't do that. Either the hardware can't do what >> would be required, or the BIOS just chose not to do it. >> >> Case 1 should work with these patches as long as the DDX will set up >> the >> initial randr rotation to match what it read out from the kms >> rotation >> property of the primary plane. > > Yes. My problem was that instead of fixing the DDX to behave properly, > reusing the same orientation as already configured, we were using iio- > sensor-proxy to trigger the initial rotation. This doesn't work if > there's no accelerometer, or orientation is locked, which is counter- > intuitive. Right, so the iio-sensor-proxy approach was a quick fix attemp and I agree with you that with e.g. orientation locking breaking this it is not a good fix. Also note that with wayland we have no DDX and relying on reading the current primary plane rotation and assuming that that is the hw lcd panel orientation seems fragile in general, what if the xserver crashes while the display is rotated ? Then the next instance will assume that this is hardware panel orientation. >> Case 2 can't work without some mechanism to query the orientation >> of the display from the firmware/etc. > > Yes. I'm not sure where we'd be exporting this quirk though, as we need > it available early enough so that it can be used by boot splashes. DMI > matches in the graphics driver? Looking at what gnome currently does on both wayland and X11 AFAICT, I would like to solve both cases in the same manner in both cases we need the compositor to be aware that it needs to apply some rotation. Also taking into account thing like the monitor configuration panel I think we need to fix this at the compositor level by having some sort of "lcd-panel-hardware-rotation" property which gets added to the rotation selected by the user at the compositor level. As for where to store this for case 1 we have the info in the kernel from the firmware (once these patches land) so we just need to export it somehow. For case 2 we do need a dmi quirk in the kernel to set fbcon.rotate properly, and once we have it there we can export it in what is hopefully the same manner. Note that the GPD-win (which is an example of case 2) seems to be a rare example and case 1 is the more common case, so lets focus on fixing that first. It seems that these 2 patches get us quite far with fixing this, we just need to add some property (sysfs file?) to the drm_connector where the initial rotation as inherited from the firmware can be read by the compositor. Once we have that in place we can add a dmi quirk to set that property for e.g. the GPD-win and somehow propagate it to fbcon.rotate (case 2 needs to be done with software rotation in fbcon because the hw cannot handle all rotation cases). Regards, Hans _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-30 19:34 ` Hans de Goede @ 2017-05-06 9:22 ` Bastien Nocera 2017-05-07 9:05 ` Hans de Goede 1 sibling, 0 replies; 14+ messages in thread From: Bastien Nocera @ 2017-05-06 9:22 UTC (permalink / raw) To: Hans de Goede, Ville Syrjälä Cc: Daniel Vetter, intel-gfx, dri-devel On Sun, 2017-04-30 at 21:34 +0200, Hans de Goede wrote: > Hi, > > On 27-04-17 18:39, Bastien Nocera wrote: > > On Thu, 2017-04-27 at 19:24 +0300, Ville Syrjälä wrote: > > > On Wed, Apr 26, 2017 at 02:28:32PM +0200, Bastien Nocera wrote: > > > > On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote: > > > > > > > > > > > > > <snip> > > > > > > > > > > I've a patch for iio-sensor-proxy which fixes the rotation > > > > > > under > > > > > > Xorg / > > > > > > Wayland when using a desktop environment which honors iio- > > > > > > sensor- > > > > > > proxy's > > > > > > rotation detection: > > > > > > https://github.com/hadess/iio-sensor-proxy/pull/162 > > > > > > > > > > Or is it just this thing that clobbers what the DDX inherited > > > > > from > > > > > the > > > > > kernel as the initial rotation? > > > > > > > > I think it's mostly got to do with the compositor (or X) not > > > > knowing > > > > what "normal" or "0 degrees rotation" corresponds to. > > > > > > Well, there are really two cases to consider: > > > > > > 1. BIOS/whatever configures display hardware rotation in a way > > > that matches the orientation of the physical display > > > 2. BIOS didn't do that. Either the hardware can't do what > > > would be required, or the BIOS just chose not to do it. > > > > > > Case 1 should work with these patches as long as the DDX will set > > > up > > > the > > > initial randr rotation to match what it read out from the kms > > > rotation > > > property of the primary plane. > > > > > Yes. My problem was that instead of fixing the DDX to behave > > properly, > > reusing the same orientation as already configured, we were using > > iio- > > sensor-proxy to trigger the initial rotation. This doesn't work if > > there's no accelerometer, or orientation is locked, which is > > counter- > > intuitive. > > Right, so the iio-sensor-proxy approach was a quick fix attemp and > I agree with you that with e.g. orientation locking breaking this > it is not a good fix. > > Also note that with wayland we have no DDX and relying on reading > the current primary plane rotation and assuming that that is the > hw lcd panel orientation seems fragile in general, what if the > xserver crashes while the display is rotated ? Then the next instance > will assume that this is hardware panel orientation. > > > > Case 2 can't work without some mechanism to query the orientation > > > of the display from the firmware/etc. > > > > Yes. I'm not sure where we'd be exporting this quirk though, as we > > need > > it available early enough so that it can be used by boot splashes. > > DMI > > matches in the graphics driver? > > Looking at what gnome currently does on both wayland and X11 AFAICT, > I > would like to solve both cases in the same manner in both cases we > need the compositor to be aware that it needs to apply some rotation. > > Also taking into account thing like the monitor configuration panel > I think we need to fix this at the compositor level by having some > sort of "lcd-panel-hardware-rotation" property which gets added to > the rotation selected by the user at the compositor level. > > As for where to store this for case 1 we have the info in the kernel > from the firmware (once these patches land) so we just need to export > it somehow. > > For case 2 we do need a dmi quirk in the kernel to set fbcon.rotate > properly, and once we have it there we can export it in what is > hopefully the same manner. > > Note that the GPD-win (which is an example of case 2) seems to be > a rare example and case 1 is the more common case, so lets focus > on fixing that first. It seems that these 2 patches get us quite > far with fixing this, we just need to add some property (sysfs file?) > to the drm_connector where the initial rotation as inherited from > the firmware can be read by the compositor. Once we have that in > place we can add a dmi quirk to set that property for e.g. the > GPD-win and somehow propagate it to fbcon.rotate (case 2 needs to > be done with software rotation in fbcon because the hw cannot handle > all rotation cases). Sounds like a good plan! _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation 2017-04-30 19:34 ` Hans de Goede 2017-05-06 9:22 ` Bastien Nocera @ 2017-05-07 9:05 ` Hans de Goede 1 sibling, 0 replies; 14+ messages in thread From: Hans de Goede @ 2017-05-07 9:05 UTC (permalink / raw) To: Bastien Nocera, Ville Syrjälä Cc: Daniel Vetter, intel-gfx, dri-devel Hi, On 30-04-17 21:34, Hans de Goede wrote: > Hi, > > On 27-04-17 18:39, Bastien Nocera wrote: >> On Thu, 2017-04-27 at 19:24 +0300, Ville Syrjälä wrote: <snip> >>> Well, there are really two cases to consider: >>> >>> 1. BIOS/whatever configures display hardware rotation in a way >>> that matches the orientation of the physical display >>> 2. BIOS didn't do that. Either the hardware can't do what >>> would be required, or the BIOS just chose not to do it. >>> >>> Case 1 should work with these patches as long as the DDX will set up >>> the >>> initial randr rotation to match what it read out from the kms >>> rotation >>> property of the primary plane. > So I've been running some tests with the 2 original patches focussing only on the normal screen is upside down, BIOS has configured hardware rotation case which is what all devices except on special device seem to have. As expected with these 2 patches fbcon and the plymouth splashscreen work fine. If I then start: a) Xorg with modesetting driver: I get the screen the right way up, but the cursor plane is upside down both in coordinates (where the cursor gets shown) as well as the cursor glyph being upside down b) Xorg with intel driver, the intel driver re-sets the primary plane's drm "rotation" property, so everthing is upside down. At least the primary and cursor planes are consistent with each other now (the touchscreen coordinates how ever are not). c) Gnome as Wayland compositor, same as Xorg with intel driver So think more about this I think that Ville's 2 original cases really are: 1. All planes support the necessary rotation in hardware, so we can fix things up fully in hardware, aka the upside down case which I believe is the majority of all troublesome hardware out there. 2. The necessary rotation is not supported in hardware. This is the case with the GPD-win where we've a 90 / 270 degrees rotation. Here even the EFI configuration menu, grub, efifb, etc. all are rotated. My conclusion of all this is that: 1. For 1. we can and should fix this entirely and transparently in the kernel, we need kernel fixes for fbcon/fbdev anyways, so we might just as well go all the way, otherwise all of Xorg + modesetting, Xorg + intel and Gnome as Wayland compositor will need separate fixes. So this is best dealt with in the kernel, esp. since there are more Wayland compositors out there. 2. There is nothing the kernel can do here, so this one we really need to fix in userspace. I've some ideas for this, but those fall outside the scope of this discussion. I've already implemented the all kernel fix for 1. It is a single patch only touching the i915 driver, since if we transparently deal with the extra rotation in the i915 code we no longer need the drm_fb_helper changes as things will just work for the fbcon too, which is really nice to see and to me shows this is the right way to solve this. The patch adds just 80 lines of code and completely fixes this issue :) I'll send out the patch right after this mail. Regards, Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-05-07 9:05 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-23 16:11 [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Hans de Goede 2017-04-23 16:11 ` [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation Hans de Goede 2017-04-26 12:13 ` Bastien Nocera 2017-04-30 19:22 ` Hans de Goede 2017-05-06 9:20 ` Bastien Nocera 2017-04-23 16:11 ` [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config Hans de Goede 2017-04-26 12:14 ` Bastien Nocera 2017-04-24 12:48 ` [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation Ville Syrjälä 2017-04-26 12:28 ` Bastien Nocera 2017-04-27 16:24 ` Ville Syrjälä 2017-04-27 16:39 ` Bastien Nocera 2017-04-30 19:34 ` Hans de Goede 2017-05-06 9:22 ` Bastien Nocera 2017-05-07 9:05 ` Hans de Goede
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.