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 00757C38145 for ; Thu, 8 Sep 2022 05:26:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 20D0210E958; Thu, 8 Sep 2022 05:26:22 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 46F0410E94E for ; Thu, 8 Sep 2022 05:26:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662614779; x=1694150779; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=g/+FfjJBJcApweaS6N86NKrOjACCbZgnEHsoz4Sztuk=; b=WIOAWC6UR19BeyMum5dbxXUpnBrF0IZ2U/nAIPqRQbzxyHZujSiSLk4H 1MC3mVzDG9cCLFCfvXVJhzh0/ZAvli5EyEp+8D961VVM8tYvyCA2gsQEi wtl4AiGE2ripiyRJl9X/stel+1tKTcykOoUvUmgEduJlVrRiylLpeRT2z 3N8vY1eSsGQR5FVxcom4VSNPnNNCyazNPf6XhaDA4Ikdv7Rs2XiA6xxZq uobRR2150YWGJgzps5R3JC10VAdhgzjcPRCeQhYlfqIS8ubk1SevNFd2k BAoi6YsNIysE+wihU4aZaWlZDbP0Rgzb2IziYRgkXP4a71X3L9xIrKK4v g==; X-IronPort-AV: E=McAfee;i="6500,9779,10463"; a="383365724" X-IronPort-AV: E=Sophos;i="5.93,299,1654585200"; d="scan'208";a="383365724" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Sep 2022 22:26:18 -0700 X-IronPort-AV: E=Sophos;i="5.93,299,1654585200"; d="scan'208";a="703874376" Received: from wbleichn-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.224.252]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Sep 2022 22:26:18 -0700 Date: Wed, 07 Sep 2022 22:26:18 -0700 Message-ID: <87a67axwqt.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Jani Nikula In-Reply-To: <87edwqb1mv.fsf@intel.com> References: <20220902235302.1112388-1-ashutosh.dixit@intel.com> <20220902235302.1112388-7-ashutosh.dixit@intel.com> <87edwqb1mv.fsf@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 6/6] drm/i915/rps: Freq caps for MTL 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, 05 Sep 2022 02:40:08 -0700, Jani Nikula wrote: > On Fri, 02 Sep 2022, Ashutosh Dixit wrote: > > For MTL, when reading from HW, RP0, RP1 (actuall RPe) and RPn freq use an > > entirely different set of registers with different fields, bitwidths and > > units. > > > > Cc: Badal Nilawar > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/i915/gt/intel_rps.c | 20 ++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ > > 2 files changed, 29 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > > index 579ae9ac089c..e7ab172698e3 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > > @@ -1085,6 +1085,23 @@ static u32 intel_rps_read_state_cap(struct intel_rps *rps) > > return intel_uncore_read(uncore, GEN6_RP_STATE_CAP); > > } > > > > +static void > > +mtl_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps) > > +{ > > + struct intel_uncore *uncore = rps_to_uncore(rps); > > + u32 rp_state_cap = rps_to_gt(rps)->type == GT_MEDIA ? > > + intel_uncore_read(uncore, MTL_MEDIAP_STATE_CAP) : > > + intel_uncore_read(uncore, MTL_RP_STATE_CAP); > > + u32 rpe = rps_to_gt(rps)->type == GT_MEDIA ? > > + intel_uncore_read(uncore, MTL_MPE_FREQUENCY) : > > + intel_uncore_read(uncore, MTL_GT_RPE_FREQUENCY); > > + > > + /* MTL values are in units of 16.67 MHz */ > > + caps->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, rp_state_cap); > > + caps->min_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, rp_state_cap); > > + caps->rp1_freq = REG_FIELD_GET(MTL_RPE_MASK, rpe); > > +} > > + > > /** > > * gen6_rps_get_freq_caps - Get freq caps exposed by HW > > * @rps: the intel_rps structure > > @@ -1098,6 +1115,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *c > > struct drm_i915_private *i915 = rps_to_i915(rps); > > u32 rp_state_cap; > > > > + if (IS_METEORLAKE(i915)) > > + return mtl_get_freq_caps(rps, caps); > > + > > Please make gen6_rps_get_freq_caps() static, and add > > intel_rps_get_freq_caps() > { > if (IS_METEORLAKE(i915)) > return mtl_get_freq_caps(rps, caps); > else > return gen6_rps_get_freq_caps(rps, caps); > } > > Or something. A general name like intel_rps_get_freq_caps name does not sit well with the current code. intel_rps_get_freq_caps was actually used in earlier versions of the patch: https://patchwork.freedesktop.org/patch/479179/?series=101606&rev=3 but was later changed to gen6_rps_get_freq_caps based on review comments. Afaiu in i915 a name such as gen6_rps_get_freq_caps implies "Gen6 and later" and the gen6_rps_get_freq_caps name has actually proved quite useful in reminding people that there are earlier/other generations not covered by the function. See intel_rps_init. Further the call stack is: intel_rps_init -> gen6_rps_init -> gen6_rps_get_freq_caps So it would look odd if we called intel_rps_get_freq_caps from gen6_rps_init. Therefore what I have done in v2 is: s/gen6_rps_get_freq_caps/__gen6_rps_get_freq_caps/ and then gen6_rps_get_freq_caps() { if (IS_METEORLAKE(i915)) return mtl_get_freq_caps(rps, caps); else return __gen6_rps_get_freq_caps(rps, caps); } Thanks. -- Ashutosh > > rp_state_cap = intel_rps_read_state_cap(rps); > > > > /* static values from HW: RP0 > RP1 > RPn (min_freq) */ > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 06d555321651..d78f9675aa57 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -1792,6 +1792,15 @@ > > #define XEHPSDV_RP_STATE_CAP _MMIO(0x250014) > > #define PVC_RP_STATE_CAP _MMIO(0x281014) > > > > +#define MTL_RP_STATE_CAP _MMIO(0x138000) > > +#define MTL_MEDIAP_STATE_CAP _MMIO(0x138020) > > +#define MTL_RP0_CAP_MASK REG_GENMASK(8, 0) > > +#define MTL_RPN_CAP_MASK REG_GENMASK(24, 16) > > + > > +#define MTL_GT_RPE_FREQUENCY _MMIO(0x13800c) > > +#define MTL_MPE_FREQUENCY _MMIO(0x13802c) > > +#define MTL_RPE_MASK REG_GENMASK(8, 0) > > + > > #define GT0_PERF_LIMIT_REASONS _MMIO(0x1381a8) > > #define GT0_PERF_LIMIT_REASONS_MASK 0xde3 > > #define PROCHOT_MASK REG_BIT(1) > > -- > Jani Nikula, Intel Open Source Graphics Center