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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 29934D3A697 for ; Tue, 29 Oct 2024 19:44:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E304110E2A3; Tue, 29 Oct 2024 19:44:03 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="RbAwBjGm"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3FC0B10E2A3 for ; Tue, 29 Oct 2024 19:44:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1730231043; x=1761767043; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=ttfYce/qdS8k+Pf1rnYy3K6/GAxjaXHWUHY9aSkQTbY=; b=RbAwBjGm24yAJ7mzr1mWRfxelKXBp2zdeFxPG81tCNm1TfmmBWD/O9mD NFP/B2GPrs3y9iKd0YrYTEK4Kc1gk9W2Dvxgk10Y6uvH8sKtw8aMiTrhi CPT3mbI2JX8wn1YJ2KeTN+kJF5DJDgynuDHW1R34D9lmPhQATDqasQ8p/ jrPCCXcTUT7nIsqhDWgD55ciXJ/KTiEqahL9D13oexJWc81OfA3bnxwXz M4lwR6DN1KgChDTs3bGUprypoAJm/mqaTS1Qcp2dr6WbwVH7pb1KrI9Ym FaoBkf3IBBD0Wbcb5WfBqzdIddN+/QoOA4RfW0de10j3zN+RBMqEx1QDN A==; X-CSE-ConnectionGUID: lk5GzxK8QOSQzgPniigqoQ== X-CSE-MsgGUID: 6nsN0rVqRnebukQOlRiv8g== X-IronPort-AV: E=McAfee;i="6700,10204,11240"; a="47379856" X-IronPort-AV: E=Sophos;i="6.11,241,1725346800"; d="scan'208";a="47379856" Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Oct 2024 12:44:03 -0700 X-CSE-ConnectionGUID: qe5UP9JHSliZYhpJGWaD8Q== X-CSE-MsgGUID: mYyN7FgiScyE34+Tap/hcw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,241,1725346800"; d="scan'208";a="82401301" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by fmviesa010-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Oct 2024 12:44:03 -0700 Date: Tue, 29 Oct 2024 12:44:02 -0700 Message-ID: <85zfmmy9rx.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Matt Roper Cc: Lucas De Marchi , Jonathan Cavitt , intel-xe@lists.freedesktop.org, saurabhg.gupta@intel.com, alex.zuo@intel.com, umesh.nerlige.ramappa@intel.com, john.c.harrison@intel.com, stable@vger.kernel.org Subject: Re: [PATCH v3] drm/xe/xe_guc_ads: save/restore OA registers In-Reply-To: <20241029193313.GY4891@mdroper-desk1.amr.corp.intel.com> References: <20241023200716.82624-1-jonathan.cavitt@intel.com> <854j4uzv79.wl-ashutosh.dixit@intel.com> <20241029193313.GY4891@mdroper-desk1.amr.corp.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-redhat-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Tue, 29 Oct 2024 12:33:13 -0700, Matt Roper wrote: > > On Tue, Oct 29, 2024 at 12:32:54PM -0500, Lucas De Marchi wrote: > > On Tue, Oct 29, 2024 at 10:15:54AM -0700, Ashutosh Dixit wrote: > > > On Tue, 29 Oct 2024 09:23:49 -0700, Lucas De Marchi wrote: > > > > > > > > On Wed, Oct 23, 2024 at 08:07:15PM +0000, Jonathan Cavitt wrote: > > > > > Several OA registers and allowlist registers were missing from the > > > > > save/restore list for GuC and could be lost during an engine reset. Add > > > > > them to the list. > > > > > > > > > > v2: > > > > > - Fix commit message (Umesh) > > > > > - Add missing closes (Ashutosh) > > > > > > > > > > v3: > > > > > - Add missing fixes (Ashutosh) > > > > > > > > > > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2249 > > > > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") > > > > > Suggested-by: Umesh Nerlige Ramappa > > > > > Suggested-by: John Harrison > > > > > Signed-off-by: Jonathan Cavitt > > > > > CC: stable@vger.kernel.org # v6.11+ > > > > > Acked-by: Ashutosh Dixit > > > > > Reviewed-by: Umesh Nerlige Ramappa > > > > > --- > > > > > drivers/gpu/drm/xe/xe_guc_ads.c | 14 ++++++++++++++ > > > > > 1 file changed, 14 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c > > > > > index 4e746ae98888..a196c4fb90fc 100644 > > > > > --- a/drivers/gpu/drm/xe/xe_guc_ads.c > > > > > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c > > > > > @@ -15,6 +15,7 @@ > > > > > #include "regs/xe_engine_regs.h" > > > > > #include "regs/xe_gt_regs.h" > > > > > #include "regs/xe_guc_regs.h" > > > > > +#include "regs/xe_oa_regs.h" > > > > > #include "xe_bo.h" > > > > > #include "xe_gt.h" > > > > > #include "xe_gt_ccs_mode.h" > > > > > @@ -740,6 +741,11 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, > > > > > guc_mmio_regset_write_one(ads, regset_map, e->reg, count++); > > > > > } > > > > > > > > > > + for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) > > > > > + guc_mmio_regset_write_one(ads, regset_map, > > > > > + RING_FORCE_TO_NONPRIV(hwe->mmio_base, i), > > > > > + count++); > > > > > > > > this is not the proper place. See drivers/gpu/drm/xe/xe_reg_whitelist.c. > > > > > > Yikes, this got merged yesterday. > > > > > > > > > > > The loop just before these added lines should be sufficient to go over > > > > all engine save/restore register and give them to guc. > > > > > > You probably mean this one? > > > > > > xa_for_each(&hwe->reg_sr.xa, idx, entry) > > > guc_mmio_regset_write_one(ads, regset_map, entry->reg, count++); > > > > > > But then how come this patch fixed GL #2249? > > > > it fixes, it just doesn't put it in the right place according to the > > driver arch. Whitelists should be in that other file so it shows up in > > debugfs, (/sys/kernel/debug/dri/*/*/register-save-restore), detect > > clashes when we try to add the same register, etc. > > Also, this patch failed pre-merge BAT since it added new regset entries > that we never actually allocated storage space for. Now that it's been > applied, we're seeing CI failures on lots of tests from this: > > https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3295 Wow, truly sorry, completely missed that BAT failures were due to this patch. How about we just revert this patch for now and redo it later? Unless you or Lucas know how to fix this immediately (I don't). Thanks. -- Ashutosh