[AMD Official Use Only - AMD Internal Distribution Only] Greetings @Yang, Philip
> This looks good. It is easier to review if one patch only fix one ignored > return code issue. Thanks for the useful comments. The next patch(es) will remove the WARN_ONCE and the user "(void)" to suppress the unneeded return code. I will also break up the next update into 3 patches (1 for each ignore ). > -----Original Message----- > From: Yang, Philip <[email protected]> > Sent: Friday, November 14, 2025 1:53 PM > To: Martin, Andrew <[email protected]>; amd- > [email protected] > Subject: Re: [PATCH v3] drm/amdkfd: Ignored various return code > > > On 2025-11-13 13:31, Andrew Martin wrote: > > The return code of a non void function should not be ignored. In cases > > where we do not care, the code needs to suppress it. > > > > Signed-off-by: Andrew Martin <[email protected]> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 13 ++++++++----- > > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++- > > 2 files changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > > index 644f79f3c9af..e4438fca6283 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > > @@ -313,8 +313,10 @@ int amdgpu_amdkfd_post_reset(struct > amdgpu_device *adev) > > void amdgpu_amdkfd_gpu_reset(struct amdgpu_device *adev) > > { > > if (amdgpu_device_should_recover_gpu(adev)) > > - amdgpu_reset_domain_schedule(adev->reset_domain, > > - &adev->kfd.reset_work); > > + WARN_ONCE(!amdgpu_reset_domain_schedule(adev- > >reset_domain, > > + &adev- > >kfd.reset_work), > > + "Failed to queue work! > at %s", > > + __func__); > amdgpu_reset_domain_schedule only return false if reset_work already exist in > the workqueue, it is fine to ignore the return value, don't want WARN_ONCE > for this. > > } > > > > int amdgpu_amdkfd_alloc_gtt_mem(struct amdgpu_device *adev, size_t > > size, @@ -715,9 +717,10 @@ void amdgpu_amdkfd_set_compute_idle(struct > amdgpu_device *adev, bool idle) > > if (gfx_block != NULL) > > gfx_block->version->funcs- > >set_powergating_state((void *)gfx_block, state); > > } > > - amdgpu_dpm_switch_power_profile(adev, > > - PP_SMC_POWER_PROFILE_COMPUTE, > > - !idle); > > + WARN_ONCE(!amdgpu_dpm_switch_power_profile(adev, > PP_SMC_POWER_PROFILE_COMPUTE, !idle), > > + "(%s) failed to disable video power profile mode", > > + __func__); > > amdgpu_dpm_switch_power_profile return false for SRIOV, pp_funcs- > >switch_power_profile may return false for many cases, like if (!hwmgr || > !hwmgr->pm_en), I think we don't need WARN for those cases. > > > + > > } > > > > bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > > index 22925df6a791..025609a4abcf 100644 > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > > @@ -2780,7 +2780,8 @@ static int runtime_enable(struct kfd_process *p, > uint64_t r_debug, > > * saved in MES. > > */ > > if (pdd->dev->kfd->shared_resources.enable_mes) > > - kfd_dbg_set_mes_debug_mode(pdd, > !kfd_dbg_has_cwsr_workaround(pdd->dev)); > > + (void)kfd_dbg_set_mes_debug_mode(pdd, > > + > !kfd_dbg_has_cwsr_workaround(pdd->dev)); > > This looks good. It is easier to review if one patch only fix one ignored > return > code issue. > > Regards, > > Philip > > > } > > > > p->runtime_info.runtime_state = DEBUG_RUNTIME_STATE_ENABLED;
