Re: [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized

On 2026. július 13., hétfő 16:39:08 közép-európai nyári idő Tvrtko Ursulin 
wrote:
> On 13/07/2026 12:16, Timur Kristóf wrote:
> > On Monday, July 13, 2026 12:25:38 PM Central European Summer Time Tvrtko
> > 
> > Ursulin wrote:
> >> On 12/07/2026 18:39, Timur Kristóf wrote:
> >>> When DPM is turned off with the amdgpu.dpm=0 module parameter,
> >>> the thermal work queue isn't initialized so we shouldn't
> >>> schedule any work on it.
> >>> 
> >>> Signed-off-by: Timur Kristóf <[email protected]>
> >>> ---
> >>> 
> >>>    drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >>> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c index
> >>> 832953941266..6a54566d1a68 100644
> >>> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >>> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >>> @@ -7692,7 +7692,7 @@ static int si_dpm_process_interrupt(struct
> >>> amdgpu_device *adev,>
> >>> 
> >>>    		break;
> >>>    	
> >>>    	}
> >>> 
> >>> -	if (queue_thermal)
> >>> +	if (queue_thermal && amdgpu_dpm)
> >>> 
> >>>    		schedule_work(&adev->pm.dpm.thermal.work);
> >>>    	
> >>>    	return 0;
> >> 
> >> I don't know this code but what is suspicious to me is that there appear
> >> to be other unguarded entry points to the un-initialized work. Like all
> >> the flush_work() calls and one cancel_work_sync() as well.
> >> 
> >> Presumably as long as si_dpm_sw_init() is returning success when
> >> amdgpu_dpm == 0 those two can get called?
> >> 
> >> Finding the right Fixes: target might be a good thing too.
> >> 
> >> Regards,
> >> 
> >> Tvrtko
> > 
> > Hi Tvrtko,
> > 
> > That's a nice find. Indeed si_dpm_sw_init doesn't initialize the
> > thermal.work when amdgpu_dpm is zero. And in fact the same issue seems to
> > be present also in kv_dpm. I can address both of these in a follow-up
> > series if you like.
> You mean merge this fix for si_dpm_process_interrupt but leave
> si_dpm_sw_fini and si_dpm_suspend for later? I am not sure if that makes
> sense TBH but I guess I don't have the full context. For example whether
> you have hit this bug or just spotted by code inspection? If you hit it,
> can't fini and suspend also be hit and if so why does it make sense not
> to immediately fix all three?

Hi,

Let's just drop this patch then. The actual important ones are the next two.

Thanks,
Timur