On 04/06/26 09:05, Tvrtko Ursulin wrote:
On 04/06/2026 12:52, Maíra Canal wrote:
Hi Tvrtko,
On 04/06/26 05:58, Tvrtko Ursulin wrote:
On 03/06/2026 23:25, Maíra Canal wrote:
+static int
+v3d_submit_jobs(struct v3d_submit *submit)
+{
+ struct v3d_dev *v3d = submit->v3d;
+ int ret = 0;
+
+ mutex_lock(&v3d->sched_lock);
+
+ for (int i = 0; i < submit->job_count; i++) {
+ struct v3d_job *job = submit->jobs[i];
+
+ v3d_push_job(job);
+
+ if (i + 1 < submit->job_count) {
+ ret = drm_sched_job_add_dependency(&submit->jobs[i +
1]- >base,
+ dma_fence_get(job->done_fence));
Is there a theoretical race where job->done_fence could be completed
and last reference dropped between push and here?
There is, but it'll be fixed in "[PATCH v3 14/14] drm/v3d: Ensure atomic
submissions in v3d_submit_jobs()".
+ if (ret)
+ goto err;
+ }
+ }
+
+err:
+ mutex_unlock(&v3d->sched_lock);
+ return ret;
+}
+
static int
v3d_setup_csd_jobs_and_bos(struct drm_file *file_priv,
struct v3d_dev *v3d,
@@ -921,18 +1048,15 @@ int
v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
- struct v3d_dev *v3d = to_v3d_dev(dev);
- struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
+ struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv
= file_priv };
struct drm_v3d_submit_cl *args = data;
struct v3d_submit_ext se = {0};
struct v3d_bin_job *bin = NULL;
struct v3d_render_job *render = NULL;
struct v3d_job *clean_job = NULL;
render and clean_job do not need to be initialised.
Ack.
+
+ ret = v3d_attach_perfmon_to_jobs(&submit, args->perfmon_id);
+ if (ret)
+ goto fail;
+
+ ret = v3d_lookup_bos(dev, file_priv,
+ submit.jobs[submit.job_count - 1],
submit.jobs[submit.job_count - 1] is repeated four times in the
function so you could assign it to a last_job local for more
readability. Up to you.
This will all disappear in the next few patches.
Considering this, would you consider R-b-ing this patch and I'll address
the initializations in the next version?
Normally best practice would be avoid leaving a know broken stage mid-
series. But, as I don't see this series getting half-reverted, but
To be fair about drm_sched_job_add_dependency() situation, it was
already broken before this series. We are just leaving it as broken as
it was to finally fix it in the end :)
instead if there will be regressions you will be the only person working
to fix them on top, I think we can dispense an exception:
Reviewed-by: Tvrtko Ursulin <[email protected]>
Thank you a lot for all the reviews!
Best regards,
- Maíra
Regards,
Tvrtko
