>-----Original Message----- >From: Ville Syrjala [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Tuesday, February 19, 2019 1:02 AM >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; Roper, Matthew D ><matthew.d.roper@xxxxxxxxx> >Subject: [PATCH 4/7] drm/i915: Clean up ilk/icl pipe/output CSC programming > >From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >We have far too much messy duplicated code in the pipe/output CSC programming. >Simply provide two functions >(ilk_update_pipe_csc() and icl_update_output_csc()) to program the relevant CSC >registers. The desired offsets and coefficients are passed in as parameters. > >Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >--- > drivers/gpu/drm/i915/intel_color.c | 168 ++++++++++++++--------------- > 1 file changed, 82 insertions(+), 86 deletions(-) > >diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >index ddc48c0d45ac..61cb69058b35 100644 >--- a/drivers/gpu/drm/i915/intel_color.c >+++ b/drivers/gpu/drm/i915/intel_color.c >@@ -40,23 +40,6 @@ > #define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1)) > > #define LEGACY_LUT_LENGTH 256 >- >-/* Post offset values for RGB->YCBCR conversion */ -#define >POSTOFF_RGB_TO_YUV_HI 0x800 -#define POSTOFF_RGB_TO_YUV_ME 0x100 - >#define POSTOFF_RGB_TO_YUV_LO 0x800 >- >-/* >- * These values are direct register values specified in the Bspec, >- * for RGB->YUV conversion matrix (colorspace BT709) >- */ >-#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8 -#define CSC_RGB_TO_YUV_BU >0x37e80000 -#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0 -#define >CSC_RGB_TO_YUV_BY 0xb5280000 -#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 - >#define CSC_RGB_TO_YUV_BV 0x1e080000 >- > /* > * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > * format). This macro takes the coefficient we want transformed and the @@ -74,6 >+57,31 @@ > #define ILK_CSC_COEFF_1_0 \ > ((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8)) > >+#define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255) >+ >+static const u16 ilk_csc_off_zero[3] = {}; >+ >+static const u16 ilk_csc_postoff_limited_range[3] = { >+ ILK_CSC_POSTOFF_LIMITED_RANGE, >+ ILK_CSC_POSTOFF_LIMITED_RANGE, >+ ILK_CSC_POSTOFF_LIMITED_RANGE, >+}; >+ >+/* >+ * These values are direct register values specified in the Bspec, >+ * for RGB->YUV conversion matrix (colorspace BT709) */ static const >+u16 ilk_csc_coeff_rgb_to_ycbcr[9] = { >+ 0x1e08, 0x9cc0, 0xb528, >+ 0x2ba8, 0x09d8, 0x37e8, >+ 0xbce8, 0x9ad8, 0x1e08, >+}; I am not sure if the matrix coefficients are correct. Can you please cross check, if I am missing something. Spec has these as values (hoping table doesn’t get distorted while sending :)) Bt.601 Bt.709 Value Program Value Program RU 0.2990 0x1990 0.21260 0x2D98 GU 0.5870 0x0968 0.71520 0x0B70 BU 0.1140 0x3E98 0.07220 0x3940 RV -0.1687 0xAAC8 -0.11460 0xBEA8 GV -0.3313 0x9A98 -0.38540 0x9C58 BV 0.5000 0x0800 0.50000 0x0800 RY 0.5000 0x0800 0.50000 0x0800 GY -0.4187 0x9D68 -0.45420 0x9E88 BY -0.0813 0xBA68 -0.04580 0xB5E0 >+ >+/* Post offset values for RGB->YCBCR conversion */ static const u16 >+ilk_csc_postoff_rgb_to_ycbcr[3] = { >+ 0x0800, 0x0100, 0x0800, >+}; >+ > static bool lut_is_legacy(const struct drm_property_blob *lut) { > return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH; @@ -113,54 >+121,60 @@ static u64 *ctm_mult_by_limited(u64 *result, const u64 *input) > return result; > } > >-static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc) >+static void ilk_update_pipe_csc(struct intel_crtc *crtc, >+ const u16 preoff[3], >+ const u16 coeff[9], >+ const u16 postoff[3]) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum pipe pipe = crtc->pipe; > >- if (INTEL_GEN(dev_priv) < 11) { >- I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); >- I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); >- I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); >+ I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), preoff[0]); >+ I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), preoff[1]); >+ I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), preoff[2]); > >- I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), >CSC_RGB_TO_YUV_RU_GU); >- I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU); >+ I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff[0] << 16 | coeff[1]); >+ I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeff[2] << 16); > >- I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), >CSC_RGB_TO_YUV_RY_GY); >- I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY); >+ I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff[3] << 16 | coeff[4]); >+ I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeff[5] << 16); > >- I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), >CSC_RGB_TO_YUV_RV_GV); >- I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV); >+ I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeff[6] << 16 | coeff[7]); >+ I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff[8] << 16); > >- 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); >- } else { >- I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_HI(pipe), 0); >- I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_ME(pipe), 0); >- I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_LO(pipe), 0); >- >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RU_GU(pipe), >- CSC_RGB_TO_YUV_RU_GU); >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BU(pipe), >CSC_RGB_TO_YUV_BU); >- >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RY_GY(pipe), >- CSC_RGB_TO_YUV_RY_GY); >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BY(pipe), >CSC_RGB_TO_YUV_BY); >- >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RV_GV(pipe), >- CSC_RGB_TO_YUV_RV_GV); >- I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BV(pipe), >CSC_RGB_TO_YUV_BV); >- >- I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_HI(pipe), >- POSTOFF_RGB_TO_YUV_HI); >- I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_ME(pipe), >- POSTOFF_RGB_TO_YUV_ME); >- I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), >- POSTOFF_RGB_TO_YUV_LO); >+ if (INTEL_GEN(dev_priv) >= 7) { >+ I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff[0]); >+ I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff[1]); >+ I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff[2]); > } > } > >+static void icl_update_output_csc(struct intel_crtc *crtc, >+ const u16 preoff[3], >+ const u16 coeff[9], >+ const u16 postoff[3]) >+{ >+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >+ enum pipe pipe = crtc->pipe; >+ >+ I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_HI(pipe), preoff[0]); >+ I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_ME(pipe), preoff[1]); >+ I915_WRITE(PIPE_CSC_OUTPUT_PREOFF_LO(pipe), preoff[2]); >+ >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RY_GY(pipe), coeff[0] << 16 | >coeff[1]); >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BY(pipe), coeff[2]); >+ >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RU_GU(pipe), coeff[3] << 16 | >coeff[4]); >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BU(pipe), coeff[5]); >+ >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_RV_GV(pipe), coeff[6] << 16 | >coeff[7]); >+ I915_WRITE(PIPE_CSC_OUTPUT_COEFF_BV(pipe), coeff[8]); >+ >+ I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_HI(pipe), postoff[0]); >+ I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_ME(pipe), postoff[1]); >+ I915_WRITE(PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), postoff[2]); } >+ > static bool ilk_csc_limited_range(const struct intel_crtc_state *crtc_state) { > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); >@@ -185,7 +199,15 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state >*crtc_state) > > if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || > crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) { >- ilk_load_ycbcr_conversion_matrix(crtc); >+ if (INTEL_GEN(dev_priv) >= 11) >+ icl_update_output_csc(crtc, ilk_csc_off_zero, >+ ilk_csc_coeff_rgb_to_ycbcr, >+ ilk_csc_postoff_rgb_to_ycbcr); >+ else >+ ilk_update_pipe_csc(crtc, ilk_csc_off_zero, >+ ilk_csc_coeff_rgb_to_ycbcr, >+ ilk_csc_postoff_rgb_to_ycbcr); >+ > I915_WRITE(PIPE_CSC_MODE(pipe), crtc_state->csc_mode); > /* > * On pre GEN11 output CSC is not there, so with 1 pipe CSC @@ - >258,38 +280,12 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state >*crtc_state) > } > } > >- I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]); >- I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16); >- >- I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]); >- I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16); >- >- I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]); >- I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16); >+ ilk_update_pipe_csc(crtc, ilk_csc_off_zero, coeffs, >+ limited_color_range ? >+ ilk_csc_postoff_limited_range : >+ ilk_csc_off_zero); > >- I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); >- I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); >- I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); >- >- if (INTEL_GEN(dev_priv) > 6) { >- u16 postoff = 0; >- >- if (limited_color_range) >- postoff = (16 * (1 << 12) / 255) & 0x1fff; >- >- I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); >- I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); >- I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); >- >- I915_WRITE(PIPE_CSC_MODE(pipe), crtc_state->csc_mode); >- } else { >- u32 mode = CSC_MODE_YUV_TO_RGB; >- >- if (limited_color_range) >- mode |= CSC_BLACK_SCREEN_OFFSET; >- >- I915_WRITE(PIPE_CSC_MODE(pipe), mode); Looks like this is not handled and got dropped. Pre Gen7 stuff. >- } >+ I915_WRITE(PIPE_CSC_MODE(pipe), crtc_state->csc_mode); > } > > /* >-- >2.19.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
