scsibus: Use bus name in log messages.

Init them to defaults.
Make CONFIG_CNTL 8 bytes so unaligned read/write is easier.
Write changes to CONFIG_CNTL even if offset + size is > 4.
Log aperture change only if first byte (LSB) of CONFIG_CNTL is written. The driver might only write to the 2 most significant bytes which won't affect aperture size.
Offset and size are applied to the destination, not to source value, so we shouldn't use extract_bits when logging value.
Use ATI bitfield enums in switch statement.

devices/common/scsi/scsibus.cpp

@@ -52,7 +52,7 @@ ScsiBus::ScsiBus(const std::string name)
 void ScsiBus::register_device(int id, ScsiDevice* dev_obj)
 {
     if (this->devices[id] != nullptr) {
-        ABORT_F("ScsiBus: device with ID %d already registered", id);
+        ABORT_F("%s: device with ID %d already registered", this->get_name().c_str(), id);
     }
 
     this->devices[id] = dev_obj;
@@ -74,7 +74,7 @@ void ScsiBus::change_bus_phase(int initiator_id)
 void ScsiBus::assert_ctrl_line(int initiator_id, uint16_t mask)
 {
     DCHECK_F(initiator_id >= 0 && initiator_id < SCSI_MAX_DEVS,
-             "ScsiBus: invalid initiator ID %d", initiator_id);
+             "%s: invalid initiator ID %d", this->get_name().c_str(), initiator_id);
 
     uint16_t new_state = 0xFFFFU & mask;
 
@@ -93,7 +93,7 @@ void ScsiBus::assert_ctrl_line(int initiator_id, uint16_t mask)
 
 void ScsiBus::release_ctrl_line(int id, uint16_t mask)
 {
-    DCHECK_F(id >= 0 && id < SCSI_MAX_DEVS, "ScsiBus: invalid initiator ID %d", id);
+    DCHECK_F(id >= 0 && id < SCSI_MAX_DEVS, "%s: invalid initiator ID %d", this->get_name().c_str(), id);
 
     uint16_t new_state = 0;
 
@@ -255,7 +255,7 @@ bool ScsiBus::pull_data(const int id, uint8_t* dst_ptr, const int size)
     }
 
     if (!this->devices[id]->send_data(dst_ptr, size)) {
-        LOG_F(ERROR, "ScsiBus: error while transferring T->I data!");
+        LOG_F(ERROR, "%s: error while transferring T->I data!", this->get_name().c_str());
         return false;
     }
 
@@ -286,12 +286,22 @@ int ScsiBus::target_xfer_data() {
 
 void ScsiBus::target_next_step()
 {
-    this->devices[this->target_id]->next_step();
+    if (target_id < 0) {
+        LOG_F(ERROR, "%s: target_id is not set yet.", this->get_name().c_str());
+    }
+    else {
+        this->devices[this->target_id]->next_step();
+    }
 }
 
 bool ScsiBus::negotiate_xfer(int& bytes_in, int& bytes_out)
 {
-    this->devices[this->target_id]->prepare_xfer(this, bytes_in, bytes_out);
+    if (target_id < 0) {
+        LOG_F(ERROR, "%s: target_id is not set yet.", this->get_name().c_str());
+    }
+    else {
+        this->devices[this->target_id]->prepare_xfer(this, bytes_in, bytes_out);
+    }
 
     return true;
 }

devices/common/viacuda.cpp

@@ -95,7 +95,7 @@ ViaCuda::ViaCuda() {
         .tm_mday = 1,
         .tm_mon  = 1 - 1,
         .tm_year = 1904 - 1900,
-        .tm_isdst = -1 // Use DST value from local time zone
+        .tm_isdst = 1 // -1 = Use DST value from local time zone; 0 = not summer; 1 = is summer time
     };
     mac_epoch = std::chrono::system_clock::from_time_t(std::mktime(&tm));
 }
@@ -496,7 +496,7 @@ void ViaCuda::process_packet() {
         for (int i = 0; i < this->in_count; i++) {
             LOG_F(9, "0x%X ,", this->in_buf[i]);
         }
-        pseudo_command(this->in_buf[1], this->in_count - 2);
+        pseudo_command();
         break;
     default:
         LOG_F(ERROR, "Cuda: unsupported packet type = %d", this->in_buf[0]);
@@ -540,24 +540,24 @@ void ViaCuda::autopoll_handler() {
 
         // draw guest system's attention
         schedule_sr_int(USECS_TO_NSECS(30));
-    } else if (this->one_sec_mode == 3) {
+    } else if (this->one_sec_mode != 0) {
         uint32_t this_time = calc_real_time();
         if (this_time != this->last_time) {
-
-            if ((this->last_time & 15) == 0) {
-                /*
-                    FIXME: This doesn't do anything in Mac OS 8.6.
-                    A real Mac probably doesn't do this. So why do I?
-                    supermario checks for packets that are not PKT_TICK
-                    to set the time but I don't know which Mac OS versions
-                    have that code and if that code is triggered by this.
-                */
-                response_header(9, 0);
-                this->out_buf[3] = CUDA_GET_REAL_TIME;
-                uint32_t real_time = this_time + time_offset;
-                WRITE_DWORD_BE_U(&this->out_buf[3], real_time);
-                this->out_count = 7;
-            } else {
+            /*
+                We'll send a time packet every 4
+                seconds just in case we get out of
+                sync.
+            */
+            bool send_time = !(this->last_time & 3);
+            if (send_time || this->one_sec_mode < 3) {
+                response_header(CUDA_PKT_PSEUDO, 0);
+                this->out_buf[2] = CUDA_GET_REAL_TIME;
+                if (send_time || this->one_sec_mode == 1) {
+                    uint32_t real_time = this_time + this->time_offset;
+                    WRITE_DWORD_BE_U(&this->out_buf[3], real_time);
+                    this->out_count = 7;
+                }
+            } else if (this->one_sec_mode == 3) {
                 response_header(CUDA_PKT_TICK, 0);
                 this->out_count = 1;
             }
@@ -573,9 +573,10 @@ void ViaCuda::autopoll_handler() {
     }
 }
 
-void ViaCuda::pseudo_command(int cmd, int data_count) {
+void ViaCuda::pseudo_command() {
     uint16_t    addr;
     int         i;
+    int         cmd = this->in_buf[1];
 
     switch (cmd) {
     case CUDA_START_STOP_AUTOPOLL:
@@ -609,7 +610,7 @@ void ViaCuda::pseudo_command(int cmd, int data_count) {
     case CUDA_GET_REAL_TIME: {
         response_header(CUDA_PKT_PSEUDO, 0);
         uint32_t this_time = this->calc_real_time();
-        uint32_t real_time = this_time + time_offset;
+        uint32_t real_time = this_time + this->time_offset;
         WRITE_DWORD_BE_U(&this->out_buf[3], real_time);
         this->out_count = 7;
         break;

devices/common/viacuda.h

@@ -250,7 +250,7 @@ private:
     void error_response(uint32_t error);
     void process_packet();
     void process_adb_command();
-    void pseudo_command(int cmd, int data_count);
+    void pseudo_command();
     uint32_t calc_real_time();
 
     void null_out_handler(void);

devices/video/appleramdac.cpp

@@ -87,13 +87,18 @@ void AppleRamdac::iodev_write(uint32_t address, uint16_t value) {
     case RamdacRegs::MULTI:
         switch (this->dac_addr) {
         case RamdacRegs::CURSOR_POS_HI:
+#ifdef CURSOR_LO_DELAY // HACK: prevents artifacts in some cases, disabled by default
             if (this->cursor_timer_id) {
                 TimerManager::get_instance()->cancel_timer(this->cursor_timer_id);
                 cursor_timer_id = 0;
             }
             this->cursor_xpos = (value << 8) | this->cursor_pos_lo;
+#else
+            this->cursor_xpos = (value << 8) | (this->cursor_xpos & 0xff);
+#endif
             break;
         case RamdacRegs::CURSOR_POS_LO:
+#ifdef CURSOR_LO_DELAY // HACK: prevents artifacts in some cases, disabled by default
             if (this->cursor_timer_id) {
                 TimerManager::get_instance()->cancel_timer(this->cursor_timer_id);
                 this->cursor_xpos = (this->cursor_xpos & 0xff00) | (this->cursor_pos_lo & 0x00ff);
@@ -101,9 +106,12 @@ void AppleRamdac::iodev_write(uint32_t address, uint16_t value) {
             }
             this->cursor_pos_lo   = value;
             this->cursor_timer_id = TimerManager::get_instance()->add_oneshot_timer(
-                1000000000 / 60, [this]() {
+                NS_PER_SEC / 60, [this]() {
                     this->cursor_xpos = (this->cursor_xpos & 0xff00) | (this->cursor_pos_lo & 0x00ff);
                 });
+#else
+            this->cursor_xpos = (this->cursor_xpos & 0xff00) | (value & 0x00ff);
+#endif
             break;
         case RamdacRegs::MISC_CTRL:
             if (bit_changed(this->dac_cr, value, 1)) {

devices/video/appleramdac.h

@@ -19,6 +19,24 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */
 
+//#define CURSOR_LO_DELAY
+/* Uncomment this to add a delay before allowing the low byte of the cursor
+   position to be latched. In that case, a write to the high byte of the
+   cursor position latches the low byte immediately since the high byte is
+   expected to always be the second byte written. This change solves the issue
+   described below.
+
+   The horizontal position of the cursor may be incorrect between the times
+   when the low byte and the high byte are written. This is true when both
+   bytes change such as when the position changes from 0xFF to 0x100. This is
+   usually not a problem since the position usually changes only during a VBL.
+
+   In the case of Open Firmware, there is at least 2 ms between writes so
+   the cursor position could be incorrect for a third of the refresh cycle.
+   Open Firmware doesn't use VBLs. It also doesn't usually use a hardware
+   cursor so this change isn't usually useful.
+*/
+
 /** Definitions for the Apple RAMDAC ASICs (RaDACal & DACula). */
 
 #ifndef APPLE_RAMDAC_H
@@ -107,7 +125,6 @@ protected:
     uint16_t    cursor_xpos     = 0; // horizontal position of the cursor region
     uint16_t    cursor_ypos     = 0;
     uint16_t    cursor_height   = 0;
-    uint8_t     cursor_pos_lo   = 0;
     uint8_t     clk_m[2]        = {};
     uint8_t     clk_pn[2]       = {};
     uint8_t     pll_cr          = 0;
@@ -118,7 +135,10 @@ protected:
     int         video_width     = 0;
     int         video_height    = 0;
     uint32_t    fb_pitch        = 0;
+#ifdef CURSOR_LO_DELAY
+    uint8_t     cursor_pos_lo   = 0;
     uint32_t    cursor_timer_id = 0;
+#endif
 };
 
 #endif // APPLE_RAMDAC_H

devices/video/atimach64gx.cpp

@@ -26,7 +26,6 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 #include <core/bitops.h>
 #include <devices/deviceregistry.h>
-#include <devices/video/atimach64defs.h>
 #include <devices/video/atimach64gx.h>
 #include <devices/video/displayid.h>
 #include <devices/video/rgb514defs.h>
@@ -132,6 +131,7 @@ AtiMach64Gx::AtiMach64Gx()
     this->vendor_id   = PCI_VENDOR_ATI;
     this->device_id   = ATI_MACH64_GX_DEV_ID;
     this->class_rev   = (0x030000 << 8) | 0x03;
+    this->irq_pin     = 1;
     for (int i = 0; i < this->aperture_count; i++) {
         this->bars_cfg[i] = (uint32_t)(-this->aperture_size[i] | this->aperture_flag[i]);
     }
@@ -189,7 +189,7 @@ void AtiMach64Gx::notify_bar_change(int bar_num)
     change_one_bar(this->aperture_base[bar_num], this->aperture_size[bar_num],
                    this->bars[bar_num] & ~15, bar_num);
     // copy aperture address to CONFIG_CNTL:CFG_MEM_AP_LOC
-    insert_bits<uint32_t>(this->config_cntl, this->aperture_base[0] >> 22,
+    insert_bits<uint32_t>(this->config_cntl[0], this->aperture_base[0] >> 22,
                           ATI_CFG_MEM_AP_LOC, ATI_CFG_MEM_AP_LOC_size);
 }
 
@@ -305,22 +305,31 @@ bool AtiMach64Gx::pci_io_write(uint32_t offset, uint32_t value, uint32_t size)
 
     // CONFIG_CNTL is accessible from I/O space only
     if ((offset >> 2) == ATI_CONFIG_CNTL) {
+        if (size + (offset & 3) > 4)
+            LOG_F(ERROR, "%s: size + offset > 4!", this->name.c_str());
         write_mem(((uint8_t *)&this->config_cntl) + (offset & 3), value, size);
-        switch (this->config_cntl & 3) {
-        case 0:
-            LOG_F(WARNING, "%s: linear aperture disabled!", this->name.c_str());
-            break;
-        case 1:
-            LOG_F(INFO, "%s: aperture size set to 4MB", this->name.c_str());
-            this->mm_regs_offset = MM_REGS_2_OFF;
-            break;
-        case 2:
-            LOG_F(INFO, "%s: aperture size set to 8MB", this->name.c_str());
-            this->mm_regs_offset = MM_REGS_0_OFF;
-            break;
-        default:
-            LOG_F(ERROR, "%s: invalid aperture size in CONFIG_CNTL", this->name.c_str());
+        if (offset == ATI_CONFIG_CNTL << 2) {
+            switch (extract_bits<uint32_t>(this->config_cntl[0], ATI_CFG_MEM_AP_SIZE, ATI_CFG_MEM_AP_SIZE_size)) {
+            case 0:
+                LOG_F(WARNING, "%s: CONFIG_CNTL linear aperture disabled!", this->name.c_str());
+                break;
+            case 1:
+                LOG_F(INFO, "%s: CONFIG_CNTL aperture size set to 4MB", this->name.c_str());
+                this->mm_regs_offset = MM_REGS_2_OFF;
+                break;
+            case 2:
+                LOG_F(INFO, "%s: CONFIG_CNTL aperture size set to 8MB", this->name.c_str());
+                this->mm_regs_offset = MM_REGS_0_OFF;
+                break;
+            default:
+                LOG_F(ERROR, "%s: CONFIG_CNTL invalid aperture size", this->name.c_str());
+            }
         }
+
+        LOG_F(INFO, "%s: write %s %04x.%c = %0*x = %08x", this->name.c_str(),
+            get_reg_name(offset >> 2), offset, SIZE_ARG(size), size * 2,
+            value, this->config_cntl[0]
+        );
     } else {
         this->write_reg(offset, BYTESWAP_SIZED(value, size), size);
     }
@@ -761,7 +770,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<uint32_t>(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, 1);
+        insert_bits<uint32_t>(this->regs[ATI_CRTC_INT_CNTL], irq_line_state, ATI_CRTC_VBLANK, irq_line_state);
         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);
@@ -797,6 +806,7 @@ void AtiMach64Gx::rgb514_write_reg(uint8_t reg_addr, uint8_t value)
     switch (reg_addr) {
     case Rgb514::CLUT_ADDR_WR:
         this->clut_index = value;
+        this->comp_index = 0;
         break;
     case Rgb514::CLUT_DATA:
         this->clut_color[this->comp_index++] = value;

devices/video/atimach64gx.h

@@ -27,6 +27,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 #include <devices/common/pci/pcidevice.h>
 #include <devices/video/displayid.h>
 #include <devices/video/videoctrl.h>
+#include <devices/video/atimach64defs.h>
 
 #include <cinttypes>
 #include <memory>
@@ -83,16 +84,16 @@ private:
     const uint32_t aperture_flag[1] = { 0 };
     uint32_t aperture_base[1] = { 0 };
 
-    uint32_t    config_cntl = 0;
-    uint32_t    mm_regs_offset = 0;
+    uint32_t    config_cntl[2] = { 2, 0 };
+    uint32_t    mm_regs_offset = MM_REGS_0_OFF;
 
     // RGB514 RAMDAC state
-    uint8_t     dac_idx_lo;
-    uint8_t     dac_idx_hi;
-    uint8_t     clut_index;
-    uint8_t     comp_index;
-    uint8_t     clut_color[3];
-    uint8_t     dac_regs[256];
+    uint8_t     dac_idx_lo = 0;
+    uint8_t     dac_idx_hi = 0;
+    uint8_t     clut_index = 0;
+    uint8_t     comp_index = 0;
+    uint8_t     clut_color[3] = {0};
+    uint8_t     dac_regs[256] = {0};
 
     std::unique_ptr<DisplayID>  disp_id;
     std::unique_ptr<uint8_t[]>  vram_ptr;