Add support for SSD1322 OLED display driver #68608

hlukasz · 2024-02-06T09:56:23Z

Initial support for SSD1322 OLED display driver. Only 1 bit color mode is supported. It it inspired by existing driver for SSD1306. Tested on 3.2 inch 256x64 module (https://www.buydisplay.com/white-3-2-inch-arduino-raspberry-pi-oled-display-module-256x64-spi)

danieldegrasse · 2024-02-13T21:53:30Z

drivers/display/ssd1322.c

+	uint16_t column_offset;
+};
+
+static inline int ssd1322_write_bus(const struct device *dev, uint8_t *buf, size_t len, bool cmd)


This display looks like it could use the new MIPI DBI API. Shouldn't be too hard to transition (mostly just need to update this function and the display_write implementation)

Can you investigate if this is possible?

Yes, it is possible. Thank you for pointing this out, I didn't notice that there is a dedicated interface for that. I have switched to MIPI-DBI in current version.

drivers/display/ssd1322.c

danieldegrasse · 2024-02-13T21:57:30Z

drivers/display/ssd1322.c

+	return 0;
+}
+
+static int ssd1322_read(const struct device *dev, const uint16_t x,


These stub functions can be dropped, see 0096a98

danieldegrasse · 2024-02-13T21:59:38Z

drivers/display/ssd1322.c

+	if (gpio_pin_set_dt(&config->reset, 1) < 0) {
+		return -1;
+	}
+	k_sleep(K_MSEC(10));


Datasheet claims the delays needed here are 100us each, any reason for the long waits?

No reason, that values was copied from some sample driver I have found. Now they are reduced to values from the datasheet.

danieldegrasse · 2024-02-13T22:00:58Z

drivers/display/ssd1322.c

+{
+	const struct ssd1322_config *config = dev->config;
+
+	if (gpio_pin_set_dt(&config->reset, 1) < 0) {


Please don't return an arbitrary value here. Propigate the return value from the GPIO API, like so:

ret = gpio_pin_set_dt; if (ret < 0) { return ret; }

Done. Changed also in few other places.

danieldegrasse · 2024-02-13T22:02:23Z

drivers/display/ssd1322.c

+	uint8_t cmd, data[2];
+
+	cmd = SSD1322_DISPLAY_OFF;
+	ret |= ssd1322_write_bus(dev, &cmd, sizeof(cmd), true);


I get the idea here, but I think we should be checking ret after each call, and returning if there is an error. This implementation will be more challenging to debug, since you won't know where something failed without stepping through with a debugger and watching the value of ret

danieldegrasse · 2024-02-22T17:30:48Z

drivers/display/ssd1322.c

+	const struct ssd1322_config *config = dev->config;
+	struct display_buffer_descriptor mipi_desc;
+
+	mipi_desc.buf_size = len;


You should propagate the height and width for the display buffer descriptor here as well- some controllers depend on these values being set to determine how much data to send.

Because of the 1 bit -> 4bit conversion and sending data in chunks I cannot propagate width and height that I am receiving from upper layer. I hope that current solution will be acceptable :)

I think this should be ok. Any MIPI controller that relies on the height and width will likely not work with this solution, but truthfully given that we are also lying to the controller about the true format of the pixel data (IE that each pixel is a nibble, not a bit), controllers of this style probably won't work either way.

danieldegrasse · 2024-03-21T14:55:11Z

drivers/display/ssd1322.c

+	size_t buf_len;
+	int ret;
+
+	if (desc->pitch < desc->width) {


We should add a check to verify the buf_size field in the display_buffer_descriptor is sufficient for the provided height and width- this will provide more robust defense against reading out of bounds in the buffer the application provides

Please look few lines below:

buf_len = MIN(desc->buf_size, desc->height * desc->width / 8);

I think that this should protect us from out of bounds access to the buffer, what do you think? Reading is limited either to buf_size, or to height and width.

Yeah, that will be fine- sorry for missing that check.

danieldegrasse · 2024-03-21T15:00:44Z

dts/bindings/display/solomon,ssd1322.yaml

+properties:
+  column_offset:
+    type: int
+    description: First visible column number, default 0.


If there is a default value, should we add default key here so that the user does not have to specify column_offset = <0>?

Also:

Please add a justification for the default. Can be as simple as "reset setting for the controller" if this is the case

property names should be styled like "column-offset"

I have rethought that and providing a default value here maybe is not the best option. I guess that this value is different for each display model, so there is not a default one. I have change this property to be required.

Initial support for SSD1322 OLED display driver. Only 1 bit color mode is supported. Signed-off-by: Lukasz Hawrylko <lukasz@hawrylko.pl>

danieldegrasse · 2024-04-01T15:39:44Z

drivers/display/ssd1322.c

+	size_t buf_len;
+	int ret;
+
+	if (desc->pitch < desc->width) {


Yeah, that will be fine- sorry for missing that check.

github-actions · 2024-06-01T00:30:03Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

danieldegrasse · 2024-06-03T15:05:06Z

@jfischer-no could you provide feedback on this PR to move it forwards?

kartben · 2024-08-10T12:31:23Z

ping @jfischer-no

jfischer-no · 2024-08-16T09:58:19Z

drivers/display/ssd1322_regs.h

+#define SSD1322_SET_COLUMN_ADDR      0x15
+#define SSD1322_ENABLE_RAM_WRITE     0x5C
+#define SSD1322_SET_ROW_ADDR         0x75
+#define SSD1322_SET_REMAP            0xA0
+#define SSD1322_BLANKING_ON          0xA4
+#define SSD1322_BLANKING_OFF         0xA6
+#define SSD1322_EXIT_PARTIAL         0xA9
+#define SSD1322_DISPLAY_OFF          0xAE
+#define SSD1322_DISPLAY_ON           0xAF
+#define SSD1322_SET_PHASE_LENGTH     0xB1
+#define SSD1322_SET_CLOCK_DIV        0xB3
+#define SSD1322_SET_GPIO             0xB5
+#define SSD1322_SET_SECOND_PRECHARGE 0xB6
+#define SSD1322_DEFAULT_GREYSCALE    0xB9
+#define SSD1322_SET_PRECHARGE        0xBB
+#define SSD1322_SET_VCOMH            0xBE
+#define SSD1322_SET_CONTRAST         0xC1
+#define SSD1322_SET_MUX_RATIO        0xCA
+#define SSD1322_COMMAND_LOCK         0xFD


19 lines could be moved to ssd1322.c

jfischer-no · 2024-08-16T10:01:19Z

drivers/display/ssd1322.c

+
+	if (desc->pitch < desc->width) {
+		LOG_ERR("Pitch is smaller than width");
+		return -1;


return -EINVAL; }

?

jfischer-no · 2024-08-16T10:02:55Z

drivers/display/ssd1322.c

+	LOG_DBG("x %u, y %u, pitch %u, width %u, height %u, buf_len %u", x, y, desc->pitch,
+			desc->width, desc->height, buf_len);
+
+	uint8_t cmd_data[2];


Variable definition should be at the top of a block.

jfischer-no · 2024-08-16T10:04:24Z

drivers/display/ssd1322.c

+	uint8_t cmd_data[2];
+
+	cmd_data[0] = config->column_offset + (x >> 2);
+	cmd_data[1] = config->column_offset + (((x + desc->width) >> 2) - 1);


cmd_data[1] = config->column_offset + ((x + desc->width) >> 2) - 1;

jfischer-no · 2024-08-16T10:05:56Z

drivers/display/ssd1322.c

+	}
+
+	cmd_data[0] = y;
+	cmd_data[1] = (y + desc->height - 1);


Acute overparenthesesis.

cmd_data[1] = y + desc->height - 1;

jfischer-no · 2024-08-16T10:09:49Z

drivers/display/ssd1322.c

+		return ret;
+	}
+
+	uint8_t *buff_ptr = (uint8_t *)buf;


buf is a buffer pointer, maybe data_ptr is better. You do not need explicit conversion here. Variable definition should be at the top of a block.

jfischer-no · 2024-08-16T10:12:02Z

drivers/display/ssd1322.c

+		}
+		ret = ssd1322_write_data(dev, pixel_buff, sizeof(pixel_buff));
+		if (ret < 0) {
+			return ret;
+		}
+		buff_ptr++;


} ret = ssd1322_write_data(dev, pixel_buff, sizeof(pixel_buff)); if (ret < 0) { return ret; } buff_ptr++;

jfischer-no · 2024-08-16T10:17:21Z

drivers/display/ssd1322.c

+	}
+
+	int ret = ssd1322_init_device(dev);
+


You can remove this empty line.

github-actions · 2024-10-16T00:33:14Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Display labels Feb 6, 2024

zephyrbot requested review from danieldegrasse, galak and jfischer-no February 6, 2024 09:56

zephyrbot assigned galak Feb 6, 2024

hlukasz force-pushed the ssd1322_driver branch 3 times, most recently from 67f53dc to 1153b68 Compare February 6, 2024 12:39

danieldegrasse reviewed Feb 13, 2024

View reviewed changes

hlukasz force-pushed the ssd1322_driver branch from 1153b68 to d51f2ec Compare February 15, 2024 13:48

hlukasz requested a review from danieldegrasse February 15, 2024 14:23

danieldegrasse reviewed Feb 22, 2024

View reviewed changes

hlukasz force-pushed the ssd1322_driver branch from d51f2ec to 255cd7d Compare February 26, 2024 13:14

hlukasz requested a review from danieldegrasse February 26, 2024 13:26

danieldegrasse reviewed Mar 21, 2024

View reviewed changes

hlukasz force-pushed the ssd1322_driver branch 2 times, most recently from 3ec74db to d745d02 Compare March 27, 2024 20:39

hlukasz requested a review from danieldegrasse March 27, 2024 21:02

drivers: display: Add ssd1322 driver

273169a

Initial support for SSD1322 OLED display driver. Only 1 bit color mode is supported. Signed-off-by: Lukasz Hawrylko <lukasz@hawrylko.pl>

hlukasz force-pushed the ssd1322_driver branch from d745d02 to 273169a Compare March 28, 2024 13:05

danieldegrasse approved these changes Apr 1, 2024

View reviewed changes

github-actions bot added the Stale label Jun 1, 2024

danieldegrasse removed the Stale label Jun 3, 2024

danieldegrasse requested a review from decsny June 3, 2024 15:05

nashif unassigned galak Jun 14, 2024

topisani mentioned this pull request Jul 7, 2024

drivers: display: ssd1322 driver #75550

Merged

jfischer-no requested changes Aug 16, 2024

View reviewed changes

github-actions bot added the Stale label Oct 16, 2024

github-actions bot closed this Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for SSD1322 OLED display driver #68608

Add support for SSD1322 OLED display driver #68608

hlukasz commented Feb 6, 2024

danieldegrasse Feb 13, 2024

hlukasz Feb 15, 2024

danieldegrasse Feb 13, 2024

hlukasz Feb 15, 2024

danieldegrasse Feb 13, 2024

hlukasz Feb 15, 2024

danieldegrasse Feb 13, 2024

hlukasz Feb 15, 2024

danieldegrasse Feb 13, 2024

hlukasz Feb 15, 2024

danieldegrasse Feb 22, 2024 •

edited

Loading

hlukasz Feb 26, 2024

danieldegrasse Mar 21, 2024

danieldegrasse Mar 21, 2024

hlukasz Mar 27, 2024

danieldegrasse Apr 1, 2024

danieldegrasse Mar 21, 2024

hlukasz Mar 27, 2024

danieldegrasse Apr 1, 2024

github-actions bot commented Jun 1, 2024

danieldegrasse commented Jun 3, 2024

kartben commented Aug 10, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

jfischer-no Aug 16, 2024

github-actions bot commented Oct 16, 2024

Add support for SSD1322 OLED display driver #68608

Add support for SSD1322 OLED display driver #68608

Conversation

hlukasz commented Feb 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieldegrasse Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 1, 2024

danieldegrasse commented Jun 3, 2024

kartben commented Aug 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2024

danieldegrasse Feb 22, 2024 •

edited

Loading