From e84e8ca333ec53ceb28de008005f9de9b47c1310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juh=C3=A1sz=20D=C3=A1niel?= Date: Thu, 6 Oct 2022 07:51:22 +0200 Subject: [PATCH 1/2] Add LCD FPS testing utility --- examples/board_hd44780/Makefile | 3 +- examples/board_hd44780/atmega48_fps_test.c | 47 ++++++++++++++++++++++ examples/board_hd44780/charlcd.c | 8 +++- 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 examples/board_hd44780/atmega48_fps_test.c diff --git a/examples/board_hd44780/Makefile b/examples/board_hd44780/Makefile index 9666fa0a8..23a4f5229 100644 --- a/examples/board_hd44780/Makefile +++ b/examples/board_hd44780/Makefile @@ -33,9 +33,10 @@ LDFLAGS += -lpthread include ../Makefile.opengl -all: obj atmega48_charlcd.axf ${target} +all: obj atmega48_charlcd.axf atmega48_fps_test.axf ${target} atmega48_charlcd.axf: atmega48_charlcd.c +atmega48_fps_test.axf: atmega48_fps_test.c include ${simavr}/Makefile.common diff --git a/examples/board_hd44780/atmega48_fps_test.c b/examples/board_hd44780/atmega48_fps_test.c new file mode 100644 index 000000000..f862a44a8 --- /dev/null +++ b/examples/board_hd44780/atmega48_fps_test.c @@ -0,0 +1,47 @@ +#undef F_CPU +#define F_CPU 16000000 + +#include + +#include "avr_mcu_section.h" +AVR_MCU(F_CPU, "atmega48"); + +#include "avr_hd44780.c" + +int main() +{ + hd44780_init(); + /* + * Clear the display. + */ + hd44780_outcmd(HD44780_CLR); + hd44780_wait_ready(1); // long wait + + /* + * Entry mode: auto-increment address counter, no display shift in + * effect. + */ + hd44780_outcmd(HD44780_ENTMODE(1, 0)); + hd44780_wait_ready(0); + + /* + * Enable display, activate non-blinking cursor. + */ + hd44780_outcmd(HD44780_DISPCTL(1, 1, 0)); + hd44780_wait_ready(0); + + uint16_t count = 0; + while (1) + { + uint16_t temp = count; + for (uint8_t i = 5; i > 0; i--) + { + hd44780_outcmd(HD44780_DDADDR(i - 1)); + hd44780_wait_ready(0); + hd44780_outdata(temp % 10 + 48); + temp /= 10; + hd44780_wait_ready(0); + } + count++; + } +} \ No newline at end of file diff --git a/examples/board_hd44780/charlcd.c b/examples/board_hd44780/charlcd.c index 4b69964c2..085e15382 100644 --- a/examples/board_hd44780/charlcd.c +++ b/examples/board_hd44780/charlcd.c @@ -146,11 +146,15 @@ main( char *argv[]) { elf_firmware_t f = {{0}}; - const char * fname = "atmega48_charlcd.axf"; + const char *fname = argc > 1 ? argv[1] : "atmega48_charlcd.axf"; // char path[256]; // sprintf(path, "%s/%s", dirname(argv[0]), fname); // printf("Firmware pathname is %s\n", path); - elf_read_firmware(fname, &f); + if (elf_read_firmware(fname, &f) == -1) + { + fprintf(stderr, "Unable to load firmware from file %s\n", fname); + exit(EXIT_FAILURE); + }; printf("firmware %s f=%d mmcu=%s\n", fname, (int) f.frequency, f.mmcu); From 68cf848e54d1d19bd04e5fdc8b03e527285f8683 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juh=C3=A1sz=20D=C3=A1niel?= Date: Thu, 6 Oct 2022 07:52:34 +0200 Subject: [PATCH 2/2] Add mutex to LCD display to prevent race condition --- examples/board_hd44780/charlcd.c | 2 + examples/parts/hd44780.c | 32 +++++++++------- examples/parts/hd44780.h | 65 ++++++++++++++++++++++++++++---- examples/parts/hd44780_glut.c | 48 +++++++++++++++++++++-- examples/parts/hd44780_glut.h | 8 ++++ 5 files changed, 130 insertions(+), 25 deletions(-) diff --git a/examples/board_hd44780/charlcd.c b/examples/board_hd44780/charlcd.c index 085e15382..aa7c56db0 100644 --- a/examples/board_hd44780/charlcd.c +++ b/examples/board_hd44780/charlcd.c @@ -172,6 +172,8 @@ main( hd44780_init(avr, &hd44780, 20, 4); + hd44780_setup_mutex_for_gl(&hd44780); + /* Connect Data Lines to Port B, 0-3 */ /* These are bidirectional too */ for (int i = 0; i < 4; i++) { diff --git a/examples/parts/hd44780.c b/examples/parts/hd44780.c index 7428be365..bcf11f418 100644 --- a/examples/parts/hd44780.c +++ b/examples/parts/hd44780.c @@ -76,7 +76,7 @@ _hd44780_busy_timer( { hd44780_t *b = (hd44780_t *) param; // printf("%s called\n", __FUNCTION__); - hd44780_set_flag(b, HD44780_FLAG_BUSY, 0); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_BUSY, 0); avr_raise_irq(b->irq + IRQ_HD44780_BUSY, 0); return 0; } @@ -189,7 +189,7 @@ hd44780_write_command( hd44780_set_flag(b, HD44780_FLAG_F, b->datapins & 4); if (!four && !hd44780_get_flag(b, HD44780_FLAG_D_L)) { printf("%s activating 4 bits mode\n", __FUNCTION__); - hd44780_set_flag(b, HD44780_FLAG_LOWNIBBLE, 0); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_LOWNIBBLE, 0); } } break; // Cursor display shift @@ -231,7 +231,7 @@ hd44780_process_write( { uint32_t delay = 0; // uS int four = !hd44780_get_flag(b, HD44780_FLAG_D_L); - int comp = four && hd44780_get_flag(b, HD44780_FLAG_LOWNIBBLE); + int comp = four && hd44780_get_private_flag(b, HD44780_PRIV_FLAG_LOWNIBBLE); int write = 0; if (four) { // 4 bits ! @@ -240,7 +240,7 @@ hd44780_process_write( else b->datapins = (b->datapins & 0xf) | ((b->pinstate >> (IRQ_HD44780_D4-4)) & 0xf0); write = comp; - b->flags ^= (1 << HD44780_FLAG_LOWNIBBLE); + b->private_flags ^= (1 << HD44780_PRIV_FLAG_LOWNIBBLE); } else { // 8 bits b->datapins = (b->pinstate >> IRQ_HD44780_D0) & 0xff; write++; @@ -249,13 +249,15 @@ hd44780_process_write( // write has 8 bits to process if (write) { - if (hd44780_get_flag(b, HD44780_FLAG_BUSY)) { + if (hd44780_get_private_flag(b, HD44780_PRIV_FLAG_BUSY)) { printf("%s command %02x write when still BUSY\n", __FUNCTION__, b->datapins); } + hd44780_lock_state(b); if (b->pinstate & (1 << IRQ_HD44780_RS)) // write data delay = hd44780_write_data(b); else // write command delay = hd44780_write_command(b); + hd44780_unlock_state(b); } return delay; } @@ -266,14 +268,14 @@ hd44780_process_read( { uint32_t delay = 0; // uS int four = !hd44780_get_flag(b, HD44780_FLAG_D_L); - int comp = four && hd44780_get_flag(b, HD44780_FLAG_LOWNIBBLE); + int comp = four && hd44780_get_private_flag(b, HD44780_PRIV_FLAG_LOWNIBBLE); int done = 0; // has something on the datapin we want if (comp) { // ready the 4 final bits on the 'actual' lcd pins b->readpins <<= 4; done++; - b->flags ^= (1 << HD44780_FLAG_LOWNIBBLE); + b->private_flags ^= (1 << HD44780_PRIV_FLAG_LOWNIBBLE); } if (!done) { // new read @@ -281,19 +283,21 @@ hd44780_process_read( if (b->pinstate & (1 << IRQ_HD44780_RS)) { // read data delay = 37; b->readpins = b->vram[b->cursor]; + hd44780_lock_state(b); hd44780_kick_cursor(b); + hd44780_unlock_state(b); } else { // read 'command' ie status register delay = 0; // no raising busy when reading busy ! // low bits are the current cursor b->readpins = b->cursor < 0x80 ? b->cursor : b->cursor-0x80; - int busy = hd44780_get_flag(b, HD44780_FLAG_BUSY); + int busy = hd44780_get_private_flag(b, HD44780_PRIV_FLAG_BUSY); b->readpins |= busy ? 0x80 : 0; // if (busy) printf("Good boy, guy's reading status byte\n"); // now that we're read the busy flag, clear it and clear // the timer too - hd44780_set_flag(b, HD44780_FLAG_BUSY, 0); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_BUSY, 0); avr_raise_irq(b->irq + IRQ_HD44780_BUSY, 0); avr_cycle_timer_cancel(b->avr, _hd44780_busy_timer, b); } @@ -301,7 +305,7 @@ hd44780_process_read( done++; if (four) - b->flags |= (1 << HD44780_FLAG_LOWNIBBLE); // for next read + b->private_flags |= (1 << HD44780_PRIV_FLAG_LOWNIBBLE); // for next read } // now send the prepared output pins to send as IRQs @@ -320,7 +324,7 @@ _hd44780_process_e_pinchange( { hd44780_t *b = (hd44780_t *) param; - hd44780_set_flag(b, HD44780_FLAG_REENTRANT, 1); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_REENTRANT, 1); #if 0 uint16_t touch = b->oldstate ^ b->pinstate; @@ -338,13 +342,13 @@ _hd44780_process_e_pinchange( delay = hd44780_process_write(b); if (delay) { - hd44780_set_flag(b, HD44780_FLAG_BUSY, 1); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_BUSY, 1); avr_raise_irq(b->irq + IRQ_HD44780_BUSY, 1); avr_cycle_timer_register_usec(b->avr, delay, _hd44780_busy_timer, b); } // b->oldstate = b->pinstate; - hd44780_set_flag(b, HD44780_FLAG_REENTRANT, 0); + hd44780_set_private_flag(b, HD44780_PRIV_FLAG_REENTRANT, 0); return 0; } @@ -373,7 +377,7 @@ hd44780_pin_changed_hook( return; // job already done! case IRQ_HD44780_D0 ... IRQ_HD44780_D7: // don't update these pins in read mode - if (hd44780_get_flag(b, HD44780_FLAG_REENTRANT)) + if (hd44780_get_private_flag(b, HD44780_PRIV_FLAG_REENTRANT)) return; break; } diff --git a/examples/parts/hd44780.h b/examples/parts/hd44780.h index c165314c9..730772ed2 100644 --- a/examples/parts/hd44780.h +++ b/examples/parts/hd44780.h @@ -79,17 +79,23 @@ enum { HD44780_FLAG_S, // 1: Follow display shift HD44780_FLAG_I_D, // 1: Increment, 0: Decrement - /* - * Internal flags, not HD44780 - */ - HD44780_FLAG_LOWNIBBLE, // 1: 4 bits mode, write/read low nibble - HD44780_FLAG_BUSY, // 1: Busy between instructions, 0: ready - HD44780_FLAG_REENTRANT, // 1: Do not update pins - + /* + * Internal flags, not HD44780 + */ HD44780_FLAG_DIRTY, // 1: needs redisplay... HD44780_FLAG_CRAM_DIRTY, // 1: Character memory has changed }; +/* + * Private internal flags. These are not protected by + * callbacks, so other threads must not access them. + */ +enum { + HD44780_PRIV_FLAG_BUSY = 0, // 1: Busy between instructions, 0: ready + HD44780_PRIV_FLAG_LOWNIBBLE, // 1: 4 bits mode, write/read low nibble + HD44780_PRIV_FLAG_REENTRANT, // 1: Do not update pins +}; + typedef struct hd44780_t { @@ -106,6 +112,21 @@ typedef struct hd44780_t uint8_t readpins; uint16_t flags; // LCD flags ( HD44780_FLAG_*) + // LCD private flags, not protected by callbacks ( HD44780_PRIV_FLAG_*) + // You must not use these flags from a different thread than the simavr one. + uint8_t private_flags; + + // These callbacks are called before and after a data or + // command packet is sent to the unit and as a + // result the internal state of the device changes. + // If you lock and unlock a mutex in these, you can guard + // the cursor, vram and flags variables with it. + // The avr thread reads these variables outside + // of these functions, but writes always happen between them. + void *on_state_lock_parameter; + void (*on_state_lock)(void *); + void *on_state_unlock_parameter; + void (*on_state_unlock)(void *); } hd44780_t; void @@ -134,4 +155,34 @@ hd44780_get_flag( return (b->flags & (1 << bit)) != 0; } +static inline int +hd44780_set_private_flag( + hd44780_t *b, uint16_t bit, int val) +{ + int old = b->private_flags & (1 << bit); + b->private_flags = (b->private_flags & ~(1 << bit)) | (val ? (1 << bit) : 0); + return old != 0; +} + +static inline int +hd44780_get_private_flag( + hd44780_t *b, uint16_t bit) +{ + return (b->private_flags & (1 << bit)) != 0; +} + +static inline void +hd44780_lock_state(hd44780_t *b) +{ + if (b->on_state_lock) + (*(b->on_state_lock))(b->on_state_lock_parameter); +} + +static inline void +hd44780_unlock_state(hd44780_t *b) +{ + if (b->on_state_unlock) + (*(b->on_state_unlock))(b->on_state_unlock_parameter); +} + #endif diff --git a/examples/parts/hd44780_glut.c b/examples/parts/hd44780_glut.c index 77c28bcbd..abec7e382 100644 --- a/examples/parts/hd44780_glut.c +++ b/examples/parts/hd44780_glut.c @@ -27,6 +27,8 @@ #else #include #endif +#include +#include #include "lcd_font.h" // generated with gimp @@ -34,6 +36,26 @@ static GLuint font_texture; static int charwidth = 5; static int charheight = 7; +static pthread_mutex_t hd44780_state_mutex = PTHREAD_MUTEX_INITIALIZER; + +void before_state_lock_cb(void *b) +{ + pthread_mutex_lock(&hd44780_state_mutex); +} + +void after_state_lock_cb(void *b) +{ + pthread_mutex_unlock(&hd44780_state_mutex); +} + +void hd44780_setup_mutex_for_gl(hd44780_t *b) +{ + b->on_state_lock = &before_state_lock_cb; + b->on_state_lock_parameter = b; + b->on_state_unlock = &after_state_lock_cb; + b->on_state_unlock_parameter = b; +} + void hd44780_gl_init() { @@ -123,6 +145,13 @@ hd44780_gl_draw( uint32_t text, uint32_t shadow) { + if (b->on_state_lock != &before_state_lock_cb || + b->on_state_unlock != &after_state_lock_cb) + { + printf("Error: the hd44780 instance is not using the mutex of the OpenGL thread!\nCall hd44780_setup_mutex_for_gl() first!\n"); + exit(EXIT_FAILURE); + } + int rows = b->w; int lines = b->h; int border = 3; @@ -139,16 +168,27 @@ hd44780_gl_draw( + (lines - 1) + border, 0); glEnd(); + // create a local copy (so we can release the mutex as fast as possible) + uint8_t vram[192]; + pthread_mutex_lock(&hd44780_state_mutex); + // cgram is not updated yet + // int cgram_dirty = hd44780_get_flag(b, HD44780_FLAG_CRAM_DIRTY); + for (uint8_t i = 0; i < 192; i++) + vram[i] = b->vram[i]; + // the values have been seen, they are not dirty anymore + hd44780_set_flag(b, HD44780_FLAG_CRAM_DIRTY, 0); + hd44780_set_flag(b, HD44780_FLAG_DIRTY, 0); + pthread_mutex_unlock(&hd44780_state_mutex); + glColor3f(1.0f, 1.0f, 1.0f); const uint8_t offset[] = { 0x00, 0x40, 0x00 + 20, 0x40 + 20 }; - for (int v = 0 ; v < b->h; v++) { + for (int v = 0 ; v < lines; v++) { glPushMatrix(); - for (int i = 0; i < b->w; i++) { - glputchar(b->vram[offset[v] + i], character, text, shadow); + for (int i = 0; i < rows; i++) { + glputchar(vram[offset[v] + i], character, text, shadow); glTranslatef(6, 0, 0); } glPopMatrix(); glTranslatef(0, 8, 0); } - hd44780_set_flag(b, HD44780_FLAG_DIRTY, 0); } diff --git a/examples/parts/hd44780_glut.h b/examples/parts/hd44780_glut.h index 15b87171d..e59cd309e 100644 --- a/examples/parts/hd44780_glut.h +++ b/examples/parts/hd44780_glut.h @@ -24,6 +24,14 @@ #include "hd44780.h" +// This sets the change callbacks of the hd44780 to +// lock and unlock the mutex of the internal display. +void +hd44780_setup_mutex_for_gl(hd44780_t *b); + +// Draws the contents of the LCD display. +// You must call hd44780_gl_init() and +// hd44780_setup_mutex_for_gl() first. void hd44780_gl_draw( hd44780_t *b,