[PATCH] drm/amdgpu: change read of GPU clock counter on Vega10 VF

[Jiang, Jerry (SW)]

Hi Alex,

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.

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.

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..

Jerry


-----Original Message-----
From: Huang, JinHuiEric <JinHuiEric.Huang at amd.com> 
Sent: Tuesday, November 5, 2019 6:48 PM
To: Alex Deucher <alexdeucher at gmail.com>
Cc: amd-gfx at lists.freedesktop.org; Jiang, Jerry (SW) <Jerry.Jiang at amd.com>
Subject: Re: [PATCH] drm/amdgpu: change read of GPU clock counter on Vega10 VF


On 2019-11-05 6:06 p.m., Alex Deucher wrote:
> On Tue, Nov 5, 2019 at 5:26 PM Huang, JinHuiEric 
> <JinHuiEric.Huang at amd.com> wrote:
>> Using unified VBIOS has performance drop in sriov environment.
>> The fix is switching to another register instead.
>>
>> Signed-off-by: Eric Huang <JinhuiEric.Huang at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 19 ++++++++++++++++---
>>   1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 829d623..e44a3ea 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -3885,9 +3885,22 @@ static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev)
>>          uint64_t clock;
>>
>>          mutex_lock(&adev->gfx.gpu_clock_mutex);
>> -       WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);
>> -       clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |
>> -               ((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);
>> +       if (adev->asic_type == CHIP_VEGA10 && amdgpu_sriov_runtime(adev)) {
>> +               uint32_t tmp, lsb, msb, i = 0;
>> +               do {
>> +                       if (i != 0)
>> +                               udelay(1);
>> +                       tmp = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_MSB);
>> +                       lsb = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_LSB);
>> +                       msb = RREG32_SOC15(GC, 0, mmRLC_REFCLOCK_TIMESTAMP_MSB);
>> +                       i++;
>> +               } while (unlikely(tmp != msb) && (i < adev->usec_timeout));
>> +               clock = (uint64_t)lsb | ((uint64_t)msb << 32ULL);
>> +       } else {
>> +               WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);
>> +               clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |
>> +                       ((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);
>> +       }
> Is there a reason we can't use the same regs on bare metal and SR-IOV?
>   I'd like to minimize the deltas if possible.

As Jerry's request, this change will avoid P1 policy protection on RLC_GPU_CLOCK_COUNT_LSB/MSB.

Eric

>
> Alex
>
>>          mutex_unlock(&adev->gfx.gpu_clock_mutex);
>>          return clock;
>>   }
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx