Re: [PATCH 2/8] drm: add extended property types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 30, 2014 at 01:12:04PM -0400, Rob Clark wrote:
> If we continue to use bitmask for type, we will quickly run out of room
> to add new types.  Split this up so existing part of bitmask range
> continues to function as before, but reserve a chunk of the remaining
> space for an integer type-id.  Wrap this all up in some type-check
> helpers to keep the backwards-compat uglyness contained.
> 
> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>

I kinda liked David's idea to just continue the enumeration, but instead
of continuing with flags we'll just start at last_flag+1, +2, ...

With or without that this is Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
>  drivers/gpu/drm/drm_crtc.c  | 26 +++++++++++++++++---------
>  include/drm/drm_crtc.h      | 17 +++++++++++++++++
>  include/uapi/drm/drm_mode.h | 13 +++++++++++++
>  3 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 02872af..0dbd6e0 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3113,6 +3113,9 @@ struct drm_property *drm_property_create(struct drm_device *dev, int flags,
>  	}
>  
>  	list_add_tail(&property->head, &dev->mode_config.property_list);
> +
> +	WARN_ON(!drm_property_type_valid(property));
> +
>  	return property;
>  fail:
>  	kfree(property->values);
> @@ -3270,14 +3273,16 @@ int drm_property_add_enum(struct drm_property *property, int index,
>  {
>  	struct drm_property_enum *prop_enum;
>  
> -	if (!(property->flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK)))
> +	if (!(drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> +			drm_property_type_is(property, DRM_MODE_PROP_BITMASK)))
>  		return -EINVAL;
>  
>  	/*
>  	 * Bitmask enum properties have the additional constraint of values
>  	 * from 0 to 63
>  	 */
> -	if ((property->flags & DRM_MODE_PROP_BITMASK) && (value > 63))
> +	if (drm_property_type_is(property, DRM_MODE_PROP_BITMASK) &&
> +			(value > 63))
>  		return -EINVAL;
>  
>  	if (!list_empty(&property->enum_blob_list)) {
> @@ -3458,10 +3463,11 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>  		goto done;
>  	}
>  
> -	if (property->flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK)) {
> +	if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> +			drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
>  		list_for_each_entry(prop_enum, &property->enum_blob_list, head)
>  			enum_count++;
> -	} else if (property->flags & DRM_MODE_PROP_BLOB) {
> +	} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
>  		list_for_each_entry(prop_blob, &property->enum_blob_list, head)
>  			blob_count++;
>  	}
> @@ -3483,7 +3489,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>  	}
>  	out_resp->count_values = value_count;
>  
> -	if (property->flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK)) {
> +	if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> +			drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
>  		if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
>  			copied = 0;
>  			enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
> @@ -3505,7 +3512,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>  		out_resp->count_enum_blobs = enum_count;
>  	}
>  
> -	if (property->flags & DRM_MODE_PROP_BLOB) {
> +	if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
>  		if ((out_resp->count_enum_blobs >= blob_count) && blob_count) {
>  			copied = 0;
>  			blob_id_ptr = (uint32_t __user *)(unsigned long)out_resp->enum_blob_ptr;
> @@ -3659,17 +3666,18 @@ static bool drm_property_change_is_valid(struct drm_property *property,
>  {
>  	if (property->flags & DRM_MODE_PROP_IMMUTABLE)
>  		return false;
> -	if (property->flags & DRM_MODE_PROP_RANGE) {
> +
> +	if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
>  		if (value < property->values[0] || value > property->values[1])
>  			return false;
>  		return true;
> -	} else if (property->flags & DRM_MODE_PROP_BITMASK) {
> +	} else if (drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
>  		int i;
>  		uint64_t valid_mask = 0;
>  		for (i = 0; i < property->num_values; i++)
>  			valid_mask |= (1ULL << property->values[i]);
>  		return !(value & ~valid_mask);
> -	} else if (property->flags & DRM_MODE_PROP_BLOB) {
> +	} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
>  		/* Only the driver knows */
>  		return true;
>  	} else {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index cdd5687..0ff9558 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -931,6 +931,23 @@ extern void drm_mode_config_cleanup(struct drm_device *dev);
>  
>  extern int drm_mode_connector_update_edid_property(struct drm_connector *connector,
>  						struct edid *edid);
> +
> +static inline bool drm_property_type_is(struct drm_property *property,
> +		uint32_t type)
> +{
> +	/* instanceof for props.. handles extended type vs original types: */
> +	if (property->flags & DRM_MODE_PROP_EXTENDED_TYPE)
> +		return (property->flags & DRM_MODE_PROP_EXTENDED_TYPE) == type;
> +	return property->flags & type;
> +}
> +
> +static inline bool drm_property_type_valid(struct drm_property *property)
> +{
> +	if (property->flags & DRM_MODE_PROP_EXTENDED_TYPE)
> +		return !(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
> +	return !!(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
> +}
> +
>  extern int drm_object_property_set_value(struct drm_mode_object *obj,
>  					 struct drm_property *property,
>  					 uint64_t val);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index f104c26..a7f546e 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -251,6 +251,19 @@ struct drm_mode_get_connector {
>  #define DRM_MODE_PROP_BLOB	(1<<4)
>  #define DRM_MODE_PROP_BITMASK	(1<<5) /* bitmask of enumerated types */
>  
> +/* non-extended types: legacy bitmask, one bit per type: */
> +#define DRM_MODE_PROP_LEGACY_TYPE  ( \
> +		DRM_MODE_PROP_RANGE | \
> +		DRM_MODE_PROP_ENUM | \
> +		DRM_MODE_PROP_BLOB | \
> +		DRM_MODE_PROP_BITMASK)
> +
> +/* extended-types: rather than continue to consume a bit per type,
> + * grab a chunk of the bits to use as integer type id.
> + */
> +#define DRM_MODE_PROP_EXTENDED_TYPE	0x0000ffc0
> +#define DRM_MODE_PROP_TYPE(n)		((n) << 6)
> +
>  struct drm_mode_property_enum {
>  	__u64 value;
>  	char name[DRM_PROP_NAME_LEN];
> -- 
> 1.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux