diff mbox series

[03/11,v4] drm/i915: Extract i9xx_get_color_config()

Message ID 1555509813-9021-4-git-send-email-swati2.sharma@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: adding state checker for gamma lut values | expand

Commit Message

Sharma, Swati2 April 17, 2019, 2:03 p.m. UTC 
v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_color.c | 50 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

Comments

Ville Syrjälä April 23, 2019, 4:13 p.m. UTC | #1 
On Wed, Apr 17, 2019 at 07:33:25PM +0530, Swati Sharma wrote:
> v4: -No need to initialize *blob [Jani]
>     -Removed right shifts [Jani]
>     -Dropped dev local var [Jani]
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
>  drivers/gpu/drm/i915/intel_color.c | 50 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9c206e8..8f2ae8a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7175,6 +7175,9 @@ enum {
>  /* legacy palette */
>  #define _LGC_PALETTE_A           0x4a000
>  #define _LGC_PALETTE_B           0x4a800
> +#define LGC_PALETTE_RED_MASK     REG_GENMASK(23, 16)
> +#define LGC_PALETTE_GREEN_MASK   REG_GENMASK(15, 8)
> +#define LGC_PALETTE_BLUE_MASK    REG_GENMASK(7, 0)
>  #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
>  
>  /* ilk/snb precision palette */
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 309f714..33223e0 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1229,6 +1229,55 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> +/* convert hw value with given bit_precision to lut property val */
> +static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
> +{
> +	u32 max = 0xffff >> (16 - bit_precision);
> +
> +	val = clamp_val(val, 0, max);
> +
> +	if (bit_precision < 16)
> +		val <<= 16 - bit_precision;
> +
> +	return val;
> +}
> +
> +static void i9xx_get_config_internal(struct intel_crtc_state *crtc_state)

The naming of these functions isn't particularly descriptive. I would
probably go for eg. i9xx_read_lut_8(), chv_read_cgm_gamma_lut(), etc.
AFAICS the series is also missing readout for degamma LUTs entirely.

We could also make these functions just return the blob instead of
assigning it internally. That way we can have the higher level
.get_color_config() implementation assign each gamma/degamma lut
apporpriately. Eg. might looks something like this for bdw:

bdw_get_color_config()
{
	if (gamma_mode == 8bit) {
		crtc_state->gamma_lut = i9xx_read_lut_8();
	} else if (gamma_mode == split) {
		crtc_state->degamma_lut = bdw_read_lut_10(SPLIT | 0);
		crtc_state->gamma_lut = bdw_read_lut_10(SPLIT | 512);
	} else if (csc_mode == gamma_before_csc) {
		crtc_state->degamma_lut = bdw_read_lut_10(0);
	} else {
		crtc_state->gamma_lut = bdw_read_lut_10(0);
	}
}

The tricky part will be to figure out how the gamma vs. degamma gets
assigned and/or checked since the choice depends on the presence of
the ctm, rgb limit range, and rgb->ycbcr conversion.

Oh, and most (all maybe?) patches seem to be missing a proper commit
message.

> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *blob_data;
> +	u32 i, val;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_lut) * 256,
> +					NULL);
> +	if (IS_ERR(blob))
> +		return;
> +
> +	blob_data = blob->data;
> +
> +	for (i = 0; i < 256; i++) {
> +		if (HAS_GMCH(dev_priv))
> +			val = I915_READ(PALETTE(pipe, i));
> +		else
> +			val = I915_READ(LGC_PALETTE(pipe, i));
> +
> +		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
> +		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
> +		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
> +	}
> +
> +	crtc_state->base.gamma_lut = blob;
> +}
> +
> +static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
> +{
> +	i9xx_get_config_internal(crtc_state);
> +}
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1249,6 +1298,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.color_check = i9xx_color_check;
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = i9xx_load_luts;
> +			dev_priv->display.get_color_config = i9xx_get_color_config;
>  		}
>  	} else {
>  		if (INTEL_GEN(dev_priv) >= 11)
> -- 
> 1.9.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c206e8..8f2ae8a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7175,6 +7175,9 @@  enum {
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
 #define _LGC_PALETTE_B           0x4a800
+#define LGC_PALETTE_RED_MASK     REG_GENMASK(23, 16)
+#define LGC_PALETTE_GREEN_MASK   REG_GENMASK(15, 8)
+#define LGC_PALETTE_BLUE_MASK    REG_GENMASK(7, 0)
 #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
 
 /* ilk/snb precision palette */
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 309f714..33223e0 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1229,6 +1229,55 @@  static int icl_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+{
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	val = clamp_val(val, 0, max);
+
+	if (bit_precision < 16)
+		val <<= 16 - bit_precision;
+
+	return val;
+}
+
+static void i9xx_get_config_internal(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * 256,
+					NULL);
+	if (IS_ERR(blob))
+		return;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < 256; i++) {
+		if (HAS_GMCH(dev_priv))
+			val = I915_READ(PALETTE(pipe, i));
+		else
+			val = I915_READ(LGC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
+	}
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	i9xx_get_config_internal(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1249,6 +1298,7 @@  void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = i9xx_load_luts;
+			dev_priv->display.get_color_config = i9xx_get_color_config;
 		}
 	} else {
 		if (INTEL_GEN(dev_priv) >= 11)