Backport of udmabuf and dma-heaps #3571

6by9 · 2020-04-23T14:31:00Z

As title. dma-heaps can hopefully simplify vcsm-cma, and they're wanted by the Libreelec folk.

They both seem to work in so far as they create /dev/udmabuf and /dev/dma_heap/linux,cma respectively. I haven't tested them from userspace.

Need to add a commit that adds them to the defconfig.

6by9 · 2020-05-06T08:59:21Z

Rebased to top of 5.4.
@popcornmix Does this actually work for Kodi/Libreelec? I'm about to see if it works for libcamera (to remove the dependency on vcsm-cma doing allocations).

6by9 · 2020-05-06T09:34:56Z

Hmm, having cherry-picked to my working branch, I'm getting /dev/dma_heap/system and /dev/dma_heap/reserved, but no longer /dev/dma_heap/linux,cma. It may be a build issue, but the option is enabled

CONFIG_DMABUF_HEAPS=y
CONFIG_DMABUF_HEAPS_SYSTEM=y
CONFIG_DMABUF_HEAPS_CMA=y

...
Ah, if there is a cma= entry on the command line then it gets names /dev/dma_heap/reserved. If it's configured by DT only, then it's /dev/dma_heap/linux,cma.
You get the messages

[    0.000000] Reserved memory: bypass linux,cma node, using cmdline CMA params instead
[    0.000000] OF: reserved mem: node linux,cma compatible matching fail
[    0.000000] cma: Reserved 384 MiB at 0x0000000016c00000

in dmesg if cma= exists.
This is going to be a pain to manage from userspace - yet another file to check for :-(

6by9 · 2020-05-08T19:47:19Z

Tested with https://github.com/6by9/userland/tree/clean having a modified user-vcsm library, and dma-heaps work.

Not tested udmabuf - it wasn't clear whether they actually help us on the Pi where most dmabuf consumers want contiguous allocations. The backport won't do any harm though.

6by9 · 2020-05-08T20:00:59Z

Question to @XECDesign.
This adds a device as /dev/dma_heap/linux,cma, however it defaults to ownership root/root, permissions 600.
What's the correct way to alter the permissions to 660 root/video or similar? I never did get udev rules to work for vcsm-cma.

XECDesign · 2020-05-08T20:12:50Z

I'm guessing copying on of the existing rules and adjusting it in the obvious way didn't work. I'd need to poke around with udevadm monitor a bit to figure and see if I can figure out the right runes.

6by9 · 2020-05-08T20:15:29Z

I never managed to get the right runes for vcsm-cma.
udevadm monitor is of limited use as the events have already happened during boot, and there's no way I know of to trigger it at a later point.

XECDesign · 2020-05-08T20:19:57Z

I seem to recall udevadm having options to rescan devices or query existing devices based on their dev nodes. I'm a little rusty with all of that but I can take a look tomorrow.

pelwell · 2020-05-08T20:21:52Z

If that doesn't work, there is a kernel function you can instrument to log the events on the way out.

6by9 · 2020-05-08T20:22:40Z

Thanks both. Things to look at.

6by9 · 2020-05-11T10:49:57Z

Found. Thanks for the guidance

$ udevadm info -n /dev/dma_heap/linux,cma
P: /devices/virtual/dma_heap/linux,cma
N: dma_heap/linux,cma
L: 0
E: DEVPATH=/devices/virtual/dma_heap/linux,cma
E: DEVNAME=/dev/dma_heap/linux,cma
E: MAJOR=253
E: MINOR=0
E: SUBSYSTEM=dma_heap

so adding

SUBSYSTEM=="dma_heap", GROUP="video", MODE="0660"

does the magic.

pelwell · 2020-05-11T11:01:25Z

Thanks for the update - that's a useful trick.

XECDesign · 2020-05-11T20:03:33Z

Should I add that for the next image?

popcornmix · 2020-05-12T15:21:06Z

@XECDesign I think it's worth adding. I believe it is working with kodi (from logs it can access /dev/dma_heap once permissions are fixed). I think buffers allocated come from here (although I'll see if I can be sure with debugger) and video plays okay.

6by9 · 2020-05-12T15:25:56Z

@XECDesign Yes please.

Now I know the runes, give me a few minutes and I'll see if I can get the runes right for vcsm-cma as well so that we can restrict it a little more than it currently is.

popcornmix · 2020-05-12T15:30:02Z

I have SUBSYSTEM=="vcsm-cma", GROUP="video", MODE="0660" in /etc/udev/rules.d/99-com.rules
Is that not what you want?

6by9 · 2020-05-12T15:35:18Z

pi@raspberrypi:~ $ udevadm info -n /dev/vcsm-cma 
P: /devices/virtual/misc/vcsm-cma
N: vcsm-cma
L: 0
E: DEVPATH=/devices/virtual/misc/vcsm-cma
E: DEVNAME=/dev/vcsm-cma
E: DEVMODE=0666
E: MAJOR=10
E: MINOR=60
E: SUBSYSTEM=misc

so that implies that SUBSYSTEM is not vcsm-cma. DRIVER might be, but that hasn't worked for me.
Trying it, your line doesn't appear to set the group to video.

The driver has a nasty hack in it to set permissions to 666 https://github.com/raspberrypi/linux/blob/rpi-5.4.y/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c#L1602

popcornmix · 2020-05-12T15:38:10Z

Ah yes - it works from the 666 permissions not the ownership.

6by9 · 2020-05-12T15:38:43Z

Further magic runes (got to love search engines)

pi@raspberrypi:~ $ udevadm info --name=/dev/vcsm-cma --attribute-walk

Udevadm info starts with the device specified by the devpath and then
walks up the chain of parent devices. It prints for every device
found, all possible attributes in the udev rules key format.
A rule to match, can be composed by the attributes of the device
and the attributes from one single parent device.

  looking at device '/devices/virtual/misc/vcsm-cma':
    KERNEL=="vcsm-cma"
    SUBSYSTEM=="misc"
    DRIVER==""

So

KERNEL=="vcsm-cma", GROUP="video", MODE="0660"

is the correct line for vcsm-cma. Can we add that one to the default rules as well please? I can then revert the kernel hack.

Commit 19d32ac upstream. Commit 7f0de8d ("dma-buf: Drop dma_buf_k(un)map") removed map/unmap handlers, but they still existed in udmabuf. Remove them there as well Signed-off-by: Maarten Lankhorst <[email protected]> Fixes: 7f0de8d ("dma-buf: Drop dma_buf_k(un)map") Cc: Sumit Semwal <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Reviewed-by: Daniel Vetter <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit bc7a71d uptream. The GEM prime helpers do it, so should we. It's also possible to make it optional later. Signed-off-by: Gurchetan Singh <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Gerd Hoffmann <[email protected]>

Commit c1bbed6 upstream. Will be used later. v2: rename 'udmabuf_misc' to 'device' (kraxel) Signed-off-by: Gurchetan Singh <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Gerd Hoffmann <[email protected]>

Commit 17a7ce2 upstream. These are nice functions and can be re-used. Signed-off-by: Gurchetan Singh <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Gerd Hoffmann <[email protected]>

Commit 284562e upstream. With the misc device, we should end up using the result of get_arch_dma_ops(..) or dma-direct ops. This can allow us to have WC mappings in the guest after synchronization. Signed-off-by: Gurchetan Singh <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Gerd Hoffmann <[email protected]>

Commit 1ffe095 upstream. I'm just going to put Chia's review comment here since it sums the issue rather nicely: "(1) Semantically, a dma-buf is in DMA domain. CPU access from the importer must be surrounded by {begin,end}_cpu_access. This gives the exporter a chance to move the buffer to the CPU domain temporarily. (2) When the exporter itself has other means to do CPU access, it is only reasonable for the exporter to move the buffer to the CPU domain before access, and to the DMA domain after access. The exporter can potentially reuse {begin,end}_cpu_access for that purpose. Because of (1), udmabuf does need to implement the {begin,end}_cpu_access hooks. But "begin" should mean dma_sync_sg_for_cpu and "end" should mean dma_sync_sg_for_device. Because of (2), if userspace wants to continuing accessing through the memfd mapping, it should call udmabuf's {begin,end}_cpu_access to avoid cache issues." Reported-by: Chia-I Wu <[email protected]> Suggested-by: Chia-I Wu <[email protected]> Fixes: 284562e ("udmabuf: implement begin_cpu_access/end_cpu_access hooks") Signed-off-by: Gurchetan Singh <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Gerd Hoffmann <[email protected]>

Commit c02a81f upstream. This framework allows a unified userspace interface for dma-buf exporters, allowing userland to allocate specific types of memory for use in dma-buf sharing. Each heap is given its own device node, which a user can allocate a dma-buf fd from using the DMA_HEAP_IOC_ALLOC. This code is an evoluiton of the Android ION implementation, and a big thanks is due to its authors/maintainers over time for their effort: Rebecca Schultz Zavin, Colin Cross, Benjamin Gaignard, Laura Abbott, and many other contributors! Cc: Laura Abbott <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Sumit Semwal <[email protected]> Cc: Liam Mark <[email protected]> Cc: Pratik Patel <[email protected]> Cc: Brian Starkey <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Sudipto Paul <[email protected]> Cc: Andrew F. Davis <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Chenbo Feng <[email protected]> Cc: Alistair Strachan <[email protected]> Cc: Hridya Valsaraju <[email protected]> Cc: Sandeep Patil <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Dave Airlie <[email protected]> Cc: [email protected] Reviewed-by: Brian Starkey <[email protected]> Acked-by: Sandeep Patil <[email protected]> Signed-off-by: Andrew F. Davis <[email protected]> Signed-off-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit 5248eb1 upstream. Add generic helper dmabuf ops for dma heaps, so we can reduce the amount of duplicative code for the exported dmabufs. This code is an evolution of the Android ION implementation, so thanks to its original authors and maintainters: Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others! Cc: Laura Abbott <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Sumit Semwal <[email protected]> Cc: Liam Mark <[email protected]> Cc: Pratik Patel <[email protected]> Cc: Brian Starkey <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Sudipto Paul <[email protected]> Cc: Andrew F. Davis <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Chenbo Feng <[email protected]> Cc: Alistair Strachan <[email protected]> Cc: Hridya Valsaraju <[email protected]> Cc: Sandeep Patil <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Dave Airlie <[email protected]> Cc: [email protected] Reviewed-by: Benjamin Gaignard <[email protected]> Reviewed-by: Brian Starkey <[email protected]> Acked-by: Sandeep Patil <[email protected]> Acked-by: Laura Abbott <[email protected]> Tested-by: Ayan Kumar Halder <[email protected]> Signed-off-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit efa04fe upstream. This patch adds system heap to the dma-buf heaps framework. This allows applications to get a page-allocator backed dma-buf for non-contiguous memory. This code is an evolution of the Android ION implementation, so thanks to its original authors and maintainters: Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others! Cc: Laura Abbott <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Sumit Semwal <[email protected]> Cc: Liam Mark <[email protected]> Cc: Pratik Patel <[email protected]> Cc: Brian Starkey <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Sudipto Paul <[email protected]> Cc: Andrew F. Davis <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Chenbo Feng <[email protected]> Cc: Alistair Strachan <[email protected]> Cc: Hridya Valsaraju <[email protected]> Cc: Sandeep Patil <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Dave Airlie <[email protected]> Cc: [email protected] Reviewed-by: Benjamin Gaignard <[email protected]> Reviewed-by: Brian Starkey <[email protected]> Acked-by: Sandeep Patil <[email protected]> Acked-by: Laura Abbott <[email protected]> Tested-by: Ayan Kumar Halder <[email protected]> Signed-off-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit b61614e upstream. This adds a CMA heap, which allows userspace to allocate a dma-buf of contiguous memory out of a CMA region. This code is an evolution of the Android ION implementation, so thanks to its original author and maintainters: Benjamin Gaignard, Laura Abbott, and others! NOTE: This patch only adds the default CMA heap. We will enable selectively adding other CMA memory regions to the dmabuf heaps interface with a later patch (which requires a dt binding) Cc: Laura Abbott <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Sumit Semwal <[email protected]> Cc: Liam Mark <[email protected]> Cc: Pratik Patel <[email protected]> Cc: Brian Starkey <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Sudipto Paul <[email protected]> Cc: Andrew F. Davis <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Chenbo Feng <[email protected]> Cc: Alistair Strachan <[email protected]> Cc: Hridya Valsaraju <[email protected]> Cc: Sandeep Patil <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Dave Airlie <[email protected]> Cc: [email protected] Reviewed-by: Benjamin Gaignard <[email protected]> Reviewed-by: Brian Starkey <[email protected]> Acked-by: Sandeep Patil <[email protected]> Acked-by: Laura Abbott <[email protected]> Tested-by: Ayan Kumar Halder <[email protected]> Signed-off-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit a877992 upstream. Add very trivial allocation and import test for dma-heaps, utilizing the vgem driver as a test importer. A good chunk of this code taken from: tools/testing/selftests/android/ion/ionmap_test.c Originally by Laura Abbott <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Sumit Semwal <[email protected]> Cc: Liam Mark <[email protected]> Cc: Pratik Patel <[email protected]> Cc: Brian Starkey <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Sudipto Paul <[email protected]> Cc: Andrew F. Davis <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Chenbo Feng <[email protected]> Cc: Alistair Strachan <[email protected]> Cc: Hridya Valsaraju <[email protected]> Cc: Sandeep Patil <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Dave Airlie <[email protected]> Cc: [email protected] Reviewed-by: Benjamin Gaignard <[email protected]> Reviewed-by: Brian Starkey <[email protected]> Acked-by: Sandeep Patil <[email protected]> Acked-by: Laura Abbott <[email protected]> Tested-by: Ayan Kumar Halder <[email protected]> Signed-off-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit b3b4346 upstream. This is more consistent with the DMA and DRM frameworks convention. This patch is only a name change, no logic is changed. Signed-off-by: Andrew F. Davis <[email protected]> Acked-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

