Hi Tvrtko,
On 07/08/26 06:22, Tvrtko Ursulin wrote:
+
+err_sched:
+ for (q--; q >= 0; q--)
+ drm_sched_entity_destroy(&vc4file->sched_entity[q]);
I keep forgetting if enum is guaranteed to be signed or not. For
avoidance of doubt you could write is as while (q-- > 0).
Apparently, it's unsigned... I'll switch the type definition to a signed
int in the next version, thank you for noticing!
+ kfree(vc4file);
+ return ret;
+
}
[...]
@@ -289,8 +321,8 @@ struct vc4_bo {
struct vc4_fence {
struct dma_fence base;
struct drm_device *dev;
- /* vc4 seqno for signaled() test */
Why this removal?
We will remove the (AFAIU deprecated) signaled() hook from vc4_fence.c
in the next patches.
uint64_t seqno;
+ enum vc4_queue queue;
};
#define to_vc4_fence(_fence) \
[...]
+void
+vc4_sched_fini(struct vc4_dev *vc4)
+{
+ enum vc4_queue q;
+
+ for (q = 0; q < VC4_MAX_QUEUES; q++) {
+ if (vc4->queue[q].sched.ready)
What is the ready check for?
It's useful in the error path, to skip schedulers that weren't
initialized yet.
+ drm_sched_fini(&vc4->queue[q].sched);
+ }
+}
Overall looks tidy to me and from the cover letter it sounds it works
well. If you haven't already, give it a lockdep pass, including the GPU
reset path and then with the entity error unwind you can have my r-b.
Thank you!
Best regards,
- Maíra
Regards,
Tvrtko
