Re: [PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4054 bytes --]



On 4/20/2018 7:37 PM, Ville Syrjälä wrote:
> On Fri, Apr 20, 2018 at 07:01:46PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> To enable aspect-ratio support in DRM, blindly exposing the aspect
>> ratio information along with mode, can break things in existing
>> user-spaces which have no intention or support to use this aspect
>> ratio information.
>>
>> To avoid this, a new drm client cap is required to enable a
>> user-space to advertise if it supports modes with aspect-ratio. Based
>> on this cap value, the kernel will take a call on exposing the aspect
>> ratio info in modes or not.
>>
>> This patch adds the client cap for aspect-ratio.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> V3: rebase
>> V4: As suggested by Marteen Lankhorst modified the commit message
>>      explaining the need to use the DRM cap for aspect-ratio. Also,
>>      tweaked the comment lines in the code for better understanding and
>>      clarity, as recommended by Shashank Sharma.
>> V5: rebase
>> V6: rebase
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> V10: added comment explaining that no userspace breaks on aspect-ratio
>>       mode bits.
>>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
>>   include/drm/drm_file.h      | 8 ++++++++
>>   include/uapi/drm/drm.h      | 7 +++++++
>>   3 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index af78291..39c8eab 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -325,6 +325,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>>   		file_priv->atomic = req->value;
>>   		file_priv->universal_planes = req->value;
>>   		break;
>> +	case DRM_CLIENT_CAP_ASPECT_RATIO:
>> +		if (req->value > 1)
>> +			return -EINVAL;
>> +	/*
>> +	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
>> +	 * wayland/weston, xserver, and hardware-composer modeset paths.
>> +	 */
> Bogus indentation.

Thanks to point that out, will fix this.

> Also where's the aspect_ratio_allowed handling for the atomic cap?
> Or did we decide against it after all?

As discussed, aspect ratio is handled in the atomic modeset path, where 
in the modeset requests with aspect-ratios
are rejected, if the aspect-ratio cap not set.
The part which is dropped is - hiding the aspect-ratio information, 
while returning a mode blob in the drm_mode_get_blob
if the cap is not set. The patch was dropped as no user-space currently 
uses getblob ioctl to get the mode-blob.

Regards,
Ankit
>
>> +		file_priv->aspect_ratio_allowed = req->value;
>> +		break;
>>   	default:
>>   		return -EINVAL;
>>   	}
>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>> index 5176c37..02b7dde 100644
>> --- a/include/drm/drm_file.h
>> +++ b/include/drm/drm_file.h
>> @@ -182,6 +182,14 @@ struct drm_file {
>>   	unsigned atomic:1;
>>   
>>   	/**
>> +	 * @aspect_ratio_allowed:
>> +	 *
>> +	 * True, if client can handle picture aspect ratios, and has requested
>> +	 * to pass this information along with the mode.
>> +	 */
>> +	unsigned aspect_ratio_allowed:1;
>> +
>> +	/**
>>   	 * @is_master:
>>   	 *
>>   	 * This client is the creator of @master. Protected by struct
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 6fdff59..9c660e1 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -680,6 +680,13 @@ struct drm_get_cap {
>>    */
>>   #define DRM_CLIENT_CAP_ATOMIC	3
>>   
>> +/**
>> + * DRM_CLIENT_CAP_ASPECT_RATIO
>> + *
>> + * If set to 1, the DRM core will provide aspect ratio information in modes.
>> + */
>> +#define DRM_CLIENT_CAP_ASPECT_RATIO    4
>> +
>>   /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>>   struct drm_set_client_cap {
>>   	__u64 capability;
>> -- 
>> 2.7.4


[-- Attachment #1.2: Type: text/html, Size: 5167 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel