✗ Fi.CI.CHECKPATCH: warning for Add Plane Color Properties (rev4)

✗ Fi.CI.CHECKPATCH: warning for Add Plane Color Properties (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

$ dim checkpatch origin/drm-tip
1676eec85114 drm: Add Enhanced Gamma LUT precision structure
e9786d7168cd drm: Add Plane Degamma properties
-:59: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#59: FILE: drivers/gpu/drm/drm_atomic.c:915:
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->degamma_lut,

-:135: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#135: FILE: drivers/gpu/drm/drm_plane.c:499:
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,

-:142: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#142: FILE: drivers/gpu/drm/drm_plane.c:506:
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,

total: 0 errors, 0 warnings, 3 checks, 159 lines checked
d137d6637c14 drm: Add Plane CTM property
-:43: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#43: FILE: drivers/gpu/drm/drm_atomic.c:922:
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->ctm,

-:104: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#104: FILE: drivers/gpu/drm/drm_plane.c:518:
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,

total: 0 errors, 0 warnings, 2 checks, 98 lines checked
c8285a5384fc drm: Add Plane Gamma properties
-:47: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#47: FILE: drivers/gpu/drm/drm_atomic.c:930:
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->gamma_lut,

-:110: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#110: FILE: drivers/gpu/drm/drm_plane.c:534:
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,

-:117: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#117: FILE: drivers/gpu/drm/drm_plane.c:541:
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,

total: 0 errors, 0 warnings, 3 checks, 117 lines checked
01d5c39de12d drm: Define helper function for plane color enabling
-:41: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#41: FILE: drivers/gpu/drm/drm_plane.c:162:
+void drm_plane_enable_color_mgmt(struct drm_plane *plane,
+				uint plane_degamma_lut_size,

-:47: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#47: FILE: drivers/gpu/drm/drm_plane.c:168:
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_property, 0);

-:49: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#49: FILE: drivers/gpu/drm/drm_plane.c:170:
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_size_property,

-:55: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#55: FILE: drivers/gpu/drm/drm_plane.c:176:
+		drm_object_attach_property(&plane->base,
+				plane->ctm_property, 0);

-:59: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#59: FILE: drivers/gpu/drm/drm_plane.c:180:
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_property, 0);

-:61: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#61: FILE: drivers/gpu/drm/drm_plane.c:182:
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_size_property,

total: 0 errors, 0 warnings, 6 checks, 57 lines checked
7f5c914396f5 drm/i915: Enable plane color features
-:52: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#52: FILE: drivers/gpu/drm/i915/intel_color.c:655:
+		drm_plane_enable_color_mgmt(plane,
+		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size,

total: 0 errors, 0 warnings, 1 checks, 63 lines checked
60155d559b3a drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
-:57: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#57: FILE: drivers/gpu/drm/i915/i915_reg.h:9706:
+#define PLANE_GAMC(pipe, plane, i)	\
+	_MMIO_PLANE_GAMC(plane, i, _PLANE_GAMC_1(pipe), _PLANE_GAMC_2(pipe))

-:68: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#68: FILE: drivers/gpu/drm/i915/i915_reg.h:9717:
+#define PLANE_GAMC16(pipe, plane, i) _MMIO_PLANE_GAMC16(plane, i, \
+				_PLANE_GAMC16_1(pipe), _PLANE_GAMC16_2(pipe))

-:93: CHECK:SPACING: No space is necessary after a cast
#93: FILE: drivers/gpu/drm/i915/intel_color.c:506:
+			(struct drm_color_lut_ext *) state->gamma_lut->data;

-:132: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#132: FILE: drivers/gpu/drm/i915/intel_color.c:545:
+	bdw_load_plane_gamma_lut(state,
+		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size);

-:143: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#143: FILE: drivers/gpu/drm/i915/intel_color.c:705:
+	if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
+		IS_BROXTON(dev_priv)) {

total: 0 errors, 0 warnings, 5 checks, 138 lines checked
930ad4d3044a drm/i915: Load plane color luts from atomic flip

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

✗ Fi.CI.SPARSE: warning for Add Plane Color Properties (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm: Add Enhanced Gamma LUT precision structure
+drivers/gpu/drm/drm_plane.c:437:10: warning: symbol 'drm_color_lut_extract_ext' was not declared. Should it be static?
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_plane.c:448:16: warning: expression using sizeof(void)

Commit: drm: Add Plane Degamma properties
Okay!

Commit: drm: Add Plane CTM property
Okay!

Commit: drm: Add Plane Gamma properties
Okay!

Commit: drm: Define helper function for plane color enabling
Okay!

Commit: drm/i915: Enable plane color features
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3685:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3690:16: warning: expression using sizeof(void)

Commit: drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
Okay!

Commit: drm/i915: Load plane color luts from atomic flip
Okay!

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

[RFC v4 0/8] Add Plane Color Properties

[Uma Shankar]

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

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

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

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

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

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

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

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

v4: Rebase

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

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

-- 
1.9.1

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

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

[Uma Shankar]

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

v4: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_plane.c | 19 +++++++++++++++++++
 include/uapi/drm/drm_mode.h | 15 +++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 6153cbd..cd71fd0 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -430,6 +430,25 @@ void drm_plane_force_disable(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_force_disable);
 
+/*
+ * Added to accommodate enhanced LUT precision.
+ * Max LUT precision is 32 bits.
+ */
+uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
+{
+	uint32_t val = user_input;
+	uint32_t max = 0xffffffff >> (32 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 32) {
+		val += 1UL << (32 - bit_precision - 1);
+		val >>= 32 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
+EXPORT_SYMBOL(drm_color_lut_extract_ext);
+
 /**
  * drm_mode_plane_set_obj_prop - set the value of a property
  * @plane: drm plane object to set property value for
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 8d67243..874407b 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -629,6 +629,21 @@ struct drm_color_lut {
 	__u16 reserved;
 };
 
+/*
+ * Creating 32 bit palette entries for better data
+ * precision. This will be required for HDR and
+ * similar color processing usecases.
+ */
+struct drm_color_lut_ext {
+	/*
+	 * Data is U0.32 fixed point format.
+	 */
+	__u32 red;
+	__u32 green;
+	__u32 blue;
+	__u32 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
1.9.1

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

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

[Uma Shankar]

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

v2: Rebase

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

v4: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 Documentation/gpu/drm-kms.rst       |  9 +++++++++
 drivers/gpu/drm/drm_atomic.c        | 13 +++++++++++++
 drivers/gpu/drm/drm_atomic_helper.c |  6 ++++++
 drivers/gpu/drm/drm_plane.c         | 35 +++++++++++++++++++++++++++++++++++
 include/drm/drm_plane.h             | 24 ++++++++++++++++++++++++
 5 files changed, 87 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 5dee6b8..8b10b12 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -551,6 +551,15 @@ Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :export:
 
+Plane Color Management Properties
+---------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: degamma_lut_property
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: degamma_lut_size_property
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e..f8cad9b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_mode_config *config = &dev->mode_config;
+	bool replaced = false;
+	int ret;
 
 	if (property == config->prop_fb_id) {
 		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
@@ -908,6 +910,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->color_encoding = val;
 	} else if (property == plane->color_range_property) {
 		state->color_range = val;
+	} else if (property == plane->degamma_lut_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->degamma_lut,
+					val, -1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -976,6 +985,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		*val = state->color_encoding;
 	} else if (property == plane->color_range_property) {
 		*val = state->color_range;
+	} else if (property == plane->degamma_lut_property) {
+		*val = (state->degamma_lut) ?
+			state->degamma_lut->base.id : 0;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
@@ -1116,6 +1128,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
 		   drm_get_color_encoding_name(state->color_encoding));
 	drm_printf(p, "\tcolor-range=%s\n",
 		   drm_get_color_range_name(state->color_range));
+	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
 
 	if (plane->funcs->atomic_print_state)
 		plane->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 38ce9a3..67c5b51 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3613,6 +3613,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	state->fence = NULL;
 	state->commit = NULL;
+
+	if (state->degamma_lut)
+		drm_property_blob_get(state->degamma_lut);
+	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
 
@@ -3657,6 +3661,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	if (state->commit)
 		drm_crtc_commit_put(state->commit);
+
+	drm_property_blob_put(state->degamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index cd71fd0..03e0560 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -478,6 +478,41 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
 
+/**
+ * DOC: degamma_lut_property
+ *
+ * degamma_lut_property:
+ *	Blob property which allows a userspace to provide LUT values
+ *	to apply degamma curve using the h/w plane degamma processing
+ *	engine, thereby making the content as linear for further color
+ *	processing.
+ *
+ * degamma_lut_size_property:
+ *	Range Property to indicate size of the plane degamma LUT.
+ */
+int drm_plane_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_size_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_create_prop);
+
 int drm_mode_getplane_res(struct drm_device *dev, void *data,
 			  struct drm_file *file_priv)
 {
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 8a152dc..28357ac 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -174,6 +174,14 @@ struct drm_plane_state {
 	 */
 	bool visible;
 
+	/* @degamma_lut:
+	 *
+	 * Lookup table for converting framebuffer pixel data before apply the
+	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
+	 */
+	struct drm_property_blob *degamma_lut;
+
 	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
@@ -184,6 +192,8 @@ struct drm_plane_state {
 
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
+
+	u8 color_mgmt_changed : 1;
 };
 
 static inline struct drm_rect
@@ -676,6 +686,18 @@ struct drm_plane {
 	 * See drm_plane_create_color_properties().
 	 */
 	struct drm_property *color_range_property;
+
+	/**
+	 * @degamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to linear gamma.
+	 */
+	struct drm_property *degamma_lut_property;
+
+	/**
+	 * @degamma_lut_size_property: Optional Plane property for the
+	 * size of the degamma LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *degamma_lut_size_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -725,6 +747,8 @@ static inline u32 drm_plane_mask(const struct drm_plane *plane)
 int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 				       struct drm_property *property,
 				       uint64_t value);
+int drm_plane_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane);
 
 /**
  * drm_plane_find - find a &drm_plane
-- 
1.9.1

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

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

[Uma Shankar]

Add a blob property for plane CSC usage.

v2: Rebase

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

v4: Rebase

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

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 8b10b12..16d6d8d 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -560,6 +560,9 @@ Plane Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_plane.c
    :doc: degamma_lut_size_property
 
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: ctm_property
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f8cad9b..ddda935 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == plane->ctm_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->ctm,
+					val,
+					sizeof(struct drm_color_ctm), -1,
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 	} else if (property == plane->degamma_lut_property) {
 		*val = (state->degamma_lut) ?
 			state->degamma_lut->base.id : 0;
+	} else if (property == plane->ctm_property) {
+		*val = (state->ctm) ? state->ctm->base.id : 0;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 67c5b51..866181f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3616,6 +3616,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	if (state->degamma_lut)
 		drm_property_blob_get(state->degamma_lut);
+	if (state->ctm)
+		drm_property_blob_get(state->ctm);
+
 	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3663,6 +3666,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 		drm_crtc_commit_put(state->commit);
 
 	drm_property_blob_put(state->degamma_lut);
+	drm_property_blob_put(state->ctm);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 03e0560..97520b1 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
  *
  * degamma_lut_size_property:
  *	Range Property to indicate size of the plane degamma LUT.
+ *
+ * ctm_property:
+ *	Blob property which allows a userspace to provide CTM coefficients
+ *	to do color space conversion or any other enhancement by doing a
+ *	matrix multiplication using the h/w CTM processing engine
  */
 int drm_plane_color_create_prop(struct drm_device *dev,
 				struct drm_plane *plane)
@@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct drm_device *dev,
 		return -ENOMEM;
 	plane->degamma_lut_size_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->ctm_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 28357ac..5143277 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -183,6 +183,14 @@ struct drm_plane_state {
 	struct drm_property_blob *degamma_lut;
 
 	/**
+	 * @ctm:
+	 *
+	 * Color transformation matrix. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is a &struct drm_color_ctm.
+	 */
+	struct drm_property_blob *ctm;
+
+	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
 	 *
@@ -698,6 +706,13 @@ struct drm_plane {
 	 * size of the degamma LUT as supported by the driver (read-only).
 	 */
 	struct drm_property *degamma_lut_size_property;
+
+	/**
+	 * @plane_ctm_property: Optional Plane property to set the
+	 * matrix used to convert colors after the lookup in the
+	 * degamma LUT.
+	 */
+	struct drm_property *ctm_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
1.9.1

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

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

[Uma Shankar]

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

v2: Rebase

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

v4: Rebase

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

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 16d6d8d..bcf9a86 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -563,6 +563,12 @@ Plane Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_plane.c
    :doc: ctm_property
 
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: gamma_lut_property
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: gamma_lut_size_property
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ddda935..8b0bf14 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -925,6 +925,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == plane->gamma_lut_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->gamma_lut,
+					val, -1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -998,6 +1005,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 			state->degamma_lut->base.id : 0;
 	} else if (property == plane->ctm_property) {
 		*val = (state->ctm) ? state->ctm->base.id : 0;
+	} else if (property == plane->gamma_lut_property) {
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 866181f..f524255 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3618,6 +3618,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 		drm_property_blob_get(state->degamma_lut);
 	if (state->ctm)
 		drm_property_blob_get(state->ctm);
+	if (state->gamma_lut)
+		drm_property_blob_put(state->gamma_lut);
 
 	state->color_mgmt_changed = false;
 }
@@ -3667,6 +3669,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	drm_property_blob_put(state->degamma_lut);
 	drm_property_blob_put(state->ctm);
+	drm_property_blob_put(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 97520b1..d0bf10b 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -494,6 +494,15 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
  *	Blob property which allows a userspace to provide CTM coefficients
  *	to do color space conversion or any other enhancement by doing a
  *	matrix multiplication using the h/w CTM processing engine
+ *
+ * gamma_lut_property:
+ *	Blob property which allows a userspace to provide LUT values
+ *	to apply gamma/tone-mapping curve using the h/w plane gamma
+ *	processing engine, thereby making the content as non-linear
+ *	or to perform any tone mapping operation for HDR usecases.
+ *
+ * gamma_lut_size_property:
+ *	Range Property to indicate size of the plane gamma LUT.
  */
 int drm_plane_color_create_prop(struct drm_device *dev,
 				struct drm_plane *plane)
@@ -521,6 +530,20 @@ int drm_plane_color_create_prop(struct drm_device *dev,
 		return -ENOMEM;
 	plane->ctm_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"PLANE_GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	plane->gamma_lut_size_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 5143277..fb466b1 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -191,6 +191,15 @@ struct drm_plane_state {
 	struct drm_property_blob *ctm;
 
 	/**
+	 * @gamma_lut:
+	 *
+	 * Lookup table for converting pixel data after the color conversion
+	 * matrix @ctm.  See drm_plane_enable_color_mgmt(). The blob (if not
+	 * NULL) is an array of &struct drm_color_lut_ext.
+	 */
+	struct drm_property_blob *gamma_lut;
+
+	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
 	 *
@@ -713,6 +722,19 @@ struct drm_plane {
 	 * degamma LUT.
 	 */
 	struct drm_property *ctm_property;
+
+	/**
+	 * @plane_gamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the colors, after the CTM matrix, to the common
+	 * gamma space chosen for blending.
+	 */
+	struct drm_property *gamma_lut_property;
+
+	/**
+	 * @plane_gamma_lut_size_property: Optional Plane property for the size
+	 * of the gamma LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *gamma_lut_size_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
1.9.1

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

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

[Uma Shankar]

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

v2: Rebase

v3: Modiefied the function to use updated property names.

v4: Rebase

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

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index d0bf10b..d1b4ac1 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -144,6 +144,48 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 }
 
 /**
+ * drm_plane_enable_color_mgmt - enable color management properties
+ * @plane: DRM Plane
+ * @plane_degamma_lut_size: the size of the degamma lut (before CSC)
+ * @plane_has_ctm: whether to attach ctm_property for CSC matrix
+ * @plane_gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction
+ * properties on a plane. This includes 3 degamma, csc and gamma
+ * properties that userspace can set and 2 size properties to inform
+ * the userspace of the lut sizes. Each of the properties are
+ * optional. The gamma and degamma properties are only attached if
+ * their size is not 0 and ctm_property is only attached if has_ctm is
+ * true.
+ */
+void drm_plane_enable_color_mgmt(struct drm_plane *plane,
+				uint plane_degamma_lut_size,
+				bool plane_has_ctm,
+				uint plane_gamma_lut_size)
+{
+	if (plane_degamma_lut_size) {
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_property, 0);
+		drm_object_attach_property(&plane->base,
+				plane->degamma_lut_size_property,
+				plane_degamma_lut_size);
+	}
+
+	if (plane_has_ctm)
+		drm_object_attach_property(&plane->base,
+				plane->ctm_property, 0);
+
+	if (plane_gamma_lut_size) {
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_property, 0);
+		drm_object_attach_property(&plane->base,
+				plane->gamma_lut_size_property,
+				plane_gamma_lut_size);
+	}
+}
+EXPORT_SYMBOL(drm_plane_enable_color_mgmt);
+
+/**
  * drm_universal_plane_init - Initialize a new universal plane object
  * @dev: DRM device
  * @plane: plane object to init
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 44f04233..9b8e566 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -68,4 +68,9 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 				      u32 supported_ranges,
 				      enum drm_color_encoding default_encoding,
 				      enum drm_color_range default_range);
+
+void drm_plane_enable_color_mgmt(struct drm_plane *plane,
+				 uint plane_degamma_lut_size,
+				 bool plane_has_ctm,
+				 uint plane_gamma_lut_size);
 #endif
-- 
1.9.1

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

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

[Uma Shankar]

Enable and initialize plane color features.

v2: Rebase and some cleanup

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

v4: Rebase

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

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

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

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

[Uma Shankar]

Implement Plane Gamma feature for BDW and Gen9 platforms.

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

v3: Rebase

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

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

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

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

[Uma Shankar]

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

v4: Rebase

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

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index dcba645..abc9051 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -224,6 +224,10 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 		intel_atomic_get_new_plane_state(state, intel_plane);
 	struct drm_crtc *crtc = new_plane_state->base.crtc ?: old_state->crtc;
 
+	if (new_plane_state->base.color_mgmt_changed) {
+		intel_color_load_plane_luts(&new_plane_state->base);
+	}
+
 	if (new_plane_state->base.visible) {
 		const struct intel_crtc_state *new_crtc_state =
 			intel_atomic_get_new_crtc_state(state, to_intel_crtc(crtc));
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 2b5c0cd..de28d6b 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -666,6 +666,14 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state)
 	dev_priv->display.load_luts(crtc_state);
 }
 
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state)
+{
+	struct drm_device *dev = plane_state->plane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	dev_priv->display.load_plane_luts(plane_state);
+}
+
 int intel_color_check(struct drm_crtc *crtc,
 		      struct drm_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3850a5b..fea772b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2179,6 +2179,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 void intel_color_set_csc(struct drm_crtc_state *crtc_state);
 void intel_color_load_luts(struct drm_crtc_state *crtc_state);
 void intel_plane_color_init(struct drm_plane *plane);
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state);
 
 /* intel_lspcon.c */
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
-- 
1.9.1

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

✓ Fi.CI.BAT: success for Add Plane Color Properties (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4685 -> Patchwork_9972 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/30875/revisions/4/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9972 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     PASS -> INCOMPLETE (fdo#107402)

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@gem_exec_basic@readonly-blt:
      fi-byt-n2820:       FAIL (fdo#105900) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-FAIL (fdo#102614) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ivb-3770:        FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402


== Participating hosts (52 -> 48) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4685 -> Patchwork_9972

  CI_DRM_4685: df7e8eddc3830216d3fec15e2c7d0b6ec97e7bae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4606: 38a44003774e35c587c67c8766b35e75dbb993b8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9972: 930ad4d3044a5ee6042b5fb66b13099add5707e6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

930ad4d3044a drm/i915: Load plane color luts from atomic flip
60155d559b3a drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms
7f5c914396f5 drm/i915: Enable plane color features
01d5c39de12d drm: Define helper function for plane color enabling
c8285a5384fc drm: Add Plane Gamma properties
d137d6637c14 drm: Add Plane CTM property
e9786d7168cd drm: Add Plane Degamma properties
1676eec85114 drm: Add Enhanced Gamma LUT precision structure

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9972/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Add Plane Color Properties (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4685_full -> Patchwork_9972_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_9972_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-kbl:          PASS -> FAIL (fdo#105010)

    
    ==== Possible fixes ====

    igt@drv_suspend@forcewake:
      shard-glk:          FAIL (fdo#103375) -> PASS

    igt@drv_suspend@shrink:
      shard-glk:          INCOMPLETE (fdo#103359, fdo#106886, k.org#198133) -> PASS

    
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4685 -> Patchwork_9972

  CI_DRM_4685: df7e8eddc3830216d3fec15e2c7d0b6ec97e7bae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4606: 38a44003774e35c587c67c8766b35e75dbb993b8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9972: 930ad4d3044a5ee6042b5fb66b13099add5707e6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9972/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:44PM +0530, Uma Shankar wrote:
> Existing LUT precision structure is having only 16 bit
> precision. This is not enough for upcoming enhanced hardwares
> and advance usecases like HDR processing. Hence added a new
> structure with 32 bit precision values. Also added the code,
> for extracting the same from values passed from userspace.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c | 19 +++++++++++++++++++
>  include/uapi/drm/drm_mode.h | 15 +++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 6153cbd..cd71fd0 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -430,6 +430,25 @@ void drm_plane_force_disable(struct drm_plane *plane)
>  }
>  EXPORT_SYMBOL(drm_plane_force_disable);
>  
> +/*
> + * Added to accommodate enhanced LUT precision.
> + * Max LUT precision is 32 bits.
> + */
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)

You should declare this function in a header file as well.

With that fixed
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>


> +{
> +	uint32_t val = user_input;
> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 32) {
> +		val += 1UL << (32 - bit_precision - 1);
> +		val >>= 32 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
> +
>  /**
>   * drm_mode_plane_set_obj_prop - set the value of a property
>   * @plane: drm plane object to set property value for
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8d67243..874407b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -629,6 +629,21 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/*
> + * Creating 32 bit palette entries for better data
> + * precision. This will be required for HDR and
> + * similar color processing usecases.
> + */
> +struct drm_color_lut_ext {
> +	/*
> +	 * Data is U0.32 fixed point format.
> +	 */
> +	__u32 red;
> +	__u32 green;
> +	__u32 blue;
> +	__u32 reserved;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:45PM +0530, Uma Shankar wrote:
> Add Plane Degamma as a blob property and plane degamma size as
> a range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst       |  9 +++++++++
>  drivers/gpu/drm/drm_atomic.c        | 13 +++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  6 ++++++
>  drivers/gpu/drm/drm_plane.c         | 35 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_plane.h             | 24 ++++++++++++++++++++++++
>  5 files changed, 87 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 5dee6b8..8b10b12 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -551,6 +551,15 @@ Color Management Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :export:
>  
> +Plane Color Management Properties
> +---------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: degamma_lut_property
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: degamma_lut_size_property
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e..f8cad9b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -908,6 +910,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->color_encoding = val;
>  	} else if (property == plane->color_range_property) {
>  		state->color_range = val;
> +	} else if (property == plane->degamma_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->degamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -976,6 +985,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		*val = state->color_encoding;
>  	} else if (property == plane->color_range_property) {
>  		*val = state->color_range;
> +	} else if (property == plane->degamma_lut_property) {
> +		*val = (state->degamma_lut) ?
> +			state->degamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> @@ -1116,6 +1128,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  		   drm_get_color_encoding_name(state->color_encoding));
>  	drm_printf(p, "\tcolor-range=%s\n",
>  		   drm_get_color_range_name(state->color_range));
> +	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
>  
>  	if (plane->funcs->atomic_print_state)
>  		plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 38ce9a3..67c5b51 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3613,6 +3613,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	state->fence = NULL;
>  	state->commit = NULL;
> +
> +	if (state->degamma_lut)
> +		drm_property_blob_get(state->degamma_lut);
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> @@ -3657,6 +3661,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	if (state->commit)
>  		drm_crtc_commit_put(state->commit);
> +
> +	drm_property_blob_put(state->degamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index cd71fd0..03e0560 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -478,6 +478,41 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  }
>  EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
>  
> +/**
> + * DOC: degamma_lut_property
> + *
> + * degamma_lut_property:
> + *	Blob property which allows a userspace to provide LUT values
> + *	to apply degamma curve using the h/w plane degamma processing
> + *	engine, thereby making the content as linear for further color
> + *	processing.
> + *
> + * degamma_lut_size_property:
> + *	Range Property to indicate size of the plane degamma LUT.
> + */
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_size_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_create_prop);
> +
>  int drm_mode_getplane_res(struct drm_device *dev, void *data,
>  			  struct drm_file *file_priv)
>  {
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8a152dc..28357ac 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -174,6 +174,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/* @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *degamma_lut;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -184,6 +192,8 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -676,6 +686,18 @@ struct drm_plane {
>  	 * See drm_plane_create_color_properties().
>  	 */
>  	struct drm_property *color_range_property;
> +
> +	/**
> +	 * @degamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_lut_property;
> +
> +	/**
> +	 * @degamma_lut_size_property: Optional Plane property for the
> +	 * size of the degamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *degamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -725,6 +747,8 @@ static inline u32 drm_plane_mask(const struct drm_plane *plane)
>  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  				       struct drm_property *property,
>  				       uint64_t value);
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

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

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:46PM +0530, Uma Shankar wrote:
> Add a blob property for plane CSC usage.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst       |  3 +++
>  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
>  include/drm/drm_plane.h             | 15 +++++++++++++++
>  5 files changed, 44 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 8b10b12..16d6d8d 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -560,6 +560,9 @@ Plane Color Management Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
>     :doc: degamma_lut_size_property
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: ctm_property
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f8cad9b..ddda935 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> +	} else if (property == plane->ctm_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->ctm,
> +					val,
> +					sizeof(struct drm_color_ctm), -1,
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  	} else if (property == plane->degamma_lut_property) {
>  		*val = (state->degamma_lut) ?
>  			state->degamma_lut->base.id : 0;
> +	} else if (property == plane->ctm_property) {
> +		*val = (state->ctm) ? state->ctm->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 67c5b51..866181f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3616,6 +3616,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	if (state->degamma_lut)
>  		drm_property_blob_get(state->degamma_lut);
> +	if (state->ctm)
> +		drm_property_blob_get(state->ctm);
> +
>  	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> @@ -3663,6 +3666,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  		drm_crtc_commit_put(state->commit);
>  
>  	drm_property_blob_put(state->degamma_lut);
> +	drm_property_blob_put(state->ctm);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 03e0560..97520b1 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>   *
>   * degamma_lut_size_property:
>   *	Range Property to indicate size of the plane degamma LUT.
> + *
> + * ctm_property:
> + *	Blob property which allows a userspace to provide CTM coefficients
> + *	to do color space conversion or any other enhancement by doing a
> + *	matrix multiplication using the h/w CTM processing engine
>   */
>  int drm_plane_color_create_prop(struct drm_device *dev,
>  				struct drm_plane *plane)
> @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct drm_device *dev,
>  		return -ENOMEM;
>  	plane->degamma_lut_size_property = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_CTM", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->ctm_property = prop;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 28357ac..5143277 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -183,6 +183,14 @@ struct drm_plane_state {
>  	struct drm_property_blob *degamma_lut;
>  
>  	/**
> +	 * @ctm:
> +	 *
> +	 * Color transformation matrix. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is a &struct drm_color_ctm.
> +	 */
> +	struct drm_property_blob *ctm;
> +
> +	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
>  	 *
> @@ -698,6 +706,13 @@ struct drm_plane {
>  	 * size of the degamma LUT as supported by the driver (read-only).
>  	 */
>  	struct drm_property *degamma_lut_size_property;
> +
> +	/**
> +	 * @plane_ctm_property: Optional Plane property to set the
> +	 * matrix used to convert colors after the lookup in the
> +	 * degamma LUT.
> +	 */
> +	struct drm_property *ctm_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

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

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:47PM +0530, Uma Shankar wrote:
> Add plane gamma as blob property and size as a
> range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst       |  6 ++++++
>  drivers/gpu/drm/drm_atomic.c        |  9 +++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  3 +++
>  drivers/gpu/drm/drm_plane.c         | 23 +++++++++++++++++++++++
>  include/drm/drm_plane.h             | 22 ++++++++++++++++++++++
>  5 files changed, 63 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 16d6d8d..bcf9a86 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -563,6 +563,12 @@ Plane Color Management Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
>     :doc: ctm_property
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: gamma_lut_property
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: gamma_lut_size_property
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ddda935..8b0bf14 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -925,6 +925,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> +	} else if (property == plane->gamma_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->gamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -998,6 +1005,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  			state->degamma_lut->base.id : 0;
>  	} else if (property == plane->ctm_property) {
>  		*val = (state->ctm) ? state->ctm->base.id : 0;
> +	} else if (property == plane->gamma_lut_property) {
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 866181f..f524255 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3618,6 +3618,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  		drm_property_blob_get(state->degamma_lut);
>  	if (state->ctm)
>  		drm_property_blob_get(state->ctm);
> +	if (state->gamma_lut)
> +		drm_property_blob_put(state->gamma_lut);
>  
>  	state->color_mgmt_changed = false;
>  }
> @@ -3667,6 +3669,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	drm_property_blob_put(state->degamma_lut);
>  	drm_property_blob_put(state->ctm);
> +	drm_property_blob_put(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 97520b1..d0bf10b 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -494,6 +494,15 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>   *	Blob property which allows a userspace to provide CTM coefficients
>   *	to do color space conversion or any other enhancement by doing a
>   *	matrix multiplication using the h/w CTM processing engine
> + *
> + * gamma_lut_property:
> + *	Blob property which allows a userspace to provide LUT values
> + *	to apply gamma/tone-mapping curve using the h/w plane gamma
> + *	processing engine, thereby making the content as non-linear
> + *	or to perform any tone mapping operation for HDR usecases.
> + *
> + * gamma_lut_size_property:
> + *	Range Property to indicate size of the plane gamma LUT.
>   */
>  int drm_plane_color_create_prop(struct drm_device *dev,
>  				struct drm_plane *plane)
> @@ -521,6 +530,20 @@ int drm_plane_color_create_prop(struct drm_device *dev,
>  		return -ENOMEM;
>  	plane->ctm_property = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_GAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->gamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_GAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->gamma_lut_size_property = prop;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 5143277..fb466b1 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -191,6 +191,15 @@ struct drm_plane_state {
>  	struct drm_property_blob *ctm;
>  
>  	/**
> +	 * @gamma_lut:
> +	 *
> +	 * Lookup table for converting pixel data after the color conversion
> +	 * matrix @ctm.  See drm_plane_enable_color_mgmt(). The blob (if not
> +	 * NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *gamma_lut;
> +
> +	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
>  	 *
> @@ -713,6 +722,19 @@ struct drm_plane {
>  	 * degamma LUT.
>  	 */
>  	struct drm_property *ctm_property;
> +
> +	/**
> +	 * @plane_gamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the colors, after the CTM matrix, to the common
> +	 * gamma space chosen for blending.
> +	 */
> +	struct drm_property *gamma_lut_property;
> +
> +	/**
> +	 * @plane_gamma_lut_size_property: Optional Plane property for the size
> +	 * of the gamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *gamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

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

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:48PM +0530, Uma Shankar wrote:
> Define helper function to enable Plane color features
> to attach plane color properties to plane structure.
> 
> v2: Rebase
> 
> v3: Modiefied the function to use updated property names.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h |  5 +++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index d0bf10b..d1b4ac1 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -144,6 +144,48 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
>  }
>  
>  /**
> + * drm_plane_enable_color_mgmt - enable color management properties
> + * @plane: DRM Plane
> + * @plane_degamma_lut_size: the size of the degamma lut (before CSC)
> + * @plane_has_ctm: whether to attach ctm_property for CSC matrix
> + * @plane_gamma_lut_size: the size of the gamma lut (after CSC)
> + *
> + * This function lets the driver enable the color correction
> + * properties on a plane. This includes 3 degamma, csc and gamma
> + * properties that userspace can set and 2 size properties to inform
> + * the userspace of the lut sizes. Each of the properties are
> + * optional. The gamma and degamma properties are only attached if
> + * their size is not 0 and ctm_property is only attached if has_ctm is
> + * true.
> + */
> +void drm_plane_enable_color_mgmt(struct drm_plane *plane,
> +				uint plane_degamma_lut_size,
> +				bool plane_has_ctm,
> +				uint plane_gamma_lut_size)
> +{
> +	if (plane_degamma_lut_size) {
> +		drm_object_attach_property(&plane->base,
> +				plane->degamma_lut_property, 0);
> +		drm_object_attach_property(&plane->base,
> +				plane->degamma_lut_size_property,
> +				plane_degamma_lut_size);
> +	}
> +
> +	if (plane_has_ctm)
> +		drm_object_attach_property(&plane->base,
> +				plane->ctm_property, 0);
> +
> +	if (plane_gamma_lut_size) {
> +		drm_object_attach_property(&plane->base,
> +				plane->gamma_lut_property, 0);
> +		drm_object_attach_property(&plane->base,
> +				plane->gamma_lut_size_property,
> +				plane_gamma_lut_size);
> +	}
> +}
> +EXPORT_SYMBOL(drm_plane_enable_color_mgmt);
> +
> +/**
>   * drm_universal_plane_init - Initialize a new universal plane object
>   * @dev: DRM device
>   * @plane: plane object to init
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 44f04233..9b8e566 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -68,4 +68,9 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  				      u32 supported_ranges,
>  				      enum drm_color_encoding default_encoding,
>  				      enum drm_color_range default_range);
> +
> +void drm_plane_enable_color_mgmt(struct drm_plane *plane,
> +				 uint plane_degamma_lut_size,
> +				 bool plane_has_ctm,
> +				 uint plane_gamma_lut_size);
>  #endif
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

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

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

[Alexandru-Cosmin Gheorghe]

Hi Uma,

On Fri, Aug 17, 2018 at 07:48:50PM +0530, Uma Shankar wrote:
> Implement Plane Gamma feature for BDW and Gen9 platforms.
> 
> v2: Used newly added drm_color_lut_ext structure for enhanced
> precision for Gamma LUT entries.
> 
> v3: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c      |  5 +++-
>  drivers/gpu/drm/i915/i915_reg.h      | 25 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.c   | 58 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  4 +++
>  drivers/gpu/drm/i915/intel_sprite.c  |  4 +++
>  5 files changed, 95 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index e931b48..40de78c 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -54,7 +54,10 @@
>  	.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
>  
>  #define BDW_COLORS \
> -	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
> +	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }, \
> +	.plane_color = { .plane_degamma_lut_size = 0, \
> +			 .plane_gamma_lut_size = 16 }
> +
>  #define CHV_COLORS \
>  	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
>  #define GLK_COLORS \
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0c9f03d..2db6a84 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -172,6 +172,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
>  #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>  
> +/* Plane Gamma Registers */
> +#define _MMIO_PLANE_GAMC(plane, i, a, b)  _MMIO(_PIPE(plane, a, b) + (i) * 4)
> +#define _MMIO_PLANE_GAMC16(plane, i, a, b)  _MMIO(_PIPE(plane, a, b) + (i) * 4)
> +
>  #define __MASKED_FIELD(mask, value) ((mask) << 16 | (value))
>  #define _MASKED_FIELD(mask, value) ({					   \
>  	if (__builtin_constant_p(mask))					   \
> @@ -9713,6 +9717,27 @@ enum skl_power_gate {
>  #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>  #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>  
> +/* Plane Gamma in Gen9+ */
> +#define _PLANE_GAMC_1_A	0x701d0
> +#define _PLANE_GAMC_1_B	0x711d0
> +#define _PLANE_GAMC_2_A	0x702d0
> +#define _PLANE_GAMC_2_B	0x712d0
> +#define _PLANE_GAMC_1(pipe)	_PIPE(pipe, _PLANE_GAMC_1_A, _PLANE_GAMC_1_B)
> +#define _PLANE_GAMC_2(pipe)	_PIPE(pipe, _PLANE_GAMC_2_A, _PLANE_GAMC_2_B)
> +#define PLANE_GAMC(pipe, plane, i)	\
> +	_MMIO_PLANE_GAMC(plane, i, _PLANE_GAMC_1(pipe), _PLANE_GAMC_2(pipe))
> +
> +#define _PLANE_GAMC16_1_A	0x70210
> +#define _PLANE_GAMC16_1_B	0x71210
> +#define _PLANE_GAMC16_2_A	0x70310
> +#define _PLANE_GAMC16_2_B	0x71310
> +#define _PLANE_GAMC16_1(pipe)	_PIPE(pipe, _PLANE_GAMC16_1_A, \
> +				     _PLANE_GAMC16_1_B)
> +#define _PLANE_GAMC16_2(pipe)	_PIPE(pipe, _PLANE_GAMC16_2_A, \
> +				     _PLANE_GAMC16_2_B)
> +#define PLANE_GAMC16(pipe, plane, i) _MMIO_PLANE_GAMC16(plane, i, \
> +				_PLANE_GAMC16_1(pipe), _PLANE_GAMC16_2(pipe))
> +
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index fb8402f..2b5c0cd 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -492,6 +492,59 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>  }
>  
> +static void bdw_load_plane_gamma_lut(const struct drm_plane_state *state,
> +				     u32 offset)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
> +	enum pipe pipe = to_intel_plane(state->plane)->pipe;
> +	enum plane_id plane = to_intel_plane(state->plane)->id;
> +	uint32_t i, lut_size =
> +			INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size;
> +
> +	if (state->gamma_lut) {
> +		struct drm_color_lut_ext *lut =
> +			(struct drm_color_lut_ext *) state->gamma_lut->data;
> +
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t word =
> +			(drm_color_lut_extract(lut[i].red, 10) << 20) |
> +			(drm_color_lut_extract(lut[i].green, 10) << 10) |
> +			drm_color_lut_extract(lut[i].blue, 10);


Shouldn't you be using drm_color_lut_extract_ext ?


> +
> +			I915_WRITE(PLANE_GAMC(pipe, plane, i), word);
> +		}
> +
> +		/* Program the max register to clamp values > 1.0. */
> +		i = lut_size - 1;
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 0),
> +			   drm_color_lut_extract(lut[i].red, 16));
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 1),
> +			   drm_color_lut_extract(lut[i].green, 16));
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 2),
> +			   drm_color_lut_extract(lut[i].blue, 16));
> +	} else {
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> +
> +			I915_WRITE(PLANE_GAMC(pipe, plane, i),
> +				   (v << 20) | (v << 10) | v);
> +		}
> +
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 0), (1 << 16) - 1);
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 1), (1 << 16) - 1);
> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 2), (1 << 16) - 1);
> +	}
> +}
> +
> +/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
> +static void broadwell_load_plane_luts(const struct drm_plane_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
> +
> +	bdw_load_plane_gamma_lut(state,
> +		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size);
> +}
> +
>  static void glk_load_degamma_lut(struct drm_crtc_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> @@ -648,6 +701,11 @@ void intel_plane_color_init(struct drm_plane *plane)
>  
>  	drm_plane_color_create_prop(plane->dev, plane);
>  
> +	if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> +		IS_BROXTON(dev_priv)) {
> +		dev_priv->display.load_plane_luts = broadwell_load_plane_luts;
> +	}
> +
>  	/* Enable color management support when we have degamma & gamma LUTs. */
>  	if (INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size != 0 &&
>  	    INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size != 0)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 690e1e8..2d15ac2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13803,6 +13803,10 @@ bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
>  						  DRM_COLOR_YCBCR_BT709,
>  						  DRM_COLOR_YCBCR_LIMITED_RANGE);
>  
> +	/* Add Plane Color properties */
> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> +		intel_plane_color_init(&primary->base);
> +
>  	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>  
>  	return primary;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f7026e8..0eeb1d3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1650,6 +1650,10 @@ struct intel_plane *
>  					  DRM_COLOR_YCBCR_BT709,
>  					  DRM_COLOR_YCBCR_LIMITED_RANGE);
>  
> +	/* Add Plane Color properties */
> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> +		intel_plane_color_init(&intel_plane->base);
> +
>  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>  
>  	return intel_plane;
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

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

[Lankhorst, Maarten]

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

fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
> Add a blob property for plane CSC usage.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst       |  3 +++
>  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
>  include/drm/drm_plane.h             | 15 +++++++++++++++
>  5 files changed, 44 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-
> kms.rst
> index 8b10b12..16d6d8d 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -560,6 +560,9 @@ Plane Color Management Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
>     :doc: degamma_lut_size_property
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: ctm_property
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c
> b/drivers/gpu/drm/drm_atomic.c
> index f8cad9b..ddda935 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> +	} else if (property == plane->ctm_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->ctm,
> +					val,
> +					sizeof(struct
> drm_color_ctm), -1,
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane,
> state,
>  				property, val);
> @@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
>  	} else if (property == plane->degamma_lut_property) {
>  		*val = (state->degamma_lut) ?
>  			state->degamma_lut->base.id : 0;
> +	} else if (property == plane->ctm_property) {
> +		*val = (state->ctm) ? state->ctm->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane,
> state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 67c5b51..866181f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3616,6 +3616,9 @@ void
> __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	if (state->degamma_lut)
>  		drm_property_blob_get(state->degamma_lut);
> +	if (state->ctm)
> +		drm_property_blob_get(state->ctm);
> +
>  	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> @@ -3663,6 +3666,7 @@ void
> __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
> *state)
>  		drm_crtc_commit_put(state->commit);
>  
>  	drm_property_blob_put(state->degamma_lut);
> +	drm_property_blob_put(state->ctm);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_plane.c
> b/drivers/gpu/drm/drm_plane.c
> index 03e0560..97520b1 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane
> *plane,
>   *
>   * degamma_lut_size_property:
>   *	Range Property to indicate size of the plane degamma LUT.
> + *
> + * ctm_property:
> + *	Blob property which allows a userspace to provide CTM
> coefficients
> + *	to do color space conversion or any other enhancement by
> doing a
> + *	matrix multiplication using the h/w CTM processing engine
>   */
>  int drm_plane_color_create_prop(struct drm_device *dev,
>  				struct drm_plane *plane)
> @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
> drm_device *dev,
>  		return -ENOMEM;
>  	plane->degamma_lut_size_property = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_CTM", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->ctm_property = prop;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 28357ac..5143277 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -183,6 +183,14 @@ struct drm_plane_state {
>  	struct drm_property_blob *degamma_lut;
>  
>  	/**
> +	 * @ctm:
> +	 *
> +	 * Color transformation matrix. See
> drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is a &struct drm_color_ctm.
> +	 */
> +	struct drm_property_blob *ctm;
> +
> +	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-
> free conditions,
>  	 * and for async plane updates.
>  	 *
> @@ -698,6 +706,13 @@ struct drm_plane {
>  	 * size of the degamma LUT as supported by the driver (read-
> only).
>  	 */
>  	struct drm_property *degamma_lut_size_property;
> +
> +	/**
> +	 * @plane_ctm_property: Optional Plane property to set the
> +	 * matrix used to convert colors after the lookup in the
> +	 * degamma LUT.
> +	 */
> +	struct drm_property *ctm_property;
> 
I'm assuming degamma is done before converting with CTM.

But how does this interact with YUV formats? I'm not sure this is
explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
tables.

There's also a enum drm_color_encoding and a enum drm_color_range. Are
these ignored if CTM is set? Would be good to mention at least.

~Maarten

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

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

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

Re: [Intel-gfx] [RFC v4 3/8] drm: Add Plane CTM property

[Ville Syrjälä]

On Wed, Aug 22, 2018 at 08:40:19AM +0000, Lankhorst, Maarten wrote:
> fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
> > Add a blob property for plane CSC usage.
> > 
> > v2: Rebase
> > 
> > v3: Fixed Sean, Paul's review comments. Moved the property from
> > mode_config to drm_plane. Created a helper function to instantiate
> > these properties and removed from drm_mode_create_standard_properties
> > Added property documentation as suggested by Daniel, Vetter.
> > 
> > v4: Rebase
> > 
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  Documentation/gpu/drm-kms.rst       |  3 +++
> >  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
> >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> >  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
> >  include/drm/drm_plane.h             | 15 +++++++++++++++
> >  5 files changed, 44 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-
> > kms.rst
> > index 8b10b12..16d6d8d 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -560,6 +560,9 @@ Plane Color Management Properties
> >  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
> >     :doc: degamma_lut_size_property
> >  
> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> > +   :doc: ctm_property
> > +
> >  Tile Group Property
> >  -------------------
> >  
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c
> > index f8cad9b..ddda935 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct
> > drm_plane *plane,
> >  					&replaced);
> >  		state->color_mgmt_changed |= replaced;
> >  		return ret;
> > +	} else if (property == plane->ctm_property) {
> > +		ret = drm_atomic_replace_property_blob_from_id(dev,
> > +					&state->ctm,
> > +					val,
> > +					sizeof(struct
> > drm_color_ctm), -1,
> > +					&replaced);
> > +		state->color_mgmt_changed |= replaced;
> > +		return ret;
> >  	} else if (plane->funcs->atomic_set_property) {
> >  		return plane->funcs->atomic_set_property(plane,
> > state,
> >  				property, val);
> > @@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct
> > drm_plane *plane,
> >  	} else if (property == plane->degamma_lut_property) {
> >  		*val = (state->degamma_lut) ?
> >  			state->degamma_lut->base.id : 0;
> > +	} else if (property == plane->ctm_property) {
> > +		*val = (state->ctm) ? state->ctm->base.id : 0;
> >  	} else if (plane->funcs->atomic_get_property) {
> >  		return plane->funcs->atomic_get_property(plane,
> > state, property, val);
> >  	} else {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 67c5b51..866181f 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3616,6 +3616,9 @@ void
> > __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> >  
> >  	if (state->degamma_lut)
> >  		drm_property_blob_get(state->degamma_lut);
> > +	if (state->ctm)
> > +		drm_property_blob_get(state->ctm);
> > +
> >  	state->color_mgmt_changed = false;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> > @@ -3663,6 +3666,7 @@ void
> > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
> > *state)
> >  		drm_crtc_commit_put(state->commit);
> >  
> >  	drm_property_blob_put(state->degamma_lut);
> > +	drm_property_blob_put(state->ctm);
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> >  
> > diff --git a/drivers/gpu/drm/drm_plane.c
> > b/drivers/gpu/drm/drm_plane.c
> > index 03e0560..97520b1 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane
> > *plane,
> >   *
> >   * degamma_lut_size_property:
> >   *	Range Property to indicate size of the plane degamma LUT.
> > + *
> > + * ctm_property:
> > + *	Blob property which allows a userspace to provide CTM
> > coefficients
> > + *	to do color space conversion or any other enhancement by
> > doing a
> > + *	matrix multiplication using the h/w CTM processing engine
> >   */
> >  int drm_plane_color_create_prop(struct drm_device *dev,
> >  				struct drm_plane *plane)
> > @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
> > drm_device *dev,
> >  		return -ENOMEM;
> >  	plane->degamma_lut_size_property = prop;
> >  
> > +	prop = drm_property_create(dev,
> > +			DRM_MODE_PROP_BLOB,
> > +			"PLANE_CTM", 0);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +	plane->ctm_property = prop;
> > +
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_plane_color_create_prop);
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index 28357ac..5143277 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -183,6 +183,14 @@ struct drm_plane_state {
> >  	struct drm_property_blob *degamma_lut;
> >  
> >  	/**
> > +	 * @ctm:
> > +	 *
> > +	 * Color transformation matrix. See
> > drm_plane_enable_color_mgmt(). The
> > +	 * blob (if not NULL) is a &struct drm_color_ctm.
> > +	 */
> > +	struct drm_property_blob *ctm;
> > +
> > +	/**
> >  	 * @commit: Tracks the pending commit to prevent use-after-
> > free conditions,
> >  	 * and for async plane updates.
> >  	 *
> > @@ -698,6 +706,13 @@ struct drm_plane {
> >  	 * size of the degamma LUT as supported by the driver (read-
> > only).
> >  	 */
> >  	struct drm_property *degamma_lut_size_property;
> > +
> > +	/**
> > +	 * @plane_ctm_property: Optional Plane property to set the
> > +	 * matrix used to convert colors after the lookup in the
> > +	 * degamma LUT.
> > +	 */
> > +	struct drm_property *ctm_property;
> > 
> I'm assuming degamma is done before converting with CTM.
> 
> But how does this interact with YUV formats? I'm not sure this is
> explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
> tables.

The pipeline is (or at least should be IMO):
ycbcr->rgb -> degamma -> ctm -> gamma

We should document the full pipeline (including the crtc portions) in
some docs.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Brian Starkey]

Hi,

On Wed, Aug 22, 2018 at 12:53:58PM +0300, Ville Syrjälä wrote:
>On Wed, Aug 22, 2018 at 08:40:19AM +0000, Lankhorst, Maarten wrote:
>> fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
>> > Add a blob property for plane CSC usage.
>> >
>> > v2: Rebase
>> >
>> > v3: Fixed Sean, Paul's review comments. Moved the property from
>> > mode_config to drm_plane. Created a helper function to instantiate
>> > these properties and removed from drm_mode_create_standard_properties
>> > Added property documentation as suggested by Daniel, Vetter.
>> >
>> > v4: Rebase
>> >
>> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> > ---
>> >  Documentation/gpu/drm-kms.rst       |  3 +++
>> >  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
>> >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>> >  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
>> >  include/drm/drm_plane.h             | 15 +++++++++++++++
>> >  5 files changed, 44 insertions(+)
>> >
>> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-
>> > kms.rst
>> > index 8b10b12..16d6d8d 100644
>> > --- a/Documentation/gpu/drm-kms.rst
>> > +++ b/Documentation/gpu/drm-kms.rst
>> > @@ -560,6 +560,9 @@ Plane Color Management Properties
>> >  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
>> >     :doc: degamma_lut_size_property
>> >
>> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
>> > +   :doc: ctm_property
>> > +
>> >  Tile Group Property
>> >  -------------------
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic.c
>> > b/drivers/gpu/drm/drm_atomic.c
>> > index f8cad9b..ddda935 100644
>> > --- a/drivers/gpu/drm/drm_atomic.c
>> > +++ b/drivers/gpu/drm/drm_atomic.c
>> > @@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct
>> > drm_plane *plane,
>> >  					&replaced);
>> >  		state->color_mgmt_changed |= replaced;
>> >  		return ret;
>> > +	} else if (property == plane->ctm_property) {
>> > +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> > +					&state->ctm,
>> > +					val,
>> > +					sizeof(struct
>> > drm_color_ctm), -1,
>> > +					&replaced);
>> > +		state->color_mgmt_changed |= replaced;
>> > +		return ret;
>> >  	} else if (plane->funcs->atomic_set_property) {
>> >  		return plane->funcs->atomic_set_property(plane,
>> > state,
>> >  				property, val);
>> > @@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct
>> > drm_plane *plane,
>> >  	} else if (property == plane->degamma_lut_property) {
>> >  		*val = (state->degamma_lut) ?
>> >  			state->degamma_lut->base.id : 0;
>> > +	} else if (property == plane->ctm_property) {
>> > +		*val = (state->ctm) ? state->ctm->base.id : 0;
>> >  	} else if (plane->funcs->atomic_get_property) {
>> >  		return plane->funcs->atomic_get_property(plane,
>> > state, property, val);
>> >  	} else {
>> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> > b/drivers/gpu/drm/drm_atomic_helper.c
>> > index 67c5b51..866181f 100644
>> > --- a/drivers/gpu/drm/drm_atomic_helper.c
>> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> > @@ -3616,6 +3616,9 @@ void
>> > __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>> >
>> >  	if (state->degamma_lut)
>> >  		drm_property_blob_get(state->degamma_lut);
>> > +	if (state->ctm)
>> > +		drm_property_blob_get(state->ctm);
>> > +
>> >  	state->color_mgmt_changed = false;
>> >  }
>> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>> > @@ -3663,6 +3666,7 @@ void
>> > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
>> > *state)
>> >  		drm_crtc_commit_put(state->commit);
>> >
>> >  	drm_property_blob_put(state->degamma_lut);
>> > +	drm_property_blob_put(state->ctm);
>> >  }
>> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>> >
>> > diff --git a/drivers/gpu/drm/drm_plane.c
>> > b/drivers/gpu/drm/drm_plane.c
>> > index 03e0560..97520b1 100644
>> > --- a/drivers/gpu/drm/drm_plane.c
>> > +++ b/drivers/gpu/drm/drm_plane.c
>> > @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane
>> > *plane,
>> >   *
>> >   * degamma_lut_size_property:
>> >   *	Range Property to indicate size of the plane degamma LUT.
>> > + *
>> > + * ctm_property:
>> > + *	Blob property which allows a userspace to provide CTM
>> > coefficients
>> > + *	to do color space conversion or any other enhancement by
>> > doing a
>> > + *	matrix multiplication using the h/w CTM processing engine
>> >   */
>> >  int drm_plane_color_create_prop(struct drm_device *dev,
>> >  				struct drm_plane *plane)
>> > @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
>> > drm_device *dev,
>> >  		return -ENOMEM;
>> >  	plane->degamma_lut_size_property = prop;
>> >
>> > +	prop = drm_property_create(dev,
>> > +			DRM_MODE_PROP_BLOB,
>> > +			"PLANE_CTM", 0);
>> > +	if (!prop)
>> > +		return -ENOMEM;
>> > +	plane->ctm_property = prop;
>> > +
>> >  	return 0;
>> >  }
>> >  EXPORT_SYMBOL(drm_plane_color_create_prop);
>> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> > index 28357ac..5143277 100644
>> > --- a/include/drm/drm_plane.h
>> > +++ b/include/drm/drm_plane.h
>> > @@ -183,6 +183,14 @@ struct drm_plane_state {
>> >  	struct drm_property_blob *degamma_lut;
>> >
>> >  	/**
>> > +	 * @ctm:
>> > +	 *
>> > +	 * Color transformation matrix. See
>> > drm_plane_enable_color_mgmt(). The
>> > +	 * blob (if not NULL) is a &struct drm_color_ctm.
>> > +	 */
>> > +	struct drm_property_blob *ctm;
>> > +
>> > +	/**
>> >  	 * @commit: Tracks the pending commit to prevent use-after-
>> > free conditions,
>> >  	 * and for async plane updates.
>> >  	 *
>> > @@ -698,6 +706,13 @@ struct drm_plane {
>> >  	 * size of the degamma LUT as supported by the driver (read-
>> > only).
>> >  	 */
>> >  	struct drm_property *degamma_lut_size_property;
>> > +
>> > +	/**
>> > +	 * @plane_ctm_property: Optional Plane property to set the
>> > +	 * matrix used to convert colors after the lookup in the
>> > +	 * degamma LUT.
>> > +	 */
>> > +	struct drm_property *ctm_property;
>> >
>> I'm assuming degamma is done before converting with CTM.
>>
>> But how does this interact with YUV formats? I'm not sure this is
>> explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
>> tables.
>
>The pipeline is (or at least should be IMO):
>ycbcr->rgb -> degamma -> ctm -> gamma
>

That's what I'm expecting too.

>We should document the full pipeline (including the crtc portions) in
>some docs.

I fully agree with Ville here, we've been around this loop a few
times, so the full pipeline and expectations need to be captured
in docs (IMO before the properties can be decided).

It's slightly unfortunate that the merged ycbcr->rgb properties only
describe the format of the incoming buffer, without any specification
of what the driver is meant to do with that information (what
conversion is meant to be performed?). Without that, this new CTM
property is somewhat useless, because generic userspace can't tell
what the input data to the matrix is in YUV cases - so we need to
decide and document that too (or change/enhance the API).

Thanks,
-Brian

>
>-- 
>Ville Syrjälä
>Intel
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC v4 3/8] drm: Add Plane CTM property

[Lankhorst, Maarten]

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

ons 2018-08-22 klockan 12:11 +0100 skrev Brian Starkey:
> Hi,
> 
> On Wed, Aug 22, 2018 at 12:53:58PM +0300, Ville Syrjälä wrote:
> > On Wed, Aug 22, 2018 at 08:40:19AM +0000, Lankhorst, Maarten wrote:
> > > fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
> > > > Add a blob property for plane CSC usage.
> > > > 
> > > > v2: Rebase
> > > > 
> > > > v3: Fixed Sean, Paul's review comments. Moved the property from
> > > > mode_config to drm_plane. Created a helper function to
> > > > instantiate
> > > > these properties and removed from
> > > > drm_mode_create_standard_properties
> > > > Added property documentation as suggested by Daniel, Vetter.
> > > > 
> > > > v4: Rebase
> > > > 
> > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > ---
> > > >  Documentation/gpu/drm-kms.rst       |  3 +++
> > > >  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
> > > >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> > > >  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
> > > >  include/drm/drm_plane.h             | 15 +++++++++++++++
> > > >  5 files changed, 44 insertions(+)
> > > > 
> > > > diff --git a/Documentation/gpu/drm-kms.rst
> > > > b/Documentation/gpu/drm-
> > > > kms.rst
> > > > index 8b10b12..16d6d8d 100644
> > > > --- a/Documentation/gpu/drm-kms.rst
> > > > +++ b/Documentation/gpu/drm-kms.rst
> > > > @@ -560,6 +560,9 @@ Plane Color Management Properties
> > > >  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
> > > >     :doc: degamma_lut_size_property
> > > > 
> > > > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> > > > +   :doc: ctm_property
> > > > +
> > > >  Tile Group Property
> > > >  -------------------
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > b/drivers/gpu/drm/drm_atomic.c
> > > > index f8cad9b..ddda935 100644
> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > @@ -917,6 +917,14 @@ static int
> > > > drm_atomic_plane_set_property(struct
> > > > drm_plane *plane,
> > > >  					&replaced);
> > > >  		state->color_mgmt_changed |= replaced;
> > > >  		return ret;
> > > > +	} else if (property == plane->ctm_property) {
> > > > +		ret =
> > > > drm_atomic_replace_property_blob_from_id(dev,
> > > > +					&state->ctm,
> > > > +					val,
> > > > +					sizeof(struct
> > > > drm_color_ctm), -1,
> > > > +					&replaced);
> > > > +		state->color_mgmt_changed |= replaced;
> > > > +		return ret;
> > > >  	} else if (plane->funcs->atomic_set_property) {
> > > >  		return plane->funcs-
> > > > >atomic_set_property(plane,
> > > > state,
> > > >  				property, val);
> > > > @@ -988,6 +996,8 @@ static int
> > > > drm_atomic_plane_set_property(struct
> > > > drm_plane *plane,
> > > >  	} else if (property == plane->degamma_lut_property) {
> > > >  		*val = (state->degamma_lut) ?
> > > >  			state->degamma_lut->base.id : 0;
> > > > +	} else if (property == plane->ctm_property) {
> > > > +		*val = (state->ctm) ? state->ctm->base.id : 0;
> > > >  	} else if (plane->funcs->atomic_get_property) {
> > > >  		return plane->funcs-
> > > > >atomic_get_property(plane,
> > > > state, property, val);
> > > >  	} else {
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index 67c5b51..866181f 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -3616,6 +3616,9 @@ void
> > > > __drm_atomic_helper_plane_duplicate_state(struct drm_plane
> > > > *plane,
> > > > 
> > > >  	if (state->degamma_lut)
> > > >  		drm_property_blob_get(state->degamma_lut);
> > > > +	if (state->ctm)
> > > > +		drm_property_blob_get(state->ctm);
> > > > +
> > > >  	state->color_mgmt_changed = false;
> > > >  }
> > > >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> > > > @@ -3663,6 +3666,7 @@ void
> > > > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
> > > > *state)
> > > >  		drm_crtc_commit_put(state->commit);
> > > > 
> > > >  	drm_property_blob_put(state->degamma_lut);
> > > > +	drm_property_blob_put(state->ctm);
> > > >  }
> > > >  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_plane.c
> > > > b/drivers/gpu/drm/drm_plane.c
> > > > index 03e0560..97520b1 100644
> > > > --- a/drivers/gpu/drm/drm_plane.c
> > > > +++ b/drivers/gpu/drm/drm_plane.c
> > > > @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct
> > > > drm_plane
> > > > *plane,
> > > >   *
> > > >   * degamma_lut_size_property:
> > > >   *	Range Property to indicate size of the plane degamma
> > > > LUT.
> > > > + *
> > > > + * ctm_property:
> > > > + *	Blob property which allows a userspace to provide
> > > > CTM
> > > > coefficients
> > > > + *	to do color space conversion or any other
> > > > enhancement by
> > > > doing a
> > > > + *	matrix multiplication using the h/w CTM processing
> > > > engine
> > > >   */
> > > >  int drm_plane_color_create_prop(struct drm_device *dev,
> > > >  				struct drm_plane *plane)
> > > > @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
> > > > drm_device *dev,
> > > >  		return -ENOMEM;
> > > >  	plane->degamma_lut_size_property = prop;
> > > > 
> > > > +	prop = drm_property_create(dev,
> > > > +			DRM_MODE_PROP_BLOB,
> > > > +			"PLANE_CTM", 0);
> > > > +	if (!prop)
> > > > +		return -ENOMEM;
> > > > +	plane->ctm_property = prop;
> > > > +
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL(drm_plane_color_create_prop);
> > > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > > > index 28357ac..5143277 100644
> > > > --- a/include/drm/drm_plane.h
> > > > +++ b/include/drm/drm_plane.h
> > > > @@ -183,6 +183,14 @@ struct drm_plane_state {
> > > >  	struct drm_property_blob *degamma_lut;
> > > > 
> > > >  	/**
> > > > +	 * @ctm:
> > > > +	 *
> > > > +	 * Color transformation matrix. See
> > > > drm_plane_enable_color_mgmt(). The
> > > > +	 * blob (if not NULL) is a &struct drm_color_ctm.
> > > > +	 */
> > > > +	struct drm_property_blob *ctm;
> > > > +
> > > > +	/**
> > > >  	 * @commit: Tracks the pending commit to prevent use-
> > > > after-
> > > > free conditions,
> > > >  	 * and for async plane updates.
> > > >  	 *
> > > > @@ -698,6 +706,13 @@ struct drm_plane {
> > > >  	 * size of the degamma LUT as supported by the driver
> > > > (read-
> > > > only).
> > > >  	 */
> > > >  	struct drm_property *degamma_lut_size_property;
> > > > +
> > > > +	/**
> > > > +	 * @plane_ctm_property: Optional Plane property to set
> > > > the
> > > > +	 * matrix used to convert colors after the lookup in
> > > > the
> > > > +	 * degamma LUT.
> > > > +	 */
> > > > +	struct drm_property *ctm_property;
> > > > 
> > > 
> > > I'm assuming degamma is done before converting with CTM.
> > > 
> > > But how does this interact with YUV formats? I'm not sure this is
> > > explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
> > > tables.
> > 
> > The pipeline is (or at least should be IMO):
> > ycbcr->rgb -> degamma -> ctm -> gamma
> > 
> 
> That's what I'm expecting too.
> 
> > We should document the full pipeline (including the crtc portions)
> > in
> > some docs.
> 
> I fully agree with Ville here, we've been around this loop a few
> times, so the full pipeline and expectations need to be captured
> in docs (IMO before the properties can be decided).
> 
> It's slightly unfortunate that the merged ycbcr->rgb properties only
> describe the format of the incoming buffer, without any specification
> of what the driver is meant to do with that information (what
> conversion is meant to be performed?). Without that, this new CTM
> property is somewhat useless, because generic userspace can't tell
> what the input data to the matrix is in YUV cases - so we need to
> decide and document that too (or change/enhance the API).
As long as it's documented I'm fine with it. :-)
Also curious on how it interacts with CRTC pipeline.

~Maarten

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

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

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

Re: [Intel-gfx] [RFC v4 3/8] drm: Add Plane CTM property

[Ville Syrjälä]

On Wed, Aug 22, 2018 at 12:11:42PM +0100, Brian Starkey wrote:
> Hi,
> 
> On Wed, Aug 22, 2018 at 12:53:58PM +0300, Ville Syrjälä wrote:
> >On Wed, Aug 22, 2018 at 08:40:19AM +0000, Lankhorst, Maarten wrote:
> >> fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
> >> > Add a blob property for plane CSC usage.
> >> >
> >> > v2: Rebase
> >> >
> >> > v3: Fixed Sean, Paul's review comments. Moved the property from
> >> > mode_config to drm_plane. Created a helper function to instantiate
> >> > these properties and removed from drm_mode_create_standard_properties
> >> > Added property documentation as suggested by Daniel, Vetter.
> >> >
> >> > v4: Rebase
> >> >
> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> > ---
> >> >  Documentation/gpu/drm-kms.rst       |  3 +++
> >> >  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
> >> >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> >> >  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
> >> >  include/drm/drm_plane.h             | 15 +++++++++++++++
> >> >  5 files changed, 44 insertions(+)
> >> >
> >> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-
> >> > kms.rst
> >> > index 8b10b12..16d6d8d 100644
> >> > --- a/Documentation/gpu/drm-kms.rst
> >> > +++ b/Documentation/gpu/drm-kms.rst
> >> > @@ -560,6 +560,9 @@ Plane Color Management Properties
> >> >  .. kernel-doc:: drivers/gpu/drm/drm_plane.c
> >> >     :doc: degamma_lut_size_property
> >> >
> >> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> >> > +   :doc: ctm_property
> >> > +
> >> >  Tile Group Property
> >> >  -------------------
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_atomic.c
> >> > b/drivers/gpu/drm/drm_atomic.c
> >> > index f8cad9b..ddda935 100644
> >> > --- a/drivers/gpu/drm/drm_atomic.c
> >> > +++ b/drivers/gpu/drm/drm_atomic.c
> >> > @@ -917,6 +917,14 @@ static int drm_atomic_plane_set_property(struct
> >> > drm_plane *plane,
> >> >  					&replaced);
> >> >  		state->color_mgmt_changed |= replaced;
> >> >  		return ret;
> >> > +	} else if (property == plane->ctm_property) {
> >> > +		ret = drm_atomic_replace_property_blob_from_id(dev,
> >> > +					&state->ctm,
> >> > +					val,
> >> > +					sizeof(struct
> >> > drm_color_ctm), -1,
> >> > +					&replaced);
> >> > +		state->color_mgmt_changed |= replaced;
> >> > +		return ret;
> >> >  	} else if (plane->funcs->atomic_set_property) {
> >> >  		return plane->funcs->atomic_set_property(plane,
> >> > state,
> >> >  				property, val);
> >> > @@ -988,6 +996,8 @@ static int drm_atomic_plane_set_property(struct
> >> > drm_plane *plane,
> >> >  	} else if (property == plane->degamma_lut_property) {
> >> >  		*val = (state->degamma_lut) ?
> >> >  			state->degamma_lut->base.id : 0;
> >> > +	} else if (property == plane->ctm_property) {
> >> > +		*val = (state->ctm) ? state->ctm->base.id : 0;
> >> >  	} else if (plane->funcs->atomic_get_property) {
> >> >  		return plane->funcs->atomic_get_property(plane,
> >> > state, property, val);
> >> >  	} else {
> >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> > b/drivers/gpu/drm/drm_atomic_helper.c
> >> > index 67c5b51..866181f 100644
> >> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> > @@ -3616,6 +3616,9 @@ void
> >> > __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> >> >
> >> >  	if (state->degamma_lut)
> >> >  		drm_property_blob_get(state->degamma_lut);
> >> > +	if (state->ctm)
> >> > +		drm_property_blob_get(state->ctm);
> >> > +
> >> >  	state->color_mgmt_changed = false;
> >> >  }
> >> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> >> > @@ -3663,6 +3666,7 @@ void
> >> > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
> >> > *state)
> >> >  		drm_crtc_commit_put(state->commit);
> >> >
> >> >  	drm_property_blob_put(state->degamma_lut);
> >> > +	drm_property_blob_put(state->ctm);
> >> >  }
> >> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_plane.c
> >> > b/drivers/gpu/drm/drm_plane.c
> >> > index 03e0560..97520b1 100644
> >> > --- a/drivers/gpu/drm/drm_plane.c
> >> > +++ b/drivers/gpu/drm/drm_plane.c
> >> > @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane
> >> > *plane,
> >> >   *
> >> >   * degamma_lut_size_property:
> >> >   *	Range Property to indicate size of the plane degamma LUT.
> >> > + *
> >> > + * ctm_property:
> >> > + *	Blob property which allows a userspace to provide CTM
> >> > coefficients
> >> > + *	to do color space conversion or any other enhancement by
> >> > doing a
> >> > + *	matrix multiplication using the h/w CTM processing engine
> >> >   */
> >> >  int drm_plane_color_create_prop(struct drm_device *dev,
> >> >  				struct drm_plane *plane)
> >> > @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
> >> > drm_device *dev,
> >> >  		return -ENOMEM;
> >> >  	plane->degamma_lut_size_property = prop;
> >> >
> >> > +	prop = drm_property_create(dev,
> >> > +			DRM_MODE_PROP_BLOB,
> >> > +			"PLANE_CTM", 0);
> >> > +	if (!prop)
> >> > +		return -ENOMEM;
> >> > +	plane->ctm_property = prop;
> >> > +
> >> >  	return 0;
> >> >  }
> >> >  EXPORT_SYMBOL(drm_plane_color_create_prop);
> >> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> >> > index 28357ac..5143277 100644
> >> > --- a/include/drm/drm_plane.h
> >> > +++ b/include/drm/drm_plane.h
> >> > @@ -183,6 +183,14 @@ struct drm_plane_state {
> >> >  	struct drm_property_blob *degamma_lut;
> >> >
> >> >  	/**
> >> > +	 * @ctm:
> >> > +	 *
> >> > +	 * Color transformation matrix. See
> >> > drm_plane_enable_color_mgmt(). The
> >> > +	 * blob (if not NULL) is a &struct drm_color_ctm.
> >> > +	 */
> >> > +	struct drm_property_blob *ctm;
> >> > +
> >> > +	/**
> >> >  	 * @commit: Tracks the pending commit to prevent use-after-
> >> > free conditions,
> >> >  	 * and for async plane updates.
> >> >  	 *
> >> > @@ -698,6 +706,13 @@ struct drm_plane {
> >> >  	 * size of the degamma LUT as supported by the driver (read-
> >> > only).
> >> >  	 */
> >> >  	struct drm_property *degamma_lut_size_property;
> >> > +
> >> > +	/**
> >> > +	 * @plane_ctm_property: Optional Plane property to set the
> >> > +	 * matrix used to convert colors after the lookup in the
> >> > +	 * degamma LUT.
> >> > +	 */
> >> > +	struct drm_property *ctm_property;
> >> >
> >> I'm assuming degamma is done before converting with CTM.
> >>
> >> But how does this interact with YUV formats? I'm not sure this is
> >> explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
> >> tables.
> >
> >The pipeline is (or at least should be IMO):
> >ycbcr->rgb -> degamma -> ctm -> gamma
> >
> 
> That's what I'm expecting too.
> 
> >We should document the full pipeline (including the crtc portions) in
> >some docs.
> 
> I fully agree with Ville here, we've been around this loop a few
> times, so the full pipeline and expectations need to be captured
> in docs (IMO before the properties can be decided).
> 
> It's slightly unfortunate that the merged ycbcr->rgb properties only
> describe the format of the incoming buffer, without any specification
> of what the driver is meant to do with that information (what
> conversion is meant to be performed?).

So far the idea has been that the internal pipeline is assumed to be
full range RGB. If/when someone needs something else they can try to
design a new API for it. If we had tried to deal with ycbcr passthrough
and whatnot when adding the encoding/range props we probably still
wouldn't have landed anything.

But yeah, the docs should of course explicitly state this fact.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [Intel-gfx] [RFC v4 3/8] drm: Add Plane CTM property

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Wednesday, August 22, 2018 6:36 PM
>To: Brian Starkey <brian.starkey@arm.com>
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>;
>dcastagna@chromium.org; intel-gfx@lists.freedesktop.org; dri-
>devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>
>Subject: Re: [Intel-gfx] [RFC v4 3/8] drm: Add Plane CTM property
>
>On Wed, Aug 22, 2018 at 12:11:42PM +0100, Brian Starkey wrote:
>> Hi,
>>
>> On Wed, Aug 22, 2018 at 12:53:58PM +0300, Ville Syrjälä wrote:
>> >On Wed, Aug 22, 2018 at 08:40:19AM +0000, Lankhorst, Maarten wrote:
>> >> fre 2018-08-17 klockan 19:48 +0530 skrev Uma Shankar:
>> >> > Add a blob property for plane CSC usage.
>> >> >
>> >> > v2: Rebase
>> >> >
>> >> > v3: Fixed Sean, Paul's review comments. Moved the property from
>> >> > mode_config to drm_plane. Created a helper function to
>> >> > instantiate these properties and removed from
>> >> > drm_mode_create_standard_properties
>> >> > Added property documentation as suggested by Daniel, Vetter.
>> >> >
>> >> > v4: Rebase
>> >> >
>> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> > ---
>> >> >  Documentation/gpu/drm-kms.rst       |  3 +++
>> >> >  drivers/gpu/drm/drm_atomic.c        | 10 ++++++++++
>> >> >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>> >> >  drivers/gpu/drm/drm_plane.c         | 12 ++++++++++++
>> >> >  include/drm/drm_plane.h             | 15 +++++++++++++++
>> >> >  5 files changed, 44 insertions(+)
>> >> >
>> >> > diff --git a/Documentation/gpu/drm-kms.rst
>> >> > b/Documentation/gpu/drm- kms.rst index 8b10b12..16d6d8d 100644
>> >> > --- a/Documentation/gpu/drm-kms.rst
>> >> > +++ b/Documentation/gpu/drm-kms.rst
>> >> > @@ -560,6 +560,9 @@ Plane Color Management Properties  ..
>> >> > kernel-doc:: drivers/gpu/drm/drm_plane.c
>> >> >     :doc: degamma_lut_size_property
>> >> >
>> >> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
>> >> > +   :doc: ctm_property
>> >> > +
>> >> >  Tile Group Property
>> >> >  -------------------
>> >> >
>> >> > diff --git a/drivers/gpu/drm/drm_atomic.c
>> >> > b/drivers/gpu/drm/drm_atomic.c index f8cad9b..ddda935 100644
>> >> > --- a/drivers/gpu/drm/drm_atomic.c
>> >> > +++ b/drivers/gpu/drm/drm_atomic.c
>> >> > @@ -917,6 +917,14 @@ static int
>> >> > drm_atomic_plane_set_property(struct
>> >> > drm_plane *plane,
>> >> >  					&replaced);
>> >> >  		state->color_mgmt_changed |= replaced;
>> >> >  		return ret;
>> >> > +	} else if (property == plane->ctm_property) {
>> >> > +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> > +					&state->ctm,
>> >> > +					val,
>> >> > +					sizeof(struct
>> >> > drm_color_ctm), -1,
>> >> > +					&replaced);
>> >> > +		state->color_mgmt_changed |= replaced;
>> >> > +		return ret;
>> >> >  	} else if (plane->funcs->atomic_set_property) {
>> >> >  		return plane->funcs->atomic_set_property(plane,
>> >> > state,
>> >> >  				property, val);
>> >> > @@ -988,6 +996,8 @@ static int
>> >> > drm_atomic_plane_set_property(struct
>> >> > drm_plane *plane,
>> >> >  	} else if (property == plane->degamma_lut_property) {
>> >> >  		*val = (state->degamma_lut) ?
>> >> >  			state->degamma_lut->base.id : 0;
>> >> > +	} else if (property == plane->ctm_property) {
>> >> > +		*val = (state->ctm) ? state->ctm->base.id : 0;
>> >> >  	} else if (plane->funcs->atomic_get_property) {
>> >> >  		return plane->funcs->atomic_get_property(plane,
>> >> > state, property, val);
>> >> >  	} else {
>> >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> >> > b/drivers/gpu/drm/drm_atomic_helper.c
>> >> > index 67c5b51..866181f 100644
>> >> > --- a/drivers/gpu/drm/drm_atomic_helper.c
>> >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> >> > @@ -3616,6 +3616,9 @@ void
>> >> > __drm_atomic_helper_plane_duplicate_state(struct drm_plane
>> >> > *plane,
>> >> >
>> >> >  	if (state->degamma_lut)
>> >> >  		drm_property_blob_get(state->degamma_lut);
>> >> > +	if (state->ctm)
>> >> > +		drm_property_blob_get(state->ctm);
>> >> > +
>> >> >  	state->color_mgmt_changed = false;  }
>> >> > EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>> >> > @@ -3663,6 +3666,7 @@ void
>> >> > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state
>> >> > *state)
>> >> >  		drm_crtc_commit_put(state->commit);
>> >> >
>> >> >  	drm_property_blob_put(state->degamma_lut);
>> >> > +	drm_property_blob_put(state->ctm);
>> >> >  }
>> >> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>> >> >
>> >> > diff --git a/drivers/gpu/drm/drm_plane.c
>> >> > b/drivers/gpu/drm/drm_plane.c index 03e0560..97520b1 100644
>> >> > --- a/drivers/gpu/drm/drm_plane.c
>> >> > +++ b/drivers/gpu/drm/drm_plane.c
>> >> > @@ -489,6 +489,11 @@ int drm_mode_plane_set_obj_prop(struct
>> >> > drm_plane *plane,
>> >> >   *
>> >> >   * degamma_lut_size_property:
>> >> >   *	Range Property to indicate size of the plane degamma LUT.
>> >> > + *
>> >> > + * ctm_property:
>> >> > + *	Blob property which allows a userspace to provide CTM
>> >> > coefficients
>> >> > + *	to do color space conversion or any other enhancement by
>> >> > doing a
>> >> > + *	matrix multiplication using the h/w CTM processing engine
>> >> >   */
>> >> >  int drm_plane_color_create_prop(struct drm_device *dev,
>> >> >  				struct drm_plane *plane)
>> >> > @@ -509,6 +514,13 @@ int drm_plane_color_create_prop(struct
>> >> > drm_device *dev,
>> >> >  		return -ENOMEM;
>> >> >  	plane->degamma_lut_size_property = prop;
>> >> >
>> >> > +	prop = drm_property_create(dev,
>> >> > +			DRM_MODE_PROP_BLOB,
>> >> > +			"PLANE_CTM", 0);
>> >> > +	if (!prop)
>> >> > +		return -ENOMEM;
>> >> > +	plane->ctm_property = prop;
>> >> > +
>> >> >  	return 0;
>> >> >  }
>> >> >  EXPORT_SYMBOL(drm_plane_color_create_prop);
>> >> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> >> > index 28357ac..5143277 100644
>> >> > --- a/include/drm/drm_plane.h
>> >> > +++ b/include/drm/drm_plane.h
>> >> > @@ -183,6 +183,14 @@ struct drm_plane_state {
>> >> >  	struct drm_property_blob *degamma_lut;
>> >> >
>> >> >  	/**
>> >> > +	 * @ctm:
>> >> > +	 *
>> >> > +	 * Color transformation matrix. See
>> >> > drm_plane_enable_color_mgmt(). The
>> >> > +	 * blob (if not NULL) is a &struct drm_color_ctm.
>> >> > +	 */
>> >> > +	struct drm_property_blob *ctm;
>> >> > +
>> >> > +	/**
>> >> >  	 * @commit: Tracks the pending commit to prevent use-after-
>> >> > free conditions,
>> >> >  	 * and for async plane updates.
>> >> >  	 *
>> >> > @@ -698,6 +706,13 @@ struct drm_plane {
>> >> >  	 * size of the degamma LUT as supported by the driver (read-
>> >> > only).
>> >> >  	 */
>> >> >  	struct drm_property *degamma_lut_size_property;
>> >> > +
>> >> > +	/**
>> >> > +	 * @plane_ctm_property: Optional Plane property to set the
>> >> > +	 * matrix used to convert colors after the lookup in the
>> >> > +	 * degamma LUT.
>> >> > +	 */
>> >> > +	struct drm_property *ctm_property;
>> >> >
>> >> I'm assuming degamma is done before converting with CTM.
>> >>
>> >> But how does this interact with YUV formats? I'm not sure this is
>> >> explicitly defined. I'm assuming R->Cr G -> Y, B = Cb for degamma
>> >> tables.
>> >
>> >The pipeline is (or at least should be IMO):
>> >ycbcr->rgb -> degamma -> ctm -> gamma
>> >
>>
>> That's what I'm expecting too.
>>
>> >We should document the full pipeline (including the crtc portions) in
>> >some docs.
>>
>> I fully agree with Ville here, we've been around this loop a few
>> times, so the full pipeline and expectations need to be captured in
>> docs (IMO before the properties can be decided).
>>
>> It's slightly unfortunate that the merged ycbcr->rgb properties only
>> describe the format of the incoming buffer, without any specification
>> of what the driver is meant to do with that information (what
>> conversion is meant to be performed?).
>
>So far the idea has been that the internal pipeline is assumed to be full range RGB.
>If/when someone needs something else they can try to design a new API for it. If
>we had tried to deal with ycbcr passthrough and whatnot when adding the
>encoding/range props we probably still wouldn't have landed anything.
>

I agree, blending limited range layers with Full range is slightly tricky as of
now. I believe we need to convert the Limited range to full range explicitly and then
flip to display (atleast when we want all the color hardware blocks to come in play)

>But yeah, the docs should of course explicitly state this fact.
>

From the pipeline perspective, Ville is right (ycbcr->rgb -> degamma -> ctm -> gamma )
and that's how it works. I will try to document these as part of kernel docs under
color management to give more clarity and avoid any kind of ambiguity.

Regards,
Uma Shankar

>--
>Ville Syrjälä
>Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Shankar, Uma]

>-----Original Message-----
>From: Alexandru-Cosmin Gheorghe [mailto:Alexandru-
>Cosmin.Gheorghe@arm.com]
>Sent: Tuesday, August 21, 2018 3:00 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>dcastagna@chromium.org; emil.l.velikov@gmail.com; seanpaul@chromium.org;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; nd@arm.com
>Subject: Re: [RFC v4 1/8] drm: Add Enhanced Gamma LUT precision structure
>
>Hi Uma,
>
>On Fri, Aug 17, 2018 at 07:48:44PM +0530, Uma Shankar wrote:
>> Existing LUT precision structure is having only 16 bit precision. This
>> is not enough for upcoming enhanced hardwares and advance usecases
>> like HDR processing. Hence added a new structure with 32 bit precision
>> values. Also added the code, for extracting the same from values
>> passed from userspace.
>>
>> v4: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/drm_plane.c | 19 +++++++++++++++++++
>> include/uapi/drm/drm_mode.h | 15 +++++++++++++++
>>  2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index 6153cbd..cd71fd0 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -430,6 +430,25 @@ void drm_plane_force_disable(struct drm_plane
>> *plane)  }  EXPORT_SYMBOL(drm_plane_force_disable);
>>
>> +/*
>> + * Added to accommodate enhanced LUT precision.
>> + * Max LUT precision is 32 bits.
>> + */
>> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t
>> +bit_precision)
>
>You should declare this function in a header file as well.

Thanks Alexandru for the review. Sure, will fix that and resend.

>With that fixed
>Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
>
>
>> +{
>> +	uint32_t val = user_input;
>> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
>> +
>> +	/* Round only if we're not using full precision. */
>> +	if (bit_precision < 32) {
>> +		val += 1UL << (32 - bit_precision - 1);
>> +		val >>= 32 - bit_precision;
>> +	}
>> +
>> +	return clamp_val(val, 0, max);
>> +}
>> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
>> +
>>  /**
>>   * drm_mode_plane_set_obj_prop - set the value of a property
>>   * @plane: drm plane object to set property value for diff --git
>> a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index
>> 8d67243..874407b 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -629,6 +629,21 @@ struct drm_color_lut {
>>  	__u16 reserved;
>>  };
>>
>> +/*
>> + * Creating 32 bit palette entries for better data
>> + * precision. This will be required for HDR and
>> + * similar color processing usecases.
>> + */
>> +struct drm_color_lut_ext {
>> +	/*
>> +	 * Data is U0.32 fixed point format.
>> +	 */
>> +	__u32 red;
>> +	__u32 green;
>> +	__u32 blue;
>> +	__u32 reserved;
>> +};
>> +
>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>--
>Cheers,
>Alex G
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Shankar, Uma]

>-----Original Message-----
>From: Alexandru-Cosmin Gheorghe [mailto:Alexandru-
>Cosmin.Gheorghe@arm.com]
>Sent: Tuesday, August 21, 2018 3:26 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>dcastagna@chromium.org; emil.l.velikov@gmail.com; seanpaul@chromium.org;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; nd@arm.com
>Subject: Re: [RFC v4 7/8] drm/i915: Implement Plane Gamma for Bdw and Gen9
>platforms
>
>Hi Uma,
>
>On Fri, Aug 17, 2018 at 07:48:50PM +0530, Uma Shankar wrote:
>> Implement Plane Gamma feature for BDW and Gen9 platforms.
>>
>> v2: Used newly added drm_color_lut_ext structure for enhanced
>> precision for Gamma LUT entries.
>>
>> v3: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_pci.c      |  5 +++-
>>  drivers/gpu/drm/i915/i915_reg.h      | 25 ++++++++++++++++
>>  drivers/gpu/drm/i915/intel_color.c   | 58
>++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c |  4 +++
>> drivers/gpu/drm/i915/intel_sprite.c  |  4 +++
>>  5 files changed, 95 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c
>> b/drivers/gpu/drm/i915/i915_pci.c index e931b48..40de78c 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -54,7 +54,10 @@
>>  	.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET,
>> IVB_CURSOR_C_OFFSET }
>>
>>  #define BDW_COLORS \
>> -	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
>> +	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }, \
>> +	.plane_color = { .plane_degamma_lut_size = 0, \
>> +			 .plane_gamma_lut_size = 16 }
>> +
>>  #define CHV_COLORS \
>>  	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }  #define
>> GLK_COLORS \ diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 0c9f03d..2db6a84 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -172,6 +172,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t
>> reg)  #define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)  #define
>> _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>>
>> +/* Plane Gamma Registers */
>> +#define _MMIO_PLANE_GAMC(plane, i, a, b)  _MMIO(_PIPE(plane, a, b) +
>> +(i) * 4) #define _MMIO_PLANE_GAMC16(plane, i, a, b)
>> +_MMIO(_PIPE(plane, a, b) + (i) * 4)
>> +
>>  #define __MASKED_FIELD(mask, value) ((mask) << 16 | (value))
>>  #define _MASKED_FIELD(mask, value) ({
>	   \
>>  	if (__builtin_constant_p(mask))					   \
>> @@ -9713,6 +9717,27 @@ enum skl_power_gate {
>>  #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe,
>_PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>>  #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe,
>_PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>>
>> +/* Plane Gamma in Gen9+ */
>> +#define _PLANE_GAMC_1_A	0x701d0
>> +#define _PLANE_GAMC_1_B	0x711d0
>> +#define _PLANE_GAMC_2_A	0x702d0
>> +#define _PLANE_GAMC_2_B	0x712d0
>> +#define _PLANE_GAMC_1(pipe)	_PIPE(pipe, _PLANE_GAMC_1_A,
>_PLANE_GAMC_1_B)
>> +#define _PLANE_GAMC_2(pipe)	_PIPE(pipe, _PLANE_GAMC_2_A,
>_PLANE_GAMC_2_B)
>> +#define PLANE_GAMC(pipe, plane, i)	\
>> +	_MMIO_PLANE_GAMC(plane, i, _PLANE_GAMC_1(pipe),
>_PLANE_GAMC_2(pipe))
>> +
>> +#define _PLANE_GAMC16_1_A	0x70210
>> +#define _PLANE_GAMC16_1_B	0x71210
>> +#define _PLANE_GAMC16_2_A	0x70310
>> +#define _PLANE_GAMC16_2_B	0x71310
>> +#define _PLANE_GAMC16_1(pipe)	_PIPE(pipe, _PLANE_GAMC16_1_A, \
>> +				     _PLANE_GAMC16_1_B)
>> +#define _PLANE_GAMC16_2(pipe)	_PIPE(pipe, _PLANE_GAMC16_2_A, \
>> +				     _PLANE_GAMC16_2_B)
>> +#define PLANE_GAMC16(pipe, plane, i) _MMIO_PLANE_GAMC16(plane, i, \
>> +				_PLANE_GAMC16_1(pipe),
>_PLANE_GAMC16_2(pipe))
>> +
>>  /* pipe CSC & degamma/gamma LUTs on CHV */
>>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index fb8402f..2b5c0cd 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -492,6 +492,59 @@ static void broadwell_load_luts(struct drm_crtc_state
>*state)
>>  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);  }
>>
>> +static void bdw_load_plane_gamma_lut(const struct drm_plane_state *state,
>> +				     u32 offset)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
>> +	enum pipe pipe = to_intel_plane(state->plane)->pipe;
>> +	enum plane_id plane = to_intel_plane(state->plane)->id;
>> +	uint32_t i, lut_size =
>> +			INTEL_INFO(dev_priv)-
>>plane_color.plane_gamma_lut_size;
>> +
>> +	if (state->gamma_lut) {
>> +		struct drm_color_lut_ext *lut =
>> +			(struct drm_color_lut_ext *) state->gamma_lut->data;
>> +
>> +		for (i = 0; i < lut_size; i++) {
>> +			uint32_t word =
>> +			(drm_color_lut_extract(lut[i].red, 10) << 20) |
>> +			(drm_color_lut_extract(lut[i].green, 10) << 10) |
>> +			drm_color_lut_extract(lut[i].blue, 10);
>
>
>Shouldn't you be using drm_color_lut_extract_ext ?

 For now, drm_color_lut_extract was enough. But will use the newly defined
drm_color_lut_extract_ext to be future proof. Thanks for pointing this out.

Regards,
Uma Shankar

>
>> +
>> +			I915_WRITE(PLANE_GAMC(pipe, plane, i), word);
>> +		}
>> +
>> +		/* Program the max register to clamp values > 1.0. */
>> +		i = lut_size - 1;
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 0),
>> +			   drm_color_lut_extract(lut[i].red, 16));
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 1),
>> +			   drm_color_lut_extract(lut[i].green, 16));
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 2),
>> +			   drm_color_lut_extract(lut[i].blue, 16));
>> +	} else {
>> +		for (i = 0; i < lut_size; i++) {
>> +			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>> +
>> +			I915_WRITE(PLANE_GAMC(pipe, plane, i),
>> +				   (v << 20) | (v << 10) | v);
>> +		}
>> +
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 0), (1 << 16) - 1);
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 1), (1 << 16) - 1);
>> +		I915_WRITE(PLANE_GAMC16(pipe, plane, 2), (1 << 16) - 1);
>> +	}
>> +}
>> +
>> +/* Loads the palette/gamma unit for the CRTC on Broadwell+. */ static
>> +void broadwell_load_plane_luts(const struct drm_plane_state *state) {
>> +	struct drm_i915_private *dev_priv = to_i915(state->plane->dev);
>> +
>> +	bdw_load_plane_gamma_lut(state,
>> +		INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size);
>> +}
>> +
>>  static void glk_load_degamma_lut(struct drm_crtc_state *state)  {
>>  	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev); @@
>> -648,6 +701,11 @@ void intel_plane_color_init(struct drm_plane *plane)
>>
>>  	drm_plane_color_create_prop(plane->dev, plane);
>>
>> +	if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
>> +		IS_BROXTON(dev_priv)) {
>> +		dev_priv->display.load_plane_luts = broadwell_load_plane_luts;
>> +	}
>> +
>>  	/* Enable color management support when we have degamma & gamma
>LUTs. */
>>  	if (INTEL_INFO(dev_priv)->plane_color.plane_degamma_lut_size != 0 &&
>>  	    INTEL_INFO(dev_priv)->plane_color.plane_gamma_lut_size != 0)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 690e1e8..2d15ac2 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13803,6 +13803,10 @@ bool skl_plane_has_planar(struct
>drm_i915_private *dev_priv,
>>  						  DRM_COLOR_YCBCR_BT709,
>>
>DRM_COLOR_YCBCR_LIMITED_RANGE);
>>
>> +	/* Add Plane Color properties */
>> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
>> +		intel_plane_color_init(&primary->base);
>> +
>>  	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>>
>>  	return primary;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index f7026e8..0eeb1d3 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -1650,6 +1650,10 @@ struct intel_plane *
>>  					  DRM_COLOR_YCBCR_BT709,
>>
>DRM_COLOR_YCBCR_LIMITED_RANGE);
>>
>> +	/* Add Plane Color properties */
>> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
>> +		intel_plane_color_init(&intel_plane->base);
>> +
>>  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>>
>>  	return intel_plane;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>--
>Cheers,
>Alex G
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx