>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Friday, December 21, 2018 11:02 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; >Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx> >Subject: Re: [v4 3/4] drm/i915/icl: Enable ICL Pipe CSC block > >On Fri, Dec 21, 2018 at 01:29:40AM +0530, Uma Shankar wrote: >> Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE >> register instead of PLANE_COLOR_CTL. >> >> ToDO: Extend the ABI to accept 32 bit coefficient values instead of >> 16bit for future platforms. >> >> v2: Addressed Maarten's review comments. >> >> v3: Addressed Matt's review comments. Removed rmw patterns as >> suggested by Matt. >> >> v4: Addressed Matt's review comments. >> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 4 +++- >> drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++-- >> 2 files changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 1852c33..565ef6a 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -9861,7 +9861,9 @@ 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_BLACK_SCREEN_OFFSET (1 << 2) >> +#define CSC_ENABLE (1 << 31) >> +#define OUTPUT_CSC_ENABLE (1 << 30) >> +#define CSC_BLACK_SCREEN_OFFSET (1 << 2) > >Bogus space->tab change. > >We should probably document which bit is for which platforms. Will rectify this. Also prefix ICL for CSC and Output CSC fields since they got introduced from Gen11 onwards. > >> #define CSC_POSITION_BEFORE_GAMMA (1 << 1) >> #define CSC_MODE_YUV_TO_RGB (1 << 0) >> #define _PIPE_A_CSC_PREOFF_HI 0x49030 >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index e72d8d6..d5b240c 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -134,7 +134,11 @@ static void ilk_load_ycbcr_conversion_matrix(struct >intel_crtc *crtc) >> I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); >> I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), >POSTOFF_RGB_TO_YUV_ME); >> I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); >> - I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> + >> + if (INTEL_GEN(dev_priv) >= 10) > >11 Yeah, will correct this. >> + I915_WRITE(PIPE_CSC_MODE(pipe), OUTPUT_CSC_ENABLE); >> + else >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> } >> >> static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state) >> @@ -242,7 +246,10 @@ static void ilk_load_csc_matrix(struct intel_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) >= 10) > >11 Will update. Thanks Ville for your comments. Regards, Uma Shankar > >> + I915_WRITE(PIPE_CSC_MODE(pipe), CSC_ENABLE); >> + else >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> } else { >> uint32_t mode = CSC_MODE_YUV_TO_RGB; >> >> @@ -715,6 +722,7 @@ void intel_color_init(struct intel_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; >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
