Skip to content

Commit

Permalink
Remove redundant static member m_prelimEnabled
Browse files Browse the repository at this point in the history 
The buffer manager detected whether the KMD supports prelim interface
and set `m_prelimEnabled` accordingly. Then the buffer manager diverged
in some operations by checking `m_prelimEnabled`. However,
`m_prelimEnabled` was a static member and kept on once it is set, which
means it would cache the result of previous detection. Assuming we have
two Intel GPUs, one (GPU A) supporting prelim interface while the other
one (GPU B) not, if we initialize the buffer manager on GPU A first, and
then initialize the buffer manager on GPU B in the same process, the
program will crash because it uses incorrect result of
`m_prelimEnabled`.

In fact, use of m_prelimEnabled is unnecessary. We can safely derive the
same semantics from checking nullness of the member `prelim` in the
buffer manager -- `prelim` would be non-null if and only if the KMD
supports prelim interface. So let's remove the redundant member.

Tracked-On: OAM-124779
Signed-off-by: Weifeng Liu <[email protected]>
  • Loading branch information
@phreer
phreer committed Sep 24, 2024
1 parent 496ef56 commit 1a9c71e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 32 deletions.
39 changes: 18 additions & 21 deletions media_softlet/linux/common/os/i915_production/mos_bufmgr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand All @@ -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]);
}

Expand Down
10 changes: 0 additions & 10 deletions media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@
} \
}

bool BufmgrPrelim::m_prelimEnabled = false;

BufmgrPrelim *BufmgrPrelim::CreatePrelim(int fd)
{
if (fd < 0) {
Expand All @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion media_softlet/linux/common/os/i915_production/mos_bufmgr_prelim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 1a9c71e

Please sign in to comment.