Re: [PATCH 19/22] drm/i915: BDW: Pipe level Gamma correction

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

 



Regards
Shashank

On 10/12/2015 11:39 PM, Rob Bradford wrote:
On Sat, 2015-10-10 at 00:59 +0530, Shashank Sharma wrote:
BDW/SKL/BXT platforms support various Gamma correction modes
which are:
1. Legacy 8-bit mode
2. 10-bit Split Gamma mode
3. 12-bit mode

This patch does the following:
1. Adds the core function to program Gamma correction values
    for BDW/SKL/BXT platforms
2. Adds Gamma correction macros/defines

Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Kausal Malladi <kausalmalladi@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_reg.h            |  25 ++-
  drivers/gpu/drm/i915/intel_color_manager.c | 248
+++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/intel_color_manager.h |   6 +
  3 files changed, 277 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 5825ab2..ed50f75 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5647,11 +5647,15 @@ enum skl_disp_power_wells {
  /* legacy palette */
  #define _LGC_PALETTE_A           0x4a000
  #define _LGC_PALETTE_B           0x4a800
-#define LGC_PALETTE(pipe, i) (_PIPE(pipe, _LGC_PALETTE_A,
_LGC_PALETTE_B) + (i) * 4)
+#define _LGC_PALETTE_C           0x4b000
+#define LGC_PALETTE(pipe, i) (_PIPE3(pipe, _LGC_PALETTE_A,
_LGC_PALETTE_B, \
+			 _LGC_PALETTE_C) + (i) * 4)

  #define _GAMMA_MODE_A		0x4a480
  #define _GAMMA_MODE_B		0x4ac80
-#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
+#define _GAMMA_MODE_C		0x4b480
+#define GAMMA_MODE(pipe) \
+	_PIPE3(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B, _GAMMA_MODE_C)
  #define GAMMA_MODE_MODE_MASK	(3 << 0)
  #define GAMMA_MODE_MODE_8BIT	(0 << 0)
  #define GAMMA_MODE_MODE_10BIT	(1 << 0)
@@ -8062,6 +8066,23 @@ enum skl_disp_power_wells {
  #define _PIPE_CSC_BASE(pipe) \
  	(_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))

+/* BDW gamma correction */
+#define PAL_PREC_INDEX_A                       0x4A400
+#define PAL_PREC_INDEX_B                       0x4AC00
+#define PAL_PREC_INDEX_C                       0x4B400
+#define PAL_PREC_DATA_A                        0x4A404
+#define PAL_PREC_DATA_B                        0x4AC04
+#define PAL_PREC_DATA_C                        0x4B404
+#define PAL_PREC_GCMAX_A			0x4A410
+#define PAL_PREC_GCMAX_B			0x4AC10
+#define PAL_PREC_GCMAX_C			0x4B410
+
+#define _PREC_PAL_INDEX(pipe) \
+	(_PIPE3(pipe, PAL_PREC_INDEX_A, PAL_PREC_INDEX_B,
PAL_PREC_INDEX_C))
+#define _PREC_PAL_DATA(pipe) \
+	(_PIPE3(pipe, PAL_PREC_DATA_A, PAL_PREC_DATA_B,
PAL_PREC_DATA_C))
+#define _PREC_PAL_GCMAX(pipe) \
+	(_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B,
PAL_PREC_GCMAX_C))


  #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index d5315b2..74f8fc3 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -26,6 +26,252 @@
   */

  #include "intel_color_manager.h"
+static void bdw_write_10bit_gamma_precision(struct drm_device *dev,
+	struct drm_r32g32b32 *correction_values, u32 pal_prec_data,
+			u32 no_of_coeff)
+{
+	u16 blue_fract, green_fract, red_fract;
+	u32 word = 0;
+	u32 count = 0;
+	u32 blue, green, red;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	while (count < no_of_coeff) {
+
+		blue = correction_values[count].b32;
+		green = correction_values[count].g32;
+		red = correction_values[count].r32;
+
+		/*
+		* Maximum possible gamma correction value supported
+		* for BDW is 0xFFFFFFFF, so clip the values
accordingly
+		*/

I think you mean clamp not clip.
Yes, will fix this.

+		if (blue >= BDW_MAX_GAMMA)
+			blue = BDW_MAX_GAMMA;
+		if (green >= BDW_MAX_GAMMA)
+			green = BDW_MAX_GAMMA;
+		if (red >= BDW_MAX_GAMMA)
+			red = BDW_MAX_GAMMA;

So this handles the issue that was raised before that 1.0 in 8.24
should map to 1023.
Yes.

+
+		/*
+		* Gamma correction values are sent in 8.24 format
+		* with 8 int and 24 fraction bits. BDW 10 bit gamma
+		* unit expects correction registers to be programmed
in
+		* 0.10 format, with 0 int and 16 fraction bits. So
take
+		* MSB 10 bit values(bits 23-14) from the fraction
part and
+		* prepare the correction registers.
+		*/
+		blue_fract = GET_BITS(blue, 14, 10);
+		green_fract = GET_BITS(green, 14, 10);
+		red_fract = GET_BITS(red, 14, 10);
+

I think this should round to the nearest rather than floor.

Why ? we are getting the exact values already.
+		/* Arrange: Red (29:20) Green (19:10) and Blue (9:0)
*/
+		SET_BITS(word, red_fract, 20, 10);
+		SET_BITS(word, red_fract, 10, 10);
+		SET_BITS(word, red_fract, 0, 10);

Red is my favourite colour too, is that why you programmed all the
channels to the red bits? :-)
Guilty :). While testing, all the channels have the same values, so it dint matter. I will fix this.

