Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GD's non-upstreamed minigbm patches #10

Open
wants to merge 20 commits into
base: minigbm-upstream
Choose a base branch
from
Open

Conversation

rsglobal
Copy link
Member

@rsglobal rsglobal commented Nov 13, 2023

This series brings 2 additional backends to minigbm:

  1. Generic (gbm_mesa). It works similarly to gbm_gralloc but has the benefits
    of using minigbm core logic/helpers to allocate more advanced format
    combinations.

  2. Lightweight (dma_heaps). It requires a full description for every platform.
    Therefore, at the moment, it is only experimental.

Unfortunately, it seems like minigbm maintainers are not motivated to accept these patches in any form.

I do not want to create/maintain a fork of minigbm, so I decided to make this PR a temporary home for those patches.
Thus making it easier to collaborate on them.

rsglobal added a commit that referenced this pull request Nov 13, 2023
For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Android.bp files are maintained in the AOSP fork repository.

Change-Id: I492e0c2154b22cd4cda85e379df2578b352c23df
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Handle has some limits and can't be used as unique buffer ID on systems
where display controller can scanout from CMA but GPU can work with both
CMA and VRAM.

Such systems have DRM/KMS and DRM/GPU drivers separated.
GBM frontend is always expecting handle for DRM/KMS driver.
In such system any attempt of importing the buffer with more
than 1 contiguous chunk into DRM/KMS driver will fail.

Using dma-buf inode as unique buffer ID is a common practice for
a last several years starting from [this kernel patch][1].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ed63bb1d1f8469586006a9ca63c42344401aa2ab
Change-Id: Ic3a69010d5da2f866a2252fc7e9eb29d67f8e1ed
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Allow backends with custom DRM probing logic or
backends that does not rely on DRM (dma-heap, ION).

Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I7bcaf10205ca051eb109d6e220b8a2af38267442

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Non-DRM drivers shouldn't rely on handles and DRM API.
Add hook to allow drivers create custom implementation.

Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I458dae38f80697184070019606b125992a9aa01d

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
... 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 <[email protected]>
Change-Id: I176eb657f72e92d6b5c7c3b25c78c56f776c20ab

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
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 <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: Ie668d4a9d8f18ff1f885979ebe1dcd0a7646fda2
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
At this moment minigbm require 2 separate logic to be implemented by the
backends:
1. Filling-up supported combination structure (used for testing)
2. Allocation itself.

Dispite that splitting looks reasonable for a first glance, implementing
and supporting 2 logics is not that easy. The combination structure list
with the priority levels makes thinks even more difficult to understand
and work with. Also such an approach doesn't allow filtering-out the
buffers with unsupported dimentions.

Combining these 2 logics into single should drasticly simplify
implementing of new drivers, e.g.:

    int bo_create_v2(..., bool test_only) {
      if (!is_bo_supported(...)) {
        return -INVAL;
      }
      if (test_only)
        return 0

      allocate();
    }

Change-Id: I86e5330759a701e309bf7e57f7b3dfd2ddf3cfa0
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: If76a87d50cf430d5480ddc92925613daa5ac36ea
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: Icce8164cde8da483d683c06a4936ffde05b6f515
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
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 <[email protected]>
Change-Id: If0f3dac42bea74f97a87e5a682380c33f4ff6837

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
WIP at this moment.
Working on RPI4. (v3d+vc4).

Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I517f095543b031e35f0d48bc03f6010a67012865

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
property name: vendor.gralloc.minigbm.backend
possible values:
1. auto (default)
2. gbm_mesa
3. dmaheaps

Change-Id: I5ab2649f98d40d139afb008ea8a804c790b81c31
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: I5d5cb9948206b15e833ff63a7dccd2b1002c8cf3
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Works from obj/AOSPEXT/MINIGBM directory

Change-Id: Ibbba57eec7b36d5efd01b48840175b9909d775d1
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: Icce8164cde8da483d683c06a4936ffde05b6f515
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
rsglobal added a commit that referenced this pull request Nov 13, 2023
Change-Id: I8cd8eef9237268ce6945d3153542eedb506614a9
Signed-off-by: Roman Stratiienko <[email protected]>

For reviews, comments, suggestions and questions visit:
#10
olvaffe and others added 11 commits November 15, 2023 01:58
When MINIGBM_DEBUG=log_bos is set, log all bos created/imported.

BUG=none
TEST=add MINIGBM_DEBUG=log_bos to /etc/chrome_dev.conf and
     see logs in /var/log/ui/ui.LATEST

Change-Id: Ib2b2d5c3181862b557dbc5499fef84f2c9ac8122
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/5014198
Commit-Queue: ChromeOS Auto Retry <[email protected]>
Reviewed-by: Dawn Han <[email protected]>
Tested-by: Chia-I Wu <[email protected]>
Previous change introduced a regression to CTS tests on other mediatek
board then MTK8173. This CL fixes this by switching to relying on
MTK_MT8173 compile define.

BUG=b:305347887
TEST=android.mediav2.cts.CodecEncoderSurfaceTest

Cq-Depend: chromium:4887244
Change-Id: I30f814222423e1b77d25c3256e9fcb0319c6339f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/4946471
Reviewed-by: Hirokazu Honda <[email protected]>
Tested-by: Bartłomiej Grzesik <[email protected]>
Commit-Queue: Kazuhiro Inaba <[email protected]>
Android.bp files are maintained in the AOSP fork repository.

Change-Id: I492e0c2154b22cd4cda85e379df2578b352c23df
Signed-off-by: Roman Stratiienko <[email protected]>
Handle has some limits and can't be used as unique buffer ID on systems
where display controller can scanout from CMA but GPU can work with both
CMA and VRAM.

Such systems have DRM/KMS and DRM/GPU drivers separated.
GBM frontend is always expecting handle for DRM/KMS driver.
In such system any attempt of importing the buffer with more
than 1 contiguous chunk into DRM/KMS driver will fail.

Using dma-buf inode as unique buffer ID is a common practice for
a last several years starting from [this kernel patch][1].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ed63bb1d1f8469586006a9ca63c42344401aa2ab
Change-Id: Ic3a69010d5da2f866a2252fc7e9eb29d67f8e1ed
Signed-off-by: Roman Stratiienko <[email protected]>
Allow backends with custom DRM probing logic or
backends that does not rely on DRM (dma-heap, ION).

Signed-off-by: Roman Stratiienko <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.

Change-Id: I987da3b353c7067be686f2d4ff2469ad4f40c5c6
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: Ie668d4a9d8f18ff1f885979ebe1dcd0a7646fda2
Signed-off-by: Roman Stratiienko <[email protected]>
At this moment minigbm require 2 separate logic to be implemented by the
backends:
1. Filling-up supported combination structure (used for testing)
2. Allocation itself.

Dispite that splitting looks reasonable for a first glance, implementing
and supporting 2 logics is not that easy. The combination structure list
with the priority levels makes thinks even more difficult to understand
and work with. Also such an approach doesn't allow filtering-out the
buffers with unsupported dimentions.

Combining these 2 logics into single should drasticly simplify
implementing of new drivers, e.g.:

    int bo_create_v2(..., bool test_only) {
      if (!is_bo_supported(...)) {
        return -INVAL;
      }
      if (test_only)
        return 0

      allocate();
    }

Change-Id: I86e5330759a701e309bf7e57f7b3dfd2ddf3cfa0
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: If76a87d50cf430d5480ddc92925613daa5ac36ea
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: Icce8164cde8da483d683c06a4936ffde05b6f515
Signed-off-by: Roman Stratiienko <[email protected]>
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 <[email protected]>
Change-Id: If0f3dac42bea74f97a87e5a682380c33f4ff6837
WIP at this moment.
Working on RPI4. (v3d+vc4).

Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I517f095543b031e35f0d48bc03f6010a67012865
property name: vendor.gralloc.minigbm.backend
possible values:
1. auto (default)
2. gbm_mesa
3. dmaheaps

Change-Id: I5ab2649f98d40d139afb008ea8a804c790b81c31
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I5d5cb9948206b15e833ff63a7dccd2b1002c8cf3
Signed-off-by: Roman Stratiienko <[email protected]>
Works from obj/AOSPEXT/MINIGBM directory

Change-Id: Ibbba57eec7b36d5efd01b48840175b9909d775d1
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: Icce8164cde8da483d683c06a4936ffde05b6f515
Signed-off-by: Roman Stratiienko <[email protected]>
Change-Id: I8cd8eef9237268ce6945d3153542eedb506614a9
Signed-off-by: Roman Stratiienko <[email protected]>
This series brings 2 additional backends to minigbm:
1. Generic (gbm_mesa). It works similarly to gbm_gralloc but has the benefits
   of using minigbm core logic/helpers to allocate more advanced format
   combinations.

2. Lightweight (dma_heaps). It requires a full description for every platform.
   Therefore, at the moment, it is only experimental.

For more information, reviews, comments, suggestions and questions visit:
#10

Change-Id: I2c3470d79c385b2b2e7a376f4ba6438f81d5c269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants