[PATCH] drm/amdgpu: Partially revert commit 2dc80b006

[William Lewis]

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.leichtzumerken at gmail.com<mailto:ckoenig.leichtzumerken at 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.Zhu at amd.com<mailto:Rex.Zhu at 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.koenig at amd.com<mailto:christian.koenig at 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 at lists.freedesktop.org<mailto:amd-gfx at 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 at lists.freedesktop.org<mailto:amd-gfx at 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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180613/afd18608/attachment-0001.html>