From fba51b2956ad4d989d24673998ac613e4fd9c0b5 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sat, 18 Jan 2020 19:12:15 -0800 Subject: [PATCH 1/9] Split drive state and drive error into 2 fields, and fix some inaccuracies In particular: - Trying to play audio in a non-ready state returns the state-specific error, not an audio buf error - Audio status cannot be requested in non-ready states - The audio buffer cannot be configured in states other than ReadyNoReadsMade - Using the stop motor command while the motor is already stopped doesn't change states Additionally, the internal state IDs are used (which distinguish ReadyNoReadsMade and Ready), instead of the state IDs exposed in request error. This makes some of the weird behavior a bit more obvious. State and error behavior of the seek command was not implemented in this commit. --- Source/Core/Core/Boot/Boot.cpp | 5 +- Source/Core/Core/HW/DVD/DVDInterface.cpp | 159 ++++++++++++++--------- Source/Core/Core/HW/DVD/DVDInterface.h | 64 +++++---- Source/Core/Core/HW/DVD/DVDThread.cpp | 2 +- 4 files changed, 134 insertions(+), 96 deletions(-) diff --git a/Source/Core/Core/Boot/Boot.cpp b/Source/Core/Core/Boot/Boot.cpp index c518601b41..02fd7beccd 100644 --- a/Source/Core/Core/Boot/Boot.cpp +++ b/Source/Core/Core/Boot/Boot.cpp @@ -250,8 +250,9 @@ bool CBoot::DVDReadDiscID(const DiscIO::VolumeDisc& disc, u32 output_address) if (!disc.Read(0, buffer.size(), buffer.data(), DiscIO::PARTITION_NONE)) return false; Memory::CopyToEmu(output_address, buffer.data(), buffer.size()); - // Clear ERROR_NO_DISKID_L, probably should check if that's currently set - DVDInterface::SetLowError(DVDInterface::ERROR_READY); + // Transition out of the DiscIdNotRead state (which the drive should be in at this point, + // on the assumption that this is only used for the first read) + DVDInterface::SetDriveState(DVDInterface::DriveState::ReadyNoReadsMade); return true; } diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index 8f8ec4f4e4..4404425e82 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -157,12 +157,12 @@ static u32 s_current_length; static u64 s_next_start; static u32 s_next_length; static u32 s_pending_samples; -static bool s_can_configure_dtk = true; static bool s_enable_dtk = false; static u8 s_dtk_buffer_length = 0; // TODO: figure out how this affects the regular buffer // Disc drive state -static u32 s_error_code = 0; +static DriveState s_drive_state; +static DriveError s_error_code; // Disc drive timing static u64 s_read_buffer_start_time; @@ -219,10 +219,10 @@ void DoState(PointerWrap& p) p.Do(s_next_start); p.Do(s_next_length); p.Do(s_pending_samples); - p.Do(s_can_configure_dtk); p.Do(s_enable_dtk); p.Do(s_dtk_buffer_length); + p.Do(s_drive_state); p.Do(s_error_code); p.Do(s_read_buffer_start_time); @@ -383,31 +383,30 @@ void Reset(bool spinup) s_current_start = 0; s_current_length = 0; s_pending_samples = 0; - s_can_configure_dtk = true; s_enable_dtk = false; s_dtk_buffer_length = 0; if (!IsDiscInside()) { - // ERROR_COVER is used when the cover is open; - // ERROR_NO_DISK_L is used when the cover is closed but there is no disc. + // CoverOpened is used when the cover is open; + // NoMediumPresent is used when the cover is closed but there is no disc. // On the Wii, this can only happen if something other than a DVD is inserted into the disc // drive (for instance, an audio CD) and only after it attempts to read it. Otherwise, it will // report the cover as opened. - SetLowError(ERROR_COVER); + SetDriveState(DriveState::CoverOpened); } else if (!spinup) { // Wii hardware tests indicate that this is used when ejecting and inserting a new disc, or // performing a reset without spinup. - SetLowError(ERROR_CHANGE_DISK); + SetDriveState(DriveState::DiscChangeDetected); } else { - SetLowError(ERROR_NO_DISKID_L); + SetDriveState(DriveState::DiscIdNotRead); } - SetHighError(ERROR_NONE); + SetDriveError(DriveError::None); // The buffer is empty at start s_read_buffer_start_offset = 0; @@ -705,32 +704,32 @@ void ClearInterrupt(DIInterruptType interrupt) } // Checks the drive state to make sure a read-like command can be performed. -// If false is returned, SetHighError will have been called, and the caller +// If false is returned, SetDriveError will have been called, and the caller // should issue a DEINT interrupt. static bool CheckReadPreconditions() { - if (!IsDiscInside()) // Implies ERROR_COVER or ERROR_NO_DISK + if (!IsDiscInside()) // Implies CoverOpened or NoMediumPresent { ERROR_LOG(DVDINTERFACE, "No disc inside."); - SetHighError(ERROR_NO_DISK_H); + SetDriveError(DriveError::MediumNotPresent); return false; } - if ((s_error_code & LOW_ERROR_MASK) == ERROR_CHANGE_DISK) + if (s_drive_state == DriveState::DiscChangeDetected) { ERROR_LOG(DVDINTERFACE, "Disc changed (motor stopped)."); - SetHighError(ERROR_MEDIUM); + SetDriveError(DriveError::MediumChanged); return false; } - if ((s_error_code & LOW_ERROR_MASK) == ERROR_MOTOR_STOP_L) + if (s_drive_state == DriveState::MotorStopped) { ERROR_LOG(DVDINTERFACE, "Motor stopped."); - SetHighError(ERROR_MOTOR_STOP_H); + SetDriveError(DriveError::MotorStopped); return false; } - if ((s_error_code & LOW_ERROR_MASK) == ERROR_NO_DISKID_L) + if (s_drive_state == DriveState::DiscIdNotRead) { ERROR_LOG(DVDINTERFACE, "Disc id not read."); - SetHighError(ERROR_NO_DISKID_H); + SetDriveError(DriveError::NoDiscID); return false; } return true; @@ -775,7 +774,7 @@ bool ExecuteReadCommand(u64 dvd_offset, u32 output_address, u32 dvd_length, u32 if (reply_type == ReplyType::IOS && partition == DiscIO::PARTITION_NONE && dvd_offset + dvd_length > 0x50000) { - SetHighError(DVDInterface::ERROR_BLOCK_OOB); + SetDriveError(DriveError::BlockOOB); *interrupt_type = DIInterruptType::DEINT; return false; } @@ -794,7 +793,7 @@ void ExecuteCommand(ReplyType reply_type) // DVDLowRequestError needs access to the error code set by the previous command if (static_cast(s_DICMDBUF[0] >> 24) != DICommand::RequestError) - SetHighError(0); + SetDriveError(DriveError::None); switch (static_cast(s_DICMDBUF[0] >> 24)) { @@ -811,7 +810,7 @@ void ExecuteCommand(ReplyType reply_type) // GC-only patched drive firmware command, used by libogc case DICommand::Unknown55: INFO_LOG(DVDINTERFACE, "SetExtension"); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; @@ -820,7 +819,7 @@ void ExecuteCommand(ReplyType reply_type) INFO_LOG(DVDINTERFACE, "DVDLowReportKey"); // Does not work on retail discs/drives // Retail games send this command to see if they are running on real retail hw - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; @@ -838,7 +837,9 @@ void ExecuteCommand(ReplyType reply_type) ", DMABuffer = %08x, SrcLength = %08x, DMALength = %08x", iDVDOffset, s_DIMAR, s_DICMDBUF[2], s_DILENGTH); - s_can_configure_dtk = false; + if (s_drive_state == DriveState::ReadyNoReadsMade) + SetDriveState(DriveState::Ready); + command_handled_by_thread = ExecuteReadCommand(iDVDOffset, s_DIMAR, s_DICMDBUF[2], s_DILENGTH, DiscIO::PARTITION_NONE, reply_type, &interrupt_type); @@ -847,20 +848,20 @@ void ExecuteCommand(ReplyType reply_type) case 0x40: // Read DiscID INFO_LOG(DVDINTERFACE, "Read DiscID: buffer %08x", s_DIMAR); - // TODO: It doesn't make sense to include ERROR_CHANGE_DISK here, as it implies that the drive - // is not spinning and reading the disc ID shouldn't change it. However, the Wii Menu breaks - // without it. - if ((s_error_code & LOW_ERROR_MASK) == ERROR_NO_DISKID_L || - (s_error_code & LOW_ERROR_MASK) == ERROR_CHANGE_DISK) + // TODO: It doesn't make sense to include DiscChangeDetected here, as it implies that the + // drive is not spinning and reading the disc ID shouldn't change it. However, the Wii Menu + // breaks without it. + if (s_drive_state == DriveState::DiscIdNotRead || + s_drive_state == DriveState::DiscChangeDetected) { - SetLowError(ERROR_READY); + SetDriveState(DriveState::ReadyNoReadsMade); } - else + else if (s_drive_state == DriveState::ReadyNoReadsMade) { // The first disc ID reading is required before DTK can be configured. // If the disc ID is read again (or any other read occurs), it no longer can // be configured. - s_can_configure_dtk = false; + SetDriveState(DriveState::Ready); } command_handled_by_thread = ExecuteReadCommand( @@ -897,33 +898,33 @@ void ExecuteCommand(ReplyType reply_type) ERROR_LOG(DVDINTERFACE, "Unknown 0xAD subcommand in %08x", s_DICMDBUF[0]); break; } - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::ReadDVD: ERROR_LOG(DVDINTERFACE, "DVDLowReadDvd"); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::ReadDVDConfig: ERROR_LOG(DVDINTERFACE, "DVDLowReadDvdConfig"); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::StopLaser: ERROR_LOG(DVDINTERFACE, "DVDLowStopLaser"); DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_DVD_LOW_STOP_LASER); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::Offset: ERROR_LOG(DVDINTERFACE, "DVDLowOffset"); DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_DVD_LOW_OFFSET); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive @@ -943,36 +944,45 @@ void ExecuteCommand(ReplyType reply_type) case DICommand::RequestDiscStatus: ERROR_LOG(DVDINTERFACE, "DVDLowRequestDiscStatus"); DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_DVD_LOW_REQUEST_DISC_STATUS); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::RequestRetryNumber: ERROR_LOG(DVDINTERFACE, "DVDLowRequestRetryNumber"); DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_DVD_LOW_REQUEST_RETRY_NUMBER); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::SetMaximumRotation: ERROR_LOG(DVDINTERFACE, "DVDLowSetMaximumRotation"); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Wii-exclusive case DICommand::SerMeasControl: ERROR_LOG(DVDINTERFACE, "DVDLowSerMeasControl"); DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_DVD_LOW_SER_MEAS_CONTROL); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; // Used by both GC and Wii case DICommand::RequestError: - INFO_LOG(DVDINTERFACE, "Requesting error... (0x%08x)", s_error_code); - s_DIIMMBUF = s_error_code; - SetHighError(0); + { + u32 drive_state; + if (s_drive_state == DriveState::Ready) + drive_state = 0; + else + drive_state = static_cast(s_drive_state) - 1; + + const u32 result = (drive_state << 24) | static_cast(s_error_code); + INFO_LOG(DVDINTERFACE, "Requesting error... (0x%08x)", result); + s_DIIMMBUF = result; + SetDriveError(DriveError::None); break; + } // Audio Stream (Immediate). Only used by some GC games, but does exist on the Wii // (command_0 >> 16) & 0xFF = Subcommand @@ -983,7 +993,6 @@ void ExecuteCommand(ReplyType reply_type) if (!CheckReadPreconditions()) { ERROR_LOG(DVDINTERFACE, "Cannot play audio (command %08x)", s_DICMDBUF[0]); - SetHighError(ERROR_AUDIO_BUF); interrupt_type = DIInterruptType::DEINT; break; } @@ -992,12 +1001,13 @@ void ExecuteCommand(ReplyType reply_type) ERROR_LOG(DVDINTERFACE, "Attempted to change playing audio while audio is disabled! (%08x %08x %08x)", s_DICMDBUF[0], s_DICMDBUF[1], s_DICMDBUF[2]); - SetHighError(ERROR_AUDIO_BUF); + SetDriveError(DriveError::NoAudioBuf); interrupt_type = DIInterruptType::DEINT; break; } - s_can_configure_dtk = false; + if (s_drive_state == DriveState::ReadyNoReadsMade) + SetDriveState(DriveState::Ready); switch ((s_DICMDBUF[0] >> 16) & 0xFF) { @@ -1035,7 +1045,7 @@ void ExecuteCommand(ReplyType reply_type) default: ERROR_LOG(DVDINTERFACE, "Invalid audio command! (%08x %08x %08x)", s_DICMDBUF[0], s_DICMDBUF[1], s_DICMDBUF[2]); - SetHighError(ERROR_INV_AUDIO); + SetDriveError(DriveError::InvalidAudioCommand); interrupt_type = DIInterruptType::DEINT; break; } @@ -1045,10 +1055,17 @@ void ExecuteCommand(ReplyType reply_type) // Request Audio Status (Immediate). Only used by some GC games, but does exist on the Wii case DICommand::RequestAudioStatus: { + if (!CheckReadPreconditions()) + { + ERROR_LOG(DVDINTERFACE, "Attempted to request audio status in an invalid state!"); + interrupt_type = DIInterruptType::DEINT; + break; + } + if (!s_enable_dtk) { ERROR_LOG(DVDINTERFACE, "Attempted to request audio status while audio is disabled!"); - SetHighError(ERROR_AUDIO_BUF); + SetDriveError(DriveError::NoAudioBuf); interrupt_type = DIInterruptType::DEINT; break; } @@ -1082,7 +1099,7 @@ void ExecuteCommand(ReplyType reply_type) default: ERROR_LOG(DVDINTERFACE, "Invalid audio status command! (%08x %08x %08x)", s_DICMDBUF[0], s_DICMDBUF[1], s_DICMDBUF[2]); - SetHighError(ERROR_INV_AUDIO); + SetDriveError(DriveError::InvalidAudioCommand); interrupt_type = DIInterruptType::DEINT; break; } @@ -1096,7 +1113,12 @@ void ExecuteCommand(ReplyType reply_type) const bool kill = (s_DICMDBUF[0] & (1 << 20)); INFO_LOG(DVDINTERFACE, "DVDLowStopMotor%s%s", eject ? " eject" : "", kill ? " kill!" : ""); - SetLowError(ERROR_MOTOR_STOP_L); + if (s_drive_state == DriveState::Ready || s_drive_state == DriveState::ReadyNoReadsMade || + s_drive_state == DriveState::DiscIdNotRead) + { + SetDriveState(DriveState::MotorStopped); + } + const bool force_eject = eject && !kill; if (Config::Get(Config::MAIN_AUTO_DISC_CHANGE) && !Movie::IsPlayingInput() && @@ -1121,22 +1143,31 @@ void ExecuteCommand(ReplyType reply_type) // The link is dead, but you can access the page using the Wayback Machine at archive.org. // This command can only be used immediately after reading the disc ID, before any other - // reads. Too early, and you get ERROR_NO_DISKID. Too late, and you get ERROR_INV_PERIOD. - if (!s_can_configure_dtk) + // reads. Too early, and you get NoDiscID. Too late, and you get InvalidPeriod. + if (!CheckReadPreconditions()) { - ERROR_LOG(DVDINTERFACE, "Attempted to change DTK configuration after a read has been made!"); - SetHighError(ERROR_INV_PERIOD); + ERROR_LOG(DVDINTERFACE, "Attempted to change DTK configuration in an invalid state!"); interrupt_type = DIInterruptType::DEINT; break; } + if (s_drive_state == DriveState::Ready) + { + ERROR_LOG(DVDINTERFACE, "Attempted to change DTK configuration after a read has been made!"); + SetDriveError(DriveError::InvalidPeriod); + interrupt_type = DIInterruptType::DEINT; + break; + } + + // Note that this can be called multiple times, as long as the drive is in the ReadyNoReadsMade + // state. Calling it does not exit that state. AudioBufferConfig((s_DICMDBUF[0] >> 16) & 1, s_DICMDBUF[0] & 0xf); break; // GC-only patched drive firmware command, used by libogc case DICommand::UnknownEE: INFO_LOG(DVDINTERFACE, "SetStatus"); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; @@ -1146,7 +1177,7 @@ void ExecuteCommand(ReplyType reply_type) // Can only be used through direct access and only after unlocked. case DICommand::Debug: ERROR_LOG(DVDINTERFACE, "Unsupported DVD Drive debug command 0x%08x", s_DICMDBUF[0]); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; @@ -1175,7 +1206,7 @@ void ExecuteCommand(ReplyType reply_type) ERROR_LOG(DVDINTERFACE, "Unknown command 0x%08x (Buffer 0x%08x, 0x%x)", s_DICMDBUF[0], s_DIMAR, s_DILENGTH); PanicAlertT("Unknown DVD command %08x - fatal error", s_DICMDBUF[0]); - SetHighError(ERROR_INV_CMD); + SetDriveError(DriveError::InvalidCommand); interrupt_type = DIInterruptType::DEINT; break; } @@ -1193,7 +1224,7 @@ void PerformDecryptingRead(u32 position, u32 length, u32 output_address, const DiscIO::Partition& partition, ReplyType reply_type) { DIInterruptType interrupt_type = DIInterruptType::TCINT; - s_can_configure_dtk = false; + SetDriveState(DriveState::Ready); const bool command_handled_by_thread = ExecuteReadCommand(static_cast(position) << 2, output_address, length, length, partition, @@ -1230,16 +1261,14 @@ void FinishExecutingCommandCallback(u64 userdata, s64 cycles_late) FinishExecutingCommand(reply_type, interrupt_type, cycles_late); } -void SetLowError(u32 low_error) +void SetDriveState(DriveState state) { - DEBUG_ASSERT((low_error & HIGH_ERROR_MASK) == 0); - s_error_code = (s_error_code & HIGH_ERROR_MASK) | (low_error & LOW_ERROR_MASK); + s_drive_state = state; } -void SetHighError(u32 high_error) +void SetDriveError(DriveError error) { - DEBUG_ASSERT((high_error & LOW_ERROR_MASK) == 0); - s_error_code = (s_error_code & LOW_ERROR_MASK) | (high_error & HIGH_ERROR_MASK); + s_error_code = error; } void FinishExecutingCommand(ReplyType reply_type, DIInterruptType interrupt_type, s64 cycles_late, diff --git a/Source/Core/Core/HW/DVD/DVDInterface.h b/Source/Core/Core/HW/DVD/DVDInterface.h index c0bd1bd4d0..7a013c484f 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.h +++ b/Source/Core/Core/HW/DVD/DVDInterface.h @@ -51,33 +51,41 @@ enum class DICommand : u8 UnknownEE = 0xee, }; -// "low" error codes -constexpr u32 ERROR_READY = 0x0000000; // Ready. -constexpr u32 ERROR_COVER = 0x01000000; // Cover is opened. -constexpr u32 ERROR_CHANGE_DISK = 0x02000000; // Disk change. -constexpr u32 ERROR_NO_DISK_L = 0x03000000; // No disk. -constexpr u32 ERROR_MOTOR_STOP_L = 0x04000000; // Motor stop. -constexpr u32 ERROR_NO_DISKID_L = 0x05000000; // Disk ID not read. -constexpr u32 LOW_ERROR_MASK = 0xff000000; +// Disc drive state. +// Reported in error codes as 0 for Ready, and value-1 for the rest +// (i.e. Ready and ReadyNoReadsMade are both reported as 0) +enum class DriveState : u8 +{ + Ready = 0, + ReadyNoReadsMade = 1, + CoverOpened = 2, + DiscChangeDetected = 3, + NoMediumPresent = 4, + MotorStopped = 5, + DiscIdNotRead = 6 +}; -// "high" error codes -constexpr u32 ERROR_NONE = 0x000000; // No error. -constexpr u32 ERROR_MOTOR_STOP_H = 0x020400; // Motor stopped. -constexpr u32 ERROR_NO_DISKID_H = 0x020401; // Disk ID not read. -constexpr u32 ERROR_NO_DISK_H = 0x023a00; // Medium not present / Cover opened. -constexpr u32 ERROR_SEEK_NDONE = 0x030200; // No seek complete. -constexpr u32 ERROR_READ = 0x031100; // Unrecovered read error. -constexpr u32 ERROR_PROTOCOL = 0x040800; // Transfer protocol error. -constexpr u32 ERROR_INV_CMD = 0x052000; // Invalid command operation code. -constexpr u32 ERROR_AUDIO_BUF = 0x052001; // Audio Buffer not set. -constexpr u32 ERROR_BLOCK_OOB = 0x052100; // Logical block address out of bounds. -constexpr u32 ERROR_INV_FIELD = 0x052400; // Invalid field in command packet. -constexpr u32 ERROR_INV_AUDIO = 0x052401; // Invalid audio command. -constexpr u32 ERROR_INV_PERIOD = 0x052402; // Configuration out of permitted period. -constexpr u32 ERROR_END_USR_AREA = 0x056300; // End of user area encountered on this track. -constexpr u32 ERROR_MEDIUM = 0x062800; // Medium may have changed. -constexpr u32 ERROR_MEDIUM_REQ = 0x0b5a01; // Operator medium removal request. -constexpr u32 HIGH_ERROR_MASK = 0x00ffffff; +// Actual drive error codes, which fill the remaining 3 bytes +// Numbers more or less match a SCSI sense key (1 nybble) followed by SCSI ASC/ASCQ (2 bytes). +enum class DriveError : u32 +{ + None = 0x00000, // No error. + MotorStopped = 0x20400, // Motor stopped. + NoDiscID = 0x20401, // Disk ID not read. + MediumNotPresent = 0x23a00, // Medium not present / Cover opened. + SeekNotDone = 0x30200, // No seek complete. + ReadError = 0x31100, // Unrecovered read error. + ProtocolError = 0x40800, // Transfer protocol error. + InvalidCommand = 0x52000, // Invalid command operation code. + NoAudioBuf = 0x52001, // Audio Buffer not set. + BlockOOB = 0x52100, // Logical block address out of bounds. + InvalidField = 0x52400, // Invalid field in command packet. + InvalidAudioCommand = 0x52401, // Invalid audio command. + InvalidPeriod = 0x52402, // Configuration out of permitted period. + EndOfUserArea = 0x56300, // End of user area encountered on this track. + MediumChanged = 0x62800, // Medium may have changed. + MediumRemovalRequest = 0xb5a01, // Operator medium removal request. +}; enum class DIInterruptType : int { @@ -130,8 +138,8 @@ void PerformDecryptingRead(u32 position, u32 length, u32 output_address, // Exposed for use by emulated BS2; does not perform any checks on drive state void AudioBufferConfig(bool enable_dtk, u8 dtk_buffer_length); -void SetLowError(u32 low_error); -void SetHighError(u32 high_error); +void SetDriveState(DriveState state); +void SetDriveError(DriveError error); // Used by DVDThread void FinishExecutingCommand(ReplyType reply_type, DIInterruptType interrupt_type, s64 cycles_late, diff --git a/Source/Core/Core/HW/DVD/DVDThread.cpp b/Source/Core/Core/HW/DVD/DVDThread.cpp index 00279c5df4..5f93a456a4 100644 --- a/Source/Core/Core/HW/DVD/DVDThread.cpp +++ b/Source/Core/Core/HW/DVD/DVDThread.cpp @@ -348,7 +348,7 @@ static void FinishRead(u64 id, s64 cycles_late) PanicAlertT("The disc could not be read (at 0x%" PRIx64 " - 0x%" PRIx64 ").", request.dvd_offset, request.dvd_offset + request.length); - DVDInterface::SetHighError(DVDInterface::ERROR_BLOCK_OOB); + DVDInterface::SetDriveError(DVDInterface::DriveError::BlockOOB); interrupt = DVDInterface::DIInterruptType::DEINT; } else From 537e40afb500c1ca8941b757dec4aa9db082099c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 19 Jan 2020 10:45:52 -0800 Subject: [PATCH 2/9] Only update DIMAR and DILENGTH if transfer completed without error Turns out, Gamecube games actually do check DILENGTH, and if DILENGTH is at 0, they'll think the transfer completed successfully even if DEINT is used, since after all, surely that means everything was sent. That caused all sorts of issues, from audio looping when a disc is removed since it's re-using the same buffer to just flat-out crashing instead of showing the disc removed screen. --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index 4404425e82..a7b04ddcd3 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -1283,8 +1283,11 @@ void FinishExecutingCommand(ReplyType reply_type, DIInterruptType interrupt_type else if (reply_type == ReplyType::Interrupt || reply_type == ReplyType::IOS) transfer_size = s_DILENGTH; - s_DIMAR += transfer_size; - s_DILENGTH -= transfer_size; + if (interrupt_type == DIInterruptType::TCINT) + { + s_DIMAR += transfer_size; + s_DILENGTH -= transfer_size; + } switch (reply_type) { From b8715b42d2cb5dbc5c43e7195487985589d0e41c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 19 Jan 2020 12:28:05 -0800 Subject: [PATCH 3/9] Only reset drive chip state (not DI registers) when changing discs Resetting the DI registers disables interrupts, which means any errors reported (for instance) are just not sent though. --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 9 ++++++++- Source/Core/Core/HW/DVD/DVDInterface.h | 1 + Source/Core/Core/IOS/DI/DI.cpp | 2 +- Source/Core/Core/IOS/MIOS.cpp | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index a7b04ddcd3..ae9bc7189e 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -375,6 +375,13 @@ void Reset(bool spinup) s_DICFG.Hex = 0; s_DICFG.CONFIG = 1; // Disable bootrom descrambler + ResetDrive(spinup); +} + +// Resets state on the MN102 chip in the drive itself, but not the DI registers exposed on the +// emulated device, or any inserted disc. +void ResetDrive(bool spinup) +{ s_stream = false; s_stop_at_track_end = false; s_audio_position = 0; @@ -453,7 +460,7 @@ void SetDisc(std::unique_ptr disc, DVDThread::SetDisc(std::move(disc)); SetLidOpen(); - Reset(false); + ResetDrive(false); } bool IsDiscInside() diff --git a/Source/Core/Core/HW/DVD/DVDInterface.h b/Source/Core/Core/HW/DVD/DVDInterface.h index 7a013c484f..4b7de92ed3 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.h +++ b/Source/Core/Core/HW/DVD/DVDInterface.h @@ -111,6 +111,7 @@ enum class EjectCause void Init(); void Reset(bool spinup = true); +void ResetDrive(bool spinup); void Shutdown(); void DoState(PointerWrap& p); diff --git a/Source/Core/Core/IOS/DI/DI.cpp b/Source/Core/Core/IOS/DI/DI.cpp index ebc5016453..a56b5e27e0 100644 --- a/Source/Core/Core/IOS/DI/DI.cpp +++ b/Source/Core/Core/IOS/DI/DI.cpp @@ -268,7 +268,7 @@ std::optional DI::StartIOCtl(const IOCtlRequest& request) { const bool spinup = Memory::Read_U32(request.address + 4); INFO_LOG(IOS_DI, "DVDLowReset %s spinup", spinup ? "with" : "without"); - DVDInterface::Reset(spinup); + DVDInterface::ResetDrive(spinup); ResetDIRegisters(); return DIResult::Success; } diff --git a/Source/Core/Core/IOS/MIOS.cpp b/Source/Core/Core/IOS/MIOS.cpp index 9148619977..901ffe6711 100644 --- a/Source/Core/Core/IOS/MIOS.cpp +++ b/Source/Core/Core/IOS/MIOS.cpp @@ -37,7 +37,7 @@ static void ReinitHardware() // HACK However, resetting DI will reset the DTK config, which is set by the system menu // (and not by MIOS), causing games that use DTK to break. Perhaps MIOS doesn't actually // reset DI fully, in such a way that the DTK config isn't cleared? - // DVDInterface::Reset(); + // DVDInterface::ResetDrive(true); PowerPC::Reset(); Wiimote::ResetAllWiimotes(); // Note: this is specific to Dolphin and is required because we initialised it in Wii mode. From 9a8d426645b55af05d5b9d727081eae1d2a0395f Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 13 Apr 2020 16:37:02 -0700 Subject: [PATCH 4/9] Implement PI_RESET_CODE resetting DI --- Source/Core/Core/HW/ProcessorInterface.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/ProcessorInterface.cpp b/Source/Core/Core/HW/ProcessorInterface.cpp index 041ff50ee8..69893f47ee 100644 --- a/Source/Core/Core/HW/ProcessorInterface.cpp +++ b/Source/Core/Core/HW/ProcessorInterface.cpp @@ -11,6 +11,7 @@ #include "Common/CommonTypes.h" #include "Core/Core.h" #include "Core/CoreTiming.h" +#include "Core/HW/DVD/DVDInterface.h" #include "Core/HW/MMIO.h" #include "Core/HW/SystemTimers.h" #include "Core/IOS/IOS.h" @@ -111,8 +112,18 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base) MMIO::ComplexWrite( [](u32, u32 val) { WARN_LOG(PROCESSORINTERFACE, "Fifo reset (%08x)", val); })); - mmio->Register(base | PI_RESET_CODE, MMIO::DirectRead(&m_ResetCode), - MMIO::DirectWrite(&m_ResetCode)); + mmio->Register(base | PI_RESET_CODE, MMIO::ComplexRead([](u32) { + DEBUG_LOG(PROCESSORINTERFACE, "Read PI_RESET_CODE: %08x", m_ResetCode); + return m_ResetCode; + }), + MMIO::ComplexWrite([](u32, u32 val) { + m_ResetCode = val; + INFO_LOG(PROCESSORINTERFACE, "Wrote PI_RESET_CODE: %08x", m_ResetCode); + if (~m_ResetCode & 0x4) + { + DVDInterface::ResetDrive(true); + } + })); mmio->Register(base | PI_FLIPPER_REV, MMIO::DirectRead(&m_FlipperRev), MMIO::InvalidWrite()); From a73eaf5712f7b74b8c9dbd35981210ec29c8a302 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 12 Apr 2020 22:28:22 -0700 Subject: [PATCH 5/9] Fix DVDLowReset spinup flag being read incorrectly --- Source/Core/Core/IOS/DI/DI.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Core/IOS/DI/DI.cpp b/Source/Core/Core/IOS/DI/DI.cpp index a56b5e27e0..308d076543 100644 --- a/Source/Core/Core/IOS/DI/DI.cpp +++ b/Source/Core/Core/IOS/DI/DI.cpp @@ -266,7 +266,7 @@ std::optional DI::StartIOCtl(const IOCtlRequest& request) return DIResult::Success; case DIIoctl::DVDLowReset: { - const bool spinup = Memory::Read_U32(request.address + 4); + const bool spinup = Memory::Read_U32(request.buffer_in + 4); INFO_LOG(IOS_DI, "DVDLowReset %s spinup", spinup ? "with" : "without"); DVDInterface::ResetDrive(spinup); ResetDIRegisters(); From 0fa96df81883410de5cafebae728578832ccc652 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 13 Apr 2020 15:01:51 -0700 Subject: [PATCH 6/9] Remove DriveState::DiscChangeDetected hack Since both GameCube and Wii resets now work correctly, this hack is not needed anymore. --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index ae9bc7189e..6cdbe46bde 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -855,11 +855,7 @@ void ExecuteCommand(ReplyType reply_type) case 0x40: // Read DiscID INFO_LOG(DVDINTERFACE, "Read DiscID: buffer %08x", s_DIMAR); - // TODO: It doesn't make sense to include DiscChangeDetected here, as it implies that the - // drive is not spinning and reading the disc ID shouldn't change it. However, the Wii Menu - // breaks without it. - if (s_drive_state == DriveState::DiscIdNotRead || - s_drive_state == DriveState::DiscChangeDetected) + if (s_drive_state == DriveState::DiscIdNotRead) { SetDriveState(DriveState::ReadyNoReadsMade); } From 4415df463f268fa9b0e13162327b6ac0c1ad7e38 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 13 Apr 2020 15:41:54 -0700 Subject: [PATCH 7/9] Remove DVDInterface::Reset It only resets the registers, which isn't something that needs to be done except for in DVDInterface::Init --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 32 +++++++++--------------- Source/Core/Core/HW/DVD/DVDInterface.h | 1 - 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index 6cdbe46bde..45ca8a8596 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -345,26 +345,8 @@ void Init() DVDThread::Start(); - Reset(); - s_DICVR.Hex = 1; // Disc Channel relies on cover being open when no disc is inserted - - s_auto_change_disc = CoreTiming::RegisterEvent("AutoChangeDisc", AutoChangeDiscCallback); - s_eject_disc = CoreTiming::RegisterEvent("EjectDisc", EjectDiscCallback); - s_insert_disc = CoreTiming::RegisterEvent("InsertDisc", InsertDiscCallback); - - s_finish_executing_command = - CoreTiming::RegisterEvent("FinishExecutingCommand", FinishExecutingCommandCallback); - - u64 userdata = PackFinishExecutingCommandUserdata(ReplyType::DTK, DIInterruptType::TCINT); - CoreTiming::ScheduleEvent(0, s_finish_executing_command, userdata); -} - -// This doesn't reset any inserted disc or the cover state. -void Reset(bool spinup) -{ - INFO_LOG(DVDINTERFACE, "Reset %s spinup", spinup ? "with" : "without"); - s_DISR.Hex = 0; + s_DICVR.Hex = 1; // Disc Channel relies on cover being open when no disc is inserted s_DICMDBUF[0] = 0; s_DICMDBUF[1] = 0; s_DICMDBUF[2] = 0; @@ -375,7 +357,17 @@ void Reset(bool spinup) s_DICFG.Hex = 0; s_DICFG.CONFIG = 1; // Disable bootrom descrambler - ResetDrive(spinup); + ResetDrive(false); + + s_auto_change_disc = CoreTiming::RegisterEvent("AutoChangeDisc", AutoChangeDiscCallback); + s_eject_disc = CoreTiming::RegisterEvent("EjectDisc", EjectDiscCallback); + s_insert_disc = CoreTiming::RegisterEvent("InsertDisc", InsertDiscCallback); + + s_finish_executing_command = + CoreTiming::RegisterEvent("FinishExecutingCommand", FinishExecutingCommandCallback); + + u64 userdata = PackFinishExecutingCommandUserdata(ReplyType::DTK, DIInterruptType::TCINT); + CoreTiming::ScheduleEvent(0, s_finish_executing_command, userdata); } // Resets state on the MN102 chip in the drive itself, but not the DI registers exposed on the diff --git a/Source/Core/Core/HW/DVD/DVDInterface.h b/Source/Core/Core/HW/DVD/DVDInterface.h index 4b7de92ed3..38ada994c1 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.h +++ b/Source/Core/Core/HW/DVD/DVDInterface.h @@ -110,7 +110,6 @@ enum class EjectCause }; void Init(); -void Reset(bool spinup = true); void ResetDrive(bool spinup); void Shutdown(); void DoState(PointerWrap& p); From e5e23c6b27b26b9fc41d20d07a15fec1eb0d842f Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 13 Apr 2020 15:58:19 -0700 Subject: [PATCH 8/9] Mark several DVDInterface functions as static --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 36 ++++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index 45ca8a8596..9d5afafd5c 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -186,19 +186,19 @@ static void EjectDiscCallback(u64 userdata, s64 cyclesLate); static void InsertDiscCallback(u64 userdata, s64 cyclesLate); static void FinishExecutingCommandCallback(u64 userdata, s64 cycles_late); -void SetLidOpen(); +static void SetLidOpen(); -void UpdateInterrupts(); -void GenerateDIInterrupt(DIInterruptType _DVDInterrupt); +static void UpdateInterrupts(); +static void GenerateDIInterrupt(DIInterruptType dvd_interrupt); -bool ExecuteReadCommand(u64 dvd_offset, u32 output_address, u32 dvd_length, u32 output_length, - const DiscIO::Partition& partition, ReplyType reply_type, - DIInterruptType* interrupt_type); +static bool ExecuteReadCommand(u64 dvd_offset, u32 output_address, u32 dvd_length, + u32 output_length, const DiscIO::Partition& partition, + ReplyType reply_type, DIInterruptType* interrupt_type); -u64 PackFinishExecutingCommandUserdata(ReplyType reply_type, DIInterruptType interrupt_type); +static u64 PackFinishExecutingCommandUserdata(ReplyType reply_type, DIInterruptType interrupt_type); -void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& partition, u32 output_address, - ReplyType reply_type); +static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& partition, + u32 output_address, ReplyType reply_type); void DoState(PointerWrap& p) { @@ -542,7 +542,7 @@ bool AutoChangeDisc() return true; } -void SetLidOpen() +static void SetLidOpen() { u32 old_value = s_DICVR.CVR; s_DICVR.CVR = IsDiscInside() ? 0 : 1; @@ -631,7 +631,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base) MMIO::InvalidWrite()); } -void UpdateInterrupts() +static void UpdateInterrupts() { const bool set_mask = (s_DISR.DEINT & s_DISR.DEINTMASK) || (s_DISR.TCINT & s_DISR.TCINTMASK) || (s_DISR.BRKINT & s_DISR.BRKINTMASK) || @@ -643,7 +643,7 @@ void UpdateInterrupts() CoreTiming::ForceExceptionCheck(50); } -void GenerateDIInterrupt(DIInterruptType dvd_interrupt) +static void GenerateDIInterrupt(DIInterruptType dvd_interrupt) { switch (dvd_interrupt) { @@ -735,9 +735,9 @@ static bool CheckReadPreconditions() } // Iff false is returned, ScheduleEvent must be used to finish executing the command -bool ExecuteReadCommand(u64 dvd_offset, u32 output_address, u32 dvd_length, u32 output_length, - const DiscIO::Partition& partition, ReplyType reply_type, - DIInterruptType* interrupt_type) +static bool ExecuteReadCommand(u64 dvd_offset, u32 output_address, u32 dvd_length, + u32 output_length, const DiscIO::Partition& partition, + ReplyType reply_type, DIInterruptType* interrupt_type) { if (!CheckReadPreconditions()) { @@ -1244,7 +1244,7 @@ void AudioBufferConfig(bool enable_dtk, u8 dtk_buffer_length) INFO_LOG(DVDINTERFACE, "DTK disabled"); } -u64 PackFinishExecutingCommandUserdata(ReplyType reply_type, DIInterruptType interrupt_type) +static u64 PackFinishExecutingCommandUserdata(ReplyType reply_type, DIInterruptType interrupt_type) { return (static_cast(reply_type) << 32) + static_cast(interrupt_type); } @@ -1317,8 +1317,8 @@ void FinishExecutingCommand(ReplyType reply_type, DIInterruptType interrupt_type // Determines from a given read request how much of the request is buffered, // and how much is required to be read from disc. -void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& partition, u32 output_address, - ReplyType reply_type) +static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& partition, + u32 output_address, ReplyType reply_type) { // The drive continues to read 1 MiB beyond the last read position when idle. // If a future read falls within this window, part of the read may be returned From 9183c0c4825b297ab841c518ccf3118447608f3a Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 19 Jan 2020 16:04:23 -0800 Subject: [PATCH 9/9] Bump state version --- Source/Core/Core/State.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 2124ebca45..789d08f275 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -74,7 +74,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 121; // Last changed in PR 8988 +constexpr u32 STATE_VERSION = 122; // Last changed in PR 8571 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list,