+		I915_WRITE(pal_prec_data, word);
+		count++;
+	}
+	DRM_DEBUG_DRIVER("Gamma correction programmed\n");
+}
+
+void bdw_write_12bit_gamma_precision(struct drm_device *dev,
+	struct drm_r32g32b32 *correction_values, u32 pal_prec_data,
+		enum pipe pipe)
+{
+	uint16_t blue_fract, green_fract, red_fract;
+	uint32_t gcmax;
+	uint32_t word = 0;
+	uint32_t count = 0;
+	uint32_t gcmax_reg;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* Program first 512 values in precision palette */
+	while (count < BDW_12BIT_GAMMA_MAX_VALS - 1) {
+		/*
+		* Framework's general gamma format is 8.24 (8 int 16
fraction)
+		* BDW Platform's supported gamma format is 16 bit
correction
+		* values in 0.16 format. So extract higher 16
fraction bits
+		* from 8.24 gamma correction values.
+		*/
+		red_fract = GET_BITS(correction_values[count].r32,
8, 16);
+		green_fract = GET_BITS(correction_values[count].g32,
8, 16);
+		blue_fract = GET_BITS(correction_values[count].b32,
8, 16);

Again, 1.0 in 8.24 will result in you programming zeros rather than
max.
Will add the max check and conversion here also.

+		/*
+		* From the bspec:
+		* For 12 bit gamma correction, program precision
palette
+		* with 16 bits per color in a 0.16 format with 0
integer and
+		* 16 fractional bits (upper 10 bits in odd indexes,
lower 6
+		* bits in even indexes)
+		*/
+
+		/* Even index: Lower 6 bits from correction should
go as MSB */
+		SET_BITS(word, GET_BITS(red_fract, 0, 6), 24, 6);
+		SET_BITS(word, GET_BITS(green_fract, 0, 6), 14, 6);
+		SET_BITS(word, GET_BITS(blue_fract, 0, 6), 4, 6);
+		I915_WRITE(pal_prec_data, word);
+
+		word = 0x0;
+		/* Odd index: Upper 10 bits of correction should go
as MSB */
+		SET_BITS(word, GET_BITS(red_fract, 6, 10), 20, 10);
+		SET_BITS(word, GET_BITS(green_fract, 6, 10), 10,
10);
+		SET_BITS(word, GET_BITS(blue_fract, 6, 10), 0, 10);
+
+		I915_WRITE(pal_prec_data, word);
+		count++;
+	}
+
+	/* Now program the 513th value in GCMAX regs */
+	word = 0;
+	gcmax_reg = _PREC_PAL_GCMAX(pipe);
+	gcmax = min_t(u32, GET_BITS(correction_values[count].r32, 8,
17),
+				BDW_MAX_GAMMA);
+	SET_BITS(word, gcmax, 0, 17);
+	I915_WRITE(gcmax_reg, word);
+	gcmax_reg += 4;
+
+	word = 0;
+	gcmax = min_t(u32, GET_BITS(correction_values[count].g32, 8,
17),
+				BDW_MAX_GAMMA);
+	SET_BITS(word, gcmax, 0, 17);
+	I915_WRITE(gcmax_reg, word);
+	gcmax_reg += 4;
+
+	word = 0;
+	gcmax = min_t(u32, GET_BITS(correction_values[count].b32, 8,
17),
+				BDW_MAX_GAMMA);
+	SET_BITS(word, gcmax, 0, 17);
+	I915_WRITE(gcmax_reg, word);
+}
+
+/* Apply unity gamma for gamma reset */
+static void bdw_reset_gamma(struct drm_i915_private *dev_priv,
+			enum pipe pipe)
+{
+	u16 count = 0;
+	u32 val;
+	u32 pal_prec_data = LGC_PALETTE(pipe, 0);
+
+	DRM_DEBUG_DRIVER("\n");
+
+	/* Reset the palette for unit gamma */
+	while (count < BDW_8BIT_GAMMA_MAX_VALS) {
+		/* Red (23:16) Green (15:8) and Blue (7:0) */
+		val = (count << 16) | (count << 8) | count;
+		I915_WRITE(pal_prec_data, val);
+		pal_prec_data += 4;
+		count++;
+	}
+}
+
+static int bdw_set_gamma(struct drm_device *dev, struct
drm_property_blob *blob,
+			struct drm_crtc *crtc)
+{
+	u16 blue_fract, green_fract, red_fract;
+	enum pipe pipe;
+	int count, num_samples;
+	u32 blue, green, red;
+	u32 mode, pal_prec_index, pal_prec_data;
+	u32 index;
+	u32 word = 0;
+	struct drm_palette *gamma_data;
+	struct drm_crtc_state *state = crtc->state;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_r32g32b32 *correction_values = NULL;
+
+	if (WARN_ON(!blob))
+		return -EINVAL;
+
+	gamma_data = (struct drm_palette *)blob->data;
+	pipe = to_intel_crtc(crtc)->pipe;
+	num_samples = gamma_data->num_samples;
+
+	pal_prec_index = _PREC_PAL_INDEX(pipe);
+	pal_prec_data = _PREC_PAL_DATA(pipe);
+
+	correction_values = (struct drm_r32g32b32 *)&gamma_data
->lut;
+	index = I915_READ(pal_prec_index);
+
+	switch (num_samples) {
+	case GAMMA_DISABLE_VALS:
+
+		/* Disable Gamma functionality on Pipe */
+		DRM_DEBUG_DRIVER("Disabling gamma on Pipe %c\n",
+		pipe_name(pipe));
+		mode = I915_READ(GAMMA_MODE(pipe));
+		if ((mode & GAMMA_MODE_MODE_MASK) ==
GAMMA_MODE_MODE_12BIT)
+			bdw_reset_gamma(dev_priv, pipe);

Why only call bdw_reset_gamma() if we were in 12-bit mode? What about
the other modes? What about if somebody has programmed the value
through the legacy ioctl?
We tested this part, its only required for 12 bit gamma. The legacy IOCTL only programs 8bit gamma, which doesnt need this anyways, coz we are overwriting legacy palette to gamma = 1, while disabling gamma. If we dont do this for 12 bit gamma, screen goes all black.

+		state->palette_after_ctm_blob = NULL;
+		word = GAMMA_MODE_MODE_8BIT;
+		break;
+
+	case BDW_8BIT_GAMMA_MAX_VALS:
+
+		/* Legacy palette */
+		pal_prec_data = LGC_PALETTE(pipe, 0);
+		count = 0;
+		while (count < BDW_8BIT_GAMMA_MAX_VALS) {
+			blue = correction_values[count].b32;
+			green = correction_values[count].g32;
+			red = correction_values[count].r32;
+
+			blue_fract = GET_BITS(blue, 16, 8);
+			green_fract = GET_BITS(green, 16, 8);
+			red_fract = GET_BITS(red, 16, 8);

Shouldn't these round? Rather than always floor.
Why ? This is exact value.
> Also 1.0 in 8.24
format will result in you programming zero into the palette. You need
to do the max clamping again.
Will fix this part.

+
+			/* Blue (7:0) Green (15:8) and Red (23:16)
*/
+			SET_BITS(word, blue_fract, 0, 8);
+			SET_BITS(word, green_fract, 8, 8);
+			SET_BITS(word, blue_fract, 16, 8);
+			I915_WRITE(pal_prec_data, word);
+			pal_prec_data += 4;
+			count++;

This code also needs to be made to work nicely with other users of
LGC_PALETTE.
This is anyways working with LGC IOCTL, but will clean it up further.

+		}
+		word = GAMMA_MODE_MODE_8BIT;
+		break;
+
+	case BDW_SPLITGAMMA_MAX_VALS:
+
+		index |= BDW_INDEX_AUTO_INCREMENT |
BDW_INDEX_SPLIT_MODE;
+		I915_WRITE(pal_prec_index, index);
+		bdw_write_10bit_gamma_precision(dev,
correction_values,
+			pal_prec_data, BDW_SPLITGAMMA_MAX_VALS);
+		word = GAMMA_MODE_MODE_SPLIT;
+		break;
+
+	case BDW_12BIT_GAMMA_MAX_VALS:
+
+		index |= BDW_INDEX_AUTO_INCREMENT;
+		index &= ~BDW_INDEX_SPLIT_MODE;
+		I915_WRITE(pal_prec_index, index);
+		bdw_write_12bit_gamma_precision(dev,
correction_values,
+			pal_prec_data, pipe);
+		word = GAMMA_MODE_MODE_12BIT;
+		break;
+
+	case BDW_10BIT_GAMMA_MAX_VALS:
+		index |= BDW_INDEX_AUTO_INCREMENT;
+		index &= ~BDW_INDEX_SPLIT_MODE;
+		I915_WRITE(pal_prec_index, index);
+		bdw_write_10bit_gamma_precision(dev,
correction_values,
+			pal_prec_data, BDW_10BIT_GAMMA_MAX_VALS);
+		word = GAMMA_MODE_MODE_10BIT;
+		break;
+
+	default:
+		DRM_ERROR("Invalid number of samples\n");
+		return -EINVAL;
+	}
+
+	/* Set gamma mode on pipe control reg */
+	mode = I915_READ(GAMMA_MODE(pipe));
+	mode &= ~GAMMA_MODE_MODE_MASK;
+	I915_WRITE(GAMMA_MODE(pipe), mode | word);
+	DRM_DEBUG_DRIVER("Gamma applied on pipe %c\n",
+	pipe_name(pipe));
+	return 0;
+}

  static s16 chv_prepare_csc_coeff(s64 csc_value)
  {
@@ -319,6 +565,8 @@ void intel_color_manager_crtc_commit(struct
drm_device *dev,
  		/* Gamma correction is platform specific */
  		if (IS_CHERRYVIEW(dev))
  			ret = chv_set_gamma(dev, blob, crtc);
+		else if (IS_BROADWELL(dev) || IS_GEN9(dev))
+			ret = bdw_set_gamma(dev, blob, crtc);

  		if (ret)
  			DRM_ERROR("set Gamma correction failed\n");
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
b/drivers/gpu/drm/i915/intel_color_manager.h
index 271246a..6c7cb08 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -92,3 +92,9 @@

  /* Gamma on BDW */
  #define BDW_SPLITGAMMA_MAX_VALS                512
+#define BDW_8BIT_GAMMA_MAX_VALS		256
+#define BDW_10BIT_GAMMA_MAX_VALS		1024
+#define BDW_12BIT_GAMMA_MAX_VALS		513
+#define BDW_MAX_GAMMA                         ((1 << 24) - 1)
+#define BDW_INDEX_AUTO_INCREMENT               (1 << 15)
+#define BDW_INDEX_SPLIT_MODE                   (1 << 31)

Rob

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux