If it helps, it would only break the SMI as it determines the Max Level by using the last line in the clock file as the max. So I made a patch to change that so that it uses the last line in the file that starts with a number (since the pstate starts with P:). But if you want to use a different interface, thatâ??s alright, I am just letting you know that the SMI would break solely due to me making assumptions about the structure of the sclk/mclk file, and that a fix would be trivial. Kent -----Original Message----- From: Deucher, Alexander Sent: Monday, January 08, 2018 4:31 PM To: Russell, Kent; Alex Deucher; Kuehling, Felix Cc: amd-gfx list Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels No, it doesn't do anything today and wouldn't do anything with this patch either. With this patch, it just prints the stable pstate clock level in the output if you read the files so you know what the clocks are when you enable the profiling modes. That's why I think we should expose the stable pstate clocks via a different interface to avoid confusion and not break anything (we can't break existing smi tools out in the wild. To actually enable the various profiling modes, you need to echo one of the profile_* options to force_performance_level. When you select a profile_* option, that disables clock and powergating and sets the stable pstate clocks. Alex -----Original Message----- From: amd-gfx [mailto:amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Russell, Kent Sent: Monday, January 8, 2018 2:52 PM To: Alex Deucher <alexdeucher at gmail.com>; Kuehling, Felix <Felix.Kuehling at amd.com> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org> Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels And yes, it will confuse the SMI in a couple functions, but thankfully it's nice enough that I can fix it with a couple lines. So just to make sure that I understand this correctly, if a user were to echo P to the pp_dpm_mclk file, it would disable clockgating and powergating and keep the memory clocks at a stable level (likely level 0)? And this would have no impact on the SCLK or Power Profile or anything else, just the mclk? Just asking so I know how to implement it in the SMI. Thanks! Kent -----Original Message----- From: Alex Deucher [mailto:alexdeucher@xxxxxxxxx] Sent: Monday, January 08, 2018 2:35 PM To: Kuehling, Felix Cc: amd-gfx list; Russell, Kent Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling <felix.kuehling at amd.com> wrote: > [+Kent] > > What does stable pstate mean? What is it used for? This is used for the profiling stuff in force_performance_level. It disables clock and power gating and sets stable clock levels for doing performance profiling. Alex > > Hi Kent, > > Is this going to confuse rocm_smi? > > Regards, > Felix > > > On 2018-01-08 04:57 AM, Rex Zhu wrote: >> The additional output are at the end of sclk/mclk info as cat >> pp_dpm_mclk >> 0: 300Mhz * >> 1: 1650Mhz >> P: 300Mhz >> >> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com> >> >> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8 >> --- >> drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++ >> drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++ >> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 2 ++ >> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++ >> 4 files changed, 8 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c >> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c >> index f68dd08..03dfba0 100644 >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c >> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> i, sclk_table->entries[i].clk / 100, >> (i == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_sclk/100); >> break; >> case PP_MCLK: >> now = >> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, >> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> CZ_NUM_NBPMEMORYCLOCK-i, data->sys_info.nbp_memory_clock[i-1] / 100, >> (CZ_NUM_NBPMEMORYCLOCK-i == >> now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_mclk/100); >> break; >> default: >> break; >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c >> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c >> index 409a56b..88c6ad8 100644 >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c >> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr, >> data->gfx_max_freq_limit / 100, >> ((data->gfx_max_freq_limit / 100) >> == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_sclk/100); >> break; >> case PP_MCLK: >> PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, >> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr, >> mclk_table->entries[i].clk / 100, >> ((mclk_table->entries[i].clk / 100) >> == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_mclk/100); >> break; >> default: >> break; >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c >> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c >> index 72031bd..1bdcd86 100644 >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c >> @@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> i, sclk_table->dpm_levels[i].value / 100, >> (i == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_sclk/100); >> break; >> case PP_MCLK: >> smum_send_msg_to_smc(hwmgr, >> PPSMC_MSG_API_GetMclkFrequency); @@ -4388,6 +4389,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> i, mclk_table->dpm_levels[i].value / 100, >> (i == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_mclk/100); >> break; >> case PP_PCIE: >> pcie_speed = smu7_get_current_pcie_speed(hwmgr); >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> index cb35f4f..cab50fc 100644 >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> @@ -4565,6 +4565,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> i, sclk_table->dpm_levels[i].value / 100, >> (i == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_sclk/100); >> break; >> case PP_MCLK: >> if (data->registry_data.mclk_dpm_key_disabled) >> @@ -4583,6 +4584,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr *hwmgr, >> size += sprintf(buf + size, "%d: %uMhz %s\n", >> i, mclk_table->dpm_levels[i].value / 100, >> (i == now) ? "*" : ""); >> + size += sprintf(buf + size, "P: %uMhz\n", >> + hwmgr->pstate_mclk/100); >> break; >> case PP_PCIE: >> PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx at lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
