On 06/13/2018 08:30 AM, Jan Vesely wrote:
Hi,

can you please improve the commit message?
seeing "Revert $HASH" conveys zero information about the code change.
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.
git commit logs like:
"revert XYZ" or "fix bug #123" make it really cumbersome to actually look at 
history and pick interesting/breaking commits.

thanks,
Jan
Just a polite agreement and seconding here.  The abundance of acronyms is 
confusing but understandable, and the weekly dumps of DC updates show not a bit 
of dialog about how or why the patches were written.  Although I read the rest 
of the emails to the list, I can usually just mark that set of 18 to 30 mails 
as read and send it straight to the trash.  It's too much work to glean useful 
information from them.

Regards,
Will


On Wed, Jun 13, 2018 at 8:46 AM, Christian König 
<ckoenig.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>> 
wrote:
Am 13.06.2018 um 13:40 schrieb Rex Zhu:
Move the CG enablement out of delay worker thread.

1. CG/PG enablement are part of gpu hw ip initialize, we should
wait for them complete. otherwise, there are some potential conflicts,
for example, Suspend and CG enablement concurrently.
2. better run ib test after hw initialize completely. That is to say,
    ib test should be after CG/PG enablement. otherwise, the test will
    not cover the cg/pg/poweroff enable case.

Signed-off-by: Rex Zhu <rex....@amd.com<mailto:rex....@amd.com>>

Yeah, that thought came to my mind as well.

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.

Patch is Reviewed-by: Christian König 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>, but there could be 
some fallout we could need to deal with.

Thanks,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 14 ++++++++------
  1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9647f54..90b78c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1709,10 +1709,6 @@ static int amdgpu_device_ip_late_set_cg_state(struct 
amdgpu_device *adev)
        if (amdgpu_emu_mode == 1)
                return 0;
  -     r = amdgpu_ib_ring_tests(adev);
-       if (r)
-               DRM_ERROR("ib ring test failed (%d).\n", r);
-
        for (i = 0; i < adev->num_ip_blocks; i++) {
                if 
(!adev->ip_blocks[i].status.va<https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fstatus.va&data=02%7C01%7C%7C3869c12d0e1d489d0f1b08d5d131d2d5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636644934270231216&sdata=U37xx4d4KwmIMbYBRB2V8ueYMIPTQx6t%2Bm90qpR60Ek%3D&reserved=0>lid)
                        continue;
@@ -1793,6 +1789,9 @@ static int amdgpu_device_ip_late_init(struct 
amdgpu_device *adev)
                }
        }
  +     amdgpu_device_ip_late_set_cg_state(adev);
+       amdgpu_device_ip_late_set_pg_state(adev);
+
        queue_delayed_work(system_wq, &adev->late_init_work,
                           msecs_to_jiffies(AMDGPU_RESUME_MS));
  @@ -1921,8 +1920,11 @@ static void 
amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
  {
        struct amdgpu_device *adev =
                container_of(work, struct amdgpu_device, 
late_init_work.work<https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flate_init_work.work&data=02%7C01%7C%7C3869c12d0e1d489d0f1b08d5d131d2d5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636644934270387474&sdata=R3%2BYpuk333iEpjzcc8lLjr6NKPi74VximJcFAGCHqUc%3D&reserved=0>);
-       amdgpu_device_ip_late_set_cg_state(adev);
-       amdgpu_device_ip_late_set_pg_state(adev);
+       int r;
+
+       r = amdgpu_ib_ring_tests(adev);
+       if (r)
+               DRM_ERROR("ib ring test failed (%d).\n", r);
  }
    /**

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx<https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7C%7C3869c12d0e1d489d0f1b08d5d131d2d5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636644934270387474&sdata=EllXqELZzZXDFafao7ingUyBAK4bl2T4Ja54uqdzR%2BE%3D&reserved=0>




_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7C%7C3869c12d0e1d489d0f1b08d5d131d2d5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636644934270387474&sdata=EllXqELZzZXDFafao7ingUyBAK4bl2T4Ja54uqdzR%2BE%3D&reserved=0


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to