Repository navigation
Upstream sync 8: AGC driver shared fixes onto Metal + bounded index reads - #468
Merged
Merged
Conversation
Report the 8-byte GS primitive payload when the shader's context registers set SPI_SHADER_IDX_FORMAT.IDX0_EXPORT_FORMAT to SPI_SHADER_2COMP, and 0 otherwise. The search runs over every context register, so an earlier SPI_SHADER_IDX_FORMAT entry with another format does not end it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit cb68666)
(cherry picked from commit cbd615b)
The hardware ignores the LOD for unnormalized coordinates and samples the base level. A sampler used only by explicit-LOD, implicit-LOD, bias and gradient samples gets a pipeline specialization constant set from its unnormalized bit when the binding plan is prepared. When the constant is set, each of its samples is emitted as OpImageSampleExplicitLod with LOD 0, which is what Vulkan requires of unnormalized samplers. Otherwise the sample keeps its own form. Such shaders are no longer refused. (cherry picked from commit 63ed0ff)
DecodeState mapped points, lists, strips, fans, rect lists and patches but threw "unsupported primitive type 3" for VGT_PRIMITIVE_TYPE 3, the line strip (DI_PT_LINESTRIP in Mesa's r600d.h). A title stops on it after a few minutes. Vulkan allows primitive restart for line strips without the list restart extension, and the precheck, the pipeline and the provoking vertex check already treat the strip class that way; only the decode lacked the case. The point and line depth bias enable stays rejected on line strips like on line lists, and a line strip gets the line primitive class for a pixel shader that reads the ancillary word. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> (cherry picked from commit 67e7a30)
… Metal) Ports the shared part of upstream 04138f5. PA_CL_VS_OUT_CNTL's clip and cull enables and its misc and distance vector bits are accepted by DecodeState and the precheck (up to eight distances), kept in State::paClVsOutCntl, and reach the recompiler's vertex info (request version 20), its cache key and the request serializer. On Metal, the draw sets the vertex info's PA_CL_VS_OUT_CNTL position export bits for programs that export pos1..pos3 (upstream's PositionExportLayout, without the snapshot cache; only draws that enable those bits scan the program), so guest clip distances become MSL [[clip_distance]] outputs. MSL has no cull distances, so a vertex program that would write them is refused on Metal. The Vulkan driver half (ShaderRegistry, ShaderValidation, VulkanDevice feature query) is not ported: the fork's Vulkan validation predates the upstream shaderClipDistance plumbing it extends. Tests: anyps5_metal_render_state decodes the enables and refuses nine distances; anyps5_metal_guest_draw_replay draws a guest program whose pos1 exports four distances and checks that Metal clips nothing, everything, and the left half. (cherry picked from commit 04138f5, shared part) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Admit PA_SC_AA_CONFIG bit29 only without active perspective or linear centroid inputs, or with a skipped pixel program. Active/unspecified centroid inputs and unrelated AA fields remain refused. Source: pinned PS5_Mesa 0b2d6d1a GFX10.3 schema/RADV producer; existing ENA/ADDR intersection. Console covered-centroid interpolation remains unmeasured. Full build/all 124 runtime libraries, conventions and registration pass. Final Windows suite: 965 pass, zero fail, seven skips of 972. Isolated state test passes 20 repeats and fails 5/5 with main State.cpp; four guard mutants fail 5/5 each. Original two title register captures reproduce the main refusal and pass this check with candidate, five times each. NVIDIA relevant groups pass; AMD draw-cache failure also occurs on unchanged main and is reported in issue4435. AI-assisted: Codex. Ported onto the fork's PA_SC_AA_CONFIG mask (AaConfigMask, which also admits AA_MASK_CENTROID_DTMN). The upstream Graphics.cpp test depends on upstream's multisample decode, so the check is in anyps5_metal_render_state instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 44c4b3f)
(cherry picked from commit 13818ef)
DB_SHADER_CONTROL.DEPTH_BEFORE_SHADER (bit 12) is what a pixel shader compiled for early depth/stencil sets: the depth and stencil tests and writes happen before the shader runs, even when it kills pixels or writes memory. The bit was accepted but never read, so such shaders got EarlyFragmentTests only when Z_ORDER was EARLY_Z_THEN_LATE_Z and they had no kill or exports. Their killed pixels then left depth unwritten, and occluded pixels still wrote memory. EARLY_Z_THEN_RE_Z (Z_ORDER 3) was not treated as early either. DecodePixelStageInfo now sets earlyZ for DEPTH_BEFORE_SHADER, and for Z_ORDER 1 or 3 under the existing kill and export conditions, including the stencil reference export. The backend no longer repeats those conditions; it keeps only the alpha-to-coverage one, so a shader with alpha-to-coverage still runs its tests late. earlyZ keeps its place in the request, so the serialization version does not change. earlyFragmentTestsTests in agc_driver_graphics checks which DB_SHADER_CONTROL values declare EarlyFragmentTests, and that alpha-to-coverage keeps Z_ORDER 1 and DEPTH_BEFORE_SHADER late, directly and after a serialization round trip. stencilRefExportTests checks that DEPTH_BEFORE_SHADER with a stencil reference export still decodes as early. TechnicalDebt records that the bit follows Mesa and that its combination with a Z export, a mask export, a stencil export or alpha-to-coverage was not measured. Metal: a pixel shader that exports depth keeps its tests late even with DEPTH_BEFORE_SHADER, because MSL refuses [[depth]] in an [[early_fragment_tests]] function (the fork has no stencil export). The upstream Graphics.cpp test leans on the stencil export tests, so the decode checks are in anyps5_metal_render_state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 320bd37)
A flip packet now delivers the queue's end-of-pipe event once the work recorded before it completes, as a RELEASE_MEM with INT_SEL does. A title that registers the graphics event, never requests an interrupt in its releases and waits on the event after every flip presented one frame and then waited forever. Vulkan: upstream's PacketExecution.cpp change, applied as is. Metal: the flip already completes all prior GPU work (CompletePriorGpuWorkAndCopyBack), so MetalPacketExecution delivers the event right after handing the frame to its output. A driver configured without an interrupt callback (presentation-only replays) raises nothing. The upstream test drives the Vulkan driver; anyps5_metal_pm4_compute_replay gets FlipRaisesEop, which submits a lone flip and checks one end-of-pipe event after GpuReady. (cherry picked from commit 326e0bc, PacketExecution.cpp; Metal port) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…emaphoreLabel sceAgcSetAmmSemaphoreMemory(memory, sizeInBytes) registers the AMM semaphore memory once and zeroes the first 8 bytes of each 32-byte label; sceAgcGetSemaphoreLabel(index, label) writes memory + 32 * index for an index inside the registered memory. Unblocks a title that imports both symbols. Matches OpenAGC and sharpemu agreements on signatures, 16 KiB alignment, 32-byte stride, and zeroed label counters. (cherry picked from commit 352d04e)
A fast clear of a packed float color target (format 36) was refused as unencodable. Give the format a DCC layout (11, 11 and 10 bit float channels) and its one value, so 0000, 0001, 1110 and 1111 clear codes fill the texels and a texture keeps those keys. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> The fork keeps the DCC layout table once, in the shared Graphics/src/DccClearEncoding.cpp that both the Vulkan and the Metal builds compile, so the change lands there instead of DccMetadata.cpp. anyps5_metal_render_state checks the encoding on the Metal build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit a96f8bd)
…check verdict The render-state fixture is a draw without a pixel program, which the precheck already refuses for that reason; the clip-distance and covered-centroid checks now compare with that verdict instead of requiring none. The early-depth decode fixture gets the SPI registers DecodePixelStageInfo reads, the covered-centroid check reads the fork's rasterSamples, and the clip-distance draw is non-indexed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Direct DRAW_INDEX_2 and DRAW_INDEX_OFFSET_2 packets may ask for more indices than their max_size readable prefix. The draw keeps its count, reads only min(count, max_size) guest indices and fetches index 0 for the rest; for OFFSET_2, max_size is already relative to the packet offset. This replaces the "index count exceeds maximum index size" rejection in Pm4::Validate, which the Metal packet loop runs outside the skipped-draw guard, and stops reads past the guest range. Shared (from the open upstream PR, read-only): Pm4::DrawParameters gets readableIndexCount / IndexReadCount(), ResolveDraw sets it, and the Vulkan draw pads its index copy (CopyBoundedIndexInput), with its alias checks limited to the real prefix. The Pm4 tests come with it. Metal: MetalDraw copies the readable prefix into a buffer of the draw's own and zero-fills the tail; a mesh path refuses bounded fetches, as upstream does. anyps5_metal_guest_draw_replay draws 6 indices with 3 and 0 readable from a borrowed range holding exactly the readable ones (a read past them fails with "not borrowed"). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ea7dfea98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+737
to
+739
| zero(cx, 0x207, ~(LayerExports | ClipCullExports), "layer, viewport or auxiliary vertex exports"); | ||
| Require(std::popcount(read(cx, 0x207) & 0xffffu) <= 8, "more than eight clip and cull distances are unsupported"); | ||
| result.paClVsOutCntl = read(cx, 0x207); |
There was a problem hiding this comment.
When a Vulkan draw enables clip or cull distances, this now admits the register state, but the regular driver path in Execution/src/Driver/Draw/Draw.cpp only calls DecodeVertexStageInfo and never copies graphics.paClVsOutCntl into the resulting ShaderVertexStageInfo; only MetalPacketExecution.mm performs that assignment. Consequently ShaderInfoCollector sees the default zero layout and does not emit the enabled gl_ClipDistance/gl_CullDistance outputs, so affected Vulkan draws render without the requested clipping or culling instead of being rejected or handled correctly.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream sync 8, slice C "AGC driver": the agc / libSceAgcDriver / libSceAgc commits in
758aca737..upstream/main(upstream at18c438b10). Sync 7 (#456) already triaged everything up to6d7b87370. This PR covers the 48 scoped commits after that, plus the generic part of the open upstream PR 4824 (bounded direct-index reads). It follows #442/#446/#456: shared driver logic is ported, and Metal gets an equivalent where one exists. Vulkan device and pipeline internals are skipped.11 commits are ported and 37 skipped, which is under the 60-commit split threshold, so this is one PR.
Ported
cb6866653sceAgcGetGsPrimPayloadcbd615bb4error for unsupported init versions352d04e8dsceAgcSetAmmSemaphoreMemory / sceAgcGetSemaphoreLabel67e7a3029primitive type 3 decodes as a line strip. Metal already maps line strips and their restart.63ed0ffeaunnormalized samplers sample the base level. This is recompiler-side. Specialization is baked in by Recompile, so the Metal bridge gets explicit-LOD-0 samples, which MSLcoord::pixelneeds.04138f50avertex clip and cull distances, shared part plus Metal.State::paClVsOutCntl, and pass them to the recompiler. That is request version 20, plus the cache key and the serializer.PositionExportLayoutwithout the snapshot cache; only draws that enable those bits scan the program. Guest clip distances become MSL[[clip_distance]].44c4b3f8bCOVERED_CENTROID_IS_CENTER is admitted when no centroid input is active. Ported onto the fork'sAaConfigMask.13818efc3a collapsed slice view keeps the array slice's pipe/bank XOR. This is shared decode, and the Metal target address follows it.320bd37c9DEPTH_BEFORE_SHADER and Z_ORDER 3 test depth early.xcrun metalrefuses[[depth]](and[[stencil]]) in an[[early_fragment_tests]]function. The kill, sample-mask and memory-write cases compile.326e0bc29a flip raises the graphics end-of-pipe event.GpuReady. A driver configured without an interrupt callback (presentation-only replays) raises nothing.a96f8bd64packed R11G11B10 float DCC fast clears. The fork keeps the DCC layout table once, in the sharedDccClearEncoding.cppthat both builds compile, so the change lands there.min(count, max_size)indices and fetch index 0 for the rest. This replaces thePm4::Validaterejection, which on Metal runs outside the skipped-draw guard.DrawParameters::readableIndexCount/IndexReadCount(), ResolveDraw, the Vulkan padded copy, and the Pm4 tests.MetalDrawcopies the readable prefix into a buffer of its own and zero-fills the tail. A mesh path refuses bounded fetches, as upstream does.Skipped
23342340c 25a83722c 247162dc4.105561775 97d192eea dd7028688 425a4daf1 60a1649e7 93a090ed8a103e48c8 13c4a2a6c 78ad5d9df 2ae227ef8 dedb869fa 0d61dad29a72dcb93a 4a4bea9d1. Metal equivalents were added instead.51c043ba5: Vulkan CMASK materialization; Metal has none ([upstream] Port shared AGC driver fixes since sync #4 onto Metal #418 leftover).6bf9ab86f: GuestBufferMemory.68c2b0f74: ShaderRegistry.396b5189a: VulkanDevice CopyBuffer.44623222c: Vulkan vertex/target alias check; MetalDraw has no such check.e5449936f: Vulkan Texture.e4a7cbfc8 cc3b47dda: storage views of depth planes. Metal refuses every storage view of a depth plane, and sampled one-layer 2D-array views already go throughresidentDepthArray.501453573 99ebc650c: Vulkan buffer-fill HLE; Metal runs the kernel as a dispatch.29b227bf3 3949fb9d4: Vulkan recorder ordering.37d9ef133: Vulkan swapchain acquire. On Metal,nextDrawablealready times out and the presentation is dropped.95f34a050: Vulkan presentation GPU timing.a910dee09: Vulkan DCC key wait.f20a32bec 14f2092c9 89414d303 dd49e1df9. The last two are Windows-only. The fork's test targets compile their sources directly.c4a61eae7.Upstream messages that named titles were reworded to "a title".
Tests
anyps5_metal_guest_draw_replay:DrawClipDistances: four guest distances in pos1 clip nothing, everything, and the left half.DrawBoundedIndices: 3 and 0 readable of 6 indices, from a borrowed range that holds only the readable ones.anyps5_metal_render_state: clip/cull decode, covered-centroid, early-depth decode, and the R11G11B10 DCC encoding.anyps5_metal_pm4_compute_replay:FlipRaisesEop.-DANYPS5_METAL_UTILITIES_ONLY=ON -DANYPS5_METAL_SHADER_BRIDGE=ON -DANYPS5_BUILD_METAL_SHADER_BRIDGE_TESTS=ON -DANYPS5_METAL_GUEST_REPLAY=ON; includes the shader-bridge tests):ctest37/37 passed.ctest -R 'agc_gs_prim_payload|agc_semaphore|agc_command|direct_memory_darwin|metal_utility_tests|rosetta_direct_graphics|rosetta_lib_binds|rosetta_runtime|rosetta_launcher|rosetta_cpuid|rosetta_shutdown_stop|audio_out2|ajm_opus_surround'16/16 passed.Graphics/src/Draw.cpp,ShaderResources.cpp,tests/Pm4.cpp,tests/TextureFormat.cpp):-fsyntax-onlyclean on macOS. Linux CI builds the rest.Title run
e1b3f522c.[metal] skippedlines (1 draw and 12 dispatches) and noAGC graphics:refusals.Flips kept pace with main up to 288 s:
At 288 s the Mac went into clamshell sleep on battery for about 220 s (pmset log, 14:18:02–14:21:38).
After waking, the title reached the 3D map (turn 1, 4000 BCE).
No skipped draws or dispatches, no
AGC graphics:refusals, no faults.🤖 Generated with Claude Code