diff mbox

[v5,6/9] drm: Handle aspect ratio info in legacy and atomic modeset paths

Message ID 1518697262-3001-7-git-send-email-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nautiyal, Ankit K Feb. 15, 2018, 12:20 p.m. UTC
From: Ankit Nautiyal <ankit.k.nautiyal@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@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(-)

Comments

Ville Syrjälä Feb. 23, 2018, 2:28 p.m. UTC | #1
On Thu, Feb 15, 2018 at 05:50:59PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@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@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
Nautiyal, Ankit K Feb. 26, 2018, 3:10 p.m. UTC | #2
Hi Ville,

I agree to all the comments here, and will correct the required things 
in the next patch-set.

On 2/23/2018 7:58 PM, Ville Syrjälä wrote:
> On Thu, Feb 15, 2018 at 05:50:59PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@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@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.

Agreed. I would add the required helper function, in the next patch-set.

>
>> +
>>   		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
> The blob length check has to be done before you access the data.

Right. Will take care in the next patch-set.

>>   		    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.

Agreed. Will correct this in the next patch-set.

>>   			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.

Agreed. Will correct this in the next patch-set.

Regards,
Ankit

>
>>   			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
diff mbox

Patch

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;
+		}
+
 		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
 		    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);
 			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);
 			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);