[Intel-gfx] [PATCH] drm/i915: adding state checker for gamma lut values
Swati Sharma
swati2.sharma at intel.com
Thu Mar 28 06:33:48 UTC 2019
Added state checker to validate gamma_lut values. This
reads hardware state, and compares the originally requested
state to the state read from hardware.
v1: -Implementation done for legacy platforms (removed all the placeholders) (Jani)
-Added inverse function of drm_color_lut_extract to convert hardware
read values back to user values (code written by Jani)
-Renamed get_config() to color_config() (Jani)
-Placed all platform specific shifts and masks in i915_reg.h (Jani)
-Renamed i9xx_get_config to i9xx_color_config and all related
functions (Jani)
-Removed debug logs from compare function (Jani)
-Renamed intel_compare_blob to intel_compare_lut and added platform specific
bit precision of the readout into the function (Jani)
-Renamed macro PIPE_CONF_CHECK_BLOB to PIPE_CONF_CHECK_COLOR_LUT (Jani)
-Added check if blobs can be NULL (Jani)
-Added function in intel_color.c that returns the bit precision (Jani),
didn't add in device info since its gonna die soon (Ville)
TODO:
-Add a separate function to log errors at the higher level
-Haven't moved intel_compare_lut() from intel_display.c to intel_color.c
Since all the comparison functions are placed in intel_display, isn't
it the right place (or) we want to move to consolidate color related functions
together? Opinion? Please correct me if I am wrong.
-Optimizations and refractoring
Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 12 +++
drivers/gpu/drm/i915/intel_color.c | 186 +++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_display.c | 48 +++++++++
drivers/gpu/drm/i915/intel_drv.h | 2 +
5 files changed, 243 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c4ffe19..b422ea6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -334,6 +334,7 @@ struct drm_i915_display_funcs {
* involved with the same commit.
*/
void (*load_luts)(const struct intel_crtc_state *crtc_state);
+ void (*color_config)(struct intel_crtc_state *crtc_state);
};
#define CSR_VERSION(major, minor) ((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c0cd7a8..2813033 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7156,6 +7156,10 @@ enum {
#define _LGC_PALETTE_B 0x4a800
#define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
+#define LGC_PALETTE_RED_MASK (0xFF << 16)
+#define LGC_PALETTE_GREEN_MASK (0xFF << 8)
+#define LGC_PALETTE_BLUE_MASK (0xFF << 0)
+
#define _GAMMA_MODE_A 0x4a480
#define _GAMMA_MODE_B 0x4ac80
#define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
@@ -10102,6 +10106,10 @@ enum skl_power_gate {
#define PRE_CSC_GAMC_INDEX(pipe) _MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
#define PRE_CSC_GAMC_DATA(pipe) _MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
+#define PREC_PAL_DATA_RED_MASK (0x3FF << 20)
+#define PREC_PAL_DATA_GREEN_MASK (0x3FF << 10)
+#define PREC_PAL_DATA_BLUE_MASK (0x3FF << 0)
+
/* pipe CSC & degamma/gamma LUTs on CHV */
#define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900)
#define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904)
@@ -10133,6 +10141,10 @@ enum skl_power_gate {
#define CGM_PIPE_GAMMA(pipe, i, w) _MMIO(_PIPE(pipe, _CGM_PIPE_A_GAMMA, _CGM_PIPE_B_GAMMA) + (i) * 8 + (w) * 4)
#define CGM_PIPE_MODE(pipe) _MMIO_PIPE(pipe, _CGM_PIPE_A_MODE, _CGM_PIPE_B_MODE)
+#define CGM_PIPE_GAMMA_RED_MASK (0x3FF << 0)
+#define CGM_PIPE_GAMMA_GREEN_MASK (0x3FF << 16)
+#define CGM_PIPE_GAMMA_BLUE_MASK (0x3FF << 0)
+
/* MIPI DSI registers */
#define _MIPI_PORT(port, a, c) (((port) == PORT_A) ? a : c) /* ports A and C only */
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index da7a07d..bd4f1b1 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -679,6 +679,172 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
dev_priv->display.load_luts(crtc_state);
}
+u32 intel_color_bit_precision(struct drm_i915_private *dev_priv)
+{
+ if (INTEL_GEN(dev_priv) >= 9)
+ return 10;
+ else
+ return 8;
+}
+
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+{
+ u32 max = 0xFFFF >> (16 - bit_precision);
+
+ val = clamp_val(val, 0, max);
+
+ if (bit_precision < 16)
+ val <<= 16 - bit_precision;
+
+ return val;
+}
+
+static void i9xx_internal_gamma_config(struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_device *dev = crtc->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 i, tmp;
+ enum pipe pipe = crtc->pipe;
+ struct drm_property_blob *blob = NULL;
+ struct drm_color_lut *blob_data;
+
+ blob = drm_property_create_blob(dev,
+ sizeof(struct drm_color_lut) * 256,
+ NULL);
+ if (IS_ERR(blob))
+ return;
+
+ blob_data = blob->data;
+
+ for (i = 0; i < 256; i++) {
+ if (HAS_GMCH(dev_priv))
+ tmp = I915_READ(PALETTE(pipe, i));
+ else
+ tmp = I915_READ(LGC_PALETTE(pipe, i));
+ blob_data[i].red = intel_color_lut_pack((tmp & LGC_PALETTE_RED_MASK) >> 16, 8);
+ blob_data[i].green = intel_color_lut_pack((tmp & LGC_PALETTE_GREEN_MASK) >> 8, 8);
+ blob_data[i].blue = intel_color_lut_pack((tmp & LGC_PALETTE_BLUE_MASK), 8);
+ }
+
+ crtc_state->base.gamma_lut = blob;
+}
+
+static void i9xx_color_config(struct intel_crtc_state *crtc_state)
+{
+ i9xx_internal_gamma_config(crtc_state);
+}
+
+static void cherryview_gamma_config(struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_device *dev = crtc->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 i, tmp, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ enum pipe pipe = crtc->pipe;
+ struct drm_property_blob *blob = NULL;
+ struct drm_color_lut *blob_data;
+
+ blob = drm_property_create_blob(dev,
+ sizeof(struct drm_color_lut) * 256,
+ NULL);
+ if (IS_ERR(blob))
+ return;
+
+ blob_data = blob->data;
+
+ for (i = 0; i < lut_size; i++) {
+ tmp = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
+ blob_data[i].green = intel_color_lut_pack((tmp & CGM_PIPE_GAMMA_GREEN_MASK) >> 16, 10);
+ blob_data[i].blue = intel_color_lut_pack((tmp & CGM_PIPE_GAMMA_BLUE_MASK), 10);
+ tmp = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
+ blob_data[i].red = intel_color_lut_pack((tmp & CGM_PIPE_GAMMA_RED_MASK), 10);
+ }
+
+ crtc_state->base.gamma_lut = blob;
+}
+
+static void bdw_gamma_config(struct intel_crtc_state *crtc_state, u32 offset)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_device *dev = crtc->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 i, tmp, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ enum pipe pipe = crtc->pipe;
+ struct drm_property_blob *blob = NULL;
+ struct drm_color_lut *blob_data;
+
+ WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
+
+ I915_WRITE(PREC_PAL_INDEX(pipe),
+ (offset ? PAL_PREC_SPLIT_MODE : 0) |
+ PAL_PREC_AUTO_INCREMENT |
+ offset);
+
+ blob = drm_property_create_blob(dev,
+ sizeof(struct drm_color_lut) * lut_size,
+ NULL);
+ if (IS_ERR(blob))
+ return;
+
+ blob_data = blob->data;
+
+ for (i = 0; i < lut_size; i++) {
+ tmp = I915_READ(PREC_PAL_DATA(pipe));
+ blob_data[i].red = intel_color_lut_pack((tmp & PREC_PAL_DATA_RED_MASK) >> 20, 10);
+ blob_data[i].green = intel_color_lut_pack((tmp & PREC_PAL_DATA_GREEN_MASK) >> 10, 10);
+ blob_data[i].blue = intel_color_lut_pack((tmp & PREC_PAL_DATA_BLUE_MASK), 10);
+ }
+
+ I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+
+ crtc_state->base.gamma_lut = blob;
+}
+
+static void cherryview_color_config(struct intel_crtc_state *crtc_state)
+{
+ if (crtc_state_is_legacy_gamma(crtc_state))
+ i9xx_color_config(crtc_state);
+ else
+ cherryview_gamma_config(crtc_state);
+}
+
+static void broadwell_color_config(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);
+
+ if (crtc_state_is_legacy_gamma(crtc_state))
+ i9xx_color_config(crtc_state);
+ else
+ bdw_gamma_config(crtc_state,
+ INTEL_INFO(dev_priv)->color.degamma_lut_size);
+}
+
+static void glk_color_config(struct intel_crtc_state *crtc_state)
+{
+ if (crtc_state_is_legacy_gamma(crtc_state))
+ i9xx_color_config(crtc_state);
+ else
+ bdw_gamma_config(crtc_state, 0);
+}
+
+static void icl_color_config(struct intel_crtc_state *crtc_state)
+{
+ if (crtc_state_is_legacy_gamma(crtc_state))
+ i9xx_color_config(crtc_state);
+ else
+ bdw_gamma_config(crtc_state, 0);
+}
+
+void intel_color_config(struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
+ dev_priv->display.color_config(crtc_state);
+}
+
void intel_color_commit(const struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
@@ -834,21 +1000,29 @@ void intel_color_init(struct intel_crtc *crtc)
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
if (HAS_GMCH(dev_priv)) {
- if (IS_CHERRYVIEW(dev_priv))
+ if (IS_CHERRYVIEW(dev_priv)) {
dev_priv->display.load_luts = cherryview_load_luts;
- else
+ dev_priv->display.color_config = cherryview_color_config;
+ } else {
dev_priv->display.load_luts = i9xx_load_luts;
+ dev_priv->display.color_config = i9xx_color_config;
+ }
dev_priv->display.color_commit = i9xx_color_commit;
} else {
- if (IS_ICELAKE(dev_priv))
+ if (IS_ICELAKE(dev_priv)) {
dev_priv->display.load_luts = icl_load_luts;
- else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ dev_priv->display.color_config = icl_color_config;
+ } else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
dev_priv->display.load_luts = glk_load_luts;
- else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+ dev_priv->display.color_config = glk_color_config;
+ } else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
dev_priv->display.load_luts = broadwell_load_luts;
- else
+ dev_priv->display.color_config = broadwell_color_config;
+ } else {
dev_priv->display.load_luts = i9xx_load_luts;
+ dev_priv->display.color_config = i9xx_color_config;
+ }
if (INTEL_GEN(dev_priv) >= 9)
dev_priv->display.color_commit = skl_color_commit;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 963b4bd..31bb652 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8212,6 +8212,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
i9xx_get_pipe_color_config(pipe_config);
+ intel_color_config(pipe_config);
+
if (INTEL_GEN(dev_priv) < 4)
pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
@@ -9284,6 +9286,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
i9xx_get_pipe_color_config(pipe_config);
+ intel_color_config(pipe_config);
+
if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
struct intel_shared_dpll *pll;
enum intel_dpll_id pll_id;
@@ -9932,6 +9936,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
i9xx_get_pipe_color_config(pipe_config);
}
+ intel_color_config(pipe_config);
+
power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
WARN_ON(power_domain_mask & BIT_ULL(power_domain));
@@ -11990,6 +11996,36 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
return false;
}
+static bool intel_compare_color_lut(struct drm_property_blob *blob1,
+ struct drm_property_blob *blob2,
+ u32 bit_precision)
+{
+ struct drm_color_lut *sw_lut = blob1->data;
+ struct drm_color_lut *hw_lut = blob2->data;
+ u32 err = 0xFFFF >> bit_precision;
+ int sw_lut_size, hw_lut_size;
+ int i;
+
+ sw_lut_size = drm_color_lut_size(blob1);
+ hw_lut_size = drm_color_lut_size(blob2);
+
+ if (IS_ERR(blob1) || IS_ERR(blob2))
+ return false;
+
+ if (sw_lut_size != hw_lut_size)
+ return false;
+
+ for (i = 0; i < sw_lut_size; i++) {
+ if (((abs((long)hw_lut[i].red - sw_lut[i].red)) >= err) ||
+ ((abs((long)hw_lut[i].blue - sw_lut[i].blue)) >= err) ||
+ ((abs((long)hw_lut[i].green - sw_lut[i].green)) >= err)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
static bool
intel_pipe_config_compare(struct drm_i915_private *dev_priv,
struct intel_crtc_state *current_config,
@@ -11997,6 +12033,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
bool adjust)
{
bool ret = true;
+ u32 bit_precision;
bool fixup_inherited = adjust &&
(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
@@ -12148,6 +12185,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
} \
} while (0)
+#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
+ if (!intel_compare_color_lut(current_config->name, pipe_config->name, bit_precision)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "hw_state doesn't match sw_state\n"); \
+ ret = false; \
+ } \
+} while (0)
+
#define PIPE_CONF_QUIRK(quirk) \
((current_config->quirks | pipe_config->quirks) & (quirk))
@@ -12287,6 +12332,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
PIPE_CONF_CHECK_INFOFRAME(spd);
PIPE_CONF_CHECK_INFOFRAME(hdmi);
+ bit_precision = intel_color_bit_precision(dev_priv);
+ PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bit_precision);
+
#undef PIPE_CONF_CHECK_X
#undef PIPE_CONF_CHECK_I
#undef PIPE_CONF_CHECK_BOOL
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 40ebc94..6a89d2d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2512,6 +2512,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
int intel_color_check(struct intel_crtc_state *crtc_state);
void intel_color_commit(const struct intel_crtc_state *crtc_state);
void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
+void intel_color_config(struct intel_crtc_state *crtc_state);
+u32 intel_color_bit_precision(struct drm_i915_private *dev_priv);
/* intel_lspcon.c */
bool lspcon_init(struct intel_digital_port *intel_dig_port);
--
1.9.1
More information about the Intel-gfx
mailing list