* [PATCH] drm/amdgpu: Add sdma single packet invalidation
@ 2021-04-22 10:49 Victor Zhao
2021-04-22 11:02 ` Christian König
0 siblings, 1 reply; 2+ messages in thread
From: Victor Zhao @ 2021-04-22 10:49 UTC (permalink / raw)
To: amd-gfx, emily.deng; +Cc: Victor Zhao
Add sdma single packet invalidation
Signed-off-by: Victor Zhao <Victor.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 8 ++++----
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ++++
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 14 +++++++++-----
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 12 ++++++++++++
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index c39ed9eb0987..b2cf0e00ce38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -435,13 +435,13 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
/*
* The latest engine allocation on gfx9/10 is:
- * Engine 2, 3: firmware
- * Engine 0, 1, 4~16: amdgpu ring,
+ * Engine 1, 2, 3, 4: firmware
+ * Engine 0, 5~16: amdgpu ring,
* subject to change when ring number changes
* Engine 17: Gart flushes
*/
-#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
-#define MMHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFE1
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFE1
int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index ca1622835296..fb3de16321ff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -192,6 +192,9 @@ struct amdgpu_ring_funcs {
void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
uint32_t reg0, uint32_t reg1,
uint32_t ref, uint32_t mask);
+ void (*vm_invalidate)(struct amdgpu_ring *ring,
+ uint32_t reg0, uint32_t reg1,
+ uint32_t req, uint32_t mask, uint32_t hub);
void (*emit_frame_cntl)(struct amdgpu_ring *ring, bool start,
bool secure);
/* Try to soft recover the ring to make the fence signal */
@@ -270,6 +273,7 @@ struct amdgpu_ring {
#define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
#define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
#define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
+#define amdgpu_ring_vm_invalidate(r, d0, d1, v, m, h) ((r)->funcs->vm_invalidate((r), (d0), (d1), (v), (m), (h)))
#define amdgpu_ring_emit_frame_cntl(r, b, s) (r)->funcs->emit_frame_cntl((r), (b), (s))
#define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
#define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 498b28a35f5b..0fdf60a7c53d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -481,11 +481,15 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
(hub->ctx_addr_distance * vmid),
upper_32_bits(pd_addr));
- amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
- hub->eng_distance * eng,
- hub->vm_inv_eng0_ack +
- hub->eng_distance * eng,
- req, 1 << vmid);
+ if ((!strcmp("sdma0", ring->name)) || (!strcmp("sdma1", ring->name)))
+ amdgpu_ring_vm_invalidate(ring, 0xffffffff, 0x1f,
+ req, 1 << vmid, 1 << ring->funcs->vmhub);
+ else
+ amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
+ hub->eng_distance * eng,
+ hub->vm_inv_eng0_ack +
+ hub->eng_distance * eng,
+ req, 1 << vmid);
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
if (use_semaphore)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 920fc6d4a127..fd7df9194f05 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1219,6 +1219,17 @@ static void sdma_v5_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
amdgpu_ring_emit_reg_wait(ring, reg1, mask, mask);
}
+static void sdma_v5_0_ring_vm_invalidate(struct amdgpu_ring *ring,
+ uint32_t reg0, uint32_t reg1,
+ uint32_t req, uint32_t mask, uint32_t hub)
+{
+ amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_POLL_REGMEM) |
+ SDMA_PKT_POLL_REGMEM_HEADER_SUB_OP(4));
+ amdgpu_ring_write(ring, req);
+ amdgpu_ring_write(ring, reg0);
+ amdgpu_ring_write(ring, ((reg1 & 0x1f) << 16) | (mask & 0xffff) | (hub << 21)); /* reference */
+}
+
static int sdma_v5_0_early_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -1654,6 +1665,7 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
.emit_wreg = sdma_v5_0_ring_emit_wreg,
.emit_reg_wait = sdma_v5_0_ring_emit_reg_wait,
.emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
+ .vm_invalidate = sdma_v5_0_ring_vm_invalidate,
.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
.patch_cond_exec = sdma_v5_0_ring_patch_cond_exec,
.preempt_ib = sdma_v5_0_ring_preempt_ib,
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amdgpu: Add sdma single packet invalidation
2021-04-22 10:49 [PATCH] drm/amdgpu: Add sdma single packet invalidation Victor Zhao
@ 2021-04-22 11:02 ` Christian König
0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2021-04-22 11:02 UTC (permalink / raw)
To: Victor Zhao, amd-gfx, emily.deng
Am 22.04.21 um 12:49 schrieb Victor Zhao:
> Add sdma single packet invalidation
Well NAK on many aspects of this.
>
> Signed-off-by: Victor Zhao <Victor.Zhao@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ++++
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 14 +++++++++-----
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 12 ++++++++++++
> 4 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index c39ed9eb0987..b2cf0e00ce38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -435,13 +435,13 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
>
> /*
> * The latest engine allocation on gfx9/10 is:
> - * Engine 2, 3: firmware
> - * Engine 0, 1, 4~16: amdgpu ring,
> + * Engine 1, 2, 3, 4: firmware
> + * Engine 0, 5~16: amdgpu ring,
> * subject to change when ring number changes
> * Engine 17: Gart flushes
> */
> -#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
> -#define MMHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
> +#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFE1
> +#define MMHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFE1
>
> int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
> {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index ca1622835296..fb3de16321ff 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -192,6 +192,9 @@ struct amdgpu_ring_funcs {
> void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
> uint32_t reg0, uint32_t reg1,
> uint32_t ref, uint32_t mask);
> + void (*vm_invalidate)(struct amdgpu_ring *ring,
> + uint32_t reg0, uint32_t reg1,
> + uint32_t req, uint32_t mask, uint32_t hub);
What is that good for? You need a much wider explanation if you want to
change the design here.
Please rather implement the emit_reg_write_reg_wait() callback properly.
> void (*emit_frame_cntl)(struct amdgpu_ring *ring, bool start,
> bool secure);
> /* Try to soft recover the ring to make the fence signal */
> @@ -270,6 +273,7 @@ struct amdgpu_ring {
> #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
> #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
> #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
> +#define amdgpu_ring_vm_invalidate(r, d0, d1, v, m, h) ((r)->funcs->vm_invalidate((r), (d0), (d1), (v), (m), (h)))
> #define amdgpu_ring_emit_frame_cntl(r, b, s) (r)->funcs->emit_frame_cntl((r), (b), (s))
> #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
> #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 498b28a35f5b..0fdf60a7c53d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -481,11 +481,15 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
> (hub->ctx_addr_distance * vmid),
> upper_32_bits(pd_addr));
>
> - amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
> - hub->eng_distance * eng,
> - hub->vm_inv_eng0_ack +
> - hub->eng_distance * eng,
> - req, 1 << vmid);
> + if ((!strcmp("sdma0", ring->name)) || (!strcmp("sdma1", ring->name)))
Seriously?
> + amdgpu_ring_vm_invalidate(ring, 0xffffffff, 0x1f,
> + req, 1 << vmid, 1 << ring->funcs->vmhub);
> + else
> + amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
> + hub->eng_distance * eng,
> + hub->vm_inv_eng0_ack +
> + hub->eng_distance * eng,
> + req, 1 << vmid);
>
> /* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
> if (use_semaphore)
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 920fc6d4a127..fd7df9194f05 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1219,6 +1219,17 @@ static void sdma_v5_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
> amdgpu_ring_emit_reg_wait(ring, reg1, mask, mask);
> }
>
> +static void sdma_v5_0_ring_vm_invalidate(struct amdgpu_ring *ring,
> + uint32_t reg0, uint32_t reg1,
> + uint32_t req, uint32_t mask, uint32_t hub)
> +{
> + amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_POLL_REGMEM) |
> + SDMA_PKT_POLL_REGMEM_HEADER_SUB_OP(4));
> + amdgpu_ring_write(ring, req);
> + amdgpu_ring_write(ring, reg0);
> + amdgpu_ring_write(ring, ((reg1 & 0x1f) << 16) | (mask & 0xffff) | (hub << 21)); /* reference */
That's not sufficient. That you use the single packet doesn't protects
you from concurrent access of the invalidation control registers.
Christian.
> +}
> +
> static int sdma_v5_0_early_init(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> @@ -1654,6 +1665,7 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
> .emit_wreg = sdma_v5_0_ring_emit_wreg,
> .emit_reg_wait = sdma_v5_0_ring_emit_reg_wait,
> .emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
> + .vm_invalidate = sdma_v5_0_ring_vm_invalidate,
> .init_cond_exec = sdma_v5_0_ring_init_cond_exec,
> .patch_cond_exec = sdma_v5_0_ring_patch_cond_exec,
> .preempt_ib = sdma_v5_0_ring_preempt_ib,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-04-22 11:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 10:49 [PATCH] drm/amdgpu: Add sdma single packet invalidation Victor Zhao
2021-04-22 11:02 ` Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).