From: Mario Limonciello <mario.limonciello@amd.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
hdegoede@redhat.com, markgross@kernel.org
Cc: platform-driver-x86@vger.kernel.org, Patil.Reddy@amd.com
Subject: Re: [PATCH v4 1/2] platform/x86/amd/pmf: Add PMF acpi debug support
Date: Wed, 10 May 2023 10:23:47 -0500 [thread overview]
Message-ID: <be84e061-5660-3fc9-7fcf-ff8b2cefaa1d@amd.com> (raw)
In-Reply-To: <20230510144751.66601-1-Shyam-sundar.S-k@amd.com>
On 5/10/23 09:47, Shyam Sundar S K wrote:
> PMF driver maintains an internal config store for each PMF feature
> after the feature init happens. Having a debug mechanism to triage
> in-field issues w.r.t to mode switch not happening based on the OEM
> fed values via the ACPI method to PMF driver is becoming the need of
> the hour. Add support to get more ACPI debug spew guarded by a CONFIG.
>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v4:
> - No change
>
> v3:
> - Based on review-hans branch
> - Address Ilpo's review remarks
> - use the right format specifiers while printing the data
>
> v2:
> - Based on review-hans branch
> - use a pointer and not create a local copy while dumping
> - use dummy #else blocks
>
> drivers/platform/x86/amd/pmf/Kconfig | 11 +++
> drivers/platform/x86/amd/pmf/auto-mode.c | 120 +++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/cnqf.c | 56 +++++++++++
> drivers/platform/x86/amd/pmf/sps.c | 55 +++++++++++
> 4 files changed, 242 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index d87986adf91e..3064bc8ea167 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -16,3 +16,14 @@ config AMD_PMF
>
> To compile this driver as a module, choose M here: the module will
> be called amd_pmf.
> +
> +config AMD_PMF_DEBUG
> + bool "PMF debug information"
> + depends on AMD_PMF
> + help
> + Enabling this option would give more debug information on the OEM fed
> + power setting values for each of the PMF feature. PMF driver gets this
> + information after evaluating a ACPI method and the information is stored
> + in the PMF config store.
> +
> + Say Y here to enable more debug logs and Say N here if you are not sure.
> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> index 96a8e1832c05..64425201d353 100644
> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> @@ -15,6 +15,100 @@
> static struct auto_mode_mode_config config_store;
> static const char *state_as_str(unsigned int state);
>
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +static void amd_pmf_dump_auto_mode_defaults(struct auto_mode_mode_config *data)
> +{
> + struct auto_mode_mode_settings *its_mode;
> +
> + pr_debug("Auto Mode Data - BEGIN\n");
> +
> + /* time constant */
> + pr_debug("balanced_to_perf: %u ms\n",
> + data->transition[AUTO_TRANSITION_TO_PERFORMANCE].time_constant);
> + pr_debug("perf_to_balanced: %u ms\n",
> + data->transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE].time_constant);
> + pr_debug("quiet_to_balanced: %u ms\n",
> + data->transition[AUTO_TRANSITION_FROM_QUIET_TO_BALANCE].time_constant);
> + pr_debug("balanced_to_quiet: %u ms\n",
> + data->transition[AUTO_TRANSITION_TO_QUIET].time_constant);
> +
> + /* power floor */
> + pr_debug("pfloor_perf: %u mW\n", data->mode_set[AUTO_PERFORMANCE].power_floor);
> + pr_debug("pfloor_balanced: %u mW\n", data->mode_set[AUTO_BALANCE].power_floor);
> + pr_debug("pfloor_quiet: %u mW\n", data->mode_set[AUTO_QUIET].power_floor);
> +
> + /* Power delta for mode change */
> + pr_debug("pd_balanced_to_perf: %u mW\n",
> + data->transition[AUTO_TRANSITION_TO_PERFORMANCE].power_delta);
> + pr_debug("pd_perf_to_balanced: %u mW\n",
> + data->transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE].power_delta);
> + pr_debug("pd_quiet_to_balanced: %u mW\n",
> + data->transition[AUTO_TRANSITION_FROM_QUIET_TO_BALANCE].power_delta);
> + pr_debug("pd_balanced_to_quiet: %u mW\n",
> + data->transition[AUTO_TRANSITION_TO_QUIET].power_delta);
> +
> + /* skin temperature limits */
> + its_mode = &data->mode_set[AUTO_PERFORMANCE_ON_LAP];
> + pr_debug("stt_apu_perf_on_lap: %u C\n",
> + its_mode->power_control.stt_skin_temp[STT_TEMP_APU]);
> + pr_debug("stt_hs2_perf_on_lap: %u C\n",
> + its_mode->power_control.stt_skin_temp[STT_TEMP_HS2]);
> + pr_debug("stt_min_limit_perf_on_lap: %u mW\n", its_mode->power_control.stt_min);
> +
> + its_mode = &data->mode_set[AUTO_PERFORMANCE];
> + pr_debug("stt_apu_perf: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_APU]);
> + pr_debug("stt_hs2_perf: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_HS2]);
> + pr_debug("stt_min_limit_perf: %u mW\n", its_mode->power_control.stt_min);
> +
> + its_mode = &data->mode_set[AUTO_BALANCE];
> + pr_debug("stt_apu_balanced: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_APU]);
> + pr_debug("stt_hs2_balanced: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_HS2]);
> + pr_debug("stt_min_limit_balanced: %u mW\n", its_mode->power_control.stt_min);
> +
> + its_mode = &data->mode_set[AUTO_QUIET];
> + pr_debug("stt_apu_quiet: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_APU]);
> + pr_debug("stt_hs2_quiet: %u C\n", its_mode->power_control.stt_skin_temp[STT_TEMP_HS2]);
> + pr_debug("stt_min_limit_quiet: %u mW\n", its_mode->power_control.stt_min);
> +
> + /* SPL based power limits */
> + its_mode = &data->mode_set[AUTO_PERFORMANCE_ON_LAP];
> + pr_debug("fppt_perf_on_lap: %u mW\n", its_mode->power_control.fppt);
> + pr_debug("sppt_perf_on_lap: %u mW\n", its_mode->power_control.sppt);
> + pr_debug("spl_perf_on_lap: %u mW\n", its_mode->power_control.spl);
> + pr_debug("sppt_apu_only_perf_on_lap: %u mW\n", its_mode->power_control.sppt_apu_only);
> +
> + its_mode = &data->mode_set[AUTO_PERFORMANCE];
> + pr_debug("fppt_perf: %u mW\n", its_mode->power_control.fppt);
> + pr_debug("sppt_perf: %u mW\n", its_mode->power_control.sppt);
> + pr_debug("spl_perf: %u mW\n", its_mode->power_control.spl);
> + pr_debug("sppt_apu_only_perf: %u mW\n", its_mode->power_control.sppt_apu_only);
> +
> + its_mode = &data->mode_set[AUTO_BALANCE];
> + pr_debug("fppt_balanced: %u mW\n", its_mode->power_control.fppt);
> + pr_debug("sppt_balanced: %u mW\n", its_mode->power_control.sppt);
> + pr_debug("spl_balanced: %u mW\n", its_mode->power_control.spl);
> + pr_debug("sppt_apu_only_balanced: %u mW\n", its_mode->power_control.sppt_apu_only);
> +
> + its_mode = &data->mode_set[AUTO_QUIET];
> + pr_debug("fppt_quiet: %u mW\n", its_mode->power_control.fppt);
> + pr_debug("sppt_quiet: %u mW\n", its_mode->power_control.sppt);
> + pr_debug("spl_quiet: %u mW\n", its_mode->power_control.spl);
> + pr_debug("sppt_apu_only_quiet: %u mW\n", its_mode->power_control.sppt_apu_only);
> +
> + /* Fan ID */
> + pr_debug("fan_id_perf: %lu\n",
> + data->mode_set[AUTO_PERFORMANCE].fan_control.fan_id);
> + pr_debug("fan_id_balanced: %lu\n",
> + data->mode_set[AUTO_BALANCE].fan_control.fan_id);
> + pr_debug("fan_id_quiet: %lu\n",
> + data->mode_set[AUTO_QUIET].fan_control.fan_id);
> +
> + pr_debug("Auto Mode Data - END\n");
> +}
> +#else
> +static void amd_pmf_dump_auto_mode_defaults(struct auto_mode_mode_config *data) {}
> +#endif
> +
> static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
> struct auto_mode_mode_config *table)
> {
> @@ -140,6 +234,30 @@ static void amd_pmf_get_power_threshold(void)
> config_store.transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE].power_threshold =
> config_store.mode_set[AUTO_PERFORMANCE].power_floor -
> config_store.transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE].power_delta;
> +
> +#ifdef CONFIG_AMD_PMF_DEBUG
> + pr_debug("[AUTO MODE TO_QUIET] pt: %u mW pf: %u mW pd: %u mW\n",
> + config_store.transition[AUTO_TRANSITION_TO_QUIET].power_threshold,
> + config_store.mode_set[AUTO_BALANCE].power_floor,
> + config_store.transition[AUTO_TRANSITION_TO_QUIET].power_delta);
> +
> + pr_debug("[AUTO MODE TO_PERFORMANCE] pt: %u mW pf: %u mW pd: %u mW\n",
> + config_store.transition[AUTO_TRANSITION_TO_PERFORMANCE].power_threshold,
> + config_store.mode_set[AUTO_BALANCE].power_floor,
> + config_store.transition[AUTO_TRANSITION_TO_PERFORMANCE].power_delta);
> +
> + pr_debug("[AUTO MODE QUIET_TO_BALANCE] pt: %u mW pf: %u mW pd: %u mW\n",
> + config_store.transition[AUTO_TRANSITION_FROM_QUIET_TO_BALANCE]
> + .power_threshold,
> + config_store.mode_set[AUTO_QUIET].power_floor,
> + config_store.transition[AUTO_TRANSITION_FROM_QUIET_TO_BALANCE].power_delta);
> +
> + pr_debug("[AUTO MODE PERFORMANCE_TO_BALANCE] pt: %u mW pf: %u mW pd: %u mW\n",
> + config_store.transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE]
> + .power_threshold,
> + config_store.mode_set[AUTO_PERFORMANCE].power_floor,
> + config_store.transition[AUTO_TRANSITION_FROM_PERFORMANCE_TO_BALANCE].power_delta);
> +#endif
> }
>
> static const char *state_as_str(unsigned int state)
> @@ -262,6 +380,8 @@ static void amd_pmf_load_defaults_auto_mode(struct amd_pmf_dev *dev)
> /* set to initial default values */
> config_store.current_mode = AUTO_BALANCE;
> dev->socket_power_history_idx = -1;
> +
> + amd_pmf_dump_auto_mode_defaults(&config_store);
> }
>
> int amd_pmf_reset_amt(struct amd_pmf_dev *dev)
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index 4beb22a19466..35af7c18f600 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -13,6 +13,61 @@
>
> static struct cnqf_config config_store;
>
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +static const char *state_as_str_cnqf(unsigned int state)
> +{
> + switch (state) {
> + case APMF_CNQF_TURBO:
> + return "turbo";
> + case APMF_CNQF_PERFORMANCE:
> + return "performance";
> + case APMF_CNQF_BALANCE:
> + return "balance";
> + case APMF_CNQF_QUIET:
> + return "quiet";
> + default:
> + return "Unknown CnQF State";
> + }
> +}
> +
> +static void amd_pmf_cnqf_dump_defaults(struct apmf_dyn_slider_output *data, int idx)
> +{
> + int i;
> +
> + pr_debug("Dynamic Slider %s Defaults - BEGIN\n", idx ? "DC" : "AC");
> + pr_debug("size: %u\n", data->size);
> + pr_debug("flags: 0x%x\n", data->flags);
> +
> + /* Time constants */
> + pr_debug("t_perf_to_turbo: %u ms\n", data->t_perf_to_turbo);
> + pr_debug("t_balanced_to_perf: %u ms\n", data->t_balanced_to_perf);
> + pr_debug("t_quiet_to_balanced: %u ms\n", data->t_quiet_to_balanced);
> + pr_debug("t_balanced_to_quiet: %u ms\n", data->t_balanced_to_quiet);
> + pr_debug("t_perf_to_balanced: %u ms\n", data->t_perf_to_balanced);
> + pr_debug("t_turbo_to_perf: %u ms\n", data->t_turbo_to_perf);
> +
> + for (i = 0 ; i < CNQF_MODE_MAX ; i++) {
> + pr_debug("pfloor_%s: %u mW\n", state_as_str_cnqf(i), data->ps[i].pfloor);
> + pr_debug("fppt_%s: %u mW\n", state_as_str_cnqf(i), data->ps[i].fppt);
> + pr_debug("sppt_%s: %u mW\n", state_as_str_cnqf(i), data->ps[i].sppt);
> + pr_debug("sppt_apuonly_%s: %u mW\n",
> + state_as_str_cnqf(i), data->ps[i].sppt_apu_only);
> + pr_debug("spl_%s: %u mW\n", state_as_str_cnqf(i), data->ps[i].spl);
> + pr_debug("stt_minlimit_%s: %u mW\n",
> + state_as_str_cnqf(i), data->ps[i].stt_min_limit);
> + pr_debug("stt_skintemp_apu_%s: %u C\n", state_as_str_cnqf(i),
> + data->ps[i].stt_skintemp[STT_TEMP_APU]);
> + pr_debug("stt_skintemp_hs2_%s: %u C\n", state_as_str_cnqf(i),
> + data->ps[i].stt_skintemp[STT_TEMP_HS2]);
> + pr_debug("fan_id_%s: %u\n", state_as_str_cnqf(i), data->ps[i].fan_id);
> + }
> +
> + pr_debug("Dynamic Slider %s Defaults - END\n", idx ? "DC" : "AC");
> +}
> +#else
> +static void amd_pmf_cnqf_dump_defaults(struct apmf_dyn_slider_output *data, int idx) {}
> +#endif
> +
> static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
> struct cnqf_config *table)
> {
> @@ -284,6 +339,7 @@ static int amd_pmf_load_defaults_cnqf(struct amd_pmf_dev *dev)
> return ret;
> }
>
> + amd_pmf_cnqf_dump_defaults(&out, i);
> amd_pmf_update_mode_set(i, &out);
> amd_pmf_update_trans_data(i, &out);
> amd_pmf_update_power_threshold(i);
> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
> index bed762d47a14..0a4d0549ea03 100644
> --- a/drivers/platform/x86/amd/pmf/sps.c
> +++ b/drivers/platform/x86/amd/pmf/sps.c
> @@ -12,6 +12,60 @@
>
> static struct amd_pmf_static_slider_granular config_store;
>
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +const char *slider_as_str(unsigned int state)
> +{
> + switch (state) {
> + case POWER_MODE_PERFORMANCE:
> + return "PERFORMANCE";
> + case POWER_MODE_BALANCED_POWER:
> + return "BALANCED_POWER";
> + case POWER_MODE_POWER_SAVER:
> + return "POWER_SAVER";
> + default:
> + return "Unknown Slider State";
> + }
> +}
> +
> +const char *source_as_str(unsigned int state)
> +{
> + switch (state) {
> + case POWER_SOURCE_AC:
> + return "AC";
> + case POWER_SOURCE_DC:
> + return "DC";
> + default:
> + return "Unknown Power State";
> + }
> +}
> +
> +static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data)
> +{
> + int i, j;
> +
> + pr_debug("Static Slider Data - BEGIN\n");
> +
> + for (i = 0; i < POWER_SOURCE_MAX; i++) {
> + for (j = 0; j < POWER_MODE_MAX; j++) {
> + pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
> + pr_debug("SPL: %u mW\n", data->prop[i][j].spl);
> + pr_debug("SPPT: %u mW\n", data->prop[i][j].sppt);
> + pr_debug("SPPT_ApuOnly: %u mW\n", data->prop[i][j].sppt_apu_only);
> + pr_debug("FPPT: %u mW\n", data->prop[i][j].fppt);
> + pr_debug("STTMinLimit: %u mW\n", data->prop[i][j].stt_min);
> + pr_debug("STT_SkinTempLimit_APU: %u C\n",
> + data->prop[i][j].stt_skin_temp[STT_TEMP_APU]);
> + pr_debug("STT_SkinTempLimit_HS2: %u C\n",
> + data->prop[i][j].stt_skin_temp[STT_TEMP_HS2]);
> + }
> + }
> +
> + pr_debug("Static Slider Data - END\n");
> +}
> +#else
> +static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data) {}
> +#endif
> +
> static void amd_pmf_load_defaults_sps(struct amd_pmf_dev *dev)
> {
> struct apmf_static_slider_granular_output output;
> @@ -36,6 +90,7 @@ static void amd_pmf_load_defaults_sps(struct amd_pmf_dev *dev)
> idx++;
> }
> }
> + amd_pmf_dump_sps_defaults(&config_store);
> }
>
> void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
next prev parent reply other threads:[~2023-05-10 15:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-10 14:47 [PATCH v4 1/2] platform/x86/amd/pmf: Add PMF acpi debug support Shyam Sundar S K
2023-05-10 14:47 ` [PATCH v4 2/2] platform/x86/amd/pmf: Add PMF debug facilities Shyam Sundar S K
2023-05-10 15:24 ` Mario Limonciello
2023-05-10 15:23 ` Mario Limonciello [this message]
2023-05-15 13:18 ` [PATCH v4 1/2] platform/x86/amd/pmf: Add PMF acpi debug support Hans de Goede
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=be84e061-5660-3fc9-7fcf-ff8b2cefaa1d@amd.com \
--to=mario.limonciello@amd.com \
--cc=Patil.Reddy@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=hdegoede@redhat.com \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox