Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221107175705.2207842-1-Perry.Yuan@amd.com> <20221107175705.2207842-3-Perry.Yuan@amd.com>
In-Reply-To: <20221107175705.2207842-3-Perry.Yuan@amd.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Thu, 10 Nov 2022 15:57:46 +0100
Message-ID: <CAJZ5v0hfy2RnCfgaXyX629GxwPyn3Nh4WcMg9cxAisVYELqrYA@mail.gmail.com>
Subject: Re: [PATCH v3 2/8] Documentation: amd-pstate: add EPP profiles introduction
To:     Perry Yuan <Perry.Yuan@amd.com>
Cc:     rafael.j.wysocki@intel.com, ray.huang@amd.com,
        viresh.kumar@linaro.org, Deepak.Sharma@amd.com,
        Mario.Limonciello@amd.com, Nathan.Fontenot@amd.com,
        Alexander.Deucher@amd.com, Shimmer.Huang@amd.com,
        Xiaojian.Du@amd.com, Li.Meng@amd.com, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Nov 7, 2022 at 7:02 PM Perry Yuan <Perry.Yuan@amd.com> wrote:
>
> The patch add AMD pstate EPP feature introduction and what EPP
> preference supported for AMD processors.
>
> User can get supported list from
> energy_performance_available_preferences attribute file, or update
> current profile to energy_performance_preference file
>
> 1) See all EPP profiles
> $ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_available_preferences
> default performance balance_performance balance_power power
>
> 2) Check current EPP profile
> $ sudo cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference
> performance
>
> 3) Set new EPP profile
> $ sudo bash -c "echo power > /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference"
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>  Documentation/admin-guide/pm/amd-pstate.rst | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 8f3d30c5a0d8..78c6525d5a49 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -262,6 +262,25 @@ lowest non-linear performance in `AMD CPPC Performance Capability
>  <perf_cap_>`_.)
>  This attribute is read-only.
>
> +``energy_performance_available_preferences``
> +
> +All the supported EPP preference could be selected, List of the strings that
> +can be set to the ``energy_performance_preference`` attribute
> +those different profiles represent different energy vs efficiency hints provided
> +to low-level firmware
> +however, the ``default`` represents the epp value is set by platform firmware
> +This attribute is read-only.
> +
> +``energy_performance_preference``
> +
> +The current energy performance preference can be read from this attribute.
> +and user can change current preference according to energy or performance needs
> +Please get all support profiles list from
> +``energy_performance_available_preferences`` attribute, all the profiles are
> +integer values defined between 0 to 255 when EPP feature is enabled by platform
> +firmware, if EPP feature is disabled, driver will ignore the written value
> +This attribute is read-write.
> +
>  Other performance and frequency values can be read back from
>  ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
>
> --

So the interface is the same as for intel_pstate AFAICS, which is
good, because it is all consistent.

However, there is some code duplication introduced in the subsequent
patches and that may be a problem from the perspective of maintaining
that consistency in the future.

Have you at least considered introducing a common EPP support header
file that could be used by both intel_pstate and amd_pstate?