* [PATCH 0/2] amd-pstate: Add unit tests for state machine @ 2024-09-01 2:49 Mario Limonciello 2024-09-01 2:49 ` [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes Mario Limonciello 2024-09-01 2:49 ` [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches Mario Limonciello 0 siblings, 2 replies; 5+ messages in thread From: Mario Limonciello @ 2024-09-01 2:49 UTC (permalink / raw) To: Meng Li, Gautham R . Shenoy, Perry Yuan Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:CPU FREQUENCY SCALING FRAMEWORK, ptr1337, Mario Limonciello From: Mario Limonciello <mario.limonciello@amd.com> The state machine for amd-pstate allows changing from any supported mode to another. This is a relatively large matrix of possible moves that doesn't have test coverage. Walk through the matrix as part of amd-pstate-ut tests to make sure that all transitions work. Mario Limonciello (2): cpufreq/amd-pstate: Export symbols for changing modes cpufreq/amd-pstate-ut: Add test case for mode switches drivers/cpufreq/amd-pstate-ut.c | 41 ++++++++++++++++++++++++++++++++- drivers/cpufreq/amd-pstate.c | 23 ++++++++---------- drivers/cpufreq/amd-pstate.h | 14 +++++++++++ 3 files changed, 64 insertions(+), 14 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes 2024-09-01 2:49 [PATCH 0/2] amd-pstate: Add unit tests for state machine Mario Limonciello @ 2024-09-01 2:49 ` Mario Limonciello 2024-09-09 15:17 ` Yuan, Perry 2024-09-01 2:49 ` [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches Mario Limonciello 1 sibling, 1 reply; 5+ messages in thread From: Mario Limonciello @ 2024-09-01 2:49 UTC (permalink / raw) To: Meng Li, Gautham R . Shenoy, Perry Yuan Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:CPU FREQUENCY SCALING FRAMEWORK, ptr1337, Mario Limonciello From: Mario Limonciello <mario.limonciello@amd.com> In order to effectively test all mode switch combinations export everything necessarily for amd-pstate-ut to trigger a mode switch. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/cpufreq/amd-pstate.c | 23 ++++++++++------------- drivers/cpufreq/amd-pstate.h | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 073ca9caf52ac..93adde45bebce 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -60,18 +60,6 @@ #define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF #define AMD_CPPC_EPP_POWERSAVE 0xFF -/* - * enum amd_pstate_mode - driver working mode of amd pstate - */ -enum amd_pstate_mode { - AMD_PSTATE_UNDEFINED = 0, - AMD_PSTATE_DISABLE, - AMD_PSTATE_PASSIVE, - AMD_PSTATE_ACTIVE, - AMD_PSTATE_GUIDED, - AMD_PSTATE_MAX, -}; - static const char * const amd_pstate_mode_string[] = { [AMD_PSTATE_UNDEFINED] = "undefined", [AMD_PSTATE_DISABLE] = "disable", @@ -81,6 +69,14 @@ static const char * const amd_pstate_mode_string[] = { NULL, }; +const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode) +{ + if (mode < 0 || mode >= AMD_PSTATE_MAX) + return NULL; + return amd_pstate_mode_string[mode]; +} +EXPORT_SYMBOL_GPL(amd_pstate_get_mode_string); + struct quirk_entry { u32 nominal_freq; u32 lowest_freq; @@ -1352,7 +1348,7 @@ static ssize_t amd_pstate_show_status(char *buf) return sysfs_emit(buf, "%s\n", amd_pstate_mode_string[cppc_state]); } -static int amd_pstate_update_status(const char *buf, size_t size) +int amd_pstate_update_status(const char *buf, size_t size) { int mode_idx; @@ -1369,6 +1365,7 @@ static int amd_pstate_update_status(const char *buf, size_t size) return 0; } +EXPORT_SYMBOL_GPL(amd_pstate_update_status); static ssize_t status_show(struct device *dev, struct device_attribute *attr, char *buf) diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h index cc8bb2bc325aa..cd573bc6b6db8 100644 --- a/drivers/cpufreq/amd-pstate.h +++ b/drivers/cpufreq/amd-pstate.h @@ -103,4 +103,18 @@ struct amd_cpudata { bool boost_state; }; +/* + * enum amd_pstate_mode - driver working mode of amd pstate + */ +enum amd_pstate_mode { + AMD_PSTATE_UNDEFINED = 0, + AMD_PSTATE_DISABLE, + AMD_PSTATE_PASSIVE, + AMD_PSTATE_ACTIVE, + AMD_PSTATE_GUIDED, + AMD_PSTATE_MAX, +}; +const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode); +int amd_pstate_update_status(const char *buf, size_t size); + #endif /* _LINUX_AMD_PSTATE_H */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes 2024-09-01 2:49 ` [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes Mario Limonciello @ 2024-09-09 15:17 ` Yuan, Perry 0 siblings, 0 replies; 5+ messages in thread From: Yuan, Perry @ 2024-09-09 15:17 UTC (permalink / raw) To: Mario Limonciello, Meng, Li (Jassmine), Shenoy, Gautham Ranjal Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:CPU FREQUENCY SCALING FRAMEWORK, ptr1337@cachyos.org, Limonciello, Mario [AMD Official Use Only - AMD Internal Distribution Only] Hi Mario, > -----Original Message----- > From: Mario Limonciello <superm1@kernel.org> > Sent: Sunday, September 1, 2024 10:49 AM > To: Meng, Li (Jassmine) <Li.Meng@amd.com>; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Yuan, Perry <Perry.Yuan@amd.com> > Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT) <linux- > kernel@vger.kernel.org>; open list:CPU FREQUENCY SCALING FRAMEWORK > <linux-pm@vger.kernel.org>; ptr1337@cachyos.org; Limonciello, Mario > <Mario.Limonciello@amd.com> > Subject: [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing > modes > > From: Mario Limonciello <mario.limonciello@amd.com> > > In order to effectively test all mode switch combinations export everything > necessarily for amd-pstate-ut to trigger a mode switch. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 23 ++++++++++------------- > drivers/cpufreq/amd-pstate.h | 14 ++++++++++++++ > 2 files changed, 24 insertions(+), 13 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 073ca9caf52ac..93adde45bebce 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -60,18 +60,6 @@ > #define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF > #define AMD_CPPC_EPP_POWERSAVE 0xFF > > -/* > - * enum amd_pstate_mode - driver working mode of amd pstate > - */ > -enum amd_pstate_mode { > - AMD_PSTATE_UNDEFINED = 0, > - AMD_PSTATE_DISABLE, > - AMD_PSTATE_PASSIVE, > - AMD_PSTATE_ACTIVE, > - AMD_PSTATE_GUIDED, > - AMD_PSTATE_MAX, > -}; > - > static const char * const amd_pstate_mode_string[] = { > [AMD_PSTATE_UNDEFINED] = "undefined", > [AMD_PSTATE_DISABLE] = "disable", > @@ -81,6 +69,14 @@ static const char * const amd_pstate_mode_string[] = > { > NULL, > }; > > +const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode) > { > + if (mode < 0 || mode >= AMD_PSTATE_MAX) > + return NULL; > + return amd_pstate_mode_string[mode]; > +} > +EXPORT_SYMBOL_GPL(amd_pstate_get_mode_string); > + > struct quirk_entry { > u32 nominal_freq; > u32 lowest_freq; > @@ -1352,7 +1348,7 @@ static ssize_t amd_pstate_show_status(char *buf) > return sysfs_emit(buf, "%s\n", > amd_pstate_mode_string[cppc_state]); > } > > -static int amd_pstate_update_status(const char *buf, size_t size) > +int amd_pstate_update_status(const char *buf, size_t size) > { > int mode_idx; > > @@ -1369,6 +1365,7 @@ static int amd_pstate_update_status(const char > *buf, size_t size) > > return 0; > } > +EXPORT_SYMBOL_GPL(amd_pstate_update_status); > > static ssize_t status_show(struct device *dev, > struct device_attribute *attr, char *buf) diff --git > a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h index > cc8bb2bc325aa..cd573bc6b6db8 100644 > --- a/drivers/cpufreq/amd-pstate.h > +++ b/drivers/cpufreq/amd-pstate.h > @@ -103,4 +103,18 @@ struct amd_cpudata { > bool boost_state; > }; > > +/* > + * enum amd_pstate_mode - driver working mode of amd pstate */ enum > +amd_pstate_mode { > + AMD_PSTATE_UNDEFINED = 0, > + AMD_PSTATE_DISABLE, > + AMD_PSTATE_PASSIVE, > + AMD_PSTATE_ACTIVE, > + AMD_PSTATE_GUIDED, > + AMD_PSTATE_MAX, > +}; > +const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode); > int > +amd_pstate_update_status(const char *buf, size_t size); > + > #endif /* _LINUX_AMD_PSTATE_H */ > -- > 2.43.0 LGTM, move the amd_pstate_mode to header file that will be used by UT test . Thanks for the improvement. Reviewed-by: Perry Yuan <Perry.Yuan@amd.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches 2024-09-01 2:49 [PATCH 0/2] amd-pstate: Add unit tests for state machine Mario Limonciello 2024-09-01 2:49 ` [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes Mario Limonciello @ 2024-09-01 2:49 ` Mario Limonciello 2024-09-09 15:24 ` Yuan, Perry 1 sibling, 1 reply; 5+ messages in thread From: Mario Limonciello @ 2024-09-01 2:49 UTC (permalink / raw) To: Meng Li, Gautham R . Shenoy, Perry Yuan Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:CPU FREQUENCY SCALING FRAMEWORK, ptr1337, Mario Limonciello From: Mario Limonciello <mario.limonciello@amd.com> There is a state machine in the amd-pstate driver utilized for switches for all modes. To make sure that cleanup and setup works properly for each mode add a unit test case that tries all combinations. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/cpufreq/amd-pstate-ut.c | 41 ++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c index b7318669485e4..c291b3dbec381 100644 --- a/drivers/cpufreq/amd-pstate-ut.c +++ b/drivers/cpufreq/amd-pstate-ut.c @@ -54,12 +54,14 @@ static void amd_pstate_ut_acpi_cpc_valid(u32 index); static void amd_pstate_ut_check_enabled(u32 index); static void amd_pstate_ut_check_perf(u32 index); static void amd_pstate_ut_check_freq(u32 index); +static void amd_pstate_ut_check_driver(u32 index); static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, {"amd_pstate_ut_check_enabled", amd_pstate_ut_check_enabled }, {"amd_pstate_ut_check_perf", amd_pstate_ut_check_perf }, - {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq } + {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq }, + {"amd_pstate_ut_check_driver", amd_pstate_ut_check_driver } }; static bool get_shared_mem(void) @@ -257,6 +259,43 @@ static void amd_pstate_ut_check_freq(u32 index) cpufreq_cpu_put(policy); } +static int amd_pstate_set_mode(enum amd_pstate_mode mode) +{ + const char *mode_str = amd_pstate_get_mode_string(mode); + + pr_debug("->setting mode to %s\n", mode_str); + + return amd_pstate_update_status(mode_str, strlen(mode_str)); +} + +static void amd_pstate_ut_check_driver(u32 index) +{ + enum amd_pstate_mode mode1, mode2; + int ret; + + for (mode1 = AMD_PSTATE_DISABLE; mode1 < AMD_PSTATE_MAX; mode1++) { + ret = amd_pstate_set_mode(mode1); + if (ret) + goto out; + for (mode2 = AMD_PSTATE_DISABLE; mode2 < AMD_PSTATE_MAX; mode2++) { + if (mode1 == mode2) + continue; + ret = amd_pstate_set_mode(mode2); + if (ret) + goto out; + } + } +out: + if (ret) + pr_warn("%s: failed to update status for %s->%s: %d\n", __func__, + amd_pstate_get_mode_string(mode1), + amd_pstate_get_mode_string(mode2), ret); + + amd_pstate_ut_cases[index].result = ret ? + AMD_PSTATE_UT_RESULT_FAIL : + AMD_PSTATE_UT_RESULT_PASS; +} + static int __init amd_pstate_ut_init(void) { u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases); -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches 2024-09-01 2:49 ` [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches Mario Limonciello @ 2024-09-09 15:24 ` Yuan, Perry 0 siblings, 0 replies; 5+ messages in thread From: Yuan, Perry @ 2024-09-09 15:24 UTC (permalink / raw) To: Mario Limonciello, Meng, Li (Jassmine), Shenoy, Gautham Ranjal Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:CPU FREQUENCY SCALING FRAMEWORK, ptr1337@cachyos.org, Limonciello, Mario [AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Mario Limonciello <superm1@kernel.org> > Sent: Sunday, September 1, 2024 10:49 AM > To: Meng, Li (Jassmine) <Li.Meng@amd.com>; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Yuan, Perry <Perry.Yuan@amd.com> > Cc: open list:X86 ARCHITECTURE (32-BIT AND 64-BIT) <linux- > kernel@vger.kernel.org>; open list:CPU FREQUENCY SCALING FRAMEWORK > <linux-pm@vger.kernel.org>; ptr1337@cachyos.org; Limonciello, Mario > <Mario.Limonciello@amd.com> > Subject: [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode > switches > > From: Mario Limonciello <mario.limonciello@amd.com> > > There is a state machine in the amd-pstate driver utilized for switches for all > modes. To make sure that cleanup and setup works properly for each mode > add a unit test case that tries all combinations. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate-ut.c | 41 > ++++++++++++++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate- > ut.c index b7318669485e4..c291b3dbec381 100644 > --- a/drivers/cpufreq/amd-pstate-ut.c > +++ b/drivers/cpufreq/amd-pstate-ut.c > @@ -54,12 +54,14 @@ static void amd_pstate_ut_acpi_cpc_valid(u32 index); > static void amd_pstate_ut_check_enabled(u32 index); static void > amd_pstate_ut_check_perf(u32 index); static void > amd_pstate_ut_check_freq(u32 index); > +static void amd_pstate_ut_check_driver(u32 index); > > static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { > {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, > {"amd_pstate_ut_check_enabled", > amd_pstate_ut_check_enabled }, > {"amd_pstate_ut_check_perf", amd_pstate_ut_check_perf }, > - {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq } > + {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq }, > + {"amd_pstate_ut_check_driver", > amd_pstate_ut_check_driver } > }; > > static bool get_shared_mem(void) > @@ -257,6 +259,43 @@ static void amd_pstate_ut_check_freq(u32 index) > cpufreq_cpu_put(policy); > } > > +static int amd_pstate_set_mode(enum amd_pstate_mode mode) { > + const char *mode_str = amd_pstate_get_mode_string(mode); > + > + pr_debug("->setting mode to %s\n", mode_str); > + > + return amd_pstate_update_status(mode_str, strlen(mode_str)); } > + > +static void amd_pstate_ut_check_driver(u32 index) { > + enum amd_pstate_mode mode1, mode2; > + int ret; > + > + for (mode1 = AMD_PSTATE_DISABLE; mode1 < AMD_PSTATE_MAX; > mode1++) { > + ret = amd_pstate_set_mode(mode1); > + if (ret) > + goto out; > + for (mode2 = AMD_PSTATE_DISABLE; mode2 < > AMD_PSTATE_MAX; mode2++) { > + if (mode1 == mode2) > + continue; > + ret = amd_pstate_set_mode(mode2); > + if (ret) > + goto out; > + } > + } Dose the mode switching test need to add some delay between the previous and new mode ? If lowlevel power firmware failed to handle the modes frequently, you can consider adding delay in future. Besides that, the patch looks good to me. Reviewed-by: Perry Yuan <perry.yuan@amd.com> > +out: > + if (ret) > + pr_warn("%s: failed to update status for %s->%s: %d\n", > __func__, > + amd_pstate_get_mode_string(mode1), > + amd_pstate_get_mode_string(mode2), ret); > + > + amd_pstate_ut_cases[index].result = ret ? > + AMD_PSTATE_UT_RESULT_FAIL : > + AMD_PSTATE_UT_RESULT_PASS; > +} > + > static int __init amd_pstate_ut_init(void) { > u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases); > -- > 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-09 15:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-01 2:49 [PATCH 0/2] amd-pstate: Add unit tests for state machine Mario Limonciello 2024-09-01 2:49 ` [PATCH 1/2] cpufreq/amd-pstate: Export symbols for changing modes Mario Limonciello 2024-09-09 15:17 ` Yuan, Perry 2024-09-01 2:49 ` [PATCH 2/2] cpufreq/amd-pstate-ut: Add test case for mode switches Mario Limonciello 2024-09-09 15:24 ` Yuan, Perry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).