From 665eb2bda7d03d3f2ad4d475348db8c8fd6bba0b Mon Sep 17 00:00:00 2001 From: Jon Waterschoot Date: Tue, 4 Aug 2026 02:18:38 +0200 Subject: [PATCH 1/3] Fix SSD1306 128x32: wrong column-start address (0x12 -> 0x10) Fixes #634. SSD130xDriver::Update() (and the equivalent switches in SSD1307Driver) set the page column-start command to 0x12 (column 32) for height==32 panels instead of the standard 0x10 (column 0) used by the default case and every reference SSD1306 driver. Every page write started 32 columns in: the intended content shifted 32 columns right, and the last 32 columns wrapped back around to the left edge, garbling whatever was drawn there. Confirmed independently by three reports on #634 (including a maintainer) against SSD130x4WireSpi128x32Driver; the same class also backs the I2C 128x32 alias. Also fixes the same line in SSD1307Driver::TransferPageDma() for consistency, though no public alias currently instantiates that class at height==32. --- src/dev/oled_ssd130x.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dev/oled_ssd130x.h b/src/dev/oled_ssd130x.h index b9c25d068..d9580ffbe 100644 --- a/src/dev/oled_ssd130x.h +++ b/src/dev/oled_ssd130x.h @@ -385,7 +385,7 @@ class SSD130xDriver uint8_t high_column_addr; switch(height) { - case 32: high_column_addr = 0x12; break; + case 32: high_column_addr = 0x10; break; default: high_column_addr = 0x10; break; } @@ -597,7 +597,7 @@ class SSD1307Driver uint8_t high_column_addr; switch(height) { - case 32: high_column_addr = 0x12; break; + case 32: high_column_addr = 0x10; break; default: high_column_addr = 0x10; break; } @@ -632,7 +632,7 @@ class SSD1307Driver uint8_t high_column_addr; switch(height) { - case 32: high_column_addr = 0x12; break; + case 32: high_column_addr = 0x10; break; // no public alias uses height 32 here yet default: high_column_addr = 0x10; break; } From 03c2061293cce9c69e569a0077f9496142f306d3 Mon Sep 17 00:00:00 2001 From: Jon Waterschoot Date: Tue, 4 Aug 2026 02:18:54 +0200 Subject: [PATCH 2/3] Batch SSD1306 I2C page writes into one transaction SSD130xI2CTransport::SendData() issued a full I2C transaction (START+address+ACK+STOP) per byte -- 512 transactions for one full 128x32 frame (4 pages x 128 bytes), plus ~12 more for per-page setup commands. The SSD1306 auto-increments its column pointer for every data byte that follows a single 0x40 prefix within a transaction, so there's no need to restart the bus per byte. Batches a page's data into one transaction (page bytes + the 0x40 prefix); measured on a 128x32 panel, this cut a full-screen Update() from ~30-40ms to a few ms. Separate from the column-start fix in the previous commit -- this is a transport-layer performance change, not a correctness fix, so it's easy to drop if unwanted. --- src/dev/oled_ssd130x.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/dev/oled_ssd130x.h b/src/dev/oled_ssd130x.h index d9580ffbe..9bf5922c0 100644 --- a/src/dev/oled_ssd130x.h +++ b/src/dev/oled_ssd130x.h @@ -48,6 +48,19 @@ class SSD130xI2CTransport void SendData(uint8_t* buff, size_t size) { + // SSD1306 auto-increments its column pointer after one 0x40 + // prefix, so a page can go out in a single transaction. + constexpr size_t kMaxBurst = 132; // widest driver width used here (128) + margin + if(size <= kMaxBurst - 1) + { + uint8_t buf[kMaxBurst]; + buf[0] = 0X40; + for(size_t i = 0; i < size; i++) + buf[1 + i] = buff[i]; + i2c_.TransmitBlocking( + i2c_address_, buf, static_cast(size + 1), 1000); + return; + } for(size_t i = 0; i < size; i++) { uint8_t buf[2] = {0X40, buff[i]}; From 3d4803e6d3e52f56d188ee618ba230f1520502d2 Mon Sep 17 00:00:00 2001 From: Jon Waterschoot Date: Sat, 8 Aug 2026 17:17:00 +0200 Subject: [PATCH 3/3] Make the SSD1306 column-start address configurable Replaces the flat 0x10 from the previous commit, per review feedback: key the offset on width as well as height, and expose it so panels that map differently can be handled without editing the library. The 0x12 came in with #326, whose author states the motivation was the kxmx_bluemchen's 64x32 I2C SSD1306. That is correct for that panel -- 64-column modules are wired to the middle 64 columns of the controller's 128-column RAM, so they need a 32-column offset (0x10 | 32 >> 4 == 0x12). The bug is only that the switch keys on height alone, so 128x32 panels got the offset too. Init() already distinguishes the two inside case 32: via `if(width == 64)` for the COM-pins command; the column-start line never got the same treatment. So the default is now geometry-derived -- 0x12 only for 64x32, 0x10 otherwise -- and lives in Config, overridable per instance. Config stays an aggregate under gnu++14 and every in-tree call site default-constructs then assigns, so no existing code changes. Applied to SSD1307Driver's Update() and TransferPageDma() too, keeping both drivers configured the same way, though no SSD1307 alias is 64x32 today. Tested on a 128x32 I2C panel: the default renders correctly, and forcing high_column_addr = 0x12 reproduces the 32-column shift and wrap from #634 exactly -- which is also what confirms the override reaches the panel. No 64x32 hardware here to verify bluemchen directly, which is why the default is derived from geometry rather than flattened to 0x10. Co-Authored-By: Claude Opus 5 --- src/dev/oled_ssd130x.h | 48 ++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/dev/oled_ssd130x.h b/src/dev/oled_ssd130x.h index 9bf5922c0..069d5d721 100644 --- a/src/dev/oled_ssd130x.h +++ b/src/dev/oled_ssd130x.h @@ -270,11 +270,21 @@ class SSD130xDriver struct Config { typename Transport::Config transport_config; + + /** Higher column start address sent before each page write. + * 64x32 panels are wired to the middle 64 columns of the + * controller's 128-column RAM and so need a 32-column offset + * (0x10 | 32 >> 4 == 0x12); every other geometry starts at + * column 0 (0x10). Override for panels that map differently: + * the offset is (controller RAM columns - panel width) / 2, + * where the SSD1306/SSD1309 have 128 columns and the SH1106 132. */ + uint8_t high_column_addr = (width == 64 && height == 32) ? 0x12 : 0x10; }; void Init(Config config) { transport_.Init(config.transport_config); + high_column_addr_ = config.high_column_addr; // Init routine... @@ -395,18 +405,11 @@ class SSD130xDriver void Update() { uint8_t i; - uint8_t high_column_addr; - switch(height) - { - case 32: high_column_addr = 0x10; break; - - default: high_column_addr = 0x10; break; - } for(i = 0; i < (height / 8); i++) { transport_.SendCommand(0xB0 + i); transport_.SendCommand(0x00); - transport_.SendCommand(high_column_addr); + transport_.SendCommand(high_column_addr_); transport_.SendData(&buffer_[width * i], width); } }; @@ -419,6 +422,7 @@ class SSD130xDriver protected: Transport transport_; uint8_t buffer_[width * height / 8]; + uint8_t high_column_addr_; }; /** @@ -495,13 +499,20 @@ class SSD1307Driver struct Config { typename Transport::Config transport_config; + + /** Higher column start address sent before each page write. + * See SSD130xDriver::Config::high_column_addr — no SSD1307 alias + * is 64x32 today, but the member is kept in step so both drivers + * are configured the same way. */ + uint8_t high_column_addr = (width == 64 && height == 32) ? 0x12 : 0x10; }; void Init(Config config) { transport_.Init(config.transport_config); - useDma_ = config.transport_config.useDma; + high_column_addr_ = config.high_column_addr; + useDma_ = config.transport_config.useDma; // Init routine... uint8_t uDispayOffset; @@ -607,18 +618,11 @@ class SSD1307Driver else { uint8_t i; - uint8_t high_column_addr; - switch(height) - { - case 32: high_column_addr = 0x10; break; - - default: high_column_addr = 0x10; break; - } for(i = 0; i < (height / 8); i++) { transport_.SendCommand(0xB0 + i); transport_.SendCommand(0x00); - transport_.SendCommand(high_column_addr); + transport_.SendCommand(high_column_addr_); transport_.SendData(&buffer_[width * i], width); } updateing_ = false; @@ -637,21 +641,15 @@ class SSD1307Driver uint8_t transferPagesCount_; uint8_t transferingPage_; bool useDma_; + uint8_t high_column_addr_; void TransferPageDma(uint8_t page) { transferingPage_ = page; - uint8_t high_column_addr; - switch(height) - { - case 32: high_column_addr = 0x10; break; // no public alias uses height 32 here yet - - default: high_column_addr = 0x10; break; - } uint8_t commands[] = {static_cast(0xB0 + transferingPage_), 0x00, - high_column_addr}; + high_column_addr_}; transport_.SendCommands(commands, 3); // transport_.SendCommand(0xB0 + transferingPage_); // transport_.SendCommand(0x00);