Commit for Foenix F256K retro-computer. #14029

dtremblay · 2025-08-05T19:58:40Z

More information about the hardware is available here: https://wiki.f256foenix.com/

For the list of supported features, see: https://wiki.f256foenix.com/index.php?title=Emulation#MAME.

For ROMs, see the release page: https://github.com/dtremblay/mame/releases
The ROM is called f256k.zip - there is also a SD Card image available there.

More information available here: https://wiki.f256foenix.com/ For the list of supported features, see: https://wiki.f256foenix.com/index.php?title=Emulation#MAME. Report defects on Discord server: https://discord.com/channels/691915291721990194/1330998392481906768

.github/workflows/ci-linux.yml

src/mame/f256/f256.cpp

angelosa · 2025-08-05T20:47:10Z

src/mame/f256/f256.cpp

+void f256_state::data_map(address_map &map)
+{
+    map(0x0000, 0x1FFF).ram().share(IOPAGE0_TAG);
+    map(0x0000, 0x1FFF).ram().share(IOPAGE1_TAG);
+    map(0x0000, 0x1FFF).ram().share(IOPAGE2_TAG);
+    map(0x0000, 0x1FFF).ram().share(IOPAGE3_TAG);
+}
+


Huh? This will just map IOPAGE3_TAG for the address_space rule of last one wins, how it's supposed to really work?

I agree this is confusing. The F256K has an IO register in byte 0x01 which determines which IO devices to map. This way more devices can be supported with only a 16-bit bus.

Removed github workflow files. Re-arranged the setters to follow the constructors in the main constructor.

dtremblay

I've removed the offending .github/workflow files.
Added the SPDX header to new files.
Fixed the setters to follow constructors.
Re-tested.

src/mame/f256/f256.cpp

dtremblay · 2025-08-06T19:38:55Z

Addressed all comments.

dtremblay

All changes committed.

dtremblay · 2025-08-08T17:38:04Z

So... am I supposed to do something else to get this PR accepted? It seems to be stuck. The workflow hasn't been approved yet.

stonedDiscord · 2025-08-08T19:56:00Z

It sometimes takes a few days or weeks until a maintainer has time to look at PRs that aren't just a few lines or a new dump.
The workflow did run and it seems it failed.
Click the red X next to your latest commit above.

src/devices/machine/bq4847.cpp

src/mame/f256/f256.h

angelosa · 2025-08-08T21:31:35Z

src/mame/f256/f256.cpp

+    uint8_t mmu = m_mmu_reg & 3;
+    uint16_t adj_addr = offset + 0x10;
+    uint8_t slot = adj_addr >> 13;
+    uint16_t low_addr = adj_addr & 0x1FFF;
+    uint8_t bank = mmu_lut[mmu * 8 + slot];
+
+    // fslot < 0x40 is RAM, greater is FLASH/ROM
+    if (bank < 0x40)
+    {
+        // Slot 6 is where I/O devices are located, when IO_DISABLE is 0
+        if (slot == 6 && (m_ioreg & 0x4) == 0)
+        {
+            switch (m_ioreg & 0x3)
+            {


Feel like this could be expressed with address_map_bank and use of memory_view specifically for the 0xc000-0xdfff area, to me it looks similar to a PC Engine or an OS-9 based machine like Hitachi S1.
Also: how much RAM this is really supposed to have?

The machine has 512K of flash, 512K of static RAM. There's also an extension cartridge for 256K available as flash or RAM.

I couldn't understand the memory bank documentation. Could this be done as an update? I intend to add other variants of the machine (with 65816 CPU, for example).

https://wiki.f256foenix.com/index.php?title=Memory_Management

Need to point me where's the I/O explaination.

PS: yeah it can run OS-9, with a special 6809 flavour if I'm getting it right?

The I/O page information is not available in the wiki, unfortunately. The PDF has the details: F256K PDF. See starting at page 14.

The whole address range from 0x0000 to 0xffff is mapped in 8K blocks using a control register at address 0x0.
The address from 0xc000 to 0xdfff can be mapped to memory or to I/O using control register at address 0x1.
I'm re-reading the MAME documentation to see how I can implement memory_bank and memory_view concepts.

MooglyGuy · 2025-08-08T23:08:46Z

One overarching bit of feedback, it's usually not ideal to suggest that feedback be done via a Discord server. Part of the point of a pull request is that the code review occurs within the pull request itself.

While there is certainly a particular generational shift towards the use of Discord as a substitute for websites, that isn't something that tends to be shared by the wider software development community, at least those old enough to have been doing professional software development for the past couple of decades.

dtremblay · 2025-08-18T19:19:49Z

One overarching bit of feedback, it's usually not ideal to suggest that feedback be done via a Discord server. Part of the point of a pull request is that the code review occurs within the pull request itself.

While there is certainly a particular generational shift towards the use of Discord as a substitute for websites, that isn't something that tends to be shared by the wider software development community, at least those old enough to have been doing professional software development for the past couple of decades.

In my commit message, I state to report "defects" in Discord. I did not suggest to perform the PR work there.
About generational comment: I think this is not warranted in a PR. And, I'm probably as old as you are if not older.

Retested with both "-validate" flag and with actual applications. SID is still noisy.

angelosa · 2025-08-18T21:57:18Z

Out of sheer curiosity: how this driver behaves when testing?
You are claiming that the driver is working, and by cursory glance I bet it really isn't.

src/devices/machine/bq4847.cpp

pmackinlay · 2025-08-19T05:01:45Z

src/devices/machine/bq4847.h

@@ -84,14 +89,21 @@ class bq4847_device : public device_t,
 	int m_rst_state;
 	int m_wdi_state;
 	bool m_writing;
+	bool has_century = false;


Please use m_ prefix on class variables to match existing style. Also initialize this variable in the constructor not the declaration (see e.g., m_writing).

src/devices/machine/bq4847.h

pmackinlay · 2025-08-19T05:03:20Z

src/devices/machine/bq4847.h

+class bq4802_device : public bq4847_device
+{
+public:
+	bq4802_device(const machine_config& mconfig, const char* tag, device_t* owner, uint32_t clock);


As before, please use right justification for references and pointers to match existing style.

All the code in this class is consistent. I could modify the rest of the class to adhere to this request.

It's not consistent with respect to the spacing around * and & tokens.

pmackinlay · 2025-08-19T05:34:56Z

src/mame/f256/f256.cpp

+    //memcpy(m_iopage[1], m_font, 0x800);
+    for (int i=0;i<0x800;i++)
+    {
+        m_iopage[1]->write(i, m_font->as_u8(i));


Is this really what happens, or should the font ROM be decoded where m_iopage[1] is currently being decoded?

The font is loaded at boot time. Users can modify the font at run time.

This answer doesn't address the intent of the question. It's very unlikely that hardware performs copy of this data into RAM. What does the hardware do, if anything? What does "loaded at boot time" mean?

This is implement with closed-source FPGA logic.

I have removed the loop and replaced with memcpy, using m_font->base() method.

pmackinlay · 2025-08-19T05:35:14Z

src/mame/f256/f256.cpp

+        0xf6, 0xf7, 0xf7, 0xf8, 0xf8, 0xf9, 0xf9, 0xfa, 0xfb, 0xfb, 0xfc, 0xfc, 0xfd, 0xfd, 0xfe, 0xff
+    };
+
+    memcpy(m_iopage[0]->pointer(), gamma_1_8, 256);


What is this gamma table in hardware?

The gamma table is used to correct the display colors. When gamma is enabled in the display register, all the colors displayed are adjusted according to this indexed table.

I understand what a gamma table is, I'm asking what hardware is involved in implementing it? Again, it's very unlikely that hardware is doing an arbitrary memcpy() from somewhere into RAM. Is it a RAMDAC or some other device with a fixed or programmable table that should be emulated?

This is implemented with closed-source FPGA logic.

pmackinlay · 2025-08-19T05:41:34Z

src/mame/f256/tiny_vicky.cpp

+
+rgb_t tiny_vicky_video_device::get_text_lut(uint8_t color_index, bool fg, bool gamma)
+{
+    uint8_t red =   iopage0_ptr[(fg ? 0x1800 : 0x1840) + color_index * 4 + 2];


Is iopage0_ptr really pointing to a palette or RAMDAC device here?

It's a color Look Up Table (LUT), indeed like a palette. But only 16 colors for foreground and 16 colors for background. Each color is represented by 4 bytes: blue, green, red, and alpha (alpha is ignore by the machine so far).

It's a color Look Up Table (LUT), indeed like a palette. But only 16 colors for foreground and 16 colors for background. Each color is represented by 4 bytes: blue, green, red, and alpha (alpha is ignore by the machine so far).

Perfect use case for a palette_device. The final bitmap should be 32bpp, so you're solid in that regard, but iit's preferable to use a palette_device for even fairly baisc LUT remapping.

The intent of the question was to prompt you to explain what the underlying hardware is, such that it could be emulated in a more realistic/accurate fashion. Functionally it's a LUT; what hardware is used to implement it?

It's implemented via closed-source FPGA logic.

src/mame/f256/tiny_vicky.h

dtremblay · 2025-08-19T07:23:55Z

Out of sheer curiosity: how this driver behaves when testing? You are claiming that the driver is working, and by cursory glance I bet it really isn't.

I'm not sure what you are basing your conclusion on. I wouldn't submit the PR if the system was not working. Maybe I misunderstand what you imply by "driver". I can give you a demo, if you like, via MS Teams or I could upload a video to YouTube. It works quite well on Windows, Linux and Mac - users in the community have tested this, not I.

You do need to have the ROMs of course (provided: here ). The ROMs include a disk operating system and SuperBASIC and other useful utilities. If a hard disk image is provided, applications can be loaded. The F256K machines have now been around for about two years and there are now new variants being manufactured now, for example with 65816 and 6809 processors (instead of 6502) and other sound chips.

angelosa · 2025-08-19T07:52:03Z

I'm not sure what you are basing your conclusion on.

From your tendency of dismissing comments without actually doing anything. #14029 (comment)
The driver is clearly not doing a logical to physical translation, and I can't see how a system that expects 512 KiB can work with a single page of 8 KiB, mapped on the same place on top.

angelosa · 2025-08-19T07:58:21Z

src/mame/f256/tiny_vicky.cpp

+            // TODO: generate start of frame (SOF) interrupt
+            m_sof_irq_handler(1);


No logic should be present in screen_update, that will break if user frameskips.

angelosa · 2025-08-19T07:59:15Z

src/mame/f256/tiny_vicky.cpp

+                if ((sol_reg & 1) != 0 && y == sol_line)
+                {
+                    // raise an interrupt
+                    m_sol_irq_handler(1);


As above, this needs to be moved in an emu_timer, latching with screen positions.

angelosa · 2025-08-19T08:01:07Z

src/mame/f256/tiny_vicky.cpp

+        cursor_counter++;
+        if (cursor_counter > cursor_flash_rate *2)
+        {
+            cursor_counter = 0;
+        }


Should use frame_number() instead of manually keeping track of the count. This will again break with frameskipping.

cuavas · 2025-08-22T19:19:45Z

src/devices/machine/bq4847.cpp

-		m_register[reg_year] = convert_to_bcd(year);
+		m_register[reg_year] = convert_to_bcd(year%100);


Please use spaces around binary operators, like year % 100 (but not unary operators like ++x).

cuavas · 2025-08-22T19:21:30Z

src/devices/machine/bq4847.h

+	void set_century(bool value)
+	{
+		m_century = value;
+	}


If this is an intrinsic characteristic of the device as opposed to an input, it shouldn’t be part of the public interface.

Moved to protected block.

cuavas · 2025-08-22T19:23:40Z

src/devices/machine/bq4847.cpp

 	return (regnum == reg_seconds || regnum == reg_minutes || regnum == reg_hours ||
 		regnum == reg_date || regnum == reg_days || regnum == reg_month
-		|| regnum == reg_year);
+		|| regnum == reg_year || regnum == reg_century);
 }


This, combined with the change to write, will change the behaviour of all devices in this file to copy the contents of what’s the century register on some devices only on an update transfer.

cuavas · 2025-08-22T19:25:24Z

src/devices/machine/bq4847.cpp

In general, adding device type flags that need to be checked all over the place is an ugly way of doing things. Can you think of a way of doing this without needing intrusive checks inserted throughout the code?

src/devices/machine/bq4847.h

cuavas · 2025-08-22T19:37:45Z

src/mame/f256/tiny_vicky.cpp

+                            uint8_t LayerMgr0 = m_iopage0_ptr[0xD002 - 0xC000] & 0x7;
+                            uint8_t LayerMgr1 = m_iopage0_ptr[0xD002 - 0xC000] >> 4;
+                            uint8_t LayerMgr2 = m_iopage0_ptr[0xD003 - 0xC000] & 0x7;


Please use snake_case for local variables. We tend to reserve LlamaCase for template parameters.

Also, please consistently use lowercase hex digits.

Fixed.
Fixed all 0x values to lowercase.

cuavas · 2025-08-22T19:38:49Z

src/mame/f256/tiny_vicky.cpp

+uint32_t tiny_vicky_video_device::screen_update(screen_device &screen, bitmap_rgb32 &bitmap, const rectangle &cliprect)
+{
+    if (m_running && m_iopage0_ptr)
+    {


In this function, you really should be making an effort to at least restrict the drawing to the clipping rectangle vertically so that raster effects have a chance of working.

cuavas · 2025-08-22T19:41:06Z

src/mame/f256/tiny_vicky.cpp

+
+void tiny_vicky_video_device::draw_mouse(uint32_t *row, bool enable_gamma, uint16_t line, uint16_t width, uint16_t height)
+{
+    uint8_t mouse_reg = m_iopage0_ptr[0xD6E0 - 0xC000];


Is this “I/O page” actually shared RAM, or is it a bunch of memory-mapped registers? If it’s memory-mapped registers, you should put proper read/write handlers in the address map rather than pretending it’s memory.

cuavas · 2025-08-22T19:42:26Z

src/mame/f256/f256k.md

Please don’t do this. MAME has documentation, you don’t need a “how to use MAME” tutorial here.

cuavas · 2025-08-22T19:43:47Z

src/mame/f256/f256.h

You shouldn’t have a separate header for this if you aren’t realistically going to need derived classes in different translation units. Just put it all in one file in an anonymous namespace.

dtremblay added 2 commits August 5, 2025 21:45

Fixed the header constant to match guard.

263cfc3

angelosa reviewed Aug 5, 2025

View reviewed changes

.github/workflows/ci-linux.yml Outdated Show resolved Hide resolved

angelosa reviewed Aug 5, 2025

View reviewed changes

src/mame/f256/f256.cpp Show resolved Hide resolved

angelosa reviewed Aug 5, 2025

View reviewed changes

src/mame/f256/f256.cpp Outdated Show resolved Hide resolved

angelosa reviewed Aug 5, 2025

View reviewed changes

Added SPDX license and my handle.

28e69a1

Removed github workflow files. Re-arranged the setters to follow the constructors in the main constructor.

dtremblay force-pushed the f256k branch from f8aff9b to 28e69a1 Compare August 5, 2025 21:26

Removed the harddriv.old file.

a6ef931

dtremblay commented Aug 5, 2025

View reviewed changes

Merge branch 'mamedev:master' into f256k

8e5147c

ajrhacker requested changes Aug 6, 2025

View reviewed changes

src/mame/f256/f256.cpp Outdated Show resolved Hide resolved

src/mame/f256/f256.cpp Outdated Show resolved Hide resolved

src/mame/f256/f256.cpp Outdated Show resolved Hide resolved

dtremblay added 2 commits August 6, 2025 21:35

Addressed comments by ajrhacker.

2bf708d

Merge branch 'f256k' of https://github.com/dtremblay/mame into f256k

52cfd24

dtremblay commented Aug 6, 2025

View reviewed changes

dtremblay requested a review from ajrhacker August 6, 2025 21:38

ajrhacker approved these changes Aug 6, 2025

View reviewed changes

dtremblay requested a review from angelosa August 7, 2025 07:29

ajrhacker requested changes Aug 8, 2025

View reviewed changes

src/devices/machine/bq4847.cpp Outdated Show resolved Hide resolved

angelosa reviewed Aug 8, 2025

View reviewed changes

src/mame/f256/f256.h Outdated Show resolved Hide resolved

angelosa reviewed Aug 8, 2025

View reviewed changes

Fixed the issue with "-validate" reporting a duplicate name.

451cb91

dtremblay added 2 commits August 18, 2025 20:17

Converted the iopages to an array.

22cac54

Used a DEFINED variable instead of magic string.

ebc5f47

Converted other devices to arrays.

a4964ed

Retested with both "-validate" flag and with actual applications. SID is still noisy.

pmackinlay requested changes Aug 19, 2025

View reviewed changes

angelosa requested changes Aug 19, 2025

View reviewed changes

Addressing comments from reviewers.

39ae79c

cuavas reviewed Aug 22, 2025

View reviewed changes

Updated code to address comments from reviewers.

cca1b72

		// TODO: generate start of frame (SOF) interrupt
		m_sof_irq_handler(1);

		m_register[reg_year] = convert_to_bcd(year);
		m_register[reg_year] = convert_to_bcd(year%100);

Commit for Foenix F256K retro-computer. #14029

Are you sure you want to change the base?

Commit for Foenix F256K retro-computer. #14029

Conversation

dtremblay commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtremblay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dtremblay commented Aug 6, 2025

Uh oh!

dtremblay left a comment

Choose a reason for hiding this comment

Uh oh!

dtremblay commented Aug 8, 2025

Uh oh!

stonedDiscord commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtremblay Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MooglyGuy commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtremblay commented Aug 18, 2025

Uh oh!

angelosa commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dtremblay commented Aug 5, 2025 •

edited

Loading

stonedDiscord commented Aug 8, 2025 •

edited

Loading

dtremblay Aug 19, 2025 •

edited

Loading

MooglyGuy commented Aug 8, 2025 •

edited

Loading

angelosa commented Aug 18, 2025 •

edited

Loading

dtremblay commented Aug 19, 2025 •

edited

Loading