From a84cd150231c29f63c27303ab56e59279cfc7ed6 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 16 Jul 2022 00:05:03 -0700 Subject: [PATCH 01/34] Implement stub version of the Wii AV Encoder --- Source/Core/Core/HW/WII_IPC.cpp | 283 +++++++++++++++++++++++++++++--- Source/Core/Core/HW/WII_IPC.h | 1 + Source/Core/Core/State.cpp | 2 +- 3 files changed, 259 insertions(+), 27 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 8ce63d7c1f73..1e53bdff1161 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -3,6 +3,9 @@ #include "Core/HW/WII_IPC.h" +#include +#include + #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" #include "Common/Logging/Log.h" @@ -60,6 +63,63 @@ enum static constexpr Common::Flags gpio_owner = {GPIO::SLOT_LED, GPIO::SLOT_IN, GPIO::SENSOR_BAR, GPIO::DO_EJECT, GPIO::AVE_SCL, GPIO::AVE_SDA}; +static u32 resets; + +struct I2CState +{ + bool active; + u8 bit_counter; + bool read_i2c_address; + bool is_correct_i2c_address; + bool is_read; + bool read_ave_address; + bool acknowledge; + u8 current_byte; + u8 current_address; +}; +I2CState i2c_state; +#pragma pack(1) +struct AVEState +{ + // See https://wiibrew.org/wiki/Hardware/AV_Encoder#Registers_description + // (note that the code snippet indicates that values are big-endian) + u8 timings; // 0x00 + u8 video_output_config; // 0x01 + u8 vertical_blanking_interval_control; // 0x02 + u8 composite_trap_filter_control; // 0x03 + u8 audio_video_output_control; // 0x04 + u8 cgms_high, cgms_low; // 0x05-0x06 + u8 pad1; // 0x07 + u8 wss_high, wss_low; // 0x08-0x09, Widescreen signaling? + u8 rgb_color_output_control; // 0x0A, only used when video_output_config is DEBUG (3)? + std::array pad2; // 0x0B-0x0F + std::array gamma_coefficients; // 0x10-0x30 + std::array pad3; // 0x31-0x3F + std::array macrovision_code; // 0x40-0x59, analog copy protection + std::array pad4; // 0x5A-0x61 + u8 rgb_switch; // 0x62, swap blue and red channels + std::array pad5; // 0x63-0x64 + u8 color_dac; // 0x65 + u8 pad6; // 0x66 + u8 color_test; // 0x67, display a color test pattern + std::array pad7; // 0x68-0x69 + u8 ccsel; // 0x6A + std::array pad8; // 0x6B-0x6C + u8 mute; // 0x6D + u8 rgb_output_filter; // 0x6E + std::array pad9; // 0x6F-0x70 + u8 right_volume; // 0x71 + u8 left_volume; // 0x72 + std::array pad10; // 0x73-0x79 + std::array closed_captioning; // 0x7A-0x7D + std::array pad11; // 0x7E-0xFF +}; +#pragma pack() +static_assert(sizeof(AVEState) == 0x100); +static AVEState ave_state; + +static CoreTiming::EventType* updateInterrupts; + WiiIPC::WiiIPC(Core::System& system) : m_system(system) { } @@ -106,6 +166,9 @@ void WiiIPC::InitState() m_resets = 0xffffffff; m_ppc_irq_masks |= INT_CAUSE_IPC_BROADWAY; + + i2c_state = {}; + ave_state = {}; } void WiiIPC::Init() @@ -125,6 +188,196 @@ void WiiIPC::Shutdown() { } +static std::string_view GetAVERegisterName(u8 address) +{ + if (address == 0x00) + return "A/V Timings"; + else if (address == 0x01) + return "Video Output configuration"; + else if (address == 0x02) + return "Vertical blanking interval (VBI) control"; + else if (address == 0x03) + return "Composite Video Trap Filter control"; + else if (address == 0x04) + return "A/V output control"; + else if (address == 0x05 || address == 0x06) + return "CGMS protection"; + else if (address == 0x08 || address == 0x09) + return "WSS (Widescreen signaling)"; + else if (address == 0x0A) + return "RGB color output control"; + else if (address >= 0x10 && address <= 0x30) + return "Gamma coefficients"; + else if (address >= 0x40 && address <= 0x59) + return "Macrovision code"; + else if (address == 0x62) + return "RGB switch control"; + else if (address == 0x65) + return "Color DAC control"; + else if (address == 0x67) + return "Color Test"; + else if (address == 0x6A) + return "CCSEL"; + else if (address == 0x6D) + return "Audio mute control"; + else if (address == 0x6E) + return "RGB output filter"; + else if (address == 0x71) + return "Audio stereo output control - right volume"; + else if (address == 0x72) + return "Audio stereo output control - right volume"; + else if (address >= 0x7a && address <= 0x7d) + return "Closed Captioning control"; + else + return fmt::format("Unknown ({:02x})", address); +} + +static u32 ReadGPIOIn(Core::System& system) +{ + Common::Flags gpio_in; + gpio_in[GPIO::SLOT_IN] = system.GetDVDInterface().IsDiscInside(); + // Note: This doesn't implement the direction logic currently (are bits not included in the + // direction treated as clear?) + if (i2c_state.bit_counter == 9 && i2c_state.acknowledge) + gpio_in[GPIO::AVE_SDA] = false; // pull low + else + gpio_in[GPIO::AVE_SDA] = true; // passive pullup + return gpio_in.m_hex; +} + +void WiiIPC::WriteGPIOOut(Core::System& system, bool broadway, u32 value) +{ + Common::Flags old_value = m_gpio_out; + + if (broadway) + m_gpio_out.m_hex = (value & gpio_owner.m_hex) | (m_gpio_out.m_hex & ~gpio_owner.m_hex); + else + m_gpio_out.m_hex = (value & ~gpio_owner.m_hex) | (m_gpio_out.m_hex & gpio_owner.m_hex); + + if (m_gpio_out[GPIO::DO_EJECT]) + { + INFO_LOG_FMT(WII_IPC, "Ejecting disc due to GPIO write"); + system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{system}, DVD::EjectCause::Software); + } + + // I²C logic for the audio/video encoder (AVE) + if (m_gpio_dir[GPIO::AVE_SCL]) + { + if (old_value[GPIO::AVE_SCL] && m_gpio_out[GPIO::AVE_SCL]) + { + // Check for changes to SDA while the clock is high. This only makes sense if the SDA pin is + // outbound. + if (m_gpio_dir[GPIO::AVE_SDA]) + { + if (old_value[GPIO::AVE_SDA] && !m_gpio_out[GPIO::AVE_SDA]) + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Start I2C"); + // SDA falling edge (now pulled low) while SCL is high indicates I²C start + i2c_state.active = true; + i2c_state.acknowledge = false; + i2c_state.bit_counter = 0; + i2c_state.read_i2c_address = false; + i2c_state.is_correct_i2c_address = false; + i2c_state.read_ave_address = false; + } + else if (!old_value[GPIO::AVE_SDA] && m_gpio_out[GPIO::AVE_SDA]) + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop + i2c_state.active = false; + i2c_state.bit_counter = 0; + } + } + } + else if (!old_value[GPIO::AVE_SCL] && m_gpio_out[GPIO::AVE_SCL]) + { + // Clock changed from low to high; transfer a new bit. + if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) + { + if (i2c_state.bit_counter == 9) + { + // Note: 9 not 8, as an extra clock is spent for acknowleding + i2c_state.acknowledge = false; + i2c_state.current_byte = 0; + i2c_state.bit_counter = 0; + } + + // Rising edge: a new bit + if (i2c_state.bit_counter < 8) + { + i2c_state.current_byte <<= 1; + if (m_gpio_out[GPIO::AVE_SDA]) + i2c_state.current_byte |= 1; + } + + if (i2c_state.bit_counter == 8) + { + i2c_state.acknowledge = true; + + DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); + + if (!i2c_state.read_i2c_address) + { + i2c_state.read_i2c_address = true; + if ((i2c_state.current_byte >> 1) == 0x70) + { + i2c_state.is_correct_i2c_address = true; + } + else + { + WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1); + i2c_state.acknowledge = false; + i2c_state.is_correct_i2c_address = false; + } + + if ((i2c_state.current_byte & 1) == 0) + { + i2c_state.is_read = false; + } + else + { + WARN_LOG_FMT(WII_IPC, "AVE: Reads aren't implemented yet"); + i2c_state.is_read = true; + i2c_state.acknowledge = false; // until reads are implemented + } + } + else if (!i2c_state.read_ave_address) + { + i2c_state.read_ave_address = true; + i2c_state.current_address = i2c_state.current_byte; + DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + } + else + { + // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes + const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; + reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; + if (old_ave_value != i2c_state.current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + } + i2c_state.current_address++; + } + } + + i2c_state.bit_counter++; + } + } + } + + // SENSOR_BAR is checked by WiimoteEmu::CameraLogic + // TODO: SLOT_LED +} + void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) { mmio->Register(base | IPC_PPCMSG, MMIO::InvalidRead(), MMIO::DirectWrite(&m_ppc_msg)); @@ -172,16 +425,7 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) mmio->Register(base | GPIOB_OUT, MMIO::DirectRead(&m_gpio_out.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); - wii_ipc.m_gpio_out.m_hex = - (val & gpio_owner.m_hex) | (wii_ipc.m_gpio_out.m_hex & ~gpio_owner.m_hex); - if (wii_ipc.m_gpio_out[GPIO::DO_EJECT]) - { - INFO_LOG_FMT(WII_IPC, "Ejecting disc due to GPIO write"); - system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{system}, - DVD::EjectCause::Software); - } - // SENSOR_BAR is checked by WiimoteEmu::CameraLogic - // TODO: AVE, SLOT_LED + wii_ipc.WriteGPIOOut(system, true, val); })); mmio->Register(base | GPIOB_DIR, MMIO::DirectRead(&m_gpio_dir.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { @@ -190,9 +434,7 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) (val & gpio_owner.m_hex) | (wii_ipc.m_gpio_dir.m_hex & ~gpio_owner.m_hex); })); mmio->Register(base | GPIOB_IN, MMIO::ComplexRead([](Core::System& system, u32) { - Common::Flags gpio_in; - gpio_in[GPIO::SLOT_IN] = system.GetDVDInterface().IsDiscInside(); - return gpio_in.m_hex; + return ReadGPIOIn(system); }), MMIO::Nop()); // Starlet GPIO registers, not normally accessible by PPC (but they can be depending on how @@ -209,16 +451,7 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) mmio->Register(base | GPIO_OUT, MMIO::DirectRead(&m_gpio_out.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); - wii_ipc.m_gpio_out.m_hex = - (wii_ipc.m_gpio_out.m_hex & gpio_owner.m_hex) | (val & ~gpio_owner.m_hex); - if (wii_ipc.m_gpio_out[GPIO::DO_EJECT]) - { - INFO_LOG_FMT(WII_IPC, "Ejecting disc due to GPIO write"); - system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{system}, - DVD::EjectCause::Software); - } - // SENSOR_BAR is checked by WiimoteEmu::CameraLogic - // TODO: AVE, SLOT_LED + wii_ipc.WriteGPIOOut(system, false, val); })); mmio->Register(base | GPIO_DIR, MMIO::DirectRead(&m_gpio_dir.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { @@ -227,9 +460,7 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) (wii_ipc.m_gpio_dir.m_hex & gpio_owner.m_hex) | (val & ~gpio_owner.m_hex); })); mmio->Register(base | GPIO_IN, MMIO::ComplexRead([](Core::System& system, u32) { - Common::Flags gpio_in; - gpio_in[GPIO::SLOT_IN] = system.GetDVDInterface().IsDiscInside(); - return gpio_in.m_hex; + return ReadGPIOIn(system); }), MMIO::Nop()); diff --git a/Source/Core/Core/HW/WII_IPC.h b/Source/Core/Core/HW/WII_IPC.h index 64144f43c069..053280b93387 100644 --- a/Source/Core/Core/HW/WII_IPC.h +++ b/Source/Core/Core/HW/WII_IPC.h @@ -140,6 +140,7 @@ class WiiIPC static void UpdateInterruptsCallback(Core::System& system, u64 userdata, s64 cycles_late); void UpdateInterrupts(); + void WriteGPIOOut(Core::System& system, bool broadway, u32 value); u32 m_ppc_msg = 0; u32 m_arm_msg = 0; diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index f9964c0fa812..daf41f4362c4 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -98,7 +98,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 10863 // Increase this if the StateExtendedHeader definition changes constexpr u32 EXTENDED_HEADER_VERSION = 1; // Last changed in PR 12217 From 76767fcfb47cf00ff46aa365a8fb2895c77133e3 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 27 Jul 2022 11:46:14 -0700 Subject: [PATCH 02/34] WII_IPC: Correct comment --- Source/Core/Core/HW/WII_IPC.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 1e53bdff1161..6f9e91e8ff55 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -151,8 +151,8 @@ void WiiIPC::InitState() m_arm_irq_flags = 0; m_arm_irq_masks = 0; - // The only inputs are POWER, EJECT_BTN, SLOT_IN, and EEP_MISO; Broadway only has access to - // SLOT_IN + // The only inputs are POWER, EJECT_BTN, SLOT_IN, EEP_MISO, and sometimes AVE_SCL and AVE_SDA; + // Broadway only has access to SLOT_IN, AVE_SCL, and AVE_SDA. m_gpio_dir = { GPIO::POWER, GPIO::SHUTDOWN, GPIO::FAN, GPIO::DC_DC, GPIO::DI_SPIN, GPIO::SLOT_LED, GPIO::SENSOR_BAR, GPIO::DO_EJECT, GPIO::EEP_CS, GPIO::EEP_CLK, GPIO::EEP_MOSI, GPIO::AVE_SCL, From 143e4a8319b0b3019ccda16946edd862b130dab9 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 27 Jul 2022 18:05:36 -0700 Subject: [PATCH 03/34] Handle dir behavior better --- Source/Core/Core/HW/WII_IPC.cpp | 36 ++++++++++++++++++++++++--------- Source/Core/Core/HW/WII_IPC.h | 3 ++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 6f9e91e8ff55..e9fb4c0add70 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -245,19 +245,25 @@ static u32 ReadGPIOIn(Core::System& system) return gpio_in.m_hex; } -void WiiIPC::WriteGPIOOut(Core::System& system, bool broadway, u32 value) +u32 WiiIPC::GetGPIOOut() { - Common::Flags old_value = m_gpio_out; + // In the direction field, a '1' bit for a pin indicates that it will behave as an output (drive), + // while a '0' bit tristates the pin and it becomes a high-impedance input. + // In practice this means that (at least for the AVE I²C pins) a 1 is output when the pin is an + // input. (RVLoader depends on this.) + // https://github.com/Aurelio92/RVLoader/blob/75732f248019f589deb1109bba7b5323a8afaadf/source/i2c.c#L101-L109 + return m_gpio_out.m_hex | ~m_gpio_dir.m_hex; +} - if (broadway) - m_gpio_out.m_hex = (value & gpio_owner.m_hex) | (m_gpio_out.m_hex & ~gpio_owner.m_hex); - else - m_gpio_out.m_hex = (value & ~gpio_owner.m_hex) | (m_gpio_out.m_hex & gpio_owner.m_hex); +void WiiIPC::GPIOOutChanged(u32 old_value_hex) +{ + Common::Flags old_value; + old_value.m_hex = old_value_hex; if (m_gpio_out[GPIO::DO_EJECT]) { INFO_LOG_FMT(WII_IPC, "Ejecting disc due to GPIO write"); - system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{system}, DVD::EjectCause::Software); + m_system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{m_system}, DVD::EjectCause::Software); } // I²C logic for the audio/video encoder (AVE) @@ -425,13 +431,18 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) mmio->Register(base | GPIOB_OUT, MMIO::DirectRead(&m_gpio_out.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); - wii_ipc.WriteGPIOOut(system, true, val); + const u32 old_out = wii_ipc.GetGPIOOut(); + wii_ipc.m_gpio_out.m_hex = + (val & gpio_owner.m_hex) | (wii_ipc.m_gpio_out.m_hex & ~gpio_owner.m_hex); + wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIOB_DIR, MMIO::DirectRead(&m_gpio_dir.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); + const u32 old_out = wii_ipc.GetGPIOOut(); wii_ipc.m_gpio_dir.m_hex = (val & gpio_owner.m_hex) | (wii_ipc.m_gpio_dir.m_hex & ~gpio_owner.m_hex); + wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIOB_IN, MMIO::ComplexRead([](Core::System& system, u32) { return ReadGPIOIn(system); @@ -451,13 +462,18 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) mmio->Register(base | GPIO_OUT, MMIO::DirectRead(&m_gpio_out.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); - wii_ipc.WriteGPIOOut(system, false, val); + const u32 old_out = wii_ipc.GetGPIOOut(); + wii_ipc.m_gpio_out.m_hex = + (val & ~gpio_owner.m_hex) | (wii_ipc.m_gpio_out.m_hex & gpio_owner.m_hex); + wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIO_DIR, MMIO::DirectRead(&m_gpio_dir.m_hex), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { auto& wii_ipc = system.GetWiiIPC(); + const u32 old_out = wii_ipc.GetGPIOOut(); wii_ipc.m_gpio_dir.m_hex = - (wii_ipc.m_gpio_dir.m_hex & gpio_owner.m_hex) | (val & ~gpio_owner.m_hex); + (val & gpio_owner.m_hex) | (wii_ipc.m_gpio_dir.m_hex & ~gpio_owner.m_hex); + wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIO_IN, MMIO::ComplexRead([](Core::System& system, u32) { return ReadGPIOIn(system); diff --git a/Source/Core/Core/HW/WII_IPC.h b/Source/Core/Core/HW/WII_IPC.h index 053280b93387..e3e4ee55756c 100644 --- a/Source/Core/Core/HW/WII_IPC.h +++ b/Source/Core/Core/HW/WII_IPC.h @@ -140,7 +140,8 @@ class WiiIPC static void UpdateInterruptsCallback(Core::System& system, u64 userdata, s64 cycles_late); void UpdateInterrupts(); - void WriteGPIOOut(Core::System& system, bool broadway, u32 value); + u32 GetGPIOOut(); + void GPIOOutChanged(u32 old_value_hex); u32 m_ppc_msg = 0; u32 m_arm_msg = 0; From 0f4dd969bbcccc4c7080c2fca11229b71899141c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 28 Jul 2022 16:41:06 -0700 Subject: [PATCH 04/34] Update GPIO constants --- Source/Core/Core/HW/WII_IPC.cpp | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index e9fb4c0add70..1b73f39db681 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -36,7 +36,8 @@ enum IPC_ARMMSG = 0x08, IPC_ARMCTRL = 0x0c, - PPCSPEED = 0x18, + VI1CFG = 0x18, + VIDIM = 0x1c, VISOLID = 0x24, PPC_IRQFLAG = 0x30, @@ -44,17 +45,31 @@ enum ARM_IRQFLAG = 0x38, ARM_IRQMASK = 0x3c, + SRNPROT = 0x60, + AHBPROT = 0x64, + + // Broadway GPIO access. We don't currently implement the interrupts. GPIOB_OUT = 0xc0, GPIOB_DIR = 0xc4, GPIOB_IN = 0xc8, - + GPIOB_INTLVL = 0xcc, + GPIOB_INTFLAG = 0xd0, + GPIOB_INTMASK = 0xd4, + GPIOB_STRAPS = 0xd8, + // Starlet GPIO access. We emulate some of these for /dev/di. + GPIO_ENABLE = 0xdc, GPIO_OUT = 0xe0, GPIO_DIR = 0xe4, GPIO_IN = 0xe8, + GPIO_INTLVL = 0xec, + GPIO_INTFLAG = 0xf0, + GPIO_INTMASK = 0xf4, + GPIO_STRAPS = 0xf8, + GPIO_OWNER = 0xfc, - HW_RESETS = 0x194, + COMPAT = 0x180, + RESETS = 0x194, - UNK_180 = 0x180, UNK_1CC = 0x1cc, UNK_1D0 = 0x1d0, }; @@ -480,7 +495,7 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) }), MMIO::Nop()); - mmio->Register(base | HW_RESETS, MMIO::DirectRead(&m_resets), + mmio->Register(base | RESETS, MMIO::DirectRead(&m_resets), MMIO::ComplexWrite([](Core::System& system, u32, u32 val) { // A reset occurs when the corresponding bit is cleared auto& wii_ipc = system.GetWiiIPC(); @@ -496,9 +511,10 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) })); // Register some stubbed/unknown MMIOs required to make Wii games work. - mmio->Register(base | PPCSPEED, MMIO::InvalidRead(), MMIO::Nop()); - mmio->Register(base | VISOLID, MMIO::InvalidRead(), MMIO::Nop()); - mmio->Register(base | UNK_180, MMIO::Constant(0), MMIO::Nop()); + mmio->Register(base | VI1CFG, MMIO::InvalidRead(), MMIO::InvalidWrite()); + mmio->Register(base | VIDIM, MMIO::InvalidRead(), MMIO::InvalidWrite()); + mmio->Register(base | VISOLID, MMIO::InvalidRead(), MMIO::InvalidWrite()); + mmio->Register(base | COMPAT, MMIO::Constant(0), MMIO::Nop()); mmio->Register(base | UNK_1CC, MMIO::Constant(0), MMIO::Nop()); mmio->Register(base | UNK_1D0, MMIO::Constant(0), MMIO::Nop()); } From 25e31d121b0ae8ba9ffd44189733031cc9ecabec Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 12:15:38 -0700 Subject: [PATCH 05/34] More accurate I2C handling I still need to document this; GPIOs are wierd with their tristate behavior --- Source/Core/Core/HW/WII_IPC.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 1b73f39db681..46e29f64520d 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -11,6 +11,7 @@ #include "Common/Logging/Log.h" #include "Core/Core.h" #include "Core/CoreTiming.h" +#include "Core/Debugger/Debugger_SymbolMap.h" #include "Core/HW/DVD/DVDInterface.h" #include "Core/HW/MMIO.h" #include "Core/HW/ProcessorInterface.h" @@ -257,6 +258,7 @@ static u32 ReadGPIOIn(Core::System& system) gpio_in[GPIO::AVE_SDA] = false; // pull low else gpio_in[GPIO::AVE_SDA] = true; // passive pullup + gpio_in[GPIO::AVE_SCL] = true; // passive pullup return gpio_in.m_hex; } @@ -267,7 +269,7 @@ u32 WiiIPC::GetGPIOOut() // In practice this means that (at least for the AVE I²C pins) a 1 is output when the pin is an // input. (RVLoader depends on this.) // https://github.com/Aurelio92/RVLoader/blob/75732f248019f589deb1109bba7b5323a8afaadf/source/i2c.c#L101-L109 - return m_gpio_out.m_hex | ~m_gpio_dir.m_hex; + return (m_gpio_out.m_hex | ~(m_gpio_dir.m_hex)) & (ReadGPIOIn(m_system) | m_gpio_dir.m_hex); } void WiiIPC::GPIOOutChanged(u32 old_value_hex) @@ -275,22 +277,25 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) Common::Flags old_value; old_value.m_hex = old_value_hex; - if (m_gpio_out[GPIO::DO_EJECT]) + Common::Flags new_value; + new_value.m_hex = GetGPIOOut(); + + if (new_value[GPIO::DO_EJECT]) { INFO_LOG_FMT(WII_IPC, "Ejecting disc due to GPIO write"); m_system.GetDVDInterface().EjectDisc(Core::CPUThreadGuard{m_system}, DVD::EjectCause::Software); } // I²C logic for the audio/video encoder (AVE) - if (m_gpio_dir[GPIO::AVE_SCL]) + if (true || m_gpio_dir[GPIO::AVE_SCL]) { - if (old_value[GPIO::AVE_SCL] && m_gpio_out[GPIO::AVE_SCL]) + if (old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) { // Check for changes to SDA while the clock is high. This only makes sense if the SDA pin is // outbound. - if (m_gpio_dir[GPIO::AVE_SDA]) + if (true || m_gpio_dir[GPIO::AVE_SDA]) { - if (old_value[GPIO::AVE_SDA] && !m_gpio_out[GPIO::AVE_SDA]) + if (old_value[GPIO::AVE_SDA] && !new_value[GPIO::AVE_SDA]) { DEBUG_LOG_FMT(WII_IPC, "AVE: Start I2C"); // SDA falling edge (now pulled low) while SCL is high indicates I²C start @@ -301,7 +306,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) i2c_state.is_correct_i2c_address = false; i2c_state.read_ave_address = false; } - else if (!old_value[GPIO::AVE_SDA] && m_gpio_out[GPIO::AVE_SDA]) + else if (!old_value[GPIO::AVE_SDA] && new_value[GPIO::AVE_SDA]) { DEBUG_LOG_FMT(WII_IPC, "AVE: Stop I2C"); // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop @@ -310,7 +315,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } } } - else if (!old_value[GPIO::AVE_SCL] && m_gpio_out[GPIO::AVE_SCL]) + else if (!old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) { // Clock changed from low to high; transfer a new bit. if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) @@ -327,7 +332,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) if (i2c_state.bit_counter < 8) { i2c_state.current_byte <<= 1; - if (m_gpio_out[GPIO::AVE_SDA]) + if (new_value[GPIO::AVE_SDA]) i2c_state.current_byte |= 1; } @@ -347,6 +352,9 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) else { WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(m_system), + Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); i2c_state.acknowledge = false; i2c_state.is_correct_i2c_address = false; } From 2ef84a46db19b2741f59237fccab0a59faf3ac58 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 12:22:56 -0700 Subject: [PATCH 06/34] Unindent --- Source/Core/Core/HW/WII_IPC.cpp | 181 +++++++++++++++----------------- 1 file changed, 86 insertions(+), 95 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 46e29f64520d..2cbe33b7aaba 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -287,119 +287,110 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } // I²C logic for the audio/video encoder (AVE) - if (true || m_gpio_dir[GPIO::AVE_SCL]) + if (old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) { - if (old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) + // Check for changes to SDA while the clock is high. + if (old_value[GPIO::AVE_SDA] && !new_value[GPIO::AVE_SDA]) { - // Check for changes to SDA while the clock is high. This only makes sense if the SDA pin is - // outbound. - if (true || m_gpio_dir[GPIO::AVE_SDA]) - { - if (old_value[GPIO::AVE_SDA] && !new_value[GPIO::AVE_SDA]) - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Start I2C"); - // SDA falling edge (now pulled low) while SCL is high indicates I²C start - i2c_state.active = true; - i2c_state.acknowledge = false; - i2c_state.bit_counter = 0; - i2c_state.read_i2c_address = false; - i2c_state.is_correct_i2c_address = false; - i2c_state.read_ave_address = false; - } - else if (!old_value[GPIO::AVE_SDA] && new_value[GPIO::AVE_SDA]) - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop - i2c_state.active = false; - i2c_state.bit_counter = 0; - } - } + DEBUG_LOG_FMT(WII_IPC, "AVE: Start I2C"); + // SDA falling edge (now pulled low) while SCL is high indicates I²C start + i2c_state.active = true; + i2c_state.acknowledge = false; + i2c_state.bit_counter = 0; + i2c_state.read_i2c_address = false; + i2c_state.is_correct_i2c_address = false; + i2c_state.read_ave_address = false; + } + else if (!old_value[GPIO::AVE_SDA] && new_value[GPIO::AVE_SDA]) + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop + i2c_state.active = false; + i2c_state.bit_counter = 0; } - else if (!old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) + } + else if (!old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) + { + // Clock changed from low to high; transfer a new bit. + if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) { - // Clock changed from low to high; transfer a new bit. - if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) + if (i2c_state.bit_counter == 9) { - if (i2c_state.bit_counter == 9) - { - // Note: 9 not 8, as an extra clock is spent for acknowleding - i2c_state.acknowledge = false; - i2c_state.current_byte = 0; - i2c_state.bit_counter = 0; - } + // Note: 9 not 8, as an extra clock is spent for acknowleding + i2c_state.acknowledge = false; + i2c_state.current_byte = 0; + i2c_state.bit_counter = 0; + } - // Rising edge: a new bit - if (i2c_state.bit_counter < 8) - { - i2c_state.current_byte <<= 1; - if (new_value[GPIO::AVE_SDA]) - i2c_state.current_byte |= 1; - } + // Rising edge: a new bit + if (i2c_state.bit_counter < 8) + { + i2c_state.current_byte <<= 1; + if (new_value[GPIO::AVE_SDA]) + i2c_state.current_byte |= 1; + } - if (i2c_state.bit_counter == 8) - { - i2c_state.acknowledge = true; + if (i2c_state.bit_counter == 8) + { + i2c_state.acknowledge = true; - DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); + DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); - if (!i2c_state.read_i2c_address) + if (!i2c_state.read_i2c_address) + { + i2c_state.read_i2c_address = true; + if ((i2c_state.current_byte >> 1) == 0x70) { - i2c_state.read_i2c_address = true; - if ((i2c_state.current_byte >> 1) == 0x70) - { - i2c_state.is_correct_i2c_address = true; - } - else - { - WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(m_system), - Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); - i2c_state.acknowledge = false; - i2c_state.is_correct_i2c_address = false; - } - - if ((i2c_state.current_byte & 1) == 0) - { - i2c_state.is_read = false; - } - else - { - WARN_LOG_FMT(WII_IPC, "AVE: Reads aren't implemented yet"); - i2c_state.is_read = true; - i2c_state.acknowledge = false; // until reads are implemented - } + i2c_state.is_correct_i2c_address = true; } - else if (!i2c_state.read_ave_address) + else { - i2c_state.read_ave_address = true; - i2c_state.current_address = i2c_state.current_byte; - DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); + WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(m_system), + Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); + i2c_state.acknowledge = false; + i2c_state.is_correct_i2c_address = false; + } + + if ((i2c_state.current_byte & 1) == 0) + { + i2c_state.is_read = false; } else { - // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes - const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; - reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; - if (old_ave_value != i2c_state.current_byte) - { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); - } - else - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); - } - i2c_state.current_address++; + WARN_LOG_FMT(WII_IPC, "AVE: Reads aren't implemented yet"); + i2c_state.is_read = true; + i2c_state.acknowledge = false; // until reads are implemented } } - - i2c_state.bit_counter++; + else if (!i2c_state.read_ave_address) + { + i2c_state.read_ave_address = true; + i2c_state.current_address = i2c_state.current_byte; + DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + } + else + { + // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes + const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; + reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; + if (old_ave_value != i2c_state.current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, GetAVERegisterName(i2c_state.current_address)); + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, GetAVERegisterName(i2c_state.current_address)); + } + i2c_state.current_address++; + } } + + i2c_state.bit_counter++; } } From 08d973bccbb3d171379a37a6967ea28607267005 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 12:28:46 -0700 Subject: [PATCH 07/34] Specialized logging --- Source/Core/Core/HW/WII_IPC.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 2cbe33b7aaba..06625c62bce2 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -4,6 +4,7 @@ #include "Core/HW/WII_IPC.h" #include +#include #include #include "Common/ChunkFile.h" @@ -133,6 +134,7 @@ struct AVEState #pragma pack() static_assert(sizeof(AVEState) == 0x100); static AVEState ave_state; +static std::bitset ave_ever_logged; // For logging only; not saved static CoreTiming::EventType* updateInterrupts; @@ -185,6 +187,7 @@ void WiiIPC::InitState() i2c_state = {}; ave_state = {}; + ave_ever_logged.reset(); } void WiiIPC::Init() @@ -376,10 +379,12 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; - if (old_ave_value != i2c_state.current_byte) + if (!ave_ever_logged[i2c_state.current_address] || + old_ave_value != i2c_state.current_byte) { INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, i2c_state.current_address, GetAVERegisterName(i2c_state.current_address)); + ave_ever_logged[i2c_state.current_address] = true; } else { From b53f6b00a43d8f734fda28a7cc050e977fd8edf0 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 12:35:25 -0700 Subject: [PATCH 08/34] Enhance Common::Flags --- Source/Core/Common/BitUtils.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Source/Core/Common/BitUtils.h b/Source/Core/Common/BitUtils.h index 8b1b196c249d..134ba1fb0716 100644 --- a/Source/Core/Common/BitUtils.h +++ b/Source/Core/Common/BitUtils.h @@ -218,6 +218,12 @@ class Flags { public: constexpr Flags() = default; + constexpr ~Flags() = default; + constexpr Flags(const Flags& other) = default; + constexpr Flags(Flags&& other) noexcept = default; + constexpr Flags& operator=(const Flags& other) = default; + constexpr Flags& operator=(Flags&& other) noexcept = default; + constexpr Flags(std::initializer_list bits) { for (auto bit : bits) @@ -225,7 +231,13 @@ class Flags m_hex |= static_cast>(bit); } } + constexpr Flags(std::underlying_type_t hex) : m_hex(hex) {} + FlagBit operator[](T bit) { return FlagBit(m_hex, bit); } + bool operator[](T bit) const + { + return (m_hex & static_cast>(bit)) != 0; + } std::underlying_type_t m_hex = 0; }; From 7cea5b9508a89ee1259bfe06c5fbabce66492982 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 12:35:35 -0700 Subject: [PATCH 09/34] Use enhanced Common::Flags --- Source/Core/Core/HW/WII_IPC.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 06625c62bce2..3cf1c76b5dc2 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -277,11 +277,8 @@ u32 WiiIPC::GetGPIOOut() void WiiIPC::GPIOOutChanged(u32 old_value_hex) { - Common::Flags old_value; - old_value.m_hex = old_value_hex; - - Common::Flags new_value; - new_value.m_hex = GetGPIOOut(); + const Common::Flags old_value(old_value_hex); + const Common::Flags new_value(GetGPIOOut()); if (new_value[GPIO::DO_EJECT]) { From 8a03b9f46e3ad9ee9d025dde1ae2bb0aedfa4bc7 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 16:22:33 -0700 Subject: [PATCH 10/34] One more enhance --- Source/Core/Common/BitUtils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Common/BitUtils.h b/Source/Core/Common/BitUtils.h index 134ba1fb0716..fefc37b730b4 100644 --- a/Source/Core/Common/BitUtils.h +++ b/Source/Core/Common/BitUtils.h @@ -234,7 +234,7 @@ class Flags constexpr Flags(std::underlying_type_t hex) : m_hex(hex) {} FlagBit operator[](T bit) { return FlagBit(m_hex, bit); } - bool operator[](T bit) const + constexpr bool operator[](T bit) const { return (m_hex & static_cast>(bit)) != 0; } From 4135f4c8bc61ca674d984f24abc14fcab6f6c8d2 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 9 Aug 2022 22:41:08 -0700 Subject: [PATCH 11/34] Start of reads The wrong value is read, but this is the correct structure, at least... --- Source/Core/Core/HW/WII_IPC.cpp | 118 ++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 30 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 3cf1c76b5dc2..e26b8bb445de 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -187,6 +187,7 @@ void WiiIPC::InitState() i2c_state = {}; ave_state = {}; + ave_state.video_output_config = 0x55; ave_ever_logged.reset(); } @@ -258,10 +259,23 @@ static u32 ReadGPIOIn(Core::System& system) // Note: This doesn't implement the direction logic currently (are bits not included in the // direction treated as clear?) if (i2c_state.bit_counter == 9 && i2c_state.acknowledge) + { gpio_in[GPIO::AVE_SDA] = false; // pull low - else + } + else if (i2c_state.is_read) + { + if (i2c_state.bit_counter < 8) + gpio_in[GPIO::AVE_SDA] = ((i2c_state.current_byte << i2c_state.bit_counter) & 0x80) != 0; + else if (i2c_state.bit_counter == 9) + gpio_in[GPIO::AVE_SDA] = true; + else + gpio_in[GPIO::AVE_SDA] = true; // passive pullup + } + else // write + { gpio_in[GPIO::AVE_SDA] = true; // passive pullup - gpio_in[GPIO::AVE_SCL] = true; // passive pullup + } + gpio_in[GPIO::AVE_SCL] = true; // passive pullup return gpio_in.m_hex; } @@ -289,28 +303,34 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) // I²C logic for the audio/video encoder (AVE) if (old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) { + INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", i2c_state.bit_counter, new_value[GPIO::AVE_SDA], + !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); // Check for changes to SDA while the clock is high. if (old_value[GPIO::AVE_SDA] && !new_value[GPIO::AVE_SDA]) { - DEBUG_LOG_FMT(WII_IPC, "AVE: Start I2C"); + INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); // SDA falling edge (now pulled low) while SCL is high indicates I²C start i2c_state.active = true; i2c_state.acknowledge = false; i2c_state.bit_counter = 0; i2c_state.read_i2c_address = false; i2c_state.is_correct_i2c_address = false; - i2c_state.read_ave_address = false; + // i2c_state.read_ave_address = false; } else if (!old_value[GPIO::AVE_SDA] && new_value[GPIO::AVE_SDA]) { - DEBUG_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop i2c_state.active = false; i2c_state.bit_counter = 0; + i2c_state.read_ave_address = false; } } else if (!old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) { + INFO_LOG_FMT(WII_IPC, "{} {} {}", i2c_state.bit_counter, new_value[GPIO::AVE_SDA], + !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); // Clock changed from low to high; transfer a new bit. if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) { @@ -318,12 +338,13 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) { // Note: 9 not 8, as an extra clock is spent for acknowleding i2c_state.acknowledge = false; - i2c_state.current_byte = 0; + if (!i2c_state.is_read) + i2c_state.current_byte = 0; i2c_state.bit_counter = 0; } // Rising edge: a new bit - if (i2c_state.bit_counter < 8) + if (!(i2c_state.is_read && i2c_state.read_i2c_address) && i2c_state.bit_counter < 8) { i2c_state.current_byte <<= 1; if (new_value[GPIO::AVE_SDA]) @@ -332,9 +353,19 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) if (i2c_state.bit_counter == 8) { - i2c_state.acknowledge = true; - - DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); + if (!i2c_state.is_read) + { + i2c_state.acknowledge = true; + DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); + } + else + { + WARN_LOG_FMT(WII_IPC, "AVE: Read ack: {}", new_value[GPIO::AVE_SDA] ? "nack" : "ack"); + if (new_value[GPIO::AVE_SDA]) // nack + { + i2c_state.is_read = false; // XXX + } + } if (!i2c_state.read_i2c_address) { @@ -359,36 +390,63 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } else { - WARN_LOG_FMT(WII_IPC, "AVE: Reads aren't implemented yet"); i2c_state.is_read = true; - i2c_state.acknowledge = false; // until reads are implemented + i2c_state.acknowledge = true; + if (!i2c_state.read_ave_address) + { + WARN_LOG_FMT(WII_IPC, "AVE: Read attempted without setting device address"); + i2c_state.acknowledge = false; + } + else + { + i2c_state.current_byte = reinterpret_cast(&ave_state)[i2c_state.current_address]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", + i2c_state.current_address, GetAVERegisterName(i2c_state.current_address), + i2c_state.current_byte); + Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); + } } } - else if (!i2c_state.read_ave_address) + else if (!i2c_state.is_read) { - i2c_state.read_ave_address = true; - i2c_state.current_address = i2c_state.current_byte; - DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); - } - else - { - // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes - const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; - reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; - if (!ave_ever_logged[i2c_state.current_address] || - old_ave_value != i2c_state.current_byte) + if (!i2c_state.read_ave_address) { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, GetAVERegisterName(i2c_state.current_address)); - ave_ever_logged[i2c_state.current_address] = true; + i2c_state.read_ave_address = true; + i2c_state.current_address = i2c_state.current_byte; + DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); } else { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, GetAVERegisterName(i2c_state.current_address)); + // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes + const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; + reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; + if (!ave_ever_logged[i2c_state.current_address] || + old_ave_value != i2c_state.current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + ave_ever_logged[i2c_state.current_address] = true; + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, + i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address)); + } + i2c_state.current_address++; } + } + else // is_read is true + { i2c_state.current_address++; + i2c_state.current_byte = reinterpret_cast(&ave_state)[i2c_state.current_address]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", i2c_state.current_address, + GetAVERegisterName(i2c_state.current_address), i2c_state.current_byte); + Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); } } From 6cff4c9a463cb03ff41ad69662754aa66cb59e89 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Fri, 26 Aug 2022 15:07:53 -0700 Subject: [PATCH 12/34] Refactoring --- Source/Core/Core/HW/WII_IPC.cpp | 103 +++++++++++++++++++++----------- 1 file changed, 68 insertions(+), 35 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index e26b8bb445de..6ba77ae187e6 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -82,19 +82,6 @@ static constexpr Common::Flags gpio_owner = {GPIO::SLOT_LED, GPIO::SLOT_IN static u32 resets; -struct I2CState -{ - bool active; - u8 bit_counter; - bool read_i2c_address; - bool is_correct_i2c_address; - bool is_read; - bool read_ave_address; - bool acknowledge; - u8 current_byte; - u8 current_address; -}; -I2CState i2c_state; #pragma pack(1) struct AVEState { @@ -136,6 +123,31 @@ static_assert(sizeof(AVEState) == 0x100); static AVEState ave_state; static std::bitset ave_ever_logged; // For logging only; not saved +class I2CBus +{ +public: + bool active; + u8 bit_counter; + bool read_i2c_address; + bool is_correct_i2c_address; + bool is_read; + bool read_ave_address; + bool acknowledge; + u8 current_byte; + u8 current_address; + + void Update(bool old_scl, bool new_scl, bool old_sda, bool new_sda); + bool GetSCL() const; + bool GetSDA() const; + +private: + void Start(); + void Stop(); + void WriteBit(bool value); + bool ReadBit(); +}; +I2CBus i2c_state; + static CoreTiming::EventType* updateInterrupts; WiiIPC::WiiIPC(Core::System& system) : m_system(system) @@ -256,27 +268,35 @@ static u32 ReadGPIOIn(Core::System& system) { Common::Flags gpio_in; gpio_in[GPIO::SLOT_IN] = system.GetDVDInterface().IsDiscInside(); - // Note: This doesn't implement the direction logic currently (are bits not included in the - // direction treated as clear?) + gpio_in[GPIO::AVE_SCL] = i2c_state.GetSCL(); + gpio_in[GPIO::AVE_SDA] = i2c_state.GetSDA(); + return gpio_in.m_hex; +} + +bool I2CBus::GetSCL() const +{ + return true; // passive pullup - no clock stretching +} + +bool I2CBus::GetSDA() const +{ if (i2c_state.bit_counter == 9 && i2c_state.acknowledge) { - gpio_in[GPIO::AVE_SDA] = false; // pull low + return false; // pull low } else if (i2c_state.is_read) { if (i2c_state.bit_counter < 8) - gpio_in[GPIO::AVE_SDA] = ((i2c_state.current_byte << i2c_state.bit_counter) & 0x80) != 0; + return ((i2c_state.current_byte << i2c_state.bit_counter) & 0x80) != 0; else if (i2c_state.bit_counter == 9) - gpio_in[GPIO::AVE_SDA] = true; + return true; else - gpio_in[GPIO::AVE_SDA] = true; // passive pullup + return true; // passive pullup } else // write { - gpio_in[GPIO::AVE_SDA] = true; // passive pullup + return true; // passive pullup } - gpio_in[GPIO::AVE_SCL] = true; // passive pullup - return gpio_in.m_hex; } u32 WiiIPC::GetGPIOOut() @@ -301,12 +321,28 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } // I²C logic for the audio/video encoder (AVE) - if (old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) + i2c_state.Update(old_value[GPIO::AVE_SCL], new_value[GPIO::AVE_SCL], old_value[GPIO::AVE_SDA], + new_value[GPIO::AVE_SDA]); + + // SENSOR_BAR is checked by WiimoteEmu::CameraLogic + // TODO: SLOT_LED +} + +void I2CBus::Update(bool old_scl, bool new_scl, bool old_sda, bool new_sda) +{ + if (old_scl != new_scl && old_sda != new_sda) { - INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", i2c_state.bit_counter, new_value[GPIO::AVE_SDA], + ERROR_LOG_FMT(WII_IPC, "Both SCL and SDA changed at the same time: SCL {} -> {} SDA {} -> {}", + old_scl, new_scl, old_sda, new_sda); + return; + } + + if (old_scl && new_scl) + { + INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", i2c_state.bit_counter, new_sda, !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); // Check for changes to SDA while the clock is high. - if (old_value[GPIO::AVE_SDA] && !new_value[GPIO::AVE_SDA]) + if (old_sda && !new_sda) { INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); // SDA falling edge (now pulled low) while SCL is high indicates I²C start @@ -317,7 +353,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) i2c_state.is_correct_i2c_address = false; // i2c_state.read_ave_address = false; } - else if (!old_value[GPIO::AVE_SDA] && new_value[GPIO::AVE_SDA]) + else if (!old_sda && new_sda) { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); @@ -327,9 +363,9 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) i2c_state.read_ave_address = false; } } - else if (!old_value[GPIO::AVE_SCL] && new_value[GPIO::AVE_SCL]) + else if (!old_scl && new_scl) { - INFO_LOG_FMT(WII_IPC, "{} {} {}", i2c_state.bit_counter, new_value[GPIO::AVE_SDA], + INFO_LOG_FMT(WII_IPC, "{} {} {}", i2c_state.bit_counter, new_sda, !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); // Clock changed from low to high; transfer a new bit. if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) @@ -347,7 +383,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) if (!(i2c_state.is_read && i2c_state.read_i2c_address) && i2c_state.bit_counter < 8) { i2c_state.current_byte <<= 1; - if (new_value[GPIO::AVE_SDA]) + if (new_sda) i2c_state.current_byte |= 1; } @@ -360,8 +396,8 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } else { - WARN_LOG_FMT(WII_IPC, "AVE: Read ack: {}", new_value[GPIO::AVE_SDA] ? "nack" : "ack"); - if (new_value[GPIO::AVE_SDA]) // nack + WARN_LOG_FMT(WII_IPC, "AVE: Read ack: {}", new_sda ? "nack" : "ack"); + if (new_sda) // nack { i2c_state.is_read = false; // XXX } @@ -391,7 +427,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) else { i2c_state.is_read = true; - i2c_state.acknowledge = true; + i2c_state.acknowledge = true; if (!i2c_state.read_ave_address) { WARN_LOG_FMT(WII_IPC, "AVE: Read attempted without setting device address"); @@ -453,9 +489,6 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) i2c_state.bit_counter++; } } - - // SENSOR_BAR is checked by WiimoteEmu::CameraLogic - // TODO: SLOT_LED } void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) From 156040b10bef9d68d997165cf826d5e45955a98c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Fri, 26 Aug 2022 15:08:50 -0700 Subject: [PATCH 13/34] More refactoring - no need for i2c_state in I2CBus functions --- Source/Core/Core/HW/WII_IPC.cpp | 159 ++++++++++++++++---------------- 1 file changed, 79 insertions(+), 80 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 6ba77ae187e6..f755b00bae59 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -136,7 +136,7 @@ class I2CBus u8 current_byte; u8 current_address; - void Update(bool old_scl, bool new_scl, bool old_sda, bool new_sda); + void Update(Core::System& system, bool old_scl, bool new_scl, bool old_sda, bool new_sda); bool GetSCL() const; bool GetSDA() const; @@ -280,15 +280,15 @@ bool I2CBus::GetSCL() const bool I2CBus::GetSDA() const { - if (i2c_state.bit_counter == 9 && i2c_state.acknowledge) + if (bit_counter == 9 && acknowledge) { return false; // pull low } - else if (i2c_state.is_read) + else if (is_read) { - if (i2c_state.bit_counter < 8) - return ((i2c_state.current_byte << i2c_state.bit_counter) & 0x80) != 0; - else if (i2c_state.bit_counter == 9) + if (bit_counter < 8) + return ((current_byte << bit_counter) & 0x80) != 0; + else if (bit_counter == 9) return true; else return true; // passive pullup @@ -321,14 +321,14 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } // I²C logic for the audio/video encoder (AVE) - i2c_state.Update(old_value[GPIO::AVE_SCL], new_value[GPIO::AVE_SCL], old_value[GPIO::AVE_SDA], - new_value[GPIO::AVE_SDA]); + i2c_state.Update(m_system, old_value[GPIO::AVE_SCL], new_value[GPIO::AVE_SCL], + old_value[GPIO::AVE_SDA], new_value[GPIO::AVE_SDA]); // SENSOR_BAR is checked by WiimoteEmu::CameraLogic // TODO: SLOT_LED } -void I2CBus::Update(bool old_scl, bool new_scl, bool old_sda, bool new_sda) +void I2CBus::Update(Core::System& system, bool old_scl, bool new_scl, bool old_sda, bool new_sda) { if (old_scl != new_scl && old_sda != new_sda) { @@ -339,154 +339,153 @@ void I2CBus::Update(bool old_scl, bool new_scl, bool old_sda, bool new_sda) if (old_scl && new_scl) { - INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", i2c_state.bit_counter, new_sda, - !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); + INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", bit_counter, new_sda, + !!Common::Flags(ReadGPIOIn(system))[GPIO::AVE_SDA]); // Check for changes to SDA while the clock is high. if (old_sda && !new_sda) { INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); // SDA falling edge (now pulled low) while SCL is high indicates I²C start - i2c_state.active = true; - i2c_state.acknowledge = false; - i2c_state.bit_counter = 0; - i2c_state.read_i2c_address = false; - i2c_state.is_correct_i2c_address = false; - // i2c_state.read_ave_address = false; + active = true; + acknowledge = false; + bit_counter = 0; + read_i2c_address = false; + is_correct_i2c_address = false; + // read_ave_address = false; } else if (!old_sda && new_sda) { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop - i2c_state.active = false; - i2c_state.bit_counter = 0; - i2c_state.read_ave_address = false; + active = false; + bit_counter = 0; + read_ave_address = false; } } else if (!old_scl && new_scl) { - INFO_LOG_FMT(WII_IPC, "{} {} {}", i2c_state.bit_counter, new_sda, - !!Common::Flags(ReadGPIOIn())[GPIO::AVE_SDA]); + INFO_LOG_FMT(WII_IPC, "{} {} {}", bit_counter, new_sda, + !!Common::Flags(ReadGPIOIn(system))[GPIO::AVE_SDA]); // Clock changed from low to high; transfer a new bit. - if (i2c_state.active && (!i2c_state.read_i2c_address || i2c_state.is_correct_i2c_address)) + if (active && (!read_i2c_address || is_correct_i2c_address)) { - if (i2c_state.bit_counter == 9) + if (bit_counter == 9) { // Note: 9 not 8, as an extra clock is spent for acknowleding - i2c_state.acknowledge = false; - if (!i2c_state.is_read) - i2c_state.current_byte = 0; - i2c_state.bit_counter = 0; + acknowledge = false; + if (!is_read) + current_byte = 0; + bit_counter = 0; } // Rising edge: a new bit - if (!(i2c_state.is_read && i2c_state.read_i2c_address) && i2c_state.bit_counter < 8) + if (!(is_read && read_i2c_address) && bit_counter < 8) { - i2c_state.current_byte <<= 1; + current_byte <<= 1; if (new_sda) - i2c_state.current_byte |= 1; + current_byte |= 1; } - if (i2c_state.bit_counter == 8) + if (bit_counter == 8) { - if (!i2c_state.is_read) + if (!is_read) { - i2c_state.acknowledge = true; - DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", i2c_state.current_byte); + acknowledge = true; + DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", current_byte); } else { WARN_LOG_FMT(WII_IPC, "AVE: Read ack: {}", new_sda ? "nack" : "ack"); if (new_sda) // nack { - i2c_state.is_read = false; // XXX + is_read = false; // XXX } } - if (!i2c_state.read_i2c_address) + if (!read_i2c_address) { - i2c_state.read_i2c_address = true; - if ((i2c_state.current_byte >> 1) == 0x70) + read_i2c_address = true; + if ((current_byte >> 1) == 0x70) { - i2c_state.is_correct_i2c_address = true; + is_correct_i2c_address = true; } else { - WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(m_system), + WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", current_byte >> 1); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); - i2c_state.acknowledge = false; - i2c_state.is_correct_i2c_address = false; + acknowledge = false; + is_correct_i2c_address = false; } - if ((i2c_state.current_byte & 1) == 0) + if ((current_byte & 1) == 0) { - i2c_state.is_read = false; + is_read = false; } else { - i2c_state.is_read = true; - i2c_state.acknowledge = true; - if (!i2c_state.read_ave_address) + is_read = true; + acknowledge = true; + if (!read_ave_address) { WARN_LOG_FMT(WII_IPC, "AVE: Read attempted without setting device address"); - i2c_state.acknowledge = false; + acknowledge = false; } else { - i2c_state.current_byte = reinterpret_cast(&ave_state)[i2c_state.current_address]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", - i2c_state.current_address, GetAVERegisterName(i2c_state.current_address), - i2c_state.current_byte); - Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + current_byte = reinterpret_cast(&ave_state)[current_address]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", current_address, + GetAVERegisterName(current_address), current_byte); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), + Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } } } - else if (!i2c_state.is_read) + else if (!is_read) { - if (!i2c_state.read_ave_address) + if (!read_ave_address) { - i2c_state.read_ave_address = true; - i2c_state.current_address = i2c_state.current_byte; - DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); + read_ave_address = true; + current_address = current_byte; + DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", current_address, + GetAVERegisterName(current_address)); } else { // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes - const u8 old_ave_value = reinterpret_cast(&ave_state)[i2c_state.current_address]; - reinterpret_cast(&ave_state)[i2c_state.current_address] = i2c_state.current_byte; - if (!ave_ever_logged[i2c_state.current_address] || - old_ave_value != i2c_state.current_byte) + const u8 old_ave_value = reinterpret_cast(&ave_state)[current_address]; + reinterpret_cast(&ave_state)[current_address] = current_byte; + if (!ave_ever_logged[current_address] || old_ave_value != current_byte) { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); - ave_ever_logged[i2c_state.current_address] = true; + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + current_address, GetAVERegisterName(current_address)); + ave_ever_logged[current_address] = true; } else { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", i2c_state.current_byte, - i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address)); + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + current_address, GetAVERegisterName(current_address)); } - i2c_state.current_address++; + current_address++; } } else // is_read is true { - i2c_state.current_address++; - i2c_state.current_byte = reinterpret_cast(&ave_state)[i2c_state.current_address]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", i2c_state.current_address, - GetAVERegisterName(i2c_state.current_address), i2c_state.current_byte); - Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + current_address++; + current_byte = reinterpret_cast(&ave_state)[current_address]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", current_address, + GetAVERegisterName(current_address), current_byte); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), + Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } } - i2c_state.bit_counter++; + bit_counter++; } } } From 3cbd4e2995db0fdd1669686042a7a3d8f36d20fc Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 27 Aug 2022 11:39:23 -0700 Subject: [PATCH 14/34] A bit more refactoring --- Source/Core/Core/HW/WII_IPC.cpp | 56 ++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index f755b00bae59..09d4fd57c68d 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -136,14 +136,15 @@ class I2CBus u8 current_byte; u8 current_address; - void Update(Core::System& system, bool old_scl, bool new_scl, bool old_sda, bool new_sda); + void Update(Core::System& system, const bool old_scl, const bool new_scl, const bool old_sda, + const bool new_sda); bool GetSCL() const; bool GetSDA() const; private: void Start(); - void Stop(); - void WriteBit(bool value); + void Stop(Core::System& system); + void WriteBit(const bool value); bool ReadBit(); }; I2CBus i2c_state; @@ -328,7 +329,29 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) // TODO: SLOT_LED } -void I2CBus::Update(Core::System& system, bool old_scl, bool new_scl, bool old_sda, bool new_sda) +void I2CBus::Start() +{ + INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); + active = true; + acknowledge = false; + bit_counter = 0; + read_i2c_address = false; + is_correct_i2c_address = false; + // read_ave_address = false; +} + +void I2CBus::Stop(Core::System& system) +{ + INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), Common::Log::LogType::WII_IPC, + Common::Log::LogLevel::LINFO); + active = false; + bit_counter = 0; + read_ave_address = false; +} + +void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl, + const bool old_sda, const bool new_sda) { if (old_scl != new_scl && old_sda != new_sda) { @@ -337,31 +360,26 @@ void I2CBus::Update(Core::System& system, bool old_scl, bool new_scl, bool old_s return; } + if (old_scl == new_scl && old_sda == new_sda) + return; // Nothing changed + + if (!new_scl) + { + // We only care about things that happen when SCL changes to high or is already high + } + if (old_scl && new_scl) { - INFO_LOG_FMT(WII_IPC, "SCL {} {} {}", bit_counter, new_sda, - !!Common::Flags(ReadGPIOIn(system))[GPIO::AVE_SDA]); // Check for changes to SDA while the clock is high. if (old_sda && !new_sda) { - INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); // SDA falling edge (now pulled low) while SCL is high indicates I²C start - active = true; - acknowledge = false; - bit_counter = 0; - read_i2c_address = false; - is_correct_i2c_address = false; - // read_ave_address = false; + Start(); } else if (!old_sda && new_sda) { - INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop - active = false; - bit_counter = 0; - read_ave_address = false; + Stop(system); } } else if (!old_scl && new_scl) From 7bb449dc2da3e1c0cf76fdc01581ac721a9b339f Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 27 Aug 2022 14:46:04 -0700 Subject: [PATCH 15/34] Temp --- Source/Core/Core/HW/WII_IPC.cpp | 62 ++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 09d4fd57c68d..fb0e3d03283e 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -123,18 +123,29 @@ static_assert(sizeof(AVEState) == 0x100); static AVEState ave_state; static std::bitset ave_ever_logged; // For logging only; not saved +// An I²C bus implementation accessed via bit-banging. +// A few assumptions and limitations exist: +// - All devices support both writes and reads. +// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. +// - Reads are performed by writing a 1-byte address, restarting into read mode, and then reading an +// arbitrary number of bytes. Immediately reading without writing an address is not allowed. Writing +// multiple bytes and then switching into read mode uses the first written byte as the address, and +// the subsequent written bytes auto-increment the address, with that incremented address used for +// reads afterwards. (This is implicit as we don't track how many bytes are written, only if an +// address _has_ been written). Each write must re-specify the address. +// - The device address is handled by this I2CBus class, instead of the device itself. +// - Timing is not implemented at all; the clock signal can be changed as fast as needed. +// - Devices are not allowed to stretch the clock signal. (Nintendo's write code does not seem to +// implement clock stretching in any case, though some homebrew does.) class I2CBus { public: bool active; u8 bit_counter; - bool read_i2c_address; - bool is_correct_i2c_address; - bool is_read; - bool read_ave_address; - bool acknowledge; u8 current_byte; - u8 current_address; + std::optional i2c_address; // Not shifted; includes the read flag + std::optional device_address; + bool acknowledge; void Update(Core::System& system, const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda); @@ -143,9 +154,7 @@ class I2CBus private: void Start(); - void Stop(Core::System& system); - void WriteBit(const bool value); - bool ReadBit(); + void Stop(); }; I2CBus i2c_state; @@ -331,23 +340,17 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) void I2CBus::Start() { - INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); + if (active) + INFO_LOG_FMT(WII_IPC, "AVE: Re-start I2C"); + else + INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); active = true; - acknowledge = false; - bit_counter = 0; - read_i2c_address = false; - is_correct_i2c_address = false; - // read_ave_address = false; } -void I2CBus::Stop(Core::System& system) +void I2CBus::Stop() { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); active = false; - bit_counter = 0; - read_ave_address = false; } void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl, @@ -363,11 +366,6 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl if (old_scl == new_scl && old_sda == new_sda) return; // Nothing changed - if (!new_scl) - { - // We only care about things that happen when SCL changes to high or is already high - } - if (old_scl && new_scl) { // Check for changes to SDA while the clock is high. @@ -379,14 +377,12 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl else if (!old_sda && new_sda) { // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop - Stop(system); + Stop(); } } else if (!old_scl && new_scl) { - INFO_LOG_FMT(WII_IPC, "{} {} {}", bit_counter, new_sda, - !!Common::Flags(ReadGPIOIn(system))[GPIO::AVE_SDA]); - // Clock changed from low to high; transfer a new bit. + // SCL rising edge indicates data clocking. For writes, we transfer in a bit. if (active && (!read_i2c_address || is_correct_i2c_address)) { if (bit_counter == 9) @@ -506,6 +502,14 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl bit_counter++; } } + else if (old_scl && !new_scl) + { + // SCL falling edge is used to advance bit_counter. + if (active) + { + bit_counter++; + } + } } void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) From 81b15a17dee04a79b7ecc7969d1d216950061c57 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 27 Aug 2022 14:51:51 -0700 Subject: [PATCH 16/34] Adjust comment --- Source/Core/Core/HW/WII_IPC.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index fb0e3d03283e..8a6af05340cb 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -126,17 +126,16 @@ static std::bitset ave_ever_logged; // For logging only; not // An I²C bus implementation accessed via bit-banging. // A few assumptions and limitations exist: // - All devices support both writes and reads. -// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. -// - Reads are performed by writing a 1-byte address, restarting into read mode, and then reading an -// arbitrary number of bytes. Immediately reading without writing an address is not allowed. Writing -// multiple bytes and then switching into read mode uses the first written byte as the address, and -// the subsequent written bytes auto-increment the address, with that incremented address used for -// reads afterwards. (This is implicit as we don't track how many bytes are written, only if an -// address _has_ been written). Each write must re-specify the address. -// - The device address is handled by this I2CBus class, instead of the device itself. // - Timing is not implemented at all; the clock signal can be changed as fast as needed. // - Devices are not allowed to stretch the clock signal. (Nintendo's write code does not seem to // implement clock stretching in any case, though some homebrew does.) +// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. +// - The device address is handled by this I2CBus class, instead of the device itself. +// - The device address is set on writes, and re-used for reads; writing an address and data and +// then switching to reading uses the incremented address. Every write must specify the address. +// - Reading without setting the device address beforehand is disallowed; this mode is hypothetically +// allowed by the I²C specification but does not seem to be used in practice here. +// - 10-bit addressing and other reserved addressing modes are not implemented. class I2CBus { public: From 91574e033123b9d343f06bb697177f859acbd98b Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 27 Aug 2022 14:56:15 -0700 Subject: [PATCH 17/34] Adjust comment further --- Source/Core/Core/HW/WII_IPC.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 8a6af05340cb..6de829326ed9 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -133,8 +133,13 @@ static std::bitset ave_ever_logged; // For logging only; not // - The device address is handled by this I2CBus class, instead of the device itself. // - The device address is set on writes, and re-used for reads; writing an address and data and // then switching to reading uses the incremented address. Every write must specify the address. -// - Reading without setting the device address beforehand is disallowed; this mode is hypothetically -// allowed by the I²C specification but does not seem to be used in practice here. +// - Reading without setting the device address beforehand is disallowed; the I²C specification +// allows such reads but does not specify how they behave (or anything about the behavior of the +// device address). +// - Switching between multiple devices using a restart does not reset the device address; the +// device address is only reset on stopping. This means that a write to one device followed by a +// read from a different device would result in reading from the last used device address (without +// any warning). // - 10-bit addressing and other reserved addressing modes are not implemented. class I2CBus { From a45f21e422e7aa733b96ad0778447a8a886f2cf9 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 28 Aug 2022 12:03:27 -0700 Subject: [PATCH 18/34] Still a WIP, but writes again work --- Source/Core/Core/HW/WII_IPC.cpp | 235 ++++++++++++++++---------------- 1 file changed, 117 insertions(+), 118 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 6de829326ed9..e3a894a932b8 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -159,6 +159,7 @@ class I2CBus private: void Start(); void Stop(); + bool WriteExpected() const; }; I2CBus i2c_state; @@ -294,22 +295,22 @@ bool I2CBus::GetSCL() const bool I2CBus::GetSDA() const { - if (bit_counter == 9 && acknowledge) + if (!active || WriteExpected()) { - return false; // pull low + return true; // passive pullup } - else if (is_read) + else { - if (bit_counter < 8) - return ((current_byte << bit_counter) & 0x80) != 0; - else if (bit_counter == 9) - return true; + if (bit_counter == 8) + { + // Acknowledge bit for a write (implied by !WriteExpected()) + return acknowledge; + } else - return true; // passive pullup - } - else // write - { - return true; // passive pullup + { + // Part of a read (implied by !WriteExpected()) + return ((current_byte << bit_counter) & 0x80) != 0; + } } } @@ -348,13 +349,39 @@ void I2CBus::Start() INFO_LOG_FMT(WII_IPC, "AVE: Re-start I2C"); else INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); + + if (bit_counter != 0) + WARN_LOG_FMT(WII_IPC, "I2C: Start happened with a nonzero bit counter: {}", bit_counter); + active = true; + bit_counter = 9; + current_byte = 0; + i2c_address.reset(); + // Note: don't reset device_address, as it's re-used for reads + acknowledge = false; } void I2CBus::Stop() { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); active = false; + bit_counter = 0; + current_byte = 0; + i2c_address.reset(); + device_address.reset(); + acknowledge = false; +} + +bool I2CBus::WriteExpected() const +{ + // If we don't have an I²C address, it needs to be written (even if the address that is later + // written is a read). + // Otherwise, check the least significant bit; it being *clear* indicates a write. + const bool is_write = !i2c_address.has_value() || ((i2c_address.value() & 1) == 0); + // The device that is otherwise recieving instead transmits an acknowledge bit after each byte. + const bool acknowledge_expected = (bit_counter == 8); + + return is_write ^ acknowledge_expected; } void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl, @@ -384,134 +411,106 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl Stop(); } } - else if (!old_scl && new_scl) + else if (active) { - // SCL rising edge indicates data clocking. For writes, we transfer in a bit. - if (active && (!read_i2c_address || is_correct_i2c_address)) + if (!old_scl && new_scl) { - if (bit_counter == 9) - { - // Note: 9 not 8, as an extra clock is spent for acknowleding - acknowledge = false; - if (!is_read) - current_byte = 0; - bit_counter = 0; - } - - // Rising edge: a new bit - if (!(is_read && read_i2c_address) && bit_counter < 8) - { - current_byte <<= 1; - if (new_sda) - current_byte |= 1; - } - - if (bit_counter == 8) + // INFO_LOG_FMT(WII_IPC, "AVE: {} rising edge: {} (write expected: {})", bit_counter, new_sda, + // WriteExpected()); + // SCL rising edge indicates data clocking. For reads, we set up data at this point. + // For writes, we instead process it on the falling edge, to better distinguish + // the start/stop condition. + if (bit_counter == 0 && !WriteExpected()) { - if (!is_read) + // Start of a read. + if (device_address.has_value()) { - acknowledge = true; - DEBUG_LOG_FMT(WII_IPC, "AVE: New byte: {:02x}", current_byte); + current_byte = reinterpret_cast(&ave_state)[device_address.value()]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), + GetAVERegisterName(device_address.value()), current_byte); } else { - WARN_LOG_FMT(WII_IPC, "AVE: Read ack: {}", new_sda ? "nack" : "ack"); - if (new_sda) // nack - { - is_read = false; // XXX - } + ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); + acknowledge = false; } - - if (!read_i2c_address) + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + // Common::Log::LogLevel::LINFO); + } + else if (old_scl && !new_scl) + { + // INFO_LOG_FMT(WII_IPC, "AVE: {} falling edge: {} (write expected: {})", bit_counter, + // new_sda, + // WriteExpected()); + // SCL falling edge is used to advance bit_counter and process wri'tes. + if (bit_counter != 9 && WriteExpected()) + { + if (bit_counter == 8) { - read_i2c_address = true; - if ((current_byte >> 1) == 0x70) - { - is_correct_i2c_address = true; - } - else - { - WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", current_byte >> 1); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), - Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); - acknowledge = false; - is_correct_i2c_address = false; - } + // Acknowledge bit for *reads*. + if (new_sda) + WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + } + else + { + current_byte <<= 1; + if (new_sda) + current_byte |= 1; - if ((current_byte & 1) == 0) + if (bit_counter == 7) { - is_read = false; - } - else - { - is_read = true; - acknowledge = true; - if (!read_ave_address) + INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); + // Write finished. + if (!i2c_address.has_value()) { - WARN_LOG_FMT(WII_IPC, "AVE: Read attempted without setting device address"); - acknowledge = false; + i2c_address = current_byte; + if ((current_byte & 1) == 0) + { + // Write, which always specifies the device address + device_address.reset(); + } + // TODO: Reads should still have the accessed device write the ACK + INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); } - else + else if (!device_address.has_value()) { - current_byte = reinterpret_cast(&ave_state)[current_address]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", current_address, - GetAVERegisterName(current_address), current_byte); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), - Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); - } - } - } - else if (!is_read) - { - if (!read_ave_address) - { - read_ave_address = true; - current_address = current_byte; - DEBUG_LOG_FMT(WII_IPC, "AVE address: {:02x} ({})", current_address, - GetAVERegisterName(current_address)); - } - else - { - // This is always inbounds, as we're indexing with a u8 and the struct is 0x100 bytes - const u8 old_ave_value = reinterpret_cast(&ave_state)[current_address]; - reinterpret_cast(&ave_state)[current_address] = current_byte; - if (!ave_ever_logged[current_address] || old_ave_value != current_byte) - { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - current_address, GetAVERegisterName(current_address)); - ave_ever_logged[current_address] = true; + device_address = current_byte; + INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); } else { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - current_address, GetAVERegisterName(current_address)); + // Actual write + const u8 old_ave_value = reinterpret_cast(&ave_state)[device_address.value()]; + reinterpret_cast(&ave_state)[device_address.value()] = current_byte; + if (!ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), GetAVERegisterName(device_address.value())); + ave_ever_logged[device_address.value()] = true; + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), GetAVERegisterName(device_address.value())); + } + device_address = device_address.value() + 1; } - current_address++; } } - else // is_read is true - { - current_address++; - current_byte = reinterpret_cast(&ave_state)[current_address]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", current_address, - GetAVERegisterName(current_address), current_byte); - Dolphin_Debugger::PrintCallstack(Core::CPUThreadGuard(system), - Common::Log::LogType::WII_IPC, - Common::Log::LogLevel::LINFO); - } } - bit_counter++; - } - } - else if (old_scl && !new_scl) - { - // SCL falling edge is used to advance bit_counter. - if (active) - { - bit_counter++; + if (bit_counter >= 8) + { + // Finished a byte and the acknowledge signal. + bit_counter = 0; + } + else + { + bit_counter++; + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, + // Common::Log::LogLevel::LINFO); } } } From 74f1ed4c4be36532108998ecd84741301290f770 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 28 Aug 2022 12:13:11 -0700 Subject: [PATCH 19/34] Even more refactoring --- Source/Core/Core/HW/WII_IPC.cpp | 177 +++++++++++++++++--------------- 1 file changed, 93 insertions(+), 84 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index e3a894a932b8..7672657a8852 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -159,6 +159,8 @@ class I2CBus private: void Start(); void Stop(); + void SCLRisingEdge(const bool sda); + void SCLFallingEdge(const bool sda); bool WriteExpected() const; }; I2CBus i2c_state; @@ -415,104 +417,111 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl { if (!old_scl && new_scl) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} rising edge: {} (write expected: {})", bit_counter, new_sda, - // WriteExpected()); - // SCL rising edge indicates data clocking. For reads, we set up data at this point. - // For writes, we instead process it on the falling edge, to better distinguish - // the start/stop condition. - if (bit_counter == 0 && !WriteExpected()) - { - // Start of a read. - if (device_address.has_value()) - { - current_byte = reinterpret_cast(&ave_state)[device_address.value()]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), - GetAVERegisterName(device_address.value()), current_byte); - } - else - { - ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); - acknowledge = false; - } - } - // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, - // Common::Log::LogLevel::LINFO); + SCLRisingEdge(new_sda); } else if (old_scl && !new_scl) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} falling edge: {} (write expected: {})", bit_counter, - // new_sda, - // WriteExpected()); - // SCL falling edge is used to advance bit_counter and process wri'tes. - if (bit_counter != 9 && WriteExpected()) + SCLFallingEdge(new_sda); + } + } +} + +void I2CBus::SCLRisingEdge(const bool sda) +{ + // INFO_LOG_FMT(WII_IPC, "AVE: {} rising edge: {} (write expected: {})", bit_counter, new_sda, + // WriteExpected()); + // SCL rising edge indicates data clocking. For reads, we set up data at this point. + // For writes, we instead process it on the falling edge, to better distinguish + // the start/stop condition. + if (bit_counter == 0 && !WriteExpected()) + { + // Start of a read. + if (device_address.has_value()) + { + current_byte = reinterpret_cast(&ave_state)[device_address.value()]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), + GetAVERegisterName(device_address.value()), current_byte); + } + else + { + ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); + acknowledge = false; + } + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); +} + +void I2CBus::SCLFallingEdge(const bool sda) +{ + // INFO_LOG_FMT(WII_IPC, "AVE: {} falling edge: {} (write expected: {})", bit_counter, new_sda, + // WriteExpected()); + // SCL falling edge is used to advance bit_counter and process wri'tes. + if (bit_counter != 9 && WriteExpected()) + { + if (bit_counter == 8) + { + // Acknowledge bit for *reads*. + if (sda) + WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + } + else + { + current_byte <<= 1; + if (sda) + current_byte |= 1; + + if (bit_counter == 7) { - if (bit_counter == 8) + INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); + // Write finished. + if (!i2c_address.has_value()) + { + i2c_address = current_byte; + if ((current_byte & 1) == 0) + { + // Write, which always specifies the device address + device_address.reset(); + } + // TODO: Reads should still have the accessed device write the ACK + INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); + } + else if (!device_address.has_value()) { - // Acknowledge bit for *reads*. - if (new_sda) - WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + device_address = current_byte; + INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); } else { - current_byte <<= 1; - if (new_sda) - current_byte |= 1; - - if (bit_counter == 7) + // Actual write + const u8 old_ave_value = reinterpret_cast(&ave_state)[device_address.value()]; + reinterpret_cast(&ave_state)[device_address.value()] = current_byte; + if (!ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), GetAVERegisterName(device_address.value())); + ave_ever_logged[device_address.value()] = true; + } + else { - INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); - // Write finished. - if (!i2c_address.has_value()) - { - i2c_address = current_byte; - if ((current_byte & 1) == 0) - { - // Write, which always specifies the device address - device_address.reset(); - } - // TODO: Reads should still have the accessed device write the ACK - INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); - } - else if (!device_address.has_value()) - { - device_address = current_byte; - INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); - } - else - { - // Actual write - const u8 old_ave_value = reinterpret_cast(&ave_state)[device_address.value()]; - reinterpret_cast(&ave_state)[device_address.value()] = current_byte; - if (!ave_ever_logged[device_address.value()] || old_ave_value != current_byte) - { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), GetAVERegisterName(device_address.value())); - ave_ever_logged[device_address.value()] = true; - } - else - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), GetAVERegisterName(device_address.value())); - } - device_address = device_address.value() + 1; - } + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), GetAVERegisterName(device_address.value())); } + device_address = device_address.value() + 1; } } - - if (bit_counter >= 8) - { - // Finished a byte and the acknowledge signal. - bit_counter = 0; - } - else - { - bit_counter++; - } - // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, - // Common::Log::LogLevel::LINFO); } } + + if (bit_counter >= 8) + { + // Finished a byte and the acknowledge signal. + bit_counter = 0; + } + else + { + bit_counter++; + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) From 1ef7d90b68472801bc40a58741343b3b5aea64f3 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 28 Aug 2022 21:39:04 -0700 Subject: [PATCH 20/34] Full implementation of reads and writes --- Source/Core/Core/HW/WII_IPC.cpp | 163 +++++++++++++++++++++----------- 1 file changed, 110 insertions(+), 53 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 7672657a8852..95a6880dda74 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -9,6 +9,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/EnumFormatter.h" #include "Common/Logging/Log.h" #include "Core/Core.h" #include "Core/CoreTiming.h" @@ -144,12 +145,21 @@ static std::bitset ave_ever_logged; // For logging only; not class I2CBus { public: - bool active; + enum class State + { + Inactive, + Activating, + SetI2CAddress, + WriteDeviceAddress, + WriteToDevice, + ReadFromDevice, + }; + + State state; u8 bit_counter; u8 current_byte; std::optional i2c_address; // Not shifted; includes the read flag std::optional device_address; - bool acknowledge; void Update(Core::System& system, const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda); @@ -216,7 +226,7 @@ void WiiIPC::InitState() i2c_state = {}; ave_state = {}; - ave_state.video_output_config = 0x55; + ave_state.video_output_config = 0x23; ave_ever_logged.reset(); } @@ -297,22 +307,26 @@ bool I2CBus::GetSCL() const bool I2CBus::GetSDA() const { - if (!active || WriteExpected()) - { - return true; // passive pullup - } - else + switch (state) { - if (bit_counter == 8) - { - // Acknowledge bit for a write (implied by !WriteExpected()) - return acknowledge; - } + case State::Inactive: + case State::Activating: + default: + return true; // passive pullup (or NACK) + + case State::SetI2CAddress: + case State::WriteDeviceAddress: + case State::WriteToDevice: + if (bit_counter < 8) + return true; // passive pullup else - { - // Part of a read (implied by !WriteExpected()) + return false; // ACK (if we need to NACK, we set the state to inactive) + + case State::ReadFromDevice: + if (bit_counter < 8) return ((current_byte << bit_counter) & 0x80) != 0; - } + else + return true; // passive pullup, receiver needs to ACK or NACK } } @@ -347,7 +361,7 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) void I2CBus::Start() { - if (active) + if (state != State::Inactive) INFO_LOG_FMT(WII_IPC, "AVE: Re-start I2C"); else INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); @@ -355,23 +369,21 @@ void I2CBus::Start() if (bit_counter != 0) WARN_LOG_FMT(WII_IPC, "I2C: Start happened with a nonzero bit counter: {}", bit_counter); - active = true; - bit_counter = 9; + state = State::Activating; + bit_counter = 0; current_byte = 0; i2c_address.reset(); // Note: don't reset device_address, as it's re-used for reads - acknowledge = false; } void I2CBus::Stop() { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - active = false; + state = State::Inactive; bit_counter = 0; current_byte = 0; i2c_address.reset(); device_address.reset(); - acknowledge = false; } bool I2CBus::WriteExpected() const @@ -413,7 +425,7 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl Stop(); } } - else if (active) + else if (state != State::Inactive) { if (!old_scl && new_scl) { @@ -428,41 +440,38 @@ void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl void I2CBus::SCLRisingEdge(const bool sda) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} rising edge: {} (write expected: {})", bit_counter, new_sda, - // WriteExpected()); + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} rising edge: {} (write expected: {})", bit_counter, state, + // sda, WriteExpected()); // SCL rising edge indicates data clocking. For reads, we set up data at this point. // For writes, we instead process it on the falling edge, to better distinguish // the start/stop condition. - if (bit_counter == 0 && !WriteExpected()) + if (state == State::ReadFromDevice && bit_counter == 0) { // Start of a read. - if (device_address.has_value()) - { - current_byte = reinterpret_cast(&ave_state)[device_address.value()]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), - GetAVERegisterName(device_address.value()), current_byte); - } - else - { - ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); - acknowledge = false; - } + ASSERT(device_address.has_value()); // Implied by the state transition in falling edge + current_byte = reinterpret_cast(&ave_state)[device_address.value()]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), + GetAVERegisterName(device_address.value()), current_byte); } // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } void I2CBus::SCLFallingEdge(const bool sda) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} falling edge: {} (write expected: {})", bit_counter, new_sda, - // WriteExpected()); - // SCL falling edge is used to advance bit_counter and process wri'tes. - if (bit_counter != 9 && WriteExpected()) + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} falling edge: {} (write expected: {})", bit_counter, state, + // sda, WriteExpected()); + // SCL falling edge is used to advance bit_counter/change states and process writes. + if (state == State::SetI2CAddress || state == State::WriteDeviceAddress || + state == State::WriteToDevice) { if (bit_counter == 8) { // Acknowledge bit for *reads*. if (sda) + { WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + state = State::Inactive; + } } else { @@ -474,18 +483,19 @@ void I2CBus::SCLFallingEdge(const bool sda) { INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); // Write finished. - if (!i2c_address.has_value()) + if (state == State::SetI2CAddress) { - i2c_address = current_byte; - if ((current_byte & 1) == 0) + if ((current_byte >> 1) != 0x70) { - // Write, which always specifies the device address - device_address.reset(); + state = State::Inactive; // NACK + WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); + } + else + { + INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); } - // TODO: Reads should still have the accessed device write the ACK - INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); } - else if (!device_address.has_value()) + else if (state == State::WriteDeviceAddress) { device_address = current_byte; INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); @@ -493,6 +503,8 @@ void I2CBus::SCLFallingEdge(const bool sda) else { // Actual write + ASSERT(state == State::WriteToDevice); + ASSERT(device_address.has_value()); // implied by state transition const u8 old_ave_value = reinterpret_cast(&ave_state)[device_address.value()]; reinterpret_cast(&ave_state)[device_address.value()] = current_byte; if (!ave_ever_logged[device_address.value()] || old_ave_value != current_byte) @@ -512,14 +524,50 @@ void I2CBus::SCLFallingEdge(const bool sda) } } - if (bit_counter >= 8) + if (state == State::Activating) { - // Finished a byte and the acknowledge signal. + // This is triggered after the start condition. + state = State::SetI2CAddress; bit_counter = 0; } - else + else if (state != State::Inactive) { - bit_counter++; + if (bit_counter >= 8) + { + // Finished a byte and the acknowledge signal. + bit_counter = 0; + switch (state) + { + case State::SetI2CAddress: + i2c_address = current_byte; + // Note: i2c_address is known to correspond to a valid device + if ((current_byte & 1) == 0) + { + state = State::WriteDeviceAddress; + device_address.reset(); + } + else + { + if (device_address.has_value()) + { + state = State::ReadFromDevice; + } + else + { + state = State::Inactive; // NACK - required for 8-bit internal addresses + ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); + } + } + break; + case State::WriteDeviceAddress: + state = State::WriteToDevice; + break; + } + } + else + { + bit_counter++; + } } // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } @@ -698,3 +746,12 @@ bool WiiIPC::IsReady() const ((m_ppc_irq_flags & INT_CAUSE_IPC_BROADWAY) == 0)); } } // namespace IOS + +template <> +struct fmt::formatter : EnumFormatter +{ + static constexpr array_type names = {"Inactive", "Activating", + "Set I2C Address", "Write Device Address", + "Write To Device", "Read From Device"}; + constexpr formatter() : EnumFormatter(names) {} +}; From 5f9648417cd3c1debaa9cf1f27fc61d05a253818 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 30 Aug 2022 12:42:45 -0700 Subject: [PATCH 21/34] Create Common/I2C Currently this is a copy of the Wii Remote I2CBus, and is unused. That will change in later commits. --- Source/Core/Common/CMakeLists.txt | 2 + Source/Core/Common/I2C.cpp | 53 ++++++++++++++++++++++ Source/Core/Common/I2C.h | 75 +++++++++++++++++++++++++++++++ Source/Core/DolphinLib.props | 2 + 4 files changed, 132 insertions(+) create mode 100644 Source/Core/Common/I2C.cpp create mode 100644 Source/Core/Common/I2C.h diff --git a/Source/Core/Common/CMakeLists.txt b/Source/Core/Common/CMakeLists.txt index 81431e564cef..048fffe2d107 100644 --- a/Source/Core/Common/CMakeLists.txt +++ b/Source/Core/Common/CMakeLists.txt @@ -79,6 +79,8 @@ add_library(common HostDisassembler.h HttpRequest.cpp HttpRequest.h + I2C.cpp + I2C.h Image.cpp Image.h IniFile.cpp diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp new file mode 100644 index 000000000000..d44f98dffbf6 --- /dev/null +++ b/Source/Core/Common/I2C.cpp @@ -0,0 +1,53 @@ +// Copyright 2022 Dolphin Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#include "Common/I2C.h" + +#include + +namespace Common +{ +void I2CBus::AddSlave(I2CSlave* slave) +{ + m_slaves.emplace_back(slave); +} + +void I2CBus::RemoveSlave(I2CSlave* slave) +{ + m_slaves.erase(std::remove(m_slaves.begin(), m_slaves.end(), slave), m_slaves.end()); +} + +void I2CBus::Reset() +{ + m_slaves.clear(); +} + +int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +{ + for (auto& slave : m_slaves) + { + auto const bytes_read = slave->BusRead(slave_addr, addr, count, data_out); + + // A slave responded, we are done. + if (bytes_read) + return bytes_read; + } + + return 0; +} + +int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +{ + for (auto& slave : m_slaves) + { + auto const bytes_written = slave->BusWrite(slave_addr, addr, count, data_in); + + // A slave responded, we are done. + if (bytes_written) + return bytes_written; + } + + return 0; +} + +}; // namespace Common diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h new file mode 100644 index 000000000000..1ea1c8198da1 --- /dev/null +++ b/Source/Core/Common/I2C.h @@ -0,0 +1,75 @@ +// Copyright 2022 Dolphin Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once + +#include +#include + +#include "Common/CommonTypes.h" + +namespace Common +{ +class I2CBus; + +class I2CSlave +{ + friend I2CBus; + +protected: + virtual ~I2CSlave() = default; + + virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0; + virtual int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) = 0; + + template + static int RawRead(T* reg_data, u8 addr, int count, u8* data_out) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + // TODO: addr wraps around after 0xff + + u8* src = reinterpret_cast(reg_data) + addr; + count = std::min(count, int(reinterpret_cast(reg_data + 1) - src)); + + std::copy_n(src, count, data_out); + + return count; + } + + template + static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + // TODO: addr wraps around after 0xff + + u8* dst = reinterpret_cast(reg_data) + addr; + count = std::min(count, int(reinterpret_cast(reg_data + 1) - dst)); + + std::copy_n(data_in, count, dst); + + return count; + } +}; + +class I2CBus +{ +public: + void AddSlave(I2CSlave* slave); + void RemoveSlave(I2CSlave* slave); + + void Reset(); + + // TODO: change int to u16 or something + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); + +private: + // Pointers are unowned: + std::vector m_slaves; +}; + +} // namespace Common diff --git a/Source/Core/DolphinLib.props b/Source/Core/DolphinLib.props index a61f0d822ac1..b6967694eeef 100644 --- a/Source/Core/DolphinLib.props +++ b/Source/Core/DolphinLib.props @@ -120,6 +120,7 @@ + @@ -812,6 +813,7 @@ + From 669cff6571f49be9e0567b8332a9e5fd60fe65d1 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 3 Sep 2022 16:21:23 -0700 Subject: [PATCH 22/34] Move I2CBus logic into I2C.cpp (without refactoring for devices) --- Source/Core/Common/I2C.cpp | 279 +++++++++++++++++++++++++++- Source/Core/Common/I2C.h | 56 +++++- Source/Core/Core/HW/WII_IPC.cpp | 316 +------------------------------- 3 files changed, 334 insertions(+), 317 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index d44f98dffbf6..843110e2fb09 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -4,25 +4,43 @@ #include "Common/I2C.h" #include +#include +#include + +#include + +#include "Common/Assert.h" +#include "Common/EnumFormatter.h" +#include "Common/Logging/Log.h" +#include "Core/Debugger/Debugger_SymbolMap.h" + +namespace IOS +{ +struct AVEState; +extern AVEState ave_state; +extern std::bitset<0x100> ave_ever_logged; // For logging only; not saved + +std::string_view GetAVERegisterName(u8 address); +} // namespace IOS namespace Common { -void I2CBus::AddSlave(I2CSlave* slave) +void I2CBusOld::AddSlave(I2CSlave* slave) { m_slaves.emplace_back(slave); } -void I2CBus::RemoveSlave(I2CSlave* slave) +void I2CBusOld::RemoveSlave(I2CSlave* slave) { m_slaves.erase(std::remove(m_slaves.begin(), m_slaves.end(), slave), m_slaves.end()); } -void I2CBus::Reset() +void I2CBusOld::Reset() { m_slaves.clear(); } -int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +int I2CBusOld::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { for (auto& slave : m_slaves) { @@ -36,7 +54,7 @@ int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) return 0; } -int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +int I2CBusOld::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { for (auto& slave : m_slaves) { @@ -50,4 +68,255 @@ int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) return 0; } +bool I2CBus::GetSCL() const +{ + return true; // passive pullup - no clock stretching +} + +bool I2CBus::GetSDA() const +{ + switch (state) + { + case State::Inactive: + case State::Activating: + default: + return true; // passive pullup (or NACK) + + case State::SetI2CAddress: + case State::WriteDeviceAddress: + case State::WriteToDevice: + if (bit_counter < 8) + return true; // passive pullup + else + return false; // ACK (if we need to NACK, we set the state to inactive) + + case State::ReadFromDevice: + if (bit_counter < 8) + return ((current_byte << bit_counter) & 0x80) != 0; + else + return true; // passive pullup, receiver needs to ACK or NACK + } +} + +void I2CBus::Start() +{ + if (state != State::Inactive) + INFO_LOG_FMT(WII_IPC, "AVE: Re-start I2C"); + else + INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); + + if (bit_counter != 0) + WARN_LOG_FMT(WII_IPC, "I2C: Start happened with a nonzero bit counter: {}", bit_counter); + + state = State::Activating; + bit_counter = 0; + current_byte = 0; + i2c_address.reset(); + // Note: don't reset device_address, as it's re-used for reads +} + +void I2CBus::Stop() +{ + INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + state = State::Inactive; + bit_counter = 0; + current_byte = 0; + i2c_address.reset(); + device_address.reset(); +} + +bool I2CBus::WriteExpected() const +{ + // If we don't have an I²C address, it needs to be written (even if the address that is later + // written is a read). + // Otherwise, check the least significant bit; it being *clear* indicates a write. + const bool is_write = !i2c_address.has_value() || ((i2c_address.value() & 1) == 0); + // The device that is otherwise recieving instead transmits an acknowledge bit after each byte. + const bool acknowledge_expected = (bit_counter == 8); + + return is_write ^ acknowledge_expected; +} + +void I2CBus::Update(const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda) +{ + if (old_scl != new_scl && old_sda != new_sda) + { + ERROR_LOG_FMT(WII_IPC, "Both SCL and SDA changed at the same time: SCL {} -> {} SDA {} -> {}", + old_scl, new_scl, old_sda, new_sda); + return; + } + + if (old_scl == new_scl && old_sda == new_sda) + return; // Nothing changed + + if (old_scl && new_scl) + { + // Check for changes to SDA while the clock is high. + if (old_sda && !new_sda) + { + // SDA falling edge (now pulled low) while SCL is high indicates I²C start + Start(); + } + else if (!old_sda && new_sda) + { + // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop + Stop(); + } + } + else if (state != State::Inactive) + { + if (!old_scl && new_scl) + { + SCLRisingEdge(new_sda); + } + else if (old_scl && !new_scl) + { + SCLFallingEdge(new_sda); + } + } +} + +void I2CBus::SCLRisingEdge(const bool sda) +{ + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} rising edge: {} (write expected: {})", bit_counter, state, + // sda, WriteExpected()); + // SCL rising edge indicates data clocking. For reads, we set up data at this point. + // For writes, we instead process it on the falling edge, to better distinguish + // the start/stop condition. + if (state == State::ReadFromDevice && bit_counter == 0) + { + // Start of a read. + ASSERT(device_address.has_value()); // Implied by the state transition in falling edge + current_byte = reinterpret_cast(&IOS::ave_state)[device_address.value()]; + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), + IOS::GetAVERegisterName(device_address.value()), current_byte); + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); +} + +void I2CBus::SCLFallingEdge(const bool sda) +{ + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} falling edge: {} (write expected: {})", bit_counter, state, + // sda, WriteExpected()); + // SCL falling edge is used to advance bit_counter/change states and process writes. + if (state == State::SetI2CAddress || state == State::WriteDeviceAddress || + state == State::WriteToDevice) + { + if (bit_counter == 8) + { + // Acknowledge bit for *reads*. + if (sda) + { + WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + state = State::Inactive; + } + } + else + { + current_byte <<= 1; + if (sda) + current_byte |= 1; + + if (bit_counter == 7) + { + INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); + // Write finished. + if (state == State::SetI2CAddress) + { + if ((current_byte >> 1) != 0x70) + { + state = State::Inactive; // NACK + WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); + } + else + { + INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); + } + } + else if (state == State::WriteDeviceAddress) + { + device_address = current_byte; + INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); + } + else + { + // Actual write + ASSERT(state == State::WriteToDevice); + ASSERT(device_address.has_value()); // implied by state transition + const u8 old_ave_value = reinterpret_cast(&IOS::ave_state)[device_address.value()]; + reinterpret_cast(&IOS::ave_state)[device_address.value()] = current_byte; + if (!IOS::ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), IOS::GetAVERegisterName(device_address.value())); + IOS::ave_ever_logged[device_address.value()] = true; + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), IOS::GetAVERegisterName(device_address.value())); + } + device_address = device_address.value() + 1; + } + } + } + } + + if (state == State::Activating) + { + // This is triggered after the start condition. + state = State::SetI2CAddress; + bit_counter = 0; + } + else if (state != State::Inactive) + { + if (bit_counter >= 8) + { + // Finished a byte and the acknowledge signal. + bit_counter = 0; + switch (state) + { + case State::SetI2CAddress: + i2c_address = current_byte; + // Note: i2c_address is known to correspond to a valid device + if ((current_byte & 1) == 0) + { + state = State::WriteDeviceAddress; + device_address.reset(); + } + else + { + if (device_address.has_value()) + { + state = State::ReadFromDevice; + } + else + { + state = State::Inactive; // NACK - required for 8-bit internal addresses + ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); + } + } + break; + case State::WriteDeviceAddress: + state = State::WriteToDevice; + break; + } + } + else + { + bit_counter++; + } + } + // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); +} + }; // namespace Common + +template <> +struct fmt::formatter : EnumFormatter +{ + static constexpr array_type names = {"Inactive", "Activating", + "Set I2C Address", "Write Device Address", + "Write To Device", "Read From Device"}; + constexpr formatter() : EnumFormatter(names) {} +}; diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index 1ea1c8198da1..fcccb4f322a4 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -4,17 +4,18 @@ #pragma once #include +#include #include #include "Common/CommonTypes.h" namespace Common { -class I2CBus; +class I2CBusOld; class I2CSlave { - friend I2CBus; + friend I2CBusOld; protected: virtual ~I2CSlave() = default; @@ -55,7 +56,7 @@ class I2CSlave } }; -class I2CBus +class I2CBusOld { public: void AddSlave(I2CSlave* slave); @@ -72,4 +73,53 @@ class I2CBus std::vector m_slaves; }; +// An I²C bus implementation accessed via bit-banging. +// A few assumptions and limitations exist: +// - All devices support both writes and reads. +// - Timing is not implemented at all; the clock signal can be changed as fast as needed. +// - Devices are not allowed to stretch the clock signal. (Nintendo's write code does not seem to +// implement clock stretching in any case, though some homebrew does.) +// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. +// - The device address is handled by this I2CBus class, instead of the device itself. +// - The device address is set on writes, and re-used for reads; writing an address and data and +// then switching to reading uses the incremented address. Every write must specify the address. +// - Reading without setting the device address beforehand is disallowed; the I²C specification +// allows such reads but does not specify how they behave (or anything about the behavior of the +// device address). +// - Switching between multiple devices using a restart does not reset the device address; the +// device address is only reset on stopping. This means that a write to one device followed by a +// read from a different device would result in reading from the last used device address (without +// any warning). +// - 10-bit addressing and other reserved addressing modes are not implemented. +class I2CBus +{ +public: + enum class State + { + Inactive, + Activating, + SetI2CAddress, + WriteDeviceAddress, + WriteToDevice, + ReadFromDevice, + }; + + State state; + u8 bit_counter; + u8 current_byte; + std::optional i2c_address; // Not shifted; includes the read flag + std::optional device_address; + + void Update(const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda); + bool GetSCL() const; + bool GetSDA() const; + +private: + void Start(); + void Stop(); + void SCLRisingEdge(const bool sda); + void SCLFallingEdge(const bool sda); + bool WriteExpected() const; +}; + } // namespace Common diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 95a6880dda74..e8be5abd5c5b 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -9,11 +9,10 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Common/EnumFormatter.h" +#include "Common/I2C.h" #include "Common/Logging/Log.h" #include "Core/Core.h" #include "Core/CoreTiming.h" -#include "Core/Debugger/Debugger_SymbolMap.h" #include "Core/HW/DVD/DVDInterface.h" #include "Core/HW/MMIO.h" #include "Core/HW/ProcessorInterface.h" @@ -121,59 +120,10 @@ struct AVEState }; #pragma pack() static_assert(sizeof(AVEState) == 0x100); -static AVEState ave_state; -static std::bitset ave_ever_logged; // For logging only; not saved - -// An I²C bus implementation accessed via bit-banging. -// A few assumptions and limitations exist: -// - All devices support both writes and reads. -// - Timing is not implemented at all; the clock signal can be changed as fast as needed. -// - Devices are not allowed to stretch the clock signal. (Nintendo's write code does not seem to -// implement clock stretching in any case, though some homebrew does.) -// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. -// - The device address is handled by this I2CBus class, instead of the device itself. -// - The device address is set on writes, and re-used for reads; writing an address and data and -// then switching to reading uses the incremented address. Every write must specify the address. -// - Reading without setting the device address beforehand is disallowed; the I²C specification -// allows such reads but does not specify how they behave (or anything about the behavior of the -// device address). -// - Switching between multiple devices using a restart does not reset the device address; the -// device address is only reset on stopping. This means that a write to one device followed by a -// read from a different device would result in reading from the last used device address (without -// any warning). -// - 10-bit addressing and other reserved addressing modes are not implemented. -class I2CBus -{ -public: - enum class State - { - Inactive, - Activating, - SetI2CAddress, - WriteDeviceAddress, - WriteToDevice, - ReadFromDevice, - }; +AVEState ave_state; +std::bitset ave_ever_logged; - State state; - u8 bit_counter; - u8 current_byte; - std::optional i2c_address; // Not shifted; includes the read flag - std::optional device_address; - - void Update(Core::System& system, const bool old_scl, const bool new_scl, const bool old_sda, - const bool new_sda); - bool GetSCL() const; - bool GetSDA() const; - -private: - void Start(); - void Stop(); - void SCLRisingEdge(const bool sda); - void SCLFallingEdge(const bool sda); - bool WriteExpected() const; -}; -I2CBus i2c_state; +Common::I2CBus i2c_state; static CoreTiming::EventType* updateInterrupts; @@ -247,7 +197,7 @@ void WiiIPC::Shutdown() { } -static std::string_view GetAVERegisterName(u8 address) +std::string_view GetAVERegisterName(u8 address) { if (address == 0x00) return "A/V Timings"; @@ -300,36 +250,6 @@ static u32 ReadGPIOIn(Core::System& system) return gpio_in.m_hex; } -bool I2CBus::GetSCL() const -{ - return true; // passive pullup - no clock stretching -} - -bool I2CBus::GetSDA() const -{ - switch (state) - { - case State::Inactive: - case State::Activating: - default: - return true; // passive pullup (or NACK) - - case State::SetI2CAddress: - case State::WriteDeviceAddress: - case State::WriteToDevice: - if (bit_counter < 8) - return true; // passive pullup - else - return false; // ACK (if we need to NACK, we set the state to inactive) - - case State::ReadFromDevice: - if (bit_counter < 8) - return ((current_byte << bit_counter) & 0x80) != 0; - else - return true; // passive pullup, receiver needs to ACK or NACK - } -} - u32 WiiIPC::GetGPIOOut() { // In the direction field, a '1' bit for a pin indicates that it will behave as an output (drive), @@ -352,226 +272,13 @@ void WiiIPC::GPIOOutChanged(u32 old_value_hex) } // I²C logic for the audio/video encoder (AVE) - i2c_state.Update(m_system, old_value[GPIO::AVE_SCL], new_value[GPIO::AVE_SCL], - old_value[GPIO::AVE_SDA], new_value[GPIO::AVE_SDA]); + i2c_state.Update(old_value[GPIO::AVE_SCL], new_value[GPIO::AVE_SCL], old_value[GPIO::AVE_SDA], + new_value[GPIO::AVE_SDA]); // SENSOR_BAR is checked by WiimoteEmu::CameraLogic // TODO: SLOT_LED } -void I2CBus::Start() -{ - if (state != State::Inactive) - INFO_LOG_FMT(WII_IPC, "AVE: Re-start I2C"); - else - INFO_LOG_FMT(WII_IPC, "AVE: Start I2C"); - - if (bit_counter != 0) - WARN_LOG_FMT(WII_IPC, "I2C: Start happened with a nonzero bit counter: {}", bit_counter); - - state = State::Activating; - bit_counter = 0; - current_byte = 0; - i2c_address.reset(); - // Note: don't reset device_address, as it's re-used for reads -} - -void I2CBus::Stop() -{ - INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - state = State::Inactive; - bit_counter = 0; - current_byte = 0; - i2c_address.reset(); - device_address.reset(); -} - -bool I2CBus::WriteExpected() const -{ - // If we don't have an I²C address, it needs to be written (even if the address that is later - // written is a read). - // Otherwise, check the least significant bit; it being *clear* indicates a write. - const bool is_write = !i2c_address.has_value() || ((i2c_address.value() & 1) == 0); - // The device that is otherwise recieving instead transmits an acknowledge bit after each byte. - const bool acknowledge_expected = (bit_counter == 8); - - return is_write ^ acknowledge_expected; -} - -void I2CBus::Update(Core::System& system, const bool old_scl, const bool new_scl, - const bool old_sda, const bool new_sda) -{ - if (old_scl != new_scl && old_sda != new_sda) - { - ERROR_LOG_FMT(WII_IPC, "Both SCL and SDA changed at the same time: SCL {} -> {} SDA {} -> {}", - old_scl, new_scl, old_sda, new_sda); - return; - } - - if (old_scl == new_scl && old_sda == new_sda) - return; // Nothing changed - - if (old_scl && new_scl) - { - // Check for changes to SDA while the clock is high. - if (old_sda && !new_sda) - { - // SDA falling edge (now pulled low) while SCL is high indicates I²C start - Start(); - } - else if (!old_sda && new_sda) - { - // SDA rising edge (now passive pullup) while SCL is high indicates I²C stop - Stop(); - } - } - else if (state != State::Inactive) - { - if (!old_scl && new_scl) - { - SCLRisingEdge(new_sda); - } - else if (old_scl && !new_scl) - { - SCLFallingEdge(new_sda); - } - } -} - -void I2CBus::SCLRisingEdge(const bool sda) -{ - // INFO_LOG_FMT(WII_IPC, "AVE: {} {} rising edge: {} (write expected: {})", bit_counter, state, - // sda, WriteExpected()); - // SCL rising edge indicates data clocking. For reads, we set up data at this point. - // For writes, we instead process it on the falling edge, to better distinguish - // the start/stop condition. - if (state == State::ReadFromDevice && bit_counter == 0) - { - // Start of a read. - ASSERT(device_address.has_value()); // Implied by the state transition in falling edge - current_byte = reinterpret_cast(&ave_state)[device_address.value()]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), - GetAVERegisterName(device_address.value()), current_byte); - } - // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); -} - -void I2CBus::SCLFallingEdge(const bool sda) -{ - // INFO_LOG_FMT(WII_IPC, "AVE: {} {} falling edge: {} (write expected: {})", bit_counter, state, - // sda, WriteExpected()); - // SCL falling edge is used to advance bit_counter/change states and process writes. - if (state == State::SetI2CAddress || state == State::WriteDeviceAddress || - state == State::WriteToDevice) - { - if (bit_counter == 8) - { - // Acknowledge bit for *reads*. - if (sda) - { - WARN_LOG_FMT(WII_IPC, "Read NACK'd"); - state = State::Inactive; - } - } - else - { - current_byte <<= 1; - if (sda) - current_byte |= 1; - - if (bit_counter == 7) - { - INFO_LOG_FMT(WII_IPC, "AVE: Byte written: {:02x}", current_byte); - // Write finished. - if (state == State::SetI2CAddress) - { - if ((current_byte >> 1) != 0x70) - { - state = State::Inactive; // NACK - WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); - } - else - { - INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); - } - } - else if (state == State::WriteDeviceAddress) - { - device_address = current_byte; - INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); - } - else - { - // Actual write - ASSERT(state == State::WriteToDevice); - ASSERT(device_address.has_value()); // implied by state transition - const u8 old_ave_value = reinterpret_cast(&ave_state)[device_address.value()]; - reinterpret_cast(&ave_state)[device_address.value()] = current_byte; - if (!ave_ever_logged[device_address.value()] || old_ave_value != current_byte) - { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), GetAVERegisterName(device_address.value())); - ave_ever_logged[device_address.value()] = true; - } - else - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), GetAVERegisterName(device_address.value())); - } - device_address = device_address.value() + 1; - } - } - } - } - - if (state == State::Activating) - { - // This is triggered after the start condition. - state = State::SetI2CAddress; - bit_counter = 0; - } - else if (state != State::Inactive) - { - if (bit_counter >= 8) - { - // Finished a byte and the acknowledge signal. - bit_counter = 0; - switch (state) - { - case State::SetI2CAddress: - i2c_address = current_byte; - // Note: i2c_address is known to correspond to a valid device - if ((current_byte & 1) == 0) - { - state = State::WriteDeviceAddress; - device_address.reset(); - } - else - { - if (device_address.has_value()) - { - state = State::ReadFromDevice; - } - else - { - state = State::Inactive; // NACK - required for 8-bit internal addresses - ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); - } - } - break; - case State::WriteDeviceAddress: - state = State::WriteToDevice; - break; - } - } - else - { - bit_counter++; - } - } - // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); -} - void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) { mmio->Register(base | IPC_PPCMSG, MMIO::InvalidRead(), MMIO::DirectWrite(&m_ppc_msg)); @@ -746,12 +453,3 @@ bool WiiIPC::IsReady() const ((m_ppc_irq_flags & INT_CAUSE_IPC_BROADWAY) == 0)); } } // namespace IOS - -template <> -struct fmt::formatter : EnumFormatter -{ - static constexpr array_type names = {"Inactive", "Activating", - "Set I2C Address", "Write Device Address", - "Write To Device", "Read From Device"}; - constexpr formatter() : EnumFormatter(names) {} -}; From 6b21b41b2fbf499a7e921ac8a06af3904e78e286 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 3 Sep 2022 17:41:22 -0700 Subject: [PATCH 23/34] Restructure device some --- Source/Core/Common/I2C.cpp | 75 ++++++++++++++++++++++--------- Source/Core/Common/I2C.h | 79 ++++++++++++++++----------------- Source/Core/Core/HW/WII_IPC.cpp | 2 + 3 files changed, 96 insertions(+), 60 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 843110e2fb09..4f0fa701ddd1 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -10,6 +10,7 @@ #include #include "Common/Assert.h" +#include "Common/ChunkFile.h" #include "Common/EnumFormatter.h" #include "Common/Logging/Log.h" #include "Core/Debugger/Debugger_SymbolMap.h" @@ -25,47 +26,70 @@ std::string_view GetAVERegisterName(u8 address); namespace Common { -void I2CBusOld::AddSlave(I2CSlave* slave) +void I2CBusBase::AddSlave(I2CSlave* slave) { m_slaves.emplace_back(slave); } -void I2CBusOld::RemoveSlave(I2CSlave* slave) +void I2CBusBase::RemoveSlave(I2CSlave* slave) { m_slaves.erase(std::remove(m_slaves.begin(), m_slaves.end(), slave), m_slaves.end()); } -void I2CBusOld::Reset() +void I2CBusBase::Reset() { m_slaves.clear(); } -int I2CBusOld::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +I2CSlave* I2CBusBase::GetSlave(u8 slave_addr) { - for (auto& slave : m_slaves) + for (I2CSlave* slave : m_slaves) { - auto const bytes_read = slave->BusRead(slave_addr, addr, count, data_out); - - // A slave responded, we are done. - if (bytes_read) - return bytes_read; + if (slave->Matches(slave_addr)) + return slave; } - - return 0; + return nullptr; } -int I2CBusOld::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { - for (auto& slave : m_slaves) + I2CSlave* slave = GetSlave(slave_addr); + if (slave != nullptr) { - auto const bytes_written = slave->BusWrite(slave_addr, addr, count, data_in); - - // A slave responded, we are done. - if (bytes_written) - return bytes_written; + for (int i = 0; i < count; i++) + { + // TODO: Does this make sense? The transmitter can't NACK a read... it's the receiver that + // does that + auto byte = slave->ReadByte(addr + i); + if (byte.has_value()) + data_out[i] = byte.value(); + else + return i; + } + return count; } + else + { + return 0; + } +} - return 0; +int I2CBusSimple::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +{ + I2CSlave* slave = GetSlave(slave_addr); + if (slave != nullptr) + { + for (int i = 0; i < count; i++) + { + if (!slave->WriteByte(addr + i, data_in[i])) + return i; + } + return count; + } + else + { + return 0; + } } bool I2CBus::GetSCL() const @@ -310,6 +334,17 @@ void I2CBus::SCLFallingEdge(const bool sda) // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } +void I2CBus::DoState(PointerWrap& p) +{ + p.Do(state); + p.Do(bit_counter); + p.Do(current_byte); + p.Do(i2c_address); + p.Do(device_address); + // TODO: verify m_devices is the same so that the state is compatible. + // (We don't take ownership of saving/loading m_devices, though). +} + }; // namespace Common template <> diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index fcccb4f322a4..16fcd8dc06d2 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -9,70 +9,67 @@ #include "Common/CommonTypes.h" +class PointerWrap; + namespace Common { -class I2CBusOld; - class I2CSlave { - friend I2CBusOld; - -protected: - virtual ~I2CSlave() = default; - - virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0; - virtual int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) = 0; +public: + virtual bool Matches(u8 slave_addr) = 0; + virtual std::optional ReadByte(u8 addr) = 0; + virtual bool WriteByte(u8 addr, u8 value) = 0; +}; - template - static int RawRead(T* reg_data, u8 addr, int count, u8* data_out) +template +class I2CSlaveSimple : I2CSlave +{ +public: + bool Matches(u8 slave_addr) override { return slave_addr == slave_addr_val; } + std::optional ReadByte(u8 addr) { return data_bytes[addr]; } + bool WriteByte(u8 addr, u8 value) { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* src = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - src)); - - std::copy_n(src, count, data_out); - - return count; + data_bytes[addr] = value; + return true; } - template - static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in) + union { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* dst = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - dst)); - - std::copy_n(data_in, count, dst); + T data; + std::array data_bytes; + }; - return count; - } + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(sizeof(T) == 0x100); }; -class I2CBusOld +class I2CBusBase { public: + virtual ~I2CBusBase() = default; + void AddSlave(I2CSlave* slave); void RemoveSlave(I2CSlave* slave); void Reset(); - // TODO: change int to u16 or something - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); +protected: + // Returns nullptr if there is no match + I2CSlave* GetSlave(u8 slave_addr); private: // Pointers are unowned: std::vector m_slaves; }; +class I2CBusSimple : public I2CBusBase +{ +public: + // TODO: change int to u16 or something + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); +}; + // An I²C bus implementation accessed via bit-banging. // A few assumptions and limitations exist: // - All devices support both writes and reads. @@ -91,7 +88,7 @@ class I2CBusOld // read from a different device would result in reading from the last used device address (without // any warning). // - 10-bit addressing and other reserved addressing modes are not implemented. -class I2CBus +class I2CBus : public I2CBusBase { public: enum class State @@ -114,6 +111,8 @@ class I2CBus bool GetSCL() const; bool GetSDA() const; + void DoState(PointerWrap& p); + private: void Start(); void Stop(); diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index e8be5abd5c5b..e8a971101997 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -144,6 +144,8 @@ void WiiIPC::DoState(PointerWrap& p) p.Do(m_arm_irq_masks); p.Do(m_gpio_dir); p.Do(m_gpio_out); + i2c_state.DoState(p); + p.Do(ave_state); p.Do(m_resets); } From 89a5108e4604541076739dcdcf9bd40d5afe1b8d Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 3 Sep 2022 18:27:55 -0700 Subject: [PATCH 24/34] Actually use the device --- Source/Core/Common/I2C.cpp | 64 +++++++++++++++++++++------------ Source/Core/Core/HW/WII_IPC.cpp | 6 ++-- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 4f0fa701ddd1..419c0e2f1f4b 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -15,15 +15,6 @@ #include "Common/Logging/Log.h" #include "Core/Debugger/Debugger_SymbolMap.h" -namespace IOS -{ -struct AVEState; -extern AVEState ave_state; -extern std::bitset<0x100> ave_ever_logged; // For logging only; not saved - -std::string_view GetAVERegisterName(u8 address); -} // namespace IOS - namespace Common { void I2CBusBase::AddSlave(I2CSlave* slave) @@ -211,9 +202,25 @@ void I2CBus::SCLRisingEdge(const bool sda) { // Start of a read. ASSERT(device_address.has_value()); // Implied by the state transition in falling edge - current_byte = reinterpret_cast(&IOS::ave_state)[device_address.value()]; - INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), - IOS::GetAVERegisterName(device_address.value()), current_byte); + ASSERT(i2c_address.has_value()); + I2CSlave* slave = GetSlave(i2c_address.value()); + ASSERT_MSG(WII_IPC, slave != nullptr, + "Expected device with ID {:02x} to be on the I2C bus as it was earlier", + i2c_address.value()); + std::optional byte = slave->ReadByte(device_address.value()); + if (!byte.has_value()) + { + WARN_LOG_FMT(WII_IPC, "Failed to read from device {:02x} at address {:02x}", + i2c_address.value(), device_address.value()); + // TODO + current_byte = 0xff; + } + else + { + current_byte = byte.value(); + } + // INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), + // IOS::GetAVERegisterName(device_address.value()), current_byte); } // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } @@ -247,7 +254,8 @@ void I2CBus::SCLFallingEdge(const bool sda) // Write finished. if (state == State::SetI2CAddress) { - if ((current_byte >> 1) != 0x70) + I2CSlave* slave = GetSlave(current_byte); + if (slave != nullptr) { state = State::Inactive; // NACK WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); @@ -267,20 +275,32 @@ void I2CBus::SCLFallingEdge(const bool sda) // Actual write ASSERT(state == State::WriteToDevice); ASSERT(device_address.has_value()); // implied by state transition - const u8 old_ave_value = reinterpret_cast(&IOS::ave_state)[device_address.value()]; - reinterpret_cast(&IOS::ave_state)[device_address.value()] = current_byte; - if (!IOS::ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + ASSERT(i2c_address.has_value()); + I2CSlave* slave = GetSlave(i2c_address.value()); + ASSERT_MSG(WII_IPC, slave != nullptr, + "Expected device with ID {:02x} to be on the I2C bus as it was earlier", + i2c_address.value()); + if (slave == nullptr) { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), IOS::GetAVERegisterName(device_address.value())); - IOS::ave_ever_logged[device_address.value()] = true; + state = State::Inactive; // NACK } else { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), IOS::GetAVERegisterName(device_address.value())); + slave->WriteByte(device_address.value(), current_byte); + /* if (!IOS::ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + { + INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), IOS::GetAVERegisterName(device_address.value())); + IOS::ave_ever_logged[device_address.value()] = true; + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, + device_address.value(), + IOS::GetAVERegisterName(device_address.value())); + }*/ + device_address = device_address.value() + 1; } - device_address = device_address.value() + 1; } } } diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index e8a971101997..76bb5f9476c1 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -120,7 +120,7 @@ struct AVEState }; #pragma pack() static_assert(sizeof(AVEState) == 0x100); -AVEState ave_state; +Common::I2CSlaveSimple<0x70, AVEState> ave_state; std::bitset ave_ever_logged; Common::I2CBus i2c_state; @@ -145,7 +145,7 @@ void WiiIPC::DoState(PointerWrap& p) p.Do(m_gpio_dir); p.Do(m_gpio_out); i2c_state.DoState(p); - p.Do(ave_state); + p.Do(ave_state.data); p.Do(m_resets); } @@ -178,7 +178,7 @@ void WiiIPC::InitState() i2c_state = {}; ave_state = {}; - ave_state.video_output_config = 0x23; + ave_state.data.video_output_config = 0x23; ave_ever_logged.reset(); } From 206b6337af75faa85af90a98fcca9e835700487f Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 12:17:55 -0700 Subject: [PATCH 25/34] Temp, doesn't build --- Source/Core/Common/I2C.cpp | 19 +---- Source/Core/Common/I2C.h | 33 ++++---- Source/Core/Core/HW/WiimoteEmu/Camera.cpp | 24 +++--- Source/Core/Core/HW/WiimoteEmu/Camera.h | 10 +-- .../HW/WiimoteEmu/Extension/Extension.cpp | 13 +++- .../Core/HW/WiimoteEmu/Extension/Extension.h | 9 ++- .../Core/Core/HW/WiimoteEmu/ExtensionPort.cpp | 4 +- .../Core/Core/HW/WiimoteEmu/ExtensionPort.h | 7 +- Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp | 53 ------------- Source/Core/Core/HW/WiimoteEmu/I2CBus.h | 75 ------------------- Source/Core/Core/HW/WiimoteEmu/MotionPlus.h | 7 +- Source/Core/Core/HW/WiimoteEmu/Speaker.cpp | 23 +++--- Source/Core/Core/HW/WiimoteEmu/Speaker.h | 9 ++- Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h | 4 +- 14 files changed, 71 insertions(+), 219 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 419c0e2f1f4b..449091281495 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -51,11 +51,7 @@ int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { // TODO: Does this make sense? The transmitter can't NACK a read... it's the receiver that // does that - auto byte = slave->ReadByte(addr + i); - if (byte.has_value()) - data_out[i] = byte.value(); - else - return i; + data_out[i] = slave->ReadByte(addr + i); } return count; } @@ -207,18 +203,7 @@ void I2CBus::SCLRisingEdge(const bool sda) ASSERT_MSG(WII_IPC, slave != nullptr, "Expected device with ID {:02x} to be on the I2C bus as it was earlier", i2c_address.value()); - std::optional byte = slave->ReadByte(device_address.value()); - if (!byte.has_value()) - { - WARN_LOG_FMT(WII_IPC, "Failed to read from device {:02x} at address {:02x}", - i2c_address.value(), device_address.value()); - // TODO - current_byte = 0xff; - } - else - { - current_byte = byte.value(); - } + current_byte = slave->ReadByte(device_address.value()); // INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), // IOS::GetAVERegisterName(device_address.value()), current_byte); } diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index 16fcd8dc06d2..a98f63fda6a2 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -17,30 +17,27 @@ class I2CSlave { public: virtual bool Matches(u8 slave_addr) = 0; - virtual std::optional ReadByte(u8 addr) = 0; + virtual u8 ReadByte(u8 addr) = 0; virtual bool WriteByte(u8 addr, u8 value) = 0; -}; -template -class I2CSlaveSimple : I2CSlave -{ -public: - bool Matches(u8 slave_addr) override { return slave_addr == slave_addr_val; } - std::optional ReadByte(u8 addr) { return data_bytes[addr]; } - bool WriteByte(u8 addr, u8 value) +protected: + ~I2CSlave() = default; + + template + static u8 RawRead(T* reg_data, u8 addr) { - data_bytes[addr] = value; - return true; + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(sizeof(T) == 0x100); + return reinterpret_cast(reg_data)[addr]; } - union + template + static void RawWrite(T* reg_data, u8 addr, u8 value) { - T data; - std::array data_bytes; - }; - - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(sizeof(T) == 0x100); + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(sizeof(T) == 0x100); + reinterpret_cast(reg_data)[addr] = value; + } }; class I2CBusBase diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp index 1dcdb7795200..b253042e689e 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp @@ -29,26 +29,20 @@ void CameraLogic::DoState(PointerWrap& p) // FYI: m_is_enabled is handled elsewhere. } -int CameraLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool CameraLogic::Matches(u8 slave_addr) { - if (I2C_ADDR != slave_addr) - return 0; - - if (!m_is_enabled) - return 0; - - return RawRead(&m_reg_data, addr, count, data_out); + return I2C_ADDR == slave_addr && m_is_enabled; } -int CameraLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +u8 CameraLogic::ReadByte(u8 addr) { - if (I2C_ADDR != slave_addr) - return 0; - - if (!m_is_enabled) - return 0; + return RawRead(&m_reg_data, addr); +} - return RawWrite(&m_reg_data, addr, count, data_in); +bool CameraLogic::WriteByte(u8 addr, u8 value) +{ + RawWrite(&m_reg_data, addr, value); + return true; } std::array diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.h b/Source/Core/Core/HW/WiimoteEmu/Camera.h index 23e21cc84e43..c3b51598c7b0 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.h +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.h @@ -5,8 +5,8 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Dynamics.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControlGroup/Cursor.h" namespace Common @@ -101,7 +101,7 @@ struct IRFull : IRExtended }; static_assert(sizeof(IRFull) == 9, "Wrong size"); -class CameraLogic : public I2CSlave +class CameraLogic : public Common::I2CSlave { public: // OEM sensor bar distance between LED clusters in meters. @@ -175,9 +175,9 @@ class CameraLogic : public I2CSlave // The real wiimote reads camera data from the i2c bus at offset 0x37: static const u8 REPORT_DATA_OFFSET = offsetof(Register, camera_data); -private: - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool Matches(u8 slave_addr) override; + u8 ReadByte(u8 addr) override; + bool WriteByte(u8 addr, u8 value) override; Register m_reg_data{}; diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp index fd2b18587276..ebc034ed7421 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp @@ -71,14 +71,19 @@ void None::DoState(PointerWrap& p) // Nothing needed. } -int None::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool None::Matches(u8 slave_addr) { - return 0; + return false; } -int None::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +std::optional None::ReadByte(u8 addr) { - return 0; + return std::nullopt; +} + +bool None::WriteByte(u8 addr, u8 value) +{ + return false; } bool EncryptedExtension::ReadDeviceDetectPin() const diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h index 4a055c597db3..e2fe729d5002 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h @@ -10,15 +10,15 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Encryption.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControllerEmu.h" namespace WiimoteEmu { struct DesiredExtensionState; -class Extension : public ControllerEmu::EmulatedController, public I2CSlave +class Extension : public ControllerEmu::EmulatedController, public Common::I2CSlave { public: explicit Extension(const char* name); @@ -56,8 +56,9 @@ class None : public Extension void Reset() override; void DoState(PointerWrap& p) override; - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool Matches(u8 slave_addr) override; + std::optional ReadByte(u8 addr) override; + bool WriteByte(u8 addr, u8 value) override; }; // This class provides the encryption and initialization behavior of most extensions. diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp index 97e73ea4bc90..c5043298b8a4 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp @@ -3,11 +3,9 @@ #include "Core/HW/WiimoteEmu/ExtensionPort.h" -#include "Common/ChunkFile.h" - namespace WiimoteEmu { -ExtensionPort::ExtensionPort(I2CBus* i2c_bus) : m_i2c_bus(*i2c_bus) +ExtensionPort::ExtensionPort(Common::I2CBusBase* i2c_bus) : m_i2c_bus(*i2c_bus) { } diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h index 86b1fd31f962..3c6c70e62a1b 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h @@ -3,9 +3,8 @@ #pragma once -#include "Common/ChunkFile.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Extension/Extension.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" namespace WiimoteEmu { @@ -35,14 +34,14 @@ class ExtensionPort static constexpr u8 REPORT_I2C_SLAVE = 0x52; static constexpr u8 REPORT_I2C_ADDR = 0x00; - explicit ExtensionPort(I2CBus* i2c_bus); + explicit ExtensionPort(Common::I2CBusBase* i2c_bus); bool IsDeviceConnected() const; void AttachExtension(Extension* dev); private: Extension* m_extension = nullptr; - I2CBus& m_i2c_bus; + Common::I2CBusBase& m_i2c_bus; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp index 1c231f794446..e69de29bb2d1 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp @@ -1,53 +0,0 @@ -// Copyright 2019 Dolphin Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#include "Core/HW/WiimoteEmu/I2CBus.h" - -#include - -namespace WiimoteEmu -{ -void I2CBus::AddSlave(I2CSlave* slave) -{ - m_slaves.emplace_back(slave); -} - -void I2CBus::RemoveSlave(I2CSlave* slave) -{ - std::erase(m_slaves, slave); -} - -void I2CBus::Reset() -{ - m_slaves.clear(); -} - -int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) -{ - for (auto& slave : m_slaves) - { - auto const bytes_read = slave->BusRead(slave_addr, addr, count, data_out); - - // A slave responded, we are done. - if (bytes_read) - return bytes_read; - } - - return 0; -} - -int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) -{ - for (auto& slave : m_slaves) - { - auto const bytes_written = slave->BusWrite(slave_addr, addr, count, data_in); - - // A slave responded, we are done. - if (bytes_written) - return bytes_written; - } - - return 0; -} - -} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h index 3a1e0a67d361..e69de29bb2d1 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h @@ -1,75 +0,0 @@ -// Copyright 2019 Dolphin Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#pragma once - -#include -#include - -#include "Common/CommonTypes.h" - -namespace WiimoteEmu -{ -class I2CBus; - -class I2CSlave -{ - friend I2CBus; - -protected: - virtual ~I2CSlave() = default; - - virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0; - virtual int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) = 0; - - template - static int RawRead(T* reg_data, u8 addr, int count, u8* data_out) - { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* src = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - src)); - - std::copy_n(src, count, data_out); - - return count; - } - - template - static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in) - { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* dst = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - dst)); - - std::copy_n(data_in, count, dst); - - return count; - } -}; - -class I2CBus -{ -public: - void AddSlave(I2CSlave* slave); - void RemoveSlave(I2CSlave* slave); - - void Reset(); - - // TODO: change int to u16 or something - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); - -private: - // Pointers are unowned: - std::vector m_slaves; -}; - -} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h index 02ddfb7ec2f0..ccbc0605c38a 100644 --- a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h +++ b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h @@ -248,8 +248,9 @@ struct MotionPlus : public Extension ActivationStatus GetActivationStatus() const; PassthroughMode GetPassthroughMode() const; - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool Matches(u8 slave_addr) override; + u8 ReadByte(u8 addr) override; + bool WriteByte(u8 addr, u8 value) override; bool ReadDeviceDetectPin() const override; @@ -259,7 +260,7 @@ struct MotionPlus : public Extension u8 m_progress_timer = {}; // The port on the end of the motion plus: - I2CBus m_i2c_bus; + Common::I2CBusSimple m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp index 23cba6bfd741..99b9b8291228 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp @@ -153,30 +153,29 @@ void SpeakerLogic::SetSpeakerEnabled(bool enabled) m_speaker_enabled = enabled; } -int SpeakerLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool SpeakerLogic::Matches(u8 slave_addr) { - if (I2C_ADDR != slave_addr) - return 0; - - return RawRead(®_data, addr, count, data_out); + return I2C_ADDR == slave_addr; } -int SpeakerLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +u8 SpeakerLogic::ReadByte(u8 addr) { - if (I2C_ADDR != slave_addr) - return 0; + return RawRead(®_data, addr); +} +bool SpeakerLogic::WriteByte(u8 addr, u8 value) +{ if (0x00 == addr) { SpeakerData(data_in, count, m_speaker_pan_setting.GetValue() / 100); - return count; + // TODO: Does writing immediately change the decoder config even when active + // or does a write to 0x08 activate the new configuration or something? } else { - // TODO: Does writing immediately change the decoder config even when active - // or does a write to 0x08 activate the new configuration or something? - return RawWrite(®_data, addr, count, data_in); + RawWrite(®_data, addr, value); } + return true; } } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.h b/Source/Core/Core/HW/WiimoteEmu/Speaker.h index 9a6a652bb1fa..995ccd2f0c14 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.h +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.h @@ -5,7 +5,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" +#include "Common/I2C.h" #include "InputCommon/ControllerEmu/Setting/NumericSetting.h" namespace WiimoteEmu @@ -17,7 +17,7 @@ struct ADPCMState class Wiimote; -class SpeakerLogic : public I2CSlave +class SpeakerLogic : public Common::I2CSlave { friend class Wiimote; @@ -63,8 +63,9 @@ class SpeakerLogic : public I2CSlave static_assert(0x100 == sizeof(Register)); - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool Matches(u8 slave_addr) override; + u8 ReadByte(u8 addr) override; + bool WriteByte(u8 addr, u8 value) override; Register reg_data{}; diff --git a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h index e364ccdb6c91..8468e4dc60a2 100644 --- a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h +++ b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h @@ -10,6 +10,7 @@ #include "Common/Common.h" #include "Common/Config/Config.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteCommon/WiimoteReport.h" @@ -17,7 +18,6 @@ #include "Core/HW/WiimoteEmu/Dynamics.h" #include "Core/HW/WiimoteEmu/Encryption.h" #include "Core/HW/WiimoteEmu/ExtensionPort.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "Core/HW/WiimoteEmu/MotionPlus.h" #include "Core/HW/WiimoteEmu/Speaker.h" @@ -316,7 +316,7 @@ class Wiimote : public ControllerEmu::EmulatedController, public WiimoteCommon:: MotionPlus m_motion_plus; CameraLogic m_camera_logic; - I2CBus m_i2c_bus; + Common::I2CBusSimple m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; From 63afdeccc366ed6072d74c122a18d0f9ab656408 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 12:26:06 -0700 Subject: [PATCH 26/34] Revert "Temp, doesn't build" This reverts commit cf90785d59af9a295825573be7813a06b7766666. --- Source/Core/Common/I2C.cpp | 19 ++++- Source/Core/Common/I2C.h | 33 ++++---- Source/Core/Core/HW/WiimoteEmu/Camera.cpp | 24 +++--- Source/Core/Core/HW/WiimoteEmu/Camera.h | 10 +-- .../HW/WiimoteEmu/Extension/Extension.cpp | 13 +--- .../Core/HW/WiimoteEmu/Extension/Extension.h | 9 +-- .../Core/Core/HW/WiimoteEmu/ExtensionPort.cpp | 4 +- .../Core/Core/HW/WiimoteEmu/ExtensionPort.h | 7 +- Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp | 53 +++++++++++++ Source/Core/Core/HW/WiimoteEmu/I2CBus.h | 75 +++++++++++++++++++ Source/Core/Core/HW/WiimoteEmu/MotionPlus.h | 7 +- Source/Core/Core/HW/WiimoteEmu/Speaker.cpp | 23 +++--- Source/Core/Core/HW/WiimoteEmu/Speaker.h | 9 +-- Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h | 4 +- 14 files changed, 219 insertions(+), 71 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 449091281495..419c0e2f1f4b 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -51,7 +51,11 @@ int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { // TODO: Does this make sense? The transmitter can't NACK a read... it's the receiver that // does that - data_out[i] = slave->ReadByte(addr + i); + auto byte = slave->ReadByte(addr + i); + if (byte.has_value()) + data_out[i] = byte.value(); + else + return i; } return count; } @@ -203,7 +207,18 @@ void I2CBus::SCLRisingEdge(const bool sda) ASSERT_MSG(WII_IPC, slave != nullptr, "Expected device with ID {:02x} to be on the I2C bus as it was earlier", i2c_address.value()); - current_byte = slave->ReadByte(device_address.value()); + std::optional byte = slave->ReadByte(device_address.value()); + if (!byte.has_value()) + { + WARN_LOG_FMT(WII_IPC, "Failed to read from device {:02x} at address {:02x}", + i2c_address.value(), device_address.value()); + // TODO + current_byte = 0xff; + } + else + { + current_byte = byte.value(); + } // INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), // IOS::GetAVERegisterName(device_address.value()), current_byte); } diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index a98f63fda6a2..16fcd8dc06d2 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -17,27 +17,30 @@ class I2CSlave { public: virtual bool Matches(u8 slave_addr) = 0; - virtual u8 ReadByte(u8 addr) = 0; + virtual std::optional ReadByte(u8 addr) = 0; virtual bool WriteByte(u8 addr, u8 value) = 0; +}; -protected: - ~I2CSlave() = default; - - template - static u8 RawRead(T* reg_data, u8 addr) +template +class I2CSlaveSimple : I2CSlave +{ +public: + bool Matches(u8 slave_addr) override { return slave_addr == slave_addr_val; } + std::optional ReadByte(u8 addr) { return data_bytes[addr]; } + bool WriteByte(u8 addr, u8 value) { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(sizeof(T) == 0x100); - return reinterpret_cast(reg_data)[addr]; + data_bytes[addr] = value; + return true; } - template - static void RawWrite(T* reg_data, u8 addr, u8 value) + union { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(sizeof(T) == 0x100); - reinterpret_cast(reg_data)[addr] = value; - } + T data; + std::array data_bytes; + }; + + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(sizeof(T) == 0x100); }; class I2CBusBase diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp index b253042e689e..1dcdb7795200 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp @@ -29,20 +29,26 @@ void CameraLogic::DoState(PointerWrap& p) // FYI: m_is_enabled is handled elsewhere. } -bool CameraLogic::Matches(u8 slave_addr) +int CameraLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { - return I2C_ADDR == slave_addr && m_is_enabled; -} + if (I2C_ADDR != slave_addr) + return 0; -u8 CameraLogic::ReadByte(u8 addr) -{ - return RawRead(&m_reg_data, addr); + if (!m_is_enabled) + return 0; + + return RawRead(&m_reg_data, addr, count, data_out); } -bool CameraLogic::WriteByte(u8 addr, u8 value) +int CameraLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { - RawWrite(&m_reg_data, addr, value); - return true; + if (I2C_ADDR != slave_addr) + return 0; + + if (!m_is_enabled) + return 0; + + return RawWrite(&m_reg_data, addr, count, data_in); } std::array diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.h b/Source/Core/Core/HW/WiimoteEmu/Camera.h index c3b51598c7b0..23e21cc84e43 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.h +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.h @@ -5,8 +5,8 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Dynamics.h" +#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControlGroup/Cursor.h" namespace Common @@ -101,7 +101,7 @@ struct IRFull : IRExtended }; static_assert(sizeof(IRFull) == 9, "Wrong size"); -class CameraLogic : public Common::I2CSlave +class CameraLogic : public I2CSlave { public: // OEM sensor bar distance between LED clusters in meters. @@ -175,9 +175,9 @@ class CameraLogic : public Common::I2CSlave // The real wiimote reads camera data from the i2c bus at offset 0x37: static const u8 REPORT_DATA_OFFSET = offsetof(Register, camera_data); - bool Matches(u8 slave_addr) override; - u8 ReadByte(u8 addr) override; - bool WriteByte(u8 addr, u8 value) override; +private: + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; Register m_reg_data{}; diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp index ebc034ed7421..fd2b18587276 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp @@ -71,19 +71,14 @@ void None::DoState(PointerWrap& p) // Nothing needed. } -bool None::Matches(u8 slave_addr) +int None::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { - return false; + return 0; } -std::optional None::ReadByte(u8 addr) +int None::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { - return std::nullopt; -} - -bool None::WriteByte(u8 addr, u8 value) -{ - return false; + return 0; } bool EncryptedExtension::ReadDeviceDetectPin() const diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h index e2fe729d5002..4a055c597db3 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h @@ -10,15 +10,15 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Encryption.h" +#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControllerEmu.h" namespace WiimoteEmu { struct DesiredExtensionState; -class Extension : public ControllerEmu::EmulatedController, public Common::I2CSlave +class Extension : public ControllerEmu::EmulatedController, public I2CSlave { public: explicit Extension(const char* name); @@ -56,9 +56,8 @@ class None : public Extension void Reset() override; void DoState(PointerWrap& p) override; - bool Matches(u8 slave_addr) override; - std::optional ReadByte(u8 addr) override; - bool WriteByte(u8 addr, u8 value) override; + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; }; // This class provides the encryption and initialization behavior of most extensions. diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp index c5043298b8a4..97e73ea4bc90 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp @@ -3,9 +3,11 @@ #include "Core/HW/WiimoteEmu/ExtensionPort.h" +#include "Common/ChunkFile.h" + namespace WiimoteEmu { -ExtensionPort::ExtensionPort(Common::I2CBusBase* i2c_bus) : m_i2c_bus(*i2c_bus) +ExtensionPort::ExtensionPort(I2CBus* i2c_bus) : m_i2c_bus(*i2c_bus) { } diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h index 3c6c70e62a1b..86b1fd31f962 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h @@ -3,8 +3,9 @@ #pragma once -#include "Common/I2C.h" +#include "Common/ChunkFile.h" #include "Core/HW/WiimoteEmu/Extension/Extension.h" +#include "Core/HW/WiimoteEmu/I2CBus.h" namespace WiimoteEmu { @@ -34,14 +35,14 @@ class ExtensionPort static constexpr u8 REPORT_I2C_SLAVE = 0x52; static constexpr u8 REPORT_I2C_ADDR = 0x00; - explicit ExtensionPort(Common::I2CBusBase* i2c_bus); + explicit ExtensionPort(I2CBus* i2c_bus); bool IsDeviceConnected() const; void AttachExtension(Extension* dev); private: Extension* m_extension = nullptr; - Common::I2CBusBase& m_i2c_bus; + I2CBus& m_i2c_bus; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp index e69de29bb2d1..19202350fd64 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp @@ -0,0 +1,53 @@ +// Copyright 2019 Dolphin Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#include "Core/HW/WiimoteEmu/I2CBus.h" + +#include + +namespace WiimoteEmu +{ +void I2CBus::AddSlave(I2CSlave* slave) +{ + m_slaves.emplace_back(slave); +} + +void I2CBus::RemoveSlave(I2CSlave* slave) +{ + m_slaves.erase(std::remove(m_slaves.begin(), m_slaves.end(), slave), m_slaves.end()); +} + +void I2CBus::Reset() +{ + m_slaves.clear(); +} + +int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +{ + for (auto& slave : m_slaves) + { + auto const bytes_read = slave->BusRead(slave_addr, addr, count, data_out); + + // A slave responded, we are done. + if (bytes_read) + return bytes_read; + } + + return 0; +} + +int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +{ + for (auto& slave : m_slaves) + { + auto const bytes_written = slave->BusWrite(slave_addr, addr, count, data_in); + + // A slave responded, we are done. + if (bytes_written) + return bytes_written; + } + + return 0; +} + +} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h index e69de29bb2d1..3a1e0a67d361 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h @@ -0,0 +1,75 @@ +// Copyright 2019 Dolphin Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once + +#include +#include + +#include "Common/CommonTypes.h" + +namespace WiimoteEmu +{ +class I2CBus; + +class I2CSlave +{ + friend I2CBus; + +protected: + virtual ~I2CSlave() = default; + + virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0; + virtual int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) = 0; + + template + static int RawRead(T* reg_data, u8 addr, int count, u8* data_out) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + // TODO: addr wraps around after 0xff + + u8* src = reinterpret_cast(reg_data) + addr; + count = std::min(count, int(reinterpret_cast(reg_data + 1) - src)); + + std::copy_n(src, count, data_out); + + return count; + } + + template + static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + // TODO: addr wraps around after 0xff + + u8* dst = reinterpret_cast(reg_data) + addr; + count = std::min(count, int(reinterpret_cast(reg_data + 1) - dst)); + + std::copy_n(data_in, count, dst); + + return count; + } +}; + +class I2CBus +{ +public: + void AddSlave(I2CSlave* slave); + void RemoveSlave(I2CSlave* slave); + + void Reset(); + + // TODO: change int to u16 or something + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); + +private: + // Pointers are unowned: + std::vector m_slaves; +}; + +} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h index ccbc0605c38a..02ddfb7ec2f0 100644 --- a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h +++ b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h @@ -248,9 +248,8 @@ struct MotionPlus : public Extension ActivationStatus GetActivationStatus() const; PassthroughMode GetPassthroughMode() const; - bool Matches(u8 slave_addr) override; - u8 ReadByte(u8 addr) override; - bool WriteByte(u8 addr, u8 value) override; + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; bool ReadDeviceDetectPin() const override; @@ -260,7 +259,7 @@ struct MotionPlus : public Extension u8 m_progress_timer = {}; // The port on the end of the motion plus: - Common::I2CBusSimple m_i2c_bus; + I2CBus m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp index 99b9b8291228..23cba6bfd741 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp @@ -153,29 +153,30 @@ void SpeakerLogic::SetSpeakerEnabled(bool enabled) m_speaker_enabled = enabled; } -bool SpeakerLogic::Matches(u8 slave_addr) +int SpeakerLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { - return I2C_ADDR == slave_addr; -} + if (I2C_ADDR != slave_addr) + return 0; -u8 SpeakerLogic::ReadByte(u8 addr) -{ - return RawRead(®_data, addr); + return RawRead(®_data, addr, count, data_out); } -bool SpeakerLogic::WriteByte(u8 addr, u8 value) +int SpeakerLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { + if (I2C_ADDR != slave_addr) + return 0; + if (0x00 == addr) { SpeakerData(data_in, count, m_speaker_pan_setting.GetValue() / 100); - // TODO: Does writing immediately change the decoder config even when active - // or does a write to 0x08 activate the new configuration or something? + return count; } else { - RawWrite(®_data, addr, value); + // TODO: Does writing immediately change the decoder config even when active + // or does a write to 0x08 activate the new configuration or something? + return RawWrite(®_data, addr, count, data_in); } - return true; } } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.h b/Source/Core/Core/HW/WiimoteEmu/Speaker.h index 995ccd2f0c14..9a6a652bb1fa 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.h +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.h @@ -5,7 +5,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Common/I2C.h" +#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/Setting/NumericSetting.h" namespace WiimoteEmu @@ -17,7 +17,7 @@ struct ADPCMState class Wiimote; -class SpeakerLogic : public Common::I2CSlave +class SpeakerLogic : public I2CSlave { friend class Wiimote; @@ -63,9 +63,8 @@ class SpeakerLogic : public Common::I2CSlave static_assert(0x100 == sizeof(Register)); - bool Matches(u8 slave_addr) override; - u8 ReadByte(u8 addr) override; - bool WriteByte(u8 addr, u8 value) override; + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; + int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; Register reg_data{}; diff --git a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h index 8468e4dc60a2..e364ccdb6c91 100644 --- a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h +++ b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h @@ -10,7 +10,6 @@ #include "Common/Common.h" #include "Common/Config/Config.h" -#include "Common/I2C.h" #include "Core/HW/WiimoteCommon/WiimoteReport.h" @@ -18,6 +17,7 @@ #include "Core/HW/WiimoteEmu/Dynamics.h" #include "Core/HW/WiimoteEmu/Encryption.h" #include "Core/HW/WiimoteEmu/ExtensionPort.h" +#include "Core/HW/WiimoteEmu/I2CBus.h" #include "Core/HW/WiimoteEmu/MotionPlus.h" #include "Core/HW/WiimoteEmu/Speaker.h" @@ -316,7 +316,7 @@ class Wiimote : public ControllerEmu::EmulatedController, public WiimoteCommon:: MotionPlus m_motion_plus; CameraLogic m_camera_logic; - Common::I2CBusSimple m_i2c_bus; + I2CBus m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; From fb274281b15aac15a032437f4869d4c7b9950c85 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 13:23:35 -0700 Subject: [PATCH 27/34] More WIP reworking --- Source/Core/Common/I2C.cpp | 264 +++++++++++++++++++++++-------------- Source/Core/Common/I2C.h | 60 ++++----- 2 files changed, 192 insertions(+), 132 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 419c0e2f1f4b..30059a8922bc 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -32,26 +32,15 @@ void I2CBusBase::Reset() m_slaves.clear(); } -I2CSlave* I2CBusBase::GetSlave(u8 slave_addr) -{ - for (I2CSlave* slave : m_slaves) - { - if (slave->Matches(slave_addr)) - return slave; - } - return nullptr; -} - int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) { - I2CSlave* slave = GetSlave(slave_addr); if (slave != nullptr) { for (int i = 0; i < count; i++) { // TODO: Does this make sense? The transmitter can't NACK a read... it's the receiver that // does that - auto byte = slave->ReadByte(addr + i); + auto byte = slave->ReadByte(); if (byte.has_value()) data_out[i] = byte.value(); else @@ -72,7 +61,7 @@ int I2CBusSimple::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { for (int i = 0; i < count; i++) { - if (!slave->WriteByte(addr + i, data_in[i])) + if (!slave->WriteByte(data_in[i])) return i; } return count; @@ -98,7 +87,6 @@ bool I2CBus::GetSDA() const return true; // passive pullup (or NACK) case State::SetI2CAddress: - case State::WriteDeviceAddress: case State::WriteToDevice: if (bit_counter < 8) return true; // passive pullup @@ -126,30 +114,20 @@ void I2CBus::Start() state = State::Activating; bit_counter = 0; current_byte = 0; - i2c_address.reset(); // Note: don't reset device_address, as it's re-used for reads } void I2CBus::Stop() { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); + for (I2CSlave* slave : m_slaves) + { + slave->Stop(); + } + state = State::Inactive; bit_counter = 0; current_byte = 0; - i2c_address.reset(); - device_address.reset(); -} - -bool I2CBus::WriteExpected() const -{ - // If we don't have an I²C address, it needs to be written (even if the address that is later - // written is a read). - // Otherwise, check the least significant bit; it being *clear* indicates a write. - const bool is_write = !i2c_address.has_value() || ((i2c_address.value() & 1) == 0); - // The device that is otherwise recieving instead transmits an acknowledge bit after each byte. - const bool acknowledge_expected = (bit_counter == 8); - - return is_write ^ acknowledge_expected; } void I2CBus::Update(const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda) @@ -193,26 +171,29 @@ void I2CBus::Update(const bool old_scl, const bool new_scl, const bool old_sda, void I2CBus::SCLRisingEdge(const bool sda) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} {} rising edge: {} (write expected: {})", bit_counter, state, - // sda, WriteExpected()); + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} rising edge: {}", bit_counter, state, sda); // SCL rising edge indicates data clocking. For reads, we set up data at this point. // For writes, we instead process it on the falling edge, to better distinguish // the start/stop condition. if (state == State::ReadFromDevice && bit_counter == 0) { // Start of a read. - ASSERT(device_address.has_value()); // Implied by the state transition in falling edge - ASSERT(i2c_address.has_value()); - I2CSlave* slave = GetSlave(i2c_address.value()); - ASSERT_MSG(WII_IPC, slave != nullptr, - "Expected device with ID {:02x} to be on the I2C bus as it was earlier", - i2c_address.value()); - std::optional byte = slave->ReadByte(device_address.value()); + std::optional byte = std::nullopt; + for (I2CSlave* slave : m_slaves) + { + std::optional byte2 = slave->ReadByte(); + if (byte.has_value() && byte2.has_value()) + { + WARN_LOG_FMT(WII_IPC, "Multiple slaves responded to read: {:02x} vs {:02x}", *byte, *byte2); + } + else if (byte2.has_value()) + { + byte = byte2; + } + } if (!byte.has_value()) { - WARN_LOG_FMT(WII_IPC, "Failed to read from device {:02x} at address {:02x}", - i2c_address.value(), device_address.value()); - // TODO + WARN_LOG_FMT(WII_IPC, "No slaves responded to I2C read"); current_byte = 0xff; } else @@ -227,11 +208,9 @@ void I2CBus::SCLRisingEdge(const bool sda) void I2CBus::SCLFallingEdge(const bool sda) { - // INFO_LOG_FMT(WII_IPC, "AVE: {} {} falling edge: {} (write expected: {})", bit_counter, state, - // sda, WriteExpected()); + // INFO_LOG_FMT(WII_IPC, "AVE: {} {} falling edge: {}", bit_counter, state, sda); // SCL falling edge is used to advance bit_counter/change states and process writes. - if (state == State::SetI2CAddress || state == State::WriteDeviceAddress || - state == State::WriteToDevice) + if (state == State::SetI2CAddress || state == State::WriteToDevice) { if (bit_counter == 8) { @@ -254,52 +233,61 @@ void I2CBus::SCLFallingEdge(const bool sda) // Write finished. if (state == State::SetI2CAddress) { - I2CSlave* slave = GetSlave(current_byte); - if (slave != nullptr) + bool got_ack = false; + for (I2CSlave* slave : m_slaves) + { + if (current_byte & 1) + { + if (slave->StartRead(current_byte >> 1)) + { + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple slaves ACKed starting read for I2C addr {:02x}", + current_byte); + } + got_ack = true; + } + } + else // (current_byte & 1) == 0 + { + if (slave->StartWrite(current_byte >> 1)) + { + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple slaves ACKed starting write for I2C addr {:02x}", + current_byte); + } + got_ack = true; + } + } + } + + if (!got_ack) { state = State::Inactive; // NACK WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); } else { + // State transition handled by bit_counter >= 8, as we still need to handle the ACK INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); } } - else if (state == State::WriteDeviceAddress) - { - device_address = current_byte; - INFO_LOG_FMT(WII_IPC, "AVE: Device address is {:02x}", current_byte); - } else { // Actual write ASSERT(state == State::WriteToDevice); - ASSERT(device_address.has_value()); // implied by state transition - ASSERT(i2c_address.has_value()); - I2CSlave* slave = GetSlave(i2c_address.value()); - ASSERT_MSG(WII_IPC, slave != nullptr, - "Expected device with ID {:02x} to be on the I2C bus as it was earlier", - i2c_address.value()); - if (slave == nullptr) + bool got_ack = false; + for (I2CSlave* slave : m_slaves) { - state = State::Inactive; // NACK - } - else - { - slave->WriteByte(device_address.value(), current_byte); - /* if (!IOS::ave_ever_logged[device_address.value()] || old_ave_value != current_byte) + if (slave->WriteByte(current_byte)) { - INFO_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), IOS::GetAVERegisterName(device_address.value())); - IOS::ave_ever_logged[device_address.value()] = true; + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple slaves responded to write of {:02x}", current_byte); + } + got_ack = true; } - else - { - DEBUG_LOG_FMT(WII_IPC, "AVE: Wrote {:02x} to {:02x} ({})", current_byte, - device_address.value(), - IOS::GetAVERegisterName(device_address.value())); - }*/ - device_address = device_address.value() + 1; } } } @@ -318,32 +306,17 @@ void I2CBus::SCLFallingEdge(const bool sda) { // Finished a byte and the acknowledge signal. bit_counter = 0; - switch (state) + if (state == State::SetI2CAddress) { - case State::SetI2CAddress: - i2c_address = current_byte; - // Note: i2c_address is known to correspond to a valid device + // Note: current_byte is known to correspond to a valid device if ((current_byte & 1) == 0) { - state = State::WriteDeviceAddress; - device_address.reset(); + state = State::WriteToDevice; } else { - if (device_address.has_value()) - { - state = State::ReadFromDevice; - } - else - { - state = State::Inactive; // NACK - required for 8-bit internal addresses - ERROR_LOG_FMT(WII_IPC, "AVE: Attempted to read device without having a read address!"); - } + state = State::ReadFromDevice; } - break; - case State::WriteDeviceAddress: - state = State::WriteToDevice; - break; } } else @@ -359,19 +332,114 @@ void I2CBus::DoState(PointerWrap& p) p.Do(state); p.Do(bit_counter); p.Do(current_byte); - p.Do(i2c_address); - p.Do(device_address); // TODO: verify m_devices is the same so that the state is compatible. // (We don't take ownership of saving/loading m_devices, though). } +bool I2CSlaveAutoIncrementing::StartWrite(u8 slave_addr) +{ + if (slave_addr == m_slave_addr) + { + INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} write started, previously active: {}", m_slave_addr, + m_active); + m_active = true; + m_device_address.reset(); + return true; + } + else + { + return false; + } +} + +bool I2CSlaveAutoIncrementing::StartRead(u8 slave_addr) +{ + if (slave_addr == m_slave_addr) + { + INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} read started, previously active: {}", m_slave_addr, + m_active); + if (m_device_address.has_value()) + { + m_active = true; + return true; + } + else + { + WARN_LOG_FMT(WII_IPC, + "I2C Device {:02x}: read attempted without having written device address", + m_slave_addr); + m_active = false; + return false; + } + } + else + { + return false; + } +} + +void I2CSlaveAutoIncrementing::Stop() +{ + m_active = false; + m_device_address.reset(); +} + +std::optional I2CSlaveAutoIncrementing::ReadByte() +{ + if (m_active) + { + ASSERT(m_device_address.has_value()); // enforced by StartRead + const u8 cur_addr = m_device_address.value(); + m_device_address = cur_addr + 1; // wrapping from 255 to 0 is the assumed behavior + return ReadByte(cur_addr); + } + else + { + return std::nullopt; + } +} + +bool I2CSlaveAutoIncrementing::WriteByte(u8 value) +{ + if (m_active) + { + if (m_device_address.has_value()) + { + WriteByte(m_device_address.value(), value); + } + else + { + m_device_address = value; + } + return true; + } + else + { + return false; + } +} + +void I2CSlaveAutoIncrementing::DoState(PointerWrap& p) +{ + u8 slave_addr = m_slave_addr; + p.Do(slave_addr); + if (slave_addr != m_slave_addr && p.IsReadMode()) + { + PanicAlertFmt("State incompatible: Mismatched I2C address: expected {:02x}, was {:02x}. " + "Aborting state load.", + m_slave_addr, slave_addr); + p.SetVerifyMode(); + } + p.Do(m_active); + p.Do(m_device_address); +} + }; // namespace Common template <> struct fmt::formatter : EnumFormatter { - static constexpr array_type names = {"Inactive", "Activating", - "Set I2C Address", "Write Device Address", + static constexpr array_type names = {"Inactive", "Activating", "Set I2C Address", "Write To Device", "Read From Device"}; constexpr formatter() : EnumFormatter(names) {} }; diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index 16fcd8dc06d2..7177830f00bc 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -16,31 +16,38 @@ namespace Common class I2CSlave { public: - virtual bool Matches(u8 slave_addr) = 0; - virtual std::optional ReadByte(u8 addr) = 0; - virtual bool WriteByte(u8 addr, u8 value) = 0; + virtual ~I2CSlave() = default; + + // NOTE: slave_addr is 7 bits, i.e. it has been shifted so the read flag is not included. + virtual bool StartWrite(u8 slave_addr) = 0; + virtual bool StartRead(u8 slave_addr) = 0; + virtual void Stop() = 0; + virtual std::optional ReadByte() = 0; + virtual bool WriteByte(u8 value) = 0; }; -template -class I2CSlaveSimple : I2CSlave +class I2CSlaveAutoIncrementing : I2CSlave { public: - bool Matches(u8 slave_addr) override { return slave_addr == slave_addr_val; } - std::optional ReadByte(u8 addr) { return data_bytes[addr]; } - bool WriteByte(u8 addr, u8 value) - { - data_bytes[addr] = value; - return true; - } + I2CSlaveAutoIncrementing(u8 slave_addr) : m_slave_addr(slave_addr) {} + virtual ~I2CSlaveAutoIncrementing() = default; - union - { - T data; - std::array data_bytes; - }; + bool StartWrite(u8 slave_addr) override; + bool StartRead(u8 slave_addr) override; + void Stop() override; + std::optional ReadByte() override; + bool WriteByte(u8 value) override; + + virtual void DoState(PointerWrap& p); - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(sizeof(T) == 0x100); +protected: + virtual u8 ReadByte(u8 addr) = 0; + virtual void WriteByte(u8 addr, u8 value) = 0; + +private: + const u8 m_slave_addr; + bool m_active = false; + std::optional m_device_address = std::nullopt; }; class I2CBusBase @@ -57,7 +64,6 @@ class I2CBusBase // Returns nullptr if there is no match I2CSlave* GetSlave(u8 slave_addr); -private: // Pointers are unowned: std::vector m_slaves; }; @@ -76,17 +82,6 @@ class I2CBusSimple : public I2CBusBase // - Timing is not implemented at all; the clock signal can be changed as fast as needed. // - Devices are not allowed to stretch the clock signal. (Nintendo's write code does not seem to // implement clock stretching in any case, though some homebrew does.) -// - All devices use a 1-byte auto-incrementing address which wraps around from 255 to 0. -// - The device address is handled by this I2CBus class, instead of the device itself. -// - The device address is set on writes, and re-used for reads; writing an address and data and -// then switching to reading uses the incremented address. Every write must specify the address. -// - Reading without setting the device address beforehand is disallowed; the I²C specification -// allows such reads but does not specify how they behave (or anything about the behavior of the -// device address). -// - Switching between multiple devices using a restart does not reset the device address; the -// device address is only reset on stopping. This means that a write to one device followed by a -// read from a different device would result in reading from the last used device address (without -// any warning). // - 10-bit addressing and other reserved addressing modes are not implemented. class I2CBus : public I2CBusBase { @@ -96,7 +91,6 @@ class I2CBus : public I2CBusBase Inactive, Activating, SetI2CAddress, - WriteDeviceAddress, WriteToDevice, ReadFromDevice, }; @@ -104,8 +98,6 @@ class I2CBus : public I2CBusBase State state; u8 bit_counter; u8 current_byte; - std::optional i2c_address; // Not shifted; includes the read flag - std::optional device_address; void Update(const bool old_scl, const bool new_scl, const bool old_sda, const bool new_sda); bool GetSCL() const; From 5893ea9660704a451580c1692527ccaf8458c317 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 13:41:50 -0700 Subject: [PATCH 28/34] Mostly refactored again --- Source/Core/Common/I2C.cpp | 230 ++++++++++++++++++++++++------------- Source/Core/Common/I2C.h | 13 ++- 2 files changed, 163 insertions(+), 80 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 30059a8922bc..43883d171a1f 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -32,44 +32,154 @@ void I2CBusBase::Reset() m_slaves.clear(); } -int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool I2CBusBase::StartWrite(u8 slave_addr) { - if (slave != nullptr) + bool got_ack = false; + for (I2CSlave* slave : m_slaves) { - for (int i = 0; i < count; i++) + if (slave->StartWrite(slave_addr)) { - // TODO: Does this make sense? The transmitter can't NACK a read... it's the receiver that - // does that - auto byte = slave->ReadByte(); - if (byte.has_value()) - data_out[i] = byte.value(); - else - return i; + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple I2C slaves ACKed starting write for I2C addr {:02x}", + slave_addr); + } + got_ack = true; } - return count; } - else + return got_ack; +} + +bool I2CBusBase::StartRead(u8 slave_addr) +{ + bool got_ack = false; + for (I2CSlave* slave : m_slaves) { - return 0; + if (slave->StartRead(slave_addr)) + { + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple I2C slaves ACKed starting read for I2C addr {:02x}", + slave_addr); + } + got_ack = true; + } + } + return got_ack; +} + +void I2CBusBase::Stop() +{ + for (I2CSlave* slave : m_slaves) + slave->Stop(); +} + +std::optional I2CBusBase::ReadByte() +{ + std::optional byte = std::nullopt; + for (I2CSlave* slave : m_slaves) + { + std::optional byte2 = slave->ReadByte(); + if (byte.has_value() && byte2.has_value()) + { + WARN_LOG_FMT(WII_IPC, "Multiple slaves responded to read: {:02x} vs {:02x}", *byte, *byte2); + } + else if (byte2.has_value()) + { + INFO_LOG_FMT(WII_IPC, "I2C: Read {:02x}", byte2.value()); + byte = byte2; + } } + return byte; +} + +bool I2CBusBase::WriteByte(u8 value) +{ + bool got_ack = false; + for (I2CSlave* slave : m_slaves) + { + if (slave->WriteByte(value)) + { + if (got_ack) + { + WARN_LOG_FMT(WII_IPC, "Multiple I2C slaves ACKed write of {:02x}", value); + } + got_ack = true; + } + } + return got_ack; } int I2CBusSimple::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) { - I2CSlave* slave = GetSlave(slave_addr); - if (slave != nullptr) + if (!StartWrite(slave_addr)) + { + WARN_LOG_FMT(WII_IPC, "I2C: Failed to start write to {:02x} ({:02x}, {:02x})", slave_addr, addr, + count); + Stop(); + return 0; + } + if (!WriteByte(addr)) { - for (int i = 0; i < count; i++) + WARN_LOG_FMT(WII_IPC, "I2C: Failed to write device address {:02x} to {:02x} ({:02x})", addr, + slave_addr, count); + Stop(); + return 0; + } + for (int i = 0; i < count; i++) + { + if (!WriteByte(data_in[i])) { - if (!slave->WriteByte(data_in[i])) - return i; + WARN_LOG_FMT(WII_IPC, + "I2C: Failed to byte {} of {} starting at device address {:02x} to {:02x}", i, + count, addr, slave_addr); + Stop(); + return i; } - return count; } - else + Stop(); + return count; +} + +int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +{ + if (!StartWrite(slave_addr)) { + WARN_LOG_FMT(WII_IPC, "I2C: Failed to start write for read from {:02x} ({:02x}, {:02x})", + slave_addr, addr, count); + Stop(); return 0; } + if (!WriteByte(addr)) + { + WARN_LOG_FMT(WII_IPC, "I2C: Failed to write device address {:02x} to {:02x} ({:02x})", addr, + slave_addr, count); + Stop(); + return 0; + } + // Note: No Stop() call before StartRead. + if (!StartRead(slave_addr)) + { + WARN_LOG_FMT(WII_IPC, "I2C: Failed to start read from {:02x} ({:02x}, {:02x})", + slave_addr, addr, count); + Stop(); + return 0; + } + for (int i = 0; i < count; i++) + { + const std::optional byte = ReadByte(); + if (!byte.has_value()) + { + WARN_LOG_FMT(WII_IPC, + "I2C: Failed to byte {} of {} starting at device address {:02x} from {:02x}", i, + count, addr, slave_addr); + Stop(); + return i; + } + data_out[i] = byte.value(); + } + Stop(); + return count; } bool I2CBus::GetSCL() const @@ -120,10 +230,7 @@ void I2CBus::Start() void I2CBus::Stop() { INFO_LOG_FMT(WII_IPC, "AVE: Stop I2C"); - for (I2CSlave* slave : m_slaves) - { - slave->Stop(); - } + I2CBusBase::Stop(); state = State::Inactive; bit_counter = 0; @@ -178,19 +285,7 @@ void I2CBus::SCLRisingEdge(const bool sda) if (state == State::ReadFromDevice && bit_counter == 0) { // Start of a read. - std::optional byte = std::nullopt; - for (I2CSlave* slave : m_slaves) - { - std::optional byte2 = slave->ReadByte(); - if (byte.has_value() && byte2.has_value()) - { - WARN_LOG_FMT(WII_IPC, "Multiple slaves responded to read: {:02x} vs {:02x}", *byte, *byte2); - } - else if (byte2.has_value()) - { - byte = byte2; - } - } + const std::optional byte = ReadByte(); if (!byte.has_value()) { WARN_LOG_FMT(WII_IPC, "No slaves responded to I2C read"); @@ -233,61 +328,42 @@ void I2CBus::SCLFallingEdge(const bool sda) // Write finished. if (state == State::SetI2CAddress) { - bool got_ack = false; - for (I2CSlave* slave : m_slaves) + const u8 slave_addr = current_byte >> 1; + if (current_byte & 1) { - if (current_byte & 1) + if (StartRead(slave_addr)) { - if (slave->StartRead(current_byte >> 1)) - { - if (got_ack) - { - WARN_LOG_FMT(WII_IPC, "Multiple slaves ACKed starting read for I2C addr {:02x}", - current_byte); - } - got_ack = true; - } + // State transition handled by bit_counter >= 8, as we still need to handle the ACK + INFO_LOG_FMT(WII_IPC, "I2C: Start read from {:02x}", slave_addr); } - else // (current_byte & 1) == 0 + else { - if (slave->StartWrite(current_byte >> 1)) - { - if (got_ack) - { - WARN_LOG_FMT(WII_IPC, "Multiple slaves ACKed starting write for I2C addr {:02x}", - current_byte); - } - got_ack = true; - } + state = State::Inactive; // NACK + WARN_LOG_FMT(WII_IPC, "I2C: No device responded to read from {:02x}", current_byte); } } - - if (!got_ack) - { - state = State::Inactive; // NACK - WARN_LOG_FMT(WII_IPC, "AVE: Unknown I2C address {:02x}", current_byte); - } else { - // State transition handled by bit_counter >= 8, as we still need to handle the ACK - INFO_LOG_FMT(WII_IPC, "AVE: I2C address is {:02x}", current_byte); + if (StartWrite(slave_addr)) + { + // State transition handled by bit_counter >= 8, as we still need to handle the ACK + INFO_LOG_FMT(WII_IPC, "I2C: Start write to {:02x}", slave_addr); + } + else + { + state = State::Inactive; // NACK + WARN_LOG_FMT(WII_IPC, "I2C: No device responded to write to {:02x}", current_byte); + } } } else { // Actual write ASSERT(state == State::WriteToDevice); - bool got_ack = false; - for (I2CSlave* slave : m_slaves) + if (!WriteByte(current_byte)) { - if (slave->WriteByte(current_byte)) - { - if (got_ack) - { - WARN_LOG_FMT(WII_IPC, "Multiple slaves responded to write of {:02x}", current_byte); - } - got_ack = true; - } + state = State::Inactive; + WARN_LOG_FMT(WII_IPC, "I2C: Write of {:02x} NACKed", current_byte); } } } diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index 7177830f00bc..df13e03585d5 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -22,6 +22,8 @@ class I2CSlave virtual bool StartWrite(u8 slave_addr) = 0; virtual bool StartRead(u8 slave_addr) = 0; virtual void Stop() = 0; + // NOTE: std::optional is for ease of implementation. I2C does not provide a way for the + // transmitting device to NACK a read; only the receiver can NACK. virtual std::optional ReadByte() = 0; virtual bool WriteByte(u8 value) = 0; }; @@ -61,9 +63,14 @@ class I2CBusBase void Reset(); protected: - // Returns nullptr if there is no match - I2CSlave* GetSlave(u8 slave_addr); + // Signals all slaves on the bus + bool StartWrite(u8 slave_addr); + bool StartRead(u8 slave_addr); + void Stop(); + std::optional ReadByte(); + bool WriteByte(u8 value); +private: // Pointers are unowned: std::vector m_slaves; }; @@ -72,8 +79,8 @@ class I2CBusSimple : public I2CBusBase { public: // TODO: change int to u16 or something - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); + int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); }; // An I²C bus implementation accessed via bit-banging. From c553cd5dbf9e944536efb93fe88007c8ececc965 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 14:00:21 -0700 Subject: [PATCH 29/34] Refactoring finished; AVE working again --- Source/Core/Common/I2C.cpp | 4 +- Source/Core/Common/I2C.h | 25 +++++- Source/Core/Core/HW/WII_IPC.cpp | 143 ++++++++++++++++++++------------ 3 files changed, 116 insertions(+), 56 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 43883d171a1f..82c2a57e9f1a 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -295,8 +295,6 @@ void I2CBus::SCLRisingEdge(const bool sda) { current_byte = byte.value(); } - // INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", device_address.value(), - // IOS::GetAVERegisterName(device_address.value()), current_byte); } // Dolphin_Debugger::PrintCallstack(Common::Log::LogType::WII_IPC, Common::Log::LogLevel::LINFO); } @@ -508,6 +506,8 @@ void I2CSlaveAutoIncrementing::DoState(PointerWrap& p) } p.Do(m_active); p.Do(m_device_address); + + DoDeviceState(p); } }; // namespace Common diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index df13e03585d5..61b3c518bda1 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -26,9 +26,28 @@ class I2CSlave // transmitting device to NACK a read; only the receiver can NACK. virtual std::optional ReadByte() = 0; virtual bool WriteByte(u8 value) = 0; + +protected: + template + static u8 RawRead(T* reg_data, u8 addr) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + return reinterpret_cast(reg_data)[addr]; + } + + template + static void RawWrite(T* reg_data, u8 addr, u8 value) + { + static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); + static_assert(0x100 == sizeof(T)); + + reinterpret_cast(reg_data)[addr] = value; + } }; -class I2CSlaveAutoIncrementing : I2CSlave +class I2CSlaveAutoIncrementing : public I2CSlave { public: I2CSlaveAutoIncrementing(u8 slave_addr) : m_slave_addr(slave_addr) {} @@ -40,12 +59,14 @@ class I2CSlaveAutoIncrementing : I2CSlave std::optional ReadByte() override; bool WriteByte(u8 value) override; - virtual void DoState(PointerWrap& p); + void DoState(PointerWrap& p); protected: virtual u8 ReadByte(u8 addr) = 0; virtual void WriteByte(u8 addr, u8 value) = 0; + virtual void DoDeviceState(PointerWrap& p) = 0; + private: const u8 m_slave_addr; bool m_active = false; diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 76bb5f9476c1..cd2647e07a09 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -120,12 +120,97 @@ struct AVEState }; #pragma pack() static_assert(sizeof(AVEState) == 0x100); -Common::I2CSlaveSimple<0x70, AVEState> ave_state; -std::bitset ave_ever_logged; -Common::I2CBus i2c_state; +std::string_view GetAVERegisterName(u8 address) +{ + if (address == 0x00) + return "A/V Timings"; + else if (address == 0x01) + return "Video Output configuration"; + else if (address == 0x02) + return "Vertical blanking interval (VBI) control"; + else if (address == 0x03) + return "Composite Video Trap Filter control"; + else if (address == 0x04) + return "A/V output control"; + else if (address == 0x05 || address == 0x06) + return "CGMS protection"; + else if (address == 0x08 || address == 0x09) + return "WSS (Widescreen signaling)"; + else if (address == 0x0A) + return "RGB color output control"; + else if (address >= 0x10 && address <= 0x30) + return "Gamma coefficients"; + else if (address >= 0x40 && address <= 0x59) + return "Macrovision code"; + else if (address == 0x62) + return "RGB switch control"; + else if (address == 0x65) + return "Color DAC control"; + else if (address == 0x67) + return "Color Test"; + else if (address == 0x6A) + return "CCSEL"; + else if (address == 0x6D) + return "Audio mute control"; + else if (address == 0x6E) + return "RGB output filter"; + else if (address == 0x71) + return "Audio stereo output control - right volume"; + else if (address == 0x72) + return "Audio stereo output control - right volume"; + else if (address >= 0x7a && address <= 0x7d) + return "Closed Captioning control"; + else + return fmt::format("Unknown ({:02x})", address); +} + +class AVEDevice : public Common::I2CSlaveAutoIncrementing +{ +public: + AVEDevice() : I2CSlaveAutoIncrementing(0x70) {} + + void Reset() + { + m_registers = {}; + ave_ever_logged = {}; + } + + AVEState m_registers{}; + +protected: + u8 ReadByte(u8 addr) override + { + const u8 result = RawRead(&m_registers, addr); + INFO_LOG_FMT(WII_IPC, "AVE: Read from {:02x} ({}) -> {:02x}", addr, GetAVERegisterName(addr), + result); + return result; + } + void WriteByte(u8 addr, u8 value) override + { + const u8 old_value = RawRead(&m_registers, addr); + RawWrite(&m_registers, addr, value); + + if (old_value != value || !ave_ever_logged[addr]) + { + INFO_LOG_FMT(WII_IPC, "AVE: Write to {:02x} ({}): {:02x} -> {:02x}", addr, + GetAVERegisterName(addr), old_value, value); + ave_ever_logged[addr] = true; + } + else + { + DEBUG_LOG_FMT(WII_IPC, "AVE: Write to {:02x} ({}): {:02x}", addr, GetAVERegisterName(addr), + value); + } + } + void DoDeviceState(PointerWrap& p) override { p.Do(m_registers); } -static CoreTiming::EventType* updateInterrupts; +private: + std::bitset ave_ever_logged{}; // logging only, not saved +}; +AVEDevice ave_state; + +Common::I2CBus i2c_state; WiiIPC::WiiIPC(Core::System& system) : m_system(system) { @@ -145,7 +230,7 @@ void WiiIPC::DoState(PointerWrap& p) p.Do(m_gpio_dir); p.Do(m_gpio_out); i2c_state.DoState(p); - p.Do(ave_state.data); + ave_state.DoState(p); p.Do(m_resets); } @@ -177,9 +262,7 @@ void WiiIPC::InitState() m_ppc_irq_masks |= INT_CAUSE_IPC_BROADWAY; i2c_state = {}; - ave_state = {}; - ave_state.data.video_output_config = 0x23; - ave_ever_logged.reset(); + ave_state.Reset(); } void WiiIPC::Init() @@ -199,50 +282,6 @@ void WiiIPC::Shutdown() { } -std::string_view GetAVERegisterName(u8 address) -{ - if (address == 0x00) - return "A/V Timings"; - else if (address == 0x01) - return "Video Output configuration"; - else if (address == 0x02) - return "Vertical blanking interval (VBI) control"; - else if (address == 0x03) - return "Composite Video Trap Filter control"; - else if (address == 0x04) - return "A/V output control"; - else if (address == 0x05 || address == 0x06) - return "CGMS protection"; - else if (address == 0x08 || address == 0x09) - return "WSS (Widescreen signaling)"; - else if (address == 0x0A) - return "RGB color output control"; - else if (address >= 0x10 && address <= 0x30) - return "Gamma coefficients"; - else if (address >= 0x40 && address <= 0x59) - return "Macrovision code"; - else if (address == 0x62) - return "RGB switch control"; - else if (address == 0x65) - return "Color DAC control"; - else if (address == 0x67) - return "Color Test"; - else if (address == 0x6A) - return "CCSEL"; - else if (address == 0x6D) - return "Audio mute control"; - else if (address == 0x6E) - return "RGB output filter"; - else if (address == 0x71) - return "Audio stereo output control - right volume"; - else if (address == 0x72) - return "Audio stereo output control - right volume"; - else if (address >= 0x7a && address <= 0x7d) - return "Closed Captioning control"; - else - return fmt::format("Unknown ({:02x})", address); -} - static u32 ReadGPIOIn(Core::System& system) { Common::Flags gpio_in; From 0658170681059d81e2638e2f44dd97b6b349da75 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 15:12:12 -0700 Subject: [PATCH 30/34] Fix reads --- Source/Core/Common/I2C.cpp | 24 ++++++++++++------------ Source/Core/Core/HW/WII_IPC.cpp | 3 +++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 82c2a57e9f1a..9f572d9d4f56 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -305,16 +305,7 @@ void I2CBus::SCLFallingEdge(const bool sda) // SCL falling edge is used to advance bit_counter/change states and process writes. if (state == State::SetI2CAddress || state == State::WriteToDevice) { - if (bit_counter == 8) - { - // Acknowledge bit for *reads*. - if (sda) - { - WARN_LOG_FMT(WII_IPC, "Read NACK'd"); - state = State::Inactive; - } - } - else + if (bit_counter != 8) { current_byte <<= 1; if (sda) @@ -337,7 +328,7 @@ void I2CBus::SCLFallingEdge(const bool sda) else { state = State::Inactive; // NACK - WARN_LOG_FMT(WII_IPC, "I2C: No device responded to read from {:02x}", current_byte); + WARN_LOG_FMT(WII_IPC, "I2C: No device responded to read from {:02x}", slave_addr); } } else @@ -350,7 +341,7 @@ void I2CBus::SCLFallingEdge(const bool sda) else { state = State::Inactive; // NACK - WARN_LOG_FMT(WII_IPC, "I2C: No device responded to write to {:02x}", current_byte); + WARN_LOG_FMT(WII_IPC, "I2C: No device responded to write to {:02x}", slave_addr); } } } @@ -392,6 +383,15 @@ void I2CBus::SCLFallingEdge(const bool sda) state = State::ReadFromDevice; } } + else if (state == State::ReadFromDevice) + { + // Acknowledge bit for *reads*. + if (sda) + { + WARN_LOG_FMT(WII_IPC, "Read NACK'd"); + state = State::Inactive; + } + } } else { diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index cd2647e07a09..817261780b54 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -261,8 +261,11 @@ void WiiIPC::InitState() m_ppc_irq_masks |= INT_CAUSE_IPC_BROADWAY; + ave_state.Reset(); i2c_state = {}; ave_state.Reset(); + ave_state.m_registers.video_output_config = 0x23; + i2c_state.AddSlave(&ave_state); } void WiiIPC::Init() From 28b082f46d529dd4aaef3418163ce35aac586136 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 15:23:18 -0700 Subject: [PATCH 31/34] One more fix --- Source/Core/Common/I2C.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index 9f572d9d4f56..aab3f7cb924b 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -160,8 +160,8 @@ int I2CBusSimple::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) // Note: No Stop() call before StartRead. if (!StartRead(slave_addr)) { - WARN_LOG_FMT(WII_IPC, "I2C: Failed to start read from {:02x} ({:02x}, {:02x})", - slave_addr, addr, count); + WARN_LOG_FMT(WII_IPC, "I2C: Failed to start read from {:02x} ({:02x}, {:02x})", slave_addr, + addr, count); Stop(); return 0; } @@ -479,7 +479,9 @@ bool I2CSlaveAutoIncrementing::WriteByte(u8 value) { if (m_device_address.has_value()) { - WriteByte(m_device_address.value(), value); + const u8 cur_addr = m_device_address.value(); + m_device_address = cur_addr + 1; // wrapping from 255 to 0 is the assumed behavior + WriteByte(cur_addr, value); } else { From e2defde8935570dbf3617afcfd402d1f39da86d7 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 6 Sep 2022 17:35:38 -0700 Subject: [PATCH 32/34] It builds! --- Source/Core/Common/I2C.cpp | 6 +- Source/Core/Common/I2C.h | 18 +- Source/Core/Core/HW/WII_IPC.cpp | 7 +- Source/Core/Core/HW/WiimoteEmu/Camera.cpp | 25 +- Source/Core/Core/HW/WiimoteEmu/Camera.h | 13 +- .../HW/WiimoteEmu/Extension/Extension.cpp | 45 ++-- .../Core/HW/WiimoteEmu/Extension/Extension.h | 36 ++- .../Core/Core/HW/WiimoteEmu/ExtensionPort.cpp | 2 +- .../Core/Core/HW/WiimoteEmu/ExtensionPort.h | 6 +- Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp | 53 ---- Source/Core/Core/HW/WiimoteEmu/I2CBus.h | 75 ------ Source/Core/Core/HW/WiimoteEmu/MotionPlus.cpp | 230 +++++++++++------- Source/Core/Core/HW/WiimoteEmu/MotionPlus.h | 53 +++- Source/Core/Core/HW/WiimoteEmu/Speaker.cpp | 103 +++++++- Source/Core/Core/HW/WiimoteEmu/Speaker.h | 14 +- Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h | 4 +- 16 files changed, 390 insertions(+), 300 deletions(-) diff --git a/Source/Core/Common/I2C.cpp b/Source/Core/Common/I2C.cpp index aab3f7cb924b..c8acb3511c78 100644 --- a/Source/Core/Common/I2C.cpp +++ b/Source/Core/Common/I2C.cpp @@ -412,7 +412,7 @@ void I2CBus::DoState(PointerWrap& p) bool I2CSlaveAutoIncrementing::StartWrite(u8 slave_addr) { - if (slave_addr == m_slave_addr) + if (DeviceEnabled() && slave_addr == m_slave_addr) { INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} write started, previously active: {}", m_slave_addr, m_active); @@ -428,7 +428,7 @@ bool I2CSlaveAutoIncrementing::StartWrite(u8 slave_addr) bool I2CSlaveAutoIncrementing::StartRead(u8 slave_addr) { - if (slave_addr == m_slave_addr) + if (DeviceEnabled() && slave_addr == m_slave_addr) { INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} read started, previously active: {}", m_slave_addr, m_active); @@ -508,8 +508,6 @@ void I2CSlaveAutoIncrementing::DoState(PointerWrap& p) } p.Do(m_active); p.Do(m_device_address); - - DoDeviceState(p); } }; // namespace Common diff --git a/Source/Core/Common/I2C.h b/Source/Core/Common/I2C.h index 61b3c518bda1..e2bb96a06b76 100644 --- a/Source/Core/Common/I2C.h +++ b/Source/Core/Common/I2C.h @@ -47,7 +47,7 @@ class I2CSlave } }; -class I2CSlaveAutoIncrementing : public I2CSlave +class I2CSlaveAutoIncrementing : public virtual I2CSlave { public: I2CSlaveAutoIncrementing(u8 slave_addr) : m_slave_addr(slave_addr) {} @@ -59,14 +59,14 @@ class I2CSlaveAutoIncrementing : public I2CSlave std::optional ReadByte() override; bool WriteByte(u8 value) override; - void DoState(PointerWrap& p); + virtual void DoState(PointerWrap& p); protected: + // i.e. should the device respond on the bus + virtual bool DeviceEnabled() { return true; } virtual u8 ReadByte(u8 addr) = 0; virtual void WriteByte(u8 addr, u8 value) = 0; - virtual void DoDeviceState(PointerWrap& p) = 0; - private: const u8 m_slave_addr; bool m_active = false; @@ -104,6 +104,16 @@ class I2CBusSimple : public I2CBusBase int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); }; +class I2CBusForwarding : public I2CBusSimple +{ +public: + using I2CBusBase::ReadByte; + using I2CBusBase::StartRead; + using I2CBusBase::StartWrite; + using I2CBusBase::Stop; + using I2CBusBase::WriteByte; +}; + // An I²C bus implementation accessed via bit-banging. // A few assumptions and limitations exist: // - All devices support both writes and reads. diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 817261780b54..5cf7ae22cb82 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -176,6 +176,12 @@ class AVEDevice : public Common::I2CSlaveAutoIncrementing ave_ever_logged = {}; } + void DoState(PointerWrap& p) override + { + I2CSlaveAutoIncrementing::DoState(p); + p.Do(m_registers); + } + AVEState m_registers{}; protected: @@ -203,7 +209,6 @@ class AVEDevice : public Common::I2CSlaveAutoIncrementing value); } } - void DoDeviceState(PointerWrap& p) override { p.Do(m_registers); } private: std::bitset ave_ever_logged{}; // logging only, not saved diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp index 1dcdb7795200..14efaed6b831 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.cpp @@ -24,31 +24,26 @@ void CameraLogic::Reset() void CameraLogic::DoState(PointerWrap& p) { + I2CSlaveAutoIncrementing::DoState(p); + p.Do(m_reg_data); // FYI: m_is_enabled is handled elsewhere. } -int CameraLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool CameraLogic::DeviceEnabled() { - if (I2C_ADDR != slave_addr) - return 0; - - if (!m_is_enabled) - return 0; - - return RawRead(&m_reg_data, addr, count, data_out); + return m_is_enabled; } -int CameraLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +u8 CameraLogic::ReadByte(u8 addr) { - if (I2C_ADDR != slave_addr) - return 0; - - if (!m_is_enabled) - return 0; + return RawRead(&m_reg_data, addr); +} - return RawWrite(&m_reg_data, addr, count, data_in); +void CameraLogic::WriteByte(u8 addr, u8 value) +{ + RawWrite(&m_reg_data, addr, value); } std::array diff --git a/Source/Core/Core/HW/WiimoteEmu/Camera.h b/Source/Core/Core/HW/WiimoteEmu/Camera.h index 23e21cc84e43..6942cf12d2df 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Camera.h +++ b/Source/Core/Core/HW/WiimoteEmu/Camera.h @@ -5,8 +5,8 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Dynamics.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControlGroup/Cursor.h" namespace Common @@ -101,9 +101,11 @@ struct IRFull : IRExtended }; static_assert(sizeof(IRFull) == 9, "Wrong size"); -class CameraLogic : public I2CSlave +class CameraLogic : public Common::I2CSlaveAutoIncrementing { public: + CameraLogic() : I2CSlaveAutoIncrementing(I2C_ADDR) {} + // OEM sensor bar distance between LED clusters in meters. static constexpr float SENSOR_BAR_LED_SEPARATION = 0.2f; @@ -132,7 +134,7 @@ class CameraLogic : public I2CSlave static constexpr int MAX_POINT_SIZE = 15; void Reset(); - void DoState(PointerWrap& p); + void DoState(PointerWrap& p) override; static std::array GetCameraPoints(const Common::Matrix44& transform, Common::Vec2 field_of_view); void Update(const std::array& camera_points); @@ -176,8 +178,9 @@ class CameraLogic : public I2CSlave static const u8 REPORT_DATA_OFFSET = offsetof(Register, camera_data); private: - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool DeviceEnabled() override; + u8 ReadByte(u8 addr) override; + void WriteByte(u8 addr, u8 value) override; Register m_reg_data{}; diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp index fd2b18587276..55e7ebecfa7f 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.cpp @@ -71,14 +71,29 @@ void None::DoState(PointerWrap& p) // Nothing needed. } -int None::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool None::StartWrite(u8 slave_addr) { - return 0; + return false; +} + +bool None::StartRead(u8 slave_addr) +{ + return false; +} + +void None::Stop() +{ + // Nothing needed. } -int None::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +std::optional None::ReadByte() { - return 0; + return std::nullopt; +} + +bool None::WriteByte(u8 value) +{ + return false; } bool EncryptedExtension::ReadDeviceDetectPin() const @@ -86,11 +101,8 @@ bool EncryptedExtension::ReadDeviceDetectPin() const return true; } -int EncryptedExtension::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +u8 EncryptedExtension::ReadByte(u8 addr) { - if (I2C_ADDR != slave_addr) - return 0; - if (0x00 == addr) { // This is where real hardware would update controller data @@ -98,7 +110,7 @@ int EncryptedExtension::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) // TAS code fails to sync data reads and such.. } - auto const result = RawRead(&m_reg, addr, count, data_out); + u8 result = RawRead(&m_reg, addr); // Encrypt data read from extension register. if (ENCRYPTION_ENABLED == m_reg.encryption) @@ -109,30 +121,25 @@ int EncryptedExtension::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) m_is_key_dirty = false; } - ext_key.Encrypt(data_out, addr, count); + ext_key.Encrypt(&result, addr, 1); } return result; } -int EncryptedExtension::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +void EncryptedExtension::WriteByte(u8 addr, u8 value) { - if (I2C_ADDR != slave_addr) - return 0; - - auto const result = RawWrite(&m_reg, addr, count, data_in); + RawWrite(&m_reg, addr, value); constexpr u8 ENCRYPTION_KEY_DATA_BEGIN = offsetof(Register, encryption_key_data); constexpr u8 ENCRYPTION_KEY_DATA_END = ENCRYPTION_KEY_DATA_BEGIN + 0x10; - if (addr + count > ENCRYPTION_KEY_DATA_BEGIN && addr < ENCRYPTION_KEY_DATA_END) + if (addr >= ENCRYPTION_KEY_DATA_BEGIN && addr < ENCRYPTION_KEY_DATA_END) { // FYI: Real extensions seem to require the key data written in specifically sized chunks. // We just run the key generation on all writes to the key area. m_is_key_dirty = true; } - - return result; } void EncryptedExtension::Reset() @@ -147,6 +154,8 @@ void EncryptedExtension::Reset() void EncryptedExtension::DoState(PointerWrap& p) { + I2CSlaveAutoIncrementing::DoState(p); + p.Do(m_reg); if (p.IsReadMode()) diff --git a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h index 4a055c597db3..bafab30a0621 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h +++ b/Source/Core/Core/HW/WiimoteEmu/Extension/Extension.h @@ -10,15 +10,19 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Encryption.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "InputCommon/ControllerEmu/ControllerEmu.h" +#ifdef _MSC_VER +#pragma warning(disable : 4250) // C4250 inherits via dominance - intended behavior here +#endif + namespace WiimoteEmu { struct DesiredExtensionState; -class Extension : public ControllerEmu::EmulatedController, public I2CSlave +class Extension : public ControllerEmu::EmulatedController, public virtual Common::I2CSlave { public: explicit Extension(const char* name); @@ -56,23 +60,39 @@ class None : public Extension void Reset() override; void DoState(PointerWrap& p) override; - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool StartWrite(u8 slave_addr) override; + bool StartRead(u8 slave_addr) override; + void Stop() override; + std::optional ReadByte() override; + bool WriteByte(u8 value) override; }; // This class provides the encryption and initialization behavior of most extensions. -class EncryptedExtension : public Extension +class EncryptedExtension : public Extension, public Common::I2CSlaveAutoIncrementing { public: static constexpr u8 I2C_ADDR = 0x52; static constexpr int CONTROLLER_DATA_BYTES = 21; - using Extension::Extension; + explicit EncryptedExtension(const char* name) + : Extension(name), I2CSlaveAutoIncrementing(I2C_ADDR) + { + } + EncryptedExtension(const char* config_name, const char* display_name) + : Extension(config_name, display_name), I2CSlaveAutoIncrementing(I2C_ADDR) + { + } // TODO: This is public for TAS reasons. // TODO: TAS handles encryption poorly. EncryptionKey ext_key; + using I2CSlaveAutoIncrementing::ReadByte; + using I2CSlaveAutoIncrementing::StartRead; + using I2CSlaveAutoIncrementing::StartWrite; + using I2CSlaveAutoIncrementing::Stop; + using I2CSlaveAutoIncrementing::WriteByte; + static constexpr int CALIBRATION_CHECKSUM_BYTES = 2; #pragma pack(push, 1) @@ -117,8 +137,8 @@ class EncryptedExtension : public Extension bool ReadDeviceDetectPin() const override; - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + u8 ReadByte(u8 addr) override; + void WriteByte(u8 addr, u8 value) override; }; class Extension1stParty : public EncryptedExtension diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp index 97e73ea4bc90..2b152686b8c2 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.cpp @@ -7,7 +7,7 @@ namespace WiimoteEmu { -ExtensionPort::ExtensionPort(I2CBus* i2c_bus) : m_i2c_bus(*i2c_bus) +ExtensionPort::ExtensionPort(Common::I2CBusBase* i2c_bus) : m_i2c_bus(*i2c_bus) { } diff --git a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h index 86b1fd31f962..bb2f0ec01210 100644 --- a/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h +++ b/Source/Core/Core/HW/WiimoteEmu/ExtensionPort.h @@ -4,8 +4,8 @@ #pragma once #include "Common/ChunkFile.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteEmu/Extension/Extension.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" namespace WiimoteEmu { @@ -35,14 +35,14 @@ class ExtensionPort static constexpr u8 REPORT_I2C_SLAVE = 0x52; static constexpr u8 REPORT_I2C_ADDR = 0x00; - explicit ExtensionPort(I2CBus* i2c_bus); + explicit ExtensionPort(Common::I2CBusBase* i2c_bus); bool IsDeviceConnected() const; void AttachExtension(Extension* dev); private: Extension* m_extension = nullptr; - I2CBus& m_i2c_bus; + Common::I2CBusBase& m_i2c_bus; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp index 19202350fd64..e69de29bb2d1 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.cpp @@ -1,53 +0,0 @@ -// Copyright 2019 Dolphin Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#include "Core/HW/WiimoteEmu/I2CBus.h" - -#include - -namespace WiimoteEmu -{ -void I2CBus::AddSlave(I2CSlave* slave) -{ - m_slaves.emplace_back(slave); -} - -void I2CBus::RemoveSlave(I2CSlave* slave) -{ - m_slaves.erase(std::remove(m_slaves.begin(), m_slaves.end(), slave), m_slaves.end()); -} - -void I2CBus::Reset() -{ - m_slaves.clear(); -} - -int I2CBus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) -{ - for (auto& slave : m_slaves) - { - auto const bytes_read = slave->BusRead(slave_addr, addr, count, data_out); - - // A slave responded, we are done. - if (bytes_read) - return bytes_read; - } - - return 0; -} - -int I2CBus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) -{ - for (auto& slave : m_slaves) - { - auto const bytes_written = slave->BusWrite(slave_addr, addr, count, data_in); - - // A slave responded, we are done. - if (bytes_written) - return bytes_written; - } - - return 0; -} - -} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h index 3a1e0a67d361..e69de29bb2d1 100644 --- a/Source/Core/Core/HW/WiimoteEmu/I2CBus.h +++ b/Source/Core/Core/HW/WiimoteEmu/I2CBus.h @@ -1,75 +0,0 @@ -// Copyright 2019 Dolphin Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#pragma once - -#include -#include - -#include "Common/CommonTypes.h" - -namespace WiimoteEmu -{ -class I2CBus; - -class I2CSlave -{ - friend I2CBus; - -protected: - virtual ~I2CSlave() = default; - - virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0; - virtual int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) = 0; - - template - static int RawRead(T* reg_data, u8 addr, int count, u8* data_out) - { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* src = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - src)); - - std::copy_n(src, count, data_out); - - return count; - } - - template - static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in) - { - static_assert(std::is_standard_layout_v && std::is_trivially_copyable_v); - static_assert(0x100 == sizeof(T)); - - // TODO: addr wraps around after 0xff - - u8* dst = reinterpret_cast(reg_data) + addr; - count = std::min(count, int(reinterpret_cast(reg_data + 1) - dst)); - - std::copy_n(data_in, count, dst); - - return count; - } -}; - -class I2CBus -{ -public: - void AddSlave(I2CSlave* slave); - void RemoveSlave(I2CSlave* slave); - - void Reset(); - - // TODO: change int to u16 or something - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out); - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in); - -private: - // Pointers are unowned: - std::vector m_slaves; -}; - -} // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.cpp b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.cpp index 9dc3b2ad1cfc..f7c5ab50a0e7 100644 --- a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.cpp @@ -189,135 +189,187 @@ ExtensionPort& MotionPlus::GetExtPort() return m_extension_port; } -int MotionPlus::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool MotionPlus::StartWrite(u8 slave_addr) { switch (GetActivationStatus()) { case ActivationStatus::Inactive: - if (INACTIVE_DEVICE_ADDR != slave_addr) - { - // Passthrough to the connected extension. (if any) - return m_i2c_bus.BusRead(slave_addr, addr, count, data_out); - } - - // Perform a normal read of the M+ register. - return RawRead(&m_reg_data, addr, count, data_out); + // m_inactive_wrapper is connected to the bus, so it will respond to 0x53 when inactive. + return m_i2c_bus.StartWrite(slave_addr); case ActivationStatus::Active: // FYI: Motion plus does not respond to 0x53 when activated. - if (ACTIVE_DEVICE_ADDR != slave_addr) - { - // No i2c passthrough when activated. - return 0; - } - - // Perform a normal read of the M+ register. - return RawRead(&m_reg_data, addr, count, data_out); + // No i2c passthrough when activated, so *only* target 0x52. + return m_active_wrapper.StartWrite(slave_addr); default: case ActivationStatus::Activating: case ActivationStatus::Deactivating: // The extension port is completely unresponsive here. - return 0; + return false; } } -int MotionPlus::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +bool MotionPlus::StartRead(u8 slave_addr) { switch (GetActivationStatus()) { case ActivationStatus::Inactive: - { - if (INACTIVE_DEVICE_ADDR != slave_addr) - { - // Passthrough to the connected extension. (if any) - return m_i2c_bus.BusWrite(slave_addr, addr, count, data_in); - } + // m_inactive_wrapper is connected to the bus, so it will respond to 0x53 when inactive. + return m_i2c_bus.StartRead(slave_addr); - DEBUG_LOG_FMT(WIIMOTE, "Inactive M+ write {:#x} : {}", addr, ArrayToString(data_in, count)); + case ActivationStatus::Active: + // FYI: Motion plus does not respond to 0x53 when activated. + // No i2c passthrough when activated, so *only* target 0x52. + return m_active_wrapper.StartRead(slave_addr); - auto const result = RawWrite(&m_reg_data, addr, count, data_in); + default: + case ActivationStatus::Activating: + case ActivationStatus::Deactivating: + // The extension port is completely unresponsive here. + return false; + } +} - if (PASSTHROUGH_MODE_OFFSET == addr) - { - OnPassthroughModeWrite(); - } +void MotionPlus::Stop() +{ + switch (GetActivationStatus()) + { + case ActivationStatus::Inactive: + // m_inactive_wrapper is connected to the bus, so it will respond to 0x53 when inactive. + m_i2c_bus.Stop(); + break; + + case ActivationStatus::Active: + // FYI: Motion plus does not respond to 0x53 when activated. + // No i2c passthrough when activated, so *only* target 0x52. + m_active_wrapper.Stop(); + break; - return result; + default: + case ActivationStatus::Activating: + case ActivationStatus::Deactivating: + // The extension port is completely unresponsive here. + break; } +} + +std::optional MotionPlus::ReadByte() +{ + switch (GetActivationStatus()) + { + case ActivationStatus::Inactive: + // m_inactive_wrapper is connected to the bus, so it will respond to 0x53 when inactive. + return m_i2c_bus.ReadByte(); case ActivationStatus::Active: + // FYI: Motion plus does not respond to 0x53 when activated. + // No i2c passthrough when activated, so *only* target 0x52. + return m_active_wrapper.ReadByte(); + + default: + case ActivationStatus::Activating: + case ActivationStatus::Deactivating: + // The extension port is completely unresponsive here. + return std::nullopt; + } +} + +bool MotionPlus::WriteByte(u8 value) +{ + switch (GetActivationStatus()) { + case ActivationStatus::Inactive: + // m_inactive_wrapper is connected to the bus, so it will respond to 0x53 when inactive. + return m_i2c_bus.WriteByte(value); + + case ActivationStatus::Active: // FYI: Motion plus does not respond to 0x53 when activated. - if (ACTIVE_DEVICE_ADDR != slave_addr) - { - // No i2c passthrough when activated. - return 0; - } + // No i2c passthrough when activated, so *only* target 0x52. + return m_active_wrapper.WriteByte(value); + + default: + case ActivationStatus::Activating: + case ActivationStatus::Deactivating: + // The extension port is completely unresponsive here. + return false; + } +} - DEBUG_LOG_FMT(WIIMOTE, "Active M+ write {:#x} : {}", addr, ArrayToString(data_in, count)); +u8 MotionPlus::RegisterWrapper::ReadByte(u8 addr) +{ + return RawRead(&m_owner->m_reg_data, addr); +} - auto const result = RawWrite(&m_reg_data, addr, count, data_in); +void MotionPlus::InactiveRegisterWrapper::WriteByte(u8 addr, u8 value) +{ + DEBUG_LOG_FMT(WIIMOTE, "Inactive M+ write {:02x} = {:02x}", addr, value); - switch (addr) - { - case offsetof(Register, init_trigger): - // It seems a write of any value here triggers deactivation on a real M+. - Deactivate(); + RawWrite(&m_owner->m_reg_data, addr, value); - // Passthrough the write to the attached extension. - // The M+ deactivation signal is cleverly the same as EXT initialization. - m_i2c_bus.BusWrite(slave_addr, addr, count, data_in); - break; + if (PASSTHROUGH_MODE_OFFSET == addr) + { + m_owner->OnPassthroughModeWrite(); + } +} + +void MotionPlus::ActiveRegisterWrapper::WriteByte(u8 addr, u8 value) +{ + DEBUG_LOG_FMT(WIIMOTE, "Inactive M+ write {:02x} = {:02x}", addr, value); + + RawWrite(&m_owner->m_reg_data, addr, value); + + switch (addr) + { + case offsetof(Register, init_trigger): + // It seems a write of any value here triggers deactivation on a real M+. + m_owner->Deactivate(); + + // Passthrough the write to the attached extension. + // The M+ deactivation signal is cleverly the same as EXT initialization. + // TODO + // m_i2c_bus.BusWrite(slave_addr, addr, count, data_in); + break; + + case offsetof(Register, challenge_type): + if (ChallengeState::ParameterXReady == m_owner->m_reg_data.challenge_state) + { + DEBUG_LOG_FMT(WIIMOTE, "M+ challenge: {:#x}", m_owner->m_reg_data.challenge_type); - case offsetof(Register, challenge_type): - if (ChallengeState::ParameterXReady == m_reg_data.challenge_state) + // After games read parameter x they write here to request y0 or y1. + if (0 == m_owner->m_reg_data.challenge_type) { - DEBUG_LOG_FMT(WIIMOTE, "M+ challenge: {:#x}", m_reg_data.challenge_type); - - // After games read parameter x they write here to request y0 or y1. - if (0 == m_reg_data.challenge_type) - { - // Preparing y0 on the real M+ is almost instant (30ms maybe). - constexpr int PREPARE_Y0_MS = 30; - m_progress_timer = ::Wiimote::UPDATE_FREQ * PREPARE_Y0_MS / 1000; - } - else - { - // A real M+ takes about 1200ms to prepare y1. - // Games seem to not care that we don't take that long. - constexpr int PREPARE_Y1_MS = 500; - m_progress_timer = ::Wiimote::UPDATE_FREQ * PREPARE_Y1_MS / 1000; - } - - // Games give the M+ a bit of time to compute the value. - // y0 gets about half a second. - // y1 gets at about 9.5 seconds. - // After this the M+ will fail the "challenge". - - m_reg_data.challenge_state = ChallengeState::PreparingY; + // Preparing y0 on the real M+ is almost instant (30ms maybe). + constexpr int PREPARE_Y0_MS = 30; + m_owner->m_progress_timer = ::Wiimote::UPDATE_FREQ * PREPARE_Y0_MS / 1000; + } + else + { + // A real M+ takes about 1200ms to prepare y1. + // Games seem to not care that we don't take that long. + constexpr int PREPARE_Y1_MS = 500; + m_owner->m_progress_timer = ::Wiimote::UPDATE_FREQ * PREPARE_Y1_MS / 1000; } - break; - case offsetof(Register, calibration_trigger): - // Games seem to invoke this to start and stop calibration. Exact consequences unknown. - DEBUG_LOG_FMT(WIIMOTE, "M+ calibration trigger: {:#x}", m_reg_data.calibration_trigger); - break; + // Games give the M+ a bit of time to compute the value. + // y0 gets about half a second. + // y1 gets at about 9.5 seconds. + // After this the M+ will fail the "challenge". - case PASSTHROUGH_MODE_OFFSET: - // Games sometimes (not often) write zero here to deactivate the M+. - OnPassthroughModeWrite(); - break; + m_owner->m_reg_data.challenge_state = ChallengeState::PreparingY; } + break; - return result; - } + case offsetof(Register, calibration_trigger): + // Games seem to invoke this to start and stop calibration. Exact consequences unknown. + DEBUG_LOG_FMT(WIIMOTE, "M+ calibration trigger: {:#x}", + m_owner->m_reg_data.calibration_trigger); + break; - default: - case ActivationStatus::Activating: - case ActivationStatus::Deactivating: - // The extension port is completely unresponsive here. - return 0; + case PASSTHROUGH_MODE_OFFSET: + // Games sometimes (not often) write zero here to deactivate the M+. + m_owner->OnPassthroughModeWrite(); + break; } } diff --git a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h index 02ddfb7ec2f0..db6bcb089b18 100644 --- a/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h +++ b/Source/Core/Core/HW/WiimoteEmu/MotionPlus.h @@ -6,15 +6,15 @@ #include #include "Common/CommonTypes.h" +#include "Common/I2C.h" #include "Common/MathUtil.h" #include "Common/Swap.h" #include "Core/HW/WiimoteEmu/Dynamics.h" #include "Core/HW/WiimoteEmu/ExtensionPort.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" namespace WiimoteEmu { -struct MotionPlus : public Extension +class MotionPlus : public Extension { public: enum class PassthroughMode : u8 @@ -241,6 +241,43 @@ struct MotionPlus : public Extension #pragma pack(pop) static_assert(0x100 == sizeof(Register), "Wrong size"); + class RegisterWrapper : public Common::I2CSlaveAutoIncrementing + { + protected: + RegisterWrapper(MotionPlus* owner, u8 slave_addr) + : I2CSlaveAutoIncrementing(slave_addr), m_owner(owner) + { + } + + u8 ReadByte(u8 addr) override; + + MotionPlus* const m_owner; + }; + + class InactiveRegisterWrapper : public RegisterWrapper + { + public: + InactiveRegisterWrapper(MotionPlus* owner) : RegisterWrapper(owner, INACTIVE_DEVICE_ADDR) {} + + using Common::I2CSlaveAutoIncrementing::ReadByte; + using Common::I2CSlaveAutoIncrementing::WriteByte; + + protected: + void WriteByte(u8 addr, u8 value) override; + }; + + class ActiveRegisterWrapper : public RegisterWrapper + { + public: + ActiveRegisterWrapper(MotionPlus* owner) : RegisterWrapper(owner, ACTIVE_DEVICE_ADDR) {} + + using Common::I2CSlaveAutoIncrementing::ReadByte; + using Common::I2CSlaveAutoIncrementing::WriteByte; + + protected: + void WriteByte(u8 addr, u8 value) override; + }; + void Activate(); void Deactivate(); void OnPassthroughModeWrite(); @@ -248,8 +285,11 @@ struct MotionPlus : public Extension ActivationStatus GetActivationStatus() const; PassthroughMode GetPassthroughMode() const; - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool StartWrite(u8 slave_addr) override; + bool StartRead(u8 slave_addr) override; + void Stop() override; + std::optional ReadByte() override; + bool WriteByte(u8 value) override; bool ReadDeviceDetectPin() const override; @@ -259,7 +299,10 @@ struct MotionPlus : public Extension u8 m_progress_timer = {}; // The port on the end of the motion plus: - I2CBus m_i2c_bus; + Common::I2CBusForwarding m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; + + InactiveRegisterWrapper m_inactive_wrapper{this}; // connected to m_i2c_bus + ActiveRegisterWrapper m_active_wrapper{this}; // *not* connected to m_i2c_bus }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp index 23cba6bfd741..6fe6cfb0e7fd 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.cpp @@ -146,6 +146,8 @@ void SpeakerLogic::DoState(PointerWrap& p) { p.Do(adpcm_state); p.Do(reg_data); + p.Do(m_i2c_active); + p.Do(m_device_address); } void SpeakerLogic::SetSpeakerEnabled(bool enabled) @@ -153,29 +155,104 @@ void SpeakerLogic::SetSpeakerEnabled(bool enabled) m_speaker_enabled = enabled; } -int SpeakerLogic::BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) +bool SpeakerLogic::StartWrite(u8 slave_addr) { - if (I2C_ADDR != slave_addr) - return 0; + if (slave_addr == I2C_ADDR) + { + INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} write started, previously active: {}", I2C_ADDR, + m_i2c_active); + m_i2c_active = true; + m_device_address.reset(); + return true; + } + else + { + return false; + } +} - return RawRead(®_data, addr, count, data_out); +bool SpeakerLogic::StartRead(u8 slave_addr) +{ + if (slave_addr == I2C_ADDR) + { + INFO_LOG_FMT(WII_IPC, "I2C Device {:02x} read started, previously active: {}", I2C_ADDR, + m_i2c_active); + if (m_device_address.has_value()) + { + m_i2c_active = true; + return true; + } + else + { + WARN_LOG_FMT(WII_IPC, + "I2C Device {:02x}: read attempted without having written device address", + I2C_ADDR); + m_i2c_active = false; + return false; + } + } + else + { + return false; + } } -int SpeakerLogic::BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) +void SpeakerLogic::Stop() { - if (I2C_ADDR != slave_addr) - return 0; + m_i2c_active = false; + m_device_address.reset(); +} + +std::optional SpeakerLogic::ReadByte() +{ + if (m_i2c_active) + { + // TODO: It seems reading address 0x00 should always return 0xff. - if (0x00 == addr) + ASSERT(m_device_address.has_value()); // enforced by StartRead + const u8 cur_addr = m_device_address.value(); + // Wrapping from 255 to 0 is the assumed behavior; this may not make sense here + m_device_address = cur_addr + 1; + return RawRead(®_data, cur_addr); + } + else { - SpeakerData(data_in, count, m_speaker_pan_setting.GetValue() / 100); - return count; + return std::nullopt; + } +} + +bool SpeakerLogic::WriteByte(u8 value) +{ + if (m_i2c_active) + { + if (m_device_address.has_value()) + { + const u8 cur_addr = m_device_address.value(); + + if (cur_addr == SPEAKER_DATA_OFFSET) // == 0 + { + // Note: No auto-incrementation in this case + SpeakerData(&value, 1, m_speaker_pan_setting.GetValue() / 100); + } + else + { + // Wrapping from 255 to 0 is the assumed behavior; this may not make sense here + m_device_address = cur_addr + 1; + + // TODO: Does writing immediately change the decoder config even when active + // or does a write to 0x08 activate the new configuration or something? + RawWrite(®_data, cur_addr, value); + } + } + else + { + m_device_address = value; + } + return true; } else { - // TODO: Does writing immediately change the decoder config even when active - // or does a write to 0x08 activate the new configuration or something? - return RawWrite(®_data, addr, count, data_in); + return false; } } diff --git a/Source/Core/Core/HW/WiimoteEmu/Speaker.h b/Source/Core/Core/HW/WiimoteEmu/Speaker.h index 9a6a652bb1fa..dcb93bca4946 100644 --- a/Source/Core/Core/HW/WiimoteEmu/Speaker.h +++ b/Source/Core/Core/HW/WiimoteEmu/Speaker.h @@ -5,7 +5,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" +#include "Common/I2C.h" #include "InputCommon/ControllerEmu/Setting/NumericSetting.h" namespace WiimoteEmu @@ -17,7 +17,7 @@ struct ADPCMState class Wiimote; -class SpeakerLogic : public I2CSlave +class SpeakerLogic : public Common::I2CSlave { friend class Wiimote; @@ -63,8 +63,11 @@ class SpeakerLogic : public I2CSlave static_assert(0x100 == sizeof(Register)); - int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override; - int BusWrite(u8 slave_addr, u8 addr, int count, const u8* data_in) override; + bool StartWrite(u8 slave_addr) override; + bool StartRead(u8 slave_addr) override; + void Stop() override; + std::optional ReadByte() override; + bool WriteByte(u8 value) override; Register reg_data{}; @@ -75,6 +78,9 @@ class SpeakerLogic : public I2CSlave ControllerEmu::SettingValue m_speaker_pan_setting; bool m_speaker_enabled = false; + + bool m_i2c_active = false; + std::optional m_device_address = std::nullopt; }; } // namespace WiimoteEmu diff --git a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h index e364ccdb6c91..8468e4dc60a2 100644 --- a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h +++ b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h @@ -10,6 +10,7 @@ #include "Common/Common.h" #include "Common/Config/Config.h" +#include "Common/I2C.h" #include "Core/HW/WiimoteCommon/WiimoteReport.h" @@ -17,7 +18,6 @@ #include "Core/HW/WiimoteEmu/Dynamics.h" #include "Core/HW/WiimoteEmu/Encryption.h" #include "Core/HW/WiimoteEmu/ExtensionPort.h" -#include "Core/HW/WiimoteEmu/I2CBus.h" #include "Core/HW/WiimoteEmu/MotionPlus.h" #include "Core/HW/WiimoteEmu/Speaker.h" @@ -316,7 +316,7 @@ class Wiimote : public ControllerEmu::EmulatedController, public WiimoteCommon:: MotionPlus m_motion_plus; CameraLogic m_camera_logic; - I2CBus m_i2c_bus; + Common::I2CBusSimple m_i2c_bus; ExtensionPort m_extension_port{&m_i2c_bus}; From 798c9826cc055af068d75bb2cac73fc300eef4af Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 4 Oct 2022 15:38:55 -0700 Subject: [PATCH 33/34] Typos --- Source/Core/Core/HW/WII_IPC.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 5cf7ae22cb82..ce6e8b9b89c3 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -5,7 +5,7 @@ #include #include -#include +#include #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" @@ -121,7 +121,7 @@ struct AVEState #pragma pack() static_assert(sizeof(AVEState) == 0x100); -std::string_view GetAVERegisterName(u8 address) +std::string GetAVERegisterName(u8 address) { if (address == 0x00) return "A/V Timings"; @@ -158,7 +158,7 @@ std::string_view GetAVERegisterName(u8 address) else if (address == 0x71) return "Audio stereo output control - right volume"; else if (address == 0x72) - return "Audio stereo output control - right volume"; + return "Audio stereo output control - left volume"; else if (address >= 0x7a && address <= 0x7d) return "Closed Captioning control"; else From 612ed106cdab011b72228fcb7a0bb29bae607320 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 3 Sep 2023 16:01:01 -0700 Subject: [PATCH 34/34] Build fix --- Source/Core/Core/HW/WII_IPC.cpp | 12 +++++++----- Source/Core/Core/HW/WII_IPC.h | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index ce6e8b9b89c3..15e0c25bd9f6 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -290,10 +290,10 @@ void WiiIPC::Shutdown() { } -static u32 ReadGPIOIn(Core::System& system) +u32 WiiIPC::ReadGPIOIn() { Common::Flags gpio_in; - gpio_in[GPIO::SLOT_IN] = system.GetDVDInterface().IsDiscInside(); + gpio_in[GPIO::SLOT_IN] = m_system.GetDVDInterface().IsDiscInside(); gpio_in[GPIO::AVE_SCL] = i2c_state.GetSCL(); gpio_in[GPIO::AVE_SDA] = i2c_state.GetSDA(); return gpio_in.m_hex; @@ -306,7 +306,7 @@ u32 WiiIPC::GetGPIOOut() // In practice this means that (at least for the AVE I²C pins) a 1 is output when the pin is an // input. (RVLoader depends on this.) // https://github.com/Aurelio92/RVLoader/blob/75732f248019f589deb1109bba7b5323a8afaadf/source/i2c.c#L101-L109 - return (m_gpio_out.m_hex | ~(m_gpio_dir.m_hex)) & (ReadGPIOIn(m_system) | m_gpio_dir.m_hex); + return (m_gpio_out.m_hex | ~(m_gpio_dir.m_hex)) & (ReadGPIOIn() | m_gpio_dir.m_hex); } void WiiIPC::GPIOOutChanged(u32 old_value_hex) @@ -389,7 +389,8 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIOB_IN, MMIO::ComplexRead([](Core::System& system, u32) { - return ReadGPIOIn(system); + auto& wii_ipc = system.GetWiiIPC(); + return wii_ipc.ReadGPIOIn(); }), MMIO::Nop()); // Starlet GPIO registers, not normally accessible by PPC (but they can be depending on how @@ -420,7 +421,8 @@ void WiiIPC::RegisterMMIO(MMIO::Mapping* mmio, u32 base) wii_ipc.GPIOOutChanged(old_out); })); mmio->Register(base | GPIO_IN, MMIO::ComplexRead([](Core::System& system, u32) { - return ReadGPIOIn(system); + auto& wii_ipc = system.GetWiiIPC(); + return wii_ipc.ReadGPIOIn(); }), MMIO::Nop()); diff --git a/Source/Core/Core/HW/WII_IPC.h b/Source/Core/Core/HW/WII_IPC.h index e3e4ee55756c..23a67e0eb558 100644 --- a/Source/Core/Core/HW/WII_IPC.h +++ b/Source/Core/Core/HW/WII_IPC.h @@ -140,6 +140,7 @@ class WiiIPC static void UpdateInterruptsCallback(Core::System& system, u64 userdata, s64 cycles_late); void UpdateInterrupts(); + u32 ReadGPIOIn(); u32 GetGPIOOut(); void GPIOOutChanged(u32 old_value_hex);