Re: [PATCH 2/8] drm: add signed-range property type

On Thu, May 29, 2014 at 4:29 AM, David Herrmann dh.herrmann@gmail.com wrote:

Hi

On Thu, May 29, 2014 at 1:57 AM, Rob Clark robdclark@gmail.com wrote:

...

Like range, but values are signed.

Signed-off-by: Rob Clark robdclark@gmail.com

drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++++++++++++++++++++++------------ include/drm/drm_crtc.h | 12 ++++++++++++ include/uapi/drm/drm_mode.h | 1 + 3 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 61ddc66..88a0741 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3242,6 +3242,22 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev, } EXPORT_SYMBOL(drm_property_create_bitmask);

+static struct drm_property *property_create_range(struct drm_device *dev,

•                                    int flags, const char *name,
  

•                                    uint64_t min, uint64_t max)
  

+{

•   struct drm_property *property;
  

•   property = drm_property_create(dev, flags, name, 2);
  

•   if (!property)
  

•           return NULL;
  

•   property->values[0] = min;
  

•   property->values[1] = max;
  

•   return property;
  

+}

/**

• drm_property_create - create a new ranged property type
• @dev: drm device

@@ -3264,21 +3280,20 @@ struct drm_property *drm_property_create_range(struct drm_device *dev, int flags const char *name, uint64_t min, uint64_t max) {

•   struct drm_property *property;
  

•   flags |= DRM_MODE_PROP_RANGE;
  

•   property = drm_property_create(dev, flags, name, 2);
  

•   if (!property)
  

•           return NULL;
  

•   property->values[0] = min;
  

•   property->values[1] = max;
  

•   return property;
  

•   return property_create_range(dev, DRM_MODE_PROP_RANGE | flags,
  

•                   name, min, max);
  

} EXPORT_SYMBOL(drm_property_create_range);

+struct drm_property *drm_property_create_signed_range(struct drm_device *dev,

•                                    int flags, const char *name,
  

•                                    int64_t min, int64_t max)
  

+{

•   return property_create_range(dev, DRM_MODE_PROP_SIGNED_RANGE | flags,
  

•                   name, I642U64(min), I642U64(max));
  

+} +EXPORT_SYMBOL(drm_property_create_signed_range);

struct drm_property *drm_property_create_object(struct drm_device *dev, int flags, const char *name, uint32_t type) { @@ -3718,6 +3733,12 @@ static bool drm_property_change_is_valid(struct drm_property *property, if (value < property->values[0] || value > property->values[1]) return false; return true;

•   } else if (drm_property_type_is(property, DRM_MODE_PROP_SIGNED_RANGE)) {
  

•           int64_t svalue = U642I64(value);
  

•           if (svalue < U642I64(property->values[0]) ||
  

•                           svalue > U642I64(property->values[1]))
  

•                   return false;
  

•           return true;
    } else if (drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
            int i;
            uint64_t valid_mask = 0;
  

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c1c243f..9bd3551 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -64,6 +64,15 @@ struct drm_object_properties { uint64_t values[DRM_OBJECT_MAX_PROPERTY]; };

+static inline int64_t U642I64(uint64_t val) +{

•   return (int64_t)*((int64_t *)&val);
  

The cast to "(int64_t)" is unneeded. Dereferencing (int64_t*) will always yield (int64_t).

Same below.

Btw., why are these macros needed? unsigned->signed conversion is well-defined. signed->unsigned is undefined, but I thought we rely on static reinterpretation in the kernel. Dunno.. maybe I'm wrong.

iirc, I added these in userspace similar to U642VOID()/VOID2U64() and copied to kernel when I needed them there ;-)

BR, -R

Anyhow, this is:

Reviewed-by: David Herrmann dh.herrmann@gmail.com

Thanks David

...

+} +static inline uint64_t I642U64(int64_t val) +{

•   return (uint64_t)*((uint64_t *)&val);
  

+}

enum drm_connector_force { DRM_FORCE_UNSPECIFIED, DRM_FORCE_OFF, @@ -982,6 +991,9 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev, struct drm_property *drm_property_create_range(struct drm_device *dev, int flags, const char *name, uint64_t min, uint64_t max); +struct drm_property *drm_property_create_signed_range(struct drm_device *dev,

•                                    int flags, const char *name,
  

•                                    int64_t min, int64_t max);
  

struct drm_property *drm_property_create_object(struct drm_device *dev, int flags, const char *name, uint32_t type); extern void drm_property_destroy(struct drm_device *dev, struct drm_property *property); diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 5b530c7..ded505e 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -264,6 +264,7 @@ struct drm_mode_get_connector { #define DRM_MODE_PROP_EXTENDED_TYPE 0x0000ffc0 #define DRM_MODE_PROP_TYPE(n) ((n) << 6) #define DRM_MODE_PROP_OBJECT DRM_MODE_PROP_TYPE(1) +#define DRM_MODE_PROP_SIGNED_RANGE DRM_MODE_PROP_TYPE(2)

struct drm_mode_property_enum { __u64 value; -- 1.9.3

---

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