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

Jan Vesely jan.vesely at rutgers.edu
Wed Jun 13 13:30:22 UTC 2018


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

On Wed, Jun 13, 2018 at 8:46 AM, Christian König <
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>
>>
>
> 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>, 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.valid)
>>                         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);
>> -       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
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180613/8962550b/attachment.html>


More information about the amd-gfx mailing list