From bb741856b776e18a0ca27cbac4300d2b9f2633be Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sun, 15 Dec 2024 22:52:06 +0000 Subject: [PATCH 1/2] Fix a timing bug in the video generation. All the timing periods (front porch, sync pulse, back porch, visible video) were one clock cycle too short thanks to an off-by-one error when I was counting the overhead of the timing PIO function. This is what led to rolling video noise and slightly fuzzy video capture - monitors and capture devices were trying to grab 640 pixels out of a period that was only 639 clock cycles in length. Now the video looks much sharper. --- src/vga/mod.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/vga/mod.rs b/src/vga/mod.rs index 4e864aa..8f108b0 100644 --- a/src/vga/mod.rs +++ b/src/vga/mod.rs @@ -803,7 +803,7 @@ impl ScanlineTimingBuffer { if vsync { value |= 1 << 1; } - value |= (period - 6) << 2; + value |= (period - FIXED_CLOCKS_PER_TIMING_PULSE) << 2; value | command << 16 } } @@ -1690,6 +1690,9 @@ static PIXEL_DATA_BUFFER_ODD: LineBuffer = LineBuffer::new_odd(); /// ``` static TEXT_COLOUR_LOOKUP: TextColourLookup = TextColourLookup::blank(); +/// How many fixed clock cycles there are per timing pulse. +const FIXED_CLOCKS_PER_TIMING_PULSE: u32 = 5; + // ----------------------------------------------------------------------------- // Functions // ----------------------------------------------------------------------------- @@ -1710,28 +1713,41 @@ pub fn init( let (mut pio, sm0, sm1, _sm2, _sm3) = pio.split(resets); // This program runs the timing loop. We post timing data (i.e. the length - // of each period, along with what the H-Sync and V-Sync pins should do) - // and it sets the GPIO pins and busy-waits the appropriate amount of - // time. It also takes an extra 'instruction' which we can use to trigger - // the appropriate interrupts. + // of each period, along with what the H-Sync and V-Sync pins should do) and + // it sets the GPIO pins and busy-waits the appropriate amount of time. It + // also takes an extra 'instruction' which we can use to trigger the + // appropriate interrupts. + // + // Note that the timing period value should be: + // + // timing_period = actual_timing_period - FIXED_CLOCKS_PER_TIMING_PULSE + // + // This is because there are unavoidable clock cycles within the algorithm. + // Currently FIXED_CLOCKS_PER_TIMING_PULSE should be set to 5. // // Post where value: // // // The SM will execute the instruction (typically either a NOP or an IRQ), - // set the H-Sync and V-Sync pins as desired, then wait the given number - // of clock cycles. + // set the H-Sync and V-Sync pins as desired, then wait the given number of + // clock cycles. // // Note: autopull should be set to 32-bits, OSR is set to shift right. let timing_program = pio_proc::pio_asm!( ".wrap_target" - // Step 1. Push next 2 bits of OSR into `pins`, to set H-Sync and V-Sync + // Step 1. Push next 2 bits of OSR into `pins`, to set H-Sync and V-Sync. + // Takes 1 clock cycle. "out pins, 2" // Step 2. Push last 14 bits of OSR into X for the timing loop. + // Takes 1 clock cycle. "out x, 14" - // Step 3. Execute bottom 16-bits of OSR as an instruction. This take two cycles. + // Step 3. Execute bottom 16-bits of OSR as an instruction. + // This take two cycles, always. "out exec, 16" // Spin until X is zero + // Takes X + 1 clock cycles because the branch is conditioned on the initial value of the register. + // i.e. X = 0 => 1 clock cycle (the jmp when x = 0) + // i.e. X = 1 => 2 clock cycles (the jmp when x = 1 and again when x = 0) "loop0:" "jmp x-- loop0" ".wrap" From 41cee805fe35451feed1d24beb08172b02fbe43e Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sun, 15 Dec 2024 22:52:38 +0000 Subject: [PATCH 2/2] Nudge the video left. Makes my VGA to HDMI converter a bit happier. Need to check with an oscilloscope to see whether it's right or not. --- src/vga/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vga/mod.rs b/src/vga/mod.rs index 8f108b0..c7e4bb4 100644 --- a/src/vga/mod.rs +++ b/src/vga/mod.rs @@ -685,7 +685,7 @@ impl ScanlineTimingBuffer { // Back porch. Adjusted by a few clocks to account for interrupt + // PIO SM start latency. Self::make_timing( - (timings.2 * Self::CLOCKS_PER_PIXEL) - 5, + (timings.2 * Self::CLOCKS_PER_PIXEL) - 10, hsync.disabled(), vsync.disabled(), RaiseIrq::None, @@ -694,7 +694,7 @@ impl ScanlineTimingBuffer { // moving. Adjusted to compensate for changes made to previous // period to ensure scan-line remains at correct length. Self::make_timing( - (timings.3 * Self::CLOCKS_PER_PIXEL) + 5, + (timings.3 * Self::CLOCKS_PER_PIXEL) + 10, hsync.disabled(), vsync.disabled(), RaiseIrq::Irq0,