Consider moving the emu ports to an include file

[jotego]

I have made this change to my framework which you may consider useful for the general MiSTer framework:

• the core ports are defined in a common file to all games

It looks like this when you use it:

module jtkicker_game(
    `include "jtframe_game_ports.inc" // see $JTFRAME/hdl/inc/jtframe_game_ports.inc
);

You can check out the included file here.

From a maintenance point of view, this is very practical because features can be added and fixed more easily for all cores. From the developer point of view, it takes clutter away and removes the need to keep track of new ports added via MISTER_ macros.

It may fit my development tool chain better than yours, but I just wanted to share my two cents. Please close the issue at any time. I am not requesting that you actually implement it, but just sharing some thoughts.

[birdybro]

I like the idea because it will take one step out of the process of updating the framework when the emu module port declarations need to be updated moving forward. If the include were placed in the sys folder, then merely replacing the sys folder removes that extra step.

One downside I could see is the ease of access to the information about those ports (whether they are inputs, outputs, what bit width they are, are they a reg or not, etc...) goes down slightly because you would have to open a second file as a reference.

Does this have any incompatibilities with simulation tools like verilator/modelsim/ghdl/etc...? I assume it might work with verilator since you use that tool specifically, but am curious about other tools other devs use of course.

[sorgelig]

Yes, i'm thinking about it some time already. But because V/SV doesn't accept default values for output ports, it will require to add dummy assigns like VGA_DISABLE=0. Otherwise it will generate warnings. So regardless include it will require adding such lines in glue logic file.

[birdybro]

Currently we get warnings already without adding the dummy assigns. As I have updated the framework on cores, I have had to add lines such as assign VGA_DISABLE=0 anyways each time to get rid of the warnings.

[sorgelig]

didn't i say the same above? ;) What i mean, that moving ports into include file won't eliminate manual editing a glue logic file to add such lines if new ports added. That's why i see no much reason to move ports into include. It will add more work while developing since you have to open that include file every time you want to check the name and size of specific port.

[birdybro]

Ahhh, sorry LOL. I misunderstood you. :)

[wickerwaka]

You could get rid of the need for default ports if you used ifdef's to indicate whether the port is present and handle that in sys/ rather than requiring cores to provide the default values.

`ifdef MISTER_HAS_VGA_DISABLE
 output VGA_DISABLE,
`endif

If a core doesn't define MISTER_HAS_VGA_DISABLE then there will be no VGA_DISABLE port in the emu module and sys_top will provide whatever default behavior is required.

jtframe does this with defines like JTFRAME_4PLAYERS

It means any new ports or changes to existing ports would require adding a new define and adding the preprocessor logic to deal with it being enabled or disabled. But sys upgrades would become a lot easier and it would make it possible to make larger changes to the emu interface without having to force painful sys upgrades down the line.

[sorgelig]

It's not convenient to add define to many such optional ports. It will get tens of such defines which will require to have many customisations in qsf file which will require to tweak it to each core. So we come to the place where we supposed to leave. Also many defines make code look awful, non readable. And will require to test in each possible set of such defines. No, thanks!