Re: [PATCH 0/5] 0 MHz is not a valid current frequency

[Alex Deucher <alexdeucher@gmail.com>]

On Wed, Oct 13, 2021 at 12:14 AM Lazar, Lijo <lijo.lazar@amd.com> 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).
>

I don't think this interface is really supposed to be the way to get
the current clock, although some use it that way.  It's supposed to
show the DPM states and which are active.  conflating the interface
with other information confuses things in my opinion.  Why is the
current clock less than the minimum clock?  Whether or not an IP is
turned off or in deep sleep or not is independent of DPM states.  When
the IP is active, it will never go below the minimum DPM level.  If we
want to query deep sleep or gfxoff we can use the amdgpu_pm_info
debugfs interface or add some other new interface.

Alex


> 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(-)
> >