[PATCH 0/4] drm/vc4: Clean-up the BO seqnos

[Maíra Canal]

On 06/12/24 11:17, Christian König wrote:
> Am 06.12.24 um 14:12 schrieb Maíra Canal:
>> This series introduces a series of clean-ups in BO reservations for the
>> VC4 driver. Currently, VC4 uses shared fences to track BO reservations as
>> several other DRM devices. But apart from the DMA reservation objects, 
>> VC4
>> also attaches seqnos to its BOs with the intention to track if the job 
>> that
>> uses such BO has finished.
>>
>> The seqno tracking system was introduced before the DMA reservation 
>> objects
>> and we ended up lefting it in the code after DMA resv was introduced. 
>> This
>> is redundant, as we can perfectly track the end of a job with a fence.
>> Therefore, this series focuses on eliminating the seqnos from the BO.
>>
>> This series introduces a series of clean-ups for BO reservations in 
>> the VC4
>> driver. Currently, VC4 uses shared fences to track BO reservations, 
>> similar
>> to many other DRM devices. However, in addition to DMA reservation 
>> objects,
>> VC4 has been maintaining a separate seqno tracking system for BOs to 
>> track
>> job completion.
>>
>> The seqno tracking system was implemented before DMA reservation objects
>> and was left in the code after DMA reservation's introduction. This
>> approach is now redundant, as job completion can be effectively tracked
>> using fences. Consequently, this series focuses on eliminating seqnos 
>> from
>> the BO implementation.
> 
> Just FYI, you duplicated the text somehow :)

Oops, that's a non-native English-speaker trying to write a v2 of a
paragraph :)

> 
>>
>> Patch Breakdown
>> ===============
>>
>> * Patch #1: Uses the DRM GEM implementation of 
>> `drm_gem_lock_reservations()`
>> and `drm_gem_unlock_reservations()` to replace the open-coded 
>> implementation
>> of this functions by VC4. A straightforward change with no functional
>> changes.
> 
> I actually pushed to remove drm_gem_lock_reservations() in favor of 
> using the drm_exec object.
> 
> It would be nice if you could use that one instead.

Okay, I'll use DRM exec in the next version. I'd appreciate if you could
take a look at the patches that remove the BOs seqnos.

Thanks for comments!

Best Regards,
- Maíra

> 
> Regards,
> Christian.
> 
>>
>> * Patch #2: Implements the VC4 wait BO IOCTL using DMA reservations. The
>> new implementation closely mirrors the V3D approach and removes the 
>> IOCTL's
>> dependency on BO sequence numbers.
>>
>> * Patch #3: The central piece of this patchset. It removes `bo->seqno`,
>> `bo->write_seqno`, and `exec->bin_dep_seqno` from the driver. As the 
>> seqno
>> tracking system is redundant, its deletion is relatively straightforward.
>> The only notable change is `vc4_async_set_fence_cb()`, which now uses
>> `dma_fence_add_callback()` to add the VC4 callback.
>>
>> * Patch #4: Deletes the now-unused function `vc4_queue_seqno_cb()`.
>>
>> Best Regards,
>> - Maíra
>>
>> Maíra Canal (4):
>>    drm/vc4: Use `drm_gem_lock_reservations()` instead of hard-coding
>>    drm/vc4: Use DMA Resv to implement VC4 wait BO IOCTL
>>    drm/vc4: Remove BOs seqnos
>>    drm/vc4: Remove `vc4_queue_seqno_cb()`
>>
>>   drivers/gpu/drm/vc4/vc4_crtc.c     |  22 ++---
>>   drivers/gpu/drm/vc4/vc4_drv.h      |  26 ++---
>>   drivers/gpu/drm/vc4/vc4_gem.c      | 147 ++++++-----------------------
>>   drivers/gpu/drm/vc4/vc4_validate.c |  11 ---
>>   4 files changed, 48 insertions(+), 158 deletions(-)
>>
>