On Wed, Mar 30, 2022 at 9:09 PM Chen-Yu Tsai wens@kernel.org wrote:
From: Chen-Yu Tsai wens@csie.org
On the SINO WEALTH SH1106, which is mostly compatible with the SSD1306, only the basic page addressing mode is supported. This addressing mode is not as easy to use compared to the currently supported horizontal addressing mode, as the page address has to be set prior to writing out each page, and each page must be written out separately as a result. Also, there is no way to force the column address to wrap around early, thus the column address must also be reset for each page to be accurate.
Thanks for including this explanation, it's very informative.
Add support for this addressing mode, with a flag to choose it. This flag is designed to be set from the device info data structure, but can be extended to be explicitly forced on through a device tree property if such a need arises.
Agreed. Unless an OLED controller supports both page addressing modes, I don't see what could be an advantage of having that property in the device tree. And even if that's the case, we could just always make it default to use horizontal addressing mode.
[snip]
+/* Set address range for horizontal/vertical addressing modes */
Thanks for adding these comments.
static int ssd130x_set_col_range(struct ssd130x_device *ssd130x, u8 col_start, u8 cols) { @@ -166,6 +173,26 @@ static int ssd130x_set_page_range(struct ssd130x_device *ssd130x, return 0; }
+/* Set page and column start address for page addressing mode */ +static int ssd130x_set_page_pos(struct ssd130x_device *ssd130x,
u8 page_start, u8 col_start)+{
int ret;u32 page, col_low, col_high;page = SSD130X_START_PAGE_ADDRESS |SSD130X_START_PAGE_ADDRESS_SET(page_start);col_low = SSD130X_PAGE_COL_START_LOW |SSD130X_PAGE_COL_START_SET(col_start & 0xf);col_high = SSD130X_PAGE_COL_START_HIGH |SSD130X_PAGE_COL_START_SET((col_start >> 4) & 0xf);
Maybe instead we should define SSD130X_PAGE_COL_START_{HIGH,LOW}_{MASK,SET} to be consistent with how the other fields are set and not using bitwise operations explicitly here ?
Other than that, the patch looks good to me.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
Best regards, Javier