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 CAC20C77B71 for ; Sat, 15 Apr 2023 00:10:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E683C10EEE1; Sat, 15 Apr 2023 00:10:39 +0000 (UTC) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id CB1CE10E1CF; Sat, 15 Apr 2023 00:10:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1681517436; x=1713053436; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=rwRC8jzF7WWo3+nmvgi26PGKk6lj/k6MQXkjBdno6WM=; b=S/mvq4IN6W2NDefAtCP7jsFFrh2VzesterpdARrvZMK8gPRpnHOj634r eoX6+Mgls+toTbYMTk+O++3w68UZfXpBNwWD8X7i2fRxRefapgNI+iLYK xp3EodrQgEQ4D8jSqPlwX9Mt84qU2WWIpDg6Obp4t2603tkBNbqpN9tT5 p//c/JOzK+qEyswuYqm17QHPa6KsAnO0ium4M6A9ICTb9w5ODaZRz4rIk RMdRfx5LiPANwg1r7t5HI4Zqjg9axHBvHGf7dTzywqCvfGsszM5HWzM7o 8MGUlLc2FYayT5aNdwDPSOMell/zsqaEWnnaa1SfKGdwAfA8C6G2xIXa/ Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10680"; a="333380602" X-IronPort-AV: E=Sophos;i="5.99,198,1677571200"; d="scan'208";a="333380602" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2023 17:10:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10680"; a="801376300" X-IronPort-AV: E=Sophos;i="5.99,198,1677571200"; d="scan'208";a="801376300" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.100.250]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2023 17:10:34 -0700 Date: Fri, 14 Apr 2023 16:49:44 -0700 Message-ID: <875y9yxbvr.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Vinay Belgaumkar In-Reply-To: <20230414223415.3077055-1-vinay.belgaumkar@intel.com> References: <20230414223415.3077055-1-vinay.belgaumkar@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 v3] drm/i915/guc/slpc: Provide sysfs for efficient freq 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, dri-devel@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Fri, 14 Apr 2023 15:34:15 -0700, Vinay Belgaumkar wrote: > > @@ -457,6 +458,34 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val) > return ret; > } > > +int intel_guc_slpc_set_ignore_eff_freq(struct intel_guc_slpc *slpc, bool val) > +{ > + struct drm_i915_private *i915 = slpc_to_i915(slpc); > + intel_wakeref_t wakeref; > + int ret = 0; > + > + /* Need a lock now since waitboost can be modifying min as well */ Delete comment. > + mutex_lock(&slpc->lock); Actually, don't need the lock itself now so delete the lock. Or, maybe the lock prevents the race if userspace writes to the sysfs when GuC reset is going on so let's retain the lock. But the comment is wrong. > + wakeref = intel_runtime_pm_get(&i915->runtime_pm); > + > + /* Ignore efficient freq if lower min freq is requested */ Delete comment, it's wrong. > + ret = slpc_set_param(slpc, > + SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, > + val); > + if (ret) { > + guc_probe_error(slpc_to_guc(slpc), "Failed to set efficient freq(%d): %pe\n", > + val, ERR_PTR(ret)); > + goto out; > + } > + > + slpc->ignore_eff_freq = val; > + This extra line can also be deleted. > +out: > + intel_runtime_pm_put(&i915->runtime_pm, wakeref); > + mutex_unlock(&slpc->lock); > + return ret; > +} > + > /** > * intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC. > * @slpc: pointer to intel_guc_slpc. > @@ -482,16 +511,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > mutex_lock(&slpc->lock); > wakeref = intel_runtime_pm_get(&i915->runtime_pm); > > - /* Ignore efficient freq if lower min freq is requested */ > - ret = slpc_set_param(slpc, > - SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, > - val < slpc->rp1_freq); > - if (ret) { > - guc_probe_error(slpc_to_guc(slpc), "Failed to toggle efficient freq: %pe\n", > - ERR_PTR(ret)); > - goto out; > - } > - Great, thanks! After taking care of the above, and seems there are also a couple of checkpatch errors, this is: Reviewed-by: Ashutosh Dixit