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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 49BBEC43334 for ; Wed, 6 Jul 2022 10:54:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233173AbiGFKyV (ORCPT ); Wed, 6 Jul 2022 06:54:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38078 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231766AbiGFKyT (ORCPT ); Wed, 6 Jul 2022 06:54:19 -0400 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FF4327B06; Wed, 6 Jul 2022 03:54:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1657104858; x=1688640858; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=qrEumAiCGHwpWr14nERu9HZ9oiyvzZusQBL2z+Vav/k=; b=QOYlxB+fisZAK/zoGiBYCkBRTNnpgHGnovUFEpzusz7Nebdlc0cL5lhz scFgaw9+ShACFcllSipQRNyFJn7qlq4qu+7JyYO4QrG3dr2qaxVhyWRnV nb1DgZT53TgjpVlXl1RcA4qap5dge3S/H6Px7EzDQnaPqlJhBl8XATddA YDqzgvJ/jNDVTo0k8OPBEkaWqJhbVwfmMDRFtDmmt9X7SUyT+j/YAm3w0 +DIsAGZfoV9h3yD9JPwFt4SduhzqvABpfk2guxm9n1Uyh1UJID1JEqla5 Ts2i5HiRaR1eng0tVazwIujiV9dDlTHUzCkCr4oQ/YxdxuHZrHgPxHgvS Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10399"; a="264131974" X-IronPort-AV: E=Sophos;i="5.92,249,1650956400"; d="scan'208";a="264131974" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jul 2022 03:51:39 -0700 X-IronPort-AV: E=Sophos;i="5.92,249,1650956400"; d="scan'208";a="620260611" Received: from mropara-mobl1.ger.corp.intel.com (HELO intel.com) ([10.252.49.154]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jul 2022 03:51:33 -0700 Date: Wed, 6 Jul 2022 12:51:31 +0200 From: Andi Shyti To: Mauro Carvalho Chehab Cc: Chris Wilson , Bruce Chang , Daniel Vetter , David Airlie , Jani Nikula , John Harrison , Joonas Lahtinen , Lucas De Marchi , Matt Roper , Matthew Brost , Rodrigo Vivi , Tejas Upadhyay , Tvrtko Ursulin , Umesh Nerlige Ramappa , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Mika Kuoppala , Chris Wilson , Thomas =?iso-8859-15?Q?Hellstr=F6m?= Subject: Re: [PATCH v3 1/2] drm/i915/gt: Serialize GRDOM access between multiple engine resets Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mauro and Chris, On Mon, Jul 04, 2022 at 09:09:28AM +0100, Mauro Carvalho Chehab wrote: > From: Chris Wilson > > Don't allow two engines to be reset in parallel, as they would both > try to select a reset bit (and send requests to common registers) > and wait on that register, at the same time. Serialize control of > the reset requests/acks using the uncore->lock, which will also ensure > that no other GT state changes at the same time as the actual reset. > > Cc: stable@vger.kernel.org # Up to 4.4 > Reported-by: Mika Kuoppala > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Reviewed-by: Andi Shyti > Acked-by: Thomas Hellström > Signed-off-by: Mauro Carvalho Chehab sorry for the delay but I wanted to understand what has been agreed between you and Tvrtko about the Cc'ing the stable list. Anyway, I confirm my review here. Andi > --- > > To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover. > See [PATCH v3 0/2] at: https://lore.kernel.org/all/cover.1656921701.git.mchehab@kernel.org/ > > drivers/gpu/drm/i915/gt/intel_reset.c | 37 ++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index a5338c3fde7a..c68d36fb5bbd 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -300,9 +300,9 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) > return err; > } > > -static int gen6_reset_engines(struct intel_gt *gt, > - intel_engine_mask_t engine_mask, > - unsigned int retry) > +static int __gen6_reset_engines(struct intel_gt *gt, > + intel_engine_mask_t engine_mask, > + unsigned int retry) > { > struct intel_engine_cs *engine; > u32 hw_mask; > @@ -321,6 +321,20 @@ static int gen6_reset_engines(struct intel_gt *gt, > return gen6_hw_domain_reset(gt, hw_mask); > } > > +static int gen6_reset_engines(struct intel_gt *gt, > + intel_engine_mask_t engine_mask, > + unsigned int retry) > +{ > + unsigned long flags; > + int ret; > + > + spin_lock_irqsave(>->uncore->lock, flags); > + ret = __gen6_reset_engines(gt, engine_mask, retry); > + spin_unlock_irqrestore(>->uncore->lock, flags); > + > + return ret; > +} > + > static struct intel_engine_cs *find_sfc_paired_vecs_engine(struct intel_engine_cs *engine) > { > int vecs_id; > @@ -487,9 +501,9 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine) > rmw_clear_fw(uncore, sfc_lock.lock_reg, sfc_lock.lock_bit); > } > > -static int gen11_reset_engines(struct intel_gt *gt, > - intel_engine_mask_t engine_mask, > - unsigned int retry) > +static int __gen11_reset_engines(struct intel_gt *gt, > + intel_engine_mask_t engine_mask, > + unsigned int retry) > { > struct intel_engine_cs *engine; > intel_engine_mask_t tmp; > @@ -583,8 +597,11 @@ static int gen8_reset_engines(struct intel_gt *gt, > struct intel_engine_cs *engine; > const bool reset_non_ready = retry >= 1; > intel_engine_mask_t tmp; > + unsigned long flags; > int ret; > > + spin_lock_irqsave(>->uncore->lock, flags); > + > for_each_engine_masked(engine, gt, engine_mask, tmp) { > ret = gen8_engine_reset_prepare(engine); > if (ret && !reset_non_ready) > @@ -612,17 +629,19 @@ static int gen8_reset_engines(struct intel_gt *gt, > * This is best effort, so ignore any error from the initial reset. > */ > if (IS_DG2(gt->i915) && engine_mask == ALL_ENGINES) > - gen11_reset_engines(gt, gt->info.engine_mask, 0); > + __gen11_reset_engines(gt, gt->info.engine_mask, 0); > > if (GRAPHICS_VER(gt->i915) >= 11) > - ret = gen11_reset_engines(gt, engine_mask, retry); > + ret = __gen11_reset_engines(gt, engine_mask, retry); > else > - ret = gen6_reset_engines(gt, engine_mask, retry); > + ret = __gen6_reset_engines(gt, engine_mask, retry); > > skip_reset: > for_each_engine_masked(engine, gt, engine_mask, tmp) > gen8_engine_reset_cancel(engine); > > + spin_unlock_irqrestore(>->uncore->lock, flags); > + > return ret; > } > > -- > 2.36.1