drm/solomon: Add support for the SSD132x controller family

PATCH Superseded Javier Martinez Canillas 4 participants 17 messages Oct 12, 2023

Subsystems (4)

Device tree bindings DRM drivers DRM misc DRM driver for solomon SSD130X oled displays

lore

Share & tools

lore

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #0 lore raw

v2 cover drm/solomon: Add support for the SSD132x controller family

Hello,

This patch-set adds support for the family of SSD132x Solomon controllers, such as the SSD1322, SSD1325 and SSD1327 chips. These are used for 16 Gray Scale Dot Matrix OLED panels.

This is a v2 that address issues pointed out during review of the v1:

https://lists.freedesktop.org/archives/dri-devel/2023-October/426039.html

The patches were tested on a Waveshare SSD1327 display using glmark2-drm, fbcon, fbtests and the retroarch emulator.

Patch #1 drops the .page_height field from the device info with a constant because it's only needed by the SSD130x family and not the SSD132x family.

Patch #2 adds a per controller family functions table to have logic that could be shared by both SSD130x and SSD132x callbacks.

Patch #3 renames some SSD130X_* commands that are shared by both families.

Patch #4 adds the support for the SSD132x controller family.

Patch #5 splits out some properties that are shared across both controller families bindings and move them into a separate solomon,ssd-common schema.

Finally patch #6 adds a DT binding schema for the SSD132x controllers.

Best regards,
Javier

Changes in v2:
- Add Geert Uytterhoeven's Reviewed-by tag to patch #1.
- Squash patch that uses drm_format_info_min_pitch() to calculate dest_pitch with the following patch (Geert Uytterhoeven).
- Store ssd13xx_family_funcs[SSD130X_FAMILY] in struct ssd130x_deviceinfo (Geert Uytterhoeven).
- Don't mix switch (family_id) and ssd13xx_funcs[family_id] (Geert Uytterhoeven).
- Replace switch (family_id) by an .set_buffer_sizes (Geert Uytterhoeven).
- Move the rect alignment to a per chip family function (Geert Uytterhoeven).
- Align the rectangle to the segment width (Geert Uytterhoeven).
- Drop patches that rename driver and prefixes (Maxime Ripard, Peter Robinson).
- Remove unnecessary 'oneOf' in the SSD132x DT binding schema (Conor Dooley).
- Remove unused DT nodes labels in the binding schema examples (Conor Dooley).
- Split out common Solomon properties into a separate schema (Rob Herring).

Javier Martinez Canillas (6):
drm/ssd130x: Replace .page_height field in device info with a constant drm/ssd130x: Add a per controller family functions table drm/ssd130x: Rename commands that are shared across chip families drm/ssd130x: Add support for the SSD132x OLED controller family
dt-bindings: display: Split common Solomon properties in their own
schema
dt-bindings: display: Add SSD132x OLED controllers

 .../bindings/display/solomon,ssd-common.yaml  |  42 ++
 .../bindings/display/solomon,ssd1307fb.yaml   |  28 +-
 .../bindings/display/solomon,ssd132x.yaml     |  89 ++++
 MAINTAINERS                                   |   3 +-
 drivers/gpu/drm/solomon/Kconfig               |  12 +-
 drivers/gpu/drm/solomon/ssd130x-i2c.c         |  18 +-
 drivers/gpu/drm/solomon/ssd130x-spi.c         |  27 +-
 drivers/gpu/drm/solomon/ssd130x.c             | 405 +++++++++++++++---
 drivers/gpu/drm/solomon/ssd130x.h             |  33 +-
 9 files changed, 552 insertions(+), 105 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd-common.yaml
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml

Signature · 1 line

2.41.0

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #1 lore raw

PATCH v2 1/6 drm/ssd130x: Replace .page_height field in device info with a constant

This deemed useful to avoid hardcoding a page height and allow to support other Solomon controller families, but dividing the screen in pages seems to be something that is specific to the SSD130x chip family.

For example, SSD132x chip family divides the screen in segments (columns) and common outputs (rows), so the concept of screen pages does not exist for the SSD132x family.

Let's drop this field from the device info struct and just use a constant SSD130X_PAGE_HEIGHT macro to define the page height. While being there, replace hardcoded 8 values in places where it is used as the page height.

Signed-off-by Javier Martinez Canillas <[email protected]>

Reviewed-by Geert Uytterhoeven <[email protected]>

---

Changes in v2:
- Add Geert Uytterhoeven's Reviewed-by tag to patch #1.

 drivers/gpu/drm/solomon/ssd130x.c | 37 +++++++++++++++----------------
 drivers/gpu/drm/solomon/ssd130x.h |  1 -
 2 files changed, 18 insertions(+), 20 deletions(-)

drivers/gpu/drm/solomon/ssd130x.c +18 ~~-19~~

index 6dcf3e041113..2852cddb098b 100644
@@ -42,6 +42,8 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
+#define SSD130X_PAGE_HEIGHT 8
+
 #define SSD130X_PAGE_COL_START_LOW		0x00
 #define SSD130X_PAGE_COL_START_HIGH		0x10
 #define SSD130X_SET_ADDRESS_MODE		0x20

@@ -102,7 +104,6 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.default_width = 132,
 		.default_height = 64,
 		.page_mode_only = 1,
-		.page_height = 8,
 	},
 	[SSD1305_ID] = {
 		.default_vcomh = 0x34,

@@ -110,7 +111,6 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.default_dclk_frq = 7,
 		.default_width = 132,
 		.default_height = 64,
-		.page_height = 8,
 	},
 	[SSD1306_ID] = {
 		.default_vcomh = 0x20,

@@ -119,7 +119,6 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.need_chargepump = 1,
 		.default_width = 128,
 		.default_height = 64,
-		.page_height = 8,
 	},
 	[SSD1307_ID] = {
 		.default_vcomh = 0x20,

@@ -128,7 +127,6 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.need_pwm = 1,
 		.default_width = 128,
 		.default_height = 39,
-		.page_height = 8,
 	},
 	[SSD1309_ID] = {
 		.default_vcomh = 0x34,

@@ -136,7 +134,6 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.default_dclk_frq = 10,
 		.default_width = 128,
 		.default_height = 64,
-		.page_height = 8,
 	}
 };
 EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD130X);

@@ -465,13 +462,13 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 	unsigned int width = drm_rect_width(rect);
 	unsigned int height = drm_rect_height(rect);
 	unsigned int line_length = DIV_ROUND_UP(width, 8);
-	unsigned int page_height = ssd130x->device_info->page_height;
+	unsigned int page_height = SSD130X_PAGE_HEIGHT;
 	unsigned int pages = DIV_ROUND_UP(height, page_height);
 	struct drm_device *drm = &ssd130x->drm;
 	u32 array_idx = 0;
 	int ret, i, j, k;
 
-	drm_WARN_ONCE(drm, y % 8 != 0, "y must be aligned to screen page\n");
+	drm_WARN_ONCE(drm, y % page_height != 0, "y must be aligned to screen page\n");
 
 	/*
 	 * The screen is divided in pages, each having a height of 8

@@ -503,27 +500,32 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 	 */
 
 	if (!ssd130x->page_address_mode) {
+		u8 page_start;
+
 		/* Set address range for horizontal addressing mode */
 		ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset + x, width);
 		if (ret < 0)
 			return ret;
 
-		ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset + y / 8, pages);
+		page_start = ssd130x->page_offset + y / page_height;
+		ret = ssd130x_set_page_range(ssd130x, page_start, pages);
 		if (ret < 0)
 			return ret;
 	}
 
 	for (i = 0; i < pages; i++) {
-		int m = 8;
+		int m = page_height;
 
 		/* Last page may be partial */
-		if (8 * (y / 8 + i + 1) > ssd130x->height)
-			m = ssd130x->height % 8;
+		if (page_height * (y / page_height + i + 1) > ssd130x->height)
+			m = ssd130x->height % page_height;
+
 		for (j = 0; j < width; j++) {
 			u8 data = 0;
 
 			for (k = 0; k < m; k++) {
-				u8 byte = buf[(8 * i + k) * line_length + j / 8];
+				u32 idx = (page_height * i + k) * line_length + j / 8;
+				u8 byte = buf[idx];
 				u8 bit = (byte >> (j % 8)) & 1;
 
 				data |= bit << k;

@@ -559,8 +561,7 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 
 static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
 {
-	unsigned int page_height = ssd130x->device_info->page_height;
-	unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height);
+	unsigned int pages = DIV_ROUND_UP(ssd130x->height, SSD130X_PAGE_HEIGHT);
 	unsigned int width = ssd130x->width;
 	int ret, i;

@@ -605,14 +606,13 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb,
 				u8 *buf, u8 *data_array)
 {
 	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
-	unsigned int page_height = ssd130x->device_info->page_height;
 	struct iosys_map dst;
 	unsigned int dst_pitch;
 	int ret = 0;
 
 	/* Align y to display page boundaries */
-	rect->y1 = round_down(rect->y1, page_height);
-	rect->y2 = min_t(unsigned int, round_up(rect->y2, page_height), ssd130x->height);
+	rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
+	rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd130x->height);
 
 	dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);

@@ -814,8 +814,7 @@ static int ssd130x_crtc_atomic_check(struct drm_crtc *crtc,
 	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	struct ssd130x_crtc_state *ssd130x_state = to_ssd130x_crtc_state(crtc_state);
-	unsigned int page_height = ssd130x->device_info->page_height;
-	unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height);
+	unsigned int pages = DIV_ROUND_UP(ssd130x->height, SSD130X_PAGE_HEIGHT);
 	int ret;
 
 	ret = drm_crtc_helper_atomic_check(crtc, state);

drivers/gpu/drm/solomon/ssd130x.h +0 -1

index aa39b13615eb..bbe374453605 100644
@@ -39,7 +39,6 @@ struct ssd130x_deviceinfo {
 	u32 default_dclk_frq;
 	u32 default_width;
 	u32 default_height;
-	u32 page_height;
 	bool need_pwm;
 	bool need_chargepump;
 	bool page_mode_only;

Signature · 1 line

2.41.0

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #2 lore raw

PATCH v2 2/6 drm/ssd130x: Add a per controller family functions table

In reply to Javier Martinez Canillas

To allow the driver to decouple the common logic in the function callbacks from the behaviour that is specific of a given Solomon controller family.

For this, include a chip family to the device info and add fields to the driver-private device struct, to store the hardware buffer maximum size, the intermediate buffer maximum size and pixel format, and a set of per chip family function handlers.

While doing this change, also use drm_format_info_min_pitch() to calculate the dest_pitch. This avoids to assume a 1 bpp and instead the pixel format info for the intermediate buffer is used.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

Changes in v2:
- Squash patch that uses drm_format_info_min_pitch() to calculate dest_pitch with the following patch (Geert Uytterhoeven).
- Store ssd13xx_family_funcs[SSD130X_FAMILY] in struct ssd130x_deviceinfo (Geert Uytterhoeven).
- Don't mix switch (family_id) and ssd13xx_funcs[family_id] (Geert Uytterhoeven).
- Replace switch (family_id) by an .set_buffer_sizes (Geert Uytterhoeven).
- Move the rect alignment to a per chip family function (Geert Uytterhoeven).

 drivers/gpu/drm/solomon/ssd130x-i2c.c |  1 +
 drivers/gpu/drm/solomon/ssd130x-spi.c |  2 +
 drivers/gpu/drm/solomon/ssd130x.c     | 95 ++++++++++++++++++++-------
 drivers/gpu/drm/solomon/ssd130x.h     | 24 +++++++
 4 files changed, 99 insertions(+), 23 deletions(-)

drivers/gpu/drm/solomon/ssd130x-i2c.c +1 -0

index b4eb2d64bf6e..8f89b89d553f 100644
@@ -54,6 +54,7 @@ static void ssd130x_i2c_shutdown(struct i2c_client *client)
 }
 
 static const struct of_device_id ssd130x_of_match[] = {
+	/* ssd130x family */
 	{
 		.compatible = "sinowealth,sh1106",
 		.data = &ssd130x_variants[SH1106_ID],

drivers/gpu/drm/solomon/ssd130x-spi.c +2 -0

index 19ab4942cb33..257819bccbc8 100644
@@ -108,6 +108,7 @@ static void ssd130x_spi_shutdown(struct spi_device *spi)
 }
 
 static const struct of_device_id ssd130x_of_match[] = {
+	/* ssd130x family */
 	{
 		.compatible = "sinowealth,sh1106",
 		.data = &ssd130x_variants[SH1106_ID],

@@ -142,6 +143,7 @@ MODULE_DEVICE_TABLE(of, ssd130x_of_match);
  * not be needed for this driver to match the registered SPI devices.
  */
 static const struct spi_device_id ssd130x_spi_table[] = {
+	/* ssd130x family */
 	{ "sh1106",  SH1106_ID },
 	{ "ssd1305", SSD1305_ID },
 	{ "ssd1306", SSD1306_ID },

drivers/gpu/drm/solomon/ssd130x.c +72 ~~-23~~

233 lines not on this page · raw message

drivers/gpu/drm/solomon/ssd130x.h +24 -0

index bbe374453605..5af251676587 100644
@@ -20,11 +20,13 @@
 #include <drm/drm_plane_helper.h>
 
 #include <linux/regmap.h>
+#include <linux/iosys-map.h>
 
 #define SSD130X_DATA				0x40
 #define SSD130X_COMMAND				0x80
 
 enum ssd130x_variants {
+	/* ssd130x family */
 	SH1106_ID,
 	SSD1305_ID,
 	SSD1306_ID,

@@ -42,6 +44,9 @@ struct ssd130x_deviceinfo {
 	bool need_pwm;
 	bool need_chargepump;
 	bool page_mode_only;
+
+	/* Chip family specific operations */
+	const struct ssd130x_funcs *funcs;
 };
 
 struct ssd130x_device {

@@ -76,6 +81,12 @@ struct ssd130x_device {
 	u32 col_offset;
 	u32 prechargep1;
 	u32 prechargep2;
+	/* HW format buffer size */
+	u32 data_array_size;
+	/* Intermediate buffer size */
+	u32 buffer_size;
+	/* Pixel format info for the intermediate buffer */
+	const struct drm_format_info *buffer_fi;
 
 	struct backlight_device *bl_dev;
 	struct pwm_device *pwm;

@@ -90,6 +101,19 @@ struct ssd130x_device {
 	u8 page_end;
 };
 
+struct ssd130x_funcs {
+	int (*init)(struct ssd130x_device *ssd130x);
+	int (*set_buffer_sizes)(struct ssd130x_device *ssd130x);
+	void (*align_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect);
+	int (*update_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect,
+			   u8 *buf, u8 *data_array);
+	void (*clear_screen)(struct ssd130x_device *ssd130x,
+			     u8 *data_array);
+	void (*fmt_convert)(struct iosys_map *dst, const unsigned int *dst_pitch,
+			    const struct iosys_map *src, const struct drm_framebuffer *fb,
+			    const struct drm_rect *clip);
+};
+
 extern const struct ssd130x_deviceinfo ssd130x_variants[];
 
 struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap);

Signature · 1 line

2.41.0

Thomas Zimmermann

maintainer

2023-10-12 07:39 UTC #3 lore raw

Re: [PATCH v2 2/6] drm/ssd130x: Add a per controller family functions table

Hi Javier

Am 12.10.23 um 08:58 schrieb Javier Martinez Canillas:
[...]

Quoted 4 lines

drivers/gpu/drm/solomon/ssd130x.h +3 -0

+struct ssd130x_funcs {
+	int (*init)(struct ssd130x_device *ssd130x);
+	int (*set_buffer_sizes)(struct ssd130x_device *ssd130x);

drivers/gpu/drm/solomon/ssd130x.h struct ssd130x_device { · line 116

Quoted 10 lines

drivers/gpu/drm/solomon/ssd130x.h +10 -0

+	void (*align_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect);
+	int (*update_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect,
+			   u8 *buf, u8 *data_array);
+	void (*clear_screen)(struct ssd130x_device *ssd130x,
+			     u8 *data_array);
+	void (*fmt_convert)(struct iosys_map *dst, const unsigned int *dst_pitch,
+			    const struct iosys_map *src, const struct drm_framebuffer *fb,
+			    const struct drm_rect *clip);
+};
+

You are reinventing DRM's atomic helpers. I strongly advised against doing that, as it often turns out bad. Maybe see my rant at [1] wrt to another driver.

It's much better to create a separate mode-setting pipeline for the ssd132x series and share the common code among pipelines. Your driver will have a clean and readable implementation for each supported chipset. Compare an old version of mgag200 [2] with the current driver to see the difference.

Best regards
Thomas

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://elixir.bootlin.com/linux/v5.5/source/drivers/gpu/drm/mgag200

Quoted 3 lines

  extern const struct ssd130x_deviceinfo ssd130x_variants[];
  
  struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap);

Signature · 6 lines

Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Javier Martinez Canillas

author maintainer

2023-10-12 08:02 UTC #4 lore raw

Re: [PATCH v2 2/6] drm/ssd130x: Add a per controller family functions table

Thomas Zimmermann <[email protected]> writes:

Hello Thomas,

Thanks a lot for your feedback.

Quoted 23 lines

Hi Javier
Am 12.10.23 um 08:58 schrieb Javier Martinez Canillas:
[...]
Quoted 4 lines
drivers/gpu/drm/solomon/ssd130x.h +3 -0
+struct ssd130x_funcs {
+	int (*init)(struct ssd130x_device *ssd130x);
+	int (*set_buffer_sizes)(struct ssd130x_device *ssd130x);
drivers/gpu/drm/solomon/ssd130x.h struct ssd130x_device { · line 116
Quoted 10 lines
drivers/gpu/drm/solomon/ssd130x.h +10 -0
+	void (*align_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect);
+	int (*update_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect,
+			   u8 *buf, u8 *data_array);
+	void (*clear_screen)(struct ssd130x_device *ssd130x,
+			     u8 *data_array);
+	void (*fmt_convert)(struct iosys_map *dst, const unsigned int *dst_pitch,
+			    const struct iosys_map *src, const struct drm_framebuffer *fb,
+			    const struct drm_rect *clip);
+};
+
You are reinventing DRM's atomic helpers. I strongly advised against doing that, as it often turns out bad. Maybe see my rant at [1] wrt to another driver.

Quoted 6 lines

It's much better to create a separate mode-setting pipeline for the ssd132x series and share the common code among pipelines. Your driver will have a clean and readable implementation for each supported chipset. Compare an old version of mgag200 [2] with the current driver to see the difference.

I see what you mean. The reason why I didn't go that route was to minimize code duplication, but you are correct that each level of indirection makes the driver harder to read, to reason about and fragile (modifying a common callback could have undesired effects on other chip families as you said).

I'll give it a try to what you propose in v3, have separate modesetting pipeline for SSD130x and SSD132x, even if this could lead to a little more duplicated code.

Quoted 3 lines

Best regards
Thomas

Signature · 5 lines

Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Thomas Zimmermann

maintainer

2023-10-12 08:21 UTC #5 lore raw

Re: [PATCH v2 2/6] drm/ssd130x: Add a per controller family functions table

Hi Javier

Am 12.10.23 um 10:02 schrieb Javier Martinez Canillas:

Quoted 36 lines

Thomas Zimmermann <[email protected]> writes:
Hello Thomas,
Thanks a lot for your feedback.
Quoted 23 lines
Hi Javier
Am 12.10.23 um 08:58 schrieb Javier Martinez Canillas:
[...]
Quoted 4 lines
drivers/gpu/drm/solomon/ssd130x.h +3 -0
+struct ssd130x_funcs {
+	int (*init)(struct ssd130x_device *ssd130x);
+	int (*set_buffer_sizes)(struct ssd130x_device *ssd130x);
drivers/gpu/drm/solomon/ssd130x.h struct ssd130x_device { · line 116
Quoted 10 lines
drivers/gpu/drm/solomon/ssd130x.h +10 -0
+	void (*align_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect);
+	int (*update_rect)(struct ssd130x_device *ssd130x, struct drm_rect *rect,
+			   u8 *buf, u8 *data_array);
+	void (*clear_screen)(struct ssd130x_device *ssd130x,
+			     u8 *data_array);
+	void (*fmt_convert)(struct iosys_map *dst, const unsigned int *dst_pitch,
+			    const struct iosys_map *src, const struct drm_framebuffer *fb,
+			    const struct drm_rect *clip);
+};
+
You are reinventing DRM's atomic helpers. I strongly advised against doing that, as it often turns out bad. Maybe see my rant at [1] wrt to another driver.
Quoted 6 lines
It's much better to create a separate mode-setting pipeline for the ssd132x series and share the common code among pipelines. Your driver will have a clean and readable implementation for each supported chipset. Compare an old version of mgag200 [2] with the current driver to see the difference.

Quoted 8 lines

I see what you mean. The reason why I didn't go that route was to minimize code duplication, but you are correct that each level of indirection makes the driver harder to read, to reason about and fragile (modifying a common callback could have undesired effects on other chip families as you said).
I'll give it a try to what you propose in v3, have separate modesetting pipeline for SSD130x and SSD132x, even if this could lead to a little more duplicated code.

Thanks a lot. I know you want to make a reference driver for others to study. So I think at least trying the multi-pipeline way is worth it.

From the mgag200 and ast drivers, I found that the refactoring patch sets can be large to the point where one wonders whether it's actually worth it. But the end results turned out ok. (ast still needs more work though.)

Best regards
Thomas

Quoted 5 lines

Quoted 4 lines
Best regards
Thomas

Signature · 6 lines

Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #6 lore raw

PATCH v2 3/6 drm/ssd130x: Rename commands that are shared across chip families

In reply to Javier Martinez Canillas

There are some commands that are shared between the SSD130x and SSD132x controller families, define these as a common SSD13XX set of commands.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

(no changes since v1)

 drivers/gpu/drm/solomon/ssd130x-spi.c |  4 +--
 drivers/gpu/drm/solomon/ssd130x.c     | 47 +++++++++++++++------------
 drivers/gpu/drm/solomon/ssd130x.h     |  4 +--
 3 files changed, 30 insertions(+), 25 deletions(-)

drivers/gpu/drm/solomon/ssd130x-spi.c +2 -2

index 257819bccbc8..89989da705d7 100644
@@ -34,10 +34,10 @@ static int ssd130x_spi_write(void *context, const void *data, size_t count)
 	struct spi_device *spi = t->spi;
 	const u8 *reg = data;
 
-	if (*reg == SSD130X_COMMAND)
+	if (*reg == SSD13XX_COMMAND)
 		gpiod_set_value_cansleep(t->dc, 0);
 
-	if (*reg == SSD130X_DATA)
+	if (*reg == SSD13XX_DATA)
 		gpiod_set_value_cansleep(t->dc, 1);
 
 	/* Remove control byte since is not used in a 4-wire SPI interface */

drivers/gpu/drm/solomon/ssd130x.c +26 ~~-21~~

125 lines not on this page · raw message

drivers/gpu/drm/solomon/ssd130x.h +2 -2

index 5af251676587..b508ab98be2f 100644
@@ -22,8 +22,8 @@
 #include <linux/regmap.h>
 #include <linux/iosys-map.h>
 
-#define SSD130X_DATA				0x40
-#define SSD130X_COMMAND				0x80
+#define SSD13XX_DATA				0x40
+#define SSD13XX_COMMAND				0x80
 
 enum ssd130x_variants {
 	/* ssd130x family */

Signature · 1 line

2.41.0

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #7 lore raw

PATCH v2 4/6 drm/ssd130x: Add support for the SSD132x OLED controller family

In reply to Javier Martinez Canillas

The Solomon SSD132x controllers (such as the SSD1322, SSD1325 and SSD1327) are used by 16 grayscale dot matrix OLED panels, extend the driver to also support this chip family.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

Changes in v2:
- Align the rectangle to the segment width (Geert Uytterhoeven).

 drivers/gpu/drm/solomon/Kconfig       |  12 +-
 drivers/gpu/drm/solomon/ssd130x-i2c.c |  17 +-
 drivers/gpu/drm/solomon/ssd130x-spi.c |  21 ++-
 drivers/gpu/drm/solomon/ssd130x.c     | 234 +++++++++++++++++++++++++-
 drivers/gpu/drm/solomon/ssd130x.h     |   4 +
 5 files changed, 275 insertions(+), 13 deletions(-)

drivers/gpu/drm/solomon/Kconfig +6 -6

index e170716d976b..c3ee956c2bb9 100644
@@ -1,31 +1,31 @@
 config DRM_SSD130X
-	tristate "DRM support for Solomon SSD130x OLED displays"
+	tristate "DRM support for Solomon SSD13xx OLED displays"
 	depends on DRM && MMU
 	select BACKLIGHT_CLASS_DEVICE
 	select DRM_GEM_SHMEM_HELPER
 	select DRM_KMS_HELPER
 	help
-	  DRM driver for the SSD130x Solomon and SINO WEALTH SH110x OLED
+	  DRM driver for the SSD13xx Solomon and SINO WEALTH SH110x OLED
 	  controllers. This is only for the core driver, a driver for the
 	  appropriate bus transport in your chip also must be selected.
 
 	  If M is selected the module will be called ssd130x.
 
 config DRM_SSD130X_I2C
-	tristate "DRM support for Solomon SSD130x OLED displays (I2C bus)"
+	tristate "DRM support for Solomon SSD13xx OLED displays (I2C bus)"
 	depends on DRM_SSD130X && I2C
 	select REGMAP_I2C
 	help
-	  Say Y here if the SSD130x or SH110x OLED display is connected via
+	  Say Y here if the SSD13xx or SH110x OLED display is connected via
 	  I2C bus.
 
 	  If M is selected the module will be called ssd130x-i2c.
 
 config DRM_SSD130X_SPI
-	tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)"
+	tristate "DRM support for Solomon SSD13xx OLED displays (SPI bus)"
 	depends on DRM_SSD130X && SPI
 	select REGMAP
 	help
-	  Say Y here if the SSD130x OLED display is connected via SPI bus.
+	  Say Y here if the SSD13xx OLED display is connected via SPI bus.
 
 	  If M is selected the module will be called ssd130x-spi.

drivers/gpu/drm/solomon/ssd130x-i2c.c +15 -2

index 8f89b89d553f..f2ccab9c06d9 100644
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * DRM driver for Solomon SSD130x OLED displays (I2C bus)
+ * DRM driver for Solomon SSD13xx OLED displays (I2C bus)
  *
  * Copyright 2022 Red Hat Inc.
  * Author: Javier Martinez Canillas <[email protected]>

@@ -14,7 +14,7 @@
 #include "ssd130x.h"
 
 #define DRIVER_NAME	"ssd130x-i2c"
-#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays (I2C)"
+#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
 
 static const struct regmap_config ssd130x_i2c_regmap_config = {
 	.reg_bits = 8,

@@ -92,6 +92,19 @@ static const struct of_device_id ssd130x_of_match[] = {
 		.compatible = "solomon,ssd1309fb-i2c",
 		.data = &ssd130x_variants[SSD1309_ID],
 	},
+	/* ssd132x family */
+	{
+		.compatible = "solomon,ssd1322",
+		.data = &ssd130x_variants[SSD1322_ID],
+	},
+	{
+		.compatible = "solomon,ssd1325",
+		.data = &ssd130x_variants[SSD1325_ID],
+	},
+	{
+		.compatible = "solomon,ssd1327",
+		.data = &ssd130x_variants[SSD1327_ID],
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ssd130x_of_match);

drivers/gpu/drm/solomon/ssd130x-spi.c +19 -2

index 89989da705d7..84e035a7ab3f 100644
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * DRM driver for Solomon SSD130X OLED displays (SPI bus)
+ * DRM driver for Solomon SSD13xx OLED displays (SPI bus)
  *
  * Copyright 2022 Red Hat Inc.
  * Authors: Javier Martinez Canillas <[email protected]>

@@ -11,7 +11,7 @@
 #include "ssd130x.h"
 
 #define DRIVER_NAME	"ssd130x-spi"
-#define DRIVER_DESC	"DRM driver for Solomon SSD130X OLED displays (SPI)"
+#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (SPI)"
 
 struct ssd130x_spi_transport {
 	struct spi_device *spi;

@@ -129,6 +129,19 @@ static const struct of_device_id ssd130x_of_match[] = {
 		.compatible = "solomon,ssd1309",
 		.data = &ssd130x_variants[SSD1309_ID],
 	},
+	/* ssd132x family */
+	{
+		.compatible = "solomon,ssd1322",
+		.data = &ssd130x_variants[SSD1322_ID],
+	},
+	{
+		.compatible = "solomon,ssd1325",
+		.data = &ssd130x_variants[SSD1325_ID],
+	},
+	{
+		.compatible = "solomon,ssd1327",
+		.data = &ssd130x_variants[SSD1327_ID],
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ssd130x_of_match);

@@ -149,6 +162,10 @@ static const struct spi_device_id ssd130x_spi_table[] = {
 	{ "ssd1306", SSD1306_ID },
 	{ "ssd1307", SSD1307_ID },
 	{ "ssd1309", SSD1309_ID },
+	/* ssd132x family */
+	{ "ssd1322", SSD1322_ID },
+	{ "ssd1325", SSD1325_ID },
+	{ "ssd1327", SSD1327_ID },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);

drivers/gpu/drm/solomon/ssd130x.c +231 -3

278 lines not on this page · raw message

drivers/gpu/drm/solomon/ssd130x.h +4 -0

index b508ab98be2f..e93288b5a9c2 100644
@@ -32,6 +32,10 @@ enum ssd130x_variants {
 	SSD1306_ID,
 	SSD1307_ID,
 	SSD1309_ID,
+	/* ssd132x family */
+	SSD1322_ID,
+	SSD1325_ID,
+	SSD1327_ID,
 	NR_SSD130X_VARIANTS
 };

Signature · 1 line

2.41.0

Geert Uytterhoeven

2023-10-12 07:57 UTC #8 lore raw

Re: [PATCH v2 4/6] drm/ssd130x: Add support for the SSD132x OLED controller family

Hi Javier,

Quoted 9 lines · Javier Martinez Canillas

The Solomon SSD132x controllers (such as the SSD1322, SSD1325 and SSD1327) are used by 16 grayscale dot matrix OLED panels, extend the driver to also support this chip family.
Signed-off-by Javier Martinez Canillas <[email protected]>
---
Changes in v2:
- Align the rectangle to the segment width (Geert Uytterhoeven).

Thanks for the update!

Quoted 7 lines

drivers/gpu/drm/solomon/ssd130x.c +4 -0

 
+static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
+                              struct drm_rect *rect, u8 *buf,
+                              u8 *data_array)
+{

drivers/gpu/drm/solomon/ssd130x.c static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array) · line 814

Quoted 10 lines

drivers/gpu/drm/solomon/ssd130x.c +10 -0

+       unsigned int x = rect->x1;
+       unsigned int y = rect->y1;
+       unsigned int segment_width = SSD132X_SEGMENT_WIDTH;
+       unsigned int width = drm_rect_width(rect);
+       unsigned int height = drm_rect_height(rect);
+       unsigned int columns = DIV_ROUND_UP(width, segment_width);
+       unsigned int rows = height;
+       struct drm_device *drm = &ssd130x->drm;
+       u32 array_idx = 0;
+       int ret, i, j;

unsigned int i, j;

Quoted 32 lines

drivers/gpu/drm/solomon/ssd130x.c +32 -0

+
+       drm_WARN_ONCE(drm, x % segment_width != 0, "x must be aligned to screen segment\n");
+
+       /*
+        * The screen is divided in Segment and Common outputs, where
+        * COM0 to COM[N - 1] are the rows and SEG0 to SEG[M - 1] are
+        * the columns.
+        *
+        * Each Segment has a 4-bit pixel and each Common output has a
+        * row of pixels. When using the (default) horizontal address
+        * increment mode, each byte of data sent to the controller has
+        * two Segments (e.g: SEG0 and SEG1) that are stored in the lower
+        * and higher nibbles of a single byte representing one column.
+        * That is, the first byte are SEG0 (D0[3:0]) and SEG1 (D0[7:4]),
+        * the second byte are SEG2 (D1[3:0]) and SEG3 (D1[7:4]) and so on.
+        */
+
+       /* Set column start and end */
+       ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width, columns - 1);
+       if (ret < 0)
+               return ret;
+
+       /* Set row start and end */
+       ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, rows - 1);
+       if (ret < 0)
+               return ret;
+
+       for (i = 0; i < height; i++) {
+               /* Process pair of pixels and combine them into a single byte */
+               for (j = 0; j < width; j += segment_width) {
+                       u8 n1 = buf[i * width + j];
+                       u8 n2 = buf[i * width + j + 1];

drivers/gpu/drm/solomon/ssd130x.c static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array) · line 856

Quoted 10 lines

drivers/gpu/drm/solomon/ssd130x.c +10 -0

+
+                       data_array[array_idx++] = (n2 << 4) | n1;
+               }
+       }
+
+       /* Write out update in one go since horizontal addressing mode is used */
+       ret = ssd130x_write_data(ssd130x, data_array, columns * rows);
+
+       return ret;
+}

Gr{oetje,eeting}s,

Geert

Signature · 5 lines

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Javier Martinez Canillas

author maintainer

2023-10-12 08:05 UTC #9 lore raw

Re: [PATCH v2 4/6] drm/ssd130x: Add support for the SSD132x OLED controller family

Geert Uytterhoeven <[email protected]> writes:

Hello Geert,

Thanks for your feedback.

Quoted 2 lines

Hi Javier,

[...]

Quoted 5 lines

drivers/gpu/drm/solomon/ssd130x.c static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array) · line 814
Quoted 2 lines
drivers/gpu/drm/solomon/ssd130x.c +2 -0
+       u32 array_idx = 0;
+       int ret, i, j;
unsigned int i, j;

Right, I'll change that in v3.

Signature · 5 lines

Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #10 lore raw

PATCH v2 5/6 dt-bindings: display: Split common Solomon properties in their own schema

In reply to Javier Martinez Canillas

There are DT properties that can be shared across different Solomon OLED Display Controller families. Split them into a separate common schema to avoid these properties to be duplicated in different DT bindings schemas.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

(no changes since v1)

 .../bindings/display/solomon,ssd-common.yaml  | 42 +++++++++++++++++++
 .../bindings/display/solomon,ssd1307fb.yaml   | 28 +------------
 MAINTAINERS                                   |  1 +
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd-common.yaml

Documentation/devicetree/bindings/display/solomon,ssd-common.yaml new file +42 -0

new file mode 100644
index 000000000000..677fd2b90960
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common properties for Solomon OLED Display Controllers
+
+maintainers:
+  - Javier Martinez Canillas <[email protected]>
+
+properties:
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  # Only required for SPI
+  dc-gpios:
+    description:
+      GPIO connected to the controller's D/C# (Data/Command) pin,
+      that is needed for 4-wire SPI to tell the controller if the
+      data sent is for a command register or the display data RAM
+    maxItems: 1
+
+  solomon,height:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Height in pixel of the screen driven by the controller.
+      The default value is controller-dependent.
+
+  solomon,width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Width in pixel of the screen driven by the controller.
+      The default value is controller-dependent.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: true
\ No newline at end of file

Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +1 ~~-27~~

index 20e2bd15d4d2..3afbb52d1b7f 100644
@@ -27,38 +27,12 @@ properties:
           - solomon,ssd1307
           - solomon,ssd1309
 
-  reg:
-    maxItems: 1
-
   pwms:
     maxItems: 1
 
-  reset-gpios:
-    maxItems: 1
-
-  # Only required for SPI
-  dc-gpios:
-    description:
-      GPIO connected to the controller's D/C# (Data/Command) pin,
-      that is needed for 4-wire SPI to tell the controller if the
-      data sent is for a command register or the display data RAM
-    maxItems: 1
-
   vbat-supply:
     description: The supply for VBAT
 
-  solomon,height:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description:
-      Height in pixel of the screen driven by the controller.
-      The default value is controller-dependent.
-
-  solomon,width:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description:
-      Width in pixel of the screen driven by the controller.
-      The default value is controller-dependent.
-
   solomon,page-offset:
     $ref: /schemas/types.yaml#/definitions/uint32
     default: 1

@@ -148,7 +122,7 @@ required:
   - reg
 
 allOf:
-  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - $ref: solomon,ssd-common.yaml#
 
   - if:
       properties:

MAINTAINERS +1 -0

index 46ca5c4affdb..4a3baf970839 100644
@@ -6732,6 +6732,7 @@ DRM DRIVER FOR SOLOMON SSD130X OLED DISPLAYS
 M:	Javier Martinez Canillas <[email protected]>
 S:	Maintained
 T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/solomon,ssd-common.yaml
 F:	Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
 F:	drivers/gpu/drm/solomon/ssd130x*

Signature · 1 line

2.41.0

Rob Herring

maintainer

2023-10-12 07:23 UTC #11 lore raw

Re: [PATCH v2 5/6] dt-bindings: display: Split common Solomon properties in their own schema

Quoted 8 lines · Javier Martinez Canillas

There are DT properties that can be shared across different Solomon OLED Display Controller families. Split them into a separate common schema to avoid these properties to be duplicated in different DT bindings schemas.
Suggested-by Rob Herring <[email protected]>
Signed-off-by Javier Martinez Canillas <[email protected]>
---

Quoted 8 lines

(no changes since v1)

 .../bindings/display/solomon,ssd-common.yaml  | 42 +++++++++++++++++++
 .../bindings/display/solomon,ssd1307fb.yaml   | 28 +------------
 MAINTAINERS                                   |  1 +
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd-common.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/solomon,ssd-common.yaml:42:27: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

Geert Uytterhoeven

2023-10-12 07:59 UTC #12 lore raw

Re: [PATCH v2 5/6] dt-bindings: display: Split common Solomon properties in their own schema

In reply to Javier Martinez Canillas

Hi Javier,

Quoted 9 lines · Javier Martinez Canillas

There are DT properties that can be shared across different Solomon OLED Display Controller families. Split them into a separate common schema to avoid these properties to be duplicated in different DT bindings schemas.
Suggested-by Rob Herring <[email protected]>
Signed-off-by Javier Martinez Canillas <[email protected]>
---
(no changes since v1)

New patch in v2.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Javier Martinez Canillas

author maintainer

2023-10-12 08:08 UTC #13 lore raw

Re: [PATCH v2 5/6] dt-bindings: display: Split common Solomon properties in their own schema

Quoted 8 lines · Geert Uytterhoeven

Hi Javier,
Quoted 4 lines · Javier Martinez Canillas
There are DT properties that can be shared across different Solomon OLED Display Controller families. Split them into a separate common schema to avoid these properties to be duplicated in different DT bindings schemas.

Quoted 8 lines

Quoted 5 lines
Suggested-by Rob Herring <[email protected]>
Signed-off-by Javier Martinez Canillas <[email protected]>
---
(no changes since v1)
New patch in v2.

Yeah, I mention that in the cover letter. That "(no changes since...)" message is automatically added by the tool I use to post patches (patman) for all patches that don't have a change history, even for new patches in a series revision. And I don't know of a way to disable it...

Maybe what I should do is to add a change history to new patches mentioned that is a new patch to prevent this message to appear.

Signature · 5 lines

Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Rob Herring

maintainer Reviewed-by on v2 5/6

2023-10-12 11:52 UTC #14 lore raw

Re: [PATCH v2 5/6] dt-bindings: display: Split common Solomon properties in their own schema

In reply to Javier Martinez Canillas

Quoted 55 lines · Javier Martinez Canillas

Signed-off-by Javier Martinez Canillas <[email protected]>

---

(no changes since v1)

 .../bindings/display/solomon,ssd-common.yaml  | 42 +++++++++++++++++++
 .../bindings/display/solomon,ssd1307fb.yaml   | 28 +------------
 MAINTAINERS                                   |  1 +
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd-common.yaml

Documentation/devicetree/bindings/display/solomon,ssd-common.yaml new file +33 -0

new file mode 100644
index 000000000000..677fd2b90960
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common properties for Solomon OLED Display Controllers
+
+maintainers:
+  - Javier Martinez Canillas <[email protected]>
+
+properties:
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  # Only required for SPI
+  dc-gpios:
+    description:
+      GPIO connected to the controller's D/C# (Data/Command) pin,
+      that is needed for 4-wire SPI to tell the controller if the
+      data sent is for a command register or the display data RAM
+    maxItems: 1
+
+  solomon,height:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Height in pixel of the screen driven by the controller.
+      The default value is controller-dependent.
+
+  solomon,width:

Documentation/devicetree/bindings/display/solomon,ssd-common.yaml line 42

Quoted 10 lines

Documentation/devicetree/bindings/display/solomon,ssd-common.yaml new file +9 -0

+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Width in pixel of the screen driven by the controller.
+      The default value is controller-dependent.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+additionalProperties: true
\ No newline at end of file

With this fixed,

Reviewed-by Rob Herring <[email protected]>

Javier Martinez Canillas

author maintainer

2023-10-12 06:58 UTC #15 lore raw

PATCH v2 6/6 dt-bindings: display: Add SSD132x OLED controllers

In reply to Javier Martinez Canillas

Add a Device Tree binding schema for the OLED panels based on the Solomon SSD132x family of controllers.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

Changes in v2:
- Remove unnecessary 'oneOf' in the SSD132x DT binding schema (Conor Dooley).
- Remove unused DT nodes labels in the binding schema examples (Conor Dooley).
- Split out common Solomon properties into a separate schema (Rob Herring).

 .../bindings/display/solomon,ssd132x.yaml     | 89 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml

Documentation/devicetree/bindings/display/solomon,ssd132x.yaml new file +89 -0

new file mode 100644
index 000000000000..0aa41bd9ddca
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd132x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Solomon SSD132x OLED Display Controllers
+
+maintainers:
+  - Javier Martinez Canillas <[email protected]>
+
+properties:
+  compatible:
+    - enum:
+        - solomon,ssd1322
+        - solomon,ssd1325
+        - solomon,ssd1327
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: solomon,ssd-common.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: solomon,ssd1322
+    then:
+      properties:
+        width:
+          default: 480
+        height:
+          default: 128
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: solomon,ssd1325
+    then:
+      properties:
+        width:
+          default: 128
+        height:
+          default: 80
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: solomon,ssd1327
+    then:
+      properties:
+        width:
+          default: 128
+        height:
+          default: 128
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            oled@3c {
+                    compatible = "solomon,ssd1327";
+                    reg = <0x3c>;
+                    reset-gpios = <&gpio2 7>;
+            };
+
+    };
+  - |
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            oled@0 {
+                    compatible = "solomon,ssd1327";
+                    reg = <0x0>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+            };
+    };

MAINTAINERS +1 -1

index 4a3baf970839..5257e0074f2b 100644
@@ -6733,7 +6733,7 @@ M:	Javier Martinez Canillas <[email protected]>
 S:	Maintained
 T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/solomon,ssd-common.yaml
-F:	Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+F:	Documentation/devicetree/bindings/display/solomon,ssd13*.yaml
 F:	drivers/gpu/drm/solomon/ssd130x*
 
 DRM DRIVER FOR ST-ERICSSON MCDE

Signature · 1 line

2.41.0

Rob Herring

maintainer Reviewed-by on v2 6/6

2023-10-12 11:53 UTC #16 lore raw

Re: [PATCH v2 6/6] dt-bindings: display: Add SSD132x OLED controllers

Quoted 11 lines · Javier Martinez Canillas

Add a Device Tree binding schema for the OLED panels based on the Solomon SSD132x family of controllers.
Signed-off-by Javier Martinez Canillas <[email protected]>
---
Changes in v2:
- Remove unnecessary 'oneOf' in the SSD132x DT binding schema (Conor Dooley).
- Remove unused DT nodes labels in the binding schema examples (Conor Dooley).
- Split out common Solomon properties into a separate schema (Rob Herring).

Quoted 4 lines

 .../bindings/display/solomon,ssd132x.yaml     | 89 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml

Reviewed-by Rob Herring <[email protected]>