Consistency and completeness across cc65 includes files for cc65 and ca65

[LX.NET]

Hello all,

 

Working with the cc65 codebase to fix the uploader functionality I ran into inconsistencies and missing hardware definitions for the Atari Lynx.

My goal is to make the definitions for Suzy, Mikey and the CPU complete and consistent across the assembler ca65 and the C compiler cc65.

I could use your opinions, insights and help.

 

alexthissen/cc65 at lynx-includes (github.com)

 

I forked the cc65 repository and created a branch `lynx-includes` for the changes to _mikey.h, _suzy.h and lynx.h for now. 
Using harddefs.i (see attachment) I worked with the original names from the Epyx Handy development kit to choose names where missing or rename where inconsistent or unclear. I changed around a couple of things, keeping in mind the effort of breaking changes to existing codebases for Lynx games.

 

Could you have a look and share your feedback, opinions and questions?

 

In particular I would like to hear:

• Are these changes reasonable and consistent?
• Is the new definition for the __cpu struct a good thing to have?
• What about moving the $FFF9-$FFFF definitions from _suzy.h to lynx.h, as these are not part of Suzy (as far as I know)?
• Should we rename the SCB definitions to also be consistent with harddefs.i?

harddefs.i

[42bs]

$FFF9 up belongs to Mikey rather than Suzy.

 

Also I suggest to move hardware related stuff into lynx_hw.h and include this one into lynx.h.

 

So sources which do not need e.g. TGI can direcly include lynx.h.

 

Does ca65 understand #define ?

Edited January 19 by 42bs

[LX.NET]

Hi Bastian,

 

Thanks for looking into this.

Let's see if we are on the same page.

• _mikey.h and _suzy.h are included in lynx.h at the moment. The convention at cc65 is that include files are named after their respective chipsets like pokey.h and so on. 
• TGI related prototypes are included in lynx.h, so Did you mean people can use the hardware includes directly (instead of lynx.h)? 

34 minutes ago, 42bs said:

So sources which do not need e.g. TGI can direcly include lynx.h.

So, did you perhaps mean `... can directly include lynx_hw.h`?

• The ca65 defines will be in the `lynx.inc` include file. I want to work on that after we settle on names in the C header files, then consistently duplicate them in the ca65 include.

What do you think of the __cpu define as it is now? 

[42bs]

Sorry, I should re-read my posts

 

_mikey.h / _suzy.h stay as before, only maybe changing the names if different from EPYX names.

 

_lynx.h => include _mikey.h / _suzy.h and add HW related stuff which is does not belong to either

 

lynx.h => has all non-EPYX related stuff like TGI, eeprom etc

lynx.h in turn includes _lynx.h

 

So, sources which do not use TGI may directly use _lynx.h

 

lynx.h:

// Masks for joy_read
#define JOY_UP_MASK             0x80
#define JOY_DOWN_MASK           0x40
#define JOY_LEFT_MASK           0x20
#define JOY_RIGHT_MASK          0x10
#define JOY_BTN_1_MASK          0x01
#define JOY_BTN_2_MASK          0x02

 

This should use the definitions from _suzy.h

 

IMO struct __cpu should be in _lynx.h.

 

Actually, I find it confusing that some types are defined all capital. But this is just my taste:

 

typedef struct SCB_REHVS_PAL { 

 

 

[+karri]

I have been thinking for a while to change the directory block structure to allow larger carts. Changing the block number from a byte to an int (16 bit). I did it on my own version and it works well. The advantage is that you can treat the AUDIN and CART1 as address bits as well.

 

What do you think?

 

The directory entry would become:

 

StartBlock .res 2

BlockOffset .res 2

DestAddr .res 2

FileLen .res 2

 

Sorry. This is a different topic that I should deal with later..

 

I believe that cleaning up the defines is a good thing.

[42bs]

Since you still are bound to the blocks, why not have it

StartBock .res 1

Bank        .res 1

..

Where Bank is:

0x80 => CART1, AUDIN = 0

0x90 => CART1, AUDIN = 1

0x10 => CART0, AUDIN = 1

and last

0x00 => CART0,AUDIN = 0
 

IODAT = (_IODAT & 0xef) | dir.bank;
if ( dir.bank > 0 ) {
  data = CART0;
} else {
  data = CART1;
}

 

[+karri]

For the linker I would like to be able to increment the block counter from 255 -> 256. So when the game grows you would not have to do anything when moving from 512k to 2048k.

 

The only "problem" is setting the boot block for both roms so that the code boots from the first rom on Lynx 1 and Lynx 2. The bootloader would direct the boot to rom 1.

[LX.NET]

6 hours ago, 42bs said:

lynx.h:

// Masks for joy_read
#define JOY_UP_MASK             0x80
#define JOY_DOWN_MASK           0x40
#define JOY_LEFT_MASK           0x20
#define JOY_RIGHT_MASK          0x10
#define JOY_BTN_1_MASK          0x01
#define JOY_BTN_2_MASK          0x02

 

This should use the definitions from _suzy.h

I agree with this, but... 

These definitions are needed for the integration in the joystick.c code from the cc65 specific libraries. 

So, the de should stay and would be appropriate at this level as a non-hardware specific definition. 

 

How about putting the __cpu in _mikey.h? The _.h files are all chipsets. Going with your remark that $FFF9-$FFFF is part of Mikey, it is a good fit there. 

We can name it appropriately and merge it into the larger Mikey struct.

 

I will add a new commit that will move things around and also change the SCB definitions to match the Epyx names. After I did this I will post a reply here and have your new ideas. 

[42bs]

15 hours ago, karri said:

For the linker I would like to be able to increment the block counter from 255 -> 256. So when the game grows you would not have to do anything when moving from 512k to 2048k.

 

The only "problem" is setting the boot block for both roms so that the code boots from the first rom on Lynx 1 and Lynx 2. The bootloader would direct the boot to rom 1.

Ok, I understand. So the "know how" has to be in the loader.

I think the linker must exclude blocks % 256 == 0

[42bs]

10 hours ago, LX.NET said:

I agree with this, but... 

These definitions are needed for the integration in the joystick.c code from the cc65 specific libraries. 

Yes, but rather

 

#define JOY_UP_MASK JOY_UP

 

JOY_UP is from _suzy.h

[+karri]

1 hour ago, 42bs said:

Ok, I understand. So the "know how" has to be in the loader.

I think the linker must exclude blocks % 256 == 0

Really good idea! That simplifies creation of the boot sector.

[LX.NET]

On 1/20/2024 at 8:06 AM, 42bs said:

Yes, but rather

 

#define JOY_UP_MASK JOY_UP

 

JOY_UP is from _suzy.h

I tried to follow this path. If I use the Epyx names (JOY_UP and others) there is a redefinition that clashes with some of the macros (JOY_UP(v) and such).

I have opted for JOYPAD_UP for now and use those in de MASK aliases in lynx.h.

 

[LX.NET]

For reference, please check out:

cc65/include/_mikey.h at lynx-includes · alexthissen/cc65 (github.com)

cc65/include/_suzy.h at lynx-includes · alexthissen/cc65 (github.com)

cc65/include/lynx.h at lynx-includes · alexthissen/cc65 (github.com)

 

Some progress on the redefinitions and some feedback requested from you all. Refer to the new _suzy.h file (link above).

 

What about grouping the bit definitions for certain hardware addresses to an enumeration?

typedef enum {
    JOY_RIGHT      = 0x10,
    JOY_LEFT       = 0x20,
    JOY_DOWN       = 0x40,
    JOY_UP         = 0x80,
    OPTION1_BUTTON = 0x08,
    OPTION2_BUTTON = 0x04,
    INNER_BUTTON   = 0x02,
    OUTER_BUTTON   = 0x01,
    A_BUTTON       = OUTER_BUTTON,
    B_BUTTON       = INNER_BUTTON
} JOYSTICK_bits;

 

where the definition inside the __suzy struct would be:

JOYSTICK_bits joystick;         // 0xFCB0  joystick and buttons

 

or as an anonymous enum:

enum {
    JOY_RIGHT      = 0x10,
    JOY_LEFT       = 0x20,
    JOY_DOWN       = 0x40,
    JOY_UP         = 0x80,
    OPTION1_BUTTON = 0x08,
    OPTION2_BUTTON = 0x04,
    INNER_BUTTON   = 0x02,
    OUTER_BUTTON   = 0x01,
    A_BUTTON       = OUTER_BUTTON,
    B_BUTTON       = INNER_BUTTON
};

where the definition inside __suzy remains an unsigned char.

 

Questions: 

• Would it be good to group to enums for clarity?
• If so, what enum definition do you prefer? A named typedef or an anonymous enum?
• What naming convention would we use? Now I have done HARDWAREADDRESS_bits, but I assume this can be better 

 

Second part is about the math in Suzy. I have defined unions for the MATHA-MATHP addresses. It is long but allows one to write and read from int and long values for the multiplication and division.

 

factor1 * factor2 = product (with accumulate)  -> CD * AB = EFGH (accumulate in JKLM)

dividend / divisor = quotient (with remainder)  -> EFGH / NP = ABCD (remainder in LM, JK always zero)

 

union {
        struct { 
            unsigned char mathd;    // 0xFC52
            unsigned char mathc;    // 0xFC53
            unsigned char mathb;    // 0xFC54
            unsigned char matha;    // 0xFC55
        };
        unsigned long quotient;     // 0xFC52 - 0xFC55
        struct {
            unsigned int factor1; // 0xFC52 - 0xFC53
            unsigned int factor2; // 0xFC54 - 0xFC55
        };
    };

 

The fragment shows how ABCD can be read as individual addresses matha ... mathd, but also read as the quotient for divisions or written to as factor 1 and 2 for multiplications.

 

The only concession is needed to make is split up the long dividend into two int dividend1 and dividend2, because of the order where the bytes are written. MATHE must be written to last, but apparently the cc65 generates code that will start with the highest byte (MATHE = 0xFC63) first. By splitting into two int values and writing to dividend1 then 2 will do the trick.

 

// Multiply
    SUZY.sprsys = ACCUMULATE;

    SUZY.factor1 = 0x1234;
    SUZY.factor2 = 0x5678;
    while ((SUZY.sprsys & MATHWORKING) != 0) ;
	// Read from SUZY.product and SUZY.accumulate

// Division
    SUZY.divisor = 0x3125;
    SUZY.dividend1 = 0x5678;
    SUZY.dividend2 = 0x1234;
    while ((SUZY.sprsys & MATHWORKING) != 0) ;
	// Read from SUZY.quotient and SUZY.remainder

• What do you think? Useful to do it like this?
• Any feedback on the other changes?

[42bs]

Just my 2cts about enums: Don't use enum!

 

enums are a source of trouble on higher CPUs I doubt they do good on 6502.

 

Did you check the resulting code?

[42bs]

BTW: Do not write SPRSYS directly! Use the shadow register to preserve former values.

[+karri]

 No enum please. Just defines.

 

I have problems with cc7800 in case there are enums. And I try to make my projects on both platforms. Lynx and 7800.

[LX.NET]

16 hours ago, 42bs said:

Just my 2cts about enums: Don't use enum!

 

enums are a source of trouble on higher CPUs I doubt they do good on 6502.

 

Did you check the resulting code?

Clear statement. 

I did check the code and the resulting enums are just simple unsigned char values. No need to prefix the values. Your code doesn't change much, except that the values are grouped together is a consistent way.

 

Will dive into the compiler to see what the benefits of the enum are in cc65, other than clarity. So far there seems to be no compiler type checking to prevent errors. That would actually be the main reason for me to introduce them: making sure you do not use the wrong #define values for a particular address. 

15 hours ago, karri said:

 No enum please. Just defines.

 

I have problems with cc7800 in case there are enums. And I try to make my projects on both platforms. Lynx and 7800.

Can you explain this a bit more, @Karri? So far, it seems that the code doesn't change in any way. Just the include .h file. Is your scenario involving code generation, or will it refer to multiple header files?

 

[LX.NET]

16 hours ago, 42bs said:

BTW: Do not write SPRSYS directly! Use the shadow register to preserve former values.

I have seem some situations where this is used in asm. Can you describe the usage and code flow for the shadow register when writing a value to SPRSYS? Thanks.

[42bs]

15 minutes ago, LX.NET said:

I have seem some situations where this is used in asm. Can you describe the usage and code flow for the shadow register when writing a value to SPRSYS? Thanks.

There should be a _SPRSYS variable. At least IIRC in new_cc65.

When SUZY is initialized _SPRSYS is also written.

So if you need to preserve setting you cannot read, you should do

_SPRSYS |= MATHACCUMULATE;
suzy.sprsys = _SPRSYS;

_SPRSYS &= ~MATHACCUMULATE;
suzy.sprsys = _SPRSYS;

 

[42bs]

As for enum: I am sorry, I mixed it up with bit fields. These are the real evil as the standard does not define the endianess.

 

So if cc65 produces for these below situations the same code, I see no problem with it.

Only: Make sure they do not clash with #defineS

enum {
  eA= 10,
  eB = 20
};

char x = eA+eB;

#define A 10
#define B 20

char y = A+B;

 

[LX.NET]

35 minutes ago, 42bs said:

As for enum: I am sorry, I mixed it up with bit fields. These are the real evil as the standard does not define the endianess.

 

So if cc65 produces for these below situations the same code, I see no problem with it.

Only: Make sure they do not clash with #defineS

Did some experiments with the output of the compiler. 

#define JOY_UP 0x80

enum 
{
    JOY_UP = 0x80
};

will not compile because of the clash.

Enums are optimized to unsigned char for value below 0xFF. It switches to a 2-byte value if bigger values are in the enum.

 

Named and anonymous enums can be interchanged. There is no type-checking.

 

Advantage of using enums is that you can mix #define macros and constant values.

#define JOY_TEST_MASK 0x01
#define JOY_TEST(v) ((v & JOY_TEST_MASK) == JOY_TEST_MASK)
enum {
    JOY_TEST
};

 

@karri If you read all this, and we would go for enums, would that cause problems for you?

Code using enums will not change.

 

[+karri]

It is not a real problem. I already had to develop a Python script for converting all enums to defines when I ported my Rainbow code to cc7800.

 

The C-compiler I am using for cc7800 is a very slimmed down thing that does not understand C-code. But it produces really small code footprint compared to cc65. It also has no libraries, no linker, no optimizer and so on. But it has all the cool special keywords to create holeydma, ramchip variables, bankswitching etc.

[LX.NET]

Some more work done. It is shaping up pretty nicely.

cc65/include/_suzy.h at lynx-includes · alexthissen/cc65 (github.com)

 

Going through the harddefs.i and the Handy Specifications and Hardware addresses documents I changed a couple of things in the Suzy definition.

 

Non-extensive list:

• Used lower casing for the SCB struct definition now (more consistent with rest of cc65 libraries)
• Changed tmpadr to unsigned char* instead of int (it is an address)
• Renamed sprbase to vidbas (as per documentation)
• Made names same as in harddefs.i and documentation (mostly renaming base to bas)
• Change hposstrt, vposstrt, sprhsiz, sprvsiz unsigned int instead of unsigned char* (these are integer values, not addresses)
• Removed PENPAL_X structs for the pen palette. It doesn't seem to be used or add anything useful.
• Introduced a union with three structures for the mapping of the math engine (more below)
• Created enums as grouped bit definitions instead of the #define that were there before
• Renamed parstat, pardata to iostatus, iodata 
• Renamed cart0 and cart1 to rcart0 and rcart1
• Renamed scb control 0 and 1 bit definitions to match the Epyx names. 
• Deleted macro defines for math elements (e.g. dividend, quotient, product) as these are now addressable through the structs

I have created three structs for the mapping of the signed and unsigned multiply, and unsigned divide in Suzy.

 

struct _math_unsigned_multiply {
    unsigned int factor1;         // 0xFC52 - 0xFC53
    unsigned int factor2;         // 0xFC54 - 0xFC55  write starts multiply
    unsigned char unused2[10];    // 0xFC56 - 0xFC5F  do not use
    unsigned long product;        // 0xFC60 - 0xFC63
    unsigned char unused3[8];     // 0xFC64 - 0xFC6B  do not use
    unsigned long accumulate;     // 0xFC6C - 0xFC6F
};

struct _math_signed_multiply {
    int factor1;                  // 0xFC52 - 0xFC53
    int factor2;                  // 0xFC54 - 0xFC55  write starts multiply
    unsigned char unused2[10];    // 0xFC56 - 0xFC5F  do not use
    long product;                 // 0xFC60 - 0xFC63
    unsigned char unused3[8];     // 0xFC64 - 0xFC6B  do not use
    long accumulate;              // 0xFC6C - 0xFC6F
};

struct _math_divide {
    unsigned long quotient;       // 0xFC52 - 0xFC55
    unsigned int divisor;         // 0xFC56 - 0xFC57
    unsigned char unused2[8];     // 0xFC58 - 0xFC5F  do not use
    unsigned int dividend2;       // 0xFC60 - 0xFC61
    unsigned int dividend1;       // 0xFC62 - 0xFC63 write starts divide
    unsigned char unused3[8];     // 0xFC64 - 0xFC6B  do not use
    unsigned long remainder;      // 0xFC6C - 0xFC6F
};

 

The definition of the math address range in Suzy (0xFC50-0xFC7F) is specific to the use case of multiple and divide with a separate struct definition.

 

In Suzy the math range becomes:

    unsigned char unused1[2];     // 0xFC50 - 0xFC51  do not use
    union {
        struct {
            unsigned char mathd;      // 0xFC52
            unsigned char mathc;      // 0xFC53
            unsigned char mathb;      // 0xFC54
            unsigned char matha;      // 0xFC55  write starts a multiply operation
            unsigned char mathp;      // 0xFC56
            unsigned char mathn;      // 0xFC57
            unsigned char unused2[8]; // 0xFC58 - 0xFC5F  do not use
            unsigned char mathh;      // 0xFC60
            unsigned char mathg;      // 0xFC61
            unsigned char mathf;      // 0xFC62
            unsigned char mathe;      // 0xFC63  write starts a divide operation
            unsigned char unused3[8]; // 0xFC64 - 0xFC6B  do not use
            unsigned char mathm;      // 0xFC6C
            unsigned char mathl;      // 0xFC6D
            unsigned char mathk;      // 0xFC6E
            unsigned char mathj;      // 0xFC6F
        };
        struct _math_unsigned_multiply math_unsigned_multiply;
        struct _math_signed_multiply math_signed_multiply;
        struct _math_divide math_divide;
    };
    unsigned char unused4[16];        // 0xFC70 - 0xFC7F  do not use

 

Works like a charm.

Examples:

 

// Should work with shadow register
SUZY.sprsys = ACCUMULATE;
SUZY.math_unsigned_multiply.accumulate = 0;
SUZY.math_unsigned_multiply.factor1 = 1234;
SUZY.math_unsigned_multiply.factor2 = 5678;
// Read from SUZY.math_unsigned_multiply.product and .accumulate as unsigned longs after math operation completes

SUZY.sprsys = ACCUMULATE | SIGNMATH;
SUZY.math_signed_multiply.accumulate = 0;
SUZY.math_signed_multiply.factor1 = -1234;
SUZY.math_signed_multiply.factor2 = 5678;
// Read from SUZY.math_signed_multiply.product and .accumulate as signed longs after math operation completes

SUZY.math_divide.divisor = 0x3125;
SUZY.math_divide.dividend2 = 0x5679;
SUZY.math_divide.dividend1 = 0x1234;
// Read from SUZY.math_divide.quotient and .remainder as unsigned longs after math operation completes

In assembler the right addresses are used in the right order.

 

Some questions:

• Okay to rename the scbctrl0 and 1 bit definitions?
• How do you like this union of structs for the math engine?
• What about the readability of the code for math?
• I find the names for the structs a bit long. Can you think of other ones?
• Should we name the union to 'math' so the syntax becomes SUZY.math.matha and SUZY.math.signed_multiply.product, SUZY.math.divide.quotient and so on

 

Plans for additional changes

• Removed a lot of the SCB variations. I think it is not necessary to have all possible combinations of optional H/V sizes, stretch and tilt, and the palette sizes for 1, 2, 3 and 4 bits per pixel in the SCB structure. 
  The most common ones seem to be scb_none, scb_hvst_pal4, scb_hvst, scb_hv. Rest can be deduced per project from scb_hvst_pal4, which is the most complete one. 
  Am I correct in assuming these are the most common ones and the rest can be done by a game developer as needed

 

Again, all feedback welcome. Thanks for all the thoughts and feedback so far.

Edited January 24 by LX.NET

[+karri]

Wow! Great work.

 

One thing I have been wondering about is the linked sprite list. Should we have some base class so you can link different types of sprites. Or should we just skip the checks and use void *.

[LX.NET]

In my experience you need to mix SCB types. For example, the first might need to load HVST values and the palette. The following ones are usually the simplest SCBs with no HVST or palette, as this has been set in the first for the entire chain of sprites. Should they change, it is again a different SCB type.

My point being: Type checking would be limiting.

 

The original definition was char*. I have changed that to unsigned char*. I wonder if it should be void* like you said @karri? I assumed void* was more for function pointers, not address. What do you think?