* [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
@ 2024-06-14 11:10 Jane Jian
2024-06-14 11:50 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Jane Jian @ 2024-06-14 11:10 UTC (permalink / raw)
To: Lijo.Lazar, Haijun.Chang, Victor.Zhao; +Cc: amd-gfx, Jane Jian
[WHY]
sriov has the higher bit violation when flushing tlb
[HOW]
for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher bit violation
since kiq ring is always local, local address without XCC ID is enough to be sent to the XCC KIQ
Signed-off-by: Jane Jian <Jane.Jian@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
index e14acab5cceb..4e38a66a52f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
@@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct amdgpu_device *adev, uint32_t xcc_mask)
{
struct amdgpu_vmhub *hub;
int i;
+ uint32_t gc_index;
for_each_inst(i, xcc_mask) {
hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
+ /* for sriov only init XCC0(lower 16-bit) to avoid higher bit violation */
+ if (amdgpu_sriov_vf(adev))
+ gc_index = 0;
+ else
+ gc_index = GET_INST(GC, i);
+
hub->ctx0_ptb_addr_lo32 =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i),
+ SOC15_REG_OFFSET(GC, gc_index,
regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
hub->ctx0_ptb_addr_hi32 =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i),
+ SOC15_REG_OFFSET(GC, gc_index,
regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
hub->vm_inv_eng0_sem =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_SEM);
+ SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_SEM);
hub->vm_inv_eng0_req =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_REQ);
+ SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_REQ);
hub->vm_inv_eng0_ack =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_ACK);
+ SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_ACK);
hub->vm_context0_cntl =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_CONTEXT0_CNTL);
+ SOC15_REG_OFFSET(GC, gc_index, regVM_CONTEXT0_CNTL);
hub->vm_l2_pro_fault_status =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i),
+ SOC15_REG_OFFSET(GC, gc_index,
regVM_L2_PROTECTION_FAULT_STATUS);
hub->vm_l2_pro_fault_cntl =
- SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_L2_PROTECTION_FAULT_CNTL);
+ SOC15_REG_OFFSET(GC, gc_index, regVM_L2_PROTECTION_FAULT_CNTL);
hub->ctx_distance = regVM_CONTEXT1_CNTL -
regVM_CONTEXT0_CNTL;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
2024-06-14 11:10 [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov Jane Jian
@ 2024-06-14 11:50 ` Lazar, Lijo
2024-06-15 2:08 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2024-06-14 11:50 UTC (permalink / raw)
To: Jane Jian, Haijun.Chang, Victor.Zhao; +Cc: amd-gfx
On 6/14/2024 4:40 PM, Jane Jian wrote:
> [WHY]
> sriov has the higher bit violation when flushing tlb
>
> [HOW]
> for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher bit violation
> since kiq ring is always local, local address without XCC ID is enough to be sent to the XCC KIQ
>
The description is incorrect.
Bits 18:20 represent xcc id. To guarantee all paths pass a local
address, you should just strip bits 18:20 in kiq/rlcg read/write
functions rather than here.
Thanks,
Lijo
> Signed-off-by: Jane Jian <Jane.Jian@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> index e14acab5cceb..4e38a66a52f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> @@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct amdgpu_device *adev, uint32_t xcc_mask)
> {
> struct amdgpu_vmhub *hub;
> int i;
> + uint32_t gc_index;
>
> for_each_inst(i, xcc_mask) {
> hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
>
> + /* for sriov only init XCC0(lower 16-bit) to avoid higher bit violation */
> + if (amdgpu_sriov_vf(adev))
> + gc_index = 0;
> + else
> + gc_index = GET_INST(GC, i);
> +
> hub->ctx0_ptb_addr_lo32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
> hub->ctx0_ptb_addr_hi32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
> hub->vm_inv_eng0_sem =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_SEM);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_SEM);
> hub->vm_inv_eng0_req =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_REQ);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_REQ);
> hub->vm_inv_eng0_ack =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_ACK);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_ACK);
> hub->vm_context0_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_CONTEXT0_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_CONTEXT0_CNTL);
> hub->vm_l2_pro_fault_status =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_L2_PROTECTION_FAULT_STATUS);
> hub->vm_l2_pro_fault_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_L2_PROTECTION_FAULT_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_L2_PROTECTION_FAULT_CNTL);
>
> hub->ctx_distance = regVM_CONTEXT1_CNTL -
> regVM_CONTEXT0_CNTL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
2024-06-14 11:50 ` Lazar, Lijo
@ 2024-06-15 2:08 ` Lazar, Lijo
2024-06-17 3:28 ` Chang, HaiJun
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2024-06-15 2:08 UTC (permalink / raw)
To: Jian, Jane, Chang, HaiJun, Zhao, Victor; +Cc: amd-gfx@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 3987 bytes --]
[AMD Official Use Only - AMD Internal Distribution Only]
Never mind, bit 16 and above is probably because of dword aligned offset.
Any reason not to do this in kiq/rlc based writes to normalise all?
Thanks,
Lijo
________________________________
From: Lazar, Lijo
Sent: Friday, June 14, 2024 5:20:30 PM
To: Jian, Jane <Jane.Jian@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
On 6/14/2024 4:40 PM, Jane Jian wrote:
> [WHY]
> sriov has the higher bit violation when flushing tlb
>
> [HOW]
> for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher bit violation
> since kiq ring is always local, local address without XCC ID is enough to be sent to the XCC KIQ
>
The description is incorrect.
Bits 18:20 represent xcc id. To guarantee all paths pass a local
address, you should just strip bits 18:20 in kiq/rlcg read/write
functions rather than here.
Thanks,
Lijo
> Signed-off-by: Jane Jian <Jane.Jian@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> index e14acab5cceb..4e38a66a52f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> @@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct amdgpu_device *adev, uint32_t xcc_mask)
> {
> struct amdgpu_vmhub *hub;
> int i;
> + uint32_t gc_index;
>
> for_each_inst(i, xcc_mask) {
> hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
>
> + /* for sriov only init XCC0(lower 16-bit) to avoid higher bit violation */
> + if (amdgpu_sriov_vf(adev))
> + gc_index = 0;
> + else
> + gc_index = GET_INST(GC, i);
> +
> hub->ctx0_ptb_addr_lo32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
> hub->ctx0_ptb_addr_hi32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
> hub->vm_inv_eng0_sem =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_SEM);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_SEM);
> hub->vm_inv_eng0_req =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_REQ);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_REQ);
> hub->vm_inv_eng0_ack =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_ACK);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_ACK);
> hub->vm_context0_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_CONTEXT0_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_CONTEXT0_CNTL);
> hub->vm_l2_pro_fault_status =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_L2_PROTECTION_FAULT_STATUS);
> hub->vm_l2_pro_fault_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_L2_PROTECTION_FAULT_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_L2_PROTECTION_FAULT_CNTL);
>
> hub->ctx_distance = regVM_CONTEXT1_CNTL -
> regVM_CONTEXT0_CNTL;
[-- Attachment #2: Type: text/html, Size: 8873 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
2024-06-15 2:08 ` Lazar, Lijo
@ 2024-06-17 3:28 ` Chang, HaiJun
2024-06-17 3:35 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Chang, HaiJun @ 2024-06-17 3:28 UTC (permalink / raw)
To: Lazar, Lijo, Jian, Jane, Zhao, Victor; +Cc: amd-gfx@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 5155 bytes --]
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Lijo,
Right, 18bits are byte aligned range of local XCC register, 16bites are dword aligned offset range
We find the normalization needs to be applied to many functions, like
* KIQ: amdgpu_kiq_r/wreg/
* RLC: amdgpu_virt_rlcg_reg_rw
* KIQ: amdgpu_gmc_fw_reg_write_reg_wait
* KIQ: amdgpu_ring_emit_reg_write_reg_wait/amdgpu_ring_emit_reg_wait/amdgpu_ring_emit_wreg
For sriov gfx register access, it only has 2 ways: rlc or kiq. Both of the ways can use local xcc offset, so we think it's simpler change to init the gfx register offsets with local xcc offset only.
Thanks,
HaiJun
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Saturday, June 15, 2024 10:09 AM
To: Jian, Jane <Jane.Jian@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
[AMD Official Use Only - AMD Internal Distribution Only]
Never mind, bit 16 and above is probably because of dword aligned offset.
Any reason not to do this in kiq/rlc based writes to normalise all?
Thanks,
Lijo
________________________________
From: Lazar, Lijo
Sent: Friday, June 14, 2024 5:20:30 PM
To: Jian, Jane <Jane.Jian@amd.com<mailto:Jane.Jian@amd.com>>; Chang, HaiJun <HaiJun.Chang@amd.com<mailto:HaiJun.Chang@amd.com>>; Zhao, Victor <Victor.Zhao@amd.com<mailto:Victor.Zhao@amd.com>>
Cc: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
On 6/14/2024 4:40 PM, Jane Jian wrote:
> [WHY]
> sriov has the higher bit violation when flushing tlb
>
> [HOW]
> for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher bit violation
> since kiq ring is always local, local address without XCC ID is enough to be sent to the XCC KIQ
>
The description is incorrect.
Bits 18:20 represent xcc id. To guarantee all paths pass a local
address, you should just strip bits 18:20 in kiq/rlcg read/write
functions rather than here.
Thanks,
Lijo
> Signed-off-by: Jane Jian <Jane.Jian@amd.com<mailto:Jane.Jian@amd.com>>
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> index e14acab5cceb..4e38a66a52f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
> @@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct amdgpu_device *adev, uint32_t xcc_mask)
> {
> struct amdgpu_vmhub *hub;
> int i;
> + uint32_t gc_index;
>
> for_each_inst(i, xcc_mask) {
> hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
>
> + /* for sriov only init XCC0(lower 16-bit) to avoid higher bit violation */
> + if (amdgpu_sriov_vf(adev))
> + gc_index = 0;
> + else
> + gc_index = GET_INST(GC, i);
> +
> hub->ctx0_ptb_addr_lo32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
> hub->ctx0_ptb_addr_hi32 =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
> hub->vm_inv_eng0_sem =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_SEM);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_SEM);
> hub->vm_inv_eng0_req =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_REQ);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_REQ);
> hub->vm_inv_eng0_ack =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_ACK);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_ACK);
> hub->vm_context0_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_CONTEXT0_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_CONTEXT0_CNTL);
> hub->vm_l2_pro_fault_status =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
> + SOC15_REG_OFFSET(GC, gc_index,
> regVM_L2_PROTECTION_FAULT_STATUS);
> hub->vm_l2_pro_fault_cntl =
> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_L2_PROTECTION_FAULT_CNTL);
> + SOC15_REG_OFFSET(GC, gc_index, regVM_L2_PROTECTION_FAULT_CNTL);
>
> hub->ctx_distance = regVM_CONTEXT1_CNTL -
> regVM_CONTEXT0_CNTL;
[-- Attachment #2: Type: text/html, Size: 16467 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
2024-06-17 3:28 ` Chang, HaiJun
@ 2024-06-17 3:35 ` Lazar, Lijo
2024-06-17 4:56 ` Chang, HaiJun
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2024-06-17 3:35 UTC (permalink / raw)
To: Chang, HaiJun, Jian, Jane, Zhao, Victor; +Cc: amd-gfx@lists.freedesktop.org
On 6/17/2024 8:58 AM, Chang, HaiJun wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
> Hi Lijo,
>
>
>
> Right, 18bits are byte aligned range of local XCC register, 16bites are
> dword aligned offset range
>
>
>
> We find the normalization needs to be applied to many functions, like
>
> * KIQ: amdgpu_kiq_r/wreg/
> * RLC: amdgpu_virt_rlcg_reg_rw
> * KIQ: amdgpu_gmc_fw_reg_write_reg_wait
> * KIQ:
> amdgpu_ring_emit_reg_write_reg_wait/amdgpu_ring_emit_reg_wait/amdgpu_ring_emit_wreg
>
>
>
> For sriov gfx register access, it only has 2 ways: rlc or kiq. Both of
> the ways can use local xcc offset, so we think it’s simpler change to
> init the gfx register offsets with local xcc offset only.
>
Ok, is this the only place? What about other calls in gfx_v9_4_3 like
WREG32_SOC15_RLC/WREG32_SOC15 etc.?
Thanks,
Lijo
>
>
> Thanks,
>
> HaiJun
>
>
>
> *From:*Lazar, Lijo <Lijo.Lazar@amd.com>
> *Sent:* Saturday, June 15, 2024 10:09 AM
> *To:* Jian, Jane <Jane.Jian@amd.com>; Chang, HaiJun
> <HaiJun.Chang@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>
> *Cc:* amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
>
>
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
>
> Never mind, bit 16 and above is probably because of dword aligned offset.
>
>
>
> Any reason not to do this in kiq/rlc based writes to normalise all?
>
>
>
> Thanks,
>
> Lijo
>
> ------------------------------------------------------------------------
>
> *From:*Lazar, Lijo
> *Sent:* Friday, June 14, 2024 5:20:30 PM
> *To:* Jian, Jane <Jane.Jian@amd.com <mailto:Jane.Jian@amd.com>>; Chang,
> HaiJun <HaiJun.Chang@amd.com <mailto:HaiJun.Chang@amd.com>>; Zhao,
> Victor <Victor.Zhao@amd.com <mailto:Victor.Zhao@amd.com>>
> *Cc:* amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>>
> *Subject:* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
>
>
>
>
>
> On 6/14/2024 4:40 PM, Jane Jian wrote:
>> [WHY]
>> sriov has the higher bit violation when flushing tlb
>>
>> [HOW]
>> for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher bit violation
>> since kiq ring is always local, local address without XCC ID is enough to be sent to the XCC KIQ
>>
>
> The description is incorrect.
>
> Bits 18:20 represent xcc id. To guarantee all paths pass a local
> address, you should just strip bits 18:20 in kiq/rlcg read/write
> functions rather than here.
>
> Thanks,
> Lijo
>
>> Signed-off-by: Jane Jian <Jane.Jian@amd.com <mailto:Jane.Jian@amd.com>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23 +++++++++++++++--------
>> 1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> index e14acab5cceb..4e38a66a52f4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> @@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct amdgpu_device *adev, uint32_t xcc_mask)
>> {
>> struct amdgpu_vmhub *hub;
>> int i;
>> + uint32_t gc_index;
>>
>> for_each_inst(i, xcc_mask) {
>> hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
>>
>> + /* for sriov only init XCC0(lower 16-bit) to avoid higher bit violation */
>> + if (amdgpu_sriov_vf(adev))
>> + gc_index = 0;
>> + else
>> + gc_index = GET_INST(GC, i);
>> +
>> hub->ctx0_ptb_addr_lo32 =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
>> hub->ctx0_ptb_addr_hi32 =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>> regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
>> hub->vm_inv_eng0_sem =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_SEM);
>> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_SEM);
>> hub->vm_inv_eng0_req =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_REQ);
>> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_REQ);
>> hub->vm_inv_eng0_ack =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_INVALIDATE_ENG0_ACK);
>> + SOC15_REG_OFFSET(GC, gc_index, regVM_INVALIDATE_ENG0_ACK);
>> hub->vm_context0_cntl =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_CONTEXT0_CNTL);
>> + SOC15_REG_OFFSET(GC, gc_index, regVM_CONTEXT0_CNTL);
>> hub->vm_l2_pro_fault_status =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>> regVM_L2_PROTECTION_FAULT_STATUS);
>> hub->vm_l2_pro_fault_cntl =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i), regVM_L2_PROTECTION_FAULT_CNTL);
>> + SOC15_REG_OFFSET(GC, gc_index, regVM_L2_PROTECTION_FAULT_CNTL);
>>
>> hub->ctx_distance = regVM_CONTEXT1_CNTL -
>> regVM_CONTEXT0_CNTL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
2024-06-17 3:35 ` Lazar, Lijo
@ 2024-06-17 4:56 ` Chang, HaiJun
0 siblings, 0 replies; 6+ messages in thread
From: Chang, HaiJun @ 2024-06-17 4:56 UTC (permalink / raw)
To: Lazar, Lijo, Jian, Jane, Zhao, Victor; +Cc: amd-gfx@lists.freedesktop.org
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Lijo,
Yes, you're right. The WREG32_SOC15_RLC/WREG32_SOC15 implementation are not correct. Actually even didn't select right xcc_id for KIQ access.
In amdgpu_sriov_wreg/rreg function, we should add normalization handling for GC_HWIP access. It can cover both amdgpu_kiq_r/wreg/ and amdgpu_virt_rlcg_reg_rw cases.
For amdgpu_gmc_fw_reg_write_reg_wait case, do you think should we add a GC/MMHUB flag as parameter and do normalization for GC in this function, or do normalization on caller function gmc_v9_0_flush_gpu_tlb.
Thanks,
HaiJun
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Monday, June 17, 2024 11:35 AM
To: Chang, HaiJun <HaiJun.Chang@amd.com>; Jian, Jane <Jane.Jian@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov
On 6/17/2024 8:58 AM, Chang, HaiJun wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
> Hi Lijo,
>
>
>
> Right, 18bits are byte aligned range of local XCC register, 16bites
> are dword aligned offset range
>
>
>
> We find the normalization needs to be applied to many functions, like
>
> * KIQ: amdgpu_kiq_r/wreg/
> * RLC: amdgpu_virt_rlcg_reg_rw
> * KIQ: amdgpu_gmc_fw_reg_write_reg_wait
> * KIQ:
>
> amdgpu_ring_emit_reg_write_reg_wait/amdgpu_ring_emit_reg_wait/amdgpu_r
> ing_emit_wreg
>
>
>
> For sriov gfx register access, it only has 2 ways: rlc or kiq. Both
> of the ways can use local xcc offset, so we think it’s simpler change
> to init the gfx register offsets with local xcc offset only.
>
Ok, is this the only place? What about other calls in gfx_v9_4_3 like
WREG32_SOC15_RLC/WREG32_SOC15 etc.?
Thanks,
Lijo
>
>
> Thanks,
>
> HaiJun
>
>
>
> *From:*Lazar, Lijo <Lijo.Lazar@amd.com>
> *Sent:* Saturday, June 15, 2024 10:09 AM
> *To:* Jian, Jane <Jane.Jian@amd.com>; Chang, HaiJun
> <HaiJun.Chang@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>
> *Cc:* amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under
> sriov
>
>
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
>
> Never mind, bit 16 and above is probably because of dword aligned offset.
>
>
>
> Any reason not to do this in kiq/rlc based writes to normalise all?
>
>
>
> Thanks,
>
> Lijo
>
> ----------------------------------------------------------------------
> --
>
> *From:*Lazar, Lijo
> *Sent:* Friday, June 14, 2024 5:20:30 PM
> *To:* Jian, Jane <Jane.Jian@amd.com <mailto:Jane.Jian@amd.com>>;
> Chang, HaiJun <HaiJun.Chang@amd.com <mailto:HaiJun.Chang@amd.com>>;
> Zhao, Victor <Victor.Zhao@amd.com <mailto:Victor.Zhao@amd.com>>
> *Cc:* amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>>
> *Subject:* Re: [PATCH] drm/amdgpu: keep init xcc0 for all xccs under
> sriov
>
>
>
>
>
> On 6/14/2024 4:40 PM, Jane Jian wrote:
>> [WHY]
>> sriov has the higher bit violation when flushing tlb
>>
>> [HOW]
>> for sriov only init XCC0(lower 16-bit) for all XCCs to avoid higher
>> bit violation since kiq ring is always local, local address without
>> XCC ID is enough to be sent to the XCC KIQ
>>
>
> The description is incorrect.
>
> Bits 18:20 represent xcc id. To guarantee all paths pass a local
> address, you should just strip bits 18:20 in kiq/rlcg read/write
> functions rather than here.
>
> Thanks,
> Lijo
>
>> Signed-off-by: Jane Jian <Jane.Jian@amd.com
>><mailto:Jane.Jian@amd.com>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 23
>>+++++++++++++++--------
>> 1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> index e14acab5cceb..4e38a66a52f4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
>> @@ -537,29 +537,36 @@ static void gfxhub_v1_2_xcc_init(struct
>>amdgpu_device *adev, uint32_t xcc_mask)
>> {
>> struct amdgpu_vmhub *hub;
>> int i;
>> + uint32_t gc_index;
>>
>> for_each_inst(i, xcc_mask) {
>> hub = &adev->vmhub[AMDGPU_GFXHUB(i)];
>>
>> + /* for sriov only init XCC0(lower 16-bit) to avoid
>> +higher bit violation */
>> + if (amdgpu_sriov_vf(adev))
>> + gc_index = 0;
>> + else
>> + gc_index = GET_INST(GC, i);
>> +
>> hub->ctx0_ptb_addr_lo32 =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>>
>>regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32);
>> hub->ctx0_ptb_addr_hi32 =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>>
>>regVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32);
>> hub->vm_inv_eng0_sem =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>>regVM_INVALIDATE_ENG0_SEM);
>> + SOC15_REG_OFFSET(GC, gc_index,
>> +regVM_INVALIDATE_ENG0_SEM);
>> hub->vm_inv_eng0_req =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>>regVM_INVALIDATE_ENG0_REQ);
>> + SOC15_REG_OFFSET(GC, gc_index,
>> +regVM_INVALIDATE_ENG0_REQ);
>> hub->vm_inv_eng0_ack =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>>regVM_INVALIDATE_ENG0_ACK);
>> + SOC15_REG_OFFSET(GC, gc_index,
>> +regVM_INVALIDATE_ENG0_ACK);
>> hub->vm_context0_cntl =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>>regVM_CONTEXT0_CNTL);
>> + SOC15_REG_OFFSET(GC, gc_index,
>> +regVM_CONTEXT0_CNTL);
>> hub->vm_l2_pro_fault_status =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>> + SOC15_REG_OFFSET(GC, gc_index,
>> regVM_L2_PROTECTION_FAULT_STATUS);
>> hub->vm_l2_pro_fault_cntl =
>> - SOC15_REG_OFFSET(GC, GET_INST(GC, i),
>>regVM_L2_PROTECTION_FAULT_CNTL);
>> + SOC15_REG_OFFSET(GC, gc_index,
>> +regVM_L2_PROTECTION_FAULT_CNTL);
>>
>> hub->ctx_distance = regVM_CONTEXT1_CNTL -
>> regVM_CONTEXT0_CNTL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-17 4:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-14 11:10 [PATCH] drm/amdgpu: keep init xcc0 for all xccs under sriov Jane Jian
2024-06-14 11:50 ` Lazar, Lijo
2024-06-15 2:08 ` Lazar, Lijo
2024-06-17 3:28 ` Chang, HaiJun
2024-06-17 3:35 ` Lazar, Lijo
2024-06-17 4:56 ` Chang, HaiJun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox