Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/mips/psyqo/gpu.hh
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,19 @@ class GPU {
enum class ColorMode { C15BITS, C24BITS };
enum class Interlace { PROGRESSIVE, INTERLACED };
enum class MiscSetting { CLEAR_VRAM, KEEP_VRAM };
// Where the two display buffers sit in VRAM. Selected at compile time; Default is the
// classic vertically-stacked pair (0,0)/(0,256). VerticalSwitch offsets the first buffer
// down by 16 lines; Horizontal places the buffers side by side.
enum class Layout { Default, VerticalSwitch, Horizontal };
#if defined(PSYQO_USE_VERTICAL_SWITCH_LAYOUT)
static constexpr Layout c_layout = Layout::VerticalSwitch;
#elif defined(PSYQO_USE_HORIZONTAL_LAYOUT)
static constexpr Layout c_layout = Layout::Horizontal;
#else
static constexpr Layout c_layout = Layout::Default;
#endif
void initialize(const Configuration &config);
void reinitialize(const Configuration &config);

static constexpr uint32_t US_PER_HBLANK = 64;
static constexpr unsigned c_chainThreshold = 56;
Expand Down Expand Up @@ -537,6 +549,7 @@ class GPU {
void scheduleChainedDMA(uintptr_t head);
void chain(uint32_t *first, uint32_t *last, size_t count);
void scheduleOTC(uint32_t *start, uint32_t count);
void setDisplayArea(bool firstBuffer);
void checkOTCAndTriggerCallback();
void prepareForTakeover();

Expand Down
159 changes: 120 additions & 39 deletions src/mips/psyqo/src/gpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,63 +51,90 @@
}
}

