[Intel-gfx] [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Nautiyal, Ankit K]

On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
> On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>>
>> This patch 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 <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_modes.h     |  4 ++++
>>   2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index c395a24..d6133e8 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1759,3 +1759,50 @@ 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;
> Odd line split here. Makes this a bit hard to read.
> I would split after the ||

Agreed. I wasn't sure how to let it have better readability and less 
than 80 char length
at the same time. Will fix this.

>
>> +}
>> +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
> Do we actually need to export these? I don't think so.
>
> But I might be wrong. It's a bit hard to see with the way
> you split this patch with the actual users in a different patch.

These helper functions are used in drm_mode_atomic.c, drm_mode_crtc.c, 
and drm_mode_get_connector.c
The patches are split as to have modes-set handling separate and the 
exposing of connector separate.
Do you suggest this patch to be merged with the patch for handling 
aspect-ratio in modeset?

>> +
>> +/**
>> + * 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;
>> +}
>> +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
>> 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,
>> -- 
>> 2.7.4

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20180423/7447adcf/attachment.html>