Merge pull request #7137 from Lberonio/fix/agc-eop-latch-queue-mode

fix(agc): suppress the flip end-of-pipe event on queues that raise release interrupts

core/libs/prx/libSceAgcDriver/CMakeLists.txt

@@ -1777,8 +1777,10 @@ if(BUILD_TESTING)
     endforeach()
     add_test(NAME agc_driver_readonly_resummarize COMMAND agc_driver_tests --readonly-resummarize)
     set_tests_properties(agc_driver_readonly_resummarize PROPERTIES TIMEOUT 10)
+    add_test(NAME agc_driver_eop_interrupts COMMAND agc_driver_tests --eop-interrupts)
+    set_tests_properties(agc_driver_eop_interrupts PROPERTIES TIMEOUT 60)
     if(WIN32)
-        set_tests_properties(agc_driver_readonly_resummarize PROPERTIES ENVIRONMENT_MODIFICATION
+        set_tests_properties(agc_driver_readonly_resummarize agc_driver_eop_interrupts PROPERTIES ENVIRONMENT_MODIFICATION
             "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
     endif()
     add_test(NAME agc_shader_mode_register COMMAND agc_shader_mode_register_tests)

core/libs/prx/libSceAgcDriver/Eq/include/Event.hpp

@@ -4,7 +4,7 @@
 #include <cstdint>
 #include "SceTypes.hpp"
 
-void AgcDriverDeliverEopInterrupt(std::uint32_t queue);
+bool AgcDriverDeliverEopInterrupt(std::uint32_t queue);
 
 extern "C" int APS5_VABI sceAgcDriverAddEqEvent(KernelEqueue eq, int id, void* udata);
 extern "C" int APS5_VABI sceAgcDriverDeleteEqEvent(KernelEqueue eq, int id);

core/libs/prx/libSceAgcDriver/Eq/src/Event.cpp

@@ -25,17 +25,20 @@ std::vector<Registration> g_registrations;
 
 }
 
-void AgcDriverDeliverEopInterrupt(std::uint32_t queue) {
+bool AgcDriverDeliverEopInterrupt(std::uint32_t queue) {
     std::vector<Registration> registrations;
     {
         std::lock_guard lock(g_mutex);
         registrations = g_registrations;
     }
+    bool delivered = false;
     for (const auto& registration : registrations) {
         if (registration.id == static_cast<int>(queue)) {
             EqueueTriggerEvent_nid_postfix(registration.eq, static_cast<uintptr_t>(registration.id), EvfiltGraphicsCore, nullptr);
+            delivered = true;
         }
     }
+    return delivered;
 }
 
 extern "C" {

core/libs/prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp

@@ -315,6 +315,8 @@ private:
     FrameTiming* frameTiming();
     FrameTiming* includeTimingSubmission(const Submission& submission, bool firstSegment);
     void execute(const Submission& submission);
+    void deliverGuestEopInterrupt(std::uint32_t queue);
+    void deliverFlipEopInterrupt(std::uint32_t queue);
     void markCompleted(std::uint64_t serial);
     static const std::atomic<std::uint64_t>*& workerQueued();
     void reapCompletionLabels();
@@ -332,6 +334,9 @@ private:
     std::atomic<std::uint64_t> flipSerial{0};
     std::atomic<std::uint64_t> flipBatchesUnsignaled{0};
 
+    std::mutex eopSelfSignalMutex;
+    std::unordered_map<std::uint32_t, std::uint32_t> eopSelfSignalFlips;
+
     alignas(64) std::atomic<int> packetsInFlight{0};
     std::atomic<std::uint64_t> packetsDone{0};
 

core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp

@@ -19,6 +19,8 @@ namespace AgcDriver::DriverDetail {
 
 namespace {
 
+constexpr std::uint32_t eopSelfSignalFlipLimit = 4;
+
 bool orderedFills() {
     static const bool ordered = std::getenv("APS5_PIPELINE_DRAIN_FILLS") == nullptr;
     return ordered;
@@ -43,6 +45,24 @@ bool keepsDrawPipeline(std::span<const std::uint32_t> packet, std::uint32_t head
 
 }
 
+void Driver::deliverGuestEopInterrupt(std::uint32_t queue) {
+    if (!AgcDriverDeliverEopInterrupt(queue)) return;
+    std::lock_guard<std::mutex> lock(eopSelfSignalMutex);
+    eopSelfSignalFlips[queue] = 0;
+}
+
+void Driver::deliverFlipEopInterrupt(std::uint32_t queue) {
+    {
+        std::lock_guard<std::mutex> lock(eopSelfSignalMutex);
+        const auto latched = eopSelfSignalFlips.find(queue);
+        if (latched != eopSelfSignalFlips.end()) {
+            if (++latched->second <= eopSelfSignalFlipLimit) return;
+            eopSelfSignalFlips.erase(latched);
+        }
+    }
+    AgcDriverDeliverEopInterrupt(queue);
+}
+
 template <typename TWork>
 void Driver::timed(double WorkerProfile::*bucket, TWork&& work) {
     static thread_local WorkerProfile profile;
@@ -276,7 +296,7 @@ void Driver::execute(const Submission& submission) {
                     Pm4::Execute(packet, queue);
                     if (const auto label = Pm4::DecodeLabelWrite(packet)) noteLabelStore(label->address, label->Bytes(), ++eventSerial, submission.queue);
                 }
-                if (endOfPipeInterrupt && !interruptDeferred) AgcDriverDeliverEopInterrupt(submission.queue);
+                if (endOfPipeInterrupt && !interruptDeferred) deliverGuestEopInterrupt(submission.queue);
             }
             if (deviceUse.owns_lock()) deviceUse.unlock();
             CheckFailure();
@@ -292,10 +312,10 @@ void Driver::execute(const Submission& submission) {
                 if (localDevice != nullptr) {
                     localDevice->FlipBatches(batchesAtFlip, unsignaledAtFlip);
                     const auto queueId = submission.queue;
-                    flipInterruptDeferred = localDevice->AfterRecordedWork([queueId] { AgcDriverDeliverEopInterrupt(queueId); }, false);
+                    flipInterruptDeferred = localDevice->AfterRecordedWork([this, queueId] { deliverFlipEopInterrupt(queueId); }, false);
                 }
             }
-            if (!flipInterruptDeferred) AgcDriverDeliverEopInterrupt(submission.queue);
+            if (!flipInterruptDeferred) deliverFlipEopInterrupt(submission.queue);
             ++flipsCounted;
             if (batchesAtFlip != 0) flipSerial = batchesAtFlip;
             flipBatchesUnsignaled += unsignaledAtFlip;
@@ -398,7 +418,7 @@ void Driver::execute(const Submission& submission) {
                     if (const auto label = Pm4::DecodeLabelWrite(packet)) noteLabelStore(label->address, label->Bytes(), ++eventSerial, submission.queue);
                 }
             }
-            if (endOfPipeInterrupt && !interruptDeferred) AgcDriverDeliverEopInterrupt(submission.queue);
+            if (endOfPipeInterrupt && !interruptDeferred) deliverGuestEopInterrupt(submission.queue);
         }
         if (drawPacket || (sampleDump && wroteOnGpu)) Graphics::Recorder::CountRecordedWork();
         timing.Mark(header == RenderingWaitPacketHeader ? "rendering_wait" : opcode == 0x3c || opcode == 0x93 ? "memory_wait" : opcode == 0x15 || opcode == 0x16 ? "dispatch" : drawPacket ? "draw" : "execute");

core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketMemory.cpp

@@ -54,7 +54,7 @@ bool Driver::preparePacketMemory(const Submission& submission, QueueState& queue
             }
             if (reason == 0 || reason == 5) {
                 const auto queueId = submission.queue;
-                interruptDeferred = localDevice->AfterRecordedWork([queueId] { AgcDriverDeliverEopInterrupt(queueId); }, submission.queue == 0);
+                interruptDeferred = localDevice->AfterRecordedWork([this, queueId] { deliverGuestEopInterrupt(queueId); }, submission.queue == 0);
                 if (interruptDeferred && workOpen) localDevice->SubmitRecorded(submission.queue == 0);
                 wroteOnGpu = true;
             } else if (reason == 1) {
@@ -397,10 +397,10 @@ void Driver::commitLabel(std::uint32_t queue, std::uint64_t address, std::span<c
     if (endOfPipeInterrupt) {
         bool deferred = false;
         if (localDevice != nullptr && (reason == 0 || reason == 5 || reason == 6)) {
-            deferred = localDevice->AfterRecordedWork([queue] { AgcDriverDeliverEopInterrupt(queue); }, queue == 0);
+            deferred = localDevice->AfterRecordedWork([this, queue] { deliverGuestEopInterrupt(queue); }, queue == 0);
             if (deferred && workOpen) localDevice->SubmitRecorded(queue == 0);
         }
-        if (!deferred) AgcDriverDeliverEopInterrupt(queue);
+        if (!deferred) deliverGuestEopInterrupt(queue);
     }
     submitDueAfterCommit(localDevice.get());
 }

core/libs/prx/libSceAgcDriver/tests/Submit.cpp

@@ -195,11 +195,26 @@ void testEndOfPipeInterrupts() {
     check(sceAgcDriverAddEqEvent(eq, 0, &graphicsTag) == 0, "graphics event registration failed");
     check(sceAgcDriverAddEqEvent(eq, 0x20, &computeTag) == 0, "compute event registration failed");
     expectFailure([] { sceAgcDriverAddEqEvent(0, 0, nullptr); });
+    struct Output final : AgcDriver::IVideoOutput, AgcDriver::IFlipRequest, std::enable_shared_from_this<Output> {
+        std::atomic<int> ready{0};
+        std::shared_ptr<AgcDriver::IFlipRequest> Reserve(const AgcDriver::FlipInfo&) override { return shared_from_this(); }
+        void GpuReady(const std::shared_ptr<AgcDriver::FrameTiming>&) override { ready.fetch_add(1); }
+        void Fail(std::exception_ptr error) noexcept override { if (!error) std::terminate(); }
+    };
+    const auto output = std::make_shared<Output>();
+    AgcDriverRegisterVideoOutput_nid_postfix(7, output);
+    std::array<std::uint32_t, 6> flip{AgcDriver::FlipPacketHeader, 7, 0xfffffffeu, 1, 0, 0};
+    Packet flipPacket{flip.data(), static_cast<std::uint32_t>(flip.size()), 0, {}};
+    std::array<KernelEvent, 2> events{};
+    check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "flip-only queue submit failed");
+    AgcDriverWaitIdle_nid_postfix();
+    check(output->ready.load() == 1, "flip-only queue flip was not handed to its output");
+    check(owner->GetTriggeredEvents(events.data(), 2) == 1 && events[0].udata == &graphicsTag && events[0].data == 1 && sceAgcDriverGetEqEventType(events.data()) == 0, "flip on a queue that never requested a release interrupt did not raise its event");
+    check(owner->GetTriggeredEvents(events.data(), 2) == 0, "flip-only queue event was not cleared");
     std::array<std::uint32_t, 8> words{0xc0064900, 0, 1u << 24u, 0, 0, 0, 0, 0};
     Packet packet{words.data(), static_cast<std::uint32_t>(words.size()), 0, {}};
     check(sceAgcDriverSubmitDcb(&packet) == 0 && sceAgcDriverSubmitDcb(&packet) == 0, "interrupt submit failed");
     AgcDriverWaitIdle_nid_postfix();
-    std::array<KernelEvent, 2> events{};
     check(owner->GetTriggeredEvents(events.data(), 2) == 1, "graphics end-of-pipe interrupt was not delivered to its queue only");
     check(events[0].filter == -14 && events[0].udata == &graphicsTag && events[0].data == 2 && sceAgcDriverGetEqEventType(events.data()) == 0, "graphics end-of-pipe event encoding is wrong");
     check(sceAgcDriverGetEqContextId(events.data()) == 0, "graphics end-of-pipe event reports another context");
@@ -212,21 +227,39 @@ void testEndOfPipeInterrupts() {
     check(sceAgcDriverSubmitDcb(&packet) == 0, "plain release submit failed");
     AgcDriverWaitIdle_nid_postfix();
     check(owner->GetTriggeredEvents(events.data(), 2) == 0, "release without INT_SEL raised an interrupt");
-    struct Output final : AgcDriver::IVideoOutput, AgcDriver::IFlipRequest, std::enable_shared_from_this<Output> {
-        std::atomic<int> ready{0};
-        std::shared_ptr<AgcDriver::IFlipRequest> Reserve(const AgcDriver::FlipInfo&) override { return shared_from_this(); }
-        void GpuReady(const std::shared_ptr<AgcDriver::FrameTiming>&) override { ready.fetch_add(1); }
-        void Fail(std::exception_ptr error) noexcept override { if (!error) std::terminate(); }
-    };
-    const auto output = std::make_shared<Output>();
-    AgcDriverRegisterVideoOutput_nid_postfix(7, output);
-    std::array<std::uint32_t, 6> flip{AgcDriver::FlipPacketHeader, 7, 0xfffffffeu, 1, 0, 0};
-    Packet flipPacket{flip.data(), static_cast<std::uint32_t>(flip.size()), 0, {}};
-    check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "flip submit failed");
+    check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "flip after a release interrupt submit failed");
     AgcDriverWaitIdle_nid_postfix();
-    check(output->ready.load() == 1, "flip was not handed to its output");
-    check(owner->GetTriggeredEvents(events.data(), 2) == 1 && events[0].udata == &graphicsTag && events[0].data == 1 && sceAgcDriverGetEqEventType(events.data()) == 0, "flip did not raise the graphics end-of-pipe event");
-    check(owner->GetTriggeredEvents(events.data(), 2) == 0, "flip event was not cleared");
+    check(output->ready.load() == 2, "flip after a release interrupt was not handed to its output");
+    check(owner->GetTriggeredEvents(events.data(), 2) == 0, "flip on a queue that already raised its own release interrupt raised a second event");
+    words[2] = 1u << 24u;
+    check(sceAgcDriverSubmitDcb(&packet) == 0, "release before sustained flips submit failed");
+    AgcDriverWaitIdle_nid_postfix();
+    check(owner->GetTriggeredEvents(events.data(), 2) == 1, "a release interrupt did not deliver on a queue whose flip event is suppressed");
+    std::uint32_t sustainedFlips = 0;
+    std::uint32_t flipsUntilEvent = 0;
+    while (sustainedFlips < 8 && flipsUntilEvent == 0) {
+        check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "sustained flip submit failed");
+        AgcDriverWaitIdle_nid_postfix();
+        ++sustainedFlips;
+        check(output->ready.load() == static_cast<int>(2 + sustainedFlips), "sustained flip was not handed to its output");
+        if (owner->GetTriggeredEvents(events.data(), 2) != 0) flipsUntilEvent = sustainedFlips;
+    }
+    check(flipsUntilEvent != 0, "a flip never raised an event again on a queue whose own release interrupts stopped");
+    check(flipsUntilEvent > 2, "a flip raised an event within two flips of the queue's own release interrupt");
+    check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "flip after the latch cleared submit failed");
+    AgcDriverWaitIdle_nid_postfix();
+    check(output->ready.load() == static_cast<int>(3 + sustainedFlips), "flip after the latch cleared was not handed to its output");
+    check(owner->GetTriggeredEvents(events.data(), 2) == 1, "a flip did not raise an event after the latch cleared");
+    words[2] = 1u << 24u;
+    check(sceAgcDriverSubmitDcb(&packet) == 0, "release after a cleared latch submit failed");
+    AgcDriverWaitIdle_nid_postfix();
+    check(owner->GetTriggeredEvents(events.data(), 2) == 1, "a release interrupt did not deliver after the latch cleared");
+    for (int index = 0; index < 2; ++index) {
+        check(sceAgcDriverSubmitDcb(&flipPacket) == 0, "flip paired with a release interrupt submit failed");
+        AgcDriverWaitIdle_nid_postfix();
+        check(output->ready.load() == static_cast<int>(4 + sustainedFlips + index), "flip paired with a release interrupt was not handed to its output");
+        check(owner->GetTriggeredEvents(events.data(), 2) == 0, "a flip raised an event within two flips of the queue's own release interrupt");
+    }
     AgcDriverUnregisterVideoOutput_nid_postfix(7, output);
     alignas(8) static volatile std::uint64_t label = 0;
     const auto labelAddress = reinterpret_cast<std::uintptr_t>(&label);
@@ -834,6 +867,12 @@ void testWorkerFailure() {
 
 int main(int argc, char** argv) {
     try {
+        if (argc == 2 && std::string_view(argv[1]) == "--eop-interrupts") {
+            testEndOfPipeInterrupts();
+            LibcRunShutdown_nid_postfix();
+            std::puts("AGC end-of-pipe interrupt queue mode tests passed");
+            return 0;
+        }
         if (argc == 2 && std::string_view(argv[1]) == "--readonly-resummarize") {
             testReadOnlyResummarize();
             LibcRunShutdown_nid_postfix();

docs/dev/debt/libSceAgcDriver.md

@@ -175,7 +175,7 @@
 - [Descriptor pool recycling](../../../core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp) avoids individually freed sets: [RADV 26.2.2](https://gitlab.freedesktop.org/mesa/mesa/-/blob/mesa-26.2.2/src/amd/vulkan/radv_descriptor_pool.c) initializes its free-set heap with 32 bytes beyond the backing allocation. An allocation-only RX 6600 test reproduces acceptance of a 4,128-byte set in a 4,096-byte pool; the path without individual frees rejects it. Pools reset after their last set is released. A cached set can retain unused slots in an exhausted pool until eviction; peak gameplay pool memory remains unmeasured.
 - [16-bit depth with stencil](../../../core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp) (libSceAgcDriver) is stored as `D24_UNORM_S8_UINT`, or `D32_SFLOAT_S8_UINT`, on a device without `D16_UNORM_S8_UINT` as an attachment. Depth comparisons, clears and the depth image see the wider format's precision, so results at the 16-bit quantization boundary can differ from the console; the constant depth bias is scaled by 256 for D24, and a non-zero constant on the D32 float fallback throws. Not measured on console hardware.
 - [Depth surfaces created over storage writes](../../../core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp) (libSceAgcDriver): a depth surface created without HTILE copies its depth plane from the storage image at the plane's address, a pending one first, when that image has the plane's extent, one level, a 2D view or a 2D array view (its layer 0 is copied) and a 16-bit format for a 16-bit plane or a 32-bit format for a 32-bit plane. A multisampled surface, a surface the device stores in another format, a surface with HTILE and a surface without such an image start from `DB_DEPTH_CLEAR`, and the stencil plane from `DB_STENCIL_CLEAR`; CPU writes to the plane's memory are not read. Not measured on the console, where the plane is that memory.
-- [Flip packets raise the graphics end-of-pipe event](../../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp) (libSceAgcDriver) - a flip delivers the queue's end-of-pipe event (`sceAgcDriverAddEqEvent`) once the work recorded before it completes, as a `RELEASE_MEM` with `INT_SEL` does. Minecraft (PPSA17221) registers the graphics event, never requests an interrupt in its releases, and waits on the event after every flip while holding its render lock, so without this it never presents a second frame. Whether the console raises the event from the flip itself or from an interrupt its flip sequence contains is not measured; a title that waits only on release interrupts sees one extra event per flip, with `data` counting it.
+- [Flip packets raise the graphics end-of-pipe event](../../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp) (libSceAgcDriver) - a flip delivers the queue's end-of-pipe event (`sceAgcDriverAddEqEvent`) once the work recorded before it completes, as a `RELEASE_MEM` with `INT_SEL` does, but only on a queue that has never delivered an interrupt from such a release: the queue is latched the first time a release interrupt reaches the event queue, and flips on that queue deliver nothing while its own release interrupts keep arriving. The latch is cleared when the queue has flipped five times without another interrupt from its own releases: the fifth flip delivers the event and unlatched the queue, so a title that waits on the flip event after latching recovers only if it submits further flips. The bound cannot be by time: the next release interrupt can arrive seconds later than a suppressed flip across loading stalls, and delivering an unexpected flip event on a queue that pairs flips with releases can crash; no fixed window short enough to unblock a waiting title keeps such a title alive. Whether the console raises the event from the flip itself or from an interrupt its flip sequence contains is not measured; nor is how the console behaves for a title that waits on the flip event and also raises release interrupts on the same queue.
 - [Host import windows and budget](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) (libSceAgcDriver): a registered range larger than 64 MiB is imported one 64 MiB window at a time for buffer and fill accesses; an access across window edges imports the cells it touches, grown to the union of the range's imports it overlaps. An address-based build (`AcquireRegistered`) imports the range whole and replaces its windows. Live imports stay under a budget: `APS5_HOST_IMPORT_MIB`, else the larger of 6 GiB and half the physical memory, at most the largest host heap less 2 GiB (imports and type-3 mirrors share it; #167's rule). A new import beyond it retires the least recently used imports not used in the last 256 lookups; one that would not fit even then is refused before anything is retired, and imports made when a range is mapped never evict. A refused import is tried again at its next use. The 64 MiB window, the 256-lookup protection and the 2 GiB reserve are choices, not measured.
 - [Registration preparation at submit](../../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp) (libSceAgcDriver) - `Driver::Submit` waits for every registration-time shader preparation queued before the call, not only those of the shaders the commands use, so a submission behind a long queue of unrelated preparations waits for all of them on the submitting thread. Waiting there instead of in the queue worker keeps the GPU side of a submission short: a title that sees a submission take seconds treats the GPU as hung (Goat Simulator 3, PPSA04159, then stops submitting and its garbage collector fails a 10 s timeout). On the console shaders are compiled ahead of time, so neither wait exists; how long a title tolerates a slow submit call is not known.
 - [Resource cache entries holding released textures](../../../core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp) (libSceAgcDriver) - a sampled texture the texture cache drops while something else holds it, or makes without caching (`admitTexture` refusing a first use), is marked released, and `ResourceCache::Insert` refuses descriptor sets that hold one and removes the entries that do. The removal waits for the next insert: until then `Find` still returns those entries and their textures stay alive. One process-wide flag starts the sweep, so while the sampled cache is over its budget every insert scans all entries (1,024 at start, up to 16,384 after growth, or `APS5_RESOURCE_CACHE_ENTRIES`, each with its textures); that cost is not measured. When `ReleaseTextureCaches` runs on device memory exhaustion, the sampled textures these entries hold are only marked released and are freed at the next insert, so the immediate allocation retry gets no memory back from them. A draw or dispatch whose textures the cache refused builds its descriptor sets again on each use for as long as the cache stays full; the count bounds (`APS5_RESOURCE_CACHE_ENTRIES`, `APS5_RESOURCE_CACHE_TABLE_ELEMENTS`) still bound nothing by bytes, and sampled textures made outside the texture cache (the disabled cache, depth-surface copies) are never marked. Measured on one title on one RTX 4090: with the default bounds, sampled textures held outside the cache peaked at 2,836 MiB (7,830 MiB before) and device-local heap 0 stayed under its 22.9 GB budget for the whole run (26,357 MiB before, which aborted with `VK_ERROR_OUT_OF_DEVICE_MEMORY` at the wolf scene). Whether the extra descriptor-set builds cost frame time elsewhere is not measured.