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 19250C3600D for ; Tue, 25 Mar 2025 22:45:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B454910E325; Tue, 25 Mar 2025 22:45:09 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="l5SaPVnZ"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id E6F3010E325 for ; Tue, 25 Mar 2025 22:45:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1742942708; x=1774478708; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=b2GPyD5jHf6bMoYJLMCoLMATsXLhcoVJn82MpitlGTk=; b=l5SaPVnZaDgo2r3Pw8uI1QsjgmLK3is23Y4b7/suuH1rqPQ6DWX0NJJF Nrzjz8zECXBse5IbUd0qISYAJ/oqcCo37h9I4cr9o4USw3ePl6lny1RAR Xo9BONQD9CQWFRGHQU3Woc49MKarLj4yunxUBQIpG9zM9+luYwAVVd3Nb 3c98avvyGRvhGMAaJQzZkOjWUEPfiCww59VZITSsQcZvI/aBUQBuA/msh cV2VjerE8MjF4xHn9drjDmjzobyYSm2WRQ9FkSLEvoG8dzdGe3n/vI0wr VpFevsS7RGY0Q9Owud1wbM82bw7AMtNab64cTkZVFybtu0HJ6J2wF3qd0 Q==; X-CSE-ConnectionGUID: 6ofTb4rFQmG/38z+dCbAhA== X-CSE-MsgGUID: gWeQSBfrTQeGX87xAbCriQ== X-IronPort-AV: E=McAfee;i="6700,10204,11384"; a="55578861" X-IronPort-AV: E=Sophos;i="6.14,276,1736841600"; d="scan'208";a="55578861" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2025 15:45:08 -0700 X-CSE-ConnectionGUID: uI7K+BmXQd21Z9hyd3FkTg== X-CSE-MsgGUID: MjyU5kA0RPib8Jb/auZeMA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,276,1736841600"; d="scan'208";a="129713027" Received: from ileong-mobl1.gar.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.91.160]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2025 15:45:08 -0700 Date: Tue, 25 Mar 2025 15:45:06 -0700 Message-ID: <87frj0ivbh.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Belgaumkar, Vinay" Cc: , Riana Tauro , Lucas De Marchi , Rodrigo Vivi Subject: Re: [PATCH v4] drm/xe/pmu: Add GT frequency events In-Reply-To: <269b993d-2a22-4080-8609-66650a60ad62@intel.com> References: <20250324232402.46481-1-vinay.belgaumkar@intel.com> <87jz8e9d3h.wl-ashutosh.dixit@intel.com> <87jz8dhw0r.wl-ashutosh.dixit@intel.com> <33ebfe1d-5ac5-4985-8c8d-c1996e5a6d14@intel.com> <87h63gixq0.wl-ashutosh.dixit@intel.com> <269b993d-2a22-4080-8609-66650a60ad62@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/29.4 (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=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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, 25 Mar 2025 15:01:47 -0700, Belgaumkar, Vinay wrote: > > > On 3/25/2025 2:53 PM, Dixit, Ashutosh wrote: > > On Tue, 25 Mar 2025 13:33:43 -0700, Belgaumkar, Vinay wrote: > >> On 3/25/2025 10:15 AM, Dixit, Ashutosh wrote: > >>> On Mon, 24 Mar 2025 19:37:32 -0700, Belgaumkar, Vinay wrote: > >>> Hi Vinay, > >>> > >>>> On 3/24/2025 5:18 PM, Dixit, Ashutosh wrote: > >>>>> On Mon, 24 Mar 2025 16:24:02 -0700, Vinay Belgaumkar wrote: > >>>>>> @@ -266,11 +274,24 @@ static u64 __xe_pmu_event_read(struct perf_e= vent *event) > >>>>>> case XE_PMU_EVENT_ENGINE_ACTIVE_TICKS: > >>>>>> case XE_PMU_EVENT_ENGINE_TOTAL_TICKS: > >>>>>> return read_engine_events(gt, event); > >>>>>> + case XE_PMU_EVENT_GT_ACTUAL_FREQUENCY: > >>>>>> + return xe_guc_pc_get_act_freq(>->uc.guc.pc); > >>>>>> + case XE_PMU_EVENT_GT_REQUESTED_FREQUENCY: > >>>>>> + if (!xe_guc_pc_get_cur_freq(>->uc.guc.pc, &cur_gt_freq)) > >>>>> This is unconditionally taking the forcewake and waking the card up= just to > >>>>> get the sample. Do we really want to do that? > >>>>> > >>>>> So if we don't do that, both the actual and requested freq will be = 0 if gt > >>>>> is in C6. > >>>> For actual frequency, the register(0xc60) does not belong to any fw = domain - > >>>> > >>>> GEN_FW_RANGE(0xc00, 0xfff, 0), > >>>> > >>>> HW will report 0 when GT is in C6. > >>> Yes, no issue about act_freq, see commit 22009b6dad66. I was referrin= g only > >>> to requested freq. > >>> > >>>> The requested freq register is a > >>>> shadowed register (0xa008), so that will not accrue fwake either. > >>>> > >>>> static const struct i915_range mtl_shadowed_regs[] =3D { > >>>> =A0=A0=A0=A0=A0=A0=A0 { .start =3D=A0=A0 0x2030, .end =3D=A0=A0 0x= 2030 }, > >>>> =A0=A0=A0=A0=A0=A0=A0 { .start =3D=A0=A0 0x2510, .end =3D=A0=A0 0x= 2550 }, > >>>> =A0=A0=A0=A0=A0=A0=A0 { .start =3D=A0=A0 0xA008, .end =3D=A0=A0 0x= A00C }, > >>> So this still doesn't make sense because: > >>> > >>> 1. The fact is that xe_guc_pc_get_cur_freq() *is* taking forcewake > >>> 2. And that is in accord with the following comment in i915/intel= _uncore.c > >>> > >>> * Shadowing only applies to writes; forcewake > >>> * must still be acquired when reading from registers in these = ranges. > >>> > >>> Also see intel_rps_read_punit_req() which is called from i915 PMU > >>> (frequency_sample()) and uses with_intel_runtime_pm_if_in_use(), so w= e'd > >>> need to do use the equivalent in xe. > >> Hi Ashutosh, > >> > >> =A0 As part of a previous decision, in the Xe PMU implementation, we = are > >> doing a runtime_get() during pmu_init for all PMU sessions. So, device= is > >> going to be awake anyways. In this case, it does not make sense to just > >> read the register without a fwake. > >=20 > > Even if that is ok... Let us take a completely idle device. What should= the > > actual and requested freq's be for this case? Both should be zero, corr= ect? > > Now, what are we going to show if we are taking fwake? At least the > > requested freq will be non-zero? Is that ok? Or the requested freq will > > show zero even if we take fwake? Thanks. > > If we take fwake before reading, requested frequency will not be zero even > if GT is idle. HW always retains the last requested frequency in that > register, never zeroes it like it does for actual_freq. So, showing the > non-zero value is the correct thing to do instead of artifically zeroing > it. So now, this I disagree with. For an idle device, hopefully in C6, there should be no reason to request a non-zero freq. So the requested freq should show 0. As i915 does (using gt parked and unparked states and using with_intel_runtime_pm_if_in_use()). If we are already "doing a runtime_get() during pmu_init" (as mentioned above) we need to devise some other way of making this happen (maybe by looking at xe_force_wake_domain ref field?). Let us wait for a second opinion on this. Thanks.