[v12,07/10] drm: Handle aspect ratio info in legacy modeset path

Commit Message

Nautiyal, Ankit K April 27, 2018, 12:14 p.m. UTC

From: Ankit Nautiyal <[e-mail]>

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. rejects the modes with aspect-ratio info, during modeset, if the
   user does not support aspect ratio.
2. does not load the aspect-ratio info in user-mode structure, if
   aspect ratio is not supported.
3. adds helper functions for determining if aspect-ratio is expected
   in user-mode and for allowing/disallowing the aspect-ratio, if its
   not expected.

Signed-off-by: Ankit Nautiyal <[e-mail]>

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.
V6: Used the helper functions for determining if aspect-ratio is
    expected in the user-mode.
V7: rebase
V8: rebase
V9: rebase
v10: Modified the commit-message
v11: rebase
v12: Merged the patch for adding aspect-ratio helper functions
     with this patch.
---
 drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
 drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_modes.h     |  4 ++++
 3 files changed, 57 insertions(+)

Comments

Ville Syrjälä April 27, 2018, 1:54 p.m. UTC | #1

On Fri, Apr 27, 2018 at 05:44:53PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <[e-mail]>
> 
> 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. rejects the modes with aspect-ratio info, during modeset, if the
>    user does not support aspect ratio.
> 2. does not load the aspect-ratio info in user-mode structure, if
>    aspect ratio is not supported.
> 3. adds helper functions for determining if aspect-ratio is expected
>    in user-mode and for allowing/disallowing the aspect-ratio, if its
>    not expected.
> 
> Signed-off-by: Ankit Nautiyal <[e-mail]>
> 
> 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.
> V6: Used the helper functions for determining if aspect-ratio is
>     expected in the user-mode.
> V7: rebase
> V8: rebase
> V9: rebase
> v10: Modified the commit-message
> v11: rebase
> v12: Merged the patch for adding aspect-ratio helper functions
>      with this patch.
> ---
>  drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
>  drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_modes.h     |  4 ++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index a231dd5..98323f4 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -449,6 +449,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  			crtc_resp->mode_valid = 0;
>  		}
>  	}
> +	drm_mode_filter_aspect_ratio_flags(file_priv, &crtc_resp->mode);
>  	drm_modeset_unlock(&crtc->mutex);
>  
>  	return 0;
> @@ -628,6 +629,13 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  			ret = -ENOMEM;
>  			goto out;
>  		}
> +		if (!drm_mode_aspect_ratio_allowed(file_priv,
> +						   &crtc_req->mode)) {
> +			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) {
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index c395a24..d6f68c8 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1759,3 +1759,48 @@ bool drm_mode_is_420(const struct drm_display_info *display,
>  		drm_mode_is_420_also(display, mode);
>  }
>  EXPORT_SYMBOL(drm_mode_is_420);
> +
> +/**
> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
> + * is expected from the user-mode.
> + *
> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
> + * allowed to contain aspect-ratio value.
> + * If the user does not set aspect-ratio cap, then the only value allowed in the
> + * flags bits is aspect-ratio NONE.
> + *
> + * @file_priv: file private structure to get the user capabilities
> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
> + * information.
> + *
> + * Returns:
> + * true if the aspect-ratio info is allowed in the user-mode flags.
> + * false, otherwise.
> + */
> +bool
> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> +			      struct drm_mode_modeinfo *umode)
> +{
> +	return file_priv->aspect_ratio_allowed || (umode->flags &
> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;

Still looks funny.

> +}
> +
> +/**
> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
> + * user-mode flags.
> + *
> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
> + *
> + * @file_priv: file private structure to get the user capabilities.
> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
> + * be filtered.
> + *
> + */
> +void
> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> +				   struct drm_mode_modeinfo *umode)
> +{
> +	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
> +		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> +}
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 2f78b7e..e0b060d 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
>  			  const struct drm_display_mode *mode);
>  bool drm_mode_is_420(const struct drm_display_info *display,
>  		     const struct drm_display_mode *mode);
> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> +				   struct drm_mode_modeinfo *umode);
> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> +					struct drm_mode_modeinfo *umode);

Since these are no longer exported the prototypes should go into
drivers/gpu/drm/drm_crtc_internal.h

>  
>  struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>  				      int hdisplay, int vdisplay, int vrefresh,
> -- 
> 2.7.4

Nautiyal, Ankit K April 30, 2018, 6:21 a.m. UTC | #2

On 4/27/2018 7:24 PM, Ville Syrjälä wrote:
> On Fri, Apr 27, 2018 at 05:44:53PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <[e-mail]>
>>
>> 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. rejects the modes with aspect-ratio info, during modeset, if the
>>     user does not support aspect ratio.
>> 2. does not load the aspect-ratio info in user-mode structure, if
>>     aspect ratio is not supported.
>> 3. adds helper functions for determining if aspect-ratio is expected
>>     in user-mode and for allowing/disallowing the aspect-ratio, if its
>>     not expected.
>>
>> Signed-off-by: Ankit Nautiyal <[e-mail]>
>>
>> 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.
>> V6: Used the helper functions for determining if aspect-ratio is
>>      expected in the user-mode.
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> v10: Modified the commit-message
>> v11: rebase
>> v12: Merged the patch for adding aspect-ratio helper functions
>>       with this patch.
>> ---
>>   drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
>>   drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_modes.h     |  4 ++++
>>   3 files changed, 57 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index a231dd5..98323f4 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -449,6 +449,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>>                        crtc_resp->mode_valid = 0;
>>                }
>>        }
>> +     drm_mode_filter_aspect_ratio_flags(file_priv, &crtc_resp->mode);
>>        drm_modeset_unlock(&crtc->mutex);
>>
>>        return 0;
>> @@ -628,6 +629,13 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>>                        ret = -ENOMEM;
>>                        goto out;
>>                }
>> +             if (!drm_mode_aspect_ratio_allowed(file_priv,
>> +                                                &crtc_req->mode)) {
>> +                     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) {
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index c395a24..d6f68c8 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1759,3 +1759,48 @@ bool drm_mode_is_420(const struct drm_display_info *display,
>>                drm_mode_is_420_also(display, mode);
>>   }
>>   EXPORT_SYMBOL(drm_mode_is_420);
>> +
>> +/**
>> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
>> + * is expected from the user-mode.
>> + *
>> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
>> + * allowed to contain aspect-ratio value.
>> + * If the user does not set aspect-ratio cap, then the only value allowed in the
>> + * flags bits is aspect-ratio NONE.
>> + *
>> + * @file_priv: file private structure to get the user capabilities
>> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
>> + * information.
>> + *
>> + * Returns:
>> + * true if the aspect-ratio info is allowed in the user-mode flags.
>> + * false, otherwise.
>> + */
>> +bool
>> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>> +                           struct drm_mode_modeinfo *umode)
>> +{
>> +     return file_priv->aspect_ratio_allowed || (umode->flags &
>> +             DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
> Still looks funny.

Alright. Let me change it to:
   if (file_priv->aspect_ratio_allowed ||
             (umode->flags & DRM_MODE_FLAG_PIC_AR_MASK) == 
DRM_MODE_FLAG_PIC_AR_NONE)
                 return true;
         return false;

>> +}
>> +
>> +/**
>> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
>> + * user-mode flags.
>> + *
>> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
>> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
>> + *
>> + * @file_priv: file private structure to get the user capabilities.
>> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
>> + * be filtered.
>> + *
>> + */
>> +void
>> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>> +                                struct drm_mode_modeinfo *umode)
>> +{
>> +     if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
>> +             umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
>> +}
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index 2f78b7e..e0b060d 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
>>                          const struct drm_display_mode *mode);
>>   bool drm_mode_is_420(const struct drm_display_info *display,
>>                     const struct drm_display_mode *mode);
>> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>> +                                struct drm_mode_modeinfo *umode);
>> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>> +                                     struct drm_mode_modeinfo *umode);
> Since these are no longer exported the prototypes should go into
> drivers/gpu/drm/drm_crtc_internal.h

Got it, will move the prototypes to the correct place.
Thanks for pointing that out.

Regards,
Ankit


>
>>   struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>>                                      int hdisplay, int vdisplay, int vrefresh,
>> --
>> 2.7.4
> --
> Ville Syrjälä
> Intel

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a231dd5..98323f4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -449,6 +449,7 @@  int drm_mode_getcrtc(struct drm_device *dev,
 			crtc_resp->mode_valid = 0;
 		}
 	}
+	drm_mode_filter_aspect_ratio_flags(file_priv, &crtc_resp->mode);
 	drm_modeset_unlock(&crtc->mutex);
 
 	return 0;
@@ -628,6 +629,13 @@  int drm_mode_setcrtc(struct drm_device *dev, void *data,
 			ret = -ENOMEM;
 			goto out;
 		}
+		if (!drm_mode_aspect_ratio_allowed(file_priv,
+						   &crtc_req->mode)) {
+			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) {
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c395a24..d6f68c8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1759,3 +1759,48 @@  bool drm_mode_is_420(const struct drm_display_info *display,
 		drm_mode_is_420_also(display, mode);
 }
 EXPORT_SYMBOL(drm_mode_is_420);
+
+/**
+ * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
+ * is expected from the user-mode.
+ *
+ * If the user has set aspect-ratio cap, then the flag of the user-mode is
+ * allowed to contain aspect-ratio value.
+ * If the user does not set aspect-ratio cap, then the only value allowed in the
+ * flags bits is aspect-ratio NONE.
+ *
+ * @file_priv: file private structure to get the user capabilities
+ * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
+ * information.
+ *
+ * Returns:
+ * true if the aspect-ratio info is allowed in the user-mode flags.
+ * false, otherwise.
+ */
+bool
+drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+			      struct drm_mode_modeinfo *umode)
+{
+	return file_priv->aspect_ratio_allowed || (umode->flags &
+		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
+}
+
+/**
+ * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
+ * user-mode flags.
+ *
+ * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
+ * bits in the user-mode flags, if aspect-ratio info is not allowed.
+ *
+ * @file_priv: file private structure to get the user capabilities.
+ * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
+ * be filtered.
+ *
+ */
+void
+drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+				   struct drm_mode_modeinfo *umode)
+{
+	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
+		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+}
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 2f78b7e..e0b060d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -461,6 +461,10 @@  bool drm_mode_is_420_also(const struct drm_display_info *display,
 			  const struct drm_display_mode *mode);
 bool drm_mode_is_420(const struct drm_display_info *display,
 		     const struct drm_display_mode *mode);
+bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+				   struct drm_mode_modeinfo *umode);
+void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+					struct drm_mode_modeinfo *umode);
 
 struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
 				      int hdisplay, int vdisplay, int vrefresh,