[PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane

Daniel Vetter daniel at ffwll.ch
Thu May 4 13:22:45 UTC 2017


On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote:
> Add standard optinal property blobs for YCbCr to RGB conversion CSC
> matrix and YCbCr preoffset vector in DRM planes. New enums are defined
> to YCBCR_ENCODING property to activate the CSC and preoffset
> properties. For simplicity the new properties are stored in the
> drm_plane object, because the YCBCR_ENCODING is already there. The
> blob contents are defined in the uapi/drm/drm_mode.h header.
> 
> Signed-off-by: Jyri Sarha <jsarha at ti.com>

Not sure we want to make this yuv2rgb specific, plenty of planes have
generic degamma/offset, csc, gamme/offset hw. I think what we want instead
is the generic csc support, plus a ycbcr2rgb mode of "bypass".

And as usual, this needs some userspace compositor which actually uses it
(not just a demo, since there's a huge difference between a demo and
something that has to Just Work like a real compositor).
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c        | 19 +++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  9 ++++++
>  drivers/gpu/drm/drm_color_mgmt.c    | 55 +++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/drm_plane.c         |  6 ++++
>  include/drm/drm_color_mgmt.h        |  3 ++
>  include/drm/drm_plane.h             |  4 +++
>  include/uapi/drm/drm_mode.h         | 12 ++++++++
>  7 files changed, 106 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index bcef93d..87a2d55 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -733,6 +733,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
>  	int ret;
> +	bool dummy;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
> @@ -777,6 +778,18 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->zpos = val;
>  	} else if (property == plane->ycbcr_encoding_property) {
>  		state->ycbcr_encoding = val;
> +	} else if (property == plane->ycbcr_decode_csc_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->ycbcr_decode_csc, val,
> +				sizeof(struct drm_ycbcr_decode_csc),
> +				&dummy);
> +		return ret;
> +	} else if (property == plane->ycbcr_csc_preoffset_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->ycbcr_csc_preoffset, val,
> +				sizeof(struct drm_ycbcr_csc_preoffset),
> +				&dummy);
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -839,6 +852,12 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		*val = state->zpos;
>  	} else if (property == plane->ycbcr_encoding_property) {
>  		*val = state->ycbcr_encoding;
> +	} else if (property == plane->ycbcr_decode_csc_property) {
> +		*val = state->ycbcr_decode_csc ?
> +			state->ycbcr_decode_csc->base.id : 0;
> +	} else if (property == plane->ycbcr_csc_preoffset_property) {
> +		*val = state->ycbcr_csc_preoffset ?
> +			state->ycbcr_csc_preoffset->base.id : 0;
>  	} 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_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 8be9719..6ecc32f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3238,6 +3238,12 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  {
>  	memcpy(state, plane->state, sizeof(*state));
>  
> +	if (state->ycbcr_decode_csc)
> +		drm_property_blob_get(state->ycbcr_decode_csc);
> +
> +	if (state->ycbcr_csc_preoffset)
> +		drm_property_blob_get(state->ycbcr_csc_preoffset);
> +
>  	if (state->fb)
>  		drm_framebuffer_get(state->fb);
>  
> @@ -3278,6 +3284,9 @@ struct drm_plane_state *
>   */
>  void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  {
> +	drm_property_blob_put(state->ycbcr_decode_csc);
> +	drm_property_blob_put(state->ycbcr_csc_preoffset);
> +
>  	if (state->fb)
>  		drm_framebuffer_put(state->fb);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 245b14a..319ed46 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -95,8 +95,23 @@
>   *
>   * "YCBCR_ENCODING"
>   * 	Optional plane enum property to control YCbCr to RGB
> - * 	conversion. The driver provides a subset of standard
> - *	enum values supported by the DRM plane.
> + * 	conversion. The driver provides a subset of standard enum
> + * 	values supported by the DRM plane. The possible encodings
> + * 	include the standard conversions and a possibility to select a
> + * 	custom conversion matrix and preoffset vector.
> + *
> + * "YCBCR_DECODE_CSC"
> + *	Optional plane property blob to set YCbCr to RGB conversion
> + *	matrix. The blob contains struct drm_ycbcr_to_rgb_csc which is
> + *	defined in uapi/drm/drm_mode.h. Whether this property is
> + *	used depends on the value of YCBCR_ENCODING property.
> + *
> + * "YCBCR_CSC_PREOFFSET"
> + *	Optional plane property blob to configure YCbCr offset before
> + *	YCbCr to RGB CSC is applied. The blob contains struct
> + *	drm_ycbcr_csc_preoffset which is defined in
> + *	uapi/drm/drm_mode.h. Whether this property is used depends on
> + *	the value of YCBCR_ENCODING property.
>   */
>  
>  /**
> @@ -351,6 +366,9 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
>  	[DRM_PLANE_YCBCR_BT601_LIMITED_RANGE] = "BT.601 limited range",
>  	[DRM_PLANE_YCBCR_BT709_LIMITED_RANGE] = "BT.709 limited range",
>  	[DRM_PLANE_YCBCR_BT2020_LIMITED_RANGE] = "BT.2020 limited range",
> +	[DRM_PLANE_YCBCR_CSC_FULL_RANGE] = "YCbCr CSC full range",
> +	[DRM_PLANE_YCBCR_CSC_LIMITED_RANGE] = "YCbCr CSC limited range",
> +	[DRM_PLANE_YCBCR_CSC_PREOFFSET] = "YCbCr CSC and preoffset vector",
>  };
>  
>  /**
> @@ -363,6 +381,8 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
>   * drm_plane object. The supported encodings should be provided in the
>   * enum_list parameter. The enum_list parameter should not contain the
>   * enum names, because the standard names are added by this function.
> + * YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties are created
> + * based on the provided enum_list.
>   */
>  int drm_plane_create_ycbcr_properties(struct drm_plane *plane,
>  			struct drm_prop_enum_list *enum_list,
> @@ -371,6 +391,8 @@ int drm_plane_create_ycbcr_properties(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_property *prop;
> +	bool ycbcr_decode_csc_create = false;
> +	bool ycbcr_csc_preoffset_create = false;
>  	unsigned int i;
>  
>  	if (WARN_ON(plane->ycbcr_encoding_property != NULL))
> @@ -380,6 +402,15 @@ int drm_plane_create_ycbcr_properties(struct drm_plane *plane,
>  		enum drm_plane_ycbcr_encoding encoding = enum_list[i].type;
>  
>  		enum_list[i].name = ycbcr_encoding_name[encoding];
> +
> +		if (encoding == DRM_PLANE_YCBCR_CSC_FULL_RANGE ||
> +		    encoding == DRM_PLANE_YCBCR_CSC_LIMITED_RANGE)
> +			ycbcr_decode_csc_create = true;
> +
> +		if (encoding == DRM_PLANE_YCBCR_CSC_PREOFFSET) {
> +			ycbcr_decode_csc_create = true;
> +			ycbcr_csc_preoffset_create = true;
> +		}
>  	}
>  
>  	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
> @@ -390,5 +421,25 @@ int drm_plane_create_ycbcr_properties(struct drm_plane *plane,
>  	plane->ycbcr_encoding_property = prop;
>  	drm_object_attach_property(&plane->base, prop, default_mode);
>  
> +	if (ycbcr_decode_csc_create) {
> +		prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC |
> +					   DRM_MODE_PROP_BLOB,
> +					   "YCBCR_DECODE_CSC", 0);
> +		if (!prop)
> +			return -ENOMEM;
> +		plane->ycbcr_decode_csc_property = prop;
> +		drm_object_attach_property(&plane->base, prop, 0);
> +	}
> +
> +	if (ycbcr_csc_preoffset_create) {
> +		prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC |
> +					   DRM_MODE_PROP_BLOB,
> +					   "YCBCR_CSC_PREOFFSET", 0);
> +		if (!prop)
> +			return -ENOMEM;
> +		plane->ycbcr_csc_preoffset_property = prop;
> +		drm_object_attach_property(&plane->base, prop, 0);
> +	}
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 007c4d7..d10c942 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -247,6 +247,12 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  	if (plane->ycbcr_encoding_property)
>  		drm_property_destroy(dev, plane->ycbcr_encoding_property);
>  
> +	if (plane->ycbcr_decode_csc_property)
> +		drm_property_destroy(dev, plane->ycbcr_decode_csc_property);
> +
> +	if (plane->ycbcr_csc_preoffset_property)
> +		drm_property_destroy(dev, plane->ycbcr_csc_preoffset_property);
> +
>  	memset(plane, 0, sizeof(*plane));
>  }
>  EXPORT_SYMBOL(drm_plane_cleanup);
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 1771394..bdfd37e 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -44,6 +44,9 @@ enum drm_plane_ycbcr_encoding {
>  	DRM_PLANE_YCBCR_BT601_LIMITED_RANGE,
>  	DRM_PLANE_YCBCR_BT709_LIMITED_RANGE,
>  	DRM_PLANE_YCBCR_BT2020_LIMITED_RANGE,
> +	DRM_PLANE_YCBCR_CSC_FULL_RANGE,
> +	DRM_PLANE_YCBCR_CSC_LIMITED_RANGE,
> +	DRM_PLANE_YCBCR_CSC_PREOFFSET,
>  };
>  
>  int drm_plane_create_ycbcr_properties(struct drm_plane *plane,
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 4d0510f..63b1e0c 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -115,6 +115,8 @@ struct drm_plane_state {
>  
>  	/* YCbCr to RGB conversion */
>  	enum drm_plane_ycbcr_encoding ycbcr_encoding;
> +	struct drm_property_blob *ycbcr_decode_csc;
> +	struct drm_property_blob *ycbcr_csc_preoffset;
>  
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
> @@ -529,6 +531,8 @@ struct drm_plane {
>  	struct drm_property *rotation_property;
>  
>  	struct drm_property *ycbcr_encoding_property;
> +	struct drm_property *ycbcr_decode_csc_property;
> +	struct drm_property *ycbcr_csc_preoffset_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8c67fc0..8c9568d 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -543,6 +543,18 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +struct drm_ycbcr_decode_csc {
> +	/* Conversion matrix in 2-complement s32.32 format. */
> +	__s64 ry, rcb, rcr;
> +	__s64 gy, gcb, gcr;
> +	__s64 by, bcb, bcr;
> +};
> +
> +struct drm_ycbcr_csc_preoffset {
> +	/* Offset vector in 2-complement s.32 format. */
> +	__s32 y, cb, cr;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list