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 <[email protected]>
---
 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

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to