diff mbox

[15/22] drm/i915: CHV: Pipe level CSC correction

Message ID 1444418952-5671-16-git-send-email-shashank.sharma@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sharma, Shashank Oct. 9, 2015, 7:29 p.m. UTC
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. Attaches CSC property to CRTC
2. Adds the core function to program CSC correction values
3. Adds CSC correction macros

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |  8 +++
 drivers/gpu/drm/i915/intel_color_manager.c | 94 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color_manager.h | 19 ++++++
 3 files changed, 121 insertions(+)

Comments

Emil Velikov Oct. 9, 2015, 11:43 p.m. UTC | #1
On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma@intel.com> wrote:
> 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. Attaches CSC property to CRTC
> 2. Adds the core function to program CSC correction values
> 3. Adds CSC correction macros
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h            |  8 +++
>  drivers/gpu/drm/i915/intel_color_manager.c | 94 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h | 19 ++++++
>  3 files changed, 121 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c32e35d..5825ab2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8056,4 +8056,12 @@ enum skl_disp_power_wells {
>  #define _PIPE_DEGAMMA_BASE(pipe) \
>         (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
>
> +#define PIPEA_CGM_CSC                          (VLV_DISPLAY_BASE + 0x67900)
> +#define PIPEB_CGM_CSC                          (VLV_DISPLAY_BASE + 0x69900)
> +#define PIPEC_CGM_CSC                          (VLV_DISPLAY_BASE + 0x6B900)
> +#define _PIPE_CSC_BASE(pipe) \
> +       (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
> +
> +
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> index bbfe185..433e50a 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,93 @@
>
>  #include "intel_color_manager.h"
>
> +static s16 chv_prepare_csc_coeff(s64 csc_value)
> +{
> +       s32 csc_int_value;
> +       u32 csc_fract_value;
> +       s16 csc_s3_12_format;
The type of csc_s3_12_format and chv_prepare_csc_coeff() does not see
correct. Seem like the fix got merged into another patch :\

[snip]
> +static int chv_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
> +               struct drm_crtc *crtc)
> +{
> +       struct drm_ctm *csc_data;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       u32 reg;
> +       enum pipe pipe;
> +       s32 word = 0, temp;
> +       int count = 0;
> +
> +       if (WARN_ON(!blob))
> +               return -EINVAL;
> +
> +       if (blob->length != sizeof(struct drm_ctm)) {
> +               DRM_ERROR("Invalid length of data received\n");
> +               return -EINVAL;
> +       }
> +
> +       csc_data = (struct drm_ctm *)blob->data;
> +       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) {
> +               temp = chv_prepare_csc_coeff(
> +                                       csc_data->ctm_coeff[count]);
> +               SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
> +
> +               /*
> +                * 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 = chv_prepare_csc_coeff(
> +                                       csc_data->ctm_coeff[count]);
> +                       SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
> +               }
This looks a bit odd. Use the same approach as in
bdw_write_12bit_gamma_precision() ?

Regards,
Emil
Sharma, Shashank Oct. 10, 2015, 5:26 a.m. UTC | #2
Regards
Shashank

On 10/10/2015 5:13 AM, Emil Velikov wrote:
> On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma@intel.com> wrote:
>> 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. Attaches CSC property to CRTC
>> 2. Adds the core function to program CSC correction values
>> 3. Adds CSC correction macros
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
>> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h            |  8 +++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 94 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color_manager.h | 19 ++++++
>>   3 files changed, 121 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index c32e35d..5825ab2 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8056,4 +8056,12 @@ enum skl_disp_power_wells {
>>   #define _PIPE_DEGAMMA_BASE(pipe) \
>>          (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
>>
>> +#define PIPEA_CGM_CSC                          (VLV_DISPLAY_BASE + 0x67900)
>> +#define PIPEB_CGM_CSC                          (VLV_DISPLAY_BASE + 0x69900)
>> +#define PIPEC_CGM_CSC                          (VLV_DISPLAY_BASE + 0x6B900)
>> +#define _PIPE_CSC_BASE(pipe) \
>> +       (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
>> +
>> +
>> +
>>   #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
>> index bbfe185..433e50a 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,93 @@
>>
>>   #include "intel_color_manager.h"
>>
>> +static s16 chv_prepare_csc_coeff(s64 csc_value)
>> +{
>> +       s32 csc_int_value;
>> +       u32 csc_fract_value;
>> +       s16 csc_s3_12_format;
> The type of csc_s3_12_format and chv_prepare_csc_coeff() does not see
> correct. Seem like the fix got merged into another patch :\
>
Can you please elaborate this comment, I dont get it.
> [snip]
>> +static int chv_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
>> +               struct drm_crtc *crtc)
>> +{
>> +       struct drm_ctm *csc_data;
>> +       struct drm_i915_private *dev_priv = dev->dev_private;
>> +       u32 reg;
>> +       enum pipe pipe;
>> +       s32 word = 0, temp;
>> +       int count = 0;
>> +
>> +       if (WARN_ON(!blob))
>> +               return -EINVAL;
>> +
>> +       if (blob->length != sizeof(struct drm_ctm)) {
>> +               DRM_ERROR("Invalid length of data received\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       csc_data = (struct drm_ctm *)blob->data;
>> +       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) {
>> +               temp = chv_prepare_csc_coeff(
>> +                                       csc_data->ctm_coeff[count]);
>> +               SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
>> +
>> +               /*
>> +                * 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 = chv_prepare_csc_coeff(
>> +                                       csc_data->ctm_coeff[count]);
>> +                       SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
>> +               }
> This looks a bit odd. Use the same approach as in
> bdw_write_12bit_gamma_precision() ?
Again, can you please give little more details here ?
>
> Regards,
> Emil
>
Emil Velikov Oct. 13, 2015, 1:33 p.m. UTC | #3
On 10 October 2015 at 06:26, Sharma, Shashank <shashank.sharma@intel.com> wrote:
> On 10/10/2015 5:13 AM, Emil Velikov wrote:
>>
>> On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma@intel.com>
>> wrote:
>>>
>>> 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. Attaches CSC property to CRTC
>>> 2. Adds the core function to program CSC correction values
>>> 3. Adds CSC correction macros
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
>>> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_reg.h            |  8 +++
>>>   drivers/gpu/drm/i915/intel_color_manager.c | 94
>>> ++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_color_manager.h | 19 ++++++
>>>   3 files changed, 121 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index c32e35d..5825ab2 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -8056,4 +8056,12 @@ enum skl_disp_power_wells {
>>>   #define _PIPE_DEGAMMA_BASE(pipe) \
>>>          (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA,
>>> PIPEC_CGM_DEGAMMA))
>>>
>>> +#define PIPEA_CGM_CSC                          (VLV_DISPLAY_BASE +
>>> 0x67900)
>>> +#define PIPEB_CGM_CSC                          (VLV_DISPLAY_BASE +
>>> 0x69900)
>>> +#define PIPEC_CGM_CSC                          (VLV_DISPLAY_BASE +
>>> 0x6B900)
>>> +#define _PIPE_CSC_BASE(pipe) \
>>> +       (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
>>> +
>>> +
>>> +
>>>   #endif /* _I915_REG_H_ */
>>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>>> b/drivers/gpu/drm/i915/intel_color_manager.c
>>> index bbfe185..433e50a 100644
>>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>>> @@ -27,6 +27,93 @@
>>>
>>>   #include "intel_color_manager.h"
>>>
>>> +static s16 chv_prepare_csc_coeff(s64 csc_value)
>>> +{
>>> +       s32 csc_int_value;
>>> +       u32 csc_fract_value;
>>> +       s16 csc_s3_12_format;
>>
>> The type of csc_s3_12_format and chv_prepare_csc_coeff() does not see
>> correct. Seem like the fix got merged into another patch :\
>>
> Can you please elaborate this comment, I dont get it.
>
You have two typos above s16 > s32 which you've fixed in the next
patch. That fix should belong here imho.

[snip]
>>> +       while (count < CSC_MAX_VALS) {
>>> +               temp = chv_prepare_csc_coeff(
>>> +                                       csc_data->ctm_coeff[count]);
>>> +               SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
>>> +
>>> +               /*
>>> +                * 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 = chv_prepare_csc_coeff(
>>> +                                       csc_data->ctm_coeff[count]);
>>> +                       SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
>>> +               }
>>
>> This looks a bit odd. Use the same approach as in
>> bdw_write_12bit_gamma_precision() ?
>
> Again, can you please give little more details here ?
Take a look at the loop construct in bdw_write_12bit_gamma_precision()
- both of them are essentially doing the same thing.

Here you have
while(i < odd_number) {
  foo()
  if (if != odd_number-1) {
    I++
    foo()
  }
}

while in the mentioned function

while (i < odd_number -1) {
  foo()
  foo()
  i++
}
foo()

Normally you'd use one or the other. Esp. since this is a single
patchset :-) I'm leaning towards the latter as it's more obvious but
others may prefer the former approach.

Regards,
Emil
Sharma, Shashank Oct. 13, 2015, 1:49 p.m. UTC | #4
Regards
Shashank

On 10/13/2015 7:03 PM, Emil Velikov wrote:
> On 10 October 2015 at 06:26, Sharma, Shashank <shashank.sharma@intel.com> wrote:
>> On 10/10/2015 5:13 AM, Emil Velikov wrote:
>>>
>>> On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma@intel.com>
>>> wrote:
>>>>
>>>> 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. Attaches CSC property to CRTC
>>>> 2. Adds the core function to program CSC correction values
>>>> 3. Adds CSC correction macros
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
>>>> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_reg.h            |  8 +++
>>>>    drivers/gpu/drm/i915/intel_color_manager.c | 94
>>>> ++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/i915/intel_color_manager.h | 19 ++++++
>>>>    3 files changed, 121 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>> index c32e35d..5825ab2 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -8056,4 +8056,12 @@ enum skl_disp_power_wells {
>>>>    #define _PIPE_DEGAMMA_BASE(pipe) \
>>>>           (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA,
>>>> PIPEC_CGM_DEGAMMA))
>>>>
>>>> +#define PIPEA_CGM_CSC                          (VLV_DISPLAY_BASE +
>>>> 0x67900)
>>>> +#define PIPEB_CGM_CSC                          (VLV_DISPLAY_BASE +
>>>> 0x69900)
>>>> +#define PIPEC_CGM_CSC                          (VLV_DISPLAY_BASE +
>>>> 0x6B900)
>>>> +#define _PIPE_CSC_BASE(pipe) \
>>>> +       (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
>>>> +
>>>> +
>>>> +
>>>>    #endif /* _I915_REG_H_ */
>>>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>>>> b/drivers/gpu/drm/i915/intel_color_manager.c
>>>> index bbfe185..433e50a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>>>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>>>> @@ -27,6 +27,93 @@
>>>>
>>>>    #include "intel_color_manager.h"
>>>>
>>>> +static s16 chv_prepare_csc_coeff(s64 csc_value)
>>>> +{
>>>> +       s32 csc_int_value;
>>>> +       u32 csc_fract_value;
>>>> +       s16 csc_s3_12_format;
>>>
>>> The type of csc_s3_12_format and chv_prepare_csc_coeff() does not see
>>> correct. Seem like the fix got merged into another patch :\
>>>
>> Can you please elaborate this comment, I dont get it.
>>
> You have two typos above s16 > s32 which you've fixed in the next
> patch. That fix should belong here imho.
>
Yes, I got that later, current patch set contains this fix.
> [snip]
>>>> +       while (count < CSC_MAX_VALS) {
>>>> +               temp = chv_prepare_csc_coeff(
>>>> +                                       csc_data->ctm_coeff[count]);
>>>> +               SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
>>>> +
>>>> +               /*
>>>> +                * 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 = chv_prepare_csc_coeff(
>>>> +                                       csc_data->ctm_coeff[count]);
>>>> +                       SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
>>>> +               }
>>>
>>> This looks a bit odd. Use the same approach as in
>>> bdw_write_12bit_gamma_precision() ?
>>
>> Again, can you please give little more details here ?
> Take a look at the loop construct in bdw_write_12bit_gamma_precision()
> - both of them are essentially doing the same thing.
>
> Here you have
> while(i < odd_number) {
>    foo()
>    if (if != odd_number-1) {
>      I++
>      foo()
>    }
> }
>
> while in the mentioned function
>
> while (i < odd_number -1) {
>    foo()
>    foo()
>    i++
> }
> foo()
>
> Normally you'd use one or the other. Esp. since this is a single
> patchset :-) I'm leaning towards the latter as it's more obvious but
> others may prefer the former approach.
>
Yes, I got this one also, later :). New patch set has an implementation 
similar to this, please have a look.

> Regards,
> Emil
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c32e35d..5825ab2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8056,4 +8056,12 @@  enum skl_disp_power_wells {
 #define _PIPE_DEGAMMA_BASE(pipe) \
 	(_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
 
+#define PIPEA_CGM_CSC				(VLV_DISPLAY_BASE + 0x67900)
+#define PIPEB_CGM_CSC				(VLV_DISPLAY_BASE + 0x69900)
+#define PIPEC_CGM_CSC				(VLV_DISPLAY_BASE + 0x6B900)
+#define _PIPE_CSC_BASE(pipe) \
+	(_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
+
+
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index bbfe185..433e50a 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,93 @@ 
 
 #include "intel_color_manager.h"
 
+static s16 chv_prepare_csc_coeff(s64 csc_value)
+{
+	s32 csc_int_value;
+	u32 csc_fract_value;
+	s16 csc_s3_12_format;
+
+	if (csc_value >= 0) {
+		csc_value += CHV_CSC_FRACT_ROUNDOFF;
+		if (csc_value > CHV_CSC_COEFF_MAX)
+			csc_value = CHV_CSC_COEFF_MAX;
+	} else {
+		csc_value = -csc_value;
+		csc_value += CHV_CSC_FRACT_ROUNDOFF;
+		if (csc_value > CHV_CSC_COEFF_MAX + 1)
+			csc_value = CHV_CSC_COEFF_MAX + 1;
+		csc_value = -csc_value;
+	}
+
+	csc_int_value = csc_value >> CHV_CSC_COEFF_SHIFT;
+	csc_int_value <<= CHV_CSC_COEFF_INT_SHIFT;
+	if (csc_value < 0)
+		csc_int_value |= CSC_COEFF_SIGN;
+
+	csc_fract_value = csc_value;
+	csc_fract_value >>= CHV_CSC_COEFF_FRACT_SHIFT;
+	csc_s3_12_format = csc_int_value | csc_fract_value;
+
+	return csc_s3_12_format;
+}
+
+static int chv_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
+		struct drm_crtc *crtc)
+{
+	struct drm_ctm *csc_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 reg;
+	enum pipe pipe;
+	s32 word = 0, temp;
+	int count = 0;
+
+	if (WARN_ON(!blob))
+		return -EINVAL;
+
+	if (blob->length != sizeof(struct drm_ctm)) {
+		DRM_ERROR("Invalid length of data received\n");
+		return -EINVAL;
+	}
+
+	csc_data = (struct drm_ctm *)blob->data;
+	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) {
+		temp = chv_prepare_csc_coeff(
+					csc_data->ctm_coeff[count]);
+		SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
+
+		/*
+		 * 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 = chv_prepare_csc_coeff(
+					csc_data->ctm_coeff[count]);
+			SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
+		}
+		I915_WRITE(reg, word);
+		reg += 4;
+		count++;
+	}
+
+	/* 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));
+	return 0;
+}
+
 static int chv_set_degamma(struct drm_device *dev,
 	struct drm_property_blob *blob, struct drm_crtc *crtc)
 {
@@ -268,4 +355,11 @@  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 			config->cm_palette_before_ctm_property, 0);
 		DRM_DEBUG_DRIVER("degamma property attached to CRTC\n");
 	}
+
+	/* CSC */
+	if (config->cm_ctm_property) {
+		drm_object_attach_property(mode_obj,
+			config->cm_ctm_property, 0);
+		DRM_DEBUG_DRIVER("CSC property attached to CRTC\n");
+	}
 }
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
index 77a2119..7b96512 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -63,10 +63,29 @@ 
 #define CHV_GAMMA_SHIFT_GREEN                  16
 #define CHV_MAX_GAMMA                          ((1 << 24) - 1)
 
+/*
+ * CSC on CHV
+ * Fractional part is 32 bit, and we need only 12 MSBs for programming
+ * into registers. ROUNDOFF is required to minimize loss of precision.
+ */
+#define CHV_CSC_FRACT_ROUNDOFF                 (1 << 19)
+/*
+ * CSC values are 64-bit values. For CHV, the maximum CSC value that
+ * user can program is 7.99999..., which can be represented in fixed point
+ * S31.32 format like this, with all fractional bits as 1
+ */
+#define CHV_CSC_COEFF_MAX                      0x00000007FFFFFFFF
+#define CHV_CSC_COEFF_SHIFT                    32
+#define CHV_CSC_COEFF_INT_SHIFT                28
+#define CSC_COEFF_SIGN                         (1 << 31)
+#define CHV_CSC_COEFF_FRACT_SHIFT              4
+#define CSC_MAX_VALS                           9
+
 /* Degamma on CHV */
 #define CHV_DEGAMMA_MSB_SHIFT                  2
 #define CHV_DEGAMMA_GREEN_SHIFT                16
 
 /* CHV CGM Block */
 #define CGM_GAMMA_EN                           (1 << 2)
+#define CGM_CSC_EN                             (1 << 1)
 #define CGM_DEGAMMA_EN                         (1 << 0)