Re: [PATCH 2/3] drm/amdgpu: Save some cycles on the job submission path

["Timur Kristóf" <timur.kristof@gmail.com>]

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 <timur.kristof@gmail.com> 
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 <timur.kristof@gmail.com>
> > 
> > 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.

Thanks & best regards,
Timur


> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: Christian König <christian.koenig@amd.com>
> > > Cc: Timur Kristóf <timur.kristof@gmail.com>
> > > ---
> > > 
> > > 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);