* [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
@ 2025-10-12 19:18 Mario Limonciello (AMD)
2025-10-13 4:54 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello (AMD) @ 2025-10-12 19:18 UTC (permalink / raw)
To: amd-gfx; +Cc: Mario Limonciello, Peyton.Lee, Sultan Alsawaf
From: Mario Limonciello <mario.limonciello@amd.com>
[Why]
Newer VPE microcode has functionality that will decrease DPM level
only when a workload has run for 2 or more seconds. If VPE is turned
off before this DPM decrease, the SOC can get stuck with a higher
DPM level.
This can happen from amdgpu's ring buffer test because it's a short
quick workload for VPE and VPE is turned off after 1s.
[How]
In idle handler besides checking fences are drained check that VPE DPM
level is really is at DPM0. If not, schedule delayed work again until
it is.
Cc: Peyton.Lee@amd.com
Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3:
* Use label to avoid a register read if fences active
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
index 474bfe36c0c2f..e8e512de5992a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
@@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct work_struct *work)
{
struct amdgpu_device *adev =
container_of(work, struct amdgpu_device, vpe.idle_work.work);
+ struct amdgpu_vpe *vpe = &adev->vpe;
unsigned int fences = 0;
+ uint32_t dpm_level;
fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
+ if (fences)
+ goto reschedule;
- if (fences == 0)
+ dpm_level = adev->pm.dpm_enabled ?
+ RREG32(vpe_get_reg_offset(vpe, 0, vpe->regs.dpm_request_lv)) : 0;
+ if (!dpm_level) {
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
- else
- schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
+ return;
+ }
+
+reschedule:
+ schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
}
static int vpe_common_init(struct amdgpu_vpe *vpe)
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
2025-10-12 19:18 [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler Mario Limonciello (AMD)
@ 2025-10-13 4:54 ` Lazar, Lijo
2025-10-13 13:50 ` Mario Limonciello
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2025-10-13 4:54 UTC (permalink / raw)
To: Mario Limonciello (AMD), amd-gfx@lists.freedesktop.org
Cc: Limonciello, Mario, Lee, Peyton, Sultan Alsawaf
[Public]
Doesn't this translate to just a higher idle timeout (VPE_IDLE_TIMEOUT ) for the particular VPE version?
Thanks,
Lijo
>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mario
>Limonciello (AMD)
>Sent: Monday, October 13, 2025 12:48 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
><Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>handler
>
>From: Mario Limonciello <mario.limonciello@amd.com>
>
>[Why]
>Newer VPE microcode has functionality that will decrease DPM level only when
>a workload has run for 2 or more seconds. If VPE is turned off before this DPM
>decrease, the SOC can get stuck with a higher DPM level.
>
>This can happen from amdgpu's ring buffer test because it's a short quick
>workload for VPE and VPE is turned off after 1s.
>
>[How]
>In idle handler besides checking fences are drained check that VPE DPM level is
>really is at DPM0. If not, schedule delayed work again until it is.
>
>Cc: Peyton.Lee@amd.com
>Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
>Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
>Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>---
>v3:
> * Use label to avoid a register read if fences active
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>index 474bfe36c0c2f..e8e512de5992a 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>@@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct
>work_struct *work) {
> struct amdgpu_device *adev =
> container_of(work, struct amdgpu_device,
>vpe.idle_work.work);
>+ struct amdgpu_vpe *vpe = &adev->vpe;
> unsigned int fences = 0;
>+ uint32_t dpm_level;
>
> fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
>+ if (fences)
>+ goto reschedule;
>
>- if (fences == 0)
>+ dpm_level = adev->pm.dpm_enabled ?
>+ RREG32(vpe_get_reg_offset(vpe, 0, vpe-
>>regs.dpm_request_lv)) : 0;
>+ if (!dpm_level) {
> amdgpu_device_ip_set_powergating_state(adev,
>AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
>- else
>- schedule_delayed_work(&adev->vpe.idle_work,
>VPE_IDLE_TIMEOUT);
>+ return;
>+ }
>+
>+reschedule:
>+ schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
> }
>
> static int vpe_common_init(struct amdgpu_vpe *vpe)
>--
>2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
2025-10-13 4:54 ` Lazar, Lijo
@ 2025-10-13 13:50 ` Mario Limonciello
2025-10-13 13:59 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello @ 2025-10-13 13:50 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
Cc: Limonciello, Mario, Lee, Peyton, Sultan Alsawaf
On 10/12/25 11:54 PM, Lazar, Lijo wrote:
> [Public]
>
> Doesn't this translate to just a higher idle timeout (VPE_IDLE_TIMEOUT ) for the particular VPE version?
>
> Thanks,
> Lijo
Yes if the VPE microcode adjusts DPM at runtime this makes sure that it
has settled when workload is complete.
I expect that a higher VPE_IDLE_TIMEOUT would work too, but it seems
less scalable to me.
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mario
>> Limonciello (AMD)
>> Sent: Monday, October 13, 2025 12:48 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>> Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>> handler
>>
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> [Why]
>> Newer VPE microcode has functionality that will decrease DPM level only when
>> a workload has run for 2 or more seconds. If VPE is turned off before this DPM
>> decrease, the SOC can get stuck with a higher DPM level.
>>
>> This can happen from amdgpu's ring buffer test because it's a short quick
>> workload for VPE and VPE is turned off after 1s.
>>
>> [How]
>> In idle handler besides checking fences are drained check that VPE DPM level is
>> really is at DPM0. If not, schedule delayed work again until it is.
>>
>> Cc: Peyton.Lee@amd.com
>> Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v3:
>> * Use label to avoid a register read if fences active
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>> index 474bfe36c0c2f..e8e512de5992a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>> @@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct
>> work_struct *work) {
>> struct amdgpu_device *adev =
>> container_of(work, struct amdgpu_device,
>> vpe.idle_work.work);
>> + struct amdgpu_vpe *vpe = &adev->vpe;
>> unsigned int fences = 0;
>> + uint32_t dpm_level;
>>
>> fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
>> + if (fences)
>> + goto reschedule;
>>
>> - if (fences == 0)
>> + dpm_level = adev->pm.dpm_enabled ?
>> + RREG32(vpe_get_reg_offset(vpe, 0, vpe-
>>> regs.dpm_request_lv)) : 0;
>> + if (!dpm_level) {
>> amdgpu_device_ip_set_powergating_state(adev,
>> AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
>> - else
>> - schedule_delayed_work(&adev->vpe.idle_work,
>> VPE_IDLE_TIMEOUT);
>> + return;
>> + }
>> +
>> +reschedule:
>> + schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
>> }
>>
>> static int vpe_common_init(struct amdgpu_vpe *vpe)
>> --
>> 2.43.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
2025-10-13 13:50 ` Mario Limonciello
@ 2025-10-13 13:59 ` Lazar, Lijo
2025-10-13 14:03 ` Mario Limonciello
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2025-10-13 13:59 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx@lists.freedesktop.org
Cc: Limonciello, Mario, Lee, Peyton, Sultan Alsawaf
[AMD Official Use Only - AMD Internal Distribution Only]
>-----Original Message-----
>From: Mario Limonciello <superm1@kernel.org>
>Sent: Monday, October 13, 2025 7:21 PM
>To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
><Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>Subject: Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>handler
>
>On 10/12/25 11:54 PM, Lazar, Lijo wrote:
>> [Public]
>>
>> Doesn't this translate to just a higher idle timeout (VPE_IDLE_TIMEOUT ) for
>the particular VPE version?
>>
>> Thanks,
>> Lijo
>
>Yes if the VPE microcode adjusts DPM at runtime this makes sure that it has
>settled when workload is complete.
>
>I expect that a higher VPE_IDLE_TIMEOUT would work too, but it seems less
>scalable to me.
>
[lijo]
I guess VPE firmware behavior could vary across generations. For ex: even if it doesn't lower the clocks in this generation, it could do so after seeing a power gate (any handshake with PMFW). Or, even if it doesn't lower the clock, it may adjust the clocks after powering up.
So probably just keeping vpe.idle_timeout as a variable based on IP version may be good enough for the current issue.
Thanks,
Lijo
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>> Mario Limonciello (AMD)
>>> Sent: Monday, October 13, 2025 12:48 AM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>>> Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>>> handler
>>>
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> [Why]
>>> Newer VPE microcode has functionality that will decrease DPM level
>>> only when a workload has run for 2 or more seconds. If VPE is turned
>>> off before this DPM decrease, the SOC can get stuck with a higher DPM level.
>>>
>>> This can happen from amdgpu's ring buffer test because it's a short
>>> quick workload for VPE and VPE is turned off after 1s.
>>>
>>> [How]
>>> In idle handler besides checking fences are drained check that VPE
>>> DPM level is really is at DPM0. If not, schedule delayed work again until it is.
>>>
>>> Cc: Peyton.Lee@amd.com
>>> Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>> v3:
>>> * Use label to avoid a register read if fences active
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
>>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>> index 474bfe36c0c2f..e8e512de5992a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>> @@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct
>>> work_struct *work) {
>>> struct amdgpu_device *adev =
>>> container_of(work, struct amdgpu_device,
>>> vpe.idle_work.work);
>>> + struct amdgpu_vpe *vpe = &adev->vpe;
>>> unsigned int fences = 0;
>>> + uint32_t dpm_level;
>>>
>>> fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
>>> + if (fences)
>>> + goto reschedule;
>>>
>>> - if (fences == 0)
>>> + dpm_level = adev->pm.dpm_enabled ?
>>> + RREG32(vpe_get_reg_offset(vpe, 0, vpe-
>>>> regs.dpm_request_lv)) : 0;
>>> + if (!dpm_level) {
>>> amdgpu_device_ip_set_powergating_state(adev,
>>> AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
>>> - else
>>> - schedule_delayed_work(&adev->vpe.idle_work,
>>> VPE_IDLE_TIMEOUT);
>>> + return;
>>> + }
>>> +
>>> +reschedule:
>>> + schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
>>> }
>>>
>>> static int vpe_common_init(struct amdgpu_vpe *vpe)
>>> --
>>> 2.43.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
2025-10-13 13:59 ` Lazar, Lijo
@ 2025-10-13 14:03 ` Mario Limonciello
2025-10-13 14:08 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello @ 2025-10-13 14:03 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx@lists.freedesktop.org, Deucher, Alexander
Cc: Limonciello, Mario, Lee, Peyton, Sultan Alsawaf
On 10/13/25 8:59 AM, Lazar, Lijo wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>> -----Original Message-----
>> From: Mario Limonciello <superm1@kernel.org>
>> Sent: Monday, October 13, 2025 7:21 PM
>> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>> Subject: Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>> handler
>>
>> On 10/12/25 11:54 PM, Lazar, Lijo wrote:
>>> [Public]
>>>
>>> Doesn't this translate to just a higher idle timeout (VPE_IDLE_TIMEOUT ) for
>> the particular VPE version?
>>>
>>> Thanks,
>>> Lijo
>>
>> Yes if the VPE microcode adjusts DPM at runtime this makes sure that it has
>> settled when workload is complete.
>>
>> I expect that a higher VPE_IDLE_TIMEOUT would work too, but it seems less
>> scalable to me.
>>
> [lijo]
>
> I guess VPE firmware behavior could vary across generations. For ex: even if it doesn't lower the clocks in this generation, it could do so after seeing a power gate (any handshake with PMFW). Or, even if it doesn't lower the clock, it may adjust the clocks after powering up.
>
> So probably just keeping vpe.idle_timeout as a variable based on IP version may be good enough for the current issue.
"Ideally" PMFW would lower clocks before turning off VPE, but that's not
the case right now on Strix Halo. We just get lucky with older VPE
microcode that it doesn't change this.
My thought was this current solution will work properly on all microcode
version on all products. If PMFW changes behavior we could add
conditional code later to only do this check for DPM level if on older PMFW.
Alex,
What do you want to see here?
>
> Thanks,
> Lijo
>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>> Mario Limonciello (AMD)
>>>> Sent: Monday, October 13, 2025 12:48 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>>>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>>>> Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>>>> handler
>>>>
>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> [Why]
>>>> Newer VPE microcode has functionality that will decrease DPM level
>>>> only when a workload has run for 2 or more seconds. If VPE is turned
>>>> off before this DPM decrease, the SOC can get stuck with a higher DPM level.
>>>>
>>>> This can happen from amdgpu's ring buffer test because it's a short
>>>> quick workload for VPE and VPE is turned off after 1s.
>>>>
>>>> [How]
>>>> In idle handler besides checking fences are drained check that VPE
>>>> DPM level is really is at DPM0. If not, schedule delayed work again until it is.
>>>>
>>>> Cc: Peyton.Lee@amd.com
>>>> Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> ---
>>>> v3:
>>>> * Use label to avoid a register read if fences active
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
>>>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>> index 474bfe36c0c2f..e8e512de5992a 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>> @@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct
>>>> work_struct *work) {
>>>> struct amdgpu_device *adev =
>>>> container_of(work, struct amdgpu_device,
>>>> vpe.idle_work.work);
>>>> + struct amdgpu_vpe *vpe = &adev->vpe;
>>>> unsigned int fences = 0;
>>>> + uint32_t dpm_level;
>>>>
>>>> fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
>>>> + if (fences)
>>>> + goto reschedule;
>>>>
>>>> - if (fences == 0)
>>>> + dpm_level = adev->pm.dpm_enabled ?
>>>> + RREG32(vpe_get_reg_offset(vpe, 0, vpe-
>>>>> regs.dpm_request_lv)) : 0;
>>>> + if (!dpm_level) {
>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>> AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
>>>> - else
>>>> - schedule_delayed_work(&adev->vpe.idle_work,
>>>> VPE_IDLE_TIMEOUT);
>>>> + return;
>>>> + }
>>>> +
>>>> +reschedule:
>>>> + schedule_delayed_work(&adev->vpe.idle_work, VPE_IDLE_TIMEOUT);
>>>> }
>>>>
>>>> static int vpe_common_init(struct amdgpu_vpe *vpe)
>>>> --
>>>> 2.43.0
>>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler
2025-10-13 14:03 ` Mario Limonciello
@ 2025-10-13 14:08 ` Lazar, Lijo
0 siblings, 0 replies; 6+ messages in thread
From: Lazar, Lijo @ 2025-10-13 14:08 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx@lists.freedesktop.org,
Deucher, Alexander
Cc: Limonciello, Mario, Lee, Peyton, Sultan Alsawaf
[AMD Official Use Only - AMD Internal Distribution Only]
>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mario
>Limonciello
>Sent: Monday, October 13, 2025 7:34 PM
>To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org; Deucher,
>Alexander <Alexander.Deucher@amd.com>
>Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
><Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>Subject: Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
>handler
>
>On 10/13/25 8:59 AM, Lazar, Lijo wrote:
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>> -----Original Message-----
>>> From: Mario Limonciello <superm1@kernel.org>
>>> Sent: Monday, October 13, 2025 7:21 PM
>>> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>>> Subject: Re: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in
>>> idle handler
>>>
>>> On 10/12/25 11:54 PM, Lazar, Lijo wrote:
>>>> [Public]
>>>>
>>>> Doesn't this translate to just a higher idle timeout
>>>> (VPE_IDLE_TIMEOUT ) for
>>> the particular VPE version?
>>>>
>>>> Thanks,
>>>> Lijo
>>>
>>> Yes if the VPE microcode adjusts DPM at runtime this makes sure that
>>> it has settled when workload is complete.
>>>
>>> I expect that a higher VPE_IDLE_TIMEOUT would work too, but it seems
>>> less scalable to me.
>>>
>> [lijo]
>>
>> I guess VPE firmware behavior could vary across generations. For ex: even if it
>doesn't lower the clocks in this generation, it could do so after seeing a power
>gate (any handshake with PMFW). Or, even if it doesn't lower the clock, it may
>adjust the clocks after powering up.
>>
>> So probably just keeping vpe.idle_timeout as a variable based on IP version
>may be good enough for the current issue.
>
>"Ideally" PMFW would lower clocks before turning off VPE, but that's not the
>case right now on Strix Halo. We just get lucky with older VPE microcode that it
>doesn't change this.
>
[lijo]
PMFW could be changing clocks only on VPE requests. Ideally, it's not required to lower DPM clock before power-off, just keeping it at deep sleep is good enough which goes even lower than DPM0 levels. Deep sleep these days is taken care by hardware.
Thanks,
Lijo
>My thought was this current solution will work properly on all microcode
>version on all products. If PMFW changes behavior we could add conditional
>code later to only do this check for DPM level if on older PMFW.
>
>Alex,
>
>What do you want to see here?
>>
>> Thanks,
>> Lijo
>>
>>>>> -----Original Message-----
>>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>>> Mario Limonciello (AMD)
>>>>> Sent: Monday, October 13, 2025 12:48 AM
>>>>> To: amd-gfx@lists.freedesktop.org
>>>>> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Lee, Peyton
>>>>> <Peyton.Lee@amd.com>; Sultan Alsawaf <sultan@kerneltoast.com>
>>>>> Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in
>>>>> idle handler
>>>>>
>>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>>
>>>>> [Why]
>>>>> Newer VPE microcode has functionality that will decrease DPM level
>>>>> only when a workload has run for 2 or more seconds. If VPE is
>>>>> turned off before this DPM decrease, the SOC can get stuck with a higher
>DPM level.
>>>>>
>>>>> This can happen from amdgpu's ring buffer test because it's a short
>>>>> quick workload for VPE and VPE is turned off after 1s.
>>>>>
>>>>> [How]
>>>>> In idle handler besides checking fences are drained check that VPE
>>>>> DPM level is really is at DPM0. If not, schedule delayed work again until it
>is.
>>>>>
>>>>> Cc: Peyton.Lee@amd.com
>>>>> Reported-by: Sultan Alsawaf <sultan@kerneltoast.com>
>>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> ---
>>>>> v3:
>>>>> * Use label to avoid a register read if fences active
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 15 ++++++++++++---
>>>>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>>> index 474bfe36c0c2f..e8e512de5992a 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
>>>>> @@ -326,14 +326,23 @@ static void vpe_idle_work_handler(struct
>>>>> work_struct *work) {
>>>>> struct amdgpu_device *adev =
>>>>> container_of(work, struct amdgpu_device,
>>>>> vpe.idle_work.work);
>>>>> + struct amdgpu_vpe *vpe = &adev->vpe;
>>>>> unsigned int fences = 0;
>>>>> + uint32_t dpm_level;
>>>>>
>>>>> fences += amdgpu_fence_count_emitted(&adev->vpe.ring);
>>>>> + if (fences)
>>>>> + goto reschedule;
>>>>>
>>>>> - if (fences == 0)
>>>>> + dpm_level = adev->pm.dpm_enabled ?
>>>>> + RREG32(vpe_get_reg_offset(vpe, 0, vpe-
>>>>>> regs.dpm_request_lv)) : 0;
>>>>> + if (!dpm_level) {
>>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>>> AMD_IP_BLOCK_TYPE_VPE, AMD_PG_STATE_GATE);
>>>>> - else
>>>>> - schedule_delayed_work(&adev->vpe.idle_work,
>>>>> VPE_IDLE_TIMEOUT);
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> +reschedule:
>>>>> + schedule_delayed_work(&adev->vpe.idle_work,
>>>>> +VPE_IDLE_TIMEOUT);
>>>>> }
>>>>>
>>>>> static int vpe_common_init(struct amdgpu_vpe *vpe)
>>>>> --
>>>>> 2.43.0
>>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-13 14:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-12 19:18 [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle handler Mario Limonciello (AMD)
2025-10-13 4:54 ` Lazar, Lijo
2025-10-13 13:50 ` Mario Limonciello
2025-10-13 13:59 ` Lazar, Lijo
2025-10-13 14:03 ` Mario Limonciello
2025-10-13 14:08 ` Lazar, Lijo
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.