From 601f2bf831fc793277aa8388fef8999757e42649 Mon Sep 17 00:00:00 2001 From: Weifeng Liu Date: Tue, 24 Sep 2024 02:20:46 +0000 Subject: [PATCH] Remove redundant static member m_prelimEnabled The buffer manager currently detects whether the KMD supports the prelim interface and sets the static member m_prelimEnabled accordingly. It then uses m_prelimEnabled to determine how to perform certain operations. However, since m_prelimEnabled is static, its value persists across all instances within the same process. This can lead to issues in multi-GPU environments. For example, consider two Intel GPUs: - GPU A supports the prelim interface. - GPU B does not support the prelim interface. If we initialize the buffer manager on GPU A first, m_prelimEnabled is set to true. When we subsequently initialize the buffer manager on GPU B within the same process, it incorrectly uses the cached value of m_prelimEnabled, potentially causing the program to crash due to incorrect assumptions about prelim support. To resolve this, we can eliminate the use of m_prelimEnabled. Instead, we can check the nullness of the instance member prelim within the buffer manager. The prelim member will be non-null if and only if the KMD supports the prelim interface for that specific GPU. This approach provides the same functionality without the drawbacks of using a static member. Tracked-On: OAM-124779 Signed-off-by: Weifeng Liu --- .../common/os/i915_production/mos_bufmgr.c | 39 +++++++++---------- .../os/i915_production/mos_bufmgr_prelim.cpp | 10 ----- .../os/i915_production/mos_bufmgr_prelim.h | 1 - 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/media_softlet/linux/common/os/i915_production/mos_bufmgr.c b/media_softlet/linux/common/os/i915_production/mos_bufmgr.c index 190bd49066..754c379f63 100644 --- a/media_softlet/linux/common/os/i915_production/mos_bufmgr.c +++ b/media_softlet/linux/common/os/i915_production/mos_bufmgr.c @@ -933,7 +933,7 @@ static uint8_t mos_gem_get_lmem_region_count(struct mos_bufmgr_gem *bufmgr_gem) { assert(nullptr != bufmgr_gem); - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->GetLmemRegionCount(); } @@ -973,7 +973,7 @@ mos_gem_bo_memzone_for_address(struct mos_bufmgr *bufmgr, uint64_t address) assert(nullptr != bufmgr); struct mos_bufmgr_gem *bufmgr_gem = (struct mos_bufmgr_gem *)bufmgr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->GetMemzoneForAddress(address); } if (address >= MEMZONE_DEVICE_START) @@ -1148,13 +1148,13 @@ mos_gem_bo_alloc_internal(struct mos_bufmgr *bufmgr, return nullptr; bo_gem->bo.size = bo_size; - bo_gem->mem_region = BufmgrPrelim::IsPrelimSupported() ? + bo_gem->mem_region = bufmgr_gem->prelim ? bufmgr_gem->prelim->GetSystemMemRegionId() : I915_MEMORY_CLASS_SYSTEM; if(bufmgr_gem->has_lmem && (alloc->ext.mem_type == MOS_MEMPOOL_VIDEOMEMORY || alloc->ext.mem_type == MOS_MEMPOOL_DEVICEMEMORY)) { - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { uint32_t handle; uint64_t size; uint32_t region; @@ -1783,7 +1783,7 @@ map_wc(struct mos_linux_bo *bo) memclear(mmap_arg); mmap_arg.handle = bo_gem->gem_handle; /* To indicate the uncached virtual mapping to KMD */ - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->SetMmapOffset(bo_gem->mem_region, mmap_arg); } else if (bufmgr_gem->has_lmem) { mmap_arg.flags = I915_MMAP_OFFSET_FIXED; @@ -1938,7 +1938,7 @@ drm_export int mos_gem_bo_map(struct mos_linux_bo *bo, int write_enable) memclear(mmap_arg); mmap_arg.handle = bo_gem->gem_handle; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->SetMmapOffset(bo_gem->mem_region, mmap_arg); } else if (bufmgr_gem->has_lmem) { mmap_arg.flags = I915_MMAP_OFFSET_FIXED; @@ -2062,7 +2062,7 @@ map_gtt(struct mos_linux_bo *bo) memclear(mmap_arg); mmap_arg.handle = bo_gem->gem_handle; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->SetMmapOffset(bo_gem->mem_region, mmap_arg); } else { mmap_arg.flags = I915_MMAP_OFFSET_FIXED; @@ -2448,7 +2448,7 @@ mos_bufmgr_gem_destroy(struct mos_bufmgr *bufmgr) mos_vma_heap_finish(&bufmgr_gem->vma_heap[MEMZONE_SYS]); mos_vma_heap_finish(&bufmgr_gem->vma_heap[MEMZONE_DEVICE]); - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->UninitVmaHeap(&bufmgr_gem->vma_heap[MEMZONE_PRIME]); BufmgrPrelim::DestroyPrelim(bufmgr_gem->prelim); @@ -2530,7 +2530,7 @@ do_bo_emit_reloc(struct mos_linux_bo *bo, uint32_t offset, bo_gem->relocs[bo_gem->reloc_count].target_handle = target_bo_gem->gem_handle; bo_gem->relocs[bo_gem->reloc_count].read_domains = read_domains; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { // if reloc handle is batch buffer itself, cannot set write domain bo_gem->relocs[bo_gem->reloc_count].write_domain = (bo_gem->bo.handle == target_bo_gem->gem_handle ? 0 : write_domain); } else { @@ -3312,7 +3312,7 @@ mos_gem_bo_check_mem_region_internal(struct mos_linux_bo *bo, struct mos_bo_gem *bo_gem = (struct mos_bo_gem *) bo; struct mos_bufmgr_gem *bufmgr_gem = (struct mos_bufmgr_gem *) bo->bufmgr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->CheckMemRegion(bo_gem->mem_region, mem_type); } @@ -3427,7 +3427,7 @@ mos_gem_bo_set_softpin(MOS_LINUX_BO *bo) if (!mos_gem_bo_is_softpin(bo)) { uint64_t offset = 0; - if (BufmgrPrelim::IsPrelimSupported() && bo_gem->mem_region == MEMZONE_PRIME) { + if (bufmgr_gem->prelim && bo_gem->mem_region == MEMZONE_PRIME) { offset = mos_gem_bo_vma_alloc(bo->bufmgr, (enum mos_memory_zone)bo_gem->mem_region, bo->size, PAGE_SIZE_2M); } else { uint64_t alignment = (bufmgr_gem->softpin_va1Malign) ? PAGE_SIZE_1M : PAGE_SIZE_64K; @@ -3509,7 +3509,7 @@ mos_gem_bo_create_from_prime(struct mos_bufmgr *bufmgr, int prime_fd, int size) bo_gem->has_error = false; bo_gem->reusable = false; bo_gem->use_48b_address_range = bufmgr_gem->bufmgr.bo_use_48b_address_range ? true : false; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bo_gem->mem_region = MEMZONE_PRIME; } @@ -4291,7 +4291,7 @@ mos_gem_get_memory_info(struct mos_bufmgr *bufmgr, char *info, uint32_t length) return -1; } - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->GetMemoryInfo(bufmgr_gem->has_lmem, info, length); } #endif @@ -4456,7 +4456,7 @@ mos_gem_context_create_shared(struct mos_bufmgr *bufmgr, if (ctx == nullptr || ctx->vm_id == INVALID_VM) return nullptr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->WaDisableSingleTimeline(bufmgr_gem->has_lmem, flags); } @@ -4517,7 +4517,7 @@ static int mos_bufmgr_query_engines_count(struct mos_bufmgr *bufmgr, assert(bufmgr); assert(nengine); struct mos_bufmgr_gem *bufmgr_gem = (struct mos_bufmgr_gem *)bufmgr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->QueryEnginesCount(nengine); } int fd = ((struct mos_bufmgr_gem*)bufmgr)->fd; @@ -4587,7 +4587,7 @@ static int mos_bufmgr_query_engines(struct mos_bufmgr *bufmgr, struct drm_i915_query_engine_info *engines = nullptr; int ret, len; struct mos_bufmgr_gem *bufmgr_gem = (struct mos_bufmgr_gem *)bufmgr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return bufmgr_gem->prelim->QueryEngines(bufmgr_gem->has_lmem, engine_class, caps, @@ -4800,7 +4800,7 @@ static int mos_gem_set_context_param_parallel(struct mos_linux_context *ctx, } struct mos_bufmgr_gem *bufmgr_gem = (struct mos_bufmgr_gem *)ctx->bufmgr; - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { return BufmgrPrelim::SetContextParamParallel( ctx, ci, @@ -5436,9 +5436,6 @@ mos_bufmgr_gem_init_i915(int fd, int batch_size) if (nullptr != bufmgr_gem->prelim) { bufmgr_gem->prelim->Init(bufmgr_gem->bufmgr, bufmgr_gem->has_lmem); - } - - if (!BufmgrPrelim::IsPrelimSupported()) { bufmgr_gem->bufmgr.has_full_vd = true; } @@ -5459,7 +5456,7 @@ mos_bufmgr_gem_init_i915(int fd, int batch_size) bufmgr_gem->use_softpin = false; mos_vma_heap_init(&bufmgr_gem->vma_heap[MEMZONE_SYS], MEMZONE_SYS_START, MEMZONE_SYS_SIZE); mos_vma_heap_init(&bufmgr_gem->vma_heap[MEMZONE_DEVICE], MEMZONE_DEVICE_START, MEMZONE_DEVICE_SIZE); - if (BufmgrPrelim::IsPrelimSupported()) { + if (bufmgr_gem->prelim) { bufmgr_gem->prelim->InitVmaHeap(&bufmgr_gem->vma_heap[MEMZONE_PRIME]); } diff --git a/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.cpp b/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.cpp index 945e30b86f..93062654c6 100644 --- a/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.cpp +++ b/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.cpp @@ -88,8 +88,6 @@ } \ } -bool BufmgrPrelim::m_prelimEnabled = false; - BufmgrPrelim *BufmgrPrelim::CreatePrelim(int fd) { if (fd < 0) { @@ -110,19 +108,11 @@ BufmgrPrelim *BufmgrPrelim::CreatePrelim(int fd) if (0 == drmIoctl(fd, DRM_IOCTL_I915_QUERY, &q) && item.length > 0) { prelim = new BufmgrPrelim(fd); - if (nullptr != prelim) { - m_prelimEnabled = true; - } } return prelim; } -bool BufmgrPrelim::IsPrelimSupported() -{ - return m_prelimEnabled; -} - void BufmgrPrelim::DestroyPrelim(BufmgrPrelim *prelim) { if (nullptr != prelim) { diff --git a/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.h b/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.h index 33daa2e193..20e602a80b 100644 --- a/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.h +++ b/media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.h @@ -104,7 +104,6 @@ class BufmgrPrelim private: uint32_t m_tileId = 0; int m_fd = -1; - static bool m_prelimEnabled; static constexpr uint32_t TYPE_DECIMAL = 10;