Re: [PATCH 2/3] drm: Add optional PIXEL_NORMALIZE_RANGE property to drm_plane

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Nov 28, 2018 at 10:38:12PM -0800, Kevin Strasser wrote:
> Add an optional property to allow applications to indicate what range their
> floating point pixel data is normalized to. Drivers are free to choose what
> ranges they want to support and can attach this property to each plane that
> actually supports floating point formats

Do we have a plan to actually use this? Earlier platforms didn't have
anything like this IIRC.

> 
> Signed-off-by: Kevin Strasser <kevin.strasser@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_atomic.c        |  2 ++
>  drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
>  drivers/gpu/drm/drm_color_mgmt.c    | 68 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc_internal.h |  1 +
>  include/drm/drm_color_mgmt.h        |  9 +++++
>  include/drm/drm_plane.h             | 14 ++++++++
>  6 files changed, 98 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 1706ed1..1f520ef 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -624,6 +624,8 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  		   drm_get_color_encoding_name(state->color_encoding));
>  	drm_printf(p, "\tcolor-range=%s\n",
>  		   drm_get_color_range_name(state->color_range));
> +	drm_printf(p, "\tpixel-normalize-range=%s\n",
> +		   drm_get_pixel_normalize_range_name(state->pixel_normalize_range));
>  
>  	if (plane->funcs->atomic_print_state)
>  		plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 86ac339..e79a23cd 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -566,6 +566,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->color_encoding = val;
>  	} else if (property == plane->color_range_property) {
>  		state->color_range = val;
> +	} else if (property == plane->pixel_normalize_range_property) {
> +		state->pixel_normalize_range = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -621,6 +623,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->color_encoding;
>  	} else if (property == plane->color_range_property) {
>  		*val = state->color_range;
> +	} else if (property == plane->pixel_normalize_range_property) {
> +		*val = state->pixel_normalize_range;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 581cc37..b1e2a0a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -472,3 +472,71 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_create_color_properties);
> +
> +static const char * const pixel_normalize_range_name[] = {
> +	[DRM_PIXEL_NORMALIZE_RANGE_0_1] = "0.0 - 1.0",
> +	[DRM_PIXEL_NORMALIZE_RANGE_0_255] = "0.0 - 255.0",
> +};
> +
> +/**
> + * drm_get_pixel_normalize_range_name - return a string for pixel normalize
> + * range
> + * @range: pixel normalize range to compute name of
> + *
> + * In contrast to the other drm_get_*_name functions this one here returns a
> + * const pointer and hence is threadsafe.
> + */
> +const char *drm_get_pixel_normalize_range_name(enum drm_pixel_normalize_range range)
> +{
> +	if (WARN_ON(range >= ARRAY_SIZE(pixel_normalize_range_name)))
> +		return "unknown";
> +
> +	return pixel_normalize_range_name[range];
> +}
> +
> +/**
> + * drm_plane_create_pixel_normalize_range_property - pixel normalize range
> + * property
> + * @plane: plane object
> + * @supported_ranges: bitfield indicating supported normalize ranges
> + * @default_range: default normalize range
> + *
> + * Create and attach plane specific PIXEL_NORMALIZE_RANGE property to @plane.
> + * The supported ranges should be provided in supported_ranges bitmask. Eeach
> + * bit set in the bitmask indicates that its number as enum value is supported.
> + */
> +int drm_plane_create_pixel_normalize_range_property(struct drm_plane *plane,
> +	u32 supported_ranges, enum drm_pixel_normalize_range default_range)
> +{
> +	struct drm_property *prop;
> +	struct drm_prop_enum_list enum_list[DRM_PIXEL_NORMALIZE_RANGE_MAX];
> +	int i, len = 0;
> +
> +	if (WARN_ON(supported_ranges == 0 ||
> +		    (supported_ranges & -BIT(DRM_PIXEL_NORMALIZE_RANGE_MAX)) != 0 ||
> +		    (supported_ranges & BIT(default_range)) == 0))
> +		return -EINVAL;
> +
> +	for (i = 0; i < DRM_PIXEL_NORMALIZE_RANGE_MAX; i++) {
> +		if ((supported_ranges & BIT(i)) == 0)
> +			continue;
> +
> +		enum_list[len].type = i;
> +		enum_list[len].name = pixel_normalize_range_name[i];
> +		len++;
> +	}
> +
> +	prop = drm_property_create_enum(plane->dev, 0, "PIXEL_NORMALIZE_RANGE",
> +					enum_list, len);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	plane->pixel_normalize_range_property = prop;
> +	drm_object_attach_property(&plane->base, prop, default_range);
> +
> +	if (plane->state)
> +		plane->state->pixel_normalize_range = default_range;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_pixel_normalize_range_property);
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 8689344..82ddc50 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -90,6 +90,7 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
>  /* drm_color_mgmt.c */
>  const char *drm_get_color_encoding_name(enum drm_color_encoding encoding);
>  const char *drm_get_color_range_name(enum drm_color_range range);
> +const char *drm_get_pixel_normalize_range_name(enum drm_pixel_normalize_range range);
>  
>  /* IOCTLs */
>  int drm_mode_gamma_get_ioctl(struct drm_device *dev,
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef999..460e31c 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -64,9 +64,18 @@ enum drm_color_range {
>  	DRM_COLOR_RANGE_MAX,
>  };
>  
> +enum drm_pixel_normalize_range {
> +	DRM_PIXEL_NORMALIZE_RANGE_0_1,
> +	DRM_PIXEL_NORMALIZE_RANGE_0_255,
> +	DRM_PIXEL_NORMALIZE_RANGE_MAX
> +};
> +
>  int drm_plane_create_color_properties(struct drm_plane *plane,
>  				      u32 supported_encodings,
>  				      u32 supported_ranges,
>  				      enum drm_color_encoding default_encoding,
>  				      enum drm_color_range default_range);
> +int drm_plane_create_pixel_normalize_range_property(struct drm_plane *plane,
> +				u32 supported_ranges,
> +				enum drm_pixel_normalize_range default_range);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 3701f56..11f5be4 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -130,6 +130,13 @@ struct drm_plane_state {
>  	uint16_t pixel_blend_mode;
>  
>  	/**
> +	 * @pixel_normalize_range:
> +	 * The range to use for floating point pixel data normalization. See
> +	 * drm_plane_create_pixel_normalize_range_property() for more details.
> +	 */
> +	enum drm_pixel_normalize_range pixel_normalize_range;
> +
> +	/**
>  	 * @rotation:
>  	 * Rotation of the plane. See drm_plane_create_rotation_property() for
>  	 * more details.
> @@ -680,6 +687,13 @@ struct drm_plane {
>  	struct drm_property *blend_mode_property;
>  
>  	/**
> +	 * @pixel_normalize_range_property:
> +	 * Optional "PIXEL_NORMALIZE_RANGE" property for this plane. See
> +	 * drm_plane_create_pixel_normalize_range_property().
> +	 */
> +	struct drm_property *pixel_normalize_range_property;
> +
> +	/**
>  	 * @color_encoding_property:
>  	 *
>  	 * Optional "COLOR_ENCODING" enum property for specifying
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx