* [PATCH 0/4] fix broken state checker and enable state checker for icl+
@ 2019-10-09 6:55 Swati Sharma
2019-10-09 6:55 ` [PATCH 1/4] [v3] drm/i915/color: fix broken gamma state-checker during boot Swati Sharma
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Swati Sharma @ 2019-10-09 6:55 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal
In this patch series, basically added 3 patches
1. Fixing broken state-checker during boot since legacy platforms
i.e. platforms for which state checker was already enabled
2. Moving gamma_enable checks in bit_precision func() to platform
specific func()
3. Enabling state checker for ICL and TGL
Swati Sharma (4):
[v3] drm/i915/color: fix broken gamma state-checker during boot
[v2] drm/i915/color: move check of gamma_enable to specific
func/platform
[v5] drm/i915/color: Extract icl_read_luts()
FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
drivers/gpu/drm/i915/display/intel_color.c | 149 ++++++++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 6 +
2 files changed, 137 insertions(+), 18 deletions(-)
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] [v3] drm/i915/color: fix broken gamma state-checker during boot
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
@ 2019-10-09 6:55 ` Swati Sharma
2019-10-09 6:55 ` [PATCH 2/4] [v2] drm/i915/color: move check of gamma_enable to specific func/platform Swati Sharma
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Swati Sharma @ 2019-10-09 6:55 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal
Premature gamma lut prepration and loading which was getting
reflected in first modeset causing different colors on
screen during boot.
Issue: In BIOS, gamma is disabled by default. However, legacy read_luts()
was setting crtc_state->base.gamma_lut and gamma_lut was programmed
with junk values which led to visual artifacts (different
colored screens instead of usual black during boot).
Fix: Calling read_luts() only when gamma is enabled which will happen
after first modeset.
This fix is independent from the revert 1b8588741fdc ("Revert
"drm/i915/color: Extract icl_read_luts()"") and should fix different colors
on screen in legacy platforms too.
v2:
-Added gamma_enable checks inside read_luts() [Ville/Jani N]
-Corrected gamma enable check for CHV [Ville]
v3:
-Added check in ilk_read_luts() [Ville]
-Simplified gamma enable check for CHV [Ville]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111809
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111885
Tested-by: Jani Saarinen <jani.saarinen@intel.com>
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 9ab34902663e..08d020d4da35 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1613,6 +1613,9 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
static void i9xx_read_luts(struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return;
+
crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
}
@@ -1659,6 +1662,9 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
static void i965_read_luts(struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return;
+
if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
else
@@ -1701,10 +1707,10 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
static void chv_read_luts(struct intel_crtc_state *crtc_state)
{
- if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
- crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
- else
+ if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
crtc_state->base.gamma_lut = chv_read_cgm_lut(crtc_state);
+ else
+ i965_read_luts(crtc_state);
}
static struct drm_property_blob *
@@ -1742,6 +1748,12 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
static void ilk_read_luts(struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return;
+
+ if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
+ return;
+
if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
else
@@ -1788,6 +1800,9 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
static void glk_read_luts(struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return;
+
if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
else
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] [v2] drm/i915/color: move check of gamma_enable to specific func/platform
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
2019-10-09 6:55 ` [PATCH 1/4] [v3] drm/i915/color: fix broken gamma state-checker during boot Swati Sharma
@ 2019-10-09 6:55 ` Swati Sharma
2019-10-09 6:55 ` [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts() Swati Sharma
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Swati Sharma @ 2019-10-09 6:55 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal
Moved common code to check gamma_enable to specific funcs per platform
in bit_precision func. icl doesn't support that and chv has separate
enable knob for CGM LUT.
v2:
-Simplified chv_gamma_precision() [Ville]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 08d020d4da35..fa44eb73d088 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1420,6 +1420,9 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
static int i9xx_gamma_precision(const struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return 0;
+
switch (crtc_state->gamma_mode) {
case GAMMA_MODE_MODE_8BIT:
return 8;
@@ -1433,6 +1436,9 @@ static int i9xx_gamma_precision(const struct intel_crtc_state *crtc_state)
static int ilk_gamma_precision(const struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return 0;
+
if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
return 0;
@@ -1457,6 +1463,9 @@ static int chv_gamma_precision(const struct intel_crtc_state *crtc_state)
static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
{
+ if (!crtc_state->gamma_enable)
+ return 0;
+
switch (crtc_state->gamma_mode) {
case GAMMA_MODE_MODE_8BIT:
return 8;
@@ -1473,9 +1482,6 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- if (!crtc_state->gamma_enable)
- return 0;
-
if (HAS_GMCH(dev_priv)) {
if (IS_CHERRYVIEW(dev_priv))
return chv_gamma_precision(crtc_state);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
2019-10-09 6:55 ` [PATCH 1/4] [v3] drm/i915/color: fix broken gamma state-checker during boot Swati Sharma
2019-10-09 6:55 ` [PATCH 2/4] [v2] drm/i915/color: move check of gamma_enable to specific func/platform Swati Sharma
@ 2019-10-09 6:55 ` Swati Sharma
2019-10-09 14:16 ` Ville Syrjälä
2019-10-09 6:55 ` [PATCH 4/4] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs Swati Sharma
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Swati Sharma @ 2019-10-09 6:55 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal
For icl+, have hw read out to create hw blob of gamma
lut values. icl+ platforms supports multi segmented gamma
mode by default, add hw lut creation for this mode.
This will be used to validate gamma programming using dsb
(display state buffer) which is a tgl specific feature.
Major change done-removal of readouts of coarse and fine segments
because PAL_PREC_DATA register isn't giving propoer values.
State checker limited only to "fine segment"
v2: -readout code for multisegmented gamma has to come
up with some intermediate entries that aren't preserved
in hardware (Jani N)
-linear interpolation (Ville)
-moved common code to check gamma_enable to specific funcs,
since icl doesn't support that
v3: -use u16 instead of __u16 [Jani N]
-used single lut [Jani N]
-improved and more readable for loops [Jani N]
-read values directly to actual locations and then fill gaps [Jani N]
-moved cleaning to patch 1 [Jani N]
-renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
make it similar to icl_load_luts()
-renamed icl_compute_interpolated_gamma_blob() to
icl_compute_interpolated_gamma_lut_values() more sensible, I guess
v4: -removed interpolated func for creating gamma lut values
-removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
correctly
v5: -added gamma_enable check inside read_luts()
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 6 ++
2 files changed, 108 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index fa44eb73d088..614e0ad386ca 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
}
}
+static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+ if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
+ return 0;
+
+ switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
+ case GAMMA_MODE_MODE_8BIT:
+ return 8;
+ case GAMMA_MODE_MODE_10BIT:
+ return 10;
+ case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
+ return 16;
+ default:
+ MISSING_CASE(crtc_state->gamma_mode);
+ return 0;
+ }
+
+}
+
int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
else
return i9xx_gamma_precision(crtc_state);
} else {
- if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ return icl_gamma_precision(crtc_state);
+ else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
return glk_gamma_precision(crtc_state);
else if (IS_IRONLAKE(dev_priv))
return ilk_gamma_precision(crtc_state);
@@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
return true;
}
+static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
+ struct drm_color_lut *lut2,
+ int lut_size, u32 err)
+{
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ if (!err_check(&lut1[i], &lut2[i], err))
+ return false;
+ }
+
+ return true;
+}
+
bool intel_color_lut_equal(struct drm_property_blob *blob1,
struct drm_property_blob *blob2,
u32 gamma_mode, u32 bit_precision)
@@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
lut_size2 = drm_color_lut_size(blob2);
/* check sw and hw lut size */
- switch (gamma_mode) {
- case GAMMA_MODE_MODE_8BIT:
- case GAMMA_MODE_MODE_10BIT:
- if (lut_size1 != lut_size2)
- return false;
- break;
- default:
- MISSING_CASE(gamma_mode);
- return false;
- }
+ if (lut_size1 != lut_size2)
+ return false;
lut1 = blob1->data;
lut2 = blob2->data;
@@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
err = 0xffff >> bit_precision;
/* check sw and hw lut entry to be equal */
- switch (gamma_mode) {
+ switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
case GAMMA_MODE_MODE_8BIT:
case GAMMA_MODE_MODE_10BIT:
if (!intel_color_lut_entry_equal(lut1, lut2,
lut_size2, err))
return false;
break;
+ case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
+ if (!intel_color_lut_entry_multi_equal(lut1, lut2,
+ lut_size2, err))
+ return false;
+ break;
default:
MISSING_CASE(gamma_mode);
return false;
@@ -1815,6 +1847,63 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
}
+static struct drm_property_blob *
+icl_read_lut_multi_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);
+ int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ enum pipe pipe = crtc->pipe;
+ struct drm_property_blob *blob;
+ struct drm_color_lut *blob_data;
+ u32 i, val1, val2;
+
+ blob = drm_property_create_blob(&dev_priv->drm,
+ sizeof(struct drm_color_lut) * lut_size,
+ NULL);
+ if (IS_ERR(blob))
+ return NULL;
+
+ blob_data = blob->data;
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+
+ for (i = 0; i < 9; i++) {
+ val1 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
+ val2 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
+
+ blob_data[i].red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
+ blob_data[i].green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
+ blob_data[i].blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
+ }
+
+ /*
+ * FIXME readouts from PAL_PREC_DATA register aren't giving correct values
+ * in the case of fine and coarse segments. Restricting readouts only for
+ * super fine segment as of now.
+ */
+
+ return blob;
+}
+
+static void icl_read_luts(struct intel_crtc_state *crtc_state)
+{
+ if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
+ return;
+
+ if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+ GAMMA_MODE_MODE_8BIT)
+ crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+ else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+ GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
+ crtc_state->base.gamma_lut = icl_read_lut_multi_segment(crtc_state);
+ else
+ crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+}
+
void intel_color_init(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1858,6 +1947,7 @@ void intel_color_init(struct intel_crtc *crtc)
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.load_luts = icl_load_luts;
+ dev_priv->display.read_luts = icl_read_luts;
} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
dev_priv->display.load_luts = glk_load_luts;
dev_priv->display.read_luts = glk_read_luts;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1dc067fc57ab..06b6398b7d62 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10584,6 +10584,12 @@ enum skl_power_gate {
#define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
#define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
+#define PAL_PREC_MULTI_SEG_RED_LDW_MASK REG_GENMASK(29, 24)
+#define PAL_PREC_MULTI_SEG_RED_UDW_MASK REG_GENMASK(29, 20)
+#define PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
+#define PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
+#define PAL_PREC_MULTI_SEG_BLUE_LDW_MASK REG_GENMASK(9, 4)
+#define PAL_PREC_MULTI_SEG_BLUE_UDW_MASK REG_GENMASK(9, 0)
#define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
_PAL_PREC_MULTI_SEG_INDEX_A, \
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
` (2 preceding siblings ...)
2019-10-09 6:55 ` [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts() Swati Sharma
@ 2019-10-09 6:55 ` Swati Sharma
2019-10-09 7:47 ` ✗ Fi.CI.CHECKPATCH: warning for fix broken state checker and enable state checker for icl+ (rev2) Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Swati Sharma @ 2019-10-09 6:55 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal
Only to print hw and sw lut values/channel.
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 614e0ad386ca..6dd0bd3aca37 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1521,6 +1521,8 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
static bool err_check(struct drm_color_lut *lut1,
struct drm_color_lut *lut2, u32 err)
{
+ DRM_DEBUG_KMS("hw_lut->red=0x%x sw_lut->red=0x%x hw_lut->blue=0x%x sw_lut->blue=0x%x hw_lut->green=0x%x sw_lut->green=0x%x", lut2->red, lut1->red, lut2->blue, lut1->blue, lut2->green, lut1->green);
+
return ((abs((long)lut2->red - lut1->red)) <= err) &&
((abs((long)lut2->blue - lut1->blue)) <= err) &&
((abs((long)lut2->green - lut1->green)) <= err);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for fix broken state checker and enable state checker for icl+ (rev2)
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
` (3 preceding siblings ...)
2019-10-09 6:55 ` [PATCH 4/4] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs Swati Sharma
@ 2019-10-09 7:47 ` Patchwork
2019-10-09 8:10 ` ✓ Fi.CI.BAT: success " Patchwork
2019-10-09 11:57 ` ✓ Fi.CI.IGT: " Patchwork
6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-10-09 7:47 UTC (permalink / raw)
To: Swati Sharma; +Cc: intel-gfx
== Series Details ==
Series: fix broken state checker and enable state checker for icl+ (rev2)
URL : https://patchwork.freedesktop.org/series/67586/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
7d21a79cea41 drm/i915/color: fix broken gamma state-checker during boot
-:18: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 1b8588741fdc ("Revert "drm/i915/color: Extract icl_read_luts()"")'
#18:
This fix is independent from the revert 1b8588741fdc ("Revert
total: 1 errors, 0 warnings, 0 checks, 52 lines checked
5b68384db53a drm/i915/color: move check of gamma_enable to specific func/platform
2e841a058358 drm/i915/color: Extract icl_read_luts()
-:33: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#33:
-removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
-:64: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#64: FILE: drivers/gpu/drm/i915/display/intel_color.c:1497:
+
+}
total: 0 errors, 1 warnings, 1 checks, 174 lines checked
8c3d9d6a4b2b FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
-:18: WARNING:LONG_LINE: line over 100 characters
#18: FILE: drivers/gpu/drm/i915/display/intel_color.c:1524:
+ DRM_DEBUG_KMS("hw_lut->red=0x%x sw_lut->red=0x%x hw_lut->blue=0x%x sw_lut->blue=0x%x hw_lut->green=0x%x sw_lut->green=0x%x", lut2->red, lut1->red, lut2->blue, lut1->blue, lut2->green, lut1->green);
total: 0 errors, 1 warnings, 0 checks, 8 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for fix broken state checker and enable state checker for icl+ (rev2)
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
` (4 preceding siblings ...)
2019-10-09 7:47 ` ✗ Fi.CI.CHECKPATCH: warning for fix broken state checker and enable state checker for icl+ (rev2) Patchwork
@ 2019-10-09 8:10 ` Patchwork
2019-10-09 11:57 ` ✓ Fi.CI.IGT: " Patchwork
6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-10-09 8:10 UTC (permalink / raw)
To: Swati Sharma; +Cc: intel-gfx
== Series Details ==
Series: fix broken state checker and enable state checker for icl+ (rev2)
URL : https://patchwork.freedesktop.org/series/67586/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7037 -> Patchwork_14718
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/index.html
Known issues
------------
Here are the changes found in Patchwork_14718 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_switch@rcs0:
- fi-bxt-dsi: [PASS][1] -> [INCOMPLETE][2] ([fdo#103927])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/fi-bxt-dsi/igt@gem_ctx_switch@rcs0.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/fi-bxt-dsi/igt@gem_ctx_switch@rcs0.html
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-icl-u2: [PASS][5] -> [FAIL][6] ([fdo#109483]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- {fi-tgl-u}: [INCOMPLETE][7] ([fdo#111593]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/fi-tgl-u/igt@gem_exec_gttfill@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/fi-tgl-u/igt@gem_exec_gttfill@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
Participating hosts (50 -> 26)
------------------------------
Additional (1): fi-icl-dsi
Missing (25): fi-apl-guc fi-icl-u3 fi-icl-y fi-skl-lmem fi-icl-guc fi-cml-u2 fi-icl-u4 fi-bdw-5557u fi-cml-s fi-byt-j1900 fi-glk-dsi fi-kbl-7500u fi-hsw-4770 fi-skl-6700k2 fi-kbl-r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-cfl-guc fi-kbl-guc fi-whl-u fi-cfl-8109u fi-kbl-8809g fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7037 -> Patchwork_14718
CI-20190529: 20190529
CI_DRM_7037: 4ff590d66f456266150c2c42c194f62338569140 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5218: 869ed1ee0b71ce17f0a864512488f8b1a6cb8545 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14718: 8c3d9d6a4b2be60bd35e4ab7cd90f84eafd17468 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8c3d9d6a4b2b FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
2e841a058358 drm/i915/color: Extract icl_read_luts()
5b68384db53a drm/i915/color: move check of gamma_enable to specific func/platform
7d21a79cea41 drm/i915/color: fix broken gamma state-checker during boot
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for fix broken state checker and enable state checker for icl+ (rev2)
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
` (5 preceding siblings ...)
2019-10-09 8:10 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-10-09 11:57 ` Patchwork
6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-10-09 11:57 UTC (permalink / raw)
To: Swati Sharma; +Cc: intel-gfx
== Series Details ==
Series: fix broken state checker and enable state checker for icl+ (rev2)
URL : https://patchwork.freedesktop.org/series/67586/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7037_full -> Patchwork_14718_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_14718_full:
### IGT changes ###
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-pwrite:
- {shard-tglb}: NOTRUN -> [INCOMPLETE][1] +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-pwrite.html
Known issues
------------
Here are the changes found in Patchwork_14718_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@in-flight-contexts-immediate:
- shard-snb: [PASS][2] -> [FAIL][3] ([fdo#111925])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-snb5/igt@gem_eio@in-flight-contexts-immediate.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-snb1/igt@gem_eio@in-flight-contexts-immediate.html
* igt@gem_exec_schedule@preempt-queue-bsd:
- shard-iclb: [PASS][4] -> [SKIP][5] ([fdo#111325]) +2 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb3/igt@gem_exec_schedule@preempt-queue-bsd.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd.html
* igt@gem_tiled_swapping@non-threaded:
- shard-glk: [PASS][6] -> [DMESG-FAIL][7] ([fdo#108686])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-glk2/igt@gem_tiled_swapping@non-threaded.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-glk8/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_userptr_blits@dmabuf-unsync:
- shard-snb: [PASS][8] -> [DMESG-WARN][9] ([fdo#111870])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-snb4/igt@gem_userptr_blits@dmabuf-unsync.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-snb6/igt@gem_userptr_blits@dmabuf-unsync.html
* igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
- shard-hsw: [PASS][10] -> [DMESG-WARN][11] ([fdo#111870])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-hsw4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-hsw4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
* igt@i915_pm_rpm@modeset-stress-extra-wait:
- shard-hsw: [PASS][12] -> [INCOMPLETE][13] ([fdo#103540] / [fdo#107807])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-hsw4/igt@i915_pm_rpm@modeset-stress-extra-wait.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-hsw8/igt@i915_pm_rpm@modeset-stress-extra-wait.html
* igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding:
- shard-apl: [PASS][14] -> [INCOMPLETE][15] ([fdo#103927]) +1 similar issue
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html
* igt@kms_cursor_crc@pipe-b-cursor-suspend:
- shard-apl: [PASS][16] -> [DMESG-WARN][17] ([fdo#108566]) +3 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-apl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
- shard-iclb: [PASS][18] -> [FAIL][19] ([fdo#103167]) +3 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: [PASS][20] -> [SKIP][21] ([fdo#109441]) +2 similar issues
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb8/igt@kms_psr@psr2_cursor_render.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][22] -> [FAIL][23] ([fdo#99912])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-apl7/igt@kms_setmode@basic.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-apl3/igt@kms_setmode@basic.html
- shard-kbl: [PASS][24] -> [FAIL][25] ([fdo#99912])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-kbl7/igt@kms_setmode@basic.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-kbl2/igt@kms_setmode@basic.html
* igt@prime_vgem@fence-wait-bsd2:
- shard-iclb: [PASS][26] -> [SKIP][27] ([fdo#109276]) +16 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb7/igt@prime_vgem@fence-wait-bsd2.html
#### Possible fixes ####
* igt@gem_exec_flush@basic-wb-pro-default:
- shard-apl: [INCOMPLETE][28] ([fdo#103927]) -> [PASS][29] +2 similar issues
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-apl8/igt@gem_exec_flush@basic-wb-pro-default.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-apl3/igt@gem_exec_flush@basic-wb-pro-default.html
* igt@gem_exec_schedule@preempt-queue-bsd2:
- shard-iclb: [SKIP][30] ([fdo#109276]) -> [PASS][31] +7 similar issues
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd2.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd2.html
* igt@gem_exec_schedule@preemptive-hang-bsd:
- shard-iclb: [SKIP][32] ([fdo#111325]) -> [PASS][33] +1 similar issue
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb7/igt@gem_exec_schedule@preemptive-hang-bsd.html
* igt@gem_userptr_blits@dmabuf-sync:
- shard-snb: [DMESG-WARN][34] ([fdo#111870]) -> [PASS][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-snb2/igt@gem_userptr_blits@dmabuf-sync.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-snb4/igt@gem_userptr_blits@dmabuf-sync.html
- shard-hsw: [DMESG-WARN][36] ([fdo#111870]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-hsw8/igt@gem_userptr_blits@dmabuf-sync.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-hsw6/igt@gem_userptr_blits@dmabuf-sync.html
* igt@kms_busy@extended-modeset-hang-newfb-render-a:
- shard-iclb: [INCOMPLETE][38] ([fdo#107713]) -> [PASS][39]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb7/igt@kms_busy@extended-modeset-hang-newfb-render-a.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb2/igt@kms_busy@extended-modeset-hang-newfb-render-a.html
* igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen:
- shard-skl: [FAIL][40] ([fdo#103232]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-skl8/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-skl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-apl: [DMESG-WARN][42] ([fdo#108566]) -> [PASS][43] +2 similar issues
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
- shard-iclb: [FAIL][44] ([fdo#103167]) -> [PASS][45] +5 similar issues
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: [FAIL][46] ([fdo#108145] / [fdo#110403]) -> [PASS][47] +1 similar issue
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
* igt@kms_psr2_su@page_flip:
- shard-iclb: [SKIP][48] ([fdo#109642] / [fdo#111068]) -> [PASS][49]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb1/igt@kms_psr2_su@page_flip.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb2/igt@kms_psr2_su@page_flip.html
* igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
- {shard-tglb}: [INCOMPLETE][50] ([fdo#111832] / [fdo#111850]) -> [PASS][51]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-tglb2/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-tglb6/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
#### Warnings ####
* igt@gem_mocs_settings@mocs-settings-bsd2:
- shard-iclb: [SKIP][52] ([fdo#109276]) -> [FAIL][53] ([fdo#111330])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7037/shard-iclb6/igt@gem_mocs_settings@mocs-settings-bsd2.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14718/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
[fdo#111714]: https://bugs.freedesktop.org/show_bug.cgi?id=111714
[fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
[fdo#111850]: https://bugs.freedesktop.org/show_bug.cgi?id=111850
[fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
[fdo#111925]: https://bugs.freedesktop.org/show_bug.cgi?id=111925
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (11 -> 11)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7037 -> Patchwork_14718
CI-20190529: 20190529
CI_DRM_7037: 4ff590d66f456266150c2c42c194f62338569140 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5218: 869ed1ee0b71ce17f0a864512488f8b1a6cb8545 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14718: 8c3d9d6a4b2be60bd35e4ab7cd90f84eafd17468 @ 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_14718/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-09 6:55 ` [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts() Swati Sharma
@ 2019-10-09 14:16 ` Ville Syrjälä
2019-10-10 10:50 ` Sharma, Swati2
2019-10-15 7:59 ` Jani Nikula
0 siblings, 2 replies; 15+ messages in thread
From: Ville Syrjälä @ 2019-10-09 14:16 UTC (permalink / raw)
To: Swati Sharma; +Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal
On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
> For icl+, have hw read out to create hw blob of gamma
> lut values. icl+ platforms supports multi segmented gamma
> mode by default, add hw lut creation for this mode.
>
> This will be used to validate gamma programming using dsb
> (display state buffer) which is a tgl specific feature.
>
> Major change done-removal of readouts of coarse and fine segments
> because PAL_PREC_DATA register isn't giving propoer values.
> State checker limited only to "fine segment"
>
> v2: -readout code for multisegmented gamma has to come
> up with some intermediate entries that aren't preserved
> in hardware (Jani N)
> -linear interpolation (Ville)
> -moved common code to check gamma_enable to specific funcs,
> since icl doesn't support that
> v3: -use u16 instead of __u16 [Jani N]
> -used single lut [Jani N]
> -improved and more readable for loops [Jani N]
> -read values directly to actual locations and then fill gaps [Jani N]
> -moved cleaning to patch 1 [Jani N]
> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
> make it similar to icl_load_luts()
> -renamed icl_compute_interpolated_gamma_blob() to
> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
> v4: -removed interpolated func for creating gamma lut values
> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
> correctly
> v5: -added gamma_enable check inside read_luts()
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++
> 2 files changed, 108 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index fa44eb73d088..614e0ad386ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
> }
> }
>
> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
> +{
> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> + return 0;
> +
> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> + case GAMMA_MODE_MODE_8BIT:
> + return 8;
> + case GAMMA_MODE_MODE_10BIT:
> + return 10;
> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> + return 16;
> + default:
> + MISSING_CASE(crtc_state->gamma_mode);
> + return 0;
> + }
> +
> +}
> +
> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
> else
> return i9xx_gamma_precision(crtc_state);
> } else {
> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + return icl_gamma_precision(crtc_state);
> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> return glk_gamma_precision(crtc_state);
> else if (IS_IRONLAKE(dev_priv))
> return ilk_gamma_precision(crtc_state);
> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
> return true;
> }
>
> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
> + struct drm_color_lut *lut2,
> + int lut_size, u32 err)
> +{
> + int i;
> +
> + for (i = 0; i < 9; i++) {
> + if (!err_check(&lut1[i], &lut2[i], err))
> + return false;
> + }
> +
> + return true;
> +}
> +
> bool intel_color_lut_equal(struct drm_property_blob *blob1,
> struct drm_property_blob *blob2,
> u32 gamma_mode, u32 bit_precision)
> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> lut_size2 = drm_color_lut_size(blob2);
>
> /* check sw and hw lut size */
> - switch (gamma_mode) {
> - case GAMMA_MODE_MODE_8BIT:
> - case GAMMA_MODE_MODE_10BIT:
> - if (lut_size1 != lut_size2)
> - return false;
> - break;
> - default:
> - MISSING_CASE(gamma_mode);
> - return false;
> - }
> + if (lut_size1 != lut_size2)
> + return false;
>
> lut1 = blob1->data;
> lut2 = blob2->data;
> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> err = 0xffff >> bit_precision;
>
> /* check sw and hw lut entry to be equal */
> - switch (gamma_mode) {
> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
> case GAMMA_MODE_MODE_8BIT:
> case GAMMA_MODE_MODE_10BIT:
> if (!intel_color_lut_entry_equal(lut1, lut2,
> lut_size2, err))
> return false;
> break;
> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
> + lut_size2, err))
I don't think you need a new function for that. Just pass 9 as the size
to intel_color_lut_entry_equal() ?
Hmm, should probably rename that to just intel_color_lut_equal() since
it checks the entire LUT (or at least the specified subset) and not
just a single entry...
> + return false;
> + break;
> default:
> MISSING_CASE(gamma_mode);
> return false;
> @@ -1815,6 +1847,63 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
> crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
> }
>
> +static struct drm_property_blob *
> +icl_read_lut_multi_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);
> + int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> + enum pipe pipe = crtc->pipe;
> + struct drm_property_blob *blob;
> + struct drm_color_lut *blob_data;
> + u32 i, val1, val2;
> +
> + blob = drm_property_create_blob(&dev_priv->drm,
> + sizeof(struct drm_color_lut) * lut_size,
> + NULL);
> + if (IS_ERR(blob))
> + return NULL;
> +
> + blob_data = blob->data;
> +
> + I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
> +
> + for (i = 0; i < 9; i++) {
> + val1 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
> + val2 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
> +
> + blob_data[i].red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
> + blob_data[i].green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
> + blob_data[i].blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
> + }
> +
> + /*
> + * FIXME readouts from PAL_PREC_DATA register aren't giving correct values
> + * in the case of fine and coarse segments. Restricting readouts only for
> + * super fine segment as of now.
> + */
> +
> + return blob;
> +}
> +
> +static void icl_read_luts(struct intel_crtc_state *crtc_state)
> +{
> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> + return;
> +
> + if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
> + GAMMA_MODE_MODE_8BIT)
Please make this a switch statement.
I pushed patches 1-2 to dinq. Thanks.
> + crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
> + else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
> + GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
> + crtc_state->base.gamma_lut = icl_read_lut_multi_segment(crtc_state);
> + else
> + crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
> +}
> +
> void intel_color_init(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1858,6 +1947,7 @@ void intel_color_init(struct intel_crtc *crtc)
>
> if (INTEL_GEN(dev_priv) >= 11) {
> dev_priv->display.load_luts = icl_load_luts;
> + dev_priv->display.read_luts = icl_read_luts;
> } else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> dev_priv->display.load_luts = glk_load_luts;
> dev_priv->display.read_luts = glk_read_luts;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1dc067fc57ab..06b6398b7d62 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10584,6 +10584,12 @@ enum skl_power_gate {
>
> #define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
> #define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
> +#define PAL_PREC_MULTI_SEG_RED_LDW_MASK REG_GENMASK(29, 24)
> +#define PAL_PREC_MULTI_SEG_RED_UDW_MASK REG_GENMASK(29, 20)
> +#define PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
> +#define PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
> +#define PAL_PREC_MULTI_SEG_BLUE_LDW_MASK REG_GENMASK(9, 4)
> +#define PAL_PREC_MULTI_SEG_BLUE_UDW_MASK REG_GENMASK(9, 0)
>
> #define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
> _PAL_PREC_MULTI_SEG_INDEX_A, \
> --
> 2.23.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-09 14:16 ` Ville Syrjälä
@ 2019-10-10 10:50 ` Sharma, Swati2
2019-10-15 12:34 ` Ville Syrjälä
2019-10-15 7:59 ` Jani Nikula
1 sibling, 1 reply; 15+ messages in thread
From: Sharma, Swati2 @ 2019-10-10 10:50 UTC (permalink / raw)
To: Ville Syrjälä
Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal
On 09-Oct-19 7:46 PM, Ville Syrjälä wrote:
> On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
>> For icl+, have hw read out to create hw blob of gamma
>> lut values. icl+ platforms supports multi segmented gamma
>> mode by default, add hw lut creation for this mode.
>>
>> This will be used to validate gamma programming using dsb
>> (display state buffer) which is a tgl specific feature.
>>
>> Major change done-removal of readouts of coarse and fine segments
>> because PAL_PREC_DATA register isn't giving propoer values.
>> State checker limited only to "fine segment"
>>
>> v2: -readout code for multisegmented gamma has to come
>> up with some intermediate entries that aren't preserved
>> in hardware (Jani N)
>> -linear interpolation (Ville)
>> -moved common code to check gamma_enable to specific funcs,
>> since icl doesn't support that
>> v3: -use u16 instead of __u16 [Jani N]
>> -used single lut [Jani N]
>> -improved and more readable for loops [Jani N]
>> -read values directly to actual locations and then fill gaps [Jani N]
>> -moved cleaning to patch 1 [Jani N]
>> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
>> make it similar to icl_load_luts()
>> -renamed icl_compute_interpolated_gamma_blob() to
>> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
>> v4: -removed interpolated func for creating gamma lut values
>> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
>> correctly
>> v5: -added gamma_enable check inside read_luts()
>>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
>> drivers/gpu/drm/i915/i915_reg.h | 6 ++
>> 2 files changed, 108 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index fa44eb73d088..614e0ad386ca 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
>> }
>> }
>>
>> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
>> +{
>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>> + return 0;
>> +
>> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> + case GAMMA_MODE_MODE_8BIT:
>> + return 8;
>> + case GAMMA_MODE_MODE_10BIT:
>> + return 10;
>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> + return 16;
>> + default:
>> + MISSING_CASE(crtc_state->gamma_mode);
>> + return 0;
>> + }
>> +
>> +}
>> +
>> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
>> {
>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
>> else
>> return i9xx_gamma_precision(crtc_state);
>> } else {
>> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + return icl_gamma_precision(crtc_state);
>> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> return glk_gamma_precision(crtc_state);
>> else if (IS_IRONLAKE(dev_priv))
>> return ilk_gamma_precision(crtc_state);
>> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
>> return true;
>> }
>>
>> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
>> + struct drm_color_lut *lut2,
>> + int lut_size, u32 err)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 9; i++) {
>> + if (!err_check(&lut1[i], &lut2[i], err))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> struct drm_property_blob *blob2,
>> u32 gamma_mode, u32 bit_precision)
>> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> lut_size2 = drm_color_lut_size(blob2);
>>
>> /* check sw and hw lut size */
>> - switch (gamma_mode) {
>> - case GAMMA_MODE_MODE_8BIT:
>> - case GAMMA_MODE_MODE_10BIT:
>> - if (lut_size1 != lut_size2)
>> - return false;
>> - break;
>> - default:
>> - MISSING_CASE(gamma_mode);
>> - return false;
>> - }
>> + if (lut_size1 != lut_size2)
>> + return false;
>>
>> lut1 = blob1->data;
>> lut2 = blob2->data;
>> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> err = 0xffff >> bit_precision;
>>
>> /* check sw and hw lut entry to be equal */
>> - switch (gamma_mode) {
>> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
>> case GAMMA_MODE_MODE_8BIT:
>> case GAMMA_MODE_MODE_10BIT:
>> if (!intel_color_lut_entry_equal(lut1, lut2,
>> lut_size2, err))
>> return false;
>> break;
>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
>> + lut_size2, err))
>
> I don't think you need a new function for that. Just pass 9 as the size
> to intel_color_lut_entry_equal() ?
I had made a separate function for multi-segmented gamma since there
will be 3 loops for comparing superfine, fine and course segments which
wont go with intel_lut_entry_equal() structure.
Right now we are limiting to superfine segment only but in future we
will add for other segments too (once we get fix from h/w)
Func() should look like this. Actually there is no need to passing
lut_size only in this function if we continue with this function only.
+static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
+ struct drm_color_lut *lut2,
+ int lut_size, u32 err)
+{
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ if (!err_check(&lut1[i], &lut2[i], err))
+ return false;
+ }
+
+ for (i = 1; i < 257; i++) {
+ if (!err_check(&lut1[i * 8], &lut2[i * 8], err))
+ return false;
+ }
+
+ for (i = 0; i < 256; i++) {
+ if (!err_check(&lut1[i * 8 * 128], &lut2[i * 8 * 128], err))
+ return false;
+ }
+
+ return true;
+}
+
Please suggest.
>
> Hmm, should probably rename that to just intel_color_lut_equal() since
> it checks the entire LUT (or at least the specified subset) and not
> just a single entry...
This will be fine for this segment but for other two segments it won't
work. Right?
>
>> + return false;
>> + break;
>> default:
>> MISSING_CASE(gamma_mode);
>> return false;
>> @@ -1815,6 +1847,63 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
>> crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>> }
>>
>> +static struct drm_property_blob *
>> +icl_read_lut_multi_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);
>> + int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>> + enum pipe pipe = crtc->pipe;
>> + struct drm_property_blob *blob;
>> + struct drm_color_lut *blob_data;
>> + u32 i, val1, val2;
>> +
>> + blob = drm_property_create_blob(&dev_priv->drm,
>> + sizeof(struct drm_color_lut) * lut_size,
>> + NULL);
>> + if (IS_ERR(blob))
>> + return NULL;
>> +
>> + blob_data = blob->data;
>> +
>> + I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>> +
>> + for (i = 0; i < 9; i++) {
>> + val1 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> + val2 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> +
>> + blob_data[i].red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
>> + blob_data[i].green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
>> + blob_data[i].blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
>> + }
>> +
>> + /*
>> + * FIXME readouts from PAL_PREC_DATA register aren't giving correct values
>> + * in the case of fine and coarse segments. Restricting readouts only for
>> + * super fine segment as of now.
>> + */
>> +
>> + return blob;
>> +}
>> +
>> +static void icl_read_luts(struct intel_crtc_state *crtc_state)
>> +{
>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>> + return;
>> +
>> + if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> + GAMMA_MODE_MODE_8BIT)
>
> Please make this a switch statement.
>
> I pushed patches 1-2 to dinq. Thanks.
>
>> + crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>> + else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> + GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
>> + crtc_state->base.gamma_lut = icl_read_lut_multi_segment(crtc_state);
>> + else
>> + crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>> +}
>> +
>> void intel_color_init(struct intel_crtc *crtc)
>> {
>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> @@ -1858,6 +1947,7 @@ void intel_color_init(struct intel_crtc *crtc)
>>
>> if (INTEL_GEN(dev_priv) >= 11) {
>> dev_priv->display.load_luts = icl_load_luts;
>> + dev_priv->display.read_luts = icl_read_luts;
>> } else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
>> dev_priv->display.load_luts = glk_load_luts;
>> dev_priv->display.read_luts = glk_read_luts;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1dc067fc57ab..06b6398b7d62 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -10584,6 +10584,12 @@ enum skl_power_gate {
>>
>> #define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
>> #define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
>> +#define PAL_PREC_MULTI_SEG_RED_LDW_MASK REG_GENMASK(29, 24)
>> +#define PAL_PREC_MULTI_SEG_RED_UDW_MASK REG_GENMASK(29, 20)
>> +#define PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
>> +#define PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
>> +#define PAL_PREC_MULTI_SEG_BLUE_LDW_MASK REG_GENMASK(9, 4)
>> +#define PAL_PREC_MULTI_SEG_BLUE_UDW_MASK REG_GENMASK(9, 0)
>>
>> #define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
>> _PAL_PREC_MULTI_SEG_INDEX_A, \
>> --
>> 2.23.0
>
--
~Swati Sharma
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-09 14:16 ` Ville Syrjälä
2019-10-10 10:50 ` Sharma, Swati2
@ 2019-10-15 7:59 ` Jani Nikula
1 sibling, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2019-10-15 7:59 UTC (permalink / raw)
To: Ville Syrjälä, Swati Sharma
Cc: daniel.vetter, intel-gfx, ankit.k.nautiyal
On Wed, 09 Oct 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
>> For icl+, have hw read out to create hw blob of gamma
>> lut values. icl+ platforms supports multi segmented gamma
>> mode by default, add hw lut creation for this mode.
>>
>> This will be used to validate gamma programming using dsb
>> (display state buffer) which is a tgl specific feature.
>>
>> Major change done-removal of readouts of coarse and fine segments
>> because PAL_PREC_DATA register isn't giving propoer values.
>> State checker limited only to "fine segment"
>>
>> v2: -readout code for multisegmented gamma has to come
>> up with some intermediate entries that aren't preserved
>> in hardware (Jani N)
>> -linear interpolation (Ville)
>> -moved common code to check gamma_enable to specific funcs,
>> since icl doesn't support that
>> v3: -use u16 instead of __u16 [Jani N]
>> -used single lut [Jani N]
>> -improved and more readable for loops [Jani N]
>> -read values directly to actual locations and then fill gaps [Jani N]
>> -moved cleaning to patch 1 [Jani N]
>> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
>> make it similar to icl_load_luts()
>> -renamed icl_compute_interpolated_gamma_blob() to
>> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
>> v4: -removed interpolated func for creating gamma lut values
>> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
>> correctly
>> v5: -added gamma_enable check inside read_luts()
>>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
>> drivers/gpu/drm/i915/i915_reg.h | 6 ++
>> 2 files changed, 108 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index fa44eb73d088..614e0ad386ca 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
>> }
>> }
>>
>> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
>> +{
>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>> + return 0;
>> +
>> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> + case GAMMA_MODE_MODE_8BIT:
>> + return 8;
>> + case GAMMA_MODE_MODE_10BIT:
>> + return 10;
>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> + return 16;
>> + default:
>> + MISSING_CASE(crtc_state->gamma_mode);
>> + return 0;
>> + }
>> +
>> +}
>> +
>> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
>> {
>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
>> else
>> return i9xx_gamma_precision(crtc_state);
>> } else {
>> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + return icl_gamma_precision(crtc_state);
>> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> return glk_gamma_precision(crtc_state);
>> else if (IS_IRONLAKE(dev_priv))
>> return ilk_gamma_precision(crtc_state);
>> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
>> return true;
>> }
>>
>> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
>> + struct drm_color_lut *lut2,
>> + int lut_size, u32 err)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 9; i++) {
>> + if (!err_check(&lut1[i], &lut2[i], err))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> struct drm_property_blob *blob2,
>> u32 gamma_mode, u32 bit_precision)
>> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> lut_size2 = drm_color_lut_size(blob2);
>>
>> /* check sw and hw lut size */
>> - switch (gamma_mode) {
>> - case GAMMA_MODE_MODE_8BIT:
>> - case GAMMA_MODE_MODE_10BIT:
>> - if (lut_size1 != lut_size2)
>> - return false;
>> - break;
>> - default:
>> - MISSING_CASE(gamma_mode);
>> - return false;
>> - }
>> + if (lut_size1 != lut_size2)
>> + return false;
>>
>> lut1 = blob1->data;
>> lut2 = blob2->data;
>> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> err = 0xffff >> bit_precision;
>>
>> /* check sw and hw lut entry to be equal */
>> - switch (gamma_mode) {
>> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
>> case GAMMA_MODE_MODE_8BIT:
>> case GAMMA_MODE_MODE_10BIT:
>> if (!intel_color_lut_entry_equal(lut1, lut2,
>> lut_size2, err))
>> return false;
>> break;
>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
>> + lut_size2, err))
>
> I don't think you need a new function for that. Just pass 9 as the size
> to intel_color_lut_entry_equal() ?
I think the idea is to prepare for if/when we can actually check the
multi-segmented case where we have items that need to be ignored with
different spacing.
BR,
Jani.
>
> Hmm, should probably rename that to just intel_color_lut_equal() since
> it checks the entire LUT (or at least the specified subset) and not
> just a single entry...
>
>> + return false;
>> + break;
>> default:
>> MISSING_CASE(gamma_mode);
>> return false;
>> @@ -1815,6 +1847,63 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
>> crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>> }
>>
>> +static struct drm_property_blob *
>> +icl_read_lut_multi_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);
>> + int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>> + enum pipe pipe = crtc->pipe;
>> + struct drm_property_blob *blob;
>> + struct drm_color_lut *blob_data;
>> + u32 i, val1, val2;
>> +
>> + blob = drm_property_create_blob(&dev_priv->drm,
>> + sizeof(struct drm_color_lut) * lut_size,
>> + NULL);
>> + if (IS_ERR(blob))
>> + return NULL;
>> +
>> + blob_data = blob->data;
>> +
>> + I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>> +
>> + for (i = 0; i < 9; i++) {
>> + val1 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> + val2 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
>> +
>> + blob_data[i].red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
>> + blob_data[i].green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
>> + blob_data[i].blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
>> + REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
>> + }
>> +
>> + /*
>> + * FIXME readouts from PAL_PREC_DATA register aren't giving correct values
>> + * in the case of fine and coarse segments. Restricting readouts only for
>> + * super fine segment as of now.
>> + */
>> +
>> + return blob;
>> +}
>> +
>> +static void icl_read_luts(struct intel_crtc_state *crtc_state)
>> +{
>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>> + return;
>> +
>> + if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> + GAMMA_MODE_MODE_8BIT)
>
> Please make this a switch statement.
>
> I pushed patches 1-2 to dinq. Thanks.
>
>> + crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>> + else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> + GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
>> + crtc_state->base.gamma_lut = icl_read_lut_multi_segment(crtc_state);
>> + else
>> + crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>> +}
>> +
>> void intel_color_init(struct intel_crtc *crtc)
>> {
>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> @@ -1858,6 +1947,7 @@ void intel_color_init(struct intel_crtc *crtc)
>>
>> if (INTEL_GEN(dev_priv) >= 11) {
>> dev_priv->display.load_luts = icl_load_luts;
>> + dev_priv->display.read_luts = icl_read_luts;
>> } else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
>> dev_priv->display.load_luts = glk_load_luts;
>> dev_priv->display.read_luts = glk_read_luts;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1dc067fc57ab..06b6398b7d62 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -10584,6 +10584,12 @@ enum skl_power_gate {
>>
>> #define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
>> #define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
>> +#define PAL_PREC_MULTI_SEG_RED_LDW_MASK REG_GENMASK(29, 24)
>> +#define PAL_PREC_MULTI_SEG_RED_UDW_MASK REG_GENMASK(29, 20)
>> +#define PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
>> +#define PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
>> +#define PAL_PREC_MULTI_SEG_BLUE_LDW_MASK REG_GENMASK(9, 4)
>> +#define PAL_PREC_MULTI_SEG_BLUE_UDW_MASK REG_GENMASK(9, 0)
>>
>> #define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
>> _PAL_PREC_MULTI_SEG_INDEX_A, \
>> --
>> 2.23.0
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-10 10:50 ` Sharma, Swati2
@ 2019-10-15 12:34 ` Ville Syrjälä
2019-10-15 14:04 ` Sharma, Swati2
0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2019-10-15 12:34 UTC (permalink / raw)
To: Sharma, Swati2; +Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal
On Thu, Oct 10, 2019 at 04:20:04PM +0530, Sharma, Swati2 wrote:
> On 09-Oct-19 7:46 PM, Ville Syrjälä wrote:
> > On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
> >> For icl+, have hw read out to create hw blob of gamma
> >> lut values. icl+ platforms supports multi segmented gamma
> >> mode by default, add hw lut creation for this mode.
> >>
> >> This will be used to validate gamma programming using dsb
> >> (display state buffer) which is a tgl specific feature.
> >>
> >> Major change done-removal of readouts of coarse and fine segments
> >> because PAL_PREC_DATA register isn't giving propoer values.
> >> State checker limited only to "fine segment"
> >>
> >> v2: -readout code for multisegmented gamma has to come
> >> up with some intermediate entries that aren't preserved
> >> in hardware (Jani N)
> >> -linear interpolation (Ville)
> >> -moved common code to check gamma_enable to specific funcs,
> >> since icl doesn't support that
> >> v3: -use u16 instead of __u16 [Jani N]
> >> -used single lut [Jani N]
> >> -improved and more readable for loops [Jani N]
> >> -read values directly to actual locations and then fill gaps [Jani N]
> >> -moved cleaning to patch 1 [Jani N]
> >> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
> >> make it similar to icl_load_luts()
> >> -renamed icl_compute_interpolated_gamma_blob() to
> >> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
> >> v4: -removed interpolated func for creating gamma lut values
> >> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
> >> correctly
> >> v5: -added gamma_enable check inside read_luts()
> >>
> >> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
> >> drivers/gpu/drm/i915/i915_reg.h | 6 ++
> >> 2 files changed, 108 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> >> index fa44eb73d088..614e0ad386ca 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
> >> }
> >> }
> >>
> >> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
> >> +{
> >> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> >> + return 0;
> >> +
> >> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> >> + case GAMMA_MODE_MODE_8BIT:
> >> + return 8;
> >> + case GAMMA_MODE_MODE_10BIT:
> >> + return 10;
> >> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> >> + return 16;
> >> + default:
> >> + MISSING_CASE(crtc_state->gamma_mode);
> >> + return 0;
> >> + }
> >> +
> >> +}
> >> +
> >> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
> >> {
> >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
> >> else
> >> return i9xx_gamma_precision(crtc_state);
> >> } else {
> >> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >> + if (INTEL_GEN(dev_priv) >= 11)
> >> + return icl_gamma_precision(crtc_state);
> >> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >> return glk_gamma_precision(crtc_state);
> >> else if (IS_IRONLAKE(dev_priv))
> >> return ilk_gamma_precision(crtc_state);
> >> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
> >> return true;
> >> }
> >>
> >> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
> >> + struct drm_color_lut *lut2,
> >> + int lut_size, u32 err)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < 9; i++) {
> >> + if (!err_check(&lut1[i], &lut2[i], err))
> >> + return false;
> >> + }
> >> +
> >> + return true;
> >> +}
> >> +
> >> bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> struct drm_property_blob *blob2,
> >> u32 gamma_mode, u32 bit_precision)
> >> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> lut_size2 = drm_color_lut_size(blob2);
> >>
> >> /* check sw and hw lut size */
> >> - switch (gamma_mode) {
> >> - case GAMMA_MODE_MODE_8BIT:
> >> - case GAMMA_MODE_MODE_10BIT:
> >> - if (lut_size1 != lut_size2)
> >> - return false;
> >> - break;
> >> - default:
> >> - MISSING_CASE(gamma_mode);
> >> - return false;
> >> - }
> >> + if (lut_size1 != lut_size2)
> >> + return false;
> >>
> >> lut1 = blob1->data;
> >> lut2 = blob2->data;
> >> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> err = 0xffff >> bit_precision;
> >>
> >> /* check sw and hw lut entry to be equal */
> >> - switch (gamma_mode) {
> >> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
> >> case GAMMA_MODE_MODE_8BIT:
> >> case GAMMA_MODE_MODE_10BIT:
> >> if (!intel_color_lut_entry_equal(lut1, lut2,
> >> lut_size2, err))
> >> return false;
> >> break;
> >> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> >> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
> >> + lut_size2, err))
> >
> > I don't think you need a new function for that. Just pass 9 as the size
> > to intel_color_lut_entry_equal() ?
>
> I had made a separate function for multi-segmented gamma since there
> will be 3 loops for comparing superfine, fine and course segments which
> wont go with intel_lut_entry_equal() structure.
>
> Right now we are limiting to superfine segment only but in future we
> will add for other segments too (once we get fix from h/w)
>
> Func() should look like this. Actually there is no need to passing
> lut_size only in this function if we continue with this function only.
>
> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
> + struct drm_color_lut *lut2,
> + int lut_size, u32 err)
> +{
> + int i;
> +
> + for (i = 0; i < 9; i++) {
> + if (!err_check(&lut1[i], &lut2[i], err))
> + return false;
> + }
> +
> + for (i = 1; i < 257; i++) {
> + if (!err_check(&lut1[i * 8], &lut2[i * 8], err))
> + return false;
> + }
> +
> + for (i = 0; i < 256; i++) {
> + if (!err_check(&lut1[i * 8 * 128], &lut2[i * 8 * 128], err))
> + return false;
> + }
> +
> + return true;
> +}
> +
> Please suggest.
There's not much point in duplicating code until it's proven to
be required. Who knows when the hw gets fixed, maybe never.
>
> >
> > Hmm, should probably rename that to just intel_color_lut_equal() since
> > it checks the entire LUT (or at least the specified subset) and not
> > just a single entry...
>
> This will be fine for this segment but for other two segments it won't
> work. Right?
We could generalize it to take start+end+stride. Dunno if that would be
particularly beneficial though.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-15 12:34 ` Ville Syrjälä
@ 2019-10-15 14:04 ` Sharma, Swati2
2019-10-15 14:53 ` Ville Syrjälä
0 siblings, 1 reply; 15+ messages in thread
From: Sharma, Swati2 @ 2019-10-15 14:04 UTC (permalink / raw)
To: Ville Syrjälä
Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal
On 15-Oct-19 6:04 PM, Ville Syrjälä wrote:
> On Thu, Oct 10, 2019 at 04:20:04PM +0530, Sharma, Swati2 wrote:
>> On 09-Oct-19 7:46 PM, Ville Syrjälä wrote:
>>> On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
>>>> For icl+, have hw read out to create hw blob of gamma
>>>> lut values. icl+ platforms supports multi segmented gamma
>>>> mode by default, add hw lut creation for this mode.
>>>>
>>>> This will be used to validate gamma programming using dsb
>>>> (display state buffer) which is a tgl specific feature.
>>>>
>>>> Major change done-removal of readouts of coarse and fine segments
>>>> because PAL_PREC_DATA register isn't giving propoer values.
>>>> State checker limited only to "fine segment"
>>>>
>>>> v2: -readout code for multisegmented gamma has to come
>>>> up with some intermediate entries that aren't preserved
>>>> in hardware (Jani N)
>>>> -linear interpolation (Ville)
>>>> -moved common code to check gamma_enable to specific funcs,
>>>> since icl doesn't support that
>>>> v3: -use u16 instead of __u16 [Jani N]
>>>> -used single lut [Jani N]
>>>> -improved and more readable for loops [Jani N]
>>>> -read values directly to actual locations and then fill gaps [Jani N]
>>>> -moved cleaning to patch 1 [Jani N]
>>>> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
>>>> make it similar to icl_load_luts()
>>>> -renamed icl_compute_interpolated_gamma_blob() to
>>>> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
>>>> v4: -removed interpolated func for creating gamma lut values
>>>> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
>>>> correctly
>>>> v5: -added gamma_enable check inside read_luts()
>>>>
>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
>>>> drivers/gpu/drm/i915/i915_reg.h | 6 ++
>>>> 2 files changed, 108 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>>>> index fa44eb73d088..614e0ad386ca 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>>> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
>>>> }
>>>> }
>>>>
>>>> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
>>>> +{
>>>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>>>> + return 0;
>>>> +
>>>> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>>>> + case GAMMA_MODE_MODE_8BIT:
>>>> + return 8;
>>>> + case GAMMA_MODE_MODE_10BIT:
>>>> + return 10;
>>>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>>>> + return 16;
>>>> + default:
>>>> + MISSING_CASE(crtc_state->gamma_mode);
>>>> + return 0;
>>>> + }
>>>> +
>>>> +}
>>>> +
>>>> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
>>>> {
>>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>>> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
>>>> else
>>>> return i9xx_gamma_precision(crtc_state);
>>>> } else {
>>>> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>>>> + if (INTEL_GEN(dev_priv) >= 11)
>>>> + return icl_gamma_precision(crtc_state);
>>>> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>>>> return glk_gamma_precision(crtc_state);
>>>> else if (IS_IRONLAKE(dev_priv))
>>>> return ilk_gamma_precision(crtc_state);
>>>> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
>>>> return true;
>>>> }
>>>>
>>>> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
>>>> + struct drm_color_lut *lut2,
>>>> + int lut_size, u32 err)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < 9; i++) {
>>>> + if (!err_check(&lut1[i], &lut2[i], err))
>>>> + return false;
>>>> + }
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> bool intel_color_lut_equal(struct drm_property_blob *blob1,
>>>> struct drm_property_blob *blob2,
>>>> u32 gamma_mode, u32 bit_precision)
>>>> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>>>> lut_size2 = drm_color_lut_size(blob2);
>>>>
>>>> /* check sw and hw lut size */
>>>> - switch (gamma_mode) {
>>>> - case GAMMA_MODE_MODE_8BIT:
>>>> - case GAMMA_MODE_MODE_10BIT:
>>>> - if (lut_size1 != lut_size2)
>>>> - return false;
>>>> - break;
>>>> - default:
>>>> - MISSING_CASE(gamma_mode);
>>>> - return false;
>>>> - }
>>>> + if (lut_size1 != lut_size2)
>>>> + return false;
>>>>
>>>> lut1 = blob1->data;
>>>> lut2 = blob2->data;
>>>> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
>>>> err = 0xffff >> bit_precision;
>>>>
>>>> /* check sw and hw lut entry to be equal */
>>>> - switch (gamma_mode) {
>>>> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
>>>> case GAMMA_MODE_MODE_8BIT:
>>>> case GAMMA_MODE_MODE_10BIT:
>>>> if (!intel_color_lut_entry_equal(lut1, lut2,
>>>> lut_size2, err))
>>>> return false;
>>>> break;
>>>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>>>> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
>>>> + lut_size2, err))
>>>
>>> I don't think you need a new function for that. Just pass 9 as the size
>>> to intel_color_lut_entry_equal() ?
>>
>> I had made a separate function for multi-segmented gamma since there
>> will be 3 loops for comparing superfine, fine and course segments which
>> wont go with intel_lut_entry_equal() structure.
>>
>> Right now we are limiting to superfine segment only but in future we
>> will add for other segments too (once we get fix from h/w)
>>
>> Func() should look like this. Actually there is no need to passing
>> lut_size only in this function if we continue with this function only.
>>
>> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
>> + struct drm_color_lut *lut2,
>> + int lut_size, u32 err)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 9; i++) {
>> + if (!err_check(&lut1[i], &lut2[i], err))
>> + return false;
>> + }
>> +
>> + for (i = 1; i < 257; i++) {
>> + if (!err_check(&lut1[i * 8], &lut2[i * 8], err))
>> + return false;
>> + }
>> +
>> + for (i = 0; i < 256; i++) {
>> + if (!err_check(&lut1[i * 8 * 128], &lut2[i * 8 * 128], err))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> Please suggest.
>
> There's not much point in duplicating code until it's proven to
> be required. Who knows when the hw gets fixed, maybe never.
Now, got your point. Thanks!
>
>>
>>>
>>> Hmm, should probably rename that to just intel_color_lut_equal() since
>>> it checks the entire LUT (or at least the specified subset) and not
>>> just a single entry...
Already intel_color_lut_equal() function exits, should i rename that to
intel_color_blob_equal() or intel_color_gamma_blob()? and make this func
intel_color_lut_entry_equal() to intel_color_lut_equal() as suggested by
you?
Please comment!
>>
>> This will be fine for this segment but for other two segments it won't
>> work. Right?
>
> We could generalize it to take start+end+stride. Dunno if that would be
> particularly beneficial though.
>
--
~Swati Sharma
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-15 14:04 ` Sharma, Swati2
@ 2019-10-15 14:53 ` Ville Syrjälä
0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2019-10-15 14:53 UTC (permalink / raw)
To: Sharma, Swati2; +Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal
On Tue, Oct 15, 2019 at 07:34:00PM +0530, Sharma, Swati2 wrote:
> On 15-Oct-19 6:04 PM, Ville Syrjälä wrote:
> > On Thu, Oct 10, 2019 at 04:20:04PM +0530, Sharma, Swati2 wrote:
> >> On 09-Oct-19 7:46 PM, Ville Syrjälä wrote:
> >>> On Wed, Oct 09, 2019 at 12:25:41PM +0530, Swati Sharma wrote:
> >>>> For icl+, have hw read out to create hw blob of gamma
> >>>> lut values. icl+ platforms supports multi segmented gamma
> >>>> mode by default, add hw lut creation for this mode.
> >>>>
> >>>> This will be used to validate gamma programming using dsb
> >>>> (display state buffer) which is a tgl specific feature.
> >>>>
> >>>> Major change done-removal of readouts of coarse and fine segments
> >>>> because PAL_PREC_DATA register isn't giving propoer values.
> >>>> State checker limited only to "fine segment"
> >>>>
> >>>> v2: -readout code for multisegmented gamma has to come
> >>>> up with some intermediate entries that aren't preserved
> >>>> in hardware (Jani N)
> >>>> -linear interpolation (Ville)
> >>>> -moved common code to check gamma_enable to specific funcs,
> >>>> since icl doesn't support that
> >>>> v3: -use u16 instead of __u16 [Jani N]
> >>>> -used single lut [Jani N]
> >>>> -improved and more readable for loops [Jani N]
> >>>> -read values directly to actual locations and then fill gaps [Jani N]
> >>>> -moved cleaning to patch 1 [Jani N]
> >>>> -renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
> >>>> make it similar to icl_load_luts()
> >>>> -renamed icl_compute_interpolated_gamma_blob() to
> >>>> icl_compute_interpolated_gamma_lut_values() more sensible, I guess
> >>>> v4: -removed interpolated func for creating gamma lut values
> >>>> -removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
> >>>> correctly
> >>>> v5: -added gamma_enable check inside read_luts()
> >>>>
> >>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
> >>>> drivers/gpu/drm/i915/i915_reg.h | 6 ++
> >>>> 2 files changed, 108 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> index fa44eb73d088..614e0ad386ca 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> @@ -1477,6 +1477,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
> >>>> }
> >>>> }
> >>>>
> >>>> +static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
> >>>> +{
> >>>> + if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> >>>> + return 0;
> >>>> +
> >>>> + switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> >>>> + case GAMMA_MODE_MODE_8BIT:
> >>>> + return 8;
> >>>> + case GAMMA_MODE_MODE_10BIT:
> >>>> + return 10;
> >>>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> >>>> + return 16;
> >>>> + default:
> >>>> + MISSING_CASE(crtc_state->gamma_mode);
> >>>> + return 0;
> >>>> + }
> >>>> +
> >>>> +}
> >>>> +
> >>>> int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
> >>>> {
> >>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >>>> @@ -1488,7 +1507,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
> >>>> else
> >>>> return i9xx_gamma_precision(crtc_state);
> >>>> } else {
> >>>> - if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >>>> + if (INTEL_GEN(dev_priv) >= 11)
> >>>> + return icl_gamma_precision(crtc_state);
> >>>> + else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >>>> return glk_gamma_precision(crtc_state);
> >>>> else if (IS_IRONLAKE(dev_priv))
> >>>> return ilk_gamma_precision(crtc_state);
> >>>> @@ -1519,6 +1540,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
> >>>> return true;
> >>>> }
> >>>>
> >>>> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
> >>>> + struct drm_color_lut *lut2,
> >>>> + int lut_size, u32 err)
> >>>> +{
> >>>> + int i;
> >>>> +
> >>>> + for (i = 0; i < 9; i++) {
> >>>> + if (!err_check(&lut1[i], &lut2[i], err))
> >>>> + return false;
> >>>> + }
> >>>> +
> >>>> + return true;
> >>>> +}
> >>>> +
> >>>> bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >>>> struct drm_property_blob *blob2,
> >>>> u32 gamma_mode, u32 bit_precision)
> >>>> @@ -1537,16 +1572,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >>>> lut_size2 = drm_color_lut_size(blob2);
> >>>>
> >>>> /* check sw and hw lut size */
> >>>> - switch (gamma_mode) {
> >>>> - case GAMMA_MODE_MODE_8BIT:
> >>>> - case GAMMA_MODE_MODE_10BIT:
> >>>> - if (lut_size1 != lut_size2)
> >>>> - return false;
> >>>> - break;
> >>>> - default:
> >>>> - MISSING_CASE(gamma_mode);
> >>>> - return false;
> >>>> - }
> >>>> + if (lut_size1 != lut_size2)
> >>>> + return false;
> >>>>
> >>>> lut1 = blob1->data;
> >>>> lut2 = blob2->data;
> >>>> @@ -1554,13 +1581,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >>>> err = 0xffff >> bit_precision;
> >>>>
> >>>> /* check sw and hw lut entry to be equal */
> >>>> - switch (gamma_mode) {
> >>>> + switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
> >>>> case GAMMA_MODE_MODE_8BIT:
> >>>> case GAMMA_MODE_MODE_10BIT:
> >>>> if (!intel_color_lut_entry_equal(lut1, lut2,
> >>>> lut_size2, err))
> >>>> return false;
> >>>> break;
> >>>> + case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> >>>> + if (!intel_color_lut_entry_multi_equal(lut1, lut2,
> >>>> + lut_size2, err))
> >>>
> >>> I don't think you need a new function for that. Just pass 9 as the size
> >>> to intel_color_lut_entry_equal() ?
> >>
> >> I had made a separate function for multi-segmented gamma since there
> >> will be 3 loops for comparing superfine, fine and course segments which
> >> wont go with intel_lut_entry_equal() structure.
> >>
> >> Right now we are limiting to superfine segment only but in future we
> >> will add for other segments too (once we get fix from h/w)
> >>
> >> Func() should look like this. Actually there is no need to passing
> >> lut_size only in this function if we continue with this function only.
> >>
> >> +static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
> >> + struct drm_color_lut *lut2,
> >> + int lut_size, u32 err)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < 9; i++) {
> >> + if (!err_check(&lut1[i], &lut2[i], err))
> >> + return false;
> >> + }
> >> +
> >> + for (i = 1; i < 257; i++) {
> >> + if (!err_check(&lut1[i * 8], &lut2[i * 8], err))
> >> + return false;
> >> + }
> >> +
> >> + for (i = 0; i < 256; i++) {
> >> + if (!err_check(&lut1[i * 8 * 128], &lut2[i * 8 * 128], err))
> >> + return false;
> >> + }
> >> +
> >> + return true;
> >> +}
> >> +
> >> Please suggest.
> >
> > There's not much point in duplicating code until it's proven to
> > be required. Who knows when the hw gets fixed, maybe never.
> Now, got your point. Thanks!
> >
> >>
> >>>
> >>> Hmm, should probably rename that to just intel_color_lut_equal() since
> >>> it checks the entire LUT (or at least the specified subset) and not
> >>> just a single entry...
> Already intel_color_lut_equal() function exits, should i rename that to
> intel_color_blob_equal() or intel_color_gamma_blob()? and make this func
> intel_color_lut_entry_equal() to intel_color_lut_equal() as suggested by
> you?
Hmm. In that case I think just s/entry/entries/ would make sense.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts()
2019-10-04 8:26 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
@ 2019-10-04 8:26 ` Swati Sharma
0 siblings, 0 replies; 15+ messages in thread
From: Swati Sharma @ 2019-10-04 8:26 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, ankit.k.nautiyal
For icl+, have hw read out to create hw blob of gamma
lut values. icl+ platforms supports multi segmented gamma
mode by default, add hw lut creation for this mode.
This will be used to validate gamma programming using dsb
(display state buffer) which is a tgl specific feature.
Major change done-removal of readouts of coarse and fine segments
because PAL_PREC_DATA register isn't giving propoer values.
State checker limited only to "fine segment"
v2: -readout code for multisegmented gamma has to come
up with some intermediate entries that aren't preserved
in hardware (Jani N)
-linear interpolation (Ville)
-moved common code to check gamma_enable to specific funcs,
since icl doesn't support that
v3: -use u16 instead of __u16 [Jani N]
-used single lut [Jani N]
-improved and more readable for loops [Jani N]
-read values directly to actual locations and then fill gaps [Jani N]
-moved cleaning to patch 1 [Jani N]
-renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
make it similar to icl_load_luts()
-renamed icl_compute_interpolated_gamma_blob() to
icl_compute_interpolated_gamma_lut_values() more sensible, I guess
v4: -removed interpolated func for creating gamma lut values
-removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
correctly
v5: -added gamma_enable check inside read_luts()
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 114 ++++++++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 6 ++
2 files changed, 108 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 44ce75f051ad..168e9daae3de 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1484,6 +1484,25 @@ static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
}
}
+static int icl_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+ if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
+ return 0;
+
+ switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
+ case GAMMA_MODE_MODE_8BIT:
+ return 8;
+ case GAMMA_MODE_MODE_10BIT:
+ return 10;
+ case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
+ return 16;
+ default:
+ MISSING_CASE(crtc_state->gamma_mode);
+ return 0;
+ }
+
+}
+
int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -1495,7 +1514,9 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
else
return i9xx_gamma_precision(crtc_state);
} else {
- if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ return icl_gamma_precision(crtc_state);
+ else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
return glk_gamma_precision(crtc_state);
else if (IS_IRONLAKE(dev_priv))
return ilk_gamma_precision(crtc_state);
@@ -1526,6 +1547,20 @@ static bool intel_color_lut_entry_equal(struct drm_color_lut *lut1,
return true;
}
+static bool intel_color_lut_entry_multi_equal(struct drm_color_lut *lut1,
+ struct drm_color_lut *lut2,
+ int lut_size, u32 err)
+{
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ if (!err_check(&lut1[i], &lut2[i], err))
+ return false;
+ }
+
+ return true;
+}
+
bool intel_color_lut_equal(struct drm_property_blob *blob1,
struct drm_property_blob *blob2,
u32 gamma_mode, u32 bit_precision)
@@ -1544,16 +1579,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
lut_size2 = drm_color_lut_size(blob2);
/* check sw and hw lut size */
- switch (gamma_mode) {
- case GAMMA_MODE_MODE_8BIT:
- case GAMMA_MODE_MODE_10BIT:
- if (lut_size1 != lut_size2)
- return false;
- break;
- default:
- MISSING_CASE(gamma_mode);
- return false;
- }
+ if (lut_size1 != lut_size2)
+ return false;
lut1 = blob1->data;
lut2 = blob2->data;
@@ -1561,13 +1588,18 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
err = 0xffff >> bit_precision;
/* check sw and hw lut entry to be equal */
- switch (gamma_mode) {
+ switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
case GAMMA_MODE_MODE_8BIT:
case GAMMA_MODE_MODE_10BIT:
if (!intel_color_lut_entry_equal(lut1, lut2,
lut_size2, err))
return false;
break;
+ case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
+ if (!intel_color_lut_entry_multi_equal(lut1, lut2,
+ lut_size2, err))
+ return false;
+ break;
default:
MISSING_CASE(gamma_mode);
return false;
@@ -1828,6 +1860,63 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
}
+static struct drm_property_blob *
+icl_read_lut_multi_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);
+ int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ enum pipe pipe = crtc->pipe;
+ struct drm_property_blob *blob;
+ struct drm_color_lut *blob_data;
+ u32 i, val1, val2;
+
+ blob = drm_property_create_blob(&dev_priv->drm,
+ sizeof(struct drm_color_lut) * lut_size,
+ NULL);
+ if (IS_ERR(blob))
+ return NULL;
+
+ blob_data = blob->data;
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+
+ for (i = 0; i < 9; i++) {
+ val1 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
+ val2 = I915_READ(PREC_PAL_MULTI_SEG_DATA(pipe));
+
+ blob_data[i].red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, val1);
+ blob_data[i].green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, val1);
+ blob_data[i].blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, val2) << 6 |
+ REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, val1);
+ }
+
+ /*
+ * FIXME readouts from PAL_PREC_DATA register aren't giving correct values
+ * in the case of fine and coarse segments. Restricting readouts only for
+ * super fine segment as of now.
+ */
+
+ return blob;
+}
+
+static void icl_read_luts(struct intel_crtc_state *crtc_state)
+{
+ if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
+ return;
+
+ if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+ GAMMA_MODE_MODE_8BIT)
+ crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+ else if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+ GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED)
+ crtc_state->base.gamma_lut = icl_read_lut_multi_segment(crtc_state);
+ else
+ crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+}
+
void intel_color_init(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1871,6 +1960,7 @@ void intel_color_init(struct intel_crtc *crtc)
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.load_luts = icl_load_luts;
+ dev_priv->display.read_luts = icl_read_luts;
} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
dev_priv->display.load_luts = glk_load_luts;
dev_priv->display.read_luts = glk_read_luts;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 058aa5ca8b73..78766b378c0f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10571,6 +10571,12 @@ enum skl_power_gate {
#define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
#define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
+#define PAL_PREC_MULTI_SEG_RED_LDW_MASK REG_GENMASK(29, 24)
+#define PAL_PREC_MULTI_SEG_RED_UDW_MASK REG_GENMASK(29, 20)
+#define PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
+#define PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
+#define PAL_PREC_MULTI_SEG_BLUE_LDW_MASK REG_GENMASK(9, 4)
+#define PAL_PREC_MULTI_SEG_BLUE_UDW_MASK REG_GENMASK(9, 0)
#define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
_PAL_PREC_MULTI_SEG_INDEX_A, \
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-10-15 14:53 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 6:55 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
2019-10-09 6:55 ` [PATCH 1/4] [v3] drm/i915/color: fix broken gamma state-checker during boot Swati Sharma
2019-10-09 6:55 ` [PATCH 2/4] [v2] drm/i915/color: move check of gamma_enable to specific func/platform Swati Sharma
2019-10-09 6:55 ` [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts() Swati Sharma
2019-10-09 14:16 ` Ville Syrjälä
2019-10-10 10:50 ` Sharma, Swati2
2019-10-15 12:34 ` Ville Syrjälä
2019-10-15 14:04 ` Sharma, Swati2
2019-10-15 14:53 ` Ville Syrjälä
2019-10-15 7:59 ` Jani Nikula
2019-10-09 6:55 ` [PATCH 4/4] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs Swati Sharma
2019-10-09 7:47 ` ✗ Fi.CI.CHECKPATCH: warning for fix broken state checker and enable state checker for icl+ (rev2) Patchwork
2019-10-09 8:10 ` ✓ Fi.CI.BAT: success " Patchwork
2019-10-09 11:57 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-10-04 8:26 [PATCH 0/4] fix broken state checker and enable state checker for icl+ Swati Sharma
2019-10-04 8:26 ` [PATCH 3/4] [v5] drm/i915/color: Extract icl_read_luts() Swati Sharma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.