Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1502789 > unrolled thread

[PATCH 0/5] drm/sun4i: Handle TV overscan

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] drm/sun4i: Handle TV overscan Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 10:40 +0200
    [PATCH 3/5] drm/sun4i: Add custom crtc state Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 10:40 +0200
    [PATCH 5/5] drm/sun4i: Add support for the overscan profiles Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 10:40 +0200
    [PATCH 2/5] drm/modes: Support modes names on the command line Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 10:40 +0200
    [PATCH 4/5] drm/sun4i: Compute TCON1 mode from tv mode Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 10:40 +0200
    Re: [PATCH 0/5] drm/sun4i: Handle TV overscan Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-18 11:30 +0200
      Re: [PATCH 0/5] drm/sun4i: Handle TV overscan Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-18 12:10 +0200
        Re: [PATCH 0/5] drm/sun4i: Handle TV overscan Jean-Francois Moine <moinejf@free.fr> - 2016-10-18 20:00 +0200

#1502789 — [PATCH 0/5] drm/sun4i: Handle TV overscan

Hi,

The Allwinner display engine doesn't have any kind of hardware help to deal
with TV overscan.

This means that if we use the only mode the hardware provides (either PAL
or NTSC, or something else), most of the screens will crop the borders of
the image, which is bad.

We can however use somekind of a hack, to instead reduce the mode exposed
to the userspace, and center it in the final image. We would expose
different overscan ratio to be able to deal with most of the screens, each
reducing more the displayable area.

The first patches are rework for the command line parser in order to be
able to use named modes. This is going to be helpful for us, since
different modes might have the same timings but only differ on a few
settings not exposed in the modes, but it might eventually be used for the
*VGA modes for example.

The last patches extend the current driver to deal with the changes used
to introduce the overscan.

Let me know what you think,
Maxime

Maxime Ripard (5):
  drm/modes: Rewrite the command line parser
  drm/modes: Support modes names on the command line
  drm/sun4i: Add custom crtc state
  drm/sun4i: Compute TCON1 mode from tv mode
  drm/sun4i: Add support for the overscan profiles

 drivers/gpu/drm/drm_connector.c       |   3 +-
 drivers/gpu/drm/drm_fb_helper.c       |   4 +-
 drivers/gpu/drm/drm_modes.c           | 324 +++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_backend.c |  18 +-
 drivers/gpu/drm/sun4i/sun4i_crtc.c    |  37 ++-
 drivers/gpu/drm/sun4i/sun4i_crtc.h    |  16 +-
 drivers/gpu/drm/sun4i/sun4i_rgb.c     |  10 +-
 drivers/gpu/drm/sun4i/sun4i_tv.c      |  75 ++++--
 include/drm/drm_connector.h           |   1 +-
 9 files changed, 342 insertions(+), 146 deletions(-)

-- 
git-series 0.8.10

[toc] | [next] | [standalone]

#1502790 — [PATCH 3/5] drm/sun4i: Add custom crtc state

We'll need a custom CRTC state to deal with the overscan setup.

We'll store in it the actual display size that can be used by the
applications, and the size to use on the plane.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 18 +++++++++-----
 drivers/gpu/drm/sun4i/sun4i_crtc.c    | 37 ++++++++++++++++++++++++++--
 drivers/gpu/drm/sun4i/sun4i_crtc.h    | 16 ++++++++++++-
 drivers/gpu/drm/sun4i/sun4i_rgb.c     | 10 ++++++++-
 drivers/gpu/drm/sun4i/sun4i_tv.c      | 14 +++++++++++-
 5 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 32c0584e3c35..9b36b7104c15 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -22,6 +22,7 @@
 #include <linux/reset.h>
 
 #include "sun4i_backend.h"
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
 
 static u32 sunxi_rgb2yuv_coef[12] = {
@@ -115,15 +116,19 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
 {
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
+	struct sun4i_crtc_state *s_state = drm_crtc_state_to_sun4i_crtc_state(state->crtc->state);
+	u16 x, y;
+
 
 	DRM_DEBUG_DRIVER("Updating layer %d\n", layer);
 
 	if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
 		DRM_DEBUG_DRIVER("Primary layer, updating global size W: %u H: %u\n",
-				 state->crtc_w, state->crtc_h);
+				 s_state->display_x_size,
+				 s_state->display_y_size);
 		regmap_write(backend->regs, SUN4I_BACKEND_DISSIZE_REG,
-			     SUN4I_BACKEND_DISSIZE(state->crtc_w,
-						   state->crtc_h));
+			     SUN4I_BACKEND_DISSIZE(s_state->display_x_size,
+						   s_state->display_y_size));
 	}
 
 	/* Set the line width */
@@ -139,11 +144,12 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
 					   state->crtc_h));
 
 	/* Set base coordinates */
+	x = s_state->plane_x_offset + state->crtc_x;
+	y = s_state->plane_y_offset + state->crtc_y;
 	DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n",
-			 state->crtc_x, state->crtc_y);
+			 x, y);
 	regmap_write(backend->regs, SUN4I_BACKEND_LAYCOOR_REG(layer),
-		     SUN4I_BACKEND_LAYCOOR(state->crtc_x,
-					   state->crtc_y));
+		     SUN4I_BACKEND_LAYCOOR(x, y));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index 4a192210574f..772e0ecd72db 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -104,9 +104,42 @@ static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = {
 	.enable		= sun4i_crtc_enable,
 };
 
+struct drm_crtc_state *sun4i_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+	struct sun4i_crtc_state *state = drm_crtc_state_to_sun4i_crtc_state(crtc->state);
+	struct sun4i_crtc_state *copy;
+
+	copy = kmalloc(sizeof(*copy), GFP_KERNEL);
+	if (!copy)
+		return NULL;
+
+	DRM_DEBUG_DRIVER("Copying state %p to %p", state, copy);
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &copy->base);
+
+	copy->display_x_size = state->display_x_size;
+	copy->display_y_size = state->display_y_size;
+
+	copy->plane_x_offset = state->plane_x_offset;
+	copy->plane_y_offset = state->plane_y_offset;
+
+	return &copy->base;
+}
+
+void sun4i_crtc_destroy_state(struct drm_crtc *crtc,
+			      struct drm_crtc_state *c_state)
+{
+	struct sun4i_crtc_state *s_state = drm_crtc_state_to_sun4i_crtc_state(c_state);
+
+	DRM_DEBUG_DRIVER("Freeing state %p", s_state);
+
+	__drm_atomic_helper_crtc_destroy_state(c_state);
+	kfree(s_state);
+}
+
 static const struct drm_crtc_funcs sun4i_crtc_funcs = {
-	.atomic_destroy_state	= drm_atomic_helper_crtc_destroy_state,
-	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state	= sun4i_crtc_destroy_state,
+	.atomic_duplicate_state	= sun4i_crtc_duplicate_state,
 	.destroy		= drm_crtc_cleanup,
 	.page_flip		= drm_atomic_helper_page_flip,
 	.reset			= drm_atomic_helper_crtc_reset,
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.h b/drivers/gpu/drm/sun4i/sun4i_crtc.h
index dec8ce4d9b25..625c9ac41434 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.h
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.h
@@ -20,11 +20,27 @@ struct sun4i_crtc {
 	struct sun4i_drv		*drv;
 };
 
+struct sun4i_crtc_state {
+	struct drm_crtc_state	base;
+
+	u32			display_x_size;
+	u32			display_y_size;
+
+	u32			plane_x_offset;
+	u32			plane_y_offset;
+};
+
 static inline struct sun4i_crtc *drm_crtc_to_sun4i_crtc(struct drm_crtc *crtc)
 {
 	return container_of(crtc, struct sun4i_crtc, crtc);
 }
 
+static inline struct sun4i_crtc_state *
+drm_crtc_state_to_sun4i_crtc_state(struct drm_crtc_state *state)
+{
+	return container_of(state, struct sun4i_crtc_state, base);
+}
+
 struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm);
 
 #endif /* _SUN4I_CRTC_H_ */
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index c3ff10f559cc..b1f792ad84c2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -17,6 +17,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_panel.h>
 
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
 #include "sun4i_tcon.h"
 #include "sun4i_rgb.h"
@@ -141,6 +142,15 @@ static int sun4i_rgb_atomic_check(struct drm_encoder *encoder,
 				  struct drm_crtc_state *crtc_state,
 				  struct drm_connector_state *conn_state)
 {
+	struct drm_display_mode *mode = &crtc_state->mode;
+	struct sun4i_crtc_state *state = drm_crtc_state_to_sun4i_crtc_state(crtc_state);
+
+	state->display_x_size = mode->hdisplay;
+	state->display_y_size = mode->vdisplay;
+
+	state->plane_x_offset = 0;
+	state->plane_y_offset = 0;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index 1dd3d9eabf2e..6f8077013be3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -22,6 +22,7 @@
 #include <drm/drm_panel.h>
 
 #include "sun4i_backend.h"
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
 #include "sun4i_tcon.h"
 
@@ -343,6 +344,19 @@ static int sun4i_tv_atomic_check(struct drm_encoder *encoder,
 				 struct drm_crtc_state *crtc_state,
 				 struct drm_connector_state *conn_state)
 {
+	struct drm_display_mode *mode = &crtc_state->mode;
+	const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
+	struct sun4i_crtc_state *state = drm_crtc_state_to_sun4i_crtc_state(crtc_state);
+
+	if (!tv_mode)
+		return -EINVAL;
+
+	state->display_x_size = tv_mode->hdisplay;
+	state->plane_x_offset = 0;
+
+	state->display_y_size = tv_mode->vdisplay;
+	state->plane_y_offset = 0;
+
 	return 0;
 }
 
-- 
git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

#1502792 — [PATCH 5/5] drm/sun4i: Add support for the overscan profiles

Create overscan profiles reducing the displayed zone.

For each TV standard (PAL and NTSC so far), we create 4 more reduced modes
by steps of 5% that the user will be able to select.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_tv.c | 60 +++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index f99886462cb8..9ee03ba086b6 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -301,27 +301,33 @@ static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct drm_display_m
 		DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
 				 mode->name, tv_mode->name);
 
-		if (!strcmp(mode->name, tv_mode->name))
+		if (!strncmp(mode->name, tv_mode->name, strlen(tv_mode->name)))
 			return tv_mode;
 	}
 
 	/* Then by number of lines */
 	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
 		const struct tv_mode *tv_mode = &tv_modes[i];
+		int j;
 
-		DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
-				 mode->name, tv_mode->name,
-				 mode->vdisplay, tv_mode->vdisplay);
+		for (j = 0; j < 20; j += 5) {
+			u32 vdisplay = tv_mode->vdisplay * (100 - j) / 100;
 
-		if (mode->vdisplay == tv_mode->vdisplay)
-			return tv_mode;
+			DRM_DEBUG_DRIVER("Comparing mode with %s (%d) (X: %d vs %d)",
+					 tv_mode->name, j,
+					 vdisplay, tv_mode->vdisplay);
+
+			if (vdisplay == tv_mode->vdisplay)
+				return tv_mode;
+		}
 	}
 
 	return NULL;
 }
 
 static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
-				      struct drm_display_mode *mode)
+				      struct drm_display_mode *mode,
+				      int overscan)
 {
 	DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
 
@@ -329,12 +335,12 @@ static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
 	mode->clock = 13500;
 	mode->flags = DRM_MODE_FLAG_INTERLACE;
 
-	mode->hdisplay = tv_mode->hdisplay;
+	mode->hdisplay = tv_mode->hdisplay * (100 - overscan) / 100;
 	mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
 	mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
 	mode->htotal = mode->hsync_end  + tv_mode->hback_porch;
 
-	mode->vdisplay = tv_mode->vdisplay;
+	mode->vdisplay = tv_mode->vdisplay * (100 - overscan) / 100;
 	mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
 	mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
 	mode->vtotal = mode->vsync_end  + tv_mode->vback_porch;
@@ -352,10 +358,10 @@ static int sun4i_tv_atomic_check(struct drm_encoder *encoder,
 		return -EINVAL;
 
 	state->display_x_size = tv_mode->hdisplay;
-	state->plane_x_offset = 0;
+	state->plane_x_offset = (tv_mode->hdisplay - mode->hdisplay) / 2;
 
 	state->display_y_size = tv_mode->vdisplay;
-	state->plane_y_offset = 0;
+	state->plane_y_offset = (tv_mode->vdisplay - mode->vdisplay) / 2;
 
 	return 0;
 }
@@ -404,7 +410,7 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder,
 	struct drm_display_mode tv_drm_mode = { 0 };
 
 	strcpy(tv_drm_mode.name, "TV");
-	sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode);
+	sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode, 0);
 	drm_mode_set_crtcinfo(&tv_drm_mode, CRTC_INTERLACE_HALVE_V);
 
 	sun4i_tcon1_mode_set(tcon, &tv_drm_mode);
@@ -526,22 +532,28 @@ static int sun4i_tv_comp_get_modes(struct drm_connector *connector)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
-		struct drm_display_mode *mode;
 		const struct tv_mode *tv_mode = &tv_modes[i];
-
-		mode = drm_mode_create(connector->dev);
-		if (!mode) {
-			DRM_ERROR("Failed to create a new display mode\n");
-			return 0;
+		int j;
+
+		for (j = 0; j < 20; j += 5) {
+			struct drm_display_mode *mode = drm_mode_create(connector->dev);
+			if (!mode) {
+				DRM_ERROR("Failed to create a new display mode\n");
+				return 0;
+			}
+
+			if (j)
+				sprintf(mode->name, "%s%d", tv_mode->name,
+					j);
+			else
+				strcpy(mode->name, tv_mode->name);
+
+			sun4i_tv_mode_to_drm_mode(tv_mode, mode, j);
+			drm_mode_probed_add(connector, mode);
 		}
-
-		strcpy(mode->name, tv_mode->name);
-
-		sun4i_tv_mode_to_drm_mode(tv_mode, mode);
-		drm_mode_probed_add(connector, mode);
 	}
 
-	return i;
+	return i * 4;
 }
 
 static int sun4i_tv_comp_mode_valid(struct drm_connector *connector,
-- 
git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

#1502793 — [PATCH 2/5] drm/modes: Support modes names on the command line

The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.

However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/drm_connector.c |  3 +-
 drivers/gpu/drm/drm_fb_helper.c |  4 +++-
 drivers/gpu/drm/drm_modes.c     | 49 +++++++++++++++++++++++-----------
 include/drm/drm_connector.h     |  1 +-
 4 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2db7fb510b6c..27a8a511257c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -147,8 +147,9 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
 		connector->force = mode->force;
 	}
 
-	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
 		      connector->name,
+		      mode->name ? mode->name : "",
 		      mode->xres, mode->yres,
 		      mode->refresh_specified ? mode->refresh : 60,
 		      mode->rb ? " reduced blanking" : "",
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 03414bde1f15..20a68305fb45 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1748,6 +1748,10 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *f
 	prefer_non_interlace = !cmdline_mode->interlace;
 again:
 	list_for_each_entry(mode, &fb_helper_conn->connector->modes, head) {
+		/* Check (optional) mode name first */
+		if (!strcmp(mode->name, cmdline_mode->name))
+			return mode;
+
 		/* check width/height */
 		if (mode->hdisplay != cmdline_mode->xres ||
 		    mode->vdisplay != cmdline_mode->yres)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 7d5bdca276f2..fdbf541a5978 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1413,7 +1413,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 					       struct drm_cmdline_mode *mode)
 {
 	const char *name;
-	bool parse_extras = false;
+	bool named_mode = false, parse_extras = false;
 	unsigned int bpp_off = 0, refresh_off = 0;
 	unsigned int mode_end = 0;
 	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1432,8 +1432,14 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 
 	name = mode_option;
 
+	/*
+	 * If the first character is not a digit, then it means that
+	 * we have a named mode.
+	 */
 	if (!isdigit(name[0]))
-		return false;
+		named_mode = true;
+	else
+		named_mode = false;
 
 	/* Try to locate the bpp and refresh specifiers, if any */
 	bpp_ptr = strchr(name, '-');
@@ -1460,12 +1466,16 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 		parse_extras = true;
 	}
 
-	ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
-					      parse_extras,
-					      connector,
-					      mode);
-	if (ret)
-		return false;
+	if (named_mode) {
+		strncpy(mode->name, name, mode_end);
+	} else {
+		ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+						      parse_extras,
+						      connector,
+						      mode);
+		if (ret)
+			return false;
+	}
 	mode->specified = true;
 
 	if (bpp_ptr) {
@@ -1493,14 +1503,23 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 		extra_ptr = refresh_end_ptr;
 
 	if (extra_ptr) {
-		int remaining = strlen(name) - (extra_ptr - name);
+		if (!named_mode) {
+			int len = strlen(name) - (extra_ptr - name);
 
-		/*
-		 * We still have characters to process, while
-		 * we shouldn't have any
-		 */
-		if (remaining > 0)
-			return false;
+			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
+							   connector, mode);
+			if (ret)
+				return false;
+		} else {
+			int remaining = strlen(name) - (extra_ptr - name);
+
+			/*
+			 * We still have characters to process, while
+			 * we shouldn't have any
+			 */
+			if (remaining > 0)
+				return false;
+		}
 	}
 
 	return true;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ac9d7d8e0e43..dce3d4b2fd33 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -485,6 +485,7 @@ struct drm_connector_funcs {
 
 /* mode specified on the command line */
 struct drm_cmdline_mode {
+	char name[DRM_DISPLAY_MODE_LEN];
 	bool specified;
 	bool refresh_specified;
 	bool bpp_specified;
-- 
git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

#1502795 — [PATCH 4/5] drm/sun4i: Compute TCON1 mode from tv mode

Since the mode passed in mode_set is probably going to be a scaled down
version of the TV mode, we cannot just use it.

Instead, try to retrieve the actual mode we want to set, and generate a drm
mode from that.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_tv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index 6f8077013be3..f99886462cb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -401,8 +401,13 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder,
 	struct sun4i_drv *drv = tv->drv;
 	struct sun4i_tcon *tcon = drv->tcon;
 	const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
+	struct drm_display_mode tv_drm_mode = { 0 };
 
-	sun4i_tcon1_mode_set(tcon, mode);
+	strcpy(tv_drm_mode.name, "TV");
+	sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode);
+	drm_mode_set_crtcinfo(&tv_drm_mode, CRTC_INTERLACE_HALVE_V);
+
+	sun4i_tcon1_mode_set(tcon, &tv_drm_mode);
 
 	/* Enable and map the DAC to the output */
 	regmap_update_bits(tv->regs, SUN4I_TVE_EN_REG,
-- 
git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

#1502821

On Tue, Oct 18, 2016 at 10:29:33AM +0200, Maxime Ripard wrote:
> The Allwinner display engine doesn't have any kind of hardware help to deal
> with TV overscan.

I'm not sure I follow.  My understanding (from reading the CEA specs)
is that TVs are expected to overscan the image, so the upper left, and
bottom right pixels are not visible.

I assume we are talking about TVs connected via HDMI.  In the HDMI AVI
infoframe, there are bits which specify whether the image should be
overscanned or underscanned - however, whether a TV implements those
bits is rather sketchy.  I assume when you say "any kind of hardware
help" you mean you can't control these bits?

However, some (most?) TVs now implement a menu option which allows the
(over)scan mode to be selected.  Others assume that if it's a TV mode,
it's supposed to be overscanned, if it's a "PC" mode, it should be
underscanned and provide no option to change the behaviour.

> This means that if we use the only mode the hardware provides (either PAL
> or NTSC, or something else), most of the screens will crop the borders of
> the image, which is bad.

I think you're trying to apply monitor-type behaviour to TVs...

> We can however use somekind of a hack, to instead reduce the mode exposed
> to the userspace, and center it in the final image. We would expose
> different overscan ratio to be able to deal with most of the screens, each
> reducing more the displayable area.

I'm not sure we need "a hack".  What if we treated the primary plane just
like any other (eg, overlay) plane?  We could then specify (eg) a 1920x1080
display mode, but with the primary plane reduced in size, positioned in
the centre of the display mode?

I know that there's hardware out there which can do exactly that - Marvell
Dove implements this: you set the display size separately from two planes,
one graphics plane and one video plane.  Both planes can be positioned
anywhere in the displayed size.

We could then specify at DRM level that a connected device overscans by
N%, and have the primary plane adjusted by DRM itself.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1502865

[Multipart message — attachments visible in raw view] — view raw

Hi Russell,

On Tue, Oct 18, 2016 at 10:24:22AM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 18, 2016 at 10:29:33AM +0200, Maxime Ripard wrote:
> > The Allwinner display engine doesn't have any kind of hardware help to deal
> > with TV overscan.
> 
> I'm not sure I follow.  My understanding (from reading the CEA specs)
> is that TVs are expected to overscan the image, so the upper left, and
> bottom right pixels are not visible.

Yes, this is why we have to work around it somehow.

> I assume we are talking about TVs connected via HDMI.  In the HDMI AVI
> infoframe, there are bits which specify whether the image should be
> overscanned or underscanned - however, whether a TV implements those
> bits is rather sketchy.  I assume when you say "any kind of hardware
> help" you mean you can't control these bits?
> 
> However, some (most?) TVs now implement a menu option which allows the
> (over)scan mode to be selected.  Others assume that if it's a TV mode,
> it's supposed to be overscanned, if it's a "PC" mode, it should be
> underscanned and provide no option to change the behaviour.

We're talking about plain dumb composite output, so no infoframes,
setup or anything here :)

> > This means that if we use the only mode the hardware provides (either PAL
> > or NTSC, or something else), most of the screens will crop the borders of
> > the image, which is bad.
> 
> I think you're trying to apply monitor-type behaviour to TVs...

Yes, kind of. Our users are usually running a desktop distro, and the
default output on those boards are just plain composite, which means
running any DE onto a TV.

Note that it's not only about the interface itself, but you'll lose
content for all pictures displayed. And no one cares about the TV safe
area anymore these days (starting with the framebuffer console).

> > We can however use somekind of a hack, to instead reduce the mode
> > exposed to the userspace, and center it in the final image. We
> > would expose different overscan ratio to be able to deal with most
> > of the screens, each reducing more the displayable area.
> 
> I'm not sure we need "a hack".  What if we treated the primary plane just
> like any other (eg, overlay) plane?  We could then specify (eg) a 1920x1080
> display mode, but with the primary plane reduced in size, positioned in
> the centre of the display mode?
> 
> I know that there's hardware out there which can do exactly that - Marvell
> Dove implements this: you set the display size separately from two planes,
> one graphics plane and one video plane.  Both planes can be positioned
> anywhere in the displayed size.

This might have been poorly worded on my side, but it's exactly what
I'm doing in those patches.

> We could then specify at DRM level that a connected device overscans by
> N%, and have the primary plane adjusted by DRM itself.

I'd agree with you, however, there's a few issues with that I
think.

The first one is that this overscanning should be reported by the
connector I guess? but this is really TV specific, so we need one way
to let the user tell how the image is displayed on its side, and we
cannot really autodetect it, and this needs to be done at runtime so
that we can present some shiny interface to let it select which
overscan ratio works for him/her.

The second one is that we still need to expose the reduced modes to
userspace, and not only the displayed size, so that the applications
know what they must draw on. But I guess this could be adjusted by the
core too.

In order to work consistently, I think all planes should be adjusted
that way, so that relative coordinates are from the primary plane
origin, and not the display origin. But that could be adjusted too by
the core I guess.

The fourth one being the major one. Every time I raised the issue on
IRC, the answer basically was "we don't care about analog", so I'm a
bit pessimistic about whether dealing with this in the core would be
accepted, hence why I chose to deal with this at the driver level.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1503263

On Tue, 18 Oct 2016 12:03:49 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> The fourth one being the major one. Every time I raised the issue on
> IRC, the answer basically was "we don't care about analog", so I'm a
> bit pessimistic about whether dealing with this in the core would be
> accepted, hence why I chose to deal with this at the driver level.

The same problem exists with HDMI and old TVs (mine is an ASUS 22T1E):
these TVs overscan as soon as AVI frames are in the stream.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web