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;
From:   Thomas Renninger <trenn@suse.de>
To:     trenn@suse.com, shuah@kernel.org, Wyes Karny <wyes.karny@amd.com>
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        rafael@kernel.org, gautham.shenoy@amd.com,
        Mario.Limonciello@amd.com, Ray.Huang@amd.com, Perry.Yuan@amd.com,
        Wyes Karny <wyes.karny@amd.com>
Subject: Re: [PATCH v3 0/5] cpupower: Add various feature control support for
 amd_pstate
Date:   Fri, 14 Jul 2023 13:19:18 +0200
Message-ID: <1896655.g5d078U9FE@p200300d2573c9800520d172709106850.dip0.t-ipconnect.de>
In-Reply-To: <20230619190503.4061-1-wyes.karny@amd.com>
References: <20230619190503.4061-1-wyes.karny@amd.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

Hi,

sorry for the late answer, I wanted to give this a test, but could not
make it (yet).

My 2 cents on this one:

what Ray already mentioned is greatly appreciated: Find common APIs, not only 
AMD vs Intel, but general ones, outside of the driver specific sysfs dir, e.g.:
/sys/devices/system/cpu/cpufreq/...

But not at all costs and if the epp part is (AMD) specific and varies to others 
in meaning or in values, it has to be a specific file/API.

(Another Example/Request at the end)

And...: It would be nice to see some documentation in form of manpage 
enhancements. At least for the newly introduced arguments.
If not yet, there should be: possible value range, which machines (CPU flags, 
Zen 4, ...), if more complex technology, some kind of introduction or 
reference where to find things would be really great.

I wonder whether it could be possible to connect this with:
Documentation/admin-guide/pm/intel_pstate.rst
Documentation/admin-guide/pm/amd-pstate.rst

I guess there is a static web site address people could rely on where above
kernel docs are hosted forever?
Then it would make sense to link the cpupower manpage with it and keep the 
description short and avoid duplication?

             Thomas


Another Example/Request at the end:

One example I recently got asked for, the bug is private:
"cpupower is not returning max turbo active cores information on latest Intel 
processors."

Here are examples of 2 newer cpus that don't return the active core 
information like this: 
    3600 MHz max turbo 4 active cores
    3600 MHz max turbo 3 active cores
    3700 MHz max turbo 2 active cores
    3800 MHz max turbo 1 active cores

Not sure whether there finally is some kind of consolidation on turbo/boost/
dynamic/hw/bios interfaces in respect to turbo/boosting technology. But if 
anyhow possible and it makes sense, it would be really great to see one sysfs 
file exposed to userspace to read/enable/disable.

Possible cpufreqs (including or only) boost freqs, maybe with active cores if 
this is a general technique optionally added if avail?

Nothing which has to be implemented at all costs and right now, just an idea 
on how things could be consolidated at some point of time which might be wise 
to think about now already...