From 84e72c185afa918b3478e37ccabe87a1309311b2 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sat, 30 Nov 2024 21:19:36 +0000 Subject: [PATCH 1/2] VideoCommon: drop CP MMIO registers that were probably added in the wrong place I think someone confused these with the actual token and bounding box registers in PE, which were added later. In CP they never did anything and it's suspicious that they have the same addresses as their PE counterparts. On real hardware they always read as zero. --- Source/Core/Core/State.cpp | 2 +- Source/Core/VideoCommon/CommandProcessor.cpp | 21 -------------------- Source/Core/VideoCommon/CommandProcessor.h | 5 ----- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index aac9d05221..21b10bf28a 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -99,7 +99,7 @@ static size_t s_state_writes_in_queue; static std::condition_variable s_state_write_queue_is_empty; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 169; // Last changed in PR 13074 +constexpr u32 STATE_VERSION = 170; // Last changed in PR 13219 // Increase this if the StateExtendedHeader definition changes constexpr u32 EXTENDED_HEADER_VERSION = 1; // Last changed in PR 12217 diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 69b52c5355..857f3239b0 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -87,11 +87,6 @@ void CommandProcessorManager::DoState(PointerWrap& p) p.Do(m_cp_status_reg); p.Do(m_cp_ctrl_reg); p.Do(m_cp_clear_reg); - p.Do(m_bbox_left); - p.Do(m_bbox_top); - p.Do(m_bbox_right); - p.Do(m_bbox_bottom); - p.Do(m_token_reg); m_fifo.DoState(p); p.Do(m_interrupt_set); @@ -118,13 +113,6 @@ void CommandProcessorManager::Init() m_cp_clear_reg.Hex = 0; - m_bbox_left = 0; - m_bbox_top = 0; - m_bbox_right = 640; - m_bbox_bottom = 480; - - m_token_reg = 0; - m_fifo.Init(); m_is_fifo_error_seen = false; @@ -138,8 +126,6 @@ void CommandProcessorManager::Init() void CommandProcessorManager::RegisterMMIO(MMIO::Mapping* mmio, u32 base) { - constexpr u16 WMASK_NONE = 0x0000; - constexpr u16 WMASK_ALL = 0xffff; constexpr u16 WMASK_LO_ALIGN_32BIT = 0xffe0; const u16 WMASK_HI_RESTRICT = GetPhysicalAddressMask(m_system.IsWii()) >> 16; @@ -153,13 +139,6 @@ void CommandProcessorManager::RegisterMMIO(MMIO::Mapping* mmio, u32 base) // For _HI registers in this range, only bits 0x03ff can be set on GCN and 0x1fff on Wii u16 wmask; } directly_mapped_vars[] = { - {FIFO_TOKEN_REGISTER, &m_token_reg, false, WMASK_ALL}, - - // Bounding box registers are read only. - {FIFO_BOUNDING_BOX_LEFT, &m_bbox_left, true, WMASK_NONE}, - {FIFO_BOUNDING_BOX_RIGHT, &m_bbox_right, true, WMASK_NONE}, - {FIFO_BOUNDING_BOX_TOP, &m_bbox_top, true, WMASK_NONE}, - {FIFO_BOUNDING_BOX_BOTTOM, &m_bbox_bottom, true, WMASK_NONE}, {FIFO_BASE_LO, MMIO::Utils::LowPart(&m_fifo.CPBase), false, WMASK_LO_ALIGN_32BIT}, {FIFO_BASE_HI, MMIO::Utils::HighPart(&m_fifo.CPBase), false, WMASK_HI_RESTRICT}, {FIFO_END_LO, MMIO::Utils::LowPart(&m_fifo.CPEnd), false, WMASK_LO_ALIGN_32BIT}, diff --git a/Source/Core/VideoCommon/CommandProcessor.h b/Source/Core/VideoCommon/CommandProcessor.h index b231eaa3dc..fcd81a893e 100644 --- a/Source/Core/VideoCommon/CommandProcessor.h +++ b/Source/Core/VideoCommon/CommandProcessor.h @@ -60,11 +60,6 @@ enum CTRL_REGISTER = 0x02, CLEAR_REGISTER = 0x04, PERF_SELECT = 0x06, - FIFO_TOKEN_REGISTER = 0x0E, - FIFO_BOUNDING_BOX_LEFT = 0x10, - FIFO_BOUNDING_BOX_RIGHT = 0x12, - FIFO_BOUNDING_BOX_TOP = 0x14, - FIFO_BOUNDING_BOX_BOTTOM = 0x16, FIFO_BASE_LO = 0x20, FIFO_BASE_HI = 0x22, FIFO_END_LO = 0x24, From 19d954eaeaa5674583cf32c15fe21b425f498cb3 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sat, 30 Nov 2024 21:20:53 +0000 Subject: [PATCH 2/2] VideoCommon: remove CP readonly field, it's now always false --- Source/Core/VideoCommon/CommandProcessor.cpp | 34 ++++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 857f3239b0..3a771e1f2a 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -133,40 +133,32 @@ void CommandProcessorManager::RegisterMMIO(MMIO::Mapping* mmio, u32 base) { u32 addr; u16* ptr; - bool readonly; // FIFO mmio regs in the range [cc000020-cc00003e] have certain bits that always read as 0 // For _LO registers in this range, only bits 0xffe0 can be set // For _HI registers in this range, only bits 0x03ff can be set on GCN and 0x1fff on Wii u16 wmask; } directly_mapped_vars[] = { - {FIFO_BASE_LO, MMIO::Utils::LowPart(&m_fifo.CPBase), false, WMASK_LO_ALIGN_32BIT}, - {FIFO_BASE_HI, MMIO::Utils::HighPart(&m_fifo.CPBase), false, WMASK_HI_RESTRICT}, - {FIFO_END_LO, MMIO::Utils::LowPart(&m_fifo.CPEnd), false, WMASK_LO_ALIGN_32BIT}, - {FIFO_END_HI, MMIO::Utils::HighPart(&m_fifo.CPEnd), false, WMASK_HI_RESTRICT}, - {FIFO_HI_WATERMARK_LO, MMIO::Utils::LowPart(&m_fifo.CPHiWatermark), false, - WMASK_LO_ALIGN_32BIT}, - {FIFO_HI_WATERMARK_HI, MMIO::Utils::HighPart(&m_fifo.CPHiWatermark), false, - WMASK_HI_RESTRICT}, - {FIFO_LO_WATERMARK_LO, MMIO::Utils::LowPart(&m_fifo.CPLoWatermark), false, - WMASK_LO_ALIGN_32BIT}, - {FIFO_LO_WATERMARK_HI, MMIO::Utils::HighPart(&m_fifo.CPLoWatermark), false, - WMASK_HI_RESTRICT}, + {FIFO_BASE_LO, MMIO::Utils::LowPart(&m_fifo.CPBase), WMASK_LO_ALIGN_32BIT}, + {FIFO_BASE_HI, MMIO::Utils::HighPart(&m_fifo.CPBase), WMASK_HI_RESTRICT}, + {FIFO_END_LO, MMIO::Utils::LowPart(&m_fifo.CPEnd), WMASK_LO_ALIGN_32BIT}, + {FIFO_END_HI, MMIO::Utils::HighPart(&m_fifo.CPEnd), WMASK_HI_RESTRICT}, + {FIFO_HI_WATERMARK_LO, MMIO::Utils::LowPart(&m_fifo.CPHiWatermark), WMASK_LO_ALIGN_32BIT}, + {FIFO_HI_WATERMARK_HI, MMIO::Utils::HighPart(&m_fifo.CPHiWatermark), WMASK_HI_RESTRICT}, + {FIFO_LO_WATERMARK_LO, MMIO::Utils::LowPart(&m_fifo.CPLoWatermark), WMASK_LO_ALIGN_32BIT}, + {FIFO_LO_WATERMARK_HI, MMIO::Utils::HighPart(&m_fifo.CPLoWatermark), WMASK_HI_RESTRICT}, // FIFO_RW_DISTANCE has some complex read code different for // single/dual core. - {FIFO_WRITE_POINTER_LO, MMIO::Utils::LowPart(&m_fifo.CPWritePointer), false, - WMASK_LO_ALIGN_32BIT}, - {FIFO_WRITE_POINTER_HI, MMIO::Utils::HighPart(&m_fifo.CPWritePointer), false, - WMASK_HI_RESTRICT}, + {FIFO_WRITE_POINTER_LO, MMIO::Utils::LowPart(&m_fifo.CPWritePointer), WMASK_LO_ALIGN_32BIT}, + {FIFO_WRITE_POINTER_HI, MMIO::Utils::HighPart(&m_fifo.CPWritePointer), WMASK_HI_RESTRICT}, // FIFO_READ_POINTER has different code for single/dual core. - {FIFO_BP_LO, MMIO::Utils::LowPart(&m_fifo.CPBreakpoint), false, WMASK_LO_ALIGN_32BIT}, - {FIFO_BP_HI, MMIO::Utils::HighPart(&m_fifo.CPBreakpoint), false, WMASK_HI_RESTRICT}, + {FIFO_BP_LO, MMIO::Utils::LowPart(&m_fifo.CPBreakpoint), WMASK_LO_ALIGN_32BIT}, + {FIFO_BP_HI, MMIO::Utils::HighPart(&m_fifo.CPBreakpoint), WMASK_HI_RESTRICT}, }; for (auto& mapped_var : directly_mapped_vars) { mmio->Register(base | mapped_var.addr, MMIO::DirectRead(mapped_var.ptr), - mapped_var.readonly ? MMIO::InvalidWrite() : - MMIO::DirectWrite(mapped_var.ptr, mapped_var.wmask)); + MMIO::DirectWrite(mapped_var.ptr, mapped_var.wmask)); } // Timing and metrics MMIOs are stubbed with fixed values.