| Message ID | c897e37cfd059a86b9cbc77fd7699e014455fd60.1497214705.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jyri,
[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20170609]
[cannot apply to v4.12-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jyri-Sarha/drm-Add-optional-COLOR_ENCODING-and-COLOR_RANGE-properties-to-drm_plane/20170612-051658
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-randconfig-x010-201724 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_color_mgmt.c: In function 'drm_plane_create_color_properties':
>> drivers/gpu/drm/drm_color_mgmt.c:395:23: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
enum_list[len].name = color_encoding_name[i];
^
drivers/gpu/drm/drm_color_mgmt.c:414:23: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
enum_list[len].name = color_range_name[i];
^
vim +/const +395 drivers/gpu/drm/drm_color_mgmt.c
379 u32 supported_ranges,
380 enum drm_color_encoding default_encoding,
381 enum drm_color_range default_range)
382 {
383 struct drm_device *dev = plane->dev;
384 struct drm_property *prop;
385 struct drm_prop_enum_list enum_list[max(DRM_COLOR_ENCODING_MAX,
386 DRM_COLOR_RANGE_MAX)];
387 int i, len;
388
389 len = 0;
390 for (i = 0; i < DRM_COLOR_ENCODING_MAX; i++) {
391 if ((supported_encodings & BIT(i)) == 0)
392 continue;
393
394 enum_list[len].type = i;
> 395 enum_list[len].name = color_encoding_name[i];
396 len++;
397 }
398
399 prop = drm_property_create_enum(dev, 0, "COLOR_ENCODING",
400 enum_list, len);
401 if (!prop)
402 return -ENOMEM;
403 plane->color_encoding_property = prop;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Jun 12, 2017 at 12:10:04AM +0300, Jyri Sarha wrote: > 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. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/drm_atomic.c | 8 ++++ > drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_color_mgmt.h | 19 +++++++++ > include/drm/drm_plane.h | 8 ++++ > 4 files changed, 126 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 77dcef0..8067681 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -786,6 +786,10 @@ 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); > @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -88,6 +88,19 @@ > * 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: > + * > + * "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. > */ > > /** > @@ -336,3 +349,81 @@ 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", I was just looking at this again (in the hopes of landing it finally), and then I realized we probably need to split this BT.2020 into constant and non-constant luminance variants. ..._BT2020_CL and ..._BT2020_NCL maybe? > +}; > + > +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 > + * @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 to the drm_plane object. The supported encodings and > + * ranges should be provided in supported_encodings and > + * supported_ranges bitmasks. Each bit set in the bitmask indicates > + * the its number as enum value being 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; > + > + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { > unsigned int zpos; > unsigned int normalized_zpos; > > + /* Color encoding for non RGB formats */ > + enum drm_color_encoding color_encoding; > + enum drm_color_range color_range; > + > /* Clipped coordinates */ > struct drm_rect src, dst; > > @@ -523,6 +528,9 @@ struct drm_plane { > > struct drm_property *zpos_property; > struct drm_property *rotation_property; > + > + struct drm_property *color_encoding_property; > + struct drm_property *color_range_property; > }; > > #define obj_to_plane(x) container_of(x, struct drm_plane, base) > -- > 1.9.1
On 02/06/2018 06:58 PM, Ville Syrjälä wrote: > On Mon, Jun 12, 2017 at 12:10:04AM +0300, Jyri Sarha wrote: >> 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. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> drivers/gpu/drm/drm_atomic.c | 8 ++++ >> drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ >> include/drm/drm_color_mgmt.h | 19 +++++++++ >> include/drm/drm_plane.h | 8 ++++ >> 4 files changed, 126 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >> index 77dcef0..8067681 100644 >> --- a/drivers/gpu/drm/drm_atomic.c >> +++ b/drivers/gpu/drm/drm_atomic.c >> @@ -786,6 +786,10 @@ 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); >> @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 >> --- a/drivers/gpu/drm/drm_color_mgmt.c >> +++ b/drivers/gpu/drm/drm_color_mgmt.c >> @@ -88,6 +88,19 @@ >> * 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: >> + * >> + * "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. >> */ >> >> /** >> @@ -336,3 +349,81 @@ 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", > > I was just looking at this again (in the hopes of landing it finally), > and then I realized we probably need to split this BT.2020 into > constant and non-constant luminance variants. > > ..._BT2020_CL and ..._BT2020_NCL maybe? This is what V4L2 uses: https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html V4L2_YCBCR_ENC_BT2020 and V4L2_YCBCR_ENC_BT2020_CONST_LUM. I've never seen V4L2_YCBCR_ENC_BT2020_CONST_LUM in the wild. Non-constant Bt.2020 is what is normally used, so I'd keep the define just as-is: DRM_COLOR_YCBCR_BT2020. And add a DRM_COLOR_YCBCR_BT2020_CONST_LUM if you think you need it. I've never seen anyone use this. And in case you wonder why: this is the BT.2020 OpenGL Y'CbCr to R'G'B' conversion: float r = y + 1.4719 * v; float g = y - 0.1646 * u - 0.5703 * v; float b = y + 1.8814 * u; And this for BT.2020 constant luminance: float b = u <= 0.0 ? y + 1.9404 * u : y + 1.5816 * u; float r = v <= 0.0 ? y + 1.7184 * v : y + 0.9936 * v; float lin_r = (r < 0.081) ? r / 4.5 : pow((r + 0.099) / 1.099, 1.0 / 0.45); float lin_b = (b < 0.081) ? b / 4.5 : pow((b + 0.099) / 1.099, 1.0 / 0.45); float lin_y = (y < 0.081) ? y / 4.5 : pow((y + 0.099) / 1.099, 1.0 / 0.45); float lin_g = lin_y / 0.6780 - lin_r * 0.2627 / 0.6780 - lin_b * 0.0593 / 0.6780; float g = (lin_g < 0.018) ? lin_g * 4.5 : 1.099 * pow(lin_g, 0.45) - 0.099; Brr :-) Regards, Hans > >> +}; >> + >> +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 >> + * @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 to the drm_plane object. The supported encodings and >> + * ranges should be provided in supported_encodings and >> + * supported_ranges bitmasks. Each bit set in the bitmask indicates >> + * the its number as enum value being 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; >> + >> + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { >> unsigned int zpos; >> unsigned int normalized_zpos; >> >> + /* Color encoding for non RGB formats */ >> + enum drm_color_encoding color_encoding; >> + enum drm_color_range color_range; >> + >> /* Clipped coordinates */ >> struct drm_rect src, dst; >> >> @@ -523,6 +528,9 @@ struct drm_plane { >> >> struct drm_property *zpos_property; >> struct drm_property *rotation_property; >> + >> + struct drm_property *color_encoding_property; >> + struct drm_property *color_range_property; >> }; >> >> #define obj_to_plane(x) container_of(x, struct drm_plane, base) >> -- >> 1.9.1 >
I have not been following the subject for half a year now. Just let me know if I can do something to help land this patch, e.g. rebase and resend it. On 02/06/18 20:08, Hans Verkuil wrote: > On 02/06/2018 06:58 PM, Ville Syrjälä wrote: >> On Mon, Jun 12, 2017 at 12:10:04AM +0300, Jyri Sarha wrote: >>> 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. >>> >>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>> --- >>> drivers/gpu/drm/drm_atomic.c | 8 ++++ >>> drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ >>> include/drm/drm_color_mgmt.h | 19 +++++++++ >>> include/drm/drm_plane.h | 8 ++++ >>> 4 files changed, 126 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >>> index 77dcef0..8067681 100644 >>> --- a/drivers/gpu/drm/drm_atomic.c >>> +++ b/drivers/gpu/drm/drm_atomic.c >>> @@ -786,6 +786,10 @@ 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); >>> @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 >>> --- a/drivers/gpu/drm/drm_color_mgmt.c >>> +++ b/drivers/gpu/drm/drm_color_mgmt.c >>> @@ -88,6 +88,19 @@ >>> * 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: >>> + * >>> + * "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. >>> */ >>> >>> /** >>> @@ -336,3 +349,81 @@ 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", >> >> I was just looking at this again (in the hopes of landing it finally), >> and then I realized we probably need to split this BT.2020 into >> constant and non-constant luminance variants. >> >> ..._BT2020_CL and ..._BT2020_NCL maybe? > > This is what V4L2 uses: > > https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html > > V4L2_YCBCR_ENC_BT2020 and V4L2_YCBCR_ENC_BT2020_CONST_LUM. > > I've never seen V4L2_YCBCR_ENC_BT2020_CONST_LUM in the wild. Non-constant > Bt.2020 is what is normally used, so I'd keep the define just as-is: > DRM_COLOR_YCBCR_BT2020. And add a DRM_COLOR_YCBCR_BT2020_CONST_LUM if > you think you need it. I've never seen anyone use this. > > And in case you wonder why: this is the BT.2020 OpenGL Y'CbCr to R'G'B' > conversion: > > float r = y + 1.4719 * v; > float g = y - 0.1646 * u - 0.5703 * v; > float b = y + 1.8814 * u; > > And this for BT.2020 constant luminance: > > float b = u <= 0.0 ? y + 1.9404 * u : y + 1.5816 * u; > float r = v <= 0.0 ? y + 1.7184 * v : y + 0.9936 * v; > float lin_r = (r < 0.081) ? r / 4.5 : pow((r + 0.099) / 1.099, 1.0 / 0.45); > float lin_b = (b < 0.081) ? b / 4.5 : pow((b + 0.099) / 1.099, 1.0 / 0.45); > float lin_y = (y < 0.081) ? y / 4.5 : pow((y + 0.099) / 1.099, 1.0 / 0.45); > float lin_g = lin_y / 0.6780 - lin_r * 0.2627 / 0.6780 - lin_b * 0.0593 / 0.6780; > float g = (lin_g < 0.018) ? lin_g * 4.5 : 1.099 * pow(lin_g, 0.45) - 0.099; > > Brr :-) > > Regards, > > Hans > >> >>> +}; >>> + >>> +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 >>> + * @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 to the drm_plane object. The supported encodings and >>> + * ranges should be provided in supported_encodings and >>> + * supported_ranges bitmasks. Each bit set in the bitmask indicates >>> + * the its number as enum value being 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; >>> + >>> + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { >>> unsigned int zpos; >>> unsigned int normalized_zpos; >>> >>> + /* Color encoding for non RGB formats */ >>> + enum drm_color_encoding color_encoding; >>> + enum drm_color_range color_range; >>> + >>> /* Clipped coordinates */ >>> struct drm_rect src, dst; >>> >>> @@ -523,6 +528,9 @@ struct drm_plane { >>> >>> struct drm_property *zpos_property; >>> struct drm_property *rotation_property; >>> + >>> + struct drm_property *color_encoding_property; >>> + struct drm_property *color_range_property; >>> }; >>> >>> #define obj_to_plane(x) container_of(x, struct drm_plane, base) >>> -- >>> 1.9.1 >> >
On Tue, Feb 06, 2018 at 08:16:56PM +0200, Jyri Sarha wrote: > > I have not been following the subject for half a year now. Just let me > know if I can do something to help land this patch, e.g. rebase and > resend it. I guess I can just pick it up and repost along with my i915 stuff. I can just toss in the const luminance thing on top I suppose. > > > On 02/06/18 20:08, Hans Verkuil wrote: > > On 02/06/2018 06:58 PM, Ville Syrjälä wrote: > >> On Mon, Jun 12, 2017 at 12:10:04AM +0300, Jyri Sarha wrote: > >>> 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. > >>> > >>> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >>> --- > >>> drivers/gpu/drm/drm_atomic.c | 8 ++++ > >>> drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ > >>> include/drm/drm_color_mgmt.h | 19 +++++++++ > >>> include/drm/drm_plane.h | 8 ++++ > >>> 4 files changed, 126 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > >>> index 77dcef0..8067681 100644 > >>> --- a/drivers/gpu/drm/drm_atomic.c > >>> +++ b/drivers/gpu/drm/drm_atomic.c > >>> @@ -786,6 +786,10 @@ 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); > >>> @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 > >>> --- a/drivers/gpu/drm/drm_color_mgmt.c > >>> +++ b/drivers/gpu/drm/drm_color_mgmt.c > >>> @@ -88,6 +88,19 @@ > >>> * 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: > >>> + * > >>> + * "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. > >>> */ > >>> > >>> /** > >>> @@ -336,3 +349,81 @@ 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", > >> > >> I was just looking at this again (in the hopes of landing it finally), > >> and then I realized we probably need to split this BT.2020 into > >> constant and non-constant luminance variants. > >> > >> ..._BT2020_CL and ..._BT2020_NCL maybe? > > > > This is what V4L2 uses: > > > > https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html > > > > V4L2_YCBCR_ENC_BT2020 and V4L2_YCBCR_ENC_BT2020_CONST_LUM. > > > > I've never seen V4L2_YCBCR_ENC_BT2020_CONST_LUM in the wild. Non-constant > > Bt.2020 is what is normally used, so I'd keep the define just as-is: > > DRM_COLOR_YCBCR_BT2020. And add a DRM_COLOR_YCBCR_BT2020_CONST_LUM if > > you think you need it. I've never seen anyone use this. > > > > And in case you wonder why: this is the BT.2020 OpenGL Y'CbCr to R'G'B' > > conversion: > > > > float r = y + 1.4719 * v; > > float g = y - 0.1646 * u - 0.5703 * v; > > float b = y + 1.8814 * u; > > > > And this for BT.2020 constant luminance: > > > > float b = u <= 0.0 ? y + 1.9404 * u : y + 1.5816 * u; > > float r = v <= 0.0 ? y + 1.7184 * v : y + 0.9936 * v; > > float lin_r = (r < 0.081) ? r / 4.5 : pow((r + 0.099) / 1.099, 1.0 / 0.45); > > float lin_b = (b < 0.081) ? b / 4.5 : pow((b + 0.099) / 1.099, 1.0 / 0.45); > > float lin_y = (y < 0.081) ? y / 4.5 : pow((y + 0.099) / 1.099, 1.0 / 0.45); > > float lin_g = lin_y / 0.6780 - lin_r * 0.2627 / 0.6780 - lin_b * 0.0593 / 0.6780; > > float g = (lin_g < 0.018) ? lin_g * 4.5 : 1.099 * pow(lin_g, 0.45) - 0.099; > > > > Brr :-) > > > > Regards, > > > > Hans > > > >> > >>> +}; > >>> + > >>> +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 > >>> + * @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 to the drm_plane object. The supported encodings and > >>> + * ranges should be provided in supported_encodings and > >>> + * supported_ranges bitmasks. Each bit set in the bitmask indicates > >>> + * the its number as enum value being 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; > >>> + > >>> + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { > >>> unsigned int zpos; > >>> unsigned int normalized_zpos; > >>> > >>> + /* Color encoding for non RGB formats */ > >>> + enum drm_color_encoding color_encoding; > >>> + enum drm_color_range color_range; > >>> + > >>> /* Clipped coordinates */ > >>> struct drm_rect src, dst; > >>> > >>> @@ -523,6 +528,9 @@ struct drm_plane { > >>> > >>> struct drm_property *zpos_property; > >>> struct drm_property *rotation_property; > >>> + > >>> + struct drm_property *color_encoding_property; > >>> + struct drm_property *color_range_property; > >>> }; > >>> > >>> #define obj_to_plane(x) container_of(x, struct drm_plane, base) > >>> -- > >>> 1.9.1 > >> > > > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Tue, Feb 06, 2018 at 07:08:34PM +0100, Hans Verkuil wrote: > On 02/06/2018 06:58 PM, Ville Syrjälä wrote: > > On Mon, Jun 12, 2017 at 12:10:04AM +0300, Jyri Sarha wrote: > >> 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. > >> > >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >> --- > >> drivers/gpu/drm/drm_atomic.c | 8 ++++ > >> drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ > >> include/drm/drm_color_mgmt.h | 19 +++++++++ > >> include/drm/drm_plane.h | 8 ++++ > >> 4 files changed, 126 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > >> index 77dcef0..8067681 100644 > >> --- a/drivers/gpu/drm/drm_atomic.c > >> +++ b/drivers/gpu/drm/drm_atomic.c > >> @@ -786,6 +786,10 @@ 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); > >> @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 > >> --- a/drivers/gpu/drm/drm_color_mgmt.c > >> +++ b/drivers/gpu/drm/drm_color_mgmt.c > >> @@ -88,6 +88,19 @@ > >> * 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: > >> + * > >> + * "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. > >> */ > >> > >> /** > >> @@ -336,3 +349,81 @@ 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", > > > > I was just looking at this again (in the hopes of landing it finally), > > and then I realized we probably need to split this BT.2020 into > > constant and non-constant luminance variants. > > > > ..._BT2020_CL and ..._BT2020_NCL maybe? > > This is what V4L2 uses: > > https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html > > V4L2_YCBCR_ENC_BT2020 and V4L2_YCBCR_ENC_BT2020_CONST_LUM. > > I've never seen V4L2_YCBCR_ENC_BT2020_CONST_LUM in the wild. Non-constant > Bt.2020 is what is normally used, so I'd keep the define just as-is: > DRM_COLOR_YCBCR_BT2020. And add a DRM_COLOR_YCBCR_BT2020_CONST_LUM if > you think you need it. I've never seen anyone use this. Hmm. I suppose that's a reasonable apporach. As long as we document that the the plain BT2020 enum value is in fact the non-const luminance variant. > > And in case you wonder why: this is the BT.2020 OpenGL Y'CbCr to R'G'B' > conversion: > > float r = y + 1.4719 * v; > float g = y - 0.1646 * u - 0.5703 * v; > float b = y + 1.8814 * u; > > And this for BT.2020 constant luminance: > > float b = u <= 0.0 ? y + 1.9404 * u : y + 1.5816 * u; > float r = v <= 0.0 ? y + 1.7184 * v : y + 0.9936 * v; > float lin_r = (r < 0.081) ? r / 4.5 : pow((r + 0.099) / 1.099, 1.0 / 0.45); > float lin_b = (b < 0.081) ? b / 4.5 : pow((b + 0.099) / 1.099, 1.0 / 0.45); > float lin_y = (y < 0.081) ? y / 4.5 : pow((y + 0.099) / 1.099, 1.0 / 0.45); > float lin_g = lin_y / 0.6780 - lin_r * 0.2627 / 0.6780 - lin_b * 0.0593 / 0.6780; > float g = (lin_g < 0.018) ? lin_g * 4.5 : 1.099 * pow(lin_g, 0.45) - 0.099; > > Brr :-) Yeah. In my HDR Weston fork I was doing the non-const luminace with a generic 4x4 matrix (could be 4x3 but that would have required some extension I didn't want to look up). Then I looked at adding the non-const luminance variant, and in the end decided that I didn't want to uglify the shader code with that :) If it wasn't for those darned different scaling factors for the <0.0 vs. >0.0 cases... > > Regards, > > Hans > > > > >> +}; > >> + > >> +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 > >> + * @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 to the drm_plane object. The supported encodings and > >> + * ranges should be provided in supported_encodings and > >> + * supported_ranges bitmasks. Each bit set in the bitmask indicates > >> + * the its number as enum value being 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; > >> + > >> + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { > >> unsigned int zpos; > >> unsigned int normalized_zpos; > >> > >> + /* Color encoding for non RGB formats */ > >> + enum drm_color_encoding color_encoding; > >> + enum drm_color_range color_range; > >> + > >> /* Clipped coordinates */ > >> struct drm_rect src, dst; > >> > >> @@ -523,6 +528,9 @@ struct drm_plane { > >> > >> struct drm_property *zpos_property; > >> struct drm_property *rotation_property; > >> + > >> + struct drm_property *color_encoding_property; > >> + struct drm_property *color_range_property; > >> }; > >> > >> #define obj_to_plane(x) container_of(x, struct drm_plane, base) > >> -- > >> 1.9.1 > >
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 77dcef0..8067681 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -786,6 +786,10 @@ 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); @@ -846,6 +850,10 @@ 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->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 3eda500..9077bf6 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -88,6 +88,19 @@ * 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: + * + * "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. */ /** @@ -336,3 +349,81 @@ 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 + * @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 to the drm_plane object. The supported encodings and + * ranges should be provided in supported_encodings and + * supported_ranges bitmasks. Each bit set in the bitmask indicates + * the its number as enum value being 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; + + 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 03a59cb..b3b6d30 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 9ab3e70..f4de5f9 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,10 @@ struct drm_plane_state { unsigned int zpos; unsigned int normalized_zpos; + /* Color encoding for non RGB formats */ + enum drm_color_encoding color_encoding; + enum drm_color_range color_range; + /* Clipped coordinates */ struct drm_rect src, dst; @@ -523,6 +528,9 @@ struct drm_plane { struct drm_property *zpos_property; struct drm_property *rotation_property; + + struct drm_property *color_encoding_property; + struct drm_property *color_range_property; }; #define obj_to_plane(x) container_of(x, struct drm_plane, base)
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. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/drm_atomic.c | 8 ++++ drivers/gpu/drm/drm_color_mgmt.c | 91 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_color_mgmt.h | 19 +++++++++ include/drm/drm_plane.h | 8 ++++ 4 files changed, 126 insertions(+)