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.
Note that I use this new infrastructure to support underscan in VC4 (path 2 and 3) but did not patch existing drivers yet, mainly because I don't want to do this work before making sure I got the generic bits right.
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.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++ drivers/gpu/drm/vc4/vc4_plane.c | 49 +++++++++++++++- include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 5 files changed, 283 insertions(+), 1 deletion(-)
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 --- drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val; + } 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); @@ -1359,6 +1365,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 (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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). + * + * 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 and horizontal and vertical borders are + * automatically chosen by the driver. + * + * underscan hborder: + * Horizontal border expressed in pixels. The border is symmetric, which + * means you'll have half of this value placed on the left and the other + * half on the right. + * + * underscan vborder: + * Vertical border expressed in pixels. The border is symmetric, which + * means you'll have half of this value placed on the top and the other + * half on the bottom. */
int drm_connector_create_standard_properties(struct drm_device *dev) @@ -1108,6 +1133,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 675cc3f8cf85..46380ee6d4ee 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,52 @@ 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 - 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 { + /** + * @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 * @connector: backpointer to the connector @@ -429,6 +475,12 @@ struct drm_connector_state { * protection. This is most commonly used for HDCP. */ unsigned int content_protection; + + /** + * @underscan: Underscan information. Most commonly used to adjust + * image when the display is doing overscan. + */ + struct drm_underscan underscan; };
/** @@ -892,6 +944,29 @@ struct drm_connector { */ struct drm_property_blob *tile_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; + /* should we poll this connector for connects and disconnects */ /* hot plug detectable */ #define DRM_CONNECTOR_POLL_HPD (1 << 0) @@ -1088,6 +1163,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_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask);
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
- 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.
How is the output scaled? What does the user mode hdisplay/vdisplay mean in this case? What if I want underscan without scaling?
- auto:
Underscan is activated and horizontal and vertical borders are
automatically chosen by the driver.
Seems overly vague to be useful. You didn't even seem to implement it for vc4.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
Seems like a slightly odd way to specify this. I think for the TV margins we have one value for each edge.
- underscan vborder:
- Vertical border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the top and the other
*/
- half on the bottom.
int drm_connector_create_standard_properties(struct drm_device *dev) @@ -1108,6 +1133,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 675cc3f8cf85..46380ee6d4ee 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,52 @@ 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 - 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 {
- /**
* @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
- @connector: backpointer to the connector
@@ -429,6 +475,12 @@ struct drm_connector_state { * protection. This is most commonly used for HDCP. */ unsigned int content_protection;
- /**
* @underscan: Underscan information. Most commonly used to adjust* image when the display is doing overscan.*/- struct drm_underscan underscan;
};
/** @@ -892,6 +944,29 @@ struct drm_connector { */ struct drm_property_blob *tile_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;
/* should we poll this connector for connects and disconnects */ /* hot plug detectable */ #define DRM_CONNECTOR_POLL_HPD (1 << 0) @@ -1088,6 +1163,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_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); -- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, 7 May 2018 18:01:44 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
- 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.How is the output scaled?
In HW. The formula is
hfactor = (hdisplay - hborder) / hdisplay vfactor = (vdisplay - vborder) / vdisplay
What does the user mode hdisplay/vdisplay mean in this case?
The same as before this patch: the output resolution. You just add black margins.
What if I want underscan without scaling?
Then don't involve the DRM driver and do that from userspace: just fill the visible portion of the framebuffer and leave the rest black. There nothing the DRM driver can do to help with that, except maybe exposing the information about the active area of the screen. It would be nice to do that, but that means patching all userspace libs to take this into account.
- auto:
Underscan is activated and horizontal and vertical borders are
automatically chosen by the driver.Seems overly vague to be useful. You didn't even seem to implement it for vc4.
Because I don't understand it either. I was just trying to keep things working for drivers already exposing these properties.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
Seems like a slightly odd way to specify this. I think for the TV margins we have one value for each edge.
Again, I just wanted existing drivers to keep working with the generic solution, but maybe we shouldn't care.
On 2018-05-07 12:19 PM, Boris Brezillon wrote:
On Mon, 7 May 2018 18:01:44 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
- 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.How is the output scaled?
In HW. The formula is
hfactor = (hdisplay - hborder) / hdisplay vfactor = (vdisplay - vborder) / vdisplay
What does the user mode hdisplay/vdisplay mean in this case?
The same as before this patch: the output resolution. You just add black margins.
What if I want underscan without scaling?
Then don't involve the DRM driver and do that from userspace: just fill the visible portion of the framebuffer and leave the rest black. There nothing the DRM driver can do to help with that, except maybe exposing the information about the active area of the screen. It would be nice to do that, but that means patching all userspace libs to take this into account.
- auto:
Underscan is activated and horizontal and vertical borders are
automatically chosen by the driver.Seems overly vague to be useful. You didn't even seem to implement it for vc4.
FWIW, amdgpu treats UNDERSCAN_AUTO like UNDERSCAN_ON. radeon and nouveau seem to do the same. So there's probably no need for auto.
Harry
Because I don't understand it either. I was just trying to keep things working for drivers already exposing these properties.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
Seems like a slightly odd way to specify this. I think for the TV margins we have one value for each edge.
Again, I just wanted existing drivers to keep working with the generic solution, but maybe we shouldn't care.
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 8 May 2018 at 04:26, Harry Wentland harry.wentland@amd.com wrote:
On 2018-05-07 12:19 PM, Boris Brezillon wrote:
On Mon, 7 May 2018 18:01:44 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
- 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.How is the output scaled?
In HW. The formula is
hfactor = (hdisplay - hborder) / hdisplay vfactor = (vdisplay - vborder) / vdisplayWhat does the user mode hdisplay/vdisplay mean in this case?
The same as before this patch: the output resolution. You just add black margins.
What if I want underscan without scaling?
Then don't involve the DRM driver and do that from userspace: just fill the visible portion of the framebuffer and leave the rest black. There nothing the DRM driver can do to help with that, except maybe exposing the information about the active area of the screen. It would be nice to do that, but that means patching all userspace libs to take this into account.
- auto:
Underscan is activated and horizontal and vertical borders are
automatically chosen by the driver.Seems overly vague to be useful. You didn't even seem to implement it for vc4.
FWIW, amdgpu treats UNDERSCAN_AUTO like UNDERSCAN_ON. radeon and nouveau seem to do the same. So there's probably no need for auto.
They're not the same. UNDERSCAN_AUTO in both nouveau and radeon attempt to enable it by default for HDMI displays that would otherwise chop the edges off the displayed image. Whereas UNDERSCAN_ON is unconditional.
Ben.
Harry
Because I don't understand it either. I was just trying to keep things working for drivers already exposing these properties.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
Seems like a slightly odd way to specify this. I think for the TV margins we have one value for each edge.
Again, I just wanted existing drivers to keep working with the generic solution, but maybe we shouldn't care.
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 8 May 2018 10:18:10 +1000 Ben Skeggs skeggsb@gmail.com wrote:
On 8 May 2018 at 04:26, Harry Wentland harry.wentland@amd.com wrote:
On 2018-05-07 12:19 PM, Boris Brezillon wrote:
On Mon, 7 May 2018 18:01:44 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
- 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.How is the output scaled?
In HW. The formula is
hfactor = (hdisplay - hborder) / hdisplay vfactor = (vdisplay - vborder) / vdisplayWhat does the user mode hdisplay/vdisplay mean in this case?
The same as before this patch: the output resolution. You just add black margins.
What if I want underscan without scaling?
Then don't involve the DRM driver and do that from userspace: just fill the visible portion of the framebuffer and leave the rest black. There nothing the DRM driver can do to help with that, except maybe exposing the information about the active area of the screen. It would be nice to do that, but that means patching all userspace libs to take this into account.
- auto:
Underscan is activated and horizontal and vertical borders are
automatically chosen by the driver.Seems overly vague to be useful. You didn't even seem to implement it for vc4.
FWIW, amdgpu treats UNDERSCAN_AUTO like UNDERSCAN_ON. radeon and nouveau seem to do the same. So there's probably no need for auto.
They're not the same. UNDERSCAN_AUTO in both nouveau and radeon attempt to enable it by default for HDMI displays that would otherwise chop the edges off the displayed image. Whereas UNDERSCAN_ON is unconditional.
Actually, It's also true for amdgpu, I just didn't notice that when I first read the code (so many parenthesis that I mixed the || and && scope).
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
I think a new section here would be good, to make it more obvious this is a group of properties. Plus a reference to drm_connector_attach_underscan_properties().
- 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 and horizontal and vertical borders are
automatically chosen by the driver.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
- underscan vborder:
- Vertical border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the top and the other
*/
- half on the bottom.
int drm_connector_create_standard_properties(struct drm_device *dev) @@ -1108,6 +1133,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 675cc3f8cf85..46380ee6d4ee 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,52 @@ 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 - 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 {
I'd call this drm_underscan_state, similar to how we have drm_tv_connector_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
- @connector: backpointer to the connector
@@ -429,6 +475,12 @@ struct drm_connector_state { * protection. This is most commonly used for HDCP. */ unsigned int content_protection;
- /**
* @underscan: Underscan information. Most commonly used to adjust* image when the display is doing overscan.*/- struct drm_underscan underscan;
};
/** @@ -892,6 +944,29 @@ struct drm_connector { */ struct drm_property_blob *tile_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;
Since you don't allow drivers to have a per-connector value range for these properties these should be in drm_mode_config I think.
Yes the content_protection_property should probably have the same treatment. Note that drm_property_blob does not derive from drm_property, so that one is ok to be there. And the scaling property is generally per-output. -Daniel
/* should we poll this connector for connects and disconnects */ /* hot plug detectable */ #define DRM_CONNECTOR_POLL_HPD (1 << 0) @@ -1088,6 +1163,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_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); -- 2.14.1
On Mon, May 07, 2018 at 05:15:33PM +0200, Daniel Vetter wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
I think a new section here would be good, to make it more obvious this is a group of properties. Plus a reference to drm_connector_attach_underscan_properties().
- 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 and horizontal and vertical borders are
automatically chosen by the driver.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
- underscan vborder:
- Vertical border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the top and the other
*/
- half on the bottom.
int drm_connector_create_standard_properties(struct drm_device *dev) @@ -1108,6 +1133,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 675cc3f8cf85..46380ee6d4ee 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,52 @@ 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 - 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 {
I'd call this drm_underscan_state, similar to how we have drm_tv_connector_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
- @connector: backpointer to the connector
@@ -429,6 +475,12 @@ struct drm_connector_state { * protection. This is most commonly used for HDCP. */ unsigned int content_protection;
- /**
* @underscan: Underscan information. Most commonly used to adjust* image when the display is doing overscan.*/- struct drm_underscan underscan;
};
/** @@ -892,6 +944,29 @@ struct drm_connector { */ struct drm_property_blob *tile_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;
Since you don't allow drivers to have a per-connector value range for these properties these should be in drm_mode_config I think.
Yes the content_protection_property should probably have the same treatment. Note that drm_property_blob does not derive from drm_property, so that one is ok to be there. And the scaling property is generally per-output.
Ah just noticed that the underscan_mode_property is potentially per-connector. I guess we can keep them here then. -Daniel
-Daniel
/* should we poll this connector for connects and disconnects */ /* hot plug detectable */ #define DRM_CONNECTOR_POLL_HPD (1 << 0) @@ -1088,6 +1163,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_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); -- 2.14.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Mon, 7 May 2018 17:25:30 +0200 Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 07, 2018 at 05:15:33PM +0200, Daniel Vetter wrote:
On Mon, May 07, 2018 at 04:44:32PM +0200, 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
drivers/gpu/drm/drm_atomic.c | 12 ++++ drivers/gpu/drm/drm_connector.c | 120 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 78 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..b7312bd172c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1278,6 +1278,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, return -EINVAL; } state->content_protection = val;
- } 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;@@ -1359,6 +1365,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;diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index dfc8ca1e9413..9937390b8a25 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,31 @@ 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).
I think a new section here would be good, to make it more obvious this is a group of properties. Plus a reference to drm_connector_attach_underscan_properties().
- 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 and horizontal and vertical borders are
automatically chosen by the driver.
- underscan hborder:
- Horizontal border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the left and the other
- half on the right.
- underscan vborder:
- Vertical border expressed in pixels. The border is symmetric, which
- means you'll have half of this value placed on the top and the other
*/
- half on the bottom.
int drm_connector_create_standard_properties(struct drm_device *dev) @@ -1108,6 +1133,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 675cc3f8cf85..46380ee6d4ee 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,52 @@ 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 - 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 {
I'd call this drm_underscan_state, similar to how we have drm_tv_connector_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
- @connector: backpointer to the connector
@@ -429,6 +475,12 @@ struct drm_connector_state { * protection. This is most commonly used for HDCP. */ unsigned int content_protection;
- /**
* @underscan: Underscan information. Most commonly used to adjust* image when the display is doing overscan.*/- struct drm_underscan underscan;
};
/** @@ -892,6 +944,29 @@ struct drm_connector { */ struct drm_property_blob *tile_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;
Since you don't allow drivers to have a per-connector value range for these properties these should be in drm_mode_config I think.
I do allow that. See the max_vborder, max_hborder args in the drm_connector_attach_underscan_properties() prototype.
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on anholt/for-next] [also build test ERROR on v4.17-rc4 next-20180507] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-connector-Provi... base: https://github.com/anholt/linux for-next config: x86_64-randconfig-x010-201818 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/gpu//drm/drm_connector.c: In function 'drm_connector_attach_underscan_properties':
drivers/gpu//drm/drm_connector.c:1188:50: warning: passing argument 3 of 'drm_property_add_enum' makes integer from pointer without a cast [-Wint-conversion]
ret = drm_property_add_enum(prop, entry->type, entry->name); ^~~~~ In file included from include/drm/drm_crtc.h:42:0, from include/drm/drmP.h:69, from drivers/gpu//drm/drm_connector.c:23: include/drm/drm_property.h:263:5: note: expected 'uint64_t {aka long long unsigned int}' but argument is of type 'const char * const' int drm_property_add_enum(struct drm_property *property, int index, ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu//drm/drm_connector.c:1188:9: error: too few arguments to function 'drm_property_add_enum'
ret = drm_property_add_enum(prop, entry->type, entry->name); ^~~~~~~~~~~~~~~~~~~~~ In file included from include/drm/drm_crtc.h:42:0, from include/drm/drmP.h:69, from drivers/gpu//drm/drm_connector.c:23: include/drm/drm_property.h:263:5: note: declared here int drm_property_add_enum(struct drm_property *property, int index, ^~~~~~~~~~~~~~~~~~~~~
vim +/drm_property_add_enum +1188 drivers/gpu//drm/drm_connector.c
1141 1142 /** 1143 * drm_connector_attach_underscan_properties - attach atomic underscan 1144 * properties 1145 * @connector: connector to attach underscan mode properties on. 1146 * @mode_mask: bitmask of %DRM_UNDERSCAN_XX modes encoding the supported 1147 * underscan modes. 1148 * @max_hborder: maximum size of the horizontal border expressed in pixels. 1149 * Should be > 0. 1150 * @max_vborder: maximum size of the vertical border expressed in pixels. 1151 * Should be > 0. 1152 * 1153 * This is used to add support for underscan to atomic drivers. 1154 * The underscan config will be set to &drm_connector_state.underscan 1155 * and can be used from &drm_connector_helper_funcs->atomic_check for 1156 * validation. 1157 * 1158 * Returns: 1159 * Zero on success, negative errno on failure. 1160 */ 1161 int drm_connector_attach_underscan_properties(struct drm_connector *connector, 1162 u32 mode_mask, u64 max_hborder, 1163 u64 max_vborder) 1164 { 1165 unsigned int i, nmodes = ARRAY_SIZE(drm_underscan_mode_enum_list); 1166 struct drm_device *dev = connector->dev; 1167 struct drm_property *prop; 1168 1169 if (!max_hborder || !max_vborder) 1170 return -EINVAL; 1171 1172 if (!hweight32(mode_mask) || (mode_mask & ~GENMASK(nmodes - 1, 0))) 1173 return -EINVAL; 1174 1175 prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, "underscan", 1176 hweight32(mode_mask)); 1177 if (!prop) 1178 return -ENOMEM; 1179 1180 for (i = 0; i < ARRAY_SIZE(drm_underscan_mode_enum_list); i++) { 1181 const struct drm_prop_enum_list *entry; 1182 int ret; 1183 1184 if (!(BIT(i) & mode_mask)) 1185 continue; 1186 1187 entry = &drm_underscan_mode_enum_list[i];
1188 ret = drm_property_add_enum(prop, entry->type, entry->name);
1189 if (ret) 1190 goto err_free_mode_prop; 1191 } 1192 1193 connector->underscan_mode_property = prop; 1194 1195 prop = drm_property_create_range(dev, 0, "underscan hborder", 0, 1196 max_hborder); 1197 if (!prop) 1198 goto err_free_mode_prop; 1199 1200 connector->underscan_hborder_property = prop; 1201 1202 prop = drm_property_create_range(dev, 0, "underscan vborder", 0, 1203 max_vborder); 1204 if (!prop) 1205 goto err_free_hborder_prop; 1206 1207 connector->underscan_vborder_property = prop; 1208 1209 drm_object_attach_property(&connector->base, 1210 connector->underscan_mode_property, 1211 DRM_UNDERSCAN_OFF); 1212 drm_object_attach_property(&connector->base, 1213 connector->underscan_hborder_property, 0); 1214 drm_object_attach_property(&connector->base, 1215 connector->underscan_vborder_property, 0); 1216 1217 return 0; 1218 1219 err_free_hborder_prop: 1220 drm_property_destroy(dev, connector->underscan_hborder_property); 1221 connector->underscan_hborder_property = NULL; 1222 1223 err_free_mode_prop: 1224 drm_property_destroy(dev, connector->underscan_mode_property); 1225 connector->underscan_mode_property = NULL; 1226 1227 return -ENOMEM; 1228 } 1229 EXPORT_SYMBOL(drm_connector_attach_underscan_properties); 1230
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 --- drivers/gpu/drm/vc4/vc4_plane.c | 49 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 71d44c357d35..7d5667b1f990 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -258,6 +258,49 @@ 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); + struct drm_connector_state *conn_state = NULL; + struct drm_connector *conn; + struct drm_crtc_state *crtc_state; + int i; + + for_each_new_connector_in_state(pstate->state, conn, conn_state, i) { + if (conn_state->crtc == pstate->crtc) + break; + } + + if (i == pstate->state->num_connector) + return 0; + + if (conn_state->underscan.mode != DRM_UNDERSCAN_ON) + return 0; + + crtc_state = drm_atomic_get_new_crtc_state(pstate->state, + pstate->crtc); + + if (conn_state->underscan.hborder >= crtc_state->mode.hdisplay || + conn_state->underscan.vborder >= crtc_state->mode.vdisplay) + return -EINVAL; + + vc4_pstate->crtc_x += conn_state->underscan.hborder / 2; + vc4_pstate->crtc_y += conn_state->underscan.vborder / 2; + vc4_pstate->crtc_w = (vc4_pstate->crtc_w * + (crtc_state->mode.hdisplay - + conn_state->underscan.hborder)) / + crtc_state->mode.hdisplay; + vc4_pstate->crtc_h = (vc4_pstate->crtc_h * + (crtc_state->mode.vdisplay - + conn_state->underscan.vborder)) / + 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 drm_plane *plane = state->plane; @@ -269,7 +312,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) int num_planes = fb->format->num_planes; u32 h_subsample = 1; u32 v_subsample = 1; - int i; + int i, ret;
for (i = 0; i < num_planes; i++) vc4_state->offsets[i] = bo->paddr + fb->offsets[i]; @@ -292,6 +335,10 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) vc4_state->crtc_w = state->crtc_w; vc4_state->crtc_h = state->crtc_h;
+ 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],
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 --- 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 1a6db291d48b..17464b5981f9 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); }
On Mon, May 07, 2018 at 04:44:34PM +0200, Boris Brezillon wrote:
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
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 1a6db291d48b..17464b5981f9 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,
We should probably sprinkle __must_check over all these :-) -Daniel
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.14.1
On Mon, 7 May 2018 17:24:08 +0200 Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 07, 2018 at 04:44:34PM +0200, Boris Brezillon wrote:
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
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 1a6db291d48b..17464b5981f9 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,
We should probably sprinkle __must_check over all these :-)
I'm perfectly fine adding __must_check to drm_connector_attach_underscan_properties(), but I'm definitely not volunteering for a massive __must_check sanitization :P.
On Wed, May 09, 2018 at 04:52:38PM +0200, Boris Brezillon wrote:
On Mon, 7 May 2018 17:24:08 +0200 Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 07, 2018 at 04:44:34PM +0200, Boris Brezillon wrote:
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
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 1a6db291d48b..17464b5981f9 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,
We should probably sprinkle __must_check over all these :-)
I'm perfectly fine adding __must_check to drm_connector_attach_underscan_properties(), but I'm definitely not volunteering for a massive __must_check sanitization :P.
Yeah I just meant for this one here. The others might be a good candidate for Documentation/gpu/todo.rst. -Daniel
Hi,
On Mon, 7 May 2018 16:44:31 +0200 Boris Brezillon boris.brezillon@bootlin.com wrote:
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.
Note that I use this new infrastructure to support underscan in VC4 (path 2 and 3) but did not patch existing drivers yet, mainly because I don't want to do this work before making sure I got the generic bits right.
Thanks everyone for your reviews. After the discussion we had on IRC and the feedback I had on this patchset it's a bit unclear to me what the next iteration should look like. Should I continue with the underscan props, should I use TV margins exposed by the TV connector state, should I create new props?
Remember that all I need is a way to define margins in order to let the VC4 HW scaler shrink the planes and adjust their positions on the screen.
Thanks,
Boris
dri-devel@lists.freedesktop.org
-
Ben Skeggs -
Boris Brezillon -
Daniel Vetter -
Harry Wentland -
kbuild test robot -
Ville Syrjälä