From: Daniel Vetter <daniel@ffwll.ch>
To: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
intel-gfx@lists.freedesktop.org, mstaudt@suse.de,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 04/16] drm/fb-helper: No need to cache rotation and sw_rotations
Date: Thu, 28 Mar 2019 09:03:39 +0100 [thread overview]
Message-ID: <20190328080339.GS2665@phenom.ffwll.local> (raw)
In-Reply-To: <20190326175546.18126-5-noralf@tronnes.org>
On Tue, Mar 26, 2019 at 06:55:34PM +0100, Noralf Trønnes wrote:
> Getting rotation info is cheap so we can do it on demand.
>
> This is done in preparation for the removal of struct drm_fb_helper_crtc.
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>
> Hans,
>
> You had this comment inline in restore_fbdev_mode_atomic() the last time
> I sent this out:
>
> We want plane_state->rotation to be set to DRM_MODE_ROTATE_0 in the else
> case, AFAIK new_plane_state starts with the current state and rotation
> may have a different value there.
>
> Otherwise this looks good to me.
>
> Rotation is reset for each plane in the code section above the one I'm
> changing.
I think best to let Hans review/test this in detail. lgtm at a glance.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Noralf.
>
> drivers/gpu/drm/drm_fb_helper.c | 131 ++++++++++++++++----------------
> include/drm/drm_fb_helper.h | 8 --
> 2 files changed, 65 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b91df658db59..e1b147fdd3f9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -387,6 +387,49 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>
> +/* Check if the plane can hw rotate to match panel orientation */
> +static bool drm_fb_helper_panel_rotation(struct drm_mode_set *modeset,
> + unsigned int *rotation)
> +{
> + struct drm_connector *connector = modeset->connectors[0];
> + struct drm_plane *plane = modeset->crtc->primary;
> + u64 valid_mask = 0;
> + unsigned int i;
> +
> + if (!modeset->num_connectors)
> + return false;
> +
> + switch (connector->display_info.panel_orientation) {
> + case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> + *rotation = DRM_MODE_ROTATE_180;
> + break;
> + case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> + *rotation = DRM_MODE_ROTATE_90;
> + break;
> + case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> + *rotation = DRM_MODE_ROTATE_270;
> + break;
> + default:
> + *rotation = DRM_MODE_ROTATE_0;
> + }
> +
> + /*
> + * TODO: support 90 / 270 degree hardware rotation,
> + * depending on the hardware this may require the framebuffer
> + * to be in a specific tiling format.
> + */
> + if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> + return false;
> +
> + for (i = 0; i < plane->rotation_property->num_values; i++)
> + valid_mask |= (1ULL << plane->rotation_property->values[i]);
> +
> + if (!(*rotation & valid_mask))
> + return false;
> +
> + return true;
> +}
> +
> static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
> {
> struct drm_device *dev = fb_helper->dev;
> @@ -427,10 +470,13 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
> for (i = 0; i < fb_helper->crtc_count; i++) {
> struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> struct drm_plane *primary = mode_set->crtc->primary;
> + unsigned int rotation;
>
> - /* Cannot fail as we've already gotten the plane state above */
> - plane_state = drm_atomic_get_new_plane_state(state, primary);
> - plane_state->rotation = fb_helper->crtc_info[i].rotation;
> + if (drm_fb_helper_panel_rotation(mode_set, &rotation)) {
> + /* Cannot fail as we've already gotten the plane state above */
> + plane_state = drm_atomic_get_new_plane_state(state, primary);
> + plane_state->rotation = rotation;
> + }
>
> ret = __drm_atomic_helper_set_config(mode_set, state);
> if (ret != 0)
> @@ -881,7 +927,6 @@ 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_MODE_ROTATE_0;
> }
>
> i = 0;
> @@ -2500,62 +2545,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> return best_score;
> }
>
> -/*
> - * This function checks if rotation is necessary because of panel orientation
> - * and if it is, if it is supported.
> - * If rotation is necessary and supported, it gets set in fb_crtc.rotation.
> - * If rotation is necessary but not supported, a DRM_MODE_ROTATE_* flag gets
> - * or-ed into fb_helper->sw_rotations. In drm_setup_crtcs_fb() we check if only
> - * one bit is set and then we set fb_info.fbcon_rotate_hint to make fbcon do
> - * the unsupported rotation.
> - */
> -static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> - struct drm_fb_helper_crtc *fb_crtc,
> - struct drm_connector *connector)
> -{
> - struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> - uint64_t valid_mask = 0;
> - int i, rotation;
> -
> - fb_crtc->rotation = DRM_MODE_ROTATE_0;
> -
> - switch (connector->display_info.panel_orientation) {
> - case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> - rotation = DRM_MODE_ROTATE_180;
> - break;
> - case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> - rotation = DRM_MODE_ROTATE_90;
> - break;
> - case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> - rotation = DRM_MODE_ROTATE_270;
> - break;
> - default:
> - rotation = DRM_MODE_ROTATE_0;
> - }
> -
> - /*
> - * TODO: support 90 / 270 degree hardware rotation,
> - * depending on the hardware this may require the framebuffer
> - * to be in a specific tiling format.
> - */
> - if (rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property) {
> - fb_helper->sw_rotations |= rotation;
> - return;
> - }
> -
> - for (i = 0; i < plane->rotation_property->num_values; i++)
> - valid_mask |= (1ULL << plane->rotation_property->values[i]);
> -
> - if (!(rotation & valid_mask)) {
> - fb_helper->sw_rotations |= rotation;
> - return;
> - }
> -
> - fb_crtc->rotation = rotation;
> - /* Rotating in hardware, fbcon should not rotate */
> - fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
> -}
> -
> static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> u32 width, u32 height)
> {
> @@ -2615,7 +2604,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> drm_fb_helper_modeset_release(fb_helper,
> &fb_helper->crtc_info[i].mode_set);
>
> - fb_helper->sw_rotations = 0;
> drm_fb_helper_for_each_connector(fb_helper, i) {
> struct drm_display_mode *mode = modes[i];
> struct drm_fb_helper_crtc *fb_crtc = crtcs[i];
> @@ -2635,7 +2623,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> modeset->mode = drm_mode_duplicate(dev,
> fb_crtc->desired_mode);
> drm_connector_get(connector);
> - drm_setup_crtc_rotation(fb_helper, fb_crtc, connector);
> modeset->connectors[modeset->num_connectors++] = connector;
> modeset->x = offset->x;
> modeset->y = offset->y;
> @@ -2658,11 +2645,23 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
> {
> struct fb_info *info = fb_helper->fbdev;
> + unsigned int rotation, sw_rotations = 0;
> int i;
>
> - for (i = 0; i < fb_helper->crtc_count; i++)
> - if (fb_helper->crtc_info[i].mode_set.num_connectors)
> - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + struct drm_mode_set *modeset = &fb_helper->crtc_info[i].mode_set;
> +
> + if (!modeset->num_connectors)
> + continue;
> +
> + modeset->fb = fb_helper->fb;
> +
> + if (drm_fb_helper_panel_rotation(modeset, &rotation))
> + /* Rotating in hardware, fbcon should not rotate */
> + sw_rotations |= DRM_MODE_ROTATE_0;
> + else
> + sw_rotations |= rotation;
> + }
>
> mutex_lock(&fb_helper->dev->mode_config.mutex);
> drm_fb_helper_for_each_connector(fb_helper, i) {
> @@ -2678,7 +2677,7 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
> }
> mutex_unlock(&fb_helper->dev->mode_config.mutex);
>
> - switch (fb_helper->sw_rotations) {
> + switch (sw_rotations) {
> case DRM_MODE_ROTATE_0:
> info->fbcon_rotate_hint = FB_ROTATE_UR;
> break;
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index bb9acea61369..cff1aa222886 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -50,7 +50,6 @@ struct drm_fb_helper_crtc {
> struct drm_mode_set mode_set;
> struct drm_display_mode *desired_mode;
> int x, y;
> - int rotation;
> };
>
> /**
> @@ -175,13 +174,6 @@ struct drm_fb_helper {
> struct drm_fb_helper_crtc *crtc_info;
> int connector_count;
> int connector_info_alloc_count;
> - /**
> - * @sw_rotations:
> - * Bitmask of all rotations requested for panel-orientation which
> - * could not be handled in hardware. If only one bit is set
> - * fbdev->fbcon_rotate_hint gets set to the requested rotation.
> - */
> - int sw_rotations;
> /**
> * @connector_info:
> *
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-03-28 8:03 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-26 17:55 [PATCH 00/16] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
2019-03-26 17:55 ` [PATCH 01/16] drm/fb-helper: Remove unused gamma_size variable Noralf Trønnes
2019-03-26 18:23 ` Daniel Vetter
2019-04-03 9:19 ` [Intel-gfx] " Noralf Trønnes
2019-03-26 17:55 ` [PATCH 02/16] drm/fb-helper: dpms_legacy(): Only set on connectors in use Noralf Trønnes
2019-03-26 18:26 ` Daniel Vetter
2019-03-26 17:55 ` [PATCH 03/16] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config() Noralf Trønnes
2019-03-26 20:48 ` Daniel Vetter
2019-03-27 13:41 ` [Intel-gfx] " Noralf Trønnes
2019-03-27 13:55 ` Daniel Vetter
2019-03-27 14:27 ` [Intel-gfx] " Noralf Trønnes
2019-03-26 17:55 ` [PATCH 04/16] drm/fb-helper: No need to cache rotation and sw_rotations Noralf Trønnes
2019-03-28 8:03 ` Daniel Vetter [this message]
2019-03-26 17:55 ` [PATCH 05/16] drm/fb-helper: Remove drm_fb_helper_crtc->{x, y, desired_mode} Noralf Trønnes
2019-03-28 8:19 ` Daniel Vetter
2019-03-26 17:55 ` [PATCH 06/16] drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper Noralf Trønnes
2019-03-27 13:33 ` Jani Nikula
2019-03-30 20:50 ` Noralf Trønnes
2019-03-26 17:55 ` [PATCH 07/16] drm/fb-helper: Remove drm_fb_helper_crtc Noralf Trønnes
2019-03-26 17:55 ` [PATCH 08/16] drm/fb-helper: Prepare to move out commit code Noralf Trønnes
2019-03-26 17:55 ` [PATCH 09/16] drm/fb-helper: Move " Noralf Trønnes
2019-03-27 14:13 ` Emmanuel Vadot
2019-03-27 15:01 ` Noralf Trønnes
2019-03-27 15:12 ` Emmanuel Vadot
2019-03-26 17:55 ` [PATCH 10/16] drm/fb-helper: Remove drm_fb_helper_connector Noralf Trønnes
2019-03-26 17:55 ` [PATCH 11/16] drm/fb-helper: Prepare to move out modeset config code Noralf Trønnes
2019-03-26 17:55 ` [PATCH 12/16] drm/fb-helper: Move " Noralf Trønnes
2019-03-26 17:55 ` [PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
2019-03-28 8:17 ` Daniel Vetter
2019-03-30 21:07 ` Noralf Trønnes
2019-04-01 7:12 ` Daniel Vetter
2019-04-01 13:31 ` Noralf Trønnes
2019-03-26 17:55 ` [PATCH 14/16] drm/client: Add display abstraction Noralf Trønnes
2019-03-26 17:55 ` [PATCH 15/16] drm/client: Hack: Add bootsplash example Noralf Trønnes
2019-03-26 17:55 ` [PATCH 16/16] drm/vc4: Call drm_dev_register() after all setup is done Noralf Trønnes
2019-03-26 20:40 ` Daniel Vetter
2019-03-27 16:36 ` Eric Anholt
2019-04-03 9:20 ` Noralf Trønnes
2019-03-27 13:45 ` [PATCH 00/16] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
2019-03-27 13:52 ` Thomas Zimmermann
2019-03-28 9:31 ` [Intel-gfx] " Daniel Vetter
2019-03-31 21:18 ` 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=20190328080339.GS2665@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hdegoede@redhat.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=mstaudt@suse.de \
--cc=noralf@tronnes.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).