diff --git a/decompiler/IR2/AtomicOp.cpp b/decompiler/IR2/AtomicOp.cpp index 9addab4dc7..4c49f6e9e9 100644 --- a/decompiler/IR2/AtomicOp.cpp +++ b/decompiler/IR2/AtomicOp.cpp @@ -539,6 +539,8 @@ goos::Object AsmOp::to_form(const std::vector& labels, const En } } + // note: to correctly represent a MOVN/MOVZ in our IR, we need to both read and write the + // destination register, so we append a read to the end here. if (m_instr.kind == InstructionKind::MOVZ || m_instr.kind == InstructionKind::MOVN) { RegisterAccess ra(AccessMode::READ, m_dst->reg(), m_dst->idx()); forms.push_back(ra.to_form(env)); @@ -610,6 +612,7 @@ void AsmOp::update_register_info() { } if (m_instr.kind == InstructionKind::MOVN || m_instr.kind == InstructionKind::MOVZ) { + // in the case that MOVN/MOVZ don't do the move, they effectively read the original value. m_read_regs.push_back(m_dst->reg()); } @@ -722,6 +725,8 @@ void AsmOp::collect_vars(RegAccessSet& vars) const { } if (m_instr.kind == InstructionKind::MOVN || m_instr.kind == InstructionKind::MOVZ) { + // the conditional moves read their write register, but don't have it listed as a write + // in the actual ASM. We handle this difference for the variable naming system here. RegisterAccess ra(AccessMode::READ, m_dst->reg(), m_dst->idx()); vars.insert(ra); } diff --git a/docs/markdown/graphics.md b/docs/markdown/graphics.md index 0383b9dc02..5b9a76537d 100644 --- a/docs/markdown/graphics.md +++ b/docs/markdown/graphics.md @@ -9,9 +9,9 @@ There are three frames in flight at a time: ## Synchronization -The PC Port only synchronizes on vsync. This waits for the "draw" frame to finish rendering, and for the buffers to be swapped. +The PC Port synchronizes on `syncv` and on `sync-path`. The `syncv` waits for an actual buffer swap and `sync-path` waits for the renderer to finish. -The game's code for this is kind of messy and confusing. On the PS2, you make sure rendering is done with `sync-path`, which waits for the DMA chain to finish. But they call this earlier than I think they need to, and I don't really understand why. I don't see any place where they read back from the finished frame or depth buffer. Or where they would overwrite the memory. There's a second call to `sync-path` right where you would expected, right before the `syncv`. After `syncv`, they call some Sony library function to actually display the correct framebuffer, then immediately start sending the next DMA chain. +The game's code for this is kind of messy and confusing, and calls `sync-path` twice. On the PS2, you make sure rendering is done with `sync-path`, which waits for the DMA chain to finish. But they call this earlier than I think they need to, and I don't really understand why. I don't see any place where they read back from the finished frame or depth buffer. Or where they would overwrite the memory. There's a second call to `sync-path` right where you would expect, imeediately before the `syncv`. After `syncv`, they call some Sony library function to actually display the correct framebuffer, then immediately start sending the next DMA chain. The stuff between `sync-path` and `syncv` is: - depth cue "calc" (seems fast) @@ -30,13 +30,11 @@ The stuff between `sync-path` and `syncv` is: - cache flush - a second `sync-path` -I'm really not sure why they have the first `sync-path` there. One theory is that they didn't want debug code and non-debug rendering running at the same time - the debug code will compete with the rendering to use the main bus, and will make the rendering slower. - -For now, the PC Port doesn't do anything on `sync-path`. I think there's two ways this could be an issue in the future: -- If they reuse the DMA buffer after `sync-path`, and our renderer is still reading from it. Currently not an issue because of dma copy, described later. -- If they need to read the completed depth buffer or frame buffer after `sync-path`. I don't see any example of this. +According to the Performance Analyzer, this takes about 1% to 2% of a frame. They subtract off 4% of a frame from the profile bar so that 100% there is really around 96% of a frame, I guess to account for this extra time. +I'm really not sure why they have the first `sync-path` there. It makes some sense in debug mode so that you can draw the profile bar for the GPU after it has finished. Another theory is that they didn't want debug code and non-debug rendering running at the same time - the debug code will compete with the rendering to use the main bus, and will make the rendering slower. But it seems like you don't want this in the release version. +For now, the PC Port does sync on `sync-path`, but it probably doesn't need to. ## DMA Copy diff --git a/game/graphics/dma/dma.h b/game/graphics/dma/dma.h index 813486dd38..a269bbf630 100644 --- a/game/graphics/dma/dma.h +++ b/game/graphics/dma/dma.h @@ -2,7 +2,7 @@ /*! * @file dma.h - * Unpacking utils for PS2 DMA, VIF, and GIF data. + * PS2 DMA and VIF types. */ #include diff --git a/game/graphics/dma/dma_chain_read.h b/game/graphics/dma/dma_chain_read.h index 8d163c9941..00114a7ab7 100644 --- a/game/graphics/dma/dma_chain_read.h +++ b/game/graphics/dma/dma_chain_read.h @@ -17,6 +17,9 @@ * } */ +/*! + * Represents a DMA transfer, including 64-bits of VIF tag. + */ struct DmaTransfer { const u8* data = nullptr; u32 data_offset = 0; diff --git a/game/graphics/dma/dma_copy.cpp b/game/graphics/dma/dma_copy.cpp index db34a88f07..4fd570caa7 100644 --- a/game/graphics/dma/dma_copy.cpp +++ b/game/graphics/dma/dma_copy.cpp @@ -3,18 +3,26 @@ #include "dma_copy.h" #include "third-party/fmt/core.h" +/*! + * Convert a DMA chain to an array of bytes that can be directly fed to VIF. + */ std::vector flatten_dma(const DmaFollower& in) { DmaFollower state = in; std::vector result; while (!state.ended()) { auto read_result = state.read_and_advance(); - result.push_back(0); // tag transfer padding - result.push_back(read_result.transferred_tag); - result.insert(result.end(), read_result.data, read_result.data + read_result.size_bytes); - // for (u32 i = 0; i < read_result.size_bytes; i += 8) { - // result.push_back(state.read_val(read_result.data_offset + i)); - // } + // insert 1 quadword of zeros. + // the first 8 bytes will remain zero and will be interpreted as VIF nop. + for (int i = 0; i < 16; i++) { + result.push_back(0); + } + + // the second will contain the transferred tag. + memcpy(result.data() + result.size() - 8, &read_result.transferred_tag, 8); + + // and the actual DMA data. + result.insert(result.end(), read_result.data, read_result.data + read_result.size_bytes); } return result; } diff --git a/game/graphics/dma/dma_copy.h b/game/graphics/dma/dma_copy.h index 4a7c2b9aa6..fff89a06b6 100644 --- a/game/graphics/dma/dma_copy.h +++ b/game/graphics/dma/dma_copy.h @@ -5,11 +5,6 @@ #include "common/common_types.h" #include "game/graphics/dma/dma_chain_read.h" -/*! - * This function is used to copy a DMA chain. To avoid needing a huge amount of memory, the DMA data - * is rearranged in memory. - */ - struct DmaData { u32 start_offset = 0; std::vector data; @@ -43,4 +38,7 @@ class FixedChunkDmaCopier { DmaData m_result; }; +/*! + * Convert a DMA chain to an array of bytes that can be directly fed to VIF. + */ std::vector flatten_dma(const DmaFollower& in); \ No newline at end of file diff --git a/game/graphics/dma/gs.h b/game/graphics/dma/gs.h index f234bb1576..9f3cd650f3 100644 --- a/game/graphics/dma/gs.h +++ b/game/graphics/dma/gs.h @@ -2,6 +2,11 @@ #include "game/graphics/dma/dma.h" +/*! + * @file gs.h + * PS2 GS/GIF hardware types + */ + struct GifTag { enum class Format : u8 { PACKED = 0, REGLIST = 1, IMAGE = 2, DISABLE = 3 }; diff --git a/game/graphics/types.h b/game/graphics/types.h deleted file mode 100644 index 8db03db38c..0000000000 --- a/game/graphics/types.h +++ /dev/null @@ -1,6 +0,0 @@ - - -#ifndef JAK_TYPES_H -#define JAK_TYPES_H - -#endif // JAK_TYPES_H diff --git a/goal_src/engine/game/main.gc b/goal_src/engine/game/main.gc index 620ae43c38..d5b00486df 100644 --- a/goal_src/engine/game/main.gc +++ b/goal_src/engine/game/main.gc @@ -944,10 +944,6 @@ ) -(defmacro dloop-dbg (str &rest args) - `(format 0 ,(string-append "[display-loop] " str) ,@args) - ) - (defun display-loop () "This is in progress..." @@ -961,9 +957,7 @@ (set! *teleport* #t) (update-per-frame-settings! *setting-control*) ;;(init-time-of-day-context *time-of-day-context*) TODO - (format 0 "DISPLAY LOOP calling sync~%") (display-sync disp) - (format 0 "first call to sync done~%") (swap-display disp) ;; touching list ;; bler init