void psyqo::GPU::initialize(const psyqo::GPU::Configuration &config) {
namespace {

// GP1(06h) horizontal display range, in video-clock units relative to HSYNC.
// X1 is the canonical first visible pixel on a normal TV set; X2 spans the
// displayed pixels, one video clock per pixel times the dotclock divider.
constexpr uint32_t hDisplayRange(unsigned pixels, unsigned cyclesPerPixel) {
constexpr uint32_t X1 = 0x260;
uint32_t X2 = X1 + pixels * cyclesPerPixel;
return 0x06000000 | X1 | (X2 << 12);
}

// GP1(07h) vertical display range, in scanlines relative to VSYNC, centered on
// the middle scanline (NTSC 0x88, PAL 0xa3) plus/minus half the displayed lines.
constexpr uint32_t vDisplayRange(unsigned midScanline, unsigned lines) {
return 0x07000000 | (midScanline - lines / 2) | ((midScanline + lines / 2) << 10);
}

} // namespace

void psyqo::GPU::reinitialize(const psyqo::GPU::Configuration &config) {
// Reset
Hardware::GPU::Ctrl = 0;
// FIFO polling mode
Hardware::GPU::Ctrl = 0x04000001;
// Display Mode
Hardware::GPU::Ctrl = 0x08000000 | (config.config.hResolution << 0) | (config.config.vResolution << 2) |
(config.config.videoMode << 3) | (config.config.colorDepth << 4) |
(config.config.videoInterlace << 5) | (config.config.hResolutionExtended << 6);
// Horizontal Range
Hardware::GPU::Ctrl = 0x06000000 | 0x260 | (0xc60 << 12);

// Vertical Range
if (config.config.videoMode == Configuration::VM_NTSC) {
Hardware::GPU::Ctrl = 0x07000000 | 16 | (255 << 10);
} else {
Hardware::GPU::Ctrl = 0x07046c2b;
}

// Display Area
Hardware::GPU::Ctrl = 0x05000000;

COUNTERS[1].mode = 0x100;
COUNTERS[1].value = 0;

if (config.config.videoInterlace == Configuration::VI_ON) {
m_interlaced = true;
m_height = 480;
} else {
m_interlaced = false;
m_height = 240;
}

// Resolution: the number of displayed pixels and the video-clock cycles
// per pixel (the GPU dotclock divider, per psx-spx).
unsigned cyclesPerPixel = 8;
if (config.config.hResolutionExtended == Configuration::HRE_NORMAL) {
switch (config.config.hResolution) {
case Configuration::HR_256:
m_width = 256;
cyclesPerPixel = 10;
break;
case Configuration::HR_320:
m_width = 320;
cyclesPerPixel = 8;
break;
case Configuration::HR_512:
m_width = 512;
cyclesPerPixel = 5;
break;
case Configuration::HR_640:
m_width = 640;
cyclesPerPixel = 4;
break;
}
} else {
m_width = 368;
cyclesPerPixel = 7;
}

// Horizontal Range
Hardware::GPU::Ctrl = hDisplayRange(m_width, cyclesPerPixel);

// Vertical Range
Hardware::GPU::Ctrl = vDisplayRange(config.config.videoMode == Configuration::VM_NTSC ? 0x88 : 0xa3, 240);

// Display Area
Hardware::GPU::Ctrl = 0x05000000;

if (config.config.videoInterlace == Configuration::VI_ON) {
m_interlaced = true;
m_height = 480;
} else {
m_interlaced = false;
m_height = 240;
}

if (config.config.videoMode == Configuration::VM_NTSC) {
m_refreshRate = 60;
} else {
m_refreshRate = 50;
}
}

Check warning on line 131 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Complex Method

psyqo::GPU::reinitialize has a cyclomatic complexity of 9, threshold = 9 This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

void psyqo::GPU::initialize(const psyqo::GPU::Configuration &config) {
COUNTERS[1].mode = 0x100;
COUNTERS[1].value = 0;

reinitialize(config);

Check notice on line 137 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Complex Method

psyqo::GPU::initialize decreases in cyclomatic complexity from 19 to 11, threshold = 9 This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check notice on line 137 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Bumpy Road Ahead

psyqo::GPU::initialize decreases from 3 to 2 logical blocks with deeply nested code, threshold is 2 blocks per function The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

// Install VBlank interrupt handler
if (Kernel::isKernelTakenOver()) {
Expand Down Expand Up @@ -213,6 +240,19 @@
}
}

void psyqo::GPU::setDisplayArea(bool firstBuffer) {
uint32_t x = 0;
uint32_t y = 0;
if constexpr (c_layout == Layout::Horizontal) {
x = firstBuffer ? 0 : m_width;
} else if constexpr (c_layout == Layout::VerticalSwitch) {
y = firstBuffer ? 256 : 16;
} else {
y = firstBuffer ? 256 : 0;
}
Hardware::GPU::Ctrl = 0x05000000 | (x << 0) | (y << 10);
}

void psyqo::GPU::flip() {
do {
pumpCallbacks();
Expand All @@ -223,12 +263,7 @@
parity ^= 1;
if (!m_interlaced) {
bool firstBuffer = !parity;
// Set Display Area
if (firstBuffer) {
Hardware::GPU::Ctrl = 0x05000000 | (256 << 10);
} else {
Hardware::GPU::Ctrl = 0x05000000;
}
setDisplayArea(firstBuffer);

Check notice on line 266 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Complex Method

psyqo::GPU::flip decreases in cyclomatic complexity from 14 to 13, threshold = 9 This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check notice on line 266 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ No longer an issue: Bumpy Road Ahead

psyqo::GPU::flip is no longer above the threshold for logical blocks with deeply nested code The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
} else if (!pcsx_present()) {
while (1) {
uint32_t stat = Hardware::GPU::Ctrl;
Expand Down Expand Up @@ -276,9 +311,22 @@
int16_t height = m_height;
bool firstBuffer = !parity || m_interlaced;

scissor.start = Prim::DrawingAreaStart(Vertex{{.x = 0, .y = firstBuffer ? int16_t(0) : int16_t(256)}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = width, .y = firstBuffer ? height : int16_t(256 + height)}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = int16_t(0), .y = firstBuffer ? int16_t(0) : int16_t(256)}});
if constexpr (c_layout == Layout::Horizontal) {
int16_t x = firstBuffer ? int16_t(0) : width;
scissor.start = Prim::DrawingAreaStart(Vertex{{.x = x, .y = 0}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = int16_t(x + width), .y = height}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = x, .y = 0}});
} else {
int16_t y;
if constexpr (c_layout == Layout::VerticalSwitch) {
y = firstBuffer ? int16_t(16) : int16_t(256);
} else {
y = firstBuffer ? int16_t(0) : int16_t(256);
}
scissor.start = Prim::DrawingAreaStart(Vertex{{.x = 0, .y = y}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = width, .y = int16_t(y + height)}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = int16_t(0), .y = y}});
}

Check warning on line 329 in src/mips/psyqo/src/gpu.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Code Duplication

The module contains 7 functions with similar structure: psyqo::GPU::getClear,psyqo::GPU::getNextClear,psyqo::GPU::getNextScissor,psyqo::GPU::getScissor and 3 more functions Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
Comment on lines +314 to +329

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and inspect the affected functions with line numbers.
git ls-files src/mips/psyqo/src/gpu.cpp
echo '--- outline ---'
ast-grep outline src/mips/psyqo/src/gpu.cpp --view expanded || true
echo '--- around getDisplayArea / getScissor / getNextScissor / getClear / getNextClear ---'
sed -n '220,420p' src/mips/psyqo/src/gpu.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 10956


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find call sites to understand whether getScissor / getNextScissor represent current vs next frame buffers.
rg -n "getScissor\\(|getNextScissor\\(|getClear\\(|getNextClear\\(|setDisplayArea\\(" src/mips/psyqo/src -g'*.cpp' -g'*.h'

Repository: grumpycoders/pcsx-redux

Length of output: 850


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect layout definitions and any docs/comments describing buffer ordering.
rg -n "Layout::Horizontal|VerticalSwitch|firstBuffer|double buffer|buffer" src/mips/psyqo/src include src/mips/psyqo -g'*.cpp' -g'*.h' -g'*.md'

Repository: grumpycoders/pcsx-redux

Length of output: 21622


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the layout-related declarations around GPU::Configuration and any comments on frame selection.
rg -n "enum class Layout|struct Configuration|firstBuffer|setDisplayArea|getNextScissor|getNextClear" src/mips/psyqo/src/gpu.cpp src/mips/psyqo/src/*.h src/mips/psyqo/include src/mips/psyqo -g'*.h' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 4698


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "enum class Layout|struct Configuration|firstBuffer|setDisplayArea|getScissor|getNextScissor|getClear|getNextClear" src/mips/psyqo/src/gpu.cpp src/mips/psyqo/src/*.h src/mips/psyqo/include src/mips/psyqo -g'*.h' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 5264


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect docs around GPU parity / double buffering to see how "firstBuffer" is defined.
sed -n '170,220p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 7412


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the cube/torus examples for the intended use of getNextClear and frame parity.
sed -n '110,170p' src/mips/psyqo/examples/cube/cube.cpp | cat -n
echo '---'
sed -n '760,800p' src/mips/psyqo/examples/torus/torus.cpp | cat -n
echo '---'
sed -n '80,110p' src/mips/psyqo/examples/hello-chained/hello-chained.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6935


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for explanations of "parity" and "buffer" in the GPU docs/source.
rg -n "parity|firstBuffer|double buffering|display area|draw buffer|current buffer|next frame" src/mips/psyqo/CONCEPTS.md src/mips/psyqo/src/gpu.cpp src/mips/psyqo/examples -g'*.md' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 5495


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Focused read of the GPU code around the parity/flip path and layout helpers.
sed -n '256,395p' src/mips/psyqo/src/gpu.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6033


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '175,210p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6425


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether these helpers are used anywhere else or only internally.
rg -n "\bgetScissor\b|\bgetNextScissor\b|\bgetClear\b|\bgetNextClear\b|\benableScissor\b|\bdisableScissor\b" src/mips/psyqo -g'*.cpp' -g'*.h'

Repository: grumpycoders/pcsx-redux

Length of output: 1119


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '768,790p' src/mips/psyqo/examples/torus/torus.cpp | cat -n
echo '---'
sed -n '128,146p' src/mips/psyqo/examples/cube/cube.cpp | cat -n
echo '---'
sed -n '92,104p' src/mips/psyqo/examples/hello-chained/hello-chained.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 2739


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '180,205p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 5396


Invert the Horizontal buffer selection
Layout::Horizontal should use the opposite x offset here and in getClear/getNextScissor/getNextClear; as written, it targets the same VRAM half as setDisplayArea, so drawing and clears land on the scanout buffer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mips/psyqo/src/gpu.cpp` around lines 314 - 329, The Horizontal buffer
selection in gpu.cpp is using the same VRAM half as setDisplayArea, so drawing
and clears end up on the scanout buffer. Update the Layout::Horizontal branch in
the scissor setup here, and make the same inversion in getClear, getNextScissor,
and getNextClear so the offscreen buffer is selected consistently. Use the
existing Layout::Horizontal handling and the related helper methods to keep the
buffer selection logic aligned.

Source: Linters/SAST tools

}

void psyqo::GPU::getNextScissor(Prim::Scissor &scissor) {
Expand All @@ -287,9 +335,22 @@
int16_t height = m_height;
bool firstBuffer = !parity || m_interlaced;

scissor.start = Prim::DrawingAreaStart(Vertex{{.x = 0, .y = firstBuffer ? int16_t(256) : int16_t(0)}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = width, .y = firstBuffer ? int16_t(256 + height) : height}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = int16_t(0), .y = firstBuffer ? int16_t(256) : int16_t(0)}});
if constexpr (c_layout == Layout::Horizontal) {
int16_t x = firstBuffer ? width : int16_t(0);
scissor.start = Prim::DrawingAreaStart(Vertex{{.x = x, .y = 0}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = int16_t(x + width), .y = height}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = x, .y = 0}});
} else {
int16_t y;
if constexpr (c_layout == Layout::VerticalSwitch) {
y = firstBuffer ? int16_t(256) : int16_t(16);
} else {
y = firstBuffer ? int16_t(256) : int16_t(0);
}
scissor.start = Prim::DrawingAreaStart(Vertex{{.x = 0, .y = y}});
scissor.end = Prim::DrawingAreaEnd(Vertex{{.x = width, .y = int16_t(y + height)}});
scissor.offset = Prim::DrawingOffset(Vertex{{.x = int16_t(0), .y = y}});
}
}

void psyqo::GPU::clear(Color bg) {
Expand All @@ -303,15 +364,35 @@
int16_t height = m_height;
bool firstBuffer = !m_parity || m_interlaced;
ff.setColor(bg);
ff.rect = Rect{0, firstBuffer ? int16_t(0) : int16_t(256), width, height};
if constexpr (c_layout == Layout::Horizontal) {
ff.rect = Rect{firstBuffer ? int16_t(0) : width, 0, width, height};
} else {
int16_t y;
if constexpr (c_layout == Layout::VerticalSwitch) {
y = firstBuffer ? int16_t(16) : int16_t(256);
} else {
y = firstBuffer ? int16_t(0) : int16_t(256);
}
ff.rect = Rect{0, y, width, height};
}
}

void psyqo::GPU::getNextClear(Prim::FastFill &ff, Color bg) const {
int16_t width = m_width;
int16_t height = m_height;
bool firstBuffer = !m_parity || m_interlaced;
ff.setColor(bg);
ff.rect = Rect{0, firstBuffer ? int16_t(256) : int16_t(0), width, height};
if constexpr (c_layout == Layout::Horizontal) {
ff.rect = Rect{firstBuffer ? width : int16_t(0), 0, width, height};
} else {
int16_t y;
if constexpr (c_layout == Layout::VerticalSwitch) {
y = firstBuffer ? int16_t(256) : int16_t(16);
} else {
y = firstBuffer ? int16_t(256) : int16_t(0);
}
ff.rect = Rect{0, y, width, height};
}
}

void psyqo::GPU::uploadToVRAM(const uint16_t *data, Rect rect) {
Expand Down
Loading