The heaps are already in a directory of heaps, adding _heap to a heap name is redundant. This patch is only a name change, no logic is changed. Signed-off-by: Andrew F. Davis <[email protected]> Acked-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit f9d3b2c upstream. The -ENOTTY error return path does not free the allocated kdata as it returns directly. Fix this by returning via the error handling label err. Addresses-Coverity: ("Resource leak") Fixes: c02a81f ("dma-buf: Add dma-buf heaps framework") Signed-off-by: Colin Ian King <[email protected]> Acked-by: John Stultz <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

Commit 7d411af upstream. Fix the following sparse warning. drivers/dma-buf/dma-heap.c:109:14: warning: symbol 'dma_heap_ioctl_cmds' was not declared. Should it be static? Acked-by: Andrew F. Davis <[email protected]> Acked-by: John Stultz <[email protected]> Signed-off-by: zhong jiang <[email protected]> Signed-off-by: Sumit Semwal <[email protected]> [sumits: rebased over IOCTL rename patches] Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

The CMA dma-heap allocator can be used in place of vcsm-cma doing the allocation side, thereby simplifying that driver. Signed-off-by: Dave Stevenson <[email protected]>

6by9 · 2020-05-13T09:23:15Z

Messages from LibreElec folk say that this works for them, and I've had it working with both libcamera and a hacked user-vcsm (need to push that).
Any further comments/objections, or can it be merged before popcornmix rebases the branch again?

pelwell · 2020-05-13T09:27:10Z

@popcornmix seems happy (and he can always revert/delete if necessary) - merging.

