Hi Iago,
On 11/27/23 05:12, Iago Toral wrote:
El jue, 23-11-2023 a las 21:47 -0300, Maíra Canal escribió:
We want to allow the IOCTLs to allocate the job without initiating
it.
This will be useful for the CPU job submission IOCTL, as the CPU job
has
the need to use information from the user extensions. Currently, the
user extensions are parsed before the job allocation, making it
impossible to fill the CPU job when parsing the user extensions.
Therefore, decouple the job allocation from the job initiation.
Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
---
drivers/gpu/drm/v3d/v3d_submit.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
b/drivers/gpu/drm/v3d/v3d_submit.c
index fe46dd316ca0..ed1a310bbd2f 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -135,6 +135,21 @@ void v3d_job_put(struct v3d_job *job)
kref_put(&job->refcount, job->free);
}
+static int
+v3d_job_allocate(void **container, size_t size)
+{
+ if (*container)
+ return 0;
Mmm... is this really what we want? At least right now we expect
Yeah, it is.
v3d_job_allocate to always allocate memory, is there any scenario in
which we would expect to call this with an already allocated container?
Take a look here:
19 int
18 v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
17 struct drm_file *file_priv)
16 {
15 struct v3d_dev *v3d = to_v3d_dev(dev);
14 struct drm_v3d_submit_cpu *args = data;
13 struct v3d_submit_ext se = {0};
12 struct v3d_submit_ext *out_se = NULL;
11 struct v3d_cpu_job *cpu_job = NULL;
10 struct v3d_csd_job *csd_job = NULL;
9 struct v3d_job *clean_job = NULL;
8 struct ww_acquire_ctx acquire_ctx;
7 int ret;
6
5 if (args->flags && !(args->flags &
DRM_V3D_SUBMIT_EXTENSION)) {
4 DRM_INFO("invalid flags: %d\n", args->flags);
3 return -EINVAL;
2 }
1
1187 ret = v3d_job_allocate((void *)&cpu_job, sizeof(*cpu_job));
1 if (ret)
2 return ret;
Here we allocate the CPU job, as we need it allocated to fill its fields
with the information in the extensions.
3
4 if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
5 ret = v3d_get_extensions(file_priv,
args->extensions, &se, cpu_job);
So, here we filling the CPU job fields with relevant information.
6 if (ret) {
7 DRM_DEBUG("Failed to get extensions.\n");
8 goto fail;
9 }
10 }
11
12 /* Every CPU job must have a CPU job user extension */
13 if (!cpu_job->job_type) {
14 DRM_DEBUG("CPU job must have a CPU job user
extension.\n");
15 goto fail;
16 }
17
18 trace_v3d_submit_cpu_ioctl(&v3d->drm, cpu_job->job_type);
19
20 ret = v3d_job_init(v3d, file_priv, (void *)&cpu_job,
sizeof(*cpu_job),
21 v3d_job_free, 0, &se, V3D_CPU);
Here we are initiating the job (drm_sched_job_init and syncobjs stuff).
If I don't have this condition in v3d_job_allocate, I would allocate
the container twice.
22 if (ret)
23 goto fail;
Best Regards,
- Maíra
Iago
+
+ *container = kcalloc(1, size, GFP_KERNEL);
+ if (!*container) {
+ DRM_ERROR("Cannot allocate memory for V3D job.\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
void **container, size_t size, void (*free)(struct kref
*ref),
@@ -145,11 +160,9 @@ v3d_job_init(struct v3d_dev *v3d, struct
drm_file *file_priv,
bool has_multisync = se && (se->flags &
DRM_V3D_EXT_ID_MULTI_SYNC);
int ret, i;
- *container = kcalloc(1, size, GFP_KERNEL);
- if (!*container) {
- DRM_ERROR("Cannot allocate memory for v3d job.");
- return -ENOMEM;
- }
+ ret = v3d_job_allocate(container, size);
+ if (ret)
+ return ret;
job = *container;
job->v3d = v3d;
