AMD General

> -----Original Message-----
> From: Lazar, Lijo <[email protected]>
> Sent: Monday, August 31, 2026 8:22 PM
> To: Wang, Kevin <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking
> <[email protected]>; Feng, Kenneth <[email protected]>
> Subject: Re: [PATCH 0/7] drm/amd/pm: stage OD reset until commit
>
>
>
> On 31-Aug-26 5:36 PM, Wang, Kevin wrote:
> > AMD General
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <[email protected]>
> >> Sent: Monday, August 31, 2026 7:06 PM
> >> To: Wang, Kevin <[email protected]>; [email protected]
> >> Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking
> >> <[email protected]>; Feng, Kenneth <[email protected]>
> >> Subject: Re: [PATCH 0/7] drm/amd/pm: stage OD reset until commit
> >>
> >>
> >>
> >> On 31-Aug-26 4:25 PM, Lazar, Lijo wrote:
> >>>
> >>>
> >>> On 31-Aug-26 10:05 AM, Kevin Wang wrote:
> >>>> [Some people who received this message don't often get email from
> >>>> [email protected]. Learn why this is important at https://aka.ms/
> >>>> LearnAboutSenderIdentification ]
> >>>>
> >>>> The pp_od_clk_voltage interface exposes a staged OverDrive workflow:
> >>>> users edit clock, voltage, and power settings, then write "c" to
> >>>> commit them to the SMU.
> >>>>
> >>>> However, PP_OD_RESTORE_DEFAULT_TABLE does not follow this
> workflow
> >> on
> >>>> every SMU version. Some backends restore their cached settings and
> >>>> wait for "c", while others upload the reset table or send
> >>>> frequency-limit commands directly from "r". As a result, the same
> >>>> userspace sequence has different hardware effects across ASICs.
> >>>>
> >>>> For example:
> >>>>
> >>>> - SMU 14.0.0 and SMU 13.0.5 stage reset limits until "c".
> >>>> - SMU 14.0.2 and SMU 13.0.6 apply reset values immediately.
> >>>> - Navi10 stages the boot OD table, while Vega20 reads the current SMU
> >>>>     table instead of restoring the saved defaults.
> >>>>
> >>>> This series makes "r" restore default values only in driver-side
> >>>> staging state. "c" remains the sole operation that uploads an OD
> >>>> table or sends frequency-limit commands to PMFW.
> >>>>
> >>>> This gives pp_od_clk_voltage one consistent transaction model:
> >>>>
> >>>>     edit/reset -> staged driver state -> commit
> >>>
> >>> Reset should be reset to defaults and shouldn't require extra commit.
> >>> This breaks existing userspace for SMU 13.0.6.
> >>>
> >>
> >> I see that this breaks existing userspace for almost all of it. SMU
> >> 13.0.2 also resets to the default clocks immediately, while others
> >> use a fallthrough logic to commit the changes immediately.applied.
> >>
> >> This behavior needs to be kept as it is.
> >
> > This behavior change is intentional, and these patch‑series introduces a
> unified transactional model for pp_od_clk_voltage:
> > - `r` restores defaults within driver cache.
> > - only `c` commits settings to PMFW.
> > The existing immediate‑reset paths are ASIC‑specific inconsistencies.
> > SMU 13.0.2 pushes default clock limits directly, whereas other backends
> achieve equivalent results via the commit path.
> > This series unifies both under PowerPlay’s staged‑reset model.
> > Btw, user space desiring immediate reset shall issue `"r"` followed by 
> > `"c"`.
> >
>
> This is what breaks existing userspace.

As stated earlier, this series fixes inconsistent driver behavior. Note that 
divergent semantics already exist across ASICs today;
without this fix, merely a different subset of ASICs would be affected. The 
series brings them under a unified transactional model.

Hi @Deucher, Alexander @Feng, Kenneth,
For driver‑behavior‑change concerns: I’m unsure which fix direction is better. 
Still, converging all hardware updates into the 'c' commit aids driver state 
maintenance.

Best Regards,
Kevin

>
> Only SMU 14.0.0/13.0.5/Navi10 are not resetting to default clocks with 'r'
> operation. The documentation also gives the impression that 'r'
> doesn't require a commit.
>
> "If you want to reset to the default power levels, write “r” (reset) to the 
> file to
> reset them"
>
> The patch should be to correct the non-conforming ones rather than
> enforcing a 'c' operation to reset.
>
> Thanks,
> Lijo
> > Best Regards,
> > Kevin
> >
> >>
> >> Thanks,
> >> Lijo
> >>
> >>
> >>> Thanks,
> >>> Lijo
> >>>
> >>>>
> >>
> >>>> It also permits userspace to reset a staged configuration, adjust
> >>>> one or more settings, and submit the final configuration with one
> >>>> commit,
> >> without temporarily applying an intermediate default configuration.
> >>>>
> >>>> Link: https://gitlab.freedesktop.org/drm/amd/-/work_items/5690
> >>>>
> >>>> Kevin Wang (7):
> >>>>     drm/amd/pm: stage od reset for smu 11.0.7
> >>>>     drm/amd/pm: stage od reset for smu 13.0.2
> >>>>     drm/amd/pm: stage od reset for smu 13.0.0/13.0.7
> >>>>     drm/amd/pm: stage od reset for smu 13.0.6
> >>>>     drm/amd/pm: stage od reset for smu 14.0.2
> >>>>     drm/amd/pm: stage od reset for smu 15.0.8
> >>>>     drm/amd/pm: stage od reset for smu vega20
> >>>>
> >>>>    .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 75
> >> ++++++++++++++--
> >>>>    .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  2 +-
> >>>>    .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 15 +---
> >>>>    .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c  |  2 +-
> >>>>    .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 85
> >>>> ++++++++++---------
> >>>>    .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c  |  2 +-
> >>>>    .../drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c  |  2 +-
> >>>>    .../drm/amd/pm/swsmu/smu15/smu_v15_0_8_ppt.c  | 50 ++++++-----
> >>>>    8 files changed, 145 insertions(+), 88 deletions(-)
> >>>>
> >>>> --
> >>>> 2.55.0
> >>>>
> >>>
> >

Reply via email to