[AMD Public Use]
OK.
I will just skip the function call for SRIOV and resend.
Rohit
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: June 7, 2021 12:42 PM
To: Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Khaire, Rohit <Rohit.Khaire@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Deng, Emily <Emily.Deng@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen, Horace <Horace.Chen@xxxxxxx>
Cc: Ming, Davis <Davis.Ming@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Use PSP to program IH_RB_CNTL_RING1/2 on SRIOV
Ah, good point. In this case we should probably rather save than sorry.
Then I suggest to clean up this patch, repeating the psp_reg_program() and error message is pretty horrible coding style.
Christian.
Am 07.06.21 um 18:36 schrieb Felix Kuehling:
> With SRIOV, the interrupt routing is setup by the hypervisor driver.
> We need the secondary IH rings in case the hypervisor enabled
> rerouting of page fault interrupts. I'm not sure what the hypervisor driver does today.
>
> Regards,
> Felix
>
>
> Am 2021-06-07 um 12:29 p.m. schrieb Christian König:
>> That's a workaround for bare metal and as far as I know doesn't apply
>> to SRIOV.
>>
>> We only need the additional IH rings for page fault handling or log
>> handling and as far as I know that is incompatible with SRIOV for the
>> moment. But Felix might have some more updates on this.
>>
>> So as long as we don't support that under SRIOV we don't need this
>> patch either.
>>
>> Christian.
>>
>> Am 07.06.21 um 17:59 schrieb Khaire, Rohit:
>>> [AMD Public Use]
>>>
>>> The hash is 5ea6f9c
>>>
>>> Rohit
>>>
>>> -----Original Message-----
>>> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
>>> Sent: June 7, 2021 11:58 AM
>>> To: Khaire, Rohit <Rohit.Khaire@xxxxxxx>;
>>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
>>> <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>;
>>> Deng, Emily <Emily.Deng@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>;
>>> Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen, Horace
>>> <Horace.Chen@xxxxxxx>
>>> Cc: Ming, Davis <Davis.Ming@xxxxxxx>
>>> Subject: Re: [PATCH] drm/amdgpu: Use PSP to program
>>> IH_RB_CNTL_RING1/2 on SRIOV
>>>
>>> Do you have the hash for this commit?
>>>
>>> Thanks,
>>> Christian.
>>>
>>> Am 07.06.21 um 17:30 schrieb Khaire, Rohit:
>>>> [AMD Public Use]
>>>>
>>>> We don't need RING1 and RING2 functionality for SRIOV afaik.
>>>>
>>>> But looking at the description of the original commit message it
>>>> affects RING0 too?
>>>>
>>>> " drm/amdgpu: add timeout flush mechanism to update wptr for self
>>>> interrupt (v2)
>>>>
>>>> outstanding log reaches threshold will trigger IH ring1/2's wptr
>>>> reported, that will avoid generating interrupts to ring0 too frequent.
>>>> But if ring1/2's wptr hasn't been increased for a long time, the
>>>> outstanding log can't reach threshold so that driver can't get
>>>> latest wptr info and miss some interrupts."
>>>>
>>>> Rohit
>>>>
>>>> -----Original Message-----
>>>> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
>>>> Sent: June 7, 2021 10:31 AM
>>>> To: Khaire, Rohit <Rohit.Khaire@xxxxxxx>;
>>>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
>>>> <Alexander.Deucher@xxxxxxx>; Zhang, Hawking
>>>> <Hawking.Zhang@xxxxxxx>; Deng, Emily <Emily.Deng@xxxxxxx>; Liu,
>>>> Monk <Monk.Liu@xxxxxxx>; Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen,
>>>> Horace <Horace.Chen@xxxxxxx>
>>>> Cc: Ming, Davis <Davis.Ming@xxxxxxx>
>>>> Subject: Re: [PATCH] drm/amdgpu: Use PSP to program
>>>> IH_RB_CNTL_RING1/2 on SRIOV
>>>>
>>>> Why are the ring 1&2 enabled on SRIOV in the first place?
>>>>
>>>> Christian.
>>>>
>>>> Am 07.06.21 um 16:23 schrieb Rohit Khaire:
>>>>> This is similar to IH_RB_CNTL programming in
>>>>> navi10_ih_toggle_ring_interrupts
>>>>>
>>>>> Signed-off-by: Rohit Khaire <rohit.khaire@xxxxxxx>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/navi10_ih.c | 20
>>>>> ++++++++++++++++++--
>>>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>>> index eac564e8dd52..e41188c04846 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
>>>>> @@ -120,11 +120,27 @@ force_update_wptr_for_self_int(struct
>>>>> amdgpu_device *adev,
>>>>> ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
>>>>> RB_USED_INT_THRESHOLD, threshold);
>>>>> - WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
>>>>> + if (amdgpu_sriov_vf(adev) &&
>>>>> amdgpu_sriov_reg_indirect_ih(adev)) {
>>>>> + if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL_RING1,
>>>>> ih_rb_cntl)) {
>>>>> + DRM_ERROR("PSP program IH_RB_CNTL_RING1 failed!\n");
>>>>> + return;
>>>>> + }
>>>>> + } else {
>>>>> + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
>>>>> + }
>>>>> +
>>>>> 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_USED_INT_THRESHOLD, threshold);
>>>>> - WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
>>>>> + if (amdgpu_sriov_vf(adev) &&
>>>>> amdgpu_sriov_reg_indirect_ih(adev)) {
>>>>> + if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL_RING2,
>>>>> ih_rb_cntl)) {
>>>>> + DRM_ERROR("PSP program IH_RB_CNTL_RING2 failed!\n");
>>>>> + return;
>>>>> + }
>>>>> + } else {
>>>>> + WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
>>>>> + }
>>>>> +
>>>>> WREG32_SOC15(OSSSYS, 0, mmIH_CNTL2, ih_cntl);
>>>>> }
>>>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
