<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p>To be accurate, the Bo is mapped to engine cache window, and the
      runtime of engine stacks, so we should save it before the
      poweroff.</p>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2021-05-17 2:15 p.m., Leo Liu wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:a09ac369-8d0c-5fc1-77c9-070498143861@amd.com">
      
      <p>The saved data are from the engine cache, it's the runtime of
        engine before suspend, it might be different after you have the
        engine powered off.</p>
      <p><br>
      </p>
      <p>Regards,</p>
      <p>Leo</p>
      <p><br>
      </p>
      <p><br>
      </p>
      <div class="moz-cite-prefix">On 2021-05-17 2:11 p.m., Zhu, James
        wrote:<br>
      </div>
      <blockquote type="cite" cite="mid:DM5PR12MB25173E8B288010950417C2E2E42D9@DM5PR12MB2517.namprd12.prod.outlook.com">
        <style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
        <p style="font-family:Arial;font-size:11pt;color:#0078D7;margin:5pt;" align="Left"> [AMD Official Use Only - Internal Distribution
          Only]<br>
        </p>
        <br>
        <div>
          <div style="font-family: Calibri, Arial, Helvetica,
            sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> save_bo
            needn't ungate vcn,  it just keeps data in memory.<br>
          </div>
          <div>
            <div style="font-family: Calibri, Arial, Helvetica,
              sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br>
            </div>
            <div id="Signature">
              <div>
                <div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000;
                  font-family:Calibri,Arial,Helvetica,sans-serif">
                  <p style="margin-top: 0px; margin-bottom: 0px;">Thanks
                    & Best Regards!</p>
                  <p style="margin-top: 0px; margin-bottom: 0px;"><br>
                  </p>
                  <p style="margin-top: 0px; margin-bottom: 0px;">James
                    Zhu<br>
                  </p>
                </div>
              </div>
            </div>
          </div>
          <hr style="display:inline-block;width:98%" tabindex="-1">
          <div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b>
              Liu, Leo <a class="moz-txt-link-rfc2396E" href="mailto:Leo.Liu@amd.com" moz-do-not-send="true"><Leo.Liu@amd.com></a><br>
              <b>Sent:</b> Monday, May 17, 2021 2:07 PM<br>
              <b>To:</b> Zhu, James <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com" moz-do-not-send="true"><James.Zhu@amd.com></a>;
              Zhu, James <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com" moz-do-not-send="true"><James.Zhu@amd.com></a>;
              <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a>
              <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true"><amd-gfx@lists.freedesktop.org></a><br>
              <b>Subject:</b> Re: [PATCH v2 1/2] drm/amdgpu: enhance
              amdgpu_vcn_suspend</font>
            <div> </div>
          </div>
          <div>
            <p>Definitely, we need to move cancel_delayed_work_sync
              moved to before power gate.<br>
            </p>
            <p>Should "save_bo" be step 4 before power gate ?<br>
            </p>
            <p>Regards,<br>
            </p>
            <p>Leo</p>
            <p><br>
            </p>
            <div class="x_moz-cite-prefix">On 2021-05-17 1:59 p.m.,
              James Zhu wrote:<br>
            </div>
            <blockquote type="cite">
              <p>Then we forgot the proposal I provided before.</p>
              <p>I think the below seq may fixed the race condition
                issue that we are facing.<br>
              </p>
              <p>1. stop scheduling new jobs <br>
              </p>
              <p>    for (i = 0; i < adev->vcn.num_vcn_inst; ++i)
                {<br>
                        if (adev->vcn.harvest_config & (1
                << i))<br>
                            continue;<br>
                <br>
                        ring = &adev->vcn.inst[i].ring_dec;<br>
                        ring->sched.ready = false;<br>
                <br>
                        for (j = 0; j < adev->vcn.num_enc_rings;
                ++j) {<br>
                            ring =
                &adev->vcn.inst[i].ring_enc[j];<br>
                            ring->sched.ready = false;<br>
                        }<br>
                    }</p>
              <p>2.   
                cancel_delayed_work_sync(&adev->vcn.idle_work);</p>
              <p>3.    <font size="2"><span style="font-size:11pt">SOC15_WAIT_ON_RREG(VCN,
                    inst_idx, mmUVD_POWER_STATUS, 1,<br>
                             UVD_POWER_STATUS__UVD_POWER_STATUS_MASK);</span></font></p>
              <p><font size="2"><span style="font-size:11pt">4.    </span></font>amdgpu_device_ip_set_powergating_state(adev,
                AMD_IP_BLOCK_TYPE_VCN,   AMD_PG_STATE_GATE);</p>
              <p>5.  saved_bo</p>
              <p>Best Regards!</p>
              <p>James<br>
              </p>
              <div class="x_moz-cite-prefix">On 2021-05-17 1:43 p.m.,
                Leo Liu wrote:<br>
              </div>
              <blockquote type="cite"><br>
                On 2021-05-17 12:54 p.m., James Zhu wrote: <br>
                <blockquote type="cite">I am wondering if there are
                  still some jobs kept in the queue, it is lucky to
                  check <br>
                </blockquote>
                <br>
                Yes it's possible, in this case delayed handler is set,
                so cancelling once is enough. <br>
                <br>
                <br>
                <blockquote type="cite"><br>
                  UVD_POWER_STATUS done, but after, fw start a new job
                  that list in the queue. <br>
                  <br>
                  To handle this situation perfectly, we need add
                  mechanism to suspend fw first. <br>
                </blockquote>
                <br>
                I think that should be handled by the sequence from
                vcn_v3_0_stop_dpg_mode(). <br>
                <br>
                <br>
                <blockquote type="cite"><br>
                  Another case, if it is unlucky, that  vcn fw hung at
                  that time, UVD_POWER_STATUS <br>
                  <br>
                  always keeps busy.   then it needs force powering gate
                  the vcn hw after certain time waiting. <br>
                </blockquote>
                <br>
                Yep, we still need to gate VCN power after certain
                timeout. <br>
                <br>
                <br>
                Regards, <br>
                <br>
                Leo <br>
                <br>
                <br>
                <br>
                <blockquote type="cite"><br>
                  Best Regards! <br>
                  <br>
                  James <br>
                  <br>
                  On 2021-05-17 12:34 p.m., Leo Liu wrote: <br>
                  <blockquote type="cite"><br>
                    On 2021-05-17 11:52 a.m., James Zhu wrote: <br>
                    <blockquote type="cite">During vcn suspends, stop
                      ring continue to receive new requests, <br>
                      and try to wait for all vcn jobs to finish
                      gracefully. <br>
                      <br>
                      v2: Forced powering gate vcn hardware after few
                      wainting retry. <br>
                      <br>
                      Signed-off-by: James Zhu <a class="x_moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com" moz-do-not-send="true">
                        <James.Zhu@amd.com></a> <br>
                      --- <br>
                        drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22
                      +++++++++++++++++++++- <br>
                        1 file changed, 21 insertions(+), 1 deletion(-)
                      <br>
                      <br>
                      diff --git
                      a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
                      b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c <br>
                      index 2016459..9f3a6e7 100644 <br>
                      --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c <br>
                      +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c <br>
                      @@ -275,9 +275,29 @@ int amdgpu_vcn_suspend(struct
                      amdgpu_device *adev) <br>
                        { <br>
                            unsigned size; <br>
                            void *ptr; <br>
                      +    int retry_max = 6; <br>
                            int i; <br>
                        -   
                      cancel_delayed_work_sync(&adev->vcn.idle_work);
                      <br>
                      +    for (i = 0; i < adev->vcn.num_vcn_inst;
                      ++i) { <br>
                      +        if (adev->vcn.harvest_config & (1
                      << i)) <br>
                      +            continue; <br>
                      +        ring =
                      &adev->vcn.inst[i].ring_dec; <br>
                      +        ring->sched.ready = false; <br>
                      + <br>
                      +        for (j = 0; j <
                      adev->vcn.num_enc_rings; ++j) { <br>
                      +            ring =
                      &adev->vcn.inst[i].ring_enc[j]; <br>
                      +            ring->sched.ready = false; <br>
                      +        } <br>
                      +    } <br>
                      + <br>
                      +    while (retry_max-- &&
                      cancel_delayed_work_sync(&adev->vcn.idle_work))
                      <br>
                      +        mdelay(5); <br>
                    </blockquote>
                    <br>
                    I think it's possible to have one pending job
                    unprocessed with VCN when suspend sequence getting
                    here, but it shouldn't be more than one,
                    cancel_delayed_work_sync probably return false after
                    the first time, so calling cancel_delayed_work_sync
                    once should be enough here. we probably need to wait
                    longer from: <br>
                    <br>
                    SOC15_WAIT_ON_RREG(VCN, inst_idx,
                    mmUVD_POWER_STATUS, 1, <br>
                            UVD_POWER_STATUS__UVD_POWER_STATUS_MASK); <br>
                    <br>
                    to make sure the unprocessed job get done. <br>
                    <br>
                    <br>
                    Regards, <br>
                    <br>
                    Leo <br>
                    <br>
                    <br>
                    <blockquote type="cite">+    if (!retry_max
                      && !amdgpu_sriov_vf(adev)) { <br>
                      +        if (RREG32_SOC15(VCN, i, mmUVD_STATUS)) {
                      <br>
                      +            dev_warn(adev->dev, "Forced
                      powering gate vcn hardware!"); <br>
                      +            vcn_v3_0_set_powergating_state(adev,
                      AMD_PG_STATE_GATE); <br>
                      +        } <br>
                      +    } <br>
                              for (i = 0; i <
                      adev->vcn.num_vcn_inst; ++i) { <br>
                                if (adev->vcn.harvest_config & (1
                      << i)) <br>
                    </blockquote>
                  </blockquote>
                </blockquote>
              </blockquote>
            </blockquote>
          </div>
        </div>
      </blockquote>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
amd-gfx mailing list
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
</pre>
    </blockquote>
  </body>
</html>