From patchwork Thu Jul 11 15:59:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 11040379 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 971E6138B for ; Thu, 11 Jul 2019 15:59:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 855A4288D8 for ; Thu, 11 Jul 2019 15:59:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7935228A1B; Thu, 11 Jul 2019 15:59:56 +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=-4.2 required=2.0 tests=BAYES_00,HK_RANDOM_FROM, 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 6513F288D8 for ; Thu, 11 Jul 2019 15:59:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 60C656E245; Thu, 11 Jul 2019 15:59:54 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 88BBC6E245 for ; Thu, 11 Jul 2019 15:59:53 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2019 08:59:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,479,1557212400"; d="scan'208";a="341422475" Received: from dawalker-mobl.ger.corp.intel.com (HELO localhost.localdomain) ([10.251.80.131]) by orsmga005.jf.intel.com with ESMTP; 11 Jul 2019 08:59:43 -0700 From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Thu, 11 Jul 2019 16:59:39 +0100 Message-Id: <20190711155939.18059-1-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190709210620.15805-3-tvrtko.ursulin@linux.intel.com> References: <20190709210620.15805-3-tvrtko.ursulin@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2 2/4] drm/i915: Fix WaProgramMgsrForL3BankSpecificMmioReads 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Tvrtko Ursulin Two issues in this code: 1. fls() usage is incorrect causing off by one in subslice mask lookup, which in other words means subslice mask of all zeroes is always used (subslice mask of a slice which is not present, or even out of bounds array access), rendering the checks in wa_init_mcr either futile or random. 2. Condition in WARN_ON is not correct. It is doing a bitwise and operation between a positive (present subslices) and negative mask (disabled L3 banks). This means that with corrected fls() usage the assert would always incorrectly fail. We can fix this by inverting the fuse bits in the check. v2: * Simplify check for logic and redability. * Improve commentary explaining what is really happening ie. what the assert is really trying to check and why. Signed-off-by: Tvrtko Ursulin Fixes: fe864b76c2ab ("drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads") Reviewed-by: Chris Wilson # v1 Cc: Michał Winiarski --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 80 ++++++++++----------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 9e069286d3ce..80f1159e5cda 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -761,7 +761,27 @@ static void wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) { const struct sseu_dev_info *sseu = &RUNTIME_INFO(i915)->sseu; - u32 mcr_slice_subslice_mask; + u32 mcr_mask, mcr; + + /* + * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl + * Before any MMIO read into slice/subslice specific registers, MCR + * packet control register needs to be programmed to point to any + * enabled s/ss pair. Otherwise, incorrect values will be returned. + * This means each subsequent MMIO read will be forwarded to an + * specific s/ss combination, but this is OK since these registers + * are consistent across s/ss in almost all cases. In the rare + * occasions, such as INSTDONE, where this value is dependent + * on s/ss combo, the read should be done with read_subslice_reg. + */ + mcr = intel_calculate_mcr_s_ss_select(i915); + + if (INTEL_GEN(i915) >= 11) + mcr_mask = GEN11_MCR_SLICE_MASK | GEN11_MCR_SUBSLICE_MASK; + else + mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK; + + wa_write_masked_or(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr); /* * WaProgramMgsrForL3BankSpecificMmioReads: cnl,icl @@ -776,49 +796,29 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) * something more complex that requires checking the range of every * MMIO read). */ - if (INTEL_GEN(i915) >= 10 && - is_power_of_2(sseu->slice_mask)) { + if (INTEL_GEN(i915) >= 10 && is_power_of_2(sseu->slice_mask)) { /* - * read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches - * enabled subslice, no need to redirect MCR packet + * GEN8_MCR_SELECTOR contains dual-purpose bits which select + * both to which subslice, or to which L3 bank, the respective + * mmio reads will go. + * Since we have selected one enabled subslice in + * WaProgramMgsrForCorrectSliceSpecificMmioReads, we now + * need to check if the L3 bank of the equal "instance" is also + * enabled. + * If that is not the case we could try to find a number which + * works for both, or going even further, implement a dynamic + * scheme where we switch at before every affected mmio read. + * Fortunately neither seems to be needed at the moment for + * current parts and current driver behaviour. */ - u32 slice = fls(sseu->slice_mask); - u32 fuse3 = - intel_uncore_read(&i915->uncore, GEN10_MIRROR_FUSE3); - u8 ss_mask = sseu->subslice_mask[slice]; - - u8 enabled_mask = (ss_mask | ss_mask >> - GEN10_L3BANK_PAIR_COUNT) & GEN10_L3BANK_MASK; - u8 disabled_mask = fuse3 & GEN10_L3BANK_MASK; + unsigned int mcr_ss = BIT((mcr >> 24) & 0x7); + unsigned int l3_fuse = + intel_uncore_read(&i915->uncore, GEN10_MIRROR_FUSE3) & + GEN10_L3BANK_MASK; + unsigned int l3_en = ~(l3_fuse << 4 | l3_fuse); - /* - * Production silicon should have matched L3Bank and - * subslice enabled - */ - WARN_ON((enabled_mask & disabled_mask) != enabled_mask); + WARN_ON(!(mcr_ss & l3_en)); } - - if (INTEL_GEN(i915) >= 11) - mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK | - GEN11_MCR_SUBSLICE_MASK; - else - mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK | - GEN8_MCR_SUBSLICE_MASK; - /* - * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl - * Before any MMIO read into slice/subslice specific registers, MCR - * packet control register needs to be programmed to point to any - * enabled s/ss pair. Otherwise, incorrect values will be returned. - * This means each subsequent MMIO read will be forwarded to an - * specific s/ss combination, but this is OK since these registers - * are consistent across s/ss in almost all cases. In the rare - * occasions, such as INSTDONE, where this value is dependent - * on s/ss combo, the read should be done with read_subslice_reg. - */ - wa_write_masked_or(wal, - GEN8_MCR_SELECTOR, - mcr_slice_subslice_mask, - intel_calculate_mcr_s_ss_select(i915)); } static void