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.
In this v2, I also converted the nouveau driver to the generic approach. The amdgpu and radeon ones can't be converted since they're still not converted to the atomic interface.
Regards,
Boris
Boris Brezillon (4): 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 drm/nouveau: Switch to the generic underscan props
drivers/gpu/drm/drm_atomic.c | 12 +++ drivers/gpu/drm/drm_connector.c | 127 ++++++++++++++++++++++++++++ drivers/gpu/drm/nouveau/nouveau_connector.c | 39 ++------- drivers/gpu/drm/nouveau/nouveau_connector.h | 9 -- drivers/gpu/drm/nouveau/nouveau_display.c | 14 --- drivers/gpu/drm/nouveau/nouveau_display.h | 3 - drivers/gpu/drm/nouveau/nv50_display.c | 17 ++-- drivers/gpu/drm/vc4/vc4_hdmi.c | 25 ++++++ drivers/gpu/drm/vc4/vc4_plane.c | 49 ++++++++++- include/drm/drm_connector.h | 80 ++++++++++++++++++ 10 files changed, 310 insertions(+), 65 deletions(-)
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 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.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.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 9b9ba5d5ec0c..826af6267a4f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -914,6 +914,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) @@ -1108,6 +1140,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..5ad968dc48c3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -372,6 +372,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 * @connector: backpointer to the connector @@ -429,6 +476,13 @@ 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 has borders covering part of the image of + * when it's doing overscan on its own. + */ + struct drm_underscan_state underscan; };
/** @@ -892,6 +946,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 +1165,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);
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 v2: - Take changes on hborder/vborder meaning into account --- 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..61ed60841cd6 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; + vc4_pstate->crtc_y += conn_state->underscan.vborder; + vc4_pstate->crtc_w = (vc4_pstate->crtc_w * + (crtc_state->mode.hdisplay - + (conn_state->underscan.hborder * 2))) / + crtc_state->mode.hdisplay; + vc4_pstate->crtc_h = (vc4_pstate->crtc_h * + (crtc_state->mode.vdisplay - + (conn_state->underscan.vborder * 2))) / + 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],
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;
border * 2 ?
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;
So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop. Maybe we just have to require the scaling mode to be set to fullscreen to allow borders to be specified explictly?
- 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],
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
Maybe we just have to require the scaling mode to be set to fullscreen to allow borders to be specified explictly?
- 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],
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode? I guess the latter would be a bit easier (would also match how the TV margins behave I suppose). But that does leave open the question of how would generic userspace know which case it's dealing with? Eg. if it wants to underscan by some specific percentage it has to calculate the borders based on the correct mode, otherwise the results aren't going to match the expectations.
Maybe we just have to require the scaling mode to be set to fullscreen to allow borders to be specified explictly?
- 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],
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode?
Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode that is about to be applied, no matter if it's a usermode or one of the mode returned by the display?
I guess the latter would be a bit easier (would also match how the TV margins behave I suppose). But that does leave open the question of how would generic userspace know which case it's dealing with? Eg. if it wants to underscan by some specific percentage it has to calculate the borders based on the correct mode, otherwise the results aren't going to match the expectations.
I don't get it, sorry. Borders are relative to the mode applied by userspace. So if it needs to express borders as a ratio of the resolution, then for sure userspace will have to do the math, but I don't see the problem here.
On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
The choice is to either do what userspace asked, or return an error.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode?
Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode that is about to be applied, no matter if it's a usermode or one of the mode returned by the display?
No. With fixed mode panels the user mode is a lie. Only hdisplay/vdisplay mean anything. The actual timings are known only to the kernel. Well, userspace can perhaps make an educated guess based on the connector's mode list and the presence of the scaling mode property.
I guess the latter would be a bit easier (would also match how the TV margins behave I suppose). But that does leave open the question of how would generic userspace know which case it's dealing with? Eg. if it wants to underscan by some specific percentage it has to calculate the borders based on the correct mode, otherwise the results aren't going to match the expectations.
I don't get it, sorry. Borders are relative to the mode applied by userspace. So if it needs to express borders as a ratio of the resolution, then for sure userspace will have to do the math, but I don't see the problem here.
On Fri, 11 May 2018 20:29:48 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
The choice is to either do what userspace asked, or return an error.
Come on! If we can't use underscan when there's a cursor plane enabled this feature is pretty much useless. But let's take a real use case to show you how negligible the lack of scaling on the cursor plane will be. Say you have borders taking 10% of you screen (which is already a lot), and your cursor is a plane of 64x64 pixels, you'll end up with a 64x64 cursor instead of 58x58. Quite frankly, I doubt you'll notice the difference.
Anyway, I'd like to hear back from Eric on that, since he is the one who asked me to work on this feature.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode?
Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode that is about to be applied, no matter if it's a usermode or one of the mode returned by the display?
No. With fixed mode panels the user mode is a lie. Only hdisplay/vdisplay mean anything. The actual timings are known only to the kernel.
So now you say that the driver can silently ignore what the userspace asked for and set what he thinks is appropriate based on what the HW supports. This clearly contradicts the statement you make a few lines above: driver should apply what userspace asked for or return an error when it's not possible.
Well, userspace can perhaps make an educated guess based on the connector's mode list and the presence of the scaling mode property.
But guess what? Where is the actual 'HW' mode stored if it's not in crtc_state->mode? I mean, the drivers needs to know what to apply, and almost all drivers are using atomic states to extract this information.
On a side note, I'd like to repeat what I already said before: I don't care how this "define borders" feature is exposed to userspace, as long as we have a way to express that the display we're using has borders and the output image should be scaled+moved accordingly.
So now it's time to reach a consensus, because I'm not going to try and guess what you expect. When I asked Daniel earlier this week whether he wanted me to do things differently than I did in my v1 or try to match what other drivers were already doing with the underscan prop, he told me we should try to converge to a generic underscan solution that keeps the existing userspace interface unchanged (basically what radeon, amdgpu and nouveau drivers expose today). You don't seem to agree with that. So now I'd like a clear answer, what should I do?
On Fri, May 11, 2018 at 09:47:49PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 20:29:48 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote: > 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 v2: > - Take changes on hborder/vborder meaning into account > --- > 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..61ed60841cd6 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;
border * 2 ?
Oops, indeed. I'll fix that.
> + > + vc4_pstate->crtc_x += conn_state->underscan.hborder; > + vc4_pstate->crtc_y += conn_state->underscan.vborder; > + vc4_pstate->crtc_w = (vc4_pstate->crtc_w * > + (crtc_state->mode.hdisplay - > + (conn_state->underscan.hborder * 2))) / > + crtc_state->mode.hdisplay; > + vc4_pstate->crtc_h = (vc4_pstate->crtc_h * > + (crtc_state->mode.vdisplay - > + (conn_state->underscan.vborder * 2))) / > + crtc_state->mode.vdisplay;
So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
The choice is to either do what userspace asked, or return an error.
Come on! If we can't use underscan when there's a cursor plane enabled this feature is pretty much useless. But let's take a real use case to show you how negligible the lack of scaling on the cursor plane will be. Say you have borders taking 10% of you screen (which is already a lot), and your cursor is a plane of 64x64 pixels, you'll end up with a 64x64 cursor instead of 58x58. Quite frankly, I doubt you'll notice the difference.
Now you're assuming the cursor is only ever used as a cursor. It can be used for other things and those may need to be positioned pixel perfect in relation to other planes/fb contents.
We used to play is fast and loose in i915 when it came to the sprite plane coordinates. People generally hated that, at least when it came to the atomic ioctl. Basically we just adjusted the src/dst coordinates until the hw was happy with them, partially ignoring what the user had asked. Maarten recently nuked that code, and so now we either respect the user's choice or we return an error.
I guess one way out of this conundrum would be to allow the cursor to violate the user's requested parameters when controlled via the legacy cursor ioctls. There are no atomicity guarantees there, so I guess we could also say there are no other correctness guarantees either. Not sure if the accuracy of the hotspot might become an issue though.
Another option might be to just scale the cursor as well. If I understand correctly the "cursor can't be scaled" limitation just comes from the fact that some vblank synced resource needs to be reconfigured whenever the scaling changes. So doing that for unthrottled cursor updates is not easy. But in this case the underscan properties are what determines the scaling so maybe that resource could be reconfigured whenever the those props change to make sure the cursor can always be scaled appropriately?
Anyway, I'd like to hear back from Eric on that, since he is the one who asked me to work on this feature.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode?
Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode that is about to be applied, no matter if it's a usermode or one of the mode returned by the display?
No. With fixed mode panels the user mode is a lie. Only hdisplay/vdisplay mean anything. The actual timings are known only to the kernel.
So now you say that the driver can silently ignore what the userspace asked for and set what he thinks is appropriate based on what the HW supports. This clearly contradicts the statement you make a few lines above: driver should apply what userspace asked for or return an error when it's not possible.
It's an unfortunate legacy uapi design we have to live with. I agree that it would be much better if userspace had more explicit control over the crtc scaling/borders/etc.
Well, userspace can perhaps make an educated guess based on the connector's mode list and the presence of the scaling mode property.
But guess what? Where is the actual 'HW' mode stored if it's not in crtc_state->mode? I mean, the drivers needs to know what to apply, and almost all drivers are using atomic states to extract this information.
adjusted_mode is there for the driver to do as they wish. Eg. in i915 LVDS/eDP/DSI/etc. encoders will overwrite the adjusted_mode with the panel's native mode at the appropriate time (for other encoder types we just populate adjusted_mode with the user mode).
For the user it will still *look* like the mode they asked for (ie. hdisplay/vdisplay determine how many visible pixels there are), but internally we've replaced the timings with something different and there may or may not scaling involved (depend on the scaling mode property).
On a side note, I'd like to repeat what I already said before: I don't care how this "define borders" feature is exposed to userspace, as long as we have a way to express that the display we're using has borders and the output image should be scaled+moved accordingly.
So now it's time to reach a consensus, because I'm not going to try and guess what you expect. When I asked Daniel earlier this week whether he wanted me to do things differently than I did in my v1 or try to match what other drivers were already doing with the underscan prop, he told me we should try to converge to a generic underscan solution that keeps the existing userspace interface unchanged (basically what radeon, amdgpu and nouveau drivers expose today). You don't seem to agree with that. So now I'd like a clear answer, what should I do?
What do those drivers do when it comes to planes? I would expect that they just scale the entire crtc instead of individual planes.
Ville Syrjälä ville.syrjala@linux.intel.com writes:
On Fri, May 11, 2018 at 09:47:49PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 20:29:48 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
> On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote: > > 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 v2: > > - Take changes on hborder/vborder meaning into account > > --- > > 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..61ed60841cd6 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; > > border * 2 ?
Oops, indeed. I'll fix that.
> > > + > > + vc4_pstate->crtc_x += conn_state->underscan.hborder; > > + vc4_pstate->crtc_y += conn_state->underscan.vborder; > > + vc4_pstate->crtc_w = (vc4_pstate->crtc_w * > > + (crtc_state->mode.hdisplay - > > + (conn_state->underscan.hborder * 2))) / > > + crtc_state->mode.hdisplay; > > + vc4_pstate->crtc_h = (vc4_pstate->crtc_h * > > + (crtc_state->mode.vdisplay - > > + (conn_state->underscan.vborder * 2))) / > > + crtc_state->mode.vdisplay; > > So you're now scaling all planes? The code seems to reject scaling for > the cursor plane, how are you dealing with that? Or just no cursor > allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
The choice is to either do what userspace asked, or return an error.
Come on! If we can't use underscan when there's a cursor plane enabled this feature is pretty much useless. But let's take a real use case to show you how negligible the lack of scaling on the cursor plane will be. Say you have borders taking 10% of you screen (which is already a lot), and your cursor is a plane of 64x64 pixels, you'll end up with a 64x64 cursor instead of 58x58. Quite frankly, I doubt you'll notice the difference.
Now you're assuming the cursor is only ever used as a cursor. It can be used for other things and those may need to be positioned pixel perfect in relation to other planes/fb contents.
We used to play is fast and loose in i915 when it came to the sprite plane coordinates. People generally hated that, at least when it came to the atomic ioctl. Basically we just adjusted the src/dst coordinates until the hw was happy with them, partially ignoring what the user had asked. Maarten recently nuked that code, and so now we either respect the user's choice or we return an error.
I guess one way out of this conundrum would be to allow the cursor to violate the user's requested parameters when controlled via the legacy cursor ioctls. There are no atomicity guarantees there, so I guess we could also say there are no other correctness guarantees either. Not sure if the accuracy of the hotspot might become an issue though.
Another option might be to just scale the cursor as well. If I understand correctly the "cursor can't be scaled" limitation just comes from the fact that some vblank synced resource needs to be reconfigured whenever the scaling changes. So doing that for unthrottled cursor updates is not easy. But in this case the underscan properties are what determines the scaling so maybe that resource could be reconfigured whenever the those props change to make sure the cursor can always be scaled appropriately?
Yeah, we had no application for it, so it wasn't supported. I don't think it would be hard to have a previously-scaled cursor move, it's just that we need to fall back to a synchronous plane update if the scaling parameters change.
Ville Syrjälä ville.syrjala@linux.intel.com writes:
On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 19:54:02 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
On Fri, 11 May 2018 18:34:50 +0300 Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
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 v2:
- Take changes on hborder/vborder meaning into account
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..61ed60841cd6 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;border * 2 ?
Oops, indeed. I'll fix that.
- vc4_pstate->crtc_x += conn_state->underscan.hborder;
- vc4_pstate->crtc_y += conn_state->underscan.vborder;
- vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
(crtc_state->mode.hdisplay -(conn_state->underscan.hborder * 2))) /crtc_state->mode.hdisplay;- vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
(crtc_state->mode.vdisplay -(conn_state->underscan.vborder * 2))) /crtc_state->mode.vdisplay;So you're now scaling all planes? The code seems to reject scaling for the cursor plane, how are you dealing with that? Or just no cursor allowed when underscanning?
No, I just didn't test with a cursor plane. We should probably avoid scaling the cursor plane and just adjust its position. Eric any opinion on that?
I don't think you can just not scale it. The user asked for the cursor to be at a specific place with a specific size. Can't just ignore that and do something else. Also eg. i915 would definitely scale the cursor since we'd just scale the entire crtc instead of scaling individual planes. Different drivers doing different things wouldn't be good.
Except in our case the scaling takes place before the composition, so we don't have a choice.
The choice is to either do what userspace asked, or return an error.
I'm also wondering if there's any way we can reconcile these border props with the scaling mode prop, should we ever wish to expose these props on connectors that already have the scaling mode prop.
Nouveau seems to expose both, and I don't see why we couldn't.
First of all the interaction of these borders with panels that have a fixed mode would need to be properly specified. Do we apply the borders against the user mode or the panel's native mode?
Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode that is about to be applied, no matter if it's a usermode or one of the mode returned by the display?
No. With fixed mode panels the user mode is a lie. Only hdisplay/vdisplay mean anything. The actual timings are known only to the kernel. Well, userspace can perhaps make an educated guess based on the connector's mode list and the presence of the scaling mode property.
Let's just not expose the property on devices with fixed mode panels. Problem solved because all this property is is a way to work around HDMI monitors doing something awful, and trying to build something more general isn't serving anyone.
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 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 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); }
Now that underscan props can be parsed by the core and assigned to conn_state->underscan.xxx, we can rely on this implementation and get rid of the nouveau-specific underscan props.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- drivers/gpu/drm/nouveau/nouveau_connector.c | 39 +++++------------------------ drivers/gpu/drm/nouveau/nouveau_connector.h | 9 ------- drivers/gpu/drm/nouveau/nouveau_display.c | 14 ----------- drivers/gpu/drm/nouveau/nouveau_display.h | 3 --- drivers/gpu/drm/nouveau/nv50_display.c | 17 +++++++++---- 5 files changed, 18 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 6ed9cb053dfa..0ce055d3b89e 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -105,12 +105,6 @@ nouveau_conn_atomic_get_property(struct drm_connector *connector,
if (property == dev->mode_config.scaling_mode_property) *val = asyc->scaler.mode; - else if (property == disp->underscan_property) - *val = asyc->scaler.underscan.mode; - else if (property == disp->underscan_hborder_property) - *val = asyc->scaler.underscan.hborder; - else if (property == disp->underscan_vborder_property) - *val = asyc->scaler.underscan.vborder; else if (property == disp->dithering_mode) *val = asyc->dither.mode; else if (property == disp->dithering_depth) @@ -170,24 +164,6 @@ nouveau_conn_atomic_set_property(struct drm_connector *connector, asyc->set.scaler = true; } } else - if (property == disp->underscan_property) { - if (asyc->scaler.underscan.mode != val) { - asyc->scaler.underscan.mode = val; - asyc->set.scaler = true; - } - } else - if (property == disp->underscan_hborder_property) { - if (asyc->scaler.underscan.hborder != val) { - asyc->scaler.underscan.hborder = val; - asyc->set.scaler = true; - } - } else - if (property == disp->underscan_vborder_property) { - if (asyc->scaler.underscan.vborder != val) { - asyc->scaler.underscan.vborder = val; - asyc->set.scaler = true; - } - } else if (property == disp->dithering_mode) { if (asyc->dither.mode != val) { asyc->dither.mode = val; @@ -256,7 +232,6 @@ nouveau_conn_reset(struct drm_connector *connector) asyc->dither.mode = DITHERING_MODE_AUTO; asyc->dither.depth = DITHERING_DEPTH_AUTO; asyc->scaler.mode = DRM_MODE_SCALE_NONE; - asyc->scaler.underscan.mode = UNDERSCAN_OFF; asyc->procamp.color_vibrance = 150; asyc->procamp.vibrant_hue = 90;
@@ -285,18 +260,16 @@ nouveau_conn_attach_properties(struct drm_connector *connector) dvi_i_subconnector_property, 0);
/* Add overscan compensation options to digital outputs. */ - if (disp->underscan_property && + if (disp->disp.oclass >= NV50_DISP && (connector->connector_type == DRM_MODE_CONNECTOR_DVID || connector->connector_type == DRM_MODE_CONNECTOR_DVII || connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)) { - drm_object_attach_property(&connector->base, - disp->underscan_property, - UNDERSCAN_OFF); - drm_object_attach_property(&connector->base, - disp->underscan_hborder_property, 0); - drm_object_attach_property(&connector->base, - disp->underscan_vborder_property, 0); + WARN_ON(drm_connector_attach_underscan_properties(connector, + BIT(DRM_UNDERSCAN_OFF) | + BIT(DRM_UNDERSCAN_ON) | + BIT(DRM_UNDERSCAN_AUTO), + 128, 128)); }
/* Add hue and saturation options. */ diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h index a4d1a059bd3d..1d3ec65288e1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.h +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h @@ -111,15 +111,6 @@ struct nouveau_conn_atom {
struct { int mode; /* DRM_MODE_SCALE_* */ - struct { - enum { - UNDERSCAN_OFF, - UNDERSCAN_ON, - UNDERSCAN_AUTO, - } mode; - u32 hborder; - u32 vborder; - } underscan; bool full; } scaler;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 7d0bec8dd03d..002c2b13571b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -303,13 +303,6 @@ struct nouveau_drm_prop_enum_list { char *name; };
-static struct nouveau_drm_prop_enum_list underscan[] = { - { 6, UNDERSCAN_AUTO, "auto" }, - { 6, UNDERSCAN_OFF, "off" }, - { 6, UNDERSCAN_ON, "on" }, - {} -}; - static struct nouveau_drm_prop_enum_list dither_mode[] = { { 7, DITHERING_MODE_AUTO, "auto" }, { 7, DITHERING_MODE_OFF, "off" }, @@ -464,13 +457,6 @@ nouveau_display_create_properties(struct drm_device *dev)
PROP_ENUM(disp->dithering_mode, gen, "dithering mode", dither_mode); PROP_ENUM(disp->dithering_depth, gen, "dithering depth", dither_depth); - PROP_ENUM(disp->underscan_property, gen, "underscan", underscan); - - disp->underscan_hborder_property = - drm_property_create_range(dev, 0, "underscan hborder", 0, 128); - - disp->underscan_vborder_property = - drm_property_create_range(dev, 0, "underscan vborder", 0, 128);
if (gen < 1) return; diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index 270ba56f2756..df2f57d72fa9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -42,9 +42,6 @@ struct nouveau_display {
struct drm_property *dithering_mode; struct drm_property *dithering_depth; - struct drm_property *underscan_property; - struct drm_property *underscan_hborder_property; - struct drm_property *underscan_vborder_property; /* not really hue and saturation: */ struct drm_property *vibrant_hue_property; struct drm_property *color_vibrance_property; diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 8bd739cfd00d..7c7dbf3bd3a0 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -2057,11 +2057,11 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh, * ratio the same as the backend mode unless overridden by the * user setting both hborder and vborder properties. */ - if ((asyc->scaler.underscan.mode == UNDERSCAN_ON || - (asyc->scaler.underscan.mode == UNDERSCAN_AUTO && + if ((asyc->state.underscan.mode == DRM_UNDERSCAN_ON || + (asyc->state.underscan.mode == DRM_UNDERSCAN_AUTO && drm_detect_hdmi_monitor(edid)))) { - u32 bX = asyc->scaler.underscan.hborder; - u32 bY = asyc->scaler.underscan.vborder; + u32 bX = asyc->state.underscan.hborder; + u32 bY = asyc->state.underscan.vborder; u32 r = (asyh->view.oH << 19) / asyh->view.oW;
if (bX) { @@ -2185,8 +2185,8 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) struct nv50_head *head = nv50_head(crtc); struct nv50_head_atom *armh = nv50_head_atom(crtc->state); struct nv50_head_atom *asyh = nv50_head_atom(state); + struct drm_connector_state *conns, *oldconns; struct nouveau_conn_atom *asyc = NULL; - struct drm_connector_state *conns; struct drm_connector *conn; int i;
@@ -2199,6 +2199,13 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) } }
+ for_each_oldnew_connector_in_state(asyh->state.state, conn, + oldconns, conns, i) { + if (memcmp(&oldconns->underscan, &conns->underscan, + sizeof(conns->underscan))) + asyc->set.scaler = true; + } + if (armh->state.active) { if (asyc) { if (asyh->state.mode_changed)
dri-devel@lists.freedesktop.org
-
Boris Brezillon -
Eric Anholt -
Ville Syrjälä