[PATCH 1/3] drm/i915: Change gamma/degamma_lut_size data type to u32

[PATCH 1/3] drm/i915: Change gamma/degamma_lut_size data type to u32

[Shashank Sharma]

Currently, data type of gamma_lut_size & degamma_lut_size elements
in intel_device_info is u16, which means it can accommodate maximum
64k values. In case of ICL multisegmented gamma, the size of gamma
LUT is 256K.

This patch changes the data type of both of these elements to u32.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 5a2e17d6146b..67677c356716 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -179,8 +179,8 @@ struct intel_device_info {
 	int cursor_offsets[I915_MAX_PIPES];
 
 	struct color_luts {
-		u16 degamma_lut_size;
-		u16 gamma_lut_size;
+		u32 degamma_lut_size;
+		u32 gamma_lut_size;
 		u32 degamma_lut_tests;
 		u32 gamma_lut_tests;
 	} color;
-- 
2.17.1

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

[PATCH 2/3] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Shashank Sharma]

From: Uma Shankar <uma.shankar@intel.com>

Add macros to define multi segmented gamma registers

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b74824f0b5b1..fc50e85ca895 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7198,6 +7198,7 @@ enum {
 #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
 #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
 #define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
+#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0)
 
 /* DMC/CSR */
 #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
@@ -10144,6 +10145,22 @@ enum skl_power_gate {
 #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
 #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
 
+/* Add registers for Gen11 Multi Segmented Gamma Mode */
+#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
+#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
+#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		BIT(15)
+#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	(0x1f << 0)
+
+#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
+#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
+
+#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
+					_PAL_PREC_MULTI_SEG_INDEX_A, \
+					_PAL_PREC_MULTI_SEG_INDEX_B)
+#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
+					_PAL_PREC_MULTI_SEG_DATA_A, \
+					_PAL_PREC_MULTI_SEG_DATA_B)
+
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
 #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
-- 
2.17.1

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

Re: [PATCH 2/3] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Ville Syrjälä]

On Fri, Apr 26, 2019 at 11:31:50PM +0530, Shashank Sharma wrote:
> From: Uma Shankar <uma.shankar@intel.com>
> 
> Add macros to define multi segmented gamma registers
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b74824f0b5b1..fc50e85ca895 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7198,6 +7198,7 @@ enum {
>  #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
>  #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>  #define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
 + /* ivb-bdw */
> +#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0)
 + /* icl+ */

So people don't get super confused about the conflicting values.

>  
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
> @@ -10144,6 +10145,22 @@ enum skl_power_gate {
>  #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>  #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>  
> +/* Add registers for Gen11 Multi Segmented Gamma Mode */
> +#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
> +#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
> +#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		BIT(15)
> +#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	(0x1f << 0)
> +
> +#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
> +#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
> +
> +#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
> +					_PAL_PREC_MULTI_SEG_INDEX_A, \
> +					_PAL_PREC_MULTI_SEG_INDEX_B)
> +#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
> +					_PAL_PREC_MULTI_SEG_DATA_A, \
> +					_PAL_PREC_MULTI_SEG_DATA_B)
> +
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Sharma, Shashank]

On 4/26/2019 11:46 PM, Ville Syrjälä wrote:
> On Fri, Apr 26, 2019 at 11:31:50PM +0530, Shashank Sharma wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>>
>> Add macros to define multi segmented gamma registers
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index b74824f0b5b1..fc50e85ca895 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7198,6 +7198,7 @@ enum {
>>   #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
>>   #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>>   #define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
>   + /* ivb-bdw */
>> +#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0)
>   + /* icl+ */
>
> So people don't get super confused about the conflicting values.

- Sure.

Regards

Shashank

>>   
>>   /* DMC/CSR */
>>   #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>> @@ -10144,6 +10145,22 @@ enum skl_power_gate {
>>   #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>>   #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>>   
>> +/* Add registers for Gen11 Multi Segmented Gamma Mode */
>> +#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
>> +#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
>> +#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		BIT(15)
>> +#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	(0x1f << 0)
>> +
>> +#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
>> +#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
>> +
>> +#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>> +					_PAL_PREC_MULTI_SEG_INDEX_A, \
>> +					_PAL_PREC_MULTI_SEG_INDEX_B)
>> +#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
>> +					_PAL_PREC_MULTI_SEG_DATA_A, \
>> +					_PAL_PREC_MULTI_SEG_DATA_B)
>> +
>>   /* pipe CSC & degamma/gamma LUTs on CHV */
>>   #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>>   #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
>> -- 
>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Jani Nikula]

On Fri, 26 Apr 2019, Shashank Sharma <shashank.sharma@intel.com> wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> Add macros to define multi segmented gamma registers
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b74824f0b5b1..fc50e85ca895 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7198,6 +7198,7 @@ enum {
>  #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
>  #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>  #define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
> +#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0)
>  
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
> @@ -10144,6 +10145,22 @@ enum skl_power_gate {
>  #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>  #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>  
> +/* Add registers for Gen11 Multi Segmented Gamma Mode */
> +#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
> +#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
> +#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		BIT(15)
> +#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	(0x1f << 0)

See REG_BIT() and REG_FIELD_PREP() at the top of the file.

BR,
Jani.


> +
> +#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
> +#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
> +
> +#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
> +					_PAL_PREC_MULTI_SEG_INDEX_A, \
> +					_PAL_PREC_MULTI_SEG_INDEX_B)
> +#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
> +					_PAL_PREC_MULTI_SEG_DATA_A, \
> +					_PAL_PREC_MULTI_SEG_DATA_B)
> +
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Sharma, Shashank]

On 4/29/2019 7:42 PM, Jani Nikula wrote:
> On Fri, 26 Apr 2019, Shashank Sharma <shashank.sharma@intel.com> wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>>
>> Add macros to define multi segmented gamma registers
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index b74824f0b5b1..fc50e85ca895 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7198,6 +7198,7 @@ enum {
>>   #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
>>   #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>>   #define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
>> +#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0)
>>   
>>   /* DMC/CSR */
>>   #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>> @@ -10144,6 +10145,22 @@ enum skl_power_gate {
>>   #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
>>   #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
>>   
>> +/* Add registers for Gen11 Multi Segmented Gamma Mode */
>> +#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
>> +#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
>> +#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		BIT(15)
>> +#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	(0x1f << 0)
> See REG_BIT() and REG_FIELD_PREP() at the top of the file.
>
> BR,
> Jani.

Got it Jani, Thanks !

Regards

Shashank

>
>> +
>> +#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
>> +#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
>> +
>> +#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>> +					_PAL_PREC_MULTI_SEG_INDEX_A, \
>> +					_PAL_PREC_MULTI_SEG_INDEX_B)
>> +#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
>> +					_PAL_PREC_MULTI_SEG_DATA_A, \
>> +					_PAL_PREC_MULTI_SEG_DATA_B)
>> +
>>   /* pipe CSC & degamma/gamma LUTs on CHV */
>>   #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>>   #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/icl: Add Multi-segmented gamma support

[Shashank Sharma]

ICL introduces a new gamma correction mode in display engine, called
multi-segmented-gamma mode. This mode allows users to program the
darker region of the gamma curve with sueprfine precision. An
example use case for this is HDR curves (like PQ ST-2084).

If we plot a gamma correction curve from value range between 0.0 to 1.0,
ICL's multi-segment has 3 different sections:
- superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
- fine segment: 257 values, ranges between 0 - 1/(128)
- corase segment: 257 values, ranges between 0 - 1

This patch:
- Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
  so that userspace can program with highest precision supported.
- Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
- Adds functions to program/detect multi-segment gamma.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |   3 +-
 drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++-
 2 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index ffa2ee70a03d..83698951760b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = {
 	GEN(11), \
 	.ddb_size = 2048, \
 	.has_logical_ring_elsq = 1, \
-	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
+	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 }
+
 
 static const struct intel_device_info intel_icelake_11_info = {
 	GEN11_FEATURES,
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index ca341a9e47e6..d1fb79a5d764 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,7 @@
 #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
 
 #define LEGACY_LUT_LENGTH		256
+#define ICL_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
 /*
  * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
  * format). This macro takes the coefficient we want transformed and the
@@ -58,6 +59,12 @@
 
 #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
 
+enum icl_ms_gamma_segments {
+	ICL_MS_GAMMA_SEG_SUPERFINE,
+	ICL_MS_GAMMA_SEG_FINE,
+	ICL_MS_GAMMA_SEG_COARSE,
+};
+
 static const u16 ilk_csc_off_zero[3] = {};
 
 static const u16 ilk_csc_coeff_identity[9] = {
@@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	}
 }
 
+/* ilk+ "12.4" interpolated format (high 10 bits) */
+static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
+{
+	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
+		(color->blue >> 6);
+}
+
+/* ilk+ "12.4" interpolated format (low 6 bits) */
+static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
+{
+	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
+		(color->blue & 0x3f);
+}
+
+static void
+icl_program_gamma_gcmax(const 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;
+
+	/*
+	 * Program the max register to clamp values > 1.0.
+	 * ToDo: Extend the ABI to be able to program values
+	 * from 1.0
+	 */
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));
+
+	/*
+	 * Program the max register to clamp values > 1.0.
+	 * ToDo: Extend the ABI to be able to program values
+	 * from 1.0 to 3.0
+	 */
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
+
+	/*
+	 * Program the gc max 2 register to clamp values > 1.0.
+	 * ToDo: Extend the ABI to be able to program values
+	 * from 3.0 to 7.0
+	 */
+	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
+	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
+	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
+}
+
+static void
+icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state,
+				const struct drm_property_blob *blob,
+				enum icl_ms_gamma_segments segment)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut = blob->data;
+	enum pipe pipe = crtc->pipe;
+	u32 i, word, seg_size, seg_step, seg_start = 0;
+	i915_reg_t index_reg, data_reg;
+
+	if (!lut || (drm_color_lut_size(blob) < ICL_MULTISEG_LUT_LENGTH)) {
+		DRM_DEBUG_KMS("Not programming gamma segment, invalid input\n");
+		return;
+	}
+
+	/*
+	 * Every entry in the multi-segment LUT is corresponding to a superfine
+	 * segment step which is 1/(8*128*256).
+	 *
+	 * Superfine segment has 9 entries, corresponding to values
+	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
+	 *
+	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
+	 * ... 256/(128*256). So in order to program fine segment of LUT we
+	 * need to pick every 8'th entry in LUT, and program 256 indexes.
+	 * Fine segment's index 0 is programmed in HW, and it starts from
+	 * index 1.
+	 *
+	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
+	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
+	 * above, we need to pick every 8 * 128 = 1024th entry in LUT, and
+	 * program 256 of those.
+	 */
+
+	switch (segment) {
+	case ICL_MS_GAMMA_SEG_SUPERFINE:
+		seg_size = 9;
+		seg_step = 1;
+		index_reg = PREC_PAL_MULTI_SEG_INDEX(pipe);
+		data_reg = PREC_PAL_MULTI_SEG_DATA(pipe);
+		break;
+
+	case ICL_MS_GAMMA_SEG_FINE:
+		seg_start = 1;
+		seg_size = 256;
+		seg_step = 8;
+		index_reg = PREC_PAL_INDEX(pipe);
+		data_reg = PREC_PAL_DATA(pipe);
+		break;
+
+	case ICL_MS_GAMMA_SEG_COARSE:
+		seg_size = 256;
+		seg_step = 1024;
+		index_reg = PREC_PAL_INDEX(pipe);
+		data_reg = PREC_PAL_DATA(pipe);
+		break;
+
+	default:
+		DRM_ERROR("Invalid gamma segment %d to program\n", segment);
+		return;
+	}
+
+	I915_WRITE(index_reg, PAL_PREC_AUTO_INCREMENT);
+
+	for (i = seg_start; i < seg_size; i++) {
+		/* Even Index */
+		word = ilk_lut_12p4_udw(&lut[i * seg_step]);
+		I915_WRITE(data_reg, word);
+
+		/* Odd index */
+		word = ilk_lut_12p4_ldw(&lut[i * seg_step]);
+		I915_WRITE(data_reg, word);
+	}
+}
+
+static void
+icl_load_gamma_multi_seg_lut(const struct intel_crtc_state *crtc_state)
+{
+	const struct drm_property_blob *lut_blob = crtc_state->base.gamma_lut;
+
+	if (!lut_blob)
+		return;
+
+	icl_program_gamma_multi_segment(crtc_state, lut_blob,
+					ICL_MS_GAMMA_SEG_SUPERFINE);
+	icl_program_gamma_multi_segment(crtc_state, lut_blob,
+					ICL_MS_GAMMA_SEG_FINE);
+	icl_program_gamma_multi_segment(crtc_state, lut_blob,
+					ICL_MS_GAMMA_SEG_COARSE);
+	icl_program_gamma_gcmax(crtc_state);
+}
+
 static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
@@ -776,6 +926,9 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 		glk_load_degamma_lut(crtc_state);
 
 	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+		GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED) {
+		icl_load_gamma_multi_seg_lut(crtc_state);
+	} else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
 	    GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
 	} else {
@@ -1187,7 +1340,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
 	    crtc_state_is_legacy_gamma(crtc_state))
 		gamma_mode |= GAMMA_MODE_MODE_8BIT;
 	else
-		gamma_mode |= GAMMA_MODE_MODE_10BIT;
+		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
 
 	return gamma_mode;
 }
-- 
2.17.1

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

Re: [PATCH 3/3] drm/i915/icl: Add Multi-segmented gamma support

[Ville Syrjälä]

On Fri, Apr 26, 2019 at 11:31:51PM +0530, Shashank Sharma wrote:
> ICL introduces a new gamma correction mode in display engine, called
> multi-segmented-gamma mode. This mode allows users to program the
> darker region of the gamma curve with sueprfine precision. An
> example use case for this is HDR curves (like PQ ST-2084).
> 
> If we plot a gamma correction curve from value range between 0.0 to 1.0,
> ICL's multi-segment has 3 different sections:
> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
> - fine segment: 257 values, ranges between 0 - 1/(128)
> - corase segment: 257 values, ranges between 0 - 1
> 
> This patch:
> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
>   so that userspace can program with highest precision supported.
> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
> - Adds functions to program/detect multi-segment gamma.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c    |   3 +-
>  drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++-
>  2 files changed, 156 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index ffa2ee70a03d..83698951760b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = {
>  	GEN(11), \
>  	.ddb_size = 2048, \
>  	.has_logical_ring_elsq = 1, \
> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 }
> +
>  
>  static const struct intel_device_info intel_icelake_11_info = {
>  	GEN11_FEATURES,
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index ca341a9e47e6..d1fb79a5d764 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,7 @@
>  #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
>  
>  #define LEGACY_LUT_LENGTH		256
> +#define ICL_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -58,6 +59,12 @@
>  
>  #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
>  
> +enum icl_ms_gamma_segments {
> +	ICL_MS_GAMMA_SEG_SUPERFINE,
> +	ICL_MS_GAMMA_SEG_FINE,
> +	ICL_MS_GAMMA_SEG_COARSE,
> +};
> +
>  static const u16 ilk_csc_off_zero[3] = {};
>  
>  static const u16 ilk_csc_coeff_identity[9] = {
> @@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> +/* ilk+ "12.4" interpolated format (high 10 bits) */
> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
> +{
> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
> +		(color->blue >> 6);
> +}
> +
> +/* ilk+ "12.4" interpolated format (low 6 bits) */
> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
> +{
> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
> +		(color->blue & 0x3f);
> +}
> +
> +static void
> +icl_program_gamma_gcmax(const 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;
> +
> +	/*
> +	 * Program the max register to clamp values > 1.0.
> +	 * ToDo: Extend the ABI to be able to program values
> +	 * from 1.0
> +	 */
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));

This one I think we want to program based on the provide LUT. It's the
last entry that still gets used in interpolation for <1.0 values.
Or at least that's the way it works with the 12p4 mode IIRC. I don't
actually remember how it goes with the multi segment mode.

> +
> +	/*
> +	 * Program the max register to clamp values > 1.0.
> +	 * ToDo: Extend the ABI to be able to program values
> +	 * from 1.0 to 3.0
> +	 */
> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
> +
> +	/*
> +	 * Program the gc max 2 register to clamp values > 1.0.
> +	 * ToDo: Extend the ABI to be able to program values
> +	 * from 3.0 to 7.0
> +	 */
> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));

And these are just ivb_load_lut_10_max(). Might need to just
s/10/ext/ in the name since I guess they apply to the 12p4 and
multi segmment modes too.

> +}
> +
> +static void
> +icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state,
> +				const struct drm_property_blob *blob,
> +				enum icl_ms_gamma_segments segment)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct drm_color_lut *lut = blob->data;
> +	enum pipe pipe = crtc->pipe;
> +	u32 i, word, seg_size, seg_step, seg_start = 0;
> +	i915_reg_t index_reg, data_reg;
> +
> +	if (!lut || (drm_color_lut_size(blob) < ICL_MULTISEG_LUT_LENGTH)) {
> +		DRM_DEBUG_KMS("Not programming gamma segment, invalid input\n");
> +		return;
> +	}
> +
> +	/*
> +	 * Every entry in the multi-segment LUT is corresponding to a superfine
> +	 * segment step which is 1/(8*128*256).
> +	 *
> +	 * Superfine segment has 9 entries, corresponding to values
> +	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
> +	 *
> +	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
> +	 * ... 256/(128*256). So in order to program fine segment of LUT we
> +	 * need to pick every 8'th entry in LUT, and program 256 indexes.
> +	 * Fine segment's index 0 is programmed in HW, and it starts from
> +	 * index 1.
> +	 *
> +	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
> +	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
> +	 * above, we need to pick every 8 * 128 = 1024th entry in LUT, and
> +	 * program 256 of those.
> +	 */
> +
> +	switch (segment) {
> +	case ICL_MS_GAMMA_SEG_SUPERFINE:
> +		seg_size = 9;
> +		seg_step = 1;
> +		index_reg = PREC_PAL_MULTI_SEG_INDEX(pipe);
> +		data_reg = PREC_PAL_MULTI_SEG_DATA(pipe);
> +		break;
> +
> +	case ICL_MS_GAMMA_SEG_FINE:
> +		seg_start = 1;
> +		seg_size = 256;
> +		seg_step = 8;
> +		index_reg = PREC_PAL_INDEX(pipe);
> +		data_reg = PREC_PAL_DATA(pipe);
> +		break;
> +
> +	case ICL_MS_GAMMA_SEG_COARSE:
> +		seg_size = 256;
> +		seg_step = 1024;
> +		index_reg = PREC_PAL_INDEX(pipe);
> +		data_reg = PREC_PAL_DATA(pipe);
> +		break;

I'm not convinced this this enum is actually helpful. I would
probably just try to provide separate functions for the segments.
Not sure if we could share the same code for the fine/coarse
segments and just pass in the differences as parameters.

> +
> +	default:
> +		DRM_ERROR("Invalid gamma segment %d to program\n", segment);
> +		return;
> +	}
> +
> +	I915_WRITE(index_reg, PAL_PREC_AUTO_INCREMENT);
> +
> +	for (i = seg_start; i < seg_size; i++) {
> +		/* Even Index */

These comments don't really provide any value.

> +		word = ilk_lut_12p4_udw(&lut[i * seg_step]);

and the 'word' variable isn't particularly useful either.
We might want a local varianble for the lut entry like we
have in the ivb/bdw functions, if only to avoid repeating the
arithmetic.

> +		I915_WRITE(data_reg, word);
> +
> +		/* Odd index */
> +		word = ilk_lut_12p4_ldw(&lut[i * seg_step]);
> +		I915_WRITE(data_reg, word);
> +	}
> +}
> +
> +static void
> +icl_load_gamma_multi_seg_lut(const struct intel_crtc_state *crtc_state)
> +{
> +	const struct drm_property_blob *lut_blob = crtc_state->base.gamma_lut;
> +
> +	if (!lut_blob)
> +		return;
> +
> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
> +					ICL_MS_GAMMA_SEG_SUPERFINE);
> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
> +					ICL_MS_GAMMA_SEG_FINE);
> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
> +					ICL_MS_GAMMA_SEG_COARSE);
> +	icl_program_gamma_gcmax(crtc_state);
> +}

And this I would just inline into icl_load_luts() just like the rest.

> +
>  static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> @@ -776,6 +926,9 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  		glk_load_degamma_lut(crtc_state);
>  
>  	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
> +		GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED) {
> +		icl_load_gamma_multi_seg_lut(crtc_state);

I'd put this last to keep things in consistent order. And probably want
to use a switch statement now that we have three options.

> +	} else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>  	    GAMMA_MODE_MODE_8BIT) {
>  		i9xx_load_luts(crtc_state);
>  	} else {
> @@ -1187,7 +1340,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
>  	    crtc_state_is_legacy_gamma(crtc_state))
>  		gamma_mode |= GAMMA_MODE_MODE_8BIT;
>  	else
> -		gamma_mode |= GAMMA_MODE_MODE_10BIT;
> +		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>  
>  	return gamma_mode;
>  }
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/icl: Add Multi-segmented gamma support

[Sharma, Shashank]

On 4/26/2019 11:59 PM, Ville Syrjälä wrote:
> On Fri, Apr 26, 2019 at 11:31:51PM +0530, Shashank Sharma wrote:
>> ICL introduces a new gamma correction mode in display engine, called
>> multi-segmented-gamma mode. This mode allows users to program the
>> darker region of the gamma curve with sueprfine precision. An
>> example use case for this is HDR curves (like PQ ST-2084).
>>
>> If we plot a gamma correction curve from value range between 0.0 to 1.0,
>> ICL's multi-segment has 3 different sections:
>> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
>> - fine segment: 257 values, ranges between 0 - 1/(128)
>> - corase segment: 257 values, ranges between 0 - 1
>>
>> This patch:
>> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
>>    so that userspace can program with highest precision supported.
>> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
>> - Adds functions to program/detect multi-segment gamma.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_pci.c    |   3 +-
>>   drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++-
>>   2 files changed, 156 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index ffa2ee70a03d..83698951760b 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = {
>>   	GEN(11), \
>>   	.ddb_size = 2048, \
>>   	.has_logical_ring_elsq = 1, \
>> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 }
>> +
>>   
>>   static const struct intel_device_info intel_icelake_11_info = {
>>   	GEN11_FEATURES,
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index ca341a9e47e6..d1fb79a5d764 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -41,6 +41,7 @@
>>   #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
>>   
>>   #define LEGACY_LUT_LENGTH		256
>> +#define ICL_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
>>   /*
>>    * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>    * format). This macro takes the coefficient we want transformed and the
>> @@ -58,6 +59,12 @@
>>   
>>   #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
>>   
>> +enum icl_ms_gamma_segments {
>> +	ICL_MS_GAMMA_SEG_SUPERFINE,
>> +	ICL_MS_GAMMA_SEG_FINE,
>> +	ICL_MS_GAMMA_SEG_COARSE,
>> +};
>> +
>>   static const u16 ilk_csc_off_zero[3] = {};
>>   
>>   static const u16 ilk_csc_coeff_identity[9] = {
>> @@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>>   	}
>>   }
>>   
>> +/* ilk+ "12.4" interpolated format (high 10 bits) */
>> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
>> +{
>> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
>> +		(color->blue >> 6);
>> +}
>> +
>> +/* ilk+ "12.4" interpolated format (low 6 bits) */
>> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
>> +{
>> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
>> +		(color->blue & 0x3f);
>> +}
>> +
>> +static void
>> +icl_program_gamma_gcmax(const 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;
>> +
>> +	/*
>> +	 * Program the max register to clamp values > 1.0.
>> +	 * ToDo: Extend the ABI to be able to program values
>> +	 * from 1.0
>> +	 */
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));
> This one I think we want to program based on the provide LUT. It's the
> last entry that still gets used in interpolation for <1.0 values.
> Or at least that's the way it works with the 12p4 mode IIRC. I don't
> actually remember how it goes with the multi segment mode.
I am a bit skeptical if its a good idea to program from the LUT, as you 
know, the drm_color_lut elements are 16bit per channel, and in order to 
program 1.0 we need 17 bits. So even if userspace wants, it cant set the 
value of 1.0 as of now. Also, wouldn't it would be good for the 
interpolation to have the accurate max value (1.0) of the curve ?
>> +
>> +	/*
>> +	 * Program the max register to clamp values > 1.0.
>> +	 * ToDo: Extend the ABI to be able to program values
>> +	 * from 1.0 to 3.0
>> +	 */
>> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
>> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
>> +	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
>> +
>> +	/*
>> +	 * Program the gc max 2 register to clamp values > 1.0.
>> +	 * ToDo: Extend the ABI to be able to program values
>> +	 * from 3.0 to 7.0
>> +	 */
>> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
>> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
>> +	I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
> And these are just ivb_load_lut_10_max(). Might need to just
> s/10/ext/ in the name since I guess they apply to the 12p4 and
> multi segmment modes too.
Got it !
>
>> +}
>> +
>> +static void
>> +icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state,
>> +				const struct drm_property_blob *blob,
>> +				enum icl_ms_gamma_segments segment)
>> +{
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	const struct drm_color_lut *lut = blob->data;
>> +	enum pipe pipe = crtc->pipe;
>> +	u32 i, word, seg_size, seg_step, seg_start = 0;
>> +	i915_reg_t index_reg, data_reg;
>> +
>> +	if (!lut || (drm_color_lut_size(blob) < ICL_MULTISEG_LUT_LENGTH)) {
>> +		DRM_DEBUG_KMS("Not programming gamma segment, invalid input\n");
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * Every entry in the multi-segment LUT is corresponding to a superfine
>> +	 * segment step which is 1/(8*128*256).
>> +	 *
>> +	 * Superfine segment has 9 entries, corresponding to values
>> +	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
>> +	 *
>> +	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
>> +	 * ... 256/(128*256). So in order to program fine segment of LUT we
>> +	 * need to pick every 8'th entry in LUT, and program 256 indexes.
>> +	 * Fine segment's index 0 is programmed in HW, and it starts from
>> +	 * index 1.
>> +	 *
>> +	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
>> +	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
>> +	 * above, we need to pick every 8 * 128 = 1024th entry in LUT, and
>> +	 * program 256 of those.
>> +	 */
>> +
>> +	switch (segment) {
>> +	case ICL_MS_GAMMA_SEG_SUPERFINE:
>> +		seg_size = 9;
>> +		seg_step = 1;
>> +		index_reg = PREC_PAL_MULTI_SEG_INDEX(pipe);
>> +		data_reg = PREC_PAL_MULTI_SEG_DATA(pipe);
>> +		break;
>> +
>> +	case ICL_MS_GAMMA_SEG_FINE:
>> +		seg_start = 1;
>> +		seg_size = 256;
>> +		seg_step = 8;
>> +		index_reg = PREC_PAL_INDEX(pipe);
>> +		data_reg = PREC_PAL_DATA(pipe);
>> +		break;
>> +
>> +	case ICL_MS_GAMMA_SEG_COARSE:
>> +		seg_size = 256;
>> +		seg_step = 1024;
>> +		index_reg = PREC_PAL_INDEX(pipe);
>> +		data_reg = PREC_PAL_DATA(pipe);
>> +		break;
> I'm not convinced this this enum is actually helpful. I would
> probably just try to provide separate functions for the segments.
> Not sure if we could share the same code for the fine/coarse
> segments and just pass in the differences as parameters.
Ah, I started just like that, having two different functions, one for 
fine + coarse segment, other one for superfile segment, and then I saw 
much of code duplication, so I changed it this way. But if you think it 
would have been better that way, I can switch to the previous version.
>> +
>> +	default:
>> +		DRM_ERROR("Invalid gamma segment %d to program\n", segment);
>> +		return;
>> +	}
>> +
>> +	I915_WRITE(index_reg, PAL_PREC_AUTO_INCREMENT);
>> +
>> +	for (i = seg_start; i < seg_size; i++) {
>> +		/* Even Index */
> These comments don't really provide any value.
Ok
>
>> +		word = ilk_lut_12p4_udw(&lut[i * seg_step]);
> and the 'word' variable isn't particularly useful either.
> We might want a local varianble for the lut entry like we
> have in the ivb/bdw functions, if only to avoid repeating the
> arithmetic.
Sure, will do that.
>> +		I915_WRITE(data_reg, word);
>> +
>> +		/* Odd index */
>> +		word = ilk_lut_12p4_ldw(&lut[i * seg_step]);
>> +		I915_WRITE(data_reg, word);
>> +	}
>> +}
>> +
>> +static void
>> +icl_load_gamma_multi_seg_lut(const struct intel_crtc_state *crtc_state)
>> +{
>> +	const struct drm_property_blob *lut_blob = crtc_state->base.gamma_lut;
>> +
>> +	if (!lut_blob)
>> +		return;
>> +
>> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
>> +					ICL_MS_GAMMA_SEG_SUPERFINE);
>> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
>> +					ICL_MS_GAMMA_SEG_FINE);
>> +	icl_program_gamma_multi_segment(crtc_state, lut_blob,
>> +					ICL_MS_GAMMA_SEG_COARSE);
>> +	icl_program_gamma_gcmax(crtc_state);
>> +}
> And this I would just inline into icl_load_luts() just like the rest.
Got it !
>> +
>>   static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   {
>>   	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> @@ -776,6 +926,9 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   		glk_load_degamma_lut(crtc_state);
>>   
>>   	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> +		GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED) {
>> +		icl_load_gamma_multi_seg_lut(crtc_state);
> I'd put this last to keep things in consistent order. And probably want
> to use a switch statement now that we have three options.

Sure !

- Shashank

>> +	} else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>>   	    GAMMA_MODE_MODE_8BIT) {
>>   		i9xx_load_luts(crtc_state);
>>   	} else {
>> @@ -1187,7 +1340,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
>>   	    crtc_state_is_legacy_gamma(crtc_state))
>>   		gamma_mode |= GAMMA_MODE_MODE_8BIT;
>>   	else
>> -		gamma_mode |= GAMMA_MODE_MODE_10BIT;
>> +		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>>   
>>   	return gamma_mode;
>>   }
>> -- 
>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/icl: Add Multi-segmented gamma support

[Ville Syrjälä]

On Mon, Apr 29, 2019 at 05:00:21PM +0530, Sharma, Shashank wrote:
> 
> On 4/26/2019 11:59 PM, Ville Syrjälä wrote:
> > On Fri, Apr 26, 2019 at 11:31:51PM +0530, Shashank Sharma wrote:
> >> ICL introduces a new gamma correction mode in display engine, called
> >> multi-segmented-gamma mode. This mode allows users to program the
> >> darker region of the gamma curve with sueprfine precision. An
> >> example use case for this is HDR curves (like PQ ST-2084).
> >>
> >> If we plot a gamma correction curve from value range between 0.0 to 1.0,
> >> ICL's multi-segment has 3 different sections:
> >> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
> >> - fine segment: 257 values, ranges between 0 - 1/(128)
> >> - corase segment: 257 values, ranges between 0 - 1
> >>
> >> This patch:
> >> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
> >>    so that userspace can program with highest precision supported.
> >> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
> >> - Adds functions to program/detect multi-segment gamma.
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>
> >> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_pci.c    |   3 +-
> >>   drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++-
> >>   2 files changed, 156 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> index ffa2ee70a03d..83698951760b 100644
> >> --- a/drivers/gpu/drm/i915/i915_pci.c
> >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> @@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = {
> >>   	GEN(11), \
> >>   	.ddb_size = 2048, \
> >>   	.has_logical_ring_elsq = 1, \
> >> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
> >> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 }
> >> +
> >>   
> >>   static const struct intel_device_info intel_icelake_11_info = {
> >>   	GEN11_FEATURES,
> >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> >> index ca341a9e47e6..d1fb79a5d764 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -41,6 +41,7 @@
> >>   #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
> >>   
> >>   #define LEGACY_LUT_LENGTH		256
> >> +#define ICL_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
> >>   /*
> >>    * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
> >>    * format). This macro takes the coefficient we want transformed and the
> >> @@ -58,6 +59,12 @@
> >>   
> >>   #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
> >>   
> >> +enum icl_ms_gamma_segments {
> >> +	ICL_MS_GAMMA_SEG_SUPERFINE,
> >> +	ICL_MS_GAMMA_SEG_FINE,
> >> +	ICL_MS_GAMMA_SEG_COARSE,
> >> +};
> >> +
> >>   static const u16 ilk_csc_off_zero[3] = {};
> >>   
> >>   static const u16 ilk_csc_coeff_identity[9] = {
> >> @@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
> >>   	}
> >>   }
> >>   
> >> +/* ilk+ "12.4" interpolated format (high 10 bits) */
> >> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
> >> +{
> >> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
> >> +		(color->blue >> 6);
> >> +}
> >> +
> >> +/* ilk+ "12.4" interpolated format (low 6 bits) */
> >> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
> >> +{
> >> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
> >> +		(color->blue & 0x3f);
> >> +}
> >> +
> >> +static void
> >> +icl_program_gamma_gcmax(const 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;
> >> +
> >> +	/*
> >> +	 * Program the max register to clamp values > 1.0.
> >> +	 * ToDo: Extend the ABI to be able to program values
> >> +	 * from 1.0
> >> +	 */
> >> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
> >> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
> >> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));
> > This one I think we want to program based on the provide LUT. It's the
> > last entry that still gets used in interpolation for <1.0 values.
> > Or at least that's the way it works with the 12p4 mode IIRC. I don't
> > actually remember how it goes with the multi segment mode.
> I am a bit skeptical if its a good idea to program from the LUT, as you 
> know, the drm_color_lut elements are 16bit per channel, and in order to 
> program 1.0 we need 17 bits. So even if userspace wants, it cant set the 
> value of 1.0 as of now. Also, wouldn't it would be good for the 
> interpolation to have the accurate max value (1.0) of the curve ?

If userspace doesn't want the max to be 1.0 then we will do the wrong
thing here. But I suppose that wouldn't be a very common thing to do.
The better arguemnt for doing what I suggest is that we already do it
like that on the other platforms with interpolated gamma (gen4/vlv/chv).

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/icl: Add Multi-segmented gamma support

[Sharma, Shashank]

On 4/29/2019 5:19 PM, Ville Syrjälä wrote:
> On Mon, Apr 29, 2019 at 05:00:21PM +0530, Sharma, Shashank wrote:
>> On 4/26/2019 11:59 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 26, 2019 at 11:31:51PM +0530, Shashank Sharma wrote:
>>>> ICL introduces a new gamma correction mode in display engine, called
>>>> multi-segmented-gamma mode. This mode allows users to program the
>>>> darker region of the gamma curve with sueprfine precision. An
>>>> example use case for this is HDR curves (like PQ ST-2084).
>>>>
>>>> If we plot a gamma correction curve from value range between 0.0 to 1.0,
>>>> ICL's multi-segment has 3 different sections:
>>>> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
>>>> - fine segment: 257 values, ranges between 0 - 1/(128)
>>>> - corase segment: 257 values, ranges between 0 - 1
>>>>
>>>> This patch:
>>>> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
>>>>     so that userspace can program with highest precision supported.
>>>> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
>>>> - Adds functions to program/detect multi-segment gamma.
>>>>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>
>>>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_pci.c    |   3 +-
>>>>    drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++-
>>>>    2 files changed, 156 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>>>> index ffa2ee70a03d..83698951760b 100644
>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>> @@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = {
>>>>    	GEN(11), \
>>>>    	.ddb_size = 2048, \
>>>>    	.has_logical_ring_elsq = 1, \
>>>> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>>>> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 }
>>>> +
>>>>    
>>>>    static const struct intel_device_info intel_icelake_11_info = {
>>>>    	GEN11_FEATURES,
>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>>> index ca341a9e47e6..d1fb79a5d764 100644
>>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>>> @@ -41,6 +41,7 @@
>>>>    #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
>>>>    
>>>>    #define LEGACY_LUT_LENGTH		256
>>>> +#define ICL_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
>>>>    /*
>>>>     * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>>>     * format). This macro takes the coefficient we want transformed and the
>>>> @@ -58,6 +59,12 @@
>>>>    
>>>>    #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
>>>>    
>>>> +enum icl_ms_gamma_segments {
>>>> +	ICL_MS_GAMMA_SEG_SUPERFINE,
>>>> +	ICL_MS_GAMMA_SEG_FINE,
>>>> +	ICL_MS_GAMMA_SEG_COARSE,
>>>> +};
>>>> +
>>>>    static const u16 ilk_csc_off_zero[3] = {};
>>>>    
>>>>    static const u16 ilk_csc_coeff_identity[9] = {
>>>> @@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>>>>    	}
>>>>    }
>>>>    
>>>> +/* ilk+ "12.4" interpolated format (high 10 bits) */
>>>> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
>>>> +{
>>>> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
>>>> +		(color->blue >> 6);
>>>> +}
>>>> +
>>>> +/* ilk+ "12.4" interpolated format (low 6 bits) */
>>>> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
>>>> +{
>>>> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
>>>> +		(color->blue & 0x3f);
>>>> +}
>>>> +
>>>> +static void
>>>> +icl_program_gamma_gcmax(const 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;
>>>> +
>>>> +	/*
>>>> +	 * Program the max register to clamp values > 1.0.
>>>> +	 * ToDo: Extend the ABI to be able to program values
>>>> +	 * from 1.0
>>>> +	 */
>>>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
>>>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
>>>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));
>>> This one I think we want to program based on the provide LUT. It's the
>>> last entry that still gets used in interpolation for <1.0 values.
>>> Or at least that's the way it works with the 12p4 mode IIRC. I don't
>>> actually remember how it goes with the multi segment mode.
>> I am a bit skeptical if its a good idea to program from the LUT, as you
>> know, the drm_color_lut elements are 16bit per channel, and in order to
>> program 1.0 we need 17 bits. So even if userspace wants, it cant set the
>> value of 1.0 as of now. Also, wouldn't it would be good for the
>> interpolation to have the accurate max value (1.0) of the curve ?
> If userspace doesn't want the max to be 1.0 then we will do the wrong
> thing here. But I suppose that wouldn't be a very common thing to do.
My point exactly. Most of the time userspace wants to apply a known curve.
> The better arguemnt for doing what I suggest is that we already do it
> like that on the other platforms with interpolated gamma (gen4/vlv/chv).

Ok, lets keep it the last value of the LUT then.

- Shashank

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

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Change gamma/degamma_lut_size data type to u32

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Change gamma/degamma_lut_size data type to u32
URL   : https://patchwork.freedesktop.org/series/60007/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6007 -> Patchwork_12886
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/60007/revisions/1/mbox/


Changes
-------

  No changes found


Participating hosts (38 -> 21)
------------------------------

  Additional (1): fi-kbl-guc 
  Missing    (18): fi-ilk-m540 fi-bxt-dsi fi-skl-gvtdvm fi-byt-j1900 fi-skl-6770hq fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-snb-2520m fi-kbl-7500u fi-kbl-x1275 fi-bxt-j4205 fi-cfl-8109u fi-bsw-kefka fi-skl-lmem fi-blb-e6850 fi-byt-n2820 fi-byt-clapper 


Build changes
-------------

  * Linux: CI_DRM_6007 -> Patchwork_12886

  CI_DRM_6007: 846376257e91f6e49cf7d6b59b0a6cbb0ce7cd53 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4968: caed251990f35bfe45368f803980071a73e36315 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12886: 134f19eee2761d15bf9502f8589384131f232328 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

134f19eee276 drm/i915/icl: Add Multi-segmented gamma support
9b3b15815c7b drm/i915/icl: Add register definitions for Multi Segmented gamma
4e85a5a0c432 drm/i915: Change gamma/degamma_lut_size data type to u32

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915: Change gamma/degamma_lut_size data type to u32

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Change gamma/degamma_lut_size data type to u32
URL   : https://patchwork.freedesktop.org/series/60007/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6007_full -> Patchwork_12886_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12886_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12886_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12886_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl6/igt@i915_selftest@mock_requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl2/igt@i915_selftest@mock_requests.html

  * igt@kms_color@pipe-a-ctm-0-5:
    - shard-iclb:         [PASS][3] -> [FAIL][4] +8 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb6/igt@kms_color@pipe-a-ctm-0-5.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb7/igt@kms_color@pipe-a-ctm-0-5.html

  * igt@runner@aborted:
    - shard-snb:          NOTRUN -> [FAIL][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-snb2/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_12886_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-apl:          [PASS][6] -> [DMESG-WARN][7] ([fdo#108566]) +4 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-apl6/igt@gem_ctx_isolation@bcs0-s3.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-apl6/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@kms_cursor_legacy@all-pipes-forked-bo:
    - shard-apl:          [PASS][8] -> [INCOMPLETE][9] ([fdo#103927])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-apl7/igt@kms_cursor_legacy@all-pipes-forked-bo.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-apl7/igt@kms_cursor_legacy@all-pipes-forked-bo.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [PASS][10] -> [SKIP][11] ([fdo#109349])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb3/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip@flip-vs-modeset-interruptible:
    - shard-hsw:          [PASS][12] -> [DMESG-WARN][13] ([fdo#102614])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-hsw5/igt@kms_flip@flip-vs-modeset-interruptible.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-hsw5/igt@kms_flip@flip-vs-modeset-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][14] -> [INCOMPLETE][15] ([fdo#103540])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-hsw6/igt@kms_flip@flip-vs-suspend.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-hsw8/igt@kms_flip@flip-vs-suspend.html
    - shard-skl:          [PASS][16] -> [INCOMPLETE][17] ([fdo#109507])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl9/igt@kms_flip@flip-vs-suspend.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl3/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][18] -> [FAIL][19] ([fdo#103167]) +6 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][20] -> [FAIL][21] ([fdo#103166])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [PASS][22] -> [SKIP][23] ([fdo#109441])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb1/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          [PASS][24] -> [FAIL][25] ([fdo#109016])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-kbl3/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-kbl6/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-skl:          [INCOMPLETE][26] ([fdo#104108] / [fdo#107773]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl6/igt@gem_eio@in-flight-suspend.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl2/igt@gem_eio@in-flight-suspend.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - shard-skl:          [INCOMPLETE][28] ([fdo#107807]) -> [PASS][29] +1 similar issue
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl1/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl8/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-iclb:         [FAIL][30] ([fdo#106641]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb8/igt@kms_available_modes_crc@available_mode_test_crc.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb4/igt@kms_available_modes_crc@available_mode_test_crc.html

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-snb:          [SKIP][32] ([fdo#109271] / [fdo#109278]) -> [PASS][33] +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-snb6/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-snb7/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b.html

  * igt@kms_color@pipe-a-degamma:
    - shard-iclb:         [FAIL][34] ([fdo#104782]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb4/igt@kms_color@pipe-a-degamma.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb5/igt@kms_color@pipe-a-degamma.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][36] ([fdo#105767]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][38] ([fdo#102887] / [fdo#105363]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-glk3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render:
    - shard-iclb:         [FAIL][40] ([fdo#103167]) -> [PASS][41] +3 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-glk:          [SKIP][42] ([fdo#109271]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-glk2/igt@kms_plane@pixel-format-pipe-b-planes.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-glk9/igt@kms_plane@pixel-format-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][44] ([fdo#108145] / [fdo#110403]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_cursor@pipe-b-overlay-size-256:
    - shard-snb:          [SKIP][46] ([fdo#109271]) -> [PASS][47] +5 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-snb6/igt@kms_plane_cursor@pipe-b-overlay-size-256.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-snb7/igt@kms_plane_cursor@pipe-b-overlay-size-256.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
    - shard-glk:          [SKIP][48] ([fdo#109271] / [fdo#109278]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-glk2/igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-glk9/igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][50] ([fdo#108341]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb1/igt@kms_psr@no_drrs.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb2/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [SKIP][52] ([fdo#109441]) -> [PASS][53] +3 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-iclb1/igt@kms_psr@psr2_cursor_plane_onoff.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][54] ([fdo#99912]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-apl8/igt@kms_setmode@basic.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-apl7/igt@kms_setmode@basic.html
    - shard-kbl:          [FAIL][56] ([fdo#99912]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-kbl7/igt@kms_setmode@basic.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-kbl2/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][58] ([fdo#108566]) -> [PASS][59] +1 similar issue
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-apl3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-apl5/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-skl:          [FAIL][60] ([fdo#103167]) -> [FAIL][61] ([fdo#108040])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6007/shard-skl9/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6007 -> Patchwork_12886

  CI_DRM_6007: 846376257e91f6e49cf7d6b59b0a6cbb0ce7cd53 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4968: caed251990f35bfe45368f803980071a73e36315 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12886: 134f19eee2761d15bf9502f8589384131f232328 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12886/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx