On 17-01-24 10:05 AM, Xue, Ken wrote:
>> From: Christian König [mailto:deathsim...@vodafone.de]
>> Sent: Tuesday, January 24, 2017 10:09 PM
>> To: Xue, Ken; amd-gfx mailing list
>> Cc: dl.SRDC_SW_GPUVirtualization
>> Subject: Re: [PATCH] drm/amdgpu: Refine the handshake between guest and
>> server by mailbox
>>
>> Am 24.01.2017 um 13:55 schrieb Xue, Ken:
>>> Add check for bit RCV_MSG_VALID of MAILBOX_CONTROL before reading
>>> message and after ACK server.
>>>
>>> Change-Id: I717a77fd90dfbdfce4dc56e978338ffc5db24fca
>>> Signed-off-by: Ken Xue <ken....@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c | 20 ++++++++++++++++++++
>>>   1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>>> b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>>> index d2622b6..b2c46db 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>>> @@ -318,10 +318,25 @@ void xgpu_vi_init_golden_registers(struct
>> amdgpu_device *adev)
>>>   static void xgpu_vi_mailbox_send_ack(struct amdgpu_device *adev)
>>>   {
>>>     u32 reg;
>>> +   int timeout = VI_MAILBOX_TIMEDOUT;
>>> +   u32 mask = REG_FIELD_MASK(MAILBOX_CONTROL, RCV_MSG_VALID);
>>>
>>>     reg = RREG32(mmMAILBOX_CONTROL);
>>>     reg = REG_SET_FIELD(reg, MAILBOX_CONTROL, RCV_MSG_ACK, 1);
>>>     WREG32(mmMAILBOX_CONTROL, reg);
>>> +
>>> +   /*Wait for RCV_MSG_VALID to be 0*/
>>> +   reg = RREG32(mmMAILBOX_CONTROL);
>>> +   while (reg & mask) {
>>> +           if (timeout <= 0) {
>>> +                   pr_err("RCV_MSG_VALID is not cleared\n");
>>> +                   break;
>>> +           }
>>> +           msleep(1);
>> Are you sure that you want to use msleep() here instead of mdelay() ?
>>
>> msleep() is horrible inaccurate, e.g. depending on the definition of HZ you 
>> can
>> sleep for 10ms instead of 1ms IIRC.
>>
>> mdelay() is a busy wait, so the CPU can't do anything else useful while 
>> waiting
>> but I don't think that this will hurt us here.
> Thanks for your suggestion.
> Currently, msleep may be a correct choice.
> 1)accuracy is not necessary here
> 2)the VI_MAILBOX_TIMEDOUT is 5000. if there is an issue from server side, 
> driver may be delayed 5 seconds
> 3)I followed the same style like other codes in the same file.

If msleep sleeps for 10ms instead of 1ms, then your loop may end up
waiting for 50s instead of 5s.

If you want the total timeout to be more predictable, it may be better
to compare jiffies rather than count loop iterations.

Regards,
  Felix

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

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

Reply via email to