From f2edc506758fb78292d4715b69e916d047422238 Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Sat, 17 Dec 2016 22:03:50 +0000 Subject: [PATCH] cdvdgigaherz: Improve prefetch logic Avoid reading past the end of the disk. Avoid waiting when there are prefetches remaining. Fix the maths so that the first prefetch after a request attempts to read the next block of sectors and not the block of sectors that was just read (which will just be skipped anyway because the data has just been cached). Avoid potential prefetch after disk is swapped (though disc swap doesn't work properly if you just eject and insert a different disk). Stop prefetching on disk read failure (Suikoden hits this case - 2048 byte reads are requested, but only 2352 byte reads will succeed). Also reduce the read retry count to 2. --- plugins/cdvdGigaherz/src/CDVD.cpp | 9 ++-- plugins/cdvdGigaherz/src/CDVD.h | 6 +++ plugins/cdvdGigaherz/src/ReadThread.cpp | 66 +++++++++++++++---------- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/plugins/cdvdGigaherz/src/CDVD.cpp b/plugins/cdvdGigaherz/src/CDVD.cpp index 72e1a64460..4fefcb9b92 100644 --- a/plugins/cdvdGigaherz/src/CDVD.cpp +++ b/plugins/cdvdGigaherz/src/CDVD.cpp @@ -113,8 +113,7 @@ bool weAreInNewDiskCB = false; std::unique_ptr src; -extern s32 prefetch_last_lba; -extern s32 prefetch_last_mode; +extern CacheRequest g_last_sector_block; /////////////////////////////////////////////////////////////////////////////// // keepAliveThread throws a read event regularly to prevent drive spin down // @@ -130,10 +129,10 @@ void keepAliveThread() []() { return !s_keepalive_is_open; })) { //printf(" * keepAliveThread: polling drive.\n"); - if (prefetch_last_mode == CDVD_MODE_2048) - src->ReadSectors2048(prefetch_last_lba, 1, throwaway); + if (g_last_sector_block.mode == CDVD_MODE_2048) + src->ReadSectors2048(g_last_sector_block.lsn, 1, throwaway); else - src->ReadSectors2352(prefetch_last_lba, 1, throwaway); + src->ReadSectors2352(g_last_sector_block.lsn, 1, throwaway); } printf(" * CDVD: KeepAlive thread finished.\n"); diff --git a/plugins/cdvdGigaherz/src/CDVD.h b/plugins/cdvdGigaherz/src/CDVD.h index 904e05bc9e..27f6aae3ed 100644 --- a/plugins/cdvdGigaherz/src/CDVD.h +++ b/plugins/cdvdGigaherz/src/CDVD.h @@ -47,6 +47,12 @@ extern track tracks[100]; extern int curDiskType; extern int curTrayStatus; +struct CacheRequest +{ + u32 lsn; + s32 mode; +}; + struct toc_entry { u32 lba; diff --git a/plugins/cdvdGigaherz/src/ReadThread.cpp b/plugins/cdvdGigaherz/src/ReadThread.cpp index b84892e297..bce351feb6 100644 --- a/plugins/cdvdGigaherz/src/ReadThread.cpp +++ b/plugins/cdvdGigaherz/src/ReadThread.cpp @@ -25,12 +25,6 @@ const u32 sectors_per_read = 16; static_assert(sectors_per_read > 1 && !(sectors_per_read & (sectors_per_read - 1)), "sectors_per_read must by a power of 2"); -struct CacheRequest -{ - u32 lsn; - s32 mode; -}; - struct SectorInfo { u32 lsn; @@ -39,11 +33,7 @@ struct SectorInfo u8 data[2352 * sectors_per_read]; }; -const s32 prefetch_max_blocks = 16; -s32 prefetch_mode = 0; -s32 prefetch_last_lba = 0; -s32 prefetch_last_mode = 0; -s32 prefetch_left = 0; +CacheRequest g_last_sector_block; static std::thread s_thread; @@ -122,8 +112,9 @@ bool cdvdReadBlockOfSectors(u32 sector, s32 mode, u8 *data) { u32 count = std::min(sectors_per_read, src->GetSectorCount() - sector); - // TODO: Is it really necessary to retry 3 times if it fails? - for (int tries = 0; tries < 4; ++tries) { + // TODO: Is it really necessary to retry if it fails? I'm not sure the + // second time is really going to be any better. + for (int tries = 0; tries < 2; ++tries) { if (mode == CDVD_MODE_2048) { if (src->ReadSectors2048(sector, count, data)) return true; @@ -175,6 +166,7 @@ bool cdvdUpdateDiscStatus() void cdvdThread() { u8 buffer[2352 * sectors_per_read]; + u32 prefetches_left = 0; printf(" * CDVD: IO thread started...\n"); std::unique_lock guard(s_notify_lock); @@ -183,15 +175,18 @@ void cdvdThread() if (cdvdUpdateDiscStatus()) { // Need to sleep some to avoid an aggressive spin that sucks the cpu dry. s_notify_cv.wait_for(guard, std::chrono::milliseconds(10)); + prefetches_left = 0; continue; } - s_notify_cv.wait_for(guard, std::chrono::milliseconds(prefetch_left ? 1 : 250)); + if (prefetches_left == 0) + s_notify_cv.wait_for(guard, std::chrono::milliseconds(250)); // check again to make sure we're not done here... if (!cdvd_is_open) break; + // Read request bool handling_request = false; CacheRequest request; @@ -201,27 +196,44 @@ void cdvdThread() request = s_request_queue.front(); s_request_queue.pop(); handling_request = true; - } else { - request.lsn = prefetch_last_lba; - request.mode = prefetch_last_mode; } } - if (handling_request || prefetch_left) { - if (!cdvdCacheCheck(request.lsn, request.mode)) - if (cdvdReadBlockOfSectors(request.lsn, request.mode, buffer)) - cdvdCacheUpdate(request.lsn, request.mode, buffer); + if (!handling_request) { + if (prefetches_left == 0) + continue; - if (handling_request) { - prefetch_last_lba = request.lsn; - prefetch_last_mode = request.mode; + --prefetches_left; - prefetch_left = prefetch_max_blocks; + u32 next_prefetch_lsn = g_last_sector_block.lsn + sectors_per_read; + request = {next_prefetch_lsn, g_last_sector_block.mode}; + } + + // Handle request + if (!cdvdCacheCheck(request.lsn, request.mode)) { + if (cdvdReadBlockOfSectors(request.lsn, request.mode, buffer)) { + cdvdCacheUpdate(request.lsn, request.mode, buffer); } else { - prefetch_last_lba += sectors_per_read; - prefetch_left--; + // If the read fails, further reads are likely to fail too. + prefetches_left = 0; + continue; } } + + g_last_sector_block = request; + + if (!handling_request) + continue; + + // Prefetch + u32 next_prefetch_lsn = g_last_sector_block.lsn + sectors_per_read; + if (next_prefetch_lsn >= src->GetSectorCount()) { + prefetches_left = 0; + } else { + const u32 max_prefetches = 16; + u32 remaining = src->GetSectorCount() - next_prefetch_lsn; + prefetches_left = std::min((remaining + sectors_per_read - 1) / sectors_per_read, max_prefetches); + } } printf(" * CDVD: IO thread finished.\n"); }