See: raspberrypi/linux#3626 kernel: VC4 DRM/KMS - use correct dma-ranges See: raspberrypi/linux#3623 kernel: media: bcm2835-unicam: Retain packing information on G_FMT See: raspberrypi/linux#3622 kernel: Switch to snd_soc_dai_set_bclk_ratio See: raspberrypi/linux#3620 kernel: V4L2 H264 framing fixes See: raspberrypi/linux#3614 kernel: drm/vc4: Fix VIC usage with Broadcast RGB See: raspberrypi/linux#3611 kernel: media: bcm2835-unicam: Always service interrupts See: raspberrypi/linux#3608 kernel: overlays: Fix audio parameter of vc4-kms-v3 See: raspberrypi/linux#2489 kernel: configs: Restore missing NF_TABLES settings See: raspberrypi/linux#3615 kernel: sc16is7xx: Fix for hardware flow control See: raspberrypi/linux#2542 kernel: Use the upstream cpufreq driver on non-BCM2835 Pis See: raspberrypi/linux#3604 kernel: Backport of udmabuf and dma-heaps See: raspberrypi/linux#3571 kernel: imx477 v4l2 driver See: raspberrypi/linux#3605 firmware: isp: fix ISP component to return non-zero focus FoMs firmware: Fix for IMX477 focal length, f_number and aperture firmware: Update firmware for USB MSD boot firmware: platform: Fix overflow on high arm overclocks firmware: video_encode: Add option to include header bytes with frame firmware: DSI display: Close I2C handle if the display doesn't probe firmware: mmal/vc: Add mapping for OMX_IndexConfigBufferStall / MMAL_PARAMETER_VIDEO_STALL_THRESHOLD See: https://www.raspberrypi.org/forums/viewtopic.php?f=70&t=273123&p=1655481 firmware: hdmi: Request an I2C interrupt for EDID reading firmware: i2c: Move using_interrupt flag into periph_setup

pelwell force-pushed the rpi-5.4.y branch 3 times, most recently from 7d6e241 to 9daf453 Compare April 24, 2020 09:15

popcornmix force-pushed the rpi-5.4.y branch 2 times, most recently from 8567cd8 to 71d5db1 Compare May 4, 2020 13:23

6by9 force-pushed the rpi-5.4.y-dma-heaps branch from c8ec606 to 2d692ac Compare May 6, 2020 08:58

6by9 changed the title ~~WIP: Backport of udmabuf and dma-heaps~~ Backport of udmabuf and dma-heaps May 8, 2020

6by9 force-pushed the rpi-5.4.y-dma-heaps branch from 2d692ac to 1c6f0e4 Compare May 8, 2020 19:46

popcornmix force-pushed the rpi-5.4.y branch from f1b9edc to ee6afe0 Compare May 11, 2020 17:34

mlankhorst and others added 3 commits May 12, 2020 16:41

gurchetansingh and others added 13 commits May 12, 2020 16:41

defconfig: Add CMA and system dma-heaps to the config

b42b100

The CMA dma-heap allocator can be used in place of vcsm-cma doing the allocation side, thereby simplifying that driver. Signed-off-by: Dave Stevenson <[email protected]>

6by9 force-pushed the rpi-5.4.y-dma-heaps branch from 1c6f0e4 to b42b100 Compare May 12, 2020 15:44

pelwell merged commit a8ec7d1 into raspberrypi:rpi-5.4.y May 13, 2020

Backport of udmabuf and dma-heaps #3571

Backport of udmabuf and dma-heaps #3571

Uh oh!

Conversation

6by9 commented Apr 23, 2020

Uh oh!

6by9 commented May 6, 2020

Uh oh!

6by9 commented May 6, 2020

Uh oh!

6by9 commented May 8, 2020

Uh oh!

6by9 commented May 8, 2020

Uh oh!

XECDesign commented May 8, 2020

Uh oh!

6by9 commented May 8, 2020

Uh oh!

XECDesign commented May 8, 2020

Uh oh!

pelwell commented May 8, 2020

Uh oh!

6by9 commented May 8, 2020

Uh oh!

6by9 commented May 11, 2020

Uh oh!

pelwell commented May 11, 2020

Uh oh!

XECDesign commented May 11, 2020

Uh oh!

popcornmix commented May 12, 2020

Uh oh!

6by9 commented May 12, 2020

Uh oh!

popcornmix commented May 12, 2020

Uh oh!

6by9 commented May 12, 2020

Uh oh!

popcornmix commented May 12, 2020

Uh oh!

6by9 commented May 12, 2020

Uh oh!

6by9 commented May 13, 2020

Uh oh!

pelwell commented May 13, 2020

Uh oh!

Uh oh!