Looks good. Reviewed-by: Alex Deucher <alexander.deucher at amd.com> Please also send a patch to bump the kernel driver version so the UMDs know when the query is available. Thanks! Alex ________________________________ From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> on behalf of Zhu, Rex <Rex.Zhu at amd.com> Sent: Wednesday, January 17, 2018 12:48:48 AM To: 'Alex Deucher' Cc: amd-gfx list Subject: RE: [PATCH 2/3] drm/amd/pp: Add stable Pstate clk display when print_clock_levels Ok, I will drop this patch. and please review the new attached patch expose stable pstate clock by ioctl. Best Regards Rex -----Original Message----- From: Alex Deucher [mailto:alexdeucher@xxxxxxxxx] Sent: Wednesday, January 17, 2018 12:03 AM To: Zhu, Rex Cc: amd-gfx list Subject: Re: [PATCH 2/3] drm/amd/pp: Add stable Pstate clk display when print_clock_levels On Tue, Jan 16, 2018 at 6:59 AM, Rex Zhu <Rex.Zhu at amd.com> 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> I'm not crazy about this patch. I think it conflates things and breaks older versions of the rocm smi tool. I'd prefer to add a new file for the pstate clocks or add a query to the amdgpu INFO ioctl to fetch them. Alex > > 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 11a900b..6f053fa 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > @@ -4301,6 +4301,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); @@ -4317,6 +4318,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 adfbbc1..d646b27b 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > @@ -4571,6 +4571,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) > @@ -4589,6 +4590,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, > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180117/2462ba31/attachment.html>