From patchwork Fri Mar 15 07:54:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Sharma, Swati2" X-Patchwork-Id: 10854207 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 321E61575 for ; Fri, 15 Mar 2019 08:00:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1079828CEC for ; Fri, 15 Mar 2019 08:00:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 048232A895; Fri, 15 Mar 2019 08:00:11 +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 46BB528CEC for ; Fri, 15 Mar 2019 08:00:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 297136E312; Fri, 15 Mar 2019 08:00:09 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id C4A166E322 for ; Fri, 15 Mar 2019 08:00:07 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Mar 2019 01:00:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,481,1544515200"; d="scan'208";a="327537261" Received: from genxfsim-shark-bay-client-platform.iind.intel.com ([10.223.74.155]) by fmsmga006.fm.intel.com with ESMTP; 15 Mar 2019 01:00:02 -0700 From: swati2.sharma@intel.com To: intel-gfx@lists.freedesktop.org Date: Fri, 15 Mar 2019 13:24:29 +0530 Message-Id: <1552636469-7479-1-git-send-email-swati2.sharma@intel.com> X-Mailer: git-send-email 1.9.1 Subject: [Intel-gfx] [RFC] 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Swati Sharma Added state checker to validate gamma_lut values. This reads hardware state, and compares the originally requested state to the state read from hardware. This implementation can be used for Gen9+ platforms, I haven't implemented it for legacy platforms. Just want to get feedback if this is the right approach to follow. Also, inverse function of drm_color_lut_extract is missing to convert hardware read values back to user values. Thinking for that. I have added all "TODOs" and "Placeholders". Another approach: Storing "word" to be written into hardware in dev_priv and instead of comparing blob, comparing "word"? In dev_priv, only pointer will be there (something like *gamma_word). For this too, I will send a patch to make it more clear. Signed-off-by: Swati Sharma --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_color.c | 127 +++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_display.c | 50 ++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + 4 files changed, 173 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c4ffe19..b41bfaa 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 (*get_config)(struct intel_crtc_state *crtc_state); }; #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index da7a07d..a515e9f 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -74,6 +74,11 @@ #define ILK_CSC_COEFF_1_0 \ ((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8)) +/* Mask to extract RGB values from registers */ +#define COLOR_BLUE_MASK 0x000003FF /* 9:0 */ +#define COLOR_GREEN_MASK 0x000FFC00 /* 19:10 */ +#define COLOR_RED_MASK 0x3FF00000 /* 29:20 */ + static bool lut_is_legacy(const struct drm_property_blob *lut) { return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH; @@ -672,6 +677,97 @@ static void cherryview_load_luts(const struct intel_crtc_state *crtc_state) i9xx_load_luts_internal(crtc_state, NULL); } +static void bdw_get_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)); + /* + * TODO: convert RGB value read from register into corresponding user value using + * some wrapper like drm_color_lut_put() (or) intel_color_lut_put() so that it + * can be compared later. + */ + blob_data[i].red = (tmp & COLOR_RED_MASK) >> 20; + blob_data[i].green = (tmp & COLOR_GREEN_MASK) >> 10; + blob_data[i].blue = tmp & COLOR_BLUE_MASK; + } + + I915_WRITE(PREC_PAL_INDEX(pipe), 0); + + crtc_state->base.gamma_lut = blob; +} + +static void i9xx_get_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)); + /* + * TODO: convert RGB value read from register into corresponding user value using + * some wrapper like drm_color_lut_put() (or) intel_color_lut_put() so that it + * can be compared later. + */ + blob_data[i].red = (tmp & COLOR_RED_MASK) >> 20; + blob_data[i].green = (tmp & COLOR_GREEN_MASK) >> 10; + blob_data[i].blue = (tmp & COLOR_BLUE_MASK); + } + + crtc_state->base.gamma_lut = blob; +} + +static void icl_get_config(struct intel_crtc_state *crtc_state) +{ + /* + * TODO: placeholder for degamma validation + * glk_get_degamma_config(crtc_state, 0); + */ + + if (crtc_state_is_legacy_gamma(crtc_state)) + i9xx_get_config(crtc_state); + else + bdw_get_gamma_config(crtc_state, 0); +} + void intel_color_load_luts(const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); @@ -679,6 +775,13 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state) dev_priv->display.load_luts(crtc_state); } +void intel_color_get_config(struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + + dev_priv->display.get_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); @@ -833,22 +936,34 @@ void intel_color_init(struct intel_crtc *crtc) drm_mode_crtc_set_gamma_size(&crtc->base, 256); + /* + * TODO: In the following if ladder, kept placeholders + * for extending lut validation for legacy platforms. + */ 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.get_config = cherryview_get_config; */ + } else { dev_priv->display.load_luts = i9xx_load_luts; + dev_priv->display.get_config = i9xx_get_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.get_config = icl_get_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.get_config = glk_get_config; */ + } else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { dev_priv->display.load_luts = broadwell_load_luts; - else + /* dev_priv->display.get_config = broadwell_get_config; */ + } else { dev_priv->display.load_luts = i9xx_load_luts; + dev_priv->display.get_config = i9xx_get_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..165d797 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8212,6 +8212,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc, i9xx_get_pipe_color_config(pipe_config); + /* + * TODO: Yet to implement for legacy platforms + * intel_color_get_config(pipe_config); + */ + if (INTEL_GEN(dev_priv) < 4) pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE; @@ -9284,6 +9289,11 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, i9xx_get_pipe_color_config(pipe_config); + /* + * TODO: Yet to implement for legacy platforms + * intel_color_get_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 +9942,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, i9xx_get_pipe_color_config(pipe_config); } + intel_color_get_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 +12002,34 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) return false; } +static bool intel_compare_blob(struct drm_property_blob *blob1, + struct drm_property_blob *blob2) +{ + struct drm_color_lut *sw_lut = blob1->data; + struct drm_color_lut *hw_lut = blob2->data; + 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 (sw_lut_size != hw_lut_size) { + DRM_DEBUG_KMS("Invalid LUT size; got %d, expected %d\n", hw_lut_size, sw_lut_size); + return false; + } + + for (i = 0; i < sw_lut_size; i++) { + if (hw_lut[i].red != sw_lut[i].red || + hw_lut[i].blue != sw_lut[i].blue || + hw_lut[i].green != sw_lut[i].green) { + DRM_DEBUG_KMS("Invalid LUT value; sw_lut value doesn't match hw_lut value\n"); + return false; + } + } + + return true; +} + static bool intel_pipe_config_compare(struct drm_i915_private *dev_priv, struct intel_crtc_state *current_config, @@ -12148,6 +12188,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) } \ } while (0) +#define PIPE_CONF_CHECK_BLOB(name) do { \ + if (!intel_compare_blob(current_config->name, pipe_config->name)) { \ + 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 +12335,8 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) PIPE_CONF_CHECK_INFOFRAME(spd); PIPE_CONF_CHECK_INFOFRAME(hdmi); + PIPE_CONF_CHECK_BLOB(base.gamma_lut); + #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..a6acd9b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2512,6 +2512,7 @@ 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_get_config(struct intel_crtc_state *crtc_state); /* intel_lspcon.c */ bool lspcon_init(struct intel_digital_port *intel_dig_port);