[PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties
Sharma, Shashank
shashank.sharma at intel.com
Fri Apr 21 12:58:06 UTC 2017
Regards
Shashank
On 4/21/2017 4:47 PM, Ville Syrjälä 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 at ti.com>
>> ---
>> 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",
> We probably don't want to conflate the YCbCr->RGB part with the colorspace
> conversion because the YCbCr->RGB part should be performed on gamma encoded
> data and the colorspace conversion on linear data. So we need a degamma
> stage in between. At least that seemed to be the general concencus after
> the last round of mails on this topic.
>
> After staring at the v4l docs on this stuff I kinda like their
> "encoding" terminology to describe the YCbCr->RGB part, so I'm now a
> little partial to calling the prop something like YCBCR_ENCODING. OTOH
> if we want to expose the raw matrix as a blob then maybe calling it a
> CSC might be better. Not sure there's much point in exposing it though.
> I don't think most people are in the habit if cooking up new ways to
> encode their pixel data.
Ville has already conveyed the zest of this design. When we want to
blend the data targeting
various framebuffers, we have to consider their color spaces too. So we
should actually come up
with a set of properties, which in a combination (and in sequence)
provide the capability to blend
a Rec 709 and Rec 2020 buffer.
I was supposed to publish a design RFC doc for the same, but it got
delayed from my side.
Will publish that in parallel, to see if we can merge these two thoughts.
- Shashank
>> + [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
More information about the dri-devel
mailing list