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 4BE45C77B7A for ; Fri, 2 Jun 2023 00:27:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3E12910E02C; Fri, 2 Jun 2023 00:27:58 +0000 (UTC) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1B97310E02C for ; Fri, 2 Jun 2023 00:27:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1685665677; x=1717201677; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=0T2DmVS2AZDLi5brChSVI9Ik3tR0JsOfOPXwnLfDiSo=; b=SjX4tqYMJPL/iuiB7ALiTWQsjpGwn7g589aEBKXwsOGpex3iWi8C9QA1 0BW3dtsuUTN5ldk8jfDKdK80KK1b7r+VkHEDoi31RvtSSSbmAVt3hvnSw 33u/caCCCm5utpvXBYmwj68sQfwUKzlpsr+L+ZwfjhbaZkvQHBdpRnWTm TKBUE6C3iNHWKy8wMes6Z4p+Jg5PYJLtlSsNdXRNxzIgXSWlr+uTcvOZs q+S+NX4gLVWGK0OfGIJp+hTqVNyS2Uxm8RzcNAnK4KqMAXwRAt/Dadtgv cDhRfYr5Re8Vbbsif+MN8DWTwJ2guOaNMMBB/gvig3Zfbx+Vag8eiHFxI Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10728"; a="353220294" X-IronPort-AV: E=Sophos;i="6.00,211,1681196400"; d="scan'208";a="353220294" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Jun 2023 17:27:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10728"; a="737316031" X-IronPort-AV: E=Sophos;i="6.00,211,1681196400"; d="scan'208";a="737316031" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.85.141]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Jun 2023 17:27:55 -0700 Date: Thu, 01 Jun 2023 17:23:24 -0700 Message-ID: <87ttvqvhj7.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Matt Atwood In-Reply-To: <87v8g7ujaj.wl-ashutosh.dixit@intel.com> References: <20230531213547.1525692-1-matthew.s.atwood@intel.com> <87wn0nujol.wl-ashutosh.dixit@intel.com> <87v8g7ujaj.wl-ashutosh.dixit@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-pc-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 Subject: Re: [Intel-gfx] [PATCH] drm/i915: sync I915_PMU_MAX_GTS to I915_MAX_GT 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, Andi Shyti Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, 01 Jun 2023 11:30:44 -0700, Dixit, Ashutosh wrote: > > On Thu, 01 Jun 2023 11:22:18 -0700, Dixit, Ashutosh wrote: > > > > On Wed, 31 May 2023 14:35:47 -0700, Matt Atwood wrote: > > > > > > > Hi Matt, > > > > > Set I915_PMU_MAX_GTS to value in I915_MAX_GT, theres no reason for these > > > values to be different. > > Also, we can't be so sure so as to be able to say "theres no reason for > these values to be different" till we have actually verified it. E.g. there > are various bitfields in the code which might not fit in a u32 if we > increase MAX_GT from 2 to 4. Has this been verified? > > If anything, to keep the code from doing unnecessary stuff, IMO I915_MAX_GT > should be reduced to 2 and should be increased to 4 only once/if we have > i915 supported platforms with 4 GT's. Matt explained the issue offline to me (it would have helped to explain the reason for the patch in the commit message). The issue is that in uses of for_each_gt such as below (there are others too in the PMU code): for_each_gt(gt, i915, i) { intel_wakeref_t wakeref; with_intel_runtime_pm(gt->uncore->rpm, wakeref) { u64 val = __get_rc6(gt); store_sample(pmu, i, __I915_SAMPLE_RC6, val); store_sample(pmu, i, __I915_SAMPLE_RC6_LAST_REPORTED, val); pmu->sleep_last[i] = ktime_get_raw(); } } static checkers are complaining that for_each_gt can read/write outside the bounds of PMU arrays. Because absent gt's will be NULL in for_each_gt this cannot really happen but we still need to keep static checkers happy. So to resolve this issue we need I915_PMU_MAX_GTS and I915_MAX_GT to have the same value. So either we need to increase I915_PMU_MAX_GTS to 4 or reduce I915_MAX_GT to 2. Regards, Ashutosh > > > > > > > > Cc: Tvrtko Ursulin > > > Cc: Umesh Nerlige Ramappa > > > Cc: Ashutosh Dixit > > > > I don't believe the mailer actually Cc'd us. I just saw this and am Cc'ing > > the people who authored/reviewed the previous series now. > > > > > Signed-off-by: Matt Atwood > > > --- > > > drivers/gpu/drm/i915/i915_pmu.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h > > > index 33d80fbaab8b..aa929d8c224a 100644 > > > --- a/drivers/gpu/drm/i915/i915_pmu.h > > > +++ b/drivers/gpu/drm/i915/i915_pmu.h > > > @@ -38,7 +38,7 @@ enum { > > > __I915_NUM_PMU_SAMPLERS > > > }; > > > > > > -#define I915_PMU_MAX_GTS 2 > > > +#define I915_PMU_MAX_GTS 4 > > > > This was a discussed during the previous review and it was decided to keep > > the two values (I915_PMU_MAX_GTS and I915_MAX_GT) different. There are > > currently no platforms and there will be no i915 supported platforms with > > MAX_GT 4. So I prefer to leave the values as they currently are. Unless > > Umesh or Tvrtko agrees to this patch. > > > > Thanks. > > -- > > Ashutosh > > > > > > > > /* > > > * How many different events we track in the global PMU mask. > > > -- > > > 2.40.0 > > >