All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] drm/fb: width/height cleanups/fixes
@ 2015-03-11 14:23 Rob Clark
  2015-03-11 14:23 ` [PATCH v2 1/7] drm/fb: document drm_fb_helper_surface_size Rob Clark
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

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
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/7] drm/fb: document drm_fb_helper_surface_size
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 14:23 ` [PATCH v2 2/7] drm/atomic: minor kerneldoc typo fix Rob Clark
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

There has been some confusion about this struct.  Lack of documentation
probably didn't help.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/drm/drm_fb_helper.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 21b944c..0dfd94def 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -44,6 +44,25 @@ struct drm_fb_helper_crtc {
 	int x, y;
 };
 
+/**
+ * struct drm_fb_helper_surface_size - describes fbdev size and scanout surface size
+ * @fb_width: fbdev width
+ * @fb_height: fbdev height
+ * @surface_width: scanout buffer width
+ * @surface_height: scanout buffer height
+ * @surface_bpp: scanout buffer bpp
+ * @surface_depth: scanout buffer depth
+ *
+ * Note that the scanout surface width/height may be larger than the fbdev
+ * width/height.  In case of multiple displays, the scanout surface is sized
+ * according to the largest width/height (so it is large enough for all CRTCs
+ * to scanout).  But the fbdev width/height is sized to the minimum width/
+ * height of all the displays.  This ensures that fbcon fits on the smallest
+ * of the attached displays.
+ *
+ * So what is passed to drm_fb_helper_fill_var() should be fb_width/fb_height,
+ * rather than the surface size.
+ */
 struct drm_fb_helper_surface_size {
 	u32 fb_width;
 	u32 fb_height;
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/7] drm/atomic: minor kerneldoc typo fix
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
  2015-03-11 14:23 ` [PATCH v2 1/7] drm/fb: document drm_fb_helper_surface_size Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 14:23 ` [PATCH v2 3/7] drm/cma: use correct fb width/height Rob Clark
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/drm/drm_crtc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index adc9ea5..7b5c661 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -915,7 +915,7 @@ struct drm_bridge {
 };
 
 /**
- * struct struct drm_atomic_state - the global state object for atomic updates
+ * struct drm_atomic_state - the global state object for atomic updates
  * @dev: parent DRM device
  * @allow_modeset: allow full modeset
  * @legacy_cursor_update: hint to enforce legacy cursor ioctl semantics
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/7] drm/cma: use correct fb width/height
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
  2015-03-11 14:23 ` [PATCH v2 1/7] drm/fb: document drm_fb_helper_surface_size Rob Clark
  2015-03-11 14:23 ` [PATCH v2 2/7] drm/atomic: minor kerneldoc typo fix Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 14:23 ` [PATCH v2 4/7] drm/exynos: " Rob Clark
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

What is passed to drm_fb_helper_fill_var() should be fb_width/fb_height,
rather than the surface size.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index cc0ae04..5c1aca4 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -304,7 +304,7 @@ static int drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	}
 
 	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth);
-	drm_fb_helper_fill_var(fbi, helper, fb->width, fb->height);
+	drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
 
 	offset = fbi->var.xoffset * bytes_per_pixel;
 	offset += fbi->var.yoffset * fb->pitches[0];
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/7] drm/exynos: use correct fb width/height
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (2 preceding siblings ...)
  2015-03-11 14:23 ` [PATCH v2 3/7] drm/cma: use correct fb width/height Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 14:23 ` [PATCH v2 5/7] drm/rockchip: " Rob Clark
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

What is passed to drm_fb_helper_fill_var() should be fb_width/fb_height,
rather than the surface size.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 84f8dfe..e71e331 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -76,6 +76,7 @@ static struct fb_ops exynos_drm_fb_ops = {
 };
 
 static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
+				     struct drm_fb_helper_surface_size *sizes,
 				     struct drm_framebuffer *fb)
 {
 	struct fb_info *fbi = helper->fbdev;
@@ -85,7 +86,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 	unsigned long offset;
 
 	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth);
-	drm_fb_helper_fill_var(fbi, helper, fb->width, fb->height);
+	drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
 
 	/* RGB formats use only one buffer */
 	buffer = exynos_drm_fb_buffer(fb, 0);
@@ -189,7 +190,7 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 		goto err_destroy_framebuffer;
 	}
 
-	ret = exynos_drm_fbdev_update(helper, helper->fb);
+	ret = exynos_drm_fbdev_update(helper, sizes, helper->fb);
 	if (ret < 0)
 		goto err_dealloc_cmap;
 
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/7] drm/rockchip: use correct fb width/height
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (3 preceding siblings ...)
  2015-03-11 14:23 ` [PATCH v2 4/7] drm/exynos: " Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 14:23 ` [PATCH v2 6/7] drm/fb: small cleanup Rob Clark
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

What is passed to drm_fb_helper_fill_var() should be fb_width/fb_height,
rather than the surface size.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
index a5d889a8..ff04877 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
@@ -106,7 +106,7 @@ static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
 
 	fb = helper->fb;
 	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth);
-	drm_fb_helper_fill_var(fbi, helper, fb->width, fb->height);
+	drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
 
 	offset = fbi->var.xoffset * bytes_per_pixel;
 	offset += fbi->var.yoffset * fb->pitches[0];
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/7] drm/fb: small cleanup
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (4 preceding siblings ...)
  2015-03-11 14:23 ` [PATCH v2 5/7] drm/rockchip: " Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 17:51   ` Laurent Pinchart
  2015-03-11 14:23 ` [PATCH v2 7/7] drm/fb: handle tiled connectors better Rob Clark
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

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 <robdclark@gmail.com>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1e6a0c7..dca98a4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -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);
+		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) {
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 7/7] drm/fb: handle tiled connectors better
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (5 preceding siblings ...)
  2015-03-11 14:23 ` [PATCH v2 6/7] drm/fb: small cleanup Rob Clark
@ 2015-03-11 14:23 ` Rob Clark
  2015-03-11 21:13   ` Daniel Vetter
  2015-03-11 16:40 ` [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Alex Deucher
  2015-03-11 17:52 ` Laurent Pinchart
  8 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-03-11 14:23 UTC (permalink / raw)
  To: dri-devel; +Cc: Laurent Pinchart, Andy Yan

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 <robdclark@gmail.com>
Tested-by: Hai Li <hali@codeaurora.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index dca98a4..1a20db7 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -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) {
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/7] drm/fb: width/height cleanups/fixes
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (6 preceding siblings ...)
  2015-03-11 14:23 ` [PATCH v2 7/7] drm/fb: handle tiled connectors better Rob Clark
@ 2015-03-11 16:40 ` Alex Deucher
  2015-03-11 17:52 ` Laurent Pinchart
  8 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2015-03-11 16:40 UTC (permalink / raw)
  To: Rob Clark; +Cc: Andy Yan, Laurent Pinchart, Maling list - DRI developers

On Wed, Mar 11, 2015 at 10:23 AM, Rob Clark <robdclark@gmail.com> wrote:
> 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.

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> 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
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 6/7] drm/fb: small cleanup
  2015-03-11 14:23 ` [PATCH v2 6/7] drm/fb: small cleanup Rob Clark
@ 2015-03-11 17:51   ` Laurent Pinchart
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2015-03-11 17:51 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, Andy Yan

Hi Rob,

Thank you for the patch.

On Wednesday 11 March 2015 10:23:13 Rob Clark wrote:
> 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 <robdclark@gmail.com>
> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index 1e6a0c7..dca98a4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -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);
> +		sizes.fb_height = min_t(u32, desired_mode->vdisplay + y,
> sizes.fb_height); }

Nitpicking, reducing the indentation level is nice, but you're making lines 
longer. I would add line breaks here.

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

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/7] drm/fb: width/height cleanups/fixes
  2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
                   ` (7 preceding siblings ...)
  2015-03-11 16:40 ` [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Alex Deucher
@ 2015-03-11 17:52 ` Laurent Pinchart
  8 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2015-03-11 17:52 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, Andy Yan

Hi Rob,

Thank you for the patches.

On Wednesday 11 March 2015 10:23:07 Rob Clark wrote:
> 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

For patches 1 to 5,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  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(-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 7/7] drm/fb: handle tiled connectors better
  2015-03-11 14:23 ` [PATCH v2 7/7] drm/fb: handle tiled connectors better Rob Clark
@ 2015-03-11 21:13   ` Daniel Vetter
  2015-03-11 21:21     ` Rob Clark
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2015-03-11 21:13 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, Laurent Pinchart, Andy Yan

On Wed, Mar 11, 2015 at 10:23:14AM -0400, Rob Clark wrote:
> 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 <robdclark@gmail.com>
> Tested-by: Hai Li <hali@codeaurora.org>

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

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index dca98a4..1a20db7 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -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) {
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 7/7] drm/fb: handle tiled connectors better
  2015-03-11 21:13   ` Daniel Vetter
@ 2015-03-11 21:21     ` Rob Clark
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-03-11 21:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, Laurent Pinchart, Andy Yan

On Wed, Mar 11, 2015 at 5:13 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Mar 11, 2015 at 10:23:14AM -0400, Rob Clark wrote:
>> 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 <robdclark@gmail.com>
>> Tested-by: Hai Li <hali@codeaurora.org>
>
> 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.
>

yeah, I was going with the "lines were already too long to begin with,
and adding some line breaks made it more ugly" exception ;-)

BR,
-R

> Thanks, Daniel
>
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++++++++---
>>  1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index dca98a4..1a20db7 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -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) {
>> --
>> 2.1.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-03-11 21:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-11 14:23 [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Rob Clark
2015-03-11 14:23 ` [PATCH v2 1/7] drm/fb: document drm_fb_helper_surface_size Rob Clark
2015-03-11 14:23 ` [PATCH v2 2/7] drm/atomic: minor kerneldoc typo fix Rob Clark
2015-03-11 14:23 ` [PATCH v2 3/7] drm/cma: use correct fb width/height Rob Clark
2015-03-11 14:23 ` [PATCH v2 4/7] drm/exynos: " Rob Clark
2015-03-11 14:23 ` [PATCH v2 5/7] drm/rockchip: " Rob Clark
2015-03-11 14:23 ` [PATCH v2 6/7] drm/fb: small cleanup Rob Clark
2015-03-11 17:51   ` Laurent Pinchart
2015-03-11 14:23 ` [PATCH v2 7/7] drm/fb: handle tiled connectors better Rob Clark
2015-03-11 21:13   ` Daniel Vetter
2015-03-11 21:21     ` Rob Clark
2015-03-11 16:40 ` [PATCH v2 0/7] drm/fb: width/height cleanups/fixes Alex Deucher
2015-03-11 17:52 ` Laurent Pinchart

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.