[REBASE 2/5] drm: Handle aspect ratio in modeset paths

Ville Syrjälä ville.syrjala at linux.intel.com
Tue Nov 21 17:11:03 UTC 2017


On Fri, Nov 17, 2017 at 03:00:29PM +0530, Shashank Sharma wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> 
> If the user mode does not support aspect-ratio, and requests for
> a modeset with aspect ratio flags, then the flag bits reprsenting
> aspect ratio must be ignored.

Rejected, not ignored. Rejection should happen when converting
the umode to mode.

And filtering should happen in getcrtc and getblob. The way you're
currently doing things will corrupt the current state, which is
not good.

> Similarly, if user space doesn't set the aspect ratio client cap,
> while preparing a usermode, the aspect-ratio info must not be given
> into it.
> 
> This patch:
> 1. adds a new bit field aspect_ratio_allowed in drm_atomic_state,
>    which is set only if the aspect-ratio is supported by the user.
> 2. discards the aspect-ratio info from the user mode while
>    preparing kernel mode structure, during modeset, if the
>    user does not support aspect ratio.
> 3. avoids setting the aspect-ratio info in user-mode, while
>    converting user-mode from the kernel-mode.
> 
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma at intel.com>
> Cc: Jose Abreu <jose.abreu at synopsys.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 40 +++++++++++++++++++++++++++++++++-------
>  drivers/gpu/drm/drm_crtc.c   | 19 +++++++++++++++++++
>  include/drm/drm_atomic.h     |  2 ++
>  3 files changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d5..b9743af 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -338,18 +338,30 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
>  	state->mode_blob = NULL;
>  
>  	if (mode) {
> -		drm_mode_convert_to_umode(&umode, mode);
> +		/*
> +		 * If the user has not asked for aspect ratio support,
> +		 * take a copy of the 'mode' and set the aspect ratio as
> +		 * None. This copy is used to prepare the user-mode with no
> +		 * aspect-ratio info.
> +		 */
> +		struct drm_display_mode ar_mode;
> +		struct drm_atomic_state *atomic_state = state->state;
> +
> +		drm_mode_copy(&ar_mode, mode);
> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
> +			ar_mode.picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +		drm_mode_convert_to_umode(&umode, &ar_mode);
>  		state->mode_blob =
>  			drm_property_create_blob(state->crtc->dev,
> -		                                 sizeof(umode),
> -		                                 &umode);
> +						 sizeof(umode),
> +						 &umode);
>  		if (IS_ERR(state->mode_blob))
>  			return PTR_ERR(state->mode_blob);
>  
> -		drm_mode_copy(&state->mode, mode);
> +		drm_mode_copy(&state->mode, &ar_mode);
>  		state->enable = true;
>  		DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
> -				 mode->name, state);
> +				 ar_mode.name, state);
>  	} else {
>  		memset(&state->mode, 0, sizeof(state->mode));
>  		state->enable = false;
> @@ -386,10 +398,23 @@ 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 *ar_umode;
> +		struct drm_atomic_state *atomic_state;
> +
> +		/*
> +		 * If the user-space does not ask for aspect-ratio
> +		 * the aspect ratio bits in the drmModeModeinfo from user,
> +		 * does not mean anything. Therefore these bits must be
> +		 * discarded.
> +		 */
> +		ar_umode = (struct drm_mode_modeinfo *) blob->data;
> +		atomic_state = state->state;
> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
> +			ar_umode->flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>  		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
>  		    drm_mode_convert_umode(&state->mode,
> -		                           (const struct drm_mode_modeinfo *)
> -		                            blob->data))
> +					   (const struct drm_mode_modeinfo *)
> +					    ar_umode))
>  			return -EINVAL;
>  
>  		state->mode_blob = drm_property_blob_get(blob);
> @@ -2229,6 +2254,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  
>  	state->acquire_ctx = &ctx;
>  	state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> +	state->allow_aspect_ratio = file_priv->aspect_ratio_allowed;
>  
>  retry:
>  	plane_mask = 0;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e6..a2d34fa 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -425,6 +425,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	drm_modeset_lock(&crtc->mutex, NULL);
>  	if (crtc->state) {
>  		if (crtc->state->enable) {
> +			/*
> +			 * If the aspect-ratio is not required by the,
> +			 * userspace, do not set the aspect-ratio flags.
> +			 */
> +			if (!file_priv->aspect_ratio_allowed)
> +				crtc->state->mode.picture_aspect_ratio =
> +					HDMI_PICTURE_ASPECT_NONE;
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -435,6 +442,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  		crtc_resp->x = crtc->x;
>  		crtc_resp->y = crtc->y;
>  		if (crtc->enabled) {
> +			/*
> +			 * If the aspect-ratio is not required by the,
> +			 * userspace, do not set the aspect-ratio flags.
> +			 */
> +			if (!file_priv->aspect_ratio_allowed)
> +				crtc->mode.picture_aspect_ratio =
> +					HDMI_PICTURE_ASPECT_NONE;
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -610,6 +624,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  			goto out;
>  		}
>  
> +		/* If the user does not ask for aspect ratio, reset the aspect
> +		 * ratio bits in the usermode.
> +		 */
> +		if (!file_priv->aspect_ratio_allowed)
> +			crtc_req->mode.flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>  		ret = drm_mode_convert_umode(mode, &crtc_req->mode);
>  		if (ret) {
>  			DRM_DEBUG_KMS("Invalid mode\n");
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 5afd6e3..ae74b2c 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -209,6 +209,7 @@ struct __drm_private_objs_state {
>   * @ref: count of all references to this state (will not be freed until zero)
>   * @dev: parent DRM device
>   * @allow_modeset: allow full modeset
> + * @allow_aspect_ratio: allow the aspect ratio info to be passes to user
>   * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
>   * @async_update: hint for asynchronous plane update
>   * @planes: pointer to array of structures with per-plane data
> @@ -224,6 +225,7 @@ struct drm_atomic_state {
>  
>  	struct drm_device *dev;
>  	bool allow_modeset : 1;
> +	bool allow_aspect_ratio : 1;
>  	bool legacy_cursor_update : 1;
>  	bool async_update : 1;
>  	struct __drm_planes_state *planes;
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC


More information about the dri-devel mailing list