dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/21] Add Support for Plane Color Lut and CSC features
@ 2021-06-01 10:51 Uma Shankar
  2021-06-01 10:51 ` [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
                   ` (21 more replies)
  0 siblings, 22 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:51 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

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 and CSC used for gamut
conversion. It also includes Gamma support 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.

Userspace 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 add the property interfaces and enable helper functions.
This series adds Intel's XE_LPD hw specific plane gamma feature. We
can build up and add other platform/hardware specific implementation
on top of this series.

Credits: Special mention and credits to Ville Syrjala for coming up
with a design for this feature and inputs. This series is based on
his original design and idea.

Note: Userspace support for this new UAPI will be done on Chrome. We
will notify the list once we have that ready for review.

ToDo: State readout for this feature will be added next.

Uma Shankar (21):
  drm: Add Enhanced Gamma and color lut range attributes
  drm: Add Plane Degamma Mode property
  drm: Add Plane Degamma Lut property
  drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
  drm/i915/xelpd: Add register definitions for Plane Degamma
  drm/i915/xelpd: Enable plane color features
  drm/i915/xelpd: Add color capabilities of SDR planes
  drm/i915/xelpd: Program Plane Degamma Registers
  drm/i915/xelpd: Add plane color check to glk_plane_color_ctl
  drm/i915/xelpd: Initialize plane color features
  drm/i915/xelpd: Load plane color luts from atomic flip
  drm: Add Plane CTM property
  drm: Add helper to attach Plane ctm property
  drm/i915/xelpd: Define Plane CSC Registers
  drm/i915/xelpd: Enable Plane CSC
  drm: Add Plane Gamma Mode property
  drm: Add Plane Gamma Lut property
  drm/i915/xelpd: Define and Initialize Plane Gamma Lut range
  drm/i915/xelpd: Add register definitions for Plane Gamma
  drm/i915/xelpd: Program Plane Gamma Registers
  drm/i915/xelpd: Enable plane gamma

 Documentation/gpu/drm-kms.rst                 |  90 +++
 drivers/gpu/drm/drm_atomic.c                  |   1 +
 drivers/gpu/drm/drm_atomic_state_helper.c     |  12 +
 drivers/gpu/drm/drm_atomic_uapi.c             |  38 ++
 drivers/gpu/drm/drm_color_mgmt.c              | 177 +++++-
 .../gpu/drm/i915/display/intel_atomic_plane.c |   6 +
 .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
 drivers/gpu/drm/i915/display/intel_color.c    | 513 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_color.h    |   2 +
 .../drm/i915/display/skl_universal_plane.c    |  15 +-
 drivers/gpu/drm/i915/i915_drv.h               |   3 +
 drivers/gpu/drm/i915/i915_reg.h               | 176 +++++-
 include/drm/drm_mode_object.h                 |   2 +-
 include/drm/drm_plane.h                       |  81 +++
 include/uapi/drm/drm_mode.h                   |  58 ++
 15 files changed, 1170 insertions(+), 6 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
@ 2021-06-01 10:51 ` Uma Shankar
  2021-06-02  9:33   ` Pekka Paalanen
  2021-06-01 10:51 ` [PATCH 02/21] drm: Add Plane Degamma Mode property Uma Shankar
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:51 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

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.

This also defines a new structure to define color lut ranges,
along with related macro definitions and enums. This will help
describe multi segmented lut ranges in the hardware.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 9b6722d45f36..d0ce48d2e732 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -819,6 +819,64 @@ struct hdr_output_metadata {
 	};
 };
 
+/*
+ * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
+ * can be used for either purpose, but not simultaneously. To expose
+ * modes that support gamma and degamma simultaneously the gamma mode
+ * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
+ * ranges.
+ */
+/* LUT is for gamma (after CTM) */
+#define DRM_MODE_LUT_GAMMA BIT(0)
+/* LUT is for degamma (before CTM) */
+#define DRM_MODE_LUT_DEGAMMA BIT(1)
+/* linearly interpolate between the points */
+#define DRM_MODE_LUT_INTERPOLATE BIT(2)
+/*
+ * the last value of the previous range is the
+ * first value of the current range.
+ */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3)
+/* the curve must be non-decreasing */
+#define DRM_MODE_LUT_NON_DECREASING BIT(4)
+/* the curve is reflected across origin for negative inputs */
+#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
+/* the same curve (red) is used for blue and green channels as well */
+#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+
+struct drm_color_lut_range {
+	/* DRM_MODE_LUT_* */
+	__u32 flags;
+	/* number of points on the curve */
+	__u16 count;
+	/* input/output bits per component */
+	__u8 input_bpc, output_bpc;
+	/* input start/end values */
+	__s32 start, end;
+	/* output min/max values */
+	__s32 min, max;
+};
+
+enum lut_type {
+	LUT_TYPE_DEGAMMA = 0,
+	LUT_TYPE_GAMMA = 1,
+};
+
+/*
+ * Creating 64 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 U32.32 fixed point format.
+	 */
+	__u64 red;
+	__u64 green;
+	__u64 blue;
+	__u64 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 02/21] drm: Add Plane Degamma Mode property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
  2021-06-01 10:51 ` [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
@ 2021-06-01 10:51 ` Uma Shankar
  2021-06-04 18:24   ` Harry Wentland
  2021-06-01 10:52 ` [PATCH 03/21] drm: Add Plane Degamma Lut property Uma Shankar
                   ` (19 subsequent siblings)
  21 siblings, 1 reply; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:51 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add Plane Degamma Mode as an enum property. Create a helper
function for all plane color management features.

This is an enum property with values as blob_id's and exposes
the various gamma modes supported and the lut ranges. Getting
the blob id in userspace, user can get the mode supported and
also the range of gamma mode supported with number of lut
coefficients. It can then set one of the modes using this
enum property.

Lut values will be sent through separate GAMMA_LUT blob property.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c              |  1 +
 drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
 drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
 drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
 include/drm/drm_mode_object.h             |  2 +-
 include/drm/drm_plane.h                   | 23 ++++++
 7 files changed, 212 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 87e5023e3f55..752be545e7d7 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -514,9 +514,99 @@ Damage Tracking 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
 
+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 a8bbb021684b..8892d03602f7 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -708,6 +708,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_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..f26b03853711 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 	state->fence = NULL;
 	state->commit = NULL;
 	state->fb_damage_clips = NULL;
+
+	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
 
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..40fa05fa33dc 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -595,6 +595,8 @@ 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_mode_property) {
+		state->degamma_mode = val;
 	} else if (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -661,6 +663,8 @@ drm_atomic_plane_get_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_mode_property) {
+		*val = state->degamma_mode;
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index bb14f488c8f6..085ed0d0db00 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -34,8 +34,8 @@
 /**
  * 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”:
@@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 }
 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_mode_property:
+ *     Blob property which advertizes the possible degamma modes and
+ *     lut ranges supported by the platform. This  allows userspace
+ *     to query and get the plane degamma color caps and choose the
+ *     appropriate degamma mode and create lut values accordingly
+ *
+ */
+int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
+					   struct drm_plane *plane,
+					   int num_values)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
+				   "PLANE_DEGAMMA_MODE", num_values);
+	if (!prop)
+		return -ENOMEM;
+
+	plane->degamma_mode_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
+
+void drm_plane_attach_degamma_properties(struct drm_plane *plane)
+{
+	if (!plane->degamma_mode_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->degamma_mode_property, 0);
+}
+EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
+
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
+						 const char *name,
+						 const struct drm_color_lut_range *ranges,
+						 size_t length, enum lut_type type)
+{
+	struct drm_property_blob *blob;
+	struct drm_property *prop = NULL;
+	int num_ranges = length / sizeof(ranges[0]);
+	int i, ret, num_types_0;
+
+	if (type == LUT_TYPE_DEGAMMA)
+		prop = plane->degamma_mode_property;
+
+	if (!prop)
+		return -EINVAL;
+
+	if (length == 0 && name)
+		return drm_property_add_enum(prop, 0, name);
+
+	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
+		return -EINVAL;
+	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
+			       DRM_MODE_LUT_DEGAMMA));
+	if (num_types_0 == 0)
+		return -EINVAL;
+
+	for (i = 1; i < num_ranges; i++) {
+		int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
+					 DRM_MODE_LUT_DEGAMMA));
+
+		/* either all ranges have DEGAMMA|GAMMA or none have it */
+		if (num_types_0 != num_types)
+			return -EINVAL;
+	}
+
+	blob = drm_property_create_blob(plane->dev, length, ranges);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	ret = drm_property_add_enum(prop, blob->base.id, name);
+	if (ret) {
+		drm_property_blob_put(blob);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
+
 /**
  * drm_color_lut_check - check validity of lookup table
  * @lut: property blob containing LUT to check
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c34a3e8030e1..d4128c7daa08 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -60,7 +60,7 @@ struct drm_mode_object {
 	void (*free_cb)(struct kref *kref);
 };
 
-#define DRM_OBJECT_MAX_PROPERTY 24
+#define DRM_OBJECT_MAX_PROPERTY 26
 /**
  * struct drm_object_properties - property tracking for &drm_mode_object
  */
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 1294610e84f4..e476a5939f8e 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -236,6 +236,15 @@ struct drm_plane_state {
 
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
+
+	/**
+	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
+	 * wrt to various gamma modes and the respective lut ranges. This also
+	 * helps user select a degamma mode amongst the supported ones.
+	 */
+	u32 degamma_mode;
+
+	u8 color_mgmt_changed : 1;
 };
 
 static inline struct drm_rect
@@ -747,6 +756,12 @@ struct drm_plane {
 	 * scaling.
 	 */
 	struct drm_property *scaling_filter_property;
+
+	/**
+	 * @degamma_mode_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to linear gamma.
+	 */
+	struct drm_property *degamma_mode_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -838,6 +853,14 @@ void drm_plane_force_disable(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_create_color_mgmt_properties(struct drm_device *dev,
+					   struct drm_plane *plane,
+					   int num_values);
+void drm_plane_attach_degamma_properties(struct drm_plane *plane);
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
+						 const char *name,
+						 const struct drm_color_lut_range *ranges,
+						 size_t length, enum lut_type type);
 
 /**
  * drm_plane_find - find a &drm_plane
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 03/21] drm: Add Plane Degamma Lut property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
  2021-06-01 10:51 ` [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
  2021-06-01 10:51 ` [PATCH 02/21] drm: Add Plane Degamma Mode property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes Uma Shankar
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add Plane Degamma Lut as a blob property. User will calculate
the lut values, create the blob and send it to driver using
this property. Lut calculation will be based on the gamma mode
chosen out of the gamma mode exposed.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  4 ++++
 drivers/gpu/drm/drm_atomic_uapi.c         | 10 ++++++++++
 drivers/gpu/drm/drm_color_mgmt.c          | 19 +++++++++++++++++++
 include/drm/drm_plane.h                   | 14 ++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index f26b03853711..6e358067cb7a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -312,6 +312,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 	state->commit = NULL;
 	state->fb_damage_clips = 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);
@@ -359,6 +362,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 		drm_crtc_commit_put(state->commit);
 
 	drm_property_blob_put(state->fb_damage_clips);
+	drm_property_blob_put(state->degamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 40fa05fa33dc..ce3cb65d415e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -597,6 +597,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->color_range = val;
 	} else if (property == plane->degamma_mode_property) {
 		state->degamma_mode = 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_ext),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -665,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = state->color_range;
 	} else if (property == plane->degamma_mode_property) {
 		*val = state->degamma_mode;
+	} else if (property == plane->degamma_lut_property) {
+		*val = (state->degamma_lut) ?
+			state->degamma_lut->base.id : 0;
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 085ed0d0db00..29d0fc1e52b5 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -596,6 +596,12 @@ EXPORT_SYMBOL(drm_plane_create_color_properties);
  *     to query and get the plane degamma color caps and choose the
  *     appropriate degamma mode and create lut values accordingly
  *
+ * 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.
+ *
  */
 int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 					   struct drm_plane *plane,
@@ -610,6 +616,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 
 	plane->degamma_mode_property = prop;
 
+	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+				   "PLANE_DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+
+	plane->degamma_lut_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
@@ -621,6 +634,12 @@ void drm_plane_attach_degamma_properties(struct drm_plane *plane)
 
 	drm_object_attach_property(&plane->base,
 				   plane->degamma_mode_property, 0);
+
+	if (!plane->degamma_lut_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->degamma_lut_property, 0);
 }
 EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
 
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index e476a5939f8e..bbd0033ed1d2 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -244,6 +244,14 @@ struct drm_plane_state {
 	 */
 	u32 degamma_mode;
 
+	/* @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;
+
 	u8 color_mgmt_changed : 1;
 };
 
@@ -762,6 +770,12 @@ struct drm_plane {
 	 * used to convert the framebuffer's colors to linear gamma.
 	 */
 	struct drm_property *degamma_mode_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;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (2 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 03/21] drm: Add Plane Degamma Lut property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-28 15:14   ` Harry Wentland
  2021-06-01 10:52 ` [PATCH 05/21] drm/i915/xelpd: Add register definitions for Plane Degamma Uma Shankar
                   ` (17 subsequent siblings)
  21 siblings, 1 reply; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Define the structure with XE_LPD degamma lut ranges. HDR and SDR
planes have different capabilities, implemented respective
structure for the HDR planes.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index dab892d2251b..c735d06a6b54 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
 	}
 }
 
+ /* FIXME input bpc? */
+__maybe_unused
+static const struct drm_color_lut_range d13_degamma_hdr[] = {
+	/* segment 1 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 128,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 0, .end = (1 << 24) - 1,
+		.min = 0, .max = (1 << 24) - 1,
+	},
+	/* segment 2 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = (1 << 24) - 1, .end = 1 << 24,
+		.min = 0, .max = (1 << 27) - 1,
+	},
+	/* Segment 3 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 1 << 24, .end = 3 << 24,
+		.min = 0, .max = (1 << 27) - 1,
+	},
+	/* Segment 4 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 3 << 24, .end = 7 << 24,
+		.min = 0, .max = (1 << 27) - 1,
+	},
+};
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 05/21] drm/i915/xelpd: Add register definitions for Plane Degamma
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (3 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 06/21] drm/i915/xelpd: Enable plane color features Uma Shankar
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add macros to define Plane Degamma registers

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 24307c49085f..9431913969f3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -262,6 +262,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 					      INTEL_INFO(dev_priv)->cursor_offsets[PIPE_A] + (reg) + \
 					      DISPLAY_MMIO_BASE(dev_priv))
 
+/* Plane Gamma Registers */
+#define _MMIO_PLANE_GAMC(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))					   \
@@ -11300,6 +11303,55 @@ enum skl_power_gate {
 					_PAL_PREC_MULTI_SEG_DATA_A, \
 					_PAL_PREC_MULTI_SEG_DATA_B)
 
+/* Display13 Plane Degmma Reg */
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_A	0x701d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_B	0x711d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_A	0x702d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_B	0x712d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_A, \
+						_PLANE_PRE_CSC_GAMC_INDEX_ENH_1_B)
+#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_A, \
+						_PLANE_PRE_CSC_GAMC_INDEX_ENH_2_B)
+#define PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe), \
+		_PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe))
+
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A	0x701d4
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_B	0x711d4
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2_A	0x702d4
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2_B	0x712d4
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A, \
+						_PLANE_PRE_CSC_GAMC_DATA_ENH_1_B)
+#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_ENH_2_A, \
+						_PLANE_PRE_CSC_GAMC_DATA_ENH_2_B)
+#define PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_ENH_1(pipe), \
+		_PLANE_PRE_CSC_GAMC_DATA_ENH_2(pipe))
+
+#define _PLANE_PRE_CSC_GAMC_INDEX_1_A	0x704d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_1_B	0x714d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_2_A	0x705d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_2_B	0x715d0
+#define _PLANE_PRE_CSC_GAMC_INDEX_1(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_1_A, \
+						_PLANE_PRE_CSC_GAMC_INDEX_1_B)
+#define _PLANE_PRE_CSC_GAMC_INDEX_2(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_2_A, \
+						_PLANE_PRE_CSC_GAMC_INDEX_2_B)
+#define PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_1(pipe), \
+		_PLANE_PRE_CSC_GAMC_INDEX_2(pipe))
+
+#define _PLANE_PRE_CSC_GAMC_DATA_1_A	0x704d4
+#define _PLANE_PRE_CSC_GAMC_DATA_1_B	0x714d4
+#define _PLANE_PRE_CSC_GAMC_DATA_2_A	0x705d4
+#define _PLANE_PRE_CSC_GAMC_DATA_2_B	0x715d4
+#define _PLANE_PRE_CSC_GAMC_DATA_1(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_1_A, \
+						_PLANE_PRE_CSC_GAMC_DATA_1_B)
+#define _PLANE_PRE_CSC_GAMC_DATA_2(pipe)	_PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_2_A, \
+						_PLANE_PRE_CSC_GAMC_DATA_2_B)
+#define PLANE_PRE_CSC_GAMC_DATA(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_1(pipe), \
+		_PLANE_PRE_CSC_GAMC_DATA_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)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 06/21] drm/i915/xelpd: Enable plane color features
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (4 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 05/21] drm/i915/xelpd: Add register definitions for Plane Degamma Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 07/21] drm/i915/xelpd: Add color capabilities of SDR planes Uma Shankar
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Enable and initialize plane color features.
Also initialize the color features of HDR planes.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 22 +++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_color.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.h            |  3 +++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index c735d06a6b54..00a0d45632fe 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -25,6 +25,7 @@
 #include "intel_color.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
+#include <drm/drm_plane.h>
 
 #define CTM_COEFF_SIGN	(1ULL << 63)
 
@@ -2094,7 +2095,6 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
 }
 
  /* FIXME input bpc? */
-__maybe_unused
 static const struct drm_color_lut_range d13_degamma_hdr[] = {
 	/* segment 1 */
 	{
@@ -2145,6 +2145,26 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = {
 	},
 };
 
+int intel_plane_color_init(struct drm_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->dev);
+	int ret = 0;
+
+	if (DISPLAY_VER(dev_priv) >= 13) {
+		drm_plane_create_color_mgmt_properties(plane->dev, plane, 2);
+		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma",
+								   NULL, 0,
+								   LUT_TYPE_DEGAMMA);
+		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma",
+								   d13_degamma_hdr,
+								   sizeof(d13_degamma_hdr),
+								   LUT_TYPE_DEGAMMA);
+		drm_plane_attach_degamma_properties(plane);
+	}
+
+	return ret;
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 173727aaa24d..b8850bb1b0c9 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -10,6 +10,7 @@
 
 struct intel_crtc_state;
 struct intel_crtc;
+struct drm_plane;
 struct drm_property_blob;
 
 void intel_color_init(struct intel_crtc *crtc);
@@ -21,5 +22,6 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
 bool intel_color_lut_equal(struct drm_property_blob *blob1,
 			   struct drm_property_blob *blob2,
 			   u32 gamma_mode, u32 bit_precision);
+int intel_plane_color_init(struct drm_plane *plane);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0f6d27da69ac..f955f039edba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -326,6 +326,9 @@ struct drm_i915_display_funcs {
 	 */
 	void (*load_luts)(const struct intel_crtc_state *crtc_state);
 	void (*read_luts)(struct intel_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);
 };
 
 struct intel_dmc {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 07/21] drm/i915/xelpd: Add color capabilities of SDR planes
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (5 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 06/21] drm/i915/xelpd: Enable plane color features Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 08/21] drm/i915/xelpd: Program Plane Degamma Registers Uma Shankar
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add the Color capabilities of SDR planes.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 67 ++++++++++++++++++++--
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 00a0d45632fe..4908e825f3cc 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -25,6 +25,7 @@
 #include "intel_color.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
+#include "intel_sprite.h"
 #include <drm/drm_plane.h>
 
 #define CTM_COEFF_SIGN	(1ULL << 63)
@@ -2145,6 +2146,57 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = {
 	},
 };
 
+ /* FIXME input bpc? */
+static const struct drm_color_lut_range d13_degamma_sdr[] = {
+	/* segment 1 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 32,
+		.input_bpc = 16, .output_bpc = 16,
+		.start = 0, .end = (1 << 16) - (1 << 16) / 33,
+		.min = 0, .max = (1 << 16) - 1,
+	},
+	/* segment 2 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 16, .output_bpc = 16,
+		.start = (1 << 16) - (1 << 16) / 33, .end = 1 << 16,
+		.min = 0, .max = 1 << 16,
+	},
+	/* Segment 3 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 16, .output_bpc = 16,
+		.start = 1 << 16, .end = 3 << 16,
+		.min = 0, .max = (8 << 16) - 1,
+	},
+	/* Segment 4 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 16, .output_bpc = 16,
+		.start = 3 << 16, .end = 7 << 16,
+		.min = 0, .max = (8 << 16) - 1,
+	},
+};
+
 int intel_plane_color_init(struct drm_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->dev);
@@ -2155,10 +2207,17 @@ int intel_plane_color_init(struct drm_plane *plane)
 		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma",
 								   NULL, 0,
 								   LUT_TYPE_DEGAMMA);
-		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma",
-								   d13_degamma_hdr,
-								   sizeof(d13_degamma_hdr),
-								   LUT_TYPE_DEGAMMA);
+		if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id))
+			ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma",
+									   d13_degamma_hdr,
+									   sizeof(d13_degamma_hdr),
+									   LUT_TYPE_DEGAMMA);
+		else
+			ret = drm_plane_color_add_gamma_degamma_mode_range(plane,
+									   "plane degamma",
+									   d13_degamma_sdr,
+									   sizeof(d13_degamma_sdr),
+									   LUT_TYPE_DEGAMMA);
 		drm_plane_attach_degamma_properties(plane);
 	}
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 08/21] drm/i915/xelpd: Program Plane Degamma Registers
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (6 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 07/21] drm/i915/xelpd: Add color capabilities of SDR planes Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 09/21] drm/i915/xelpd: Add plane color check to glk_plane_color_ctl Uma Shankar
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Extract the LUT and program plane degamma registers.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 116 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h            |   2 +
 2 files changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 4908e825f3cc..4e5733573fd8 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -126,6 +126,29 @@ static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state *crtc_state
 		lut_is_legacy(crtc_state->hw.gamma_lut);
 }
 
+/*
+ * Added to accommodate enhanced LUT precision.
+ * Max LUT precision is 32 bits.
+ */
+static u64 drm_color_lut_extract_ext(u64 user_input, u32 bit_precision)
+{
+	u64 val = user_input & 0xffffffff;
+	u32 max;
+
+	if (bit_precision > 32)
+		return 0;
+
+	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 ((user_input & 0xffffffff00000000) |
+		clamp_val(val, 0, max));
+}
+
 /*
  * When using limited range, multiply the matrix given by userspace by
  * the matrix that we would use for the limited range.
@@ -2197,6 +2220,97 @@ static const struct drm_color_lut_range d13_degamma_sdr[] = {
 	},
 };
 
+static void d13_program_plane_degamma_lut(const struct drm_plane_state *state,
+					  struct drm_color_lut_ext *degamma_lut,
+					  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;
+	u32 i, lut_size;
+
+	if (icl_is_hdr_plane(dev_priv, plane)) {
+		lut_size = 128;
+
+		intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
+			       PLANE_PAL_PREC_AUTO_INCREMENT);
+
+		if (degamma_lut) {
+			for (i = 0; i < lut_size; i++) {
+				u64 word = drm_color_lut_extract_ext(degamma_lut[i].green, 24);
+				u32 lut_val = (word & 0xffffff);
+
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       lut_val);
+			}
+
+			/* Program the max register to clamp values > 1.0. */
+			while (i < 131)
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       degamma_lut[i++].green);
+		} else {
+			for (i = 0; i < lut_size; i++) {
+				u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
+
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), v);
+			}
+
+			do {
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       1 << 24);
+			} while (i++ < 130);
+		}
+
+		intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
+	} else {
+		lut_size = 32;
+
+		/*
+		 * First 3 planes are HDR, so reduce by 3 to get to the right
+		 * SDR plane offset
+		 */
+		plane = plane - 3;
+
+		intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, 0),
+			       PLANE_PAL_PREC_AUTO_INCREMENT);
+
+		if (degamma_lut) {
+			for (i = 0; i < lut_size; i++)
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					       degamma_lut[i].green);
+			/* Program the max register to clamp values > 1.0. */
+			while (i < 35)
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					       degamma_lut[i++].green);
+		} else {
+			for (i = 0; i < lut_size; i++) {
+				u32 v = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), v);
+			}
+
+			do {
+				intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					       1 << 16);
+			} while (i++ < 34);
+		}
+
+		intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, 0), 0);
+	}
+}
+
+static void d13_plane_load_luts(const struct drm_plane_state *plane_state)
+{
+	const struct drm_property_blob *degamma_lut_blob =
+					plane_state->degamma_lut;
+	struct drm_color_lut_ext *degamma_lut = NULL;
+
+	if (degamma_lut_blob) {
+		degamma_lut = degamma_lut_blob->data;
+		d13_program_plane_degamma_lut(plane_state, degamma_lut, 0);
+	}
+}
+
 int intel_plane_color_init(struct drm_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->dev);
@@ -2218,6 +2332,8 @@ int intel_plane_color_init(struct drm_plane *plane)
 									   d13_degamma_sdr,
 									   sizeof(d13_degamma_sdr),
 									   LUT_TYPE_DEGAMMA);
+
+		dev_priv->display.load_plane_luts = d13_plane_load_luts;
 		drm_plane_attach_degamma_properties(plane);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9431913969f3..ede7dca440e2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7189,6 +7189,7 @@ enum {
 #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
 #define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* ICL+ */
 #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
+#define   PLANE_PRE_CSC_GAMMA_ENABLE		(1 << 14)
 #define   PLANE_COLOR_CSC_MODE_BYPASS			(0 << 17)
 #define   PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601		(1 << 17)
 #define   PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709		(2 << 17)
@@ -11315,6 +11316,7 @@ enum skl_power_gate {
 #define PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, i)	\
 		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe), \
 		_PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe))
+#define	 PLANE_PAL_PREC_AUTO_INCREMENT		(1 << 10)
 
 #define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A	0x701d4
 #define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_B	0x711d4
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 09/21] drm/i915/xelpd: Add plane color check to glk_plane_color_ctl
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (7 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 08/21] drm/i915/xelpd: Program Plane Degamma Registers Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 10/21] drm/i915/xelpd: Initialize plane color features Uma Shankar
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Extended glk_plane_color_ctl to have plane color checks. This helps
enabling the degamma or gamma block based on user inputs.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 92a4fd508e92..ae439dca4b3c 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -960,6 +960,11 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	u32 plane_color_ctl = 0;
 
 	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
+
+	/* FIXME needs hw.degamma_lut */
+	if (plane_state->uapi.degamma_lut)
+		plane_color_ctl |= PLANE_PRE_CSC_GAMMA_ENABLE;
+
 	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
 
 	if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 10/21] drm/i915/xelpd: Initialize plane color features
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (8 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 09/21] drm/i915/xelpd: Add plane color check to glk_plane_color_ctl Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 11/21] drm/i915/xelpd: Load plane color luts from atomic flip Uma Shankar
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Initialize plane color features for XE_LPD.

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

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index dc4d05e75e1c..c809f522a710 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -65,5 +65,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 				      bool can_position);
 void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
 			       struct intel_plane_state *plane_state);
+int intel_plane_color_init(struct drm_plane *plane);
 
 #endif /* __INTEL_ATOMIC_PLANE_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index ae439dca4b3c..aadb984fdf77 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -2184,6 +2184,8 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
 						BIT(DRM_SCALING_FILTER_DEFAULT) |
 						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
 
+	intel_plane_color_init(&plane->base);
+
 	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
 
 	return plane;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 11/21] drm/i915/xelpd: Load plane color luts from atomic flip
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (9 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 10/21] drm/i915/xelpd: Initialize plane color features Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 12/21] drm: Add Plane CTM property Uma Shankar
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

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

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

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 36f52a1d7552..5de9c98beaf6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -513,6 +513,9 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state,
 		struct intel_plane_state *new_plane_state =
 			intel_atomic_get_new_plane_state(state, plane);
 
+		if (new_plane_state->uapi.color_mgmt_changed)
+			intel_color_load_plane_luts(&new_plane_state->uapi);
+
 		if (new_plane_state->uapi.visible ||
 		    new_plane_state->planar_slave) {
 			intel_update_plane(plane, new_crtc_state, new_plane_state);
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index c809f522a710..1ba3b524cee2 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -66,5 +66,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
 			       struct intel_plane_state *plane_state);
 int intel_plane_color_init(struct drm_plane *plane);
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state);
 
 #endif /* __INTEL_ATOMIC_PLANE_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 4e5733573fd8..6d57a47d8a60 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include "intel_atomic_plane.h"
 #include "intel_color.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
@@ -2311,6 +2312,14 @@ static void d13_plane_load_luts(const struct drm_plane_state *plane_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_plane_color_init(struct drm_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->dev);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 12/21] drm: Add Plane CTM property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (10 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 11/21] drm/i915/xelpd: Load plane color luts from atomic flip Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 13/21] drm: Add helper to attach Plane ctm property Uma Shankar
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add a blob property for plane CSC usage.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  3 +++
 drivers/gpu/drm/drm_atomic_uapi.c         | 10 ++++++++++
 drivers/gpu/drm/drm_color_mgmt.c          | 11 +++++++++++
 include/drm/drm_plane.h                   | 15 +++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 6e358067cb7a..fafb8af1c9cb 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -314,6 +314,8 @@ 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;
 }
@@ -363,6 +365,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	drm_property_blob_put(state->fb_damage_clips);
 	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_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ce3cb65d415e..6af2afe362ff 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -604,6 +604,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 (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -675,6 +683,8 @@ drm_atomic_plane_get_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 (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 29d0fc1e52b5..83832adf3adf 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -602,6 +602,10 @@ EXPORT_SYMBOL(drm_plane_create_color_properties);
  *	engine, thereby making the content as linear for further color
  *	processing.
  *
+ * 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_create_color_mgmt_properties(struct drm_device *dev,
 					   struct drm_plane *plane,
@@ -623,6 +627,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 
 	plane->degamma_lut_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_create_color_mgmt_properties);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index bbd0033ed1d2..83d11918333b 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -252,6 +252,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;
+
 	u8 color_mgmt_changed : 1;
 };
 
@@ -776,6 +784,13 @@ struct drm_plane {
 	 * used to convert the framebuffer's colors to linear gamma.
 	 */
 	struct drm_property *degamma_lut_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)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 13/21] drm: Add helper to attach Plane ctm property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (11 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 12/21] drm: Add Plane CTM property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 14/21] drm/i915/xelpd: Define Plane CSC Registers Uma Shankar
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add a DRM helper to attach ctm property.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 10 ++++++++++
 include/drm/drm_plane.h          |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 83832adf3adf..5c3138497b9c 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -654,6 +654,16 @@ void drm_plane_attach_degamma_properties(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
 
+void drm_plane_attach_ctm_property(struct drm_plane *plane)
+{
+	if (!plane->ctm_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->ctm_property, 0);
+}
+EXPORT_SYMBOL(drm_plane_attach_ctm_property);
+
 int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
 						 const char *name,
 						 const struct drm_color_lut_range *ranges,
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 83d11918333b..4557f59cf3cf 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -886,6 +886,7 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 					   struct drm_plane *plane,
 					   int num_values);
 void drm_plane_attach_degamma_properties(struct drm_plane *plane);
+void drm_plane_attach_ctm_property(struct drm_plane *plane);
 int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
 						 const char *name,
 						 const struct drm_color_lut_range *ranges,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 14/21] drm/i915/xelpd: Define Plane CSC Registers
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (12 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 13/21] drm: Add helper to attach Plane ctm property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 15/21] drm/i915/xelpd: Enable Plane CSC Uma Shankar
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Define Register macros for plane CSC.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 43 +++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ede7dca440e2..df8500a86e9d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7397,6 +7397,49 @@ enum {
 #define PLANE_COLOR_CTL(pipe, plane)	\
 	_MMIO_PLANE(plane, _PLANE_COLOR_CTL_1(pipe), _PLANE_COLOR_CTL_2(pipe))
 
+/* Plane CSC Registers */
+#define _PLANE_CSC_RY_GY_1_A	0x70210
+#define _PLANE_CSC_RY_GY_2_A	0x70310
+
+#define _PLANE_CSC_RY_GY_1_B	0x71210
+#define _PLANE_CSC_RY_GY_2_B	0x71310
+
+#define _PLANE_CSC_RY_GY_1(pipe)	_PIPE(pipe, _PLANE_CSC_RY_GY_1_A, \
+					      _PLANE_CSC_RY_GY_1_B)
+#define _PLANE_CSC_RY_GY_2(pipe)	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \
+					      _PLANE_INPUT_CSC_RY_GY_2_B)
+#define PLANE_CSC_COEFF(pipe, plane, index)	_MMIO_PLANE(plane, \
+							    _PLANE_CSC_RY_GY_1(pipe) +  (index) * 4, \
+							    _PLANE_CSC_RY_GY_2(pipe) + (index) * 4)
+
+#define _PLANE_CSC_PREOFF_HI_1_A		0x70228
+#define _PLANE_CSC_PREOFF_HI_2_A		0x70328
+
+#define _PLANE_CSC_PREOFF_HI_1_B		0x71228
+#define _PLANE_CSC_PREOFF_HI_2_B		0x71328
+
+#define _PLANE_CSC_PREOFF_HI_1(pipe)	_PIPE(pipe, _PLANE_CSC_PREOFF_HI_1_A, \
+					      _PLANE_CSC_PREOFF_HI_1_B)
+#define _PLANE_CSC_PREOFF_HI_2(pipe)	_PIPE(pipe, _PLANE_CSC_PREOFF_HI_2_A, \
+					      _PLANE_CSC_PREOFF_HI_2_B)
+#define PLANE_CSC_PREOFF(pipe, plane, index)	_MMIO_PLANE(plane, _PLANE_CSC_PREOFF_HI_1(pipe) + \
+							    (index) * 4, _PLANE_CSC_PREOFF_HI_2(pipe) + \
+							    (index) * 4)
+
+#define _PLANE_CSC_POSTOFF_HI_1_A		0x70234
+#define _PLANE_CSC_POSTOFF_HI_2_A		0x70334
+
+#define _PLANE_CSC_POSTOFF_HI_1_B		0x71234
+#define _PLANE_CSC_POSTOFF_HI_2_B		0x71334
+
+#define _PLANE_CSC_POSTOFF_HI_1(pipe)	_PIPE(pipe, _PLANE_CSC_POSTOFF_HI_1_A, \
+					      _PLANE_CSC_POSTOFF_HI_1_B)
+#define _PLANE_CSC_POSTOFF_HI_2(pipe)	_PIPE(pipe, _PLANE_CSC_POSTOFF_HI_2_A, \
+					      _PLANE_CSC_POSTOFF_HI_2_B)
+#define PLANE_CSC_POSTOFF(pipe, plane, index)	_MMIO_PLANE(plane, _PLANE_CSC_POSTOFF_HI_1(pipe) + \
+							    (index) * 4, _PLANE_CSC_POSTOFF_HI_2(pipe) + \
+							    (index) * 4)
+
 #define _SEL_FETCH_PLANE_BASE_1_A		0x70890
 #define _SEL_FETCH_PLANE_BASE_2_A		0x708B0
 #define _SEL_FETCH_PLANE_BASE_3_A		0x708D0
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 15/21] drm/i915/xelpd: Enable Plane CSC
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (13 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 14/21] drm/i915/xelpd: Define Plane CSC Registers Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 16/21] drm: Add Plane Gamma Mode property Uma Shankar
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Implement plane CSC for ICL+

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |  5 +-
 drivers/gpu/drm/i915/display/intel_color.c    | 82 +++++++++++++++++++
 .../drm/i915/display/skl_universal_plane.c    |  4 +
 drivers/gpu/drm/i915/i915_reg.h               |  1 +
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 5de9c98beaf6..ec7646790892 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -499,6 +499,7 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct skl_ddb_entry entries_y[I915_MAX_PLANES];
 	struct skl_ddb_entry entries_uv[I915_MAX_PLANES];
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 update_mask = new_crtc_state->update_planes;
 	struct intel_plane *plane;
 
@@ -513,8 +514,10 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state,
 		struct intel_plane_state *new_plane_state =
 			intel_atomic_get_new_plane_state(state, plane);
 
-		if (new_plane_state->uapi.color_mgmt_changed)
+		if (new_plane_state->uapi.color_mgmt_changed) {
 			intel_color_load_plane_luts(&new_plane_state->uapi);
+			dev_priv->display.load_plane_csc_matrix(&new_plane_state->uapi);
+		}
 
 		if (new_plane_state->uapi.visible ||
 		    new_plane_state->planar_slave) {
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 6d57a47d8a60..8b4f653b213d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2119,6 +2119,83 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
 	}
 }
 
+static void icl_load_plane_csc_matrix(const struct drm_plane_state *state)
+{
+	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;
+	struct drm_color_ctm *ctm;
+	const u64 *input;
+	u16 coeffs[9] = {};
+	u16 postoff = 0;
+	int i;
+
+	if (!icl_is_hdr_plane(dev_priv, plane) || !state->ctm)
+		return;
+
+	ctm = state->ctm->data;
+	input = ctm->matrix;
+
+	/*
+	 * Convert fixed point S31.32 input to format supported by the
+	 * hardware.
+	 */
+	for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+		u64 abs_coeff = ((1ULL << 63) - 1) & input[i];
+
+		/*
+		 * Clamp input value to min/max supported by
+		 * hardware.
+		 */
+		abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);
+
+		/* sign bit */
+		if (CTM_COEFF_NEGATIVE(input[i]))
+			coeffs[i] |= 1 << 15;
+
+		if (abs_coeff < CTM_COEFF_0_125)
+			coeffs[i] |= (3 << 12) |
+				ILK_CSC_COEFF_FP(abs_coeff, 12);
+		else if (abs_coeff < CTM_COEFF_0_25)
+			coeffs[i] |= (2 << 12) |
+				ILK_CSC_COEFF_FP(abs_coeff, 11);
+		else if (abs_coeff < CTM_COEFF_0_5)
+			coeffs[i] |= (1 << 12) |
+				ILK_CSC_COEFF_FP(abs_coeff, 10);
+		else if (abs_coeff < CTM_COEFF_1_0)
+			coeffs[i] |= ILK_CSC_COEFF_FP(abs_coeff, 9);
+		else if (abs_coeff < CTM_COEFF_2_0)
+			coeffs[i] |= (7 << 12) |
+				ILK_CSC_COEFF_FP(abs_coeff, 8);
+		else
+			coeffs[i] |= (6 << 12) |
+				ILK_CSC_COEFF_FP(abs_coeff, 7);
+	}
+
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 0),
+		       coeffs[0] << 16 | coeffs[1]);
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 1),
+		       coeffs[2] << 16);
+
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 2),
+		       coeffs[3] << 16 | coeffs[4]);
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 3),
+		       coeffs[5] << 16);
+
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 4),
+		       coeffs[6] << 16 | coeffs[7]);
+	intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 5),
+		       coeffs[8] << 16);
+
+	intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 0), 0);
+	intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 1), 0);
+	intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 2), 0);
+
+	intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 0), postoff);
+	intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 1), postoff);
+	intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 2), postoff);
+}
+
  /* FIXME input bpc? */
 static const struct drm_color_lut_range d13_degamma_hdr[] = {
 	/* segment 1 */
@@ -2343,7 +2420,12 @@ int intel_plane_color_init(struct drm_plane *plane)
 									   LUT_TYPE_DEGAMMA);
 
 		dev_priv->display.load_plane_luts = d13_plane_load_luts;
+		dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix;
+
 		drm_plane_attach_degamma_properties(plane);
+
+		if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id))
+			drm_plane_attach_ctm_property(plane);
 	}
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index aadb984fdf77..6ba670b6a5c9 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -965,6 +965,10 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	if (plane_state->uapi.degamma_lut)
 		plane_color_ctl |= PLANE_PRE_CSC_GAMMA_ENABLE;
 
+	/* FIXME needs hw.ctm */
+	if (plane_state->uapi.ctm)
+		plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE;
+
 	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
 
 	if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index df8500a86e9d..a8e35357aea0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7187,6 +7187,7 @@ enum {
 #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
 #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30) /* Pre-ICL */
 #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
+#define   PLANE_COLOR_PLANE_CSC_ENABLE		(1 << 21) /* ICL+ */
 #define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* ICL+ */
 #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
 #define   PLANE_PRE_CSC_GAMMA_ENABLE		(1 << 14)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 16/21] drm: Add Plane Gamma Mode property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (14 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 15/21] drm/i915/xelpd: Enable Plane CSC Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 17/21] drm: Add Plane Gamma Lut property Uma Shankar
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add Plane Gamma Mode as a blob property. This is an enum property
with values as blob_id's and exposes the various gamma modes
supported and the lut ranges. Getting the blob id in userspace,
user can get the mode supported and also the range of gamma mode
supported with number of lut coefficients. It can then set one of
the modes using this enum property.

Lut values will be sent through a separate GAMMA_LUT blob property.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
 drivers/gpu/drm/drm_color_mgmt.c  | 26 ++++++++++++++++++++++++++
 include/drm/drm_plane.h           | 14 ++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6af2afe362ff..6e3958491d10 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -612,6 +612,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == plane->gamma_mode_property) {
+		state->gamma_mode = val;
 	} else if (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -685,6 +687,8 @@ drm_atomic_plane_get_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_mode_property) {
+		*val = state->gamma_mode;
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 5c3138497b9c..02367e691cf3 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -606,6 +606,13 @@ EXPORT_SYMBOL(drm_plane_create_color_properties);
  *	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_mode_property:
+ *     Blob property which advertizes the possible gamma modes and
+ *     lut ranges supported by the platform. This  allows userspace
+ *     to query and get the plane gamma color caps and choose the
+ *     appropriate gamma mode and create lut values accordingly
+ *
  */
 int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 					   struct drm_plane *plane,
@@ -634,6 +641,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 
 	plane->ctm_property = prop;
 
+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
+				   "PLANE_GAMMA_MODE", num_values);
+	if (!prop)
+		return -ENOMEM;
+
+	plane->gamma_mode_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
@@ -664,6 +678,16 @@ void drm_plane_attach_ctm_property(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_attach_ctm_property);
 
+void drm_plane_attach_gamma_properties(struct drm_plane *plane)
+{
+	if (!plane->gamma_mode_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->gamma_mode_property, 0);
+}
+EXPORT_SYMBOL(drm_plane_attach_gamma_properties);
+
 int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
 						 const char *name,
 						 const struct drm_color_lut_range *ranges,
@@ -676,6 +700,8 @@ int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
 
 	if (type == LUT_TYPE_DEGAMMA)
 		prop = plane->degamma_mode_property;
+	else
+		prop = plane->gamma_mode_property;
 
 	if (!prop)
 		return -EINVAL;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 4557f59cf3cf..a7b7c8599702 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -260,6 +260,13 @@ struct drm_plane_state {
 	 */
 	struct drm_property_blob *ctm;
 
+	/**
+	 * @gamma_mode: This is a blob_id and exposes the platform capabilities
+	 * wrt to various gamma modes and the respective lut ranges. This also
+	 * helps user select a gamma mode amongst the supported ones.
+	 */
+	u32 gamma_mode;
+
 	u8 color_mgmt_changed : 1;
 };
 
@@ -791,6 +798,12 @@ struct drm_plane {
 	 * degamma LUT.
 	 */
 	struct drm_property *ctm_property;
+
+	/**
+	 * @gamma_mode_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to non-linear gamma.
+	 */
+	struct drm_property *gamma_mode_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -887,6 +900,7 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 					   int num_values);
 void drm_plane_attach_degamma_properties(struct drm_plane *plane);
 void drm_plane_attach_ctm_property(struct drm_plane *plane);
+void drm_plane_attach_gamma_properties(struct drm_plane *plane);
 int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
 						 const char *name,
 						 const struct drm_color_lut_range *ranges,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 17/21] drm: Add Plane Gamma Lut property
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (15 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 16/21] drm: Add Plane Gamma Mode property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 18/21] drm/i915/xelpd: Define and Initialize Plane Gamma Lut range Uma Shankar
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add Plane Gamma Lut as a blob property.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  3 +++
 drivers/gpu/drm/drm_atomic_uapi.c         | 10 ++++++++++
 drivers/gpu/drm/drm_color_mgmt.c          | 18 ++++++++++++++++++
 include/drm/drm_plane.h                   | 14 ++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index fafb8af1c9cb..7ddf6e4b956b 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -316,6 +316,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_get(state->gamma_lut);
 
 	state->color_mgmt_changed = false;
 }
@@ -366,6 +368,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 	drm_property_blob_put(state->fb_damage_clips);
 	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_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6e3958491d10..4f5b7f76208d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -614,6 +614,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		return ret;
 	} else if (property == plane->gamma_mode_property) {
 		state->gamma_mode = val;
+	} 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_ext),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -689,6 +696,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = (state->ctm) ? state->ctm->base.id : 0;
 	} else if (property == plane->gamma_mode_property) {
 		*val = state->gamma_mode;
+	} else if (property == plane->gamma_lut_property) {
+		*val = (state->gamma_lut) ?
+			state->gamma_lut->base.id : 0;
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 02367e691cf3..b5b3ff7f654d 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -613,6 +613,11 @@ EXPORT_SYMBOL(drm_plane_create_color_properties);
  *     to query and get the plane gamma color caps and choose the
  *     appropriate gamma mode and create lut values accordingly
  *
+ * gamma_lut_property:
+ *	Blob property which allows a userspace to provide LUT values
+ *	to apply gamma curve using the h/w plane degamma processing
+ *	engine, thereby making the content as non-linear.
+ *
  */
 int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 					   struct drm_plane *plane,
@@ -648,6 +653,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
 
 	plane->gamma_mode_property = prop;
 
+	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+				   "PLANE_GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+
+	plane->gamma_lut_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
@@ -685,6 +697,12 @@ void drm_plane_attach_gamma_properties(struct drm_plane *plane)
 
 	drm_object_attach_property(&plane->base,
 				   plane->gamma_mode_property, 0);
+
+	if (!plane->gamma_lut_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->gamma_lut_property, 0);
 }
 EXPORT_SYMBOL(drm_plane_attach_gamma_properties);
 
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index a7b7c8599702..8989bb1aa46c 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -267,6 +267,14 @@ struct drm_plane_state {
 	 */
 	u32 gamma_mode;
 
+	/* @gamma_lut:
+	 *
+	 * Lookup table for converting framebuffer pixel data after applying 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;
+
 	u8 color_mgmt_changed : 1;
 };
 
@@ -804,6 +812,12 @@ struct drm_plane {
 	 * used to convert the framebuffer's colors to non-linear gamma.
 	 */
 	struct drm_property *gamma_mode_property;
+
+	/**
+	 * @gamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to non-linear gamma.
+	 */
+	struct drm_property *gamma_lut_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 18/21] drm/i915/xelpd: Define and Initialize Plane Gamma Lut range
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (16 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 17/21] drm: Add Plane Gamma Lut property Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 19/21] drm/i915/xelpd: Add register definitions for Plane Gamma Uma Shankar
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Define the structure with XE_LPD gamma lut ranges. HDR and SDR planes
have different capabilities, implemented respective structure for
the HDR planes. Degamma and GAMMA has same Lut caps for SDR planes,
extended the same.

Initialize the mode range caps as well.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 112 ++++++++++++++++++---
 1 file changed, 99 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 8b4f653b213d..7f091dd0bb19 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2248,7 +2248,7 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = {
 };
 
  /* FIXME input bpc? */
-static const struct drm_color_lut_range d13_degamma_sdr[] = {
+static const struct drm_color_lut_range d13_gamma_degamma_sdr[] = {
 	/* segment 1 */
 	{
 		.flags = (DRM_MODE_LUT_GAMMA |
@@ -2298,6 +2298,63 @@ static const struct drm_color_lut_range d13_degamma_sdr[] = {
 	},
 };
 
+ /* FIXME input bpc? */
+static const struct drm_color_lut_range d13_gamma_hdr[] = {
+	/*
+	 * ToDo: Add Segment 1
+	 * There is an optional fine segment added with 9 lut values
+	 * Will be added later
+	 */
+
+	/* segment 2 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 32,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 0, .end = (1 << 24) - 1,
+		.min = 0, .max = (1 << 24) - 1,
+	},
+	/* segment 3 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = (1 << 24) - 1, .end = 1 << 24,
+		.min = 0, .max = 1 << 24,
+	},
+	/* Segment 4 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 1 << 24, .end = 3 << 24,
+		.min = 0, .max = (3 << 24),
+	},
+	/* Segment 5 */
+	{
+		.flags = (DRM_MODE_LUT_GAMMA |
+			  DRM_MODE_LUT_REFLECT_NEGATIVE |
+			  DRM_MODE_LUT_INTERPOLATE |
+			  DRM_MODE_LUT_REUSE_LAST |
+			  DRM_MODE_LUT_NON_DECREASING),
+		.count = 1,
+		.input_bpc = 24, .output_bpc = 16,
+		.start = 3 << 24, .end = 7 << 24,
+		.min = 0, .max = (7 << 24),
+	},
+};
+
 static void d13_program_plane_degamma_lut(const struct drm_plane_state *state,
 					  struct drm_color_lut_ext *degamma_lut,
 					  u32 offset)
@@ -2407,26 +2464,55 @@ int intel_plane_color_init(struct drm_plane *plane)
 		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma",
 								   NULL, 0,
 								   LUT_TYPE_DEGAMMA);
-		if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id))
+		if (ret)
+			return ret;
+
+		ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no gamma",
+								   NULL, 0,
+								   LUT_TYPE_GAMMA);
+		if (ret)
+			return ret;
+
+		if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) {
 			ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma",
 									   d13_degamma_hdr,
 									   sizeof(d13_degamma_hdr),
 									   LUT_TYPE_DEGAMMA);
-		else
-			ret = drm_plane_color_add_gamma_degamma_mode_range(plane,
-									   "plane degamma",
-									   d13_degamma_sdr,
-									   sizeof(d13_degamma_sdr),
+			if (ret)
+				return ret;
+
+			ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane gamma",
+									   d13_gamma_hdr,
+									   sizeof(d13_gamma_hdr),
+									   LUT_TYPE_GAMMA);
+			if (ret)
+				return ret;
+		} else {
+			ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma",
+									   d13_gamma_degamma_sdr,
+									   sizeof(d13_gamma_degamma_sdr),
 									   LUT_TYPE_DEGAMMA);
+			if (ret)
+				return ret;
+
+			ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane gamma",
+									   d13_gamma_degamma_sdr,
+									   sizeof(d13_gamma_degamma_sdr),
+									   LUT_TYPE_GAMMA);
+			if (ret)
+				return ret;
+		}
+	}
 
-		dev_priv->display.load_plane_luts = d13_plane_load_luts;
-		dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix;
+	dev_priv->display.load_plane_luts = d13_plane_load_luts;
+	dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix;
 
-		drm_plane_attach_degamma_properties(plane);
+	drm_plane_attach_degamma_properties(plane);
 
-		if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id))
-			drm_plane_attach_ctm_property(plane);
-	}
+	if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id))
+		drm_plane_attach_ctm_property(plane);
+
+	drm_plane_attach_gamma_properties(plane);
 
 	return ret;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 19/21] drm/i915/xelpd: Add register definitions for Plane Gamma
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (17 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 18/21] drm/i915/xelpd: Define and Initialize Plane Gamma Lut range Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 20/21] drm/i915/xelpd: Program Plane Gamma Registers Uma Shankar
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Add macros to define Plane Gamma registers

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 73 +++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a8e35357aea0..2ebc92104f64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11398,6 +11398,79 @@ enum skl_power_gate {
 		_MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_1(pipe), \
 		_PLANE_PRE_CSC_GAMC_DATA_2(pipe))
 
+/* Display13 Plane Gamma Reg */
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_A	0x70160
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_B	0x71160
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_A	0x70260
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_B	0x71260
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_A, \
+							_PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_B)
+#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_A, \
+							_PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_B)
+#define PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1(pipe), \
+		_PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2(pipe))
+
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_A	0x70164
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_B	0x71164
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_A	0x70264
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_B	0x71264
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_A, \
+							_PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_B)
+#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_A, \
+							_PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_B)
+#define PLANE_POST_CSC_GAMC_SEG0_DATA_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1(pipe), \
+		_PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2(pipe))
+
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1_A	0x701d8
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1_B	0x711d8
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2_A	0x702d8
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2_B	0x712d8
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_ENH_1_A, \
+						_PLANE_POST_CSC_GAMC_INDEX_ENH_1_B)
+#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_ENH_2_A, \
+						_PLANE_POST_CSC_GAMC_INDEX_ENH_2_B)
+#define PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_INDEX_ENH_1(pipe), \
+		_PLANE_POST_CSC_GAMC_INDEX_ENH_2(pipe))
+
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_1_A	0x701dc
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_1_B	0x711dc
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_2_A	0x702dc
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_2_B	0x712dc
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_ENH_1_A, \
+						_PLANE_POST_CSC_GAMC_DATA_ENH_1_B)
+#define _PLANE_POST_CSC_GAMC_DATA_ENH_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_ENH_2_A, \
+						_PLANE_POST_CSC_GAMC_DATA_ENH_2_B)
+#define PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_DATA_ENH_1(pipe), \
+		_PLANE_POST_CSC_GAMC_DATA_ENH_2(pipe))
+
+#define _PLANE_POST_CSC_GAMC_INDEX_1_A	0x704d8
+#define _PLANE_POST_CSC_GAMC_INDEX_1_B	0x714d8
+#define _PLANE_POST_CSC_GAMC_INDEX_2_A	0x705d8
+#define _PLANE_POST_CSC_GAMC_INDEX_2_B	0x715d8
+#define _PLANE_POST_CSC_GAMC_INDEX_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_1_A, \
+						_PLANE_POST_CSC_GAMC_INDEX_1_B)
+#define _PLANE_POST_CSC_GAMC_INDEX_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_2_A, \
+						_PLANE_POST_CSC_GAMC_INDEX_2_B)
+#define PLANE_POST_CSC_GAMC_INDEX(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_INDEX_1(pipe), \
+		_PLANE_POST_CSC_GAMC_INDEX_2(pipe))
+
+#define _PLANE_POST_CSC_GAMC_DATA_1_A	0x704dc
+#define _PLANE_POST_CSC_GAMC_DATA_1_B	0x714dc
+#define _PLANE_POST_CSC_GAMC_DATA_2_A	0x705dc
+#define _PLANE_POST_CSC_GAMC_DATA_2_B	0x715dc
+#define _PLANE_POST_CSC_GAMC_DATA_1(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_1_A, \
+						_PLANE_POST_CSC_GAMC_DATA_1_B)
+#define _PLANE_POST_CSC_GAMC_DATA_2(pipe)	_PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_2_A, \
+						_PLANE_POST_CSC_GAMC_DATA_2_B)
+#define PLANE_POST_CSC_GAMC_DATA(pipe, plane, i)	\
+		_MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_DATA_1(pipe), \
+		_PLANE_POST_CSC_GAMC_DATA_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)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 20/21] drm/i915/xelpd: Program Plane Gamma Registers
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (18 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 19/21] drm/i915/xelpd: Add register definitions for Plane Gamma Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-01 10:52 ` [PATCH 21/21] drm/i915/xelpd: Enable plane gamma Uma Shankar
  2021-06-02  9:28 ` [PATCH 00/21] Add Support for Plane Color Lut and CSC features Pekka Paalanen
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Extract the LUT and program plane gamma registers.
Enabled multi segmented lut as well.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 89 ++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h            |  9 ++-
 2 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 7f091dd0bb19..daf2148fb2df 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -27,6 +27,9 @@
 #include "intel_de.h"
 #include "intel_display_types.h"
 #include "intel_sprite.h"
+
+#include "skl_universal_plane.h"
+
 #include <drm/drm_plane.h>
 
 #define CTM_COEFF_SIGN	(1ULL << 63)
@@ -2434,16 +2437,102 @@ static void d13_program_plane_degamma_lut(const struct drm_plane_state *state,
 	}
 }
 
+static void d13_program_plane_gamma_lut(const struct drm_plane_state *state,
+					struct drm_color_lut_ext *gamma_lut,
+					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;
+	u32 i, lut_size;
+
+	if (icl_is_hdr_plane(dev_priv, plane)) {
+		intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
+			       offset | PLANE_PAL_PREC_AUTO_INCREMENT);
+		if (gamma_lut) {
+			lut_size = 32;
+			for (i = 0; i < lut_size; i++) {
+				u64 word = drm_color_lut_extract_ext(gamma_lut[i].green, 24);
+				u32 lut_val = (word & 0xffffff);
+
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       lut_val);
+			}
+
+			do {
+				/* Program the max register to clamp values > 1.0. */
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       gamma_lut[i].green);
+			} while (i++ < 34);
+		} else {
+			lut_size = 32;
+			for (i = 0; i < lut_size; i++) {
+				u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
+
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), v);
+			}
+
+			do {
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					       1 << 24);
+			} while (i++ < 34);
+		}
+
+		intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
+	} else {
+		lut_size = 32;
+		/*
+		 * First 3 planes are HDR, so reduce by 3 to get to the right
+		 * SDR plane offset
+		 */
+		plane = plane - 3;
+
+		intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0),
+			       offset | PLANE_PAL_PREC_AUTO_INCREMENT);
+
+		if (gamma_lut) {
+			for (i = 0; i < lut_size; i++)
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					       gamma_lut[i].green & 0xffff);
+			/* Program the max register to clamp values > 1.0. */
+			while (i < 35)
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					       gamma_lut[i++].green & 0x3ffff);
+		} else {
+			for (i = 0; i < lut_size; i++) {
+				u32 v = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), v);
+			}
+
+			do {
+				intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					       (1 << 16));
+			} while (i++ < 34);
+		}
+
+		intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0), 0);
+	}
+}
+
 static void d13_plane_load_luts(const struct drm_plane_state *plane_state)
 {
 	const struct drm_property_blob *degamma_lut_blob =
 					plane_state->degamma_lut;
+	const struct drm_property_blob *gamma_lut_blob =
+					plane_state->gamma_lut;
 	struct drm_color_lut_ext *degamma_lut = NULL;
+	struct drm_color_lut_ext *gamma_lut = NULL;
 
 	if (degamma_lut_blob) {
 		degamma_lut = degamma_lut_blob->data;
 		d13_program_plane_degamma_lut(plane_state, degamma_lut, 0);
 	}
+
+	if (gamma_lut_blob) {
+		gamma_lut = gamma_lut_blob->data;
+		d13_program_plane_gamma_lut(plane_state, gamma_lut, 0);
+	}
 }
 
 void intel_color_load_plane_luts(const struct drm_plane_state *plane_state)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2ebc92104f64..6ab4adbe7f56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7187,10 +7187,11 @@ enum {
 #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
 #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30) /* Pre-ICL */
 #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
-#define   PLANE_COLOR_PLANE_CSC_ENABLE		(1 << 21) /* ICL+ */
-#define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* ICL+ */
-#define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
-#define   PLANE_PRE_CSC_GAMMA_ENABLE		(1 << 14)
+#define   PLANE_COLOR_PLANE_CSC_ENABLE			(1 << 21) /* ICL+ */
+#define   PLANE_COLOR_INPUT_CSC_ENABLE			(1 << 20) /* ICL+ */
+#define   PLANE_COLOR_PIPE_CSC_ENABLE			(1 << 23) /* Pre-ICL */
+#define   PLANE_POST_CSC_GAMMA_MULTI_SEGMENT_ENABLE	(1 << 15)
+#define   PLANE_PRE_CSC_GAMMA_ENABLE			(1 << 14)
 #define   PLANE_COLOR_CSC_MODE_BYPASS			(0 << 17)
 #define   PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601		(1 << 17)
 #define   PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709		(2 << 17)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 21/21] drm/i915/xelpd: Enable plane gamma
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (19 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 20/21] drm/i915/xelpd: Program Plane Gamma Registers Uma Shankar
@ 2021-06-01 10:52 ` Uma Shankar
  2021-06-02  9:28 ` [PATCH 00/21] Add Support for Plane Color Lut and CSC features Pekka Paalanen
  21 siblings, 0 replies; 38+ messages in thread
From: Uma Shankar @ 2021-06-01 10:52 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Uma Shankar, bhanuprakash.modem

Enable plane gamma feature in check callbacks. Decide
based on the user input.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 6ba670b6a5c9..5d527d12ec45 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -959,7 +959,9 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	u32 plane_color_ctl = 0;
 
-	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
+	/* FIXME needs hw.gamma_lut */
+	if (!plane_state->uapi.gamma_lut)
+		plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
 
 	/* FIXME needs hw.degamma_lut */
 	if (plane_state->uapi.degamma_lut)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
                   ` (20 preceding siblings ...)
  2021-06-01 10:52 ` [PATCH 21/21] drm/i915/xelpd: Enable plane gamma Uma Shankar
@ 2021-06-02  9:28 ` Pekka Paalanen
  2021-06-02 20:22   ` Shankar, Uma
  21 siblings, 1 reply; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-02  9:28 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx, dri-devel, bhanuprakash.modem

[-- Attachment #1: Type: text/plain, Size: 7412 bytes --]

On Tue,  1 Jun 2021 16:21:57 +0530
Uma Shankar <uma.shankar@intel.com> 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

Hi,

this is an excellent picture. I have long been wanting schematics like
that in the DRM UAPI documentation. Another note on that:
https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html

But the schematic for DRM UAPI documentation needs to be written in
terms of the abstract KMS pipeline with property names spelled out,
like in what Ville sketched in that email.

> This patch series adds properties for plane color features. It adds
> properties for degamma used to linearize data and CSC used for gamut
> conversion. It also includes Gamma support 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.

This is very much welcome!

There is also the thread:
https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html

Everything mentioned will interact with each other by changing what the
abstract KMS pixel pipeline does. I think you and Harry should probably
look at each others' suggestions and see how to fit them all into a
single abstract KMS pipeline.

People are adding new pieces into KMS left and right, and I fear we
lose sight of how everything will actually work together when all KMS
properties are supposed to be generic and potentially present
simultaneously. This is why I would very much like to have that *whole*
abstract KMS pipeline documented with *everything*. Otherwise it is
coming really hard fast to figure out how generic userspace should use
all these KMS properties together.

Or if there cannot be a single abstract KMS pipeline, then sure, have
multiple, as long as they are documented and how userspace will know
which pipeline it is dealing with, and what things are mutually
exclusive so we can avoid writing userspace code for combinations that
will never exist.


Thanks,
pq

> Userspace 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 add the property interfaces and enable helper functions.
> This series adds Intel's XE_LPD hw specific plane gamma feature. We
> can build up and add other platform/hardware specific implementation
> on top of this series.
> 
> Credits: Special mention and credits to Ville Syrjala for coming up
> with a design for this feature and inputs. This series is based on
> his original design and idea.
> 
> Note: Userspace support for this new UAPI will be done on Chrome. We
> will notify the list once we have that ready for review.
> 
> ToDo: State readout for this feature will be added next.
> 
> Uma Shankar (21):
>   drm: Add Enhanced Gamma and color lut range attributes
>   drm: Add Plane Degamma Mode property
>   drm: Add Plane Degamma Lut property
>   drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
>   drm/i915/xelpd: Add register definitions for Plane Degamma
>   drm/i915/xelpd: Enable plane color features
>   drm/i915/xelpd: Add color capabilities of SDR planes
>   drm/i915/xelpd: Program Plane Degamma Registers
>   drm/i915/xelpd: Add plane color check to glk_plane_color_ctl
>   drm/i915/xelpd: Initialize plane color features
>   drm/i915/xelpd: Load plane color luts from atomic flip
>   drm: Add Plane CTM property
>   drm: Add helper to attach Plane ctm property
>   drm/i915/xelpd: Define Plane CSC Registers
>   drm/i915/xelpd: Enable Plane CSC
>   drm: Add Plane Gamma Mode property
>   drm: Add Plane Gamma Lut property
>   drm/i915/xelpd: Define and Initialize Plane Gamma Lut range
>   drm/i915/xelpd: Add register definitions for Plane Gamma
>   drm/i915/xelpd: Program Plane Gamma Registers
>   drm/i915/xelpd: Enable plane gamma
> 
>  Documentation/gpu/drm-kms.rst                 |  90 +++
>  drivers/gpu/drm/drm_atomic.c                  |   1 +
>  drivers/gpu/drm/drm_atomic_state_helper.c     |  12 +
>  drivers/gpu/drm/drm_atomic_uapi.c             |  38 ++
>  drivers/gpu/drm/drm_color_mgmt.c              | 177 +++++-
>  .../gpu/drm/i915/display/intel_atomic_plane.c |   6 +
>  .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
>  drivers/gpu/drm/i915/display/intel_color.c    | 513 ++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_color.h    |   2 +
>  .../drm/i915/display/skl_universal_plane.c    |  15 +-
>  drivers/gpu/drm/i915/i915_drv.h               |   3 +
>  drivers/gpu/drm/i915/i915_reg.h               | 176 +++++-
>  include/drm/drm_mode_object.h                 |   2 +-
>  include/drm/drm_plane.h                       |  81 +++
>  include/uapi/drm/drm_mode.h                   |  58 ++
>  15 files changed, 1170 insertions(+), 6 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
  2021-06-01 10:51 ` [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
@ 2021-06-02  9:33   ` Pekka Paalanen
  2021-06-02 20:26     ` Shankar, Uma
  0 siblings, 1 reply; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-02  9:33 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx, dri-devel, bhanuprakash.modem

[-- Attachment #1: Type: text/plain, Size: 3243 bytes --]

On Tue,  1 Jun 2021 16:21:58 +0530
Uma Shankar <uma.shankar@intel.com> 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.
> 
> This also defines a new structure to define color lut ranges,
> along with related macro definitions and enums. This will help
> describe multi segmented lut ranges in the hardware.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 9b6722d45f36..d0ce48d2e732 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -819,6 +819,64 @@ struct hdr_output_metadata {
>  	};
>  };
>  
> +/*
> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> + * can be used for either purpose, but not simultaneously. To expose
> + * modes that support gamma and degamma simultaneously the gamma mode
> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> + * ranges.
> + */
> +/* LUT is for gamma (after CTM) */
> +#define DRM_MODE_LUT_GAMMA BIT(0)
> +/* LUT is for degamma (before CTM) */
> +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> +/* linearly interpolate between the points */
> +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> +/*
> + * the last value of the previous range is the
> + * first value of the current range.
> + */
> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> +/* the curve must be non-decreasing */
> +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> +/* the curve is reflected across origin for negative inputs */
> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> +/* the same curve (red) is used for blue and green channels as well */
> +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> +
> +struct drm_color_lut_range {
> +	/* DRM_MODE_LUT_* */
> +	__u32 flags;
> +	/* number of points on the curve */
> +	__u16 count;
> +	/* input/output bits per component */
> +	__u8 input_bpc, output_bpc;
> +	/* input start/end values */
> +	__s32 start, end;
> +	/* output min/max values */
> +	__s32 min, max;
> +};
> +
> +enum lut_type {

Unprefixed type name in UAPI headers is probably not a good idea.

> +	LUT_TYPE_DEGAMMA = 0,
> +	LUT_TYPE_GAMMA = 1,
> +};

All the above stuff seems to be the same in your other patch series'
patch "[PATCH 1/9] drm: Add gamma mode property". Is this series
replacing the series "[PATCH 0/9] Enhance pipe color support for multi
segmented luts" or what does this mean?


Thanks,
pq

> +
> +/*
> + * Creating 64 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 U32.32 fixed point format.
> +	 */
> +	__u64 red;
> +	__u64 green;
> +	__u64 blue;
> +	__u64 reserved;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-02  9:28 ` [PATCH 00/21] Add Support for Plane Color Lut and CSC features Pekka Paalanen
@ 2021-06-02 20:22   ` Shankar, Uma
  2021-06-02 23:42     ` Harry Wentland
  0 siblings, 1 reply; 38+ messages in thread
From: Shankar, Uma @ 2021-06-02 20:22 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: intel-gfx, dri-devel, Modem,  Bhanuprakash



> -----Original Message-----
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Wednesday, June 2, 2021 2:59 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
> Bhanuprakash <bhanuprakash.modem@intel.com>; Harry Wentland
> <harry.wentland@amd.com>
> Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
> 
> On Tue,  1 Jun 2021 16:21:57 +0530
> Uma Shankar <uma.shankar@intel.com> 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
> 
> Hi,
> 
> this is an excellent picture. I have long been wanting schematics like that in the DRM
> UAPI documentation. Another note on that:
> https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html
> 
> But the schematic for DRM UAPI documentation needs to be written in terms of the
> abstract KMS pipeline with property names spelled out, like in what Ville sketched in
> that email.

Sure Pekka, I can add that.

> > This patch series adds properties for plane color features. It adds
> > properties for degamma used to linearize data and CSC used for gamut
> > conversion. It also includes Gamma support 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.
> 
> This is very much welcome!
> 
> There is also the thread:
> https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html
> 
> Everything mentioned will interact with each other by changing what the abstract
> KMS pixel pipeline does. I think you and Harry should probably look at each others'
> suggestions and see how to fit them all into a single abstract KMS pipeline.
> 
> People are adding new pieces into KMS left and right, and I fear we lose sight of how
> everything will actually work together when all KMS properties are supposed to be
> generic and potentially present simultaneously. This is why I would very much like to
> have that *whole* abstract KMS pipeline documented with *everything*. Otherwise
> it is coming really hard fast to figure out how generic userspace should use all these
> KMS properties together.
> 
> Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long
> as they are documented and how userspace will know which pipeline it is dealing
> with, and what things are mutually exclusive so we can avoid writing userspace code
> for combinations that will never exist.

This is a good suggestion to have the whole pipeline and properties documented along with
the exact usages. We may end with 2 properties almost doing similar work but needed due to
underlying hardware, but we can get that properly documented and defined. 

I will discuss with Harry and Ville as well to define this.

Regards,
Uma Shankar
> 
> Thanks,
> pq
> 
> > Userspace 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 add the property interfaces and enable helper functions.
> > This series adds Intel's XE_LPD hw specific plane gamma feature. We
> > can build up and add other platform/hardware specific implementation
> > on top of this series.
> >
> > Credits: Special mention and credits to Ville Syrjala for coming up
> > with a design for this feature and inputs. This series is based on his
> > original design and idea.
> >
> > Note: Userspace support for this new UAPI will be done on Chrome. We
> > will notify the list once we have that ready for review.
> >
> > ToDo: State readout for this feature will be added next.
> >
> > Uma Shankar (21):
> >   drm: Add Enhanced Gamma and color lut range attributes
> >   drm: Add Plane Degamma Mode property
> >   drm: Add Plane Degamma Lut property
> >   drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
> >   drm/i915/xelpd: Add register definitions for Plane Degamma
> >   drm/i915/xelpd: Enable plane color features
> >   drm/i915/xelpd: Add color capabilities of SDR planes
> >   drm/i915/xelpd: Program Plane Degamma Registers
> >   drm/i915/xelpd: Add plane color check to glk_plane_color_ctl
> >   drm/i915/xelpd: Initialize plane color features
> >   drm/i915/xelpd: Load plane color luts from atomic flip
> >   drm: Add Plane CTM property
> >   drm: Add helper to attach Plane ctm property
> >   drm/i915/xelpd: Define Plane CSC Registers
> >   drm/i915/xelpd: Enable Plane CSC
> >   drm: Add Plane Gamma Mode property
> >   drm: Add Plane Gamma Lut property
> >   drm/i915/xelpd: Define and Initialize Plane Gamma Lut range
> >   drm/i915/xelpd: Add register definitions for Plane Gamma
> >   drm/i915/xelpd: Program Plane Gamma Registers
> >   drm/i915/xelpd: Enable plane gamma
> >
> >  Documentation/gpu/drm-kms.rst                 |  90 +++
> >  drivers/gpu/drm/drm_atomic.c                  |   1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c     |  12 +
> >  drivers/gpu/drm/drm_atomic_uapi.c             |  38 ++
> >  drivers/gpu/drm/drm_color_mgmt.c              | 177 +++++-
> >  .../gpu/drm/i915/display/intel_atomic_plane.c |   6 +
> >  .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
> >  drivers/gpu/drm/i915/display/intel_color.c    | 513 ++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_color.h    |   2 +
> >  .../drm/i915/display/skl_universal_plane.c    |  15 +-
> >  drivers/gpu/drm/i915/i915_drv.h               |   3 +
> >  drivers/gpu/drm/i915/i915_reg.h               | 176 +++++-
> >  include/drm/drm_mode_object.h                 |   2 +-
> >  include/drm/drm_plane.h                       |  81 +++
> >  include/uapi/drm/drm_mode.h                   |  58 ++
> >  15 files changed, 1170 insertions(+), 6 deletions(-)
> >


^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
  2021-06-02  9:33   ` Pekka Paalanen
@ 2021-06-02 20:26     ` Shankar, Uma
  2021-06-04 15:23       ` Harry Wentland
  0 siblings, 1 reply; 38+ messages in thread
From: Shankar, Uma @ 2021-06-02 20:26 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: intel-gfx, dri-devel, Modem,  Bhanuprakash



> -----Original Message-----
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Wednesday, June 2, 2021 3:04 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
> Bhanuprakash <bhanuprakash.modem@intel.com>
> Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range
> attributes
> 
> On Tue,  1 Jun 2021 16:21:58 +0530
> Uma Shankar <uma.shankar@intel.com> 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.
> >
> > This also defines a new structure to define color lut ranges, along
> > with related macro definitions and enums. This will help describe
> > multi segmented lut ranges in the hardware.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  include/uapi/drm/drm_mode.h | 58
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 9b6722d45f36..d0ce48d2e732 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -819,6 +819,64 @@ struct hdr_output_metadata {
> >  	};
> >  };
> >
> > +/*
> > + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means
> the LUT
> > + * can be used for either purpose, but not simultaneously. To expose
> > + * modes that support gamma and degamma simultaneously the gamma mode
> > + * must declare distinct DRM_MODE_LUT_GAMMA and
> DRM_MODE_LUT_DEGAMMA
> > + * ranges.
> > + */
> > +/* LUT is for gamma (after CTM) */
> > +#define DRM_MODE_LUT_GAMMA BIT(0)
> > +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA
> > +BIT(1)
> > +/* linearly interpolate between the points */ #define
> > +DRM_MODE_LUT_INTERPOLATE BIT(2)
> > +/*
> > + * the last value of the previous range is the
> > + * first value of the current range.
> > + */
> > +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> > +/* the curve must be non-decreasing */ #define
> > +DRM_MODE_LUT_NON_DECREASING BIT(4)
> > +/* the curve is reflected across origin for negative inputs */
> > +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> > +/* the same curve (red) is used for blue and green channels as well
> > +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> > +
> > +struct drm_color_lut_range {
> > +	/* DRM_MODE_LUT_* */
> > +	__u32 flags;
> > +	/* number of points on the curve */
> > +	__u16 count;
> > +	/* input/output bits per component */
> > +	__u8 input_bpc, output_bpc;
> > +	/* input start/end values */
> > +	__s32 start, end;
> > +	/* output min/max values */
> > +	__s32 min, max;
> > +};
> > +
> > +enum lut_type {
> 
> Unprefixed type name in UAPI headers is probably not a good idea.

Ok, will rename these.

> > +	LUT_TYPE_DEGAMMA = 0,
> > +	LUT_TYPE_GAMMA = 1,
> > +};
> 
> All the above stuff seems to be the same in your other patch series'
> patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the
> series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what
> does this mean?

The concept and idea is similar and the range definition is also common. But this series
focuses on plane color management while the other one is for pipe/crtc color features.
Hence separated and floated them as unique series for review.

Regards,
Uma Shankar
> 
> Thanks,
> pq
> 
> > +
> > +/*
> > + * Creating 64 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 U32.32 fixed point format.
> > +	 */
> > +	__u64 red;
> > +	__u64 green;
> > +	__u64 blue;
> > +	__u64 reserved;
> > +};
> > +
> >  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
> > DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
> > DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-02 20:22   ` Shankar, Uma
@ 2021-06-02 23:42     ` Harry Wentland
  2021-06-03  8:47       ` Pekka Paalanen
  0 siblings, 1 reply; 38+ messages in thread
From: Harry Wentland @ 2021-06-02 23:42 UTC (permalink / raw)
  To: Shankar, Uma, Pekka Paalanen; +Cc: intel-gfx, dri-devel, Modem, Bhanuprakash



On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
> 
> 
>> -----Original Message-----
>> From: Pekka Paalanen <ppaalanen@gmail.com>
>> Sent: Wednesday, June 2, 2021 2:59 PM
>> To: Shankar, Uma <uma.shankar@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
>> Bhanuprakash <bhanuprakash.modem@intel.com>; Harry Wentland
>> <harry.wentland@amd.com>
>> Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
>>
>> On Tue,  1 Jun 2021 16:21:57 +0530
>> Uma Shankar <uma.shankar@intel.com> 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
>>
>> Hi,
>>
>> this is an excellent picture. I have long been wanting schematics like that in the DRM
>> UAPI documentation. Another note on that:
>> https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html>>>
>> But the schematic for DRM UAPI documentation needs to be written in terms of the
>> abstract KMS pipeline with property names spelled out, like in what Ville sketched in
>> that email.
> 
> Sure Pekka, I can add that.
> 
>>> This patch series adds properties for plane color features. It adds
>>> properties for degamma used to linearize data and CSC used for gamut
>>> conversion. It also includes Gamma support 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.
>>
>> This is very much welcome!
>>
>> There is also the thread:
>> https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html>>>
>> Everything mentioned will interact with each other by changing what the abstract
>> KMS pixel pipeline does. I think you and Harry should probably look at each others'
>> suggestions and see how to fit them all into a single abstract KMS pipeline.
>>
>> People are adding new pieces into KMS left and right, and I fear we lose sight of how
>> everything will actually work together when all KMS properties are supposed to be
>> generic and potentially present simultaneously. This is why I would very much like to
>> have that *whole* abstract KMS pipeline documented with *everything*. Otherwise
>> it is coming really hard fast to figure out how generic userspace should use all these
>> KMS properties together.
>>
>> Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long
>> as they are documented and how userspace will know which pipeline it is dealing
>> with, and what things are mutually exclusive so we can avoid writing userspace code
>> for combinations that will never exist.
> 
> This is a good suggestion to have the whole pipeline and properties documented along with
> the exact usages. We may end with 2 properties almost doing similar work but needed due to
> underlying hardware, but we can get that properly documented and defined. 
> 
> I will discuss with Harry and Ville as well to define this.
> 

Just wanted to let you know that I've seen and read through both of Shankar's patchsets
and had some thoughts but haven't found the time to respond. I will respond soon.

I very much agree with Pekka. We need to make sure this all plays well together and is
well documented. Maybe a library to deal with DRM KMS color management/HDR would even
be helpful. Not sure yet how I feel about that.

Harry

> Regards,
> Uma Shankar
>>
>> Thanks,
>> pq
>>
>>> Userspace 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 add the property interfaces and enable helper functions.
>>> This series adds Intel's XE_LPD hw specific plane gamma feature. We
>>> can build up and add other platform/hardware specific implementation
>>> on top of this series.
>>>
>>> Credits: Special mention and credits to Ville Syrjala for coming up
>>> with a design for this feature and inputs. This series is based on his
>>> original design and idea.
>>>
>>> Note: Userspace support for this new UAPI will be done on Chrome. We
>>> will notify the list once we have that ready for review.
>>>
>>> ToDo: State readout for this feature will be added next.
>>>
>>> Uma Shankar (21):
>>>   drm: Add Enhanced Gamma and color lut range attributes
>>>   drm: Add Plane Degamma Mode property
>>>   drm: Add Plane Degamma Lut property
>>>   drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
>>>   drm/i915/xelpd: Add register definitions for Plane Degamma
>>>   drm/i915/xelpd: Enable plane color features
>>>   drm/i915/xelpd: Add color capabilities of SDR planes
>>>   drm/i915/xelpd: Program Plane Degamma Registers
>>>   drm/i915/xelpd: Add plane color check to glk_plane_color_ctl
>>>   drm/i915/xelpd: Initialize plane color features
>>>   drm/i915/xelpd: Load plane color luts from atomic flip
>>>   drm: Add Plane CTM property
>>>   drm: Add helper to attach Plane ctm property
>>>   drm/i915/xelpd: Define Plane CSC Registers
>>>   drm/i915/xelpd: Enable Plane CSC
>>>   drm: Add Plane Gamma Mode property
>>>   drm: Add Plane Gamma Lut property
>>>   drm/i915/xelpd: Define and Initialize Plane Gamma Lut range
>>>   drm/i915/xelpd: Add register definitions for Plane Gamma
>>>   drm/i915/xelpd: Program Plane Gamma Registers
>>>   drm/i915/xelpd: Enable plane gamma
>>>
>>>  Documentation/gpu/drm-kms.rst                 |  90 +++
>>>  drivers/gpu/drm/drm_atomic.c                  |   1 +
>>>  drivers/gpu/drm/drm_atomic_state_helper.c     |  12 +
>>>  drivers/gpu/drm/drm_atomic_uapi.c             |  38 ++
>>>  drivers/gpu/drm/drm_color_mgmt.c              | 177 +++++-
>>>  .../gpu/drm/i915/display/intel_atomic_plane.c |   6 +
>>>  .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
>>>  drivers/gpu/drm/i915/display/intel_color.c    | 513 ++++++++++++++++++
>>>  drivers/gpu/drm/i915/display/intel_color.h    |   2 +
>>>  .../drm/i915/display/skl_universal_plane.c    |  15 +-
>>>  drivers/gpu/drm/i915/i915_drv.h               |   3 +
>>>  drivers/gpu/drm/i915/i915_reg.h               | 176 +++++-
>>>  include/drm/drm_mode_object.h                 |   2 +-
>>>  include/drm/drm_plane.h                       |  81 +++
>>>  include/uapi/drm/drm_mode.h                   |  58 ++
>>>  15 files changed, 1170 insertions(+), 6 deletions(-)
>>>
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-02 23:42     ` Harry Wentland
@ 2021-06-03  8:47       ` Pekka Paalanen
  2021-06-03 12:30         ` Sebastian Wick
  0 siblings, 1 reply; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-03  8:47 UTC (permalink / raw)
  To: Harry Wentland
  Cc: intel-gfx, dri-devel, Sebastian Wick, Shankar, Uma,
	Vitaly Prosyak, Modem, Bhanuprakash

[-- Attachment #1: Type: text/plain, Size: 7045 bytes --]

On Wed, 2 Jun 2021 19:42:19 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
> > 
> >   
> >> -----Original Message-----
> >> From: Pekka Paalanen <ppaalanen@gmail.com>
> >> Sent: Wednesday, June 2, 2021 2:59 PM
> >> To: Shankar, Uma <uma.shankar@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
> >> Bhanuprakash <bhanuprakash.modem@intel.com>; Harry Wentland
> >> <harry.wentland@amd.com>
> >> Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
> >>
> >> On Tue,  1 Jun 2021 16:21:57 +0530
> >> Uma Shankar <uma.shankar@intel.com> wrote:
> >>  
> >>> This is how a typical display color hardware pipeline looks like:

...

> >>> This patch series adds properties for plane color features. It adds
> >>> properties for degamma used to linearize data and CSC used for gamut
> >>> conversion. It also includes Gamma support 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.  
> >>
> >> This is very much welcome!
> >>
> >> There is also the thread:
> >> https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html>>>
> >> Everything mentioned will interact with each other by changing what the abstract
> >> KMS pixel pipeline does. I think you and Harry should probably look at each others'
> >> suggestions and see how to fit them all into a single abstract KMS pipeline.
> >>
> >> People are adding new pieces into KMS left and right, and I fear we lose sight of how
> >> everything will actually work together when all KMS properties are supposed to be
> >> generic and potentially present simultaneously. This is why I would very much like to
> >> have that *whole* abstract KMS pipeline documented with *everything*. Otherwise
> >> it is coming really hard fast to figure out how generic userspace should use all these
> >> KMS properties together.
> >>
> >> Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long
> >> as they are documented and how userspace will know which pipeline it is dealing
> >> with, and what things are mutually exclusive so we can avoid writing userspace code
> >> for combinations that will never exist.  
> > 
> > This is a good suggestion to have the whole pipeline and properties documented along with
> > the exact usages. We may end with 2 properties almost doing similar work but needed due to
> > underlying hardware, but we can get that properly documented and defined. 
> > 
> > I will discuss with Harry and Ville as well to define this.
> >   
> 
> Just wanted to let you know that I've seen and read through both of Shankar's patchsets
> and had some thoughts but haven't found the time to respond. I will respond soon.

Hi Harry,

awesome!

> I very much agree with Pekka. We need to make sure this all plays well together and is
> well documented. Maybe a library to deal with DRM KMS color management/HDR would even
> be helpful. Not sure yet how I feel about that.

That is an excellent question. While I am working on Weston CM&HDR, I
already have issues with how to represent the color related
transformations. These new hardware features exposed here are nothing I
have prepared for, and would probably need changes to accommodate.

The main Weston roadmap is drafted in
https://gitlab.freedesktop.org/wayland/weston/-/issues/467

The MR that introduces the concept of a color transformation, and also
the whole beginnings of color management, is
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582

In that MR, there is a patch introducing struct weston_color_transform:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?commit_id=cffbf7c6b2faf7391b73ff9202774f660343bd34#ba0b86259533d5000d81c9c88109c9010eb0f641_0_77

The design idea there is that libweston shall have what I call "color
manager" module. That module handles all the policy decisions about
color, it uses a CMM (Little CMS 2 in this case) for all the color
profile computations, and based on all information it has available
from display EDID, ICC profile files, Wayland clients via the CM&HDR
protocol extension and more, it will ultimately produce
weston_color_transform objects.

weston_color_transform is a complete description of how to map a pixel
in one color model/space/encoding into another, maybe with user
preferred tuning/tone-mapping. E.g. from client content to the output's
blending space (output space but light-linear), or from output's
blending space to output's framebuffer space or maybe even monitor wire
space.

The mapping described by weston_color_transform shall be implemented by
libweston's GL-renderer or by the DRM-backend using KMS properties,
whatever works for each case. So the description cannot be opaque, it
has to map to GLSL shaders (easy) and KMS properties (???).

Now the problem is, what should weston_color_transform look like?

The current design has two steps in a color transform:
- Transfer function: identity, the traditional set of three 1D LUTs, or
  something else.
- Color mapping: identity, a 3D LUT, or something else.

"Something else" is a placeholder for whatever we want to have, but the
problem in adding new types of transfer function or color mapping
representations (e.g. the fancy new GAMMA_MODEs) is how will the color
manager create the parameters for those?

If we have ICC profiles as the original data, then we are probably
limited to what LCMS2 can produce. The issue with ICC profiles is that
they may contain 3D LUTs themselves, so not what I would call a
parametric model. OTOH, if we have, say, enumerated operations defined
by various HDR standards, we have to code those ourselves and then
producing whatever fancy representation is less of a problem.

Maybe that is how it has to be. If the color transformations are
defined by ICC profiles, we might be stuck with old-school KMS color
properties, but HDR stuff that doesn't rely on ICC can use the fancier
KMS properties. I'm sure interesting questions will arise when e.g. you
have the monitor in HDR mode, described with standard HDR terms, and
then you have application content described with an ICC profile (maybe
SDR, maybe not).

We can always get a 3D LUT out of LCMS2, so theoretically it would be
possible to get a huge LUT and then optimise whatever parameterised
model you have to that data set. But I worry that might be too costly
to do in-flight, at least in a way that blocks the compositor. Maybe do
what I hear shader compilers do: produce an unoptimal model fast, then
compute an optimised model asynchronously and replace when ready. And
disk cache(?).

A library probably makes sense in the long run, but for now, I would
have no idea at all what it should look like.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-03  8:47       ` Pekka Paalanen
@ 2021-06-03 12:30         ` Sebastian Wick
  2021-06-03 12:58           ` Pekka Paalanen
  0 siblings, 1 reply; 38+ messages in thread
From: Sebastian Wick @ 2021-06-03 12:30 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: intel-gfx, dri-devel, Shankar, Uma, Vitaly Prosyak, Modem, Bhanuprakash

On 2021-06-03 10:47, Pekka Paalanen wrote:
> On Wed, 2 Jun 2021 19:42:19 -0400
> Harry Wentland <harry.wentland@amd.com> wrote:
> 
>> On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Pekka Paalanen <ppaalanen@gmail.com>
>> >> Sent: Wednesday, June 2, 2021 2:59 PM
>> >> To: Shankar, Uma <uma.shankar@intel.com>
>> >> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
>> >> Bhanuprakash <bhanuprakash.modem@intel.com>; Harry Wentland
>> >> <harry.wentland@amd.com>
>> >> Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
>> >>
>> >> On Tue,  1 Jun 2021 16:21:57 +0530
>> >> Uma Shankar <uma.shankar@intel.com> wrote:
>> >>
>> >>> This is how a typical display color hardware pipeline looks like:
> 
> ...
> 
>> >>> This patch series adds properties for plane color features. It adds
>> >>> properties for degamma used to linearize data and CSC used for gamut
>> >>> conversion. It also includes Gamma support 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.
>> >>
>> >> This is very much welcome!
>> >>
>> >> There is also the thread:
>> >> https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html>>>
>> >> Everything mentioned will interact with each other by changing what the abstract
>> >> KMS pixel pipeline does. I think you and Harry should probably look at each others'
>> >> suggestions and see how to fit them all into a single abstract KMS pipeline.
>> >>
>> >> People are adding new pieces into KMS left and right, and I fear we lose sight of how
>> >> everything will actually work together when all KMS properties are supposed to be
>> >> generic and potentially present simultaneously. This is why I would very much like to
>> >> have that *whole* abstract KMS pipeline documented with *everything*. Otherwise
>> >> it is coming really hard fast to figure out how generic userspace should use all these
>> >> KMS properties together.
>> >>
>> >> Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long
>> >> as they are documented and how userspace will know which pipeline it is dealing
>> >> with, and what things are mutually exclusive so we can avoid writing userspace code
>> >> for combinations that will never exist.
>> >
>> > This is a good suggestion to have the whole pipeline and properties documented along with
>> > the exact usages. We may end with 2 properties almost doing similar work but needed due to
>> > underlying hardware, but we can get that properly documented and defined.
>> >
>> > I will discuss with Harry and Ville as well to define this.
>> >
>> 
>> Just wanted to let you know that I've seen and read through both of 
>> Shankar's patchsets
>> and had some thoughts but haven't found the time to respond. I will 
>> respond soon.
> 
> Hi Harry,
> 
> awesome!
> 
>> I very much agree with Pekka. We need to make sure this all plays well 
>> together and is
>> well documented. Maybe a library to deal with DRM KMS color 
>> management/HDR would even
>> be helpful. Not sure yet how I feel about that.
> 
> That is an excellent question. While I am working on Weston CM&HDR, I
> already have issues with how to represent the color related
> transformations. These new hardware features exposed here are nothing I
> have prepared for, and would probably need changes to accommodate.
> 
> The main Weston roadmap is drafted in
> https://gitlab.freedesktop.org/wayland/weston/-/issues/467
> 
> The MR that introduces the concept of a color transformation, and also
> the whole beginnings of color management, is
> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582
> 
> In that MR, there is a patch introducing struct weston_color_transform:
> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?commit_id=cffbf7c6b2faf7391b73ff9202774f660343bd34#ba0b86259533d5000d81c9c88109c9010eb0f641_0_77
> 
> The design idea there is that libweston shall have what I call "color
> manager" module. That module handles all the policy decisions about
> color, it uses a CMM (Little CMS 2 in this case) for all the color
> profile computations, and based on all information it has available
> from display EDID, ICC profile files, Wayland clients via the CM&HDR
> protocol extension and more, it will ultimately produce
> weston_color_transform objects.
> 
> weston_color_transform is a complete description of how to map a pixel
> in one color model/space/encoding into another, maybe with user
> preferred tuning/tone-mapping. E.g. from client content to the output's
> blending space (output space but light-linear), or from output's
> blending space to output's framebuffer space or maybe even monitor wire
> space.
> 
> The mapping described by weston_color_transform shall be implemented by
> libweston's GL-renderer or by the DRM-backend using KMS properties,
> whatever works for each case. So the description cannot be opaque, it
> has to map to GLSL shaders (easy) and KMS properties (???).
> 
> Now the problem is, what should weston_color_transform look like?
> 
> The current design has two steps in a color transform:
> - Transfer function: identity, the traditional set of three 1D LUTs, or
>   something else.
> - Color mapping: identity, a 3D LUT, or something else.
> 
> "Something else" is a placeholder for whatever we want to have, but the
> problem in adding new types of transfer function or color mapping
> representations (e.g. the fancy new GAMMA_MODEs) is how will the color
> manager create the parameters for those?

I think the weston_color_transform is going a bit in the wrong
direction. While the 3D LUT can describe everything if it has enough
precision it indeed makes sense to apply a transform before to get the
required precision down. It doesn't have to be a TF though and we really
don't care what it is as long as in the end the content is in the
correct color space and dynamic range. This might be enough to get
something off the ground right now though.

In the long run however it probably makes more sense to convert the
color transform to a complete pipeline of enumerated, parametric and
numerical elements together with some helpers to lower (enumerated >
parametric > numerical) and fuse elements (to the point that you can
always convert the pipeline to a 3D LUT). The color manager ideally
should provide a pipeline with the highest abstraction and avoid fusing
elements if it would result in a lose of information. This is a lot more
complex but it also gives us much better chances of finding a way to
offload the transform.

AFAIR lcms uses such a model and gives you access to the pipeline. If we
want to be independent of lcms we would need our own descriptions and
possibly lower some lcms elements to our own stuff. I'm also not sure
how good lcms is at retaining the high level description if possible.

> If we have ICC profiles as the original data, then we are probably
> limited to what LCMS2 can produce. The issue with ICC profiles is that
> they may contain 3D LUTs themselves, so not what I would call a
> parametric model. OTOH, if we have, say, enumerated operations defined
> by various HDR standards, we have to code those ourselves and then
> producing whatever fancy representation is less of a problem.
> 
> Maybe that is how it has to be. If the color transformations are
> defined by ICC profiles, we might be stuck with old-school KMS color
> properties, but HDR stuff that doesn't rely on ICC can use the fancier
> KMS properties. I'm sure interesting questions will arise when e.g. you
> have the monitor in HDR mode, described with standard HDR terms, and
> then you have application content described with an ICC profile (maybe
> SDR, maybe not).
> 
> We can always get a 3D LUT out of LCMS2, so theoretically it would be
> possible to get a huge LUT and then optimise whatever parameterised
> model you have to that data set. But I worry that might be too costly
> to do in-flight, at least in a way that blocks the compositor. Maybe do
> what I hear shader compilers do: produce an unoptimal model fast, then
> compute an optimised model asynchronously and replace when ready. And
> disk cache(?).
> 
> A library probably makes sense in the long run, but for now, I would
> have no idea at all what it should look like.
> 
> 
> Thanks,
> pq

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
  2021-06-03 12:30         ` Sebastian Wick
@ 2021-06-03 12:58           ` Pekka Paalanen
  0 siblings, 0 replies; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-03 12:58 UTC (permalink / raw)
  To: Sebastian Wick
  Cc: intel-gfx, dri-devel, Shankar, Uma, Vitaly Prosyak, Modem, Bhanuprakash

[-- Attachment #1: Type: text/plain, Size: 8663 bytes --]

On Thu, 03 Jun 2021 14:30:41 +0200
Sebastian Wick <sebastian@sebastianwick.net> wrote:

> On 2021-06-03 10:47, Pekka Paalanen wrote:
> > On Wed, 2 Jun 2021 19:42:19 -0400
> > Harry Wentland <harry.wentland@amd.com> wrote:
> >   
> >> On 2021-06-02 4:22 p.m., Shankar, Uma wrote:  
> >> >
> >> >  
> >> >> -----Original Message-----
> >> >> From: Pekka Paalanen <ppaalanen@gmail.com>
> >> >> Sent: Wednesday, June 2, 2021 2:59 PM
> >> >> To: Shankar, Uma <uma.shankar@intel.com>
> >> >> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
> >> >> Bhanuprakash <bhanuprakash.modem@intel.com>; Harry Wentland
> >> >> <harry.wentland@amd.com>
> >> >> Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
> >> >>
> >> >> On Tue,  1 Jun 2021 16:21:57 +0530
> >> >> Uma Shankar <uma.shankar@intel.com> wrote:
> >> >>  
> >> >>> This is how a typical display color hardware pipeline looks like:  
> > 
> > ...
> >   
> >> >>> This patch series adds properties for plane color features. It adds
> >> >>> properties for degamma used to linearize data and CSC used for gamut
> >> >>> conversion. It also includes Gamma support 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.  
> >> >>
> >> >> This is very much welcome!
> >> >>
> >> >> There is also the thread:
> >> >> https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html>>>
> >> >> Everything mentioned will interact with each other by changing what the abstract
> >> >> KMS pixel pipeline does. I think you and Harry should probably look at each others'
> >> >> suggestions and see how to fit them all into a single abstract KMS pipeline.
> >> >>
> >> >> People are adding new pieces into KMS left and right, and I fear we lose sight of how
> >> >> everything will actually work together when all KMS properties are supposed to be
> >> >> generic and potentially present simultaneously. This is why I would very much like to
> >> >> have that *whole* abstract KMS pipeline documented with *everything*. Otherwise
> >> >> it is coming really hard fast to figure out how generic userspace should use all these
> >> >> KMS properties together.
> >> >>
> >> >> Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long
> >> >> as they are documented and how userspace will know which pipeline it is dealing
> >> >> with, and what things are mutually exclusive so we can avoid writing userspace code
> >> >> for combinations that will never exist.  
> >> >
> >> > This is a good suggestion to have the whole pipeline and properties documented along with
> >> > the exact usages. We may end with 2 properties almost doing similar work but needed due to
> >> > underlying hardware, but we can get that properly documented and defined.
> >> >
> >> > I will discuss with Harry and Ville as well to define this.
> >> >  
> >> 
> >> Just wanted to let you know that I've seen and read through both of 
> >> Shankar's patchsets
> >> and had some thoughts but haven't found the time to respond. I will 
> >> respond soon.  
> > 
> > Hi Harry,
> > 
> > awesome!
> >   
> >> I very much agree with Pekka. We need to make sure this all plays well 
> >> together and is
> >> well documented. Maybe a library to deal with DRM KMS color 
> >> management/HDR would even
> >> be helpful. Not sure yet how I feel about that.  
> > 
> > That is an excellent question. While I am working on Weston CM&HDR, I
> > already have issues with how to represent the color related
> > transformations. These new hardware features exposed here are nothing I
> > have prepared for, and would probably need changes to accommodate.
> > 
> > The main Weston roadmap is drafted in
> > https://gitlab.freedesktop.org/wayland/weston/-/issues/467
> > 
> > The MR that introduces the concept of a color transformation, and also
> > the whole beginnings of color management, is
> > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582
> > 
> > In that MR, there is a patch introducing struct weston_color_transform:
> > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?commit_id=cffbf7c6b2faf7391b73ff9202774f660343bd34#ba0b86259533d5000d81c9c88109c9010eb0f641_0_77
> > 
> > The design idea there is that libweston shall have what I call "color
> > manager" module. That module handles all the policy decisions about
> > color, it uses a CMM (Little CMS 2 in this case) for all the color
> > profile computations, and based on all information it has available
> > from display EDID, ICC profile files, Wayland clients via the CM&HDR
> > protocol extension and more, it will ultimately produce
> > weston_color_transform objects.
> > 
> > weston_color_transform is a complete description of how to map a pixel
> > in one color model/space/encoding into another, maybe with user
> > preferred tuning/tone-mapping. E.g. from client content to the output's
> > blending space (output space but light-linear), or from output's
> > blending space to output's framebuffer space or maybe even monitor wire
> > space.
> > 
> > The mapping described by weston_color_transform shall be implemented by
> > libweston's GL-renderer or by the DRM-backend using KMS properties,
> > whatever works for each case. So the description cannot be opaque, it
> > has to map to GLSL shaders (easy) and KMS properties (???).
> > 
> > Now the problem is, what should weston_color_transform look like?
> > 
> > The current design has two steps in a color transform:
> > - Transfer function: identity, the traditional set of three 1D LUTs, or
> >   something else.
> > - Color mapping: identity, a 3D LUT, or something else.
> > 
> > "Something else" is a placeholder for whatever we want to have, but the
> > problem in adding new types of transfer function or color mapping
> > representations (e.g. the fancy new GAMMA_MODEs) is how will the color
> > manager create the parameters for those?  
> 
> I think the weston_color_transform is going a bit in the wrong
> direction. While the 3D LUT can describe everything if it has enough
> precision it indeed makes sense to apply a transform before to get the
> required precision down. It doesn't have to be a TF though and we really
> don't care what it is as long as in the end the content is in the
> correct color space and dynamic range. This might be enough to get
> something off the ground right now though.

Ok, please leave these comments in
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582 and
propose something? A new name for what I now called "transfer
function"? It is desperately needing comments.

> In the long run however it probably makes more sense to convert the
> color transform to a complete pipeline of enumerated, parametric and
> numerical elements together with some helpers to lower (enumerated >
> parametric > numerical) and fuse elements (to the point that you can
> always convert the pipeline to a 3D LUT). The color manager ideally
> should provide a pipeline with the highest abstraction and avoid fusing
> elements if it would result in a lose of information. This is a lot more
> complex but it also gives us much better chances of finding a way to
> offload the transform.
> 
> AFAIR lcms uses such a model and gives you access to the pipeline. If we
> want to be independent of lcms we would need our own descriptions and
> possibly lower some lcms elements to our own stuff. I'm also not sure
> how good lcms is at retaining the high level description if possible.

Yes, I think LCMS and even ICC use such a model. Access to the pipeline
in LCMS I don't know about yet. You can construct, but inspect? I also
think you can plug custom code in LCMS into the pipeline, so it can be
totally arbitrary and also opaque.

Using our own description is what I'm doing, since I am keeping LCMS
contained in the color manager plugin.

Lowering that into what can fit in KMS properties though is a huge
problem. It can reduce precision, and how much precision are you good
to lose? That's a policy decision, and I'd like to contain policy in
color manager.

I would rather model weston_color_transform based on what KMS can do
than based on what CMM can do. That way the hard problems are left for
the color manager, while the rest of libweston can just implement the
model directly as much as it fits e.g. in KMS.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
  2021-06-02 20:26     ` Shankar, Uma
@ 2021-06-04 15:23       ` Harry Wentland
  2021-06-07 17:19         ` Shankar, Uma
  0 siblings, 1 reply; 38+ messages in thread
From: Harry Wentland @ 2021-06-04 15:23 UTC (permalink / raw)
  To: Shankar, Uma, Pekka Paalanen; +Cc: intel-gfx, dri-devel, Modem, Bhanuprakash

On 2021-06-02 4:26 p.m., Shankar, Uma wrote:
> 
> 
>> -----Original Message-----
>> From: Pekka Paalanen <ppaalanen@gmail.com>
>> Sent: Wednesday, June 2, 2021 3:04 PM
>> To: Shankar, Uma <uma.shankar@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
>> Bhanuprakash <bhanuprakash.modem@intel.com>
>> Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range
>> attributes
>>
>> On Tue,  1 Jun 2021 16:21:58 +0530
>> Uma Shankar <uma.shankar@intel.com> 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.
>>>
>>> This also defines a new structure to define color lut ranges, along
>>> with related macro definitions and enums. This will help describe
>>> multi segmented lut ranges in the hardware.
>>>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> ---
>>>  include/uapi/drm/drm_mode.h | 58
>>> +++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 58 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>> index 9b6722d45f36..d0ce48d2e732 100644
>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -819,6 +819,64 @@ struct hdr_output_metadata {
>>>  	};
>>>  };
>>>
>>> +/*
>>> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means
>> the LUT
>>> + * can be used for either purpose, but not simultaneously. To expose
>>> + * modes that support gamma and degamma simultaneously the gamma mode
>>> + * must declare distinct DRM_MODE_LUT_GAMMA and
>> DRM_MODE_LUT_DEGAMMA
>>> + * ranges.
>>> + */
>>> +/* LUT is for gamma (after CTM) */
>>> +#define DRM_MODE_LUT_GAMMA BIT(0)
>>> +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA
>>> +BIT(1)
>>> +/* linearly interpolate between the points */ #define
>>> +DRM_MODE_LUT_INTERPOLATE BIT(2)
>>> +/*
>>> + * the last value of the previous range is the
>>> + * first value of the current range.
>>> + */
>>> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
>>> +/* the curve must be non-decreasing */ #define
>>> +DRM_MODE_LUT_NON_DECREASING BIT(4)
>>> +/* the curve is reflected across origin for negative inputs */
>>> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
>>> +/* the same curve (red) is used for blue and green channels as well
>>> +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
>>> +
>>> +struct drm_color_lut_range {
>>> +	/* DRM_MODE_LUT_* */
>>> +	__u32 flags;
>>> +	/* number of points on the curve */
>>> +	__u16 count;
>>> +	/* input/output bits per component */
>>> +	__u8 input_bpc, output_bpc;
>>> +	/* input start/end values */
>>> +	__s32 start, end;
>>> +	/* output min/max values */
>>> +	__s32 min, max;
>>> +};
>>> +
>>> +enum lut_type {
>>
>> Unprefixed type name in UAPI headers is probably not a good idea.
> 
> Ok, will rename these.
> 
>>> +	LUT_TYPE_DEGAMMA = 0,
>>> +	LUT_TYPE_GAMMA = 1,
>>> +};
>>
>> All the above stuff seems to be the same in your other patch series'
>> patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the
>> series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what
>> does this mean?
> 
> The concept and idea is similar and the range definition is also common. But this series
> focuses on plane color management while the other one is for pipe/crtc color features.
> Hence separated and floated them as unique series for review.
> 

Might be better in this case to combine both patchsets. It wasn't entirely clear to
me whether I could base one patchset on top of the other (doesn't look like it) and
what base to apply them on. I had success applying the plane stuff on drm-next and
the crtc stuff on drm-intel.

Harry

> Regards,
> Uma Shankar
>>
>> Thanks,
>> pq
>>
>>> +
>>> +/*
>>> + * Creating 64 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 U32.32 fixed point format.
>>> +	 */
>>> +	__u64 red;
>>> +	__u64 green;
>>> +	__u64 blue;
>>> +	__u64 reserved;
>>> +};
>>> +
>>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
>>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
>>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
  2021-06-01 10:51 ` [PATCH 02/21] drm: Add Plane Degamma Mode property Uma Shankar
@ 2021-06-04 18:24   ` Harry Wentland
  2021-06-07 11:00     ` Pekka Paalanen
  2021-06-07 17:34     ` Shankar, Uma
  0 siblings, 2 replies; 38+ messages in thread
From: Harry Wentland @ 2021-06-04 18:24 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx, dri-devel; +Cc: bhanuprakash.modem

On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> Add Plane Degamma Mode as an enum property. Create a helper
> function for all plane color management features.
> 
> This is an enum property with values as blob_id's and exposes
> the various gamma modes supported and the lut ranges. Getting
> the blob id in userspace, user can get the mode supported and
> also the range of gamma mode supported with number of lut
> coefficients. It can then set one of the modes using this
> enum property.
> 
> Lut values will be sent through separate GAMMA_LUT blob property.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c              |  1 +
>  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
>  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
>  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
>  include/drm/drm_mode_object.h             |  2 +-
>  include/drm/drm_plane.h                   | 23 ++++++
>  7 files changed, 212 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 87e5023e3f55..752be545e7d7 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -514,9 +514,99 @@ Damage Tracking 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"
> +      }
> +

It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
between the FB and Plane degamma, i.e.
  YCbCr coefficients > plane degamma > csc > ...

One problem with this view is that not all HW will support all (or any) of these
CM blocks on all planes. For example, on AMD HW cursors are very different from
other planes and don't really have full CM support.

> +      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.
> +

This would mean you're blending in gamma space which is likely not what
most compositors expect. There are numerous articles that describe why
blending in gamma space is problematic, such as [1]

[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html

To blend in linear space this should be configured to do

  Plane Degamma > Plane CTM > CRTC Gamma

I think it would also be good if we moved away from calling this gamma. It's
really only gamma for legacy SDR scenarios. For HDR cases I would never expect
these to use gamma and even though the sRGB transfer function is based on gamma
functions it's complicated [2].

[2] https://en.wikipedia.org/wiki/SRGB

A better way to describe these would be as "transfer function" and "inverse
transfer function." The space at various stages could then be described as linear
or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer
function.

Harry

> +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
>  
> +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 a8bbb021684b..8892d03602f7 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -708,6 +708,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_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index ddcf5c2c8e6a..f26b03853711 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  	state->fence = NULL;
>  	state->commit = NULL;
>  	state->fb_damage_clips = NULL;
> +
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 438e9585b225..40fa05fa33dc 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -595,6 +595,8 @@ 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_mode_property) {
> +		state->degamma_mode = val;
>  	} else if (property == config->prop_fb_damage_clips) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->fb_damage_clips,
> @@ -661,6 +663,8 @@ drm_atomic_plane_get_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_mode_property) {
> +		*val = state->degamma_mode;
>  	} else if (property == config->prop_fb_damage_clips) {
>  		*val = (state->fb_damage_clips) ?
>  			state->fb_damage_clips->base.id : 0;
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index bb14f488c8f6..085ed0d0db00 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -34,8 +34,8 @@
>  /**
>   * 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”:
> @@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  }
>  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_mode_property:
> + *     Blob property which advertizes the possible degamma modes and
> + *     lut ranges supported by the platform. This  allows userspace
> + *     to query and get the plane degamma color caps and choose the
> + *     appropriate degamma mode and create lut values accordingly
> + *
> + */
> +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> +					   struct drm_plane *plane,
> +					   int num_values)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
> +				   "PLANE_DEGAMMA_MODE", num_values);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	plane->degamma_mode_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
> +
> +void drm_plane_attach_degamma_properties(struct drm_plane *plane)
> +{
> +	if (!plane->degamma_mode_property)
> +		return;
> +
> +	drm_object_attach_property(&plane->base,
> +				   plane->degamma_mode_property, 0);
> +}
> +EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
> +
> +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
> +						 const char *name,
> +						 const struct drm_color_lut_range *ranges,
> +						 size_t length, enum lut_type type)
> +{
> +	struct drm_property_blob *blob;
> +	struct drm_property *prop = NULL;
> +	int num_ranges = length / sizeof(ranges[0]);
> +	int i, ret, num_types_0;
> +
> +	if (type == LUT_TYPE_DEGAMMA)
> +		prop = plane->degamma_mode_property;
> +
> +	if (!prop)
> +		return -EINVAL;
> +
> +	if (length == 0 && name)
> +		return drm_property_add_enum(prop, 0, name);
> +
> +	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> +		return -EINVAL;
> +	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> +			       DRM_MODE_LUT_DEGAMMA));
> +	if (num_types_0 == 0)
> +		return -EINVAL;
> +
> +	for (i = 1; i < num_ranges; i++) {
> +		int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
> +					 DRM_MODE_LUT_DEGAMMA));
> +
> +		/* either all ranges have DEGAMMA|GAMMA or none have it */
> +		if (num_types_0 != num_types)
> +			return -EINVAL;
> +	}
> +
> +	blob = drm_property_create_blob(plane->dev, length, ranges);
> +	if (IS_ERR(blob))
> +		return PTR_ERR(blob);
> +
> +	ret = drm_property_add_enum(prop, blob->base.id, name);
> +	if (ret) {
> +		drm_property_blob_put(blob);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
> +
>  /**
>   * drm_color_lut_check - check validity of lookup table
>   * @lut: property blob containing LUT to check
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c34a3e8030e1..d4128c7daa08 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -60,7 +60,7 @@ struct drm_mode_object {
>  	void (*free_cb)(struct kref *kref);
>  };
>  
> -#define DRM_OBJECT_MAX_PROPERTY 24
> +#define DRM_OBJECT_MAX_PROPERTY 26
>  /**
>   * struct drm_object_properties - property tracking for &drm_mode_object
>   */
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 1294610e84f4..e476a5939f8e 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -236,6 +236,15 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	/**
> +	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
> +	 * wrt to various gamma modes and the respective lut ranges. This also
> +	 * helps user select a degamma mode amongst the supported ones.
> +	 */
> +	u32 degamma_mode;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -747,6 +756,12 @@ struct drm_plane {
>  	 * scaling.
>  	 */
>  	struct drm_property *scaling_filter_property;
> +
> +	/**
> +	 * @degamma_mode_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_mode_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -838,6 +853,14 @@ void drm_plane_force_disable(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_create_color_mgmt_properties(struct drm_device *dev,
> +					   struct drm_plane *plane,
> +					   int num_values);
> +void drm_plane_attach_degamma_properties(struct drm_plane *plane);
> +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
> +						 const char *name,
> +						 const struct drm_color_lut_range *ranges,
> +						 size_t length, enum lut_type type);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
  2021-06-04 18:24   ` Harry Wentland
@ 2021-06-07 11:00     ` Pekka Paalanen
  2021-06-07 17:34     ` Shankar, Uma
  1 sibling, 0 replies; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-07 11:00 UTC (permalink / raw)
  To: Harry Wentland
  Cc: intel-gfx, dri-devel, Sebastian Wick, Uma Shankar,
	Vitaly Prosyak, bhanuprakash.modem

[-- Attachment #1: Type: text/plain, Size: 6782 bytes --]

On Fri, 4 Jun 2021 14:24:28 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper
> > function for all plane color management features.
> > 
> > This is an enum property with values as blob_id's and exposes
> > the various gamma modes supported and the lut ranges. Getting
> > the blob id in userspace, user can get the mode supported and
> > also the range of gamma mode supported with number of lut
> > coefficients. It can then set one of the modes using this
> > enum property.
> > 
> > Lut values will be sent through separate GAMMA_LUT blob property.
> > 
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> >  drivers/gpu/drm/drm_atomic.c              |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> >  include/drm/drm_mode_object.h             |  2 +-
> >  include/drm/drm_plane.h                   | 23 ++++++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 87e5023e3f55..752be545e7d7 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking 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"
> > +      }
> > +  
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of these
> CM blocks on all planes. For example, on AMD HW cursors are very different from
> other planes and don't really have full CM support.
> 
> > +      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.
> > +  
> 
> This would mean you're blending in gamma space which is likely not what
> most compositors expect. There are numerous articles that describe why
> blending in gamma space is problematic, such as [1]
> 
> [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html
> 
> To blend in linear space this should be configured to do
> 
>   Plane Degamma > Plane CTM > CRTC Gamma
> 
> I think it would also be good if we moved away from calling this gamma. It's
> really only gamma for legacy SDR scenarios. For HDR cases I would never expect
> these to use gamma and even though the sRGB transfer function is based on gamma
> functions it's complicated [2].
> 
> [2] https://en.wikipedia.org/wiki/SRGB
> 
> A better way to describe these would be as "transfer function" and "inverse
> transfer function." The space at various stages could then be described as linear
> or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer
> function.

Hi,

incidentally, I, Sebastian and Vitaly are discussing on what to call
these pieces in Weston:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582#note_947292

They might not even be the transfer functions but just some curves,
e.g. optimized to keep a 3D LUT small without sacrificing precision.


Thanks,
pq

> > +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
> >  
> > +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
> >  -------------------

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
  2021-06-04 15:23       ` Harry Wentland
@ 2021-06-07 17:19         ` Shankar, Uma
  0 siblings, 0 replies; 38+ messages in thread
From: Shankar, Uma @ 2021-06-07 17:19 UTC (permalink / raw)
  To: Harry Wentland, Pekka Paalanen; +Cc: intel-gfx, dri-devel, Modem,  Bhanuprakash



> -----Original Message-----
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Friday, June 4, 2021 8:53 PM
> To: Shankar, Uma <uma.shankar@intel.com>; Pekka Paalanen
> <ppaalanen@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem,
> Bhanuprakash <bhanuprakash.modem@intel.com>
> Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range
> attributes
> 
> On 2021-06-02 4:26 p.m., Shankar, Uma wrote:
> >
> >
> >> -----Original Message-----
> >> From: Pekka Paalanen <ppaalanen@gmail.com>
> >> Sent: Wednesday, June 2, 2021 3:04 PM
> >> To: Shankar, Uma <uma.shankar@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> >> Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> >> Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut
> >> range attributes
> >>
> >> On Tue,  1 Jun 2021 16:21:58 +0530
> >> Uma Shankar <uma.shankar@intel.com> 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.
> >>>
> >>> This also defines a new structure to define color lut ranges, along
> >>> with related macro definitions and enums. This will help describe
> >>> multi segmented lut ranges in the hardware.
> >>>
> >>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >>> ---
> >>>  include/uapi/drm/drm_mode.h | 58
> >>> +++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 58 insertions(+)
> >>>
> >>> diff --git a/include/uapi/drm/drm_mode.h
> >>> b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732
> >>> 100644
> >>> --- a/include/uapi/drm/drm_mode.h
> >>> +++ b/include/uapi/drm/drm_mode.h
> >>> @@ -819,6 +819,64 @@ struct hdr_output_metadata {
> >>>  	};
> >>>  };
> >>>
> >>> +/*
> >>> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and
> means
> >> the LUT
> >>> + * can be used for either purpose, but not simultaneously. To
> >>> + expose
> >>> + * modes that support gamma and degamma simultaneously the gamma
> >>> + mode
> >>> + * must declare distinct DRM_MODE_LUT_GAMMA and
> >> DRM_MODE_LUT_DEGAMMA
> >>> + * ranges.
> >>> + */
> >>> +/* LUT is for gamma (after CTM) */
> >>> +#define DRM_MODE_LUT_GAMMA BIT(0)
> >>> +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA
> >>> +BIT(1)
> >>> +/* linearly interpolate between the points */ #define
> >>> +DRM_MODE_LUT_INTERPOLATE BIT(2)
> >>> +/*
> >>> + * the last value of the previous range is the
> >>> + * first value of the current range.
> >>> + */
> >>> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> >>> +/* the curve must be non-decreasing */ #define
> >>> +DRM_MODE_LUT_NON_DECREASING BIT(4)
> >>> +/* the curve is reflected across origin for negative inputs */
> >>> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> >>> +/* the same curve (red) is used for blue and green channels as well
> >>> +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> >>> +
> >>> +struct drm_color_lut_range {
> >>> +	/* DRM_MODE_LUT_* */
> >>> +	__u32 flags;
> >>> +	/* number of points on the curve */
> >>> +	__u16 count;
> >>> +	/* input/output bits per component */
> >>> +	__u8 input_bpc, output_bpc;
> >>> +	/* input start/end values */
> >>> +	__s32 start, end;
> >>> +	/* output min/max values */
> >>> +	__s32 min, max;
> >>> +};
> >>> +
> >>> +enum lut_type {
> >>
> >> Unprefixed type name in UAPI headers is probably not a good idea.
> >
> > Ok, will rename these.
> >
> >>> +	LUT_TYPE_DEGAMMA = 0,
> >>> +	LUT_TYPE_GAMMA = 1,
> >>> +};
> >>
> >> All the above stuff seems to be the same in your other patch series'
> >> patch "[PATCH 1/9] drm: Add gamma mode property". Is this series
> >> replacing the series "[PATCH 0/9] Enhance pipe color support for
> >> multi segmented luts" or what does this mean?
> >
> > The concept and idea is similar and the range definition is also
> > common. But this series focuses on plane color management while the other one
> is for pipe/crtc color features.
> > Hence separated and floated them as unique series for review.
> >
> 
> Might be better in this case to combine both patchsets. It wasn't entirely clear to me
> whether I could base one patchset on top of the other (doesn't look like it) and what
> base to apply them on. I had success applying the plane stuff on drm-next and the
> crtc stuff on drm-intel.

Sure Harry, I guess its better to combine both the series to avoid any confusions.
I will send out next version with some more UAPI documentation based on the feedback
received.

Regards,
Uma Shankar
> 
> Harry
> 
> > Regards,
> > Uma Shankar
> >>
> >> Thanks,
> >> pq
> >>
> >>> +
> >>> +/*
> >>> + * Creating 64 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 U32.32 fixed point format.
> >>> +	 */
> >>> +	__u64 red;
> >>> +	__u64 green;
> >>> +	__u64 blue;
> >>> +	__u64 reserved;
> >>> +};
> >>> +
> >>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
> >>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
> >>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> >


^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH 02/21] drm: Add Plane Degamma Mode property
  2021-06-04 18:24   ` Harry Wentland
  2021-06-07 11:00     ` Pekka Paalanen
@ 2021-06-07 17:34     ` Shankar, Uma
  2021-06-08  8:34       ` Pekka Paalanen
  1 sibling, 1 reply; 38+ messages in thread
From: Shankar, Uma @ 2021-06-07 17:34 UTC (permalink / raw)
  To: Harry Wentland, intel-gfx, dri-devel; +Cc: Modem, Bhanuprakash



> -----Original Message-----
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Friday, June 4, 2021 11:54 PM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Cyr, Aric
> <Aric.Cyr@amd.com>
> Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> 
> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper function
> > for all plane color management features.
> >
> > This is an enum property with values as blob_id's and exposes the
> > various gamma modes supported and the lut ranges. Getting the blob id
> > in userspace, user can get the mode supported and also the range of
> > gamma mode supported with number of lut coefficients. It can then set
> > one of the modes using this enum property.
> >
> > Lut values will be sent through separate GAMMA_LUT blob property.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> >  drivers/gpu/drm/drm_atomic.c              |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> >  include/drm/drm_mode_object.h             |  2 +-
> >  include/drm/drm_plane.h                   | 23 ++++++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-kms.rst
> > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking 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"
> > +      }
> > +
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of these CM
> blocks on all planes. For example, on AMD HW cursors are very different from other
> planes and don't really have full CM support.
> 
> > +      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.
> > +
> 
> This would mean you're blending in gamma space which is likely not what most
> compositors expect. There are numerous articles that describe why blending in
> gamma space is problematic, such as [1]
> 
> [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-
> blend.html
> 
> To blend in linear space this should be configured to do
> 
>   Plane Degamma > Plane CTM > CRTC Gamma
> 
> I think it would also be good if we moved away from calling this gamma. It's really
> only gamma for legacy SDR scenarios. For HDR cases I would never expect these to
> use gamma and even though the sRGB transfer function is based on gamma
> functions it's complicated [2].
> 
> [2] https://en.wikipedia.org/wiki/SRGB
> 
> A better way to describe these would be as "transfer function" and "inverse transfer
> function." The space at various stages could then be described as linear or non-
> linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.

Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we
can rename these to have them named generically, as gamma lut actually can be used for
Tone mapping as well and its not just meant for applying a fixed non liner curve only. However
naming based on transfer function is also not optimum as Tone mapping is not related to transfer
function. May be we can name them as:
linear_lut and nl_tone_mapper_lut.

Other ideas for names are welcome.

Regards,
Uma Shankar


> Harry
> 
> > +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
> >
> > +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 a8bbb021684b..8892d03602f7 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -708,6 +708,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_state_helper.c
> > b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index ddcf5c2c8e6a..f26b03853711 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> >  	state->fence = NULL;
> >  	state->commit = NULL;
> >  	state->fb_damage_clips = NULL;
> > +
> > +	state->color_mgmt_changed = false;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 438e9585b225..40fa05fa33dc 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -595,6 +595,8 @@ 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_mode_property) {
> > +		state->degamma_mode = val;
> >  	} else if (property == config->prop_fb_damage_clips) {
> >  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >  					&state->fb_damage_clips,
> > @@ -661,6 +663,8 @@ drm_atomic_plane_get_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_mode_property) {
> > +		*val = state->degamma_mode;
> >  	} else if (property == config->prop_fb_damage_clips) {
> >  		*val = (state->fb_damage_clips) ?
> >  			state->fb_damage_clips->base.id : 0; diff --git
> > a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index bb14f488c8f6..085ed0d0db00 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -34,8 +34,8 @@
> >  /**
> >   * 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”:
> > @@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct
> > drm_plane *plane,  }
> > 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_mode_property:
> > + *     Blob property which advertizes the possible degamma modes and
> > + *     lut ranges supported by the platform. This  allows userspace
> > + *     to query and get the plane degamma color caps and choose the
> > + *     appropriate degamma mode and create lut values accordingly
> > + *
> > + */
> > +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> > +					   struct drm_plane *plane,
> > +					   int num_values)
> > +{
> > +	struct drm_property *prop;
> > +
> > +	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
> > +				   "PLANE_DEGAMMA_MODE", num_values);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	plane->degamma_mode_property = prop;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
> > +
> > +void drm_plane_attach_degamma_properties(struct drm_plane *plane) {
> > +	if (!plane->degamma_mode_property)
> > +		return;
> > +
> > +	drm_object_attach_property(&plane->base,
> > +				   plane->degamma_mode_property, 0); }
> > +EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
> > +
> > +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
> *plane,
> > +						 const char *name,
> > +						 const struct drm_color_lut_range
> *ranges,
> > +						 size_t length, enum lut_type type)
> {
> > +	struct drm_property_blob *blob;
> > +	struct drm_property *prop = NULL;
> > +	int num_ranges = length / sizeof(ranges[0]);
> > +	int i, ret, num_types_0;
> > +
> > +	if (type == LUT_TYPE_DEGAMMA)
> > +		prop = plane->degamma_mode_property;
> > +
> > +	if (!prop)
> > +		return -EINVAL;
> > +
> > +	if (length == 0 && name)
> > +		return drm_property_add_enum(prop, 0, name);
> > +
> > +	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> > +		return -EINVAL;
> > +	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> > +			       DRM_MODE_LUT_DEGAMMA));
> > +	if (num_types_0 == 0)
> > +		return -EINVAL;
> > +
> > +	for (i = 1; i < num_ranges; i++) {
> > +		int num_types = hweight8(ranges[i].flags &
> (DRM_MODE_LUT_GAMMA |
> > +					 DRM_MODE_LUT_DEGAMMA));
> > +
> > +		/* either all ranges have DEGAMMA|GAMMA or none have it */
> > +		if (num_types_0 != num_types)
> > +			return -EINVAL;
> > +	}
> > +
> > +	blob = drm_property_create_blob(plane->dev, length, ranges);
> > +	if (IS_ERR(blob))
> > +		return PTR_ERR(blob);
> > +
> > +	ret = drm_property_add_enum(prop, blob->base.id, name);
> > +	if (ret) {
> > +		drm_property_blob_put(blob);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
> > +
> >  /**
> >   * drm_color_lut_check - check validity of lookup table
> >   * @lut: property blob containing LUT to check diff --git
> > a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index
> > c34a3e8030e1..d4128c7daa08 100644
> > --- a/include/drm/drm_mode_object.h
> > +++ b/include/drm/drm_mode_object.h
> > @@ -60,7 +60,7 @@ struct drm_mode_object {
> >  	void (*free_cb)(struct kref *kref);
> >  };
> >
> > -#define DRM_OBJECT_MAX_PROPERTY 24
> > +#define DRM_OBJECT_MAX_PROPERTY 26
> >  /**
> >   * struct drm_object_properties - property tracking for &drm_mode_object
> >   */
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> > 1294610e84f4..e476a5939f8e 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -236,6 +236,15 @@ struct drm_plane_state {
> >
> >  	/** @state: backpointer to global drm_atomic_state */
> >  	struct drm_atomic_state *state;
> > +
> > +	/**
> > +	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
> > +	 * wrt to various gamma modes and the respective lut ranges. This also
> > +	 * helps user select a degamma mode amongst the supported ones.
> > +	 */
> > +	u32 degamma_mode;
> > +
> > +	u8 color_mgmt_changed : 1;
> >  };
> >
> >  static inline struct drm_rect
> > @@ -747,6 +756,12 @@ struct drm_plane {
> >  	 * scaling.
> >  	 */
> >  	struct drm_property *scaling_filter_property;
> > +
> > +	/**
> > +	 * @degamma_mode_property: Optional Plane property to set the LUT
> > +	 * used to convert the framebuffer's colors to linear gamma.
> > +	 */
> > +	struct drm_property *degamma_mode_property;
> >  };
> >
> >  #define obj_to_plane(x) container_of(x, struct drm_plane, base) @@
> > -838,6 +853,14 @@ void drm_plane_force_disable(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_create_color_mgmt_properties(struct drm_device *dev,
> > +					   struct drm_plane *plane,
> > +					   int num_values);
> > +void drm_plane_attach_degamma_properties(struct drm_plane *plane);
> > +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
> *plane,
> > +						 const char *name,
> > +						 const struct drm_color_lut_range
> *ranges,
> > +						 size_t length, enum lut_type type);
> >
> >  /**
> >   * drm_plane_find - find a &drm_plane
> >


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
  2021-06-07 17:34     ` Shankar, Uma
@ 2021-06-08  8:34       ` Pekka Paalanen
  0 siblings, 0 replies; 38+ messages in thread
From: Pekka Paalanen @ 2021-06-08  8:34 UTC (permalink / raw)
  To: Shankar, Uma, Harry Wentland
  Cc: intel-gfx, Vitaly Prosyak, Sebastian Wick, dri-devel, Modem,
	Bhanuprakash

[-- Attachment #1: Type: text/plain, Size: 9881 bytes --]

On Mon, 7 Jun 2021 17:34:06 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Harry Wentland <harry.wentland@amd.com>
> > Sent: Friday, June 4, 2021 11:54 PM
> > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org
> > Cc: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Cyr, Aric
> > <Aric.Cyr@amd.com>
> > Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> > 
> > On 2021-06-01 6:51 a.m., Uma Shankar wrote:  
> > > Add Plane Degamma Mode as an enum property. Create a helper function
> > > for all plane color management features.
> > >
> > > This is an enum property with values as blob_id's and exposes the
> > > various gamma modes supported and the lut ranges. Getting the blob id
> > > in userspace, user can get the mode supported and also the range of
> > > gamma mode supported with number of lut coefficients. It can then set
> > > one of the modes using this enum property.
> > >
> > > Lut values will be sent through separate GAMMA_LUT blob property.
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > ---
> > >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> > >  drivers/gpu/drm/drm_atomic.c              |  1 +
> > >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> > >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> > >  include/drm/drm_mode_object.h             |  2 +-
> > >  include/drm/drm_plane.h                   | 23 ++++++
> > >  7 files changed, 212 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/drm-kms.rst
> > > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > > 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -514,9 +514,99 @@ Damage Tracking 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"
> > > +      }
> > > +  
> > 
> > It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> > between the FB and Plane degamma, i.e.
> >   YCbCr coefficients > plane degamma > csc > ...
> > 
> > One problem with this view is that not all HW will support all (or any) of these CM
> > blocks on all planes. For example, on AMD HW cursors are very different from other
> > planes and don't really have full CM support.
> >   
> > > +      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.
> > > +  
> > 
> > This would mean you're blending in gamma space which is likely not what most
> > compositors expect. There are numerous articles that describe why blending in
> > gamma space is problematic, such as [1]
> > 
> > [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-
> > blend.html
> > 
> > To blend in linear space this should be configured to do
> > 
> >   Plane Degamma > Plane CTM > CRTC Gamma
> > 
> > I think it would also be good if we moved away from calling this gamma. It's really
> > only gamma for legacy SDR scenarios. For HDR cases I would never expect these to
> > use gamma and even though the sRGB transfer function is based on gamma
> > functions it's complicated [2].
> > 
> > [2] https://en.wikipedia.org/wiki/SRGB
> > 
> > A better way to describe these would be as "transfer function" and "inverse transfer
> > function." The space at various stages could then be described as linear or non-
> > linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.  
> 
> Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we
> can rename these to have them named generically, as gamma lut actually can be used for
> Tone mapping as well and its not just meant for applying a fixed non liner curve only. However
> naming based on transfer function is also not optimum as Tone mapping is not related to transfer
> function. May be we can name them as:
> linear_lut and nl_tone_mapper_lut.
> 
> Other ideas for names are welcome.

I would suggest to avoid any kind of naming that explicitly refers to
gamma, EOTF, EOTF^-1, tone map, or anything else that has a specific
meaning. Likewise, the diagram should not claim that the pixel data at
certain stages of the pipeline is in certain form, e.g. linear,
non-linear, or in certain color space. Doing that would semantically
restrict what userspace can do. After all, kernel UAPI design is
mechanism, not policy, right?

However, to make the pipeline understandable, the documentation could
show example configurations, where you *can* say that this LUT
implements EOTF, pixels are light-linear at that point, the color space
at another point is XXX, and so on.

What exactly the LUT is, are pixel values linear or non-linear, and
what the color space is at any step are something that userspace will
define through providing the parameters for each processing step in the
pipeline.

Unless, your hardware has been designed for a certain policy and
deviating from that policy should be discouraged?

So what to name these:

That's the question we are struggling with at Weston too. You can seek
inspiration from CMM data structures (e.g. Little CMS 2) or from ICC
specifications. Just need to be careful to not pick a term that has
more meaning than kernel UAPI wants.

I personally like the terms "color curve" for 1D LUT -like things, and
"color mapping" for matrix or 3D LUT -like things. I'm using the term
"color transformation" for going from one encoding, color space, gamut,
and dynamic range to another with a specific render intent, which may
require a sequence of several color curves and color mappings to
achieve. But that's just me.

Why bother with all this indirection:

Use cases that want blending in non-linear spaces are one thing, but
probably less useful with HDR. Instead, I would imagine that doing
numerical optimization on a complete pipeline to achieve the best
precision given hardware limitations or minimizing memory consumption
or other, may result in different curves and mapping tables than simply
plugging in the EOTFs and CSCs into their intended places.

I have actually started to think that when Weston's DRM-backend is
trying to fit a window on a KMS plane, it would need to get the color
pipeline capabilities of that specific plane and ask the color manager
component to optimize the color transformation against the exact KMS
capabilities, evaluating whether a sufficient precision can be
achieved. Then the color manager gives back a description of how the
KMS plane would need to be configured, and the DRM-backend continues
with an atomic TEST_ONLY commit to see if it would work. The color
manager would quite likely delegate that optimization to a CMM, like
Little CMS 2 if possible. But this is all still so much hand-waving and
the KMS UAPI is not really there yet, that I will mostly just ignore
this idea for now and do something much simpler.

Or maybe all this optimization has already been done when designing the
new hardware, and we really do need to simply plug in the EOTFs and CSCs
without thinking?

Then again, if hardware changes every generation, that implies the
optimization is not complete, and there might be something to gain from
software optimization still. Old hardware always exists, too.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
  2021-06-01 10:52 ` [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes Uma Shankar
@ 2021-06-28 15:14   ` Harry Wentland
  2021-06-30 11:36     ` Shankar, Uma
  0 siblings, 1 reply; 38+ messages in thread
From: Harry Wentland @ 2021-06-28 15:14 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx, dri-devel; +Cc: bhanuprakash.modem

On 2021-06-01 6:52 a.m., Uma Shankar wrote:
> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> planes have different capabilities, implemented respective
> structure for the HDR planes.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index dab892d2251b..c735d06a6b54 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> + /* FIXME input bpc? */
> +__maybe_unused
> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> +	/* segment 1 */
> +	{
> +		.flags = (DRM_MODE_LUT_GAMMA |

Why are these using DRM_MODE_LUT_GAMMA and not DRM_MODE_LUT_DEGAMMA when
the lut_type for this LUT is LUT_TYPE_DEGAMMA?


> +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> +			  DRM_MODE_LUT_INTERPOLATE |
> +			  DRM_MODE_LUT_NON_DECREASING),
> +		.count = 128,
> +		.input_bpc = 24, .output_bpc = 16,

Why do we need more than 16 bpc for LUT? FP16 is enough to represent HDR in 
linear space. Wouldn't 16 bpc be enough?


> +		.start = 0, .end = (1 << 24) - 1,
> +		.min = 0, .max = (1 << 24) - 1,
> +	},
> +	/* segment 2 */
> +	{
> +		.flags = (DRM_MODE_LUT_GAMMA |
> +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> +			  DRM_MODE_LUT_INTERPOLATE |
> +			  DRM_MODE_LUT_REUSE_LAST |
> +			  DRM_MODE_LUT_NON_DECREASING),
> +		.count = 1,
> +		.input_bpc = 24, .output_bpc = 16,
> +		.start = (1 << 24) - 1, .end = 1 << 24,
> +		.min = 0, .max = (1 << 27) - 1,

How can max be 1 << 27 if input_bpc is 24?

Harry

> +	},
> +	/* Segment 3 */
> +	{
> +		.flags = (DRM_MODE_LUT_GAMMA |
> +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> +			  DRM_MODE_LUT_INTERPOLATE |
> +			  DRM_MODE_LUT_REUSE_LAST |
> +			  DRM_MODE_LUT_NON_DECREASING),
> +		.count = 1,
> +		.input_bpc = 24, .output_bpc = 16,
> +		.start = 1 << 24, .end = 3 << 24,
> +		.min = 0, .max = (1 << 27) - 1,
> +	},
> +	/* Segment 4 */
> +	{
> +		.flags = (DRM_MODE_LUT_GAMMA |
> +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> +			  DRM_MODE_LUT_INTERPOLATE |
> +			  DRM_MODE_LUT_REUSE_LAST |
> +			  DRM_MODE_LUT_NON_DECREASING),
> +		.count = 1,
> +		.input_bpc = 24, .output_bpc = 16,
> +		.start = 3 << 24, .end = 7 << 24,
> +		.min = 0, .max = (1 << 27) - 1,
> +	},
> +};
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
  2021-06-28 15:14   ` Harry Wentland
@ 2021-06-30 11:36     ` Shankar, Uma
  0 siblings, 0 replies; 38+ messages in thread
From: Shankar, Uma @ 2021-06-30 11:36 UTC (permalink / raw)
  To: Harry Wentland, intel-gfx, dri-devel; +Cc: Modem, Bhanuprakash



> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Harry
> Wentland
> Sent: Monday, June 28, 2021 8:45 PM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Subject: Re: [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for
> HDR planes
> 
> On 2021-06-01 6:52 a.m., Uma Shankar wrote:
> > Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > planes have different capabilities, implemented respective structure
> > for the HDR planes.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_color.c | 52
> > ++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index dab892d2251b..c735d06a6b54 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)
> >  	}
> >  }
> >
> > + /* FIXME input bpc? */
> > +__maybe_unused
> > +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > +	/* segment 1 */
> > +	{
> > +		.flags = (DRM_MODE_LUT_GAMMA |
> 
> Why are these using DRM_MODE_LUT_GAMMA and not
> DRM_MODE_LUT_DEGAMMA when the lut_type for this LUT is
> LUT_TYPE_DEGAMMA?

Thanks Harry for the comments.

Yeah this is an oversight, will fix this.
> 
> 
> > +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> > +			  DRM_MODE_LUT_INTERPOLATE |
> > +			  DRM_MODE_LUT_NON_DECREASING),
> > +		.count = 128,
> > +		.input_bpc = 24, .output_bpc = 16,
> 
> Why do we need more than 16 bpc for LUT? FP16 is enough to represent HDR in
> linear space. Wouldn't 16 bpc be enough?

Pipe sometimes works internally on higher precision (just to take care of rounding etc.), later the
extra data gets dropped at the end of the pipe. So from source side you are right, 16bpc is enough
but the lut precision can go higher.

> 
> > +		.start = 0, .end = (1 << 24) - 1,
> > +		.min = 0, .max = (1 << 24) - 1,
> > +	},
> > +	/* segment 2 */
> > +	{
> > +		.flags = (DRM_MODE_LUT_GAMMA |
> > +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> > +			  DRM_MODE_LUT_INTERPOLATE |
> > +			  DRM_MODE_LUT_REUSE_LAST |
> > +			  DRM_MODE_LUT_NON_DECREASING),
> > +		.count = 1,
> > +		.input_bpc = 24, .output_bpc = 16,
> > +		.start = (1 << 24) - 1, .end = 1 << 24,
> > +		.min = 0, .max = (1 << 27) - 1,
> 
> How can max be 1 << 27 if input_bpc is 24?

This is to take care of > 1.0 section. 1.0 to 3.0 and 3.0 to 7.0.
So we have 3.24 format for Lut to take care of this. 

Also, I have an action to update the series with UAPI doc and new naming for the property.
My apologies for being late on that one. Will update and send that out soon.

Thanks & Regards,
Uma Shankar
> 
> Harry
> 
> > +	},
> > +	/* Segment 3 */
> > +	{
> > +		.flags = (DRM_MODE_LUT_GAMMA |
> > +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> > +			  DRM_MODE_LUT_INTERPOLATE |
> > +			  DRM_MODE_LUT_REUSE_LAST |
> > +			  DRM_MODE_LUT_NON_DECREASING),
> > +		.count = 1,
> > +		.input_bpc = 24, .output_bpc = 16,
> > +		.start = 1 << 24, .end = 3 << 24,
> > +		.min = 0, .max = (1 << 27) - 1,
> > +	},
> > +	/* Segment 4 */
> > +	{
> > +		.flags = (DRM_MODE_LUT_GAMMA |
> > +			  DRM_MODE_LUT_REFLECT_NEGATIVE |
> > +			  DRM_MODE_LUT_INTERPOLATE |
> > +			  DRM_MODE_LUT_REUSE_LAST |
> > +			  DRM_MODE_LUT_NON_DECREASING),
> > +		.count = 1,
> > +		.input_bpc = 24, .output_bpc = 16,
> > +		.start = 3 << 24, .end = 7 << 24,
> > +		.min = 0, .max = (1 << 27) - 1,
> > +	},
> > +};
> > +
> >  void intel_color_init(struct intel_crtc *crtc)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >


^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2021-06-30 11:36 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 10:51 [PATCH 00/21] Add Support for Plane Color Lut and CSC features Uma Shankar
2021-06-01 10:51 ` [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
2021-06-02  9:33   ` Pekka Paalanen
2021-06-02 20:26     ` Shankar, Uma
2021-06-04 15:23       ` Harry Wentland
2021-06-07 17:19         ` Shankar, Uma
2021-06-01 10:51 ` [PATCH 02/21] drm: Add Plane Degamma Mode property Uma Shankar
2021-06-04 18:24   ` Harry Wentland
2021-06-07 11:00     ` Pekka Paalanen
2021-06-07 17:34     ` Shankar, Uma
2021-06-08  8:34       ` Pekka Paalanen
2021-06-01 10:52 ` [PATCH 03/21] drm: Add Plane Degamma Lut property Uma Shankar
2021-06-01 10:52 ` [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes Uma Shankar
2021-06-28 15:14   ` Harry Wentland
2021-06-30 11:36     ` Shankar, Uma
2021-06-01 10:52 ` [PATCH 05/21] drm/i915/xelpd: Add register definitions for Plane Degamma Uma Shankar
2021-06-01 10:52 ` [PATCH 06/21] drm/i915/xelpd: Enable plane color features Uma Shankar
2021-06-01 10:52 ` [PATCH 07/21] drm/i915/xelpd: Add color capabilities of SDR planes Uma Shankar
2021-06-01 10:52 ` [PATCH 08/21] drm/i915/xelpd: Program Plane Degamma Registers Uma Shankar
2021-06-01 10:52 ` [PATCH 09/21] drm/i915/xelpd: Add plane color check to glk_plane_color_ctl Uma Shankar
2021-06-01 10:52 ` [PATCH 10/21] drm/i915/xelpd: Initialize plane color features Uma Shankar
2021-06-01 10:52 ` [PATCH 11/21] drm/i915/xelpd: Load plane color luts from atomic flip Uma Shankar
2021-06-01 10:52 ` [PATCH 12/21] drm: Add Plane CTM property Uma Shankar
2021-06-01 10:52 ` [PATCH 13/21] drm: Add helper to attach Plane ctm property Uma Shankar
2021-06-01 10:52 ` [PATCH 14/21] drm/i915/xelpd: Define Plane CSC Registers Uma Shankar
2021-06-01 10:52 ` [PATCH 15/21] drm/i915/xelpd: Enable Plane CSC Uma Shankar
2021-06-01 10:52 ` [PATCH 16/21] drm: Add Plane Gamma Mode property Uma Shankar
2021-06-01 10:52 ` [PATCH 17/21] drm: Add Plane Gamma Lut property Uma Shankar
2021-06-01 10:52 ` [PATCH 18/21] drm/i915/xelpd: Define and Initialize Plane Gamma Lut range Uma Shankar
2021-06-01 10:52 ` [PATCH 19/21] drm/i915/xelpd: Add register definitions for Plane Gamma Uma Shankar
2021-06-01 10:52 ` [PATCH 20/21] drm/i915/xelpd: Program Plane Gamma Registers Uma Shankar
2021-06-01 10:52 ` [PATCH 21/21] drm/i915/xelpd: Enable plane gamma Uma Shankar
2021-06-02  9:28 ` [PATCH 00/21] Add Support for Plane Color Lut and CSC features Pekka Paalanen
2021-06-02 20:22   ` Shankar, Uma
2021-06-02 23:42     ` Harry Wentland
2021-06-03  8:47       ` Pekka Paalanen
2021-06-03 12:30         ` Sebastian Wick
2021-06-03 12:58           ` Pekka Paalanen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).