<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left">
[AMD Official Use Only]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Flelix,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I have tested YiFan's patch and pco s3 test can pass.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
The original s3 issue always occurred on resume, And YiFan's patches</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
are only related to initialization.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi YiFan,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Reviewed-by:<span style="color:#c0c0c0"> </span>James<span style="color:#c0c0c0">
</span>Zhu<span style="color:#c0c0c0"> </span><James.Zhu@amd.com><span style="color:#c0c0c0">
</span><span style="color:#ff9d04">for</span><span style="color:#c0c0c0"> </span>
the<span style="color:#c0c0c0"> </span>series
<pre style="margin-top:0px;margin-bottom:0px">Tested-by:<span style="color:#c0c0c0"> </span>James<span style="color:#c0c0c0"> </span>Zhu<span style="color:#c0c0c0"> </span><James.Zhu@amd.com><span style="color:#c0c0c0"> </span><span style="color:#ff9d04">for</span><span style="color:#c0c0c0"> </span>the<span style="color:#c0c0c0"> </span>series</pre>
<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>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Kuehling, Felix <Felix.Kuehling@amd.com><br>
<b>Sent:</b> Monday, October 4, 2021 11:00 AM<br>
<b>To:</b> Zhang, Yifan <Yifan1.Zhang@amd.com>; Zhu, James <James.Zhu@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Subject:</b> Re: [PATCH 2/2] drm/amdgpu: init iommu after amdkfd device init</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">I'm trying to understand what the end result is after James' and your<br>
patches. If I'm reading it correctly, we now initialize IOMMUv2 after<br>
kfd_device_init during initialization, but before kfd_device_init during<br>
resume from S3. Is that the correct understanding?<br>
<br>
My concern is, that we may run into related problems again if the root<br>
cause and the mechanism of the fix are poorly understood. Do you have an<br>
explanation why this different sequence is needed during init and resume?<br>
<br>
That said, given that your patches are tested and reviewed by James, you<br>
can add my Acked-by to both patches. The series is<br>
<br>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com><br>
<br>
Thanks,<br>
Felix<br>
<br>
<br>
Am 2021-10-04 um 5:42 a.m. schrieb Zhang, Yifan:<br>
><br>
> [AMD Official Use Only]<br>
><br>
> <br>
><br>
> Hi Felix,<br>
><br>
> <br>
><br>
> After sync w/ James, we agree that this patch series could fix both<br>
> our problems, and he verified this patch series will not cause<br>
> regression of his previous issue. Do you have more comments regarding<br>
> this patch series ? Thanks.<br>
><br>
> <br>
><br>
> BRs,<br>
><br>
> Yifan<br>
><br>
> <br>
><br>
> *From:* Zhu, James <James.Zhu@amd.com><br>
> *Sent:* Wednesday, September 29, 2021 9:19 PM<br>
> *To:* Kuehling, Felix <Felix.Kuehling@amd.com>; Zhang, Yifan<br>
> <Yifan1.Zhang@amd.com>; amd-gfx@lists.freedesktop.org<br>
> *Subject:* Re: [PATCH 2/2] drm/amdgpu: init iommu after amdkfd device init<br>
><br>
> <br>
><br>
> [AMD Official Use Only]<br>
><br>
> <br>
><br>
> H Felix,<br>
><br>
> <br>
><br>
> Since the previous patch can help on PCO suspend/resume hung issue.<br>
> Let me work with YiFan to see if<br>
><br>
> there is proper way to cover both cases.<br>
><br>
> <br>
><br>
> Thanks & Best Regards!<br>
><br>
> <br>
><br>
> James Zhu<br>
><br>
> ------------------------------------------------------------------------<br>
><br>
> *From:*Kuehling, Felix <Felix.Kuehling@amd.com<br>
> <<a href="mailto:Felix.Kuehling@amd.com">mailto:Felix.Kuehling@amd.com</a>>><br>
> *Sent:* Tuesday, September 28, 2021 11:41 AM<br>
> *To:* Zhang, Yifan <Yifan1.Zhang@amd.com<br>
> <<a href="mailto:Yifan1.Zhang@amd.com">mailto:Yifan1.Zhang@amd.com</a>>>; amd-gfx@lists.freedesktop.org<br>
> <<a href="mailto:amd-gfx@lists.freedesktop.org">mailto:amd-gfx@lists.freedesktop.org</a>> <amd-gfx@lists.freedesktop.org<br>
> <<a href="mailto:amd-gfx@lists.freedesktop.org">mailto:amd-gfx@lists.freedesktop.org</a>>>; Zhu, James <James.Zhu@amd.com<br>
> <<a href="mailto:James.Zhu@amd.com">mailto:James.Zhu@amd.com</a>>><br>
> *Subject:* Re: [PATCH 2/2] drm/amdgpu: init iommu after amdkfd device<br>
> init<br>
><br>
> <br>
><br>
> [+James]<br>
><br>
> This basically undoes James's change "drm/amdgpu: move iommu_resume<br>
> before ip init/resume". I assume James made his change for a reason. Can<br>
> you please discuss the issue with him and determine a solution that<br>
> solves both your problem and his?<br>
><br>
> If James' patch series was a mistake, I'd prefer to revert his patches,<br>
> because his patches complicated the initialization sequence and exposed<br>
> the iommu init sequence in amdgpu.<br>
><br>
> Thanks,<br>
> Felix<br>
><br>
><br>
> Am 2021-09-28 um 4:28 a.m. schrieb Yifan Zhang:<br>
> > This patch is to fix clinfo failure in Raven/Picasso:<br>
> ><br>
> > Number of platforms: 1<br>
> > Platform Profile: FULL_PROFILE<br>
> > Platform Version: OpenCL 2.2 AMD-APP (3364.0)<br>
> > Platform Name: AMD Accelerated Parallel Processing<br>
> > Platform Vendor: Advanced Micro Devices, Inc.<br>
> > Platform Extensions: cl_khr_icd cl_amd_event_callback<br>
> ><br>
> > Platform Name: AMD Accelerated Parallel Processing Number of<br>
> devices: 0<br>
> ><br>
> > Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com<br>
> <<a href="mailto:yifan1.zhang@amd.com">mailto:yifan1.zhang@amd.com</a>>><br>
> > ---<br>
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++----<br>
> > 1 file changed, 4 insertions(+), 4 deletions(-)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> > index 4c8f2f4647c0..89ed9b091386 100644<br>
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> > @@ -2393,10 +2393,6 @@ static int amdgpu_device_ip_init(struct<br>
> amdgpu_device *adev)<br>
> > if (r)<br>
> > goto init_failed;<br>
> > <br>
> > - r = amdgpu_amdkfd_resume_iommu(adev);<br>
> > - if (r)<br>
> > - goto init_failed;<br>
> > -<br>
> > r = amdgpu_device_ip_hw_init_phase1(adev);<br>
> > if (r)<br>
> > goto init_failed;<br>
> > @@ -2435,6 +2431,10 @@ static int amdgpu_device_ip_init(struct<br>
> amdgpu_device *adev)<br>
> > if (!adev->gmc.xgmi.pending_reset)<br>
> > amdgpu_amdkfd_device_init(adev);<br>
> > <br>
> > + r = amdgpu_amdkfd_resume_iommu(adev);<br>
> > + if (r)<br>
> > + goto init_failed;<br>
> > +<br>
> > amdgpu_fru_get_product_info(adev);<br>
> > <br>
> > init_failed:<br>
><br>
</div>
</span></font></div>
</div>
</body>
</html>