From patchwork Thu Mar 28 06:33:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Sharma, Swati2" X-Patchwork-Id: 10874493 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E94F81575 for ; Thu, 28 Mar 2019 06:39:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D2BDE287A1 for ; Thu, 28 Mar 2019 06:39:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C55A12880A; Thu, 28 Mar 2019 06:39:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B907C28BE1 for ; Thu, 28 Mar 2019 06:39:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9DE0C6E2DC; Thu, 28 Mar 2019 06:39:42 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id A4EE0895EE for ; Thu, 28 Mar 2019 06:39:40 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Mar 2019 23:39:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,278,1549958400"; d="scan'208";a="159122544" Received: from genxfsim-shark-bay-client-platform.iind.intel.com ([10.223.74.155]) by fmsmga001.fm.intel.com with ESMTP; 27 Mar 2019 23:39:37 -0700 From: Swati Sharma To: intel-gfx@lists.freedesktop.org Date: Thu, 28 Mar 2019 12:03:48 +0530 Message-Id: <1553754828-14696-1-git-send-email-swati2.sharma@intel.com> X-Mailer: git-send-email 1.9.1 Subject: [Intel-gfx] [PATCH] drm/i915: adding state checker for gamma lut values X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jani.nikula@intel.com, daniel.vetter@ffwll.ch, ankit.k.nautiyal@intel.com MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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);