On 10/13/25 8:59 AM, Lazar, Lijo wrote:
[AMD Official Use Only - AMD Internal Distribution Only]

-----Original Message-----
From: Mario Limonciello <[email protected]>
Sent: Monday, October 13, 2025 7:21 PM
To: Lazar, Lijo <[email protected]>; [email protected]
Cc: Limonciello, Mario <[email protected]>; Lee, Peyton
<[email protected]>; Sultan Alsawaf <[email protected]>
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 <[email protected]> On Behalf Of
Mario Limonciello (AMD)
Sent: Monday, October 13, 2025 12:48 AM
To: [email protected]
Cc: Limonciello, Mario <[email protected]>; Lee, Peyton
<[email protected]>; Sultan Alsawaf <[email protected]>
Subject: [PATCH v3] drm/amd: Check that VPE has reached DPM0 in idle
handler

From: Mario Limonciello <[email protected]>

[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: [email protected]
Reported-by: Sultan Alsawaf <[email protected]>
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4615
Signed-off-by: Mario Limonciello <[email protected]>
---
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



Reply via email to