Re: [PATCH RFC 1/2] drm: add bitmask property type

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

 



On Thu, Mar 29, 2012 at 8:02 PM, Rob Clark <rob.clark@xxxxxxxxxx> wrote:
> From: Rob Clark <rob@xxxxxx>
>
> A bitmask property is similar to an enum.  The enum value is a bit
> position (0-63), and valid property values consist of a mask of
> zero or more of (1 << enum_val[n]).
>
> TODO: word commit msg better
> TODO: maybe "flags" would be a better name for the property type?
> ---
> See https://github.com/robclark/kernel-omap4/commit/970b7bb95993fc43b4977976bf8005dc2e1a4ad3#L6R411
> for an example usage.  In this case combinations of "x-invert", "y-invert"
> and "xy-flip" can express all possible combinations of rotations of
> multiples of 90 degrees plus mirroring.  Which is sufficient for an
> xrandr v1.2 rotation support.  For arbitrary transforms in xrandr v1.3
> a different property with a transform matrix (if supported by the
> driver) should be used.

oh, and this shows the mapping between xrandr rotation/reflection mask
and x-invert/y-invert/xy-flip:
https://github.com/robclark/xf86-video-omap/commit/87ffbaf9d282831bf03da457e6f6c4e45a0d6b2b#L0R222

The other option is of course to make the rotation bitmask mirror the
xrandr rotation mask values, which might be a better option to support
drivers which only provide rotation and not mirroring.  I'm ok with
either option, whatever others prefer.

BR,
-R

>
> Note: I've not finished updating libdrm and ddx driver to use this
> yet, so other than compile and boot test, you can consider this as
> untested.  But I figure that it is at least worthwhile to send as
> an RFC at this point to get feedback.
>
>  drivers/gpu/drm/drm_crtc.c |   46 +++++++++++++++++++++++++++++++++++++++++--
>  include/drm/drm_crtc.h     |    3 ++
>  include/drm/drm_mode.h     |    1 +
>  3 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 95c7ab2..2b462f6 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2713,6 +2713,33 @@ struct drm_property *drm_property_create_enum(struct drm_device *dev, int flags,
>  }
>  EXPORT_SYMBOL(drm_property_create_enum);
>
> +struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
> +                                        int flags, const char *name,
> +                                        const struct drm_prop_enum_list *props, int num_values)
> +{
> +       struct drm_property *property;
> +       int i, ret;
> +
> +       flags |= DRM_MODE_PROP_BITMASK;
> +
> +       property = drm_property_create(dev, flags, name, num_values);
> +       if (!property)
> +               return NULL;
> +
> +       for (i = 0; i < num_values; i++) {
> +               ret = drm_property_add_enum(property, i,
> +                                     props[i].type,
> +                                     props[i].name);
> +               if (ret) {
> +                       drm_property_destroy(dev, property);
> +                       return NULL;
> +               }
> +       }
> +
> +       return property;
> +}
> +EXPORT_SYMBOL(drm_property_create_bitmask);
> +
>  struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
>                                         const char *name,
>                                         uint64_t min, uint64_t max)
> @@ -2737,7 +2764,14 @@ int drm_property_add_enum(struct drm_property *property, int index,
>  {
>        struct drm_property_enum *prop_enum;
>
> -       if (!(property->flags & DRM_MODE_PROP_ENUM))
> +       if (!(property->flags & (DRM_MODE_PROP_ENUM | 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))
>                return -EINVAL;
>
>        if (!list_empty(&property->enum_blob_list)) {
> @@ -2881,7 +2915,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>        }
>        property = obj_to_property(obj);
>
> -       if (property->flags & DRM_MODE_PROP_ENUM) {
> +       if (property->flags & (DRM_MODE_PROP_ENUM | 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) {
> @@ -2906,7 +2940,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>        }
>        out_resp->count_values = value_count;
>
> -       if (property->flags & DRM_MODE_PROP_ENUM) {
> +       if (property->flags & (DRM_MODE_PROP_ENUM | 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;
> @@ -3063,6 +3097,12 @@ static bool drm_property_change_is_valid(struct drm_property *property,
>                if (value > property->values[1])
>                        return false;
>                return true;
> +       } else if (property->flags & DRM_MODE_PROP_BITMASK) {
> +               int i;
> +               __u64 valid_mask = 0;
> +               for (i = 0; i < property->num_values; i++)
> +                       valid_mask |= (1 << property->values[i]);
> +               return !(value & ~valid_mask);
>        } else {
>                int i;
>                for (i = 0; i < property->num_values; i++)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 32e9c51..28e9a78 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -940,6 +940,9 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
>                                         const char *name,
>                                         const struct drm_prop_enum_list *props,
>                                         int num_values);
> +struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
> +                                        int flags, const char *name,
> +                                        const struct drm_prop_enum_list *props, int num_values);
>  struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
>                                         const char *name,
>                                         uint64_t min, uint64_t max);
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index de5de2a..3190dfe 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -228,6 +228,7 @@ struct drm_mode_get_connector {
>  #define DRM_MODE_PROP_IMMUTABLE        (1<<2)
>  #define DRM_MODE_PROP_ENUM     (1<<3) /* enumerated type with text strings */
>  #define DRM_MODE_PROP_BLOB     (1<<4)
> +#define DRM_MODE_PROP_BITMASK  (1<<5) /* bitmask of enumerated types */
>
>  struct drm_mode_property_enum {
>        __u64 value;
> --
> 1.7.9.1
>
_______________________________________________
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