AMD General I just think either way is ok. It is a user convention definition. Thanks.
-----Original Message----- From: Wang, Kevin <[email protected]> Sent: Monday, August 31, 2026 8:48 PM To: Lazar, Lijo <[email protected]>; [email protected]; Deucher, Alexander <[email protected]>; Feng, Kenneth <[email protected]> Cc: Zhang, Hawking <[email protected]> Subject: RE: [PATCH 0/7] drm/amd/pm: stage OD reset until commit 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 > >>>> > >>> > >
