From 336a68a078a214f89610db6e9c308ba135d0b433 Mon Sep 17 00:00:00 2001 From: Ankit Goyal Date: Fri, 31 Mar 2023 16:53:52 -0700 Subject: gralloc4: Remove dead code from ion Most of the removal comes from the fact that ion_client is always going to be 0 which helps in cleaning up all the setup code needed to make sure ion_client is valid. Bug: 275481134 Bug: 245053092 Test: Boot to home Change-Id: Icc2cdb5eb9d6235ab37de4e7449724b5c546fc3f --- gralloc4/src/4.x/GrallocMapper.cpp | 9 +- gralloc4/src/aidl/GrallocAllocator.cpp | 4 +- gralloc4/src/allocator/mali_gralloc_ion.cpp | 187 ++-------------------------- gralloc4/src/allocator/mali_gralloc_ion.h | 3 - 4 files changed, 15 insertions(+), 188 deletions(-) diff --git a/gralloc4/src/4.x/GrallocMapper.cpp b/gralloc4/src/4.x/GrallocMapper.cpp index e0ac1de..c0b5f5f 100644 --- a/gralloc4/src/4.x/GrallocMapper.cpp +++ b/gralloc4/src/4.x/GrallocMapper.cpp @@ -35,14 +35,9 @@ using android::hardware::hidl_vec; using android::hardware::Void; -GrallocMapper::GrallocMapper() -{ -} +GrallocMapper::GrallocMapper() {} -GrallocMapper::~GrallocMapper() -{ - mali_gralloc_ion_close(); -} +GrallocMapper::~GrallocMapper() {} Return GrallocMapper::createDescriptor(const BufferDescriptorInfo &descriptorInfo, createDescriptor_cb hidl_cb) { diff --git a/gralloc4/src/aidl/GrallocAllocator.cpp b/gralloc4/src/aidl/GrallocAllocator.cpp index d7addc5..6945505 100644 --- a/gralloc4/src/aidl/GrallocAllocator.cpp +++ b/gralloc4/src/aidl/GrallocAllocator.cpp @@ -24,9 +24,7 @@ unsigned long callingPid() { GrallocAllocator::GrallocAllocator() {} -GrallocAllocator::~GrallocAllocator() { - mali_gralloc_ion_close(); -} +GrallocAllocator::~GrallocAllocator() {} ndk::ScopedAStatus GrallocAllocator::allocate(const std::vector& descriptor, int32_t count, AidlAllocator::AllocationResult* result) { diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp index bfd92d7..175626c 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.cpp +++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp @@ -54,17 +54,6 @@ #include #include -#define INIT_ZERO(obj) (memset(&(obj), 0, sizeof((obj)))) - -#define HEAP_MASK_FROM_ID(id) (1 << id) -#define HEAP_MASK_FROM_TYPE(type) (1 << type) - -#if defined(ION_HEAP_SECURE_MASK) -#if (HEAP_MASK_FROM_TYPE(ION_HEAP_TYPE_SECURE) != ION_HEAP_SECURE_MASK) -#error "ION_HEAP_TYPE_SECURE value is not compatible with ION_HEAP_SECURE_MASK" -#endif -#endif - static const char kDmabufSensorDirectHeapName[] = "sensor_direct_heap"; static const char kDmabufFaceauthTpuHeapName[] = "faceauth_tpu-secure"; static const char kDmabufFaceauthImgHeapName[] = "faimg-secure"; @@ -76,45 +65,18 @@ static const char kDmabufVstreamSecureHeapName[] = "vstream-secure"; struct ion_device { - int client() - { - return ion_client; - } - - static void close() - { - ion_device &dev = get_inst(); - if (dev.ion_client >= 0) - { - exynos_ion_close(dev.ion_client); - dev.ion_client = -1; - } - - dev.buffer_allocator.reset(); - } - static ion_device *get() { - ion_device &dev = get_inst(); + static ion_device dev; if (!dev.buffer_allocator) { dev.buffer_allocator = std::make_unique(); - if (!dev.buffer_allocator) + if (!dev.buffer_allocator) { ALOGE("Unable to create BufferAllocator object"); - } - - if (dev.ion_client < 0) - { - if (dev.open_and_query_ion() != 0) - { - close(); + return nullptr; } } - if (dev.ion_client < 0) - { - return nullptr; - } return &dev; } @@ -125,14 +87,12 @@ struct ion_device * @param size [in] Requested buffer size (in bytes). * @param heap_type [in] Requested heap type. * @param flags [in] ION allocation attributes defined by ION_FLAG_*. - * @param min_pgsz [out] Minimum page size (in bytes). * @buffer_name [in] Optional name specifying what the buffer is for. * * @return File handle which can be used for allocation, on success * -1, otherwise. */ - int alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, int *min_pgsz, - const std::string& buffer_name = std::string()); + int alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name = std::string()); /* * Signals the start or end of a region where the CPU is accessing a @@ -150,28 +110,8 @@ struct ion_device int sync(int fd, bool read, bool write, bool start); private: - int ion_client; std::unique_ptr buffer_allocator; - ion_device() - : ion_client(-1) - { - } - - static ion_device& get_inst() - { - static ion_device dev; - return dev; - } - - /* - * Opens the ION module. Queries heap information and stores it for later use - * - * @return 0 in case of success - * -1 for all error cases - */ - int open_and_query_ion(); - /* * Allocates in the DMA-BUF heap with name @heap_name. If allocation fails from * the DMA-BUF heap or if it does not exist, falls back to an ION heap of the @@ -207,14 +147,6 @@ static void set_ion_flags(uint64_t usage, unsigned int *ion_flags) { *ion_flags |= ION_FLAG_PROTECTED; } - - /* TODO: used for exynos3830. Add this as an option to Android.bp */ -#if defined(GRALLOC_SCALER_WFD) && GRALLOC_SCALER_WFD == 1 - if (usage & GRALLOC_USAGE_PRIVATE_NONSECURE && usage & GRALLOC_USAGE_HW_COMPOSER) - { - *ion_flags |= ION_FLAG_PROTECTED; - } -#endif /* Sensor direct channels require uncached allocations. */ if (usage & GRALLOC_USAGE_SENSOR_DIRECT_DATA) { @@ -295,13 +227,6 @@ static unsigned int select_heap_mask(uint64_t usage) heap_mask = EXYNOS_ION_HEAP_VIDEO_FRAME_MASK; } } - /* TODO: used for exynos3830. Add this as a an option to Android.bp */ -#if defined(GRALLOC_SCALER_WFD) && GRALLOC_SCALER_WFD == 1 - else if (usage & GRALLOC_USAGE_PRIVATE_NONSECURE && usage & GRALLOC_USAGE_HW_COMPOSER) - { - heap_mask = EXYNOS_ION_HEAP_EXT_UI_MASK; - } -#endif else if (usage & GRALLOC_USAGE_SENSOR_DIRECT_DATA) { heap_mask = EXYNOS_ION_HEAP_SENSOR_DIRECT_MASK; @@ -372,12 +297,10 @@ int ion_device::alloc_from_dmabuf_heap(const std::string& heap_name, size_t size return shared_fd; } -int ion_device::alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, int *min_pgsz, - const std::string& buffer_name) +int ion_device::alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name) { ATRACE_CALL(); - /* TODO: remove min_pgsz? I don't think this is useful on Exynos */ - if (size == 0 || min_pgsz == NULL) + if (size == 0) { return -1; } @@ -392,37 +315,12 @@ int ion_device::alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int fl } else { - if (ion_client < 0) - { - return -1; - } - - shared_fd = exynos_ion_alloc(ion_client, size, heap_mask, flags); + shared_fd = exynos_ion_alloc(0, size, heap_mask, flags); } - *min_pgsz = SZ_4K; - return shared_fd; } -int ion_device::open_and_query_ion() -{ - if (ion_client >= 0) - { - MALI_GRALLOC_LOGW("ION device already open"); - return 0; - } - - ion_client = exynos_ion_open(); - if (ion_client < 0) - { - MALI_GRALLOC_LOGE("ion_open failed with %s", strerror(errno)); - return -1; - } - - return 0; -} - static SyncType sync_type_for_flags(const bool read, const bool write) { if (read && !write) @@ -568,15 +466,14 @@ int mali_gralloc_ion_allocate_attr(private_handle_t *hnd) int idx = hnd->get_share_attr_fd_index(); int ion_flags = 0; - int min_pgsz; uint64_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN; ion_flags = ION_FLAG_CACHED; - hnd->fds[idx] = dev->alloc_from_ion_heap(usage, hnd->attr_size, ion_flags, &min_pgsz); + hnd->fds[idx] = dev->alloc_from_ion_heap(usage, hnd->attr_size, ion_flags); if (hnd->fds[idx] < 0) { - MALI_GRALLOC_LOGE("ion_alloc failed from client ( %d )", dev->client()); + MALI_GRALLOC_LOGE("ion_alloc failed"); return -1; } @@ -606,7 +503,6 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, uint64_t usage; uint32_t i; unsigned int ion_flags = 0; - int min_pgsz = 0; int fds[MAX_FDS]; std::fill(fds, fds + MAX_FDS, -1); @@ -630,11 +526,11 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, fds[fidx] = ion_fd; } else { fds[fidx] = dev->alloc_from_ion_heap(usage, bufDescriptor->alloc_sizes[fidx], ion_flags, - &min_pgsz, bufDescriptor->name); + bufDescriptor->name); } if (fds[fidx] < 0) { - MALI_GRALLOC_LOGE("ion_alloc failed from client ( %d )", dev->client()); + MALI_GRALLOC_LOGE("ion_alloc failed"); for (int cidx = 0; cidx < fidx; cidx++) { @@ -692,7 +588,7 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, if (MAP_FAILED == cpu_ptr) { - MALI_GRALLOC_LOGE("mmap failed from client ( %d ), fd ( %d )", dev->client(), hnd->fds[0]); + MALI_GRALLOC_LOGE("mmap failed for fd ( %d )", hnd->fds[0]); mali_gralloc_ion_free_internal(pHandle, numDescriptors); return -1; } @@ -760,59 +656,6 @@ int mali_gralloc_ion_map(private_handle_t *hnd) return 0; } -int import_exynos_ion_handles(private_handle_t *hnd) -{ - int retval = -1; - - ion_device *dev = ion_device::get(); - - for (int idx = 0; idx < hnd->fd_count; idx++) - { - if (hnd->fds[idx] >= 0) - { - retval = exynos_ion_import_handle(dev->client(), hnd->fds[idx], &hnd->ion_handles[idx]); - if (retval) - { - MALI_GRALLOC_LOGE("error importing ion_handle. ion_client(%d), ion_handle[%d](%d) format(%s %#" PRIx64 ")", - dev->client(), idx, hnd->ion_handles[idx], format_name(hnd->alloc_format), hnd->alloc_format); - goto error; - } - } - } - - return retval; - -error: - for (int idx = 0; idx < hnd->fd_count; idx++) - { - if (hnd->ion_handles[idx]) - { - exynos_ion_free_handle(dev->client(), hnd->ion_handles[idx]); - } - } - - return retval; -} - -void free_exynos_ion_handles(private_handle_t *hnd) -{ - ion_device *dev = ion_device::get(); - - for (int idx = 0; idx < hnd->fd_count; idx++) - { - if (hnd->ion_handles[idx]) - { - if (hnd->ion_handles[idx] && - exynos_ion_free_handle(dev->client(), hnd->ion_handles[idx])) - { - MALI_GRALLOC_LOGE("error freeing ion_handle. ion_client(%d), ion_handle[%d](%d) format(%s %#" PRIx64 ")", - dev->client(), idx, hnd->ion_handles[idx], format_name(hnd->alloc_format), hnd->alloc_format); - } - } - } -} - - void mali_gralloc_ion_unmap(private_handle_t *hnd) { for (int i = 0; i < hnd->fd_count; i++) @@ -838,9 +681,3 @@ void mali_gralloc_ion_unmap(private_handle_t *hnd) hnd->cpu_read = 0; hnd->cpu_write = 0; } - -void mali_gralloc_ion_close(void) -{ - ion_device::close(); -} - diff --git a/gralloc4/src/allocator/mali_gralloc_ion.h b/gralloc4/src/allocator/mali_gralloc_ion.h index 3877c5e..5f55c2f 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.h +++ b/gralloc4/src/allocator/mali_gralloc_ion.h @@ -32,9 +32,6 @@ int mali_gralloc_ion_sync_end(const private_handle_t * const hnd, const bool read, const bool write); int mali_gralloc_ion_map(private_handle_t *hnd); void mali_gralloc_ion_unmap(private_handle_t *hnd); -void mali_gralloc_ion_close(void); int mali_gralloc_attr_allocate(void); -int import_exynos_ion_handles(private_handle_t *hnd); -void free_exynos_ion_handles(private_handle_t *hnd); #endif /* MALI_GRALLOC_ION_H_ */ -- cgit v1.2.3 From eff215aefb27c8fe99e82c467372da2118ae247e Mon Sep 17 00:00:00 2001 From: Ankit Goyal Date: Fri, 31 Mar 2023 17:36:21 -0700 Subject: gralloc4: Import system and vscaler heap from libion system, system-uncached and vscaler were the only used heaps from libion. Bug: 275481134 Bug: 245053092 Test: Boot to home Change-Id: If0f58a77640e18f4f021fe87d90fdc3e20bca5f1 --- gralloc4/src/allocator/mali_gralloc_ion.cpp | 82 +++++++++-------------------- 1 file changed, 25 insertions(+), 57 deletions(-) diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp index 175626c..29920e0 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.cpp +++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp @@ -29,7 +29,6 @@ #include #include - #include #include #include @@ -62,6 +61,7 @@ static const char kDmabufFaceauthPrevHeapName[] = "faprev-secure"; static const char kDmabufFaceauthModelHeapName[] = "famodel-secure"; static const char kDmabufVframeSecureHeapName[] = "vframe-secure"; static const char kDmabufVstreamSecureHeapName[] = "vstream-secure"; +static const char kDmabufVscalerSecureHeapName[] = "vscaler-secure"; struct ion_device { @@ -81,18 +81,17 @@ struct ion_device } /* - * Identifies a heap and retrieves file descriptor from ION for allocation + * Identifies a heap and allocates from that heap * * @param usage [in] Producer and consumer combined usage. * @param size [in] Requested buffer size (in bytes). - * @param heap_type [in] Requested heap type. * @param flags [in] ION allocation attributes defined by ION_FLAG_*. * @buffer_name [in] Optional name specifying what the buffer is for. * * @return File handle which can be used for allocation, on success - * -1, otherwise. + * -EINVAL, otherwise. */ - int alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name = std::string()); + int alloc_from_dmabuf_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name = ""); /* * Signals the start or end of a region where the CPU is accessing a @@ -111,25 +110,6 @@ struct ion_device private: std::unique_ptr buffer_allocator; - - /* - * Allocates in the DMA-BUF heap with name @heap_name. If allocation fails from - * the DMA-BUF heap or if it does not exist, falls back to an ION heap of the - * same name. - * - * @param heap_name [in] DMA-BUF heap name for allocation - * @param size [in] Requested buffer size (in bytes). - * @param flags [in] ION allocation attributes defined by ION_FLAG_* to - * be used for ION allocations. Will not be used with - * DMA-BUF heaps since the framework does not support - * allocation flags. - * @buffer_name [in] Name specifying what the buffer is for. - * - * @return fd of the allocated buffer on success, -1 otherwise; - */ - - int alloc_from_dmabuf_heap(const std::string& heap_name, size_t size, unsigned int flags, - const std::string& buffer_name); }; static void set_ion_flags(uint64_t usage, unsigned int *ion_flags) @@ -249,8 +229,10 @@ static unsigned int select_heap_mask(uint64_t usage) * @heap_mask. * */ -static std::string select_dmabuf_heap(unsigned int heap_mask) +static std::string select_dmabuf_heap(unsigned int heap_mask, unsigned int ion_flags) { + bool cached = ion_flags & ION_FLAG_CACHED; + switch (heap_mask) { case EXYNOS_ION_HEAP_SENSOR_DIRECT_MASK: return kDmabufSensorDirectHeapName; @@ -264,24 +246,34 @@ static std::string select_dmabuf_heap(unsigned int heap_mask) return kDmabufFaceauthPrevHeapName; case EXYNOS_ION_HEAP_FA_MODEL_MASK: return kDmabufFaceauthModelHeapName; + case EXYNOS_ION_HEAP_VIDEO_SCALER_MASK: + return kDmabufVscalerSecureHeapName; case EXYNOS_ION_HEAP_VIDEO_FRAME_MASK: return kDmabufVframeSecureHeapName; case EXYNOS_ION_HEAP_VIDEO_STREAM_MASK: return kDmabufVstreamSecureHeapName; + case EXYNOS_ION_HEAP_SYSTEM_MASK: + return cached ? kDmabufSystemHeapName : kDmabufSystemUncachedHeapName; default: return {}; } } -int ion_device::alloc_from_dmabuf_heap(const std::string& heap_name, size_t size, - unsigned int flags, const std::string& buffer_name) +int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name) { - ATRACE_NAME(("alloc_from_dmabuf_heap " + heap_name).c_str()); - if (!buffer_allocator) - { - return -1; + ATRACE_CALL(); + if (size == 0) { return -1; } + if (!buffer_allocator) { return -1; } + + unsigned int heap_mask = select_heap_mask(usage); + + auto heap_name = select_dmabuf_heap(heap_mask, flags); + if (heap_name.empty()) { + MALI_GRALLOC_LOGW("No heap found for usage: %s (0x%" PRIx64 ")", describe_usage(usage).c_str(), usage); + return -EINVAL; } + ATRACE_NAME(("alloc_from_dmabuf_heap " + heap_name).c_str()); int shared_fd = buffer_allocator->Alloc(heap_name, size, flags); if (shared_fd < 0) { @@ -297,30 +289,6 @@ int ion_device::alloc_from_dmabuf_heap(const std::string& heap_name, size_t size return shared_fd; } -int ion_device::alloc_from_ion_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name) -{ - ATRACE_CALL(); - if (size == 0) - { - return -1; - } - - unsigned int heap_mask = select_heap_mask(usage); - - int shared_fd; - auto dmabuf_heap_name = select_dmabuf_heap(heap_mask); - if (!dmabuf_heap_name.empty()) - { - shared_fd = alloc_from_dmabuf_heap(dmabuf_heap_name, size, flags, buffer_name); - } - else - { - shared_fd = exynos_ion_alloc(0, size, heap_mask, flags); - } - - return shared_fd; -} - static SyncType sync_type_for_flags(const bool read, const bool write) { if (read && !write) @@ -470,7 +438,7 @@ int mali_gralloc_ion_allocate_attr(private_handle_t *hnd) ion_flags = ION_FLAG_CACHED; - hnd->fds[idx] = dev->alloc_from_ion_heap(usage, hnd->attr_size, ion_flags); + hnd->fds[idx] = dev->alloc_from_dmabuf_heap(usage, hnd->attr_size, ion_flags); if (hnd->fds[idx] < 0) { MALI_GRALLOC_LOGE("ion_alloc failed"); @@ -525,7 +493,7 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, if (ion_fd >= 0 && fidx == 0) { fds[fidx] = ion_fd; } else { - fds[fidx] = dev->alloc_from_ion_heap(usage, bufDescriptor->alloc_sizes[fidx], ion_flags, + fds[fidx] = dev->alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], ion_flags, bufDescriptor->name); } if (fds[fidx] < 0) -- cgit v1.2.3 From 8c6ee82767ba2155352e37ecd56ea4fbad1446a9 Mon Sep 17 00:00:00 2001 From: Ankit Goyal Date: Fri, 31 Mar 2023 18:33:50 -0700 Subject: gralloc4: Remove dependence on libion_google This patch also removes the allocation for NONSECURE usage bit as that is marked as invalid in mali_gralloc_usages.h Bug: 275481134 Bug: 245053092 Test: Boot to home Test: gfx-gralloc-alloc-test Change-Id: Id506dc806829b846562faed33d3270a17710f1d8 --- gralloc4/src/4.x/Android.bp | 1 - gralloc4/src/Android.bp | 1 - gralloc4/src/allocator/Android.bp | 1 - gralloc4/src/allocator/mali_gralloc_ion.cpp | 189 +++++++++------------------- 4 files changed, 62 insertions(+), 130 deletions(-) diff --git a/gralloc4/src/4.x/Android.bp b/gralloc4/src/4.x/Android.bp index 88c7a01..16eec6a 100644 --- a/gralloc4/src/4.x/Android.bp +++ b/gralloc4/src/4.x/Android.bp @@ -50,7 +50,6 @@ cc_defaults { "liblog", "libcutils", "libdmabufheap", - "libion_google", "libsync", "libutils", "libnativewindow", diff --git a/gralloc4/src/Android.bp b/gralloc4/src/Android.bp index 3511d12..a813fc8 100644 --- a/gralloc4/src/Android.bp +++ b/gralloc4/src/Android.bp @@ -68,7 +68,6 @@ cc_library_shared { "libhidlbase", "libhidltransport", "libnativewindow", - "libion_google", "android.hardware.graphics.common@1.2", "android.hardware.graphics.common-V4-ndk", "android.hardware.graphics.mapper@4.0", diff --git a/gralloc4/src/allocator/Android.bp b/gralloc4/src/allocator/Android.bp index 7a3cb31..fe42411 100644 --- a/gralloc4/src/allocator/Android.bp +++ b/gralloc4/src/allocator/Android.bp @@ -86,7 +86,6 @@ arm_gralloc_allocator_cc_defaults { "liblog", "libcutils", "libdmabufheap", - "libion_google", "libsync", "libutils", "libnativewindow", diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp index 29920e0..7cf81c6 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.cpp +++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp @@ -36,9 +36,6 @@ #include #include -#include -#include - #include #include "mali_gralloc_buffer.h" #include "gralloc_helper.h" @@ -85,13 +82,12 @@ struct ion_device * * @param usage [in] Producer and consumer combined usage. * @param size [in] Requested buffer size (in bytes). - * @param flags [in] ION allocation attributes defined by ION_FLAG_*. * @buffer_name [in] Optional name specifying what the buffer is for. * * @return File handle which can be used for allocation, on success * -EINVAL, otherwise. */ - int alloc_from_dmabuf_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name = ""); + int alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name = ""); /* * Signals the start or end of a region where the CPU is accessing a @@ -112,169 +108,116 @@ private: std::unique_ptr buffer_allocator; }; -static void set_ion_flags(uint64_t usage, unsigned int *ion_flags) -{ - if (ion_flags == nullptr) - return; - - if ((usage & GRALLOC_USAGE_SW_READ_MASK) == GRALLOC_USAGE_SW_READ_OFTEN) - { - *ion_flags |= ION_FLAG_CACHED; - } - - // DRM or Secure Camera - if (usage & (GRALLOC_USAGE_PROTECTED)) - { - *ion_flags |= ION_FLAG_PROTECTED; - } - /* Sensor direct channels require uncached allocations. */ - if (usage & GRALLOC_USAGE_SENSOR_DIRECT_DATA) - { - *ion_flags &= ~ION_FLAG_CACHED; - } -} - -static unsigned int select_faceauth_heap_mask(uint64_t usage) +static std::string select_dmabuf_heap(uint64_t usage) { struct HeapSpecifier { uint64_t usage_bits; // exact match required - unsigned int mask; + std::string name; }; - static constexpr std::array faceauth_heaps = + static const std::array faceauth_heaps = {{ + // Faceauth heaps { // isp_image_heap GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_CAMERA_WRITE | GS101_GRALLOC_USAGE_TPU_INPUT, - EXYNOS_ION_HEAP_FA_IMG_MASK + kDmabufFaceauthImgHeapName }, { // isp_internal_heap GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_CAMERA_WRITE | GRALLOC_USAGE_HW_CAMERA_READ, - EXYNOS_ION_HEAP_FA_RAWIMG_MASK + kDmabufFaceauthRawImgHeapName }, { // isp_preview_heap GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_CAMERA_WRITE | GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_TEXTURE, - EXYNOS_ION_HEAP_FA_PREV_MASK + kDmabufFaceauthPrevHeapName }, { // ml_model_heap GRALLOC_USAGE_PROTECTED | GS101_GRALLOC_USAGE_TPU_INPUT, - EXYNOS_ION_HEAP_FA_MODEL_MASK + kDmabufFaceauthModelHeapName }, { // tpu_heap GRALLOC_USAGE_PROTECTED | GS101_GRALLOC_USAGE_TPU_OUTPUT | GS101_GRALLOC_USAGE_TPU_INPUT, - EXYNOS_ION_HEAP_FA_TPU_MASK - } + kDmabufFaceauthTpuHeapName + }, }}; - for (const HeapSpecifier &heap : faceauth_heaps) - { - if (usage == heap.usage_bits) + static const std::array other_heaps = + {{ + // If GPU, use vframe-secure { - ALOGV("Using FaceAuth heap mask 0x%x for usage 0x%" PRIx64 "\n", - heap.mask, usage); - return heap.mask; - } - } - - return 0; -} + GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_TEXTURE, + kDmabufVframeSecureHeapName + }, + { + GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_RENDER, + kDmabufVframeSecureHeapName + }, -static unsigned int select_heap_mask(uint64_t usage) -{ - if (unsigned int faceauth_heap_mask = select_faceauth_heap_mask(usage); - faceauth_heap_mask != 0) - { - return faceauth_heap_mask; - } + // If HWC but not GPU + { + GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_COMPOSER, + kDmabufVscalerSecureHeapName + }, - unsigned int heap_mask; + // Catchall for protected + { + GRALLOC_USAGE_PROTECTED, + kDmabufVframeSecureHeapName + }, - if (usage & GRALLOC_USAGE_PROTECTED) - { - if (usage & GRALLOC_USAGE_PRIVATE_NONSECURE) + // Sensor heap { - heap_mask = EXYNOS_ION_HEAP_SYSTEM_MASK; - } - else if ((usage & GRALLOC_USAGE_HW_COMPOSER) && - !(usage & GRALLOC_USAGE_HW_TEXTURE) && - !(usage & GRALLOC_USAGE_HW_RENDER)) + GRALLOC_USAGE_SENSOR_DIRECT_DATA, + kDmabufSensorDirectHeapName + }, + + // Catchall to system { - heap_mask = EXYNOS_ION_HEAP_VIDEO_SCALER_MASK; + 0, + kDmabufSystemUncachedHeapName } - else + }}; + + for (const HeapSpecifier &heap : faceauth_heaps) + { + if (usage == heap.usage_bits) { - heap_mask = EXYNOS_ION_HEAP_VIDEO_FRAME_MASK; + return heap.name; } } - else if (usage & GRALLOC_USAGE_SENSOR_DIRECT_DATA) - { - heap_mask = EXYNOS_ION_HEAP_SENSOR_DIRECT_MASK; - } - else + + std::string heap_name; + for (const HeapSpecifier &heap : other_heaps) { - heap_mask = EXYNOS_ION_HEAP_SYSTEM_MASK; + if ((usage & heap.usage_bits) == heap.usage_bits) + { + heap_name = heap.name; + break; + } } - return heap_mask; -} + if (heap_name == kDmabufSystemUncachedHeapName && + ((usage & GRALLOC_USAGE_SW_READ_MASK) == GRALLOC_USAGE_SW_READ_OFTEN)) + heap_name = kDmabufSystemHeapName; -/* - * Selects a DMA-BUF heap name. - * - * @param heap_mask [in] heap_mask for which the equivalent DMA-BUF heap - * name must be found. - * - * @return the name of the DMA-BUF heap equivalent to the ION heap of mask - * @heap_mask. - * - */ -static std::string select_dmabuf_heap(unsigned int heap_mask, unsigned int ion_flags) -{ - bool cached = ion_flags & ION_FLAG_CACHED; - - switch (heap_mask) { - case EXYNOS_ION_HEAP_SENSOR_DIRECT_MASK: - return kDmabufSensorDirectHeapName; - case EXYNOS_ION_HEAP_FA_TPU_MASK: - return kDmabufFaceauthTpuHeapName; - case EXYNOS_ION_HEAP_FA_IMG_MASK: - return kDmabufFaceauthImgHeapName; - case EXYNOS_ION_HEAP_FA_RAWIMG_MASK: - return kDmabufFaceauthRawImgHeapName; - case EXYNOS_ION_HEAP_FA_PREV_MASK: - return kDmabufFaceauthPrevHeapName; - case EXYNOS_ION_HEAP_FA_MODEL_MASK: - return kDmabufFaceauthModelHeapName; - case EXYNOS_ION_HEAP_VIDEO_SCALER_MASK: - return kDmabufVscalerSecureHeapName; - case EXYNOS_ION_HEAP_VIDEO_FRAME_MASK: - return kDmabufVframeSecureHeapName; - case EXYNOS_ION_HEAP_VIDEO_STREAM_MASK: - return kDmabufVstreamSecureHeapName; - case EXYNOS_ION_HEAP_SYSTEM_MASK: - return cached ? kDmabufSystemHeapName : kDmabufSystemUncachedHeapName; - default: - return {}; - } + return heap_name; } -int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, unsigned int flags, const std::string& buffer_name) +int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name) { ATRACE_CALL(); if (size == 0) { return -1; } if (!buffer_allocator) { return -1; } - unsigned int heap_mask = select_heap_mask(usage); - - auto heap_name = select_dmabuf_heap(heap_mask, flags); + auto heap_name = select_dmabuf_heap(usage); if (heap_name.empty()) { MALI_GRALLOC_LOGW("No heap found for usage: %s (0x%" PRIx64 ")", describe_usage(usage).c_str(), usage); return -EINVAL; } ATRACE_NAME(("alloc_from_dmabuf_heap " + heap_name).c_str()); - int shared_fd = buffer_allocator->Alloc(heap_name, size, flags); + int shared_fd = buffer_allocator->Alloc(heap_name, size, 0); if (shared_fd < 0) { ALOGE("Allocation failed for heap %s error: %d\n", heap_name.c_str(), shared_fd); @@ -433,12 +376,9 @@ int mali_gralloc_ion_allocate_attr(private_handle_t *hnd) } int idx = hnd->get_share_attr_fd_index(); - int ion_flags = 0; uint64_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN; - ion_flags = ION_FLAG_CACHED; - - hnd->fds[idx] = dev->alloc_from_dmabuf_heap(usage, hnd->attr_size, ion_flags); + hnd->fds[idx] = dev->alloc_from_dmabuf_heap(usage, hnd->attr_size); if (hnd->fds[idx] < 0) { MALI_GRALLOC_LOGE("ion_alloc failed"); @@ -470,7 +410,6 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, unsigned int priv_heap_flag = 0; uint64_t usage; uint32_t i; - unsigned int ion_flags = 0; int fds[MAX_FDS]; std::fill(fds, fds + MAX_FDS, -1); @@ -485,16 +424,12 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, buffer_descriptor_t *bufDescriptor = (buffer_descriptor_t *)(descriptors[i]); usage = bufDescriptor->consumer_usage | bufDescriptor->producer_usage; - ion_flags = 0; - set_ion_flags(usage, &ion_flags); - for (int fidx = 0; fidx < bufDescriptor->fd_count; fidx++) { if (ion_fd >= 0 && fidx == 0) { fds[fidx] = ion_fd; } else { - fds[fidx] = dev->alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], ion_flags, - bufDescriptor->name); + fds[fidx] = dev->alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], bufDescriptor->name); } if (fds[fidx] < 0) { -- cgit v1.2.3 From 1c68ad1661d199a0b2baf71980d2c960f0ef706d Mon Sep 17 00:00:00 2001 From: Ankit Goyal Date: Fri, 31 Mar 2023 18:46:49 -0700 Subject: gralloc4: Remove redundant ion_device class and stale comments Bug: 275481134 Bug: 245053092 Test: Boot to home Change-Id: I042fb7fb0cbc9656b382ca78a9c6d6ce7004bfee --- gralloc4/src/allocator/mali_gralloc_ion.cpp | 124 +++++----------------------- 1 file changed, 19 insertions(+), 105 deletions(-) diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp index 7cf81c6..634e336 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.cpp +++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp @@ -60,62 +60,20 @@ static const char kDmabufVframeSecureHeapName[] = "vframe-secure"; static const char kDmabufVstreamSecureHeapName[] = "vstream-secure"; static const char kDmabufVscalerSecureHeapName[] = "vscaler-secure"; -struct ion_device -{ - static ion_device *get() - { - static ion_device dev; - if (!dev.buffer_allocator) - { - dev.buffer_allocator = std::make_unique(); - if (!dev.buffer_allocator) { - ALOGE("Unable to create BufferAllocator object"); - return nullptr; - } - } - - return &dev; - } +BufferAllocator& get_allocator() { + static BufferAllocator allocator; + return allocator; +} - /* - * Identifies a heap and allocates from that heap - * - * @param usage [in] Producer and consumer combined usage. - * @param size [in] Requested buffer size (in bytes). - * @buffer_name [in] Optional name specifying what the buffer is for. - * - * @return File handle which can be used for allocation, on success - * -EINVAL, otherwise. - */ - int alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name = ""); - - /* - * Signals the start or end of a region where the CPU is accessing a - * buffer, allowing appropriate cache synchronization. - * - * @param fd [in] fd for the buffer - * @param read [in] True if the CPU is reading from the buffer - * @param write [in] True if the CPU is writing to the buffer - * @param start [in] True if the CPU has not yet performed the - * operations; false if the operations are - * completed. - * - * @return 0 on success; an error code otherwise. - */ - int sync(int fd, bool read, bool write, bool start); - -private: - std::unique_ptr buffer_allocator; -}; - -static std::string select_dmabuf_heap(uint64_t usage) +std::string select_dmabuf_heap(uint64_t usage) { struct HeapSpecifier { - uint64_t usage_bits; // exact match required + uint64_t usage_bits; std::string name; }; + // exact match required static const std::array faceauth_heaps = {{ // Faceauth heaps @@ -204,11 +162,10 @@ static std::string select_dmabuf_heap(uint64_t usage) return heap_name; } -int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name) +int alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name = "") { ATRACE_CALL(); if (size == 0) { return -1; } - if (!buffer_allocator) { return -1; } auto heap_name = select_dmabuf_heap(usage); if (heap_name.empty()) { @@ -217,14 +174,14 @@ int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::s } ATRACE_NAME(("alloc_from_dmabuf_heap " + heap_name).c_str()); - int shared_fd = buffer_allocator->Alloc(heap_name, size, 0); + int shared_fd = get_allocator().Alloc(heap_name, size, 0); if (shared_fd < 0) { ALOGE("Allocation failed for heap %s error: %d\n", heap_name.c_str(), shared_fd); } if (!buffer_name.empty()) { - if (buffer_allocator->DmabufSetName(shared_fd, buffer_name)) { + if (get_allocator().DmabufSetName(shared_fd, buffer_name)) { ALOGW("Unable to set buffer name %s: %s", buffer_name.c_str(), strerror(errno)); } } @@ -232,7 +189,7 @@ int ion_device::alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::s return shared_fd; } -static SyncType sync_type_for_flags(const bool read, const bool write) +SyncType sync_type_for_flags(const bool read, const bool write) { if (read && !write) { @@ -249,24 +206,19 @@ static SyncType sync_type_for_flags(const bool read, const bool write) } } -int ion_device::sync(const int fd, const bool read, const bool write, const bool start) +int sync(const int fd, const bool read, const bool write, const bool start) { - if (!buffer_allocator) - { - return -1; - } - if (start) { - return buffer_allocator->CpuSyncStart(fd, sync_type_for_flags(read, write)); + return get_allocator().CpuSyncStart(fd, sync_type_for_flags(read, write)); } else { - return buffer_allocator->CpuSyncEnd(fd, sync_type_for_flags(read, write)); + return get_allocator().CpuSyncEnd(fd, sync_type_for_flags(read, write)); } } -static int mali_gralloc_ion_sync(const private_handle_t * const hnd, +int mali_gralloc_ion_sync(const private_handle_t * const hnd, const bool read, const bool write, const bool start) @@ -276,16 +228,10 @@ static int mali_gralloc_ion_sync(const private_handle_t * const hnd, return -EINVAL; } - ion_device *dev = ion_device::get(); - if (!dev) - { - return -1; - } - for (int i = 0; i < hnd->fd_count; i++) { const int fd = hnd->fds[i]; - if (const int ret = dev->sync(fd, read, write, start)) + if (const int ret = sync(fd, read, write, start)) { return ret; } @@ -295,16 +241,6 @@ static int mali_gralloc_ion_sync(const private_handle_t * const hnd, } -/* - * Signal start of CPU access to the DMABUF exported from ION. - * - * @param hnd [in] Buffer handle - * @param read [in] Flag indicating CPU read access to memory - * @param write [in] Flag indicating CPU write access to memory - * - * @return 0 in case of success - * errno for all error cases - */ int mali_gralloc_ion_sync_start(const private_handle_t * const hnd, const bool read, const bool write) @@ -313,16 +249,6 @@ int mali_gralloc_ion_sync_start(const private_handle_t * const hnd, } -/* - * Signal end of CPU access to the DMABUF exported from ION. - * - * @param hnd [in] Buffer handle - * @param read [in] Flag indicating CPU read access to memory - * @param write [in] Flag indicating CPU write access to memory - * - * @return 0 in case of success - * errno for all error cases - */ int mali_gralloc_ion_sync_end(const private_handle_t * const hnd, const bool read, const bool write) @@ -353,7 +279,7 @@ void mali_gralloc_ion_free(private_handle_t * const hnd) delete hnd; } -static void mali_gralloc_ion_free_internal(buffer_handle_t * const pHandle, +void mali_gralloc_ion_free_internal(buffer_handle_t * const pHandle, const uint32_t num_hnds) { for (uint32_t i = 0; i < num_hnds; i++) @@ -369,16 +295,11 @@ static void mali_gralloc_ion_free_internal(buffer_handle_t * const pHandle, int mali_gralloc_ion_allocate_attr(private_handle_t *hnd) { ATRACE_CALL(); - ion_device *dev = ion_device::get(); - if (!dev) - { - return -1; - } int idx = hnd->get_share_attr_fd_index(); uint64_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN; - hnd->fds[idx] = dev->alloc_from_dmabuf_heap(usage, hnd->attr_size); + hnd->fds[idx] = alloc_from_dmabuf_heap(usage, hnd->attr_size); if (hnd->fds[idx] < 0) { MALI_GRALLOC_LOGE("ion_alloc failed"); @@ -413,12 +334,6 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, int fds[MAX_FDS]; std::fill(fds, fds + MAX_FDS, -1); - ion_device *dev = ion_device::get(); - if (!dev) - { - return -1; - } - for (i = 0; i < numDescriptors; i++) { buffer_descriptor_t *bufDescriptor = (buffer_descriptor_t *)(descriptors[i]); @@ -429,7 +344,7 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, if (ion_fd >= 0 && fidx == 0) { fds[fidx] = ion_fd; } else { - fds[fidx] = dev->alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], bufDescriptor->name); + fds[fidx] = alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], bufDescriptor->name); } if (fds[fidx] < 0) { @@ -520,7 +435,6 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors, return 0; } - int mali_gralloc_ion_map(private_handle_t *hnd) { uint64_t usage = hnd->producer_usage | hnd->consumer_usage; -- cgit v1.2.3 From 4180098644e9f51ed1b4ce6c0581035c65883461 Mon Sep 17 00:00:00 2001 From: Ankit Goyal Date: Tue, 4 Apr 2023 15:54:09 -0700 Subject: gralloc4: Choose framebuffer-secure for FB if available Bug: 245053092 Test: gfx-gralloc-alloc-test Change-Id: If72289abf0dde9701456f0d6e3fb09885b6bf529 --- gralloc4/src/allocator/mali_gralloc_ion.cpp | 40 +++++++++++++++++++---------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp index 634e336..8bfc6d8 100644 --- a/gralloc4/src/allocator/mali_gralloc_ion.cpp +++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp @@ -59,12 +59,23 @@ static const char kDmabufFaceauthModelHeapName[] = "famodel-secure"; static const char kDmabufVframeSecureHeapName[] = "vframe-secure"; static const char kDmabufVstreamSecureHeapName[] = "vstream-secure"; static const char kDmabufVscalerSecureHeapName[] = "vscaler-secure"; +static const char kDmabufFramebufferSecureHeapName[] = "framebuffer-secure"; BufferAllocator& get_allocator() { static BufferAllocator allocator; return allocator; } +std::string find_first_available_heap(const std::initializer_list&& options) { + static auto available_heaps = BufferAllocator::GetDmabufHeapList(); + + for (const auto& heap: options) + if (available_heaps.find(heap) != available_heaps.end()) + return heap; + + return ""; +} + std::string select_dmabuf_heap(uint64_t usage) { struct HeapSpecifier @@ -73,8 +84,7 @@ std::string select_dmabuf_heap(uint64_t usage) std::string name; }; - // exact match required - static const std::array faceauth_heaps = + static const std::array exact_usage_heaps = {{ // Faceauth heaps { // isp_image_heap @@ -98,9 +108,15 @@ std::string select_dmabuf_heap(uint64_t usage) GRALLOC_USAGE_PROTECTED | GS101_GRALLOC_USAGE_TPU_OUTPUT | GS101_GRALLOC_USAGE_TPU_INPUT, kDmabufFaceauthTpuHeapName }, + + { + GRALLOC_USAGE_PROTECTED | GRALLOC_USAGE_HW_TEXTURE | GRALLOC_USAGE_HW_RENDER | + GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_FB, + find_first_available_heap({kDmabufFramebufferSecureHeapName, kDmabufVframeSecureHeapName}) + }, }}; - static const std::array other_heaps = + static const std::array inexact_usage_heaps = {{ // If GPU, use vframe-secure { @@ -137,7 +153,7 @@ std::string select_dmabuf_heap(uint64_t usage) } }}; - for (const HeapSpecifier &heap : faceauth_heaps) + for (const HeapSpecifier &heap : exact_usage_heaps) { if (usage == heap.usage_bits) { @@ -145,21 +161,19 @@ std::string select_dmabuf_heap(uint64_t usage) } } - std::string heap_name; - for (const HeapSpecifier &heap : other_heaps) + for (const HeapSpecifier &heap : inexact_usage_heaps) { if ((usage & heap.usage_bits) == heap.usage_bits) { - heap_name = heap.name; - break; + if (heap.name == kDmabufSystemUncachedHeapName && + ((usage & GRALLOC_USAGE_SW_READ_MASK) == GRALLOC_USAGE_SW_READ_OFTEN)) + return kDmabufSystemHeapName; + + return heap.name; } } - if (heap_name == kDmabufSystemUncachedHeapName && - ((usage & GRALLOC_USAGE_SW_READ_MASK) == GRALLOC_USAGE_SW_READ_OFTEN)) - heap_name = kDmabufSystemHeapName; - - return heap_name; + return ""; } int alloc_from_dmabuf_heap(uint64_t usage, size_t size, const std::string& buffer_name = "") -- cgit v1.2.3