Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
On Mon, May 06, 2019 at 06:44:43PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
> On 5/6/2019 6:41 PM, Ville Syrjälä wrote:
> > On Mon, May 06, 2019 at 06:25:19PM +0530, Sharma, Shashank wrote:
> >> On 5/6/2019 5:55 PM, Ville Syrjälä wrote:
> >>> On Mon, May 06, 2019 at 04:09:33PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
> On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
> > On Tue, Apr 30, 2019 at 08:51:08PM +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.
> >>
> >> V2: Addressed review comments from Ville
> >>- separate function for superfine and fine segments.
> >>- remove enum for segments.
> >>- reuse last entry of the LUT as gc_max value.
> >>- replace if() cond with switch...case in icl_load_luts.
> >>- add an entry variable, instead of 'word'
> >>
> >> Cc: Ville Syrjälä
> >> Cc: Maarten Lankhorst
> >> Cc: Daniel Vetter
> >>
> >> Suggested-by: Ville Syrjälä
> >> Signed-off-by: Shashank Sharma
> >> Signed-off-by: Uma Shankar
> >> ---
> >> drivers/gpu/drm/i915/i915_pci.c| 3 +-
> >> drivers/gpu/drm/i915/intel_color.c | 125
> >> -
> >> 2 files changed, 123 insertions(+), 5 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 }
> > Ugh. Thats one big LUT. But looks correct.
> >
> >> +
> > Bogus newline.
> Got it.
> >>
> >> 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 6c341bea514c..49831e8d02fb 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -41,6 +41,9 @@
> >> #define CTM_COEFF_ABS(coeff) ((coeff) &
> >> (CTM_COEFF_SIGN - 1))
> >>
> >> #define LEGACY_LUT_LENGTH 256
> >> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
> >> +#define ICL_GAMMA_SUPERFINE_SEG_LENGTH9
> >> +
> >> /*
> >> * Extract the CSC coefficient from a CTM coefficient (in U32.32
> >> fixed point
> >> * format). This macro takes the coefficient we want transformed
> >> and the
> >> @@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
> >> + const struct drm_color_lut *entry)
> > Indentation looks off. Also s/entry/color/ to match the other similarish
> > funcs maybe?
> Sure.
> >> +{
>
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
Regards
Shashank
On 5/6/2019 6:41 PM, Ville Syrjälä wrote:
On Mon, May 06, 2019 at 06:25:19PM +0530, Sharma, Shashank wrote:
On 5/6/2019 5:55 PM, Ville Syrjälä wrote:
On Mon, May 06, 2019 at 04:09:33PM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
On Tue, Apr 30, 2019 at 08:51:08PM +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.
V2: Addressed review comments from Ville
- separate function for superfine and fine segments.
- remove enum for segments.
- reuse last entry of the LUT as gc_max value.
- replace if() cond with switch...case in icl_load_luts.
- add an entry variable, instead of 'word'
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Suggested-by: Ville Syrjälä
Signed-off-by: Shashank Sharma
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_pci.c| 3 +-
drivers/gpu/drm/i915/intel_color.c | 125 -
2 files changed, 123 insertions(+), 5 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 }
Ugh. Thats one big LUT. But looks correct.
+
Bogus newline.
Got it.
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 6c341bea514c..49831e8d02fb 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,9 @@
#define CTM_COEFF_ABS(coeff)((coeff) & (CTM_COEFF_SIGN - 1))
#define LEGACY_LUT_LENGTH 256
+#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
+#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
+
/*
* Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
* format). This macro takes the coefficient we want transformed and the
@@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
+ const struct drm_color_lut *entry)
Indentation looks off. Also s/entry/color/ to match the other similarish
funcs maybe?
Sure.
+{
+ 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;
+
+ /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
+}
+
+static void
+icl_program_gamma_superfine_segment(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);
+ const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+ const struct drm_color_lut *lut = blob->data;
+ enum pipe pipe = crtc->pipe;
+ u32 i;
+
+ if (!lut || drm_color_lut_size(blob) < ICL_GAMMA_SUPERFINE_SEG_LENGTH)
+ return;
These checks aren't needed. Just dead code.
Will remove this and similars.
+
+
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
On Mon, May 06, 2019 at 06:25:19PM +0530, Sharma, Shashank wrote:
>
> On 5/6/2019 5:55 PM, Ville Syrjälä wrote:
> > On Mon, May 06, 2019 at 04:09:33PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >> On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
> >>> On Tue, Apr 30, 2019 at 08:51:08PM +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.
>
> V2: Addressed review comments from Ville
> - separate function for superfine and fine segments.
> - remove enum for segments.
> - reuse last entry of the LUT as gc_max value.
> - replace if() cond with switch...case in icl_load_luts.
> - add an entry variable, instead of 'word'
>
> Cc: Ville Syrjälä
> Cc: Maarten Lankhorst
> Cc: Daniel Vetter
>
> Suggested-by: Ville Syrjälä
> Signed-off-by: Shashank Sharma
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/i915/i915_pci.c| 3 +-
> drivers/gpu/drm/i915/intel_color.c | 125 -
> 2 files changed, 123 insertions(+), 5 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 }
> >>> Ugh. Thats one big LUT. But looks correct.
> >>>
> +
> >>> Bogus newline.
> >> Got it.
>
> 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 6c341bea514c..49831e8d02fb 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,9 @@
> #define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
>
> #define LEGACY_LUT_LENGTH 256
> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
> +#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
> +
> /*
> * Extract the CSC coefficient from a CTM coefficient (in U32.32
> fixed point
> * format). This macro takes the coefficient we want transformed and
> the
> @@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
> +const struct drm_color_lut *entry)
> >>> Indentation looks off. Also s/entry/color/ to match the other similarish
> >>> funcs maybe?
> >> Sure.
> +{
> +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;
> +
> +/* Fixme: LUT entries are 16 bit only, so we can prog 0x
> max */
> +I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
> +
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
On 5/6/2019 5:55 PM, Ville Syrjälä wrote:
On Mon, May 06, 2019 at 04:09:33PM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
On Tue, Apr 30, 2019 at 08:51:08PM +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.
V2: Addressed review comments from Ville
- separate function for superfine and fine segments.
- remove enum for segments.
- reuse last entry of the LUT as gc_max value.
- replace if() cond with switch...case in icl_load_luts.
- add an entry variable, instead of 'word'
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Suggested-by: Ville Syrjälä
Signed-off-by: Shashank Sharma
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_pci.c| 3 +-
drivers/gpu/drm/i915/intel_color.c | 125 -
2 files changed, 123 insertions(+), 5 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 }
Ugh. Thats one big LUT. But looks correct.
+
Bogus newline.
Got it.
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 6c341bea514c..49831e8d02fb 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,9 @@
#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
#define LEGACY_LUT_LENGTH 256
+#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
+#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
+
/*
* Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
* format). This macro takes the coefficient we want transformed and the
@@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
+ const struct drm_color_lut *entry)
Indentation looks off. Also s/entry/color/ to match the other similarish
funcs maybe?
Sure.
+{
+ 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;
+
+ /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
+}
+
+static void
+icl_program_gamma_superfine_segment(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);
+ const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+ const struct drm_color_lut *lut = blob->data;
+ enum pipe pipe = crtc->pipe;
+ u32 i;
+
+ if (!lut || drm_color_lut_size(blob) < ICL_GAMMA_SUPERFINE_SEG_LENGTH)
+ return;
These checks aren't needed. Just dead code.
Will remove this and similars.
+
+ /*
+* Every entry in the multi-segment LUT is corresponding to a superfine
+* segment step which is 1/(8 * 128 * 256).
+*
+
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
On Mon, May 06, 2019 at 04:09:33PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
> On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
> > On Tue, Apr 30, 2019 at 08:51:08PM +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.
> >>
> >> V2: Addressed review comments from Ville
> >> - separate function for superfine and fine segments.
> >> - remove enum for segments.
> >> - reuse last entry of the LUT as gc_max value.
> >> - replace if() cond with switch...case in icl_load_luts.
> >> - add an entry variable, instead of 'word'
> >>
> >> Cc: Ville Syrjälä
> >> Cc: Maarten Lankhorst
> >> Cc: Daniel Vetter
> >>
> >> Suggested-by: Ville Syrjälä
> >> Signed-off-by: Shashank Sharma
> >> Signed-off-by: Uma Shankar
> >> ---
> >> drivers/gpu/drm/i915/i915_pci.c| 3 +-
> >> drivers/gpu/drm/i915/intel_color.c | 125 -
> >> 2 files changed, 123 insertions(+), 5 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 }
> > Ugh. Thats one big LUT. But looks correct.
> >
> >> +
> > Bogus newline.
> Got it.
> >>
> >> 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 6c341bea514c..49831e8d02fb 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -41,6 +41,9 @@
> >> #define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
> >>
> >> #define LEGACY_LUT_LENGTH256
> >> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
> >> +#define ICL_GAMMA_SUPERFINE_SEG_LENGTH9
> >> +
> >> /*
> >>* Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed
> >> point
> >>* format). This macro takes the coefficient we want transformed and the
> >> @@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
> >> + const struct drm_color_lut *entry)
> > Indentation looks off. Also s/entry/color/ to match the other similarish
> > funcs maybe?
> Sure.
> >> +{
> >> + 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;
> >> +
> >> + /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
> >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
> >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
> >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
> >> +}
> >> +
> >> +static void
> >> +icl_program_gamma_superfine_segment(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);
> >> + const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
> >> + const struct drm_color_lut *lut =
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
Regards
Shashank
On 5/3/2019 9:20 PM, Ville Syrjälä wrote:
On Tue, Apr 30, 2019 at 08:51:08PM +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.
V2: Addressed review comments from Ville
- separate function for superfine and fine segments.
- remove enum for segments.
- reuse last entry of the LUT as gc_max value.
- replace if() cond with switch...case in icl_load_luts.
- add an entry variable, instead of 'word'
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Suggested-by: Ville Syrjälä
Signed-off-by: Shashank Sharma
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_pci.c| 3 +-
drivers/gpu/drm/i915/intel_color.c | 125 -
2 files changed, 123 insertions(+), 5 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 }
Ugh. Thats one big LUT. But looks correct.
+
Bogus newline.
Got it.
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 6c341bea514c..49831e8d02fb 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,9 @@
#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
#define LEGACY_LUT_LENGTH 256
+#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
+#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
+
/*
* Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
* format). This macro takes the coefficient we want transformed and the
@@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
+ const struct drm_color_lut *entry)
Indentation looks off. Also s/entry/color/ to match the other similarish
funcs maybe?
Sure.
+{
+ 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;
+
+ /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
+}
+
+static void
+icl_program_gamma_superfine_segment(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);
+ const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+ const struct drm_color_lut *lut = blob->data;
+ enum pipe pipe = crtc->pipe;
+ u32 i;
+
+ if (!lut || drm_color_lut_size(blob) < ICL_GAMMA_SUPERFINE_SEG_LENGTH)
+ return;
These checks aren't needed. Just dead code.
Will remove this and similars.
+
+ /*
+* 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 *
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
On Tue, Apr 30, 2019 at 08:51:08PM +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.
>
> V2: Addressed review comments from Ville
> - separate function for superfine and fine segments.
> - remove enum for segments.
> - reuse last entry of the LUT as gc_max value.
> - replace if() cond with switch...case in icl_load_luts.
> - add an entry variable, instead of 'word'
>
> Cc: Ville Syrjälä
> Cc: Maarten Lankhorst
> Cc: Daniel Vetter
>
> Suggested-by: Ville Syrjälä
> Signed-off-by: Shashank Sharma
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/i915/i915_pci.c| 3 +-
> drivers/gpu/drm/i915/intel_color.c | 125 -
> 2 files changed, 123 insertions(+), 5 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 }
Ugh. Thats one big LUT. But looks correct.
> +
Bogus newline.
>
> 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 6c341bea514c..49831e8d02fb 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,9 @@
> #define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
>
> #define LEGACY_LUT_LENGTH256
> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH(256 * 128 * 8)
> +#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
> +
> /*
> * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
> * format). This macro takes the coefficient we want transformed and the
> @@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
> + const struct drm_color_lut *entry)
Indentation looks off. Also s/entry/color/ to match the other similarish
funcs maybe?
> +{
> + 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;
> +
> + /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
> +}
> +
> +static void
> +icl_program_gamma_superfine_segment(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);
> + const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
> + const struct drm_color_lut *lut = blob->data;
> + enum pipe pipe = crtc->pipe;
> + u32 i;
> +
> + if (!lut || drm_color_lut_size(blob) < ICL_GAMMA_SUPERFINE_SEG_LENGTH)
> + return;
These checks aren't needed. Just dead code.
> +
> + /*
> + * 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
> + *
[Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Add Multi-segmented gamma support
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.
V2: Addressed review comments from Ville
- separate function for superfine and fine segments.
- remove enum for segments.
- reuse last entry of the LUT as gc_max value.
- replace if() cond with switch...case in icl_load_luts.
- add an entry variable, instead of 'word'
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Suggested-by: Ville Syrjälä
Signed-off-by: Shashank Sharma
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/i915/i915_pci.c| 3 +-
drivers/gpu/drm/i915/intel_color.c | 125 -
2 files changed, 123 insertions(+), 5 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 6c341bea514c..49831e8d02fb 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,9 @@
#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1))
#define LEGACY_LUT_LENGTH 256
+#define ICL_GAMMA_MULTISEG_LUT_LENGTH (256 * 128 * 8)
+#define ICL_GAMMA_SUPERFINE_SEG_LENGTH 9
+
/*
* Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
* format). This macro takes the coefficient we want transformed and the
@@ -767,6 +770,113 @@ 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_load_gcmax(const struct intel_crtc_state *crtc_state,
+ const struct drm_color_lut *entry)
+{
+ 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;
+
+ /* Fixme: LUT entries are 16 bit only, so we can prog 0x max */
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), entry->red);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), entry->green);
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), entry->blue);
+}
+
+static void
+icl_program_gamma_superfine_segment(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);
+ const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+ const struct drm_color_lut *lut = blob->data;
+ enum pipe pipe = crtc->pipe;
+ u32 i;
+
+ if (!lut || drm_color_lut_size(blob) < ICL_GAMMA_SUPERFINE_SEG_LENGTH)
+ 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).
+*/
+ I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+
+ for (i = 0; i < 9; i++) {
+ const struct drm_color_lut *entry = [i];
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
+ ilk_lut_12p4_udw(entry));
+ I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
+
