Hi Tvrtko,

On 05/03/26 09:19, Tvrtko Ursulin wrote:

On 05/02/2026 21:31, Maíra Canal wrote:

[...]

Ack to all previous comments, thanks a lot!


+    spin_unlock_irqrestore(&vc4->job_lock, irqflags);
+
+    vc4_flush_caches(dev);
+
+    fence = vc4_fence_create(vc4, VC4_BIN);
+    if (IS_ERR(fence))
+        return NULL;

Exits with vc4->bin_job set, safe or accidental?

Accidental 😅


+
+    if (job->base.irq_fence)
+        dma_fence_put(job->base.irq_fence);

When can this happen, on re-submit?

Exactly.


+    job->base.irq_fence = dma_fence_get(fence);
+
+    trace_vc4_submit_cl(dev, false, to_vc4_fence(fence)->seqno,
+                job->ct0ca, job->ct0ea);
+
+    vc4_switch_perfmon(vc4, &job->base);
+
+    V3D_WRITE(V3D_CTNCA(0), job->ct0ca);
+    V3D_WRITE(V3D_CTNEA(0), job->ct0ea);
+
+    return fence;
+}
+
+static struct dma_fence *vc4_render_job_run(struct drm_sched_job *sched_job)
+{
+    struct vc4_render_job *job = to_render_job(sched_job);
+    struct vc4_dev *vc4 = job->base.vc4;
+    struct drm_device *dev = &vc4->base;
+    struct dma_fence *fence;
+
+    if (unlikely(job->base.base.s_fence->finished.error)) {
+        vc4->render_job = NULL;

No spinlock for render, only bin? By design?

The RENDER job is only changed in two places: when the job is starting
(here) and when the job is finishing. As the two situations are mutually
exclusive, it's fine not to have the lock. The BIN job is also used in
the memory overflow worker and that's why we need the lock.

Best regards,
- Maíra


Regards,

Tvrtko

Reply via email to