<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>