All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.