-
Notifications
You must be signed in to change notification settings - Fork 59
Add support for a peripheral attached to the user port #355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 18 commits
7e746fb
de038af
e88430a
0f5a281
a2a07ff
306fbb6
5a5bd77
1a53e35
cd6b1a9
d0c6a02
81134a6
93e5de7
050b3e6
7f5efe3
2eb9604
a073871
6056aac
f015c22
e6f857d
41e42cd
efe08b5
f93da6b
7e8be97
f492c4a
ebbabaa
f5ffd64
78c2a92
0e0b09c
7bb9768
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #pragma once | ||
|
|
||
| #ifdef _WIN32 | ||
| #include <windows.h> | ||
| #define LIBRARY_TYPE HMODULE | ||
| #define LOAD_LIBRARY(name) LoadLibrary(name) | ||
| #define GET_FUNCTION(lib, name) (void *)GetProcAddress(lib, name) | ||
| #define CLOSE_LIBRARY(lib) FreeLibrary(lib) | ||
| #define LIBRARY_ERROR() "[ unknown error ]" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can add that but I can't test it. I just pulled these macros out of midi.c, there just wasn't one for errors. |
||
| #else | ||
| #include <dlfcn.h> | ||
| #define LIBRARY_TYPE void* | ||
| #define LOAD_LIBRARY(name) dlopen(name, RTLD_LAZY) | ||
| #define GET_FUNCTION(lib, name) dlsym(lib, name) | ||
| #define CLOSE_LIBRARY(lib) dlclose(lib) | ||
| #define LIBRARY_ERROR() dlerror() | ||
| #endif | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -131,6 +131,7 @@ bool testbench = false; | |||||
| bool enable_midline = false; | ||||||
| bool ym2151_irq_support = false; | ||||||
| char *cartridge_path = NULL; | ||||||
| char *user_peripheral_path = NULL; | ||||||
|
||||||
| char *user_peripheral_path = NULL; | |
| char *user_peripheral_plugin_path = NULL; |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| via2_init(user_peripheral_path); | |
| via2_init(user_peripheral_plugin_path); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| user_peripheral_path = argv[0]; | |
| user_peripheral_plugin_path = argv[0]; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,104 @@ | ||||||
| #pragma once | ||||||
|
|
||||||
| // Include this header in a user peripheral library. | ||||||
| // | ||||||
| // A user peripheral library must export a [user_port_init_t x16_user_port_init] to be | ||||||
| // consumed at runtime by the emulator during initialization of via2. [x16_user_port_init] | ||||||
|
||||||
| // consumed at runtime by the emulator during initialization of via2. [x16_user_port_init] | |
| // called at runtime by the emulator during initialization of via2. [x16_user_port_init] |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true / false or 0 / -1 would be better than allowing any negative number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is? user_pins_t is uint32_t.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Bit assignments for each 65c22 pin exposed to the user port, for use with | |
| // Bit assignments for each 65C22 pin exposed to the user port, for use with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this relevant to implementers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's easy enough to overlook that I think it's worth calling out, especially since the emulator does actually do I2C, but it's a special case. That said, I think I could add pull-up/pull-down masks to the user_port initialization, and treat output->input changes in the ddrs as a 'write' of the pulled value, and input->output as a write of the register value.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a void *userdata - global state in plugins is a pain.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| user_pin_t (*read)(); | |
| user_pin_t (*read)(void); |
I forgot which C standard dropped () declarations meaning "variable number of arguments", but I don't want to find out the hard way...
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just guarantee that they're zero here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As implemented that is guaranteed, I could make the comment stronger. Mostly I meant to imply that a zero in a bit not connected does not mean the pin is low, it doesn't mean anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check isn't implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is actually, via2_read, via2_write and via2_step check that user_port.whatever != NULL before invoking it. I figure it could be valid for a device not to need some subset of the functions.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You made the struct API extensible, but not the init function - and the plugin has no idea what version you want from it. I'd recommend adding another struct:
typedef struct __attribute__((aligned(void *))) user_port_init_args {
int api_version;
} user_port_init_args;and modifying user_port_init to take a pointer to that struct. That way the plugin can determine whether it actually supports the given API version, and we can pass additional arguments in the future by extending the struct. (Force pointer alignment so that adding a pointer later doesn't cause headaches.)
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,13 @@ | |
| //XXX | ||
| #include "glue.h" | ||
| #include "joystick.h" | ||
| #include "dylib.h" | ||
| #include "user_pins.h" | ||
|
|
||
| #define CA1 (1 << 0) | ||
| #define CA2 (1 << 1) | ||
| #define CB1 (1 << 2) | ||
| #define CB2 (1 << 3) | ||
|
|
||
| typedef struct { | ||
| unsigned timer_count[2]; | ||
|
|
@@ -21,6 +28,7 @@ typedef struct { | |
| bool timer1_m1; | ||
| bool timer_running[2]; | ||
| bool pb7_output; | ||
| uint8_t cacb; | ||
| } via_t; | ||
|
|
||
| static via_t via[2]; | ||
|
|
@@ -38,6 +46,7 @@ via_init(via_t *via) | |
| via->timer_running[1] = false; | ||
| via->timer1_m1 = false; | ||
| via->pb7_output = true; | ||
| via->cacb = 0; | ||
| } | ||
|
|
||
| static void | ||
|
|
@@ -331,28 +340,134 @@ via1_irq() | |
| // for now, just assume that all user ports are not connected | ||
| // and reads return output register (open bus behavior) | ||
|
|
||
|
|
||
| static bool attempt_peripheral_load = true; | ||
| static LIBRARY_TYPE user_peripheral_dl = NULL; | ||
| static user_port_init_t user_port_init = NULL; | ||
|
|
||
| static user_port_t user_port; | ||
|
|
||
| void | ||
| via2_init() | ||
| via2_init(char const *user_peripheral_path) | ||
| { | ||
| via_init(&via[1]); | ||
| if (attempt_peripheral_load && user_peripheral_path) { | ||
| attempt_peripheral_load = false; | ||
| user_peripheral_dl = LOAD_LIBRARY(user_peripheral_path); | ||
| if (user_peripheral_dl) { | ||
| user_port_init = GET_FUNCTION(user_peripheral_dl, "x16_user_port_init"); | ||
| } | ||
| if (user_peripheral_dl == NULL || user_port_init == NULL) { | ||
| fprintf(stderr, "failed to load user peripheral %s:\n\t%s\n", | ||
| user_peripheral_path, LIBRARY_ERROR()); | ||
| fprintf(stderr, "continuing with empty user port.\n"); | ||
| if (user_peripheral_dl) CLOSE_LIBRARY(user_peripheral_dl); | ||
| } | ||
| } | ||
| // TODO Do we really want to reset the user peripherals every time? | ||
| if (user_port_init) { | ||
| bool error = false; | ||
| if (user_port_init(&user_port) < 0) { | ||
| fprintf(stderr, "error initializing user peripheral\n"); | ||
| error = true; | ||
|
||
| } | ||
| else if (user_port.api_version != X16_USER_PORT_API_VERSION) { | ||
| fprintf(stderr, "user peripheral version mismatch: expected: %d, got: %d\n", | ||
| X16_USER_PORT_API_VERSION, user_port.api_version); | ||
| error = true; | ||
| } | ||
| if (error) { | ||
| user_port_init = NULL; | ||
| memset(&user_port, 0, sizeof(user_port)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| uint8_t | ||
| via2_read(uint8_t reg, bool debug) | ||
| { | ||
| return via_read(&via[1], reg, debug); | ||
| uint8_t regval = via_read(&via[1], reg, debug); | ||
| if (user_port.read) { | ||
| switch (reg) { | ||
| case 0: { | ||
| uint8_t mask = user_port.connected_pins >> 8; // PB is 2nd byte | ||
| if (mask) { | ||
| uint8_t user_pins = user_port.read() >> 8; | ||
| // PB only returns pin values on inputs | ||
| mask &= ~via[1].registers[2]; | ||
| return (regval & ~mask) | (user_pins & mask); | ||
| } | ||
| break; | ||
| } | ||
| case 1: | ||
| case 15: { | ||
| uint8_t mask = user_port.connected_pins; // PA is 1st byte | ||
| if (mask) { | ||
| uint8_t user_pins = user_port.read(); | ||
| // Port A always returns pin values on a read, even on output pins | ||
| return (regval & ~mask) | (user_pins & mask); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return regval; | ||
| } | ||
|
|
||
| void | ||
| via2_write(uint8_t reg, uint8_t value) | ||
| { | ||
| via_write(&via[1], reg, value); | ||
| switch (reg) { | ||
| case 0: | ||
| case 1: | ||
| case 2: | ||
| case 3: | ||
| case 15: { | ||
| if (user_port.write) { | ||
| user_pin_t pa = via[1].registers[1] & via[1].registers[3]; | ||
| user_pin_t pb = via[1].registers[0] & via[1].registers[2]; | ||
| user_pin_t pins = (pa | (pb << 8)) & user_port.connected_pins; | ||
| user_port.write(pins); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void | ||
| via2_step(unsigned clocks) | ||
| { | ||
| via_step(&via[1], clocks); | ||
| via_step(&via[1], clocks); | ||
| if (user_port.step) { | ||
| user_pin_t pins = user_port.step((double)clocks * 1000.0 / MHZ); | ||
| // TODO CA2/CB2 | ||
| if (user_port.connected_pins & CA1_PIN) { | ||
| uint8_t new_ca1 = pins & CA1_PIN ? CA1 : 0; | ||
| if (new_ca1 != (via[1].cacb & CA1)) { | ||
| bool ca1_positive_active_edge = via[1].registers[12] & 0x01; | ||
| if ((ca1_positive_active_edge && new_ca1) || (!ca1_positive_active_edge && !new_ca1)) | ||
| via[1].registers[13] |= 0x02; | ||
| } | ||
| via[1].cacb &= ~CA1; | ||
| via[1].cacb |= new_ca1; | ||
| } | ||
| // CB1 shares a user pin with PB6, so only check if it's an input | ||
| if ((user_port.connected_pins & CB1_PIN) && !(via[1].registers[2] & 0x40)) { | ||
| uint8_t new_cb1 = pins & CB1_PIN ? CB1 : 0; | ||
| if (new_cb1 != (via[1].cacb & CB1)) { | ||
| bool cb1_positive_active_edge = via[1].registers[12] & 0x10; | ||
| if ((cb1_positive_active_edge && new_cb1) || (!cb1_positive_active_edge && !new_cb1)) | ||
| via[1].registers[13] |= 0x10; | ||
| } | ||
| via[1].cacb &= ~CB1; | ||
| via[1].cacb |= new_cb1; | ||
| } | ||
|
|
||
| // We could update input pin values here, but it doesn't really matter unless they | ||
| // fire interrupts. The state of the pins is invisible until the cpu invokes | ||
| // [via2_read]. | ||
| } | ||
| } | ||
|
|
||
| bool | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -14,7 +14,7 @@ void via1_write(uint8_t reg, uint8_t value); | |||||
| void via1_step(unsigned clocks); | ||||||
| bool via1_irq(); | ||||||
|
|
||||||
| void via2_init(); | ||||||
| void via2_init(char const *user_peripheral_so); | ||||||
|
||||||
| void via2_init(char const *user_peripheral_so); | |
| void via2_init(char const *user_peripheral_plugin); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macOS uses
.dylib. The extension also isn't included inmain.c(and I'm not sure whether it should be).