[PATCH v3 0/3] drm/connector: Provide generic support for underscan

[PATCH v3 0/3] drm/connector: Provide generic support for underscan

[Boris Brezillon]

Hello,

This is an attempt at providing generic support for underscan connector
props. We already have 3 drivers defining the same underscan, underscan
vborder and underscan hborder properties (amd, radeon and nouveau) and
I am about to add a new one, hence my proposal to put the prop parsing
code in the core and add ->underscan fields to drm_connector_state.

This v3 was based on the "VC4 cursor scaling" changes, and thus address
Ville's concern about cursor plane not being rescaled with the other
ones.

Note that I dropped the nouveau conversion. I'll post a new version
once/if the generic stuff are accepted.

Regards,

Boris 

Boris Brezillon (3):
  drm/connector: Add generic underscan properties
  drm/vc4: Take underscan setup into account when updating planes
  drm/vc4: Attach underscan props to the HDMI connector

 drivers/gpu/drm/drm_atomic_uapi.c |  12 +++
 drivers/gpu/drm/drm_connector.c   | 127 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_crtc.c    |  47 +++++++++++
 drivers/gpu/drm/vc4/vc4_drv.h     |   3 +
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  25 ++++++
 drivers/gpu/drm/vc4/vc4_plane.c   |  50 ++++++++++++
 include/drm/drm_connector.h       |  80 +++++++++++++++++++
 7 files changed, 344 insertions(+)

-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v3 1/3] drm/connector: Add generic underscan properties

[Boris Brezillon]

We have 3 drivers defining the "underscan", "underscan hborder" and
"underscan vborder" properties (radeon, amd and nouveau) and we are
about to add the same kind of thing in VC4.

Define generic underscan props and add new fields to the drm_connector
state so that the property parsing logic can be shared by all DRM
drivers.

A driver can now attach underscan properties to its connector through
the drm_connector_attach_underscan_properties() helper, and can
check/apply the underscan setup based on the
drm_connector_state->underscan fields.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- None

Changes in v2:
- Add a new section in the connector props doc to describe underscan
  props
- Fix description of auto mode (auto means apply underscan for HDMI
  monitors only)
- Fix description of vborder/hborder:
    right_border = left_border = hborder
    top_border = bottom_border = vborder
  not
    right_border = left_border = hborder / 2
    top_border = bottom_border = vborder / 2
- Rename drm_underscan into drm_underscan_state
---
 drivers/gpu/drm/drm_atomic_uapi.c |  12 +++
 drivers/gpu/drm/drm_connector.c   | 127 ++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       |  80 +++++++++++++++++++
 3 files changed, 219 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f315098c..39db6e31c565 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -740,6 +740,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 
 		return set_out_fence_for_connector(state->state, connector,
 						   fence_ptr);
+	} else if (property == connector->underscan_mode_property) {
+		state->underscan.mode = val;
+	} else if (property == connector->underscan_hborder_property) {
+		state->underscan.hborder = val;
+	} else if (property == connector->underscan_vborder_property) {
+		state->underscan.vborder = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -799,6 +805,12 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->scaling_mode;
 	} else if (property == connector->content_protection_property) {
 		*val = state->content_protection;
+	} else if (property == connector->underscan_mode_property) {
+		*val = state->underscan.mode;
+	} else if (property == connector->underscan_hborder_property) {
+		*val = state->underscan.hborder;
+	} else if (property == connector->underscan_vborder_property) {
+		*val = state->underscan.vborder;
 	} else if (property == config->writeback_fb_id_property) {
 		/* Writeback framebuffer is one-shot, write and forget */
 		*val = 0;
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c555e17ab8d7..170317248da6 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -971,6 +971,38 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
  *	can also expose this property to external outputs, in which case they
  *	must support "None", which should be the default (since external screens
  *	have a built-in scaler).
+ *
+ * Connectors for non-analog outputs may also have standardized underscan
+ * properties (drivers can set this up by calling
+ * drm_connector_attach_content_protection_property() on initialization):
+ *
+ * underscan:
+ *	This properties defines whether underscan is activated or not, and when
+ *	it is activated, how the horizontal and vertical borders are calculated:
+ *
+ *	off:
+ *		Underscan is disabled. The output image shouldn't be scaled to
+ *		take screen borders into account.
+ *	on:
+ *		Underscan is activated and horizontal and vertical borders are
+ *		specified through the "underscan hborder" and
+ *		"underscan vborder" properties.
+ *	auto:
+ *		Underscan is activated only for HDMI monitors.
+ *
+ * underscan hborder:
+ *	Horizontal border expressed in pixels. The border is symmetric, which
+ *	means you'll have a border of 'hborder' pixels on the left and on the
+ *	same border on the right.
+ *	When this value is 0 and underscan is "on" or "auto", the driver will
+ *	pick a default value (the default value is driver dependent).
+ *
+ * underscan vborder:
+ *	Vertical border expressed in pixels. The border is symmetric, which
+ *	means you'll have a border of 'vborder' pixels on the top and the same
+ *	border on the bottom.
+ *	When this value is 0 and underscan is "on" or "auto", the driver will
+ *	pick a default value (the default value is driver dependent).
  */
 
 int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1241,6 +1273,101 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_mode_create_tv_properties);
 
+static const struct drm_prop_enum_list drm_underscan_mode_enum_list[] = {
+	{ DRM_UNDERSCAN_OFF, "off" },
+	{ DRM_UNDERSCAN_ON, "on" },
+	{ DRM_UNDERSCAN_AUTO, "auto" },
+};
+
+/**
+ * drm_connector_attach_underscan_properties - attach atomic underscan
+ *					       properties
+ * @connector: connector to attach underscan mode properties on.
+ * @mode_mask: bitmask of %DRM_UNDERSCAN_XX modes encoding the supported
+ *	       underscan modes.
+ * @max_hborder: maximum size of the horizontal border expressed in pixels.
+ *		 Should be > 0.
+ * @max_vborder: maximum size of the vertical border expressed in pixels.
+ *		 Should be > 0.
+ *
+ * This is used to add support for underscan to atomic drivers.
+ * The underscan config will be set to &drm_connector_state.underscan
+ * and can be used from &drm_connector_helper_funcs->atomic_check for
+ * validation.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_underscan_properties(struct drm_connector *connector,
+					      u32 mode_mask, u64 max_hborder,
+					      u64 max_vborder)
+{
+	unsigned int i, nmodes = ARRAY_SIZE(drm_underscan_mode_enum_list);
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (!max_hborder || !max_vborder)
+		return -EINVAL;
+
+	if (!hweight32(mode_mask) || (mode_mask & ~GENMASK(nmodes - 1, 0)))
+		return -EINVAL;
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, "underscan",
+				   hweight32(mode_mask));
+	if (!prop)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(drm_underscan_mode_enum_list); i++) {
+		const struct drm_prop_enum_list *entry;
+		int ret;
+
+		if (!(BIT(i) & mode_mask))
+			continue;
+
+		entry = &drm_underscan_mode_enum_list[i];
+		ret = drm_property_add_enum(prop, entry->type, entry->name);
+		if (ret)
+			goto err_free_mode_prop;
+	}
+
+	connector->underscan_mode_property = prop;
+
+	prop = drm_property_create_range(dev, 0, "underscan hborder", 0,
+					 max_hborder);
+	if (!prop)
+		goto err_free_mode_prop;
+
+	connector->underscan_hborder_property = prop;
+
+	prop = drm_property_create_range(dev, 0, "underscan vborder", 0,
+					 max_vborder);
+	if (!prop)
+		goto err_free_hborder_prop;
+
+	connector->underscan_vborder_property = prop;
+
+	drm_object_attach_property(&connector->base,
+				   connector->underscan_mode_property,
+				   DRM_UNDERSCAN_OFF);
+	drm_object_attach_property(&connector->base,
+				   connector->underscan_hborder_property, 0);
+	drm_object_attach_property(&connector->base,
+				   connector->underscan_vborder_property, 0);
+
+	return 0;
+
+err_free_hborder_prop:
+	drm_property_destroy(dev, connector->underscan_hborder_property);
+	connector->underscan_hborder_property = NULL;
+
+err_free_mode_prop:
+	drm_property_destroy(dev, connector->underscan_mode_property);
+	connector->underscan_mode_property = NULL;
+
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(drm_connector_attach_underscan_properties);
+
 /**
  * drm_mode_create_scaling_mode_property - create scaling mode property
  * @dev: DRM device
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5b3cf909fd5e..14d423dc7a14 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -377,6 +377,53 @@ struct drm_tv_connector_state {
 	unsigned int hue;
 };
 
+/**
+ * enum drm_underscan_mode - Underscan mode
+ *
+ * This enum is used to track the underscan mode.
+ *
+ * @DRM_UNDERSCAN_OFF: No underscan applied, the output image will be unchanged
+ * @DRM_UNDERSCAN_ON: Underscan is enabled, and horizontal/vertical border size
+ *		      are specified through the &struct_drm_underscan->hborder
+ *		      and &struct_drm_underscan->vborder fields.
+ * @DRM_UNDERSCAN_AUTO: Underscan is enabled and &struct_drm_underscan->hborder
+ *			and &struct_drm_underscan->vborder are guessed by the
+ *			driver.
+ */
+enum drm_underscan_mode {
+	DRM_UNDERSCAN_OFF,
+	DRM_UNDERSCAN_ON,
+	DRM_UNDERSCAN_AUTO,
+};
+
+/**
+ * struct drm_underscan_state - Underscan properties attached to a connector
+ *				state
+ *
+ * This can be used to tell the CRTC how the image should be scaled/placed in
+ * order to cover fit in the display connected through this connector. Most of
+ * the time used to address situations where the display borders are hidden.
+ * Can also be used to compensate overscan done on the display side.
+ */
+struct drm_underscan_state {
+	/**
+	 * @mode: Underscan mode.
+	 */
+	enum drm_underscan_mode mode;
+
+	/**
+	 * @hborder: Horizontal border. This values encodes both the left and
+	 * right borders: left_border = right_border = hborder / 2.
+	 */
+	unsigned int hborder;
+
+	/**
+	 * @vborder: Vertical border. This values encodes both the top and
+	 * bottom borders: top_border = bottom_border = vborder / 2.
+	 */
+	unsigned int vborder;
+};
+
 /**
  * struct drm_connector_state - mutable connector state
  */
@@ -461,6 +508,13 @@ struct drm_connector_state {
 	 * drm_writeback_signal_completion()
 	 */
 	struct drm_writeback_job *writeback_job;
+
+	/**
+	 * @underscan: Underscan information. Most commonly used to adjust
+	 * image when the display has borders covering part of the image of
+	 * when it's doing overscan on its own.
+	 */
+	struct drm_underscan_state underscan;
 };
 
 /**
@@ -924,6 +978,29 @@ struct drm_connector {
 	 */
 	struct drm_property_blob *path_blob_ptr;
 
+	/**
+	 * @underscan_mode_property: Optional connector underscan mode. Used by
+	 * the driver to scale the output image and compensate an overscan done
+	 * on the display side.
+	 */
+	struct drm_property *underscan_mode_property;
+
+	/**
+	 * @underscan_hborder_property: Optional connector underscan horizontal
+	 * border (expressed in pixels). Used by the driver to adjust the
+	 * output image position and compensate an overscan done on the display
+	 * side.
+	 */
+	struct drm_property *underscan_hborder_property;
+
+	/**
+	 * @underscan_hborder_property: Optional connector underscan vertical
+	 * border (expressed in pixels). Used by the driver to adjust the
+	 * output image position and compensate an overscan done on the display
+	 * side.
+	 */
+	struct drm_property *underscan_vborder_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev);
 int drm_mode_create_tv_properties(struct drm_device *dev,
 				  unsigned int num_modes,
 				  const char * const modes[]);
+int drm_connector_attach_underscan_properties(struct drm_connector *connector,
+					      u32 mode_mask, u64 max_hborder,
+					      u64 max_vborder);
 int drm_mode_create_scaling_mode_property(struct drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
-- 
2.17.1

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

[PATCH v3 2/3] drm/vc4: Take underscan setup into account when updating planes

[Boris Brezillon]

Applying an underscan setup is just a matter of scaling all planes
appropriately and adjusting the CRTC X/Y offset to account for the
horizontal and vertical border.

Create an vc4_plane_underscan_adj() function doing that and call it from
vc4_plane_setup_clipping_and_scaling() so that we are ready to attach
underscan properties to the HDMI connector.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- Rebase on top of the "cursor rescaling" changes

Changes in v2:
- Take changes on hborder/vborder meaning into account
---
 drivers/gpu/drm/vc4/vc4_crtc.c  | 47 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_drv.h   |  3 ++
 drivers/gpu/drm/vc4/vc4_plane.c | 50 +++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 3ce136ba8791..3ace68186f07 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -49,6 +49,11 @@ struct vc4_crtc_state {
 	struct drm_mm_node mm;
 	bool feed_txp;
 	bool txp_armed;
+
+	struct {
+		unsigned int vborder;
+		unsigned int hborder;
+	} underscan;
 };
 
 static inline struct vc4_crtc_state *
@@ -624,6 +629,39 @@ static enum drm_mode_status vc4_crtc_mode_valid(struct drm_crtc *crtc,
 	return MODE_OK;
 }
 
+void vc4_crtc_get_underscan_borders(struct drm_crtc_state *state,
+				    unsigned int *vborder,
+				    unsigned int *hborder)
+{
+	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
+	struct drm_connector_state *conn_state;
+	struct drm_connector *conn;
+	int i;
+
+	*vborder = vc4_state->underscan.vborder;
+	*hborder = vc4_state->underscan.hborder;
+
+	/* We have to interate over all new connector states because
+	 * vc4_crtc_get_underscan_borders() might be called before
+	 * vc4_crtc_atomic_check() which means underscan info in vc4_crtc_state
+	 * might be outdated.
+	 */
+	for_each_new_connector_in_state(state->state, conn, conn_state, i) {
+		if (conn_state->crtc != state->crtc)
+			continue;
+
+		if (conn_state->underscan.mode == DRM_UNDERSCAN_ON) {
+			*vborder = conn_state->underscan.vborder;
+			*hborder = conn_state->underscan.hborder;
+		} else {
+			*vborder = 0;
+			*hborder = 0;
+		}
+
+		break;
+	}
+}
+
 static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
 				 struct drm_crtc_state *state)
 {
@@ -657,6 +695,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
 		return ret;
 
 	for_each_new_connector_in_state(state->state, conn, conn_state, i) {
+		unsigned int vborder = 0, hborder = 0;
 		if (conn_state->crtc != crtc)
 			continue;
 
@@ -671,6 +710,13 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
 			vc4_state->feed_txp = false;
 		}
 
+		if (conn_state->underscan.mode == DRM_UNDERSCAN_ON) {
+			vborder = conn_state->underscan.vborder;
+			hborder = conn_state->underscan.hborder;
+		}
+
+		vc4_state->underscan.vborder = vborder;
+		vc4_state->underscan.hborder = hborder;
 		break;
 	}
 
@@ -972,6 +1018,7 @@ static struct drm_crtc_state *vc4_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	old_vc4_state = to_vc4_crtc_state(crtc->state);
 	vc4_state->feed_txp = old_vc4_state->feed_txp;
+	vc4_state->underscan = old_vc4_state->underscan;
 
 	__drm_atomic_helper_crtc_duplicate_state(crtc, &vc4_state->base);
 	return &vc4_state->base;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index d1000c4805c2..ce08c5dc199d 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -707,6 +707,9 @@ bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 			     const struct drm_display_mode *mode);
 void vc4_crtc_handle_vblank(struct vc4_crtc *crtc);
 void vc4_crtc_txp_armed(struct drm_crtc_state *state);
+void vc4_crtc_get_underscan_borders(struct drm_crtc_state *state,
+				    unsigned int *vborder,
+				    unsigned int *hborder);
 
 /* vc4_debugfs.c */
 int vc4_debugfs_init(struct drm_minor *minor);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 8cda0d460a6d..78705d9ae25d 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -258,6 +258,52 @@ static u32 vc4_get_scl_field(struct drm_plane_state *state, int plane)
 	}
 }
 
+static int vc4_plane_underscan_adj(struct drm_plane_state *pstate)
+{
+	struct vc4_plane_state *vc4_pstate = to_vc4_plane_state(pstate);
+	unsigned int vborder, hborder, adjhdisplay, adjvdisplay;
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_new_crtc_state(pstate->state,
+						   pstate->crtc);
+
+	vc4_crtc_get_underscan_borders(crtc_state, &vborder, &hborder);
+	if (!vborder && !hborder)
+		return 0;
+
+	if (hborder * 2 >= crtc_state->mode.hdisplay ||
+	    vborder * 2 >= crtc_state->mode.vdisplay)
+		return -EINVAL;
+
+	adjhdisplay = crtc_state->mode.hdisplay - (2 * hborder);
+	vc4_pstate->crtc_x = DIV_ROUND_CLOSEST(vc4_pstate->crtc_x *
+					       adjhdisplay,
+					       crtc_state->mode.hdisplay);
+	vc4_pstate->crtc_x += hborder;
+	if (vc4_pstate->crtc_x > crtc_state->mode.hdisplay - hborder)
+		vc4_pstate->crtc_x = crtc_state->mode.hdisplay - hborder;
+
+	adjvdisplay = crtc_state->mode.vdisplay - (2 * vborder);
+	vc4_pstate->crtc_y = DIV_ROUND_CLOSEST(vc4_pstate->crtc_y *
+					       adjvdisplay,
+					       crtc_state->mode.vdisplay);
+	vc4_pstate->crtc_y += vborder;
+	if (vc4_pstate->crtc_y > crtc_state->mode.vdisplay - vborder)
+		vc4_pstate->crtc_y = crtc_state->mode.vdisplay - vborder;
+
+	vc4_pstate->crtc_w = DIV_ROUND_CLOSEST(vc4_pstate->crtc_w *
+					       adjhdisplay,
+					       crtc_state->mode.hdisplay);
+	vc4_pstate->crtc_h = DIV_ROUND_CLOSEST(vc4_pstate->crtc_h *
+					       adjvdisplay,
+					       crtc_state->mode.vdisplay);
+
+	if (!vc4_pstate->crtc_w || !vc4_pstate->crtc_h)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state)
 {
 	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
@@ -306,6 +352,10 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state)
 	vc4_state->crtc_w = state->dst.x2 - state->dst.x1;
 	vc4_state->crtc_h = state->dst.y2 - state->dst.y1;
 
+	ret = vc4_plane_underscan_adj(state);
+	if (ret)
+		return ret;
+
 	vc4_state->x_scaling[0] = vc4_get_scaling_mode(vc4_state->src_w[0],
 						       vc4_state->crtc_w);
 	vc4_state->y_scaling[0] = vc4_get_scaling_mode(vc4_state->src_h[0],
-- 
2.17.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v3 3/3] drm/vc4: Attach underscan props to the HDMI connector

[Boris Brezillon]

Now that the plane code takes the underscan setup into account, we can
safely attach the underscan props to the HDMI connector.

We also take care of filling AVI infoframes correctly to expose the
top/botton/left/right bar.

Note that these underscan props match pretty well the
overscan_{left,right,top,bottom} properties defined in config.txt and
parsed by the VC4 firmware.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- none

Changes in v2:
- none
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fd5522fd179e..b1b39256f4bc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -323,6 +323,16 @@ static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
 			   DRM_MODE_CONNECTOR_HDMIA);
 	drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
 
+	/* The hborder and vborder limit is arbitrarily set to 1024 which
+	 * should be more than enough for real use cases. Note that the actual
+	 * limitation comes from the display mode:
+	 *	hborder < hdisplay && vborder < vdisplay
+	 */
+	drm_connector_attach_underscan_properties(connector,
+						  BIT(DRM_UNDERSCAN_OFF) |
+						  BIT(DRM_UNDERSCAN_ON),
+						  1024, 1024);
+
 	connector->polled = (DRM_CONNECTOR_POLL_CONNECT |
 			     DRM_CONNECTOR_POLL_DISCONNECT);
 
@@ -408,6 +418,9 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder,
 static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 {
 	struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+	struct vc4_dev *vc4 = encoder->dev->dev_private;
+	struct vc4_hdmi *hdmi = vc4->hdmi;
+	struct drm_connector_state *cstate = hdmi->connector->state;
 	struct drm_crtc *crtc = encoder->crtc;
 	const struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 	union hdmi_infoframe frame;
@@ -426,6 +439,18 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 					   vc4_encoder->rgb_range_selectable,
 					   false);
 
+	if (cstate->underscan.mode == DRM_UNDERSCAN_ON) {
+		if (cstate->underscan.hborder) {
+			frame.avi.right_bar = cstate->underscan.hborder / 2;
+			frame.avi.left_bar = frame.avi.right_bar;
+		}
+
+		if (cstate->underscan.vborder) {
+			frame.avi.top_bar = cstate->underscan.vborder / 2;
+			frame.avi.bottom_bar = frame.avi.top_bar;
+		}
+	}
+
 	vc4_hdmi_write_infoframe(encoder, &frame);
 }
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Brian Starkey]

Hi Boris,

Just because I happened to read the docs in here, one typo below:

On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
>We have 3 drivers defining the "underscan", "underscan hborder" and
>"underscan vborder" properties (radeon, amd and nouveau) and we are
>about to add the same kind of thing in VC4.
>
>Define generic underscan props and add new fields to the drm_connector
>state so that the property parsing logic can be shared by all DRM
>drivers.
>
>A driver can now attach underscan properties to its connector through
>the drm_connector_attach_underscan_properties() helper, and can
>check/apply the underscan setup based on the
>drm_connector_state->underscan fields.
>
>Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>---
>Changes in v3:
>- None
>
>Changes in v2:
>- Add a new section in the connector props doc to describe underscan
>  props
>- Fix description of auto mode (auto means apply underscan for HDMI
>  monitors only)
>- Fix description of vborder/hborder:
>    right_border = left_border = hborder
>    top_border = bottom_border = vborder
>  not
>    right_border = left_border = hborder / 2
>    top_border = bottom_border = vborder / 2
>- Rename drm_underscan into drm_underscan_state
>---
> drivers/gpu/drm/drm_atomic_uapi.c |  12 +++
> drivers/gpu/drm/drm_connector.c   | 127 ++++++++++++++++++++++++++++++
> include/drm/drm_connector.h       |  80 +++++++++++++++++++
> 3 files changed, 219 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>index d5b7f315098c..39db6e31c565 100644
>--- a/drivers/gpu/drm/drm_atomic_uapi.c
>+++ b/drivers/gpu/drm/drm_atomic_uapi.c
>@@ -740,6 +740,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>
> 		return set_out_fence_for_connector(state->state, connector,
> 						   fence_ptr);
>+	} else if (property == connector->underscan_mode_property) {
>+		state->underscan.mode = val;
>+	} else if (property == connector->underscan_hborder_property) {
>+		state->underscan.hborder = val;
>+	} else if (property == connector->underscan_vborder_property) {
>+		state->underscan.vborder = val;
> 	} else if (connector->funcs->atomic_set_property) {
> 		return connector->funcs->atomic_set_property(connector,
> 				state, property, val);
>@@ -799,6 +805,12 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> 		*val = state->scaling_mode;
> 	} else if (property == connector->content_protection_property) {
> 		*val = state->content_protection;
>+	} else if (property == connector->underscan_mode_property) {
>+		*val = state->underscan.mode;
>+	} else if (property == connector->underscan_hborder_property) {
>+		*val = state->underscan.hborder;
>+	} else if (property == connector->underscan_vborder_property) {
>+		*val = state->underscan.vborder;
> 	} else if (property == config->writeback_fb_id_property) {
> 		/* Writeback framebuffer is one-shot, write and forget */
> 		*val = 0;
>diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>index c555e17ab8d7..170317248da6 100644
>--- a/drivers/gpu/drm/drm_connector.c
>+++ b/drivers/gpu/drm/drm_connector.c
>@@ -971,6 +971,38 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>  *	can also expose this property to external outputs, in which case they
>  *	must support "None", which should be the default (since external screens
>  *	have a built-in scaler).
>+ *
>+ * Connectors for non-analog outputs may also have standardized underscan
>+ * properties (drivers can set this up by calling
>+ * drm_connector_attach_content_protection_property() on initialization):

Should be drm_connector_attach_underscan_properties()

Cheers,
-Brian

>+ *
>+ * underscan:
>+ *	This properties defines whether underscan is activated or not, and when
>+ *	it is activated, how the horizontal and vertical borders are calculated:
>+ *
>+ *	off:
>+ *		Underscan is disabled. The output image shouldn't be scaled to
>+ *		take screen borders into account.
>+ *	on:
>+ *		Underscan is activated and horizontal and vertical borders are
>+ *		specified through the "underscan hborder" and
>+ *		"underscan vborder" properties.
>+ *	auto:
>+ *		Underscan is activated only for HDMI monitors.
>+ *
>+ * underscan hborder:
>+ *	Horizontal border expressed in pixels. The border is symmetric, which
>+ *	means you'll have a border of 'hborder' pixels on the left and on the
>+ *	same border on the right.
>+ *	When this value is 0 and underscan is "on" or "auto", the driver will
>+ *	pick a default value (the default value is driver dependent).
>+ *
>+ * underscan vborder:
>+ *	Vertical border expressed in pixels. The border is symmetric, which
>+ *	means you'll have a border of 'vborder' pixels on the top and the same
>+ *	border on the bottom.
>+ *	When this value is 0 and underscan is "on" or "auto", the driver will
>+ *	pick a default value (the default value is driver dependent).
>  */
>
> int drm_connector_create_standard_properties(struct drm_device *dev)
>@@ -1241,6 +1273,101 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_mode_create_tv_properties);
>
>+static const struct drm_prop_enum_list drm_underscan_mode_enum_list[] = {
>+	{ DRM_UNDERSCAN_OFF, "off" },
>+	{ DRM_UNDERSCAN_ON, "on" },
>+	{ DRM_UNDERSCAN_AUTO, "auto" },
>+};
>+
>+/**
>+ * drm_connector_attach_underscan_properties - attach atomic underscan
>+ *					       properties
>+ * @connector: connector to attach underscan mode properties on.
>+ * @mode_mask: bitmask of %DRM_UNDERSCAN_XX modes encoding the supported
>+ *	       underscan modes.
>+ * @max_hborder: maximum size of the horizontal border expressed in pixels.
>+ *		 Should be > 0.
>+ * @max_vborder: maximum size of the vertical border expressed in pixels.
>+ *		 Should be > 0.
>+ *
>+ * This is used to add support for underscan to atomic drivers.
>+ * The underscan config will be set to &drm_connector_state.underscan
>+ * and can be used from &drm_connector_helper_funcs->atomic_check for
>+ * validation.
>+ *
>+ * Returns:
>+ * Zero on success, negative errno on failure.
>+ */
>+int drm_connector_attach_underscan_properties(struct drm_connector *connector,
>+					      u32 mode_mask, u64 max_hborder,
>+					      u64 max_vborder)
>+{
>+	unsigned int i, nmodes = ARRAY_SIZE(drm_underscan_mode_enum_list);
>+	struct drm_device *dev = connector->dev;
>+	struct drm_property *prop;
>+
>+	if (!max_hborder || !max_vborder)
>+		return -EINVAL;
>+
>+	if (!hweight32(mode_mask) || (mode_mask & ~GENMASK(nmodes - 1, 0)))
>+		return -EINVAL;
>+
>+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, "underscan",
>+				   hweight32(mode_mask));
>+	if (!prop)
>+		return -ENOMEM;
>+
>+	for (i = 0; i < ARRAY_SIZE(drm_underscan_mode_enum_list); i++) {
>+		const struct drm_prop_enum_list *entry;
>+		int ret;
>+
>+		if (!(BIT(i) & mode_mask))
>+			continue;
>+
>+		entry = &drm_underscan_mode_enum_list[i];
>+		ret = drm_property_add_enum(prop, entry->type, entry->name);
>+		if (ret)
>+			goto err_free_mode_prop;
>+	}
>+
>+	connector->underscan_mode_property = prop;
>+
>+	prop = drm_property_create_range(dev, 0, "underscan hborder", 0,
>+					 max_hborder);
>+	if (!prop)
>+		goto err_free_mode_prop;
>+
>+	connector->underscan_hborder_property = prop;
>+
>+	prop = drm_property_create_range(dev, 0, "underscan vborder", 0,
>+					 max_vborder);
>+	if (!prop)
>+		goto err_free_hborder_prop;
>+
>+	connector->underscan_vborder_property = prop;
>+
>+	drm_object_attach_property(&connector->base,
>+				   connector->underscan_mode_property,
>+				   DRM_UNDERSCAN_OFF);
>+	drm_object_attach_property(&connector->base,
>+				   connector->underscan_hborder_property, 0);
>+	drm_object_attach_property(&connector->base,
>+				   connector->underscan_vborder_property, 0);
>+
>+	return 0;
>+
>+err_free_hborder_prop:
>+	drm_property_destroy(dev, connector->underscan_hborder_property);
>+	connector->underscan_hborder_property = NULL;
>+
>+err_free_mode_prop:
>+	drm_property_destroy(dev, connector->underscan_mode_property);
>+	connector->underscan_mode_property = NULL;
>+
>+	return -ENOMEM;
>+}
>+EXPORT_SYMBOL(drm_connector_attach_underscan_properties);
>+
> /**
>  * drm_mode_create_scaling_mode_property - create scaling mode property
>  * @dev: DRM device
>diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>index 5b3cf909fd5e..14d423dc7a14 100644
>--- a/include/drm/drm_connector.h
>+++ b/include/drm/drm_connector.h
>@@ -377,6 +377,53 @@ struct drm_tv_connector_state {
> 	unsigned int hue;
> };
>
>+/**
>+ * enum drm_underscan_mode - Underscan mode
>+ *
>+ * This enum is used to track the underscan mode.
>+ *
>+ * @DRM_UNDERSCAN_OFF: No underscan applied, the output image will be unchanged
>+ * @DRM_UNDERSCAN_ON: Underscan is enabled, and horizontal/vertical border size
>+ *		      are specified through the &struct_drm_underscan->hborder
>+ *		      and &struct_drm_underscan->vborder fields.
>+ * @DRM_UNDERSCAN_AUTO: Underscan is enabled and &struct_drm_underscan->hborder
>+ *			and &struct_drm_underscan->vborder are guessed by the
>+ *			driver.
>+ */
>+enum drm_underscan_mode {
>+	DRM_UNDERSCAN_OFF,
>+	DRM_UNDERSCAN_ON,
>+	DRM_UNDERSCAN_AUTO,
>+};
>+
>+/**
>+ * struct drm_underscan_state - Underscan properties attached to a connector
>+ *				state
>+ *
>+ * This can be used to tell the CRTC how the image should be scaled/placed in
>+ * order to cover fit in the display connected through this connector. Most of
>+ * the time used to address situations where the display borders are hidden.
>+ * Can also be used to compensate overscan done on the display side.
>+ */
>+struct drm_underscan_state {
>+	/**
>+	 * @mode: Underscan mode.
>+	 */
>+	enum drm_underscan_mode mode;
>+
>+	/**
>+	 * @hborder: Horizontal border. This values encodes both the left and
>+	 * right borders: left_border = right_border = hborder / 2.
>+	 */
>+	unsigned int hborder;
>+
>+	/**
>+	 * @vborder: Vertical border. This values encodes both the top and
>+	 * bottom borders: top_border = bottom_border = vborder / 2.
>+	 */
>+	unsigned int vborder;
>+};
>+
> /**
>  * struct drm_connector_state - mutable connector state
>  */
>@@ -461,6 +508,13 @@ struct drm_connector_state {
> 	 * drm_writeback_signal_completion()
> 	 */
> 	struct drm_writeback_job *writeback_job;
>+
>+	/**
>+	 * @underscan: Underscan information. Most commonly used to adjust
>+	 * image when the display has borders covering part of the image of
>+	 * when it's doing overscan on its own.
>+	 */
>+	struct drm_underscan_state underscan;
> };
>
> /**
>@@ -924,6 +978,29 @@ struct drm_connector {
> 	 */
> 	struct drm_property_blob *path_blob_ptr;
>
>+	/**
>+	 * @underscan_mode_property: Optional connector underscan mode. Used by
>+	 * the driver to scale the output image and compensate an overscan done
>+	 * on the display side.
>+	 */
>+	struct drm_property *underscan_mode_property;
>+
>+	/**
>+	 * @underscan_hborder_property: Optional connector underscan horizontal
>+	 * border (expressed in pixels). Used by the driver to adjust the
>+	 * output image position and compensate an overscan done on the display
>+	 * side.
>+	 */
>+	struct drm_property *underscan_hborder_property;
>+
>+	/**
>+	 * @underscan_hborder_property: Optional connector underscan vertical
>+	 * border (expressed in pixels). Used by the driver to adjust the
>+	 * output image position and compensate an overscan done on the display
>+	 * side.
>+	 */
>+	struct drm_property *underscan_vborder_property;
>+
> #define DRM_CONNECTOR_POLL_HPD (1 << 0)
> #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
> #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
>@@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> int drm_mode_create_tv_properties(struct drm_device *dev,
> 				  unsigned int num_modes,
> 				  const char * const modes[]);
>+int drm_connector_attach_underscan_properties(struct drm_connector *connector,
>+					      u32 mode_mask, u64 max_hborder,
>+					      u64 max_vborder);
> int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> int drm_connector_attach_content_type_property(struct drm_connector *dev);
> int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
>-- 
>2.17.1
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1032 bytes --]

Brian Starkey <Brian.Starkey@arm.com> writes:

> Hi Boris,
>
> Just because I happened to read the docs in here, one typo below:
>
> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
>>diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>index c555e17ab8d7..170317248da6 100644
>>--- a/drivers/gpu/drm/drm_connector.c
>>+++ b/drivers/gpu/drm/drm_connector.c
>>@@ -971,6 +971,38 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>>  *	can also expose this property to external outputs, in which case they
>>  *	must support "None", which should be the default (since external screens
>>  *	have a built-in scaler).
>>+ *
>>+ * Connectors for non-analog outputs may also have standardized underscan
>>+ * properties (drivers can set this up by calling
>>+ * drm_connector_attach_content_protection_property() on initialization):
>
> Should be drm_connector_attach_underscan_properties()

Other than this typo, this series is:

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Ville Syrjälä]

On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
> @@ -924,6 +978,29 @@ struct drm_connector {
>  	 */
>  	struct drm_property_blob *path_blob_ptr;
>  
> +	/**
> +	 * @underscan_mode_property: Optional connector underscan mode. Used by
> +	 * the driver to scale the output image and compensate an overscan done
> +	 * on the display side.
> +	 */
> +	struct drm_property *underscan_mode_property;
> +
> +	/**
> +	 * @underscan_hborder_property: Optional connector underscan horizontal
> +	 * border (expressed in pixels). Used by the driver to adjust the
> +	 * output image position and compensate an overscan done on the display
> +	 * side.
> +	 */
> +	struct drm_property *underscan_hborder_property;
> +
> +	/**
> +	 * @underscan_hborder_property: Optional connector underscan vertical
> +	 * border (expressed in pixels). Used by the driver to adjust the
> +	 * output image position and compensate an overscan done on the display
> +	 * side.
> +	 */
> +	struct drm_property *underscan_vborder_property;

I'm wondering why we're adding these new props when we already have the
(slightly more flexible) margin properties for TV out. We could just
reuse those AFAICS.

> +
>  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev);
>  int drm_mode_create_tv_properties(struct drm_device *dev,
>  				  unsigned int num_modes,
>  				  const char * const modes[]);
> +int drm_connector_attach_underscan_properties(struct drm_connector *connector,
> +					      u32 mode_mask, u64 max_hborder,
> +					      u64 max_vborder);
>  int drm_mode_create_scaling_mode_property(struct drm_device *dev);
>  int drm_connector_attach_content_type_property(struct drm_connector *dev);
>  int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Boris Brezillon]

On Mon, 3 Dec 2018 16:40:11 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
> > @@ -924,6 +978,29 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property_blob *path_blob_ptr;
> >  
> > +	/**
> > +	 * @underscan_mode_property: Optional connector underscan mode. Used by
> > +	 * the driver to scale the output image and compensate an overscan done
> > +	 * on the display side.
> > +	 */
> > +	struct drm_property *underscan_mode_property;
> > +
> > +	/**
> > +	 * @underscan_hborder_property: Optional connector underscan horizontal
> > +	 * border (expressed in pixels). Used by the driver to adjust the
> > +	 * output image position and compensate an overscan done on the display
> > +	 * side.
> > +	 */
> > +	struct drm_property *underscan_hborder_property;
> > +
> > +	/**
> > +	 * @underscan_hborder_property: Optional connector underscan vertical
> > +	 * border (expressed in pixels). Used by the driver to adjust the
> > +	 * output image position and compensate an overscan done on the display
> > +	 * side.
> > +	 */
> > +	struct drm_property *underscan_vborder_property;  
> 
> I'm wondering why we're adding these new props when we already have the
> (slightly more flexible) margin properties for TV out. We could just
> reuse those AFAICS.

I'm not against the idea, but I can't use
drm_mode_create_tv_properties() directly, as most props created by this
function are not applicable to an HDMI displays. Should I move the
margins props out of the tv_connector_state and provide new helpers to
create those props?

> 
> > +
> >  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
> >  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
> >  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> > @@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> >  int drm_mode_create_tv_properties(struct drm_device *dev,
> >  				  unsigned int num_modes,
> >  				  const char * const modes[]);
> > +int drm_connector_attach_underscan_properties(struct drm_connector *connector,
> > +					      u32 mode_mask, u64 max_hborder,
> > +					      u64 max_vborder);
> >  int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> >  int drm_connector_attach_content_type_property(struct drm_connector *dev);
> >  int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
> > -- 
> > 2.17.1  
> 

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

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1855 bytes --]

Boris Brezillon <boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> writes:

> On Mon, 3 Dec 2018 16:40:11 +0200
> Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>
>> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
>> > @@ -924,6 +978,29 @@ struct drm_connector {
>> >  	 */
>> >  	struct drm_property_blob *path_blob_ptr;
>> >  
>> > +	/**
>> > +	 * @underscan_mode_property: Optional connector underscan mode. Used by
>> > +	 * the driver to scale the output image and compensate an overscan done
>> > +	 * on the display side.
>> > +	 */
>> > +	struct drm_property *underscan_mode_property;
>> > +
>> > +	/**
>> > +	 * @underscan_hborder_property: Optional connector underscan horizontal
>> > +	 * border (expressed in pixels). Used by the driver to adjust the
>> > +	 * output image position and compensate an overscan done on the display
>> > +	 * side.
>> > +	 */
>> > +	struct drm_property *underscan_hborder_property;
>> > +
>> > +	/**
>> > +	 * @underscan_hborder_property: Optional connector underscan vertical
>> > +	 * border (expressed in pixels). Used by the driver to adjust the
>> > +	 * output image position and compensate an overscan done on the display
>> > +	 * side.
>> > +	 */
>> > +	struct drm_property *underscan_vborder_property;  
>> 
>> I'm wondering why we're adding these new props when we already have the
>> (slightly more flexible) margin properties for TV out. We could just
>> reuse those AFAICS.
>
> I'm not against the idea, but I can't use
> drm_mode_create_tv_properties() directly, as most props created by this
> function are not applicable to an HDMI displays. Should I move the
> margins props out of the tv_connector_state and provide new helpers to
> create those props?

TV margin props look good to me, FWIW.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 1/3] drm/connector: Add generic underscan properties

[Daniel Vetter]

On Mon, Dec 03, 2018 at 07:50:53AM -0800, Eric Anholt wrote:
> Boris Brezillon <boris.brezillon@bootlin.com> writes:
> 
> > On Mon, 3 Dec 2018 16:40:11 +0200
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> >> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
> >> > @@ -924,6 +978,29 @@ struct drm_connector {
> >> >  	 */
> >> >  	struct drm_property_blob *path_blob_ptr;
> >> >  
> >> > +	/**
> >> > +	 * @underscan_mode_property: Optional connector underscan mode. Used by
> >> > +	 * the driver to scale the output image and compensate an overscan done
> >> > +	 * on the display side.
> >> > +	 */
> >> > +	struct drm_property *underscan_mode_property;
> >> > +
> >> > +	/**
> >> > +	 * @underscan_hborder_property: Optional connector underscan horizontal
> >> > +	 * border (expressed in pixels). Used by the driver to adjust the
> >> > +	 * output image position and compensate an overscan done on the display
> >> > +	 * side.
> >> > +	 */
> >> > +	struct drm_property *underscan_hborder_property;
> >> > +
> >> > +	/**
> >> > +	 * @underscan_hborder_property: Optional connector underscan vertical
> >> > +	 * border (expressed in pixels). Used by the driver to adjust the
> >> > +	 * output image position and compensate an overscan done on the display
> >> > +	 * side.
> >> > +	 */
> >> > +	struct drm_property *underscan_vborder_property;  
> >> 
> >> I'm wondering why we're adding these new props when we already have the
> >> (slightly more flexible) margin properties for TV out. We could just
> >> reuse those AFAICS.
> >
> > I'm not against the idea, but I can't use
> > drm_mode_create_tv_properties() directly, as most props created by this
> > function are not applicable to an HDMI displays. Should I move the
> > margins props out of the tv_connector_state and provide new helpers to
> > create those props?
> 
> TV margin props look good to me, FWIW.

Yeah extracting the margin props from the tv props sounds like a good
idea. If we go full ocd we'd also split out margin_connector_state or
something like that (should be doable with some cocci), but not sure
that's fully worth it. Tuning margins is largely an analog TV issue I
think, so could just leave them there.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel