[PATCH 00/16] Color Manager Implementation

[PATCH 00/16] Color Manager Implementation

[Kausal Malladi]

This patch set adds Color Manager implementation in DRM layer. Color Manager is
an extension in DRM framework to support color correction/enhancement. Various
Hardware platforms can support several color correction capabilities.
Color Manager provides abstraction of these capabilities and allows a
user space UI agent to correct/enhance the display using the DRM property interface.

How is this going to work?
==========================
1. This patch series adds a few new properties in DRM framework. These properties are:
	a. color_capabilities property (type blob)
	b. Color Transformation Matrix property for corrections like CSC (called CTM, type blob)
	c. Palette correction properties for corrections like gamma fixup (called palette_correction, type blob)
2. Also, this patch series adds few structures to indicate specifications of a property like size, no_of_samples for correction etc.
3. These properties are present in mode_config.
4. When the platform's display driver loads, it fills up the values of color_capabilities property using the standard structures (added in step 2).
   For example, Intel's I915 driver adds following color correction capabilities:
	a. gamma correction capability as palette correction property, with 257 correction coefficients and a max/min value
	b. csc correction capability as CTM correction property, with 3x3 transformation matrix values and max/min values
5. Now when userspace comes up, it queries the platform's color capabilities by doing a get_property() on color_capabilities DRM property
6. Reading the blob, the userspace understands the color capabilities of the platform.
   For example, userspace will understand it can support:
	a. palette_correction with 257 coefficients
	b. CSC correction with 3x3 = 9 values
7. To set color correction values, userspace:
	a. creates a blob using the create_blob_ioctl in standard palette_correction structure format, with the correction values
	b. calls the set_property_ioctl with the blob_id as value for the property
8. Driver refers to the blob, gets the correction values and applies the correction in HW.
9. To get currently applied color correction values, userspace:
	a. calls a get_property_ioctl on that color property
	b. gets the blob_id for the currently applied correction from DRM infrastructure
	c. gets the blob using get_blob_ioctl and hence the currently applied values

That's all! :)

About the patch series:
=======================
The first patch adds fix for ensuring atomic commit for CRTC properties.
The subsequent patches add code for the framework, which will be common across all the Hardware platforms.
1. Create Color Management DRM properties
2. Attach color properties to CRTC
3. Add structures at DRM level for color management

The generic properties supported in this patch set are
1. Color Transformation Matrix (CTM) for generic usecases like color space conversion and Gamut Mapping
2. Palette correction before CTM for specific usecases like DeGamma color correction
3. Palette correction after CTM for specific usecases like Gamma color correction

In the subsequent patches, we are adding support for Gamma, DeGamma and CSC color properties for one of the Intel platforms, CHV, as an example.

Our thanks to all the reviewers who have given valuable comments in terms of design and implementation to our previous sets of patches.
Special mention of thanks should go to Matt Roper for all his inputs/suggestions in implementation of this module, using DRM atomic CRTC commit path.

Kausal Malladi (15):
  drm: Create Color Management DRM properties
  drm/i915: Attach color properties to CRTC
  drm: Add structure for querying palette color capabilities
  drm: Export drm_property_replace_global_blob function
  drm/i915: Load gamma color capabilities for CHV CRTC
  drm/i915: Add atomic set property interface for CRTC
  drm: Add blob properties to CRTC state for color properties
  drm: Add structures to set/get a palette color property
  drm/i915: Add set_property handler for pipe Gamma correction on
    CHV/BSW
  drm/i915: Add pipe level Gamma correction for CHV/BSW
  drm/i915: Add set_property handler for pipe deGamma correction on
    CHV/BSW
  drm/i915: Add DeGamma correction for CHV/BSW
  drm: Add structure for set/get a CTM color property
  drm/i915: Add set_property handler for CSC correction on CHV/BSW
  drm/i915: Add CSC correction for CHV/BSW

Matt Roper (1):
  drm/i915: Atomic commit path fix for CRTC properties

 drivers/gpu/drm/drm_crtc.c                 |  29 +-
 drivers/gpu/drm/i915/Makefile              |   3 +-
 drivers/gpu/drm/i915/i915_reg.h            |  22 ++
 drivers/gpu/drm/i915/intel_atomic.c        |  33 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 535 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  79 +++++
 drivers/gpu/drm/i915/intel_display.c       |   7 +
 drivers/gpu/drm/i915/intel_drv.h           |  19 +
 include/drm/drm_crtc.h                     |  17 +
 include/uapi/drm/drm.h                     |  50 +++
 10 files changed, 792 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h

-- 
2.4.5

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

[PATCH 01/16] drm/i915: Atomic commit path fix for CRTC properties

[Kausal Malladi]

From: Matt Roper <matthew.d.roper@intel.com>

The intel_atomic_check() function had some simple testing to make
sure that an atomic update isn't updating more than one CRTC at a time.
The logic assumed that a plane was always being updated, so it figured
out the "nuclear pipe" from the first plane it encountered and just made
sure that all CRTC's matched that nuclear pipe.
But when someone is adding CRTC properties, it's valid to update
just a CRTC property and not have any plane updates in the transaction,
which means the initial 'nuclear_pipe' value was never set.

