<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> [AMD Official Use Only]<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Lazar, Lijo <Lijo.Lazar@amd.com> Sent: Thursday, August 19, 2021 10:36 PM To: Zhu, James <James.Zhu@amd.com>; Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> Subject: RE: [PATCH] drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only]<o:p></o:p> <o:p> </o:p> If that is done – <o:p></o:p> <o:p> </o:p> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD, + AMD_PG_STATE_GATE); + amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD, + AMD_CG_STATE_GATE);<o:p></o:p> <o:p> </o:p> Usual order is CG followed by PG. It comes in the else part, so less likely to happen. Nice to fix for code correctness purpose.<o:p></o:p> [Quan, Evan] Thanks Lijo. Make sense to me. However, actually these code were copied from amdgpu_uvd_idle_work_handler() of amdgpu_uvd.c. Same logic was used there. So, maybe <a id="OWAAM189CC8F3B09847EEA52C17F4845930BF" href="mailto:James.Zhu@amd.com">@Zhu, James</a> or <a id="OWAAM005ACA333EA6426EAFED0E7C5A403D96" href="mailto:Leo.Liu@amd.com">@Liu, Leo</a> can share some insights about this.<o:p></o:p> <o:p> </o:p> BR<o:p></o:p> Evan<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Lijo<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Zhu, James <<a href="mailto:James.Zhu@amd.com">James.Zhu@amd.com</a>> Sent: Thursday, August 19, 2021 7:49 PM To: Quan, Evan <<a href="mailto:Evan.Quan@amd.com">Evan.Quan@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Cc: Deucher, Alexander <<a href="mailto:Alexander.Deucher@amd.com">Alexander.Deucher@amd.com</a>>; Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>>; Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>>; Pan, Xinhui <<a href="mailto:Xinhui.Pan@amd.com">Xinhui.Pan@amd.com</a>> Subject: Re: [PATCH] drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only]<o:p></o:p> <o:p> </o:p> <div> <div> <o:p> </o:p> </div> <div> Why not move changes into hw_fini?<o:p></o:p> </div> <div> <div> <o:p> </o:p> </div> <div id="Signature"> <div> <div id="divtagdefaultwrapper"> Best Regards!<o:p></o:p> <o:p> </o:p> James Zhu<o:p></o:p> </div> </div> </div> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> From: amd-gfx <<a href="mailto:amd-gfx-bounces@lists.freedesktop.org">amd-gfx-bounces@lists.freedesktop.org</a>> on behalf of Evan Quan <<a href="mailto:evan.quan@amd.com">evan.quan@amd.com</a>> Sent: Wednesday, August 18, 2021 11:08 PM To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>> Cc: Deucher, Alexander <<a href="mailto:Alexander.Deucher@amd.com">Alexander.Deucher@amd.com</a>>; Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>>; Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>>; Quan, Evan <<a href="mailto:Evan.Quan@amd.com">Evan.Quan@amd.com</a>>; Pan, Xinhui <<a href="mailto:Xinhui.Pan@amd.com">Xinhui.Pan@amd.com</a>> Subject: [PATCH] drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> Perform proper cleanups on UVD/VCE suspend: powergate enablement, clockgating enablement and dpm disablement. This can fix some hangs observed on suspending when UVD/VCE still using(e.g. issue "pm-suspend" when video is still playing). Change-Id: I36f39d9731e0a9638b52d5d92558b0ee9c23a9ed Signed-off-by: Evan Quan <<a href="mailto:evan.quan@amd.com">evan.quan@amd.com</a>> Signed-off-by: xinhui pan <<a href="mailto:xinhui.pan@amd.com">xinhui.pan@amd.com</a>> --- drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 23 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c index 4eebf973a065..d0fc6ec18c29 100644 --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c @@ -554,6 +554,30 @@ static int uvd_v6_0_suspend(void *handle) int r; struct amdgpu_device *adev = (struct amdgpu_device *)handle; + /* + * Proper cleanups before halting the HW engine: + * - cancel the delayed idle work + * - enable powergating + * - enable clockgating + * - disable dpm + * + * TODO: to align with the VCN implementation, move the + * jobs for clockgating/powergating/dpm setting to + * ->set_powergating_state(). + */ + cancel_delayed_work_sync(&adev->uvd.idle_work); + + if (adev->pm.dpm_enabled) { + amdgpu_dpm_enable_uvd(adev, false); + } else { + amdgpu_asic_set_uvd_clocks(adev, 0, 0); + /* shutdown the UVD block */ + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD, + AMD_PG_STATE_GATE); + amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD, + AMD_CG_STATE_GATE); + } + r = uvd_v6_0_hw_fini(adev); if (r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c index 6d9108fa22e0..a594ade5d30a 100644 --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c @@ -503,6 +503,29 @@ static int vce_v3_0_suspend(void *handle) int r; struct amdgpu_device *adev = (struct amdgpu_device *)handle; + /* + * Proper cleanups before halting the HW engine: + * - cancel the delayed idle work + * - enable powergating + * - enable clockgating + * - disable dpm + * + * TODO: to align with the VCN implementation, move the + * jobs for clockgating/powergating/dpm setting to + * ->set_powergating_state(). + */ + cancel_delayed_work_sync(&adev->vce.idle_work); + + if (adev->pm.dpm_enabled) { + amdgpu_dpm_enable_vce(adev, false); + } else { + amdgpu_asic_set_vce_clocks(adev, 0, 0); + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE, + AMD_PG_STATE_GATE); + amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE, + AMD_CG_STATE_GATE); + } + r = vce_v3_0_hw_fini(adev); if (r) return r; -- 2.29.0<o:p></o:p> </div> </div> </div> </div> </div> </body> </html>