From 282ae8fa51e060e6d4ef026b734aa871b1b9331e Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sun, 6 Aug 2023 09:38:16 +0200 Subject: Query Cache: address issues --- src/video_core/query_cache/bank_base.h | 16 +++---- src/video_core/query_cache/query_base.h | 44 +++++++++--------- src/video_core/query_cache/query_cache.h | 66 +++++++++++++++------------ src/video_core/query_cache/query_cache_base.h | 8 ++-- src/video_core/query_cache/query_stream.h | 22 ++++++--- 5 files changed, 85 insertions(+), 71 deletions(-) (limited to 'src/video_core/query_cache') diff --git a/src/video_core/query_cache/bank_base.h b/src/video_core/query_cache/bank_base.h index 4246a609d..420927091 100644 --- a/src/video_core/query_cache/bank_base.h +++ b/src/video_core/query_cache/bank_base.h @@ -7,21 +7,19 @@ #include #include - #include "common/common_types.h" namespace VideoCommon { class BankBase { protected: - const size_t base_bank_size; - size_t bank_size; - std::atomic references; - size_t current_slot; + const size_t base_bank_size{}; + size_t bank_size{}; + std::atomic references{}; + size_t current_slot{}; public: - BankBase(size_t bank_size_) - : base_bank_size{bank_size_}, bank_size(bank_size_), references(0), current_slot(0) {} + explicit BankBase(size_t bank_size_) : base_bank_size{bank_size_}, bank_size(bank_size_) {} virtual ~BankBase() = default; @@ -58,11 +56,11 @@ public: bank_size = current_slot; } - constexpr bool IsClosed() { + bool IsClosed() const { return current_slot >= bank_size; } - bool IsDead() { + bool IsDead() const { return IsClosed() && references == 0; } }; diff --git a/src/video_core/query_cache/query_base.h b/src/video_core/query_cache/query_base.h index 0ae23af9f..993a13eac 100644 --- a/src/video_core/query_cache/query_base.h +++ b/src/video_core/query_cache/query_base.h @@ -9,28 +9,28 @@ namespace VideoCommon { enum class QueryFlagBits : u32 { - HasTimestamp = 1 << 0, ///< Indicates if this query has a tiemstamp. - IsFinalValueSynced = 1 << 1, ///< Indicates if the query has been synced in the host - IsHostSynced = 1 << 2, ///< Indicates if the query has been synced in the host - IsGuestSynced = 1 << 3, ///< Indicates if the query has been synced with the guest. - IsHostManaged = 1 << 4, ///< Indicates if this query points to a host query - IsRewritten = 1 << 5, ///< Indicates if this query was rewritten by another query - IsInvalidated = 1 << 6, ///< Indicates the value of th query has been nullified. - IsOrphan = 1 << 7, ///< Indicates the query has not been set by a guest query. - IsFence = 1 << 8, ///< Indicates the query is a fence. - IsQueuedForAsyncFlush = 1 <<9,///< Indicates that the query can be flushed at any moment + HasTimestamp = 1 << 0, ///< Indicates if this query has a timestamp. + IsFinalValueSynced = 1 << 1, ///< Indicates if the query has been synced in the host + IsHostSynced = 1 << 2, ///< Indicates if the query has been synced in the host + IsGuestSynced = 1 << 3, ///< Indicates if the query has been synced with the guest. + IsHostManaged = 1 << 4, ///< Indicates if this query points to a host query + IsRewritten = 1 << 5, ///< Indicates if this query was rewritten by another query + IsInvalidated = 1 << 6, ///< Indicates the value of th query has been nullified. + IsOrphan = 1 << 7, ///< Indicates the query has not been set by a guest query. + IsFence = 1 << 8, ///< Indicates the query is a fence. + IsQueuedForAsyncFlush = 1 << 9, ///< Indicates that the query can be flushed at any moment }; DECLARE_ENUM_FLAG_OPERATORS(QueryFlagBits) class QueryBase { public: - VAddr guest_address; - QueryFlagBits flags; - u64 value; + VAddr guest_address{}; + QueryFlagBits flags{}; + u64 value{}; protected: // Default constructor - QueryBase() : guest_address(0), flags{}, value{} {} + QueryBase() = default; // Parameterized constructor QueryBase(VAddr address, QueryFlagBits flags_, u64 value_) @@ -51,23 +51,21 @@ public: class HostQueryBase : public QueryBase { public: // Default constructor - HostQueryBase() - : QueryBase(0, QueryFlagBits::IsHostManaged | QueryFlagBits::IsOrphan, 0), start_bank_id{}, - size_banks{}, start_slot{}, size_slots{} {} + HostQueryBase() : QueryBase(0, QueryFlagBits::IsHostManaged | QueryFlagBits::IsOrphan, 0) {} // Parameterized constructor - HostQueryBase(bool isLong, VAddr address) + HostQueryBase(bool has_timestamp, VAddr address) : QueryBase(address, QueryFlagBits::IsHostManaged, 0), start_bank_id{}, size_banks{}, start_slot{}, size_slots{} { - if (isLong) { + if (has_timestamp) { flags |= QueryFlagBits::HasTimestamp; } } - u32 start_bank_id; - u32 size_banks; - size_t start_slot; - size_t size_slots; + u32 start_bank_id{}; + u32 size_banks{}; + size_t start_slot{}; + size_t size_slots{}; }; } // namespace VideoCommon \ No newline at end of file diff --git a/src/video_core/query_cache/query_cache.h b/src/video_core/query_cache/query_cache.h index f1393d5c7..042af053c 100644 --- a/src/video_core/query_cache/query_cache.h +++ b/src/video_core/query_cache/query_cache.h @@ -54,7 +54,7 @@ public: return new_id; } - bool HasPendingSync() override { + bool HasPendingSync() const override { return !pending_sync.empty(); } @@ -71,8 +71,10 @@ public: continue; } query.flags |= QueryFlagBits::IsHostSynced; - sync_values.emplace_back(query.guest_address, query.value, - True(query.flags & QueryFlagBits::HasTimestamp) ? 8 : 4); + sync_values.emplace_back(SyncValuesStruct{ + .address = query.guest_address, + .value = query.value, + .size = static_cast(True(query.flags & QueryFlagBits::HasTimestamp) ? 8 : 4)}); } pending_sync.clear(); if (sync_values.size() > 0) { @@ -90,15 +92,20 @@ class StubStreamer : public GuestStreamer { public: using RuntimeType = typename Traits::RuntimeType; - StubStreamer(size_t id_, RuntimeType& runtime_) : GuestStreamer(id_, runtime_) {} + StubStreamer(size_t id_, RuntimeType& runtime_, u32 stub_value_) + : GuestStreamer(id_, runtime_), stub_value{stub_value_} {} ~StubStreamer() override = default; size_t WriteCounter(VAddr address, bool has_timestamp, [[maybe_unused]] u32 value, std::optional subreport = std::nullopt) override { - size_t new_id = GuestStreamer::WriteCounter(address, has_timestamp, 1U, subreport); + size_t new_id = + GuestStreamer::WriteCounter(address, has_timestamp, stub_value, subreport); return new_id; } + +private: + u32 stub_value; }; template @@ -113,7 +120,7 @@ struct QueryCacheBase::QueryCacheBaseImpl { for (size_t i = 0; i < static_cast(QueryType::MaxQueryTypes); i++) { streamers[i] = runtime.GetStreamerInterface(static_cast(i)); if (streamers[i]) { - streamer_mask |= 1ULL << i; + streamer_mask |= 1ULL << streamers[i]->GetId(); } } } @@ -152,7 +159,7 @@ struct QueryCacheBase::QueryCacheBaseImpl { QueryCacheBase* owner; VideoCore::RasterizerInterface& rasterizer; Core::Memory::Memory& cpu_memory; - Traits::RuntimeType& runtime; + RuntimeType& runtime; Tegra::GPU& gpu; std::array(QueryType::MaxQueryTypes)> streamers; u64 streamer_mask; @@ -223,15 +230,11 @@ void QueryCacheBase::CounterReport(GPUVAddr addr, QueryType counter_type const bool is_fence = True(flags & QueryPropertiesFlags::IsAFence); size_t streamer_id = static_cast(counter_type); auto* streamer = impl->streamers[streamer_id]; - if (!streamer) [[unlikely]] { - if (has_timestamp) { - u64 timestamp = impl->gpu.GetTicks(); - gpu_memory->Write(addr + 8, timestamp); - gpu_memory->Write(addr, 1ULL); - } else { - gpu_memory->Write(addr, 1U); - } - return; + if (streamer == nullptr) [[unlikely]] { + counter_type = QueryType::Payload; + payload = 1U; + streamer_id = static_cast(counter_type); + streamer = impl->streamers[streamer_id]; } auto cpu_addr_opt = gpu_memory->GpuToCpuAddress(addr); if (!cpu_addr_opt) [[unlikely]] { @@ -403,12 +406,6 @@ bool QueryCacheBase::AccelerateHostConditionalRendering() { impl->runtime.EndHostConditionalRendering(); return false; } - /*if (!Settings::IsGPULevelHigh()) { - impl->runtime.EndHostConditionalRendering(); - return gpu_memory->IsMemoryDirty(regs.render_enable.Address(), 24, - VideoCommon::CacheType::BufferCache | - VideoCommon::CacheType::QueryCache); - }*/ const ComparisonMode mode = static_cast(regs.render_enable.mode); const GPUVAddr address = regs.render_enable.Address(); switch (mode) { @@ -442,6 +439,9 @@ bool QueryCacheBase::AccelerateHostConditionalRendering() { // Async downloads template void QueryCacheBase::CommitAsyncFlushes() { + // Make sure to have the results synced in Host. + NotifyWFI(); + u64 mask{}; { std::scoped_lock lk(impl->flush_guard); @@ -458,8 +458,19 @@ void QueryCacheBase::CommitAsyncFlushes() { if (mask == 0) { return; } - impl->ForEachStreamerIn(mask, - [](StreamerInterface* streamer) { streamer->PushUnsyncedQueries(); }); + u64 ran_mask = ~mask; + while (mask) { + impl->ForEachStreamerIn(mask, [&mask, &ran_mask](StreamerInterface* streamer) { + u64 dep_mask = streamer->GetDependentMask(); + if ((dep_mask & ~ran_mask) != 0) { + return; + } + u64 index = streamer->GetId(); + ran_mask |= (1ULL << index); + mask &= ~(1ULL << index); + streamer->PushUnsyncedQueries(); + }); + } } template @@ -489,13 +500,11 @@ void QueryCacheBase::PopAsyncFlushes() { if (mask == 0) { return; } - u64 ran_mask = 0; - u64 next_phase = 0; + u64 ran_mask = ~mask; while (mask) { - impl->ForEachStreamerIn(mask, [&mask, &ran_mask, &next_phase](StreamerInterface* streamer) { + impl->ForEachStreamerIn(mask, [&mask, &ran_mask](StreamerInterface* streamer) { u64 dep_mask = streamer->GetDependenceMask(); if ((dep_mask & ~ran_mask) != 0) { - next_phase |= dep_mask; return; } u64 index = streamer->GetId(); @@ -503,7 +512,6 @@ void QueryCacheBase::PopAsyncFlushes() { mask &= ~(1ULL << index); streamer->PopUnsyncedQueries(); }); - ran_mask |= next_phase; } } diff --git a/src/video_core/query_cache/query_cache_base.h b/src/video_core/query_cache/query_cache_base.h index 55f508dd1..07be421c6 100644 --- a/src/video_core/query_cache/query_cache_base.h +++ b/src/video_core/query_cache/query_cache_base.h @@ -47,7 +47,7 @@ public: BitField<0, 27, u32> query_id; u32 raw; - std::pair unpack() { + std::pair unpack() const { return {static_cast(stream_id.Value()), static_cast(query_id.Value())}; } }; @@ -73,7 +73,7 @@ public: } } - static u64 BuildMask(std::span types) { + static u64 BuildMask(std::span types) { u64 mask = 0; for (auto query_type : types) { mask |= 1ULL << (static_cast(query_type)); @@ -160,7 +160,7 @@ protected: } } - using ContentCache = typename std::unordered_map>; + using ContentCache = std::unordered_map>; void InvalidateQuery(QueryLocation location); bool IsQueryDirty(QueryLocation location); @@ -175,7 +175,7 @@ protected: friend struct QueryCacheBaseImpl; friend RuntimeType; - std::unique_ptr impl; + std::unique_ptr impl; }; } // namespace VideoCommon \ No newline at end of file diff --git a/src/video_core/query_cache/query_stream.h b/src/video_core/query_cache/query_stream.h index 0e9275565..e7aac955b 100644 --- a/src/video_core/query_cache/query_stream.h +++ b/src/video_core/query_cache/query_stream.h @@ -16,7 +16,7 @@ namespace VideoCommon { class StreamerInterface { public: - StreamerInterface(size_t id_, u64 dependance_mask_ = 0) : id{id_}, dependance_mask{dependance_mask_} {} + explicit StreamerInterface(size_t id_) : id{id_}, dependence_mask{}, dependent_mask{} {} virtual ~StreamerInterface() = default; virtual QueryBase* GetQuery(size_t id) = 0; @@ -37,7 +37,7 @@ public: /* Do Nothing */ } - virtual bool HasPendingSync() { + virtual bool HasPendingSync() const { return false; } @@ -52,7 +52,7 @@ public: virtual size_t WriteCounter(VAddr address, bool has_timestamp, u32 value, std::optional subreport = std::nullopt) = 0; - virtual bool HasUnsyncedQueries() { + virtual bool HasUnsyncedQueries() const { return false; } @@ -71,18 +71,28 @@ public: } u64 GetDependenceMask() const { - return dependance_mask; + return dependence_mask; + } + + u64 GetDependentMask() const { + return dependence_mask; } protected: + void MakeDependent(StreamerInterface* depend_on) { + dependence_mask |= 1ULL << depend_on->id; + depend_on->dependent_mask |= 1ULL << id; + } + const size_t id; - const u64 dependance_mask; + u64 dependence_mask; + u64 dependent_mask; }; template class SimpleStreamer : public StreamerInterface { public: - SimpleStreamer(size_t id_, u64 dependance_mask_ = 0) : StreamerInterface{id_, dependance_mask_} {} + explicit SimpleStreamer(size_t id_) : StreamerInterface{id_} {} virtual ~SimpleStreamer() = default; protected: -- cgit v1.2.3