From c6ede090350c9f413b1df8a4de850da5345797e2 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 27 Jun 2021 17:24:53 +0200 Subject: [PATCH 1/2] DVDInterface: Don't evict cache block i unless block i + 2 was read Intends to fix https://bugs.dolphin-emu.org/issues/12279. I have hardware tested the behavior, but I haven't tested the game. --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 29 ++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index dd9a2c0ccf..018492319e 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -1512,27 +1512,38 @@ static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& parti dvd_offset += DVD_ECC_BLOCK_SIZE; } while (length > 0); - // Update the buffer based on this read. Based on experimental testing, - // we will only reuse the old buffer while reading forward. Note that the - // buffer start we calculate here is not the actual start of the buffer - - // it is just the start of the portion we need to read. + // Evict blocks from the buffer which are unlikely to be used again after this read, + // so that the buffer gets space for prefetching new blocks. Based on hardware testing, + // the blocks which are kept are the most recently accessed block, the block immediately + // before it, and all blocks after it. + // + // If the block immediately before the most recently accessed block is not kept, loading + // screens in Pitfall: The Lost Expedition are longer than they should be. + // https://bugs.dolphin-emu.org/issues/12279 const u64 last_block = dvd_offset; - if (last_block == buffer_start + DVD_ECC_BLOCK_SIZE && buffer_start != buffer_end) + constexpr u32 BUFFER_BACKWARD_SEEK_LIMIT = DVD_ECC_BLOCK_SIZE * 2; + if (last_block - buffer_start <= BUFFER_BACKWARD_SEEK_LIMIT && buffer_start != buffer_end) { - // Special case: reading less than one block at the start of the - // buffer won't change the buffer state + // Special case: reading the first two blocks of the buffer doesn't change the buffer state } else { + // Note that the s_read_buffer_start_offset value we calculate here is not the + // actual start of the buffer - it is just the start of the portion we need to read. + // The actual start of the buffer is s_read_buffer_end_offset - STREAMING_BUFFER_SIZE. if (last_block >= buffer_end) + { // Full buffer read s_read_buffer_start_offset = last_block; + } else + { // Partial buffer read s_read_buffer_start_offset = buffer_end; + } - s_read_buffer_end_offset = last_block + STREAMING_BUFFER_SIZE - DVD_ECC_BLOCK_SIZE; - // Assume the buffer starts reading right after the end of the last operation + s_read_buffer_end_offset = last_block + STREAMING_BUFFER_SIZE - BUFFER_BACKWARD_SEEK_LIMIT; + // Assume the buffer starts prefetching new blocks right after the end of the last operation s_read_buffer_start_time = current_time + ticks_until_completion; s_read_buffer_end_time = s_read_buffer_start_time + From 4648e1a035c02c120d4350e10d6bad4c43ee53a5 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 28 Jun 2021 10:48:38 +0200 Subject: [PATCH 2/2] DVDInterface: Fix small backwards seek after non-cached seek If we seek to a block that isn't in the cache, the block prior to it doesn't end up getting read into the buffer. --- Source/Core/Core/HW/DVD/DVDInterface.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Source/Core/Core/HW/DVD/DVDInterface.cpp b/Source/Core/Core/HW/DVD/DVDInterface.cpp index 018492319e..1abf249c81 100644 --- a/Source/Core/Core/HW/DVD/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVD/DVDInterface.cpp @@ -1365,6 +1365,9 @@ static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& parti const u32 ticks_per_second = SystemTimers::GetTicksPerSecond(); const bool wii_disc = DVDThread::GetDiscType() == DiscIO::Platform::WiiDisc; + // Whether we have performed a seek. + bool seek = false; + // Where the DVD read head is (usually parked at the end of the buffer, // unless we've interrupted it mid-buffer-read). u64 head_position; @@ -1379,6 +1382,7 @@ static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& parti // It's rounded to a whole ECC block and never uses Wii partition addressing. u64 dvd_offset = DVDThread::PartitionOffsetToRawOffset(offset, partition); dvd_offset = Common::AlignDown(dvd_offset, DVD_ECC_BLOCK_SIZE); + const u64 first_block = dvd_offset; if (SConfig::GetInstance().bFastDiscSpeed) { @@ -1473,6 +1477,7 @@ static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& parti if (dvd_offset != head_position) { // Unbuffered seek+read + seek = true; ticks_until_completion += static_cast( ticks_per_second * DVDMath::CalculateSeekTime(head_position, dvd_offset)); @@ -1543,6 +1548,13 @@ static void ScheduleReads(u64 offset, u32 length, const DiscIO::Partition& parti } s_read_buffer_end_offset = last_block + STREAMING_BUFFER_SIZE - BUFFER_BACKWARD_SEEK_LIMIT; + if (seek) + { + // If we seek, the block preceding the first accessed block never gets read into the buffer + s_read_buffer_end_offset = + std::max(s_read_buffer_end_offset, first_block + STREAMING_BUFFER_SIZE); + } + // Assume the buffer starts prefetching new blocks right after the end of the last operation s_read_buffer_start_time = current_time + ticks_until_completion; s_read_buffer_end_time =