Add minigbm driver that provides generic gralloc functionality based on mesa/gbm library #2

rsglobal · 2022-09-12T17:22:22Z

Mainline CL: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2982542

Allow backends with custom DRM probing logic or backends that does not rely on DRM (dma-heap, ION). Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I7bcaf10205ca051eb109d6e220b8a2af38267442

Non-DRM drivers shouldn't rely on handles and DRM API. Add hook to allow drivers create custom implementation. Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I458dae38f80697184070019606b125992a9aa01d

... to avoid compile-time error on C++: error: ISO C++11 does not allow conversion from string literal to 'char *' [-Werror,-Wwritable-strings] Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I176eb657f72e92d6b5c7c3b25c78c56f776c20ab

Some drivers may copy/convert the buffer during mapping and in some cases stride of copied image can be different from original. Android uses pixel_stride for CPU access and need map_time stride instead of original stride in this cases. Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>

This is adaptation of the Rob Herring's gbm_gralloc HAL [1] to work as minigbm backend, optimised to work in modern Android conditions. gbm_gralloc has a huge potential and can provide more-or-less optimal allocation by using mesa3d allocation APIs. It should work out of the box for the all hardware mesa3d is supported. Limitations: This backend doesn't care of any other SOC-specific graphical components such as camera, hardware video codecs, etc. Differences between gbm_gralloc: 1. Designed to distinguish between Allocator and Mapper-sphal users. For Allocator gbm driver is initialized using standalone KMS card node (but only if 'lima', 'panfrost' or 'v3d' GPU was detected). For Mapper requests driver is initialized only using GPU render node. (which require less permissions for regular apps but suffitient for mapping). 2. GBM driver is initialized and bo is imported into gbm_mesa only if Mapper imports bo with software usage flags. (no time wasted in case SW access isn't required) 3. gbm_gralloc supported only HAL_PIXEL_FORMAT_YV12 video format. This driver aims to support more formats (using linear modifier only). * NOTE: This may also require: 1. Adding secomp rule into the mediaswcodec.policy file "sched_getaffinity: 1" 2. Adding records to selinux vendor/file_contexts file "/vendor/lib{64}?/libgbm_mesa_wrapper.so u:object_r:same_process_hal_file:s0" "/vendor/bin/hw/android\.hardware\.graphics\.allocator@4\.0-service\.minigbm_gbm_mesa u:object_r:hal_graphics_allocator_default_exec:s0" 3. Kernel v5.3+ for using fstat(dma-buf)->inode as unique buffer id [1]: https://github.com/robherring/gbm_gralloc v2: - Fixed incorrect size calculations for NV12 buffers - Add etnaviv, freedreno and vc4 to gpus list which require kmsro entry - Rebased v3: Squashed local fix commits: - RPI4: Add alignment for CSI camera - gbm_mesa: Handle DRM_FORMAT_R8 format to handle BLOBS - gbm_mesa: Always use DRM_FORMAT_R8 for buffers that unsupported by GBM - gbm_mesa: Add RGB888 format support for CPU access - gbm_mesa: Remove incorrect negation - gbm_mesa: Fallback COMPOSER buffers into VRAM - gbm_mesa: Add combinations to support external camera v4: - Use dlopen/dlsym to access gbm_wrapper - Add fallback allocation for unsupported by mesa3d formats v5: - Obtain map-time stride and report it to Android as pixel-stride. Map-time strides are different after gbm_create and gbm_import. Use map stride after gbm_import. This fixes artifacts on Intel and Nouveau. - GBM wrapper has been converted from cpp to c - Code refactor and cleanup - Licence headers added Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: If0f3dac42bea74f97a87e5a682380c33f4ff6837

WIP at this moment. Working on RPI4. (v3d+vc4). Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I517f095543b031e35f0d48bc03f6010a67012865

rsglobal · 2022-09-14T17:12:36Z

CC: @aleasto, @maurossi, @KonstaT, @goffioul

rsglobal · 2022-09-14T17:13:00Z

v5:

Obtain map-time stride and report it to Android as pixel-stride.
Map-time strides are different after gbm_create and gbm_import.
Use map stride after gbm_import.
This fixes artifacts on Intel and Nouveau.
GBM wrapper has been converted from cpp to c
Code refactor and cleanup
Licence headers added

rsglobal · 2022-09-14T17:14:03Z

Review / feedback are welcome.

aleasto · 2022-09-14T17:24:21Z

v5 verified working on waydroid - intel

KonstaT · 2022-09-15T19:29:49Z

Tested this on Pi 4, Android 13 & 5.15 kernel.

Using minigbm_gbm_mesa everything is working just the same as with the previous minigbm version I've used. OpenGL, Vulkan, libcamera, AOSP external camera HAL and v4l2_codec2 all work to the same extent as before.

Using minigbm_dmabuf seems still overall quite unstable. There's random crashes on occasion that cause a soft reboot.

OpenGL & Vulkan works
CSI camera modules (libcamera) crash and freeze the camera app, missing alignment?
UVC webcams (AOSP external camera HAL) works
v4l2_codec2 allocating using ION "works" (I've brought back ION on 5.15 kernel as it's removed from upstream)
v4l2_codec2 allocating using gralloc (i.e. setting debug.stagefright.c2-poolmask=0xfc0000 instead of 0xf50000) crashes though I'm not entirely sure if it's even supposed to work this way(?).

Great work as always!

Just a small note that files in dmabuf_driver are missing copyright headers.

goffioul · 2022-09-15T19:52:06Z

Would it be possible to handle gbm_mesa and dmabuf like the other backends in drv.c, and control their respective selection in drv_get_backend using Android properties? E.g. something like this pseudo-logic:

if debug.minigbm.backend == gbm_mesa
    return gbm_mes_backend
elif debug.minigbm.backend == dmabuf
    return dmabuf_backend
else
    return backend based on drm_version->name

This could be handy on Android-x86, which targets multiple hardware and tries to stay generic, but also for testing purpose for a specific platform.

rsglobal · 2022-09-16T09:58:50Z

Would it be possible to handle gbm_mesa and dmabuf like the other backends in drv.c, and control their respective selection in drv_get_backend using Android properties? E.g. something like this pseudo-logic:
if debug.minigbm.backend == gbm_mesa
    return gbm_mes_backend
elif debug.minigbm.backend == dmabuf
    return dmabuf_backend
else
    return backend based on drm_version->name
This could be handy on Android-x86, which targets multiple hardware and tries to stay generic, but also for testing purpose for a specific platform.

It's planned for next iterations in some or another forms.

goffioul · 2022-09-16T10:19:24Z

I tested video playback (using codec2) in QEMU/virgl running my Android 13 build, using the 3 flavors: minigbm_intel, minigbm_gbm_mesa and minigbm_dmabuf.

minigbm_intel

This produces recognizable, but incorrect, result:

minigbm_gbm_mesa

This produces garbled result:

Also there is these logs constantly repeated in QEMU standard error:

vrend_renderer_transfer_internal: context error reported 18 "oid.avc.decoder" IOV data size exceeds resource capacity 496
vrend_renderer_transfer_internal: context error reported 18 "oid.avc.decoder" IOV data size exceeds resource capacity 504
vrend_renderer_transfer_internal: context error reported 18 "oid.avc.decoder" IOV data size exceeds resource capacity 494
vrend_renderer_transfer_internal: context error reported 18 "oid.avc.decoder" IOV data size exceeds resource capacity 503

And also these logcat outputs at the start of the playback (not constantly repeated):

09-16 09:45:12.822   580   580 V [minigbm:gbm_mesa_internals.cpp(395)]: Unable to allocate 0x37393939 format, allocate as 1D buffer
09-16 09:45:12.822   580   580 V [minigbm:gbm_mesa_internals.cpp(405)]: Allocate 1D buffer as 4096x338 R8 2D texture
09-16 09:45:12.825   580   580 V [minigbm:gbm_mesa_internals.cpp(425)]: Allocated: 1280x720, stride: 4096, map_stride: 4096

minigbm_dmabuf

Note: this is the first time I actually try minigbm_dmabuf, so I might be missing something.

with skia renderengine, SurfaceFlinger crashes here: https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/renderengine/skia/AutoBackendTexture.cpp;l=134?q=AutoBackendTexture
with threaded renderengine, the system boots, but produces no display; one can see repeated logcat outputs like:

09-16 10:00:14.452   721   727 W EGL-MAIN: failed to create DRI image from FD
09-16 10:00:14.465   592   686 E RenderEngine: Failed to bind framebuffer! Aborting GPU composition for buffer (0x7abeeff4dcc0).
09-16 10:00:14.465   581   655 E hwc-display: Client layer must be always usable by DRM/KMS
09-16 10:00:14.465   592   592 W HwcComposer: command 0x2010000 generated error 3

KonstaT · 2022-09-16T13:04:07Z

minigbm_intel
This produces recognizable, but incorrect, result:

That actually looks pretty similar to what I have on Pi 4 using minigbm_gbm_mesa (GloDroid/glodroid_manifest#160 (comment)). There's also some other artifacts present in the playback that are likely because hw codecs on Pi 4 produce buffers that use their own custom modifier. So, this makes me now think there's two separate issues with this.

minigbm_dmabuf
Note: this is the first time I actually try minigbm_dmabuf, so I might be missing something.

Kernel needs to be compiled with following options enabled.

CONFIG_DMABUF_HEAPS=y
CONFIG_DMABUF_HEAPS_DEFERRED_FREE=y
CONFIG_DMABUF_HEAPS_PAGE_POOL=y
CONFIG_DMABUF_HEAPS_SYSTEM=y

Default ueventd.rc already has permission for these (https://cs.android.com/android/platform/superproject/+/master:system/core/rootdir/ueventd.rc;l=46-48).

In addition I also have CONFIG_DMABUF_HEAPS_CMA enabled in my kernel and /dev/dma_heap/linux,cma 0666 system graphics in my device's ueventd.rc.

Could be due to some entirely different issue of course but that's at least easy to check.

goffioul · 2022-09-16T13:10:10Z

Kernel needs to be compiled with following options enabled.
CONFIG_DMABUF_HEAPS=y
CONFIG_DMABUF_HEAPS_DEFERRED_FREE=y
CONFIG_DMABUF_HEAPS_PAGE_POOL=y
CONFIG_DMABUF_HEAPS_SYSTEM=y

Thanks. I already have these enabled, because codec2 depends on it (and ION drivers were removed from kernel 5.15, as you already know :)).

rsglobal · 2022-09-16T14:14:37Z

dmabuf driver is very dumb right now and in future would require external constraints configuration file

rsglobal · 2022-09-16T14:16:45Z

Same for hwcodecs and camera. There's no way to solve this issue with gbm/mesa, at least it would require platform-specific constraints configuration file.

goffioul · 2022-09-17T17:59:27Z

It's a bit off-topic, but since it's been mentioned, does somebody know whether v4l2_codec2 can be used with Intel GPU?

rsglobal · 2022-09-19T10:29:39Z

It's a bit off-topic, but since it's been mentioned, does somebody know whether v4l2_codec2 can be used with Intel GPU?

No, it can't

ADD: But artifacts you attached can be fixed with proper allocation constraints adjustment.

rsglobal added 3 commits September 12, 2022 20:16

minigbm: Add external driver support

179dfb9

Allow backends with custom DRM probing logic or backends that does not rely on DRM (dma-heap, ION). Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I7bcaf10205ca051eb109d6e220b8a2af38267442

minigbm: Add bo_get_plane_fd backend hook

1c4ff09

Non-DRM drivers shouldn't rely on handles and DRM API. Add hook to allow drivers create custom implementation. Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I458dae38f80697184070019606b125992a9aa01d

rsglobal mentioned this pull request Sep 12, 2022

Downstream minigbm frontend -> gbm backend maintaining aosp-developers-community/aosp-developers-community.github.io#5

Closed

rsglobal added 3 commits September 14, 2022 15:59

minigbm: Add dmabuf_heap-based backend

35a38ee

WIP at this moment. Working on RPI4. (v3d+vc4). Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com> Change-Id: I517f095543b031e35f0d48bc03f6010a67012865

rsglobal force-pushed the gbm_mesa branch from 5b2ad4b to 35a38ee Compare September 14, 2022 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add minigbm driver that provides generic gralloc functionality based on mesa/gbm library #2

Add minigbm driver that provides generic gralloc functionality based on mesa/gbm library #2

rsglobal commented Sep 12, 2022

rsglobal commented Sep 14, 2022

rsglobal commented Sep 14, 2022

rsglobal commented Sep 14, 2022

aleasto commented Sep 14, 2022

KonstaT commented Sep 15, 2022

goffioul commented Sep 15, 2022

rsglobal commented Sep 16, 2022

goffioul commented Sep 16, 2022

KonstaT commented Sep 16, 2022

goffioul commented Sep 16, 2022

rsglobal commented Sep 16, 2022

rsglobal commented Sep 16, 2022

goffioul commented Sep 17, 2022

rsglobal commented Sep 19, 2022 •

edited

Add minigbm driver that provides generic gralloc functionality based on mesa/gbm library #2

Are you sure you want to change the base?

Add minigbm driver that provides generic gralloc functionality based on mesa/gbm library #2

Conversation

rsglobal commented Sep 12, 2022

rsglobal commented Sep 14, 2022

rsglobal commented Sep 14, 2022

rsglobal commented Sep 14, 2022

aleasto commented Sep 14, 2022

KonstaT commented Sep 15, 2022

goffioul commented Sep 15, 2022

rsglobal commented Sep 16, 2022

goffioul commented Sep 16, 2022

minigbm_intel

minigbm_gbm_mesa

minigbm_dmabuf

KonstaT commented Sep 16, 2022

goffioul commented Sep 16, 2022

rsglobal commented Sep 16, 2022

rsglobal commented Sep 16, 2022

goffioul commented Sep 17, 2022

rsglobal commented Sep 19, 2022 • edited

rsglobal commented Sep 19, 2022 •

edited