[RFC v5 0/8] Add Plane Color Properties

[RFC v5 0/8] Add Plane Color Properties

[Uma Shankar]

This is how a typical display color hardware pipeline looks like:
 +-------------------------------------------+
 |                RAM                        |
 |  +------+    +---------+    +---------+   |
 |  | FB 1 |    |  FB 2   |    | FB N    |   |
 |  +------+    +---------+    +---------+   |
 +-------------------------------------------+
       |  Plane Color Hardware Block |
 +--------------------------------------------+
 | +---v-----+   +---v-------+   +---v------+ |
 | | Plane A |   | Plane B   |   | Plane N  | |
 | | DeGamma |   | Degamma   |   | Degamma  | |
 | +---+-----+   +---+-------+   +---+------+ |
 |     |             |               |        |
 | +---v-----+   +---v-------+   +---v------+ |
 | |Plane A  |   | Plane B   |   | Plane N  | |
 | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
 | +---+-----+   +----+------+   +----+-----+ |
 |     |              |               |       |
 | +---v-----+   +----v------+   +----v-----+ |
 | | Plane A |   | Plane B   |   | Plane N  | |
 | | Gamma   |   | Gamma     |   | Gamma    | |
 | +---+-----+   +----+------+   +----+-----+ |
 |     |              |               |       |
 +--------------------------------------------+
+------v--------------v---------------v-------|
||                                           ||
||           Pipe Blender                    ||
+--------------------+------------------------+
|                    |                        |
|        +-----------v----------+             |
|        |  Pipe DeGamma        |             |
|        |                      |             |
|        +-----------+----------+             |
|                    |            Pipe Color  |
|        +-----------v----------+ Hardware    |
|        |  Pipe CSC/CTM        |             |
|        |                      |             |
|        +-----------+----------+             |
|                    |                        |
|        +-----------v----------+             |
|        |  Pipe Gamma          |             |
|        |                      |             |
|        +-----------+----------+             |
|                    |                        |
+---------------------------------------------+
                     |
                     v
               Pipe Output

This patch series adds properties for plane color features. It adds
properties for degamma used to linearize data, CSC used for gamut
conversion, and gamma used to again non-linearize data as per panel
supported color space. These can be utilize by user space to convert
planes from one format to another, one color space to another etc.

Usersapce can take smart blending decisions and utilize these hardware
supported plane color features to get accurate color profile. The same
can help in consistent color quality from source to panel taking
advantage of advanced color features in hardware.

These patches just add the property interfaces and enable helper
functions.

This series adds Intel Gen9 specific plane gamma feature. We can
build up and add other platform/hardware specific implementation
on top of this series

Note: This is just to get a design feedback whether these interfaces
look ok. Based on community feedback on interfaces, we will implement
IGT tests to validate plane color features. This is un-tested currently.

Userspace implementation using these properties have been done in drm
hwcomposer by "Alexandru-Cosmin Gheorghe Alexandru-Cosmin.Gheorghe@arm.com"
from ARM. A merge request has been opened by Alexandru for drm_hwcomposer,
implementing the property changes for the same. Please review that as well:
https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25

v2: Dropped legacy gamma table for plane as suggested by Maarten. Added
Gen9/BDW plane gamma feature and rebase on tot.

v3: Added a new drm_color_lut_ext structure to accommodate 32 bit precision
entries, pointed to by Brian, Starkey for HDR usecases. Addressed Sean,Paul
comments and moved plane color properties to drm_plane instead of
mode_config. Added property documentation as suggested by Daniel, Vetter.
Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov.

v4: Rebase

v5: Added "Display Color Hardware Pipeline" flow to kernel
documentation as suggested by "Ville Syrjala" and "Brian Starkey".
Moved the property creation to drm_color_mgmt.c file to consolidate
all color operations at one place. Addressed Alexandru's review comments.

Uma Shankar (8):
  drm: Add Enhanced Gamma LUT precision structure
  drm: Add Plane Degamma properties
  drm: Add Plane CTM property
  drm: Add Plane Gamma properties
  drm: Define helper function for plane color enabling
  drm/i915: Enable plane color features
  drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
  drm/i915: Load plane color luts from atomic flip

 Documentation/gpu/drm-kms.rst             |  99 +++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c              |  32 +++++++
 drivers/gpu/drm/drm_atomic_helper.c       |  13 +++
 drivers/gpu/drm/drm_color_mgmt.c          | 140 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h           |   5 ++
 drivers/gpu/drm/i915/i915_pci.c           |   5 +-
 drivers/gpu/drm/i915/i915_reg.h           |  25 ++++++
 drivers/gpu/drm/i915/intel_atomic_plane.c |   4 +
 drivers/gpu/drm/i915/intel_color.c        |  80 +++++++++++++++++
 drivers/gpu/drm/i915/intel_device_info.h  |   5 ++
 drivers/gpu/drm/i915/intel_display.c      |   4 +
 drivers/gpu/drm/i915/intel_drv.h          |  10 +++
 drivers/gpu/drm/i915/intel_sprite.c       |   4 +
 include/drm/drm_color_mgmt.h              |   6 ++
 include/drm/drm_plane.h                   |  61 +++++++++++++
 include/uapi/drm/drm_mode.h               |  15 ++++
 16 files changed, 504 insertions(+), 4 deletions(-)

-- 
1.9.1

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

[RFC v5 1/8] drm: Add Enhanced Gamma LUT precision structure

[Uma Shankar]

Existing LUT precision structure is having only 16 bit
precision. This is not enough for upcoming enhanced hardwares
and advance usecases like HDR processing. Hence added a new
structure with 32 bit precision values. Also added the code,
for extracting the same from values passed from userspace.

v4: Rebase

v5: Relocated the helper function to drm_color_mgmt.c. Declared
the same in a header file (Alexandru Gheorghe)

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  1 +
 include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index b97e2de..1bdcc1a 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
 }
 EXPORT_SYMBOL(drm_color_lut_extract);
 
+/*
+ * Added to accommodate enhanced LUT precision.
+ * Max LUT precision is 32 bits.
+ */
+uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
+{
+	uint32_t val = user_input;
+	uint32_t max = 0xffffffff >> (32 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 32) {
+		val += 1UL << (32 - bit_precision - 1);
+		val >>= 32 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
+EXPORT_SYMBOL(drm_color_lut_extract_ext);
+
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 44f04233..78b5a37 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,6 +29,7 @@
 struct drm_plane;
 
 uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 8d67243..874407b 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -629,6 +629,21 @@ struct drm_color_lut {
 	__u16 reserved;
 };
 
+/*
+ * Creating 32 bit palette entries for better data
+ * precision. This will be required for HDR and
+ * similar color processing usecases.
+ */
+struct drm_color_lut_ext {
+	/*
+	 * Data is U0.32 fixed point format.
+	 */
+	__u32 red;
+	__u32 green;
+	__u32 blue;
+	__u32 reserved;
+};
+
 #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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 2/8] drm: Add Plane Degamma properties

[Uma Shankar]

Add Plane Degamma as a blob property and plane degamma size as
a range property.

v2: Rebase

v3: Fixed Sean, Paul's review comments. Moved the property from
mode_config to drm_plane. Created a helper function to instantiate
these properties and removed from drm_mode_create_standard_properties
Added property documentation as suggested by Daniel, Vetter.

v4: Rebase

v5: Added "Display Color Hardware Pipeline" flow to kernel
documentation as suggested by "Ville Syrjala" and "Brian Starkey".
Moved the property creation to drm_color_mgmt.c file to consolidate
all color operations at one place.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c        | 13 ++++++
 drivers/gpu/drm/drm_atomic_helper.c |  6 +++
 drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
 include/drm/drm_plane.h             | 24 ++++++++++
 5 files changed, 174 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index f8f5bf1..253d546 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -551,12 +551,102 @@ Plane Composition Properties
 Color Management Properties
 ---------------------------
 
+Below is how a typical hardware pipeline for color
+will look like:
+
+.. kernel-render:: DOT
+   :alt: Display Color Pipeline
+   :caption: Display Color Pipeline Overview
+
+   digraph "KMS" {
+      node [shape=box]
+
+      subgraph cluster_static {
+          style=dashed
+          label="Display Color Hardware Blocks"
+
+          node [bgcolor=grey style=filled]
+          "Plane Degamma A" -> "Plane CSC/CTM A"
+          "Plane CSC/CTM A" -> "Plane Gamma A"
+          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
+          "Plane Gamma A" -> "Pipe Blender"
+	  "Pipe Blender" -> "Pipe DeGamma"
+          "Pipe DeGamma" -> "Pipe CSC/CTM"
+          "Pipe CSC/CTM" -> "Pipe Gamma"
+          "Pipe Gamma" -> "Pipe Output"
+      }
+
+      subgraph cluster_static {
+          style=dashed
+
+          node [shape=box]
+          "Plane Degamma B" -> "Plane CSC/CTM B"
+          "Plane CSC/CTM B" -> "Plane Gamma B"
+          "Plane Gamma B" -> "Pipe Blender"
+      }
+
+      subgraph cluster_static {
+          style=dashed
+
+          node [shape=box]
+          "Plane Degamma C" -> "Plane CSC/CTM C"
+          "Plane CSC/CTM C" -> "Plane Gamma C"
+          "Plane Gamma C" -> "Pipe Blender"
+      }
+
+      subgraph cluster_fb {
+          style=dashed
+          label="RAM"
+
+          node [shape=box width=1.7 height=0.2]
+
+          "FB 1" -> "Plane Degamma A"
+          "FB 2" -> "Plane Degamma B"
+          "FB 3" -> "Plane Degamma C"
+      }
+   }
+
+In real world usecases,
+
+1. Plane Degamma can be used to linearize a non linear gamma
+encoded framebuffer. This is needed to do any linear math like
+color space conversion. For ex, linearize frames encoded in SRGB
+or by HDR curve.
+
+2. Later Plane CTM block can convert the content to some different
+colorspace. For ex, SRGB to BT2020 etc.
+
+3. Plane Gamma block can be used later to re-apply the non-linear
+curve. This can also be used to apply Tone Mapping for HDR usecases.
+
+All the layers or framebuffers need to be converted to same color
+space and format before blending. The plane color hardware blocks
+can help with this. Once the Data is blended, similar color processing
+can be done on blended output using pipe color hardware blocks.
+
+DRM Properties have been created to define and expose all these
+hardware blocks to userspace. A userspace application (compositor
+or any color app) can use these interfaces and define policies to
+efficiently use the display hardware for such color operations.
+
+Pipe Color Management Properties
+---------------------------------
+
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :doc: overview
 
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :export:
 
+Plane Color Management Properties
+---------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: Plane Color Properties
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: export
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d0478ab..e716614 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_mode_config *config = &dev->mode_config;
+	bool replaced = false;
+	int ret;
 
 	if (property == config->prop_fb_id) {
 		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
@@ -910,6 +912,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->color_encoding = val;
 	} else if (property == plane->color_range_property) {
 		state->color_range = val;
+	} else if (property == plane->degamma_lut_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->degamma_lut,
+					val, -1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -980,6 +989,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		*val = state->color_encoding;
 	} else if (property == plane->color_range_property) {
 		*val = state->color_range;
+	} else if (property == plane->degamma_lut_property) {
+		*val = (state->degamma_lut) ?
+			state->degamma_lut->base.id : 0;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
@@ -1120,6 +1132,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
 		   drm_get_color_encoding_name(state->color_encoding));
 	drm_printf(p, "\tcolor-range=%s\n",
 		   drm_get_color_range_name(state->color_range));
+	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
 
 	if (plane->funcs->atomic_print_state)
 		plane->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2c23a48..203137e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3614,6 +3614,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	state->fence = NULL;
 	state->commit = NULL;
+
+	if (state->degamma_lut)
+		drm_property_blob_get(state->degamma_lut);
+	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
 
@@ -3658,6 +3662,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	if (state->commit)
 		drm_crtc_commit_put(state->commit);
+
+	drm_property_blob_put(state->degamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 1bdcc1a..d8a9e8b 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -29,11 +29,11 @@
 /**
  * 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
+ * Pipe 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().
  *
- * "DEGAMMA_LUT”:
+ * "DEGAMMA_LUT
  *	Blob property to set the degamma lookup table (LUT) mapping pixel data
  *	from the framebuffer before it is given to the transformation matrix.
  *	The data is interpreted as an array of &struct drm_color_lut elements.
@@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_color_properties);
+
+/**
+ * DOC: Plane Color Properties
+ *
+ * Plane Color management or color space adjustments is supported
+ * through a set of 5 properties on the &drm_plane object.
+ *
+ * degamma_lut_property:
+ *     Blob property which allows a userspace to provide LUT values
+ *     to apply degamma curve using the h/w plane degamma processing
+ *     engine, thereby making the content as linear for further color
+ *     processing.
+ *
+ * degamma_lut_size_property:
+ *     Range Property to indicate size of the plane degamma LUT.
+ */
+int drm_plane_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_size_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 16f5b666..7f0d961 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -182,6 +182,14 @@ struct drm_plane_state {
 	 */
 	bool visible;
 
+	/* @degamma_lut:
+	 *
+	 * Lookup table for converting framebuffer pixel data before apply the
+	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
+	 */
+	struct drm_property_blob *degamma_lut;
+
 	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
@@ -192,6 +200,8 @@ struct drm_plane_state {
 
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
+
+	u8 color_mgmt_changed : 1;
 };
 
 static inline struct drm_rect
@@ -692,6 +702,18 @@ struct drm_plane {
 	 * See drm_plane_create_color_properties().
 	 */
 	struct drm_property *color_range_property;
+
+	/**
+	 * @degamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to linear gamma.
+	 */
+	struct drm_property *degamma_lut_property;
+
+	/**
+	 * @degamma_lut_size_property: Optional Plane property for the
+	 * size of the degamma LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *degamma_lut_size_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -741,6 +763,8 @@ static inline u32 drm_plane_mask(const struct drm_plane *plane)
 int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 				       struct drm_property *property,
 				       uint64_t value);
+int drm_plane_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane);
 
 /**
  * drm_plane_find - find a &drm_plane
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 3/8] drm: Add Plane CTM property

[Uma Shankar]

Add a blob property for plane CSC usage.

v2: Rebase

v3: Fixed Sean, Paul's review comments. Moved the property from
mode_config to drm_plane. Created a helper function to instantiate
these properties and removed from drm_mode_create_standard_properties
Added property documentation as suggested by Daniel, Vetter.

v4: Rebase

v5: Moved property creation to drm_color_mgmt.c file to have all
color operations consolidated at one place. No logical change.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 Documentation/gpu/drm-kms.rst       |  3 +++
 drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
 drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
 drivers/gpu/drm/drm_color_mgmt.c    | 12 ++++++++++++
 include/drm/drm_plane.h             | 15 +++++++++++++++
 5 files changed, 44 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 253d546..d0391a9 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -647,6 +647,9 @@ Plane Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :doc: export
 
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: ctm_property
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e716614..fd1d0d1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -919,6 +919,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == plane->ctm_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->ctm,
+					val,
+					sizeof(struct drm_color_ctm), -1,
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -992,6 +1000,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 	} else if (property == plane->degamma_lut_property) {
 		*val = (state->degamma_lut) ?
 			state->degamma_lut->base.id : 0;
+	} else if (property == plane->ctm_property) {
+		*val = (state->ctm) ? state->ctm->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 203137e..75428425 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3617,6 +3617,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	if (state->degamma_lut)
 		drm_property_blob_get(state->degamma_lut);
+	if (state->ctm)
+		drm_property_blob_get(state->ctm);
+
 	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3664,6 +3667,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 		drm_crtc_commit_put(state->commit);
 
 	drm_property_blob_put(state->degamma_lut);
+	drm_property_blob_put(state->ctm);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index d8a9e8b..be418e2 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -506,6 +506,11 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
  *
  * degamma_lut_size_property:
  *     Range Property to indicate size of the plane degamma LUT.
+ *
+ * ctm_property:
+ *	Blob property which allows a userspace to provide CTM coefficients
+ *	to do color space conversion or any other enhancement by doing a
+ *	matrix multiplication using the h/w CTM processing engine
  */
 int drm_plane_color_create_prop(struct drm_device *dev,
 				struct drm_plane *plane)
@@ -526,6 +531,13 @@ int drm_plane_color_create_prop(struct drm_device *dev,
 		return -ENOMEM;
 	plane->degamma_lut_size_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->ctm_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 7f0d961..335632d 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -191,6 +191,14 @@ struct drm_plane_state {
 	struct drm_property_blob *degamma_lut;
 
 	/**
+	 * @ctm:
+	 *
+	 * Color transformation matrix. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is a &struct drm_color_ctm.
+	 */
+	struct drm_property_blob *ctm;
+
+	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
 	 *
@@ -714,6 +722,13 @@ struct drm_plane {
 	 * size of the degamma LUT as supported by the driver (read-only).
 	 */
 	struct drm_property *degamma_lut_size_property;
+
+	/**
+	 * @plane_ctm_property: Optional Plane property to set the
+	 * matrix used to convert colors after the lookup in the
+	 * degamma LUT.
+	 */
+	struct drm_property *ctm_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 4/8] drm: Add Plane Gamma properties

[Uma Shankar]

Add plane gamma as blob property and size as a
range property.

v2: Rebase

v3: Fixed Sean, Paul's review comments. Moved the property from
mode_config to drm_plane. Created a helper function to instantiate
these properties and removed from drm_mode_create_standard_properties
Added property documentation as suggested by Daniel, Vetter.

v4: Rebase

v5: Moved property creation to drm_color_mgmt.c file to have all
color operations consolidated at one place. No logical change.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 Documentation/gpu/drm-kms.rst       |  6 ++++++
 drivers/gpu/drm/drm_atomic.c        |  9 +++++++++
 drivers/gpu/drm/drm_atomic_helper.c |  3 +++
 drivers/gpu/drm/drm_color_mgmt.c    | 23 +++++++++++++++++++++++
 include/drm/drm_plane.h             | 22 ++++++++++++++++++++++
 5 files changed, 63 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index d0391a9..723fc57 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -650,6 +650,12 @@ Plane Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :doc: ctm_property
 
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: gamma_lut_property
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: gamma_lut_size_property
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index fd1d0d1..0832bd3 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -927,6 +927,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == plane->gamma_lut_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->gamma_lut,
+					val, -1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -1002,6 +1009,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 			state->degamma_lut->base.id : 0;
 	} else if (property == plane->ctm_property) {
 		*val = (state->ctm) ? state->ctm->base.id : 0;
+	} else if (property == plane->gamma_lut_property) {
+		*val = (state->gamma_lut) ? state->gamma_lut->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 75428425..27a2d0b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3619,6 +3619,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 		drm_property_blob_get(state->degamma_lut);
 	if (state->ctm)
 		drm_property_blob_get(state->ctm);
+	if (state->gamma_lut)
+		drm_property_blob_put(state->gamma_lut);
 
 	state->color_mgmt_changed = false;
 }
@@ -3668,6 +3670,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	drm_property_blob_put(state->degamma_lut);
 	drm_property_blob_put(state->ctm);
+	drm_property_blob_put(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index be418e2..5155add 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -511,6 +511,15 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
  *	Blob property which allows a userspace to provide CTM coefficients
  *	to do color space conversion or any other enhancement by doing a
  *	matrix multiplication using the h/w CTM processing engine
+ *
+ * gamma_lut_property:
+ *	Blob property which allows a userspace to provide LUT values
+ *	to apply gamma/tone-mapping curve using the h/w plane gamma
+ *	processing engine, thereby making the content as non-linear
+ *	or to perform any tone mapping operation for HDR usecases.
+ *
+ * gamma_lut_size_property:
+ *	Range Property to indicate size of the plane gamma LUT.
  */
 int drm_plane_color_create_prop(struct drm_device *dev,
 				struct drm_plane *plane)
@@ -538,6 +547,20 @@ int drm_plane_color_create_prop(struct drm_device *dev,
 		return -ENOMEM;
 	plane->ctm_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"PLANE_GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	plane->gamma_lut_size_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 335632d..5eeb9a8 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -199,6 +199,15 @@ struct drm_plane_state {
 	struct drm_property_blob *ctm;
 
 	/**
+	 * @gamma_lut:
+	 *
+	 * Lookup table for converting pixel data after the color conversion
+	 * matrix @ctm.  See drm_plane_enable_color_mgmt(). The blob (if not
+	 * NULL) is an array of &struct drm_color_lut_ext.
+	 */
+	struct drm_property_blob *gamma_lut;
+
+	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
 	 *
@@ -729,6 +738,19 @@ struct drm_plane {
 	 * degamma LUT.
 	 */
 	struct drm_property *ctm_property;
+
+	/**
+	 * @plane_gamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the colors, after the CTM matrix, to the common
+	 * gamma space chosen for blending.
+	 */
+	struct drm_property *gamma_lut_property;
+
+	/**
+	 * @plane_gamma_lut_size_property: Optional Plane property for the size
+	 * of the gamma LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *gamma_lut_size_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 5/8] drm: Define helper function for plane color enabling

[Uma Shankar]

Define helper function to enable Plane color features
to attach plane color properties to plane structure.

v2: Rebase

v3: Modiefied the function to use updated property names.

v4: Rebase

v5: Moved helper function to drm_color_mgmt.c file to have all
color operations consolidated at one place. No logical change.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 42 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  5 +++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 5155add..82e0bd1 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -493,6 +493,48 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 EXPORT_SYMBOL(drm_plane_create_color_properties);
 
 /**
+ * drm_plane_enable_color_mgmt - enable color management properties
+ * @plane: DRM Plane
+ * @plane_degamma_lut_size: the size of the degamma lut (before CSC)
+ * @plane_has_ctm: whether to attach ctm_property for CSC matrix
+ * @plane_gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction
+ * properties on a plane. This includes 3 degamma, csc and gamma
+ * properties that userspace can set and 2 size properties to inform
+ * the userspace of the lut sizes. Each of the properties are
+ * optional. The gamma and degamma properties are only attached if
+ * their size is not 0 and ctm_property is only attached if has_ctm is
+ * true.
+ */
+void drm_plane_enable_color_mgmt(struct drm_plane *plane,
+				uint plane_degamma_lut_size,
+				bool plane_has_ctm,
+				uint plane_gamma_lut_size)
+{
+	if (plane_degamma_lut_size) {
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_property, 0);
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_size_property,
+				plane_degamma_lut_size);
+	}
+
+	if (plane_has_ctm)
+		drm_object_attach_property(&plane->base,
+				plane->ctm_property, 0);
+
+	if (plane_gamma_lut_size) {
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_property, 0);
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_size_property,
+				plane_gamma_lut_size);
+	}
+}
+EXPORT_SYMBOL(drm_plane_enable_color_mgmt);
+
+/**
  * DOC: Plane Color Properties
  *
  * Plane Color management or color space adjustments is supported
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 78b5a37..ce5969a 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -69,4 +69,9 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 				      u32 supported_ranges,
 				      enum drm_color_encoding default_encoding,
 				      enum drm_color_range default_range);
+
+void drm_plane_enable_color_mgmt(struct drm_plane *plane,
+				 uint plane_degamma_lut_size,
+				 bool plane_has_ctm,
+				 uint plane_gamma_lut_size);
 #endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 6/8] drm/i915: Enable plane color features

[Uma Shankar]

Enable and initialize plane color features.

v2: Rebase and some cleanup

v3: Updated intel_plane_color_init to call
drm_plane_color_create_prop function, which will
in turn create plane color properties.

v4: Rebase

v5: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  5 +++++
 drivers/gpu/drm/i915/intel_color.c       | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_device_info.h |  5 +++++
 drivers/gpu/drm/i915/intel_drv.h         |  9 +++++++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 767615e..8234741 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -455,6 +455,11 @@ struct drm_i915_display_funcs {
 
 	void (*load_csc_matrix)(struct drm_crtc_state *crtc_state);
 	void (*load_luts)(struct drm_crtc_state *crtc_state);
+	/* Add Plane Color callbacks */
+	void (*load_plane_csc_matrix)(const struct drm_plane_state
+				      *plane_state);
+	void (*load_plane_luts)(const struct drm_plane_state
+				*plane_state);
 };
 
 #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index c6a7bea..fb8402f 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -642,6 +642,20 @@ int intel_color_check(struct drm_crtc *crtc,
 	return -EINVAL;
 }
 
+void intel_plane_color_init(struct drm_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->dev);
+
+	drm_plane_color_create_prop(plane->dev, plane);
+
+	/* Enable color management support when we have degamma & gamma LUTs. */
+	if (INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size != 0 &&
+	    INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size != 0)
+		drm_plane_enable_color_mgmt(plane,
+		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size,
+		true, INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size);
+}
+
 void intel_color_init(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 6eecd64..71132ad 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -182,6 +182,11 @@ struct intel_device_info {
 		u16 degamma_lut_size;
 		u16 gamma_lut_size;
 	} color;
+
+	struct plane_color_luts {
+		u16 plane_degamma_lut_size;
+		u16 plane_gamma_lut_size;
+	} plane_color;
 };
 
 struct intel_driver_caps {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f573121..5d442e5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -536,6 +536,14 @@ struct intel_plane_state {
 	 */
 	int scaler_id;
 
+	/*
+	 * Use reduced/limited/broadcast rbg range, compressing from the full
+	 * range fed into the crtcs.
+	 */
+	bool limited_color_range;
+	/* Gamma mode programmed on the plane */
+	uint32_t gamma_mode;
+
 	struct drm_intel_sprite_colorkey ckey;
 };
 
@@ -2187,6 +2195,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
 void intel_color_set_csc(struct drm_crtc_state *crtc_state);
 void intel_color_load_luts(struct drm_crtc_state *crtc_state);
+void intel_plane_color_init(struct drm_plane *plane);
 
 /* intel_lspcon.c */
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC v5 7/8] drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms

[Uma Shankar]

Implement Plane Gamma feature for BDW and Gen9 platforms.

v2: Used newly added drm_color_lut_ext structure for enhanced
precision for Gamma LUT entries.

v3: Rebase

v4: Used extended function for LUT extraction (pointed by
Alexandru).

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c      |  5 +++-
 drivers/gpu/drm/i915/i915_reg.h      | 25 ++++++++++++++++
 drivers/gpu/drm/i915/intel_color.c   | 58 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  4 +++
 drivers/gpu/drm/i915/intel_sprite.c  |  4 +++
 5 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index d6f7b9f..74e2b1d 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -54,7 +54,10 @@
 	.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
 
 #define BDW_COLORS \
-	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }, \
+	.plane_color = { .plane_degamma_lut_size = 0, \
+			 .plane_gamma_lut_size = 16 }
+
 #define CHV_COLORS \
 	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
 #define GLK_COLORS \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 09bc8e7..e8c06e8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -172,6 +172,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
 #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
 
+/* Plane Gamma Registers */
+#define _MMIO_PLANE_GAMC(plane, i, a, b)  _MMIO(_PIPE(plane, a, b) + (i) * 4)
+#define _MMIO_PLANE_GAMC16(plane, i, a, b)  _MMIO(_PIPE(plane, a, b) + (i) * 4)
+
 #define __MASKED_FIELD(mask, value) ((mask) << 16 | (value))
 #define _MASKED_FIELD(mask, value) ({					   \
 	if (__builtin_constant_p(mask))					   \
@@ -9702,6 +9706,27 @@ enum skl_power_gate {
 #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
 #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
 
+/* Plane Gamma in Gen9+ */
+#define _PLANE_GAMC_1_A	0x701d0
+#define _PLANE_GAMC_1_B	0x711d0
+#define _PLANE_GAMC_2_A	0x702d0
+#define _PLANE_GAMC_2_B	0x712d0
+#define _PLANE_GAMC_1(pipe)	_PIPE(pipe, _PLANE_GAMC_1_A, _PLANE_GAMC_1_B)
+#define _PLANE_GAMC_2(pipe)	_PIPE(pipe, _PLANE_GAMC_2_A, _PLANE_GAMC_2_B)
+#define PLANE_GAMC(pipe, plane, i)	\
+	_MMIO_PLANE_GAMC(plane, i, _PLANE_GAMC_1(pipe), _PLANE_GAMC_2(pipe))
+
+#define _PLANE_GAMC16_1_A	0x70210
+#define _PLANE_GAMC16_1_B	0x71210
+#define _PLANE_GAMC16_2_A	0x70310
+#define _PLANE_GAMC16_2_B	0x71310
+#define _PLANE_GAMC16_1(pipe)	_PIPE(pipe, _PLANE_GAMC16_1_A, \
+				     _PLANE_GAMC16_1_B)
+#define _PLANE_GAMC16_2(pipe)	_PIPE(pipe, _PLANE_GAMC16_2_A, \
+				     _PLANE_GAMC16_2_B)
+#define PLANE_GAMC16(pipe, plane, i) _MMIO_PLANE_GAMC16(plane, i, \
+				_PLANE_GAMC16_1(pipe), _PLANE_GAMC16_2(pipe))
+
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
 #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index fb8402f..c56fa18 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -492,6 +492,59 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
+static void bdw_load_plane_gamma_lut(const struct drm_plane_state *state,
+				     u32 offset)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
+	enum pipe pipe = to_intel_plane(state->plane)->pipe;
+	enum plane_id plane = to_intel_plane(state->plane)->id;
+	uint32_t i, lut_size =
+			INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size;
+
+	if (state->gamma_lut) {
+		struct drm_color_lut_ext *lut =
+			(struct drm_color_lut_ext *) state->gamma_lut->data;
+
+		for (i = 0; i < lut_size; i++) {
+			uint32_t word =
+			(drm_color_lut_extract_ext(lut[i].red, 10) << 20) |
+			(drm_color_lut_extract_ext(lut[i].green, 10) << 10) |
+			drm_color_lut_extract_ext(lut[i].blue, 10);
+
+			I915_WRITE(PLANE_GAMC(pipe, plane, i), word);
+		}
+
+		/* Program the max register to clamp values > 1.0. */
+		i = lut_size - 1;
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 0),
+			   drm_color_lut_extract_ext(lut[i].red, 16));
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 1),
+			   drm_color_lut_extract_ext(lut[i].green, 16));
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 2),
+			   drm_color_lut_extract_ext(lut[i].blue, 16));
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
+
+			I915_WRITE(PLANE_GAMC(pipe, plane, i),
+				   (v << 20) | (v << 10) | v);
+		}
+
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 0), (1 << 16) - 1);
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 1), (1 << 16) - 1);
+		I915_WRITE(PLANE_GAMC16(pipe, plane, 2), (1 << 16) - 1);
+	}
+}
+
+/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
+static void broadwell_load_plane_luts(const struct drm_plane_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
+
+	bdw_load_plane_gamma_lut(state,
+		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size);
+}
+
 static void glk_load_degamma_lut(struct drm_crtc_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
@@ -648,6 +701,11 @@ void intel_plane_color_init(struct drm_plane *plane)
 
 	drm_plane_color_create_prop(plane->dev, plane);
 
+	if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
+		IS_BROXTON(dev_priv)) {
+		dev_priv->display.load_plane_luts = broadwell_load_plane_luts;
+	}
+
 	/* Enable color management support when we have degamma & gamma LUTs. */
 	if (INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size != 0 &&
 	    INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size != 0)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1bd14c6..7c95db3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13810,6 +13810,10 @@ bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
 						  DRM_COLOR_YCBCR_BT709,
 						  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
+	/* Add Plane Color properties */
+	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
+		intel_plane_color_init(&primary->base);
+
 	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
 	return primary;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9600ccf..a516b9f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1651,6 +1651,10 @@ struct intel_plane *
 					  DRM_COLOR_YCBCR_BT709,
 					  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
+	/* Add Plane Color properties */
+	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
+		intel_plane_color_init(&intel_plane->base);
+
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
 	return intel_plane;
-- 
1.9.1

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

[RFC v5 8/8] drm/i915: Load plane color luts from atomic flip

[Uma Shankar]

Load plane color luts as part of atomic plane updates.
This will be done only if the plane color luts are changed.

v4: Rebase

v5: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ++++
 drivers/gpu/drm/i915/intel_color.c        | 8 ++++++++
 drivers/gpu/drm/i915/intel_drv.h          | 1 +
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index fa7df5f..565c473 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -226,6 +226,10 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 		intel_atomic_get_new_plane_state(state, intel_plane);
 	struct drm_crtc *crtc = new_plane_state->base.crtc ?: old_state->crtc;
 
+	if (new_plane_state->base.color_mgmt_changed) {
+		intel_color_load_plane_luts(&new_plane_state->base);
+	}
+
 	if (new_plane_state->base.visible) {
 		const struct intel_crtc_state *new_crtc_state =
 			intel_atomic_get_new_crtc_state(state, to_intel_crtc(crtc));
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index c56fa18..988dc5c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -666,6 +666,14 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state)
 	dev_priv->display.load_luts(crtc_state);
 }
 
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state)
+{
+	struct drm_device *dev = plane_state->plane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	dev_priv->display.load_plane_luts(plane_state);
+}
+
 int intel_color_check(struct drm_crtc *crtc,
 		      struct drm_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5d442e5..76721e8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2196,6 +2196,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 void intel_color_set_csc(struct drm_crtc_state *crtc_state);
 void intel_color_load_luts(struct drm_crtc_state *crtc_state);
 void intel_plane_color_init(struct drm_plane *plane);
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state);
 
 /* intel_lspcon.c */
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Add Plane Color Properties (rev5)

[Patchwork]

== Series Details ==

Series: Add Plane Color Properties (rev5)
URL   : https://patchwork.freedesktop.org/series/30875/
State : failure

== Summary ==

Applying: drm: Add Enhanced Gamma LUT precision structure
Applying: drm: Add Plane Degamma properties
error: sha1 information is lacking or useless (drivers/gpu/drm/drm_color_mgmt.c).
error: could not build fake ancestor
Patch failed at 0002 drm: Add Plane Degamma properties
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v5 0/8] Add Plane Color Properties

[Alexandru-Cosmin Gheorghe]

On Sun, Sep 16, 2018 at 01:45:23PM +0530, Uma Shankar wrote:
> This is how a typical display color hardware pipeline looks like:
>  +-------------------------------------------+
>  |                RAM                        |
>  |  +------+    +---------+    +---------+   |
>  |  | FB 1 |    |  FB 2   |    | FB N    |   |
>  |  +------+    +---------+    +---------+   |
>  +-------------------------------------------+
>        |  Plane Color Hardware Block |
>  +--------------------------------------------+
>  | +---v-----+   +---v-------+   +---v------+ |
>  | | Plane A |   | Plane B   |   | Plane N  | |
>  | | DeGamma |   | Degamma   |   | Degamma  | |
>  | +---+-----+   +---+-------+   +---+------+ |
>  |     |             |               |        |
>  | +---v-----+   +---v-------+   +---v------+ |
>  | |Plane A  |   | Plane B   |   | Plane N  | |
>  | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
>  | +---+-----+   +----+------+   +----+-----+ |
>  |     |              |               |       |
>  | +---v-----+   +----v------+   +----v-----+ |
>  | | Plane A |   | Plane B   |   | Plane N  | |
>  | | Gamma   |   | Gamma     |   | Gamma    | |
>  | +---+-----+   +----+------+   +----+-----+ |
>  |     |              |               |       |
>  +--------------------------------------------+
> +------v--------------v---------------v-------|
> ||                                           ||
> ||           Pipe Blender                    ||
> +--------------------+------------------------+
> |                    |                        |
> |        +-----------v----------+             |
> |        |  Pipe DeGamma        |             |
> |        |                      |             |
> |        +-----------+----------+             |
> |                    |            Pipe Color  |
> |        +-----------v----------+ Hardware    |
> |        |  Pipe CSC/CTM        |             |
> |        |                      |             |
> |        +-----------+----------+             |
> |                    |                        |
> |        +-----------v----------+             |
> |        |  Pipe Gamma          |             |
> |        |                      |             |
> |        +-----------+----------+             |
> |                    |                        |
> +---------------------------------------------+
>                      |
>                      v
>                Pipe Output
> 
> This patch series adds properties for plane color features. It adds
> properties for degamma used to linearize data, CSC used for gamut
> conversion, and gamma used to again non-linearize data as per panel
> supported color space. These can be utilize by user space to convert
> planes from one format to another, one color space to another etc.
> 
> Usersapce can take smart blending decisions and utilize these hardware
> supported plane color features to get accurate color profile. The same
> can help in consistent color quality from source to panel taking
> advantage of advanced color features in hardware.
> 
> These patches just add the property interfaces and enable helper
> functions.
> 
> This series adds Intel Gen9 specific plane gamma feature. We can
> build up and add other platform/hardware specific implementation
> on top of this series
> 
> Note: This is just to get a design feedback whether these interfaces
> look ok. Based on community feedback on interfaces, we will implement
> IGT tests to validate plane color features. This is un-tested currently.
> 
> Userspace implementation using these properties have been done in drm
> hwcomposer by "Alexandru-Cosmin Gheorghe Alexandru-Cosmin.Gheorghe@arm.com"
> from ARM. A merge request has been opened by Alexandru for drm_hwcomposer,
> implementing the property changes for the same. Please review that as well:
> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25
>

Just heads-up I just pushed a v2 for that.
 
> v2: Dropped legacy gamma table for plane as suggested by Maarten. Added
> Gen9/BDW plane gamma feature and rebase on tot.
> 
> v3: Added a new drm_color_lut_ext structure to accommodate 32 bit precision
> entries, pointed to by Brian, Starkey for HDR usecases. Addressed Sean,Paul
> comments and moved plane color properties to drm_plane instead of
> mode_config. Added property documentation as suggested by Daniel, Vetter.
> Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place. Addressed Alexandru's review comments.
> 
> Uma Shankar (8):
>   drm: Add Enhanced Gamma LUT precision structure
>   drm: Add Plane Degamma properties
>   drm: Add Plane CTM property
>   drm: Add Plane Gamma properties
>   drm: Define helper function for plane color enabling
>   drm/i915: Enable plane color features
>   drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
>   drm/i915: Load plane color luts from atomic flip
> 
>  Documentation/gpu/drm-kms.rst             |  99 +++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c              |  32 +++++++
>  drivers/gpu/drm/drm_atomic_helper.c       |  13 +++
>  drivers/gpu/drm/drm_color_mgmt.c          | 140 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_drv.h           |   5 ++
>  drivers/gpu/drm/i915/i915_pci.c           |   5 +-
>  drivers/gpu/drm/i915/i915_reg.h           |  25 ++++++
>  drivers/gpu/drm/i915/intel_atomic_plane.c |   4 +
>  drivers/gpu/drm/i915/intel_color.c        |  80 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_device_info.h  |   5 ++
>  drivers/gpu/drm/i915/intel_display.c      |   4 +
>  drivers/gpu/drm/i915/intel_drv.h          |  10 +++
>  drivers/gpu/drm/i915/intel_sprite.c       |   4 +
>  include/drm/drm_color_mgmt.h              |   6 ++
>  include/drm/drm_plane.h                   |  61 +++++++++++++
>  include/uapi/drm/drm_mode.h               |  15 ++++
>  16 files changed, 504 insertions(+), 4 deletions(-)
> 
> -- 
> 1.9.1

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

Re: [RFC v5 2/8] drm: Add Plane Degamma properties

[Alexandru-Cosmin Gheorghe]

Hi,

On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
> Add Plane Degamma as a blob property and plane degamma size as
> a range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>  include/drm/drm_plane.h             | 24 ++++++++++
>  5 files changed, 174 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f8f5bf1..253d546 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -551,12 +551,102 @@ Plane Composition Properties
>  Color Management Properties
>  ---------------------------
>  
> +Below is how a typical hardware pipeline for color
> +will look like:
> +
> +.. kernel-render:: DOT
> +   :alt: Display Color Pipeline
> +   :caption: Display Color Pipeline Overview
> +
> +   digraph "KMS" {
> +      node [shape=box]
> +
> +      subgraph cluster_static {
> +          style=dashed
> +          label="Display Color Hardware Blocks"
> +
> +          node [bgcolor=grey style=filled]
> +          "Plane Degamma A" -> "Plane CSC/CTM A"
> +          "Plane CSC/CTM A" -> "Plane Gamma A"
> +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> +          "Plane Gamma A" -> "Pipe Blender"
> +	  "Pipe Blender" -> "Pipe DeGamma"
> +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> +          "Pipe CSC/CTM" -> "Pipe Gamma"
> +          "Pipe Gamma" -> "Pipe Output"
> +      }
> +
> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma B" -> "Plane CSC/CTM B"
> +          "Plane CSC/CTM B" -> "Plane Gamma B"
> +          "Plane Gamma B" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma C" -> "Plane CSC/CTM C"
> +          "Plane CSC/CTM C" -> "Plane Gamma C"
> +          "Plane Gamma C" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_fb {
> +          style=dashed
> +          label="RAM"
> +
> +          node [shape=box width=1.7 height=0.2]
> +
> +          "FB 1" -> "Plane Degamma A"
> +          "FB 2" -> "Plane Degamma B"
> +          "FB 3" -> "Plane Degamma C"
> +      }
> +   }
> +
> +In real world usecases,
> +
> +1. Plane Degamma can be used to linearize a non linear gamma
> +encoded framebuffer. This is needed to do any linear math like
> +color space conversion. For ex, linearize frames encoded in SRGB
> +or by HDR curve.
> +
> +2. Later Plane CTM block can convert the content to some different
> +colorspace. For ex, SRGB to BT2020 etc.
> +
> +3. Plane Gamma block can be used later to re-apply the non-linear
> +curve. This can also be used to apply Tone Mapping for HDR usecases.
> +
> +All the layers or framebuffers need to be converted to same color
> +space and format before blending. The plane color hardware blocks
> +can help with this. Once the Data is blended, similar color processing
> +can be done on blended output using pipe color hardware blocks.
> +
> +DRM Properties have been created to define and expose all these
> +hardware blocks to userspace. A userspace application (compositor
> +or any color app) can use these interfaces and define policies to
> +efficiently use the display hardware for such color operations.
> +
> +Pipe Color Management Properties
> +---------------------------------
> +
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :doc: overview
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :export:
>  
> +Plane Color Management Properties
> +---------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: Plane Color Properties
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: export
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d0478ab..e716614 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -910,6 +912,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->color_encoding = val;
>  	} else if (property == plane->color_range_property) {
>  		state->color_range = val;
> +	} else if (property == plane->degamma_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->degamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),

Sorry I missed this thing the first time, but this should be
sizeof drm_color_lut_ext.

Also I think we have the same problem as for crtc properties when
doing suspend/resume.
I think color_mgmt_changed should just be set in
drm_atomic_helper_check_planes
https://lists.freedesktop.org/archives/dri-devel/2018-September/189265.html

> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		*val = state->color_encoding;
>  	} else if (property == plane->color_range_property) {
>  		*val = state->color_range;
> +	} else if (property == plane->degamma_lut_property) {
> +		*val = (state->degamma_lut) ?
> +			state->degamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> @@ -1120,6 +1132,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  		   drm_get_color_encoding_name(state->color_encoding));
>  	drm_printf(p, "\tcolor-range=%s\n",
>  		   drm_get_color_range_name(state->color_range));
> +	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
>  
>  	if (plane->funcs->atomic_print_state)
>  		plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48..203137e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3614,6 +3614,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	state->fence = NULL;
>  	state->commit = NULL;
> +
> +	if (state->degamma_lut)
> +		drm_property_blob_get(state->degamma_lut);
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> @@ -3658,6 +3662,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	if (state->commit)
>  		drm_crtc_commit_put(state->commit);
> +
> +	drm_property_blob_put(state->degamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 1bdcc1a..d8a9e8b 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -29,11 +29,11 @@
>  /**
>   * 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
> + * Pipe 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().
>   *
> - * "DEGAMMA_LUT”:
> + * "DEGAMMA_LUT
>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>   *	from the framebuffer before it is given to the transformation matrix.
>   *	The data is interpreted as an array of &struct drm_color_lut elements.
> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_create_color_properties);
> +
> +/**
> + * DOC: Plane Color Properties
> + *
> + * Plane Color management or color space adjustments is supported
> + * through a set of 5 properties on the &drm_plane object.
> + *
> + * degamma_lut_property:
> + *     Blob property which allows a userspace to provide LUT values
> + *     to apply degamma curve using the h/w plane degamma processing
> + *     engine, thereby making the content as linear for further color
> + *     processing.
> + *
> + * degamma_lut_size_property:
> + *     Range Property to indicate size of the plane degamma LUT.
> + */
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_size_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 16f5b666..7f0d961 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -182,6 +182,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/* @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *degamma_lut;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -192,6 +200,8 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -692,6 +702,18 @@ struct drm_plane {
>  	 * See drm_plane_create_color_properties().
>  	 */
>  	struct drm_property *color_range_property;
> +
> +	/**
> +	 * @degamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_lut_property;
> +
> +	/**
> +	 * @degamma_lut_size_property: Optional Plane property for the
> +	 * size of the degamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *degamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -741,6 +763,8 @@ static inline u32 drm_plane_mask(const struct drm_plane *plane)
>  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  				       struct drm_property *property,
>  				       uint64_t value);
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> -- 
> 1.9.1

-- 
Cheers,
Alex G
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v5 0/8] Add Plane Color Properties

[Lankhorst, Maarten]

[-- Attachment #1.1: Type: text/plain, Size: 6645 bytes --]

Hey,

sön 2018-09-16 klockan 13:45 +0530 skrev Uma Shankar:
> This is how a typical display color hardware pipeline looks like:
>  +-------------------------------------------+
>  |                RAM                        |
>  |  +------+    +---------+    +---------+   |
>  |  | FB 1 |    |  FB 2   |    | FB N    |   |
>  |  +------+    +---------+    +---------+   |
>  +-------------------------------------------+
>        |  Plane Color Hardware Block |
>  +--------------------------------------------+

Should be some mention of color conversion (YUV -> RGB) through
drm_color_encoding and drm_color_range enums interacting here?

>  | +---v-----+   +---v-------+   +---v------+ |
>  | | Plane A |   | Plane B   |   | Plane N  | |
>  | | DeGamma |   | Degamma   |   | Degamma  | |
>  | +---+-----+   +---+-------+   +---+------+ |
>  |     |             |               |        |
>  | +---v-----+   +---v-------+   +---v------+ |
>  | |Plane A  |   | Plane B   |   | Plane N  | |
>  | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
>  | +---+-----+   +----+------+   +----+-----+ |
>  |     |              |               |       |
>  | +---v-----+   +----v------+   +----v-----+ |
>  | | Plane A |   | Plane B   |   | Plane N  | |
>  | | Gamma   |   | Gamma     |   | Gamma    | |
>  | +---+-----+   +----+------+   +----+-----+ |
>  |     |              |               |       |
>  +--------------------------------------------+
> +------v--------------v---------------v-------|
> > >                                           ||
> > >           Pipe Blender                    ||
> 
> +--------------------+------------------------+
> >                    |                        |
> >        +-----------v----------+             |
> >        |  Pipe DeGamma        |             |
> >        |                      |             |
> >        +-----------+----------+             |
> >                    |            Pipe Color  |
> >        +-----------v----------+ Hardware    |
> >        |  Pipe CSC/CTM        |             |
> >        |                      |             |
> >        +-----------+----------+             |
> >                    |                        |
> >        +-----------v----------+             |
> >        |  Pipe Gamma          |             |
> >        |                      |             |
> >        +-----------+----------+             |
> >                    |                        |
Likewise, should probably be some mention about setting colorspace on
the connector, so the output will knows what format is used?

Perhaps pull it to this series since one can't work really well without
the other?
> 
> +---------------------------------------------+
>                      |
>                      v
>                Pipe Output
> 
> This patch series adds properties for plane color features. It adds
> properties for degamma used to linearize data, CSC used for gamut
> conversion, and gamma used to again non-linearize data as per panel
> supported color space. These can be utilize by user space to convert
> planes from one format to another, one color space to another etc.
> 
> Usersapce can take smart blending decisions and utilize these
> hardware
> supported plane color features to get accurate color profile. The
> same
> can help in consistent color quality from source to panel taking
> advantage of advanced color features in hardware.
> 
> These patches just add the property interfaces and enable helper
> functions.
> 
> This series adds Intel Gen9 specific plane gamma feature. We can
> build up and add other platform/hardware specific implementation
> on top of this series
> 
> Note: This is just to get a design feedback whether these interfaces
> look ok. Based on community feedback on interfaces, we will implement
> IGT tests to validate plane color features. This is un-tested
> currently.
> 
> Userspace implementation using these properties have been done in drm
> hwcomposer by "Alexandru-Cosmin Gheorghe Alexandru-Cosmin.Gheorghe@ar
> m.com"
> from ARM. A merge request has been opened by Alexandru for
> drm_hwcomposer,
> implementing the property changes for the same. Please review that as
> well:
> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_re
> quests/25
> 
> v2: Dropped legacy gamma table for plane as suggested by Maarten.
> Added
> Gen9/BDW plane gamma feature and rebase on tot.
> 
> v3: Added a new drm_color_lut_ext structure to accommodate 32 bit
> precision
> entries, pointed to by Brian, Starkey for HDR usecases. Addressed
> Sean,Paul
> comments and moved plane color properties to drm_plane instead of
> mode_config. Added property documentation as suggested by Daniel,
> Vetter.
> Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place. Addressed Alexandru's review
> comments.
> 
> Uma Shankar (8):
>   drm: Add Enhanced Gamma LUT precision structure
>   drm: Add Plane Degamma properties
>   drm: Add Plane CTM property
>   drm: Add Plane Gamma properties
>   drm: Define helper function for plane color enabling
>   drm/i915: Enable plane color features
>   drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
>   drm/i915: Load plane color luts from atomic flip
> 
>  Documentation/gpu/drm-kms.rst             |  99
> +++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c              |  32 +++++++
>  drivers/gpu/drm/drm_atomic_helper.c       |  13 +++
>  drivers/gpu/drm/drm_color_mgmt.c          | 140
> +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_drv.h           |   5 ++
>  drivers/gpu/drm/i915/i915_pci.c           |   5 +-
>  drivers/gpu/drm/i915/i915_reg.h           |  25 ++++++
>  drivers/gpu/drm/i915/intel_atomic_plane.c |   4 +
>  drivers/gpu/drm/i915/intel_color.c        |  80 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_device_info.h  |   5 ++
>  drivers/gpu/drm/i915/intel_display.c      |   4 +
>  drivers/gpu/drm/i915/intel_drv.h          |  10 +++
>  drivers/gpu/drm/i915/intel_sprite.c       |   4 +
>  include/drm/drm_color_mgmt.h              |   6 ++
>  include/drm/drm_plane.h                   |  61 +++++++++++++
>  include/uapi/drm/drm_mode.h               |  15 ++++
>  16 files changed, 504 insertions(+), 4 deletions(-)
> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3282 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v5 0/8] Add Plane Color Properties

[Shankar, Uma]

>-----Original Message-----
>From: Lankhorst, Maarten
>Sent: Wednesday, September 26, 2018 1:05 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; alexandru-
>cosmin.gheorghe@arm.com; emil.l.velikov@gmail.com;
>seanpaul@chromium.org; harry.wentland@amd.com; brian.starkey@arm.com;
>dcastagna@chromium.org; Sharma, Shashank <shashank.sharma@intel.com>
>Subject: Re: [RFC v5 0/8] Add Plane Color Properties
>
>Hey,
>
>sön 2018-09-16 klockan 13:45 +0530 skrev Uma Shankar:
>> This is how a typical display color hardware pipeline looks like:
>>  +-------------------------------------------+
>>  |                RAM                        |
>>  |  +------+    +---------+    +---------+   |
>>  |  | FB 1 |    |  FB 2   |    | FB N    |   |
>>  |  +------+    +---------+    +---------+   |
>>  +-------------------------------------------+
>>        |  Plane Color Hardware Block |
>>  +--------------------------------------------+
>
>Should be some mention of color conversion (YUV -> RGB) through
>drm_color_encoding and drm_color_range enums interacting here?

Thanks Maarten for taking it up for review.

Sure, I can add those details as well here.

>>  | +---v-----+   +---v-------+   +---v------+ |
>>  | | Plane A |   | Plane B   |   | Plane N  | |
>>  | | DeGamma |   | Degamma   |   | Degamma  | |
>>  | +---+-----+   +---+-------+   +---+------+ |
>>  |     |             |               |        |
>>  | +---v-----+   +---v-------+   +---v------+ |
>>  | |Plane A  |   | Plane B   |   | Plane N  | |
>>  | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
>>  | +---+-----+   +----+------+   +----+-----+ |
>>  |     |              |               |       |
>>  | +---v-----+   +----v------+   +----v-----+ |
>>  | | Plane A |   | Plane B   |   | Plane N  | |
>>  | | Gamma   |   | Gamma     |   | Gamma    | |
>>  | +---+-----+   +----+------+   +----+-----+ |
>>  |     |              |               |       |
>>  +--------------------------------------------+
>> +------v--------------v---------------v-------|
>> > >                                           ||
>> > >           Pipe Blender                    ||
>>
>> +--------------------+------------------------+
>> >                    |                        |
>> >        +-----------v----------+             |
>> >        |  Pipe DeGamma        |             |
>> >        |                      |             |
>> >        +-----------+----------+             |
>> >                    |            Pipe Color  |
>> >        +-----------v----------+ Hardware    |
>> >        |  Pipe CSC/CTM        |             |
>> >        |                      |             |
>> >        +-----------+----------+             |
>> >                    |                        |
>> >        +-----------v----------+             |
>> >        |  Pipe Gamma          |             |
>> >        |                      |             |
>> >        +-----------+----------+             |
>> >                    |                        |
>Likewise, should probably be some mention about setting colorspace on
>the connector, so the output will knows what format is used?
>
>Perhaps pull it to this series since one can't work really well without
>the other?
>>

I put it here for painting the complete color hardware pipeline. Since this was
just for plane color features, kept the colorspace series separate which was mainly
for pipe/connector . So even now if user wants to blend various layers using display
plane hardware, it can do that.

I can combine them together if you recommend. But I feel though they both are related
but they can be enabled/used independently. Will do as you suggest.

Regards,
Uma Shankar

>> +---------------------------------------------+
>>                      |
>>                      v
>>                Pipe Output
>>
>> This patch series adds properties for plane color features. It adds
>> properties for degamma used to linearize data, CSC used for gamut
>> conversion, and gamma used to again non-linearize data as per panel
>> supported color space. These can be utilize by user space to convert
>> planes from one format to another, one color space to another etc.
>>
>> Usersapce can take smart blending decisions and utilize these
>> hardware
>> supported plane color features to get accurate color profile. The
>> same
>> can help in consistent color quality from source to panel taking
>> advantage of advanced color features in hardware.
>>
>> These patches just add the property interfaces and enable helper
>> functions.
>>
>> This series adds Intel Gen9 specific plane gamma feature. We can
>> build up and add other platform/hardware specific implementation
>> on top of this series
>>
>> Note: This is just to get a design feedback whether these interfaces
>> look ok. Based on community feedback on interfaces, we will implement
>> IGT tests to validate plane color features. This is un-tested
>> currently.
>>
>> Userspace implementation using these properties have been done in drm
>> hwcomposer by "Alexandru-Cosmin Gheorghe Alexandru-Cosmin.Gheorghe@ar
>> m.com"
>> from ARM. A merge request has been opened by Alexandru for
>> drm_hwcomposer,
>> implementing the property changes for the same. Please review that as
>> well:
>> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_re
>> quests/25
>>
>> v2: Dropped legacy gamma table for plane as suggested by Maarten.
>> Added
>> Gen9/BDW plane gamma feature and rebase on tot.
>>
>> v3: Added a new drm_color_lut_ext structure to accommodate 32 bit
>> precision
>> entries, pointed to by Brian, Starkey for HDR usecases. Addressed
>> Sean,Paul
>> comments and moved plane color properties to drm_plane instead of
>> mode_config. Added property documentation as suggested by Daniel,
>> Vetter.
>> Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov.
>>
>> v4: Rebase
>>
>> v5: Added "Display Color Hardware Pipeline" flow to kernel
>> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
>> Moved the property creation to drm_color_mgmt.c file to consolidate
>> all color operations at one place. Addressed Alexandru's review
>> comments.
>>
>> Uma Shankar (8):
>>   drm: Add Enhanced Gamma LUT precision structure
>>   drm: Add Plane Degamma properties
>>   drm: Add Plane CTM property
>>   drm: Add Plane Gamma properties
>>   drm: Define helper function for plane color enabling
>>   drm/i915: Enable plane color features
>>   drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
>>   drm/i915: Load plane color luts from atomic flip
>>
>>  Documentation/gpu/drm-kms.rst             |  99
>> +++++++++++++++++++++
>>  drivers/gpu/drm/drm_atomic.c              |  32 +++++++
>>  drivers/gpu/drm/drm_atomic_helper.c       |  13 +++
>>  drivers/gpu/drm/drm_color_mgmt.c          | 140
>> +++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/i915/i915_drv.h           |   5 ++
>>  drivers/gpu/drm/i915/i915_pci.c           |   5 +-
>>  drivers/gpu/drm/i915/i915_reg.h           |  25 ++++++
>>  drivers/gpu/drm/i915/intel_atomic_plane.c |   4 +
>>  drivers/gpu/drm/i915/intel_color.c        |  80 +++++++++++++++++
>>  drivers/gpu/drm/i915/intel_device_info.h  |   5 ++
>>  drivers/gpu/drm/i915/intel_display.c      |   4 +
>>  drivers/gpu/drm/i915/intel_drv.h          |  10 +++
>>  drivers/gpu/drm/i915/intel_sprite.c       |   4 +
>>  include/drm/drm_color_mgmt.h              |   6 ++
>>  include/drm/drm_plane.h                   |  61 +++++++++++++
>>  include/uapi/drm/drm_mode.h               |  15 ++++
>>  16 files changed, 504 insertions(+), 4 deletions(-)
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC v5 1/8] drm: Add Enhanced Gamma LUT precision structure

[Matt Roper]

On Sun, Sep 16, 2018 at 01:45:24PM +0530, Uma Shankar wrote:
> Existing LUT precision structure is having only 16 bit
> precision. This is not enough for upcoming enhanced hardwares
> and advance usecases like HDR processing. Hence added a new
> structure with 32 bit precision values. Also added the code,
> for extracting the same from values passed from userspace.
> 
> v4: Rebase
> 
> v5: Relocated the helper function to drm_color_mgmt.c. Declared
> the same in a header file (Alexandru Gheorghe)
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  1 +
>  include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index b97e2de..1bdcc1a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/*
> + * Added to accommodate enhanced LUT precision.
> + * Max LUT precision is 32 bits.
> + */
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
> +{
> +	uint32_t val = user_input;
> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 32) {
> +		val += 1UL << (32 - bit_precision - 1);

Does val need to be a 64-bit type to prevent this from overflowing?


Matt

> +		val >>= 32 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 44f04233..78b5a37 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,7 @@
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8d67243..874407b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -629,6 +629,21 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/*
> + * Creating 32 bit palette entries for better data
> + * precision. This will be required for HDR and
> + * similar color processing usecases.
> + */
> +struct drm_color_lut_ext {
> +	/*
> +	 * Data is U0.32 fixed point format.
> +	 */
> +	__u32 red;
> +	__u32 green;
> +	__u32 blue;
> +	__u32 reserved;
> +};
> +
>  #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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [RFC v5 1/8] drm: Add Enhanced Gamma LUT precision structure

[Daniel Vetter]

On Sun, Sep 16, 2018 at 01:45:24PM +0530, Uma Shankar wrote:
> Existing LUT precision structure is having only 16 bit
> precision. This is not enough for upcoming enhanced hardwares
> and advance usecases like HDR processing. Hence added a new
> structure with 32 bit precision values. Also added the code,
> for extracting the same from values passed from userspace.
> 
> v4: Rebase
> 
> v5: Relocated the helper function to drm_color_mgmt.c. Declared
> the same in a header file (Alexandru Gheorghe)
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

Why is this not enough? If it's just about rounding, then I've already
made a comment internally how that should be handled, so that we don't
have rounding artifacts when upgrading a 16bit entry to e.g. 20 bits.

Creating new uapi is generally really hard, and needs a bunch more thought
than this here. If possible it's much better (or at least, much easier,
and much less effort) if this can be avoided.
-Daniel

> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  1 +
>  include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index b97e2de..1bdcc1a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/*
> + * Added to accommodate enhanced LUT precision.
> + * Max LUT precision is 32 bits.
> + */
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
> +{
> +	uint32_t val = user_input;
> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 32) {
> +		val += 1UL << (32 - bit_precision - 1);
> +		val >>= 32 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 44f04233..78b5a37 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,7 @@
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8d67243..874407b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -629,6 +629,21 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/*
> + * Creating 32 bit palette entries for better data
> + * precision. This will be required for HDR and
> + * similar color processing usecases.
> + */
> +struct drm_color_lut_ext {
> +	/*
> +	 * Data is U0.32 fixed point format.
> +	 */
> +	__u32 red;
> +	__u32 green;
> +	__u32 blue;
> +	__u32 reserved;
> +};
> +
>  #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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC v5 2/8] drm: Add Plane Degamma properties

[Matt Roper]

On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
> Add Plane Degamma as a blob property and plane degamma size as
> a range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>  include/drm/drm_plane.h             | 24 ++++++++++
>  5 files changed, 174 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f8f5bf1..253d546 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -551,12 +551,102 @@ Plane Composition Properties
>  Color Management Properties
>  ---------------------------
>  
> +Below is how a typical hardware pipeline for color
> +will look like:
> +
> +.. kernel-render:: DOT
> +   :alt: Display Color Pipeline
> +   :caption: Display Color Pipeline Overview
> +
> +   digraph "KMS" {
> +      node [shape=box]
> +
> +      subgraph cluster_static {
> +          style=dashed
> +          label="Display Color Hardware Blocks"
> +
> +          node [bgcolor=grey style=filled]
> +          "Plane Degamma A" -> "Plane CSC/CTM A"
> +          "Plane CSC/CTM A" -> "Plane Gamma A"
> +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> +          "Plane Gamma A" -> "Pipe Blender"
> +	  "Pipe Blender" -> "Pipe DeGamma"
> +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> +          "Pipe CSC/CTM" -> "Pipe Gamma"
> +          "Pipe Gamma" -> "Pipe Output"
> +      }
> +
> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma B" -> "Plane CSC/CTM B"
> +          "Plane CSC/CTM B" -> "Plane Gamma B"
> +          "Plane Gamma B" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma C" -> "Plane CSC/CTM C"
> +          "Plane CSC/CTM C" -> "Plane Gamma C"
> +          "Plane Gamma C" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_fb {
> +          style=dashed
> +          label="RAM"
> +
> +          node [shape=box width=1.7 height=0.2]
> +
> +          "FB 1" -> "Plane Degamma A"
> +          "FB 2" -> "Plane Degamma B"
> +          "FB 3" -> "Plane Degamma C"
> +      }
> +   }
> +
> +In real world usecases,
> +
> +1. Plane Degamma can be used to linearize a non linear gamma
> +encoded framebuffer. This is needed to do any linear math like
> +color space conversion. For ex, linearize frames encoded in SRGB
> +or by HDR curve.
> +
> +2. Later Plane CTM block can convert the content to some different
> +colorspace. For ex, SRGB to BT2020 etc.
> +
> +3. Plane Gamma block can be used later to re-apply the non-linear
> +curve. This can also be used to apply Tone Mapping for HDR usecases.
> +
> +All the layers or framebuffers need to be converted to same color
> +space and format before blending. The plane color hardware blocks
> +can help with this. Once the Data is blended, similar color processing
> +can be done on blended output using pipe color hardware blocks.
> +
> +DRM Properties have been created to define and expose all these
> +hardware blocks to userspace. A userspace application (compositor
> +or any color app) can use these interfaces and define policies to
> +efficiently use the display hardware for such color operations.
> +
> +Pipe Color Management Properties
> +---------------------------------
> +
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :doc: overview
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :export:
>  
> +Plane Color Management Properties
> +---------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: Plane Color Properties
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: export
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d0478ab..e716614 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -910,6 +912,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->color_encoding = val;
>  	} else if (property == plane->color_range_property) {
>  		state->color_range = val;
> +	} else if (property == plane->degamma_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->degamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		*val = state->color_encoding;
>  	} else if (property == plane->color_range_property) {
>  		*val = state->color_range;
> +	} else if (property == plane->degamma_lut_property) {
> +		*val = (state->degamma_lut) ?
> +			state->degamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> @@ -1120,6 +1132,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  		   drm_get_color_encoding_name(state->color_encoding));
>  	drm_printf(p, "\tcolor-range=%s\n",
>  		   drm_get_color_range_name(state->color_range));
> +	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
>  
>  	if (plane->funcs->atomic_print_state)
>  		plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48..203137e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3614,6 +3614,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	state->fence = NULL;
>  	state->commit = NULL;
> +
> +	if (state->degamma_lut)
> +		drm_property_blob_get(state->degamma_lut);
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> @@ -3658,6 +3662,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	if (state->commit)
>  		drm_crtc_commit_put(state->commit);
> +
> +	drm_property_blob_put(state->degamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 1bdcc1a..d8a9e8b 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -29,11 +29,11 @@
>  /**
>   * 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
> + * Pipe 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().
>   *
> - * "DEGAMMA_LUT”:
> + * "DEGAMMA_LUT

This change seems unrelated/unwanted.

>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>   *	from the framebuffer before it is given to the transformation matrix.
>   *	The data is interpreted as an array of &struct drm_color_lut elements.
> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_create_color_properties);
> +
> +/**
> + * DOC: Plane Color Properties
> + *
> + * Plane Color management or color space adjustments is supported
> + * through a set of 5 properties on the &drm_plane object.
> + *
> + * degamma_lut_property:
> + *     Blob property which allows a userspace to provide LUT values
> + *     to apply degamma curve using the h/w plane degamma processing
> + *     engine, thereby making the content as linear for further color
> + *     processing.
> + *
> + * degamma_lut_size_property:
> + *     Range Property to indicate size of the plane degamma LUT.

Elsewhere we're documenting the string name for the property, which is
part of the ABI (e.g., "DEGAMMA_LUT") but here you're documenting the
property variable name in the code.

I think there was also discussion at one point of having a more complex
LUT interface that deals with things like non-linear ranges and such.
If that's still planned, would it be layered on top of these properties,
or would it be a completely separate, replacement ABI down the road?

> + */
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;

If this fails, should we try to clean up the earlier property?  It looks
the i915 callsite for this function (in patch #6 of the series) doesn't
check the return value, so even if we return -ENOMEM, the driver will
continue with some properties created and some not.

Actually, aside from using names "PLANE_FOO" instead of "FOO," is there
a reason why we need to create new properties for planes?  I think we
could still attach the same properties we're already using for CRTC's
(dev->mode_config.degamma_lut_property and such) since afaik there isn't
anything that prevents us from attaching the same property to different
kinds of drm_object's.


Matt

> +	plane->degamma_lut_size_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 16f5b666..7f0d961 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -182,6 +182,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/* @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *degamma_lut;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -192,6 +200,8 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -692,6 +702,18 @@ struct drm_plane {
>  	 * See drm_plane_create_color_properties().
>  	 */
>  	struct drm_property *color_range_property;
> +
> +	/**
> +	 * @degamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_lut_property;
> +
> +	/**
> +	 * @degamma_lut_size_property: Optional Plane property for the
> +	 * size of the degamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *degamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -741,6 +763,8 @@ static inline u32 drm_plane_mask(const struct drm_plane *plane)
>  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  				       struct drm_property *property,
>  				       uint64_t value);
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v5 2/8] drm: Add Plane Degamma properties

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Wednesday, November 14, 2018 6:39 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>dcastagna@chromium.org; alexandru-cosmin.gheorghe@arm.com;
>seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>;
>harry.wentland@amd.com; Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [RFC v5 2/8] drm: Add Plane Degamma properties
>
>On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
>> Add Plane Degamma as a blob property and plane degamma size as a range
>> property.
>>
>> v2: Rebase
>>
>> v3: Fixed Sean, Paul's review comments. Moved the property from
>> mode_config to drm_plane. Created a helper function to instantiate
>> these properties and removed from drm_mode_create_standard_properties
>> Added property documentation as suggested by Daniel, Vetter.
>>
>> v4: Rebase
>>
>> v5: Added "Display Color Hardware Pipeline" flow to kernel
>> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
>> Moved the property creation to drm_color_mgmt.c file to consolidate
>> all color operations at one place.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
>> ---
>>  Documentation/gpu/drm-kms.rst       | 90
>+++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>>  include/drm/drm_plane.h             | 24 ++++++++++
>>  5 files changed, 174 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/gpu/drm-kms.rst
>> b/Documentation/gpu/drm-kms.rst index f8f5bf1..253d546 100644
>> --- a/Documentation/gpu/drm-kms.rst
>> +++ b/Documentation/gpu/drm-kms.rst
>> @@ -551,12 +551,102 @@ Plane Composition Properties  Color Management
>> Properties
>>  ---------------------------
>>
>> +Below is how a typical hardware pipeline for color will look like:
>> +
>> +.. kernel-render:: DOT
>> +   :alt: Display Color Pipeline
>> +   :caption: Display Color Pipeline Overview
>> +
>> +   digraph "KMS" {
>> +      node [shape=box]
>> +
>> +      subgraph cluster_static {
>> +          style=dashed
>> +          label="Display Color Hardware Blocks"
>> +
>> +          node [bgcolor=grey style=filled]
>> +          "Plane Degamma A" -> "Plane CSC/CTM A"
>> +          "Plane CSC/CTM A" -> "Plane Gamma A"
>> +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
>> +          "Plane Gamma A" -> "Pipe Blender"
>> +	  "Pipe Blender" -> "Pipe DeGamma"
>> +          "Pipe DeGamma" -> "Pipe CSC/CTM"
>> +          "Pipe CSC/CTM" -> "Pipe Gamma"
>> +          "Pipe Gamma" -> "Pipe Output"
>> +      }
>> +
>> +      subgraph cluster_static {
>> +          style=dashed
>> +
>> +          node [shape=box]
>> +          "Plane Degamma B" -> "Plane CSC/CTM B"
>> +          "Plane CSC/CTM B" -> "Plane Gamma B"
>> +          "Plane Gamma B" -> "Pipe Blender"
>> +      }
>> +
>> +      subgraph cluster_static {
>> +          style=dashed
>> +
>> +          node [shape=box]
>> +          "Plane Degamma C" -> "Plane CSC/CTM C"
>> +          "Plane CSC/CTM C" -> "Plane Gamma C"
>> +          "Plane Gamma C" -> "Pipe Blender"
>> +      }
>> +
>> +      subgraph cluster_fb {
>> +          style=dashed
>> +          label="RAM"
>> +
>> +          node [shape=box width=1.7 height=0.2]
>> +
>> +          "FB 1" -> "Plane Degamma A"
>> +          "FB 2" -> "Plane Degamma B"
>> +          "FB 3" -> "Plane Degamma C"
>> +      }
>> +   }
>> +
>> +In real world usecases,
>> +
>> +1. Plane Degamma can be used to linearize a non linear gamma encoded
>> +framebuffer. This is needed to do any linear math like color space
>> +conversion. For ex, linearize frames encoded in SRGB or by HDR curve.
>> +
>> +2. Later Plane CTM block can convert the content to some different
>> +colorspace. For ex, SRGB to BT2020 etc.
>> +
>> +3. Plane Gamma block can be used later to re-apply the non-linear
>> +curve. This can also be used to apply Tone Mapping for HDR usecases.
>> +
>> +All the layers or framebuffers need to be converted to same color
>> +space and format before blending. The plane color hardware blocks can
>> +help with this. Once the Data is blended, similar color processing
>> +can be done on blended output using pipe color hardware blocks.
>> +
>> +DRM Properties have been created to define and expose all these
>> +hardware blocks to userspace. A userspace application (compositor or
>> +any color app) can use these interfaces and define policies to
>> +efficiently use the display hardware for such color operations.
>> +
>> +Pipe Color Management Properties
>> +---------------------------------
>> +
>>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>>     :doc: overview
>>
>>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>>     :export:
>>
>> +Plane Color Management Properties
>> +---------------------------------
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>> +   :doc: Plane Color Properties
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>> +   :doc: export
>> +
>>  Tile Group Property
>>  -------------------
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c
>> b/drivers/gpu/drm/drm_atomic.c index d0478ab..e716614 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct
>> drm_plane *plane,  {
>>  	struct drm_device *dev = plane->dev;
>>  	struct drm_mode_config *config = &dev->mode_config;
>> +	bool replaced = false;
>> +	int ret;
>>
>>  	if (property == config->prop_fb_id) {
>>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev,
>NULL,
>> val); @@ -910,6 +912,13 @@ static int drm_atomic_plane_set_property(struct
>drm_plane *plane,
>>  		state->color_encoding = val;
>>  	} else if (property == plane->color_range_property) {
>>  		state->color_range = val;
>> +	} else if (property == plane->degamma_lut_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +					&state->degamma_lut,
>> +					val, -1, sizeof(struct drm_color_lut),
>> +					&replaced);
>> +		state->color_mgmt_changed |= replaced;
>> +		return ret;
>>  	} else if (plane->funcs->atomic_set_property) {
>>  		return plane->funcs->atomic_set_property(plane, state,
>>  				property, val);
>> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_property(struct
>drm_plane *plane,
>>  		*val = state->color_encoding;
>>  	} else if (property == plane->color_range_property) {
>>  		*val = state->color_range;
>> +	} else if (property == plane->degamma_lut_property) {
>> +		*val = (state->degamma_lut) ?
>> +			state->degamma_lut->base.id : 0;
>>  	} else if (plane->funcs->atomic_get_property) {
>>  		return plane->funcs->atomic_get_property(plane, state,
>property, val);
>>  	} else {
>> @@ -1120,6 +1132,7 @@ static void drm_atomic_plane_print_state(struct
>drm_printer *p,
>>  		   drm_get_color_encoding_name(state->color_encoding));
>>  	drm_printf(p, "\tcolor-range=%s\n",
>>  		   drm_get_color_range_name(state->color_range));
>> +	drm_printf(p, "\tcolor_mgmt_changed=%d\n",
>> +state->color_mgmt_changed);
>>
>>  	if (plane->funcs->atomic_print_state)
>>  		plane->funcs->atomic_print_state(p, state); diff --git
>> a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index 2c23a48..203137e 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -3614,6 +3614,10 @@ void
>> __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>>
>>  	state->fence = NULL;
>>  	state->commit = NULL;
>> +
>> +	if (state->degamma_lut)
>> +		drm_property_blob_get(state->degamma_lut);
>> +	state->color_mgmt_changed = false;
>>  }
>>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>>
>> @@ -3658,6 +3662,8 @@ void
>> __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>>
>>  	if (state->commit)
>>  		drm_crtc_commit_put(state->commit);
>> +
>> +	drm_property_blob_put(state->degamma_lut);
>>  }
>>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
>> b/drivers/gpu/drm/drm_color_mgmt.c
>> index 1bdcc1a..d8a9e8b 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -29,11 +29,11 @@
>>  /**
>>   * 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
>> + * Pipe 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().
>>   *
>> - * "DEGAMMA_LUT”:
>> + * "DEGAMMA_LUT
>
>This change seems unrelated/unwanted.

Updated Color Management to Pipe Color Management here. The last part,
DEGAMMA_LUT change can be dropped and kept as is. Will update this.

>>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>>   *	from the framebuffer before it is given to the transformation matrix.
>>   *	The data is interpreted as an array of &struct drm_color_lut elements.
>> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct
>drm_plane *plane,
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(drm_plane_create_color_properties);
>> +
>> +/**
>> + * DOC: Plane Color Properties
>> + *
>> + * Plane Color management or color space adjustments is supported
>> + * through a set of 5 properties on the &drm_plane object.
>> + *
>> + * degamma_lut_property:
>> + *     Blob property which allows a userspace to provide LUT values
>> + *     to apply degamma curve using the h/w plane degamma processing
>> + *     engine, thereby making the content as linear for further color
>> + *     processing.
>> + *
>> + * degamma_lut_size_property:
>> + *     Range Property to indicate size of the plane degamma LUT.
>
>Elsewhere we're documenting the string name for the property, which is part of
>the ABI (e.g., "DEGAMMA_LUT") but here you're documenting the property
>variable name in the code.

Ok, will change this to be consistent with pipe color property documentation. Thanks..

>I think there was also discussion at one point of having a more complex LUT
>interface that deals with things like non-linear ranges and such.
>If that's still planned, would it be layered on top of these properties, or would it
>be a completely separate, replacement ABI down the road?

The non-linear and multi-segment part is a bit tricky.  We can develop that on top
of this property by creating a segment lut property blob which just tells the number of
segments and size of each segment. The actual lut values can be passed through the
current property. Driver can then use data of both these properties and use it as per
respective hardware to program LUT.

If the blob property is not set while plane degamma is set, driver functions in the legacy way
and program luts in a linear fashion (basically as per legacy design).
This is just my thought on that one. Better ideas are welcome.

>> + */
>> +int drm_plane_color_create_prop(struct drm_device *dev,
>> +				struct drm_plane *plane)
>> +{
>> +	struct drm_property *prop;
>> +
>> +	prop = drm_property_create(dev,
>> +			DRM_MODE_PROP_BLOB,
>> +			"PLANE_DEGAMMA_LUT", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	plane->degamma_lut_property = prop;
>> +
>> +	prop = drm_property_create_range(dev,
>> +			DRM_MODE_PROP_IMMUTABLE,
>> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
>> +	if (!prop)
>> +		return -ENOMEM;
>
>If this fails, should we try to clean up the earlier property?  It looks the i915
>callsite for this function (in patch #6 of the series) doesn't check the return value,
>so even if we return -ENOMEM, the driver will continue with some properties
>created and some not.

Sure, I will fix this. Thanks..

>Actually, aside from using names "PLANE_FOO" instead of "FOO," is there a
>reason why we need to create new properties for planes?  I think we could still
>attach the same properties we're already using for CRTC's (dev-
>>mode_config.degamma_lut_property and such) since afaik there isn't anything
>that prevents us from attaching the same property to different kinds of
>drm_object's.

This point was discussed when we were planning this. Plane color is little different than
pipe color, and comes handy for making pre blending framebuffer conversion. Also, we
can extend it to handle multi segment and non-linear LUT's. Hence, we thought to keep it
separate from pipe color properties. 

Regards,
Uma Shankar
>
>Matt
>
>> +	plane->degamma_lut_size_property = prop;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_plane_color_create_prop);
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
>> 16f5b666..7f0d961 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -182,6 +182,14 @@ struct drm_plane_state {
>>  	 */
>>  	bool visible;
>>
>> +	/* @degamma_lut:
>> +	 *
>> +	 * Lookup table for converting framebuffer pixel data before apply the
>> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt().
>The
>> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
>> +	 */
>> +	struct drm_property_blob *degamma_lut;
>> +
>>  	/**
>>  	 * @commit: Tracks the pending commit to prevent use-after-free
>conditions,
>>  	 * and for async plane updates.
>> @@ -192,6 +200,8 @@ struct drm_plane_state {
>>
>>  	/** @state: backpointer to global drm_atomic_state */
>>  	struct drm_atomic_state *state;
>> +
>> +	u8 color_mgmt_changed : 1;
>>  };
>>
>>  static inline struct drm_rect
>> @@ -692,6 +702,18 @@ struct drm_plane {
>>  	 * See drm_plane_create_color_properties().
>>  	 */
>>  	struct drm_property *color_range_property;
>> +
>> +	/**
>> +	 * @degamma_lut_property: Optional Plane property to set the LUT
>> +	 * used to convert the framebuffer's colors to linear gamma.
>> +	 */
>> +	struct drm_property *degamma_lut_property;
>> +
>> +	/**
>> +	 * @degamma_lut_size_property: Optional Plane property for the
>> +	 * size of the degamma LUT as supported by the driver (read-only).
>> +	 */
>> +	struct drm_property *degamma_lut_size_property;
>>  };
>>
>>  #define obj_to_plane(x) container_of(x, struct drm_plane, base) @@
>> -741,6 +763,8 @@ static inline u32 drm_plane_mask(const struct
>> drm_plane *plane)  int drm_mode_plane_set_obj_prop(struct drm_plane
>*plane,
>>  				       struct drm_property *property,
>>  				       uint64_t value);
>> +int drm_plane_color_create_prop(struct drm_device *dev,
>> +				struct drm_plane *plane);
>>
>>  /**
>>   * drm_plane_find - find a &drm_plane
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Matt Roper
>Graphics Software Engineer
>IoTG Platform Enabling & Development
>Intel Corporation
>(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx