<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2021-12-10 11:19 a.m., Quan, Evan
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:DM6PR12MB2619D1CD28F5D22322CDC40EE4719@DM6PR12MB2619.namprd12.prod.outlook.com">
      <pre class="moz-quote-pre" wrap="">[AMD Official Use Only]



</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">-----Original Message-----
From: Lazar, Lijo <a class="moz-txt-link-rfc2396E" href="mailto:Lijo.Lazar@amd.com"><Lijo.Lazar@amd.com></a>
Sent: Friday, December 10, 2021 8:25 PM
To: Gong, Curry <a class="moz-txt-link-rfc2396E" href="mailto:Curry.Gong@amd.com"><Curry.Gong@amd.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
Cc: Deucher, Alexander <a class="moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"><Alexander.Deucher@amd.com></a>; Zhu, James
<a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com"><James.Zhu@amd.com></a>; Liu, Leo <a class="moz-txt-link-rfc2396E" href="mailto:Leo.Liu@amd.com"><Leo.Liu@amd.com></a>; Quan, Evan
<a class="moz-txt-link-rfc2396E" href="mailto:Evan.Quan@amd.com"><Evan.Quan@amd.com></a>
Subject: Re: [PATCH] drm/amdgpu: When the VCN(1.0) block is suspended,
powergating is explicitly enabled



On 12/10/2021 5:11 PM, chen gong wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">Play a video on the raven (or PCO, raven2) platform, and then do the
S3 test. When resume, the following error will be reported:

amdgpu 0000:02:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR*
ring vcn_dec test failed (-110)
[drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">IP
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">block <vcn_v1_0> failed -110 amdgpu 0000:02:00.0: amdgpu:
amdgpu_device_ip_resume failed (-110).
PM: dpm_run_callback(): pci_pm_resume+0x0/0x90 returns -110

[why]
When playing the video: The power state flag of the vcn block is set
to POWER_STATE_ON.

When doing suspend: There is no change to the power state flag of the
vcn block, it is still POWER_STATE_ON.

When doing resume: Need to open the power gate of the vcn block and
set the power state flag of the VCN block to POWER_STATE_ON.
But at this time, the power state flag of the vcn block is already
POWER_STATE_ON. The power status flag check in the "8f2cdef
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">drm/amd/pm:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">avoid duplicate powergate/ungate setting" patch will return the
amdgpu_dpm_set_powergating_by_smu function directly.
As a result, the gate of the power was not opened, causing the
subsequent ring test to fail.

[how]
In the suspend function of the vcn block, explicitly change the power
state flag of the vcn block to POWER_STATE_OFF.

Signed-off-by: chen gong <a class="moz-txt-link-rfc2396E" href="mailto:curry.gong@amd.com"><curry.gong@amd.com></a>
---
  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 7 +++++++
  1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index d54d720..d73676b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -246,6 +246,13 @@ static int vcn_v1_0_suspend(void *handle)
  {
        int r;
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+       bool cancel_success;
+
+       cancel_success = cancel_delayed_work_sync(&adev-
vcn.idle_work);
+       if (cancel_success) {
+               if (adev->pm.dpm_enabled)
+                       amdgpu_dpm_enable_uvd(adev, false);
+       }

</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
Probably this is a common issue. Can you try moving this to
amdgpu_vcn_suspend?
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">[Quan, Evan] Yes, amdgpu_vcn_suspend seems a more proper place. However, the vcn code is not consistent.
For vcn v2 and later, they already do the manual gate operation in their suspend routine(like vcn_v2_0_stop).
So, it is actually only vcn_v1_0.c suffers this issue.</pre>
    </blockquote>
    <font color="#288aeb">[JZ] Then what is concern for vcn1 not doing
      the same thing as vcn_v2_0_stop?</font><br>
    <blockquote type="cite" cite="mid:DM6PR12MB2619D1CD28F5D22322CDC40EE4719@DM6PR12MB2619.namprd12.prod.outlook.com">
      <pre class="moz-quote-pre" wrap="">
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
if (adev->pm.dpm_enabled)
    amdgpu_device_ip_set_powergating_state(adev,
AMD_IP_BLOCK_TYPE_VCN,AMD_PG_STATE_GATE);

Call this after cancel_delayed_work_sync. Shouldn't have any effect if idle
work already put it in PG state. Evan, what do you think?
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">[Quan, Evan] Should not for now. But I'm considering to raise the dev_dbg prompt in amdgpu_dpm_set_powergating_by_smu for double gate/ungate to dev_info or dev_warn.
Hopefully that can help to capture some potential issues like this. So, better to keep the check for the return value of cancel_delayed_work_sync here.

BR
Evan
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
Thanks,
Lijo

</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">         r = vcn_v1_0_hw_fini(adev);
        if (r)

</pre>
        </blockquote>
      </blockquote>
    </blockquote>
  </body>
</html>