[PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
Lazar, Lijo
lijo.lazar at amd.com
Tue Oct 19 04:38:11 UTC 2021
On 10/19/2021 9:45 AM, Luben Tuikov wrote:
> On 2021-10-18 23:38, Lazar, Lijo wrote:
>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:
>>> A current value of a clock frequency of 0, means
>>> that the IP block is in some kind of low power
>>> state. Ignore it and don't report it here. Here we
>>> only report the possible operating (non-zero)
>>> frequencies of the block requested. So, if the
>>> current clock value is 0, then print the DPM
>>> frequencies, but don't report a current value.
>>>
>>> v2: Don't report the minimum one as the current
>>> one when reported one is 0, i.e. don't add an
>>> asterisk (Lijo). LT: It is conceivable that this
>>> may confuse user-mode tools if they scan and look
>>> for a current one, i.e. look for an asterisk, but
>>> they'll have to adapt and use other methods for
>>> finding power states of the chip--we can't report
>>> 0 as current.
>>> v3: Start the subject title with a verb. (PaulM)
>>>
>>> Cc: Alex Deucher<Alexander.Deucher at amd.com>
>>> Cc: Lijo Lazar<Lijo.Lazar at amd.com>
>>> Cc: Paul Menzel<pmenzel at molgen.mpg.de>
>>> Signed-off-by: Luben Tuikov<luben.tuikov at amd.com>
>>> ---
>>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 60 ++++++++++++-------
>>> 1 file changed, 40 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> index f630d5e928ccfe..6fe792be77dbbb 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> @@ -1040,7 +1040,8 @@ static void sienna_cichlid_get_od_setting_range(struct smu_11_0_7_overdrive_tabl
>>> }
>>>
>>> static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> - enum smu_clk_type clk_type, char *buf)
>>> + enum smu_clk_type clk_type,
>>> + char *buf)
>>> {
>>> struct amdgpu_device *adev = smu->adev;
>>> struct smu_table_context *table_context = &smu->smu_table;
>>> @@ -1052,12 +1053,12 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> OverDriveTable_t *od_table =
>>> (OverDriveTable_t *)table_context->overdrive_table;
>>> int i, size = 0, ret = 0;
>>> - uint32_t curr_value = 0, value = 0, count = 0;
>>> + uint32_t curr_value, value, count;
>>> uint32_t freq_value[3] = {0, 0, 0};
>>> - uint32_t mark_index = 0;
>>> uint32_t gen_speed, lane_width;
>>> uint32_t min_value, max_value;
>>> uint32_t smu_version;
>>> + bool fine_grained;
>>>
>>> smu_cmn_get_sysfs_buf(&buf, &size);
>>>
>>> @@ -1077,6 +1078,20 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> + ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
>>> + &freq_value[0]);
>>> + if (ret)
>>> + goto print_clk_out;
>>> +
>>> + /* A current value of a clock frequency of 0, means
>>> + * that the IP block is in some kind of low power
>>> + * state. Ignore it and don't report it here. Here we
>>> + * only report the possible operating (non-zero)
>>> + * frequencies of the block requested. So, if the
>>> + * current clock value is 0, then we don't report a
>>> + * "current" value from the DPM states, i.e. we don't
>>> + * add an asterisk.
>>> + */
>>>
>>> /* no need to disable gfxoff when retrieving the current gfxclk */
>>> if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
>>> @@ -1086,38 +1101,43 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> - if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
>>> - for (i = 0; i < count; i++) {
>>> + fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu, clk_type);
>>> + if (!fine_grained) {
>>> + /* We already got the 0-th index--print it
>>> + * here and continue thereafter.
>>> + */
>>> + size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
>>> + curr_value == freq_value[0] ? "*" : "");
>>> + for (i = 1; i < count; i++) {
>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
>>> if (ret)
>>> goto print_clk_out;
>>> -
>>> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
>>> curr_value == value ? "*" : "");
>>> }
>>> } else {
>>> - ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
>>> - if (ret)
>>> - goto print_clk_out;
>>> + freq_value[1] = curr_value ?: freq_value[0];
>> Omitting second expression is not standard C -
>> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
>
> Lijo just clarified to me that:
>
>> well, i had to look up as I haven't seen it before
>
> I hope the following should make it clear about its usage:
>
> $cd linux/
> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
> 1042
> $_
>
Thanks Luben!
Thanks,
Lijo
> Regards,
> Luben
>
>> Thanks,
>> Lijo
>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> - freq_value[1] = curr_value;
>>> - mark_index = curr_value == freq_value[0] ? 0 :
>>> - curr_value == freq_value[2] ? 2 : 1;
>>> -
>>> - count = 3;
>>> - if (mark_index != 1) {
>>> + if (freq_value[1] == freq_value[0]) {
>>> + i = 1;
>>> + count = 3;
>>> + } else if (freq_value[1] == freq_value[2]) {
>>> + i = 0;
>>> count = 2;
>>> - freq_value[1] = freq_value[2];
>>> + } else {
>>> + i = 0;
>>> + count = 3;
>>> }
>>>
>>> - for (i = 0; i < count; i++) {
>>> - size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
>>> - curr_value == freq_value[i] ? "*" : "");
>>> + for ( ; i < count; i++) {
>>> + size += sysfs_emit_at(buf, size,
>>> + "%d: %uMhz %s\n",
>>> + i, freq_value[i],
>>> + curr_value == freq_value[i] ? "*" : "");
>>> }
>>> -
>>> }
>>> break;
>>> case SMU_PCIE:
>>>
>
More information about the amd-gfx
mailing list