[Intel-gfx] [v2, 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane

Hans Verkuil hverkuil at xs4all.nl
Fri Nov 30 14:40:46 UTC 2018


On 11/30/18 15:20, Andrzej Hajda wrote:
> Hi Ville,
> 
> As Christoph cannot respond till middle next week I can try to respond
> in his absence, as I am familiar with the subject.
> 
> On 30.11.2018 14:25, Ville Syrjälä wrote:
>> On Fri, Nov 30, 2018 at 02:08:11PM +0100, Christoph Manszewski wrote:
>>> Hi,
>>>
>>> I am looking for a way to export the color encoding and range selection
>>> to user space. I came across those properties and am wondering, why
>>> they are meant only for non RGB color encodings. Would it be okay, to
>>> modify them and use with RGB formats as well?
>> What you trying to do? Input limited range RGB data and expand to full
>> range?
> 
> 
> For example. But there are two more general questions, which
> surprisingly we have not found answer for.
> 
> 1. What color encoding and range drm should expect on its input RGB
> buffers by default?

While I am not a drm expert, I am pretty certain it always expects
full range RGB.

There is a real use-case for being able to give drm limited range RGB:
if the image was filled from an HDMI receiver and that receiver got
limited range RGB. That said, most (but not all) receivers can expand
it to full range before writing to memory.

> 2. How userspace should inform drm that given buffer has specified
> non-default color encoding and range?
> 
> 
> Hopefully this patch introduces such properties but only for YCbCr
> formats, the question is what should be the best way to expand it to RGB
> formats:
> 
> A. Add another enums: DRM_COLOR_RGB_BT601 and friends.
> 
> B. Reuse current enums, but remove format information from them:
> DRM_COLOR_YCBCR_BT601 => DRM_COLOR_BT601.

The colorspace (BT601, REC709, BT2020) is independent of the quantization
range (full/limited) and of the color encoding (YCbCr, RGB). There is also
the transfer function, another independent setting.

More background info is here:

https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces.html

If all you use is a desktop, then it is all simple: full range sRGB. But if
you start mixing video captured from an HDMI receiver, then it can become
much more complex.

Regards,

	Hans

> 
> 
> Regards
> 
> Andrzej
> 
>>
>>> Regards,
>>> Chris
>>>
>>>
>>> On 02/19/2018 09:28 PM, Ville Syrjala wrote:
>>>> From: Jyri Sarha <jsarha at ti.com>
>>>>
>>>> Add a standard optional properties to support different non RGB color
>>>> encodings in DRM planes. COLOR_ENCODING select the supported non RGB
>>>> color encoding, for instance ITU-R BT.709 YCbCr. COLOR_RANGE selects
>>>> the value ranges within the selected color encoding. The properties
>>>> are stored to drm_plane object to allow different set of supported
>>>> encoding for different planes on the device.
>>>>
>>>> v2: Add/fix kerneldocs, verify bitmasks (danvet)
>>>>
>>>> Cc: Harry Wentland <harry.wentland at amd.com>
>>>> Cc: Daniel Vetter <daniel at ffwll.ch>
>>>> Cc: Daniel Stone <daniel at fooishbar.org>
>>>> Cc: Russell King - ARM Linux <linux at armlinux.org.uk>
>>>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>>>> Cc: Hans Verkuil <hverkuil at xs4all.nl>
>>>> Cc: Uma Shankar <uma.shankar at intel.com>
>>>> Cc: Shashank Sharma <shashank.sharma at intel.com>
>>>> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>> Signed-off-by: Jyri Sarha <jsarha at ti.com>
>>>> [vsyrjala v2: Add/fix kerneldocs, verify bitmasks]
>>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>> ---
>>>>   drivers/gpu/drm/drm_atomic.c     |   8 +++
>>>>   drivers/gpu/drm/drm_color_mgmt.c | 103 +++++++++++++++++++++++++++++++++++++++
>>>>   include/drm/drm_color_mgmt.h     |  19 ++++++++
>>>>   include/drm/drm_plane.h          |  32 ++++++++++++
>>>>   4 files changed, 162 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 46733d534587..452a0b0bafbc 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -759,6 +759,10 @@ static 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->color_encoding_property) {
>>>> +		state->color_encoding = val;
>>>> +	} else if (property == plane->color_range_property) {
>>>> +		state->color_range = val;
>>>>   	} else if (plane->funcs->atomic_set_property) {
>>>>   		return plane->funcs->atomic_set_property(plane, state,
>>>>   				property, val);
>>>> @@ -818,6 +822,10 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>>>>   		*val = state->rotation;
>>>>   	} else if (property == plane->zpos_property) {
>>>>   		*val = state->zpos;
>>>> +	} else if (property == plane->color_encoding_property) {
>>>> +		*val = state->color_encoding;
>>>> +	} else if (property == plane->color_range_property) {
>>>> +		*val = state->color_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 0d002b045bd2..4b83e078d3e9 100644
>>>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>>>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>>>> @@ -88,6 +88,20 @@
>>>>    * 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.
>>>> + *
>>>> + * Support for different non RGB color encodings is controlled through
>>>> + * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties. They
>>>> + * are set up by calling drm_plane_create_color_properties().
>>>> + *
>>>> + * "COLOR_ENCODING"
>>>> + * 	Optional plane enum property to support different non RGB
>>>> + * 	color encodings. The driver can provide a subset of standard
>>>> + * 	enum values supported by the DRM plane.
>>>> + *
>>>> + * "COLOR_RANGE"
>>>> + * 	Optional plane enum property to support different non RGB
>>>> + * 	color parameter ranges. The driver can provide a subset of
>>>> + * 	standard enum values supported by the DRM plane.
>>>>    */
>>>>   
>>>>   /**
>>>> @@ -339,3 +353,92 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
>>>>   	drm_modeset_unlock(&crtc->mutex);
>>>>   	return ret;
>>>>   }
>>>> +
>>>> +static const char * const color_encoding_name[] = {
>>>> +	[DRM_COLOR_YCBCR_BT601] = "ITU-R BT.601 YCbCr",
>>>> +	[DRM_COLOR_YCBCR_BT709] = "ITU-R BT.709 YCbCr",
>>>> +	[DRM_COLOR_YCBCR_BT2020] = "ITU-R BT.2020 YCbCr",
>>>> +};
>>>> +
>>>> +static const char * const color_range_name[] = {
>>>> +	[DRM_COLOR_YCBCR_FULL_RANGE] = "YCbCr full range",
>>>> +	[DRM_COLOR_YCBCR_LIMITED_RANGE] = "YCbCr limited range",
>>>> +};
>>>> +
>>>> +/**
>>>> + * drm_plane_create_color_properties - color encoding related plane properties
>>>> + * @plane: plane object
>>>> + * @supported_encodings: bitfield indicating supported color encodings
>>>> + * @supported_ranges: bitfileld indicating supported color ranges
>>>> + * @default_encoding: default color encoding
>>>> + * @default_range: default color range
>>>> + *
>>>> + * Create and attach plane specific COLOR_ENCODING and COLOR_RANGE
>>>> + * properties to @plane. The supported encodings and ranges should
>>>> + * be provided in supported_encodings and supported_ranges bitmasks.
>>>> + * Each bit set in the bitmask indicates that its number as enum
>>>> + * value is supported.
>>>> + */
>>>> +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)
>>>> +{
>>>> +	struct drm_device *dev = plane->dev;
>>>> +	struct drm_property *prop;
>>>> +	struct drm_prop_enum_list enum_list[max(DRM_COLOR_ENCODING_MAX,
>>>> +						DRM_COLOR_RANGE_MAX)];
>>>> +	int i, len;
>>>> +
>>>> +	if (WARN_ON(supported_encodings == 0 ||
>>>> +		    (supported_encodings & -BIT(DRM_COLOR_ENCODING_MAX)) != 0 ||
>>>> +		    (supported_encodings & BIT(default_encoding)) == 0))
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (WARN_ON(supported_ranges == 0 ||
>>>> +		    (supported_ranges & -BIT(DRM_COLOR_RANGE_MAX)) != 0 ||
>>>> +		    (supported_ranges & BIT(default_range)) == 0))
>>>> +		return -EINVAL;
>>>> +
>>>> +	len = 0;
>>>> +	for (i = 0; i < DRM_COLOR_ENCODING_MAX; i++) {
>>>> +		if ((supported_encodings & BIT(i)) == 0)
>>>> +			continue;
>>>> +
>>>> +		enum_list[len].type = i;
>>>> +		enum_list[len].name = color_encoding_name[i];
>>>> +		len++;
>>>> +	}
>>>> +
>>>> +	prop = drm_property_create_enum(dev, 0, "COLOR_ENCODING",
>>>> +					enum_list, len);
>>>> +	if (!prop)
>>>> +		return -ENOMEM;
>>>> +	plane->color_encoding_property = prop;
>>>> +	drm_object_attach_property(&plane->base, prop, default_encoding);
>>>> +	if (plane->state)
>>>> +		plane->state->color_encoding = default_encoding;
>>>> +
>>>> +	len = 0;
>>>> +	for (i = 0; i < DRM_COLOR_RANGE_MAX; i++) {
>>>> +		if ((supported_ranges & BIT(i)) == 0)
>>>> +			continue;
>>>> +
>>>> +		enum_list[len].type = i;
>>>> +		enum_list[len].name = color_range_name[i];
>>>> +		len++;
>>>> +	}
>>>> +
>>>> +	prop = drm_property_create_enum(dev, 0,	"COLOR_RANGE",
>>>> +					enum_list, len);
>>>> +	if (!prop)
>>>> +		return -ENOMEM;
>>>> +	plane->color_range_property = prop;
>>>> +	drm_object_attach_property(&plane->base, prop, default_range);
>>>> +	if (plane->state)
>>>> +		plane->state->color_range = default_range;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_plane_create_color_properties);
>>>> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
>>>> index 03a59cbce621..b3b6d302ca8c 100644
>>>> --- a/include/drm/drm_color_mgmt.h
>>>> +++ b/include/drm/drm_color_mgmt.h
>>>> @@ -26,6 +26,7 @@
>>>>   #include <linux/ctype.h>
>>>>   
>>>>   struct drm_crtc;
>>>> +struct drm_plane;
>>>>   
>>>>   uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
>>>>   
>>>> @@ -37,4 +38,22 @@ 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_color_encoding {
>>>> +	DRM_COLOR_YCBCR_BT601,
>>>> +	DRM_COLOR_YCBCR_BT709,
>>>> +	DRM_COLOR_YCBCR_BT2020,
>>>> +	DRM_COLOR_ENCODING_MAX,
>>>> +};
>>>> +
>>>> +enum drm_color_range {
>>>> +	DRM_COLOR_YCBCR_LIMITED_RANGE,
>>>> +	DRM_COLOR_YCBCR_FULL_RANGE,
>>>> +	DRM_COLOR_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);
>>>>   #endif
>>>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>>>> index 8185e3468a23..f7bf4a48b1c3 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,20 @@ struct drm_plane_state {
>>>>   	unsigned int zpos;
>>>>   	unsigned int normalized_zpos;
>>>>   
>>>> +	/**
>>>> +	 * @color_encoding:
>>>> +	 *
>>>> +	 * Color encoding for non RGB formats
>>>> +	 */
>>>> +	enum drm_color_encoding color_encoding;
>>>> +
>>>> +	/**
>>>> +	 * @color_range:
>>>> +	 *
>>>> +	 * Color range for non RGB formats
>>>> +	 */
>>>> +	enum drm_color_range color_range;
>>>> +
>>>>   	/* Clipped coordinates */
>>>>   	struct drm_rect src, dst;
>>>>   
>>>> @@ -558,6 +573,23 @@ struct drm_plane {
>>>>   
>>>>   	struct drm_property *zpos_property;
>>>>   	struct drm_property *rotation_property;
>>>> +
>>>> +	/**
>>>> +	 * @color_encoding_property:
>>>> +	 *
>>>> +	 * Optional "COLOR_ENCODING" enum property for specifying
>>>> +	 * color encoding for non RGB formats.
>>>> +	 * See drm_plane_create_color_properties().
>>>> +	 */
>>>> +	struct drm_property *color_encoding_property;
>>>> +	/**
>>>> +	 * @color_range_property:
>>>> +	 *
>>>> +	 * Optional "COLOR_RANGE" enum property for specifying
>>>> +	 * color range for non RGB formats.
>>>> +	 * See drm_plane_create_color_properties().
>>>> +	 */
>>>> +	struct drm_property *color_range_property;
>>>>   };
>>>>   
>>>>   #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> 
> 



More information about the Intel-gfx mailing list