Hi Tvrtko,
On 06/03/26 08:28, Tvrtko Ursulin wrote:
On 05/02/2026 21:31, Maíra Canal wrote:
Introduce vc4_submit.c with the job submission path rewritten to
integrate with the DRM GPU scheduler. Most of this code is adapted from
vc4_gem.c, with key changes concentrated in the job creation and
lifecycle management. This implementation follows the same design as the
v3d driver.
This code coexists with the legacy path until the switchover commit.
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/vc4/vc4_submit.c | 509 +++++++++++++++++++++++++++++
++++++++++
1 file changed, 509 insertions(+)
[...]
+static int
+vc4_lookup_bos(struct drm_device *dev, struct drm_file *file_priv,
+ struct vc4_render_job *job, u64 bo_handles, u32 bo_count)
+{
+ int ret = 0;
+ int i;
+
+ job->bo_count = bo_count;
+
+ if (!job->bo_count) {
+ drm_warn(dev, "Rendering requires BOs to validate\n");
+ return -EINVAL;
+ }
+
+ ret = drm_gem_objects_lookup(file_priv, u64_to_user_ptr(bo_handles),
+ job->bo_count, &job->bo);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < job->bo_count; i++) {
+ ret = vc4_bo_inc_usecnt(to_vc4_bo(job->bo[i]));
Quite odd model in vc4 that this can fail for completely different
reasons. Looking into vc4_bo_inc_usecnt().. There are reference counts
for the same bo? One GEM and one vc4, why?
I just noticed that I didn't answer this question. I'm sorry for my
overlook.
So, this refcount tracks active usage: whether the BO is currently in
use by the GPU or display. When the refcount drops to zero and madv ==
DONTNEED, the BO goes to a purgeable pool. On the GPU path, this
refcount is indeed redundant, but it's genuinely needed in the display
paths (prepare_fb/cleanup_fb, async page flip), because those don't use
dma_resv fences to track activity.
I agree it's an odd design, but I believe it was needed at some point in
history to conciliate vc4's display and rendering capabilities in a
single driver (that is, vc4->gen == VC4_GEN_4 // RPi 0-3).
To make things less redundant, I'm thinking about adding a
vc4_bo_active() function for BO activity tracking, which will account
both for the internal refcount and dma_resv.
The runtime invariant check is needed at this level, I mean this one?:
if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4))
return -ENODEV;
It cannot be moved somewhere higher up the chain? There can legitimately
be a mix of bos with different vc4_dev?
No, vc4->gen is a attribute related to the HW generation. That is, for a
single device, there can be only one generation. From my understanding,
this guard is just a sanity-check. Just to make sure nobody will call
this function for KMS-only devices (RPi 4-5). For RPi 4-5, there is no
need to use the refcount path, as vc4 is not responsible for rendering
(v3d is).
Best regards,
- Maíra
