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 0276FC6FA83 for ; Tue, 13 Sep 2022 00:09:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D27CA10E4A8; Tue, 13 Sep 2022 00:09:16 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id CEC9D10E4A8 for ; Tue, 13 Sep 2022 00:09:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663027753; x=1694563753; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=iUCeFR/f5yNlyKlSCxsqJSr91YtUaJxKGNo/B6dr1zU=; b=gybMJMgLYzwVAkqfbymXPcLbK/Te8VamcDguer1ySnduXNWShYR39ABd SF+R4ajca6dSuODBkVxWb+dRV/b+uG6omH+9qXuQo5FHdFCSik6q52d5t x3Nk6MC1nNcjVllYV/gSECjmyYxCFnjrR2/wqUPsqzMtrARJqsnF/IYDS KJ7OMhtuampXgtlAQIIAwxIAl2BdOOsjVcSsK/+FwLdRlWJAv3H/wxWov h77QSi8U9FUr53tdOSO81CBymC9IExOyJybtCgPsbxQD2mtIs6WTV5dWy FdCg70Rb8Y0kdjNCFDcvbtVLtL+0eNWxaERXdWCEOH8eJ2uNwwTzecWvh A==; X-IronPort-AV: E=McAfee;i="6500,9779,10468"; a="277733312" X-IronPort-AV: E=Sophos;i="5.93,311,1654585200"; d="scan'208";a="277733312" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Sep 2022 17:09:13 -0700 X-IronPort-AV: E=Sophos;i="5.93,311,1654585200"; d="scan'208";a="616253595" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.54.92]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Sep 2022 17:09:12 -0700 Date: Mon, 12 Sep 2022 17:09:12 -0700 Message-ID: <87czc05e53.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Nilawar, Badal" In-Reply-To: <1ce34139-0b3f-6709-597f-e55437bccc0d@intel.com> References: <20220909025646.3397620-1-badal.nilawar@intel.com> <20220909025646.3397620-5-badal.nilawar@intel.com> <87k06c577l.wl-ashutosh.dixit@intel.com> <1ce34139-0b3f-6709-597f-e55437bccc0d@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.1 (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 4/6] drm/i915: Use GEN12 RPSTAT register 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 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote: > > >> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > >> index 958b37123bf1..a24704ec2c18 100644 > >> --- a/drivers/gpu/drm/i915/i915_pmu.c > >> +++ b/drivers/gpu/drm/i915/i915_pmu.c > >> @@ -371,7 +371,6 @@ static void > >> frequency_sample(struct intel_gt *gt, unsigned int period_ns) > >> { > >> struct drm_i915_private *i915 = gt->i915; > >> - struct intel_uncore *uncore = gt->uncore; > >> struct i915_pmu *pmu = &i915->pmu; > >> struct intel_rps *rps = >->rps; > >> > >> @@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > >> * case we assume the system is running at the intended > >> * frequency. Fortunately, the read should rarely fail! > >> */ > >> - val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1); > >> + val = intel_rps_read_rpstat(rps); > > > > Hmm, we got rid of _fw which the comment above refers to. Maybe we need a > > fw flag to intel_rps_read_rpstat? > > Above function before reading rpstat it checks if gt is awake. Ok, so you are referring to intel_gt_pm_get_if_awake check in frequency_sample. > So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with > forcewake.In that case we can remove above comment. Let me know your > thoughts on this. I am not entirely sure about this. For example in c1c82d267ae8 intel_uncore_read_fw was introduced with the same intel_gt_pm_get_if_awake check. So this would mean even if gt is awake not taking forcewake makes a difference. The same code pattern was retained in b66ecd0438bf. Maybe it's because there are no locks? Under the circumstances I think we could do one of two things: 1. If we want to drop _fw, we should do it as a separate patch with its own justification so it can be reviewed separately. 2. Otherwise as I mentioned we should retain the _fw and add a fw flag to intel_rps_read_rpstat. Thanks. -- Ashutosh > >> if (val) > >> val = intel_rps_get_cagf(rps, val); > >> else > >> -- > >> 2.25.1 > >>