* [PATCH v2] drm/msm: Handle in-place remaps
@ 2025-08-05 16:44 Rob Clark
2025-08-05 21:32 ` Connor Abbott
0 siblings, 1 reply; 2+ messages in thread
From: Rob Clark @ 2025-08-05 16:44 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, freedreno, Danilo Krummrich, Connor Abbott,
Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, open list
Detect and handle the special case of a MAP op simply updating the vma
flags of an existing vma, and skip the pgtable updates. This allows
turnip to set the MSM_VMA_DUMP flag on an existing mapping without
requiring additional synchronization against commands running on the
GPU.
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_gem_vma.c | 41 ++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index d1f5bb2e0a16..00d0f3b7ba32 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -451,6 +451,8 @@ msm_gem_vm_bo_validate(struct drm_gpuvm_bo *vm_bo, struct drm_exec *exec)
struct op_arg {
unsigned flags;
struct msm_vm_bind_job *job;
+ const struct msm_vm_bind_op *op;
+ bool kept;
};
static void
@@ -472,14 +474,18 @@ vma_from_op(struct op_arg *arg, struct drm_gpuva_op_map *op)
}
static int
-msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *arg)
+msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg)
{
- struct msm_vm_bind_job *job = ((struct op_arg *)arg)->job;
+ struct op_arg *arg = _arg;
+ struct msm_vm_bind_job *job = arg->job;
struct drm_gem_object *obj = op->map.gem.obj;
struct drm_gpuva *vma;
struct sg_table *sgt;
unsigned prot;
+ if (arg->kept)
+ return 0;
+
vma = vma_from_op(arg, &op->map);
if (WARN_ON(IS_ERR(vma)))
return PTR_ERR(vma);
@@ -599,15 +605,41 @@ msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg)
}
static int
-msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *arg)
+msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg)
{
- struct msm_vm_bind_job *job = ((struct op_arg *)arg)->job;
+ struct op_arg *arg = _arg;
+ struct msm_vm_bind_job *job = arg->job;
struct drm_gpuva *vma = op->unmap.va;
struct msm_gem_vma *msm_vma = to_msm_vma(vma);
vm_dbg("%p:%p:%p: %016llx %016llx", vma->vm, vma, vma->gem.obj,
vma->va.addr, vma->va.range);
+ /*
+ * Detect in-place remap. Turnip does this to change the vma flags,
+ * in particular MSM_VMA_DUMP. In this case we want to avoid actually
+ * touching the page tables, as that would require synchronization
+ * against SUBMIT jobs running on the GPU.
+ */
+ if (op->unmap.keep &&
+ (arg->op->op == MSM_VM_BIND_OP_MAP) &&
+ (vma->gem.obj == arg->op->obj) &&
+ (vma->gem.offset == arg->op->obj_offset) &&
+ (vma->va.addr == arg->op->iova) &&
+ (vma->va.range == arg->op->range)) {
+ /* We are only expecting a single in-place unmap+map cb pair: */
+ WARN_ON(arg->kept);
+
+ /* Leave the existing VMA in place, but signal that to the map cb: */
+ arg->kept = true;
+
+ /* Only flags are changing, so update that in-place: */
+ unsigned orig_flags = vma->flags & (DRM_GPUVA_USERBITS - 1);
+ vma->flags = orig_flags | arg->flags;
+
+ return 0;
+ }
+
if (!msm_vma->mapped)
goto out_close;
@@ -1268,6 +1300,7 @@ vm_bind_job_prepare(struct msm_vm_bind_job *job)
const struct msm_vm_bind_op *op = &job->ops[i];
struct op_arg arg = {
.job = job,
+ .op = op,
};
switch (op->op) {
--
2.50.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] drm/msm: Handle in-place remaps
2025-08-05 16:44 [PATCH v2] drm/msm: Handle in-place remaps Rob Clark
@ 2025-08-05 21:32 ` Connor Abbott
0 siblings, 0 replies; 2+ messages in thread
From: Connor Abbott @ 2025-08-05 21:32 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, linux-arm-msm, freedreno, Danilo Krummrich,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, open list
On Tue, Aug 5, 2025 at 12:44 PM Rob Clark <robin.clark@oss.qualcomm.com> wrote:
>
> Detect and handle the special case of a MAP op simply updating the vma
> flags of an existing vma, and skip the pgtable updates. This allows
> turnip to set the MSM_VMA_DUMP flag on an existing mapping without
> requiring additional synchronization against commands running on the
> GPU.
>
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
Tested that this fixes the occasional page faults with the zink a618
jobs on Mesa CI with my Mesa MR.
Tested-by: Connor Abbott <cwabbott0@gmail.com>
> ---
> drivers/gpu/drm/msm/msm_gem_vma.c | 41 ++++++++++++++++++++++++++++---
> 1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index d1f5bb2e0a16..00d0f3b7ba32 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -451,6 +451,8 @@ msm_gem_vm_bo_validate(struct drm_gpuvm_bo *vm_bo, struct drm_exec *exec)
> struct op_arg {
> unsigned flags;
> struct msm_vm_bind_job *job;
> + const struct msm_vm_bind_op *op;
> + bool kept;
> };
>
> static void
> @@ -472,14 +474,18 @@ vma_from_op(struct op_arg *arg, struct drm_gpuva_op_map *op)
> }
>
> static int
> -msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *arg)
> +msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg)
> {
> - struct msm_vm_bind_job *job = ((struct op_arg *)arg)->job;
> + struct op_arg *arg = _arg;
> + struct msm_vm_bind_job *job = arg->job;
> struct drm_gem_object *obj = op->map.gem.obj;
> struct drm_gpuva *vma;
> struct sg_table *sgt;
> unsigned prot;
>
> + if (arg->kept)
> + return 0;
> +
> vma = vma_from_op(arg, &op->map);
> if (WARN_ON(IS_ERR(vma)))
> return PTR_ERR(vma);
> @@ -599,15 +605,41 @@ msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg)
> }
>
> static int
> -msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *arg)
> +msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg)
> {
> - struct msm_vm_bind_job *job = ((struct op_arg *)arg)->job;
> + struct op_arg *arg = _arg;
> + struct msm_vm_bind_job *job = arg->job;
> struct drm_gpuva *vma = op->unmap.va;
> struct msm_gem_vma *msm_vma = to_msm_vma(vma);
>
> vm_dbg("%p:%p:%p: %016llx %016llx", vma->vm, vma, vma->gem.obj,
> vma->va.addr, vma->va.range);
>
> + /*
> + * Detect in-place remap. Turnip does this to change the vma flags,
> + * in particular MSM_VMA_DUMP. In this case we want to avoid actually
> + * touching the page tables, as that would require synchronization
> + * against SUBMIT jobs running on the GPU.
> + */
> + if (op->unmap.keep &&
> + (arg->op->op == MSM_VM_BIND_OP_MAP) &&
> + (vma->gem.obj == arg->op->obj) &&
> + (vma->gem.offset == arg->op->obj_offset) &&
> + (vma->va.addr == arg->op->iova) &&
> + (vma->va.range == arg->op->range)) {
> + /* We are only expecting a single in-place unmap+map cb pair: */
> + WARN_ON(arg->kept);
> +
> + /* Leave the existing VMA in place, but signal that to the map cb: */
> + arg->kept = true;
> +
> + /* Only flags are changing, so update that in-place: */
> + unsigned orig_flags = vma->flags & (DRM_GPUVA_USERBITS - 1);
> + vma->flags = orig_flags | arg->flags;
> +
> + return 0;
> + }
> +
> if (!msm_vma->mapped)
> goto out_close;
>
> @@ -1268,6 +1300,7 @@ vm_bind_job_prepare(struct msm_vm_bind_job *job)
> const struct msm_vm_bind_op *op = &job->ops[i];
> struct op_arg arg = {
> .job = job,
> + .op = op,
> };
>
> switch (op->op) {
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-05 21:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-05 16:44 [PATCH v2] drm/msm: Handle in-place remaps Rob Clark
2025-08-05 21:32 ` Connor Abbott
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).