On 9/9/24 3:15 PM, Antonino Maniscalco wrote:
On 9/6/24 9:54 PM, Akhil P Oommen wrote:
On Thu, Sep 05, 2024 at 04:51:22PM +0200, Antonino Maniscalco wrote:
This patch implements preemption feature for A6xx targets, this allows
the GPU to switch to a higher priority ringbuffer if one is ready. A6XX
hardware as such supports multiple levels of preemption granularities,
ranging from coarse grained(ringbuffer level) to a more fine grained
such as draw-call level or a bin boundary level preemption. This patch
enables the basic preemption level, with more fine grained preemption
support to follow.
Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
Signed-off-by: Antonino Maniscalco <antomani103@xxxxxxxxx>
Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8650-QRD
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 293 +++++++++++++++++++++-
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 161 ++++++++++++
...
we can use the lighter smp variant here.
+
+ if (a6xx_gpu->cur_ring == ring)
+ gpu_write(gpu, REG_A6XX_CP_RB_WPTR, wptr);
+ else
+ ring->skip_inline_wptr = true;
+ } else {
+ ring->skip_inline_wptr = true;
+ }
+
+ spin_unlock_irqrestore(&ring->preempt_lock, flags);
}
static void get_stats_counter(struct msm_ringbuffer *ring, u32
counter,
@@ -138,12 +231,14 @@ static void a6xx_set_pagetable(struct a6xx_gpu
*a6xx_gpu,
set_pagetable checks "cur_ctx_seqno" to see if pt switch is needed or
not. This is currently not tracked separately for each ring. Can you
please check that?
I totally missed that. Thanks for catching it!
I wonder why that didn't cause any gpu errors in testing. Not sure if I
am missing something.
I think this is because, so long as a single context doesn't submit to
two different rings with differenr priorities, we will only be incorrect
in the sense that we emit more page table switches than necessary and
never less. However untrusted userspace could create a context that
submits to two different rings and that would lead to execution in the
wrong context so we must fix this.
/*
* Write the new TTBR0 to the memstore. This is good for
debugging.
+ * Needed for preemption
*/
- OUT_PKT7(ring, CP_MEM_WRITE, 4);
+ OUT_PKT7(ring, CP_MEM_WRITE, 5);
OUT_RING(ring, CP_MEM_WRITE_0_ADDR_LO(lower_32_bits(memptr)));
OUT_RING(ring, CP_MEM_WRITE_1_ADDR_HI(upper_32_bits(memptr)));
OUT_RING(ring, lower_32_bits(ttbr));
- OUT_RING(ring, (asid << 16) | upper_32_bits(ttbr));
+ OUT_RING(ring, upper_32_bits(ttbr));
+ OUT_RING(ring, ctx->seqno);
/*
* Sync both threads after switching pagetables and enable BR only
@@ -268,6 +363,43 @@ static void a6xx_submit(struct msm_gpu *gpu,
struct msm_gem_submit *submit)
a6xx_flush(gpu, ring);
}
...
+ struct a6xx_preempt_record *record_ptr =
+ a6xx_gpu->preempt[ring->id] + PREEMPT_OFFSET_PRIV_NON_SECURE;
+ u64 ttbr0 = ring->memptrs->ttbr0;
+ u32 context_idr = ring->memptrs->context_idr;
+
+ smmu_info_ptr->ttbr0 = ttbr0;
+ smmu_info_ptr->context_idr = context_idr;
+ record_ptr->wptr = get_wptr(ring);
+
+ /*
+ * The GPU will write the wptr we set above when we preempt. Reset
+ * skip_inline_wptr to make sure that we don't write WPTR to the
same
+ * thing twice. It's still possible subsequent submissions will
update
+ * wptr again, in which case they will set the flag to true.
This has
+ * to be protected by the lock for setting the flag and updating
wptr
+ * to be atomic.
+ */
+ ring->skip_inline_wptr = false;
+
+ spin_unlock_irqrestore(&ring->preempt_lock, flags);
+
+ gpu_write64(gpu,
+ REG_A6XX_CP_CONTEXT_SWITCH_SMMU_INFO,
+ a6xx_gpu->preempt_iova[ring->id] + PREEMPT_OFFSET_SMMU_INFO);
+
+ gpu_write64(gpu,
+ REG_A6XX_CP_CONTEXT_SWITCH_PRIV_NON_SECURE_RESTORE_ADDR,
+ a6xx_gpu->preempt_iova[ring->id] +
PREEMPT_OFFSET_PRIV_NON_SECURE);
+
+ preempt_offset_priv_secure =
+
PREEMPT_OFFSET_PRIV_SECURE(adreno_gpu->info->preempt_record_size);
+ gpu_write64(gpu,
+ REG_A6XX_CP_CONTEXT_SWITCH_PRIV_SECURE_RESTORE_ADDR,
+ a6xx_gpu->preempt_iova[ring->id] + preempt_offset_priv_secure);
Secure buffers are not supported currently, so we can skip this and the
context record allocation. Anyway this has to be a separate buffer
mapped in secure pagetable which don't currently have. We can skip the
same in pseudo register packet too.
Mmm it would appear that not setting it causes an hang very early. I'll
see if I can find out more about what is going on.
Actually it was a mistake I had made when testing. The secure record
will be gone from the next revision.
+
+ a6xx_gpu->next_ring = ring;
+
...
struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id,
--
2.46.0
Best regards,
Best regards,
--
Antonino Maniscalco <antomani103@xxxxxxxxx>
