[2/6] drm: introduce color correction properties

Submitted by Lionel Landwerlin on Feb. 9, 2016, 12:19 p.m.

Details

Message ID 1455020358-18926-3-git-send-email-lionel.g.landwerlin@intel.com
State New
Headers show
Series "Pipe level color management" ( rev: 4 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Lionel Landwerlin Feb. 9, 2016, 12:19 p.m.
Patch based on a previous series by Shashank Sharma.

v2: Register LUT size properties as range

v3: Fix round in drm_color_lut_get_value() helper
    More docs on how degamma/gamma properties are used

v4: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 Documentation/DocBook/gpu.tmpl      |  58 +++++++++++++++++++-
 drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
 drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
 include/drm/drm_crtc.h              |  44 +++++++++++++++-
 include/drm/drm_crtc_helper.h       |   3 ++
 include/uapi/drm/drm_mode.h         |  15 ++++++
 8 files changed, 295 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index fe6b36a..7c49a92 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -1816,7 +1816,7 @@  void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >Description/Restrictions</td>
 	</tr>
 	<tr>
-	<td rowspan="37" valign="top" >DRM</td>
+	<td rowspan="42" valign="top" >DRM</td>
 	<td valign="top" >Generic</td>
 	<td valign="top" >“rotation”</td>
 	<td valign="top" >BITMASK</td>
@@ -2068,7 +2068,7 @@  void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >property to suggest an Y offset for a connector</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >Optional</td>
+	<td rowspan="8" valign="top" >Optional</td>
 	<td valign="top" >“scaling mode”</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
@@ -2092,6 +2092,60 @@  void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >“DEGAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the degamma LUT mapping
+		pixel data from the framebuffer before it is given to the
+		transformation matrix. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]). </td>
+	</tr>
+	<tr>
+	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the DEGAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“CTM_MATRIX”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the transformation
+		matrix apply to pixel data after the lookup through the
+		degamma LUT and before the lookup through the gamma LUT. The
+		data is an interpreted as a struct drm_color_ctm.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the gamma LUT mapping
+		pixel data after to the transformation matrix to data sent to
+		the connector. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the GAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c..7507762 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@ 
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
 
 /**
@@ -388,6 +389,58 @@  int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+				 struct drm_property_blob *new_blob,
+				 size_t expected_size,
+				 bool *replaced)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return 0;
+
+	if (new_blob != NULL && new_blob->length != expected_size)
+		return -EINVAL;
+
+	if (old_blob != NULL)
+		drm_property_unreference_blob(old_blob);
+	*blob = new_blob;
+	*replaced = true;
+
+	return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 size_t expected_size,
+					 bool *replaced)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+	}
+
+	return drm_atomic_replace_property_blob(blob, new_blob,
+						expected_size, replaced);
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -419,8 +472,47 @@  int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
 		drm_property_unreference_blob(mode);
 		return ret;
-	}
-	else if (crtc->funcs->atomic_set_property)
+	} else if (property == config->degamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->degamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->degamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->ctm_matrix_property) {
+		bool replaced = false;
+
+		ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->ctm_matrix,
+					val,
+					sizeof(struct drm_color_ctm),
+							       &replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->gamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->gamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->gamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
 		return -EINVAL;
@@ -456,6 +548,12 @@  drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->degamma_lut_property)
+		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
+	else if (property == config->ctm_matrix_property)
+		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2b430b0..d13dfd8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2513,10 +2513,17 @@  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 
 	if (state->mode_blob)
 		drm_property_reference_blob(state->mode_blob);
+	if (state->degamma_lut)
+		drm_property_reference_blob(state->degamma_lut);
+	if (state->ctm_matrix)
+		drm_property_reference_blob(state->ctm_matrix);
+	if (state->gamma_lut)
+		drm_property_reference_blob(state->gamma_lut);
 	state->mode_changed = false;
 	state->active_changed = false;
 	state->planes_changed = false;
 	state->connectors_changed = false;
+	state->color_mgmt_changed = false;
 	state->event = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
@@ -2557,6 +2564,9 @@  void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
 	drm_property_unreference_blob(state->mode_blob);
+	drm_property_unreference_blob(state->degamma_lut);
+	drm_property_unreference_blob(state->ctm_matrix);
+	drm_property_unreference_blob(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b19c8f4..8e8f57a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1565,6 +1565,41 @@  static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_size_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"CTM_MATRIX", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.ctm_matrix_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_size_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 9f8b894..3cd8cbb 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1073,3 +1073,36 @@  int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+
+/**
+ * drm_helper_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction properties on a
+ * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
+ * set and 2 size properties to inform the userspace of the lut sizes.
+ */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       int degamma_lut_size,
+				       int gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->ctm_matrix_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_property, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_size_property,
+				   degamma_lut_size);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_size_property,
+				   gamma_lut_size);
+}
+EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8c7fb3d..e07b174 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,6 +305,8 @@  struct drm_plane_helper_funcs;
  * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
  * @active_changed: crtc_state->active has been toggled.
  * @connectors_changed: connectors to this crtc have been updated
+ * @color_mgmt_changed: color management properties have changed (degamma or
+ *	gamma LUT or CSC matrix)
  * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
  * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
  * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
@@ -312,6 +314,11 @@  struct drm_plane_helper_funcs;
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @degamma_lut: Lookup table for converting framebuffer pixel data
+ *	before apply the conversion matrix
+ * @ctm_matrix: Transformation matrix
+ * @gamma_lut: Lookup table for converting pixel data after the
+ *	conversion matrix
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -333,6 +340,7 @@  struct drm_crtc_state {
 	bool mode_changed : 1;
 	bool active_changed : 1;
 	bool connectors_changed : 1;
+	bool color_mgmt_changed : 1;
 
 	/* attached planes bitmask:
 	 * WARNING: transitional helpers do not maintain plane_mask so
@@ -355,6 +363,11 @@  struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob property to expose color management to userspace */
+	struct drm_property_blob *degamma_lut;
+	struct drm_property_blob *ctm_matrix;
+	struct drm_property_blob *gamma_lut;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -757,7 +770,7 @@  struct drm_crtc {
 	int x, y;
 	const struct drm_crtc_funcs *funcs;
 
-	/* CRTC gamma size for reporting to userspace */
+	/* Legacy FB CRTC gamma size for reporting to userspace */
 	uint32_t gamma_size;
 	uint16_t *gamma_store;
 
@@ -2025,6 +2038,15 @@  struct drm_mode_config_funcs {
  * @property_blob_list: list of all the blob property objects
  * @blob_lock: mutex for blob property allocation and management
  * @*_property: core property tracking
+ * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
+ *	gamma
+ * @degamma_lut_size_property: size of the degamma LUT as supported by the
+ *	driver (read-only)
+ * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
+ *	degamma LUT
+ * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
+ *	the gamma space of the connected screen (read-only)
+ * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @async_page_flip: does this device support async flips on the primary plane?
@@ -2126,6 +2148,13 @@  struct drm_mode_config {
 	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
+	/* Optional color correction properties */
+	struct drm_property *degamma_lut_property;
+	struct drm_property *degamma_lut_size_property;
+	struct drm_property *ctm_matrix_property;
+	struct drm_property *gamma_lut_property;
+	struct drm_property *gamma_lut_size_property;
+
 	/* properties for virtual machine layout */
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
@@ -2548,6 +2577,19 @@  static inline struct drm_property *drm_property_find(struct drm_device *dev,
 	return mo ? obj_to_property(mo) : NULL;
 }
 
+/*
+ * Extract a degamma/gamma LUT value provided by user and round it to the
+ * precision supported by the hardware.
+ */
+static inline uint32_t drm_color_lut_extract(uint32_t user_input,
+					     uint32_t bit_precision)
+{
+	uint32_t val = ((user_input << 1) + 1) >> (16 - bit_precision + 1);
+	uint32_t max = 0xffff >> (16 - bit_precision);
+
+	return clamp_val(val, 0, max);
+}
+
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 4b37afa..97fa894 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,6 +48,9 @@  extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
+extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+					      int degamma_lut_size,
+					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50adb46..c021743 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -487,6 +487,21 @@  struct drm_mode_crtc_lut {
 	__u64 blue;
 };
 
+struct drm_color_ctm {
+	/* Conversion matrix in S31.32 format. */
+	__s64 matrix[9];
+};
+
+struct drm_color_lut {
+	/*
+	 * Data is U0.16 fixed point format.
+	 */
+	__u16 red;
+	__u16 green;
+	__u16 blue;
+	__u16 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)

Comments

On Tue, Feb 09, 2016 at 12:19:14PM +0000, Lionel Landwerlin wrote:
> Patch based on a previous series by Shashank Sharma.
> 
> v2: Register LUT size properties as range
> 
> v3: Fix round in drm_color_lut_get_value() helper
>     More docs on how degamma/gamma properties are used
> 
> v4: Update contributors

A couple minor notes:
 * In the kerneldoc, you might want to spell out the first use of the
   acronyms for clarity.  I.e., Color Transformation Matrix, Lookup
   Table.
 * You might want to add DRM_DEBUG_KMS() calls to some of the error
   conditions (e.g., when the blob has an invalid size) so that it will
   be more apparent why we failed when looking at kernel logs.
 * Looks like one minor whitespace error, noted below.
 * One minor helper note below.

Aside from those,
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


Also, somewhere (probably as a followup patch to this series), I think
we need some kerneldoc text that describes the relationship between this
color management work and the old legacy gamma ioctl.  I.e., what
happens if you intermix calls to the legacy ioctl with updates to the
color manager properties.


Matt


> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
...
> @@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
>  		drm_property_unreference_blob(mode);
>  		return ret;
> -	}
> -	else if (crtc->funcs->atomic_set_property)
> +	} else if (property == config->degamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->degamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->degamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->ctm_matrix_property) {
> +		bool replaced = false;
> +
> +		ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->ctm_matrix,
> +					val,
> +					sizeof(struct drm_color_ctm),
> +							       &replaced);

Looks like there might be something off with the whitespace here.


> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->gamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->gamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->gamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (crtc->funcs->atomic_set_property)
>  		return crtc->funcs->atomic_set_property(crtc, state, property, val);
>  	else
>  		return -EINVAL;
> @@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = state->active;
>  	else if (property == config->prop_mode_id)
>  		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> +	else if (property == config->degamma_lut_property)
> +		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> +	else if (property == config->ctm_matrix_property)
> +		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
> +	else if (property == config->gamma_lut_property)
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2b430b0..d13dfd8 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2513,10 +2513,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  
>  	if (state->mode_blob)
>  		drm_property_reference_blob(state->mode_blob);
> +	if (state->degamma_lut)
> +		drm_property_reference_blob(state->degamma_lut);
> +	if (state->ctm_matrix)
> +		drm_property_reference_blob(state->ctm_matrix);
> +	if (state->gamma_lut)
> +		drm_property_reference_blob(state->gamma_lut);
>  	state->mode_changed = false;
>  	state->active_changed = false;
>  	state->planes_changed = false;
>  	state->connectors_changed = false;
> +	state->color_mgmt_changed = false;
>  	state->event = NULL;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
> @@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					    struct drm_crtc_state *state)
>  {
>  	drm_property_unreference_blob(state->mode_blob);
> +	drm_property_unreference_blob(state->degamma_lut);
> +	drm_property_unreference_blob(state->ctm_matrix);
> +	drm_property_unreference_blob(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b19c8f4..8e8f57a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1565,6 +1565,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"DEGAMMA_LUT", 0);

Not a big deal either way, but there's a drm_property_create_blob() you
could use to make the code slightly simpler for the blob properties
here.


> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_size_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"CTM_MATRIX", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.ctm_matrix_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"GAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"GAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_size_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 9f8b894..3cd8cbb 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1073,3 +1073,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	return drm_plane_helper_commit(plane, plane_state, old_fb);
>  }
>  EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
> +
> +/**
> + * drm_helper_crtc_enable_color_mgmt - enable color management properties
> + * @crtc: DRM CRTC
> + * @degamma_lut_size: the size of the degamma lut (before CSC)
> + * @gamma_lut_size: the size of the gamma lut (after CSC)
> + *
> + * This function lets the driver enable the color correction properties on a
> + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
> + * set and 2 size properties to inform the userspace of the lut sizes.
> + */
> +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +				       int degamma_lut_size,
> +				       int gamma_lut_size)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->ctm_matrix_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_property, 0);
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_size_property,
> +				   degamma_lut_size);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_size_property,
> +				   gamma_lut_size);
> +}
> +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8c7fb3d..e07b174 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -305,6 +305,8 @@ struct drm_plane_helper_funcs;
>   * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
>   * @active_changed: crtc_state->active has been toggled.
>   * @connectors_changed: connectors to this crtc have been updated
> + * @color_mgmt_changed: color management properties have changed (degamma or
> + *	gamma LUT or CSC matrix)
>   * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
>   * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
>   * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
> @@ -312,6 +314,11 @@ struct drm_plane_helper_funcs;
>   * 	update to ensure framebuffer cleanup isn't done too early
>   * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
>   * @mode: current mode timings
> + * @degamma_lut: Lookup table for converting framebuffer pixel data
> + *	before apply the conversion matrix
> + * @ctm_matrix: Transformation matrix
> + * @gamma_lut: Lookup table for converting pixel data after the
> + *	conversion matrix
>   * @event: optional pointer to a DRM event to signal upon completion of the
>   * 	state update
>   * @state: backpointer to global drm_atomic_state
> @@ -333,6 +340,7 @@ struct drm_crtc_state {
>  	bool mode_changed : 1;
>  	bool active_changed : 1;
>  	bool connectors_changed : 1;
> +	bool color_mgmt_changed : 1;
>  
>  	/* attached planes bitmask:
>  	 * WARNING: transitional helpers do not maintain plane_mask so
> @@ -355,6 +363,11 @@ struct drm_crtc_state {
>  	/* blob property to expose current mode to atomic userspace */
>  	struct drm_property_blob *mode_blob;
>  
> +	/* blob property to expose color management to userspace */
> +	struct drm_property_blob *degamma_lut;
> +	struct drm_property_blob *ctm_matrix;
> +	struct drm_property_blob *gamma_lut;
> +
>  	struct drm_pending_vblank_event *event;
>  
>  	struct drm_atomic_state *state;
> @@ -757,7 +770,7 @@ struct drm_crtc {
>  	int x, y;
>  	const struct drm_crtc_funcs *funcs;
>  
> -	/* CRTC gamma size for reporting to userspace */
> +	/* Legacy FB CRTC gamma size for reporting to userspace */
>  	uint32_t gamma_size;
>  	uint16_t *gamma_store;
>  
> @@ -2025,6 +2038,15 @@ struct drm_mode_config_funcs {
>   * @property_blob_list: list of all the blob property objects
>   * @blob_lock: mutex for blob property allocation and management
>   * @*_property: core property tracking
> + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
> + *	gamma
> + * @degamma_lut_size_property: size of the degamma LUT as supported by the
> + *	driver (read-only)
> + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
> + *	degamma LUT
> + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
> + *	the gamma space of the connected screen (read-only)
> + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
>   * @preferred_depth: preferred RBG pixel depth, used by fb helpers
>   * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
>   * @async_page_flip: does this device support async flips on the primary plane?
> @@ -2126,6 +2148,13 @@ struct drm_mode_config {
>  	struct drm_property *aspect_ratio_property;
>  	struct drm_property *dirty_info_property;
>  
> +	/* Optional color correction properties */
> +	struct drm_property *degamma_lut_property;
> +	struct drm_property *degamma_lut_size_property;
> +	struct drm_property *ctm_matrix_property;
> +	struct drm_property *gamma_lut_property;
> +	struct drm_property *gamma_lut_size_property;
> +
>  	/* properties for virtual machine layout */
>  	struct drm_property *suggested_x_property;
>  	struct drm_property *suggested_y_property;
> @@ -2548,6 +2577,19 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
>  	return mo ? obj_to_property(mo) : NULL;
>  }
>  
> +/*
> + * Extract a degamma/gamma LUT value provided by user and round it to the
> + * precision supported by the hardware.
> + */
> +static inline uint32_t drm_color_lut_extract(uint32_t user_input,
> +					     uint32_t bit_precision)
> +{
> +	uint32_t val = ((user_input << 1) + 1) >> (16 - bit_precision + 1);
> +	uint32_t max = 0xffff >> (16 - bit_precision);
> +
> +	return clamp_val(val, 0, max);
> +}
> +
>  /* Plane list iterator for legacy (overlay only) planes. */
>  #define drm_for_each_legacy_plane(plane, dev) \
>  	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 4b37afa..97fa894 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  				     struct drm_display_mode *mode,
>  				     int x, int y,
>  				     struct drm_framebuffer *old_fb);
> +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +					      int degamma_lut_size,
> +					      int gamma_lut_size);
>  extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
>  extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..c021743 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
>  	__u64 blue;
>  };
>  
> +struct drm_color_ctm {
> +	/* Conversion matrix in S31.32 format. */
> +	__s64 matrix[9];
> +};
> +
> +struct drm_color_lut {
> +	/*
> +	 * Data is U0.16 fixed point format.
> +	 */
> +	__u16 red;
> +	__u16 green;
> +	__u16 blue;
> +	__u16 reserved;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
> -- 
> 2.7.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel