<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=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:新細明體;
panose-1:2 2 5 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
@font-face
{font-family:"\@新細明體";
panose-1:2 1 6 1 0 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
code
{mso-style-priority:99;
font-family:"Courier New";}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.EmailStyle22
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.25in 1.0in 1.25in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:windowtext">I don’t mean to read it twice. The solution is to make first read later. I didn’t modify the original code to make code difference less and simple. I guess it should work to remove the original read there.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="color:windowtext">From:</span></b><span style="color:windowtext"> Liu, Leo <Leo.Liu@amd.com>
<br>
<b>Sent:</b> Tuesday, May 28, 2019 12:40 AM<br>
<b>To:</b> Li, Ching-shih (Louis) <Ching-shih.Li@amd.com>; S, Shirish <Shirish.S@amd.com>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>; Zhang, Jerry <Jerry.Zhang@amd.com>; Deng, Emily <Emily.Deng@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com><br>
<b>Cc:</b> amd-gfx@lists.freedesktop.org<br>
<b>Subject:</b> Re: [PATCH] drm/amdgpu: fix ring test failure issue during s3 in vce 3.0<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p>int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)<br>
{<br>
struct amdgpu_device *adev = ring->adev;<o:p></o:p></p>
<p><span style="color:red"> uint32_t rptr = amdgpu_ring_get_rptr(ring);</span><o:p></o:p></p>
<p> unsigned i;<br>
int r, timeout = adev->usec_timeout;<br>
<br>
/* skip ring test for sriov*/<br>
if (amdgpu_sriov_vf(adev))<br>
return 0;<br>
<br>
r = amdgpu_ring_alloc(ring, 16);<br>
if (r)<br>
return r;<br>
<br>
amdgpu_ring_write(ring, VCE_CMD_END);<br>
amdgpu_ring_commit(ring);<o:p></o:p></p>
<p><o:p> </o:p></p>
<p>Above is original code, rptr is updated when called, and below is your patch, my question is why do you need to get rptr twice?<o:p></o:p></p>
<pre>@@ -1084,6 +1084,8 @@ int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)<o:p></o:p></pre>
<pre> if (r)<o:p></o:p></pre>
<pre> return r;<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>+ rptr = amdgpu_ring_get_rptr(ring);<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre> amdgpu_ring_write(ring, VCE_CMD_END);<o:p></o:p></pre>
<pre> amdgpu_ring_commit(ring);<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<p><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 5/27/19 12:22 PM, Li, Ching-shih (Louis) wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="color:windowtext">Hi Leo,</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext"> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext">Yes, I confirm it is the root cause
<b>for the Chrome S3 issue</b>. Whenever system is resumed, the original instruction always gets zero. However, I have no idea why it fails, and didn’t verify this problem on CRB or any other Linux platform yet.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext">Although I think the ideal solution is an indicator, e.g. a register, for driver to check if related firmware and hardware are ready to work. So driver can make sure it is ok to read rptr. Without any reference
document, I can only try to solve the problem by modifying driver. Debug traces reveal that only first rptr read fails, but the read in check loop is ok. Therefore, a solution comes to mind: to update rptr later for initial rptr value. Tests prove it working
in Chrome platforms. Fyi~</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext"> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext">BR,</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext">Louis</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:windowtext"> </span><o:p></o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="color:windowtext">From:</span></b><span style="color:windowtext"> Liu, Leo
<a href="mailto:Leo.Liu@amd.com"><Leo.Liu@amd.com></a> <br>
<b>Sent:</b> Monday, May 27, 2019 9:01 PM<br>
<b>To:</b> S, Shirish <a href="mailto:Shirish.S@amd.com"><Shirish.S@amd.com></a>; Grodzovsky, Andrey
<a href="mailto:Andrey.Grodzovsky@amd.com"><Andrey.Grodzovsky@amd.com></a>; Zhang, Jerry
<a href="mailto:Jerry.Zhang@amd.com"><Jerry.Zhang@amd.com></a>; Deng, Emily <a href="mailto:Emily.Deng@amd.com">
<Emily.Deng@amd.com></a>; Deucher, Alexander <a href="mailto:Alexander.Deucher@amd.com">
<Alexander.Deucher@amd.com></a><br>
<b>Cc:</b> <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Li, Ching-shih (Louis)
<a href="mailto:Ching-shih.Li@amd.com"><Ching-shih.Li@amd.com></a><br>
<b>Subject:</b> Re: [PATCH] drm/amdgpu: fix ring test failure issue during s3 in vce 3.0</span><o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
<p> <o:p></o:p></p>
<div>
<p class="MsoNormal">On 5/27/19 3:42 AM, S, Shirish wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>From: Louis Li <a href="mailto:Ching-shih.Li@amd.com"><Ching-shih.Li@amd.com></a><o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>[What]<o:p></o:p></pre>
<pre>vce ring test fails consistently during resume in s3 cycle, due to<o:p></o:p></pre>
<pre>mismatch read & write pointers.<o:p></o:p></pre>
<pre>On debug/analysis its found that rptr to be compared is not being<o:p></o:p></pre>
<pre>correctly updated/read, which leads to this failure.<o:p></o:p></pre>
<pre>Below is the failure signature:<o:p></o:p></pre>
<pre> [drm:amdgpu_vce_ring_test_ring] *ERROR* amdgpu: ring 12 test failed<o:p></o:p></pre>
<pre> [drm:amdgpu_device_ip_resume_phase2] *ERROR* resume of IP block <vce_v3_0> failed -110<o:p></o:p></pre>
<pre> [drm:amdgpu_device_resume] *ERROR* amdgpu_device_ip_resume failed (-110).<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>[How]<o:p></o:p></pre>
<pre>fetch rptr appropriately, meaning move its read location further down<o:p></o:p></pre>
<pre>in the code flow.<o:p></o:p></pre>
<pre>With this patch applied the s3 failure is no more seen for >5k s3 cycles,<o:p></o:p></pre>
<pre>which otherwise is pretty consistent.<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>Signed-off-by: Louis Li <a href="mailto:Ching-shih.Li@amd.com"><Ching-shih.Li@amd.com></a><o:p></o:p></pre>
<pre>---<o:p></o:p></pre>
<pre> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 ++<o:p></o:p></pre>
<pre> 1 file changed, 2 insertions(+)<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c<o:p></o:p></pre>
<pre>index c021b11..92f9d46 100644<o:p></o:p></pre>
<pre>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c<o:p></o:p></pre>
<pre>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c<o:p></o:p></pre>
<pre>@@ -1084,6 +1084,8 @@ int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)<o:p></o:p></pre>
<pre> if (r)<o:p></o:p></pre>
<pre> return r;<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>+ rptr = amdgpu_ring_get_rptr(ring);<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
</blockquote>
<p>The rptr update is there:<o:p></o:p></p>
<pre><code> uint32_t rptr = amdgpu_ring_get_rptr(ring);</code><o:p></o:p></pre>
<pre><code> </code><o:p></o:p></pre>
<pre><code>Are you sure this is the root cause?</code><o:p></o:p></pre>
<pre><code> </code><o:p></o:p></pre>
<pre><code>Regards,</code><o:p></o:p></pre>
<pre><code>Leo</code><o:p></o:p></pre>
<pre><code> </code><o:p></o:p></pre>
<p> <o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> <o:p></o:p></pre>
<pre> amdgpu_ring_write(ring, VCE_CMD_END);<o:p></o:p></pre>
<pre> amdgpu_ring_commit(ring);<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
</blockquote>
</blockquote>
</div>
</body>
</html>