Re: [PATCH 04/13] drm/i915: Clean up various indexed LUT registers

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

 


On 11/23/2022 8:56 PM, Ville Syrjala wrote:

From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Use REG_BIT() & co. for the LUT index registers, and also
use the REG_FIELD_PREP() stuff a bit more consistently when
generating the values for said registers.

Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_color.c | 46 +++++++++++++++-------
  drivers/gpu/drm/i915/i915_reg.h            | 18 +++++----
  2 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 956b221860e6..c960c2aaf328 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -910,7 +910,8 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
  	enum pipe pipe = crtc->pipe;
for (i = 0; i < lut_size; i++) { 
-		intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), prec_index++);
+		intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+				  prec_index + i);
  		intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
  				  ilk_lut_10(&lut[i]));
  	}
@@ -919,7 +920,8 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
  	 * Reset the index, otherwise it prevents the legacy palette to be
  	 * written properly.
  	 */
-	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0);
+	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+			  PAL_PREC_INDEX_VALUE(0));
  }
/* On BDW+ the index auto increment mode actually works */ 
@@ -933,7 +935,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
  	enum pipe pipe = crtc->pipe;
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 
-			  prec_index | PAL_PREC_AUTO_INCREMENT);
+			  PAL_PREC_AUTO_INCREMENT |
+			  prec_index);
for (i = 0; i < lut_size; i++) 
  		intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
@@ -943,7 +946,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
  	 * Reset the index, otherwise it prevents the legacy palette to be
  	 * written properly.
  	 */
-	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0);
+	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+			  PAL_PREC_INDEX_VALUE(0));
  }
static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state) 
@@ -1049,9 +1053,11 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state,
  	 * ignore the index bits, so we need to reset it to index 0
  	 * separately.
  	 */
-	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0);
  	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
-			  PRE_CSC_GAMC_AUTO_INCREMENT);
+			  PRE_CSC_GAMC_INDEX_VALUE(0));
+	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
+			  PRE_CSC_GAMC_AUTO_INCREMENT |
+			  PRE_CSC_GAMC_INDEX_VALUE(0));
for (i = 0; i < lut_size; i++) { 
  		/*
@@ -1165,7 +1171,9 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
  	 * seg2[0] being unused by the hardware.
  	 */
  	intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
-			    PAL_PREC_AUTO_INCREMENT);
+			    PAL_PREC_AUTO_INCREMENT |
+			    PAL_PREC_INDEX_VALUE(0));
+
  	for (i = 1; i < 257; i++) {
  		entry = &lut[i * 8];
  		intel_dsb_indexed_reg_write(crtc_state, PREC_PAL_DATA(pipe),
@@ -2756,7 +2764,8 @@ static struct drm_property_blob *ivb_read_lut_10(struct intel_crtc *crtc,
  		ilk_lut_10_pack(&lut[i], val);
  	}
- intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0); 
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
+			  PAL_PREC_INDEX_VALUE(0));
return blob; 
  }
@@ -2811,7 +2820,8 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
  	lut = blob->data;
intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 
-			  prec_index | PAL_PREC_AUTO_INCREMENT);
+			  PAL_PREC_AUTO_INCREMENT |
+			  prec_index);
for (i = 0; i < lut_size; i++) { 
  		u32 val = intel_de_read_fw(i915, PREC_PAL_DATA(pipe));
@@ -2819,7 +2829,8 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
  		ilk_lut_10_pack(&lut[i], val);
  	}
- intel_de_write_fw(i915, PREC_PAL_INDEX(pipe), 0); 
+	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
+			  PAL_PREC_INDEX_VALUE(0));
return blob; 
  }
@@ -2876,9 +2887,11 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc)
  	 * ignore the index bits, so we need to reset it to index 0
  	 * separately.
  	 */
-	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
  	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
-			  PRE_CSC_GAMC_AUTO_INCREMENT);
+			  PRE_CSC_GAMC_INDEX_VALUE(0));
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
+			  PRE_CSC_GAMC_AUTO_INCREMENT |
+			  PRE_CSC_GAMC_INDEX_VALUE(0));
for (i = 0; i < lut_size; i++) { 
  		u32 val = intel_de_read_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe));
@@ -2888,7 +2901,8 @@ static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc)
  		lut[i].blue = val;
  	}
- intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0); 
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
+			  PRE_CSC_GAMC_INDEX_VALUE(0));
return blob; 
  }
@@ -2934,7 +2948,8 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
  	lut = blob->data;
intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe), 
-			  PAL_PREC_AUTO_INCREMENT);
+			  PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
+			  PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
for (i = 0; i < 9; i++) { 
  		u32 ldw = intel_de_read_fw(i915, PREC_PAL_MULTI_SEG_DATA(pipe));
@@ -2943,7 +2958,8 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
  		ilk_lut_12p4_pack(&lut[i], ldw, udw);
  	}
- intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe), 0); 
+	intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
+			  PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
/* 
  	 * FIXME readouts from PAL_PREC_DATA register aren't giving
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 80ac50d80af4..22fb9fd78483 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7531,11 +7531,10 @@ enum skl_power_gate {
  #define _PAL_PREC_INDEX_A	0x4A400
  #define _PAL_PREC_INDEX_B	0x4AC00
  #define _PAL_PREC_INDEX_C	0x4B400
-#define   PAL_PREC_10_12_BIT		(0 << 31)
-#define   PAL_PREC_SPLIT_MODE		(1 << 31)
-#define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
-#define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
-#define   PAL_PREC_INDEX_VALUE(x)	((x) << 0)
+#define   PAL_PREC_SPLIT_MODE		REG_BIT(31)
+#define   PAL_PREC_AUTO_INCREMENT	REG_BIT(15)
+#define   PAL_PREC_INDEX_VALUE_MASK	REG_GENMASK(9, 0)
+#define   PAL_PREC_INDEX_VALUE(x)	REG_FIELD_PREP(PAL_PREC_INDEX_VALUE_MASK, (x))
  #define _PAL_PREC_DATA_A	0x4A404
  #define _PAL_PREC_DATA_B	0x4AC04
  #define _PAL_PREC_DATA_C	0x4B404
@@ -7559,7 +7558,9 @@ enum skl_power_gate {
  #define _PRE_CSC_GAMC_INDEX_A	0x4A484
  #define _PRE_CSC_GAMC_INDEX_B	0x4AC84
  #define _PRE_CSC_GAMC_INDEX_C	0x4B484
-#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
+#define   PRE_CSC_GAMC_AUTO_INCREMENT	REG_BIT(10)
+#define   PRE_CSC_GAMC_INDEX_VALUE_MASK	REG_GENMASK(7, 0)
PRE_CSC_GAMC_INDEX_VALUE_MASK till TGL seem to be using bits 0:5. For ADL+ this seem to be 0:7 though. Would it make sense to use separate masks? 


Regards,

Ankit



+#define   PRE_CSC_GAMC_INDEX_VALUE(x)	REG_FIELD_PREP(PRE_CSC_GAMC_INDEX_VALUE_MASK, (x))
  #define _PRE_CSC_GAMC_DATA_A	0x4A488
  #define _PRE_CSC_GAMC_DATA_B	0x4AC88
  #define _PRE_CSC_GAMC_DATA_C	0x4B488
@@ -7570,8 +7571,9 @@ enum skl_power_gate {
  /* ICL Multi segmented gamma */
  #define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
  #define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
-#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		REG_BIT(15)
-#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	REG_GENMASK(4, 0)
+#define   PAL_PREC_MULTI_SEG_AUTO_INCREMENT	REG_BIT(15)
+#define   PAL_PREC_MULTI_SEG_INDEX_VALUE_MASK	REG_GENMASK(4, 0)
+#define   PAL_PREC_MULTI_SEG_INDEX_VALUE(x)	REG_FIELD_PREP(PAL_PREC_MULTI_SEG_INDEX_VALUE_MASK, (x))
#define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C 
  #define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux