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

[Xue, Ken]

Regards,
Ken


> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf Of
> Felix Kuehling
> Sent: Wednesday, January 25, 2017 3:05 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Refine the handshake between guest and
> server by mailbox
> 
> 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.

Sure. 
I will send a new patch. 
And I think I also need to replace the rest "msleep" in mxgpu_vi.c for predictable timeout.

Thanks