[RFC v4 03/25] drm/fb-helper: No need to cache rotation and sw_rotations

[Noralf Trønnes]

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 
Signed-off-by: Noralf Trønnes 
---
 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 0646b108030b..e48ace2d55f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -361,6 +361,48 @@ 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_connector *connector,
+struct drm_plane *plane,
+unsigned int *rotation)
+{
+   uint64_t valid_mask = 0;
+   unsigned int i;
+
+   if (!connector)
+   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;
@@ -406,10 +448,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 = 
_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->connectors[0], 
primary, )) {
+   /* 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)
@@ -841,7 +886,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;
@@ -2414,62 +2458,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, its 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:
-   

Re: [RFC v4 03/25] drm/fb-helper: No need to cache rotation and sw_rotations

[Hans de Goede]

Hi Noralf,

1 comment inline.

On 13-04-18 18:53, 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 
Signed-off-by: Noralf Trønnes 
---
  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 0646b108030b..e48ace2d55f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -361,6 +361,48 @@ 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_connector *connector,
+struct drm_plane *plane,
+unsigned int *rotation)
+{
+   uint64_t valid_mask = 0;
+   unsigned int i;
+
+   if (!connector)
+   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;
@@ -406,10 +448,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 = 
_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->connectors[0], primary, 
)) {
+   /* 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;
+   }


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.

Regards,

Hans




ret = __drm_atomic_helper_set_config(mode_set, state);
if (ret != 0)
@@ -841,7 +886,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;

@@ -2414,62 +2458,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, its 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 

[RFC v4 03/25] drm/fb-helper: No need to cache rotation and sw_rotations

[Noralf Trønnes]

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 
Signed-off-by: Noralf Trønnes 
---
 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 0646b108030b..e48ace2d55f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -361,6 +361,48 @@ 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_connector *connector,
+struct drm_plane *plane,
+unsigned int *rotation)
+{
+   uint64_t valid_mask = 0;
+   unsigned int i;
+
+   if (!connector)
+   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;
@@ -406,10 +448,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 = 
_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->connectors[0], 
primary, )) {
+   /* 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)
@@ -841,7 +886,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;
@@ -2414,62 +2458,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, its 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:
-   

[RFC v4 03/25] drm/fb-helper: No need to cache rotation and sw_rotations

[Noralf Trønnes]

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 
Signed-off-by: Noralf Trønnes 
---
 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 0646b108030b..e48ace2d55f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -361,6 +361,48 @@ 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_connector *connector,
+struct drm_plane *plane,
+unsigned int *rotation)
+{
+   uint64_t valid_mask = 0;
+   unsigned int i;
+
+   if (!connector)
+   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;
@@ -406,10 +448,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 = 
_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->connectors[0], 
primary, )) {
+   /* 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)
@@ -841,7 +886,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;
@@ -2414,62 +2458,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, its 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:
-