Re: [PATCH 02/12] drm/i915: Introduce intel_csc_matrix struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/29/2023 7:19 PM, Ville Syrjala wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Introduce a structure that can hold our CSC matrices. In there
we shall have the preoffsets, postoffsets, and coefficients,
all in platform specific format (at least for now).

We shall start by converting the ilk+ code to make use of
the new structure. chv will come later.

Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_color.c    | 188 +++++++++---------
  .../drm/i915/display/intel_display_types.h    |   6 +
  2 files changed, 97 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3c3e2f5a5cde..b1059e0c0665 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -120,40 +120,42 @@ struct intel_color_funcs {
  #define ILK_CSC_COEFF_LIMITED_RANGE ((235 - 16) << (12 - 8)) /* exponent 0 */
  #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 << (12 - 8))
-/* Nop pre/post offsets */
-static const u16 ilk_csc_off_zero[3] = {};
-
-/* Identity matrix */
-static const u16 ilk_csc_coeff_identity[9] = {
-	ILK_CSC_COEFF_1_0, 0, 0,
-	0, ILK_CSC_COEFF_1_0, 0,
-	0, 0, ILK_CSC_COEFF_1_0,
-};
-
-/* Limited range RGB post offsets */
-static const u16 ilk_csc_postoff_limited_range[3] = {
-	ILK_CSC_POSTOFF_LIMITED_RANGE,
-	ILK_CSC_POSTOFF_LIMITED_RANGE,
-	ILK_CSC_POSTOFF_LIMITED_RANGE,
+static const struct intel_csc_matrix ilk_csc_matrix_identity = {
+	.preoff = {},
+	.coeff = {
+		ILK_CSC_COEFF_1_0, 0, 0,
+		0, ILK_CSC_COEFF_1_0, 0,
+		0, 0, ILK_CSC_COEFF_1_0,
+	},
+	.postoff = {},
  };
/* Full range RGB -> limited range RGB matrix */
-static const u16 ilk_csc_coeff_limited_range[9] = {
-	ILK_CSC_COEFF_LIMITED_RANGE, 0, 0,
-	0, ILK_CSC_COEFF_LIMITED_RANGE, 0,
-	0, 0, ILK_CSC_COEFF_LIMITED_RANGE,
+static const struct intel_csc_matrix ilk_csc_matrix_limited_range = {
+	.preoff = {},
+	.coeff = {
+		ILK_CSC_COEFF_LIMITED_RANGE, 0, 0,
+		0, ILK_CSC_COEFF_LIMITED_RANGE, 0,
+		0, 0, ILK_CSC_COEFF_LIMITED_RANGE,
+	},
+	.postoff = {
+		ILK_CSC_POSTOFF_LIMITED_RANGE,
+		ILK_CSC_POSTOFF_LIMITED_RANGE,
+		ILK_CSC_POSTOFF_LIMITED_RANGE,
+	},
  };
/* BT.709 full range RGB -> limited range YCbCr matrix */
-static const u16 ilk_csc_coeff_rgb_to_ycbcr[9] = {
-	0x1e08, 0x9cc0, 0xb528,
-	0x2ba8, 0x09d8, 0x37e8,
-	0xbce8, 0x9ad8, 0x1e08,
-};
-
-/* Limited range YCbCr post offsets */
-static const u16 ilk_csc_postoff_rgb_to_ycbcr[3] = {
-	0x0800, 0x0100, 0x0800,
+static const struct intel_csc_matrix ilk_csc_matrix_rgb_to_ycbcr = {
+	.preoff = {},
+	.coeff = {
+		0x1e08, 0x9cc0, 0xb528,
+		0x2ba8, 0x09d8, 0x37e8,
+		0xbce8, 0x9ad8, 0x1e08,
+	},
+	.postoff = {
+		0x0800, 0x0100, 0x0800,
+	},
  };
static bool lut_is_legacy(const struct drm_property_blob *lut)
@@ -188,69 +190,66 @@ static u64 *ctm_mult_by_limited(u64 *result, const u64 *input)
  }
static void ilk_update_pipe_csc(struct intel_crtc *crtc,
-				const u16 preoff[3],
-				const u16 coeff[9],
-				const u16 postoff[3])
+				const struct intel_csc_matrix *csc)
  {
  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
  	enum pipe pipe = crtc->pipe;
- intel_de_write_fw(i915, PIPE_CSC_PREOFF_HI(pipe), preoff[0]);
-	intel_de_write_fw(i915, PIPE_CSC_PREOFF_ME(pipe), preoff[1]);
-	intel_de_write_fw(i915, PIPE_CSC_PREOFF_LO(pipe), preoff[2]);
+	intel_de_write_fw(i915, PIPE_CSC_PREOFF_HI(pipe), csc->preoff[0]);
+	intel_de_write_fw(i915, PIPE_CSC_PREOFF_ME(pipe), csc->preoff[1]);
+	intel_de_write_fw(i915, PIPE_CSC_PREOFF_LO(pipe), csc->preoff[2]);
intel_de_write_fw(i915, PIPE_CSC_COEFF_RY_GY(pipe),
-			  coeff[0] << 16 | coeff[1]);
-	intel_de_write_fw(i915, PIPE_CSC_COEFF_BY(pipe), coeff[2] << 16);
+			  csc->coeff[0] << 16 | csc->coeff[1]);
+	intel_de_write_fw(i915, PIPE_CSC_COEFF_BY(pipe),
+			  csc->coeff[2] << 16);
intel_de_write_fw(i915, PIPE_CSC_COEFF_RU_GU(pipe),
-			  coeff[3] << 16 | coeff[4]);
-	intel_de_write_fw(i915, PIPE_CSC_COEFF_BU(pipe), coeff[5] << 16);
+			  csc->coeff[3] << 16 | csc->coeff[4]);
+	intel_de_write_fw(i915, PIPE_CSC_COEFF_BU(pipe),
+			  csc->coeff[5] << 16);
intel_de_write_fw(i915, PIPE_CSC_COEFF_RV_GV(pipe),
-			  coeff[6] << 16 | coeff[7]);
-	intel_de_write_fw(i915, PIPE_CSC_COEFF_BV(pipe), coeff[8] << 16);
+			  csc->coeff[6] << 16 | csc->coeff[7]);
+	intel_de_write_fw(i915, PIPE_CSC_COEFF_BV(pipe),
+			  csc->coeff[8] << 16);
- if (DISPLAY_VER(i915) >= 7) {
-		intel_de_write_fw(i915, PIPE_CSC_POSTOFF_HI(pipe),
-				  postoff[0]);
-		intel_de_write_fw(i915, PIPE_CSC_POSTOFF_ME(pipe),
-				  postoff[1]);
-		intel_de_write_fw(i915, PIPE_CSC_POSTOFF_LO(pipe),
-				  postoff[2]);
-	}
+	if (DISPLAY_VER(i915) < 7)
+		return;
+
+	intel_de_write_fw(i915, PIPE_CSC_POSTOFF_HI(pipe), csc->postoff[0]);
+	intel_de_write_fw(i915, PIPE_CSC_POSTOFF_ME(pipe), csc->postoff[1]);
+	intel_de_write_fw(i915, PIPE_CSC_POSTOFF_LO(pipe), csc->postoff[2]);
  }
static void icl_update_output_csc(struct intel_crtc *crtc,
-				  const u16 preoff[3],
-				  const u16 coeff[9],
-				  const u16 postoff[3])
+				  const struct intel_csc_matrix *csc)
  {
  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
  	enum pipe pipe = crtc->pipe;
- intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_HI(pipe), preoff[0]);
-	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_ME(pipe), preoff[1]);
-	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_LO(pipe), preoff[2]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_HI(pipe), csc->preoff[0]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_ME(pipe), csc->preoff[1]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_PREOFF_LO(pipe), csc->preoff[2]);
intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_RY_GY(pipe),
-			  coeff[0] << 16 | coeff[1]);
+			  csc->coeff[0] << 16 | csc->coeff[1]);
  	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_BY(pipe),
-			  coeff[2] << 16);
+			  csc->coeff[2] << 16);
intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_RU_GU(pipe),
-			  coeff[3] << 16 | coeff[4]);
+			  csc->coeff[3] << 16 | csc->coeff[4]);
  	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_BU(pipe),
-			  coeff[5] << 16);
+			  csc->coeff[5] << 16);
intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_RV_GV(pipe),
-			  coeff[6] << 16 | coeff[7]);
+			  csc->coeff[6] << 16 | csc->coeff[7]);
  	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_COEFF_BV(pipe),
-			  coeff[8] << 16);
+			  csc->coeff[8] << 16);
- intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_HI(pipe), postoff[0]);
-	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_ME(pipe), postoff[1]);
-	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), postoff[2]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_HI(pipe), csc->postoff[0]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_ME(pipe), csc->postoff[1]);
+	intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), csc->postoff[2]);
  }
static bool ilk_limited_range(const struct intel_crtc_state *crtc_state)
@@ -294,13 +293,20 @@ static bool ilk_csc_limited_range(const struct intel_crtc_state *crtc_state)
  }
static void ilk_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
-				u16 coeffs[9], bool limited_color_range)
+				struct intel_csc_matrix *csc,
+				bool limited_color_range)
  {
  	const struct drm_color_ctm *ctm = crtc_state->hw.ctm->data;
  	const u64 *input;
  	u64 temp[9];
  	int i;
+ /* for preoff/postoff */
+	if (limited_color_range)
+		*csc = ilk_csc_matrix_limited_range;
+	else
+		*csc = ilk_csc_matrix_identity;


Lets merge this if block with the below if block, as we are again checking limited_color_range.

Otherwise patch looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>


+
  	if (limited_color_range)
  		input = ctm_mult_by_limited(temp, ctm->matrix);
  	else
@@ -319,28 +325,28 @@ static void ilk_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
  		 */
  		abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);
- coeffs[i] = 0;
+		csc->coeff[i] = 0;
/* sign bit */
  		if (CTM_COEFF_NEGATIVE(input[i]))
-			coeffs[i] |= 1 << 15;
+			csc->coeff[i] |= 1 << 15;
if (abs_coeff < CTM_COEFF_0_125)
-			coeffs[i] |= (3 << 12) |
+			csc->coeff[i] |= (3 << 12) |
  				ILK_CSC_COEFF_FP(abs_coeff, 12);
  		else if (abs_coeff < CTM_COEFF_0_25)
-			coeffs[i] |= (2 << 12) |
+			csc->coeff[i] |= (2 << 12) |
  				ILK_CSC_COEFF_FP(abs_coeff, 11);
  		else if (abs_coeff < CTM_COEFF_0_5)
-			coeffs[i] |= (1 << 12) |
+			csc->coeff[i] |= (1 << 12) |
  				ILK_CSC_COEFF_FP(abs_coeff, 10);
  		else if (abs_coeff < CTM_COEFF_1_0)
-			coeffs[i] |= ILK_CSC_COEFF_FP(abs_coeff, 9);
+			csc->coeff[i] |= ILK_CSC_COEFF_FP(abs_coeff, 9);
  		else if (abs_coeff < CTM_COEFF_2_0)
-			coeffs[i] |= (7 << 12) |
+			csc->coeff[i] |= (7 << 12) |
  				ILK_CSC_COEFF_FP(abs_coeff, 8);
  		else
-			coeffs[i] |= (6 << 12) |
+			csc->coeff[i] |= (6 << 12) |
  				ILK_CSC_COEFF_FP(abs_coeff, 7);
  	}
  }
@@ -352,21 +358,15 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state)
  	bool limited_color_range = ilk_csc_limited_range(crtc_state);
if (crtc_state->hw.ctm) {
-		u16 coeff[9];
+		struct intel_csc_matrix tmp;
- ilk_csc_convert_ctm(crtc_state, coeff, limited_color_range);
-		ilk_update_pipe_csc(crtc, ilk_csc_off_zero, coeff,
-				    limited_color_range ?
-				    ilk_csc_postoff_limited_range :
-				    ilk_csc_off_zero);
+		ilk_csc_convert_ctm(crtc_state, &tmp, limited_color_range);
+
+		ilk_update_pipe_csc(crtc, &tmp);
  	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
-		ilk_update_pipe_csc(crtc, ilk_csc_off_zero,
-				    ilk_csc_coeff_rgb_to_ycbcr,
-				    ilk_csc_postoff_rgb_to_ycbcr);
+		ilk_update_pipe_csc(crtc, &ilk_csc_matrix_rgb_to_ycbcr);
  	} else if (limited_color_range) {
-		ilk_update_pipe_csc(crtc, ilk_csc_off_zero,
-				    ilk_csc_coeff_limited_range,
-				    ilk_csc_postoff_limited_range);
+		ilk_update_pipe_csc(crtc, &ilk_csc_matrix_limited_range);
  	} else if (crtc_state->csc_enable) {
  		/*
  		 * On GLK both pipe CSC and degamma LUT are controlled
@@ -376,9 +376,7 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state)
  		 */
  		drm_WARN_ON(&i915->drm, !IS_GEMINILAKE(i915));
- ilk_update_pipe_csc(crtc, ilk_csc_off_zero,
-				    ilk_csc_coeff_identity,
-				    ilk_csc_off_zero);
+		ilk_update_pipe_csc(crtc, &ilk_csc_matrix_identity);
  	}
  }
@@ -387,21 +385,17 @@ static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
if (crtc_state->hw.ctm) {
-		u16 coeff[9];
+		struct intel_csc_matrix tmp;
- ilk_csc_convert_ctm(crtc_state, coeff, false);
-		ilk_update_pipe_csc(crtc, ilk_csc_off_zero,
-				    coeff, ilk_csc_off_zero);
+		ilk_csc_convert_ctm(crtc_state, &tmp, false);
+
+		ilk_update_pipe_csc(crtc, &tmp);
  	}
if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
-		icl_update_output_csc(crtc, ilk_csc_off_zero,
-				      ilk_csc_coeff_rgb_to_ycbcr,
-				      ilk_csc_postoff_rgb_to_ycbcr);
+		icl_update_output_csc(crtc, &ilk_csc_matrix_rgb_to_ycbcr);
  	} else if (crtc_state->limited_color_range) {
-		icl_update_output_csc(crtc, ilk_csc_off_zero,
-				      ilk_csc_coeff_limited_range,
-				      ilk_csc_postoff_limited_range);
+		icl_update_output_csc(crtc, &ilk_csc_matrix_limited_range);
  	}
  }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ab146b5b68bd..4829399ea700 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -985,6 +985,12 @@ struct intel_link_m_n {
  	u32 link_n;
  };
+struct intel_csc_matrix {
+	u16 coeff[9];
+	u16 preoff[3];
+	u16 postoff[3];
+};
+
  struct intel_crtc_state {
  	/*
  	 * uapi (drm) state. This is the software state shown to userspace.



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux