[PATCH v2] drm/amdgpu: fix scheduler timeout calc

[Koenig, Christian]

Am 28.06.19 um 07:32 schrieb Cui, Flora:
> 在 6/27/2019 6:17 PM, Christian König 写道:
>> Am 27.06.19 um 12:03 schrieb Cui, Flora:
>>> scheduler timeout is in jiffies
>>> v2: move timeout check to amdgpu_device_get_job_timeout_settings after
>>> parsing the value
>>>
>>> Change-Id: I26708c163db943ff8d930dd81bcab4b4b9d84eb2
>>> Signed-off-by: Flora Cui <flora.cui at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 7 +++++--
>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index e74a175..cc29d70 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -1300,7 +1300,9 @@ int
>>> amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>>>         * By default timeout for non compute jobs is 10000.
>>>         * And there is no timeout enforced on compute jobs.
>>>         */
>>> -    adev->gfx_timeout = adev->sdma_timeout = adev->video_timeout =
>>> 10000;
>>> +    adev->gfx_timeout = \
>>> +        adev->sdma_timeout = \
>>> +        adev->video_timeout = msecs_to_jiffies(10000);
>> Of hand that looks very odd to me. This is not a macro so why the \ here?
> will update in v3
>>>        adev->compute_timeout = MAX_SCHEDULE_TIMEOUT;
>>>          if (strnlen(input, AMDGPU_MAX_TIMEOUT_PARAM_LENTH)) {
>>> @@ -1314,7 +1316,8 @@ int
>>> amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>>>                if (timeout <= 0) {
>>>                    index++;
>>>                    continue;
>>> -            }
>>> +            } else
>>> +                timeout = msecs_to_jiffies(timeout);
>> You can actually remove the "if (timeout <= 0)" as well,
>> msecs_to_jiffies will do the right thing for negative values.
> IMHO check for timeout==0 is still needed. msecs_to_jiffies() would
> return 0 and that's not desired for scheduler timer.

Good point, so 0 would use the default value and negative values would 
use infinity.

That sounds like a good solution to me,
Christian.

>> Christian.
>>
>>>                  switch (index++) {
>>>                case 0: