AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Felix Kuehling <felix.kuehling@amd.com>
To: christian.koenig@amd.com, "Sierra Guiza,
	Alejandro (Alex)" <Alex.Sierra@amd.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: enable 48-bit IH timestamp counter
Date: Fri, 13 Nov 2020 12:15:05 -0500	[thread overview]
Message-ID: <46ad25b3-be04-8d5a-7134-722020143358@amd.com> (raw)
In-Reply-To: <288f55c4-62a4-0249-7a9c-9dc417caa462@gmail.com>

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@amd.com>
>>> Sent: Tuesday, November 10, 2020 11:55 AM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Koenig, Christian <Christian.Koenig@amd.com>; Kuehling, Felix
>>> <Felix.Kuehling@amd.com>; Sierra Guiza, Alejandro (Alex)
>>> <Alex.Sierra@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@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@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cfelix.kuehling%40amd.com%7C2227acf915064b27b07c08d887e93027%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408782891525552%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=E482HSkR2W3XrGRFNd5%2FbY1vrR5H7DmoAqwMhDfP%2FM0%3D&amp;reserved=0
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cfelix.kuehling%40amd.com%7C2227acf915064b27b07c08d887e93027%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408782891535517%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7oJGQTnBArrurCsXNog0RW6rdzZi3ANZOVOAH8UW7i0%3D&amp;reserved=0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2020-11-13 17:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 17:55 [PATCH] drm/amdgpu: enable 48-bit IH timestamp counter Alex Sierra
2020-11-10 17:57 ` Sierra Guiza, Alejandro (Alex)
2020-11-16 11:31   ` Christian König
2020-11-13 17:11     ` Sierra Guiza, Alejandro (Alex)
2020-11-13 17:15     ` Felix Kuehling [this message]
2020-11-14  8:26       ` Christian König
  -- strict thread matches above, loose matches on Subject: below --
2020-11-10  3:20 Alex Sierra
2020-11-10 11:54 ` Christian König
2020-11-10 18:08 ` philip yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46ad25b3-be04-8d5a-7134-722020143358@amd.com \
    --to=felix.kuehling@amd.com \
    --cc=Alex.Sierra@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox