<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
The current bare metal registers also get reset with gfxoff and GPU reset.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Alex</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Jiang, Jerry (SW) <Jerry.Jiang@amd.com><br>
<b>Sent:</b> Tuesday, November 5, 2019 7:04 PM<br>
<b>To:</b> Huang, JinHuiEric <JinHuiEric.Huang@amd.com>; Alex Deucher <alexdeucher@gmail.com><br>
<b>Cc:</b> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Subject:</b> RE: [PATCH] drm/amdgpu: change read of GPU clock counter on Vega10 VF</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi Alex,<br>
<br>
Under the SRIOV mode, the L1 policy prevent the write to mmRLC_CAPTURE_GPU_CLOCK_COUNT on VF, so the RLC_GPU_CLOCK_COUNT_LSB/MSB doesn't get updated for each read under SRIOV VF.<br>
<br>
On the other hand, the read from RLC_REFCLOCK_TIMESTAMP_LSB/MSB doesn't rely on any write, so it is available on VF. And of cause it is available for bare metal also. However, the RLC_REFCLOCK_TIMESTAMP_LSB/MSB will reset to zero under two cases: 1) GFXOFF
 2) Reset. Since SRIOV doesn't have GFXOFF, and after reset, the guest will have to restart the app, so there is no problem for SRIOV to switch to this new register set.<br>
<br>
On bare metal, there is concern with these two cases. We didn't get confirmation from Rocm team that the counter reset to zero after  1) GFXOFF 2) Reset will not affect the usage of GPU timestamp, we can't make the switch for bare metal. I would be more than
 happy to switch all over if we can..<br>
<br>
Jerry<br>
<br>
<br>
-----Original Message-----<br>
From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com> <br>
Sent: Tuesday, November 5, 2019 6:48 PM<br>
To: Alex Deucher <alexdeucher@gmail.com><br>
Cc: amd-gfx@lists.freedesktop.org; Jiang, Jerry (SW) <Jerry.Jiang@amd.com><br>
Subject: Re: [PATCH] drm/amdgpu: change read of GPU clock counter on Vega10 VF<br>
<br>
<br>
On 2019-11-05 6:06 p.m., Alex Deucher wrote:<br>
> On Tue, Nov 5, 2019 at 5:26 PM Huang, JinHuiEric <br>
> <JinHuiEric.Huang@amd.com> wrote:<br>
>> Using unified VBIOS has performance drop in sriov environment.<br>
>> The fix is switching to another register instead.<br>
>><br>
>> Signed-off-by: Eric Huang <JinhuiEric.Huang@amd.com><br>
>> ---<br>
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 19 ++++++++++++++++---<br>
>>   1 file changed, 16 insertions(+), 3 deletions(-)<br>
>><br>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c <br>
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
>> index 829d623..e44a3ea 100644<br>
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
>> @@ -3885,9 +3885,22 @@ static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev)<br>
>>          uint64_t clock;<br>
>><br>
>>          mutex_lock(&adev->gfx.gpu_clock_mutex);<br>
>> -       WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);<br>
>> -       clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |<br>
>> -               ((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);<br>
>> +       if (adev->asic_type == CHIP_VEGA10 && amdgpu_sriov_runtime(adev)) {<br>
>> +               uint32_t tmp, lsb, msb, i = 0;<br>
>> +               do {<br>
>> +                       if (i != 0)<br>
>> +                               udelay(1);<br>
>> +                       tmp = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_MSB);<br>
>> +                       lsb = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_LSB);<br>
>> +                       msb = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_MSB);<br>
>> +                       i++;<br>
>> +               } while (unlikely(tmp != msb) && (i < adev->usec_timeout));<br>
>> +               clock = (uint64_t)lsb | ((uint64_t)msb << 32ULL);<br>
>> +       } else {<br>
>> +               WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);<br>
>> +               clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |<br>
>> +                       ((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);<br>
>> +       }<br>
> Is there a reason we can't use the same regs on bare metal and SR-IOV?<br>
>   I'd like to minimize the deltas if possible.<br>
<br>
As Jerry's request, this change will avoid P1 policy protection on RLC_GPU_CLOCK_COUNT_LSB/MSB.<br>
<br>
Eric<br>
<br>
><br>
> Alex<br>
><br>
>>          mutex_unlock(&adev->gfx.gpu_clock_mutex);<br>
>>          return clock;<br>
>>   }<br>
>> --<br>
>> 2.7.4<br>
>><br>
>> _______________________________________________<br>
>> amd-gfx mailing list<br>
>> amd-gfx@lists.freedesktop.org<br>
>> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></div>
</span></font></div>
</body>
</html>