[v2,04/12] drm/fb-helper: No need to cache rotation and sw_rotations

Commit Message

Noralf Trønnes April 7, 2019, 4:52 p.m. UTC

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 <[e-mail]>
Signed-off-by: Noralf Trønnes <[e-mail]>
Acked-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_fb_helper.c | 131 ++++++++++++++++----------------
 include/drm/drm_fb_helper.h     |   8 --
 2 files changed, 65 insertions(+), 74 deletions(-)

Comments

Maxime Ripard April 16, 2019, 9:28 a.m. UTC | #1

On Sun, Apr 07, 2019 at 06:52:35PM +0200, 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 <[e-mail]>
> Signed-off-by: Noralf Trønnes <[e-mail]>
> Acked-by: Daniel Vetter <[e-mail]>

Reviewed-by: Maxime Ripard <[e-mail]>

Maxime

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

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index eda8b63f350d..2f71160cc904 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -388,6 +388,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;
@@ -428,10 +471,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)
@@ -871,7 +917,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 struct drm_fb_helper_crtc *
 drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
 {
@@ -2801,7 +2790,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];
@@ -2821,7 +2809,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;
@@ -2844,11 +2831,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) {
@@ -2864,7 +2863,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 4db7de19a491..d954f48c9636 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;
 };
 
 /**
@@ -150,13 +149,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:
 	 *