>-----Original Message-----
>From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
>Sent: Wednesday, October 24, 2018 7:08 PM
>To: Shankar, Uma <uma.shan...@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrj...@intel.com>; Lankhorst, Maarten
><maarten.lankho...@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.shan...@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;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
