public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
To: Mario Limonciello <mario.limonciello@amd.com>
Cc: Perry Yuan <perry.yuan@amd.com>,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
Subject: Re: [PATCH 11/15] cpufreq/amd-pstate: Cache EPP value and use that everywhere
Date: Fri, 6 Dec 2024 21:44:38 +0530	[thread overview]
Message-ID: <Z1Mi7hclcaSaRRjO@BLRRASHENOY1.amd.com> (raw)
In-Reply-To: <20241205222847.7889-12-mario.limonciello@amd.com>

Hello Mario,

On Thu, Dec 05, 2024 at 04:28:43PM -0600, Mario Limonciello wrote:
> Cache the value in cpudata->epp_cached, and use that for all callers.
> As all callers use cached value merge amd_pstate_get_energy_pref_index()
> into show_energy_performance_preference().
> 
> Check if the EPP value is changed before writing it to MSR or
> shared memory region.

The patch looks ok to me. Just one comment below:

[..snip..]

> @@ -1610,6 +1591,8 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>  
>  	cpudata->policy = policy->policy;
>  
> +	guard(mutex)(&amd_pstate_limits_lock);
> +

Shouldn't this hunk be a different patch. The changelog doesn't say
anything about making amd_pstate_epp_update_limit() call while holding
the amd_pstate_limits_lock.


>  	ret = amd_pstate_epp_update_limit(policy);
>  	if (ret)
>  		return ret;
> -- 
> 2.43.0
>

For the rest of the patch

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

--
Thanks and Regards
gautham.

  reply	other threads:[~2024-12-06 16:14 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-05 22:28 [PATCH 00/15] amd-pstate 6.14 cleanups and improvements Mario Limonciello
2024-12-05 22:28 ` [PATCH 01/15] cpufreq/amd-pstate: Add trace event for EPP perf updates Mario Limonciello
2024-12-06  5:05   ` Yuan, Perry
2024-12-06  5:44   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 02/15] cpufreq/amd-pstate: convert mutex use to guard() Mario Limonciello
2024-12-06  6:15   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 03/15] cpufreq/amd-pstate: Drop cached epp_policy variable Mario Limonciello
2024-12-06  6:16   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 04/15] cpufreq/amd-pstate: Use FIELD_PREP and FIELD_GET macros Mario Limonciello
2024-12-06  6:21   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 05/15] cpufreq/amd-pstate: Store the boost numerator as highest perf again Mario Limonciello
2024-12-06  6:25   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 06/15] cpufreq/amd-pstate: Use boost numerator for upper bound of frequencies Mario Limonciello
2024-12-06  6:27   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 07/15] cpufreq/amd-pstate: Only update the cached value in msr_set_epp() on success Mario Limonciello
2024-12-06  6:32   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 08/15] cpufreq/amd-pstate: store all values in cpudata struct in khz Mario Limonciello
2024-12-06  6:43   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 09/15] cpufreq/amd-pstate: Change amd_pstate_update_perf() to return an int Mario Limonciello
2024-12-06  6:43   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 10/15] cpufreq/amd-pstate: Move limit updating code Mario Limonciello
2024-12-06  3:10   ` kernel test robot
2024-12-06 15:19   ` Gautham R. Shenoy
2024-12-05 22:28 ` [PATCH 11/15] cpufreq/amd-pstate: Cache EPP value and use that everywhere Mario Limonciello
2024-12-06 16:14   ` Gautham R. Shenoy [this message]
2024-12-06 16:16     ` Mario Limonciello
2024-12-05 22:28 ` [PATCH 12/15] cpufreq/amd-pstate: Always write EPP value when updating perf Mario Limonciello
2024-12-05 22:28 ` [PATCH 13/15] cpufreq/amd-pstate: Check if CPPC request has changed before writing to the MSR or shared memory Mario Limonciello
2024-12-05 22:28 ` [PATCH 14/15] cpufreq/amd-pstate: Drop ret variable from amd_pstate_set_energy_pref_index() Mario Limonciello
2024-12-05 22:28 ` [PATCH 15/15] cpufreq/amd-pstate: Set different default EPP policy for Epyc and Ryzen Mario Limonciello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z1Mi7hclcaSaRRjO@BLRRASHENOY1.amd.com \
    --to=gautham.shenoy@amd.com \
    --cc=Dhananjay.Ugwekar@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=perry.yuan@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox