From 33c2bcbe52eafa11cafcd92849a16f3d25bd43f2 Mon Sep 17 00:00:00 2001 From: cepetr Date: Mon, 24 Jun 2024 17:00:51 +0200 Subject: [PATCH] refactor(core/embed): simplify ensure_compatible_settings [no changelog] --- core/embed/boardloader/main.c | 3 +++ core/embed/bootloader/emulator.c | 2 -- core/embed/bootloader/emulator.h | 4 +--- core/embed/bootloader/main.c | 3 +++ core/embed/models/T2T1/boards/trezor_t.h | 7 +++++++ core/embed/models/T2T1/compat_settings.c | 4 ++++ core/embed/trezorhal/display.h | 6 ++++++ core/embed/trezorhal/stm32f4/common.c | 16 ---------------- core/embed/trezorhal/stm32f4/displays/st7789v.c | 8 +++++++- core/embed/trezorhal/stm32f4/platform.h | 1 - core/embed/trezorhal/stm32f4/supervise.c | 4 +++- core/embed/trezorhal/stm32f4/supervise.h | 1 - .../stm32f4/xdisplay/st-7789/display_driver.c | 16 ++++++++++++++-- .../xdisplay/stm32f429i-disc1/display_driver.c | 2 -- .../stm32f4/xdisplay/ug-2828/display_driver.c | 2 -- .../stm32f4/xdisplay/vg-2864/display_driver.c | 2 -- core/embed/trezorhal/stm32u5/common.c | 5 ----- core/embed/trezorhal/stm32u5/platform.h | 1 - core/embed/trezorhal/unix/display-unix.c | 14 +++++++------- core/embed/trezorhal/unix/display_driver.c | 8 ++------ core/embed/trezorhal/xdisplay.h | 5 ----- core/site_scons/models/T2T1/trezor_t.py | 1 + 22 files changed, 58 insertions(+), 57 deletions(-) create mode 100644 core/embed/models/T2T1/compat_settings.c diff --git a/core/embed/boardloader/main.c b/core/embed/boardloader/main.c index 9c07c2c2f..b9f5cbb8d 100644 --- a/core/embed/boardloader/main.c +++ b/core/embed/boardloader/main.c @@ -326,7 +326,10 @@ int main(void) { write_bootloader_min_version(hdr->monotonic); display_deinit(DISPLAY_RETAIN_CONTENT); + +#ifdef ENSURE_COMPATIBLE_SETTINGS ensure_compatible_settings(); +#endif mpu_config_off(); diff --git a/core/embed/bootloader/emulator.c b/core/embed/bootloader/emulator.c index 473357a22..d69d56dd7 100644 --- a/core/embed/bootloader/emulator.c +++ b/core/embed/bootloader/emulator.c @@ -188,5 +188,3 @@ __attribute__((noreturn)) void jump_to(uint32_t address) { "STORAGE WAS RETAINED"); } } - -void ensure_compatible_settings(void) {} diff --git a/core/embed/bootloader/emulator.h b/core/embed/bootloader/emulator.h index 9facab471..f9ab58037 100644 --- a/core/embed/bootloader/emulator.h +++ b/core/embed/bootloader/emulator.h @@ -15,8 +15,6 @@ void emulator_poll_events(void); void set_core_clock(int); void mpu_config_bootloader(void); void mpu_config_off(void); -void display_set_little_endian(void); -void jump_to(uint32_t address); -void ensure_compatible_settings(void); +void jump_to(void *addr); #endif diff --git a/core/embed/bootloader/main.c b/core/embed/bootloader/main.c index 4df0c964e..1a9febca3 100644 --- a/core/embed/bootloader/main.c +++ b/core/embed/bootloader/main.c @@ -331,7 +331,10 @@ void real_jump_to_firmware(void) { } display_deinit(DISPLAY_RETAIN_CONTENT); + +#ifdef ENSURE_COMPATIBLE_SETTINGS ensure_compatible_settings(); +#endif mpu_config_off(); jump_to(IMAGE_CODE_ALIGN(FIRMWARE_START + vhdr.hdrlen + IMAGE_HEADER_SIZE)); diff --git a/core/embed/models/T2T1/boards/trezor_t.h b/core/embed/models/T2T1/boards/trezor_t.h index 14e2ffd3f..25a8711b7 100644 --- a/core/embed/models/T2T1/boards/trezor_t.h +++ b/core/embed/models/T2T1/boards/trezor_t.h @@ -58,4 +58,11 @@ #define SD_ENABLE_PORT GPIOC #define SD_ENABLE_PIN GPIO_PIN_0 +// Ensure compatible hardware settings before jumping to +// the different booting stage. This function is used to +// ensure backward compatibility with older versions of +// released bootloaders and firmware. +#define ENSURE_COMPATIBLE_SETTINGS +extern void ensure_compatible_settings(void); + #endif //_TREZOR_T_H diff --git a/core/embed/models/T2T1/compat_settings.c b/core/embed/models/T2T1/compat_settings.c new file mode 100644 index 000000000..a737b3c37 --- /dev/null +++ b/core/embed/models/T2T1/compat_settings.c @@ -0,0 +1,4 @@ + +#include "platform.h" + +void ensure_compatible_settings(void) { set_core_clock(CLOCK_168_MHZ); } diff --git a/core/embed/trezorhal/display.h b/core/embed/trezorhal/display.h index 38091eec6..34def49aa 100644 --- a/core/embed/trezorhal/display.h +++ b/core/embed/trezorhal/display.h @@ -93,6 +93,12 @@ static inline void display_init(display_content_mode_t mode) { } static inline void display_deinit(display_content_mode_t mode) { + +#ifdef TREZOR_MODEL_T + if (mode == DISPLAY_RESET_CONTENT) { + display_orientation(0); + } +#endif display_finish_actions(); } diff --git a/core/embed/trezorhal/stm32f4/common.c b/core/embed/trezorhal/stm32f4/common.c index dafde04c1..8ebab7425 100644 --- a/core/embed/trezorhal/stm32f4/common.c +++ b/core/embed/trezorhal/stm32f4/common.c @@ -117,22 +117,6 @@ void collect_hw_entropy(void) { NULL); } -// this function resets settings changed in one layer (bootloader/firmware), -// which might be incompatible with the other layers older versions, -// where this setting might be unknown -void ensure_compatible_settings(void) { -#ifdef TREZOR_MODEL_T -#ifdef NEW_RENDERING - display_set_compatible_settings(); -#else - display_set_big_endian(); -#endif - display_orientation(0); - set_core_clock(CLOCK_168_MHZ); - backlight_pwm_deinit(BACKLIGHT_RETAIN); -#endif -} - void invalidate_firmware(void) { // erase start of the firmware (metadata) -> invalidate FW ensure(flash_unlock_write(), NULL); diff --git a/core/embed/trezorhal/stm32f4/displays/st7789v.c b/core/embed/trezorhal/stm32f4/displays/st7789v.c index 0ece17280..68fc9ebaf 100644 --- a/core/embed/trezorhal/stm32f4/displays/st7789v.c +++ b/core/embed/trezorhal/stm32f4/displays/st7789v.c @@ -723,6 +723,7 @@ void display_finish_actions(void) { #ifndef BOARDLOADER bg_copy_wait(); #endif + backlight_pwm_deinit(BACKLIGHT_RETAIN); } #else // NOT FRAMEBUFFER @@ -755,6 +756,11 @@ uint16_t display_get_window_offset(void) { return 0; } void display_shift_window(uint16_t pixels) {} -void display_finish_actions(void) {} +void display_finish_actions(void) { + backlight_pwm_deinit(BACKLIGHT_RETAIN); +#ifdef TREZOR_MODEL_T + display_set_big_endian(); +#endif +} #endif diff --git a/core/embed/trezorhal/stm32f4/platform.h b/core/embed/trezorhal/stm32f4/platform.h index 710712761..00a2ce256 100644 --- a/core/embed/trezorhal/stm32f4/platform.h +++ b/core/embed/trezorhal/stm32f4/platform.h @@ -35,7 +35,6 @@ void set_core_clock(clock_settings_t settings); void memset_reg(volatile void *start, volatile void *stop, uint32_t val); void jump_to(uint32_t address); void jump_to_with_flag(uint32_t address, uint32_t register_flag); -void ensure_compatible_settings(void); void clear_otg_hs_memory(void); void drop_privileges(void); diff --git a/core/embed/trezorhal/stm32f4/supervise.c b/core/embed/trezorhal/stm32f4/supervise.c index 83191ed08..bcb6e6767 100644 --- a/core/embed/trezorhal/stm32f4/supervise.c +++ b/core/embed/trezorhal/stm32f4/supervise.c @@ -31,8 +31,10 @@ __attribute__((noreturn)) static void _reboot_to_bootloader( #else __attribute__((noreturn)) static void _reboot_to_bootloader( boot_command_t boot_command) { - display_deinit(DISPLAY_RETAIN_CONTENT); + display_deinit(DISPLAY_RESET_CONTENT); +#ifdef ENSURE_COMPATIBLE_SETTINGS ensure_compatible_settings(); +#endif mpu_config_bootloader(); jump_to_with_flag(IMAGE_CODE_ALIGN(BOOTLOADER_START + IMAGE_HEADER_SIZE), boot_command); diff --git a/core/embed/trezorhal/stm32f4/supervise.h b/core/embed/trezorhal/stm32f4/supervise.h index 664939426..3d478e882 100644 --- a/core/embed/trezorhal/stm32f4/supervise.h +++ b/core/embed/trezorhal/stm32f4/supervise.h @@ -18,7 +18,6 @@ extern uint32_t systick_val_copy; // from util.s extern void shutdown_privileged(void); -extern void ensure_compatible_settings(void); // Initializes the supervise module // diff --git a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c index b49fcc0be..5395f44f5 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c @@ -72,9 +72,23 @@ void display_init(display_content_mode_t mode) { void display_deinit(display_content_mode_t mode) { #ifdef XFRAMEBUFFER #ifndef BOARDLOADER + // Ensure that the ready frame buffer is transfered to + // the display controller display_ensure_refreshed(); + // Disable periodical interrupt svc_disableIRQ(DISPLAY_TE_INTERRUPT_NUM); #endif +#endif + + backlight_pwm_deinit(mode == DISPLAY_RESET_CONTENT ? BACKLIGHT_RESET : BACKLIGHT_RETAIN); + +#ifdef TREZOR_MODEL_T + // This ensures backward compatibility with legacy bootloader/firmware + // that relies on this hardware settings from the previous boot stage + if (mode == DISPLAY_RESET_CONTENT) { + display_set_orientation(0); + } + display_panel_set_big_endian(); #endif } @@ -123,5 +137,3 @@ int display_get_orientation(void) { return drv->orientation_angle; } - -void display_set_compatible_settings(void) { display_panel_set_big_endian(); } diff --git a/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c index 27fa3c130..28d576336 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c @@ -107,8 +107,6 @@ void display_refresh(void) { // Do nothing as using just a single frame buffer } -void display_set_compatible_settings() {} - void display_fill(const gfx_bitblt_t *bb) { display_driver_t *drv = &g_display_driver; diff --git a/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c index 9de46d12c..b249a58fa 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c @@ -370,8 +370,6 @@ display_fb_info_t display_get_frame_buffer(void) { void display_refresh(void) { display_sync_with_fb(); } -void display_set_compatible_settings() {} - void display_fill(const gfx_bitblt_t *bb) { display_driver_t *drv = &g_display_driver; diff --git a/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c index a3c4a8196..d922e4462 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c @@ -351,8 +351,6 @@ void display_refresh(void) { display_sync_with_fb(drv); } -void display_set_compatible_settings() {} - void display_fill(const gfx_bitblt_t *bb) { display_driver_t *drv = &g_display_driver; diff --git a/core/embed/trezorhal/stm32u5/common.c b/core/embed/trezorhal/stm32u5/common.c index 677678780..a10a6ccd0 100644 --- a/core/embed/trezorhal/stm32u5/common.c +++ b/core/embed/trezorhal/stm32u5/common.c @@ -101,11 +101,6 @@ void collect_hw_entropy(void) { NULL); } -// this function resets settings changed in one layer (bootloader/firmware), -// which might be incompatible with the other layers older versions, -// where this setting might be unknown -void ensure_compatible_settings(void) {} - void invalidate_firmware(void) { // on stm32u5, we need to disable the instruction cache before erasing the // firmware - otherwise, the write check will fail diff --git a/core/embed/trezorhal/stm32u5/platform.h b/core/embed/trezorhal/stm32u5/platform.h index 6a84e773e..fce29fb6b 100644 --- a/core/embed/trezorhal/stm32u5/platform.h +++ b/core/embed/trezorhal/stm32u5/platform.h @@ -34,7 +34,6 @@ typedef enum { } clock_settings_t; void set_core_clock(clock_settings_t settings); -void ensure_compatible_settings(void); void drop_privileges(void); // the following functions are defined in util.s diff --git a/core/embed/trezorhal/unix/display-unix.c b/core/embed/trezorhal/unix/display-unix.c index 60bbf1acc..07a45c822 100644 --- a/core/embed/trezorhal/unix/display-unix.c +++ b/core/embed/trezorhal/unix/display-unix.c @@ -76,7 +76,7 @@ void display_pixeldata(pixel_color c) { c = (c & 0x8410) ? 0xFFFF : 0x0000; #endif if (!RENDERER) { - display_init(); + display_init_all(); } if (PIXELWINDOW.pos.x <= PIXELWINDOW.end.x && PIXELWINDOW.pos.y <= PIXELWINDOW.end.y) { @@ -97,7 +97,7 @@ void display_reset_state() {} void display_init_seq(void) {} -void display_deinit(void) { +void display_exit_handler(void) { SDL_FreeSurface(PREV_SAVED); SDL_FreeSurface(BUFFER); if (BACKGROUND != NULL) { @@ -115,12 +115,12 @@ void display_deinit(void) { SDL_Quit(); } -void display_init(void) { +void display_init_all(void) { if (SDL_Init(SDL_INIT_VIDEO) != 0) { printf("%s\n", SDL_GetError()); error_shutdown("SDL_Init error"); } - atexit(display_deinit); + atexit(display_exit_handler); char *window_title = NULL; char *window_title_alloc = NULL; @@ -197,7 +197,7 @@ void display_init(void) { void display_set_window(uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) { if (!RENDERER) { - display_init(); + display_init_all(); } PIXELWINDOW.start.x = x0; PIXELWINDOW.start.y = y0; @@ -211,7 +211,7 @@ void display_sync(void) {} void display_refresh(void) { if (!RENDERER) { - display_init(); + display_init_all(); } if (BACKGROUND) { const SDL_Rect r = {0, 0, WINDOW_WIDTH, WINDOW_HEIGHT}; @@ -267,7 +267,7 @@ int display_backlight(int val) { const char *display_save(const char *prefix) { if (!RENDERER) { - display_init(); + display_init_all(); } static int count; static char filename[256]; diff --git a/core/embed/trezorhal/unix/display_driver.c b/core/embed/trezorhal/unix/display_driver.c index 6a433c2e3..bcaada40f 100644 --- a/core/embed/trezorhal/unix/display_driver.c +++ b/core/embed/trezorhal/unix/display_driver.c @@ -254,7 +254,7 @@ void display_refresh(void) { display_driver_t *drv = &g_display_driver; if (!drv->renderer) { - display_init(); + return; } #ifdef DISPLAY_MONO @@ -287,10 +287,6 @@ void display_refresh(void) { SDL_RenderPresent(drv->renderer); } -void display_set_compatible_settings(void) { - // not used -} - #ifndef DISPLAY_MONO void display_fill(const gfx_bitblt_t *bb) { @@ -365,7 +361,7 @@ const char *display_save(const char *prefix) { display_driver_t *drv = &g_display_driver; if (!drv->renderer) { - display_init(); + return NULL; } #ifdef DISPLAY_MONO diff --git a/core/embed/trezorhal/xdisplay.h b/core/embed/trezorhal/xdisplay.h index 8e856c7fd..28f09485d 100644 --- a/core/embed/trezorhal/xdisplay.h +++ b/core/embed/trezorhal/xdisplay.h @@ -127,11 +127,6 @@ void display_wait_for_sync(void); // swaps the active (currently displayed) and the inactive frame buffers. void display_refresh(void); -// Sets display to the mode compatible with the legacy bootloader code. -// -// This is used when switching between the firmware and the bootloader. -void display_set_compatible_settings(void); - // Following functions define display's bitblt interface. // // These functions draw directly to to display or to the diff --git a/core/site_scons/models/T2T1/trezor_t.py b/core/site_scons/models/T2T1/trezor_t.py index 237db9aa9..de100f8c1 100644 --- a/core/site_scons/models/T2T1/trezor_t.py +++ b/core/site_scons/models/T2T1/trezor_t.py @@ -40,6 +40,7 @@ def configure( defines += [f"HW_REVISION={hw_revision}"] sources += [ "embed/models/T2T1/model_T2T1_layout.c", + "embed/models/T2T1/compat_settings.c", ] if "new_rendering" in features_wanted: sources += ["embed/trezorhal/xdisplay_legacy.c"]