This patch adds a fix for it and makes sure CRTC properties also get
through atomic commit path.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 5c79a31..6ce6c3d 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -80,6 +80,15 @@ int intel_atomic_check(struct drm_device *dev,
 	state->allow_modeset = false;
 	for (i = 0; i < ncrtcs; i++) {
 		struct intel_crtc *crtc = to_intel_crtc(state->crtcs[i]);
+
+		/*
+		 * If we're only updating CRTC properties, we may not have
+		 * established a 'nuclear pipe' yet.  In that case, the first
+		 * CRTC we encounter here should be taken as the 'nuclear
+		 * pipe.'
+		 */
+		if (nuclear_pipe == INVALID_PIPE && crtc)
+			nuclear_pipe = crtc->pipe;
 		if (crtc)
 			memset(&crtc->atomic, 0, sizeof(crtc->atomic));
 		if (crtc && crtc->pipe != nuclear_pipe)
-- 
2.4.5

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

[PATCH 02/16] drm: Create Color Management DRM properties

[Kausal Malladi]

Color Management is an extension to Kernel display framework. It allows
abstraction of hardware color correction and enhancement capabilities by
virtue of DRM properties.

This patch initializes color management framework by :
1. Introducing new pointers in DRM mode_config structure to
   carry CTM and Palette color correction properties.
2. Creating these DRM properties in DRM standard properties creation
   sequence.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a717d18..21da106 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1462,6 +1462,32 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	/* Color Management properties */
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB | DRM_MODE_PROP_IMMUTABLE,
+			"COLOR_CAPABILITIES", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_color_capabilities = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB, "PALETTE_AFTER_CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_palette_after_ctm = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB, "PALETTE_BEFORE_CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_palette_before_ctm = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB, "CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_ctm = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 57ca8cc..408d39a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1178,6 +1178,12 @@ struct drm_mode_config {
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
 
+	/* Color Management Properties */
+	struct drm_property *prop_color_capabilities;
+	struct drm_property *prop_palette_before_ctm;
+	struct drm_property *prop_palette_after_ctm;
+	struct drm_property *prop_ctm;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
-- 
2.4.5

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

[PATCH 03/16] drm/i915: Attach color properties to CRTC

[Kausal Malladi]

This patch does the following:
1. Adds new files intel_color_manager(.c/.h)
2. Attaches color properties to CRTC while initialization

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/Makefile              |  3 +-
 drivers/gpu/drm/i915/intel_color_manager.c | 49 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h | 29 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c       |  2 ++
 drivers/gpu/drm/i915/intel_drv.h           |  4 +++
 5 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index de21965..ad928d8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -56,7 +56,8 @@ i915-y += intel_audio.o \
 	  intel_overlay.o \
 	  intel_psr.o \
 	  intel_sideband.o \
-	  intel_sprite.o
+	  intel_sprite.o \
+	  intel_color_manager.o
 i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
 i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
new file mode 100644
index 0000000..baa4536
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma <shashank.sharma@intel.com>
+ * Kausal Malladi <Kausal.Malladi@intel.com>
+ */
+
+#include "intel_color_manager.h"
+
+void intel_attach_color_properties_to_crtc(struct drm_device *dev,
+		struct drm_mode_object *mode_obj)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (IS_CHERRYVIEW(dev)) {
+		if (config->prop_color_capabilities)
+			drm_object_attach_property(mode_obj,
+					config->prop_color_capabilities, 0);
+		if (config->prop_palette_before_ctm)
+			drm_object_attach_property(mode_obj,
+					config->prop_palette_before_ctm, 0);
+		if (config->prop_palette_after_ctm)
+			drm_object_attach_property(mode_obj,
+					config->prop_palette_after_ctm, 0);
+		if (config->prop_ctm)
+			drm_object_attach_property(mode_obj,
+					config->prop_ctm, 0);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
new file mode 100644
index 0000000..04c921d
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma <shashank.sharma@intel.com>
+ * Kausal Malladi <Kausal.Malladi@intel.com>
+ */
+#include <drm/drmP.h>
+#include <drm/drm_crtc_helper.h>
+#include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb58cb6..b6e1dc5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14111,6 +14111,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	intel_crtc->wm.cxsr_allowed = true;
 
+	intel_attach_color_properties_to_crtc(dev, &intel_crtc->base.base);
+
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b9c01c5..05c809b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1446,4 +1446,8 @@ void intel_plane_destroy_state(struct drm_plane *plane,
 			       struct drm_plane_state *state);
 extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 
+/* intel_color_manager.c */
+void intel_attach_color_properties_to_crtc(struct drm_device *dev,
+		struct drm_mode_object *mode_obj);
+
 #endif /* __INTEL_DRV_H__ */
-- 
2.4.5

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

[PATCH 04/16] drm: Add structure for querying palette color capabilities

[Kausal Malladi]

The DRM color management framework is targeting various hardware
platforms and drivers. Different platforms can have different color
correction and enhancement capabilities.

A commom user space application can query these capabilities using the
DRM property interface. Each driver can fill this property with its
platform's palette color capabilities.

This patch adds new structure in DRM layer for querying palette color
capabilities. This structure will be used by all user space
agents to configure appropriate color configurations.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 include/uapi/drm/drm.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..e3c642f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -829,6 +829,17 @@ struct drm_event_vblank {
 	__u32 reserved;
 };
 
+struct drm_palette_caps {
+	/* Structure version. Should be 1 currently */
+	__u32 version;
+	/* For padding and future use */
+	__u32 reserved;
+	/* This may be 0 if not supported. e.g. plane palette or VLV pipe */
+	__u32 num_samples_before_ctm;
+	/* This will be non-zero for pipe. May be zero for planes on some HW */
+	__u32 num_samples_after_ctm;
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
2.4.5

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

[PATCH 05/16] drm: Export drm_property_replace_global_blob function

[Kausal Malladi]

drm_property_replace_global_blob() is getting used by many wrapper
functions to replace an existing blob with new values. Because this
function was static, modules are forced to create wrapper functions in
same file. Exporting this function will remove need for such wrapper
functions.

This patch makes the function drm_property_replace_global_blob() be
accessible globally.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 3 ++-
 include/drm/drm_crtc.h     | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 21da106..a83154b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4462,7 +4462,7 @@ EXPORT_SYMBOL(drm_property_lookup_blob);
  * a completely atomic update. The access to path_blob_ptr is protected by the
  * caller holding a lock on the connector.
  */
-static int drm_property_replace_global_blob(struct drm_device *dev,
+int drm_property_replace_global_blob(struct drm_device *dev,
                                             struct drm_property_blob **replace,
                                             size_t length,
                                             const void *data,
@@ -4506,6 +4506,7 @@ err_created:
 	drm_property_unreference_blob(new_blob);
 	return ret;
 }
+EXPORT_SYMBOL(drm_property_replace_global_blob);
 
 /**
  * drm_mode_getblob_ioctl - get the contents of a blob property value
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 408d39a..821424e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1541,6 +1541,12 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device
 							      unsigned int supported_rotations);
 extern unsigned int drm_rotation_simplify(unsigned int rotation,
 					  unsigned int supported_rotations);
+extern int drm_property_replace_global_blob(struct drm_device *dev,
+					struct drm_property_blob **replace,
+					size_t length,
+					const void *data,
+					struct drm_mode_object *obj_holds_id,
+					struct drm_property *prop_holds_id);
 
 /* Helpers */
 
-- 
2.4.5

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

[PATCH 06/16] drm/i915: Load gamma color capabilities for CHV CRTC

[Kausal Malladi]

As per Color Manager design, each driver is responsible to load its
palette color correction and enhancement capabilities in the form of
a DRM blob property, so that user space can query and read.

This patch loads all CHV platform specific gamma color capabilities
for CRTC into a blob that can be accessible by user space to
query capabilities via DRM property interface.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_color_manager.c | 48 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  4 +++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index baa4536..def20d0f 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,15 +27,63 @@
 
 #include "intel_color_manager.h"
 
+int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
+		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
+{
+	struct drm_property_blob *blob = NULL;
+	struct drm_mode_config *config = &dev->mode_config;
+	int ret;
+
+	/*
+	 * This function exposes best capability for DeGamma and Gamma
+	 * For CHV, the DeGamma LUT has 65 entries
+	 * and the best Gamma capability has 257 entries (CGM unit)
+	 */
+	palette_caps->version = CHV_PALETTE_STRUCT_VERSION;
+	palette_caps->num_samples_before_ctm =
+		CHV_DEGAMMA_MAX_VALS;
+	palette_caps->num_samples_after_ctm =
+		CHV_10BIT_GAMMA_MAX_VALS;
+
+	ret = drm_property_replace_global_blob(dev, &blob,
+			sizeof(struct drm_palette_caps),
+			(const void *)palette_caps,
+			&crtc->base, config->prop_color_capabilities);
+	if (ret) {
+		DRM_ERROR("Error updating Gamma blob\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+int get_pipe_gamma_capabilities(struct drm_device *dev,
+		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
+{
+	if (IS_CHERRYVIEW(dev))
+		return get_chv_pipe_gamma_capabilities(dev, palette_caps, crtc);
+	return -EINVAL;
+}
+
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 		struct drm_mode_object *mode_obj)
 {
 	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_palette_caps *palette_caps;
+	struct drm_crtc *crtc;
+	int ret;
 
 	if (IS_CHERRYVIEW(dev)) {
+		crtc = obj_to_crtc(mode_obj);
 		if (config->prop_color_capabilities)
 			drm_object_attach_property(mode_obj,
 					config->prop_color_capabilities, 0);
+		palette_caps = kzalloc(sizeof(struct drm_palette_caps),
+				GFP_KERNEL);
+		ret = get_pipe_gamma_capabilities(dev, palette_caps, crtc);
+		if (ret)
+			DRM_ERROR("Error getting gamma capability for CHV\n");
+
 		if (config->prop_palette_before_ctm)
 			drm_object_attach_property(mode_obj,
 					config->prop_palette_before_ctm, 0);
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 04c921d..51aeb91 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -27,3 +27,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include "i915_drv.h"
+
+#define CHV_PALETTE_STRUCT_VERSION		1
+#define CHV_DEGAMMA_MAX_VALS			65
+#define CHV_10BIT_GAMMA_MAX_VALS		257
-- 
2.4.5

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

[PATCH 07/16] drm/i915: Add atomic set property interface for CRTC

[Kausal Malladi]

This patch adds atomic set property interface for Intel CRTC. This
interface will be used to set color correction DRM properties.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  | 11 +++++++++++
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
 3 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 6ce6c3d..d873bda 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -34,6 +34,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_plane_helper.h>
 #include "intel_drv.h"
+#include "intel_color_manager.h"
 
 
 /**
@@ -465,3 +466,13 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
 	drm_atomic_state_default_clear(&state->base);
 	state->dpll_set = false;
 }
+
+
+int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
+				   struct drm_crtc_state *state,
+				   struct drm_property *property,
+				   uint64_t val)
+{
+	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
+	return -EINVAL;
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b6e1dc5..11d491e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13477,6 +13477,8 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = drm_atomic_helper_crtc_set_property,
+	.atomic_set_property = intel_crtc_atomic_set_property,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 05c809b..1e61036 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1438,6 +1438,10 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
 int intel_atomic_setup_scalers(struct drm_device *dev,
 	struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
+int intel_crtc_atomic_set_property(struct drm_crtc *plane,
+				   struct drm_crtc_state *state,
+				   struct drm_property *property,
+				   uint64_t val);
 
 /* intel_atomic_plane.c */
 struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
-- 
2.4.5

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

[PATCH 08/16] drm: Add blob properties to CRTC state for color properties

[Kausal Malladi]

This patch adds blob properties to CRTC state to hold the respective
blobs for color properties. These will be required by set_property calls
to attach blobs for atomic commit later.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 include/drm/drm_crtc.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 821424e..dea81ed 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -302,6 +302,11 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob properties to hold the color properties' blobs */
+	struct drm_property_blob *palette_before_ctm_blob;
+	struct drm_property_blob *palette_after_ctm_blob;
+	struct drm_property_blob *ctm_blob;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
-- 
2.4.5

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

[PATCH 09/16] drm: Add structures to set/get a palette color property

[Kausal Malladi]

This patch adds new structures in DRM layer for Palette color
correction.

These structures will be used by user space agents to configure
appropriate number of samples and Palette LUT for a platform.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 include/uapi/drm/drm.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index e3c642f..f72b916 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -840,6 +840,33 @@ struct drm_palette_caps {
 	__u32 num_samples_after_ctm;
 };
 
+struct drm_r32g32b32 {
+	/*
+	 * Data is in U8.24 fixed point format.
+	 * All platforms support values within [0, 1.0] range,
+	 * for Red, Green and Blue colors.
+	 */
+	__u32 r32;
+	__u32 g32;
+	__u32 b32;
+};
+
+struct drm_palette {
+	/* Structure version. Should be 1 currently */
+	__u32 version;
+	/*
+	 * This has to be a supported value during get call.
+	 * Feature will be disabled if this is 0 while set
+	 */
+	__u32 num_samples;
+	/*
+	 * Starting of palette LUT in R32G32B32 format.
+	 * Each of RGB value is in U8.24 fixed point format.
+	 * Actual number of samples will depend upon num_samples
+	 */
+	struct drm_r32g32b32 lut[0];
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
2.4.5

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

[PATCH 10/16] drm/i915: Add set_property handler for pipe Gamma correction on CHV/BSW

[Kausal Malladi]

This patch adds set_property handler for Gamma color correction and
enhancement capability at Pipe level on CHV/BSW platform. The set
function just attaches the Gamma blob to CRTC state, that later gets
committed using atomic path.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c        |  7 +++++++
 drivers/gpu/drm/i915/intel_color_manager.c | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h           |  3 +++
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index d873bda..72d6b37 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -473,6 +473,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
 				   struct drm_property *property,
 				   uint64_t val)
 {
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (property == config->prop_palette_after_ctm)
+		return intel_color_manager_set_pipe_gamma(dev, state,
+				&crtc->base, val);
+
 	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
 	return -EINVAL;
 }
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index def20d0f..70c0d42 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,25 @@
 
 #include "intel_color_manager.h"
 
+int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_property_blob *blob;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\n");
+		return -EINVAL;
+	}
+
+	if (crtc_state->palette_after_ctm_blob)
+		drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
+
+	crtc_state->palette_after_ctm_blob = blob;
+	return 0;
+}
+
 int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
 		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1e61036..45c42f0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1453,5 +1453,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 /* intel_color_manager.c */
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 		struct drm_mode_object *mode_obj);
+int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id);
 
 #endif /* __INTEL_DRV_H__ */
-- 
2.4.5

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

[PATCH 11/16] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Kausal Malladi]

CHV/BSW platform supports various Gamma correction modes, which are:
1. Legacy 8-bit mode
2. 10-bit CGM (Color Gamut Mapping) mode

This patch does the following:
1. Adds the core function to program Gamma correction values for CHV/BSW
   platform
2. Adds Gamma correction macros/defines

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |  12 +++
 drivers/gpu/drm/i915/intel_color_manager.c | 149 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  20 ++++
 drivers/gpu/drm/i915/intel_display.c       |   3 +
 drivers/gpu/drm/i915/intel_drv.h           |   2 +
 5 files changed, 186 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 059de0f..4ec2e4f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7906,4 +7906,16 @@ enum skl_disp_power_wells {
 #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
 #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
 
+/* Color Management */
+#define PIPEA_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x67A00)
+#define PIPEB_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x69A00)
+#define PIPEC_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x6BA00)
+#define PIPEA_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x67000)
+#define PIPEB_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x69000)
+#define PIPEC_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x6B000)
+#define _PIPE_CGM_CONTROL(pipe) \
+	(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
+#define _PIPE_GAMMA_BASE(pipe) \
+	(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index 70c0d42..7e9e934 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,155 @@
 
 #include "intel_color_manager.h"
 
+int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
+		  struct drm_crtc *crtc)
+{
+	struct drm_palette *gamma_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 cgm_control_reg = 0;
+	u32 cgm_gamma_reg = 0;
+	enum pipe pipe;
+	u32 red, green, blue;
+	u8 red_int, blue_int, green_int;
+	u16 red_fract, green_fract, blue_fract;
+	u32 count = 0;
+	struct drm_r32g32b32 *correction_values = NULL;
+	u32 num_samples;
+	u32 word;
+	int ret = 0, length, flag = 0;
+
+	if (!blob) {
+		DRM_ERROR("NULL Blob\n");
+		return -EINVAL;
+	}
+
+	gamma_data = (struct drm_palette *)blob->data;
+
+	if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
+		DRM_ERROR("Invalid Gamma Data struct version\n");
+		return -EINVAL;
+	}
+
+	pipe = to_intel_crtc(crtc)->pipe;
+	num_samples = gamma_data->num_samples;
+	length = num_samples * sizeof(struct drm_r32g32b32);
+
+	if (num_samples == 0) {
+
+		/* Disable Gamma functionality on Pipe - CGM Block */
+		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+		cgm_control_reg &= ~CGM_GAMMA_EN;
+		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+		DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
+				pipe_name(pipe));
+		ret = 0;
+	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS ||
+			num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
+		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
+
+		count = 0;
+		correction_values = (struct drm_r32g32b32 *)&gamma_data->lut;
+		while (count < num_samples) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue_int = _GAMMA_INT_PART(blue);
+			if (blue_int > GAMMA_INT_MAX)
+				blue = CHV_MAX_GAMMA;
+			green_int = _GAMMA_INT_PART(green);
+			if (green_int > GAMMA_INT_MAX)
+				green = CHV_MAX_GAMMA;
+			red_int = _GAMMA_INT_PART(red);
+			if (red_int > GAMMA_INT_MAX)
+				red = CHV_MAX_GAMMA;
+
+			blue_fract = _GAMMA_FRACT_PART(blue);
+			green_fract = _GAMMA_FRACT_PART(green);
+			red_fract = _GAMMA_FRACT_PART(red);
+
+			blue_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
+			green_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
+			red_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
+
+			/* Green (25:16) and Blue (9:0) to be written */
+			word = green_fract;
+			word <<= CHV_GAMMA_SHIFT_GREEN;
+			word = word | blue_fract;
+			I915_WRITE(cgm_gamma_reg, word);
+			cgm_gamma_reg += 4;
+
+			/* Red (9:0) to be written */
+			word = red_fract;
+			I915_WRITE(cgm_gamma_reg, word);
+
+			cgm_gamma_reg += 4;
+			count++;
+
+			/*
+			 * On CHV, the best supported Gamma capability is
+			 * CGM block, that takes 257 samples.
+			 * If user gives 256 samples (legacy mode), then
+			 * duplicate the 256th value to 257th.
+			 * "flag" is used to make sure it happens only once
+			 */
+			if (num_samples == CHV_8BIT_GAMMA_MAX_VALS
+					&& count == CHV_8BIT_GAMMA_MAX_VALS
+					&& !flag) {
+				count--;
+				flag = 1;
+			}
+		}
+
+		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
+			pipe_name(pipe));
+
+		/* Enable (CGM) Gamma on Pipe */
+		I915_WRITE(_PIPE_CGM_CONTROL(pipe),
+			I915_READ(_PIPE_CGM_CONTROL(pipe))
+			| CGM_GAMMA_EN);
+		DRM_DEBUG_DRIVER("CGM Gamma enabled on Pipe %c\n",
+				pipe_name(pipe));
+
+		ret = 0;
+	} else {
+		DRM_ERROR("Invalid number of samples for Gamma LUT\n");
+		return -EINVAL;
+	}
+
+	ret = drm_property_replace_global_blob(dev, &blob, length,
+			(void *) gamma_data, &crtc->base,
+			config->prop_palette_after_ctm);
+
+	if (ret) {
+		DRM_ERROR("Error updating Gamma blob\n");
+		return -EFAULT;
+	}
+
+	return ret;
+}
+
+void intel_color_manager_crtc_commit(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state)
+{
+	struct drm_property_blob *blob;
+	struct drm_crtc *crtc = crtc_state->crtc;
+	int ret = 0;
+
+	if (IS_CHERRYVIEW(dev)) {
+		blob = crtc_state->palette_after_ctm_blob;
+		if (blob) {
+			ret = chv_set_gamma(dev, blob, crtc);
+			if (!ret)
+				DRM_DEBUG_DRIVER(
+					"Gamma registers Commit success!\n");
+		}
+
+	}
+}
+
 int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id)
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 51aeb91..8bbac20 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -28,6 +28,26 @@
 #include <drm/drm_crtc_helper.h>
 #include "i915_drv.h"
 
+#define _GAMMA_INT_PART(value)			(value >> GAMMA_INT_SHIFT)
+#define _GAMMA_FRACT_PART(value)		(value >> GAMMA_FRACT_SHIFT)
+
 #define CHV_PALETTE_STRUCT_VERSION		1
 #define CHV_DEGAMMA_MAX_VALS			65
 #define CHV_10BIT_GAMMA_MAX_VALS		257
+
+/* Gamma correction */
+#define CHV_GAMMA_DATA_STRUCT_VERSION		1
+#define CHV_10BIT_GAMMA_MAX_VALS		257
+#define CHV_8BIT_GAMMA_MAX_VALS			256
+#define CHV_10BIT_GAMMA_MSB_SHIFT		6
+#define CHV_GAMMA_SHIFT_GREEN			16
+/* Gamma values are u8.24 format */
+#define GAMMA_INT_SHIFT				24
+#define GAMMA_FRACT_SHIFT			8
+/* Max int value for Gamma is 1 */
+#define GAMMA_INT_MAX				1
+/* Max value for Gamma on CHV */
+#define CHV_MAX_GAMMA				0x10000
+
+/* CHV CGM Block */
+#define CGM_GAMMA_EN				(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 11d491e..9e994fc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13773,6 +13773,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 
 	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
 		skl_detach_scalers(intel_crtc);
+
+	if (!needs_modeset(crtc->state))
+		intel_color_manager_crtc_commit(dev, crtc->state);
 }
 
 static void intel_finish_crtc_commit(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 45c42f0..d8d8326 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1456,5 +1456,7 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id);
+void intel_color_manager_crtc_commit(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state);
 
 #endif /* __INTEL_DRV_H__ */
-- 
2.4.5

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

[PATCH 12/16] drm/i915: Add set_property handler for pipe deGamma correction on CHV/BSW

[Kausal Malladi]

This patch adds set_property handler for deGamma color correction and
enhancement capability at Pipe level on CHV/BSW platform. The set
function just attaches the deGamma blob to CRTC state, that later gets
committed using atomic path.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c        |  3 +++
 drivers/gpu/drm/i915/intel_color_manager.c | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h           |  3 +++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 72d6b37..c74ca31 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -476,6 +476,9 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_mode_config *config = &dev->mode_config;
 
+	if (property == config->prop_palette_before_ctm)
+		return intel_color_manager_set_pipe_degamma(dev, state,
+				&crtc->base, val);
 	if (property == config->prop_palette_after_ctm)
 		return intel_color_manager_set_pipe_gamma(dev, state,
 				&crtc->base, val);
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index 7e9e934..908b7ed 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -176,6 +176,25 @@ void intel_color_manager_crtc_commit(struct drm_device *dev,
 	}
 }
 
+int intel_color_manager_set_pipe_degamma(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_property_blob *blob;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\n");
+		return -EINVAL;
+	}
+
+	if (crtc_state->palette_before_ctm_blob)
+		drm_property_unreference_blob(crtc_state->palette_before_ctm_blob);
+
+	crtc_state->palette_before_ctm_blob = blob;
+	return 0;
+}
+
 int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d8d8326..03c7468 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1456,6 +1456,9 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id);
+int intel_color_manager_set_pipe_degamma(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id);
 void intel_color_manager_crtc_commit(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state);
 
-- 
2.4.5

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

[PATCH 13/16] drm/i915: Add DeGamma correction for CHV/BSW

[Kausal Malladi]

CHV/BSW supports DeGamma color correction feature, which linearizes all
the non-linear color values. This will be applied before Color
Transformation.

This patch does the following:
1. Adds the core function to program DeGamma correction values for
   CHV/BSW platform
2. Adds DeGamma correction macros/defines

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |   5 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 122 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |   6 ++
 3 files changed, 133 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4ec2e4f..b95862e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7913,9 +7913,14 @@ enum skl_disp_power_wells {
 #define PIPEA_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x67000)
 #define PIPEB_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x69000)
 #define PIPEC_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x6B000)
+#define PIPEA_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x66000)
+#define PIPEB_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x68000)
+#define PIPEC_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x6A000)
 #define _PIPE_CGM_CONTROL(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
 #define _PIPE_GAMMA_BASE(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
+#define _PIPE_DEGAMMA_BASE(pipe) \
+	(_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index 908b7ed..121ec43 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,121 @@
 
 #include "intel_color_manager.h"
 
+int chv_set_degamma(struct drm_device *dev, struct drm_property_blob *blob,
+		struct drm_crtc *crtc)
+{
+	struct drm_palette *degamma_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 cgm_control_reg = 0;
+	u32 cgm_degamma_reg = 0;
+	enum pipe pipe;
+	u32 red, green, blue;
+	u8 red_int, green_int, blue_int;
+	u16 red_fract, green_fract, blue_fract;
+	u32 count = 0;
+	struct drm_r32g32b32 *correction_values = NULL;
+	u32 num_samples;
+	u32 word;
+	int ret = 0, length;
+
+	if (!blob) {
+		DRM_ERROR("NULL Blob\n");
+		return -EINVAL;
+	}
+
+	degamma_data = (struct drm_palette *)blob->data;
+
+	if (degamma_data->version != CHV_DEGAMMA_DATA_STRUCT_VERSION) {
+		DRM_ERROR("Invalid DeGamma Data struct version\n");
+		return -EINVAL;
+	}
+
+	pipe = to_intel_crtc(crtc)->pipe;
+	num_samples = degamma_data->num_samples;
+	length = num_samples * sizeof(struct drm_r32g32b32);
+
+	if (num_samples == 0) {
+
+		/* Disable DeGamma functionality on Pipe - CGM Block */
+		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+		cgm_control_reg &= ~CGM_DEGAMMA_EN;
+		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+		DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n",
+				pipe_name(pipe));
+		ret = 0;
+	} else if (num_samples == CHV_DEGAMMA_MAX_VALS) {
+		cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe);
+
+		count = 0;
+		correction_values = (struct drm_r32g32b32 *)&degamma_data->lut;
+		while (count < CHV_DEGAMMA_MAX_VALS) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue_int = _GAMMA_INT_PART(blue);
+			if (blue_int > GAMMA_INT_MAX)
+				blue = CHV_MAX_GAMMA;
+			green_int = _GAMMA_INT_PART(green);
+			if (green_int > GAMMA_INT_MAX)
+				green = CHV_MAX_GAMMA;
+			red_int = _GAMMA_INT_PART(red);
+			if (red_int > GAMMA_INT_MAX)
+				red = CHV_MAX_GAMMA;
+
+			blue_fract = _GAMMA_FRACT_PART(blue);
+			green_fract = _GAMMA_FRACT_PART(green);
+			red_fract = _GAMMA_FRACT_PART(red);
+
+			blue_fract >>= CHV_DEGAMMA_MSB_SHIFT;
+			green_fract >>= CHV_DEGAMMA_MSB_SHIFT;
+			red_fract >>= CHV_DEGAMMA_MSB_SHIFT;
+
+			/* Green (29:16) and Blue (13:0) in DWORD1 */
+			word = green_fract;
+			word <<= CHV_DEGAMMA_GREEN_SHIFT;
+			word = word | blue;
+			I915_WRITE(cgm_degamma_reg, word);
+
+			cgm_degamma_reg += 4;
+
+			/* Red (13:0) to be written to DWORD2 */
+			word = red_fract;
+			I915_WRITE(cgm_degamma_reg, word);
+
+			cgm_degamma_reg += 4;
+			count++;
+		}
+
+		DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n",
+				pipe_name(pipe));
+
+		/* Enable DeGamma on Pipe */
+		I915_WRITE(_PIPE_CGM_CONTROL(pipe),
+			I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN);
+
+		DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n",
+				pipe_name(pipe));
+		ret = 0;
+	} else {
+		DRM_ERROR("Invalid number of samples for DeGamma LUT\n");
+		return -EINVAL;
+	}
+
+	ret = drm_property_replace_global_blob(dev, &blob, length,
+			(void *) degamma_data, &crtc->base,
+			config->prop_palette_before_ctm);
+
+	if (ret) {
+		DRM_ERROR("Error updating DeGamma blob\n");
+		return -EFAULT;
+	}
+
+	return ret;
+}
+
 int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
 		  struct drm_crtc *crtc)
 {
@@ -173,6 +288,13 @@ void intel_color_manager_crtc_commit(struct drm_device *dev,
 					"Gamma registers Commit success!\n");
 		}
 
+		blob = crtc_state->palette_before_ctm_blob;
+		if (blob) {
+			ret = chv_set_degamma(dev, blob, crtc);
+			if (!ret)
+				DRM_DEBUG_DRIVER(
+					"DeGamma registers Commit success!\n");
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 8bbac20..6a4fff2 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -49,5 +49,11 @@
 /* Max value for Gamma on CHV */
 #define CHV_MAX_GAMMA				0x10000
 
+/* DeGamma correction */
+#define CHV_DEGAMMA_DATA_STRUCT_VERSION		1
+#define CHV_DEGAMMA_MSB_SHIFT			2
+#define CHV_DEGAMMA_GREEN_SHIFT			16
+
 /* CHV CGM Block */
 #define CGM_GAMMA_EN				(1 << 2)
+#define CGM_DEGAMMA_EN				(1 << 0)
-- 
2.4.5

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

[PATCH 14/16] drm: Add structure for set/get a CTM color property

[Kausal Malladi]

Color Manager framework defines a color correction property for color
space transformation and Gamut mapping. This property is called CTM (Color
Transformation Matrix).

This patch adds a new structure in DRM layer for CTM color correction.
This structure will be used by all user space agents to configure CTM
coefficients for color correction.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 include/uapi/drm/drm.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index f72b916..9580772 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -867,6 +867,18 @@ struct drm_palette {
 	struct drm_r32g32b32 lut[0];
 };
 
+struct drm_ctm {
+	/* Structure version. Should be 1 currently */
+	__u32 version;
+	/*
+	 * Each value is in S31.32 format.
+	 * This is 3x3 matrix in row major format.
+	 * Integer part will be clipped to nearest
+	 * max/min boundary as supported by the HW platform.
+	 */
+	__s64 ctm_coeff[9];
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
2.4.5

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

[PATCH 15/16] drm/i915: Add set_property handler for CSC correction on CHV/BSW

[Kausal Malladi]

This patch adds set_property handler for CSC color correction
and enhancement capability at Pipe level on CHV/BSW platform. The set
function just attaches the CSC blob to CRTC state, that later
gets committed using atomic path.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c        |  3 +++
 drivers/gpu/drm/i915/intel_color_manager.c | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h           |  3 +++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index c74ca31..6b3b88c 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -482,6 +482,9 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
 	if (property == config->prop_palette_after_ctm)
 		return intel_color_manager_set_pipe_gamma(dev, state,
 				&crtc->base, val);
+	if (property == config->prop_ctm)
+		return intel_color_manager_set_pipe_csc(dev, state,
+				&crtc->base, val);
 
 	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
 	return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index 121ec43..bd60fad 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -298,6 +298,25 @@ void intel_color_manager_crtc_commit(struct drm_device *dev,
 	}
 }
 
+int intel_color_manager_set_pipe_csc(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_property_blob *blob;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\n");
+		return -EINVAL;
+	}
+
+	if (crtc_state->ctm_blob)
+		drm_property_unreference_blob(crtc_state->ctm_blob);
+
+	crtc_state->ctm_blob = blob;
+	return 0;
+}
+
 int intel_color_manager_set_pipe_degamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 03c7468..42214aa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1459,6 +1459,9 @@ int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
 int intel_color_manager_set_pipe_degamma(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state,
 		struct drm_mode_object *obj, uint32_t blob_id);
+int intel_color_manager_set_pipe_csc(struct drm_device *dev,
+		struct drm_crtc_state *crtc_state,
+		struct drm_mode_object *obj, uint32_t blob_id);
 void intel_color_manager_crtc_commit(struct drm_device *dev,
 		struct drm_crtc_state *crtc_state);
 
-- 
2.4.5

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

[PATCH 16/16] drm/i915: Add CSC correction for CHV/BSW

[Kausal Malladi]

CHV/BSW supports Color Space Conversion (CSC) using a 3x3 matrix
that needs to be programmed into CGM (Color Gamut Mapping) registers.

This patch does the following:
1. Adds the core function to program CSC correction values for
   CHV/BSW platform
2. Adds CSC correction macros/defines

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |   5 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 110 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  20 ++++++
 3 files changed, 135 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b95862e..9bf5720 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7916,11 +7916,16 @@ enum skl_disp_power_wells {
 #define PIPEA_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x66000)
 #define PIPEB_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x68000)
 #define PIPEC_CGM_DEGAMMA			(VLV_DISPLAY_BASE + 0x6A000)
+#define PIPEA_CGM_CSC				(VLV_DISPLAY_BASE + 0x67900)
+#define PIPEB_CGM_CSC				(VLV_DISPLAY_BASE + 0x69900)
+#define PIPEC_CGM_CSC				(VLV_DISPLAY_BASE + 0x6B900)
 #define _PIPE_CGM_CONTROL(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
 #define _PIPE_GAMMA_BASE(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
 #define _PIPE_DEGAMMA_BASE(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
+#define _PIPE_CSC_BASE(pipe) \
+	(_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index bd60fad..e855d5d 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,108 @@
 
 #include "intel_color_manager.h"
 
+s16 get_csc_s3_12_format(s64 csc_value)
+{
+	s32 csc_int_value;
+	u32 csc_fract_value;
+	s16 csc_s3_12_format;
+
+	if (csc_value >= 0) {
+		csc_value += CHV_CSC_FRACT_ROUNDOFF;
+		if (csc_value > CHV_CSC_COEFF_MAX)
+			csc_value = CHV_CSC_COEFF_MAX;
+	} else {
+		csc_value = -csc_value;
+		csc_value += CHV_CSC_FRACT_ROUNDOFF;
+		if (csc_value > CHV_CSC_COEFF_MAX + 1)
+			csc_value = CHV_CSC_COEFF_MAX + 1;
+		csc_value = -csc_value;
+	}
+
+	csc_int_value = csc_value >> CHV_CSC_COEFF_SHIFT;
+	csc_int_value <<= CHV_CSC_COEFF_INT_SHIFT;
+	if (csc_value < 0)
+		csc_int_value |= CSC_COEFF_SIGN;
+	csc_fract_value = csc_value;
+	csc_fract_value >>= CHV_CSC_COEFF_FRACT_SHIFT;
+	csc_s3_12_format = csc_int_value | csc_fract_value;
+
+	return csc_s3_12_format;
+}
+
+int chv_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
+		struct drm_crtc *crtc)
+{
+	struct drm_ctm *csc_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 reg;
+	enum pipe pipe;
+	s32 word, temp;
+	int ret, count = 0;
+
+	if (!blob) {
+		DRM_ERROR("NULL Blob\n");
+		return -EINVAL;
+	}
+
+	if (blob->length != sizeof(struct drm_ctm)) {
+		DRM_ERROR("Invalid length of data received\n");
+		return -EINVAL;
+	}
+
+	csc_data = (struct drm_ctm *)blob->data;
+	if (csc_data->version != CHV_CSC_DATA_STRUCT_VERSION) {
+		DRM_ERROR("Invalid CSC Data struct version\n");
+		return -EINVAL;
+	}
+
+	pipe = to_intel_crtc(crtc)->pipe;
+
+	/* Disable CSC functionality */
+	reg = _PIPE_CGM_CONTROL(pipe);
+	I915_WRITE(reg, I915_READ(reg) & (~CGM_CSC_EN));
+
+	DRM_DEBUG_DRIVER("Disabled CSC Functionality on Pipe %c\n",
+			pipe_name(pipe));
+
+	reg = _PIPE_CSC_BASE(pipe);
+	while (count < CSC_MAX_VALS) {
+		word = get_csc_s3_12_format(csc_data->ctm_coeff[count]);
+
+		/*
+		 * Last value to be written in 1 register.
+		 * Otherwise, each pair of CSC values go
+		 * into 1 register
+		 */
+		if (count != (CSC_MAX_VALS - 1)) {
+			count++;
+			temp = get_csc_s3_12_format(csc_data->ctm_coeff[count]);
+			word |= temp;
+		}
+		I915_WRITE(reg, word);
+		reg += 4;
+		count++;
+	}
+
+	DRM_DEBUG_DRIVER("All CSC values written to registers\n");
+
+	/* Enable CSC functionality */
+	reg = _PIPE_CGM_CONTROL(pipe);
+	I915_WRITE(reg, I915_READ(reg) | CGM_CSC_EN);
+	DRM_DEBUG_DRIVER("CSC enabled on Pipe %c\n", pipe_name(pipe));
+
+	ret = drm_property_replace_global_blob(dev, &blob,
+			sizeof(struct drm_ctm), (void *) csc_data,
+			&crtc->base, config->prop_ctm);
+	if (ret) {
+		DRM_ERROR("Error updating CSC blob\n");
+		return -EFAULT;
+	}
+
+	return ret;
+}
+
 int chv_set_degamma(struct drm_device *dev, struct drm_property_blob *blob,
 		struct drm_crtc *crtc)
 {
@@ -295,6 +397,14 @@ void intel_color_manager_crtc_commit(struct drm_device *dev,
 				DRM_DEBUG_DRIVER(
 					"DeGamma registers Commit success!\n");
 		}
+
+		blob = crtc_state->ctm_blob;
+		if (blob) {
+			ret = chv_set_csc(dev, blob, crtc);
+			if (!ret)
+				DRM_DEBUG_DRIVER(
+					"CSC registers Commit success!\n");
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 6a4fff2..b2ee847 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -54,6 +54,26 @@
 #define CHV_DEGAMMA_MSB_SHIFT			2
 #define CHV_DEGAMMA_GREEN_SHIFT			16
 
+/* CSC correction */
+#define CHV_CSC_DATA_STRUCT_VERSION		1
+/*
+ * Fractional part is 32 bit, and we need only 12 MSBs for programming
+ * into registers. ROUNDOFF is required to minimize loss of precision.
+ */
+#define CHV_CSC_FRACT_ROUNDOFF			(1 << 19)
+/*
+ * CSC values are 64-bit values. For CHV, the maximum CSC value that
+ * user can program is 7.99999..., which can be represented in fixed point
+ * S31.32 format like this, with all fractional bits as 1
+ */
+#define CHV_CSC_COEFF_MAX			0x00000007FFFFFFFF
+#define CHV_CSC_COEFF_SHIFT			32
+#define CHV_CSC_COEFF_INT_SHIFT			28
+#define CSC_COEFF_SIGN				(1 << 31)
+#define CHV_CSC_COEFF_FRACT_SHIFT		20
+#define CSC_MAX_VALS				9
+
 /* CHV CGM Block */
 #define CGM_GAMMA_EN				(1 << 2)
+#define CGM_CSC_EN				(1 << 1)
 #define CGM_DEGAMMA_EN				(1 << 0)
-- 
2.4.5

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

Re: [PATCH 02/16] drm: Create Color Management DRM properties

[Thierry Reding]

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

On Wed, Jul 15, 2015 at 06:39:26PM +0530, Kausal Malladi wrote:
[...]
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 57ca8cc..408d39a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1178,6 +1178,12 @@ struct drm_mode_config {
>  	struct drm_property *suggested_x_property;
>  	struct drm_property *suggested_y_property;
>  
> +	/* Color Management Properties */
> +	struct drm_property *prop_color_capabilities;
> +	struct drm_property *prop_palette_before_ctm;
> +	struct drm_property *prop_palette_after_ctm;
> +	struct drm_property *prop_ctm;

All existing properties are named *_property, and it'd be good to keep
things consistent by following that scheme. Perhaps they could have a
common prefix (cm_?) to group them?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

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

RE: [PATCH 02/16] drm: Create Color Management DRM properties

[Sharma, Shashank]

Thanks for the comments, Thierry. 
We can surely do this. 

Regards
Shashank
-----Original Message-----
From: Thierry Reding [mailto:thierry.reding@gmail.com] 
Sent: Wednesday, July 15, 2015 6:55 PM
To: Malladi, Kausal
Cc: Roper, Matthew D; Barnes, Jesse; Lespiau, Damien; Jindal, Sonika; R, Durgadoss; Purushothaman, Vijay A; intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; hverkuil@xs4all.nl; daniel@fooishbar.org; Matheson, Annie J; Palleti, Avinash Reddy; Mukherjee, Indranil; R, Dhanya p; Kamath, Sunil; Vetter, Daniel; Bhattacharjee, Susanta; Kumar, Kiran S; Sharma, Shashank
Subject: Re: [PATCH 02/16] drm: Create Color Management DRM properties

On Wed, Jul 15, 2015 at 06:39:26PM +0530, Kausal Malladi wrote:
[...]
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 
> 57ca8cc..408d39a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1178,6 +1178,12 @@ struct drm_mode_config {
>  	struct drm_property *suggested_x_property;
>  	struct drm_property *suggested_y_property;
>  
> +	/* Color Management Properties */
> +	struct drm_property *prop_color_capabilities;
> +	struct drm_property *prop_palette_before_ctm;
> +	struct drm_property *prop_palette_after_ctm;
> +	struct drm_property *prop_ctm;

All existing properties are named *_property, and it'd be good to keep things consistent by following that scheme. Perhaps they could have a common prefix (cm_?) to group them?

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

Re: [PATCH 03/16] drm/i915: Attach color properties to CRTC

[Bish, Jim]

On 07/15/2015 06:09 AM, Kausal Malladi wrote:
> This patch does the following:
> 1. Adds new files intel_color_manager(.c/.h)
> 2. Attaches color properties to CRTC while initialization
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile              |  3 +-
>  drivers/gpu/drm/i915/intel_color_manager.c | 49 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h | 29 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c       |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h           |  4 +++
>  5 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index de21965..ad928d8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -56,7 +56,8 @@ i915-y += intel_audio.o \
>  	  intel_overlay.o \
>  	  intel_psr.o \
>  	  intel_sideband.o \
> -	  intel_sprite.o
> +	  intel_sprite.o \
> +	  intel_color_manager.o
>  i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
>  i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
>  
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> new file mode 100644
> index 0000000..baa4536
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>
> + */
> +
> +#include "intel_color_manager.h"
> +
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (IS_CHERRYVIEW(dev)) {
> +		if (config->prop_color_capabilities)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_color_capabilities, 0);
> +		if (config->prop_palette_before_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_before_ctm, 0);
> +		if (config->prop_palette_after_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_after_ctm, 0);
> +		if (config->prop_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_ctm, 0);
> +	}
why only CHT?  Seems we should be putting cases for all of our devices.
> +}
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> new file mode 100644
> index 0000000..04c921d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>
> + */
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc_helper.h>
> +#include "i915_drv.h"
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bb58cb6..b6e1dc5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14111,6 +14111,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	intel_attach_color_properties_to_crtc(dev, &intel_crtc->base.base);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b9c01c5..05c809b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1446,4 +1446,8 @@ void intel_plane_destroy_state(struct drm_plane *plane,
>  			       struct drm_plane_state *state);
>  extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>  
> +/* intel_color_manager.c */
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj);
> +
>  #endif /* __INTEL_DRV_H__ */
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/16] drm: Add structure for querying palette color capabilities

[Bish, Jim]

On 07/15/2015 06:09 AM, Kausal Malladi wrote:
> The DRM color management framework is targeting various hardware
> platforms and drivers. Different platforms can have different color
> correction and enhancement capabilities.
> 
> A commom user space application can query these capabilities using the
> DRM property interface. Each driver can fill this property with its
> platform's palette color capabilities.
> 
> This patch adds new structure in DRM layer for querying palette color
> capabilities. This structure will be used by all user space
> agents to configure appropriate color configurations.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  include/uapi/drm/drm.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 3801584..e3c642f 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -829,6 +829,17 @@ struct drm_event_vblank {
>  	__u32 reserved;
>  };
>  
> +struct drm_palette_caps {
> +	/* Structure version. Should be 1 currently */
> +	__u32 version;
> +	/* For padding and future use */
> +	__u32 reserved;
> +	/* This may be 0 if not supported. e.g. plane palette or VLV pipe */
> +	__u32 num_samples_before_ctm;
> +	/* This will be non-zero for pipe. May be zero for planes on some HW */
> +	__u32 num_samples_after_ctm;
> +};
this structure does not match what is documented in the design document.  are we 
missing updates to the design document?
> +
>  /* typedef area */
>  #ifndef __KERNEL__
>  typedef struct drm_clip_rect drm_clip_rect_t;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/16] drm/i915: Attach color properties to CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:27PM +0530, Kausal Malladi wrote:
> This patch does the following:
> 1. Adds new files intel_color_manager(.c/.h)
> 2. Attaches color properties to CRTC while initialization
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>

I'd suggest moving this patch to the end of the series.  If someone is
bisecting an issue and they land somewhere in the middle of this
patchset, you'll be advertising properties to userspace that aren't
fully functional which may cause unexpected test failures (and make
bisect decisions harder).

Generally we like to "flip the switch" to make new functionality visible
only at the very end when all the internal plumbing is complete.


Matt

> ---
>  drivers/gpu/drm/i915/Makefile              |  3 +-
>  drivers/gpu/drm/i915/intel_color_manager.c | 49 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h | 29 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c       |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h           |  4 +++
>  5 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index de21965..ad928d8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -56,7 +56,8 @@ i915-y += intel_audio.o \
>  	  intel_overlay.o \
>  	  intel_psr.o \
>  	  intel_sideband.o \
> -	  intel_sprite.o
> +	  intel_sprite.o \
> +	  intel_color_manager.o
>  i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
>  i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
>  
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> new file mode 100644
> index 0000000..baa4536
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>
> + */
> +
> +#include "intel_color_manager.h"
> +
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (IS_CHERRYVIEW(dev)) {
> +		if (config->prop_color_capabilities)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_color_capabilities, 0);
> +		if (config->prop_palette_before_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_before_ctm, 0);
> +		if (config->prop_palette_after_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_after_ctm, 0);
> +		if (config->prop_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_ctm, 0);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> new file mode 100644
> index 0000000..04c921d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>
> + */
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc_helper.h>
> +#include "i915_drv.h"
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bb58cb6..b6e1dc5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14111,6 +14111,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	intel_attach_color_properties_to_crtc(dev, &intel_crtc->base.base);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b9c01c5..05c809b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1446,4 +1446,8 @@ void intel_plane_destroy_state(struct drm_plane *plane,
>  			       struct drm_plane_state *state);
>  extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>  
> +/* intel_color_manager.c */
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj);
> +
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 2.4.5
> 

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

Re: [PATCH 06/16] drm/i915: Load gamma color capabilities for CHV CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:30PM +0530, Kausal Malladi wrote:
> As per Color Manager design, each driver is responsible to load its
> palette color correction and enhancement capabilities in the form of
> a DRM blob property, so that user space can query and read.
> 
> This patch loads all CHV platform specific gamma color capabilities
> for CRTC into a blob that can be accessible by user space to
> query capabilities via DRM property interface.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color_manager.c | 48 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h |  4 +++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> index baa4536..def20d0f 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,15 +27,63 @@
>  
>  #include "intel_color_manager.h"
>  
> +int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
> +		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> +{
> +	struct drm_property_blob *blob = NULL;
> +	struct drm_mode_config *config = &dev->mode_config;
> +	int ret;
> +
> +	/*
> +	 * This function exposes best capability for DeGamma and Gamma
> +	 * For CHV, the DeGamma LUT has 65 entries
> +	 * and the best Gamma capability has 257 entries (CGM unit)
> +	 */
> +	palette_caps->version = CHV_PALETTE_STRUCT_VERSION;
> +	palette_caps->num_samples_before_ctm =
> +		CHV_DEGAMMA_MAX_VALS;
> +	palette_caps->num_samples_after_ctm =
> +		CHV_10BIT_GAMMA_MAX_VALS;
> +
> +	ret = drm_property_replace_global_blob(dev, &blob,

We're only calling this once at startup and never actually replacing the
capabilities, right?  Is there any difference between this and just
manually doing a drm_property_create_blob() and passing that blob's ID
as the value when we attach the property to the CRTC?  It feels a bit
strange to be calling 'replace' on a local variable that isn't
initialized and is never used/saved.


> +			sizeof(struct drm_palette_caps),
> +			(const void *)palette_caps,
> +			&crtc->base, config->prop_color_capabilities);
> +	if (ret) {
> +		DRM_ERROR("Error updating Gamma blob\n");
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +
> +int get_pipe_gamma_capabilities(struct drm_device *dev,
> +		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> +{
> +	if (IS_CHERRYVIEW(dev))
> +		return get_chv_pipe_gamma_capabilities(dev, palette_caps, crtc);
> +	return -EINVAL;
> +}
> +
>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>  		struct drm_mode_object *mode_obj)
>  {
>  	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_palette_caps *palette_caps;
> +	struct drm_crtc *crtc;
> +	int ret;
>  
>  	if (IS_CHERRYVIEW(dev)) {
> +		crtc = obj_to_crtc(mode_obj);
>  		if (config->prop_color_capabilities)
>  			drm_object_attach_property(mode_obj,
>  					config->prop_color_capabilities, 0);
> +		palette_caps = kzalloc(sizeof(struct drm_palette_caps),
> +				GFP_KERNEL);
> +		ret = get_pipe_gamma_capabilities(dev, palette_caps, crtc);

Is palette_caps ever freed?  I believe the data is copied when a blob is
created, so I think you should be free to kfree() this allocation right
away.


Matt

> +		if (ret)
> +			DRM_ERROR("Error getting gamma capability for CHV\n");
> +
>  		if (config->prop_palette_before_ctm)
>  			drm_object_attach_property(mode_obj,
>  					config->prop_palette_before_ctm, 0);
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> index 04c921d..51aeb91 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -27,3 +27,7 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>  #include "i915_drv.h"
> +
> +#define CHV_PALETTE_STRUCT_VERSION		1
> +#define CHV_DEGAMMA_MAX_VALS			65
> +#define CHV_10BIT_GAMMA_MAX_VALS		257
> -- 
> 2.4.5
> 

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

Re: [PATCH 07/16] drm/i915: Add atomic set property interface for CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:31PM +0530, Kausal Malladi wrote:
> This patch adds atomic set property interface for Intel CRTC. This
> interface will be used to set color correction DRM properties.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>

I think we also need .get_property / .atomic_get_property entrypoints as
well.  When userspace tries to query properties, the DRM core will want
to call into i915 for any non-core property that it doesn't recognize.

We don't really have any examples of .atomic_get_property in i915 today,
so you might want to look at omap's omap_plane_atomic_get_property() for
a very basic example (but rather than returning a raw value for color
correction properties, you'll return blob->base.id for the relevant
property).


Matt

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 6ce6c3d..d873bda 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include "intel_drv.h"
> +#include "intel_color_manager.h"
>  
>  
>  /**
> @@ -465,3 +466,13 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
>  	drm_atomic_state_default_clear(&state->base);
>  	state->dpll_set = false;
>  }
> +
> +
> +int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> +				   struct drm_crtc_state *state,
> +				   struct drm_property *property,
> +				   uint64_t val)
> +{
> +	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> +	return -EINVAL;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b6e1dc5..11d491e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13477,6 +13477,8 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = intel_crtc_set_config,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
> +	.set_property = drm_atomic_helper_crtc_set_property,
> +	.atomic_set_property = intel_crtc_atomic_set_property,
>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
>  	.atomic_destroy_state = intel_crtc_destroy_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 05c809b..1e61036 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1438,6 +1438,10 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
>  int intel_atomic_setup_scalers(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state);
> +int intel_crtc_atomic_set_property(struct drm_crtc *plane,
> +				   struct drm_crtc_state *state,
> +				   struct drm_property *property,
> +				   uint64_t val);
>  
>  /* intel_atomic_plane.c */
>  struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
> -- 
> 2.4.5
> 

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

Re: [PATCH 10/16] drm/i915: Add set_property handler for pipe Gamma correction on CHV/BSW

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:34PM +0530, Kausal Malladi wrote:
> This patch adds set_property handler for Gamma color correction and
> enhancement capability at Pipe level on CHV/BSW platform. The set
> function just attaches the Gamma blob to CRTC state, that later gets
> committed using atomic path.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c        |  7 +++++++
>  drivers/gpu/drm/i915/intel_color_manager.c | 19 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h           |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index d873bda..72d6b37 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -473,6 +473,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
>  				   struct drm_property *property,
>  				   uint64_t val)
>  {
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (property == config->prop_palette_after_ctm)
> +		return intel_color_manager_set_pipe_gamma(dev, state,
> +				&crtc->base, val);
> +
>  	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>  	return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> index def20d0f..70c0d42 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,25 @@
>  
>  #include "intel_color_manager.h"
>  
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> +		struct drm_crtc_state *crtc_state,
> +		struct drm_mode_object *obj, uint32_t blob_id)
> +{
> +	struct drm_property_blob *blob;
> +
> +	blob = drm_property_lookup_blob(dev, blob_id);
> +	if (!blob) {
> +		DRM_ERROR("Invalid Blob ID\n");
> +		return -EINVAL;
> +	}

I'm not terribly familiar the semantics of the color correction
stuff...this prevents userspace from removing the gamma setting by
passing a 0; is that expected that gamma can't be disabled once set?
Same question for your degamma patch later in the series.


Matt

> +
> +	if (crtc_state->palette_after_ctm_blob)
> +		drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
> +
> +	crtc_state->palette_after_ctm_blob = blob;
> +	return 0;
> +}
> +
>  int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
>  		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1e61036..45c42f0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1453,5 +1453,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>  /* intel_color_manager.c */
>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>  		struct drm_mode_object *mode_obj);
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> +		struct drm_crtc_state *crtc_state,
> +		struct drm_mode_object *obj, uint32_t blob_id);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 2.4.5
> 

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

Re: [PATCH 11/16] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:35PM +0530, Kausal Malladi wrote:
> CHV/BSW platform supports various Gamma correction modes, which are:
> 1. Legacy 8-bit mode
> 2. 10-bit CGM (Color Gamut Mapping) mode
> 
> This patch does the following:
> 1. Adds the core function to program Gamma correction values for CHV/BSW
>    platform
> 2. Adds Gamma correction macros/defines
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h            |  12 +++
>  drivers/gpu/drm/i915/intel_color_manager.c | 149 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h |  20 ++++
>  drivers/gpu/drm/i915/intel_display.c       |   3 +
>  drivers/gpu/drm/i915/intel_drv.h           |   2 +
>  5 files changed, 186 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 059de0f..4ec2e4f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7906,4 +7906,16 @@ enum skl_disp_power_wells {
>  #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>  #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>  
> +/* Color Management */
> +#define PIPEA_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x67A00)
> +#define PIPEB_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x69A00)
> +#define PIPEC_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x6BA00)
> +#define PIPEA_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x67000)
> +#define PIPEB_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x69000)
> +#define PIPEC_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x6B000)
> +#define _PIPE_CGM_CONTROL(pipe) \
> +	(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
> +#define _PIPE_GAMMA_BASE(pipe) \
> +	(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> index 70c0d42..7e9e934 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,155 @@
>  
>  #include "intel_color_manager.h"
>  
> +int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
> +		  struct drm_crtc *crtc)
> +{
> +	struct drm_palette *gamma_data;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_mode_config *config = &dev->mode_config;
> +	u32 cgm_control_reg = 0;
> +	u32 cgm_gamma_reg = 0;
> +	enum pipe pipe;
> +	u32 red, green, blue;
> +	u8 red_int, blue_int, green_int;
> +	u16 red_fract, green_fract, blue_fract;
> +	u32 count = 0;
> +	struct drm_r32g32b32 *correction_values = NULL;
> +	u32 num_samples;
> +	u32 word;
> +	int ret = 0, length, flag = 0;
> +
> +	if (!blob) {
> +		DRM_ERROR("NULL Blob\n");
> +		return -EINVAL;
> +	}
> +
> +	gamma_data = (struct drm_palette *)blob->data;
> +
> +	if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
> +		DRM_ERROR("Invalid Gamma Data struct version\n");
> +		return -EINVAL;
> +	}
> +
> +	pipe = to_intel_crtc(crtc)->pipe;
> +	num_samples = gamma_data->num_samples;
> +	length = num_samples * sizeof(struct drm_r32g32b32);
> +
> +	if (num_samples == 0) {
> +
> +		/* Disable Gamma functionality on Pipe - CGM Block */
> +		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> +		cgm_control_reg &= ~CGM_GAMMA_EN;
> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> +		DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
> +				pipe_name(pipe));
> +		ret = 0;
> +	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS ||
> +			num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
> +		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
> +
> +		count = 0;
> +		correction_values = (struct drm_r32g32b32 *)&gamma_data->lut;
> +		while (count < num_samples) {
> +			blue = correction_values[count].b32;
> +			green = correction_values[count].g32;
> +			red = correction_values[count].r32;
> +
> +			blue_int = _GAMMA_INT_PART(blue);
> +			if (blue_int > GAMMA_INT_MAX)
> +				blue = CHV_MAX_GAMMA;
> +			green_int = _GAMMA_INT_PART(green);
> +			if (green_int > GAMMA_INT_MAX)
> +				green = CHV_MAX_GAMMA;
> +			red_int = _GAMMA_INT_PART(red);
> +			if (red_int > GAMMA_INT_MAX)
> +				red = CHV_MAX_GAMMA;
> +
> +			blue_fract = _GAMMA_FRACT_PART(blue);
> +			green_fract = _GAMMA_FRACT_PART(green);
> +			red_fract = _GAMMA_FRACT_PART(red);
> +
> +			blue_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
> +			green_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
> +			red_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
> +
> +			/* Green (25:16) and Blue (9:0) to be written */
> +			word = green_fract;
> +			word <<= CHV_GAMMA_SHIFT_GREEN;
> +			word = word | blue_fract;
> +			I915_WRITE(cgm_gamma_reg, word);
> +			cgm_gamma_reg += 4;
> +
> +			/* Red (9:0) to be written */
> +			word = red_fract;
> +			I915_WRITE(cgm_gamma_reg, word);
> +
> +			cgm_gamma_reg += 4;
> +			count++;
> +
> +			/*
> +			 * On CHV, the best supported Gamma capability is
> +			 * CGM block, that takes 257 samples.
> +			 * If user gives 256 samples (legacy mode), then
> +			 * duplicate the 256th value to 257th.
> +			 * "flag" is used to make sure it happens only once
> +			 */
> +			if (num_samples == CHV_8BIT_GAMMA_MAX_VALS
> +					&& count == CHV_8BIT_GAMMA_MAX_VALS
> +					&& !flag) {
> +				count--;
> +				flag = 1;
> +			}
> +		}
> +
> +		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
> +			pipe_name(pipe));
> +
> +		/* Enable (CGM) Gamma on Pipe */
> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe),
> +			I915_READ(_PIPE_CGM_CONTROL(pipe))
> +			| CGM_GAMMA_EN);
> +		DRM_DEBUG_DRIVER("CGM Gamma enabled on Pipe %c\n",
> +				pipe_name(pipe));
> +
> +		ret = 0;
> +	} else {
> +		DRM_ERROR("Invalid number of samples for Gamma LUT\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = drm_property_replace_global_blob(dev, &blob, length,
> +			(void *) gamma_data, &crtc->base,
> +			config->prop_palette_after_ctm);

Is this supposed to be here?  This function seems to be for just
programming the hardware with the contents of the current blob.  I'm not
sure I understand why we try to alter the blob itself at the end of
that.  I see the same in your degamma patch.

> +
> +	if (ret) {
> +		DRM_ERROR("Error updating Gamma blob\n");
> +		return -EFAULT;
> +	}
> +
> +	return ret;
> +}
> +
> +void intel_color_manager_crtc_commit(struct drm_device *dev,
> +		struct drm_crtc_state *crtc_state)
> +{
> +	struct drm_property_blob *blob;
> +	struct drm_crtc *crtc = crtc_state->crtc;
> +	int ret = 0;
> +
> +	if (IS_CHERRYVIEW(dev)) {
> +		blob = crtc_state->palette_after_ctm_blob;
> +		if (blob) {
> +			ret = chv_set_gamma(dev, blob, crtc);
> +			if (!ret)
> +				DRM_DEBUG_DRIVER(
> +					"Gamma registers Commit success!\n");
> +		}
> +
> +	}
> +}
> +
>  int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>  		struct drm_crtc_state *crtc_state,
>  		struct drm_mode_object *obj, uint32_t blob_id)
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> index 51aeb91..8bbac20 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -28,6 +28,26 @@
>  #include <drm/drm_crtc_helper.h>
>  #include "i915_drv.h"
>  
> +#define _GAMMA_INT_PART(value)			(value >> GAMMA_INT_SHIFT)
> +#define _GAMMA_FRACT_PART(value)		(value >> GAMMA_FRACT_SHIFT)
> +
>  #define CHV_PALETTE_STRUCT_VERSION		1
>  #define CHV_DEGAMMA_MAX_VALS			65
>  #define CHV_10BIT_GAMMA_MAX_VALS		257
> +
> +/* Gamma correction */
> +#define CHV_GAMMA_DATA_STRUCT_VERSION		1
> +#define CHV_10BIT_GAMMA_MAX_VALS		257
> +#define CHV_8BIT_GAMMA_MAX_VALS			256
> +#define CHV_10BIT_GAMMA_MSB_SHIFT		6
> +#define CHV_GAMMA_SHIFT_GREEN			16
> +/* Gamma values are u8.24 format */
> +#define GAMMA_INT_SHIFT				24
> +#define GAMMA_FRACT_SHIFT			8
> +/* Max int value for Gamma is 1 */
> +#define GAMMA_INT_MAX				1
> +/* Max value for Gamma on CHV */
> +#define CHV_MAX_GAMMA				0x10000
> +
> +/* CHV CGM Block */
> +#define CGM_GAMMA_EN				(1 << 2)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 11d491e..9e994fc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13773,6 +13773,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
>  
>  	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
>  		skl_detach_scalers(intel_crtc);
> +
> +	if (!needs_modeset(crtc->state))
> +		intel_color_manager_crtc_commit(dev, crtc->state);

It's not immediately clear to me why this is only for the
'!needs_modeset' case.  Does color management get setup somewhere else
in cases where a display update does involve a modeset?


Matt

>  }
>  
>  static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 45c42f0..d8d8326 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1456,5 +1456,7 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>  int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>  		struct drm_crtc_state *crtc_state,
>  		struct drm_mode_object *obj, uint32_t blob_id);
> +void intel_color_manager_crtc_commit(struct drm_device *dev,
> +		struct drm_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 2.4.5
> 

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

Re: [PATCH 10/16] drm/i915: Add set_property handler for pipe Gamma correction on CHV/BSW

[Malladi, Kausal]

On Tuesday 21 July 2015 05:33 AM, Matt Roper wrote:
> On Wed, Jul 15, 2015 at 06:39:34PM +0530, Kausal Malladi wrote:
>> This patch adds set_property handler for Gamma color correction and
>> enhancement capability at Pipe level on CHV/BSW platform. The set
>> function just attaches the Gamma blob to CRTC state, that later gets
>> committed using atomic path.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_atomic.c        |  7 +++++++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 19 +++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_drv.h           |  3 +++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index d873bda..72d6b37 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -473,6 +473,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
>>   				   struct drm_property *property,
>>   				   uint64_t val)
>>   {
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (property == config->prop_palette_after_ctm)
>> +		return intel_color_manager_set_pipe_gamma(dev, state,
>> +				&crtc->base, val);
>> +
>>   	DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>>   	return -EINVAL;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
>> index def20d0f..70c0d42 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,25 @@
>>   
>>   #include "intel_color_manager.h"
>>   
>> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>> +		struct drm_crtc_state *crtc_state,
>> +		struct drm_mode_object *obj, uint32_t blob_id)
>> +{
>> +	struct drm_property_blob *blob;
>> +
>> +	blob = drm_property_lookup_blob(dev, blob_id);
>> +	if (!blob) {
>> +		DRM_ERROR("Invalid Blob ID\n");
>> +		return -EINVAL;
>> +	}
> I'm not terribly familiar the semantics of the color correction
> stuff...this prevents userspace from removing the gamma setting by
> passing a 0; is that expected that gamma can't be disabled once set?
> Same question for your degamma patch later in the series.
If you mean to say this is preventing user space from disabling gamma, 
we have another provision for the same. During set property, if 
num_samples is 0 in the blob that was passed, it will disable gamma 
correction.

This check is just preventing user space to pass garbage blob ID.

Thanks
Kausal
>
> Matt
>
>> +
>> +	if (crtc_state->palette_after_ctm_blob)
>> +		drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
>> +
>> +	crtc_state->palette_after_ctm_blob = blob;
>> +	return 0;
>> +}
>> +
>>   int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
>>   		struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
>>   {
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 1e61036..45c42f0 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1453,5 +1453,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>>   /* intel_color_manager.c */
>>   void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>>   		struct drm_mode_object *mode_obj);
>> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>> +		struct drm_crtc_state *crtc_state,
>> +		struct drm_mode_object *obj, uint32_t blob_id);
>>   
>>   #endif /* __INTEL_DRV_H__ */
>> -- 
>> 2.4.5
>>

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

Re: [PATCH 11/16] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Malladi, Kausal]

On Tuesday 21 July 2015 05:33 AM, Matt Roper wrote:
> On Wed, Jul 15, 2015 at 06:39:35PM +0530, Kausal Malladi wrote:
>> CHV/BSW platform supports various Gamma correction modes, which are:
>> 1. Legacy 8-bit mode
>> 2. 10-bit CGM (Color Gamut Mapping) mode
>>
>> This patch does the following:
>> 1. Adds the core function to program Gamma correction values for CHV/BSW
>>     platform
>> 2. Adds Gamma correction macros/defines
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h            |  12 +++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 149 +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color_manager.h |  20 ++++
>>   drivers/gpu/drm/i915/intel_display.c       |   3 +
>>   drivers/gpu/drm/i915/intel_drv.h           |   2 +
>>   5 files changed, 186 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 059de0f..4ec2e4f 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7906,4 +7906,16 @@ enum skl_disp_power_wells {
>>   #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>>   #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>>   
>> +/* Color Management */
>> +#define PIPEA_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x67A00)
>> +#define PIPEB_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x69A00)
>> +#define PIPEC_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x6BA00)
>> +#define PIPEA_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x67000)
>> +#define PIPEB_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x69000)
>> +#define PIPEC_CGM_GAMMA				(VLV_DISPLAY_BASE + 0x6B000)
>> +#define _PIPE_CGM_CONTROL(pipe) \
>> +	(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
>> +#define _PIPE_GAMMA_BASE(pipe) \
>> +	(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
>> +
>>   #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 70c0d42..7e9e934 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,155 @@
>>   
>>   #include "intel_color_manager.h"
>>   
>> +int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
>> +		  struct drm_crtc *crtc)
>> +{
>> +	struct drm_palette *gamma_data;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +	u32 cgm_control_reg = 0;
>> +	u32 cgm_gamma_reg = 0;
>> +	enum pipe pipe;
>> +	u32 red, green, blue;
>> +	u8 red_int, blue_int, green_int;
>> +	u16 red_fract, green_fract, blue_fract;
>> +	u32 count = 0;
>> +	struct drm_r32g32b32 *correction_values = NULL;
>> +	u32 num_samples;
>> +	u32 word;
>> +	int ret = 0, length, flag = 0;
>> +
>> +	if (!blob) {
>> +		DRM_ERROR("NULL Blob\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	gamma_data = (struct drm_palette *)blob->data;
>> +
>> +	if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
>> +		DRM_ERROR("Invalid Gamma Data struct version\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pipe = to_intel_crtc(crtc)->pipe;
>> +	num_samples = gamma_data->num_samples;
>> +	length = num_samples * sizeof(struct drm_r32g32b32);
>> +
>> +	if (num_samples == 0) {
>> +
>> +		/* Disable Gamma functionality on Pipe - CGM Block */
>> +		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
>> +		cgm_control_reg &= ~CGM_GAMMA_EN;
>> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
>> +
>> +		DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
>> +				pipe_name(pipe));
>> +		ret = 0;
>> +	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS ||
>> +			num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
>> +		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
>> +
>> +		count = 0;
>> +		correction_values = (struct drm_r32g32b32 *)&gamma_data->lut;
>> +		while (count < num_samples) {
>> +			blue = correction_values[count].b32;
>> +			green = correction_values[count].g32;
>> +			red = correction_values[count].r32;
>> +
>> +			blue_int = _GAMMA_INT_PART(blue);
>> +			if (blue_int > GAMMA_INT_MAX)
>> +				blue = CHV_MAX_GAMMA;
>> +			green_int = _GAMMA_INT_PART(green);
>> +			if (green_int > GAMMA_INT_MAX)
>> +				green = CHV_MAX_GAMMA;
>> +			red_int = _GAMMA_INT_PART(red);
>> +			if (red_int > GAMMA_INT_MAX)
>> +				red = CHV_MAX_GAMMA;
>> +
>> +			blue_fract = _GAMMA_FRACT_PART(blue);
>> +			green_fract = _GAMMA_FRACT_PART(green);
>> +			red_fract = _GAMMA_FRACT_PART(red);
>> +
>> +			blue_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
>> +			green_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
>> +			red_fract >>= CHV_10BIT_GAMMA_MSB_SHIFT;
>> +
>> +			/* Green (25:16) and Blue (9:0) to be written */
>> +			word = green_fract;
>> +			word <<= CHV_GAMMA_SHIFT_GREEN;
>> +			word = word | blue_fract;
>> +			I915_WRITE(cgm_gamma_reg, word);
>> +			cgm_gamma_reg += 4;
>> +
>> +			/* Red (9:0) to be written */
>> +			word = red_fract;
>> +			I915_WRITE(cgm_gamma_reg, word);
>> +
>> +			cgm_gamma_reg += 4;
>> +			count++;
>> +
>> +			/*
>> +			 * On CHV, the best supported Gamma capability is
>> +			 * CGM block, that takes 257 samples.
>> +			 * If user gives 256 samples (legacy mode), then
>> +			 * duplicate the 256th value to 257th.
>> +			 * "flag" is used to make sure it happens only once
>> +			 */
>> +			if (num_samples == CHV_8BIT_GAMMA_MAX_VALS
>> +					&& count == CHV_8BIT_GAMMA_MAX_VALS
>> +					&& !flag) {
>> +				count--;
>> +				flag = 1;
>> +			}
>> +		}
>> +
>> +		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
>> +			pipe_name(pipe));
>> +
>> +		/* Enable (CGM) Gamma on Pipe */
>> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe),
>> +			I915_READ(_PIPE_CGM_CONTROL(pipe))
>> +			| CGM_GAMMA_EN);
>> +		DRM_DEBUG_DRIVER("CGM Gamma enabled on Pipe %c\n",
>> +				pipe_name(pipe));
>> +
>> +		ret = 0;
>> +	} else {
>> +		DRM_ERROR("Invalid number of samples for Gamma LUT\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = drm_property_replace_global_blob(dev, &blob, length,
>> +			(void *) gamma_data, &crtc->base,
>> +			config->prop_palette_after_ctm);
> Is this supposed to be here?  This function seems to be for just
> programming the hardware with the contents of the current blob.  I'm not
> sure I understand why we try to alter the blob itself at the end of
> that.  I see the same in your degamma patch.

We were trying to free the existing blob assuming it can cause memory 
leak, but after your comment, it makes sense to keep the blob as it is 
created by the user space and it might want to contain it. We will make 
this change.
>
>> +
>> +	if (ret) {
>> +		DRM_ERROR("Error updating Gamma blob\n");
>> +		return -EFAULT;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +void intel_color_manager_crtc_commit(struct drm_device *dev,
>> +		struct drm_crtc_state *crtc_state)
>> +{
>> +	struct drm_property_blob *blob;
>> +	struct drm_crtc *crtc = crtc_state->crtc;
>> +	int ret = 0;
>> +
>> +	if (IS_CHERRYVIEW(dev)) {
>> +		blob = crtc_state->palette_after_ctm_blob;
>> +		if (blob) {
>> +			ret = chv_set_gamma(dev, blob, crtc);
>> +			if (!ret)
>> +				DRM_DEBUG_DRIVER(
>> +					"Gamma registers Commit success!\n");
>> +		}
>> +
>> +	}
>> +}
>> +
>>   int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>>   		struct drm_crtc_state *crtc_state,
>>   		struct drm_mode_object *obj, uint32_t blob_id)
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
>> index 51aeb91..8bbac20 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.h
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
>> @@ -28,6 +28,26 @@
>>   #include <drm/drm_crtc_helper.h>
>>   #include "i915_drv.h"
>>   
>> +#define _GAMMA_INT_PART(value)			(value >> GAMMA_INT_SHIFT)
>> +#define _GAMMA_FRACT_PART(value)		(value >> GAMMA_FRACT_SHIFT)
>> +
>>   #define CHV_PALETTE_STRUCT_VERSION		1
>>   #define CHV_DEGAMMA_MAX_VALS			65
>>   #define CHV_10BIT_GAMMA_MAX_VALS		257
>> +
>> +/* Gamma correction */
>> +#define CHV_GAMMA_DATA_STRUCT_VERSION		1
>> +#define CHV_10BIT_GAMMA_MAX_VALS		257
>> +#define CHV_8BIT_GAMMA_MAX_VALS			256
>> +#define CHV_10BIT_GAMMA_MSB_SHIFT		6
>> +#define CHV_GAMMA_SHIFT_GREEN			16
>> +/* Gamma values are u8.24 format */
>> +#define GAMMA_INT_SHIFT				24
>> +#define GAMMA_FRACT_SHIFT			8
>> +/* Max int value for Gamma is 1 */
>> +#define GAMMA_INT_MAX				1
>> +/* Max value for Gamma on CHV */
>> +#define CHV_MAX_GAMMA				0x10000
>> +
>> +/* CHV CGM Block */
>> +#define CGM_GAMMA_EN				(1 << 2)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 11d491e..9e994fc 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13773,6 +13773,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
>>   
>>   	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
>>   		skl_detach_scalers(intel_crtc);
>> +
>> +	if (!needs_modeset(crtc->state))
>> +		intel_color_manager_crtc_commit(dev, crtc->state);
> It's not immediately clear to me why this is only for the
> '!needs_modeset' case.  Does color management get setup somewhere else
> in cases where a display update does involve a modeset?
This is just an extra precaution. Do you think that we should remove it?

Kausal
>
>
> Matt
>
>>   }
>>   
>>   static void intel_finish_crtc_commit(struct drm_crtc *crtc)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 45c42f0..d8d8326 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1456,5 +1456,7 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>>   int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>>   		struct drm_crtc_state *crtc_state,
>>   		struct drm_mode_object *obj, uint32_t blob_id);
>> +void intel_color_manager_crtc_commit(struct drm_device *dev,
>> +		struct drm_crtc_state *crtc_state);
>>   
>>   #endif /* __INTEL_DRV_H__ */
>> -- 
>> 2.4.5
>>

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

Re: [PATCH 11/16] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Matt Roper]

On Tue, Jul 21, 2015 at 04:40:08PM +0530, Malladi, Kausal wrote:
> On Tuesday 21 July 2015 05:33 AM, Matt Roper wrote:
> >On Wed, Jul 15, 2015 at 06:39:35PM +0530, Kausal Malladi wrote:
...
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index 11d491e..9e994fc 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -13773,6 +13773,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> >>  	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
> >>  		skl_detach_scalers(intel_crtc);
> >>+
> >>+	if (!needs_modeset(crtc->state))
> >>+		intel_color_manager_crtc_commit(dev, crtc->state);
> >It's not immediately clear to me why this is only for the
> >'!needs_modeset' case.  Does color management get setup somewhere else
> >in cases where a display update does involve a modeset?
> This is just an extra precaution. Do you think that we should remove it?
> 
> Kausal

It seems to me that if you updated your color settings in the process of
also doing a full modeset, then those new settings wouldn't take effect
until you followed up with another, non-modeset, pageflip.  Unless I'm
overlooking something that would cause them to be programmed on the
first frame anyway?


Matt

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

Re: [PATCH 03/16] drm/i915: Attach color properties to CRTC

[Sharma, Shashank]

HI Jim,
Thanks for the review.
My comments inline. 

Regards
Shashank
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Bish, Jim
Sent: Tuesday, July 21, 2015 3:44 AM
To: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 03/16] drm/i915: Attach color properties to CRTC



On 07/15/2015 06:09 AM, Kausal Malladi wrote:
> This patch does the following:
> 1. Adds new files intel_color_manager(.c/.h) 2. Attaches color 
> properties to CRTC while initialization
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile              |  3 +-
>  drivers/gpu/drm/i915/intel_color_manager.c | 49 
> ++++++++++++++++++++++++++++++  drivers/gpu/drm/i915/intel_color_manager.h | 29 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c       |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h           |  4 +++
>  5 files changed, 86 insertions(+), 1 deletion(-)  create mode 100644 
> drivers/gpu/drm/i915/intel_color_manager.c
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile 
> b/drivers/gpu/drm/i915/Makefile index de21965..ad928d8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -56,7 +56,8 @@ i915-y += intel_audio.o \
>  	  intel_overlay.o \
>  	  intel_psr.o \
>  	  intel_sideband.o \
> -	  intel_sprite.o
> +	  intel_sprite.o \
> +	  intel_color_manager.o
>  i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
>  i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
>  
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> new file mode 100644
> index 0000000..baa4536
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including 
> +the next
> + * paragraph) shall be included in all copies or substantial portions 
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
> +OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
> +OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>  */
> +
> +#include "intel_color_manager.h"
> +
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (IS_CHERRYVIEW(dev)) {
> +		if (config->prop_color_capabilities)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_color_capabilities, 0);
> +		if (config->prop_palette_before_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_before_ctm, 0);
> +		if (config->prop_palette_after_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_after_ctm, 0);
> +		if (config->prop_ctm)
> +			drm_object_attach_property(mode_obj,
> +					config->prop_ctm, 0);
> +	}
why only CHT?  Seems we should be putting cases for all of our devices.
Yes, this should be for all, We will changes this. 
> +}
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
> b/drivers/gpu/drm/i915/intel_color_manager.h
> new file mode 100644
> index 0000000..04c921d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including 
> +the next
> + * paragraph) shall be included in all copies or substantial portions 
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
> +OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
> +OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Kausal Malladi <Kausal.Malladi@intel.com>  */ #include 
> +<drm/drmP.h> #include <drm/drm_crtc_helper.h> #include "i915_drv.h"
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index bb58cb6..b6e1dc5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14111,6 +14111,8 @@ static void intel_crtc_init(struct drm_device 
> *dev, int pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	intel_attach_color_properties_to_crtc(dev, &intel_crtc->base.base);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = 
> &intel_crtc->base; diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index b9c01c5..05c809b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1446,4 +1446,8 @@ void intel_plane_destroy_state(struct drm_plane *plane,
>  			       struct drm_plane_state *state);  extern const struct 
> drm_plane_helper_funcs intel_plane_helper_funcs;
>  
> +/* intel_color_manager.c */
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj);
> +
>  #endif /* __INTEL_DRV_H__ */
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/16] drm: Add structure for querying palette color capabilities

[Sharma, Shashank]

Regards
Shashank

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Bish, Jim
Sent: Tuesday, July 21, 2015 3:48 AM
To: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 04/16] drm: Add structure for querying palette color capabilities



On 07/15/2015 06:09 AM, Kausal Malladi wrote:
> The DRM color management framework is targeting various hardware 
> platforms and drivers. Different platforms can have different color 
> correction and enhancement capabilities.
> 
> A commom user space application can query these capabilities using the 
> DRM property interface. Each driver can fill this property with its 
> platform's palette color capabilities.
> 
> This patch adds new structure in DRM layer for querying palette color 
> capabilities. This structure will be used by all user space agents to 
> configure appropriate color configurations.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  include/uapi/drm/drm.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 
> 3801584..e3c642f 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -829,6 +829,17 @@ struct drm_event_vblank {
>  	__u32 reserved;
>  };
>  
> +struct drm_palette_caps {
> +	/* Structure version. Should be 1 currently */
> +	__u32 version;
> +	/* For padding and future use */
> +	__u32 reserved;
> +	/* This may be 0 if not supported. e.g. plane palette or VLV pipe */
> +	__u32 num_samples_before_ctm;
> +	/* This will be non-zero for pipe. May be zero for planes on some HW */
> +	__u32 num_samples_after_ctm;
> +};
this structure does not match what is documented in the design document.  are we missing updates to the design document?
Yes, Susanta will be updating the document in a day or two. Thanks for reminding us. 
> +
>  /* typedef area */
>  #ifndef __KERNEL__
>  typedef struct drm_clip_rect drm_clip_rect_t;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx