[PATCH] drm/amdgpu: Refine the handshake between guest and server by mailbox

[Felix Kuehling]

On 17-01-24 10:05 AM, Xue, Ken wrote:
>> From: Christian König [mailto:deathsimple at 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.Xue at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx