Slot Floppies Patch Rating: 8,2/10 9393 reviews
| ID | Category [?] | Severity [?] | Reproducibility | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 06640 | Core | Major | Always | Aug 1, 2017, 21:38 | Aug 4, 2017, 06:43 |
| Tester | MrGW | View Status | Public | Platform | MAME (Official Binary) |
| Assigned To | Duke | Resolution | Fixed | OS | Windows 10 (32-bit) |
| Status [?] | Resolved | Driver | coco3.cpp | ||
| Version | 0.188 | Fixed in Version | 0.189 | Build | Normal |
| Summary | 06640: coco3, coco3h: TRS-80 Color Computer FDC data corruption issue | ||||
| Description | David Ladd (davidwladd@gmail.com) has discovered a serious data corruption issue when using the FDC with the COCOx drivers in MAME. He originally submitted MAME bug #6639 as part of an earlier issue he saw when running OS9 and accessing /d0 and /d1 drives (under certain circumstances). http://mametesters.org/view.php?id=6639 It looks like this latest discovery is what was causing the earlier issue. In any case, MAME 0.162 was the last version known to work correctly with the FDC. Something changed in MAME 0.163 to 'break' the FDC and it's been that way ever since. | ||||
| Steps To Reproduce | Here's a simple test that will demonstrate the issue: Startup CoCo 2 or CoCo3 emulation Create a new disk image in drive 0 Create a new disk image in drive 1 DSKINI 0 DSKINI 1 Enter a quick basic program like this and save it to disk: 10 PRINT 'DIR 0' 20 DIR 0 30 PRINT 'DIR 1' 40 DIR 1 50 GOTO 10 SAVE'TEST1.BAS' Then all you have to do to see the problem is to: DIR 0:DIR 1 You will see TEST1.BAS show up twice. Which it should not. The issue is also reproducible using the methods detailed in the videos below: | ||||
| Additional Information | Credit goes to David Ladd for finding this issue (twice). It was never resolved in the original MAME bug report: http://mametesters.org/view.php?id=6639 | ||||
| Flags | |||||
| Regression Version | |||||
| Affected Sets / Systems | coco3, coco3h | ||||
Attached Files | test_disks.zip (799 bytes) Aug 1, 2017, 23:53 Uploaded by Tafoid Test Disk Images - disk1 has program, disk2 is empty - both are formatted | ||||
coco.diff (683 bytes) Aug 2, 2017, 07:50 Uploaded by Osso [Show Content] | |||||
| ||||||
Video Slots: Most players prefer this type of free slot games no download no registration because of their practically unlimited choices as far as lines, reels, bonuses, and gigantic jackpots go. The most playable free slots machines are the Vegas slots that are ordinarily crafted to reflect the essential attributes of the world-known land. Apr 10, 2018 Commissar goes further, offering shirts that say “Slot Floppies,” a phrase that is sometimes used as a hashtag on Instagram and other social media platforms to promote Rhodesian-themed posts.
This is because physical floppy drives are a rarity and Floppies are loosing quality over years, Disk-archives are a requirement. BTW: Disk-Manager v0.17 fixes an issue, which may have caused copies of 360KB disks being incorrect.
Notes
Slot Floppies Patch Notes
Bletch Aug 1, 2017, 22:11 | From the MAME 0.163 release notes: - update all drivers to use the new wd floppy controller implementation and remove the legacy one. I'm guessing that this is more than a coincidence. |
|---|---|
Tafoid Aug 1, 2017, 23:52 | Added disk1 and disk2 images to assist in testing. I've noticed that it doesn't always file the first time, but usually fails the 'DIR 0:DIR 1' test the 2nd time |
star2root Aug 2, 2017, 02:03 | I think it might have to with these changes… 85 src/emu/bus/coco/coco_fdc.c @@ -291,16 +298,17 @@ void coco_fdc_device::dskreg_w(UINT8 data) else if (data & 0x40) drive = 3; - legacy_floppy_image_device *floppy[4]; + floppy_image_device *floppy[4]; - floppy[0] = subdevice<legacy_floppy_image_device>(FLOPPY_0); - floppy[1] = subdevice<legacy_floppy_image_device>(FLOPPY_1); - floppy[2] = subdevice<legacy_floppy_image_device>(FLOPPY_2); - floppy[3] = subdevice<legacy_floppy_image_device>(FLOPPY_3); + floppy[0] = subdevice<floppy_connector>(WD_TAG ':0')->get_device(); + floppy[1] = subdevice<floppy_connector>(WD_TAG ':1')->get_device(); + floppy[2] = subdevice<floppy_connector>(WD_TAG ':2')->get_device(); + floppy[3] = subdevice<floppy_connector>(WD_TAG ':3')->get_device(); for (int i = 0; i < 4; i++) { - floppy[i]->floppy_mon_w(i drive ? CLEAR_LINE : ASSERT_LINE); + if (floppy[i]) + floppy[i]->mon_w(i drive ? CLEAR_LINE : ASSERT_LINE); } head = ((data & 0x40) && (drive != 3)) ? 1 : 0; @@ -309,8 +317,11 @@ void coco_fdc_device::dskreg_w(UINT8 data) update_lines(); - m_wd17xx->set_drive(drive); - m_wd17xx->set_side(head); + m_wd17xx->set_floppy(floppy[drive]); + + if (floppy[drive]) + floppy[drive]->ss_w(head); + m_wd17xx->dden_w(!BIT(m_dskreg, 5)); } @@ -379,7 +390,7 @@ WRITE8_MEMBER(coco_fdc_device::write) dskreg_w(data); break; case 8: - m_wd17xx->command_w(space, 0, data); + m_wd17xx->cmd_w(space, 0, data); break; case 9: m_wd17xx->track_w(space, 0, data); @@ -388,6 +399,7 @@ WRITE8_MEMBER(coco_fdc_device::write) m_wd17xx->sector_w(space, 0, data); break; case 11: + //printf('data w %02xn', data); m_wd17xx->data_w(space, 0, data); break; }; @@ -413,10 +425,12 @@ WRITE8_MEMBER(coco_fdc_device::write) //************************************************************************** static MACHINE_CONFIG_FRAGMENT(dragon_fdc) - MCFG_DEVICE_ADD(WD2797_TAG, WD2797, 0) - MCFG_WD17XX_DEFAULT_DRIVE4_TAGS + MCFG_WD2797_ADD(WD2797_TAG, XTAL_1MHz) - MCFG_LEGACY_FLOPPY_4_DRIVES_ADD(coco_floppy_interface) + MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':0', coco_fdc_floppies, 'qd', coco_fdc_device::floppy_formats) + MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':1', coco_fdc_floppies, 'qd', coco_fdc_device::floppy_formats) + MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':2', coco_fdc_floppies, ', coco_fdc_device::floppy_formats) + MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':3', coco_fdc_floppies, ', coco_fdc_device::floppy_formats) MACHINE_CONFIG_END @@ -511,10 +525,20 @@ void dragon_fdc_device::dskreg_w(UINT8 data) data); } - if (data & 0x04) - m_wd2797->set_drive(data & 0x03); + floppy_image_device *floppy = NULL; + + switch (data & 0x03) + { + case 0: floppy = subdevice<floppy_connector>(WD2797_TAG ':0')->get_device(); break; + case 1: floppy = subdevice<floppy_connector>(WD2797_TAG ':1')->get_device(); break; + case 2: floppy = subdevice<floppy_connector>(WD2797_TAG ':2')->get_device(); break; + case 3: floppy = subdevice<floppy_connector>(WD2797_TAG ':3')->get_device(); break; + } + + m_wd2797->set_floppy(floppy); m_wd2797->dden_w(BIT(data, 3)); + m_dskreg = data; } @@ -556,12 +580,7 @@ WRITE8_MEMBER(dragon_fdc_device::write) switch(offset & 0xEF) { case 0: - m_wd2797->command_w(space, 0, data); - - /* disk head is encoded in the command byte */ - /* Only for type 3 & 4 commands */ - if (data & 0x80) - m_wd2797->set_side((data & 0x02) ? 1 : 0); + m_wd2797->cmd_w(space, 0, data); break; case 1: m_wd2797->track_w(space, 0, data); |
Osso Aug 2, 2017, 07:52 | Attached a small patch which seems to work for this case, but may actually cause horrible breakage elsewhere since I know nothing about the coco hardware and haven't tested anything else. None the less, it might be useful as a hint of what's going wrong. |
Haze Aug 2, 2017, 13:33 | the original code is definitely suspicious, it looks like it will turn off the motor for every drive other than the one you're trying to access. so in that sense, I'd say your fix looks sane even if I know nothing about the system |
lostwizard Aug 2, 2017, 19:14 | In case it helps, the correct drive motor behaviour on the Coco FDC is: If the motor enable bit (bit 3 of FF40) is enabled, *all* drive motors should start and *stay* on until the motor enable bit is cleared. That is, the drive select lines have no effect on whether a drive is spinning or not. All are or none are. Edit: Also, if the drive motors are turned off (bit 3 of FF40 clear), the drive selects still operate and 'enable' the specified drive(s). The drive light actually comes on with the motors turned off on the real hardware in that case. However, I suspect another issue in addition to the drive motor behaviour since the drive not being spun up properly should result in a read failure (CRC fail leading to I/O error or obviously garbled data) rather than the described behaviour. |
No.14058 Tester | Osso, I just tested the patch you supplied. It doesn't fully address the issue. You can also do a DIR 1:DIR 0 and then the directory on drive 0 doesn't show up correctly. Also you can have up to 4 single side floppy drives or 3 double sided floppy drives connected to a real CoCo. |
No.14059 Tester | Ciaran Anscomb (from the Xroar project) provided this comment and patch to try: 'They've come up with a patch, but that fails in a different way. Suggest the attached: update motor state on *all* drives, not just selected.' diff --git a/src/devices/bus/coco/coco_fdc.cpp b/src/devices/bus/coco/coco_fdc.cpp index f56bc047c5..cf9bd3038c 100644 --- a/src/devices/bus/coco/coco_fdc.cpp +++ b/src/devices/bus/coco/coco_fdc.cpp @@ -277,7 +277,7 @@ void coco_fdc_device_base::dskreg_w(uint8_t data) { floppy_image_device *floppy = m_floppies[i]->get_device(); if (floppy) - floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE); + floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE); } head = ((data & 0x40) && (drive != 3)) ? 1 : 0; |
No.14060 Developer | Thanks lostwizard, just committed a patch according to your explanation. It seems to fully address the issue here. Would be good if someone could try 06639 now. |
No.14063 Tester | Just so everyone knows that Ciaran Anscomb patch seems to have fixed the Disk Basic & OS-9/NitrOS-9 problems I ran into and was doing random reading & writing to both drives for a few minutes and did not notice any corruptions at this time. Before if accessing both floppy drives at the same time would crash OS-9 before within a few seconds. The DIFF code Ciaran and supplied to us is as follows: diff --git a/src/devices/bus/coco/coco_fdc.cpp b/src/devices/bus/coco/coco_fdc.cpp index f56bc047c5..cf9bd3038c 100644 --- a/src/devices/bus/coco/coco_fdc.cpp +++ b/src/devices/bus/coco/coco_fdc.cpp @@ -277,7 +277,7 @@ void coco_fdc_device_base::dskreg_w(uint8_t data) { floppy_image_device *floppy = m_floppies[i]->get_device(); if (floppy) - floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE); + floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE); } head = ((data & 0x40) && (drive != 3)) ? 1 : 0; |
No.14064 Tester edited on: Aug 3, 2017, 02:16 | While we are in this file, I would like suggest a patch that addresses another minor issue. Currently the CoCo's available floppy setting are either none, or quad density. Most CoCo's had either SSDD or DSDD 5 1/4 drives. This patch below set the default number of drives to 3, and adds the ability to select SSDD, DSDD, or QD (quad density) drives as an option. It also includes the patch above to fix the data corruption issue. Ileft the default at QD since due to another existing bug I have not been able to fix yet, you can't change the drive setting until you select 'reset' from the slot devices menu and QD drives do work with almost all existing disk images. --- src/devices/bus/coco/coco_fdc.cpp 2017-08-02 01:36:40.000000000 -0400 +++ src/devices/bus/coco/coco_fdc.cpp 2017-08-02 22:13:09.000000000 -0400 @@ -118,6 +118,8 @@ FLOPPY_FORMATS_END static SLOT_INTERFACE_START( coco_fdc_floppies ) + SLOT_INTERFACE('ssdd', FLOPPY_525_SSDD) + SLOT_INTERFACE('dsdd', FLOPPY_525_DD) SLOT_INTERFACE('qd', FLOPPY_525_QD) SLOT_INTERFACE_END @@ -130,7 +132,7 @@ MCFG_FLOPPY_DRIVE_SOUND(true) MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':1', coco_fdc_floppies, 'qd', coco_fdc_device_base::floppy_formats) MCFG_FLOPPY_DRIVE_SOUND(true) - MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':2', coco_fdc_floppies, nullptr, coco_fdc_device_base::floppy_formats) + MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':2', coco_fdc_floppies, 'qd', coco_fdc_device_base::floppy_formats) MCFG_FLOPPY_DRIVE_SOUND(true) MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':3', coco_fdc_floppies, nullptr, coco_fdc_device_base::floppy_formats) MCFG_FLOPPY_DRIVE_SOUND(true) @@ -273,25 +275,32 @@ else if (data & 0x40) drive = 3; + // Enable/disable the motor line for all drives since it is common + // to all drives. for (int i = 0; i < 4; i++) { floppy_image_device *floppy = m_floppies[i]->get_device(); if (floppy) - floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE); + floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE); } + // Determine the value of the head (side) select line. head = ((data & 0x40) && (drive != 3)) ? 1 : 0; + // Update the floppy controller register set_dskreg(data); update_lines(); + // Select the floppy drive that is active. floppy_image_device *selected_floppy = m_floppies[drive]->get_device(); m_wd17xx->set_floppy(selected_floppy); + // Apply the side select value. if (selected_floppy) selected_floppy->ss_w(head); + // Set the density m_wd17xx->dden_w(!BIT(dskreg(), 5)); } |
drencorxeen Aug 4, 2017, 06:43 | Duke, So after hours of testing it does seem that the patch was made for coco_fdc.cpp has at least resolved this issue at this time. |