Skip to content
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

Proposal: split video.c #331

Open
lmihalkovic opened this issue Mar 5, 2021 · 6 comments
Open

Proposal: split video.c #331

lmihalkovic opened this issue Mar 5, 2021 · 6 comments

Comments

@lmihalkovic
Copy link

lmihalkovic commented Mar 5, 2021

Video.c contains really 2 different layers of code:

  • the SDL user interface
  • the VERA emulation

proof that they do not belong together? even though there are some scared related call inside video.c, the bulk of the scared code is not inside video.c

so ... following that logic, this is about creating vera_video.h/vera_video.c

Your code is so well written that I did it yesterday afternoon without any problems whatsoever, in the process creating a new struct to describe the card:

typedef struct vera_video_card {
    uint8_t *video_ram; 							// [0x20000];
    uint8_t palette[256 * 2];
    uint8_t sprite_data[128][8];

    // I/O registers
    uint32_t io_addr[2];
    uint8_t io_rddata[2];
    uint8_t io_inc[2];
    uint8_t io_addrsel;
    uint8_t io_dcsel;

    uint8_t ien;
    uint8_t isr;

    uint16_t irq_line;

    uint8_t reg_layer[2][7];
    uint8_t reg_composer[8];

    uint8_t layer_line[2][SCREEN_WIDTH];
    uint8_t sprite_line_col[SCREEN_WIDTH];
    uint8_t sprite_line_z[SCREEN_WIDTH];
    uint8_t sprite_line_mask[SCREEN_WIDTH];
    uint8_t sprite_line_collisions;
    bool layer_line_enable[2];
    bool sprite_line_enable;

    float scan_pos_x;
    uint16_t scan_pos_y;
    int frame_count;

    uint8_t framebuffer[SCREEN_WIDTH * SCREEN_HEIGHT * 4];

    video_layer_properties_t *layer_properties; 	// [2];
    video_sprite_properties_t *sprite_properties;   // [128];
    video_palette_t video_palette;

    vera_card_config_t config;
} vera_video_card;

and yes ... the work of malloc-ing the data is not complete.

so this proposal is

  • create vera_video.c/vera_video.h
  • add API to allocate cards
    vera_allocate_card(vera_mode mode, vera_card_config_t config, vera_card_t **card) {
    void vera_delete_card(vera_card_t **card) {
  • parameterize the resolution/memory allocation. at the moment I have only isolated these, but more params need to be extracted (like SCREEN_WIDTH and SCREEN_HEIGHT for eg)
static vera_card_config_t default_card_config = {
    .vram_size = 128 * 1024, // 0x20000,
    .sprites   = 128,
    .layers    = 2,
    .warp_mode = false
};

I promise... there is a real reason for doing all this

@lmihalkovic
Copy link
Author

and yes ... even the memory save can be cleanly abstracted

void
video_save(SDL_RWops *f)
{
	vera_video_save(vera_card0, (vera_video_stream_writer)f->write, f);
	// SDL_RWwrite(f, &video_ram[0], sizeof(uint8_t), sizeof(video_ram));
	// SDL_RWwrite(f, &reg_composer[0], sizeof(uint8_t), sizeof(reg_composer));
	// SDL_RWwrite(f, &palette[0], sizeof(uint8_t), sizeof(palette));
	// SDL_RWwrite(f, &reg_layer[0][0], sizeof(uint8_t), sizeof(reg_layer));
	// SDL_RWwrite(f, &sprite_data[0], sizeof(uint8_t), sizeof(sprite_data));
}

@Elektron72
Copy link
Contributor

Just a quick suggestion: the labeling on the Commander X16 prototype itself (which can be seen in this video) refers to the VERA board as a "module," so you may want to consider changing the terminology used in the code.

@lmihalkovic
Copy link
Author

I was struggling with that ... VERA is referred to as a module. at this module has multiple capabilities: ps/2 ports, sound, video. my issue was that looking at the code in the emu it looks like they are also not following the extension cards model they have devised for x16. For eg: the PS2 ports are exposed through VIA2 while the video card is connected through memory mapped ports. So I am not sure what a module is, if it is anything connected via port mapping or ....

as this was not clear, I was trying to follow the naming conventions inside the source code ... .. and nothing in there refers to the things that are mem mapped. there is just a bunch of code that compares 'magic' addresses.

if someone decide on a nomenclature, I will happily follow it. In the meantime, I will test a 2 vera_video config ... where the second one will only be known via its memory mapped registers.... and then, a more ambitious step will be to ..... :)

@greg-king5
Copy link

VERA handles SPI not PS/2.

@lmihalkovic
Copy link
Author

lmihalkovic commented Mar 6, 2021

hmm ... .. for some reasons I was under the impression that VERA was where the PS/2 ports where located. ... but I am glad to be wrong. so, if I may ask: what are the features that are for sure located on the VERA board? I believe sound is there. of course video is there too... last I understood, the RTC was now accessible via SPI.. which means that it is on VERA too? ... I am looking at a photo of the blue photo (v2) ... from the top, looking left to right:

  • white connector (looks like DVI) ... how stupid I was ... sdcard
  • cylindrical 'shiny' barrel (looks like svideo type)
  • black square (??)
  • VGA
  • PS/2-1 (?) keyboard connector ... same comment ... board below
  • PS/2-2 (?) no header yet (mouse I presume)

then towards the back of the card

  • 2x6 header (looks like the same as the bus extensions located between the 2 VIAs) - I believe there is a CS signal somewhere in that connector which is directly connected to the address decoding logic near the row of RAM chips on the mainboard)

from what I could gather VERA is not behind a VIA but directly connected to the bus .. which reinforce my assumption that there is a CS in the header to activate the mem mapped registers. ...

anywho ... I hope to be able to test dual screen support today or tomorrow... then will come the harder step...

@lmihalkovic
Copy link
Author

VERA is now a standalone c library. the code uses the standard approach of exporting a 'blind' (handle-like) typedef for the internals of the video card and define a separate struct where the real state is stored. I have video.c starting with 2 separate SDL windows each running their own separate video card. cleaning up the memory mapped area code makes it trivial to defined these things (could do it in JSON) ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants