From 6cdcc65fdb0bc59bfca75d0b6fdc54d6ca347ddf Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 11 Aug 2023 03:06:25 +0200 Subject: drm/nouveau: sched: avoid job races between entities If a sched job depends on a dma-fence from a job from the same GPU scheduler instance, but a different scheduler entity, the GPU scheduler does only wait for the particular job to be scheduled, rather than for the job to fully complete. This is due to the GPU scheduler assuming that there is a scheduler instance per ring. However, the current implementation, in order to avoid arbitrary amounts of kthreads, has a single scheduler instance while scheduler entities represent rings. As a workaround, set the DRM_SCHED_FENCE_DONT_PIPELINE for all out-fences in order to force the scheduler to wait for full job completion for dependent jobs from different entities and same scheduler instance. There is some work in progress [1] to address the issues of firmware schedulers; once it is in-tree the scheduler topology in Nouveau should be re-worked accordingly. [1] https://lore.kernel.org/dri-devel/20230801205103.627779-1-matthew.brost@intel.com/ Signed-off-by: Danilo Krummrich Reviewed-by: Faith Ekstrand Link: https://patchwork.freedesktop.org/patch/msgid/20230811010632.2473-1-dakr@redhat.com --- drivers/gpu/drm/nouveau/nouveau_sched.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'drivers/gpu/drm/nouveau') diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c index 3424a1bf6af3..88217185e0f3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_sched.c +++ b/drivers/gpu/drm/nouveau/nouveau_sched.c @@ -292,6 +292,28 @@ nouveau_job_submit(struct nouveau_job *job) if (job->sync) done_fence = dma_fence_get(job->done_fence); + /* If a sched job depends on a dma-fence from a job from the same GPU + * scheduler instance, but a different scheduler entity, the GPU + * scheduler does only wait for the particular job to be scheduled, + * rather than for the job to fully complete. This is due to the GPU + * scheduler assuming that there is a scheduler instance per ring. + * However, the current implementation, in order to avoid arbitrary + * amounts of kthreads, has a single scheduler instance while scheduler + * entities represent rings. + * + * As a workaround, set the DRM_SCHED_FENCE_DONT_PIPELINE for all + * out-fences in order to force the scheduler to wait for full job + * completion for dependent jobs from different entities and same + * scheduler instance. + * + * There is some work in progress [1] to address the issues of firmware + * schedulers; once it is in-tree the scheduler topology in Nouveau + * should be re-worked accordingly. + * + * [1] https://lore.kernel.org/dri-devel/20230801205103.627779-1-matthew.brost@intel.com/ + */ + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &job->done_fence->flags); + if (job->ops->armed_submit) job->ops->armed_submit(job); -- cgit v1.2.3 From c6b9075cfbd624f2b33bd6fd388dc6f0b7027472 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 21 Aug 2023 00:29:16 +0200 Subject: drm/nouveau: uvmm: fix unset region pointer on remap Transfer the region pointer of a uvma to the new uvma(s) on re-map to prevent potential shader faults when the re-mapped uvma(s) are unmapped. Signed-off-by: Danilo Krummrich Reviewed-by: Dave Airlie Link: https://patchwork.freedesktop.org/patch/msgid/20230820222920.2344-1-dakr@redhat.com --- drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/nouveau') diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c index 3a1e8538f205..aae780e4a4aa 100644 --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c @@ -639,6 +639,7 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm, struct drm_gpuva *va = r->unmap->va; struct uvmm_map_args remap_args = { .kind = uvma_from_va(va)->kind, + .region = uvma_from_va(va)->region, }; u64 ustart = va->va.addr; u64 urange = va->va.range; -- cgit v1.2.3 From 443f9e0b1ab5e3b95abf8606097d13e30e2f2413 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 23 Aug 2023 20:15:34 +0200 Subject: drm/nouveau: uapi: don't pass NO_PREFETCH flag implicitly Currently, NO_PREFETCH is passed implicitly through drm_nouveau_gem_pushbuf_push::length and drm_nouveau_exec_push::va_len. Since this is a direct representation of how the HW is programmed it isn't really future proof for a uAPI. Hence, fix this up for the new uAPI and split up the va_len field of struct drm_nouveau_exec_push, such that we keep 32bit for va_len and 32bit for flags. For drm_nouveau_gem_pushbuf_push::length at least provide NOUVEAU_GEM_PUSHBUF_NO_PREFETCH to indicate the bit shift. While at it, fix up nv50_dma_push() as well, such that the caller doesn't need to encode the NO_PREFETCH flag into the length parameter. Signed-off-by: Danilo Krummrich Reviewed-by: Faith Ekstrand Link: https://patchwork.freedesktop.org/patch/msgid/20230823181746.3446-1-dakr@redhat.com --- drivers/gpu/drm/nouveau/nouveau_dma.c | 7 +++++-- drivers/gpu/drm/nouveau/nouveau_dma.h | 8 ++++++-- drivers/gpu/drm/nouveau/nouveau_exec.c | 19 ++++++++++++++++--- drivers/gpu/drm/nouveau/nouveau_gem.c | 6 ++++-- include/uapi/drm/nouveau_drm.h | 8 +++++++- 5 files changed, 38 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/nouveau') diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.c b/drivers/gpu/drm/nouveau/nouveau_dma.c index b90cac6d5772..b01c029f3a90 100644 --- a/drivers/gpu/drm/nouveau/nouveau_dma.c +++ b/drivers/gpu/drm/nouveau/nouveau_dma.c @@ -69,16 +69,19 @@ READ_GET(struct nouveau_channel *chan, uint64_t *prev_get, int *timeout) } void -nv50_dma_push(struct nouveau_channel *chan, u64 offset, int length) +nv50_dma_push(struct nouveau_channel *chan, u64 offset, u32 length, + bool no_prefetch) { struct nvif_user *user = &chan->drm->client.device.user; struct nouveau_bo *pb = chan->push.buffer; int ip = (chan->dma.ib_put * 2) + chan->dma.ib_base; BUG_ON(chan->dma.ib_free < 1); + WARN_ON(length > NV50_DMA_PUSH_MAX_LENGTH); nouveau_bo_wr32(pb, ip++, lower_32_bits(offset)); - nouveau_bo_wr32(pb, ip++, upper_32_bits(offset) | length << 8); + nouveau_bo_wr32(pb, ip++, upper_32_bits(offset) | length << 8 | + (no_prefetch ? (1 << 31) : 0)); chan->dma.ib_put = (chan->dma.ib_put + 1) & chan->dma.ib_max; diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h b/drivers/gpu/drm/nouveau/nouveau_dma.h index 035a709c7be1..1744d95b233e 100644 --- a/drivers/gpu/drm/nouveau/nouveau_dma.h +++ b/drivers/gpu/drm/nouveau/nouveau_dma.h @@ -31,7 +31,8 @@ #include "nouveau_chan.h" int nouveau_dma_wait(struct nouveau_channel *, int slots, int size); -void nv50_dma_push(struct nouveau_channel *, u64 addr, int length); +void nv50_dma_push(struct nouveau_channel *, u64 addr, u32 length, + bool no_prefetch); /* * There's a hw race condition where you can't jump to your PUT offset, @@ -45,6 +46,9 @@ void nv50_dma_push(struct nouveau_channel *, u64 addr, int length); */ #define NOUVEAU_DMA_SKIPS (128 / 4) +/* Maximum push buffer size. */ +#define NV50_DMA_PUSH_MAX_LENGTH 0x7fffff + /* Object handles - for stuff that's doesn't use handle == oclass. */ enum { NvDmaFB = 0x80000002, @@ -89,7 +93,7 @@ FIRE_RING(struct nouveau_channel *chan) if (chan->dma.ib_max) { nv50_dma_push(chan, chan->push.addr + (chan->dma.put << 2), - (chan->dma.cur - chan->dma.put) << 2); + (chan->dma.cur - chan->dma.put) << 2, false); } else { WRITE_PUT(chan->dma.cur); } diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c b/drivers/gpu/drm/nouveau/nouveau_exec.c index 0f927adda4ed..a90c4cd8cbb2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_exec.c +++ b/drivers/gpu/drm/nouveau/nouveau_exec.c @@ -164,8 +164,10 @@ nouveau_exec_job_run(struct nouveau_job *job) } for (i = 0; i < exec_job->push.count; i++) { - nv50_dma_push(chan, exec_job->push.s[i].va, - exec_job->push.s[i].va_len); + struct drm_nouveau_exec_push *p = &exec_job->push.s[i]; + bool no_prefetch = p->flags & DRM_NOUVEAU_EXEC_PUSH_NO_PREFETCH; + + nv50_dma_push(chan, p->va, p->va_len, no_prefetch); } ret = nouveau_fence_emit(fence, chan); @@ -223,7 +225,18 @@ nouveau_exec_job_init(struct nouveau_exec_job **pjob, { struct nouveau_exec_job *job; struct nouveau_job_args args = {}; - int ret; + int i, ret; + + for (i = 0; i < __args->push.count; i++) { + struct drm_nouveau_exec_push *p = &__args->push.s[i]; + + if (unlikely(p->va_len > NV50_DMA_PUSH_MAX_LENGTH)) { + NV_PRINTK(err, nouveau_cli(__args->file_priv), + "pushbuf size exceeds limit: 0x%x max 0x%x\n", + p->va_len, NV50_DMA_PUSH_MAX_LENGTH); + return -EINVAL; + } + } job = *pjob = kzalloc(sizeof(*job), GFP_KERNEL); if (!job) diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index f39360870c70..c0b10d8d3d03 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -856,9 +856,11 @@ revalidate: for (i = 0; i < req->nr_push; i++) { struct nouveau_vma *vma = (void *)(unsigned long) bo[push[i].bo_index].user_priv; + u64 addr = vma->addr + push[i].offset; + u32 length = push[i].length & ~NOUVEAU_GEM_PUSHBUF_NO_PREFETCH; + bool no_prefetch = push[i].length & NOUVEAU_GEM_PUSHBUF_NO_PREFETCH; - nv50_dma_push(chan, vma->addr + push[i].offset, - push[i].length); + nv50_dma_push(chan, addr, length, no_prefetch); } } else if (drm->client.device.info.chipset >= 0x25) { diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h index b1ad9d5ffce8..8d7402c13e56 100644 --- a/include/uapi/drm/nouveau_drm.h +++ b/include/uapi/drm/nouveau_drm.h @@ -138,6 +138,7 @@ struct drm_nouveau_gem_pushbuf_push { __u32 pad; __u64 offset; __u64 length; +#define NOUVEAU_GEM_PUSHBUF_NO_PREFETCH (1 << 23) }; struct drm_nouveau_gem_pushbuf { @@ -338,7 +339,12 @@ struct drm_nouveau_exec_push { /** * @va_len: the length of the push buffer mapping */ - __u64 va_len; + __u32 va_len; + /** + * @flags: the flags for this push buffer mapping + */ + __u32 flags; +#define DRM_NOUVEAU_EXEC_PUSH_NO_PREFETCH 0x1 }; /** -- cgit v1.2.3