From d5c45cb13c18803ddfbad8ac30cc91499d6755bc Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Sun, 9 Aug 2026 22:17:54 -0700 Subject: [PATCH 1/2] ati: Fix VBLANK status field updates insert_bits expects its final argument to be the field width, but both ATI VBL callbacks passed the current VBL state. When VBL ended, that meant using zero as the width, so the update was a no-op. After the first vertical blank began, the ATI_CRTC_VBLANK status bit was never cleared, confusing guest code that polls the live VBL state. --- devices/video/atimach64gx.cpp | 2 +- devices/video/atirage.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/devices/video/atimach64gx.cpp b/devices/video/atimach64gx.cpp index 91ec258cf1..7b744d6cd6 100644 --- a/devices/video/atimach64gx.cpp +++ b/devices/video/atimach64gx.cpp @@ -904,7 +904,7 @@ void AtiMach64Gx::get_cursor_position(int& x, int& y) { int AtiMach64Gx::device_postinit() { this->vbl_cb = [this](uint8_t irq_line_state) { - insert_bits(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, irq_line_state); + insert_bits(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, 1); if (irq_line_state) { set_bit(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VBLANK_INT); set_bit(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VLINE_INT); diff --git a/devices/video/atirage.cpp b/devices/video/atirage.cpp index e56a1b28ae..5390e36222 100644 --- a/devices/video/atirage.cpp +++ b/devices/video/atirage.cpp @@ -1033,7 +1033,7 @@ void ATIRage::get_cursor_position(int& x, int& y) { int ATIRage::device_postinit() { this->vbl_cb = [this](uint8_t irq_line_state) { - insert_bits(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, irq_line_state); + insert_bits(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, 1); if (irq_line_state) { set_bit(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VBLANK_INT); set_bit(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VLINE_INT); From 1cc1fde2bdf77abd7c646616a9409948fe638273 Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Sun, 9 Aug 2026 22:18:03 -0700 Subject: [PATCH 2/2] ati: Latch enabled display interrupts until acknowledged More accurately model the VBL interrupt behavior of ATI chips. When VBL begins, set the interrupt-pending bits and derive the PCI IRQ from each pending bit and its matching enable bit. Keep the pending state and PCI IRQ asserted until the guest driver acknowledges the event, even if VBL has already ended. The interrupt may therefore be asserted for less or more time than the physical VBL interval, depending on when the guest acknowledges it. This allows the driver to acknowledge the interrupt and resume other work while VBL may still be active. Previously the PCI IRQ followed the entire VBL interval, so it could remain asserted after the pending event had been acknowledged. Track the PCI IRQ output level and notify the interrupt controller only when that level changes. --- devices/video/atimach64gx.cpp | 35 +++++++++++++++++++---------------- devices/video/atimach64gx.h | 2 ++ devices/video/atirage.cpp | 30 ++++++++++++++++++------------ devices/video/atirage.h | 2 ++ 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/devices/video/atimach64gx.cpp b/devices/video/atimach64gx.cpp index 7b744d6cd6..8d928308b8 100644 --- a/devices/video/atimach64gx.cpp +++ b/devices/video/atimach64gx.cpp @@ -525,6 +525,7 @@ void AtiMach64Gx::write_reg(uint32_t reg_offset, uint32_t value, uint32_t size) new_value = (old_value & bits_read_only) | (new_value & ~bits_read_only); WRITE_VALUE_AND_LOG(ATIINTERRUPT); + this->update_interrupt(); return; } case ATI_CRTC_GEN_CNTL: @@ -914,26 +915,28 @@ int AtiMach64Gx::device_postinit() #endif } - bool do_interrupt = - bit_set(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VBLANK_INT_EN) || - bit_set(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VLINE_INT_EN) || -#if 1 -#else - bit_set(this->regs[ATI_CRTC_GEN_CNTL], ATI_CRTC_VSYNC_INT_EN) || -#endif - 0; - - LOG_F(ATIINTERRUPT, "%s: irq_line_state:%d do_interrupt:%d CRTC_INT_CNTL:%08x", - this->name.c_str(), irq_line_state, do_interrupt, - this->regs[ATI_CRTC_INT_CNTL]); - - if (do_interrupt) { - this->pci_interrupt(irq_line_state); - } + this->update_interrupt(); }; return 0; } +void AtiMach64Gx::update_interrupt() +{ + uint32_t int_cntl = this->regs[ATI_CRTC_INT_CNTL]; + bool new_pci_irq_line_state = + (bit_set(int_cntl, ATI_CRTC_VBLANK_INT_EN) && + bit_set(int_cntl, ATI_CRTC_VBLANK_INT)) || + (bit_set(int_cntl, ATI_CRTC_VLINE_INT_EN) && + bit_set(int_cntl, ATI_CRTC_VLINE_INT)); + + if (new_pci_irq_line_state != this->pci_irq_line_state) { + this->pci_irq_line_state = new_pci_irq_line_state; + LOG_F(ATIINTERRUPT, "%s: pci_irq_line_state:%d CRTC_INT_CNTL:%08x", + this->name.c_str(), this->pci_irq_line_state, int_cntl); + this->pci_interrupt(this->pci_irq_line_state); + } +} + // ========================== IBM RGB514 related code ========================== const char* AtiMach64Gx::rgb514_get_reg_name(uint32_t reg_addr) { diff --git a/devices/video/atimach64gx.h b/devices/video/atimach64gx.h index 582975578d..43fc8e142b 100644 --- a/devices/video/atimach64gx.h +++ b/devices/video/atimach64gx.h @@ -76,8 +76,10 @@ class AtiMach64Gx : public PCIDevice, public VideoCtrlBase { private: void change_one_bar(uint32_t &aperture, uint32_t aperture_size, uint32_t aperture_new, int bar_num); + void update_interrupt(); uint32_t regs[256] = {}; // internal registers + bool pci_irq_line_state = false; int vram_size; diff --git a/devices/video/atirage.cpp b/devices/video/atirage.cpp index 5390e36222..1fd54d32fb 100644 --- a/devices/video/atirage.cpp +++ b/devices/video/atirage.cpp @@ -658,6 +658,8 @@ void ATIRage::write_reg(uint32_t reg_offset, uint32_t value, uint32_t size) { } WRITE_VALUE_AND_LOG(9); + if (reg_num == ATI_CRTC_INT_CNTL) + this->update_interrupt(); } bool ATIRage::io_access_allowed(uint32_t offset) { @@ -1043,22 +1045,26 @@ int ATIRage::device_postinit() #endif } - bool do_interrupt = - bit_set(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VBLANK_INT_EN) || - bit_set(this->regs[ATI_CRTC_INT_CNTL], ATI_CRTC_VLINE_INT_EN) || -#if 1 -#else - bit_set(this->regs[ATI_CRTC_GEN_CNTL], ATI_CRTC_VSYNC_INT_EN) || -#endif - 0; - - if (do_interrupt) { - this->pci_interrupt(irq_line_state); - } + this->update_interrupt(); }; return 0; } +void ATIRage::update_interrupt() +{ + uint32_t int_cntl = this->regs[ATI_CRTC_INT_CNTL]; + bool new_pci_irq_line_state = + (bit_set(int_cntl, ATI_CRTC_VBLANK_INT_EN) && + bit_set(int_cntl, ATI_CRTC_VBLANK_INT)) || + (bit_set(int_cntl, ATI_CRTC_VLINE_INT_EN) && + bit_set(int_cntl, ATI_CRTC_VLINE_INT)); + + if (new_pci_irq_line_state != this->pci_irq_line_state) { + this->pci_irq_line_state = new_pci_irq_line_state; + this->pci_interrupt(this->pci_irq_line_state); + } +} + // =================================== Draw Engine ===================================== void ATIRage::begin_drawing(uint32_t initiator, uint32_t value) { switch(initiator) { diff --git a/devices/video/atirage.h b/devices/video/atirage.h index 2b636197e5..0bd6853384 100644 --- a/devices/video/atirage.h +++ b/devices/video/atirage.h @@ -89,6 +89,7 @@ class ATIRage : public PCIDevice, public VideoCtrlBase { private: void change_one_bar(uint32_t &aperture, uint32_t aperture_size, uint32_t aperture_new, int bar_num); + void update_interrupt(); void begin_drawing(uint32_t initiator, uint32_t value); void draw_rect(uint32_t width, uint32_t height); @@ -102,6 +103,7 @@ class ATIRage : public PCIDevice, public VideoCtrlBase { uint8_t plls[64] = {}; // internal PLL registers uint8_t cmd_fifo_size = 0; + bool pci_irq_line_state = false; bool host_data_active = false; uint32_t host_dst_width = 0;