On 14/7/2021 10:23 AM, Richael Zhuang wrote:

-----Original Message-----
From: David Hunt <david.h...@intel.com>
Sent: Wednesday, July 14, 2021 5:15 PM
To: Richael Zhuang <richael.zhu...@arm.com>; dev@dpdk.org
Cc: zhiminx.hu...@intel.com; sta...@dpdk.org
Subject: Re: [PATCH v1 1/1] test/power: check cpuinfo cur freq before scaling
cur freq


On 14/7/2021 9:44 AM, Richael Zhuang wrote:
For acpi_cpufreq and cppc_cpufreq, both cpuinfo_cur_freq and
scaling_cur_freq exist. For pstate, only scaling_cur_freq exists.
And value in scaling_cur_freq and cpuinfo_cur_freq may not be the
same. For acpi_cpufreq and cppc_cpufreq, we should check
cpuinfo_cur_freq. So here checking cpuinfo_cur_freq before
scaling_cur_freq to make sure it works for all cpufreq drivers.

Fixes: ff6dfb8e492f ("test/power: fix CPU frequency check")
Cc: david.h...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Richael Zhuang <richael.zhu...@arm.com>
---
   app/test/test_power_cpufreq.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test/test_power_cpufreq.c
b/app/test/test_power_cpufreq.c index b8fc53925c..f56abb6f86 100644
--- a/app/test/test_power_cpufreq.c
+++ b/app/test/test_power_cpufreq.c
@@ -62,13 +62,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx,
bool turbo)
        int i;

        if (snprintf(fullpath, sizeof(fullpath),
-               TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
+               TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
                return 0;
        }
        f = fopen(fullpath, "r");
        if (f == NULL) {
                if (snprintf(fullpath, sizeof(fullpath),
-                       TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0)
{
+                       TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0)
{
                        return 0;
                }
                f = fopen(fullpath, "r");

Hi Richael, I don't think this patch fixes anything. If the scaling file is not
available, it will then attempt to open the cpuinfo file.
Changing the order does not address the underlying issue.

It looks like the test is failing in check_cur_req, which is only rounding for
cppc driver. I think it also needs to round for the other drivers. I've just
checked intel_pstate driver now, and it needs the rounding. I would think
that acpi driver also needs it. I'll do a bit more investigation and see if I
can  change to acpi and attempt to confirm that all drivers need the rounding.

Rgds,
Dave.


Hi David,
For acpi_cpufreq and cppc_cpufreq, both two files exist. So with the current 
code, it will check the scaling_cur_freq. But I think for this two drivers, it 
should check cpuinfo_cur_freq but not scaling_cur_freq. From my system, for 
acpi cpufreq, the value in cpuinfo_cur_freq and scaling_cur_freq are not the 
same.


Ah OK. Now I see different the values in scaling_cur_freq and cpuinfo_cur_freq on a system here with acpi driver. The cpuinfo_cur_freq is already rounded, but the scaling_cur_freq is not.

# cat /sys/devices/system/cpu/cpu2/cpufreq/scaling_cur_freq
800259
# cat /sys/devices/system/cpu/cpu2/cpufreq/cpuinfo_cur_freq
800000

So I think it would make sense to include the re-order of the filenames as you suggest, but maybe also add rounding for the pstate driver, as that is needed.

-               if (env == PM_ENV_CPPC_CPUFREQ) {
+               if ((env == PM_ENV_CPPC_CPUFREQ) || (env == PM_ENV_PSTATE_CPUFREQ)) {

That avoids the complication of the acpi_cpufreq driver when turbo is enabled (2301000 for any turbo freq).

I've just run the test with the two changes (reorder filenames and round on pstate), and the power_cpufreq_autotest passes on both intel_pstate and acpi-cpufreq driver systems.

Regards,
Dave.



   So if not changing the check sequence, the result is:
##########
RTE>>power_cpufreq_autotest
POWER: Env isn't set yet!
POWER: Attempting to initialise ACPI cpufreq power management...
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 has exited from 'userspace' mode and been 
set back to the original
POWER: Lcore id 128 can not exceeds 127
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 is in use
POWER: Invalid lcore ID
POWER: NULL buffer supplied
POWER: Buffer size is not enough
POWER: Invalid lcore ID
POWER: Power management of lcore 2 has exited from 'userspace' mode and been 
set back to the original
Test Failed
RTE>>quit
#############

But after changing the check sequence, the result is OK from my test:
######
RTE>>power_cpufreq_autotest
POWER: Env isn't set yet!
POWER: Attempting to initialise ACPI cpufreq power management...
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 has exited from 'userspace' mode and been 
set back to the original
POWER: Lcore id 128 can not exceeds 127
POWER: Initialized successfully for lcore 2 power management
POWER: Power management of lcore 2 is in use
POWER: Invalid lcore ID
POWER: NULL buffer supplied
POWER: Buffer size is not enough
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid frequency index 64, which should be less than 3
POWER: Invalid frequency index 3, which should be less than 3
POWER: Failed to enable turbo on lcore 2
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid lcore ID
POWER: Invalid lcore ID
Turbo not available on lcore 2, skipping test
POWER: Power management of lcore 2 has exited from 'userspace' mode and been 
set back to the original
POWER: Power management of lcore 2 is not used
POWER: Lcore id 128 can not exceeds 127
Test OK
############

Reply via email to