On 28/02/2025 03:58, Nicolas Dufresne wrote:

Le jeudi 27 février 2025 à 23:46 +0300, Mikhail Rudenko a écrit :

quoted

Hi Jacopo,

On 2025-02-27 at 18:05 +01, Jacopo Mondi [off-list ref] wrote:

quoted

Hi Mikhail

On Tue, Jan 14, 2025 at 07:00:39PM +0300, Mikhail Rudenko wrote:

quoted

Hi Laurent,

On 2025-01-03 at 17:23 +02, Laurent Pinchart [off-list ref] wrote:

quoted

On Thu, Jan 02, 2025 at 06:35:00PM +0300, Mikhail Rudenko wrote:

quoted

Currently, the rkisp1 driver always uses coherent DMA allocations for
video capture buffers. However, on some platforms, using non-coherent
buffers can improve performance, especially when CPU processing of
MMAP'ed video buffers is required.

For example, on the Rockchip RK3399 running at maximum CPU frequency,
the time to memcpy a frame from a 1280x720 XRGB32 MMAP'ed buffer to a
malloc'ed userspace buffer decreases from 7.7 ms to 1.1 ms when using
non-coherent DMA allocation. CPU usage also decreases accordingly.

What's the time taken by the cache management operations ?

Sorry for the late reply, your question turned out a little more
interesting than I expected initially. :)

When capturing using Yavta with MMAP buffers under the conditions mentioned
in the commit message, ftrace gives 437.6 +- 1.1 us for
dma_sync_sgtable_for_cpu and 409 +- 14 us for
dma_sync_sgtable_for_device. Thus, it looks like using non-coherent
buffers in this case is more CPU-efficient even when considering cache
management overhead.

When trying to do the same measurements with libcamera, I failed. In a
typical libcamera use case when MMAP buffers are allocated from a
device, exported as dmabufs and then used for capture on the same device
with DMABUF memory type, cache management in kernel is skipped [1]
[2]. Also, vb2_dc_dmabuf_ops_{begin,end}_cpu_access are no-ops [3], so
DMA_BUF_IOCTL_SYNC from userspace does not work either.

So it looks like to make this change really useful, the above issue of
cache management for libcamera/DMABUF/videobuf2-dma-contig has to be
solved. I'm not an expert in this area, so any advice is kindly welcome. :)

It would be shame if we let this discussion drop dead.. cache
management policies are relevant for performances, specifically for
cpu access, and your above 7.7ms vs 1.1 ms test clearly shows that.

quoted

[1] https://git.linuxtv.org/media.git/tree/drivers/media/common/videobuf2/videobuf2-core.c?id=94794b5ce4d90ab134b0b101a02fddf6e74c437d#n411

I would like to know from Hans if the decision to disallow cache-hints
for dmabuf importers is a design choice or is deeply rooted in other
reasons I might be missing.

I'm asking because the idea is for libcamera to act solely as dma-buf
importer, the current alloc-export-then-import trick is an helper for
applications to work around the absence of a system allocator.

If the requirement to disable cache-hints for importers cannot be
lifted, for libcamera it means we would not be able to use it.

Meanwhile, I have posted a patch, which re-enables cache management ops
for non-coherent dmabufs exported from dma-contig allocator [1]. It is
currently waiting for review.

[1] https://lore.kernel.org/all/20250128-b4-rkisp-noncoherent-v3-1-baf39c997d2a@gmail.com/ (local)

I rely heavily on Tomasz Figa for review of such patches. He was OK with the
patch, but he suggested waiting a bit for more comments. My plan was to pick it
up end of next week if there are no further comments.

Regards,

	Hans

Thanks for you work, this matches what I was referring to missing in my
previous reply.

I don't think there is any intention to block or deprecate it, but
partially enabling leads to problems. Do we need something in the SG
allocator to ?

Nicolas

quoted

quoted

quoted

[2] https://git.linuxtv.org/media.git/tree/drivers/media/common/videobuf2/videobuf2-core.c?id=94794b5ce4d90ab134b0b101a02fddf6e74c437d#n829
[3] https://git.linuxtv.org/media.git/tree/drivers/media/common/videobuf2/videobuf2-dma-contig.c?id=94794b5ce4d90ab134b0b101a02fddf6e74c437d#n426

--
Best regards,
Mikhail Rudenko

--
Best regards,
Mikhail Rudenko