* [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.