From 4ea9287a097e26f1a8e6aca7c4416b4cca0dfc7a Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 13 Feb 2022 14:08:16 -0800 Subject: [PATCH 1/2] CommandProcessor: Move unknown opcode log message before the panic alert This way, the extra information is already in the log by the time the panic alert appears, which is slightly more convenient for debugging. --- Source/Core/VideoCommon/CommandProcessor.cpp | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 2203e235a7..41c773429a 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -628,22 +628,6 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess) // to keep around for unexpected cases. const bool suppress_panic_alert = (cmd_byte <= 0x7) || (cmd_byte == 0x3f); - if (!s_is_fifo_error_seen && !suppress_panic_alert) - { - s_is_fifo_error_seen = true; - - // TODO(Omega): Maybe dump FIFO to file on this error - PanicAlertFmtT("GFX FIFO: Unknown Opcode ({0:#04x} @ {1}, preprocess={2}).\n" - "This means one of the following:\n" - "* The emulated GPU got desynced, disabling dual core can help\n" - "* Command stream corrupted by some spurious memory bug\n" - "* This really is an unknown opcode (unlikely)\n" - "* Some other sort of bug\n\n" - "Further errors will be sent to the Video Backend log and\n" - "Dolphin will now likely crash or hang. Enjoy.", - cmd_byte, fmt::ptr(buffer), preprocess); - } - // We always generate this log message, though we only generate the panic alerts once. // // PC and LR are generally inaccurate in dual-core and are still misleading in single-core @@ -674,6 +658,22 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess) fifo.bFF_GPLinkEnable.load(std::memory_order_relaxed) ? "true" : "false", fifo.bFF_HiWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", PC, LR); + + if (!s_is_fifo_error_seen && !suppress_panic_alert) + { + s_is_fifo_error_seen = true; + + // TODO(Omega): Maybe dump FIFO to file on this error + PanicAlertFmtT("GFX FIFO: Unknown Opcode ({0:#04x} @ {1}, preprocess={2}).\n" + "This means one of the following:\n" + "* The emulated GPU got desynced, disabling dual core can help\n" + "* Command stream corrupted by some spurious memory bug\n" + "* This really is an unknown opcode (unlikely)\n" + "* Some other sort of bug\n\n" + "Further errors will be sent to the Video Backend log and\n" + "Dolphin will now likely crash or hang. Enjoy.", + cmd_byte, fmt::ptr(buffer), preprocess); + } } } // namespace CommandProcessor From 07578d8f1dc47513da1b3378fd4386defd3accbd Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Sun, 13 Feb 2022 14:14:46 -0800 Subject: [PATCH 2/2] CommandProcessor: Log ignored unknown opcodes at warn level Large amounts of logging can have an impact on performance, so moving the ones that have been determined to not matter to the warn level gives a way to hide those messages without hiding actual errors (and also gives a fast visual way of distinguishing between ignored and non-ignored ones due to the different colors). --- Source/Core/VideoCommon/CommandProcessor.cpp | 46 +++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 41c773429a..890d7ebfa4 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -628,6 +628,9 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess) // to keep around for unexpected cases. const bool suppress_panic_alert = (cmd_byte <= 0x7) || (cmd_byte == 0x3f); + const auto log_level = + suppress_panic_alert ? Common::Log::LogLevel::LWARNING : Common::Log::LogLevel::LERROR; + // We always generate this log message, though we only generate the panic alerts once. // // PC and LR are generally inaccurate in dual-core and are still misleading in single-core @@ -637,27 +640,28 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess) // PC and LR are meaningless when using the fifoplayer, and will generally not be helpful if the // unknown opcode is inside of a display list. Also note that the changes in GPFifo.h are not // accurate and may introduce timing issues. - ERROR_LOG_FMT(VIDEO, - "FIFO: Unknown Opcode {:#04x} @ {}, preprocessing = {}, CPBase: {:#010x}, CPEnd: " - "{:#010x}, CPHiWatermark: {:#010x}, CPLoWatermark: {:#010x}, CPReadWriteDistance: " - "{:#010x}, CPWritePointer: {:#010x}, CPReadPointer: {:#010x}, CPBreakpoint: " - "{:#010x}, bFF_GPReadEnable: {}, bFF_BPEnable: {}, bFF_BPInt: {}, bFF_Breakpoint: " - "{}, bFF_GPLinkEnable: {}, bFF_HiWatermarkInt: {}, bFF_LoWatermarkInt: {}, " - "approximate PC: {:08x}, approximate LR: {:08x}", - cmd_byte, fmt::ptr(buffer), preprocess ? "yes" : "no", - fifo.CPBase.load(std::memory_order_relaxed), - fifo.CPEnd.load(std::memory_order_relaxed), fifo.CPHiWatermark, fifo.CPLoWatermark, - fifo.CPReadWriteDistance.load(std::memory_order_relaxed), - fifo.CPWritePointer.load(std::memory_order_relaxed), - fifo.CPReadPointer.load(std::memory_order_relaxed), - fifo.CPBreakpoint.load(std::memory_order_relaxed), - fifo.bFF_GPReadEnable.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_BPEnable.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_BPInt.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_Breakpoint.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_GPLinkEnable.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_HiWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", - fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", PC, LR); + GENERIC_LOG_FMT( + Common::Log::LogType::VIDEO, log_level, + "FIFO: Unknown Opcode {:#04x} @ {}, preprocessing = {}, CPBase: {:#010x}, CPEnd: " + "{:#010x}, CPHiWatermark: {:#010x}, CPLoWatermark: {:#010x}, CPReadWriteDistance: " + "{:#010x}, CPWritePointer: {:#010x}, CPReadPointer: {:#010x}, CPBreakpoint: " + "{:#010x}, bFF_GPReadEnable: {}, bFF_BPEnable: {}, bFF_BPInt: {}, bFF_Breakpoint: " + "{}, bFF_GPLinkEnable: {}, bFF_HiWatermarkInt: {}, bFF_LoWatermarkInt: {}, " + "approximate PC: {:08x}, approximate LR: {:08x}", + cmd_byte, fmt::ptr(buffer), preprocess ? "yes" : "no", + fifo.CPBase.load(std::memory_order_relaxed), fifo.CPEnd.load(std::memory_order_relaxed), + fifo.CPHiWatermark, fifo.CPLoWatermark, + fifo.CPReadWriteDistance.load(std::memory_order_relaxed), + fifo.CPWritePointer.load(std::memory_order_relaxed), + fifo.CPReadPointer.load(std::memory_order_relaxed), + fifo.CPBreakpoint.load(std::memory_order_relaxed), + fifo.bFF_GPReadEnable.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_BPEnable.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_BPInt.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_Breakpoint.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_GPLinkEnable.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_HiWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", + fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", PC, LR); if (!s_is_fifo_error_seen && !suppress_panic_alert) {