2023-11-21 00:40:16

by Rob Clark

[permalink] [raw]
Subject: [PATCH v2 1/7] drm/msm/gem: Remove "valid" tracking

From: Rob Clark <[email protected]>

This was a small optimization for pre-soft-pin userspace. But mesa
switched to soft-pin nearly 5yrs ago. So lets drop the optimization
and simplify the code.

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.h | 2 --
drivers/gpu/drm/msm/msm_gem_submit.c | 44 +++++-----------------------
2 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 8ddef5443140..c36c1c1fa222 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -271,7 +271,6 @@ struct msm_gem_submit {
struct msm_gpu_submitqueue *queue;
struct pid *pid; /* submitting process */
bool fault_dumped; /* Limit devcoredump dumping to one per submit */
- bool valid; /* true if no cmdstream patching needed */
bool in_rb; /* "sudo" mode, copy cmds into RB */
struct msm_ringbuffer *ring;
unsigned int nr_cmds;
@@ -288,7 +287,6 @@ struct msm_gem_submit {
} *cmd; /* array of size nr_cmds */
struct {
/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
-#define BO_VALID 0x8000 /* is current addr in cmdstream correct/valid? */
#define BO_LOCKED 0x4000 /* obj lock is held */
#define BO_PINNED 0x2000 /* obj (pages) is pinned and on active list */
uint32_t flags;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 6d8ec1337e8b..996274ef32a6 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -150,8 +150,6 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,

submit->bos[i].handle = submit_bo.handle;
submit->bos[i].flags = submit_bo.flags;
- /* in validate_objects() we figure out if this is true: */
- submit->bos[i].iova = submit_bo.presumed;
}

spin_lock(&file->table_lock);
@@ -278,9 +276,6 @@ static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, int i)
{
unsigned cleanup_flags = BO_PINNED | BO_LOCKED;
submit_cleanup_bo(submit, i, cleanup_flags);
-
- if (!(submit->bos[i].flags & BO_VALID))
- submit->bos[i].iova = 0;
}

/* This is where we make sure all the bo's are reserved and pin'd: */
@@ -390,8 +385,6 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
struct msm_drm_private *priv = submit->dev->dev_private;
int i, ret = 0;

- submit->valid = true;
-
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
struct msm_gem_vma *vma;
@@ -407,14 +400,7 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
if (ret)
break;

- if (vma->iova == submit->bos[i].iova) {
- submit->bos[i].flags |= BO_VALID;
- } else {
- submit->bos[i].iova = vma->iova;
- /* iova changed, so address in cmdstream is not valid: */
- submit->bos[i].flags &= ~BO_VALID;
- submit->valid = false;
- }
+ submit->bos[i].iova = vma->iova;
}

/*
@@ -451,7 +437,7 @@ static void submit_attach_object_fences(struct msm_gem_submit *submit)
}

static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
- struct drm_gem_object **obj, uint64_t *iova, bool *valid)
+ struct drm_gem_object **obj, uint64_t *iova)
{
if (idx >= submit->nr_bos) {
SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
@@ -463,8 +449,6 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
*obj = submit->bos[idx].obj;
if (iova)
*iova = submit->bos[idx].iova;
- if (valid)
- *valid = !!(submit->bos[idx].flags & BO_VALID);

return 0;
}
@@ -477,9 +461,6 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
uint32_t *ptr;
int ret = 0;

- if (!nr_relocs)
- return 0;
-
if (offset % 4) {
SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n", offset);
return -EINVAL;
@@ -500,7 +481,6 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
struct drm_msm_gem_submit_reloc submit_reloc = relocs[i];
uint32_t off;
uint64_t iova;
- bool valid;

if (submit_reloc.submit_offset % 4) {
SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
@@ -519,13 +499,10 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
goto out;
}

- ret = submit_bo(submit, submit_reloc.reloc_idx, NULL, &iova, &valid);
+ ret = submit_bo(submit, submit_reloc.reloc_idx, NULL, &iova);
if (ret)
goto out;

- if (valid)
- continue;
-
iova += submit_reloc.reloc_offset;

if (submit_reloc.shift < 0)
@@ -879,8 +856,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_gem_object *obj;
uint64_t iova;

- ret = submit_bo(submit, submit->cmd[i].idx,
- &obj, &iova, NULL);
+ ret = submit_bo(submit, submit->cmd[i].idx, &obj, &iova);
if (ret)
goto out;

@@ -894,17 +870,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,

submit->cmd[i].iova = iova + (submit->cmd[i].offset * 4);

- if (submit->valid)
+ if (likely(!submit->cmd[i].nr_relocs))
continue;

if (!gpu->allow_relocs) {
- if (submit->cmd[i].nr_relocs) {
- SUBMIT_ERROR(submit, "relocs not allowed\n");
- ret = -EINVAL;
- goto out;
- }
-
- continue;
+ SUBMIT_ERROR(submit, "relocs not allowed\n");
+ ret = -EINVAL;
+ goto out;
}

ret = submit_reloc(submit, obj, submit->cmd[i].offset * 4,
--
2.42.0


2023-12-03 11:29:24

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] drm/msm/gem: Remove "valid" tracking

On 21/11/2023 02:38, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> This was a small optimization for pre-soft-pin userspace. But mesa
> switched to soft-pin nearly 5yrs ago. So lets drop the optimization
> and simplify the code.
>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_gem.h | 2 --
> drivers/gpu/drm/msm/msm_gem_submit.c | 44 +++++-----------------------
> 2 files changed, 8 insertions(+), 38 deletions(-)

Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry