<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> <p style="margin-top:0;margin-bottom:0">Hi Jan,</p> <p style="margin-top:0;margin-bottom:0"><br> </p> Thanks for your suggestion. Obvious the patch title was i<span>nappropriate. I will update it.</span> <div><br> <span></span> <div>In fact, I originally plan to send this patch internally for discussion. So just thought that "Revert xxxxx" can cause the attention.</div> <div><br> </div> <div>Best Regards</div> <div>Rex</div> <div><br> <span></span> <div><br> </div> <div><br> <br> <div style="color: rgb(0, 0, 0);"> <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> jv356@scarletmail.rutgers.edu <jv356@scarletmail.rutgers.edu> on behalf of Jan Vesely <jan.vesely@rutgers.edu><br> <b>Sent:</b> Wednesday, June 13, 2018 9:30 PM<br> <b>To:</b> Koenig, Christian<br> <b>Cc:</b> Zhu, Rex; amd-gfx list<br> <b>Subject:</b> Re: [PATCH] drm/amdgpu: Partially revert commit 2dc80b006</font> <div> </div> </div> <meta content="text/html; charset=utf-8"> <div> <div dir="ltr">Hi, <div><br> </div> <div>can you please improve the commit message?</div> <div>seeing "Revert $HASH" conveys zero information about the code change.</div> <div>I'm sorry for bringing this up again, but following AMDGPU/Radeon driver development is an exercise in frustration for anyone who is not on AMD's payroll.</div> <div>git commit logs like:</div> <div>"revert XYZ" or "fix bug #123" make it really cumbersome to actually look at history and pick interesting/breaking commits.</div> <div><br> </div> <div>thanks,</div> <div>Jan</div> </div> <div class="x_gmail_extra"><br> <div class="x_gmail_quote">On Wed, Jun 13, 2018 at 8:46 AM, Christian König <span dir="ltr"> <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank" id="LPlnk23845" class="OWAAutoLink" previewremoved="true">ckoenig.leichtzumerken@gmail.com</a>></span> wrote:<br> <blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> <span class="">Am 13.06.2018 um 13:40 schrieb Rex Zhu:<br> <blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> Move the CG enablement out of delay worker thread.<br> <br> 1. CG/PG enablement are part of gpu hw ip initialize, we should<br> wait for them complete. otherwise, there are some potential conflicts,<br> for example, Suspend and CG enablement concurrently.<br> 2. better run ib test after hw initialize completely. That is to say,<br> ib test should be after CG/PG enablement. otherwise, the test will<br> not cover the cg/pg/poweroff enable case.<br> <br> Signed-off-by: Rex Zhu <<a href="mailto:Rex.Zhu@amd.com" target="_blank" id="LPlnk396741" class="OWAAutoLink" previewremoved="true">Rex.Zhu@amd.com</a>><br> </blockquote> <br> </span>Yeah, that thought came to my mind as well.<br> <br> Essentially the IB test should simulate a submission from userspace to make sure that the stack is working as expected. I think it was just moved before CG/PG to avoid issues with that, which is actually not very clever.<br> <br> Patch is Reviewed-by: Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank" id="LPlnk442308" class="OWAAutoLink" previewremoved="true">christian.koenig@amd.com</a>>, but there could be some fallout we could need to deal with.<br> <br> Thanks,<br> Christian. <div class="x_HOEnZb"> <div class="x_h5"><br> <br> <blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> ---<br> drivers/gpu/drm/amd/amdgpu/amd<wbr>gpu_device.c | 14 ++++++++------<br> 1 file changed, 8 insertions(+), 6 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/amd/amdgpu/a<wbr>mdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/a<wbr>mdgpu_device.c<br> index 9647f54..90b78c7 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/a<wbr>mdgpu_device.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/a<wbr>mdgpu_device.c<br> @@ -1709,10 +1709,6 @@ static int amdgpu_device_ip_late_set_cg_s<wbr>tate(struct amdgpu_device *adev)<br> if (amdgpu_emu_mode == 1)<br> return 0;<br> - r = amdgpu_ib_ring_tests(adev);<br> - if (r)<br> - DRM_ERROR("ib ring test failed (%d).\n", r);<br> -<br> for (i = 0; i < adev->num_ip_blocks; i++) {<br> if (!adev->ip_blocks[i].<a href="http://status.va" id="LPlnk710404" class="OWAAutoLink" previewremoved="true">status.va</a><wbr>lid)<br> continue;<br> @@ -1793,6 +1789,9 @@ static int amdgpu_device_ip_late_init(str<wbr>uct amdgpu_device *adev)<br> }<br> }<br> + amdgpu_device_ip_late_set_cg_<wbr>state(adev);<br> + amdgpu_device_ip_late_set_pg_<wbr>state(adev);<br> +<br> queue_delayed_work(system_wq, &adev->late_init_work,<br> msecs_to_jiffies(AMDGPU_RESUM<wbr>E_MS));<br> @@ -1921,8 +1920,11 @@ static void amdgpu_device_ip_late_init_fun<wbr>c_handler(struct work_struct *work)<br> {<br> struct amdgpu_device *adev =<br> container_of(work, struct amdgpu_device, <a href="http://late_init_work.work" rel="noreferrer" target="_blank" id="LPlnk420709" class="OWAAutoLink" previewremoved="true"> late_init_work.work</a>);<br> - amdgpu_device_ip_late_set_cg_<wbr>state(adev);<br> - amdgpu_device_ip_late_set_pg_<wbr>state(adev);<br> + int r;<br> +<br> + r = amdgpu_ib_ring_tests(adev);<br> + if (r)<br> + DRM_ERROR("ib ring test failed (%d).\n", r);<br> }<br> /**<br> </blockquote> <br> </div> </div> <div class="x_HOEnZb"> <div class="x_h5">______________________________<wbr>_________________<br> amd-gfx mailing list<br> <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank" id="LPlnk537513" class="OWAAutoLink" previewremoved="true">amd-gfx@lists.freedesktop.org</a><br> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank" id="LPlnk798304" class="OWAAutoLink" previewremoved="true">https://lists.freedesktop.org/<wbr>mailman/listinfo/amd-gfx</a><br> </div> </div> </blockquote> </div> <br> </div> </div> </div> </div> </div> </div> </div> </body> </html>