[PATCH 5/8] drm/amd/pm: Treat zero vblank time as too short in si_dpm (v2)

[Alex Deucher]

On Wed, Aug 27, 2025 at 10:27 AM Timur Kristóf <timur.kristof at gmail.com> wrote:
>
> On Tue, 2025-08-26 at 16:05 -0400, Alex Deucher wrote:
> > On Mon, Aug 25, 2025 at 5:46 PM Timur Kristóf
> > <timur.kristof at gmail.com> wrote:
> > >
> > > Some parts of the code base expect that MCLK switching is turned
> > > off when the vblank time is set to zero.
> > >
> > > According to pp_pm_compute_clocks the non-DC code has issues
> > > with MCLK switching with refresh rates over 120 Hz.
> > >
> > > Fixes: 841686df9f7d ("drm/amdgpu: add SI DPM support (v4)")
> > > Signed-off-by: Timur Kristóf <timur.kristof at gmail.com>
> > > ---
> > >  drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> > > b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> > > index db46fc0817a7..1e2aeea0b552 100644
> > > --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> > > +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> > > @@ -3082,8 +3082,8 @@ static bool si_dpm_vblank_too_short(void
> > > *handle)
> > >  {
> > >         struct amdgpu_device *adev = (struct amdgpu_device
> > > *)handle;
> > >         u32 vblank_time = amdgpu_dpm_get_vblank_time(adev);
> > > -       /* we never hit the non-gddr5 limit so disable it */
> > > -       u32 switch_limit = adev->gmc.vram_type ==
> > > AMDGPU_VRAM_TYPE_GDDR5 ? 450 : 0;
> > > +       /* we never hit the non-gddr5 limit so disable it (but
> > > treat 0 as too short) */
> > > +       u32 switch_limit = adev->gmc.vram_type ==
> > > AMDGPU_VRAM_TYPE_GDDR5 ? 450 : 1;
> >
> > Took me a while to wrap my head around this.  It might be clearer to
> > just return early if the vblank_time is 0.
>
> Sure, I can do that if you think that makes it easier to read.
>
> > That said, if there are no
> > displays attached there is no reason to not enable mclk switching.
> >
>
> The function is only called when there are displays attached.
> Should I mention that in a comment?
>

Sure that sounds good.  Thanks!

Alex

> >
> > >
> > >         if (vblank_time < switch_limit)
> > >                 return true;
> > > --
> > > 2.50.1
> > >