Regards

Shashank


On 5/5/2017 12:28 PM, Daniel Vetter wrote:
On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote:
On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote:
On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote:
Add standard optinal property blobs for YCbCr to RGB conversion CSC
matrix and YCbCr preoffset vector in DRM planes. New enums are defined
to YCBCR_ENCODING property to activate the CSC and preoffset
properties. For simplicity the new properties are stored in the
drm_plane object, because the YCBCR_ENCODING is already there. The
blob contents are defined in the uapi/drm/drm_mode.h header.

Signed-off-by: Jyri Sarha <jsa...@ti.com>
Not sure we want to make this yuv2rgb specific, plenty of planes have
generic degamma/offset, csc, gamme/offset hw. I think what we want instead
is the generic csc support, plus a ycbcr2rgb mode of "bypass".
My idea is to not expose this. And instead just expose a normal
ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm
that can't be done by the hw.
But that means we'd need to have a bit-perfect match with a canonical
conversion matrix (even if maybe the hw has a rounding bug and implements
something slightly different than the standard). That seems a bit an
awkward interface. Or is your idea that hw with a ctm would simply not
expose the colorspace enum, if it doesn't also have fixed-function bits on
top of the ctm?
-Daniel
I think the idea is to have separate properties for CTM and Gamut Mapping, so that we can have more control over linear and non-linear data blocks. All transformation should happen in one property whereas all gamut mapping should go into other, which IMHO seems to be the correct way to operate.

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

---
  drivers/gpu/drm/drm_atomic.c        | 19 +++++++++++++
  drivers/gpu/drm/drm_atomic_helper.c |  9 ++++++
  drivers/gpu/drm/drm_color_mgmt.c    | 55 +++++++++++++++++++++++++++++++++++--
  drivers/gpu/drm/drm_plane.c         |  6 ++++
  include/drm/drm_color_mgmt.h        |  3 ++
  include/drm/drm_plane.h             |  4 +++
  include/uapi/drm/drm_mode.h         | 12 ++++++++
  7 files changed, 106 insertions(+), 2 deletions(-)

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

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to