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.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 74097C48BDF for ; Tue, 15 Jun 2021 09:08:22 +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 25E126100A for ; Tue, 15 Jun 2021 09:08:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 25E126100A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C21AC6E221; Tue, 15 Jun 2021 09:08:21 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 01CA16E221 for ; Tue, 15 Jun 2021 09:08:20 +0000 (UTC) IronPort-SDR: 3fgxkewTyH3jq+pI2Ed46DzoO9kBlF96R7qrEMKY8g9UNWgDRAOz9xrttSEdRWksb8AbDCQZL7 G8W7zYnux4pw== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="185647089" X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="185647089" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 02:08:20 -0700 IronPort-SDR: 7ymJMSL8FfkuXoinGbpfW6/ECEBNxs8cVV9OguGaPeKOTf0fjInCk5dKh/PFoq5bbRyi+b3+Q7 qRixCC31Aa8g== X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="553626252" Received: from webbersh-mobl.amr.corp.intel.com (HELO intel.com) ([10.254.187.3]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 02:08:19 -0700 Date: Tue, 15 Jun 2021 05:08:17 -0400 From: Rodrigo Vivi To: Matt Roper Message-ID: References: <20210615033433.1574397-1-matthew.d.roper@intel.com> <20210615033433.1574397-3-matthew.d.roper@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210615033433.1574397-3-matthew.d.roper@intel.com> Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Add GT support for multiple types of multicast steering 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: , Cc: intel-gfx@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, Jun 14, 2021 at 08:34:32PM -0700, Matt Roper wrote: > Although most of our multicast registers are replicated per-subslice, we > also have a small number of multicast registers that are replicated > per-l3 bank instead. For both types of multicast registers we need to > make sure we steer reads of these registers to a valid instance. > Ideally we'd like to find a specific instance ID that would steer reads > of either type of multicast register to a valid instance (i.e., not > fused off and not powered down), but sometimes the combination of > part-specific fusing and the additional restrictions imposed by Render > Power Gating make it impossible to find any overlap between the set of > valid subslices and valid l3 banks. This problem will become even more > noticeable on our upcoming platforms since they will be adding > additional types of multicast registers with new types of replication > and rules for finding valid instances for reads. > > To handle this we'll continue to pick a suitable subslice instance at > driver startup and program this as the default (sliceid,subsliceid) > setting in the steering control register (0xFDC). In cases where we > need to read another type of multicast GT register, but the default > subslice steering would not correspond to a valid instance, we'll > explicitly re-steer the single read to a valid value, perform the read, > and then reset the steering to it's "subslice" default. > > This patch adds the general functionality to prepare for this explicit > steering of other multicast register types. We'll plug L3 bank steering > into this in the next patch, and then add additional types of multicast > registers when the support for our next upcoming platform arrives. > > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 84 +++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_gt.h | 8 ++ > drivers/gpu/drm/i915/gt/intel_gt_types.h | 22 +++++ > drivers/gpu/drm/i915/gt/intel_workarounds.c | 28 ++++--- > .../gpu/drm/i915/gt/selftest_workarounds.c | 2 +- > 5 files changed, 131 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index 2161bf01ef8b..f2bea1c20d56 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -697,6 +697,90 @@ void intel_gt_driver_late_release(struct intel_gt *gt) > intel_engines_free(gt); > } > > +/** > + * intel_gt_reg_needs_read_steering - determine whether a register read > + * requires explicit steering > + * @gt: GT structure > + * @reg: the register to check steering requirements for > + * @type: type of multicast steering to check > + * > + * Determines whether @reg needs explicit steering of a specific type for > + * reads. > + * > + * Returns false if @reg does not belong to a register range of the given > + * steering type, or if the default (subslice-based) steering IDs are suitable > + * for @type steering too. > + */ > +static bool intel_gt_reg_needs_read_steering(struct intel_gt *gt, > + i915_reg_t reg, > + enum intel_steering_type type) > +{ > + const u32 offset = i915_mmio_reg_offset(reg); > + const struct intel_mmio_range *entry; > + > + if (likely(!intel_gt_needs_read_steering(gt, type))) > + return false; > + > + for (entry = gt->steering_table[type]; entry->start < 0xFFFFFF; entry++) { I'm not comfortable with this stop condition... we should know the right amount of entries that we have. Even if that means having another intermediate struct. > + if (offset >= entry->start && offset <= entry->end) > + return true; > + } > + > + return false; > +} > + > +/** > + * intel_gt_get_valid_steering - determines valid IDs for a class of MCR steering > + * @gt: GT structure > + * @type: multicast register type > + * @sliceid: Slice ID returned > + * @subsliceid: Subslice ID returned > + * > + * Determines sliceid and subsliceid values that will steer reads > + * of a specific multicast register class to a valid value. > + */ > +static void intel_gt_get_valid_steering(struct intel_gt *gt, > + enum intel_steering_type type, > + u8 *sliceid, u8 *subsliceid) > +{ > + switch (type) { > + default: > + MISSING_CASE(type); I understand that we are preparing the infra for the upcoming cases, but adding a missing_case warn by default doesn't look the right way... did CI not complain?! > + *sliceid = 0; > + *subsliceid = 0; > + } > +} > + > +/** > + * intel_gt_read_register_fw - reads a GT register with support for multicast > + * @gt: GT structure > + * @reg: register to read > + * > + * This function will read a GT register. If the register is a multicast > + * register, the read will be steered to a valid instance (i.e., one that > + * isn't fused off or powered down by power gating). > + * > + * Returns the value from a valid instance of @reg. > + */ > +u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) > +{ > + int type; > + u8 sliceid, subsliceid; > + > + for (type = 0; type < NUM_STEERING_TYPES; type++) { > + if (intel_gt_reg_needs_read_steering(gt, reg, type)) { > + intel_gt_get_valid_steering(gt, type, &sliceid, > + &subsliceid); > + return intel_uncore_read_with_mcr_steering_fw(gt->uncore, > + reg, > + sliceid, > + subsliceid); > + } > + } > + > + return intel_uncore_read_fw(gt->uncore, reg); > +} > + > void intel_gt_info_print(const struct intel_gt_info *info, > struct drm_printer *p) > { > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h > index 7ec395cace69..e7aabe0cc5bf 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt.h > @@ -75,6 +75,14 @@ static inline bool intel_gt_is_wedged(const struct intel_gt *gt) > return unlikely(test_bit(I915_WEDGED, >->reset.flags)); > } > > +static inline bool intel_gt_needs_read_steering(struct intel_gt *gt, > + enum intel_steering_type type) > +{ > + return gt->steering_table[type]; then if we know the right amount we also don't need this function right?! > +} > + > +u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg); > + > void intel_gt_info_print(const struct intel_gt_info *info, > struct drm_printer *p); > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index fecfacf551d5..47957837c8c0 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -31,6 +31,26 @@ struct i915_ggtt; > struct intel_engine_cs; > struct intel_uncore; > > +struct intel_mmio_range { > + u32 start; > + u32 end; > +}; > + > +/* > + * The hardware has multiple kinds of multicast register ranges that need > + * special register steering (and future platforms are expected to add > + * additional types). > + * > + * During driver startup, we initialize the steering control register to > + * direct reads to a slice/subslice that are valid for the 'subslice' class > + * of multicast registers. If another type of steering does not have any > + * overlap in valid steering targets with 'subslice' style registers, we will > + * need to explicitly re-steer reads of registers of the other type. > + */ > +enum intel_steering_type { > + NUM_STEERING_TYPES > +}; > + > enum intel_submission_method { > INTEL_SUBMISSION_RING, > INTEL_SUBMISSION_ELSP, > @@ -145,6 +165,8 @@ struct intel_gt { > > struct i915_vma *scratch; > > + const struct intel_mmio_range *steering_table[NUM_STEERING_TYPES]; > + > struct intel_gt_info { > intel_engine_mask_t engine_mask; > u8 num_engines; > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index b62d1e31a645..689045d3752b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -1247,8 +1247,9 @@ wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from) > } > > static void > -wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal) > +wa_list_apply(struct intel_gt *gt, const struct i915_wa_list *wal) > { > + struct intel_uncore *uncore = gt->uncore; > enum forcewake_domains fw; > unsigned long flags; > struct i915_wa *wa; > @@ -1263,13 +1264,16 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal) > intel_uncore_forcewake_get__locked(uncore, fw); > > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { > - if (wa->clr) > - intel_uncore_rmw_fw(uncore, wa->reg, wa->clr, wa->set); > - else > - intel_uncore_write_fw(uncore, wa->reg, wa->set); > + u32 val, old = 0; > + > + /* open-coded rmw due to steering */ > + old = wa->clr ? intel_gt_read_register_fw(gt, wa->reg) : 0; > + val = (old & ~wa->clr) | wa->set; > + if (val != old || !wa->clr) > + intel_uncore_write_fw(uncore, wa->reg, val); > + > if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) > - wa_verify(wa, > - intel_uncore_read_fw(uncore, wa->reg), > + wa_verify(wa, intel_gt_read_register_fw(gt, wa->reg), > wal->name, "application"); > } > > @@ -1279,10 +1283,10 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal) > > void intel_gt_apply_workarounds(struct intel_gt *gt) > { > - wa_list_apply(gt->uncore, >->i915->gt_wa_list); > + wa_list_apply(gt, >->i915->gt_wa_list); > } > > -static bool wa_list_verify(struct intel_uncore *uncore, > +static bool wa_list_verify(struct intel_gt *gt, > const struct i915_wa_list *wal, > const char *from) > { > @@ -1292,7 +1296,7 @@ static bool wa_list_verify(struct intel_uncore *uncore, > > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) > ok &= wa_verify(wa, > - intel_uncore_read(uncore, wa->reg), > + intel_gt_read_register_fw(gt, wa->reg), > wal->name, from); > > return ok; > @@ -1300,7 +1304,7 @@ static bool wa_list_verify(struct intel_uncore *uncore, > > bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from) > { > - return wa_list_verify(gt->uncore, >->i915->gt_wa_list, from); > + return wa_list_verify(gt, >->i915->gt_wa_list, from); > } > > __maybe_unused > @@ -2081,7 +2085,7 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine) > > void intel_engine_apply_workarounds(struct intel_engine_cs *engine) > { > - wa_list_apply(engine->uncore, &engine->wa_list); > + wa_list_apply(engine->gt, &engine->wa_list); > } > > struct mcr_range { > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > index c30754daf4b1..7ebc4edb8ecf 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > @@ -1147,7 +1147,7 @@ verify_wa_lists(struct intel_gt *gt, struct wa_lists *lists, > enum intel_engine_id id; > bool ok = true; > > - ok &= wa_list_verify(gt->uncore, &lists->gt_wa_list, str); > + ok &= wa_list_verify(gt, &lists->gt_wa_list, str); > > for_each_engine(engine, gt, id) { > struct intel_context *ce; > -- > 2.25.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx