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

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

 




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@xxxxxxxxxxxxxxx>
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>

Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux