[PATCH v5 6/9] drm: Handle aspect ratio info in legacy and atomic modeset paths
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Feb 23 14:28:45 UTC 2018
On Thu, Feb 15, 2018 at 05:50:59PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>
> If the user-space does not support aspect-ratio, and requests for a
> modeset with mode having aspect ratio bits set, then the given
> user-mode must be rejected. Secondly, while preparing a user-mode from
> kernel mode, the aspect-ratio info must not be given, if aspect-ratio
> is not supported by the user.
>
> This patch:
> 1. passes the file_priv structure from the drm_mode_atomic_ioctl till
> the drm_mode_crtc_set_mode_prop, to get the user capability.
> 2. rejects the modes with aspect-ratio info, during modeset, if the
> user does not support aspect ratio.
> 3. does not load the aspect-ratio info in user-mode structure, if
> aspect ratio is not supported.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>
> V3: Addressed review comments from Ville:
> Do not corrupt the current crtc state by updating aspect ratio on
> the fly.
> V4: rebase
> V5: As suggested by Ville, rejected the modeset calls for modes with
> aspect ratio, if the user does not set aspect ratio cap.
> ---
> drivers/gpu/drm/drm_atomic.c | 30 ++++++++++++++++++++++--------
> drivers/gpu/drm/drm_atomic_helper.c | 6 +++---
> drivers/gpu/drm/drm_crtc.c | 15 +++++++++++++++
> drivers/gpu/drm/drm_crtc_internal.h | 3 ++-
> drivers/gpu/drm/drm_mode_object.c | 9 ++++++---
> drivers/gpu/drm/drm_modes.c | 1 +
> include/drm/drm_atomic.h | 5 +++--
> 7 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 86b483e..7e78305 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -368,6 +368,7 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);
> * drm_atomic_set_mode_prop_for_crtc - set mode for CRTC
> * @state: the CRTC whose incoming state to update
> * @blob: pointer to blob property to use for mode
> + * @file_priv: file priv structure, to get the userspace capabilities
> *
> * Set a mode (originating from a blob property) on the desired CRTC state.
> * This function will take a reference on the blob property for the CRTC state,
> @@ -378,7 +379,8 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);
> * Zero on success, error code on failure. Cannot return -EDEADLK.
> */
> int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> - struct drm_property_blob *blob)
> + struct drm_property_blob *blob,
> + struct drm_file *file_priv)
> {
> if (blob == state->mode_blob)
> return 0;
> @@ -389,10 +391,20 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> memset(&state->mode, 0, sizeof(state->mode));
>
> if (blob) {
> + struct drm_mode_modeinfo *u_mode;
> +
> + u_mode = (struct drm_mode_modeinfo *) blob->data;
> + if (!file_priv->aspect_ratio_allowed &&
> + (u_mode->flags & DRM_MODE_FLAG_PIC_AR_MASK) !=
> + DRM_MODE_FLAG_PIC_AR_NONE) {
> + DRM_DEBUG_ATOMIC("Unexpected aspect-ratio flag bits\n");
> + return -EINVAL;
> + }
We should probably pull this logic into a small helper so that we don't
have to duplicate it in both the setprop and setcrtc code paths.
> +
> if (blob->length != sizeof(struct drm_mode_modeinfo) ||
The blob length check has to be done before you access the data.
> drm_mode_convert_umode(state->crtc->dev, &state->mode,
> - (const struct drm_mode_modeinfo *)
> - blob->data))
> + (const struct drm_mode_modeinfo *)
> + u_mode))
> return -EINVAL;
>
> state->mode_blob = drm_property_blob_get(blob);
> @@ -441,6 +453,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> * @state: the state object to update with the new property value
> * @property: the property to set
> * @val: the new property value
> + * @file_priv: the file private structure, to get the user capabilities
> *
> * This function handles generic/core properties and calls out to driver's
> * &drm_crtc_funcs.atomic_set_property for driver properties. To ensure
> @@ -452,7 +465,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> */
> int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state, struct drm_property *property,
> - uint64_t val)
> + uint64_t val, struct drm_file *file_priv)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_mode_config *config = &dev->mode_config;
> @@ -465,7 +478,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_property_blob *mode =
> drm_property_lookup_blob(dev, val);
> mode->is_video_mode = true;
> - ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
> + ret = drm_atomic_set_mode_prop_for_crtc(state, mode, file_priv);
> drm_property_blob_put(mode);
> return ret;
> } else if (property == config->degamma_lut_property) {
> @@ -1918,7 +1931,8 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
> int drm_atomic_set_property(struct drm_atomic_state *state,
> struct drm_mode_object *obj,
> struct drm_property *prop,
> - uint64_t prop_value)
> + uint64_t prop_value,
> + struct drm_file *file_priv)
> {
> struct drm_mode_object *ref;
> int ret;
> @@ -1952,7 +1966,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> }
>
> ret = drm_atomic_crtc_set_property(crtc,
> - crtc_state, prop, prop_value);
> + crtc_state, prop, prop_value, file_priv);
> break;
> }
> case DRM_MODE_OBJECT_PLANE: {
> @@ -2341,7 +2355,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> }
>
> ret = drm_atomic_set_property(state, obj, prop,
> - prop_value);
> + prop_value, file_priv);
> if (ret) {
> drm_mode_object_put(obj);
> goto out;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ae3cbfe..781ebd6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -186,7 +186,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>
> if (!crtc_state->connector_mask) {
> ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> - NULL);
> + NULL, NULL);
> if (ret < 0)
> goto out;
>
> @@ -2749,7 +2749,7 @@ static int update_output_state(struct drm_atomic_state *state,
>
> if (!new_crtc_state->connector_mask) {
> ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
> - NULL);
> + NULL, NULL);
> if (ret < 0)
> return ret;
>
> @@ -2930,7 +2930,7 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>
> crtc_state->active = false;
>
> - ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
> + ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL, NULL);
> if (ret < 0)
> goto free;
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 353e24f..e36a971 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -430,6 +430,9 @@ int drm_mode_getcrtc(struct drm_device *dev,
> if (crtc->state) {
> if (crtc->state->enable) {
> drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
> + if (!file_priv->aspect_ratio_allowed)
> + crtc->state->mode.flags &=
> + (~DRM_MODE_FLAG_PIC_AR_MASK);
Should be manling crtc_resp->mode instead of the internal mode.
> crtc_resp->mode_valid = 1;
>
> } else {
> @@ -440,6 +443,9 @@ int drm_mode_getcrtc(struct drm_device *dev,
> crtc_resp->y = crtc->y;
> if (crtc->enabled) {
> drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
> + if (!file_priv->aspect_ratio_allowed)
> + crtc->mode.flags &=
> + (~DRM_MODE_FLAG_PIC_AR_MASK);
ditto
Should probably pull this flag filtering logic into a small helper
as well.
> crtc_resp->mode_valid = 1;
>
> } else {
> @@ -614,6 +620,15 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> goto out;
> }
>
> + if (!file_priv->aspect_ratio_allowed &&
> + (crtc_req->mode.flags & DRM_MODE_FLAG_PIC_AR_MASK) !=
> + DRM_MODE_FLAG_PIC_AR_NONE) {
> + DRM_DEBUG_KMS("Unexpected aspect-ratio flag bits\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> +
> ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode);
> if (ret) {
> DRM_DEBUG_KMS("Invalid mode\n");
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index af00f42..7c3338f 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -186,7 +186,8 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
> int drm_atomic_set_property(struct drm_atomic_state *state,
> struct drm_mode_object *obj,
> struct drm_property *prop,
> - uint64_t prop_value);
> + uint64_t prop_value,
> + struct drm_file *file_priv);
> int drm_atomic_get_property(struct drm_mode_object *obj,
> struct drm_property *property, uint64_t *val);
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index ce4d2fb..1c3d498 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -452,7 +452,8 @@ static int set_property_legacy(struct drm_mode_object *obj,
>
> static int set_property_atomic(struct drm_mode_object *obj,
> struct drm_property *prop,
> - uint64_t prop_value)
> + uint64_t prop_value,
> + struct drm_file *file_priv)
> {
> struct drm_device *dev = prop->dev;
> struct drm_atomic_state *state;
> @@ -476,7 +477,8 @@ static int set_property_atomic(struct drm_mode_object *obj,
> obj_to_connector(obj),
> prop_value);
> } else {
> - ret = drm_atomic_set_property(state, obj, prop, prop_value);
> + ret = drm_atomic_set_property(state, obj, prop, prop_value,
> + file_priv);
> if (ret)
> goto out;
> ret = drm_atomic_commit(state);
> @@ -519,7 +521,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
> goto out_unref;
>
> if (drm_drv_uses_atomic_modeset(property->dev))
> - ret = set_property_atomic(arg_obj, property, arg->value);
> + ret = set_property_atomic(arg_obj, property, arg->value,
> + file_priv);
> else
> ret = set_property_legacy(arg_obj, property, arg->value);
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 6ca1f3b..ca4c5cc 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1620,6 +1620,7 @@ EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode);
> * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo
> * @out: drm_mode_modeinfo struct to return to the user
> * @in: drm_display_mode to use
> + * @file_priv: file_priv structure, to get the user capabilities
> *
> * Convert a drm_display_mode into a drm_mode_modeinfo structure to return to
> * the user.
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index cf13842..d3ad5d1 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -367,7 +367,7 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> struct drm_crtc *crtc);
> int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state, struct drm_property *property,
> - uint64_t val);
> + uint64_t val, struct drm_file *file_priv);
> struct drm_plane_state * __must_check
> drm_atomic_get_plane_state(struct drm_atomic_state *state,
> struct drm_plane *plane);
> @@ -583,7 +583,8 @@ drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
> const struct drm_display_mode *mode);
> int __must_check
> drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> - struct drm_property_blob *blob);
> + struct drm_property_blob *blob,
> + struct drm_file *file_priv);
> int __must_check
> drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
> struct drm_crtc *crtc);
> --
> 2.7.4
--
Ville Syrjälä
Intel OTC
More information about the dri-devel
mailing list