From 0819f1ef1564bbb05e37ab9a436f790083a4d3f0 Mon Sep 17 00:00:00 2001 From: jpolo1224 Date: Thu, 13 Aug 2026 22:28:52 -0400 Subject: [PATCH] RSX: return the renderer to the 0.6 path, keeping the FIFO idle fix and ADPF Testers consistently report the best performance on the build with the 0.6 renderer, so 0.7's graphics work goes back out. The Arkham City measurement behind it (62.8 -> 51.2 ms) was one game on one device and did not survive contact with a wider set of hardware. Two files are kept from 0.7 because neither is render pass work and both are measured wins on their own: RSXFIFO's idle spin plus WFE park, which took ~11% of total CPU off sched_yield, and RSXThread's ADPF feed, without which the performance-hint setting reports nothing and does nothing. Everything else under Emu/RSX is byte-identical to 0.6. The removed work is not lost -- it is in c4b45eee2 and can come back a piece at a time with testing behind each one, which is how it should have gone in the first place. --- .../Emu/RSX/Common/texture_cache_predictor.h | 17 -- rpcs3/Emu/RSX/RSXZCULL.cpp | 54 ---- rpcs3/Emu/RSX/RSXZCULL.h | 16 -- rpcs3/Emu/RSX/VK/VKDraw.cpp | 147 +--------- rpcs3/Emu/RSX/VK/VKGSRender.cpp | 193 +------------ rpcs3/Emu/RSX/VK/VKGSRender.h | 28 -- rpcs3/Emu/RSX/VK/VKPresent.cpp | 34 --- rpcs3/Emu/RSX/VK/VKProgramPipeline.cpp | 8 +- rpcs3/Emu/RSX/VK/VKQueryPool.cpp | 134 +++------ rpcs3/Emu/RSX/VK/VKQueryPool.h | 15 - rpcs3/Emu/RSX/VK/VKRenderPass.cpp | 175 +----------- rpcs3/Emu/RSX/VK/VKRenderPass.h | 62 ----- rpcs3/Emu/RSX/VK/VKRenderTargets.cpp | 256 +----------------- rpcs3/Emu/RSX/VK/VKRenderTargets.h | 139 +--------- rpcs3/Emu/RSX/VK/vk_android_loader.cpp | 4 - rpcs3/Emu/RSX/VK/vk_android_loader.h | 1 - rpcs3/Emu/RSX/VK/vkutils/barriers.cpp | 134 ++------- rpcs3/Emu/RSX/VK/vkutils/chip_class.h | 26 -- rpcs3/Emu/RSX/VK/vkutils/device.h | 19 -- rpcs3/Emu/RSX/VK/vkutils/instance.cpp | 36 +-- rpcs3/Emu/RSX/VK/vkutils/memory.cpp | 27 -- rpcs3/Emu/RSX/VK/vkutils/swapchain.cpp | 184 +------------ .../Emu/RSX/VK/vkutils/swapchain_android.hpp | 43 --- rpcs3/Emu/RSX/VK/vkutils/swapchain_core.h | 19 +- rpcs3/Emu/RSX/rsx_profiler.cpp | 43 +-- rpcs3/Emu/RSX/rsx_profiler.h | 26 +- 26 files changed, 100 insertions(+), 1740 deletions(-) diff --git a/rpcs3/Emu/RSX/Common/texture_cache_predictor.h b/rpcs3/Emu/RSX/Common/texture_cache_predictor.h index 354f9386b..bfa500ef1 100644 --- a/rpcs3/Emu/RSX/Common/texture_cache_predictor.h +++ b/rpcs3/Emu/RSX/Common/texture_cache_predictor.h @@ -122,30 +122,13 @@ namespace rsx texture_cache_predictor_entry_history_queue write_history; static const u32 max_confidence = 8; // Cannot be more "confident" than this value -#ifdef __ANDROID__ - // Mobile-tiler tuning. Here the GPU is usually idle while the CPU stalls on synchronous - // colour-buffer readbacks (Write Color Buffers titles such as Demon's Souls). With no GPU - // contention a speculative pre-flush is close to free, so engage the predictor sooner: - // cross the threshold on the first repeat of a stable readback region, so the pre-flush - // fires at framebuffer setup and the later CPU read finds an already-signalled fence - // instead of spinning on a fresh GPU copy. - static const u32 confident_threshold = 4; - static const u32 starting_confidence = 4; -#else static const u32 confident_threshold = 6; // We are confident if confidence >= confidence_threshold static const u32 starting_confidence = 3; -#endif static const u32 confidence_guessed_flush = 2; // Confidence granted when we correctly guess there will be a flush static const u32 confidence_guessed_no_flush = 1; // Confidence granted when we correctly guess there won't be a flush static const u32 confidence_incorrect_guess = -2; // Confidence granted when our guess is incorrect -#ifdef __ANDROID__ - // A wrong speculative flush costs little on an idle GPU, so do not punish it into a - // multi-frame confidence rebuild. - static const u32 confidence_mispredict = -2; -#else static const u32 confidence_mispredict = -4; // Confidence granted when a speculative flush is incorrect -#endif u32 confidence; diff --git a/rpcs3/Emu/RSX/RSXZCULL.cpp b/rpcs3/Emu/RSX/RSXZCULL.cpp index 2010e6974..51052c887 100644 --- a/rpcs3/Emu/RSX/RSXZCULL.cpp +++ b/rpcs3/Emu/RSX/RSXZCULL.cpp @@ -445,33 +445,6 @@ namespace rsx u32 processed = 0; const bool has_unclaimed = (m_pending_writes.back().sink == 0); - // Batch-prefetch every GPU occlusion result this drain is about to read, in one round - // trip. The VK backend collapses N blocking reads into a single copy + fence and primes - // its per-query cache; other backends no-op and the loop below reads per-query as before. - // The filter is implemented && num_draws, a superset of what the loop actually reads -- - // the dynamic have_result early-out only skips some, costing at most a wasted copy. - { - std::vector prefetch_set; - prefetch_set.reserve(m_pending_writes.size()); - - for (auto& writer : m_pending_writes) - { - if (!writer.sink) - break; - - auto query = writer.query; - if (!query || !query->num_draws) - continue; - - if (writer.type == CELL_GCM_ZPASS_PIXEL_CNT || writer.type == CELL_GCM_ZCULL_STATS3) - prefetch_set.push_back(query); - } - - if (prefetch_set.size() > 1) - prefetch_occlusion_query_results(prefetch_set); - } - - // Write all claimed reports unconditionally for (auto& writer : m_pending_writes) { @@ -606,33 +579,6 @@ namespace rsx } u32 processed = 0; - - // Batch-prefetch every GPU occlusion result this drain is about to read, in one round - // trip. The VK backend collapses N blocking reads into a single copy + fence and primes - // its per-query cache; other backends no-op and the loop below reads per-query as before. - // The filter is implemented && num_draws, a superset of what the loop actually reads -- - // the dynamic have_result early-out only skips some, costing at most a wasted copy. - { - std::vector prefetch_set; - prefetch_set.reserve(m_pending_writes.size()); - - for (auto& writer : m_pending_writes) - { - if (!writer.sink) - break; - - auto query = writer.query; - if (!query || !query->num_draws) - continue; - - if (writer.type == CELL_GCM_ZPASS_PIXEL_CNT || writer.type == CELL_GCM_ZCULL_STATS3) - prefetch_set.push_back(query); - } - - if (prefetch_set.size() > 1) - prefetch_occlusion_query_results(prefetch_set); - } - for (auto& writer : m_pending_writes) { if (!writer.sink) diff --git a/rpcs3/Emu/RSX/RSXZCULL.h b/rpcs3/Emu/RSX/RSXZCULL.h index 79cf58bae..d44cba5c9 100644 --- a/rpcs3/Emu/RSX/RSXZCULL.h +++ b/rpcs3/Emu/RSX/RSXZCULL.h @@ -87,19 +87,8 @@ namespace rsx enum constants { -#ifdef __ANDROID__ - // Mobile-tiler tuning. The GPU finishes occlusion queries in microseconds but is - // otherwise idle (we are sync-bound, not GPU-bound), so the desktop cadence leaves - // completed results unharvested for up to 300us and the guest's Reports-area read - // then force-drains them one at a time -- the per-frame ZCULL hitch under Accurate - // ZCULL stats. Harvest 4x more often so ready results are picked up without blocking - // before the guest asks for them. - max_zcull_delay_us = 100, // Delay before a report update operation is forced to retire - min_zcull_tick_us = 25, // Default tick duration. To avoid hardware spam, we schedule peeks in multiples of this. -#else max_zcull_delay_us = 300, // Delay before a report update operation is forced to retire min_zcull_tick_us = 100, // Default tick duration. To avoid hardware spam, we schedule peeks in multiples of this. -#endif occlusion_query_count = 2048, // Number of occlusion query slots available. Real hardware actually has far fewer units before choking max_safe_queue_depth = 1792, // Number of in-flight queries before we start forcefully flushing data from the GPU device. max_stat_registers = 8192 // Size of the statistics cache @@ -214,11 +203,6 @@ namespace rsx virtual void end_occlusion_query(occlusion_query_info* /*query*/) {} virtual bool check_occlusion_query_status(occlusion_query_info* /*query*/) { return true; } virtual void get_occlusion_query_result(occlusion_query_info* query) { query->result = -1; } - - // Optional batch hint: the backend may fetch every result this drain is about to read in - // one round trip and prime its per-query cache, so the reads below are then free. Purely - // an optimization -- the default does nothing and the per-query path still works. - virtual void prefetch_occlusion_query_results(const std::vector&) {} virtual void discard_occlusion_query(occlusion_query_info* /*query*/) {} }; diff --git a/rpcs3/Emu/RSX/VK/VKDraw.cpp b/rpcs3/Emu/RSX/VK/VKDraw.cpp index c00c88cfa..ce7528477 100644 --- a/rpcs3/Emu/RSX/VK/VKDraw.cpp +++ b/rpcs3/Emu/RSX/VK/VKDraw.cpp @@ -46,32 +46,6 @@ namespace vk } } - // The render target this sampled image is, if it is one and if the sample park applies to it. - // Null on every other target, on every non-RTT, and whenever the tunable is off, so all the - // gating lives in one place and the switch below reads as a plain "park or do what we did". - static vk::render_target* sample_park_candidate(vk::image* raw, const rsx::sampled_image_descriptor_base* sampler_state) - { -#ifdef __ANDROID__ - if constexpr (vk::s_sample_park_frames == 0) - { - return nullptr; - } - - if (sampler_state->upload_context != rsx::texture_upload_context::framebuffer_storage) - { - return nullptr; - } - - // dynamic_cast rather than vk::as_rtt: as_rtt asserts, and a framebuffer_storage - // descriptor can still hand over a temporary subresource that is a plain viewable_image. - return dynamic_cast(raw); -#else - static_cast(raw); - static_cast(sampler_state); - return nullptr; -#endif - } - void validate_image_layout_for_read_access( vk::command_buffer& cmd, vk::image_view* view, @@ -100,33 +74,6 @@ namespace vk break; } - // A sample-parked surface arrives here on every sample after the first, because it - // was left in GENERAL instead of being moved to SHADER_READ_ONLY_OPTIMAL. - // - // It still needs a write -> read barrier whenever it has been written since the last - // one, and that barrier still has to end the open pass: an unbound RTT is not an - // attachment of the pass in flight, so VUID-vkCmdPipelineBarrier-image-04073 rules - // out keeping it. What it must NOT do is move the layout, or the surface un-parks and - // pays the return trip after all. - // - // The "has been written since" test is the load-bearing part. Without parking the - // layout itself answered it - already in SHADER_READ_ONLY_OPTIMAL meant already - // synchronized, so the second through five-hundredth draw sampling a shadow map fell - // through the default case and issued nothing. Leaving the surface in GENERAL erases - // that signal, and issuing a barrier per sampling draw instead of per write would be - // a catastrophic regression rather than a win. render_target carries the answer - // explicitly now; see sample_park_needs_read_barrier. - if (auto parked = sample_park_candidate(raw, sampler_state); - parked && raw->current_layout == vk::render_target::get_sample_park_layout()) - { - if (!parked->sample_park_needs_read_barrier()) - { - // Already synchronized for reading, and already in a layout that is legal to - // sample from. Nothing at all to record. - break; - } - } - // This was used in a cyclic ref before, but is missing a barrier // No need for a full stall, use a custom barrier instead VkPipelineStageFlags src_stage; @@ -146,74 +93,22 @@ namespace vk dst_stage |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; } - { - // Stay in the park layout if this surface is parked, otherwise the historical - // move to SHADER_READ_ONLY_OPTIMAL. Note this keeps oldLayout == newLayout in - // the parked case, which is the only form a barrier is allowed to take inside a - // render pass - not that it can stay inside one here, but it means the barrier - // carries no transition cost of its own either. - auto parked = sample_park_candidate(raw, sampler_state); - const bool stay_parked = parked && parked->try_arm_sample_park() && - raw->current_layout == vk::render_target::get_sample_park_layout(); + vk::insert_image_memory_barrier( + cmd, + raw->value, + raw->current_layout, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, + src_stage, dst_stage, + src_access, dst_access, + { raw->aspect(), 0, 1, 0, 1 }); - const VkImageLayout target_layout = stay_parked - ? vk::render_target::get_sample_park_layout() - : VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; - - vk::insert_image_memory_barrier( - cmd, - raw->value, - raw->current_layout, target_layout, - src_stage, dst_stage, - src_access, dst_access, - { raw->aspect(), 0, 1, 0, 1 }); - - raw->current_layout = target_layout; - - if (stay_parked) - { - parked->on_sample_park_synced(); - } - else if (parked) - { - parked->clear_sample_park(); - } - } + raw->current_layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; break; case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: ensure(sampler_state->upload_context == rsx::texture_upload_context::framebuffer_storage); if (!sampler_state->is_cyclic_reference) [[ likely ]] { - // Standard pre-read barrier, and the first half of the sample/rebind cycle. - // - // This transition is the floor. The surface was just rendered to, so the read - // needs a write -> read barrier, and a barrier cannot stay inside a pass for an - // image the pass does not have attached. What the park changes is the - // destination: GENERAL is legal to sample from and legal to attach, so the next - // bind has nothing to undo, whereas SHADER_READ_ONLY_OPTIMAL is not a legal - // attachment layout and forces a second teardown to leave it. - // - // The transition itself is issued through the same change_layout as before, so - // the access/stage scopes image_helpers derives for it are unchanged apart from - // the destination, and it is still charged to ImgHelper:43. - if (auto parked = sample_park_candidate(raw, sampler_state)) - { - if (parked->try_arm_sample_park()) - { - raw->change_layout(cmd, vk::render_target::get_sample_park_layout()); - parked->on_sample_park_synced(); - break; - } - - // Declined - bound, multisampled, or no renderer. Drop any window left over - // from an earlier park so the deadline can never outlive the layout it - // describes. Both readers already re-check current_layout, so this is - // tidiness rather than a fix, but it keeps the state machine to one rule: - // a live deadline always means the surface is in the park layout. - parked->clear_sample_park(); - } - + // Standard pre-read barrier. raw->change_layout(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); break; } @@ -235,11 +130,6 @@ void VKGSRender::begin_render_pass() get_render_pass(), m_draw_fbo->value, { positionu{0u, 0u}, sizeu{m_draw_fbo->width(), m_draw_fbo->height()} }); - - // Publish what this pass actually has attached, so a barrier that wants to stay inside it can - // check VUID-vkCmdPipelineBarrier-image-04073 rather than trust the caller. m_fbo_images is - // the exact list the framebuffer was built from a few lines up the call chain in prepare_rtts. - vk::set_renderpass_attachments(*m_current_command_buffer, m_fbo_images); } void VKGSRender::close_render_pass() @@ -1208,24 +1098,7 @@ void VKGSRender::emit_geometry(u32 sub_index) if (pass) { // Subpass mismatch, end it before proceeding - if (rsx::prof::enabled()) [[unlikely]] - { - rsx::prof::g_rp_sites[1]++; - - // Which of the two mismatches fired. The framebuffer changing is the game - // switching render target and nothing here can avoid it; the render pass handle - // changing under an unchanged framebuffer can only be the attachment layouts, - // since format and sample count cannot move without the framebuffer moving too. - // - // This site only became loud once parking stopped the bind-time layout change - // from ending the pass earlier in the draw, which left the previous draw's pass - // open to be torn down here instead. Whether that is the same teardown relocated - // or an extra one caused by layout churn is exactly what these two counters - // separate, and it decides whether parking longer is worth anything. - rsx::prof::g_rp_sites[(m_draw_fbo->value != fbo) ? 19 : 18]++; - } - - vk::end_renderpass(cmd); + if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[1]++; vk::end_renderpass(cmd); } // Starting a new renderpass should clobber dynamic state diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.cpp b/rpcs3/Emu/RSX/VK/VKGSRender.cpp index bd36944d1..5468b3fa7 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.cpp +++ b/rpcs3/Emu/RSX/VK/VKGSRender.cpp @@ -483,13 +483,6 @@ VKGSRender::VKGSRender(utils::serial* ar) noexcept : GSRender(ar) if (!m_swapchain->init(m_swapchain_dims.width, m_swapchain_dims.height)) { swapchain_unavailable = true; -#ifdef ANDROID - // The VkSurfaceKHR is bound to the ANativeWindow captured at create time, so a surface - // lost during boot-time init stays lost however often we re-query it. Flag it so the first - // reinitialize_swapchain() takes the recreate branch with the new window instead of - // soft-looping forever against a dead surface. - m_surface_lost = true; -#endif } else { @@ -522,37 +515,9 @@ VKGSRender::VKGSRender(utils::serial* ar) noexcept : GSRender(ar) m_occlusion_query_manager->set_control_flags(VK_QUERY_CONTROL_PRECISE_BIT, 0); } - // The vertex cache can only retain an entry while the ring memory naming it survives, so - // retention engages only once the ring holds the in-flight headroom plus another frame of - // geometry -- four frames at the current headroom. Arkham City peaks near 31MB of geometry a - // frame, needing ~124MB, so the 64MB default leaves retention permanently disengaged in - // exactly the heavy scenes that stand to gain from it. The heap is growable, but it grows on - // allocation pressure and wrapping relieves that pressure, so it never reaches a size that - // would let entries live: the initial size is the only lever. Scale it against the memory - // budget rather than taking a fixed 192MB, because that is a fifth of the floor budget we - // hand a 4GB phone. - u32 attrib_ring_size_m = VK_ATTRIB_RING_BUFFER_SIZE_M; - -#ifdef __ANDROID__ - { - // Flat, deliberately. This was first scaled off get_budgetable_device_memory, which was - // wrong twice over: that figure is a texture-cache quota the ring does not draw from, and - // it is derived from free memory *after* the emulator has taken its RAM, so it floors at - // 1024M on an 8GB device and every tier collapsed back to 64M. - // - // 192M covers a peak frame up to 48M against the 4x rule; measured peaks here run 11-32M. - // 128M would only just clear the observed 31.8M peak and would flap on anything heavier. - attrib_ring_size_m = 192u; - - // Named because the retention log reports the ring size it had to work with, and a reader - // otherwise cannot tell a ring that was sized down from one that was never sized up. - rsx_log.notice("Attribute ring sized to %uM.", attrib_ring_size_m); - } -#endif - // VRAM allocation // This first set is bound persistently, so grow notifications are enabled. - m_attrib_ring_info.create(VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT, attrib_ring_size_m * 0x100000, vk::heap_pool_default, "attrib buffer", 0x400000, VK_TRUE); + m_attrib_ring_info.create(VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT, VK_ATTRIB_RING_BUFFER_SIZE_M * 0x100000, vk::heap_pool_default, "attrib buffer", 0x400000, VK_TRUE); m_fragment_env_ring_info.create(VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, VK_UBO_RING_BUFFER_SIZE_M * 0x100000, vk::heap_pool_low_latency, "fragment env buffer", 0x10000, VK_TRUE); m_vertex_env_ring_info.create(VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, VK_UBO_RING_BUFFER_SIZE_M * 0x100000, vk::heap_pool_default, "vertex env buffer", 0x10000, VK_TRUE); m_fragment_texture_params_ring_info.create(VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, VK_UBO_RING_BUFFER_SIZE_M * 0x100000, vk::heap_pool_low_latency, "fragment texture params buffer", 0x10000, VK_TRUE); @@ -606,9 +571,7 @@ VKGSRender::VKGSRender(utils::serial* ar) noexcept : GSRender(ar) m_fragment_constants_buffer_info = { *m_fragment_constants_ring_info.heap, 0, VK_WHOLE_SIZE }; const auto& limits = m_device->gpu().get_limits(); - // Clamped to the device limit, not the ring size: views window across the ring (see - // upload_vertex_data), so a ring larger than one view is fine and is the normal case now. - m_texbuffer_view_size = std::min(limits.maxTexelBufferElements, attrib_ring_size_m * 0x100000u); + m_texbuffer_view_size = std::min(limits.maxTexelBufferElements, VK_ATTRIB_RING_BUFFER_SIZE_M * 0x100000u); // Initialize bulk allocators m_vertex_env_allocator = std::make_unique>( @@ -2926,85 +2889,6 @@ bool VKGSRender::check_occlusion_query_status(rsx::reports::occlusion_query_info return m_occlusion_query_manager->check_query_status(oldest); } -// Collapse the drain's N blocking per-query reads into one GPU copy plus a single fence wait, -// then prime the per-query cache so the reads that follow cost nothing. On a tiler each -// individual read is a full round trip, so N of them back to back is most of the ZCULL cost. -// Best-effort: on any bail the caller's per-query path still runs unchanged. -// Ported from ouroboros420/rpcsx (7ed3365bc). -void VKGSRender::prefetch_occlusion_query_results(const std::vector& queries) -{ - if (queries.size() < 2) - { - // Not worth a batch round trip; the per-query loop handles it with no extra hard sync. - return; - } - - std::vector indices; - indices.reserve(queries.size() * 4); - - bool needs_hard_sync = false; - - for (auto* query : queries) - { - if (!query) continue; - - auto& data = m_occlusion_map[query->driver_handle]; - if (data.indices.empty()) continue; - - // A query begun in the current command buffer has not been ENDED yet, so copying it - // would need a hard sync -- exactly what the per-query path deliberately avoids. Skip - // the whole batch rather than force one. - if (data.is_current(m_current_command_buffer)) - { - needs_hard_sync = true; - break; - } - - for (const auto id : data.indices) - { - indices.push_back(id); - } - } - - if (needs_hard_sync || indices.size() < 2) - { - return; - } - - // Sorted so the pool-aware coalescing in copy_query_results actually finds runs. - std::sort(indices.begin(), indices.end()); - indices.erase(std::unique(indices.begin(), indices.end()), indices.end()); - - const u64 required = indices.size() * 4ull; - - if (!m_occlusion_readback_buffer || m_occlusion_readback_buffer->size() < required) - { - const u64 alloc_size = std::max(required, 4096); - m_occlusion_readback_buffer = std::make_unique(*m_device, - alloc_size, - m_device->get_memory_mapping().host_visible_coherent, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, - VK_BUFFER_USAGE_TRANSFER_DST_BIT, 0, - VMM_ALLOCATION_POOL_SYSTEM); - } - - m_occlusion_query_manager->copy_query_results(*m_current_command_buffer, indices, m_occlusion_readback_buffer->value); - - // One fence wait drains the whole batch. flush_command_queue(true) submits and waits, which - // is the single round trip this exists to pay instead of N. - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_flush_sites[11]++; - flush_command_queue(true); - - if (auto* mapped = static_cast(m_occlusion_readback_buffer->map(0, required))) - { - for (usz i = 0; i < indices.size(); ++i) - { - m_occlusion_query_manager->prime_query_result(indices[i], mapped[i]); - } - - m_occlusion_readback_buffer->unmap(); - } -} - void VKGSRender::get_occlusion_query_result(rsx::reports::occlusion_query_info* query) { auto &data = m_occlusion_map[query->driver_handle]; @@ -3029,82 +2913,9 @@ void VKGSRender::get_occlusion_query_result(rsx::reports::occlusion_query_info* data.sync(); - // On a tile-based renderer the result is usually a whole tiling pass away, so this wait - // is long -- and it must stay interruptible. get_query_result() blocks without ever - // checking external_interrupt_lock, and a PPU thread that faults on RSX-guarded memory - // during that window spins in on_access_violation() waiting for an ack this thread can no - // longer give: PPU pinned in sched_yield, RSX parked in the query wait, presenting as a - // freeze. Wait here instead, servicing external interrupts the way the FIFO - // semaphore_acquire loop does, and only call get_query_result() once the value is ready. - // Ported from rfandango/rpcsx (a560768ce). - static const bool needs_interruptible_wait = vk::is_tile_based_renderer(vk::get_driver_vendor()); - - bool aborted = false; - // Gather data for (const auto occlusion_id : data.indices) { - if (needs_interruptible_wait) - { - u32 wait_iterations = 0; - bool rescued = false; - - while (!m_occlusion_query_manager->check_query_status(occlusion_id)) - { - // Rescue path. A query begun in the current command buffer is not even ENDED - // until close_and_submit_command_buffer(), so if the is_current() bookkeeping - // above mis-reported, the value can never arrive without a flush. Rather than - // pay a hard sync on every ZCULL read, give it a generous window then force the - // flush once. If the query still never readies after this fires, the GPU is not - // retiring work at all -- a driver hang, not a bookkeeping bug -- and this - // warning is the breadcrumb that tells the two apart. - if (!rescued && ++wait_iterations >= 4096) - { - rescued = true; - rsx_log.warning("ZCULL result did not arrive; forcing command flush (query=%d)", occlusion_id); - - std::lock_guard lock(m_flush_queue_mutex); - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_flush_sites[11]++; flush_command_queue(); - - if (m_flush_requests.pending()) - { - m_flush_requests.clear_pending_flag(); - } - - continue; - } - - // Not optional: a PPU thread that faults on RSX-guarded memory posts a flush - // request in on_access_violation() and spins in producer_wait() until this - // thread consumes it. Servicing only external_interrupt_lock is not enough -- - // that was a measured deadlock. The FIFO semaphore_acquire wait survives the - // same situation precisely because cpu_wait() makes this call. - on_semaphore_acquire_wait(); - - if (external_interrupt_lock) - { - wait_pause(); - } - else if (state & cpu_flag::exit) - { - // The result may never arrive during shutdown, so do not read it. - aborted = true; - break; - } - else - { - // Park at near-zero power; the architected event stream bounds the poll - // period to tens of microseconds. - utils::wait_for_event(); - } - } - - if (aborted) - { - break; - } - } - query->result += m_occlusion_query_manager->get_query_result(occlusion_id); if (query->result && !g_cfg.video.precise_zpass_count) { diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.h b/rpcs3/Emu/RSX/VK/VKGSRender.h index c2ab8341f..df1657654 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.h +++ b/rpcs3/Emu/RSX/VK/VKGSRender.h @@ -70,8 +70,6 @@ private: output_scaling_mode m_output_scaling{output_scaling_mode::bilinear}; std::unique_ptr m_cond_render_buffer; - // Host-visible scratch for the batched ZCULL readback. Allocated on first use. - std::unique_ptr m_occlusion_readback_buffer; u64 m_cond_render_sync_tag = 0; shared_mutex m_sampler_mutex; @@ -104,8 +102,6 @@ private: bool m_surface_lost = false; vk::instance m_instance; vk::render_device *m_device; - // Timestamp of the last periodic pipeline-cache serialize (see flip()). - u64 m_last_pipeline_cache_save_time = 0; //Vulkan internals std::unique_ptr m_occlusion_query_manager; @@ -148,23 +144,6 @@ private: rsx::simple_array m_flushable_data_heaps; // List of heaps that can be 'dirty' and need manual flush -#ifdef __ANDROID__ - // Cross-frame vertex cache retention. m_attrib_ring_info is a ring, so a cached - // offset_in_heap stays meaningful only until the ring laps back over it; the cache used to be - // purged every frame because nothing tracked that. See vertex_cache_on_frame_end(). - u64 m_vtx_heap_cursor = 0; // Monotonic byte count ever handed out by m_attrib_ring_info - usz m_vtx_heap_put = 0; // Ring PUT at the last cursor sample - u64 m_vtx_retain_floor = 0; // Entries stamped below this cursor value are evicted - u64 m_vtx_frame_base = 0; // Cursor as of the start of the current frame - u64 m_vtx_frame_peak = 0; // Largest single-frame consumption in the recent window - u32 m_vtx_peak_ttl = 0; // Frames left before the peak is allowed to decay - u64 m_vtx_reserve_bytes = 0; // Ring bytes withheld from the allocator (0 = retention off) - u64 m_vtx_frame_marks[8]{}; // Cursor at the end of each of the last 8 frames - u32 m_vtx_frame_index = 0; - bool m_vtx_retention_locked_out = false; - VkBuffer m_vtx_heap_handle = VK_NULL_HANDLE; // Detects a grow(), which discards the whole heap -#endif - VkDescriptorBufferInfoEx m_vertex_env_buffer_info {}; VkDescriptorBufferInfoEx m_fragment_env_buffer_info {}; VkDescriptorBufferInfoEx m_vertex_layout_stream_info {}; @@ -268,12 +247,6 @@ private: vk::vertex_upload_info upload_vertex_data(); rsx::simple_array m_scratch_mem; -#ifdef __ANDROID__ - void vertex_cache_sample_heap(); // Advance the ring cursor, and detect a heap reallocation - void vertex_cache_on_heap_reset(); // Drop everything; the heap the offsets named is gone - void vertex_cache_on_frame_end(); // Size the reservation and evict what it cannot cover -#endif - bool load_program(); void load_program_env(); void update_vertex_env(u32 id, const vk::vertex_upload_info& vertex_info); @@ -298,7 +271,6 @@ public: void end_occlusion_query(rsx::reports::occlusion_query_info* query) override; bool check_occlusion_query_status(rsx::reports::occlusion_query_info* query) override; void get_occlusion_query_result(rsx::reports::occlusion_query_info* query) override; - void prefetch_occlusion_query_results(const std::vector& queries) override; void discard_occlusion_query(rsx::reports::occlusion_query_info* query) override; // External callback in case we need to suddenly submit a commandlist unexpectedly, e.g in a violation handler diff --git a/rpcs3/Emu/RSX/VK/VKPresent.cpp b/rpcs3/Emu/RSX/VK/VKPresent.cpp index 58059a2ab..b551991b5 100644 --- a/rpcs3/Emu/RSX/VK/VKPresent.cpp +++ b/rpcs3/Emu/RSX/VK/VKPresent.cpp @@ -1,6 +1,5 @@ #include "stdafx.h" #include "VKGSRender.h" -#include "Emu/Cell/timers.hpp" #include "vkutils/buffer_object.h" #include "vkutils/memory.h" #include "Emu/RSX/Overlays/overlay_manager.h" @@ -244,27 +243,9 @@ void VKGSRender::advance_queued_frames() vk::remove_unused_framebuffers(); - // Vertex cache retention was removed in 0.7.1: reusing entries across frames handed - // draws stale geometry and was the cause of the flashing in Sonic Unleashed and others. - // Back to purging every frame, as 0.6 did. m_vertex_cache->purge(); - m_current_frame->tag_frame_end(); -#ifdef __ANDROID__ - // tag_frame_end() records PUT-1 for every managed heap, which is what frame_context_cleanup - // later publishes as that heap's GET once this frame retires - i.e. "the GPU is done with - // everything before here, reuse it". For the attribute ring that is exactly the statement that - // makes a retained entry unsafe, so publish the retention floor instead. It is always at or - // behind the value this would otherwise carry, so the allocator ends up strictly more - // conservative than before and in-flight frames stay protected as they already were. - if (m_vtx_reserve_bytes && m_attrib_ring_info.size()) - { - m_current_frame->heap_snapshot[&m_attrib_ring_info] = - static_cast(m_vtx_retain_floor % m_attrib_ring_info.size()); - } -#endif - // Throttle here rather than by accident. // // The queue used to stay at one entry because the poll above blocked until the oldest @@ -1239,19 +1220,4 @@ void VKGSRender::flip(const rsx::display_flip_info_t& info) if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_flush_sites[20]++; flush_command_queue(true); } } - - // Crash-resilient pipeline-cache persistence. The blob is otherwise only written on - // clean teardown, so a crash or an Android LMK kill mid-session loses the entire - // warmup -- and a cold-boot compile-burst crash then keeps every later boot cold too. - // save_pipeline_cache() skips the write when the cache size has not moved, so steady - // state costs one size query per interval. - if (const u64 now = get_system_time(); now >= m_last_pipeline_cache_save_time + 120'000'000) - { - m_last_pipeline_cache_save_time = now; - - if (m_device) - { - m_device->save_pipeline_cache(); - } - } } diff --git a/rpcs3/Emu/RSX/VK/VKProgramPipeline.cpp b/rpcs3/Emu/RSX/VK/VKProgramPipeline.cpp index a9132df9a..7ae53c4c5 100644 --- a/rpcs3/Emu/RSX/VK/VKProgramPipeline.cpp +++ b/rpcs3/Emu/RSX/VK/VKProgramPipeline.cpp @@ -269,17 +269,13 @@ namespace vk { VkGraphicsPipelineCreateInfo create_info = *p_graphics_info; create_info.layout = m_pipeline_layout; - // Shared driver cache, so the SPIR-V -> ISA compile is not redone every cold boot. - // Null when unavailable, which is exactly what was passed here before. - const VkPipelineCache pipe_cache = g_render_device ? g_render_device->get_pipeline_cache() : VK_NULL_HANDLE; - CHECK_RESULT(vkCreateGraphicsPipelines(m_device, pipe_cache, 1, &create_info, nullptr, &m_pipeline)); + CHECK_RESULT(vkCreateGraphicsPipelines(m_device, nullptr, 1, &create_info, nullptr, &m_pipeline)); } else { VkComputePipelineCreateInfo create_info = *p_compute_info; create_info.layout = m_pipeline_layout; - const VkPipelineCache pipe_cache = g_render_device ? g_render_device->get_pipeline_cache() : VK_NULL_HANDLE; - CHECK_RESULT(vkCreateComputePipelines(m_device, pipe_cache, 1, &create_info, nullptr, &m_pipeline)); + CHECK_RESULT(vkCreateComputePipelines(m_device, nullptr, 1, &create_info, nullptr, &m_pipeline)); } m_linked = true; diff --git a/rpcs3/Emu/RSX/VK/VKQueryPool.cpp b/rpcs3/Emu/RSX/VK/VKQueryPool.cpp index 9347f3753..90ad4460f 100644 --- a/rpcs3/Emu/RSX/VK/VKQueryPool.cpp +++ b/rpcs3/Emu/RSX/VK/VKQueryPool.cpp @@ -61,7 +61,6 @@ namespace vk owner = &dev; query_type = type; query_slot_status.resize(num_entries, {}); - tile_based_renderer = vk::is_tile_based_renderer(vk::get_driver_vendor()); for (unsigned i = 0; i < num_entries; ++i) { @@ -143,16 +142,6 @@ namespace vk { control_flags = control_; result_flags = result_; - - if (tile_based_renderer) - { - // PARTIAL_BIT exists so the poll loop can finish early the moment any sample is known - // to have passed. That can only happen on an immediate-mode renderer, where fragments - // rasterize as they arrive. A TBDR has nothing to report until the tiling pass ends, so - // the flag can never pay off here and only asks the driver for extra work. - // Ported from rfandango/rpcsx (a560768ce). - result_flags &= ~VK_QUERY_RESULT_PARTIAL_BIT; - } } void query_pool_manager::begin_query(vk::command_buffer& cmd, u32 index) @@ -244,28 +233,29 @@ namespace vk } } +#ifdef __ANDROID__ // Spin briefly, then hand the core back. // // A pure pause() spin is reasonable on a desktop, where the result lands in - // microseconds and there are cores to spare. On a tiled renderer the result is - // not available until the tile pass resolves, so the wait is far longer, and a - // handheld runs eleven hot emulator threads across five usable cores. Burning - // one of them on a spin costs an SPU thread that had real work to do. + // microseconds and there are cores to spare. On a tiled mobile GPU the + // result is not available until the tile pass resolves, so the wait is far + // longer, and this device runs eleven hot emulator threads across five + // usable cores. Burning one of them on a spin costs an SPU thread that had + // real work to do. // - // The short spin first keeps the fast case fast, since a result that is nearly - // ready still returns without a scheduler round trip. - // - // Gated on the driver vendor rather than __ANDROID__: an immediate-mode mobile - // GPU wants the desktop path, and a tiler on any other platform wants this one. - if (tile_based_renderer && spins >= 64) - { - std::this_thread::yield(); - } - else + // The short spin first keeps the fast case fast, since a result that is + // nearly ready still returns without a scheduler round trip. + if (spins < 64) { utils::pause(); } - + else + { + std::this_thread::yield(); + } +#else + utils::pause(); +#endif poke_query(query_info, index, result_flags); } } @@ -273,31 +263,6 @@ namespace vk return query_info.data; } - // Shared by both readback paths. vkCmdCopyQueryPoolResults MUST be recorded outside a render - // pass instance; inside one it is undefined behaviour, and a tiler does not tolerate what an - // IMR desktop GPU does. See the detailed history on get_query_result_indirect below. - void query_pool_manager::end_renderpass_for_readback(vk::command_buffer& cmd) - { - if (!vk::is_renderpass_open(cmd)) - { - return; - } - - // A query that began inside a render pass instance has to end inside that same instance. - // Ending the pass underneath an open one leaves it permanently unavailable: the driver - // never marks it ready, get_query_result spins on poke_query forever, and the RSX thread - // stops with audio and vblank still alive -- a hang, not a crash. The submit-time ensure() - // does not catch it either, because end_occlusion_query closes the query a moment later so - // the flag reads clear while the result never arrives. - if (cmd.flags & vk::command_buffer::cb_has_open_query) - { - vk::do_query_cleanup(cmd); - } - - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[2]++; - vk::end_renderpass(cmd); - } - void query_pool_manager::get_query_result_indirect(vk::command_buffer& cmd, u32 index, u32 count, VkBuffer dst, VkDeviceSize dst_offset) { // Not "technically supposed to" -- vkCmdCopyQueryPoolResults MUST be recorded outside a @@ -316,60 +281,31 @@ namespace vk // // The upstream comment feared the flush cost. It is paid only when a pass is actually // open, on a path that already stalls for a GPU result. - end_renderpass_for_readback(cmd); - - vkCmdCopyQueryPoolResults(cmd, *query_slot_status[index].pool, index, count, dst, dst_offset, 4, VK_QUERY_RESULT_WAIT_BIT); - } - - // Batched readback. Copies each result in `indices` order to `dst`, one 4-byte word per - // entry at its position in the list, coalescing contiguous index runs into single copies. - // - // Pool-aware on purpose: a numerically contiguous run can still span a pool reallocation, - // and copying across that boundary would read from the wrong VkQueryPool. Ported from - // ouroboros420/rpcsx (7ed3365bc), reusing our renderpass handling rather than theirs -- - // theirs omits the open-query cleanup above. - void query_pool_manager::copy_query_results(vk::command_buffer& cmd, const std::vector& indices, VkBuffer dst) - { - if (indices.empty()) + if (vk::is_renderpass_open(cmd)) { - return; - } - - end_renderpass_for_readback(cmd); - - const usz n = indices.size(); - - for (usz i = 0; i < n;) - { - const u32 base = indices[i]; - const auto* pool = query_slot_status[base].pool; - - usz j = i + 1; - while (j < n && indices[j] == indices[j - 1] + 1 && query_slot_status[indices[j]].pool == pool) + // A query that began inside a render pass instance has to end inside that same + // instance. Ending the pass underneath an open one leaves it permanently + // unavailable: the driver never marks it ready, so get_query_result spins on + // poke_query forever and the RSX thread stops with everything else still alive. + // + // Nothing catches it either. The submit-time ensure() in commands.cpp only checks + // that the query was closed, and end_occlusion_query does close it a moment later, + // so the flag is clear and the assert passes while the result never arrives. That + // cost Web of Shadows a hang here after the device loss below was fixed. + // + // Closing it first keeps begin and end within one pass, which is the ordering the + // spec asks for. The query is cut short, as it is anywhere do_query_cleanup is + // used, and a truncated occlusion result beats a stalled thread. + if (cmd.flags & vk::command_buffer::cb_has_open_query) { - j++; + vk::do_query_cleanup(cmd); } - const u32 count = static_cast(j - i); - - // The run is contiguous in both hardware index and list position, so query[base + k] - // lands at destination word (i + k). - vkCmdCopyQueryPoolResults(cmd, *query_slot_status[base].pool, base, count, dst, - static_cast(i) * 4, 4, VK_QUERY_RESULT_WAIT_BIT); - - i = j; + if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[2]++; + vk::end_renderpass(cmd); } - } - // Seed a slot from a value already read back on the host, so the next get_query_result - // returns it without issuing another blocking read. Mirrors poke_query's VK_SUCCESS path. - // Only valid for a value that was waited on (the copy above uses WAIT_BIT). - void query_pool_manager::prime_query_result(u32 index, u32 value) - { - auto& query = query_slot_status[index]; - query.ready = true; - query.data = value; - query.any_passed = (value != 0); + vkCmdCopyQueryPoolResults(cmd, *query_slot_status[index].pool, index, count, dst, dst_offset, 4, VK_QUERY_RESULT_WAIT_BIT); } void query_pool_manager::free_query(vk::command_buffer&/*cmd*/, u32 index) diff --git a/rpcs3/Emu/RSX/VK/VKQueryPool.h b/rpcs3/Emu/RSX/VK/VKQueryPool.h index 84c408969..ee2480b8f 100644 --- a/rpcs3/Emu/RSX/VK/VKQueryPool.h +++ b/rpcs3/Emu/RSX/VK/VKQueryPool.h @@ -1,6 +1,4 @@ #pragma once - -#include #include "VulkanAPI.h" #include @@ -48,13 +46,7 @@ namespace vk vk::render_device* owner = nullptr; std::vector query_slot_status; - // A tile-based renderer cannot produce a query result before its tiling pass resolves. - // Decided once, at construction. More precise than an __ANDROID__ check, which would - // misclassify an immediate-mode mobile part and miss a tiler on any other platform. - bool tile_based_renderer = false; - bool poke_query(query_slot_info& query, u32 index, VkQueryResultFlags flags); - void end_renderpass_for_readback(vk::command_buffer& cmd); void allocate_new_pool(vk::command_buffer& cmd); void reallocate_pool(vk::command_buffer& cmd); void run_pool_cleanup(); @@ -72,13 +64,6 @@ namespace vk u32 get_query_result(u32 index); void get_query_result_indirect(vk::command_buffer& cmd, u32 index, u32 count, VkBuffer dst, VkDeviceSize dst_offset); - // Batched readback: copy every result in `indices` to `dst` (4-byte word per entry, at its - // position in the list), coalescing contiguous same-pool runs into single copies. - void copy_query_results(vk::command_buffer& cmd, const std::vector& indices, VkBuffer dst); - - // Seed a slot from a host-read value so the next get_query_result needs no GPU round trip. - void prime_query_result(u32 index, u32 value); - u32 allocate_query(vk::command_buffer& cmd); void free_query(vk::command_buffer&/*cmd*/, u32 index); diff --git a/rpcs3/Emu/RSX/VK/VKRenderPass.cpp b/rpcs3/Emu/RSX/VK/VKRenderPass.cpp index 0c124afdc..21f780e70 100644 --- a/rpcs3/Emu/RSX/VK/VKRenderPass.cpp +++ b/rpcs3/Emu/RSX/VK/VKRenderPass.cpp @@ -15,25 +15,11 @@ namespace vk { VkRenderPass pass = VK_NULL_HANDLE; VkFramebuffer fbo = VK_NULL_HANDLE; - - // Images backing the attachments of this pass, published by whoever opened it. - // Empty means "unknown", which is treated as "covers nothing". - std::vector attachments; - }; - - // The last pass closed on a command buffer, for the redundancy check in begin_renderpass. - // Profiling only; never read for correctness. - struct closed_renderpass_info_t - { - VkRenderPass pass = VK_NULL_HANDLE; - VkFramebuffer fbo = VK_NULL_HANDLE; - const void* caller = nullptr; }; atomic_t g_cached_renderpass_key = 0; VkRenderPass g_cached_renderpass = VK_NULL_HANDLE; rsx::unordered_map g_current_renderpass; - rsx::unordered_map g_last_closed_renderpass; shared_mutex g_renderpass_cache_mutex; rsx::unordered_map g_renderpass_cache; @@ -363,23 +349,10 @@ namespace vk // Framebuffer-local stages only. VK_DEPENDENCY_BY_REGION_BIT is what makes the // dependency tile-local rather than a full pipeline flush, and it requires every stage // named here to be framebuffer-space, which rules out the vertex stage. - // - // Symmetric on purpose. The feedback case needs write -> read (draw writes the pixel, the - // next draw samples it) and the fall-out case needs read -> write (the sampling draw is - // done, the next draw writes the attachment again). Declaring only the first direction - // left render_target::post_texture_barrier with no dependency it could match, so the only - // legal thing it could do was end the pass, which is what it did. - // - // Every stage named is framebuffer-space (FRAGMENT_SHADER, EARLY/LATE_FRAGMENT_TESTS, - // COLOR_ATTACHMENT_OUTPUT), which is what VK_DEPENDENCY_BY_REGION_BIT requires of a - // self-dependency, and each access bit is one its stage supports. Widening a subpass - // dependency only adds synchronization; it is not part of render pass compatibility, so - // no cached pipeline is invalidated by this. VkSubpassDependency self_dependency = {}; self_dependency.srcSubpass = 0; self_dependency.dstSubpass = 0; self_dependency.srcStageMask = - VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; @@ -389,100 +362,23 @@ namespace vk VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; self_dependency.srcAccessMask = - VK_ACCESS_SHADER_READ_BIT | - VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; self_dependency.dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; self_dependency.dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT; - VkSubpassDependency dependencies[2] = { self_dependency, {} }; - u32 dependency_count = 1; - -#ifdef __ANDROID__ - if constexpr (s_surface_parking_enabled) - { - // The dependency parking removed, put back where it costs nothing. - // - // Without parking, a render target that has been sampled sits in - // SHADER_READ_ONLY_OPTIMAL and is dragged back to COLOR_/DEPTH_STENCIL_ATTACHMENT_- - // OPTIMAL when it is next bound. That transition is what carried the read -> write - // dependency: image_helpers.cpp gives it srcAccess = SHADER_READ | TRANSFER_READ at - // FRAGMENT_SHADER | TRANSFER, dstAccess = COLOR_ATTACHMENT_WRITE at - // COLOR_ATTACHMENT_OUTPUT. Parking declines the transition, so that barrier is gone - // and nothing else orders the previous pass's sample against this pass's write. - // - // A render pass boundary is not a synchronization boundary in Vulkan, and the - // implicit VK_SUBPASS_EXTERNAL dependency has srcStageMask = TOP_OF_PIPE, which is - // no execution dependency on anything. So this has to be declared, or parking is - // shipping a write-after-read hazard on every RTT it touches. That class of omission - // is exactly what produced visible corruption in the first attempt at this work. - // - // Declared on the pass rather than issued per draw because it is free here: it is - // baked into the render pass object, applies once at pass start, and a tiler already - // orders passes on the same attachment. A per-draw barrier would cost a teardown and - // give back everything parking wins. - // - // Deliberately NOT VK_DEPENDENCY_BY_REGION_BIT. By-region is for tile-local - // self-dependencies; this crosses a pass boundary, where the producer's fragment may - // be anywhere in the framebuffer relative to the consumer's. - // - // Covers write-after-write too (srcAccess names the attachment writes), which the - // same removed transition used to cover for a surface written, sampled, and written - // again without changing layout. - // - // A dependency is not part of render pass compatibility (Vulkan 8.2: compatibility is - // attachment references, formats and sample counts), so adding one does not - // invalidate a single cached pipeline. - VkSubpassDependency& entry_dependency = dependencies[dependency_count++]; - entry_dependency.srcSubpass = VK_SUBPASS_EXTERNAL; - entry_dependency.dstSubpass = 0; - // - // The source scope mirrors what image_helpers.cpp put on the GENERAL -> - // ATTACHMENT_OPTIMAL transition that parking removes, plus the vertex stage: vertex - // textures reach validate_image_layout_for_read_access too (VKDraw.cpp passes - // VK_PIPELINE_STAGE_VERTEX_SHADER_BIT there), so a vertex shader sampling a parked - // RTT is a reader this has to order against. Naming a non-framebuffer-space stage is - // fine here precisely because this is not a by-region self-dependency. - entry_dependency.srcStageMask = - VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | - VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | - VK_PIPELINE_STAGE_TRANSFER_BIT | - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | - VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - entry_dependency.dstStageMask = - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | - VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - entry_dependency.srcAccessMask = - VK_ACCESS_SHADER_READ_BIT | - VK_ACCESS_TRANSFER_READ_BIT | - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - entry_dependency.dstAccessMask = - VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - entry_dependency.dependencyFlags = 0; - } -#endif - VkRenderPassCreateInfo rp_info = {}; rp_info.sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO; rp_info.attachmentCount = ::size32(attachments); rp_info.pAttachments = attachments.data(); rp_info.subpassCount = 1; rp_info.pSubpasses = &subpass; - rp_info.dependencyCount = dependency_count; - rp_info.pDependencies = dependencies; + rp_info.dependencyCount = 1; + rp_info.pDependencies = &self_dependency; VkRenderPass result; CHECK_RESULT(vkCreateRenderPass(dev, &rp_info, NULL, &result)); @@ -497,7 +393,6 @@ namespace vk g_cached_renderpass_key = 0; g_cached_renderpass = VK_NULL_HANDLE; g_current_renderpass.clear(); - g_last_closed_renderpass.clear(); // Destroy cache for (const auto &renderpass : g_renderpass_cache) @@ -542,28 +437,6 @@ namespace vk rsx::prof::g_pass_width[rsx::prof::g_pass_ordinal] = static_cast(framebuffer_region.width); rsx::prof::g_pass_height[rsx::prof::g_pass_ordinal] = static_cast(framebuffer_region.height); } - - // Reopening the identical (pass, framebuffer) that was just closed. The teardown in - // between bought nothing: the tile was stored and reloaded to record a barrier, and - // the pass came straight back. - // - // This is the number that says how much any barrier-suppression work can possibly - // win, and it is the one the site counters cannot give. A site counter says a - // teardown was charged there; it cannot say whether the pass was going to end anyway - // because the next draw switches framebuffer. Removing a teardown that was going to - // happen just relocates the charge - which is what happened to iteration 3, where - // ImgHelper:43 fell and (Draw:1093 fbo) rose by the same amount. - // - // So: total passes minus redundant passes is the floor. If redundant is small, no - // amount of barrier work moves the frame, and the honest answer is to stop. - auto& closed = g_last_closed_renderpass[cmd]; - if (closed.pass == pass && closed.fbo == target) - { - rsx::prof::g_rp_reopened++; - rsx::prof::note_rp_teardown(closed.caller, 2); - } - - closed = {}; } // The draw region was declared and never recorded anywhere, so the one figure that @@ -617,15 +490,6 @@ namespace vk // After the pass ends, so the tile store it triggers is charged to the region. vk::get_gpu_timer().end(cmd, vk::gpu_timer::region::draw); - // Remember what was closed and by whom, so the next begin can tell whether this teardown - // was load-bearing or whether the same pass simply came back. __builtin_return_address - // here is the teardown site itself - end_renderpass is called directly from all of them. - if (rsx::prof::enabled()) [[unlikely]] - { - const auto& active = g_current_renderpass[cmd]; - g_last_closed_renderpass[cmd] = { active.pass, active.fbo, __builtin_return_address(0) }; - } - g_current_renderpass[cmd] = {}; } @@ -634,39 +498,6 @@ namespace vk return g_current_renderpass[cmd].pass != VK_NULL_HANDLE; } - void set_renderpass_attachments(const vk::command_buffer& cmd, const std::vector& images) - { - auto& info = g_current_renderpass[cmd]; - if (info.pass == VK_NULL_HANDLE) - { - // Nothing open. Do not record a set that would outlive the pass it describes. - return; - } - - info.attachments.clear(); - info.attachments.reserve(images.size()); - - for (const auto* img : images) - { - if (img) - { - info.attachments.push_back(img->value); - } - } - } - - bool renderpass_covers_image(const vk::command_buffer& cmd, VkImage image) - { - const auto& info = g_current_renderpass[cmd]; - if (info.pass == VK_NULL_HANDLE || image == VK_NULL_HANDLE) - { - return false; - } - - // At most 5 entries (4 MRTs + depth), so a scan beats anything with a hash in it. - return std::find(info.attachments.begin(), info.attachments.end(), image) != info.attachments.end(); - } - void renderpass_op(const vk::command_buffer& cmd, const renderpass_op_callback_t& op) { const auto& active = g_current_renderpass[cmd]; diff --git a/rpcs3/Emu/RSX/VK/VKRenderPass.h b/rpcs3/Emu/RSX/VK/VKRenderPass.h index 971f18be5..5c23e749d 100644 --- a/rpcs3/Emu/RSX/VK/VKRenderPass.h +++ b/rpcs3/Emu/RSX/VK/VKRenderPass.h @@ -8,50 +8,6 @@ namespace vk class image; class command_buffer; - // Surface layout parking policy. - // - // "Parking" is declining to move a render target out of a layout that is legal both for - // sampling and for use as an attachment, so a sample-then-rebind cycle costs no layout - // transition at all. A transition can never be recorded inside a render pass instance - // (VUID-vkCmdPipelineBarrier-oldLayout-01181), so every transition avoided is a render pass - // teardown avoided - on a tiler, a tile store plus a full reload. - // - // Both knobs live here rather than next to vk::render_target because the render pass builder - // has to see them too. The barrier that parking removes is the one that used to carry the - // read -> write dependency across the cycle, so the pass has to declare that dependency - // itself; see the VK_SUBPASS_EXTERNAL dependency in get_renderpass. Setting these to 0 has to - // take that declaration away with them, otherwise "0 reproduces current behaviour" is a lie. - // - // Both are counted in flips, not binds. A render graph repeats per frame, so "was this - // surface used this way recently" is a per-frame question; the number of unrelated binds - // collected in between is a property of the scene's draw count and is not a useful unit. - // Two frames of slack rather than one so a surface used on alternate frames, or one that - // misses a frame to a paused or duplicate flip, does not oscillate. - - // Feedback loops: a surface written and sampled by the same draw, parked in - // ATTACHMENT_FEEDBACK_LOOP_OPTIMAL_EXT where the device supports it and GENERAL otherwise. - // Iteration 3. Measured effect on Arkham City: close to zero, because the teardown it - // removes at the bind site relocates to the draw site as a framebuffer mismatch. - // Held at 0 pending the baseline `redundant` reading: iteration 3 measured close to zero here, - // and at 0 this also drops the extra pass dependency below, which is the only thing that was - // covering the read->write hazard this park had been running without. - inline constexpr u64 s_feedback_park_frames = 0; - - // Non-cyclic sampling of a render target: the RTT is left in GENERAL for the read instead of - // being dragged to SHADER_READ_ONLY_OPTIMAL and back. GENERAL, not the feedback-loop layout, - // deliberately - see the note in render_target::get_sample_park_layout. - // - // This is the independent A/B knob. Set to 0 to get the previous behaviour exactly while - // leaving feedback parking alone, and vice versa. - // Held at 0 for the same reason. `redundant` in the RSXPROF line is the hard ceiling on what - // suppressing these teardowns can win; read that first, because if it is small then this trade - // (GENERAL costs UBWC while rendering, and HiZ on depth) cannot pay for itself and the answer - // is to leave both at 0. - inline constexpr u64 s_sample_park_frames = 0; - - // Any parking at all. Gates the extra render pass dependency that makes parking safe. - inline constexpr bool s_surface_parking_enabled = (s_feedback_park_frames != 0) || (s_sample_park_frames != 0); - u64 get_renderpass_key(const std::vector& images, const std::vector& input_attachment_ids = {}); u64 get_renderpass_key(const std::vector& images, u64 previous_key); u64 get_renderpass_key(VkFormat surface_format, u8 sample_count = 1); @@ -67,24 +23,6 @@ namespace vk void end_renderpass(const vk::command_buffer& cmd); bool is_renderpass_open(const vk::command_buffer& cmd); - // Attachment set of the pass currently open on this command buffer. - // - // VUID-vkCmdPipelineBarrier-image-04073 only allows a barrier to be recorded inside a render - // pass instance when the image is an attachment of the current subpass. "The caller only ever - // does this for a bound render target" is not the same statement: the pass that is open is the - // one from the previous draw, and it is not re-created until the draw call itself, so between - // a framebuffer switch and the next draw the open pass belongs to the *previous* framebuffer. - // A surface that needs no layout change (because it is parked) does not end the pass on the - // way in, so it can reach a barrier site with a stale pass open and its own image absent from - // it. That is the corruption an earlier attempt hit; this makes the rule checkable instead of - // assumed. - // - // The set is cleared whenever a pass begins, so any pass opened by code that does not publish - // its attachments (overlays, texture cache, present) reads as covering nothing and in-pass - // barriers are refused for it. Fail-closed by construction. - void set_renderpass_attachments(const vk::command_buffer& cmd, const std::vector& images); - bool renderpass_covers_image(const vk::command_buffer& cmd, VkImage image); - using renderpass_op_callback_t = std::function; void renderpass_op(const vk::command_buffer& cmd, const renderpass_op_callback_t& op); } diff --git a/rpcs3/Emu/RSX/VK/VKRenderTargets.cpp b/rpcs3/Emu/RSX/VK/VKRenderTargets.cpp index baef500d3..aaeb1bfdb 100644 --- a/rpcs3/Emu/RSX/VK/VKRenderTargets.cpp +++ b/rpcs3/Emu/RSX/VK/VKRenderTargets.cpp @@ -1,6 +1,5 @@ #include "vkutils/data_heap.h" #include "VKRenderTargets.h" -#include "VKRenderPass.h" #include "VKResourceManager.h" #include "Emu/RSX/rsx_methods.h" #include "Emu/RSX/RSXThread.h" @@ -652,14 +651,6 @@ namespace vk void render_target::unspill(vk::command_buffer& cmd) { - // The image below is a different VkImage with different contents, so any park describing - // the old one is meaningless. Both readers already re-check current_layout and unspill - // leaves it in an attachment or transfer layout rather than GENERAL, so this is belt and - // braces - but the read-synchronization tag is keyed on last_use_tag, which the recreate - // does not touch, and that is a hazard rather than a missed optimization if it ever - // lines up. - clear_sample_park(); - // Recreate the image const auto pdev = vk::get_current_renderer(); create_impl(*pdev, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, pdev->get_memory_mapping().device_local, VMM_ALLOCATION_POOL_SURFACE_CACHE); @@ -856,182 +847,12 @@ namespace vk return (scaled_w == width()) && (scaled_h == height()); } - VkImageLayout render_target::get_feedback_loop_layout(const vk::command_buffer& cmd) - { - // The layout a surface is parked in while it is both written and sampled by the same - // draw. Both of these are legal attachment layouts as well as legal sampling layouts, - // which is the property try_park_in_feedback_loop relies on: an attachment left here is - // still a valid attachment, and the render pass key encoder accepts both. - return cmd.get_command_pool().get_owner().get_framebuffer_loops_support() - ? VK_IMAGE_LAYOUT_ATTACHMENT_FEEDBACK_LOOP_OPTIMAL_EXT - : VK_IMAGE_LAYOUT_GENERAL; - } - - // The sample park. - // - // Iterations 1-3 attacked the feedback loop, which turned out to be a rounding error: the - // measured split of Draw:1093 came back 100% framebuffer mismatch and 0% render pass key, so - // layout churn was contributing nothing there. What did not move across any of the three was - // ImgHelper:43, which is change_image_layout ending the open pass, and the largest single - // contributor to it is the ordinary render-to-texture cycle: - // - // validate_image_layout_for_read_access drags an unbound RTT from - // COLOR_/DEPTH_STENCIL_ATTACHMENT_OPTIMAL to SHADER_READ_ONLY_OPTIMAL so it can be sampled, - // and prepare_surface_for_drawing drags it straight back the next time the game renders to - // it. Two layout changes, therefore two teardowns, per sample-then-rebind cycle. - // - // Only the first is a floor. The read genuinely needs a write -> read barrier and a barrier - // cannot be recorded inside a pass unless the image is an attachment of it, which a sampled - // unbound RTT is not. The return trip is pure overhead: GENERAL is a legal sampling layout - // AND a legal attachment layout, the descriptor path takes imageLayout from - // image->current_layout at bind time (vkutils/ex.cpp), and the render pass key encoder - // already accepts GENERAL. So the surface can simply be left where it is. - // - // What this does NOT do is make the first half free, and the honest read of the win is - // "half of the sample/rebind pairs, if and only if those teardowns were not going to happen - // anyway". That last clause is why g_rp_reopened exists: see rsx_profiler.h. - bool render_target::try_arm_sample_park() - { - if constexpr (s_sample_park_frames == 0) - { - return false; - } - - // A bound attachment must never be parked from here. Its layout is baked into - // m_current_renderpass_key, which prepare_rtts computed at the end of the bind and which - // nothing recomputes between there and the draw. Moving a bound surface to GENERAL would - // leave the pass declaring initialLayout = ATTACHMENT_OPTIMAL for an image that is not in - // it, which is undefined contents rather than a validation error. - // - // Today the same mistake is caught loudly - renderpass_key_blob::set_layout throws on - // SHADER_READ_ONLY_OPTIMAL - and GENERAL would encode silently, so this check is what - // replaces the throw. The sampled-while-bound case is a feedback loop and belongs to - // texture_barrier regardless. - if (is_bound) - { - return false; - } - - // MSAA surfaces sample through a resolve target and carry a second image whose contents - // are produced by resolve/unresolve passes that move layouts on their own schedule. - // Not worth the reasoning for the volume involved. - if (samples() > 1) - { - return false; - } - - const auto renderer = rsx::get_current_renderer(); - if (!renderer) - { - return false; - } - - m_sample_park_deadline = renderer->int_flip_index + s_sample_park_frames + 1; - return true; - } - - bool render_target::try_park_for_sampling() - { - if (!m_sample_park_deadline) - { - return false; - } - - // Same honesty check the feedback park makes: parking never suppresses a transition, it - // only declines to start one. If anything has moved the layout since - a blit, a spill, - // an unspill that rebuilt the image as UNDEFINED, the zeta fall-out valve - the park is - // over and the normal path runs. - if (current_layout != get_sample_park_layout()) - { - clear_sample_park(); - return false; - } - - const auto renderer = rsx::get_current_renderer(); - if (renderer && renderer->int_flip_index >= m_sample_park_deadline) - { - // Expired. Let the surface go back to the attachment-optimal layout and get its - // compression back. On Adreno that is UBWC on a full-size render target, which is - // not something to hold hostage to a surface that has stopped being sampled. - clear_sample_park(); - return false; - } - - return true; - } - - bool render_target::sample_park_needs_read_barrier() const - { - // last_use_tag advances on every write: on_write / on_write_fast at the end of each draw - // that had this surface bound with writes enabled, on_write after a clear or a memory - // initialize, on_write_copy after a blit. Equal tags therefore mean "nothing has written - // this since the barrier that made the previous read safe". - return !m_sample_park_sync_tag || m_sample_park_sync_tag != last_use_tag; - } - - void render_target::on_sample_park_synced() - { - m_sample_park_sync_tag = last_use_tag; - } - - void render_target::clear_sample_park() - { - m_sample_park_deadline = 0; - m_sample_park_sync_tag = 0; - } - - void render_target::arm_feedback_park() - { - if constexpr (s_feedback_park_frames == 0) - { - return; - } - - const auto renderer = rsx::get_current_renderer(); - if (!renderer) - { - return; - } - - // Deadline is exclusive, so the arming frame itself is always covered even when the - // window is one flip. int_flip_index only moves on the RSX thread, which is also the - // only thread that reaches this, so no synchronization is needed on it. - m_feedback_park_deadline = renderer->int_flip_index + s_feedback_park_frames + 1; - } - - bool render_target::try_park_in_feedback_loop(const vk::command_buffer& cmd) - { - // The layout test is what keeps this honest: parking never suppresses a transition, it - // only declines to start one. A surface that is not already sitting in the loop layout - // takes the normal path, so nothing here can produce a barrier with old != new, and - // nothing here can leave an attachment in a layout the render pass key does not encode. - if (!m_feedback_park_deadline || current_layout != get_feedback_loop_layout(cmd)) - { - // Either never armed, or something outside parking has already moved the layout - - // a non-cyclic sample dragging it to SHADER_READ_ONLY_OPTIMAL, the zeta fall-out - // valve in VKDraw restoring HiZ, a blit, a spill. The park is over either way, and - // leaving the deadline set would let a later unrelated visit to the loop layout - // re-park a surface that never asked for it. - m_feedback_park_deadline = 0; - return false; - } - - const auto renderer = rsx::get_current_renderer(); - if (renderer && renderer->int_flip_index >= m_feedback_park_deadline) - { - // Expired. Fall through to the normal path, which puts the surface back in the - // attachment-optimal layout and hands its compression back. - m_feedback_park_deadline = 0; - return false; - } - - return true; - } - void render_target::texture_barrier(vk::command_buffer& cmd) { const auto is_framebuffer_read_only = is_depth_surface() && !rsx::method_registers.depth_write_enabled(); - const auto optimal_layout = get_feedback_loop_layout(cmd); + const auto supports_fbo_loops = cmd.get_command_pool().get_owner().get_framebuffer_loops_support(); + const auto optimal_layout = supports_fbo_loops ? VK_IMAGE_LAYOUT_ATTACHMENT_FEEDBACK_LOOP_OPTIMAL_EXT + : VK_IMAGE_LAYOUT_GENERAL; if (m_cyclic_ref_tracker.can_skip() && current_layout == optimal_layout && is_framebuffer_read_only) { @@ -1058,35 +879,6 @@ namespace vk vk::insert_texture_barrier(cmd, this, optimal_layout, preserve_renderpass); m_cyclic_ref_tracker.on_insert_texture_barrier(); - // Open (or re-open) the park window. This is the only writer. - // - // The first attempt keyed parking off m_cyclic_ref_tracker.is_enabled() and it could not - // hold for more than a single bind, because the tracker is a within-loop signal that - // three separate paths clear before the next bind ever runs: - // - prepare_surface_for_drawing calls reset_surface_counters() at every bind; - // - post_texture_barrier() resets it, and that is the *normal* end of an episode - the - // first draw after the loop with the surface still bound trips - // requires_post_loop_barrier() and resets before the next bind is reached; - // - memory_barrier() resets it when the layout moved elsewhere. - // So the predicate was true only on a bind directly preceded by a feedback draw, and the - // surface was dragged out of the loop layout on the very next bind and back in on the one - // after. Measured on Arkham City: the transitions moved from the bind site to the barrier - // site and the pass count did not change. - // - // A window rather than a flag because the useful question is "has this surface been in a - // loop recently", and an episode that recurs within a few binds should not pay to leave - // and re-enter the layout in between. It decays so a surface that has genuinely stopped - // looping goes back to the attachment-optimal layout and gets its compression back; the - // depth case is the one that matters, HiZ is not free to give up indefinitely. - // - // The second attempt made that window four binds and that was still the wrong unit: see - // s_feedback_park_frames. It is counted in flips now, so an episode that recurs every - // frame - which is what a render graph does - keeps the surface parked continuously - // instead of dropping it between episodes. -#ifdef __ANDROID__ - arm_feedback_park(); -#endif - if (is_framebuffer_read_only) { m_cyclic_ref_tracker.allow_skip(); @@ -1108,47 +900,23 @@ namespace vk VkPipelineStageFlags src_stage, dst_stage; VkAccessFlags src_access, dst_access; - // This barrier does not move the layout - it is the read-completes-before-write half of a - // feedback loop on a surface that is still the bound attachment. Both of the conditions - // that let a barrier stay inside the pass therefore hold, and it was still ending the - // pass on every fall-out purely because it never asked not to. The render pass now - // declares the read -> write direction of the self-dependency, so it can ask. - // - // The vertex stage has to come off the source scope when the barrier lands inside the - // pass: VK_DEPENDENCY_BY_REGION_BIT admits framebuffer-space stages only, and a vertex - // shader sampling a live attachment is not a tile-local dependency. That is the same - // trade texture_barrier already makes on this path, and it is sound for what this - // synchronizes - the fall-out is a fragment-shader read of the attachment the next draw - // writes. Outside the pass the wider scope is kept. -#ifdef __ANDROID__ - const bool preserve_renderpass = vk::renderpass_covers_image(cmd, value); -#else - constexpr bool preserve_renderpass = false; -#endif - if (!is_depth_surface()) [[likely]] { - src_stage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; + src_stage = VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; dst_stage = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; src_access = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; dst_access = VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; } else { - src_stage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + src_stage = VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; dst_stage = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; src_access = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; dst_access = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; } - if (!preserve_renderpass) - { - src_stage |= VK_PIPELINE_STAGE_VERTEX_SHADER_BIT; - } - vk::insert_image_memory_barrier(cmd, value, current_layout, current_layout, - src_stage, dst_stage, src_access, dst_access, { aspect(), 0, 1, 0, 1 }, - preserve_renderpass); + src_stage, dst_stage, src_access, dst_access, { aspect(), 0, 1, 0, 1 }); m_cyclic_ref_tracker.reset(); } @@ -1157,18 +925,6 @@ namespace vk { frame_tag = 0; m_cyclic_ref_tracker.reset(); - - // Belt and braces for the sample park's read-synchronization tag. This is called from - // prepare_surface_for_drawing and from the zeta fall-out valve, both of which mean "this - // surface is or was just an attachment", so the next sample must re-establish a - // write -> read barrier. last_use_tag alone would already say so for anything that - // reaches on_write; this covers the paths that write pixels through some route that does - // not. Costs at most one extra barrier on a surface that was bound and never written. - // - // Deliberately does NOT clear m_sample_park_deadline. The park has to outlive the bind - // that resets the counters or it can never hold for more than one bind - the same - // mistake the first feedback park made. - m_sample_park_sync_tag = 0; } image_view* render_target::get_view(const rsx::texture_channel_remap_t& remap, VkImageAspectFlags mask) diff --git a/rpcs3/Emu/RSX/VK/VKRenderTargets.h b/rpcs3/Emu/RSX/VK/VKRenderTargets.h index 58d38b79b..1e6355b7f 100644 --- a/rpcs3/Emu/RSX/VK/VKRenderTargets.h +++ b/rpcs3/Emu/RSX/VK/VKRenderTargets.h @@ -5,7 +5,6 @@ #include "VKFormats.h" #include "VKHelpers.h" -#include "VKRenderPass.h" #include "vkutils/barriers.h" #include "vkutils/buffer_object.h" #include "vkutils/device.h" @@ -73,35 +72,6 @@ namespace vk // Cyclic reference hazard tracking image_reference_sync_barrier m_cyclic_ref_tracker; - // Flip index this surface stops being left in the feedback-loop layout at. 0 = not parked. - // - // Deliberately NOT part of m_cyclic_ref_tracker, and deliberately not cleared by - // reset_surface_counters. The tracker answers "is a loop in progress right now", which is - // a within-draw question, and every bind resets it. What parking needs is "was this - // surface in a loop recently", which has to outlive the bind that resets the tracker or - // it can never hold for more than one bind. See the comment on texture_barrier. - u64 m_feedback_park_deadline = 0; - - // Same idea, for the far more common case: a render target sampled while it is NOT bound. - // Flip index the sample park expires at; 0 = not parked. - u64 m_sample_park_deadline = 0; - - // last_use_tag as of the last read barrier issued for this surface while parked. The - // surface is safe to sample with no barrier at all exactly while this still matches - // last_use_tag, because that tag advances on every write (surface_store::on_write and - // on_write_fast at end of draw, on_write_copy after a blit). - // - // This is the piece the layout used to carry implicitly. Without parking, "already in - // SHADER_READ_ONLY_OPTIMAL" meant "already synchronized for reading", so the second and - // later draws sampling the same texture fell through validate_image_layout_for_read_access - // with no barrier. Leave the surface in GENERAL and that signal is gone: every sampling - // draw would look identical to the first and issue its own barrier, which for a shadow - // map sampled by hundreds of draws would turn one teardown into hundreds. The tag makes - // the same statement explicitly and does not depend on the layout. - // - // 0 = never synchronized, always barrier. - u64 m_sample_park_sync_tag = 0; - // Memory spilling support std::unique_ptr m_spilled_mem; @@ -152,44 +122,6 @@ namespace vk void memory_barrier(vk::command_buffer& cmd, rsx::surface_access access); void read_barrier(vk::command_buffer& cmd) { memory_barrier(cmd, rsx::surface_access::shader_read); } void write_barrier(vk::command_buffer& cmd) { memory_barrier(cmd, rsx::surface_access::shader_write); } - - // Layout a surface is parked in while it is written and sampled by the same draw. - // See vk::s_feedback_park_frames in VKRenderPass.h for the window and the A/B knob. - static VkImageLayout get_feedback_loop_layout(const vk::command_buffer& cmd); - // True when this bind may leave the surface in the feedback-loop layout instead of - // dragging it back to the attachment-optimal one. - bool try_park_in_feedback_loop(const vk::command_buffer& cmd); - // Arms the park window. Called from the barrier sites that establish a feedback loop. - void arm_feedback_park(); - - // Layout a surface is parked in while it is sampled without being bound. - // - // Plain GENERAL, never ATTACHMENT_FEEDBACK_LOOP_OPTIMAL_EXT, and that is a deliberate - // narrowing rather than an oversight. The feedback-loop layout is only meaningful for an - // image that is simultaneously an attachment and a sampled texture, and using it legally - // requires the pipelines and framebuffer to be created with the matching - // VK_PIPELINE_CREATE_COLOR_ATTACHMENT_FEEDBACK_LOOP_BIT_EXT / - // VK_IMAGE_VIEW_CREATE_..._EXT flags, which nothing in this backend sets. The existing - // feedback park already sits on that exposure; there is no reason to enlarge it for a - // case that is not a feedback loop at all. GENERAL is unconditionally legal both as an - // attachment layout and as a sampled layout, and the render pass key encoder accepts it - // (VKRenderPass.cpp set_layout), which is everything this needs. - static constexpr VkImageLayout get_sample_park_layout() { return VK_IMAGE_LAYOUT_GENERAL; } - - // True when a non-cyclic sample of this surface should leave it in GENERAL rather than - // dragging it to SHADER_READ_ONLY_OPTIMAL. Arms the park window as a side effect. - // Takes no command buffer: unlike the feedback layout, the sample park layout is a - // constant and needs no device query. - bool try_arm_sample_park(); - // True when this bind may leave a sample-parked surface where it is. - bool try_park_for_sampling(); - // True when a parked surface has been written since its last read barrier and therefore - // needs another one. False means the sample can be issued with no barrier at all. - bool sample_park_needs_read_barrier() const; - // Records that a read barrier covering the current contents has just been issued. - void on_sample_park_synced(); - // Drops the park. Any path that moves the layout out from under it lands here. - void clear_sample_park(); }; static inline vk::render_target* as_rtt(vk::image* t) @@ -507,72 +439,13 @@ namespace vk // Special case barrier surface->memory_barrier(cmd, rsx::surface_access::gpu_reference); - // A surface in a feedback loop is dragged back and forth for no reason. The loop - // parks it in GENERAL (or ATTACHMENT_FEEDBACK_LOOP_OPTIMAL), this snapped it back to - // COLOR_/DEPTH_STENCIL_ATTACHMENT_OPTIMAL on the next bind, and the next draw's - // texture barrier moved it straight back. Both halves of that round trip are layout - // changes, and a layout change cannot happen inside a render pass, so each one ends - // the pass and forces a tile store plus a full reload. - // - // It shows up three times in the same profile of Arkham City, which draws 93 to 113 - // render passes a frame for 6 logical ones: - // - change_image_layout from bind_surface_address ends 34.6 passes a frame, - // and another 11.0. This is that snap-back. - // - the return trip in texture_barrier is a second layout change. Android asks it - // to keep the pass open, which was recorded as an in-pass layout change and is - // undefined; with the surface already parked the barrier no longer moves the - // layout and becomes legal. - // - the layout is part of the render pass key, so every flip also invalidates the - // cached render pass and costs a restart at the draw site (24.3 a frame). - // - // Both parking layouts are legal for an attachment as well as for sampling, and - // VKRenderPass.cpp's key encoder already accepts both, so the surface can simply be - // left where it is. This is not suppressing a required pass break: there is no - // transition left to require one. - // - // The park has to outlive reset_surface_counters() below, which is why it is a - // separate window on the surface rather than a read of m_cyclic_ref_tracker. Keyed - // off the tracker it survived exactly one bind, which bought nothing: the round trip - // moved from this site to the barrier site and the frame's pass count did not move. - // - // Note what this site is and is not. Declining the change_layout here is what leaves - // the previous draw's render pass open into the rest of prepare_rtts, so a teardown - // that used to be charged to this site is now charged to whichever site first finds - // a mismatch - usually the draw call. That relocation is not a cost; the win is only - // real where the surface is still parked on the next bind and there is no round trip - // to pay for at all, which is what the frames-not-binds window is for. - // - // Android only. This is where the tile traffic was measured, and a desktop GPU pays - // for GENERAL attachments (lost depth/colour compression) without the tile store to - // win back. - // - // Two independent parks meet here, and the second is the one this iteration adds. - // The feedback park covers a surface written and sampled by the same draw, which is - // rare; the sample park covers a surface sampled while unbound and then rendered to - // again, which is the ordinary render-to-texture cycle and is where the volume is. - // Either one may hold, and both leave the surface in a layout that is legal as an - // attachment and encodable in the render pass key. - // - // Order matters only for the side effects: try_park_in_feedback_loop clears its own - // deadline when the layout has moved, so it must still run even when the sample park - // would answer first. || would short-circuit it, so both are evaluated. - bool keep_current_layout = false; -#ifdef __ANDROID__ - const bool feedback_parked = surface->try_park_in_feedback_loop(cmd); - const bool sample_parked = surface->try_park_for_sampling(); - keep_current_layout = feedback_parked || sample_parked; -#endif - - if (!keep_current_layout) + if (surface->aspect() == VK_IMAGE_ASPECT_COLOR_BIT) { - if (surface->aspect() == VK_IMAGE_ASPECT_COLOR_BIT) - { - surface->change_layout(cmd, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); - } - else - { - surface->change_layout(cmd, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL); - } + surface->change_layout(cmd, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); + } + else + { + surface->change_layout(cmd, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL); } surface->reset_surface_counters(); diff --git a/rpcs3/Emu/RSX/VK/vk_android_loader.cpp b/rpcs3/Emu/RSX/VK/vk_android_loader.cpp index f1acb8872..9f1087b2a 100644 --- a/rpcs3/Emu/RSX/VK/vk_android_loader.cpp +++ b/rpcs3/Emu/RSX/VK/vk_android_loader.cpp @@ -96,7 +96,6 @@ extern "C" PFN_vkDestroyInstance vkDestroyInstance = nullptr; PFN_vkDestroyPipeline vkDestroyPipeline = nullptr; PFN_vkDestroyPipelineCache vkDestroyPipelineCache = nullptr; - PFN_vkGetPipelineCacheData vkGetPipelineCacheData = nullptr; PFN_vkDestroyPipelineLayout vkDestroyPipelineLayout = nullptr; PFN_vkDestroyQueryPool vkDestroyQueryPool = nullptr; PFN_vkDestroyRenderPass vkDestroyRenderPass = nullptr; @@ -340,8 +339,6 @@ namespace vk::android if (!vkDestroyPipeline) vkDestroyPipeline = reinterpret_cast(dlsym(handle, "vkDestroyPipeline")); vkDestroyPipelineCache = reinterpret_cast(vkGetInstanceProcAddr(nullptr, "vkDestroyPipelineCache")); if (!vkDestroyPipelineCache) vkDestroyPipelineCache = reinterpret_cast(dlsym(handle, "vkDestroyPipelineCache")); - vkGetPipelineCacheData = reinterpret_cast(vkGetInstanceProcAddr(nullptr, "vkGetPipelineCacheData")); - if (!vkGetPipelineCacheData) vkGetPipelineCacheData = reinterpret_cast(dlsym(handle, "vkGetPipelineCacheData")); vkDestroyPipelineLayout = reinterpret_cast(vkGetInstanceProcAddr(nullptr, "vkDestroyPipelineLayout")); if (!vkDestroyPipelineLayout) vkDestroyPipelineLayout = reinterpret_cast(dlsym(handle, "vkDestroyPipelineLayout")); vkDestroyQueryPool = reinterpret_cast(vkGetInstanceProcAddr(nullptr, "vkDestroyQueryPool")); @@ -535,7 +532,6 @@ namespace vk::android if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyInstance")) vkDestroyInstance = reinterpret_cast(p); if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyPipeline")) vkDestroyPipeline = reinterpret_cast(p); if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyPipelineCache")) vkDestroyPipelineCache = reinterpret_cast(p); - if (auto p = vkGetInstanceProcAddr(instance, "vkGetPipelineCacheData")) vkGetPipelineCacheData = reinterpret_cast(p); if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyPipelineLayout")) vkDestroyPipelineLayout = reinterpret_cast(p); if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyQueryPool")) vkDestroyQueryPool = reinterpret_cast(p); if (auto p = vkGetInstanceProcAddr(instance, "vkDestroyRenderPass")) vkDestroyRenderPass = reinterpret_cast(p); diff --git a/rpcs3/Emu/RSX/VK/vk_android_loader.h b/rpcs3/Emu/RSX/VK/vk_android_loader.h index aae33d6fb..557dc8cc8 100644 --- a/rpcs3/Emu/RSX/VK/vk_android_loader.h +++ b/rpcs3/Emu/RSX/VK/vk_android_loader.h @@ -117,7 +117,6 @@ extern "C" extern PFN_vkDestroyInstance vkDestroyInstance; extern PFN_vkDestroyPipeline vkDestroyPipeline; extern PFN_vkDestroyPipelineCache vkDestroyPipelineCache; - extern PFN_vkGetPipelineCacheData vkGetPipelineCacheData; extern PFN_vkDestroyPipelineLayout vkDestroyPipelineLayout; extern PFN_vkDestroyQueryPool vkDestroyQueryPool; extern PFN_vkDestroyRenderPass vkDestroyRenderPass; diff --git a/rpcs3/Emu/RSX/VK/vkutils/barriers.cpp b/rpcs3/Emu/RSX/VK/vkutils/barriers.cpp index 0b4ab914b..dbe3b0244 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/barriers.cpp +++ b/rpcs3/Emu/RSX/VK/vkutils/barriers.cpp @@ -8,62 +8,6 @@ namespace vk { - // When a barrier may stay inside an open render pass instance, and when it may not. - // - // Written out because getting this wrong is not a validation warning, it is corruption. An - // earlier attempt at cutting the render pass count on Android set preserve_renderpass on - // every sampled render target and produced garbage output, because two of the three rules - // below were never checked. - // - // 1. VUID-vkCmdPipelineBarrier-None-07890. The render pass must declare a dependency from - // the current subpass to itself, with stage and access scopes that are supersets of the - // barrier's, and it must not declare VK_DEPENDENCY_BY_REGION_BIT unless the barrier - // passes that flag too. VKRenderPass.cpp declares exactly one self-dependency and it does - // carry BY_REGION, so an in-pass barrier here has to pass VK_DEPENDENCY_BY_REGION_BIT, - // and may only name framebuffer-space stages: COLOR_ATTACHMENT_OUTPUT, - // EARLY/LATE_FRAGMENT_TESTS and FRAGMENT_SHADER. Naming VERTEX_SHADER or TRANSFER, or - // leaving dependencyFlags at zero, makes the barrier invalid against that declaration. - // - // 2. VUID-vkCmdPipelineBarrier-oldLayout-01181. Inside a render pass instance oldLayout and - // newLayout must be equal. A real layout transition can never stay inside the pass, no - // matter what the self-dependency says. This is the hard floor on how far the pass count - // can be cut by suppressing barriers, and it is why the useful work is in removing the - // need for a transition rather than in hiding the pass break it forces. - // - // 3. VUID-vkCmdPipelineBarrier-image-04073. The image must be an attachment of the current - // subpass. A surface the draw merely samples is not one, and a subpass self-dependency - // says nothing about it. Only the feedback case, an attachment sampled while it is still - // bound, qualifies - render_target::texture_barrier and its fall-out counterpart - // render_target::post_texture_barrier, and nothing else. - // - // The strict reading of rule 3 also wants the attachment declared as an input attachment of - // the subpass. The subpass built in VKRenderPass.cpp never declares one: input_attachments_mask - // exists but no caller ever sets it. Declaring one would change render pass compatibility and - // therefore invalidate every cached pipeline, so it is left alone here and noted rather than - // silently assumed away. - // - // None of this is gated per-platform because every caller that asks to preserve the pass is - // already gated: texture_barrier and post_texture_barrier pass preserve_renderpass = true on - // Android only. On every other target the in-pass paths below are unreachable. - - // Rules 2 and 3 in one place, so no caller can opt out of them by asking nicely. - // - // Rule 3 is checked against the attachments the open pass actually has, not against what the - // caller believes is bound. The two differ: the render pass in flight is the previous draw's, - // and it is only re-created at the draw call, so a barrier issued during draw setup after a - // framebuffer switch sees a pass whose attachments are the *old* surfaces. That window used - // to be closed by accident - the layout change on the way in ended the pass - and surface - // parking is precisely the change that stops ending it. - static bool can_keep_pass_open( - const vk::command_buffer& cmd, VkImage image, - VkImageLayout current_layout, VkImageLayout new_layout, - bool preserve_renderpass) - { - return preserve_renderpass && - current_layout == new_layout && // rule 2 - vk::renderpass_covers_image(cmd, image); // rule 3 - } - void insert_image_memory_barrier( const vk::command_buffer& cmd, VkImage image, VkImageLayout current_layout, VkImageLayout new_layout, @@ -72,27 +16,14 @@ namespace vk const VkImageSubresourceRange& range, bool preserve_renderpass) { - // A caller asking to preserve the pass across a layout change, or for an image the open - // pass does not have attached, is asking for an illegal barrier. Take the teardown - // instead of recording one. - const bool keep_pass_open = can_keep_pass_open(cmd, image, current_layout, new_layout, preserve_renderpass); - bool inside_renderpass = false; - - if (vk::is_renderpass_open(cmd)) + if (!preserve_renderpass && vk::is_renderpass_open(cmd)) { - if (!keep_pass_open) - { - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[10]++; vk::end_renderpass(cmd); - } - else - { - inside_renderpass = true; - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::note_pass_barrier(false); - } + if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[10]++; vk::end_renderpass(cmd); + } + else if (rsx::prof::enabled() && vk::is_renderpass_open(cmd)) [[unlikely]] + { + rsx::prof::note_pass_barrier(false); } - - // Rule 1. Must match the BY_REGION self-dependency when the barrier lands inside the pass. - const VkDependencyFlags dependency_flags = inside_renderpass ? VK_DEPENDENCY_BY_REGION_BIT : 0; VkImageMemoryBarrier barrier = {}; barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; @@ -105,7 +36,7 @@ namespace vk barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; barrier.subresourceRange = range; - vkCmdPipelineBarrier(cmd, src_stage, dst_stage, dependency_flags, 0, nullptr, 0, nullptr, 1, &barrier); + vkCmdPipelineBarrier(cmd, src_stage, dst_stage, 0, 0, nullptr, 0, nullptr, 1, &barrier); } void insert_buffer_memory_barrier( @@ -163,33 +94,15 @@ namespace vk // Transition to GENERAL if this resource is both input and output // TODO: This implicitly makes the target incompatible with the renderpass declaration; investigate a proper workaround // TODO: This likely throws out hw optimizations on the rest of the renderpass, manage carefully - - // Rule 2 again, and this one does fire. The caller parks the surface in GENERAL or - // ATTACHMENT_FEEDBACK_LOOP_OPTIMAL for the duration of a feedback loop, so the first - // call of a loop moves the layout while later calls do not. Only the later ones may - // stay inside the pass; the first has to end it, even though Android asks to preserve. - // This was previously recorded as an in-pass layout change, which is undefined. - // - // Rule 3 fires here too, and only became reachable with parking: a parked surface no - // longer ends the pass when it is re-bound, so this can now be called while the previous - // draw's pass is still open and does not have this image attached. - const bool keep_pass_open = can_keep_pass_open(cmd, image, current_layout, new_layout, preserve_renderpass); - bool inside_renderpass = false; - - if (vk::is_renderpass_open(cmd)) + if (!preserve_renderpass && vk::is_renderpass_open(cmd)) { - if (!keep_pass_open) - { - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[13]++; vk::end_renderpass(cmd); - } - else - { - inside_renderpass = true; - - // Kept the pass open, so the barrier lands inside it. On a tiler that is a resolve - // and a re-fetch of the tile, which is the cost this is here to find. - if (rsx::prof::enabled()) [[unlikely]] rsx::prof::note_pass_barrier(true); - } + if (rsx::prof::enabled()) [[unlikely]] rsx::prof::g_rp_sites[13]++; vk::end_renderpass(cmd); + } + else if (rsx::prof::enabled() && vk::is_renderpass_open(cmd)) [[unlikely]] + { + // Kept the pass open, so the barrier lands inside it. On a tiler that is a resolve + // and a re-fetch of the tile, which is the cost this is here to find. + rsx::prof::note_pass_barrier(true); } VkAccessFlags src_access, dst_access; @@ -209,7 +122,7 @@ namespace vk dst_stage = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; } - if (inside_renderpass) + if (preserve_renderpass) { // Issued inside the pass, so it must match the by-region self-dependency the // render pass declares, and that permits framebuffer-local stages only. The @@ -219,10 +132,6 @@ namespace vk // sampling the attachment its own fragments write. A vertex shader sampling a // live render target would need the pass ended anyway, which is what the caller // gets by leaving preserve_renderpass false. - // - // Keyed off inside_renderpass rather than preserve_renderpass: when the pass had - // to be ended for a layout change the barrier is outside it again, and the wider - // scope is both legal and wanted. dst_stage |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; } else @@ -230,15 +139,6 @@ namespace vk dst_stage |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_VERTEX_SHADER_BIT; } - // Rule 1. The declared self-dependency carries VK_DEPENDENCY_BY_REGION_BIT, so a barrier - // recorded inside the pass has to carry it as well or it does not match the declaration. - // It was being recorded with dependencyFlags = 0, on every in-pass texture barrier - // Android takes - 20 to 23 a frame in Arkham City. BY_REGION is also the semantics this - // wants: a fragment reading the pixel its own draw wrote is a tile-local dependency, and - // a tiler handed a non-by-region dependency mid-pass has to assume the whole framebuffer - // is involved. - const VkDependencyFlags dependency_flags = inside_renderpass ? VK_DEPENDENCY_BY_REGION_BIT : 0; - VkImageMemoryBarrier barrier = {}; barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; barrier.newLayout = new_layout; @@ -250,7 +150,7 @@ namespace vk barrier.srcAccessMask = src_access; barrier.dstAccessMask = dst_access; - vkCmdPipelineBarrier(cmd, src_stage, dst_stage, dependency_flags, 0, nullptr, 0, nullptr, 1, &barrier); + vkCmdPipelineBarrier(cmd, src_stage, dst_stage, 0, 0, nullptr, 0, nullptr, 1, &barrier); } void insert_texture_barrier(const vk::command_buffer& cmd, vk::image* image, VkImageLayout new_layout, bool preserve_renderpass) diff --git a/rpcs3/Emu/RSX/VK/vkutils/chip_class.h b/rpcs3/Emu/RSX/VK/vkutils/chip_class.h index 2d73a05fc..b42ea50a2 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/chip_class.h +++ b/rpcs3/Emu/RSX/VK/vkutils/chip_class.h @@ -73,32 +73,6 @@ namespace vk driver_vendor get_driver_vendor(); - // True for tile-based deferred renderers. - // - // This matters for anything that reads back GPU-produced results mid-frame. A TBDR defers - // rasterization to an end-of-renderpass tiling pass, so a result that trickles in - // progressively on an immediate-mode GPU arrives here in one lump at the very end. Code - // that polls for early partial results cannot succeed early on these parts -- it only - // spins. See vk::query_pool_manager::get_query_result(). - constexpr bool is_tile_based_renderer(driver_vendor vendor) - { - switch (vendor) - { - case driver_vendor::TURNIP: - case driver_vendor::ADRENO: - case driver_vendor::ARM_MALI: - case driver_vendor::PANVK: - case driver_vendor::V3DV: - case driver_vendor::BROADCOM: - case driver_vendor::POWERVR: - case driver_vendor::MVK: - case driver_vendor::HONEYKRISP: - return true; - default: - return false; - } - } - struct chip_family_table { chip_class default_ = chip_class::unknown; diff --git a/rpcs3/Emu/RSX/VK/vkutils/device.h b/rpcs3/Emu/RSX/VK/vkutils/device.h index df4f9b70c..77c98fee5 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/device.h +++ b/rpcs3/Emu/RSX/VK/vkutils/device.h @@ -172,18 +172,6 @@ namespace vk VkQueue m_present_queue = VK_NULL_HANDLE; VkQueue m_transfer_queue = VK_NULL_HANDLE; - // Driver pipeline cache (SPIR-V -> ISA). RPCS3's own shader_cache only avoids - // re-GENERATING SPIR-V; without this the driver still re-runs its backend compile - // for every pipeline on every cold boot. VK_NULL_HANDLE on any failure, which is - // exactly what the create calls passed before, so every path stays safe. - VkPipelineCache m_pipeline_cache = VK_NULL_HANDLE; - // Last serialized size, so an unchanged cache skips the file write entirely. - mutable usz m_pipeline_cache_saved_size = 0; - - std::string get_pipeline_cache_path() const; - void load_pipeline_cache(); - void save_and_destroy_pipeline_cache(); - u32 m_graphics_queue_family = 0; u32 m_present_queue_family = 0; u32 m_transfer_queue_family = 0; @@ -253,13 +241,6 @@ namespace vk mem_allocator_base* get_allocator() const { return m_allocator.get(); } - // May legitimately be VK_NULL_HANDLE (unavailable or failed), which is the value - // the create calls used before this existed, so it is always safe to pass on. - VkPipelineCache get_pipeline_cache() const { return m_pipeline_cache; } - - // Serialize without destroying, for the periodic mid-session save. - void save_pipeline_cache() const; - operator VkDevice() const { return dev; } }; diff --git a/rpcs3/Emu/RSX/VK/vkutils/instance.cpp b/rpcs3/Emu/RSX/VK/vkutils/instance.cpp index 3d6edc6e8..3010487da 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/instance.cpp +++ b/rpcs3/Emu/RSX/VK/vkutils/instance.cpp @@ -54,19 +54,11 @@ namespace vk m_debugger = nullptr; } -#if defined(ANDROID) - // Destroy EVERY surface made on this instance, not just the most recent. Reaping them - // via vkDestroyInstance leaves the ANativeWindow "in use" and the next session fails - // with VK_ERROR_NATIVE_WINDOW_IN_USE_KHR. Every swapchain built on them is gone by now. - destroy_WSI_surfaces(m_instance); - m_surface = VK_NULL_HANDLE; -#else if (m_surface) { vkDestroySurfaceKHR(m_instance, m_surface, nullptr); m_surface = VK_NULL_HANDLE; } -#endif vkDestroyInstance(m_instance, nullptr); m_instance = VK_NULL_HANDLE; @@ -264,26 +256,13 @@ namespace vk instance_info.flags |= VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR; #endif - VkResult result = vkCreateInstance(&instance_info, nullptr, &m_instance); - - if (result == VK_ERROR_LAYER_NOT_PRESENT && !layers.empty()) + if (VkResult result = vkCreateInstance(&instance_info, nullptr, &m_instance); result != VK_SUCCESS) { - // The only layer we ever request here is VK_LAYER_KHRONOS_validation, enabled by the - // "Debug output" video setting. It ships with the Vulkan SDK and is absent on virtually - // every release Android device, so its absence must not take rendering down with it. - // Drop the optional layers and retry: toggling Debug Output on such a device then simply - // runs without GPU validation instead of failing with "No Vulkan device was created". - rsx_log.warning("Vulkan validation layer unavailable on this device; continuing without it."); + if (result == VK_ERROR_LAYER_NOT_PRESENT) + { + rsx_log.fatal("Could not initialize layer VK_LAYER_KHRONOS_validation"); + } - layers.clear(); - instance_info.enabledLayerCount = 0; - instance_info.ppEnabledLayerNames = nullptr; - - result = vkCreateInstance(&instance_info, nullptr, &m_instance); - } - - if (result != VK_SUCCESS) - { return false; } @@ -333,16 +312,11 @@ namespace vk VkSurfaceKHR instance::recreate_surface(display_handle_t window_handle) { -#if !defined(ANDROID) - // Desktop: release the previous surface before making a new one. On Android the - // lifetime is tracked globally (swapchain_android.hpp) and released at teardown or at - // swapchain reinit, so this instance never re-owns a surface to destroy here. if (m_surface != VK_NULL_HANDLE) { vkDestroySurfaceKHR(m_instance, m_surface, nullptr); m_surface = VK_NULL_HANDLE; } -#endif WSI_config surface_config { diff --git a/rpcs3/Emu/RSX/VK/vkutils/memory.cpp b/rpcs3/Emu/RSX/VK/vkutils/memory.cpp index b2ffade86..df96e329b 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/memory.cpp +++ b/rpcs3/Emu/RSX/VK/vkutils/memory.cpp @@ -588,35 +588,8 @@ namespace vk memory_block::~memory_block() { - // DIAGNOSTIC (restart crash): m_mem_allocator is BORROWED -- cached from - // get_current_mem_allocator() at construction and freed through here, with nothing tying - // its lifetime to the allocator's. Pressing Restart faults inside - // VmaAllocator_T::UpdateVulkanBudget reached from this free(), jumping through a garbage - // function pointer, which is what reading a dead VmaAllocator_T looks like. - // - // Everything that would explain it by ordering has been checked and does not: the heaps are - // freed at VKGSRender.cpp:879 while the device is not destroyed until :926, there is exactly - // one swapchain->destroy() caller, and data_heap_manager::reset() clears its set. So compare - // the allocator this block was built against with the one that is current now: if they - // differ, the block outlived its allocator and this names it. if (m_mem_allocator && m_mem_handle) { - // Not get_current_mem_allocator(): that dereferences g_render_device unconditionally, - // and a teardown where the device pointer is already gone is precisely the state being - // investigated -- the diagnostic must not be the thing that crashes. - const auto current = g_render_device ? g_render_device->get_allocator() : nullptr; - - if (current != m_mem_allocator) - { - rsx_log.error("[memblock] allocator changed under a live block: built=%p now=%p size=%llu -- skipping free", - static_cast(m_mem_allocator), static_cast(current), m_size); - - // Freeing through the stale pointer is the crash. The device that owned this memory - // is going away regardless, and vkDestroyDevice releases its allocations, so - // declining to free here loses nothing that is not already lost. - return; - } - m_mem_allocator->free(m_mem_handle); } } diff --git a/rpcs3/Emu/RSX/VK/vkutils/swapchain.cpp b/rpcs3/Emu/RSX/VK/vkutils/swapchain.cpp index f579ad94b..ef00a2957 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/swapchain.cpp +++ b/rpcs3/Emu/RSX/VK/vkutils/swapchain.cpp @@ -1,11 +1,6 @@ #include "stdafx.h" #include "swapchain.h" -#ifdef ANDROID -#include "Emu/Cell/timers.hpp" -#include -#endif - namespace vk { // Swapchain image RPCS3 @@ -240,33 +235,10 @@ namespace vk } u32 nb_available_modes = 0; - { - const VkResult res = vkGetPhysicalDeviceSurfacePresentModesKHR(gpu, m_surface, &nb_available_modes, nullptr); -#ifdef ANDROID - // A surface bounce -- the app being backgrounded during the boot/compile splash -- makes - // this query return SURFACE_LOST. That is recoverable, so bail and let the per-frame - // reinitialize_swapchain() rebuild the surface rather than dying here. - if (res == VK_ERROR_SURFACE_LOST_KHR) - { - rsx_log.warning("Swapchain: surface lost while querying present mode count; will recreate."); - return false; - } -#endif - CHECK_RESULT(res); - } + CHECK_RESULT(vkGetPhysicalDeviceSurfacePresentModesKHR(gpu, m_surface, &nb_available_modes, nullptr)); std::vector present_modes(nb_available_modes); - { - const VkResult res = vkGetPhysicalDeviceSurfacePresentModesKHR(gpu, m_surface, &nb_available_modes, present_modes.data()); -#ifdef ANDROID - if (res == VK_ERROR_SURFACE_LOST_KHR) - { - rsx_log.warning("Swapchain: surface lost while querying present modes; will recreate."); - return false; - } -#endif - CHECK_RESULT(res); - } + CHECK_RESULT(vkGetPhysicalDeviceSurfacePresentModesKHR(gpu, m_surface, &nb_available_modes, present_modes.data())); VkPresentModeKHR swapchain_present_mode = VK_PRESENT_MODE_FIFO_KHR; std::vector preferred_modes; @@ -380,24 +352,7 @@ namespace vk rsx_log.notice("Swapchain: requesting full screen exclusive mode %d.", static_cast(full_screen_exclusive_info.fullScreenExclusive)); #endif -#ifdef ANDROID - // This create was previously unchecked. A surface lost here is recoverable, so bail BEFORE - // destroying old_swapchain or calling init_swapchain_images (which would throw on zero - // images), keeping old_swapchain as the live handle for create() to reclaim on the recovery - // pass. Every other create failure stays fatal. - { - const VkResult res = _vkCreateSwapchainKHR(dev, &swap_info, nullptr, &m_vk_swapchain); - if (res == VK_ERROR_SURFACE_LOST_KHR) - { - rsx_log.warning("Swapchain: surface lost during vkCreateSwapchainKHR; will recreate."); - m_vk_swapchain = old_swapchain; - return false; - } - CHECK_RESULT(res); - } -#else _vkCreateSwapchainKHR(dev, &swap_info, nullptr, &m_vk_swapchain); -#endif if (old_swapchain) { @@ -413,143 +368,8 @@ namespace vk return true; } -#ifdef ANDROID - // Tell the panel what cadence the content actually runs at, so a 90/120Hz display can - // align its refresh to a clean multiple for a steady 30/60fps game: less judder, less - // power. Purely advisory, and only re-pushed when the snapped rate changes. - // - // Ported from ouroboros420/rpcsx (9e06434bfe, c8e52ecf9c, 511ded8830, 88db6c8eed, - // ea69eebdb7), with the frame period measured here instead of through their global - // ADPF reporter, so this carries no dependency outside the swapchain. - void swapchain_WSI::push_frame_rate_hint() - { - // ANativeWindow_setFrameRate* is API 30+ while minSdk is 29, so this resolves at - // runtime rather than through a compile-time availability guard. - // - // The symbols live in libnativewindow.so, NOT libandroid.so -- libandroid only - // provides ANativeWindow_fromSurface/_acquire. libnativewindow is not in this - // object's DT_NEEDED set, so dlsym(RTLD_DEFAULT, ...) cannot see them under - // Android's namespaced linker. dlopen it by name; the handle is deliberately - // never released, since the library stays resident for the process anyway. - using sfr_strat_fn = int32_t (*)(ANativeWindow*, float, int8_t, int8_t); - using sfr_plain_fn = int32_t (*)(ANativeWindow*, float, int8_t); - - struct sfr_fns { sfr_strat_fn strat; sfr_plain_fn plain; }; - static const sfr_fns s_sfr = []() -> sfr_fns - { - void* const lib = dlopen("libnativewindow.so", RTLD_NOW); - if (!lib) - { - rsx_log.error("Android: libnativewindow.so failed to load; no frame rate hint."); - return { nullptr, nullptr }; - } - - return { - reinterpret_cast(dlsym(lib, "ANativeWindow_setFrameRateWithChangeStrategy")), - reinterpret_cast(dlsym(lib, "ANativeWindow_setFrameRate")), - }; - }(); - - if (!s_sfr.strat && !s_sfr.plain) - { - return; - } - - const u64 now = get_system_time(); - const u64 last = std::exchange(m_last_present_time, now); - - if (!last || now <= last) - { - return; - } - - const float fps = 1.0e6f / static_cast(now - last); - - if (fps <= 1.0f || fps >= 1000.0f) - { - return; - } - - // Snap the noisy per-present rate to a real PS3 cadence. Pushing the raw value would - // have SurfaceFlinger renegotiating the panel almost every frame, which produces - // beat-frequency judder under FIFO -- the exact opposite of the point. - const auto quantize_cadence = [](float f) -> float - { - static constexpr float cadences[] = { 24.f, 25.f, 30.f, 50.f, 60.f }; - - for (const float c : cadences) - { - if ((f > c ? f - c : c - f) <= c * 0.12f) - { - return c; - } - } - - return static_cast(static_cast(f + 0.5f)); - }; - - const float snapped = quantize_cadence(fps); - - // Roughly 0.75s of presents. A transient (a loading hitch, a boot-time outlier) - // resets the counter and never reaches the panel; only a sustained cadence does. - constexpr u32 stable_frames_required = 24; - - if (snapped == m_pending_frame_rate_hint) - { - if (m_frame_rate_hint_stable_count < stable_frames_required) - { - ++m_frame_rate_hint_stable_count; - } - } - else - { - m_pending_frame_rate_hint = snapped; - m_frame_rate_hint_stable_count = 1; - } - - if (m_frame_rate_hint_stable_count < stable_frames_required || snapped == m_last_frame_rate_hint) - { - return; - } - - auto awnd = std::get_if(&window_handle); - if (!awnd || !*awnd) - { - return; - } - - // FIXED_SOURCE (1): emulated output is fixed-cadence content, so the system should - // aim for a clean multiple. ONLY_IF_SEAMLESS (0): never trigger a visible, flickering - // mode switch just to honour a hint. - constexpr int8_t compat_fixed_source = 1; - constexpr int8_t change_only_if_seamless = 0; - - if (s_sfr.strat) - { - s_sfr.strat(*awnd, snapped, compat_fixed_source, change_only_if_seamless); - } - else - { - s_sfr.plain(*awnd, snapped, compat_fixed_source); - } - - m_last_frame_rate_hint = snapped; - - if (static bool s_logged_once = false; !s_logged_once) - { - rsx_log.notice("Android: frame rate hint active (%.0f fps, %s)", - static_cast(snapped), s_sfr.strat ? "seamless" : "legacy"); - s_logged_once = true; - } - } -#endif - VkResult swapchain_WSI::present(VkSemaphore semaphore, u32 image) { -#ifdef ANDROID - push_frame_rate_hint(); -#endif - VkPresentInfoKHR present = {}; present.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR; present.pNext = nullptr; diff --git a/rpcs3/Emu/RSX/VK/vkutils/swapchain_android.hpp b/rpcs3/Emu/RSX/VK/vkutils/swapchain_android.hpp index 79ef8fbd8..dc3b5ef8f 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/swapchain_android.hpp +++ b/rpcs3/Emu/RSX/VK/vkutils/swapchain_android.hpp @@ -2,53 +2,12 @@ #include "swapchain_core.h" -#include -#include - namespace vk { #if defined(ANDROID) using swapchain_ANDROID = native_swapchain_base; using swapchain_NATIVE = swapchain_ANDROID; - // Android permits only one *connected* VkSurfaceKHR per ANativeWindow, and the - // Adreno/Turnip driver only releases the window's producer claim on an explicit - // vkDestroySurfaceKHR -- NOT when the surface is reaped as a child of vkDestroyInstance. - // Across a savestate reload the renderer and its VkInstance are destroyed and rebuilt, and - // opening the home menu reinitializes the swapchain (a second make_WSI_surface), leaving - // the previous surface for vkDestroyInstance to reap. That leftover keeps the window "in - // use", so the next session's vkCreateAndroidSurfaceKHR fails with - // VK_ERROR_NATIVE_WINDOW_IN_USE_KHR. Track every WSI surface so each gets an explicit - // destroy. Create/destroy is serialized by the emulation lifecycle (one renderer at a - // time), so no lock. Ported from ouroboros420/rpcsx (7a82c7647, 52a020464). - inline std::vector> g_wsi_surfaces; - - static inline void track_WSI_surface(VkInstance vk_instance, VkSurfaceKHR surface) - { - if (surface != VK_NULL_HANDLE) - { - g_wsi_surfaces.emplace_back(vk_instance, surface); - } - } - - // Destroy every surface tracked for this instance. Safe to call whenever no swapchain - // is built on them; callers guarantee that. - static inline void destroy_WSI_surfaces(VkInstance vk_instance) - { - for (auto it = g_wsi_surfaces.begin(); it != g_wsi_surfaces.end();) - { - if (it->first == vk_instance) - { - vkDestroySurfaceKHR(it->first, it->second, nullptr); - it = g_wsi_surfaces.erase(it); - } - else - { - ++it; - } - } - } - [[maybe_unused]] static VkSurfaceKHR make_WSI_surface(VkInstance vk_instance, display_handle_t window_handle, WSI_config* /*config*/) { @@ -65,8 +24,6 @@ namespace vk createInfo.window = std::get(window_handle); CHECK_RESULT(vkCreateAndroidSurfaceKHR(vk_instance, &createInfo, nullptr, &result)); - - track_WSI_surface(vk_instance, result); return result; } #endif diff --git a/rpcs3/Emu/RSX/VK/vkutils/swapchain_core.h b/rpcs3/Emu/RSX/VK/vkutils/swapchain_core.h index fd67b6526..d5113be9d 100644 --- a/rpcs3/Emu/RSX/VK/vkutils/swapchain_core.h +++ b/rpcs3/Emu/RSX/VK/vkutils/swapchain_core.h @@ -206,17 +206,6 @@ namespace vk bool m_wm_reports_flag = false; -#ifdef ANDROID - // Frame-pacing hint state. The raw present interval is far too noisy to hand to - // the compositor directly, so it is quantized to a real cadence and debounced. - u64 m_last_present_time = 0; - float m_pending_frame_rate_hint = 0.f; - float m_last_frame_rate_hint = 0.f; - u32 m_frame_rate_hint_stable_count = 0; - - void push_frame_rate_hint(); -#endif - protected: void init_swapchain_images(render_device& dev, u32 preferred_count = 0) override; @@ -225,12 +214,8 @@ namespace vk ~swapchain_WSI() override = default; - void create(display_handle_t& handle) override - { - // Keep the native window: present-time hints (ANativeWindow_setFrameRate on - // Android) need it, and discarding it here is why they were inert. - window_handle = handle; - } + void create(display_handle_t&) override + {} void destroy(bool = true) override; diff --git a/rpcs3/Emu/RSX/rsx_profiler.cpp b/rpcs3/Emu/RSX/rsx_profiler.cpp index 914f5862d..cc2d0c44b 100644 --- a/rpcs3/Emu/RSX/rsx_profiler.cpp +++ b/rpcs3/Emu/RSX/rsx_profiler.cpp @@ -52,7 +52,6 @@ namespace rsx::prof u64 g_fifo_refill_stalls = 0; u64 g_fifo_refill_stall_us = 0; u64 g_render_passes = 0; - u64 g_rp_reopened = 0; u64 g_mprotect_calls = 0; u64 g_mprotect_bytes = 0; u64 g_access_violations = 0; @@ -76,18 +75,6 @@ namespace rsx::prof "TexCache:1227", "ImgHelper:43", "GSR:2811", - // Draw:1093 split by what actually mismatched. It is the only teardown site that can - // fire for two unrelated reasons, and they have opposite prognoses: a framebuffer - // switch is the game changing render target and is irreducible, while a render pass - // handle mismatch on the same framebuffer can only come from the attachment layouts, - // which are part of the render pass key and are what surface parking controls. - // Without the split, parking moving work between the two is indistinguishable from - // parking creating work. - // - // Bracketed because these two are a breakdown of Draw:1093 and not sites of their own. - // They sum to it, so the frame's teardown total is still the unbracketed names alone. - "(Draw:1093 key)", - "(Draw:1093 fbo)", }; u64 g_flush_sites[flush_site_count] = {}; const char* g_flush_site_names[flush_site_count] = { @@ -114,8 +101,8 @@ namespace rsx::prof "Present:1102", }; - const void* g_rp_callers[rp_caller_levels][rp_caller_slots] = {}; - u64 g_rp_caller_counts[rp_caller_levels][rp_caller_slots] = {}; + const void* g_rp_callers[2][rp_caller_slots] = {}; + u64 g_rp_caller_counts[2][rp_caller_slots] = {}; void note_rp_teardown(const void* caller, u32 level) { @@ -415,14 +402,8 @@ namespace rsx::prof static_cast(g_mprotect_bytes) / 1048576.0 / frames, static_cast(g_access_violations) / frames); - // Redundant means the identical (pass, framebuffer) came straight back after the - // teardown, so the tile store and reload bought nothing. Passes minus redundant is - // the floor: no amount of barrier suppression can go below it, because the rest are - // framebuffer switches the game asked for. - fmt::append(report, "\n\trender passes %.1f/frame, %.1f redundant (floor %.1f)", - static_cast(g_render_passes) / frames, - static_cast(g_rp_reopened) / frames, - static_cast(g_render_passes - std::min(g_rp_reopened, g_render_passes)) / frames); + fmt::append(report, "\n\trender passes %.1f/frame", + static_cast(g_render_passes) / frames); { std::string sites; @@ -447,7 +428,7 @@ namespace rsx::prof static_cast(g_fifo_refill_bytes) / static_cast(g_fifo_refills)); } - for (u32 level = 0; level < rp_caller_levels; level++) + for (u32 level = 0; level < 2; level++) { std::pair top[6] = {}; for (usz i = 0; i < rp_caller_slots; i++) @@ -490,17 +471,8 @@ namespace rsx::prof fmt::append(list, "\n\t %-44s %6.1f/frame", where, static_cast(count) / frames); } - // Level 2 is the one to act on: same addresses as level 0, filtered to the teardowns - // that were actually wasted. A caller that is large in level 0 and small here was - // tearing down a pass that was ending anyway. - static const char* const level_names[rp_caller_levels] = - { - "direct caller", - "change_layout caller", - "REDUNDANT teardown caller" - }; - - fmt::append(report, "\n\trp teardown by %s%s", level_names[level], list); + fmt::append(report, "\n\trp teardown by %s%s", + level == 0 ? "direct caller" : "change_layout caller", list); } { @@ -716,7 +688,6 @@ namespace rsx::prof g_fifo_refill_stalls = 0; g_fifo_refill_stall_us = 0; g_render_passes = 0; - g_rp_reopened = 0; g_mprotect_calls = 0; g_mprotect_bytes = 0; g_access_violations = 0; diff --git a/rpcs3/Emu/RSX/rsx_profiler.h b/rpcs3/Emu/RSX/rsx_profiler.h index b60252b8d..c47d52c01 100644 --- a/rpcs3/Emu/RSX/rsx_profiler.h +++ b/rpcs3/Emu/RSX/rsx_profiler.h @@ -460,19 +460,6 @@ namespace rsx::prof // its per-frame event cap was blown out by roughly 1800 per frame, so count them plainly. extern u64 g_render_passes; - // Of those, how many reopened the identical (pass, framebuffer) that had just been closed. - // - // A site counter can only say where a teardown was charged, never whether it was needed. If - // the next draw switches framebuffer, the pass was ending regardless, and removing the - // barrier that got there first only moves the charge to the draw site - measured in - // iteration 3, where ImgHelper:43 fell and (Draw:1093 fbo) rose by the same amount for no - // change in frame time. - // - // This counts only the teardowns that were genuinely wasted: tile stored, barrier recorded, - // same tile reloaded. It is therefore the ceiling on what any amount of barrier suppression - // can win, and (render passes - redundant) is the floor no such work can go below. - extern u64 g_rp_reopened; - // Page protection traffic. Every change is an mprotect, which on ARM forces TLB // maintenance, and every fault on a protected page is a SIGSEGV round trip through the // handler before it. The RSX thread was measured at about 34% kernel time reached @@ -495,23 +482,16 @@ namespace rsx::prof * 75 sites by hand is not worth it and would still miss the next one; recording the return * address costs one instruction on a path taken about twenty times a frame. * - * Three levels, because most callers do not call it directly: image::change_layout funnels + * Two levels, because most callers do not call it directly: image::change_layout funnels * them, so a single level would report that function for nearly everything. The first table * is whoever called change_image_layout, the second is whoever called change_layout. * - * The third is different in kind: it is charged at begin_renderpass, not at the teardown, and - * only for teardowns that turned out to be redundant (see g_rp_reopened). Same addresses, - * filtered down to the ones that actually cost something. Read level 2 against level 0: a - * site that is large in level 0 and absent from level 2 is a site whose pass was ending - * anyway, and removing its barrier will not move the frame. - * * Addresses are reported raw plus an offset from the module base, which llvm-symbolizer * turns into names against the unstripped build-android copy. */ inline constexpr usz rp_caller_slots = 24; - inline constexpr u32 rp_caller_levels = 3; - extern const void* g_rp_callers[rp_caller_levels][rp_caller_slots]; - extern u64 g_rp_caller_counts[rp_caller_levels][rp_caller_slots]; + extern const void* g_rp_callers[2][rp_caller_slots]; + extern u64 g_rp_caller_counts[2][rp_caller_slots]; void note_rp_teardown(const void* caller, u32 level);