olikraus / u8g2

U8glib library for monochrome displays, version 2
Other
5.15k stars 1.05k forks source link

Hardware SPI implementation on AVR #1954

Closed dolence closed 1 year ago

dolence commented 2 years ago

I've been trying to implement hardware SPI on AVR baremetal. IC is an Atmega64 running at 12 MHz. I even tried to hardcode SPI speed but nothing is being showed. This is what I did following the Arduino implementation and avr-libc example:

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <u8g2.h>

#define DISPLAY_SCK_DIR DDRB
#define DISPLAY_SCK_PORT PORTB
#define DISPLAY_SCK_PIN 1

#define DISPLAY_MOSI_DIR DDRB
#define DISPLAY_MOSI_PORT PORTB
#define DISPLAY_MOSI_PIN 2

#define DISPLAY_CS_DIR DDRB
#define DISPLAY_CS_PORT PORTB
#define DISPLAY_CS_PIN 0

u8g2_t u8g2;

uint8_t my_u8x8_byte_avr_hw_spi (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
    uint8_t *data;

    switch (msg) {

        case U8X8_MSG_BYTE_INIT:
        if ( u8x8->bus_clock == 0 )     /* issue 769 */
        u8x8->bus_clock = u8x8->display_info->sck_clock_hz;     
        /* disable chipselect */
        u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);      
        DISPLAY_MOSI_DIR |= (1 << DISPLAY_MOSI_PIN);        
        DISPLAY_SCK_DIR |= (1 << DISPLAY_SCK_PIN);              
        SPCR = ((1 << SPE) | (1 << MSTR));
        switch (u8x8->display_info->spi_mode) {
            case 0:
            break;
            case 1:
            SPCR |= (1 << CPHA);
            break;
            case 2:
            SPCR |= (1 << CPOL);
            break;
            case 3:         
            SPCR |= (1 << CPOL);
            SPCR |= (1 << CPHA);
            break;
        };
        /*
        switch (F_CPU / u8x8->display_info->sck_clock_hz) {
            case 2:         
            SPSR |= (1 << SPI2X);
            break;
            case 4:         
            break;
            case 8:
            SPSR |= (1 << SPI2X);
            SPCR |= (1 << SPR0);
            break;
            case 16:        
            SPCR |= (1 << SPR0);
            break;
            case 32:            
            SPSR |= (1 << SPI2X);
            SPCR |= (1 << SPR1);
            break;
            case 64:            
            SPCR |= (1 << SPR1);
            break;
            case 128:           
            SPCR |= (1 << SPR1);
            SPCR |= (1 << SPR0);
            break;
        }
        */
        SPCR |= (1 << SPR0);

        u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
        break;

        case U8X8_MSG_BYTE_SET_DC:
        u8x8_gpio_SetDC(u8x8, arg_int);
        break;

        case U8X8_MSG_BYTE_START_TRANSFER:
        u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_enable_level);
        u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->post_chip_enable_wait_ns, NULL);
        break;

        case U8X8_MSG_BYTE_SEND:
        data = (uint8_t *) arg_ptr;
        while (arg_int > 0) {
            SPDR = (uint8_t) * data;
            while (!(SPSR & (1 << SPIF)));
            data++;
            arg_int--;
        }
        break;

        case U8X8_MSG_BYTE_END_TRANSFER:
        u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->pre_chip_disable_wait_ns, NULL);
        u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
        break;
        default:
        return 0;
    }

    return 1;
}

uint8_t my_u8x8_gpio_and_delay (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
    // Re-use library for delays
    //if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
    //return 1;

    switch (msg) {
        // called once during init phase of u8g2/u8x8
        // can be used to setup pins
        case U8X8_MSG_GPIO_AND_DELAY_INIT:
        DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
        //DC_DDR |= _BV(DC_BIT);
        //RESET_DDR |= _BV(RESET_BIT);
        break;
        // CS (chip select) pin: Output level in arg_int
        case U8X8_MSG_GPIO_CS:
        if (arg_int)
        DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
        else
        DISPLAY_CS_DIR &= ~(1 << DISPLAY_CS_PIN);
        break;
        // DC (data/cmd, A0, register select) pin: Output level in arg_int
        case U8X8_MSG_GPIO_DC:
        if (arg_int)
        //DC_PORT |= _BV(DC_BIT);
        //else
        //DC_PORT &= ~_BV(DC_BIT);
        break;
        // Reset pin: Output level in arg_int
        case U8X8_MSG_GPIO_RESET:
        if (arg_int)
        //RESET_PORT |= _BV(RESET_BIT);
        //else
        //RESET_PORT &= ~_BV(RESET_BIT);
        break;
        default:
        u8x8_SetGPIOResult(u8x8, 1);
        break;
    }
    return 1;
}

int main(void)
{
    sei();
    DDRC |= (1 << 4);
    PORTC |= (1 << 4);

    u8g2_Setup_st7920_128x64_f(&u8g2, U8G2_R0, my_u8x8_byte_avr_hw_spi, my_u8x8_gpio_and_delay);    
    u8g2_InitDisplay(&u8g2);
    u8g2_SetPowerSave(&u8g2, 0);

    /* full buffer example, setup procedure ends in _f */
    u8g2_ClearBuffer(&u8g2);
    u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
    u8g2_DrawStr(&u8g2, 1, 18, "U8g2 on AVR");
    u8g2_SendBuffer(&u8g2);

    /* Replace with your application code */
    while (1) 
    {

    }
}

Any thoughts on this?

olikraus commented 2 years ago

my_u8x8_gpio_and_delay: Handling of the delay messages is missing: https://github.com/olikraus/u8g2/wiki/Porting-to-new-MCU-platform#template-for-the-gpio-and-delay-callback

I think at least DELAY_MILLI should be there, probably also the 100NANO delay.

dolence commented 2 years ago

Thank you. I missed the delay functions. I don't know if I did it right, but after implementing it still not working. I've made a swap on that 12 MHz crystal oscillator for an 16 MHz just to be sure it wasn't the case. I'm sure wiring is fine because I have tried an old firmware using u8g and m2tklib and it worked as expected. I made some clean up, mainly on DC and RESET pins I'm not using. Still missing something...

EDIT 1: this is the line where the program is halting. while (!(SPSR & (1 << SPIF)));

EDIT 2: registers seen to be properly set image

EDIT 3: delay received message is 32, which weirdly doesn't map to any delay message definition image

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <u8g2.h>

#define DISPLAY_SCK_DIR DDRB
#define DISPLAY_SCK_PORT PORTB
#define DISPLAY_SCK_PIN 1

#define DISPLAY_MOSI_DIR DDRB
#define DISPLAY_MOSI_PORT PORTB
#define DISPLAY_MOSI_PIN 2

#define DISPLAY_CS_DIR DDRB
#define DISPLAY_CS_PORT PORTB
#define DISPLAY_CS_PIN 0

u8g2_t u8g2;

#define P_CPU_NS (1000000000UL / F_CPU)

uint8_t my_u8x8_avr_delay(u8x8_t *u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr)
{
    uint8_t cycles;

    switch(msg)
    {
        case U8X8_MSG_DELAY_NANO:     // delay arg_int * 1 nano second
        // At 20Mhz, each cycle is 50ns, the call itself is slower.
        break;
        case U8X8_MSG_DELAY_100NANO:    // delay arg_int * 100 nano seconds
        // Approximate best case values...
        #define CALL_CYCLES 26UL
        #define CALC_CYCLES 4UL
        #define RETURN_CYCLES 4UL
        #define CYCLES_PER_LOOP 4UL

        cycles = (100UL * arg_int) / (P_CPU_NS * CYCLES_PER_LOOP);

        if(cycles > CALL_CYCLES + RETURN_CYCLES + CALC_CYCLES)
        break;

        __asm__ __volatile__ (
        "1: sbiw %0,1" "\n\t" // 2 cycles
        "brne 1b" : "=w" (cycles) : "0" (cycles) // 2 cycles
        );
        break;
        case U8X8_MSG_DELAY_10MICRO:    // delay arg_int * 10 micro seconds
        for(int i=0 ; i < arg_int ; i++)
        _delay_us(10);
        break;
        case U8X8_MSG_DELAY_MILLI:      // delay arg_int * 1 milli second
        for(int i=0 ; i < arg_int ; i++)
        _delay_ms(1);
        break;
        default:
        return 0;
    }
    return 1;
}

uint8_t my_u8x8_byte_avr_hw_spi (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
    uint8_t *data;

    switch (msg) {

        case U8X8_MSG_BYTE_INIT:
            if ( u8x8->bus_clock == 0 )     /* issue 769 */
            u8x8->bus_clock = u8x8->display_info->sck_clock_hz;     
            /* disable chipselect */
            u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);      
            DISPLAY_MOSI_DIR |= (1 << DISPLAY_MOSI_PIN);        
            DISPLAY_SCK_DIR |= (1 << DISPLAY_SCK_PIN);              
            SPCR = ((1 << SPE) | (1 << MSTR));
            switch (u8x8->display_info->spi_mode) {
                case 0:
                break;
                case 1:
                SPCR |= (1 << CPHA);
                break;
                case 2:
                SPCR |= (1 << CPOL);
                break;
                case 3:         
                SPCR |= (1 << CPOL);
                SPCR |= (1 << CPHA);
                break;
            };

            switch (F_CPU / u8x8->display_info->sck_clock_hz) {
                case 2:         
                SPSR |= (1 << SPI2X);
                break;
                case 4:         
                break;
                case 8:
                SPSR |= (1 << SPI2X);
                SPCR |= (1 << SPR0);
                break;
                case 16:                
                SPCR |= (1 << SPR0);
                break;
                case 32:            
                SPSR |= (1 << SPI2X);
                SPCR |= (1 << SPR1);
                break;
                case 64:            
                SPCR |= (1 << SPR1);
                break;
                case 128:           
                SPCR |= (1 << SPR1);
                SPCR |= (1 << SPR0);
                break;
            }

            u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
            break;

        case U8X8_MSG_BYTE_START_TRANSFER:
            u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_enable_level);
            u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->post_chip_enable_wait_ns, NULL);
            break;

        case U8X8_MSG_BYTE_SEND:
            data = (uint8_t *) arg_ptr;
            while (arg_int > 0) {
                SPDR = (uint8_t) * data;
                while (!(SPSR & (1 << SPIF)));
                data++;
                arg_int--;
            }
            break;

        case U8X8_MSG_BYTE_END_TRANSFER:
            u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->pre_chip_disable_wait_ns, NULL);
            u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
            break;

        default:
            if (my_u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr)) // check for any delay msgs
            return 1;
            u8x8_SetGPIOResult(u8x8, 1);      // default return value
            break;
    }

    return 1;
}

uint8_t my_u8x8_gpio_and_delay (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
    // Re-use library for delays
    //if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
    //return 1;

    switch (msg) {
        // called once during init phase of u8g2/u8x8
        // can be used to setup pins
        case U8X8_MSG_GPIO_AND_DELAY_INIT:
        DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);

        break;
        // CS (chip select) pin: Output level in arg_int
        case U8X8_MSG_GPIO_CS:
        if (arg_int)
        DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
        else
        DISPLAY_CS_DIR &= ~(1 << DISPLAY_CS_PIN);
        break;
        default:
        u8x8_SetGPIOResult(u8x8, 1);
        break;
    }
    return 1;
}

int main(void)
{
    sei();
    DDRC |= (1 << 4);
    PORTC |= (1 << 4);

    u8g2_Setup_st7920_128x64_f(&u8g2, U8G2_R0, my_u8x8_byte_avr_hw_spi, my_u8x8_gpio_and_delay);    
    u8g2_InitDisplay(&u8g2);
    u8g2_SetPowerSave(&u8g2, 0);

    /* full buffer example, setup procedure ends in _f */
    u8g2_ClearBuffer(&u8g2);
    u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
    u8g2_DrawStr(&u8g2, 1, 18, "U8g2 on AVR");
    u8g2_SendBuffer(&u8g2);

    /* Replace with your application code */
    while (1) 
    {

    }
}
dolence commented 2 years ago

I've checked the initialization of SPI and all the registers seem to be right but unfortunately the display doesn't initialize. Could it be related to the fact my display doesn't use all the four wires as in #1949 ? It doesn't use the DC pin. I could give the Arduino version a try and see if this display work in 3-wire mode...

olikraus commented 2 years ago

while (!(SPSR & (1 << SPIF)));

Not sure whether I can help here. This is uC specific.

delay received message is 32

I don't exactly which messages are used in your case, but you should probably implement all delay messages listed in the code template to be on the safe side.

It doesn't use the DC pin. I could give the Arduino version a try and see if this display work in 3-wire mode...

I am a little bit confused. Let me start from the beginning: There are different protocols with your display two of them are: 3-wire and 4-wire SPI.

It you choose one of them, you need to:

  1. Select the proper protocol on the PCB of the display itself. It will require to solder a specific protocol, which is the fixed with your display
  2. U8g2 then need to follow that protocol, which is from now on expected by your display. You can not just change the protocol on software side. Instead software has to fit to your display hardware configuration.
  3. 3-wire vs 4-wire differes in the usage of the DC line. and the size of one token (8 bits vs 9 bits)
  4. 3-wire is not supported in the c interface, it is only there with the Arduino C++ interface

You have actually asked for 3wire, so I pointed out the code in the C++ part of u8g2 (see #1949), but your above code doesn't seem to include the code. So your code above looks more like a 4-wire code.

As a starting point, my suggestion is this: Configure your display for 4-wire communication and then use the existing 4-wire communication in u8g2.

dolence commented 2 years ago

Oli, thank you for the clarification!

1) Display is set to use SPI on hardware side, it was tested and working with the former version of the library. 2) I undertood this, thanks. 3-4) My display only have 3 pins for SPI usage. They are RS, R/W and E. image

olikraus commented 2 years ago

Oh, it looks like you want to use a ST7920 in serial mode. I think this wasn't mentioned before. ST7920 is special: You need to use 4-wire protocol (this means: a very normal byte procedure), however the ST7920 indeed does not have a DC line. So your byte procedure should look like a 4-wire byte procedure.

dolence commented 2 years ago

Ok, so I got everything working following this tutorial. It was very easy. No delay functions to be implemented, just a few pin definitions and a couple of messages that later I made some cleanup removing uneeded code for my setup. Maybe it would be the case of updating documentation to reflect this.

A simple hello world code ended like this:

#include <avr/io.h>
#include <util/delay.h>
#include <avr/power.h>
#include "u8g2.h"
#include "u8x8_avr.h"

#define CS_DDR DDRB
#define CS_PORT PORTB
#define CS_BIT 0

uint8_t u8x8_gpio_and_delay(u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
    // Re-use library for delays
    if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
    return 1;

    switch (msg) {
        // called once during init phase of u8g2/u8x8
        // can be used to setup pins
        case U8X8_MSG_GPIO_AND_DELAY_INIT:
            CS_DDR |= _BV(CS_BIT);      
            break;

        // CS (chip select) pin: Output level in arg_int
        case U8X8_MSG_GPIO_CS:
            if (arg_int)
                CS_PORT |= _BV(CS_BIT);
            else
                CS_PORT &= ~_BV(CS_BIT);
            break;

        default:
        u8x8_SetGPIOResult(u8x8, 1);
        break;
    }
    return 1;
}

u8g2_t u8g2;
int main (void) {
    u8g2_Setup_st7920_s_128x64_f(&u8g2, U8G2_R0, u8x8_byte_avr_hw_spi,  u8x8_gpio_and_delay);
    u8g2_InitDisplay(&u8g2);
    u8g2_SetPowerSave(&u8g2, 0);

    u8g2_ClearBuffer(&u8g2);
    u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
    u8g2_DrawStr(&u8g2, 0, 15, "HELLO!");
    u8g2_SendBuffer(&u8g2);

    while (1) {

    }
}

I also had to define some symbols, most important is AVR_USE_HW_SPI and some SPI pin definitions: image

And that is it!

Now I'm facing some problems with the C version of MUI. I'm trying a basic example following the MUIMinimal.ino as a guide but MUIF list is throwing an "initializer element is not constant" error and Form Definition String fds_t do not appear to be a recognized symbol. I'm doing something wrong or it is just untested in C? Should I stick to the Arduino version to use MUI? image

olikraus commented 2 years ago

... updating documentation to reflect this

https://github.com/olikraus/u8g2/wiki/Porting-to-new-MCU-platform#atmel-avr

"initializer element is not constant" error and Form Definition String fds_t do not appear to be a recognized symbol. I'm doing something wrong or it is just untested in C?

In fact the development of MUI happend in C only. Did you include both mui.h and mui_u8g2.h? See also here: https://github.com/olikraus/u8g2/blob/master/sys/sdl/mui_blink/main.c

dolence commented 2 years ago

In fact the development of MUI happend in C only. Did you include both mui.h and mui_u8g2.h? See also here:

Yes, both are included. I'm trying a very minimal test. It's a form with just a label on it:

muif_t muif_list[] MUI_PROGMEM = {
MUIF_LABEL(mui_u8g2_draw_text)
};

fds_t fds[] MUI_PROGMEM =
MUI_FORM(1)
MUI_LABEL(5,12, "A label")
;

This is the line where the error message is pointing to: image

dolence commented 2 years ago

Oli, I'm still stuck with this initializer element is not constant error. I double checked everything. I have both mui_u8g2.h and mui.h included on my main.c, right above u8g2.h and u8x8_avr.h. Is there anything wrong in here? It was tested on baremetal AVR or just the C on Arduino? I think will test it under Arduino interface and see what happens...

muif_t muif_list[] MUI_PROGMEM = {
    MUIF_LABEL(mui_u8g2_draw_text)
};

fds_t fds[] MUI_PROGMEM =
MUI_FORM(1)
MUI_LABEL(5,12, "A label")
;
olikraus commented 2 years ago

I have created a test and it works as expected: https://github.com/olikraus/u8g2/blob/db3888e8d3490e5b7e6f95a8ee869b8bf38950f1/sys/sdl/mui_issue_1954/main.c#L13-L25

I am not sure whether MUI_PROGMEM is required. I thought that I have moved this into the struct typedef. Maybe try without MUI_PROGMEM. For the SDL code above MUI_PROGMEM is anyways disabled, so the problem might be indeed MUI_PROGMEM.

dolence commented 2 years ago

I've tried with and without MUI_PROGMEM and even setup an entire new environment in a different machine. After installing Microchip Studio from scratch and creating an entire new project, including the library files etc, still the same error.

        "C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "lib/u8x8_avr.d" -MT"lib/u8x8_avr.d" -MT"lib/u8x8_avr.o"   -o "lib/u8x8_avr.o" "../lib/u8x8_avr.c" 
        Finished building: ../lib/u8x8_avr.c
        Building file: .././main.c
        Invoking: AVR/GNU C Compiler : 5.4.0
        In file included from .././main.c:11:0:
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
         #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
                                     ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,35): info: in definition of macro 'MUIF'
         #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
                                           ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
         MUIF_LABEL(mui_u8g2_draw_text)
         ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): info: (near initialization for 'muif_list[0].id0')
         #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
                                     ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,35): info: in definition of macro 'MUIF'
         #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
                                           ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
         MUIF_LABEL(mui_u8g2_draw_text)
         ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
         #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
                                     ^
        "C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "main.d" -MT"main.d" -MT"main.o"   -o "main.o" ".././main.c" 
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\Debug\Makefile(1458,1): error: recipe for target 'main.o' failed
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,42): info: in definition of macro 'MUIF'
         #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
                                                  ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
         MUIF_LABEL(mui_u8g2_draw_text)
         ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): info: (near initialization for 'muif_list[0].id1')
         #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
                                     ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,42): info: in definition of macro 'MUIF'
         #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
                                                  ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
         MUIF_LABEL(mui_u8g2_draw_text)
         ^
        make: *** [main.o] Error 1
        make: *** Waiting for unfinished jobs....
        Building file: ../csrc/u8x8_fonts.c
        Invoking: AVR/GNU C Compiler : 5.4.0
        "C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "csrc/u8x8_fonts.d" -MT"csrc/u8x8_fonts.d" -MT"csrc/u8x8_fonts.o"   -o "csrc/u8x8_fonts.o" "../csrc/u8x8_fonts.c" 
        Finished building: ../csrc/u8x8_fonts.c
        Building file: ../csrc/u8g2_fonts.c
        Invoking: AVR/GNU C Compiler : 5.4.0
        "C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "csrc/u8g2_fonts.d" -MT"csrc/u8g2_fonts.d" -MT"csrc/u8g2_fonts.o"   -o "csrc/u8g2_fonts.o" "../csrc/u8g2_fonts.c" 
        Finished building: ../csrc/u8g2_fonts.c
    Done executing task "RunCompilerTask" -- FAILED.
Done building target "CoreBuild" in project "LQ-01_u8g2_mui.cproj" -- FAILED.
Done building project "LQ-01_u8g2_mui.cproj" -- FAILED.

Build FAILED.
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========
olikraus commented 2 years ago

Hmm... difficult to tell. Not sure how to continue from here. MUI works in unix/SDL and Arduino environment.

dolence commented 2 years ago

Could it be something specific to AVR-GCC? I could upload a simple project in case someone else want to test it. Anyway, I've just made to switch to the Arduino version until I got it working on Studio.

olikraus commented 2 years ago

Could it be something specific to AVR-GCC?

Hmm... MUI also works without problems on Arduino UNO

dolence commented 2 years ago

Different compiler versions, maybe? Or am I barking up the wrong tree?

olikraus commented 2 years ago

I think Arduino 1.8.4 or 1.8.10 did work for me.

kasitoru commented 2 years ago

I have the same problem. After spending some time, I found the reason. When using PROGMEM, all values should be constants, but in some defines this is not the case. Example:

#define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb}

The problem is in id[0] and id[1]. There are several other similar definitions.

olikraus commented 2 years ago

Usually all provided data are constant. Moreover: This error didn't happen to me.

kasitoru commented 2 years ago

However, as a temporary solution, @dolence can make the following changes to the mui.h file (lines 158-172):

//#if defined(__GNUC__) && defined(__AVR__)
//#  define muif_get_id0(muif) mui_pgm_read(&((muif)->id0))
//#  define muif_get_id1(muif) mui_pgm_read(&((muif)->id1))
//#  define muif_get_cflags(muif) mui_pgm_read(&((muif)->cflags))
//#  define muif_get_extra(muif) mui_pgm_read(&((muif)->extra))
//#  define muif_get_data(muif) ((void *)mui_pgm_wread(&((muif)->data)))
//#  define muif_get_cb(muif) ((muif_cb)mui_pgm_wread(&((muif)->cb)))
//#else
#  define muif_get_id0(muif) ((muif)->id0)
#  define muif_get_id1(muif) ((muif)->id1)
#  define muif_get_cflags(muif) ((muif)->cflags)
#  define muif_get_extra(muif) ((muif)->extra)
#  define muif_get_data(muif) ((muif)->data)
#  define muif_get_cb(muif) ((muif)->cb)
//#endif

And accordingly remove MUI_PROGMEM for muif_list. It worked for me.

olikraus commented 2 years ago

I am even more confused. I neither understand the original problem nor do I understand how the changes from @kasitoru are related to the original problem.

kasitoru commented 2 years ago

As I understand it, the problem is that some versions of the compiler for avr (in this case, we are talking about the official toolchain from Microchip) consider it unacceptable to use calculated values (such as id[0] from ".L") for PROGMEM variables. You can add an additional define (for example MUIF_DISABLE_PGM) for these problematic compilers:

/* assumes that pointers are 16 bit so encapusalte the wread i another ifdef __AVR__ */
#if defined(__GNUC__) && defined(__AVR__) && !defined(MUIF_DISABLE_PGM)
#  define muif_get_id0(muif) mui_pgm_read(&((muif)->id0))
#  define muif_get_id1(muif) mui_pgm_read(&((muif)->id1))
#  define muif_get_cflags(muif) mui_pgm_read(&((muif)->cflags))
#  define muif_get_extra(muif) mui_pgm_read(&((muif)->extra))
#  define muif_get_data(muif) ((void *)mui_pgm_wread(&((muif)->data)))
#  define muif_get_cb(muif) ((muif_cb)mui_pgm_wread(&((muif)->cb)))
#else
#  define muif_get_id0(muif) ((muif)->id0)
#  define muif_get_id1(muif) ((muif)->id1)
#  define muif_get_cflags(muif) ((muif)->cflags)
#  define muif_get_extra(muif) ((muif)->extra)
#  define muif_get_data(muif) ((muif)->data)
#  define muif_get_cb(muif) ((muif)->cb)
#endif

When compiling, pass this argument to GCC:

avr-gcc.exe $(CCFLAGS) -DMUIF_DISABLE_PGM -c u8g2/csrc/*.c
olikraus commented 2 years ago

But the above get macros are not used in the errors reported by @dolence

kasitoru commented 2 years ago

Part of his error messages:

D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
         #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)

This is because he set MUI_PROGMEM for muif_list. If remove this, there will be no errors, but the display will be empty (because the reading will still come from the progmem area, but is no data there). That's why need to disable this macros.

olikraus commented 2 years ago

Ok, understand. But this still doesn't explain the root problem. 🤔

kasitoru commented 2 years ago

I don't understand it either. I think the problem is in the avr-gcc compiler. Because the code looks good.

kasitoru commented 2 years ago

It was my mistake. It was necessary to move the functions that are used in muif_list to the global scope.

qianfan-Zhao commented 3 months ago
#include <stdio.h>

int a = "1234567"[0];

int main(void)
{
    printf("%d\n", a);
    return 0;
}

this code can not compiled before gcc-8, it will report errors in my gcc-7:

$  gcc 1.c       
1.c:3:9: error: initializer element is not constant
 int a = "1234567"[0];
         ^~~~~~~~~
$ gcc -v 
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.5.0-6ubuntu2' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,ob
j-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-
gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
 --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multia
rch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver -
-enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 7.5.0 (Ubuntu 7.5.0-6ubuntu2)