diff mbox series

[3/3] drm/i915/icl: Enable ICL Pipe CSC block

Message ID 1540327255-25639-4-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add support for Gen 11 pipe color features | expand

Commit Message

Shankar, Uma Oct. 23, 2018, 8:40 p.m. UTC
Enable ICL pipe csc hardware. CSC block is enabled
in CSC_MODE register instead of PLANE_COLOR_CTL.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    | 1 +
 drivers/gpu/drm/i915/intel_color.c | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Maarten Lankhorst Oct. 24, 2018, 1:38 p.m. UTC | #1
Op 23-10-18 om 22:40 schreef Uma Shankar:
> Enable ICL pipe csc hardware. CSC block is enabled
> in CSC_MODE register instead of PLANE_COLOR_CTL.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    | 1 +
>  drivers/gpu/drm/i915/intel_color.c | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dd0514e..0178761 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9700,6 +9700,7 @@ enum skl_power_gate {
>  #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
>  #define _PIPE_A_CSC_COEFF_BV	0x49024
>  #define _PIPE_A_CSC_MODE	0x49028
> +#define   CSC_ENABLE			(1 << 31)
>  #define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
>  #define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>  #define   CSC_MODE_YUV_TO_RGB		(1 << 0)
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 7860244..2ebfe3a 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -239,7 +239,11 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state)
>  		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
>  		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
>  
> -		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			I915_WRITE(PIPE_CSC_MODE(pipe),
> +				I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE);
Just write CSC_ENABLE here, I think.
> +		else
> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>  	} else {
>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
Should there be a case in  ilk_load_ycbcr_conversion_matrix as well?

Though it seems wrong that function ignore crtc_state->ctm in that case.
> @@ -736,6 +740,7 @@ void intel_color_init(struct drm_crtc *crtc)
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = glk_load_luts;
>  	} else if (IS_ICELAKE(dev_priv)) {
> +		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = icl_load_luts;
>  	} else {
>  		dev_priv->display.load_luts = i9xx_load_luts;
Shankar, Uma Oct. 24, 2018, 2 p.m. UTC | #2
>-----Original Message-----
>From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>Sent: Wednesday, October 24, 2018 7:08 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915/icl: Enable ICL Pipe CSC block
>
>Op 23-10-18 om 22:40 schreef Uma Shankar:
>> Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE
>> register instead of PLANE_COLOR_CTL.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    | 1 +
>>  drivers/gpu/drm/i915/intel_color.c | 7 ++++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index dd0514e..0178761 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -9700,6 +9700,7 @@ enum skl_power_gate {
>>  #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
>>  #define _PIPE_A_CSC_COEFF_BV	0x49024
>>  #define _PIPE_A_CSC_MODE	0x49028
>> +#define   CSC_ENABLE			(1 << 31)
>>  #define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
>>  #define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>>  #define   CSC_MODE_YUV_TO_RGB		(1 << 0)
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 7860244..2ebfe3a 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -239,7 +239,11 @@ static void ilk_load_csc_matrix(struct drm_crtc_state
>*crtc_state)
>>  		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
>>  		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
>>
>> -		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>> +		if (INTEL_GEN(dev_priv) >= 11)
>> +			I915_WRITE(PIPE_CSC_MODE(pipe),
>> +				I915_READ(PIPE_CSC_MODE(pipe)) |
>CSC_ENABLE);
>Just write CSC_ENABLE here, I think.

Yes, this could be done as of now since PIPE OUTPUT CSC is not enabled.
Was keeping it as a failsafe for future once that feature gets enabled, this
may start overwriting that and will cause some bug and debug to hunt where
the value got overwritten :)

>> +		else
>> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>>  	} else {
>>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>Should there be a case in  ilk_load_ycbcr_conversion_matrix as well?

>Though it seems wrong that function ignore crtc_state->ctm in that case.

This is a good catch and it should be added there as well. Since this was a case
of hardcoded co-efficients, I believe it doesn't bother about userspace LUT blob,
so that should be ok.

Thanks Maarten for the review and very useful comments. I will update the patches
and refloat.

Regards,
Uma Shankar

>> @@ -736,6 +740,7 @@ void intel_color_init(struct drm_crtc *crtc)
>>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>>  		dev_priv->display.load_luts = glk_load_luts;
>>  	} else if (IS_ICELAKE(dev_priv)) {
>> +		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>>  		dev_priv->display.load_luts = icl_load_luts;
>>  	} else {
>>  		dev_priv->display.load_luts = i9xx_load_luts;
>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dd0514e..0178761 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9700,6 +9700,7 @@  enum skl_power_gate {
 #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
 #define _PIPE_A_CSC_COEFF_BV	0x49024
 #define _PIPE_A_CSC_MODE	0x49028
+#define   CSC_ENABLE			(1 << 31)
 #define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
 #define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
 #define   CSC_MODE_YUV_TO_RGB		(1 << 0)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 7860244..2ebfe3a 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -239,7 +239,11 @@  static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state)
 		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
 		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
 
-		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+		if (INTEL_GEN(dev_priv) >= 11)
+			I915_WRITE(PIPE_CSC_MODE(pipe),
+				I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE);
+		else
+			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
 	} else {
 		uint32_t mode = CSC_MODE_YUV_TO_RGB;
 
@@ -736,6 +740,7 @@  void intel_color_init(struct drm_crtc *crtc)
 		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = glk_load_luts;
 	} else if (IS_ICELAKE(dev_priv)) {
+		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = icl_load_luts;
 	} else {
 		dev_priv->display.load_luts = i9xx_load_luts;