[PATCH] drm/amdgpu: enable 48-bit IH timestamp counter

Christian König christian.koenig at amd.com
Sat Nov 14 08:26:45 UTC 2020


Yes, exactly.

Is the timer guaranteed to monotonous increment? I strongly suspect yes 
and then a simple "if (old > new) ++upper_32_bits;" should be sufficient.

Regards,
Christian.

Am 13.11.20 um 18:15 schrieb Felix Kuehling:
> I'd feel better with wrap-around handling. I think having a system up
> for that long is not likely but not impossible. Having a known hard
> limit on uptime is probably a bad thing. Imagine someone trying to
> reproduce the problem ...
>
> Regards,
>    Felix
>
> Am 2020-11-16 um 6:31 a.m. schrieb Christian König:
>> Feel free to keep my rb for this, but is 455 days enough in general or
>> should we add wrap around handling?
>>
>> Christian.
>>
>> Am 10.11.20 um 18:57 schrieb Sierra Guiza, Alejandro (Alex):
>>> [AMD Public Use]
>>>
>>> I just added support for vega10_ih too.
>>>
>>> Regards,
>>> Alex
>>>
>>>> -----Original Message-----
>>>> From: Sierra Guiza, Alejandro (Alex) <Alex.Sierra at amd.com>
>>>> Sent: Tuesday, November 10, 2020 11:55 AM
>>>> To: amd-gfx at lists.freedesktop.org
>>>> Cc: Koenig, Christian <Christian.Koenig at amd.com>; Kuehling, Felix
>>>> <Felix.Kuehling at amd.com>; Sierra Guiza, Alejandro (Alex)
>>>> <Alex.Sierra at amd.com>
>>>> Subject: [PATCH] drm/amdgpu: enable 48-bit IH timestamp counter
>>>>
>>>> By default this timestamp is based on a 32 bit counter.
>>>> This is used by the amdgpu_gmc_filter_faults, to avoid process the same
>>>> interrupt in retry configuration.
>>>> Apparently there's a problem when the timestamp coming from IH
>>>> overflows
>>>> and compares against timestamp coming from the the hash table.
>>>> This patch only extends the time overflow from 10 minutes to aprx
>>>> 455 days.
>>>>
>>>> Signed-off-by: Alex Sierra <alex.sierra at amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/navi10_ih.c | 6 ++++++
>>>> drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 6 ++++++
>>>>    2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>> b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>> index 837769fcb35b..bda916f33805 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>> @@ -94,6 +94,8 @@ static void navi10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>
>>>>        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RB_ENABLE, 1);
>>>>        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, ENABLE_INTR,
>>>> 1);
>>>> +    ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
>>>> +                   RB_GPU_TS_ENABLE, 1);
>>>>        if (amdgpu_sriov_vf(adev) && adev->asic_type < CHIP_NAVI10) {
>>>>            if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL,
>>>> ih_rb_cntl)) {
>>>>                DRM_ERROR("PSP program IH_RB_CNTL failed!\n");
>>>> @@ -109,6 +111,8 @@ static void navi10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>            ih_rb_cntl = RREG32_SOC15(OSSSYS, 0,
>>>> mmIH_RB_CNTL_RING1);
>>>>            ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
>>>>                           RB_ENABLE, 1);
>>>> +        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
>>>> +                       RB_GPU_TS_ENABLE, 1);
>>>>            if (amdgpu_sriov_vf(adev) && adev->asic_type <
>>>> CHIP_NAVI10) {
>>>>                if (psp_reg_program(&adev->psp,
>>>> PSP_REG_IH_RB_CNTL_RING1,
>>>>                            ih_rb_cntl)) {
>>>> @@ -125,6 +129,8 @@ static void navi10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>            ih_rb_cntl = RREG32_SOC15(OSSSYS, 0,
>>>> mmIH_RB_CNTL_RING2);
>>>>            ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
>>>>                           RB_ENABLE, 1);
>>>> +        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
>>>> +                       RB_GPU_TS_ENABLE, 1);
>>>>            if (amdgpu_sriov_vf(adev) && adev->asic_type <
>>>> CHIP_NAVI10) {
>>>>                if (psp_reg_program(&adev->psp,
>>>> PSP_REG_IH_RB_CNTL_RING2,
>>>>                            ih_rb_cntl)) {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> index 407c6093c2ec..35d68bc5d95e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> @@ -50,6 +50,8 @@ static void vega10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>
>>>>        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RB_ENABLE, 1);
>>>>        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, ENABLE_INTR,
>>>> 1);
>>>> +    ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
>>>> +                   RB_GPU_TS_ENABLE, 1);
>>>>        if (amdgpu_sriov_vf(adev)) {
>>>>            if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL,
>>>> ih_rb_cntl)) {
>>>>                DRM_ERROR("PSP program IH_RB_CNTL failed!\n");
>>>> @@ -64,6 +66,8 @@ static void vega10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>            ih_rb_cntl = RREG32_SOC15(OSSSYS, 0,
>>>> mmIH_RB_CNTL_RING1);
>>>>            ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
>>>>                           RB_ENABLE, 1);
>>>> +        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
>>>> +                       RB_GPU_TS_ENABLE, 1);
>>>>            if (amdgpu_sriov_vf(adev)) {
>>>>                if (psp_reg_program(&adev->psp,
>>>> PSP_REG_IH_RB_CNTL_RING1,
>>>>                            ih_rb_cntl)) {
>>>> @@ -80,6 +84,8 @@ static void vega10_ih_enable_interrupts(struct
>>>> amdgpu_device *adev)
>>>>            ih_rb_cntl = RREG32_SOC15(OSSSYS, 0,
>>>> mmIH_RB_CNTL_RING2);
>>>>            ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
>>>>                           RB_ENABLE, 1);
>>>> +        ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
>>>> +                       RB_GPU_TS_ENABLE, 1);
>>>>            if (amdgpu_sriov_vf(adev)) {
>>>>                if (psp_reg_program(&adev->psp,
>>>> PSP_REG_IH_RB_CNTL_RING2,
>>>>                            ih_rb_cntl)) {
>>>> -- 
>>>> 2.17.1
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C2227acf915064b27b07c08d887e93027%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408782891525552%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=E482HSkR2W3XrGRFNd5%2FbY1vrR5H7DmoAqwMhDfP%2FM0%3D&reserved=0
>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C2227acf915064b27b07c08d887e93027%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408782891535517%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7oJGQTnBArrurCsXNog0RW6rdzZi3ANZOVOAH8UW7i0%3D&reserved=0
>>



More information about the amd-gfx mailing list