Re: [PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties

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

 



On 05/02/17 11:33, Daniel Vetter wrote:
> On Fri, Apr 21, 2017 at 12:51:14PM +0300, Jyri Sarha wrote:
>> Add standard properties to control YCbCr to RGB conversion in DRM
>> planes. The created properties are stored to drm_plane object to allow
>> different set of supported conversion modes for different planes on
>> the device. For simplicity the related property blobs for CSC matrix
>> and YCbCr preoffsets are also stored in the same place. The blob
>> contents are defined in the uapi/drm/drm_mode.h header.
>>
>> Signed-off-by: Jyri Sarha <jsarha@xxxxxx>
> 
> Just to make sure there's no surprises: We need the userspace for this
> too. -modesetting (for Xv maybe), some wayland compositor (weston, ozone,
> whatever you feel like) or drm_hwcomposer.
> 
> But yeah might be good to bikeshed the uabi first a bit more and get at
> least some agreement on that.
> 

In the first phase I have been using kms++ [1]. And when/if we have an
agreement about the API, I will push my patches there.

With X, wayland, and other compositors, we would in the end need some
video player supporting the different YCbCr encodings according to the
video stream. This sounds like a relatively big task, but surely I
volunteer to assist, when we get there.

Cheer,
Jyri

[1] https://github.com/tomba/kmsxx/

> Thanks, Daniel
>> ---
>>  drivers/gpu/drm/drm_atomic.c        |  26 +++++++
>>  drivers/gpu/drm/drm_atomic_helper.c |   9 +++
>>  drivers/gpu/drm/drm_color_mgmt.c    | 136 +++++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/drm_plane.c         |  10 +++
>>  include/drm/drm_color_mgmt.h        |  23 ++++++
>>  include/drm/drm_plane.h             |  10 +++
>>  include/uapi/drm/drm_mode.h         |  12 ++++
>>  7 files changed, 223 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index f881319..d1512aa 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -732,6 +732,8 @@ 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);
>> @@ -774,6 +776,22 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>>  		state->rotation = val;
>>  	} else if (property == plane->zpos_property) {
>>  		state->zpos = val;
>> +	} else if (property == plane->ycbcr_to_rgb_mode_property) {
>> +		state->ycbcr_to_rgb_mode = val;
>> +	} else if (property == plane->ycbcr_to_rgb_csc_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +				&state->ycbcr_to_rgb_csc,
>> +				val,
>> +				sizeof(struct drm_ycbcr_to_rgb_csc),
>> +				&dummy);
>> +		return ret;
>> +	} else if (property == plane->ycbcr_to_rgb_preoffset_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +				&state->ycbcr_to_rgb_preoffset,
>> +				val,
>> +				sizeof(struct drm_ycbcr_to_rgb_preoffset),
>> +				&dummy);
>> +		return ret;
>>  	} else if (plane->funcs->atomic_set_property) {
>>  		return plane->funcs->atomic_set_property(plane, state,
>>  				property, val);
>> @@ -834,6 +852,14 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>>  		*val = state->rotation;
>>  	} else if (property == plane->zpos_property) {
>>  		*val = state->zpos;
>> +	} else if (property == plane->ycbcr_to_rgb_mode_property) {
>> +		*val = state->ycbcr_to_rgb_mode;
>> +	} else if (property == plane->ycbcr_to_rgb_csc_property) {
>> +		*val = state->ycbcr_to_rgb_csc ?
>> +			state->ycbcr_to_rgb_csc->base.id : 0;
>> +	} else if (property == plane->ycbcr_to_rgb_preoffset_property) {
>> +		*val = state->ycbcr_to_rgb_preoffset ?
>> +			state->ycbcr_to_rgb_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 c3994b4..89fd826 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -3196,6 +3196,12 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>>  {
>>  	memcpy(state, plane->state, sizeof(*state));
>>  
>> +	if (state->ycbcr_to_rgb_csc)
>> +		drm_property_blob_get(state->ycbcr_to_rgb_csc);
>> +
>> +	if (state->ycbcr_to_rgb_preoffset)
>> +		drm_property_blob_get(state->ycbcr_to_rgb_preoffset);
>> +
>>  	if (state->fb)
>>  		drm_framebuffer_get(state->fb);
>>  
>> @@ -3236,6 +3242,9 @@ struct drm_plane_state *
>>   */
>>  void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>>  {
>> +	drm_property_blob_put(state->ycbcr_to_rgb_csc);
>> +	drm_property_blob_put(state->ycbcr_to_rgb_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 cc23b9a..badaddd 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -29,9 +29,14 @@
>>  /**
>>   * DOC: overview
>>   *
>> - * Color management or color space adjustments is supported through a set of 5
>> - * properties on the &drm_crtc object. They are set up by calling
>> - * drm_crtc_enable_color_mgmt().
>> + * Color management or color space adjustments in CRTCs is supported
>> + * through a set of 5 properties on the &drm_crtc object. They are set
>> + * up by calling drm_crtc_enable_color_mgmt().
>> + *
>> + * Color space conversions from YCbCr to RGB color space in planes is
>> + * supporter trough 3 optional properties in &drm_plane object.
>> + *
>> + * The &drm_crtc object's properties are:
>>   *
>>   * "DEGAMMA_LUT”:
>>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>> @@ -85,6 +90,28 @@
>>   * drm_mode_crtc_set_gamma_size(). Drivers which support both should use
>>   * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with the
>>   * "GAMMA_LUT" property above.
>> + *
>> + * The &drm_plane object's properties are:
>> + *
>> + * "YCBCR_TO_RGB_MODE"
>> + * 	Optional plane enum property to configure YCbCr to RGB
>> + * 	conversion. The possible modes include a number of standard
>> + * 	conversions and a possibility to select custom conversion
>> + * 	matrix and preoffset vector. The driver should select the
>> + *	supported subset of of the modes.
>> + *
>> + * "YCBCR_TO_RGB_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
>> + *	active dependent on YCBCR_TO_RGB_MODE property.
>> + *
>> + * "YCBCR_TO_RGB_PREOFFSET"
>> + *	Optional plane property blob to configure YCbCr offset before
>> + *	YCbCr to RGB CSC is applied. The blob contains struct
>> + *	drm_ycbcr_to_rgb_preoffset which is defined in
>> + *	uapi/drm/drm_mode.h. Whether this property is active dependent
>> + *	on YCBCR_TO_RGB_MODE property.
>>   */
>>  
>>  /**
>> @@ -330,3 +357,106 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
>>  	drm_modeset_unlock_all(dev);
>>  	return ret;
>>  }
>> +
>> +static char *ycbcr_to_rgb_mode_name[] = {
>> +	[DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT601_FULL] =
>> +		"YCbCr BT.601 limited range TO RGB BT.601 full range",
>> +	[DRM_PLANE_YCBCR_BT601_FULL_TO_RGB_BT601_FULL] =
>> +		"YCbCr BT.601 full range TO RGB BT.601 full range",
>> +	[DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT709_FULL] =
>> +		"YCbCr BT.709 limited range TO RGB BT.709 full range",
>> +	[DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT2020_FULL] =
>> +		"YCbCr BT.2020 limited range TO RGB BT.2020 full range",
>> +	[DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT709_FULL] =
>> +		"YCbCr BT.601 limited range TO RGB BT.709 full range",
>> +	[DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT2020_FULL] =
>> +		"YCbCr BT.601 limited range TO RGB BT.2020 full range",
>> +	[DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT601_FULL] =
>> +		"YCbCr BT.709 limited range TO RGB BT.601 full range",
>> +	[DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT2020_FULL] =
>> +		"YCbCr BT.709 limited range TO RGB BT.2020 full range",
>> +	[DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT601_FULL] =
>> +		"YCbCr BT.2020 limited range TO RGB BT.601 full range",
>> +	[DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT709_FULL] =
>> +		"YCbCr BT.2020 limited range TO RGB BT.709 full range",
>> +	[DRM_PLANE_YCBCR_TO_RGB_CSC_LIM_PREOFFSET] =
>> +		"YCbCr TO RGB CSC limited range preoffset",
>> +	[DRM_PLANE_YCBCR_TO_RGB_CSC_FULL_PREOFFSET] =
>> +		"YCbCr TO RGB CSC full range preoffset",
>> +	[DRM_PLANE_YCBCR_TO_RGB_CSC_PREOFFSET_VECTOR] =
>> +		"YCBCR TO RGB CSC preoffset vector",
>> +};
>> +
>> +/**
>> + * drm_plane_create_ycbcr_to_rgb_properties - ycbcr to rgb related properties
>> + * @enum_list: drm_prop_enum_list array of supported modes without names
>> + * @enum_list_len: length of enum_list array
>> + * @default_mode: default csc mode
>> + *
>> + * Create and attach plane specific YCbCr to RGB conversion related
>> + * properties to to the drm_plane object. The YCBCR_TO_RGB_MODE propety
>> + * created and an the supported modes should be provided the enum_list
>> + * parameter. YCBCR_TO_RGB_CSC and YCBCR_TO_RGB_PREOFFSET properties are
>> + * created based on supported conversion modes. The enum_list parameter
>> + * should not contain the enum names, because the standard names are
>> + * added by this function.
>> + */
>> +int drm_plane_create_ycbcr_to_rgb_properties(struct drm_plane *plane,
>> +			struct drm_prop_enum_list *enum_list,
>> +			unsigned int enum_list_len,
>> +			enum drm_plane_ycbcr_to_rgb_mode default_mode)
>> +{
>> +	struct drm_device *dev = plane->dev;
>> +	struct drm_property *prop;
>> +	bool ycbcr_to_rgb_csc_create = false;
>> +	bool ycbcr_to_rgb_preoffset_create = false;
>> +	int i;
>> +
>> +	if (WARN_ON(plane->ycbcr_to_rgb_mode_property != NULL))
>> +		return -EEXIST;
>> +
>> +	for (i = 0; i < enum_list_len; i++) {
>> +		enum drm_plane_ycbcr_to_rgb_mode mode = enum_list[i].type;
>> +
>> +		enum_list[i].name = ycbcr_to_rgb_mode_name[mode];
>> +
>> +		if (mode == DRM_PLANE_YCBCR_TO_RGB_CSC_LIM_PREOFFSET ||
>> +		    mode == DRM_PLANE_YCBCR_TO_RGB_CSC_FULL_PREOFFSET)
>> +			ycbcr_to_rgb_csc_create = true;
>> +
>> +		if (mode == DRM_PLANE_YCBCR_TO_RGB_CSC_PREOFFSET_VECTOR) {
>> +			ycbcr_to_rgb_csc_create = true;
>> +			ycbcr_to_rgb_preoffset_create = true;
>> +		}
>> +	}
>> +
>> +	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
>> +					"YCBCR_TO_RGB_MODE",
>> +					enum_list, enum_list_len);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	plane->ycbcr_to_rgb_mode_property = prop;
>> +	drm_object_attach_property(&plane->base, prop, default_mode);
>> +
>> +	if (ycbcr_to_rgb_csc_create) {
>> +		prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC |
>> +					   DRM_MODE_PROP_BLOB,
>> +					   "YCBCR_TO_RGB_CSC", 0);
>> +		if (!prop)
>> +			return -ENOMEM;
>> +		plane->ycbcr_to_rgb_csc_property = prop;
>> +		drm_object_attach_property(&plane->base, prop, 0);
>> +	}
>> +
>> +	if (ycbcr_to_rgb_preoffset_create) {
>> +		prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC |
>> +					   DRM_MODE_PROP_BLOB,
>> +					   "YCBCR_TO_RGB_PREOFFSET", 0);
>> +		if (!prop)
>> +			return -ENOMEM;
>> +		plane->ycbcr_to_rgb_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 bc71aa2..4c7e827 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -244,6 +244,16 @@ void drm_plane_cleanup(struct drm_plane *plane)
>>  
>>  	kfree(plane->name);
>>  
>> +	if (plane->ycbcr_to_rgb_mode_property)
>> +		drm_property_destroy(dev, plane->ycbcr_to_rgb_mode_property);
>> +
>> +	if (plane->ycbcr_to_rgb_csc_property)
>> +		drm_property_destroy(dev, plane->ycbcr_to_rgb_csc_property);
>> +
>> +	if (plane->ycbcr_to_rgb_preoffset_property)
>> +		drm_property_destroy(dev,
>> +				     plane->ycbcr_to_rgb_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 03a59cb..a20b3ff 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -26,6 +26,8 @@
>>  #include <linux/ctype.h>
>>  
>>  struct drm_crtc;
>> +struct drm_plane;
>> +struct drm_prop_enum_list;
>>  
>>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
>>  
>> @@ -37,4 +39,25 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>>  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>>  				 int gamma_size);
>>  
>> +enum drm_plane_ycbcr_to_rgb_mode {
>> +	DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT601_FULL = 0,
>> +	DRM_PLANE_YCBCR_BT601_FULL_TO_RGB_BT601_FULL,
>> +	DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT709_FULL,
>> +	DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT2020_FULL,
>> +	DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT709_FULL,
>> +	DRM_PLANE_YCBCR_BT601_LIM_TO_RGB_BT2020_FULL,
>> +	DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT601_FULL,
>> +	DRM_PLANE_YCBCR_BT709_LIM_TO_RGB_BT2020_FULL,
>> +	DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT601_FULL,
>> +	DRM_PLANE_YCBCR_BT2020_LIM_TO_RGB_BT709_FULL,
>> +	DRM_PLANE_YCBCR_TO_RGB_CSC_LIM_PREOFFSET,
>> +	DRM_PLANE_YCBCR_TO_RGB_CSC_FULL_PREOFFSET,
>> +	DRM_PLANE_YCBCR_TO_RGB_CSC_PREOFFSET_VECTOR,
>> +};
>> +
>> +int drm_plane_create_ycbcr_to_rgb_properties(struct drm_plane *plane,
>> +			struct drm_prop_enum_list *enum_list,
>> +			unsigned int enum_list_len,
>> +			enum drm_plane_ycbcr_to_rgb_mode default_mode);
>> +
>>  #endif
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> index 9ab3e70..41dcde2 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -26,6 +26,7 @@
>>  #include <linux/list.h>
>>  #include <linux/ctype.h>
>>  #include <drm/drm_mode_object.h>
>> +#include <drm/drm_color_mgmt.h>
>>  
>>  struct drm_crtc;
>>  struct drm_printer;
>> @@ -112,6 +113,11 @@ struct drm_plane_state {
>>  	unsigned int zpos;
>>  	unsigned int normalized_zpos;
>>  
>> +	/* YCbCr to RGB conversion */
>> +	enum drm_plane_ycbcr_to_rgb_mode ycbcr_to_rgb_mode;
>> +	struct drm_property_blob *ycbcr_to_rgb_csc;
>> +	struct drm_property_blob *ycbcr_to_rgb_preoffset;
>> +
>>  	/* Clipped coordinates */
>>  	struct drm_rect src, dst;
>>  
>> @@ -523,6 +529,10 @@ struct drm_plane {
>>  
>>  	struct drm_property *zpos_property;
>>  	struct drm_property *rotation_property;
>> +
>> +	struct drm_property *ycbcr_to_rgb_mode_property;
>> +	struct drm_property *ycbcr_to_rgb_csc_property;
>> +	struct drm_property *ycbcr_to_rgb_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..27e0bee 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_to_rgb_csc {
>> +	/* Conversion matrix in 2-complement s32.32 format. */
>> +	__s64 ry, rcb, rcr;
>> +	__s64 gy, gcb, gcr;
>> +	__s64 by, bcb, bcr;
>> +};
>> +
>> +struct drm_ycbcr_to_rgb_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
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://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