[PATCH 00/12] Color Manager Implementation

[PATCH 00/12] Color Manager Implementation

[Kausal Malladi]

NOTE: Re-sending the same series for more feedback, as we realized the mails didn't reach the dri-devel and intel-gfx lists because of some problem.

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.

Kausal Malladi (11):
  drm: Create Color Management DRM properties
  drm/i915: Attach color properties to CRTC
  drm: Add structures for querying color capabilities
  drm/i915: Load color capabilities for CHV CRTC
  drm/i915: Add atomic set property interface for CRTC
  drm: Add structures to set/get a palette color property
  drm: Export drm_property_replace_global_blob function
  drm/i915: Add pipe level Gamma correction for CHV/BSW
  drm/i915: Add DeGamma correction for CHV/BSW
  drm: Add structure for set/get a CTM color property
  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                 |  26 +-
 drivers/gpu/drm/i915/Makefile              |   3 +-
 drivers/gpu/drm/i915/i915_reg.h            |  18 +
 drivers/gpu/drm/i915/intel_atomic.c        |  30 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 530 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  74 ++++
 drivers/gpu/drm/i915/intel_display.c       |   5 +
 drivers/gpu/drm/i915/intel_drv.h           |  14 +
 include/drm/drm_crtc.h                     |  12 +
 include/uapi/drm/drm.h                     |  51 +++
 10 files changed, 761 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/12] 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 0aeced8..126e092 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/12] 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 | 24 ++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2d57fc5..5d12ea9 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1462,6 +1462,30 @@ 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);
+	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

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

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

[Matt Roper]

On Fri, Jul 03, 2015 at 09:01:37AM +0530, Kausal Malladi wrote:
> 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 | 24 ++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  6 ++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 2d57fc5..5d12ea9 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1462,6 +1462,30 @@ 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);

Is there a specific reason you don't check for NULL on this property
like you do on all the others, or is this just an oversight?


Matt

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

-- 
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 02/12] drm: Create Color Management DRM properties

[Malladi, Kausal]

Thanks for your review, Matt! Our responses inline...

Kausal

On Wednesday 08 July 2015 04:53 AM, Matt Roper wrote:
> On Fri, Jul 03, 2015 at 09:01:37AM +0530, Kausal Malladi wrote:
>> 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 | 24 ++++++++++++++++++++++++
>>   include/drm/drm_crtc.h     |  6 ++++++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 2d57fc5..5d12ea9 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -1462,6 +1462,30 @@ 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);
> Is there a specific reason you don't check for NULL on this property
> like you do on all the others, or is this just an oversight?
Oops, it was an oversight. Thanks for bringing it to notice.
>
>
> Matt
>
>> +	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
>>

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

[PATCH 03/12] 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 | 61 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h | 28 ++++++++++++++
 drivers/gpu/drm/i915/intel_display.c       |  3 ++
 drivers/gpu/drm/i915/intel_drv.h           |  4 ++
 5 files changed, 98 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..9280ea6
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -0,0 +1,61 @@
+/*
+ * 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_color_manager_attach(struct drm_device *dev,
+		struct drm_mode_object *mode_obj)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (mode_obj->type == DRM_MODE_OBJECT_CRTC) {
+		if (config->prop_color_capabilities) {
+			drm_object_attach_property(mode_obj,
+					config->prop_color_capabilities, 0);
+			DRM_DEBUG_DRIVER("Attached Color Caps property to CRTC\n");
+		} else
+			DRM_ERROR("Error attaching Color Capabilities property to CRTC\n");
+		if (config->prop_palette_before_ctm) {
+			drm_object_attach_property(mode_obj,
+					config->prop_palette_before_ctm, 0);
+			DRM_DEBUG_DRIVER("Attached Palette (before CTM) property to CRTC\n");
+		} else
+			DRM_ERROR("Error attaching Palette (before CTM) property to CRTC\n");
+		if (config->prop_palette_after_ctm) {
+			drm_object_attach_property(mode_obj,
+					config->prop_palette_after_ctm, 0);
+			DRM_DEBUG_DRIVER("Attached Palette (after CTM) property to CRTC\n");
+		} else
+			DRM_ERROR("Error attaching Palette (after CTM) property to CRTC\n");
+		if (config->prop_ctm) {
+			drm_object_attach_property(mode_obj,
+					config->prop_ctm, 0);
+			DRM_DEBUG_DRIVER("Attached CTM property to CRTC\n");
+		} else
+			DRM_ERROR("Error attaching CTM property to CRTC\n");
+	}
+}
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..c564761
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -0,0 +1,28 @@
+/*
+ * 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>
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..e175df2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14105,6 +14105,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	intel_crtc->wm.cxsr_allowed = true;
 
+	/* Attaching color properties to the CRTC */
+	intel_color_manager_attach(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 3f0a890..1e18a7e 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_color_manager_attach(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

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

[Matt Roper]

On Fri, Jul 03, 2015 at 09:01:38AM +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>
> ---
>  drivers/gpu/drm/i915/Makefile              |  3 +-
>  drivers/gpu/drm/i915/intel_color_manager.c | 61 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h | 28 ++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c       |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h           |  4 ++
>  5 files changed, 98 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..9280ea6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,61 @@
> +/*
> + * 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_color_manager_attach(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (mode_obj->type == DRM_MODE_OBJECT_CRTC) {

Is the expectation that this function will grow to include plane
properties as well?  Personally I think it would be a little easier to
read/follow if we had separate functions for attaching the crtc
properties and the (eventual) plane properties, but it's not a huge
deal.

> +		if (config->prop_color_capabilities) {
> +			drm_object_attach_property(mode_obj,
> +					config->prop_color_capabilities, 0);
> +			DRM_DEBUG_DRIVER("Attached Color Caps property to CRTC\n");
> +		} else
> +			DRM_ERROR("Error attaching Color Capabilities property to CRTC\n");
> +		if (config->prop_palette_before_ctm) {
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_before_ctm, 0);
> +			DRM_DEBUG_DRIVER("Attached Palette (before CTM) property to CRTC\n");
> +		} else
> +			DRM_ERROR("Error attaching Palette (before CTM) property to CRTC\n");
> +		if (config->prop_palette_after_ctm) {
> +			drm_object_attach_property(mode_obj,
> +					config->prop_palette_after_ctm, 0);
> +			DRM_DEBUG_DRIVER("Attached Palette (after CTM) property to CRTC\n");
> +		} else
> +			DRM_ERROR("Error attaching Palette (after CTM) property to CRTC\n");
> +		if (config->prop_ctm) {
> +			drm_object_attach_property(mode_obj,
> +					config->prop_ctm, 0);
> +			DRM_DEBUG_DRIVER("Attached CTM property to CRTC\n");
> +		} else
> +			DRM_ERROR("Error attaching CTM property to CRTC\n");
> +	}

I agree with Damien's note that we can probably drop the messages here.
Also note that in cases like this the kernel coding style standards
indicate that even though your 'else' branches are only a single
statement, we should still use braces because braces were used on the
'if' branch.

Do we intend to expose these properties to userspace on all hardware
generations?  I'd expect us to only add the properties to the crtc's
(and planes) if the hardware can actually support it.


Matt

> +}
> 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..c564761
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -0,0 +1,28 @@
> +/*
> + * 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>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 724b0e3..e175df2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14105,6 +14105,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	/* Attaching color properties to the CRTC */
> +	intel_color_manager_attach(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 3f0a890..1e18a7e 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_color_manager_attach(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 03/12] drm/i915: Attach color properties to CRTC

[Malladi, Kausal]

On Wednesday 08 July 2015 04:53 AM, Matt Roper wrote:
> On Fri, Jul 03, 2015 at 09:01:38AM +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>
>> ---
>>   drivers/gpu/drm/i915/Makefile              |  3 +-
>>   drivers/gpu/drm/i915/intel_color_manager.c | 61 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color_manager.h | 28 ++++++++++++++
>>   drivers/gpu/drm/i915/intel_display.c       |  3 ++
>>   drivers/gpu/drm/i915/intel_drv.h           |  4 ++
>>   5 files changed, 98 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..9280ea6
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -0,0 +1,61 @@
>> +/*
>> + * 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_color_manager_attach(struct drm_device *dev,
>> +		struct drm_mode_object *mode_obj)
>> +{
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +
>> +	if (mode_obj->type == DRM_MODE_OBJECT_CRTC) {
> Is the expectation that this function will grow to include plane
> properties as well?  Personally I think it would be a little easier to
> read/follow if we had separate functions for attaching the crtc
> properties and the (eventual) plane properties, but it's not a huge
> deal.
Yes, it is expected to grow to include plane properties as well.
>
>> +		if (config->prop_color_capabilities) {
>> +			drm_object_attach_property(mode_obj,
>> +					config->prop_color_capabilities, 0);
>> +			DRM_DEBUG_DRIVER("Attached Color Caps property to CRTC\n");
>> +		} else
>> +			DRM_ERROR("Error attaching Color Capabilities property to CRTC\n");
>> +		if (config->prop_palette_before_ctm) {
>> +			drm_object_attach_property(mode_obj,
>> +					config->prop_palette_before_ctm, 0);
>> +			DRM_DEBUG_DRIVER("Attached Palette (before CTM) property to CRTC\n");
>> +		} else
>> +			DRM_ERROR("Error attaching Palette (before CTM) property to CRTC\n");
>> +		if (config->prop_palette_after_ctm) {
>> +			drm_object_attach_property(mode_obj,
>> +					config->prop_palette_after_ctm, 0);
>> +			DRM_DEBUG_DRIVER("Attached Palette (after CTM) property to CRTC\n");
>> +		} else
>> +			DRM_ERROR("Error attaching Palette (after CTM) property to CRTC\n");
>> +		if (config->prop_ctm) {
>> +			drm_object_attach_property(mode_obj,
>> +					config->prop_ctm, 0);
>> +			DRM_DEBUG_DRIVER("Attached CTM property to CRTC\n");
>> +		} else
>> +			DRM_ERROR("Error attaching CTM property to CRTC\n");
>> +	}
> I agree with Damien's note that we can probably drop the messages here.
> Also note that in cases like this the kernel coding style standards
> indicate that even though your 'else' branches are only a single
> statement, we should still use braces because braces were used on the
> 'if' branch.
Sure, got it.
>
> Do we intend to expose these properties to userspace on all hardware
> generations?  I'd expect us to only add the properties to the crtc's
> (and planes) if the hardware can actually support it.
Yes, sure.
>
>
> Matt
>
>> +}
>> 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..c564761
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * 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>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 724b0e3..e175df2 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14105,6 +14105,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>>   
>>   	intel_crtc->wm.cxsr_allowed = true;
>>   
>> +	/* Attaching color properties to the CRTC */
>> +	intel_color_manager_attach(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 3f0a890..1e18a7e 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_color_manager_attach(struct drm_device *dev,
>> +		struct drm_mode_object *mode_obj);
>> +
>>   #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 04/12] drm: Add structures for querying 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 color capabilities.

This patch adds new structures in DRM layer for querying color
capabilities. These structures 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 | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..d9562a2 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -829,6 +829,40 @@ struct drm_event_vblank {
 	__u32 reserved;
 };
 
+struct drm_palette_sampling_details {
+	__u32 sample_fract_precision;
+	__u32 last_sample_int_max;
+	__u32 remaining_sample_int_max;
+	__u32 num_samples;
+};
+
+struct drm_palette_caps {
+	__u32 version;
+	__u32 num_supported_types;
+	struct drm_palette_sampling_details palette_sampling_types[4];
+};
+
+struct drm_ctm_caps {
+	__u32 version;
+	__u32 ctm_coeff_fract_precision;
+	__u32 ctm_coeff_int_max;
+	__s32 ctm_coeff_int_min;
+};
+
+struct drm_cge_caps {
+	__u32 version;
+	__u32 cge_max_weight;
+};
+
+struct drm_color_caps {
+	__u32 version;
+	__u32 reserved;
+	struct drm_palette_caps palette_caps_after_ctm;
+	struct drm_palette_caps palette_caps_before_ctm;
+	struct drm_ctm_caps ctm_caps;
+	struct drm_cge_caps cge_caps;
+};
+
 /* 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 05/12] drm/i915: Load color capabilities for CHV CRTC

[Kausal Malladi]

As per Color Manager design, each driver is responsible to load its
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 color capabilities for CRTC
into a blob that can be accessible by user space to
query capabilities via DRM property interface.

CRTC properties added in this patch for CHV are:
1. DeGamma (Palette correction before CTM)
2. Gamma (Palette correction after CTM)
3. CSC (CTM)

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 | 95 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h | 20 +++++++
 2 files changed, 115 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index 9280ea6..71b4c05 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,18 +27,113 @@
 
 #include "intel_color_manager.h"
 
+int get_chv_pipe_capabilities(struct drm_device *dev,
+		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
+{
+	struct drm_palette_caps palette_caps_after_ctm;
+	struct drm_palette_caps palette_caps_before_ctm;
+	struct drm_palette_sampling_details
+		palette_sampling_before_ctm[CHV_MAX_PALETTE_CAPS_BEFORE_CTM];
+	struct drm_palette_sampling_details
+		palette_sampling_after_ctm[CHV_MAX_PALETTE_CAPS_AFTER_CTM];
+	struct drm_ctm_caps ctm_caps;
+	struct drm_property_blob *blob = NULL;
+	struct drm_mode_config *config = &dev->mode_config;
+	int ret;
+
+	palette_sampling_before_ctm[0].sample_fract_precision =
+		CHV_DEGAMMA_PRECISION;
+	palette_sampling_before_ctm[0].last_sample_int_max =
+		CHV_DEGAMMA_LAST_SAMPLE_INT_MAX;
+	palette_sampling_before_ctm[0].remaining_sample_int_max =
+		CHV_DEGAMMA_SAMPLE_INT_MAX;
+	palette_sampling_before_ctm[0].num_samples =
+		CHV_DEGAMMA_MAX_VALS;
+
+	palette_caps_before_ctm.version = CHV_PALETTE_STRUCT_VERSION;
+	palette_caps_before_ctm.num_supported_types =
+		CHV_MAX_PALETTE_CAPS_BEFORE_CTM;
+	palette_caps_before_ctm.palette_sampling_types[0] =
+		palette_sampling_before_ctm[0];
+
+	palette_sampling_after_ctm[0].sample_fract_precision =
+		CHV_10BIT_GAMMA_PRECISION;
+	palette_sampling_after_ctm[0].last_sample_int_max =
+		CHV_GAMMA_LAST_SAMPLE_INT_MAX;
+	palette_sampling_after_ctm[0].remaining_sample_int_max =
+		CHV_GAMMA_SAMPLE_INT_MAX;
+	palette_sampling_after_ctm[0].num_samples =
+		CHV_10BIT_GAMMA_MAX_VALS;
+
+	palette_sampling_after_ctm[1].sample_fract_precision =
+		CHV_8BIT_GAMMA_PRECISION;
+	palette_sampling_after_ctm[1].last_sample_int_max =
+		CHV_GAMMA_LAST_SAMPLE_INT_MAX;
+	palette_sampling_after_ctm[1].remaining_sample_int_max =
+		CHV_GAMMA_SAMPLE_INT_MAX;
+	palette_sampling_after_ctm[1].num_samples =
+		CHV_8BIT_GAMMA_MAX_VALS;
+
+	palette_caps_after_ctm.version = CHV_PALETTE_STRUCT_VERSION;
+	palette_caps_after_ctm.num_supported_types =
+		CHV_MAX_PALETTE_CAPS_AFTER_CTM;
+	palette_caps_after_ctm.palette_sampling_types[0] =
+		palette_sampling_after_ctm[0];
+	palette_caps_after_ctm.palette_sampling_types[1] =
+		palette_sampling_after_ctm[1];
+
+	ctm_caps.version = CHV_CTM_STRUCT_VERSION;
+	ctm_caps.ctm_coeff_fract_precision = CHV_CSC_COEFF_MAX_PRECISION;
+	ctm_caps.ctm_coeff_int_max = CHV_CSC_COEFF_MAX_INT;
+	ctm_caps.ctm_coeff_int_min = CHV_CSC_COEFF_MIN_INT;
+
+	color_caps->version = CHV_PLATFORM_STRUCT_VERSION;
+	color_caps->palette_caps_after_ctm = palette_caps_after_ctm;
+	color_caps->palette_caps_before_ctm = palette_caps_before_ctm;
+	color_caps->ctm_caps = ctm_caps;
+
+	ret = drm_property_replace_global_blob(dev, &blob,
+			sizeof(struct drm_color_caps),
+			(const void *)color_caps,
+			&crtc->base, config->prop_color_capabilities);
+	if (ret) {
+		DRM_ERROR("Error updating Gamma blob\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+int get_pipe_capabilities(struct drm_device *dev,
+		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
+{
+	if (IS_CHERRYVIEW(dev))
+		return get_chv_pipe_capabilities(dev, color_caps, crtc);
+	return -EINVAL;
+}
+
 void intel_color_manager_attach(struct drm_device *dev,
 		struct drm_mode_object *mode_obj)
 {
 	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_color_caps *color_caps;
+	struct drm_crtc *crtc;
+	int ret;
 
 	if (mode_obj->type == DRM_MODE_OBJECT_CRTC) {
+		crtc = obj_to_crtc(mode_obj);
 		if (config->prop_color_capabilities) {
 			drm_object_attach_property(mode_obj,
 					config->prop_color_capabilities, 0);
 			DRM_DEBUG_DRIVER("Attached Color Caps property to CRTC\n");
 		} else
 			DRM_ERROR("Error attaching Color Capabilities property to CRTC\n");
+
+		color_caps = kzalloc(sizeof(struct drm_color_caps), GFP_KERNEL);
+		ret = get_pipe_capabilities(dev, color_caps, crtc);
+		if (ret)
+			DRM_ERROR("Error getting CRTC capabilities for platform\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 c564761..32262ac 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -26,3 +26,23 @@
  */
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include "i915_drv.h"
+
+#define CHV_PALETTE_STRUCT_VERSION		1
+#define CHV_CTM_STRUCT_VERSION			1
+#define CHV_PLATFORM_STRUCT_VERSION		1
+#define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
+#define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
+#define CHV_DEGAMMA_PRECISION			14
+#define CHV_DEGAMMA_LAST_SAMPLE_INT_MAX		0
+#define CHV_DEGAMMA_SAMPLE_INT_MAX		0
+#define CHV_DEGAMMA_MAX_VALS			65
+#define CHV_10BIT_GAMMA_PRECISION		10
+#define CHV_GAMMA_LAST_SAMPLE_INT_MAX		0
+#define CHV_GAMMA_SAMPLE_INT_MAX		0
+#define CHV_10BIT_GAMMA_MAX_VALS		257
+#define CHV_8BIT_GAMMA_PRECISION		8
+#define CHV_8BIT_GAMMA_MAX_VALS			256
+#define CHV_CSC_COEFF_MAX_PRECISION		12
+#define CHV_CSC_COEFF_MAX_INT			7
+#define CHV_CSC_COEFF_MIN_INT			-7
-- 
2.4.5

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

[PATCH 06/12] 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 126e092..d2674a6 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 e175df2..d541617 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13471,6 +13471,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 1e18a7e..053ceb0 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 07/12] 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index d9562a2..04a8f2a 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -863,6 +863,18 @@ struct drm_color_caps {
 	struct drm_cge_caps cge_caps;
 };
 
+struct drm_r32g32b32 {
+	__u32 r32;
+	__u32 g32;
+	__u32 b32;
+};
+
+struct drm_palette {
+	__u32 version;
+	__u32 palette_num_samples;
+	struct drm_r32g32b32 palette_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

Re: [PATCH 07/12] drm: Add structures to set/get a palette color property

[Emil Velikov]

Hi Kausal,

On 03/07/15 04:31, Kausal Malladi wrote:
> 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 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index d9562a2..04a8f2a 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -863,6 +863,18 @@ struct drm_color_caps {
>  	struct drm_cge_caps cge_caps;
>  };
>  
> +struct drm_r32g32b32 {
> +	__u32 r32;
> +	__u32 g32;
> +	__u32 b32;
> +};
> +
I don't think this will work on a 64bit kernel with 32 bit userspace...
although I'm low on caffeine I could be imagining.


> +struct drm_palette {
> +	__u32 version;
> +	__u32 palette_num_samples;
> +	struct drm_r32g32b32 palette_lut[0];
If memory serves me right, I've mentioned earlier that using zero sized
arrays might not be so good considering portability and using non GCC
compilers.

I believe your consern was about that using a pointer is inefficient.
Can you provide some references/hints how is that so ?

Thanks
Emil

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

[PATCH 08/12] 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 | 2 +-
 include/drm/drm_crtc.h     | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5d12ea9..15263a1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4458,7 +4458,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,
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

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

[PATCH 09/12] 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            |  10 ++
 drivers/gpu/drm/i915/intel_atomic.c        |   6 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 154 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |  12 +++
 drivers/gpu/drm/i915/intel_drv.h           |   2 +
 5 files changed, 184 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 313b1f9..36672e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7900,4 +7900,14 @@ 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 PIPEA_CGM_GAMMA_MIN			(VLV_DISPLAY_BASE + 0x67000)
+#define CGM_OFFSET				0x2000
+#define GAMMA_OFFSET				0x2000
+#define _PIPE_CGM_CONTROL(pipe) \
+	(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
+#define _PIPE_GAMMA_BASE(pipe) \
+	(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index d2674a6..21f0ac2 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -473,6 +473,12 @@ 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_gamma(dev, &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 71b4c05..84cc3e47 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,160 @@
 
 #include "intel_color_manager.h"
 
+int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
+		  struct drm_crtc *crtc)
+{
+	struct drm_palette *gamma_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property_blob *blob;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 cgm_control_reg = 0;
+	u32 cgm_gamma_reg = 0;
+	u32 reg, val;
+	enum pipe pipe;
+	u16 red, green, blue;
+	u32 count = 0;
+	struct drm_r32g32b32 *correction_values = NULL;
+	u32 num_samples;
+	u32 word;
+	u32 palette;
+	int ret = 0, length;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\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->palette_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_8BIT_GAMMA_MAX_VALS) {
+
+		/* Disable CGM Gamma, if already set */
+		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+		cgm_control_reg &= ~CGM_GAMMA_EN;
+		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+		/* Write LUT values to registers */
+		palette = _PIPE_GAMMA_BASE(pipe);
+		count = 0;
+		correction_values =
+			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
+		while (count < num_samples) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue = blue >> CHV_8BIT_GAMMA_MSB_SHIFT;
+			green = green >> CHV_8BIT_GAMMA_MSB_SHIFT;
+			red = red >> CHV_8BIT_GAMMA_MSB_SHIFT;
+
+			/* Red (23:16), Green (15:8), Blue (7:0) */
+			word = (red << CHV_8BIT_GAMMA_SHIFT_RED_REG) |
+				(green <<
+				 CHV_8BIT_GAMMA_SHIFT_GREEN_REG) |
+				blue;
+			I915_WRITE(palette, word);
+
+			palette += 4;
+			count++;
+		}
+		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
+				pipe_name(pipe));
+
+		/* Enable Gamma on Pipe */
+		reg = PIPECONF(pipe);
+		val = I915_READ(reg) | PIPECONF_GAMMA;
+		I915_WRITE(reg, val);
+		DRM_DEBUG_DRIVER("Legacy Gamma enabled on Pipe %c\n",
+				pipe_name(pipe));
+
+		ret = 0;
+	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
+		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
+
+		count = 0;
+		correction_values =
+			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
+		while (count < num_samples) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue = blue >> CHV_10BIT_GAMMA_MSB_SHIFT;
+			green = green >> CHV_10BIT_GAMMA_MSB_SHIFT;
+			red = red >> CHV_10BIT_GAMMA_MSB_SHIFT;
+
+			/* Green (25:16) and Blue (9:0) to be written */
+			word = (green << CHV_GAMMA_SHIFT_GREEN) | blue;
+			I915_WRITE(cgm_gamma_reg, word);
+			cgm_gamma_reg += 4;
+
+			/* Red (9:0) to be written */
+			word = red;
+			I915_WRITE(cgm_gamma_reg, word);
+
+			cgm_gamma_reg += 4;
+			count++;
+		}
+		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;
+}
+
+int intel_color_manager_set_gamma(struct drm_device *dev,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_crtc *crtc = obj_to_crtc(obj);
+
+	if (IS_CHERRYVIEW(dev))
+		return chv_set_gamma(dev, blob_id, crtc);
+
+	return -EINVAL;
+}
+
 int get_chv_pipe_capabilities(struct drm_device *dev,
 		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 32262ac..d83567a 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -31,6 +31,8 @@
 #define CHV_PALETTE_STRUCT_VERSION		1
 #define CHV_CTM_STRUCT_VERSION			1
 #define CHV_PLATFORM_STRUCT_VERSION		1
+#define CHV_GAMMA_DATA_STRUCT_VERSION		1
+
 #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
 #define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
 #define CHV_DEGAMMA_PRECISION			14
@@ -43,6 +45,16 @@
 #define CHV_10BIT_GAMMA_MAX_VALS		257
 #define CHV_8BIT_GAMMA_PRECISION		8
 #define CHV_8BIT_GAMMA_MAX_VALS			256
+#define CHV_8BIT_GAMMA_MSB_SHIFT		8
+#define CHV_8BIT_GAMMA_SHIFT_RED_REG		16
+#define CHV_8BIT_GAMMA_SHIFT_GREEN_REG		8
+#define CHV_10BIT_GAMMA_MSB_SHIFT		6
+#define CHV_GAMMA_SHIFT_GREEN			16
+
 #define CHV_CSC_COEFF_MAX_PRECISION		12
 #define CHV_CSC_COEFF_MAX_INT			7
 #define CHV_CSC_COEFF_MIN_INT			-7
+
+/* CHV CGM Block */
+/* Bit 2 to be enabled in CGM block for CHV */
+#define CGM_GAMMA_EN				4
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 053ceb0..a7aaadf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1453,5 +1453,7 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 /* intel_color_manager.c */
 void intel_color_manager_attach(struct drm_device *dev,
 		struct drm_mode_object *mode_obj);
+int intel_color_manager_set_gamma(struct drm_device *dev,
+		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 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Matt Roper]

On Fri, Jul 03, 2015 at 09:01:44AM +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            |  10 ++
>  drivers/gpu/drm/i915/intel_atomic.c        |   6 ++
>  drivers/gpu/drm/i915/intel_color_manager.c | 154 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h |  12 +++
>  drivers/gpu/drm/i915/intel_drv.h           |   2 +
>  5 files changed, 184 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 313b1f9..36672e7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7900,4 +7900,14 @@ 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 PIPEA_CGM_GAMMA_MIN			(VLV_DISPLAY_BASE + 0x67000)
> +#define CGM_OFFSET				0x2000
> +#define GAMMA_OFFSET				0x2000
> +#define _PIPE_CGM_CONTROL(pipe) \
> +	(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
> +#define _PIPE_GAMMA_BASE(pipe) \
> +	(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index d2674a6..21f0ac2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -473,6 +473,12 @@ 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_gamma(dev, &crtc->base, val);
> +

I think we discussed this on a previous iteration of the patch series,
but .atomic_set_property() isn't supposed to actually update the
hardware at all itself (as you're doing here); it's only supposed to
update the 'state' parameter that was passed here.  Further down the
atomic pipeline the driver will decide whether it really wants to
program any of the state or not and then the CRTC's atomic commit
function should program the registers according to whatever value is set
in the state object.

>  	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 71b4c05..84cc3e47 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,160 @@
>  
>  #include "intel_color_manager.h"
>  
> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
> +		  struct drm_crtc *crtc)
> +{
> +	struct drm_palette *gamma_data;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_property_blob *blob;
> +	struct drm_mode_config *config = &dev->mode_config;
> +	u32 cgm_control_reg = 0;
> +	u32 cgm_gamma_reg = 0;
> +	u32 reg, val;
> +	enum pipe pipe;
> +	u16 red, green, blue;
> +	u32 count = 0;
> +	struct drm_r32g32b32 *correction_values = NULL;
> +	u32 num_samples;
> +	u32 word;
> +	u32 palette;
> +	int ret = 0, length;
> +
> +	blob = drm_property_lookup_blob(dev, blob_id);
> +	if (!blob) {
> +		DRM_ERROR("Invalid Blob ID\n");
> +		return -EINVAL;
> +	}
> +
> +	gamma_data = (struct drm_palette *)blob->data;

Do we need to validate that the blob we receive is of the expected size
or does something in the DRM core's blob handling take care of that for
us?  We don't want userspace to be able to trigger a panic by passing a
smaller than expected blob here.


> +
> +	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->palette_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_8BIT_GAMMA_MAX_VALS) {
> +
> +		/* Disable CGM Gamma, if already set */
> +		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> +		cgm_control_reg &= ~CGM_GAMMA_EN;
> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> +		/* Write LUT values to registers */
> +		palette = _PIPE_GAMMA_BASE(pipe);
> +		count = 0;
> +		correction_values =
> +			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
> +		while (count < num_samples) {
> +			blue = correction_values[count].b32;
> +			green = correction_values[count].g32;
> +			red = correction_values[count].r32;
> +
> +			blue = blue >> CHV_8BIT_GAMMA_MSB_SHIFT;
> +			green = green >> CHV_8BIT_GAMMA_MSB_SHIFT;
> +			red = red >> CHV_8BIT_GAMMA_MSB_SHIFT;
> +
> +			/* Red (23:16), Green (15:8), Blue (7:0) */
> +			word = (red << CHV_8BIT_GAMMA_SHIFT_RED_REG) |
> +				(green <<
> +				 CHV_8BIT_GAMMA_SHIFT_GREEN_REG) |
> +				blue;
> +			I915_WRITE(palette, word);
> +
> +			palette += 4;
> +			count++;
> +		}
> +		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
> +				pipe_name(pipe));
> +
> +		/* Enable Gamma on Pipe */
> +		reg = PIPECONF(pipe);
> +		val = I915_READ(reg) | PIPECONF_GAMMA;
> +		I915_WRITE(reg, val);
> +		DRM_DEBUG_DRIVER("Legacy Gamma enabled on Pipe %c\n",
> +				pipe_name(pipe));
> +
> +		ret = 0;
> +	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
> +		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
> +
> +		count = 0;
> +		correction_values =
> +			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
> +		while (count < num_samples) {
> +			blue = correction_values[count].b32;
> +			green = correction_values[count].g32;
> +			red = correction_values[count].r32;
> +
> +			blue = blue >> CHV_10BIT_GAMMA_MSB_SHIFT;
> +			green = green >> CHV_10BIT_GAMMA_MSB_SHIFT;
> +			red = red >> CHV_10BIT_GAMMA_MSB_SHIFT;
> +
> +			/* Green (25:16) and Blue (9:0) to be written */
> +			word = (green << CHV_GAMMA_SHIFT_GREEN) | blue;
> +			I915_WRITE(cgm_gamma_reg, word);
> +			cgm_gamma_reg += 4;
> +
> +			/* Red (9:0) to be written */
> +			word = red;
> +			I915_WRITE(cgm_gamma_reg, word);
> +
> +			cgm_gamma_reg += 4;
> +			count++;
> +		}
> +		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;
> +}
> +
> +int intel_color_manager_set_gamma(struct drm_device *dev,
> +		struct drm_mode_object *obj, uint32_t blob_id)
> +{
> +	struct drm_crtc *crtc = obj_to_crtc(obj);
> +
> +	if (IS_CHERRYVIEW(dev))
> +		return chv_set_gamma(dev, blob_id, crtc);
> +
> +	return -EINVAL;

As noted earlier, it seems confusing to me to have userspace see a CRTC
property for functionality that isn't supported at all on the platform,
but if we're going to reject invalid platforms here, we should have at
least a DRM_DEBUG message to give some clues as to why the -EINVAL was
returned.


> +}
> +
>  int get_chv_pipe_capabilities(struct drm_device *dev,
>  		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> index 32262ac..d83567a 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -31,6 +31,8 @@
>  #define CHV_PALETTE_STRUCT_VERSION		1
>  #define CHV_CTM_STRUCT_VERSION			1
>  #define CHV_PLATFORM_STRUCT_VERSION		1
> +#define CHV_GAMMA_DATA_STRUCT_VERSION		1
> +
>  #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
>  #define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
>  #define CHV_DEGAMMA_PRECISION			14
> @@ -43,6 +45,16 @@
>  #define CHV_10BIT_GAMMA_MAX_VALS		257
>  #define CHV_8BIT_GAMMA_PRECISION		8
>  #define CHV_8BIT_GAMMA_MAX_VALS			256
> +#define CHV_8BIT_GAMMA_MSB_SHIFT		8
> +#define CHV_8BIT_GAMMA_SHIFT_RED_REG		16
> +#define CHV_8BIT_GAMMA_SHIFT_GREEN_REG		8
> +#define CHV_10BIT_GAMMA_MSB_SHIFT		6
> +#define CHV_GAMMA_SHIFT_GREEN			16
> +
>  #define CHV_CSC_COEFF_MAX_PRECISION		12
>  #define CHV_CSC_COEFF_MAX_INT			7
>  #define CHV_CSC_COEFF_MIN_INT			-7
> +
> +/* CHV CGM Block */
> +/* Bit 2 to be enabled in CGM block for CHV */
> +#define CGM_GAMMA_EN				4
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 053ceb0..a7aaadf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1453,5 +1453,7 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>  /* intel_color_manager.c */
>  void intel_color_manager_attach(struct drm_device *dev,
>  		struct drm_mode_object *mode_obj);
> +int intel_color_manager_set_gamma(struct drm_device *dev,
> +		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 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Malladi, Kausal]

On Wednesday 08 July 2015 04:53 AM, Matt Roper wrote:
> On Fri, Jul 03, 2015 at 09:01:44AM +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            |  10 ++
>>   drivers/gpu/drm/i915/intel_atomic.c        |   6 ++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 154 +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color_manager.h |  12 +++
>>   drivers/gpu/drm/i915/intel_drv.h           |   2 +
>>   5 files changed, 184 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 313b1f9..36672e7 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7900,4 +7900,14 @@ 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 PIPEA_CGM_GAMMA_MIN			(VLV_DISPLAY_BASE + 0x67000)
>> +#define CGM_OFFSET				0x2000
>> +#define GAMMA_OFFSET				0x2000
>> +#define _PIPE_CGM_CONTROL(pipe) \
>> +	(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
>> +#define _PIPE_GAMMA_BASE(pipe) \
>> +	(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
>> +
>>   #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index d2674a6..21f0ac2 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -473,6 +473,12 @@ 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_gamma(dev, &crtc->base, val);
>> +
> I think we discussed this on a previous iteration of the patch series,
> but .atomic_set_property() isn't supposed to actually update the
> hardware at all itself (as you're doing here); it's only supposed to
> update the 'state' parameter that was passed here.  Further down the
> atomic pipeline the driver will decide whether it really wants to
> program any of the state or not and then the CRTC's atomic commit
> function should program the registers according to whatever value is set
> in the state object.
Thanks Matt for your suggestions offline. We will implement it this way 
in our next set of patches.
>
>>   	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 71b4c05..84cc3e47 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,160 @@
>>   
>>   #include "intel_color_manager.h"
>>   
>> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
>> +		  struct drm_crtc *crtc)
>> +{
>> +	struct drm_palette *gamma_data;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_property_blob *blob;
>> +	struct drm_mode_config *config = &dev->mode_config;
>> +	u32 cgm_control_reg = 0;
>> +	u32 cgm_gamma_reg = 0;
>> +	u32 reg, val;
>> +	enum pipe pipe;
>> +	u16 red, green, blue;
>> +	u32 count = 0;
>> +	struct drm_r32g32b32 *correction_values = NULL;
>> +	u32 num_samples;
>> +	u32 word;
>> +	u32 palette;
>> +	int ret = 0, length;
>> +
>> +	blob = drm_property_lookup_blob(dev, blob_id);
>> +	if (!blob) {
>> +		DRM_ERROR("Invalid Blob ID\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	gamma_data = (struct drm_palette *)blob->data;
> Do we need to validate that the blob we receive is of the expected size
> or does something in the DRM core's blob handling take care of that for
> us?  We don't want userspace to be able to trigger a panic by passing a
> smaller than expected blob here.
Yes, it was an oversight.
>
>
>> +
>> +	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->palette_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_8BIT_GAMMA_MAX_VALS) {
>> +
>> +		/* Disable CGM Gamma, if already set */
>> +		cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
>> +		cgm_control_reg &= ~CGM_GAMMA_EN;
>> +		I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
>> +
>> +		/* Write LUT values to registers */
>> +		palette = _PIPE_GAMMA_BASE(pipe);
>> +		count = 0;
>> +		correction_values =
>> +			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
>> +		while (count < num_samples) {
>> +			blue = correction_values[count].b32;
>> +			green = correction_values[count].g32;
>> +			red = correction_values[count].r32;
>> +
>> +			blue = blue >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +			green = green >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +			red = red >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +
>> +			/* Red (23:16), Green (15:8), Blue (7:0) */
>> +			word = (red << CHV_8BIT_GAMMA_SHIFT_RED_REG) |
>> +				(green <<
>> +				 CHV_8BIT_GAMMA_SHIFT_GREEN_REG) |
>> +				blue;
>> +			I915_WRITE(palette, word);
>> +
>> +			palette += 4;
>> +			count++;
>> +		}
>> +		DRM_DEBUG_DRIVER("Gamma LUT loaded successfully for Pipe %c\n",
>> +				pipe_name(pipe));
>> +
>> +		/* Enable Gamma on Pipe */
>> +		reg = PIPECONF(pipe);
>> +		val = I915_READ(reg) | PIPECONF_GAMMA;
>> +		I915_WRITE(reg, val);
>> +		DRM_DEBUG_DRIVER("Legacy Gamma enabled on Pipe %c\n",
>> +				pipe_name(pipe));
>> +
>> +		ret = 0;
>> +	} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
>> +		cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
>> +
>> +		count = 0;
>> +		correction_values =
>> +			(struct drm_r32g32b32 *)&gamma_data->palette_lut;
>> +		while (count < num_samples) {
>> +			blue = correction_values[count].b32;
>> +			green = correction_values[count].g32;
>> +			red = correction_values[count].r32;
>> +
>> +			blue = blue >> CHV_10BIT_GAMMA_MSB_SHIFT;
>> +			green = green >> CHV_10BIT_GAMMA_MSB_SHIFT;
>> +			red = red >> CHV_10BIT_GAMMA_MSB_SHIFT;
>> +
>> +			/* Green (25:16) and Blue (9:0) to be written */
>> +			word = (green << CHV_GAMMA_SHIFT_GREEN) | blue;
>> +			I915_WRITE(cgm_gamma_reg, word);
>> +			cgm_gamma_reg += 4;
>> +
>> +			/* Red (9:0) to be written */
>> +			word = red;
>> +			I915_WRITE(cgm_gamma_reg, word);
>> +
>> +			cgm_gamma_reg += 4;
>> +			count++;
>> +		}
>> +		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;
>> +}
>> +
>> +int intel_color_manager_set_gamma(struct drm_device *dev,
>> +		struct drm_mode_object *obj, uint32_t blob_id)
>> +{
>> +	struct drm_crtc *crtc = obj_to_crtc(obj);
>> +
>> +	if (IS_CHERRYVIEW(dev))
>> +		return chv_set_gamma(dev, blob_id, crtc);
>> +
>> +	return -EINVAL;
> As noted earlier, it seems confusing to me to have userspace see a CRTC
> property for functionality that isn't supported at all on the platform,
> but if we're going to reject invalid platforms here, we should have at
> least a DRM_DEBUG message to give some clues as to why the -EINVAL was
> returned.
As suggested by you, if we put check while attaching, this is not 
required. So will have it there and remove here.
>
>
>> +}
>> +
>>   int get_chv_pipe_capabilities(struct drm_device *dev,
>>   		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
>>   {
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
>> index 32262ac..d83567a 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.h
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
>> @@ -31,6 +31,8 @@
>>   #define CHV_PALETTE_STRUCT_VERSION		1
>>   #define CHV_CTM_STRUCT_VERSION			1
>>   #define CHV_PLATFORM_STRUCT_VERSION		1
>> +#define CHV_GAMMA_DATA_STRUCT_VERSION		1
>> +
>>   #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
>>   #define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
>>   #define CHV_DEGAMMA_PRECISION			14
>> @@ -43,6 +45,16 @@
>>   #define CHV_10BIT_GAMMA_MAX_VALS		257
>>   #define CHV_8BIT_GAMMA_PRECISION		8
>>   #define CHV_8BIT_GAMMA_MAX_VALS			256
>> +#define CHV_8BIT_GAMMA_MSB_SHIFT		8
>> +#define CHV_8BIT_GAMMA_SHIFT_RED_REG		16
>> +#define CHV_8BIT_GAMMA_SHIFT_GREEN_REG		8
>> +#define CHV_10BIT_GAMMA_MSB_SHIFT		6
>> +#define CHV_GAMMA_SHIFT_GREEN			16
>> +
>>   #define CHV_CSC_COEFF_MAX_PRECISION		12
>>   #define CHV_CSC_COEFF_MAX_INT			7
>>   #define CHV_CSC_COEFF_MIN_INT			-7
>> +
>> +/* CHV CGM Block */
>> +/* Bit 2 to be enabled in CGM block for CHV */
>> +#define CGM_GAMMA_EN				4
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 053ceb0..a7aaadf 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1453,5 +1453,7 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>>   /* intel_color_manager.c */
>>   void intel_color_manager_attach(struct drm_device *dev,
>>   		struct drm_mode_object *mode_obj);
>> +int intel_color_manager_set_gamma(struct drm_device *dev,
>> +		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 10/12] 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            |   4 ++
 drivers/gpu/drm/i915/intel_atomic.c        |   2 +
 drivers/gpu/drm/i915/intel_color_manager.c | 110 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |   5 ++
 drivers/gpu/drm/i915/intel_drv.h           |   2 +
 5 files changed, 123 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36672e7..58a1414 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7903,11 +7903,15 @@ enum skl_disp_power_wells {
 /* Color Management */
 #define PIPEA_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x67A00)
 #define PIPEA_CGM_GAMMA_MIN			(VLV_DISPLAY_BASE + 0x67000)
+#define PIPEA_CGM_DEGAMMA_MIN			(VLV_DISPLAY_BASE + 0x66000)
 #define CGM_OFFSET				0x2000
 #define GAMMA_OFFSET				0x2000
+#define DEGAMMA_OFFSET				0x2000
 #define _PIPE_CGM_CONTROL(pipe) \
 	(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
 #define _PIPE_GAMMA_BASE(pipe) \
 	(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
+#define _PIPE_DEGAMMA_BASE(pipe) \
+	(PIPEA_CGM_DEGAMMA_MIN + (pipe * DEGAMMA_OFFSET))
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 21f0ac2..570af9d 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -478,6 +478,8 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
 
 	if (property == config->prop_palette_after_ctm)
 		return intel_color_manager_set_gamma(dev, &crtc->base, val);
+	if (property == config->prop_palette_before_ctm)
+		return intel_color_manager_set_degamma(dev, &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 84cc3e47..21c499f 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,105 @@
 
 #include "intel_color_manager.h"
 
+int chv_set_degamma(struct drm_device *dev, uint32_t blob_id,
+		  struct drm_crtc *crtc)
+{
+	struct drm_palette *degamma_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property_blob *blob;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 cgm_control_reg = 0;
+	u32 cgm_degamma_reg = 0;
+	enum pipe pipe;
+	u16 red, green, blue;
+	u32 count = 0;
+	struct drm_r32g32b32 *correction_values = NULL;
+	u32 num_samples;
+	u32 word;
+	int ret = 0, length;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\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->palette_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->palette_lut;
+		while (count < CHV_DEGAMMA_MAX_VALS) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue = blue >> CHV_DEGAMMA_MSB_SHIFT;
+			green = green >> CHV_DEGAMMA_MSB_SHIFT;
+			red = red >> CHV_DEGAMMA_MSB_SHIFT;
+
+			/* Green (29:16) and Blue (13:0) in DWORD1 */
+			word = (green << CHV_DEGAMMA_GREEN_SHIFT) | blue;
+			I915_WRITE(cgm_degamma_reg, word);
+
+			cgm_degamma_reg += 4;
+
+			/* Red (13:0) to be written to DWORD2 */
+			word = red;
+			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, uint32_t blob_id,
 		  struct drm_crtc *crtc)
 {
@@ -181,6 +280,17 @@ int intel_color_manager_set_gamma(struct drm_device *dev,
 	return -EINVAL;
 }
 
+int intel_color_manager_set_degamma(struct drm_device *dev,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_crtc *crtc = obj_to_crtc(obj);
+
+	if (IS_CHERRYVIEW(dev))
+		return chv_set_degamma(dev, blob_id, crtc);
+
+	return -EINVAL;
+}
+
 int get_chv_pipe_capabilities(struct drm_device *dev,
 		struct drm_color_caps *color_caps, struct drm_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index d83567a..64468c1 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -32,6 +32,7 @@
 #define CHV_CTM_STRUCT_VERSION			1
 #define CHV_PLATFORM_STRUCT_VERSION		1
 #define CHV_GAMMA_DATA_STRUCT_VERSION		1
+#define CHV_DEGAMMA_DATA_STRUCT_VERSION		1
 
 #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
 #define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
@@ -51,6 +52,9 @@
 #define CHV_10BIT_GAMMA_MSB_SHIFT		6
 #define CHV_GAMMA_SHIFT_GREEN			16
 
+#define CHV_DEGAMMA_MSB_SHIFT			2
+#define CHV_DEGAMMA_GREEN_SHIFT			16
+
 #define CHV_CSC_COEFF_MAX_PRECISION		12
 #define CHV_CSC_COEFF_MAX_INT			7
 #define CHV_CSC_COEFF_MIN_INT			-7
@@ -58,3 +62,4 @@
 /* CHV CGM Block */
 /* Bit 2 to be enabled in CGM block for CHV */
 #define CGM_GAMMA_EN				4
+#define CGM_DEGAMMA_EN				1
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a7aaadf..f47d9d6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1455,5 +1455,7 @@ void intel_color_manager_attach(struct drm_device *dev,
 		struct drm_mode_object *mode_obj);
 int intel_color_manager_set_gamma(struct drm_device *dev,
 		struct drm_mode_object *obj, uint32_t blob_id);
+int intel_color_manager_set_degamma(struct drm_device *dev,
+		struct drm_mode_object *obj, uint32_t blob_id);
 
 #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 11/12] 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 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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 04a8f2a..974a147 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -875,6 +875,11 @@ struct drm_palette {
 	struct drm_r32g32b32 palette_lut[0];
 };
 
+struct drm_ctm {
+	__u32 version;
+	__s32 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 12/12] 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            |   4 ++
 drivers/gpu/drm/i915/intel_atomic.c        |   2 +
 drivers/gpu/drm/i915/intel_color_manager.c | 110 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h |   9 +++
 drivers/gpu/drm/i915/intel_drv.h           |   2 +
 5 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 58a1414..7476132 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7904,14 +7904,18 @@ enum skl_disp_power_wells {
 #define PIPEA_CGM_CONTROL			(VLV_DISPLAY_BASE + 0x67A00)
 #define PIPEA_CGM_GAMMA_MIN			(VLV_DISPLAY_BASE + 0x67000)
 #define PIPEA_CGM_DEGAMMA_MIN			(VLV_DISPLAY_BASE + 0x66000)
+#define PIPEA_CGM_CSC_MIN			(VLV_DISPLAY_BASE + 0x67900)
 #define CGM_OFFSET				0x2000
 #define GAMMA_OFFSET				0x2000
 #define DEGAMMA_OFFSET				0x2000
+#define CGM_CSC_OFFSET				0x2000
 #define _PIPE_CGM_CONTROL(pipe) \
 	(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
 #define _PIPE_GAMMA_BASE(pipe) \
 	(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
 #define _PIPE_DEGAMMA_BASE(pipe) \
 	(PIPEA_CGM_DEGAMMA_MIN + (pipe * DEGAMMA_OFFSET))
+#define _PIPE_CSC_BASE(pipe) \
+	(PIPEA_CGM_CSC_MIN + (pipe * CGM_CSC_OFFSET))
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 570af9d..8a90d59 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -480,6 +480,8 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
 		return intel_color_manager_set_gamma(dev, &crtc->base, val);
 	if (property == config->prop_palette_before_ctm)
 		return intel_color_manager_set_degamma(dev, &crtc->base, val);
+	if (property == config->prop_ctm)
+		return intel_color_manager_set_csc(dev, &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 21c499f..25045b3 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,105 @@
 
 #include "intel_color_manager.h"
 
+s16 get_csc_s3_12_format(s32 csc_value)
+{
+	s16 csc_int_value;
+	u16 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, uint64_t blob_id,
+		struct drm_crtc *crtc)
+{
+	struct drm_ctm *csc_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property_blob *blob;
+	struct drm_mode_config *config = &dev->mode_config;
+	u32 reg;
+	enum pipe pipe;
+	s32 word, temp;
+	int ret, count = 0;
+
+	blob = drm_property_lookup_blob(dev, blob_id);
+	if (!blob) {
+		DRM_ERROR("Invalid Blob ID\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, uint32_t blob_id,
 		  struct drm_crtc *crtc)
 {
@@ -269,6 +368,17 @@ int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
 	return ret;
 }
 
+int intel_color_manager_set_csc(struct drm_device *dev,
+		struct drm_mode_object *obj, uint32_t blob_id)
+{
+	struct drm_crtc *crtc = obj_to_crtc(obj);
+
+	if (IS_CHERRYVIEW(dev))
+		return chv_set_csc(dev, blob_id, crtc);
+
+	return -EINVAL;
+}
+
 int intel_color_manager_set_gamma(struct drm_device *dev,
 		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 64468c1..a596175 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -33,6 +33,7 @@
 #define CHV_PLATFORM_STRUCT_VERSION		1
 #define CHV_GAMMA_DATA_STRUCT_VERSION		1
 #define CHV_DEGAMMA_DATA_STRUCT_VERSION		1
+#define CHV_CSC_DATA_STRUCT_VERSION		1
 
 #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM		1
 #define CHV_MAX_PALETTE_CAPS_AFTER_CTM		2
@@ -55,11 +56,19 @@
 #define CHV_DEGAMMA_MSB_SHIFT			2
 #define CHV_DEGAMMA_GREEN_SHIFT			16
 
+#define CSC_MAX_VALS				9
 #define CHV_CSC_COEFF_MAX_PRECISION		12
 #define CHV_CSC_COEFF_MAX_INT			7
 #define CHV_CSC_COEFF_MIN_INT			-7
+#define CHV_CSC_COEFF_MAX			((7 << 16) + 0xFFFF)
+#define CHV_CSC_FRACT_ROUNDOFF			(1 << 3)
+#define CHV_CSC_COEFF_SHIFT			16
+#define CHV_CSC_COEFF_FRACT_SHIFT		4
+#define CHV_CSC_COEFF_INT_SHIFT			12
+#define CSC_COEFF_SIGN				(1 << 15)
 
 /* CHV CGM Block */
 /* Bit 2 to be enabled in CGM block for CHV */
 #define CGM_GAMMA_EN				4
 #define CGM_DEGAMMA_EN				1
+#define CGM_CSC_EN				2
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f47d9d6..dc30d56 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1457,5 +1457,7 @@ int intel_color_manager_set_gamma(struct drm_device *dev,
 		struct drm_mode_object *obj, uint32_t blob_id);
 int intel_color_manager_set_degamma(struct drm_device *dev,
 		struct drm_mode_object *obj, uint32_t blob_id);
+int intel_color_manager_set_csc(struct drm_device *dev,
+		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 07/12] drm: Add structures to set/get a palette color property

[Damien Lespiau]

On Wed, Jul 01, 2015 at 09:18:17PM +0530, Kausal Malladi wrote:
> From: Kausal Malladi <Kausal.Malladi@intel.com>
> 
> 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 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index d9562a2..04a8f2a 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -863,6 +863,18 @@ struct drm_color_caps {
>  	struct drm_cge_caps cge_caps;
>  };
>  
> +struct drm_r32g32b32 {
> +	__u32 r32;
> +	__u32 g32;
> +	__u32 b32;
> +};

r32 means red is an u32 to me. Is that the right encoding? I thought we
need values > 1.0 for our hw?

> +
> +struct drm_palette {
> +	__u32 version;
> +	__u32 palette_num_samples;

No need to prefix the fields with the structure name.

> +	struct drm_r32g32b32 palette_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