All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized
@ 2026-07-12 17:39 Timur Kristóf
  2026-07-12 17:39 ` [PATCH 2/3] drm/amd/pm/si: Fix AC/DC switch notification Timur Kristóf
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Timur Kristóf @ 2026-07-12 17:39 UTC (permalink / raw)
  To: amd-gfx, Alexander.Deucher, Natalie Vock, Mario Limonciello,
	Tvrtko Ursulin
  Cc: Timur Kristóf

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] drm/amd/pm/si: Fix AC/DC switch notification
  2026-07-12 17:39 [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Timur Kristóf
@ 2026-07-12 17:39 ` Timur Kristóf
  2026-07-12 17:39 ` [PATCH 3/3] drm/amd/pm/smu7: " Timur Kristóf
  2026-07-13 10:25 ` [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Tvrtko Ursulin
  2 siblings, 0 replies; 6+ messages in thread
From: Timur Kristóf @ 2026-07-12 17:39 UTC (permalink / raw)
  To: amd-gfx, Alexander.Deucher, Natalie Vock, Mario Limonciello,
	Tvrtko Ursulin
  Cc: Timur Kristóf, Jeremy Klarenbeek

There were two mistakes in the previous implementation:

The check for ATOM_PP_PLATFORM_CAP_HARDWAREDC should be
inverted. We recently learned that the kernel should send
PPSMC_MSG_RunningOnAC when the flag is set, and not the
other way around.

The clocks also need to be recomputed, because the code in
the si_apply_state_adjust_rules() function selects different
limits on AC and DC.

Fixes: 2d071f6457af ("drm/amd/pm/si: Notify the SMC when switching to AC")
Tested-by: Jeremy Klarenbeek <jeremy.klarenbeek99@gmail.com>
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

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 6a54566d1a68..a0c28de691f8 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
@@ -3892,13 +3892,16 @@ static void si_notify_hw_of_powersource(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
-	/* Check if the platform already manages the AC/DC switch via dedicated GPIO. */
-	if (adev->pm.dpm.platform_caps & ATOM_PP_PLATFORM_CAP_HARDWAREDC)
-		return;
-
-	/* The SMU automatically notices DC, but needs to be notified when switching to AC. */
-	if (adev->pm.ac_power)
+	/*
+	 * Check if the platform already manages the AC/DC switch via dedicated GPIO.
+	 * Otherwise SMU automatically notices DC, but needs to be notified of AC.
+	 */
+	if (adev->pm.ac_power &&
+	    (adev->pm.dpm.platform_caps & ATOM_PP_PLATFORM_CAP_HARDWAREDC))
 		amdgpu_si_send_msg_to_smc(adev, PPSMC_MSG_RunningOnAC);
+
+	/* Recompute clocks with updated max_limits. */
+	amdgpu_legacy_dpm_compute_clocks(adev);
 }
 
 static PPSMC_Result si_send_msg_to_smc_with_parameter(struct amdgpu_device *adev,
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] drm/amd/pm/smu7: Fix AC/DC switch notification
  2026-07-12 17:39 [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Timur Kristóf
  2026-07-12 17:39 ` [PATCH 2/3] drm/amd/pm/si: Fix AC/DC switch notification Timur Kristóf
@ 2026-07-12 17:39 ` Timur Kristóf
  2026-07-13 10:25 ` [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Tvrtko Ursulin
  2 siblings, 0 replies; 6+ messages in thread
From: Timur Kristóf @ 2026-07-12 17:39 UTC (permalink / raw)
  To: amd-gfx, Alexander.Deucher, Natalie Vock, Mario Limonciello,
	Tvrtko Ursulin
  Cc: Timur Kristóf

There were two mistakes in the previous implementation:

The check for AutomaticDCTransition should be inverted.
We recently learned that the kernel should send
PPSMC_MSG_RunningOnAC when the flag is set, and not the
other way around.

The clocks also need to be recomputed, because the code in
the smu7_apply_state_adjust_rules() function selects
different limits on AC and DC.

Fixes: 96da0d86614e ("drm/amd/pm/smu7: Notify SMU7 of DC->AC switch")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c  | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index be824e3d276b..17ac4c0eb8e2 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -5887,15 +5887,19 @@ static int smu7_power_off_asic(struct pp_hwmgr *hwmgr)
 static void smu7_notify_ac_dc(struct pp_hwmgr *hwmgr)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)(hwmgr->adev);
+	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
 
-	/* Check if the platform already manages the AC/DC switch via dedicated GPIO. */
-	if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
+	/*
+	 * Check if the platform already manages the AC/DC switch via dedicated GPIO.
+	 * Otherwise SMU automatically notices DC, but needs to be notified of AC.
+	 */
+	if (adev->pm.ac_power &&
+	    phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
 			    PHM_PlatformCaps_AutomaticDCTransition))
-		return;
-
-	/* The SMU automatically notices DC, but needs to be notified when switching to AC. */
-	if (adev->pm.ac_power)
 		smum_send_msg_to_smc(hwmgr, PPSMC_MSG_RunningOnAC, NULL);
+
+	/* Recompute clocks with updated max_limits. */
+	pp_funcs->pm_compute_clocks(adev->powerplay.pp_handle);
 }
 
 static const struct pp_hwmgr_func smu7_hwmgr_funcs = {
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized
  2026-07-12 17:39 [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Timur Kristóf
  2026-07-12 17:39 ` [PATCH 2/3] drm/amd/pm/si: Fix AC/DC switch notification Timur Kristóf
  2026-07-12 17:39 ` [PATCH 3/3] drm/amd/pm/smu7: " Timur Kristóf
@ 2026-07-13 10:25 ` Tvrtko Ursulin
  2026-07-13 11:16   ` Timur Kristóf
  2 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2026-07-13 10:25 UTC (permalink / raw)
  To: Timur Kristóf, amd-gfx, Alexander.Deucher, Natalie Vock,
	Mario Limonciello


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized
  2026-07-13 10:25 ` [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Tvrtko Ursulin
@ 2026-07-13 11:16   ` Timur Kristóf
  2026-07-13 14:39     ` Tvrtko Ursulin
  0 siblings, 1 reply; 6+ messages in thread
From: Timur Kristóf @ 2026-07-13 11:16 UTC (permalink / raw)
  To: amd-gfx, Alexander.Deucher, Natalie Vock, Mario Limonciello,
	Tvrtko Ursulin

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

Thanks & best regards,
Timur




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized
  2026-07-13 11:16   ` Timur Kristóf
@ 2026-07-13 14:39     ` Tvrtko Ursulin
  0 siblings, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2026-07-13 14:39 UTC (permalink / raw)
  To: Timur Kristóf, amd-gfx, Alexander.Deucher, Natalie Vock,
	Mario Limonciello



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

Regards,

Tvrtko


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-13 14:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12 17:39 [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Timur Kristóf
2026-07-12 17:39 ` [PATCH 2/3] drm/amd/pm/si: Fix AC/DC switch notification Timur Kristóf
2026-07-12 17:39 ` [PATCH 3/3] drm/amd/pm/smu7: " Timur Kristóf
2026-07-13 10:25 ` [PATCH 1/3] drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized Tvrtko Ursulin
2026-07-13 11:16   ` Timur Kristóf
2026-07-13 14:39     ` Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.