On Wed, Oct 13, 2021 at 1:06 PM Lazar, Lijo <Lijo.Lazar@xxxxxxx> wrote: > > [AMD Official Use Only] > > > >Or maybe just a list without default hint, i.e. no asterisk? > > I think this is also fine meaning we are having trouble in determining the current frequency or DPM level. Evan/Alex? Don't know if this will crash the tools. > That seems reasonable to me. Alex > Thanks, > Lijo > ________________________________ > From: Tuikov, Luben <Luben.Tuikov@xxxxxxx> > Sent: Wednesday, October 13, 2021 9:52:09 PM > To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Subject: Re: [PATCH 0/5] 0 MHz is not a valid current frequency > > On 2021-10-13 00:14, Lazar, Lijo wrote: > > > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: > >> Some ASIC support low-power functionality for the whole ASIC or just > >> an IP block. When in such low-power mode, some sysfs interfaces would > >> report a frequency of 0, e.g., > >> > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk > >> 0: 500Mhz > >> 1: 0Mhz * > >> 2: 2200Mhz > >> $_ > >> > >> An operating frequency of 0 MHz doesn't make sense, and this interface > >> is designed to report only operating clock frequencies, i.e. non-zero, > >> and possibly the current one. > >> > >> When in this low-power state, round to the smallest > >> operating frequency, for this interface, as follows, > >> > > Would rather avoid this - > > > > 1) It is manipulating FW reported value. If at all there is an uncaught > > issue in FW reporting of frequency values, that is masked here. > > 2) Otherwise, if 0MHz is described as GFX power gated case, this > > provides a convenient interface to check if GFX is power gated. > > > > If seeing a '0' is not pleasing, consider changing to something like > > "NA" - not available (frequency cannot be fetched at the moment). > > There's a ROCm tool which literally asserts if the values are not ordered in increasing order. Now since 0 < 550, but 0 is listed as the second entry, the tool simply asserts and crashes. > > It is not clear what you'd rather see here: > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > 0: 550Mhz > 1: N/A * > 2: 2200MHz > $_ > > Is this what you want to see? (That'll crash other tools which expect %uMhz.) > > Or maybe just a list without default hint, i.e. no asterisk? > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > 0: 550Mhz > 1: 2200MHz > $_ > > What should the output be? > > We want to avoid showing 0, but still show numbers. > > Regards, > Luben > > > > > Thanks, > > Lijo > > > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk > >> 0: 500Mhz * > >> 1: 2200Mhz > >> $_ > >> > >> Luben Tuikov (5): > >> drm/amd/pm: Slight function rename > >> drm/amd/pm: Rename cur_value to curr_value > >> drm/amd/pm: Rename freq_values --> freq_value > >> dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency > >> dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency > >> > >> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +++++++++------ > >> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 ++++++++++++------- > >> 2 files changed, 86 insertions(+), 47 deletions(-) > >> >