From c31768ec15c75d4b8b3323b88fbc8c085dcb0041 Mon Sep 17 00:00:00 2001 From: Zephyron Date: Sat, 8 Feb 2025 19:29:49 +1000 Subject: [PATCH] core: improve nvdrv and buffer queue implementations Buffer Queue Changes: - Add assertion for buffer state in free slots - Improve error handling for buffer dequeuing - Add buffer count validation checks - Update log levels for better diagnostics NVDRV Changes: - Add host1x reference to nvhost_nvdec_common - Improve ioctl error reporting with more detailed messages - Reorder function declarations in nvhost_ctrl_gpu - Add stub for unimplemented ioctl command 0x13 - Clean up initialization of boolean flags These changes improve error handling and debugging capabilities while adding additional safety checks for buffer management. The nvdrv interface is also made more robust with better error reporting and proper hardware access patterns. --- .../service/nvdrv/devices/nvhost_ctrl_gpu.cpp | 6 +++- .../service/nvdrv/devices/nvhost_ctrl_gpu.h | 3 +- .../nvdrv/devices/nvhost_nvdec_common.cpp | 5 ++-- .../nvdrv/devices/nvhost_nvdec_common.h | 1 + .../hle/service/nvdrv/devices/nvhost_vic.cpp | 1 + src/core/hle/service/nvdrv/nvdrv_interface.h | 2 +- .../nvnflinger/buffer_queue_producer.cpp | 28 ++++++++++++++----- 7 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.cpp b/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.cpp index a83cc6b6a..f03d80955 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.cpp @@ -74,6 +74,9 @@ NvResult nvhost_ctrl_gpu::Ioctl3(DeviceFD fd, Ioctl command, std::span case 0x6: return WrapFixedInlOut(this, &nvhost_ctrl_gpu::GetTPCMasks3, input, output, inline_output); + case 0x13: + LOG_DEBUG(Service_NVDRV, "(STUBBED) called."); + return NvResult::NotImplemented; default: break; } @@ -81,7 +84,8 @@ NvResult nvhost_ctrl_gpu::Ioctl3(DeviceFD fd, Ioctl command, std::span default: break; } - UNIMPLEMENTED_MSG("Unimplemented ioctl={:08X}", command.raw); + UNIMPLEMENTED_MSG("Unimplemented ioctl={:08X}, group={:01X}, command={:01X}", command.raw, + command.group, command.cmd); return NvResult::NotImplemented; } diff --git a/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.h b/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.h index 7258c362c..0bfa47c03 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.h +++ b/src/core/hle/service/nvdrv/devices/nvhost_ctrl_gpu.h @@ -205,9 +205,10 @@ private: std::span gpu_characteristics); NvResult GetTPCMasks1(IoctlGpuGetTpcMasksArgs& params); + NvResult GetTpcMasks2(IoctlGetTpcMasks& params); NvResult GetTPCMasks3(IoctlGpuGetTpcMasksArgs& params, std::span tpc_mask); - NvResult GetTpcMasks2(IoctlGetTpcMasks& params); + NvResult GetActiveSlotMask(IoctlActiveSlotMask& params); NvResult ZCullGetCtxSize(IoctlZcullGetCtxSize& params); diff --git a/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.cpp b/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.cpp index a0a7bfa40..cc3737059 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.cpp @@ -55,8 +55,9 @@ std::size_t WriteVectors(std::span dst, const std::vector& src, std::size nvhost_nvdec_common::nvhost_nvdec_common(Core::System& system_, NvCore::Container& core_, NvCore::ChannelType channel_type_) - : nvdevice{system_}, core{core_}, syncpoint_manager{core.GetSyncpointManager()}, - nvmap{core.GetNvMapFile()}, channel_type{channel_type_} { + : nvdevice{system_}, host1x{system_.Host1x()}, core{core_}, + syncpoint_manager{core.GetSyncpointManager()}, nvmap{core.GetNvMapFile()}, + channel_type{channel_type_} { auto& syncpts_accumulated = core.Host1xDeviceFile().syncpts_accumulated; if (syncpts_accumulated.empty()) { channel_syncpoint = syncpoint_manager.AllocateSyncpoint(false); diff --git a/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.h b/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.h index 900db81d2..63e637760 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.h +++ b/src/core/hle/service/nvdrv/devices/nvhost_nvdec_common.h @@ -119,6 +119,7 @@ protected: Kernel::KEvent* QueryEvent(u32 event_id) override; + Tegra::Host1x::Host1x& host1x; u32 channel_syncpoint; s32_le nvmap_fd{}; u32_le submit_timeout{}; diff --git a/src/core/hle/service/nvdrv/devices/nvhost_vic.cpp b/src/core/hle/service/nvdrv/devices/nvhost_vic.cpp index bf090f5eb..b077b33e5 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_vic.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_vic.cpp @@ -7,6 +7,7 @@ #include "core/hle/service/nvdrv/core/container.h" #include "core/hle/service/nvdrv/devices/ioctl_serialization.h" #include "core/hle/service/nvdrv/devices/nvhost_vic.h" +#include "video_core/host1x/host1x.h" #include "video_core/renderer_base.h" namespace Service::Nvidia::Devices { diff --git a/src/core/hle/service/nvdrv/nvdrv_interface.h b/src/core/hle/service/nvdrv/nvdrv_interface.h index c9cbb182b..9a79f598c 100644 --- a/src/core/hle/service/nvdrv/nvdrv_interface.h +++ b/src/core/hle/service/nvdrv/nvdrv_interface.h @@ -38,7 +38,7 @@ private: std::shared_ptr nvdrv; u64 pid{}; - bool is_initialized{false}; + bool is_initialized{}; NvCore::SessionId session_id{}; Common::ScratchBuffer output_buffer; Common::ScratchBuffer inline_output_buffer; diff --git a/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp b/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp index 6f2b7cbbb..da196b48f 100644 --- a/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp +++ b/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp @@ -138,6 +138,7 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St // Free up any buffers that are in slots beyond the max buffer count for (s32 s = max_buffer_count; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) { + ASSERT(slots[s].buffer_state == BufferState::Free); if (slots[s].graphic_buffer != nullptr && slots[s].buffer_state == BufferState::Free && !slots[s].is_preallocated) { core->FreeBufferLocked(s); @@ -176,10 +177,26 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St return Status::InvalidOperation; } - // Handle queue size limits + // See whether a buffer has been queued since the last SetBufferCount so we know whether to + // perform the min undequeued buffers check below + if (core->buffer_has_been_queued) { + // Make sure the producer is not trying to dequeue more buffers than allowed + const s32 new_undequeued_count = max_buffer_count - (dequeued_count + 1); + const s32 min_undequeued_count = core->GetMinUndequeuedBufferCountLocked(async); + if (new_undequeued_count < min_undequeued_count) { + LOG_ERROR(Service_Nvnflinger, + "min undequeued buffer count({}) exceeded (dequeued={} undequeued={})", + min_undequeued_count, dequeued_count, new_undequeued_count); + return Status::InvalidOperation; + } + } + + // If we disconnect and reconnect quickly, we can be in a state where our slots are empty + // but we have many buffers in the queue. This can cause us to run out of memory if we + // outrun the consumer. Wait here if it looks like we have too many buffers queued up. const bool too_many_buffers = core->queue.size() > static_cast(max_buffer_count); if (too_many_buffers) { - LOG_WARNING(Service_Nvnflinger, "Queue size {} exceeds max buffer count {}, waiting", + LOG_ERROR(Service_Nvnflinger, "Queue size {} exceeds max buffer count {}, waiting", core->queue.size(), max_buffer_count); } @@ -191,11 +208,8 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St } if (!core->WaitForDequeueCondition(lk)) { - if (core->is_abandoned) { - LOG_ERROR(Service_Nvnflinger, "BufferQueue was abandoned while waiting"); - return Status::NoInit; - } - continue; + // We are no longer running + return Status::NoError; } } }