>-----Original Message-----
>From: Ville Syrjala [mailto:[email protected]]
>Sent: Tuesday, February 19, 2019 1:02 AM
>To: [email protected]
>Cc: Shankar, Uma <[email protected]>; Roper, Matthew D
><[email protected]>
>Subject: [PATCH 4/7] drm/i915: Clean up ilk/icl pipe/output CSC programming
>
>From: Ville Syrjälä <[email protected]>
>
>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ä <[email protected]>
>---
> 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
