From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 10EBAC4338F for ; Mon, 2 Aug 2021 21:08:02 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C643460F36 for ; Mon, 2 Aug 2021 21:08:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C643460F36 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6E8F86E0C1; Mon, 2 Aug 2021 21:08:01 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 012806E0C1 for ; Mon, 2 Aug 2021 21:07:59 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10064"; a="193128078" X-IronPort-AV: E=Sophos;i="5.84,289,1620716400"; d="scan'208";a="193128078" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 14:07:59 -0700 X-IronPort-AV: E=Sophos;i="5.84,289,1620716400"; d="scan'208";a="510402317" Received: from ideak-desk.fi.intel.com ([10.237.68.141]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 14:07:58 -0700 Date: Tue, 3 Aug 2021 00:07:54 +0300 From: Imre Deak To: "Souza, Jose" Cc: "intel-gfx@lists.freedesktop.org" Message-ID: <20210802210754.GA2115791@ideak-desk.fi.intel.com> References: <20210802190148.2099625-1-imre.deak@intel.com> <70c089841d159b322f6c6749fad8e416766676dc.camel@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <70c089841d159b322f6c6749fad8e416766676dc.camel@intel.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Apply CMTG clock disabling WA while DPLL0 is enabled X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, Aug 02, 2021 at 11:52:41PM +0300, Souza, Jose wrote: > On Mon, 2021-08-02 at 22:01 +0300, Imre Deak wrote: > > CI test results/further experiments show that the workaround added in > > > > commit 573d7ce4f69a ("drm/i915/adlp: Add workaround to disable CMTG clock gating") > > > > can be applied only while DPLL0 is enabled. If it's disabled the > > TRANS_CMTG_CHICKEN register is not accessible. Accordingly move the WA > > to DPLL0 HW state sanitization and enabling. > > > > This fixes an issue where the WA won't get applied (and a WARN is thrown > > due to an unexpected value in TRANS_CMTG_CHICKEN) if the driver is > > loaded without DPLL0 being enabled: booting without BIOS enabling an > > output with this PLL, or reloading the driver. > > > > While at it also add a debug print for the unexpected register value. > > Workaround do not mention nothing about this DPLL0 dependency, maybe > would be nice to comment in HSD about this. Ok, can add comment. > Have you tried to check if the workaround applies if DPLL1 is enabled? > We could comment DPLL0 out from the adlp_plls table. No, only DPLL0 makes it work, DPLL1 being enabled is not enough. You can experiment with this by unloading the driver and simply enable/disable DPLL0/1 (both power and enable flag) and try to read/modify/re-read the CMTG_CHICKEN reg. > > Cc: José Roberto de Souza > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 18 ---------- > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 34 ++++++++++++++++++- > > 2 files changed, 33 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 4ca354f154215..98f7fbede6226 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -13284,24 +13284,6 @@ static void intel_early_display_was(struct drm_i915_private *dev_priv) > > KBL_ARB_FILL_SPARE_13 | KBL_ARB_FILL_SPARE_14, > > KBL_ARB_FILL_SPARE_14); > > } > > - > > - if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) { > > - u32 val; > > - > > - /* > > - * Wa_16011069516:adl-p[a0] > > - * > > - * All CMTG regs are unreliable until CMTG clock gating is > > - * disabled, so we can only assume the default CMTG_CHICKEN > > - * reg value and sanity check this assumption with a double > > - * read, which presumably returns the correct value even with > > - * clock gating on. > > - */ > > - val = intel_de_read(dev_priv, TRANS_CMTG_CHICKEN); > > - val = intel_de_read(dev_priv, TRANS_CMTG_CHICKEN); > > - intel_de_write(dev_priv, TRANS_CMTG_CHICKEN, DISABLE_DPT_CLK_GATING); > > - drm_WARN_ON(&dev_priv->drm, val & ~DISABLE_DPT_CLK_GATING); > > - } > > } > > > > static void ibx_sanitize_pch_hdmi_port(struct drm_i915_private *dev_priv, > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > index 0d72917e5670f..5c91d125a3371 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > @@ -3735,6 +3735,31 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv, > > drm_err(&dev_priv->drm, "PLL %d not locked\n", pll->info->id); > > } > > > > +static void adlp_cmtg_clock_gating_wa(struct drm_i915_private *i915, struct intel_shared_dpll *pll) > > +{ > > + u32 val; > > + > > + if (!IS_ADLP_DISPLAY_STEP(i915, STEP_A0, STEP_B0) || > > + pll->info->id != DPLL_ID_ICL_DPLL0) > > + return; > > + /* > > + * Wa_16011069516:adl-p[a0] > > + * > > + * All CMTG regs are unreliable until CMTG clock gating is disabled, > > + * so we can only assume the default TRANS_CMTG_CHICKEN reg value and > > + * sanity check this assumption with a double read, which presumably > > + * returns the correct value even with clock gating on. > > + * > > + * Instead of the usual place for workarounds we apply this one here, > > + * since TRANS_CMTG_CHICKEN is only accessible while DPLL0 is enabled. > > + */ > > + val = intel_de_read(i915, TRANS_CMTG_CHICKEN); > > + val = intel_de_read(i915, TRANS_CMTG_CHICKEN); > > + intel_de_write(i915, TRANS_CMTG_CHICKEN, DISABLE_DPT_CLK_GATING); > > + if (drm_WARN_ON(&i915->drm, val & ~DISABLE_DPT_CLK_GATING)) > > + drm_dbg_kms(&i915->drm, "Unexpected flags in TRANS_CMTG_CHICKEN: %08x\n", val); > > +} > > + > > static void combo_pll_enable(struct drm_i915_private *dev_priv, > > struct intel_shared_dpll *pll) > > { > > @@ -3764,6 +3789,8 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv, > > > > icl_pll_enable(dev_priv, pll, enable_reg); > > > > + adlp_cmtg_clock_gating_wa(dev_priv, pll); > > + > > /* DVFS post sequence would be here. See the comment above. */ > > } > > > > @@ -4273,7 +4300,12 @@ void intel_dpll_readout_hw_state(struct drm_i915_private *i915) > > static void sanitize_dpll_state(struct drm_i915_private *i915, > > struct intel_shared_dpll *pll) > > { > > - if (!pll->on || pll->active_mask) > > + if (!pll->on) > > + return; > > + > > + adlp_cmtg_clock_gating_wa(i915, pll); > > + > > + if (pll->active_mask) > > return; > > > > drm_dbg_kms(&i915->drm, >