Re: [PATCH 1/2] drm/amdgpu: add kiq version interface for RREG32

From: "Christian König" <christian.koenig@amd.com>
To: Huang Rui <ray.huang@amd.com>, chen gong <curry.gong@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
	Felix Kuehling <Felix.Kuehling@amd.com>,
	amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: add kiq version interface for RREG32
Date: Mon, 13 Jan 2020 22:07:23 +0100	[thread overview]
Message-ID: <1eaed3a3-a4eb-8903-e4ee-6b89c70208a4@amd.com> (raw)
In-Reply-To: <20200114042005.GA13044@jenkins-Celadon-RN>

Am 14.01.20 um 05:20 schrieb Huang Rui:
> On Mon, Jan 13, 2020 at 11:56:46PM +0800, Huang Rui wrote:
>> On Mon, Jan 13, 2020 at 06:16:21PM +0800, chen gong wrote:
>>> Reading some registers by mmio will result in hang when GPU is in
>>> "gfxoff" state.
>>>
>>> This problem can be solved by GPU in "ring command packages" way.
>>>
>>> Signed-off-by: chen gong <curry.gong@amd.com>
>> Acked-by: Huang Rui <ray.huang@amd.com>
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 63eab0c..92c5ee4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1016,10 +1016,13 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>>>   
>>>   #define AMDGPU_REGS_IDX       (1<<0)
>>>   #define AMDGPU_REGS_NO_KIQ    (1<<1)
>>> +#define AMDGPU_REGS_KIQ       (1<<2)
>>>   
>>>   #define RREG32_NO_KIQ(reg) amdgpu_mm_rreg(adev, (reg), AMDGPU_REGS_NO_KIQ)
>>>   #define WREG32_NO_KIQ(reg, v) amdgpu_mm_wreg(adev, (reg), (v), AMDGPU_REGS_NO_KIQ)
>>>   
>>> +#define RREG32_KIQ(reg) amdgpu_mm_rreg(adev, (reg), AMDGPU_REGS_KIQ)
>>> +
>>>   #define RREG8(reg) amdgpu_mm_rreg8(adev, (reg))
>>>   #define WREG8(reg, v) amdgpu_mm_wreg8(adev, (reg), (v))
>>>   
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 2c64d2a..4045b56 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -217,7 +217,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
>>>   {
>>>   	uint32_t ret;
>>>   
>>> -	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
>>> +	if ((acc_flags & AMDGPU_REGS_KIQ) || (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev)))
>>>   		return amdgpu_virt_kiq_rreg(adev, reg);
> Hi Curry,
>
> I read this patch again, we would better move the
> amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg out of amdgpu_virt to amdgpu.h.
>
> Because these functions are not only used for virtualization but also for
> the case while gfxoff enabled. We will continue using this interface
> instead of MMIO access in KFD driver as well.

Yes, that is a really good idea.

We should probably start a amdgpu_kiq.c file and move quite a bunch of 
stuff over there.

Regards,
Christian.

>
> Thanks,
> Ray
>
>>>   
>>>   	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
>>> -- 
>>> 2.7.4
>>>
>>> _______________________________________________
>>> 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=02%7C01%7Cray.huang%40amd.com%7C30ace3a1329f439cb69508d79811d5aa%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637145074676514100&amp;sdata=t6rAUcThU9cmcu5Rwh%2BuHFDtj7Yie4tRN%2FgF2OSBZ00%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx