On Mon, Jun 29, 2026 at 6:31 PM Timur Kristóf <[email protected]> wrote: > > On Monday, June 29, 2026 5:28:14 PM Central European Summer Time Alex Deucher > wrote: > > On Fri, Jun 26, 2026 at 1:10 PM Timur Kristóf <[email protected]> > wrote: > > > On 2026. június 26., péntek 10:55:57 közép-európai nyári idő Tvrtko > > > Ursulin > > > > > > wrote: > > > > Every job submission on the Steam Deck ends up walking the list of IP > > > > blocks looking for AMD_IP_BLOCK_TYPE_SMC. Half of the call chain is like > > > > > > > > the below, while the second half is from > > > > amdgpu_gfx_profile_ring_end_use: > > > > amdgpu_gfx_profile_ring_begin_use > > > > > > > > amdgpu_dpm_is_overdrive_enabled > > > > > > > > is_support_sw_smu > > > > > > > > amdgpu_device_ip_is_valid > > > > > > > > On a game menu screen at 90Hz refresh rate we end up with ~840 calls per > > > > > > > > second which sticks out when the submission worker is profiled with > > > > perf: > > > > 13.78% [kernel] [k] __lock_text_start > > > > 10.86% [kernel] [k] __lookup_object > > > > > > > > 8.76% [kernel] [k] __mod_timer > > > > 4.94% [kernel] [k] queued_spin_lock_slowpath > > > > 1.66% [kernel] [k] amdgpu_device_ip_is_valid > > > > 1.54% [kernel] [k] preempt_count_add > > > > 1.42% [kernel] [k] amdgpu_sync_peek_fence > > > > 1.18% [kernel] [k] amdgpu_vmid_grab > > > > 1.17% [kernel] [k] amdgpu_ib_schedule > > > > 1.14% [kernel] [k] kthread_worker_fn > > > > > > > > Lets short-circuit this walk by simply caching the result of > > > > is_support_sw_smu() in the device. > > > > > > > > This is a micro-improvement but it is at least conceptually nicer to > > > > avoid > > > > repeating the same walk so much. > > > > > > Hi, > > > > > > I agree with cleaning up this thing. > > > Reviewed-by: Timur Kristóf <[email protected]> > > > > > > That being said, I think is_support_sw_smu() is horrible and should be > > > removed alltogether, because it goes against how the rest of the power > > > management code works. > > > > > > In my opinion, we should instead: > > > > > > 1. Hook up some function pointers and check those instead, > > > For example in amdgpu_pm_acpi_event_handler() we should just hook up > > > smu_set_ac_dc() to the notify_ac_dc() function pointer. There are plenty > > > of > > > other similar cases. > > > Another example, for amdgpu_dpm_mode1_reset() we should introduce a new > > > asic_reset_mode_1() pointer in amd_pm_funcs() similar to how it works with > > > MODE2 reset for consistency. > > > > > > 2. Eliminate redundant functions where the same thing is already done > > > elsewhere. > > > For example in amdgpu_dpm_is_mode1_reset_supported() it checks > > > smu_mode1_reset_is_support() which is redundant because the supported > > > reset > > > type is available on the ASIC functions already and we can just use that. > > > > > > What do you think? > > > > I agree. This has been a todo for a while. > > > > Thanks Alex. I'd be happy to work on this cleanup. However can you please > apply Tvrtko's patch? I think it's a nice improvement until that cleanup is > complete.
Sure. Applied the series. Thanks! Alex > > Thanks & best regards, > Timur > > > > > > Signed-off-by: Tvrtko Ursulin <[email protected]> > > > > Cc: Alex Deucher <[email protected]> > > > > Cc: Christian König <[email protected]> > > > > Cc: Timur Kristóf <[email protected]> > > > > --- > > > > > > > > v2: > > > > * Approach changed to cache sw_smu status only. > > > > > > > > --- > > > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++ > > > > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 14 +++++--------- > > > > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 8 +++++++- > > > > 4 files changed, 16 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 7b09410d6d8f..9803967d15f9 > > > > 100644 > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > > @@ -851,6 +851,7 @@ struct amdgpu_device { > > > > > > > > struct dev_pm_domain vga_pm_domain; > > > > bool have_disp_power_ref; > > > > bool have_atomics_support; > > > > > > > > + bool is_sw_smu; > > > > > > > > /* BIOS */ > > > > bool is_atom_fw; > > > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index > > > > 1e6b75ecafe4..7f935a5778b0 100644 > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > > @@ -74,6 +74,7 @@ > > > > > > > > #include "amdgpu_ras.h" > > > > #include "amdgpu_ras_mgr.h" > > > > #include "amdgpu_pmu.h" > > > > > > > > +#include "amdgpu_smu.h" > > > > > > > > #include "amdgpu_fru_eeprom.h" > > > > #include "amdgpu_reset.h" > > > > #include "amdgpu_virt.h" > > > > > > > > @@ -2130,6 +2131,8 @@ static int amdgpu_device_ip_early_init(struct > > > > amdgpu_device *adev) adev->cg_flags &= amdgpu_cg_mask; > > > > > > > > adev->pg_flags &= amdgpu_pg_mask; > > > > > > > > + amdgpu_smu_early_init(adev); > > > > + > > > > > > > > return 0; > > > > > > > > } > > > > > > > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > > > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index > > > > 208a2fba6d40..82c9ae6a5092 100644 > > > > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > > > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > > > @@ -591,17 +591,13 @@ static int smu_get_power_num_states(void *handle, > > > > > > > > return 0; > > > > > > > > } > > > > > > > > -bool is_support_sw_smu(struct amdgpu_device *adev) > > > > +void amdgpu_smu_early_init(struct amdgpu_device *adev) > > > > > > > > { > > > > > > > > /* vega20 is 11.0.2, but it's supported via the powerplay code */ > > > > > > > > - if (adev->asic_type == CHIP_VEGA20) > > > > - return false; > > > > - > > > > - if ((amdgpu_ip_version(adev, MP1_HWIP, 0) >= IP_VERSION(11, 0, 0)) > > > > > > && > > > > > > > - amdgpu_device_ip_is_valid(adev, AMD_IP_BLOCK_TYPE_SMC)) > > > > - return true; > > > > - > > > > - return false; > > > > + adev->is_sw_smu = adev->asic_type != CHIP_VEGA20 && > > > > + (amdgpu_ip_version(adev, MP1_HWIP, 0) >= > > > > + IP_VERSION(11, 0, 0) && > > > > + amdgpu_device_ip_is_valid(adev, > > > > > > AMD_IP_BLOCK_TYPE_SMC)); > > > > > > > } > > > > > > > > bool is_support_cclk_dpm(struct amdgpu_device *adev) > > > > > > > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > > > > b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h index > > > > d76e0b005308..efc52d97058b 100644 > > > > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > > > > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > > > > @@ -1952,7 +1952,13 @@ int smu_link_reset(struct smu_context *smu); > > > > > > > > extern const struct amd_ip_funcs smu_ip_funcs; > > > > > > > > -bool is_support_sw_smu(struct amdgpu_device *adev); > > > > +void amdgpu_smu_early_init(struct amdgpu_device *adev); > > > > + > > > > +static inline bool is_support_sw_smu(struct amdgpu_device *adev) > > > > +{ > > > > + return adev->is_sw_smu; > > > > +} > > > > + > > > > > > > > bool is_support_cclk_dpm(struct amdgpu_device *adev); > > > > int smu_write_watermarks_table(struct smu_context *smu); > > > >
