drm/fb: width/height cleanups/fixes

Flip conditional to reduce indentation level of rest of fxn, and use min/max to make the code clearer.

v2: surface_width -> surface_height typo

Signed-off-by Rob Clark <[email protected]>

Reviewed-by Daniel Kurtz <[email protected]>

---
 drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

(quoted diff) +0 -0

b/drivers/gpu/drm/drm_fb_helper.c index 1e6a0c7..dca98a4 100644

drivers/gpu/drm/drm_fb_helper.c +14 -13

@@ -1035,22 +1035,24 @@ static int drm_fb_helper_single_fb_probe(struct
 drm_fb_helper *fb_helper, for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
 		int x, y;
+
 		desired_mode = fb_helper->crtc_info[i].desired_mode;
+
+		if (!desired_mode)
+			continue;
+
+		crtc_count++;
+
 		x = fb_helper->crtc_info[i].x;
 		y = fb_helper->crtc_info[i].y;
-		if (desired_mode) {
-			if (gamma_size == 0)
-				gamma_size = fb_helper->crtc_info[i].mode_set.crtc-
 gamma_size;
-			if (desired_mode->hdisplay + x < sizes.fb_width)
-				sizes.fb_width = desired_mode->hdisplay + x;
-			if (desired_mode->vdisplay + y < sizes.fb_height)
-				sizes.fb_height = desired_mode->vdisplay + y;
-			if (desired_mode->hdisplay + x > sizes.surface_width)
-				sizes.surface_width = desired_mode->hdisplay + x;
-			if (desired_mode->vdisplay + y > sizes.surface_height)
-				sizes.surface_height = desired_mode->vdisplay + y;
-			crtc_count++;
-		}
+
+		if (gamma_size == 0)
+			gamma_size = fb_helper->crtc_info[i].mode_set.crtc->gamma_size;
+
+		sizes.surface_width  = max_t(u32, desired_mode->hdisplay + x,
 sizes.surface_width);
+		sizes.surface_height = max_t(u32, desired_mode->vdisplay + y,
 sizes.surface_height);
+		sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x,

sizes.fb_width);

drivers/gpu/drm/drm_fb_helper.c +1 -0

+		sizes.fb_height = min_t(u32, desired_mode->vdisplay + y,

sizes.fb_height); }

if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {

We don't want tile 0,0 to artificially constrain the size of the legacy fbdev device. Instead when reducing fb_size to be the minimum of all displays, only consider the rightmost and bottommost tiles.

Signed-off-by Rob Clark <[email protected]>

Tested-by Hai Li <[email protected]>

---
 drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

drivers/gpu/drm/drm_fb_helper.c +23 -3

index dca98a4..1a20db7 100644
@@ -1034,9 +1034,16 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	crtc_count = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
-		int x, y;
+		struct drm_mode_set *mode_set;
+		int x, y, j;
+		/* in case of tile group, are we the last tile vert or horiz?
+		 * If no tile group you are always the last one both vertically
+		 * and horizontally
+		 */
+		bool lastv = true, lasth = true;
 
 		desired_mode = fb_helper->crtc_info[i].desired_mode;
+		mode_set = &fb_helper->crtc_info[i].mode_set;
 
 		if (!desired_mode)
 			continue;

@@ -1051,8 +1058,21 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 
 		sizes.surface_width  = max_t(u32, desired_mode->hdisplay + x, sizes.surface_width);
 		sizes.surface_height = max_t(u32, desired_mode->vdisplay + y, sizes.surface_height);
-		sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
-		sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
+
+		for (j = 0; j < mode_set->num_connectors; j++) {
+			struct drm_connector *connector = mode_set->connectors[j];
+			if (connector->has_tile) {
+				lasth = (connector->tile_h_loc == (connector->num_h_tile - 1));
+				lastv = (connector->tile_v_loc == (connector->num_v_tile - 1));
+				/* cloning to multiple tiles is just crazy-talk, so: */
+				break;
+			}
+		}
+
+		if (lasth)
+			sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
+		if (lastv)
+			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
 	}
 
 	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {

Signature · 1 line

2.1.0

Quoted 4 lines · Rob Clark

We don't want tile 0,0 to artificially constrain the size of the legacy fbdev device. Instead when reducing fb_size to be the minimum of all displays, only consider the rightmost and bottommost tiles.

Quoted 2 lines

Signed-off-by Rob Clark <[email protected]>

Tested-by Hai Li <[email protected]>

Yeah checkpatch isn't really happy about this and the previous one now, but I didn't really see a easy way to fix it and it's late ;-) So pulled them all into drm-misc.

Thanks, Daniel

Quoted 49 lines

---
 drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

drivers/gpu/drm/drm_fb_helper.c +23 -3

index dca98a4..1a20db7 100644
@@ -1034,9 +1034,16 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
      crtc_count = 0;
      for (i = 0; i < fb_helper->crtc_count; i++) {
              struct drm_display_mode *desired_mode;
-             int x, y;
+             struct drm_mode_set *mode_set;
+             int x, y, j;
+             /* in case of tile group, are we the last tile vert or horiz?
+              * If no tile group you are always the last one both vertically
+              * and horizontally
+              */
+             bool lastv = true, lasth = true;
 
              desired_mode = fb_helper->crtc_info[i].desired_mode;
+             mode_set = &fb_helper->crtc_info[i].mode_set;
 
              if (!desired_mode)
                      continue;

@@ -1051,8 +1058,21 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 
              sizes.surface_width  = max_t(u32, desired_mode->hdisplay + x, sizes.surface_width);
              sizes.surface_height = max_t(u32, desired_mode->vdisplay + y, sizes.surface_height);
-             sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
-             sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
+
+             for (j = 0; j < mode_set->num_connectors; j++) {
+                     struct drm_connector *connector = mode_set->connectors[j];
+                     if (connector->has_tile) {
+                             lasth = (connector->tile_h_loc == (connector->num_h_tile - 1));
+                             lastv = (connector->tile_v_loc == (connector->num_v_tile - 1));
+                             /* cloning to multiple tiles is just crazy-talk, so: */
+                             break;
+                     }
+             }
+
+             if (lasth)
+                     sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
+             if (lastv)
+                     sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
      }
 

Quoted 4 lines

drivers/gpu/drm/drm_fb_helper.c +0 -1

      if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
--

2.1.0

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

In the process of fixing fb_width/fb_height problems with tiled displays (such as DP MST), I realized there were a few drivers having confusion about drm_fb_helper_surface_size.

So, document the struct, fix the drivers, simplify the logic in drm_fb_helper_single_fb_probe() that figures out the sizes, and finally fix the logic for tiled displays.

Oh and a minor completely unrelated kerneldoc fix for a typo that I noticed.

Rob Clark (7):
drm/fb: document drm_fb_helper_surface_size
drm/atomic: minor kerneldoc typo fix
drm/cma: use correct fb width/height
drm/exynos: use correct fb width/height
drm/rockchip: use correct fb width/height
drm/fb: small cleanup
drm/fb: handle tiled connectors better

 drivers/gpu/drm/drm_fb_cma_helper.c           |  2 +-
 drivers/gpu/drm/drm_fb_helper.c               | 48 +++++++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c     |  5 +--
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c |  2 +-
 include/drm/drm_crtc.h                        |  2 +-
 include/drm/drm_fb_helper.h                   | 19 +++++++++++
 6 files changed, 60 insertions(+), 18 deletions(-)

--
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

In the process of fixing fb_width/fb_height problems with tiled displays (such as DP MST), I realized there were a few drivers having confusion about drm_fb_helper_surface_size.

So, document the struct, fix the drivers, simplify the logic in drm_fb_helper_single_fb_probe() that figures out the sizes, and finally fix the logic for tiled displays.

Oh and a minor completely unrelated kerneldoc fix for a typo that I noticed.

Rob Clark (7):
drm/fb: document drm_fb_helper_surface_size
drm/atomic: minor kerneldoc typo fix
drm/cma: use correct fb width/height
drm/exynos: use correct fb width/height
drm/rockchip: use correct fb width/height
drm/fb: small cleanup
drm/fb: handle tiled connectors better

 drivers/gpu/drm/drm_fb_cma_helper.c           |  2 +-
 drivers/gpu/drm/drm_fb_helper.c               | 48 ++++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c     |  5 +--
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c |  2 +-
 include/drm/drm_crtc.h                        |  2 +-
 include/drm/drm_fb_helper.h                   | 19 +++++++++++
 6 files changed, 60 insertions(+), 18 deletions(-)