linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family
@ 2023-10-09 18:34 Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx Javier Martinez Canillas
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Conor Dooley, Daniel Vetter,
	David Airlie, Krzysztof Kozlowski, Maarten Lankhorst,
	Rob Herring, Thierry Reding, Uwe Kleine-König, devicetree,
	dri-devel, linux-pwm

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.

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

Patch #1 renames the ssd130x driver to ssd13xx since it will support both
the SSD130x and SSD132x Solomon families and at least the SSD133x family
in the future.

Patch #2 also renames the data structures and functions prefixes with the
ssd13xx name.

Patch #3 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 #4 changes the driver to use drm_format_info_min_pitch() instead of
open coding the same calculation.

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

Patch #6 renames some SSD130X_* commands that are shared by both families
and patch #7 adds the support for the SSD132x controller family.

Finally patch #8 adds a DT binding schema for the SSD132x device nodes.

Best regards,
Javier


Javier Martinez Canillas (8):
  drm/solomon: Rename ssd130x driver to ssd13xx
  drm/ssd13xx: Rename data structures and functions prefixes
  drm/ssd13xx: Replace .page_height field in device info with a constant
  drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the
    dest_pitch
  drm/ssd13xx: Add a per controller family functions table
  drm/ssd13xx: Rename commands that are shared across chip families
  drm/ssd13xx: Add support for the SSD132x OLED controller family
  dt-bindings: display: Add SSD132x OLED controllers

 .../bindings/display/solomon,ssd132x.yaml     |  116 ++
 MAINTAINERS                                   |    6 +-
 drivers/gpu/drm/solomon/Kconfig               |   28 +-
 drivers/gpu/drm/solomon/Makefile              |    6 +-
 drivers/gpu/drm/solomon/ssd130x-i2c.c         |  112 --
 drivers/gpu/drm/solomon/ssd130x.c             | 1260 --------------
 drivers/gpu/drm/solomon/ssd13xx-i2c.c         |  126 ++
 .../solomon/{ssd130x-spi.c => ssd13xx-spi.c}  |  104 +-
 drivers/gpu/drm/solomon/ssd13xx.c             | 1508 +++++++++++++++++
 .../gpu/drm/solomon/{ssd130x.h => ssd13xx.h}  |   63 +-
 10 files changed, 1876 insertions(+), 1453 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
 delete mode 100644 drivers/gpu/drm/solomon/ssd130x-i2c.c
 delete mode 100644 drivers/gpu/drm/solomon/ssd130x.c
 create mode 100644 drivers/gpu/drm/solomon/ssd13xx-i2c.c
 rename drivers/gpu/drm/solomon/{ssd130x-spi.c => ssd13xx-spi.c} (54%)
 create mode 100644 drivers/gpu/drm/solomon/ssd13xx.c
 rename drivers/gpu/drm/solomon/{ssd130x.h => ssd13xx.h} (51%)

-- 
2.41.0


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-10  8:09   ` Maxime Ripard
  2023-10-09 18:34 ` [PATCH 2/8] drm/ssd13xx: Rename data structures and functions prefixes Javier Martinez Canillas
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

The driver only supports the SSD130x family of Solomon OLED controllers
now, but will be extended to also support the SSD132x (4-bit grayscale)
and SSD133x (16-bit RGB) controller families. Rename driver to ssd13xx.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 MAINTAINERS                                   |  6 ++--
 drivers/gpu/drm/solomon/Kconfig               | 28 +++++++++----------
 drivers/gpu/drm/solomon/Makefile              |  6 ++--
 .../solomon/{ssd130x-i2c.c => ssd13xx-i2c.c}  | 10 +++----
 .../solomon/{ssd130x-spi.c => ssd13xx-spi.c}  | 14 +++++-----
 .../gpu/drm/solomon/{ssd130x.c => ssd13xx.c}  | 10 +++----
 .../gpu/drm/solomon/{ssd130x.h => ssd13xx.h}  |  8 +++---
 7 files changed, 41 insertions(+), 41 deletions(-)
 rename drivers/gpu/drm/solomon/{ssd130x-i2c.c => ssd13xx-i2c.c} (92%)
 rename drivers/gpu/drm/solomon/{ssd130x-spi.c => ssd13xx-spi.c} (93%)
 rename drivers/gpu/drm/solomon/{ssd130x.c => ssd13xx.c} (99%)
 rename drivers/gpu/drm/solomon/{ssd130x.h => ssd13xx.h} (94%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 46ca5c4affdb..8eab0d9dca89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6728,12 +6728,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
 F:	drivers/gpu/drm/tiny/st7735r.c
 
-DRM DRIVER FOR SOLOMON SSD130X OLED DISPLAYS
+DRM DRIVER FOR SOLOMON SSD13XX OLED DISPLAYS
 M:	Javier Martinez Canillas <javierm@redhat.com>
 S:	Maintained
 T:	git git://anongit.freedesktop.org/drm/drm-misc
-F:	Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
-F:	drivers/gpu/drm/solomon/ssd130x*
+F:	Documentation/devicetree/bindings/display/solomon,ssd13*.yaml
+F:	drivers/gpu/drm/solomon/ssd13*
 
 DRM DRIVER FOR ST-ERICSSON MCDE
 M:	Linus Walleij <linus.walleij@linaro.org>
diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig
index e170716d976b..ba9f631402bc 100644
--- a/drivers/gpu/drm/solomon/Kconfig
+++ b/drivers/gpu/drm/solomon/Kconfig
@@ -1,31 +1,31 @@
-config DRM_SSD130X
-	tristate "DRM support for Solomon SSD130x OLED displays"
+config DRM_SSD13XX
+	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.
+	  If M is selected the module will be called ssd13xx.
 
-config DRM_SSD130X_I2C
-	tristate "DRM support for Solomon SSD130x OLED displays (I2C bus)"
-	depends on DRM_SSD130X && I2C
+config DRM_SSD13XX_I2C
+	tristate "DRM support for Solomon SSD13xx OLED displays (I2C bus)"
+	depends on DRM_SSD13XX && 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.
+	  If M is selected the module will be called ssd13xx-i2c.
 
-config DRM_SSD130X_SPI
-	tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)"
-	depends on DRM_SSD130X && SPI
+config DRM_SSD13XX_SPI
+	tristate "DRM support for Solomon SSD13xx OLED displays (SPI bus)"
+	depends on DRM_SSD13XX && 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.
+	  If M is selected the module will be called ssd13xx-spi.
diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile
index b5fc792257d7..6cec14c4062d 100644
--- a/drivers/gpu/drm/solomon/Makefile
+++ b/drivers/gpu/drm/solomon/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_DRM_SSD130X)	+= ssd130x.o
-obj-$(CONFIG_DRM_SSD130X_I2C)	+= ssd130x-i2c.o
-obj-$(CONFIG_DRM_SSD130X_SPI)	+= ssd130x-spi.o
+obj-$(CONFIG_DRM_SSD13XX)	+= ssd13xx.o
+obj-$(CONFIG_DRM_SSD13XX_I2C)	+= ssd13xx-i2c.o
+obj-$(CONFIG_DRM_SSD13XX_SPI)	+= ssd13xx-spi.o
diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
similarity index 92%
rename from drivers/gpu/drm/solomon/ssd130x-i2c.c
rename to drivers/gpu/drm/solomon/ssd13xx-i2c.c
index b4eb2d64bf6e..f4d0feaa8515 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
@@ -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 <javierm@redhat.com>
@@ -11,10 +11,10 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 
-#include "ssd130x.h"
+#include "ssd13xx.h"
 
-#define DRIVER_NAME	"ssd130x-i2c"
-#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays (I2C)"
+#define DRIVER_NAME	"ssd13xx-i2c"
+#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
 
 static const struct regmap_config ssd130x_i2c_regmap_config = {
 	.reg_bits = 8,
@@ -109,4 +109,4 @@ module_i2c_driver(ssd130x_i2c_driver);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
 MODULE_LICENSE("GPL v2");
-MODULE_IMPORT_NS(DRM_SSD130X);
+MODULE_IMPORT_NS(DRM_SSD13XX);
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd13xx-spi.c
similarity index 93%
rename from drivers/gpu/drm/solomon/ssd130x-spi.c
rename to drivers/gpu/drm/solomon/ssd13xx-spi.c
index 19ab4942cb33..20d1e3711e2f 100644
--- a/drivers/gpu/drm/solomon/ssd130x-spi.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-spi.c
@@ -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 <javierm@redhat.com>
@@ -8,10 +8,10 @@
 #include <linux/spi/spi.h>
 #include <linux/module.h>
 
-#include "ssd130x.h"
+#include "ssd13xx.h"
 
-#define DRIVER_NAME	"ssd130x-spi"
-#define DRIVER_DESC	"DRM driver for Solomon SSD130X OLED displays (SPI)"
+#define DRIVER_NAME	"ssd13xx-spi"
+#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (SPI)"
 
 struct ssd130x_spi_transport {
 	struct spi_device *spi;
@@ -25,7 +25,7 @@ struct ssd130x_spi_transport {
  * prior to every data byte, that contains a bit with the D/C# value.
  *
  * These control bytes are considered registers by the ssd130x core driver
- * and can be used by the ssd130x SPI driver to determine if the data sent
+ * and can be used by the ssd13xx SPI driver to determine if the data sent
  * is for a command register or for the Graphic Display Data RAM (GDDRAM).
  */
 static int ssd130x_spi_write(void *context, const void *data, size_t count)
@@ -132,7 +132,7 @@ static const struct of_device_id ssd130x_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, ssd130x_of_match);
 
-#if IS_MODULE(CONFIG_DRM_SSD130X_SPI)
+#if IS_MODULE(CONFIG_DRM_SSD13XX_SPI)
 /*
  * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
  * if the device was registered via OF. This means that the module will not be
@@ -166,4 +166,4 @@ module_spi_driver(ssd130x_spi_driver);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
 MODULE_LICENSE("GPL");
-MODULE_IMPORT_NS(DRM_SSD130X);
+MODULE_IMPORT_NS(DRM_SSD13XX);
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd13xx.c
similarity index 99%
rename from drivers/gpu/drm/solomon/ssd130x.c
rename to drivers/gpu/drm/solomon/ssd13xx.c
index 6dcf3e041113..0bf12965719a 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * DRM driver for Solomon SSD130x OLED displays
+ * DRM driver for Solomon SSD13xx OLED displays
  *
  * Copyright 2022 Red Hat Inc.
  * Author: Javier Martinez Canillas <javierm@redhat.com>
@@ -34,10 +34,10 @@
 #include <drm/drm_rect.h>
 #include <drm/drm_probe_helper.h>
 
-#include "ssd130x.h"
+#include "ssd13xx.h"
 
-#define DRIVER_NAME	"ssd130x"
-#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays"
+#define DRIVER_NAME	"ssd13xx"
+#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays"
 #define DRIVER_DATE	"20220131"
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
@@ -139,7 +139,7 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.page_height = 8,
 	}
 };
-EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD130X);
+EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD13XX);
 
 struct ssd130x_crtc_state {
 	struct drm_crtc_state base;
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd13xx.h
similarity index 94%
rename from drivers/gpu/drm/solomon/ssd130x.h
rename to drivers/gpu/drm/solomon/ssd13xx.h
index aa39b13615eb..f89ccd11cd29 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
  * Header file for:
- * DRM driver for Solomon SSD130x OLED displays
+ * DRM driver for Solomon SSD13xx OLED displays
  *
  * Copyright 2022 Red Hat Inc.
  * Author: Javier Martinez Canillas <javierm@redhat.com>
@@ -10,8 +10,8 @@
  * Copyright 2012 Free Electrons
  */
 
-#ifndef __SSD130X_H__
-#define __SSD130X_H__
+#ifndef __SSD13XX_H__
+#define __SSD13XX_H__
 
 #include <drm/drm_connector.h>
 #include <drm/drm_crtc.h>
@@ -97,4 +97,4 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap);
 void ssd130x_remove(struct ssd130x_device *ssd130x);
 void ssd130x_shutdown(struct ssd130x_device *ssd130x);
 
-#endif /* __SSD130X_H__ */
+#endif /* __SSD13XX_H__ */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/8] drm/ssd13xx: Rename data structures and functions prefixes
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant Javier Martinez Canillas
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Thierry Reding, Uwe Kleine-König,
	dri-devel, linux-pwm

Since the driver is now called ssd13xx and will support other Solomon OLED
controller families, rename the structs and functions prefixes as well.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/gpu/drm/solomon/ssd13xx-i2c.c |  63 +--
 drivers/gpu/drm/solomon/ssd13xx-spi.c |  73 +--
 drivers/gpu/drm/solomon/ssd13xx.c     | 644 +++++++++++++-------------
 drivers/gpu/drm/solomon/ssd13xx.h     |  18 +-
 4 files changed, 400 insertions(+), 398 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd13xx-i2c.c b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
index f4d0feaa8515..d9cece374331 100644
--- a/drivers/gpu/drm/solomon/ssd13xx-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
@@ -16,95 +16,96 @@
 #define DRIVER_NAME	"ssd13xx-i2c"
 #define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
 
-static const struct regmap_config ssd130x_i2c_regmap_config = {
+static const struct regmap_config ssd13xx_i2c_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 };
 
-static int ssd130x_i2c_probe(struct i2c_client *client)
+static int ssd13xx_i2c_probe(struct i2c_client *client)
 {
-	struct ssd130x_device *ssd130x;
+	struct ssd13xx_device *ssd13xx;
 	struct regmap *regmap;
 
-	regmap = devm_regmap_init_i2c(client, &ssd130x_i2c_regmap_config);
+	regmap = devm_regmap_init_i2c(client, &ssd13xx_i2c_regmap_config);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	ssd130x = ssd130x_probe(&client->dev, regmap);
-	if (IS_ERR(ssd130x))
-		return PTR_ERR(ssd130x);
+	ssd13xx = ssd13xx_probe(&client->dev, regmap);
+	if (IS_ERR(ssd13xx))
+		return PTR_ERR(ssd13xx);
 
-	i2c_set_clientdata(client, ssd130x);
+	i2c_set_clientdata(client, ssd13xx);
 
 	return 0;
 }
 
-static void ssd130x_i2c_remove(struct i2c_client *client)
+static void ssd13xx_i2c_remove(struct i2c_client *client)
 {
-	struct ssd130x_device *ssd130x = i2c_get_clientdata(client);
+	struct ssd13xx_device *ssd13xx = i2c_get_clientdata(client);
 
-	ssd130x_remove(ssd130x);
+	ssd13xx_remove(ssd13xx);
 }
 
-static void ssd130x_i2c_shutdown(struct i2c_client *client)
+static void ssd13xx_i2c_shutdown(struct i2c_client *client)
 {
-	struct ssd130x_device *ssd130x = i2c_get_clientdata(client);
+	struct ssd13xx_device *ssd13xx = i2c_get_clientdata(client);
 
-	ssd130x_shutdown(ssd130x);
+	ssd13xx_shutdown(ssd13xx);
 }
 
-static const struct of_device_id ssd130x_of_match[] = {
+static const struct of_device_id ssd13xx_of_match[] = {
+	/* ssd130x family */
 	{
 		.compatible = "sinowealth,sh1106",
-		.data = &ssd130x_variants[SH1106_ID],
+		.data = &ssd13xx_variants[SH1106_ID],
 	},
 	{
 		.compatible = "solomon,ssd1305",
-		.data = &ssd130x_variants[SSD1305_ID],
+		.data = &ssd13xx_variants[SSD1305_ID],
 	},
 	{
 		.compatible = "solomon,ssd1306",
-		.data = &ssd130x_variants[SSD1306_ID],
+		.data = &ssd13xx_variants[SSD1306_ID],
 	},
 	{
 		.compatible = "solomon,ssd1307",
-		.data = &ssd130x_variants[SSD1307_ID],
+		.data = &ssd13xx_variants[SSD1307_ID],
 	},
 	{
 		.compatible = "solomon,ssd1309",
-		.data = &ssd130x_variants[SSD1309_ID],
+		.data = &ssd13xx_variants[SSD1309_ID],
 	},
 	/* Deprecated but kept for backward compatibility */
 	{
 		.compatible = "solomon,ssd1305fb-i2c",
-		.data = &ssd130x_variants[SSD1305_ID],
+		.data = &ssd13xx_variants[SSD1305_ID],
 	},
 	{
 		.compatible = "solomon,ssd1306fb-i2c",
-		.data = &ssd130x_variants[SSD1306_ID],
+		.data = &ssd13xx_variants[SSD1306_ID],
 	},
 	{
 		.compatible = "solomon,ssd1307fb-i2c",
-		.data = &ssd130x_variants[SSD1307_ID],
+		.data = &ssd13xx_variants[SSD1307_ID],
 	},
 	{
 		.compatible = "solomon,ssd1309fb-i2c",
-		.data = &ssd130x_variants[SSD1309_ID],
+		.data = &ssd13xx_variants[SSD1309_ID],
 	},
 	{ /* sentinel */ }
 };
-MODULE_DEVICE_TABLE(of, ssd130x_of_match);
+MODULE_DEVICE_TABLE(of, ssd13xx_of_match);
 
-static struct i2c_driver ssd130x_i2c_driver = {
+static struct i2c_driver ssd13xx_i2c_driver = {
 	.driver = {
 		.name = DRIVER_NAME,
-		.of_match_table = ssd130x_of_match,
+		.of_match_table = ssd13xx_of_match,
 	},
-	.probe = ssd130x_i2c_probe,
-	.remove = ssd130x_i2c_remove,
-	.shutdown = ssd130x_i2c_shutdown,
+	.probe = ssd13xx_i2c_probe,
+	.remove = ssd13xx_i2c_remove,
+	.shutdown = ssd13xx_i2c_shutdown,
 };
-module_i2c_driver(ssd130x_i2c_driver);
+module_i2c_driver(ssd13xx_i2c_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
diff --git a/drivers/gpu/drm/solomon/ssd13xx-spi.c b/drivers/gpu/drm/solomon/ssd13xx-spi.c
index 20d1e3711e2f..a5ebe5475a49 100644
--- a/drivers/gpu/drm/solomon/ssd13xx-spi.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-spi.c
@@ -13,7 +13,7 @@
 #define DRIVER_NAME	"ssd13xx-spi"
 #define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (SPI)"
 
-struct ssd130x_spi_transport {
+struct ssd13xx_spi_transport {
 	struct spi_device *spi;
 	struct gpio_desc *dc;
 };
@@ -28,9 +28,9 @@ struct ssd130x_spi_transport {
  * and can be used by the ssd13xx SPI driver to determine if the data sent
  * is for a command register or for the Graphic Display Data RAM (GDDRAM).
  */
-static int ssd130x_spi_write(void *context, const void *data, size_t count)
+static int ssd13xx_spi_write(void *context, const void *data, size_t count)
 {
-	struct ssd130x_spi_transport *t = context;
+	struct ssd13xx_spi_transport *t = context;
 	struct spi_device *spi = t->spi;
 	const u8 *reg = data;
 
@@ -45,24 +45,24 @@ static int ssd130x_spi_write(void *context, const void *data, size_t count)
 }
 
 /* The ssd130x driver does not read registers but regmap expects a .read */
-static int ssd130x_spi_read(void *context, const void *reg, size_t reg_size,
+static int ssd13xx_spi_read(void *context, const void *reg, size_t reg_size,
 			    void *val, size_t val_size)
 {
 	return -EOPNOTSUPP;
 }
 
-static const struct regmap_config ssd130x_spi_regmap_config = {
+static const struct regmap_config ssd13xx_spi_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
-	.write = ssd130x_spi_write,
-	.read = ssd130x_spi_read,
+	.write = ssd13xx_spi_write,
+	.read = ssd13xx_spi_read,
 	.can_multi_write = true,
 };
 
-static int ssd130x_spi_probe(struct spi_device *spi)
+static int ssd13xx_spi_probe(struct spi_device *spi)
 {
-	struct ssd130x_spi_transport *t;
-	struct ssd130x_device *ssd130x;
+	struct ssd13xx_spi_transport *t;
+	struct ssd13xx_device *ssd13xx;
 	struct regmap *regmap;
 	struct gpio_desc *dc;
 	struct device *dev = &spi->dev;
@@ -80,57 +80,58 @@ static int ssd130x_spi_probe(struct spi_device *spi)
 	t->spi = spi;
 	t->dc = dc;
 
-	regmap = devm_regmap_init(dev, NULL, t, &ssd130x_spi_regmap_config);
+	regmap = devm_regmap_init(dev, NULL, t, &ssd13xx_spi_regmap_config);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	ssd130x = ssd130x_probe(dev, regmap);
-	if (IS_ERR(ssd130x))
-		return PTR_ERR(ssd130x);
+	ssd13xx = ssd13xx_probe(dev, regmap);
+	if (IS_ERR(ssd13xx))
+		return PTR_ERR(ssd13xx);
 
-	spi_set_drvdata(spi, ssd130x);
+	spi_set_drvdata(spi, ssd13xx);
 
 	return 0;
 }
 
-static void ssd130x_spi_remove(struct spi_device *spi)
+static void ssd13xx_spi_remove(struct spi_device *spi)
 {
-	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+	struct ssd13xx_device *ssd13xx = spi_get_drvdata(spi);
 
-	ssd130x_remove(ssd130x);
+	ssd13xx_remove(ssd13xx);
 }
 
-static void ssd130x_spi_shutdown(struct spi_device *spi)
+static void ssd13xx_spi_shutdown(struct spi_device *spi)
 {
-	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+	struct ssd13xx_device *ssd13xx = spi_get_drvdata(spi);
 
-	ssd130x_shutdown(ssd130x);
+	ssd13xx_shutdown(ssd13xx);
 }
 
-static const struct of_device_id ssd130x_of_match[] = {
+static const struct of_device_id ssd13xx_of_match[] = {
+	/* ssd130x family */
 	{
 		.compatible = "sinowealth,sh1106",
-		.data = &ssd130x_variants[SH1106_ID],
+		.data = &ssd13xx_variants[SH1106_ID],
 	},
 	{
 		.compatible = "solomon,ssd1305",
-		.data = &ssd130x_variants[SSD1305_ID],
+		.data = &ssd13xx_variants[SSD1305_ID],
 	},
 	{
 		.compatible = "solomon,ssd1306",
-		.data = &ssd130x_variants[SSD1306_ID],
+		.data = &ssd13xx_variants[SSD1306_ID],
 	},
 	{
 		.compatible = "solomon,ssd1307",
-		.data = &ssd130x_variants[SSD1307_ID],
+		.data = &ssd13xx_variants[SSD1307_ID],
 	},
 	{
 		.compatible = "solomon,ssd1309",
-		.data = &ssd130x_variants[SSD1309_ID],
+		.data = &ssd13xx_variants[SSD1309_ID],
 	},
 	{ /* sentinel */ }
 };
-MODULE_DEVICE_TABLE(of, ssd130x_of_match);
+MODULE_DEVICE_TABLE(of, ssd13xx_of_match);
 
 #if IS_MODULE(CONFIG_DRM_SSD13XX_SPI)
 /*
@@ -141,7 +142,7 @@ MODULE_DEVICE_TABLE(of, ssd130x_of_match);
  * To workaround this issue, add a SPI device ID table. Even when this should
  * not be needed for this driver to match the registered SPI devices.
  */
-static const struct spi_device_id ssd130x_spi_table[] = {
+static const struct spi_device_id ssd13xx_spi_table[] = {
 	{ "sh1106",  SH1106_ID },
 	{ "ssd1305", SSD1305_ID },
 	{ "ssd1306", SSD1306_ID },
@@ -149,19 +150,19 @@ static const struct spi_device_id ssd130x_spi_table[] = {
 	{ "ssd1309", SSD1309_ID },
 	{ /* sentinel */ }
 };
-MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
+MODULE_DEVICE_TABLE(spi, ssd13xx_spi_table);
 #endif
 
-static struct spi_driver ssd130x_spi_driver = {
+static struct spi_driver ssd13xx_spi_driver = {
 	.driver = {
 		.name = DRIVER_NAME,
-		.of_match_table = ssd130x_of_match,
+		.of_match_table = ssd13xx_of_match,
 	},
-	.probe = ssd130x_spi_probe,
-	.remove = ssd130x_spi_remove,
-	.shutdown = ssd130x_spi_shutdown,
+	.probe = ssd13xx_spi_probe,
+	.remove = ssd13xx_spi_remove,
+	.shutdown = ssd13xx_spi_shutdown,
 };
-module_spi_driver(ssd130x_spi_driver);
+module_spi_driver(ssd13xx_spi_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index 0bf12965719a..10a767fb614c 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -94,7 +94,7 @@
 
 #define MAX_CONTRAST 255
 
-const struct ssd130x_deviceinfo ssd130x_variants[] = {
+const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 	[SH1106_ID] = {
 		.default_vcomh = 0x40,
 		.default_dclk_div = 1,
@@ -139,52 +139,52 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
 		.page_height = 8,
 	}
 };
-EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD13XX);
+EXPORT_SYMBOL_NS_GPL(ssd13xx_variants, DRM_SSD13XX);
 
-struct ssd130x_crtc_state {
+struct ssd13xx_crtc_state {
 	struct drm_crtc_state base;
 	/* Buffer to store pixels in HW format and written to the panel */
 	u8 *data_array;
 };
 
-struct ssd130x_plane_state {
+struct ssd13xx_plane_state {
 	struct drm_shadow_plane_state base;
 	/* Intermediate buffer to convert pixels from XRGB8888 to HW format */
 	u8 *buffer;
 };
 
-static inline struct ssd130x_crtc_state *to_ssd130x_crtc_state(struct drm_crtc_state *state)
+static inline struct ssd13xx_crtc_state *to_ssd13xx_crtc_state(struct drm_crtc_state *state)
 {
-	return container_of(state, struct ssd130x_crtc_state, base);
+	return container_of(state, struct ssd13xx_crtc_state, base);
 }
 
-static inline struct ssd130x_plane_state *to_ssd130x_plane_state(struct drm_plane_state *state)
+static inline struct ssd13xx_plane_state *to_ssd13xx_plane_state(struct drm_plane_state *state)
 {
-	return container_of(state, struct ssd130x_plane_state, base.base);
+	return container_of(state, struct ssd13xx_plane_state, base.base);
 }
 
-static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
+static inline struct ssd13xx_device *drm_to_ssd13xx(struct drm_device *drm)
 {
-	return container_of(drm, struct ssd130x_device, drm);
+	return container_of(drm, struct ssd13xx_device, drm);
 }
 
 /*
  * Helper to write data (SSD130X_DATA) to the device.
  */
-static int ssd130x_write_data(struct ssd130x_device *ssd130x, u8 *values, int count)
+static int ssd13xx_write_data(struct ssd13xx_device *ssd13xx, u8 *values, int count)
 {
-	return regmap_bulk_write(ssd130x->regmap, SSD130X_DATA, values, count);
+	return regmap_bulk_write(ssd13xx->regmap, SSD130X_DATA, values, count);
 }
 
 /*
  * Helper to write command (SSD130X_COMMAND). The fist variadic argument
  * is the command to write and the following are the command options.
  *
- * Note that the ssd130x protocol requires each command and option to be
+ * Note that the ssd13xx protocol requires each command and option to be
  * written as a SSD130X_COMMAND device register value. That is why a call
  * to regmap_write(..., SSD130X_COMMAND, ...) is done for each argument.
  */
-static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
+static int ssd13xx_write_cmd(struct ssd13xx_device *ssd13xx, int count,
 			     /* u8 cmd, u8 option, ... */...)
 {
 	va_list ap;
@@ -195,7 +195,7 @@ static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
 
 	do {
 		value = va_arg(ap, int);
-		ret = regmap_write(ssd130x->regmap, SSD130X_COMMAND, value);
+		ret = regmap_write(ssd13xx->regmap, SSD130X_COMMAND, value);
 		if (ret)
 			goto out_end;
 	} while (--count);
@@ -207,44 +207,44 @@ static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
 }
 
 /* Set address range for horizontal/vertical addressing modes */
-static int ssd130x_set_col_range(struct ssd130x_device *ssd130x,
+static int ssd13xx_set_col_range(struct ssd13xx_device *ssd13xx,
 				 u8 col_start, u8 cols)
 {
 	u8 col_end = col_start + cols - 1;
 	int ret;
 
-	if (col_start == ssd130x->col_start && col_end == ssd130x->col_end)
+	if (col_start == ssd13xx->col_start && col_end == ssd13xx->col_end)
 		return 0;
 
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD130X_SET_COL_RANGE, col_start, col_end);
+	ret = ssd13xx_write_cmd(ssd13xx, 3, SSD130X_SET_COL_RANGE, col_start, col_end);
 	if (ret < 0)
 		return ret;
 
-	ssd130x->col_start = col_start;
-	ssd130x->col_end = col_end;
+	ssd13xx->col_start = col_start;
+	ssd13xx->col_end = col_end;
 	return 0;
 }
 
-static int ssd130x_set_page_range(struct ssd130x_device *ssd130x,
+static int ssd13xx_set_page_range(struct ssd13xx_device *ssd13xx,
 				  u8 page_start, u8 pages)
 {
 	u8 page_end = page_start + pages - 1;
 	int ret;
 
-	if (page_start == ssd130x->page_start && page_end == ssd130x->page_end)
+	if (page_start == ssd13xx->page_start && page_end == ssd13xx->page_end)
 		return 0;
 
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD130X_SET_PAGE_RANGE, page_start, page_end);
+	ret = ssd13xx_write_cmd(ssd13xx, 3, SSD130X_SET_PAGE_RANGE, page_start, page_end);
 	if (ret < 0)
 		return ret;
 
-	ssd130x->page_start = page_start;
-	ssd130x->page_end = page_end;
+	ssd13xx->page_start = page_start;
+	ssd13xx->page_end = page_end;
 	return 0;
 }
 
 /* Set page and column start address for page addressing mode */
-static int ssd130x_set_page_pos(struct ssd130x_device *ssd130x,
+static int ssd13xx_set_page_pos(struct ssd13xx_device *ssd13xx,
 				u8 page_start, u8 col_start)
 {
 	int ret;
@@ -256,67 +256,67 @@ static int ssd130x_set_page_pos(struct ssd130x_device *ssd130x,
 		  SSD130X_PAGE_COL_START_LOW_SET(col_start);
 	col_high = SSD130X_PAGE_COL_START_HIGH |
 		   SSD130X_PAGE_COL_START_HIGH_SET(col_start);
-	ret = ssd130x_write_cmd(ssd130x, 3, page, col_low, col_high);
+	ret = ssd13xx_write_cmd(ssd13xx, 3, page, col_low, col_high);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
 
-static int ssd130x_pwm_enable(struct ssd130x_device *ssd130x)
+static int ssd13xx_pwm_enable(struct ssd13xx_device *ssd13xx)
 {
-	struct device *dev = ssd130x->dev;
+	struct device *dev = ssd13xx->dev;
 	struct pwm_state pwmstate;
 
-	ssd130x->pwm = pwm_get(dev, NULL);
-	if (IS_ERR(ssd130x->pwm)) {
+	ssd13xx->pwm = pwm_get(dev, NULL);
+	if (IS_ERR(ssd13xx->pwm)) {
 		dev_err(dev, "Could not get PWM from firmware description!\n");
-		return PTR_ERR(ssd130x->pwm);
+		return PTR_ERR(ssd13xx->pwm);
 	}
 
-	pwm_init_state(ssd130x->pwm, &pwmstate);
+	pwm_init_state(ssd13xx->pwm, &pwmstate);
 	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
-	pwm_apply_state(ssd130x->pwm, &pwmstate);
+	pwm_apply_state(ssd13xx->pwm, &pwmstate);
 
 	/* Enable the PWM */
-	pwm_enable(ssd130x->pwm);
+	pwm_enable(ssd13xx->pwm);
 
 	dev_dbg(dev, "Using PWM %s with a %lluns period.\n",
-		ssd130x->pwm->label, pwm_get_period(ssd130x->pwm));
+		ssd13xx->pwm->label, pwm_get_period(ssd13xx->pwm));
 
 	return 0;
 }
 
-static void ssd130x_reset(struct ssd130x_device *ssd130x)
+static void ssd13xx_reset(struct ssd13xx_device *ssd13xx)
 {
-	if (!ssd130x->reset)
+	if (!ssd13xx->reset)
 		return;
 
 	/* Reset the screen */
-	gpiod_set_value_cansleep(ssd130x->reset, 1);
+	gpiod_set_value_cansleep(ssd13xx->reset, 1);
 	udelay(4);
-	gpiod_set_value_cansleep(ssd130x->reset, 0);
+	gpiod_set_value_cansleep(ssd13xx->reset, 0);
 	udelay(4);
 }
 
-static int ssd130x_power_on(struct ssd130x_device *ssd130x)
+static int ssd13xx_power_on(struct ssd13xx_device *ssd13xx)
 {
-	struct device *dev = ssd130x->dev;
+	struct device *dev = ssd13xx->dev;
 	int ret;
 
-	ssd130x_reset(ssd130x);
+	ssd13xx_reset(ssd13xx);
 
-	ret = regulator_enable(ssd130x->vcc_reg);
+	ret = regulator_enable(ssd13xx->vcc_reg);
 	if (ret) {
 		dev_err(dev, "Failed to enable VCC: %d\n", ret);
 		return ret;
 	}
 
-	if (ssd130x->device_info->need_pwm) {
-		ret = ssd130x_pwm_enable(ssd130x);
+	if (ssd13xx->device_info->need_pwm) {
+		ret = ssd13xx_pwm_enable(ssd13xx);
 		if (ret) {
 			dev_err(dev, "Failed to enable PWM: %d\n", ret);
-			regulator_disable(ssd130x->vcc_reg);
+			regulator_disable(ssd13xx->vcc_reg);
 			return ret;
 		}
 	}
@@ -324,75 +324,75 @@ static int ssd130x_power_on(struct ssd130x_device *ssd130x)
 	return 0;
 }
 
-static void ssd130x_power_off(struct ssd130x_device *ssd130x)
+static void ssd13xx_power_off(struct ssd13xx_device *ssd13xx)
 {
-	pwm_disable(ssd130x->pwm);
-	pwm_put(ssd130x->pwm);
+	pwm_disable(ssd13xx->pwm);
+	pwm_put(ssd13xx->pwm);
 
-	regulator_disable(ssd130x->vcc_reg);
+	regulator_disable(ssd13xx->vcc_reg);
 }
 
-static int ssd130x_init(struct ssd130x_device *ssd130x)
+static int ssd13xx_init(struct ssd13xx_device *ssd13xx)
 {
 	u32 precharge, dclk, com_invdir, compins, chargepump, seg_remap;
 	bool scan_mode;
 	int ret;
 
 	/* Set initial contrast */
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_CONTRAST, ssd130x->contrast);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_CONTRAST, ssd13xx->contrast);
 	if (ret < 0)
 		return ret;
 
 	/* Set segment re-map */
 	seg_remap = (SSD130X_SET_SEG_REMAP |
-		     SSD130X_SET_SEG_REMAP_SET(ssd130x->seg_remap));
-	ret = ssd130x_write_cmd(ssd130x, 1, seg_remap);
+		     SSD130X_SET_SEG_REMAP_SET(ssd13xx->seg_remap));
+	ret = ssd13xx_write_cmd(ssd13xx, 1, seg_remap);
 	if (ret < 0)
 		return ret;
 
 	/* Set COM direction */
 	com_invdir = (SSD130X_SET_COM_SCAN_DIR |
-		      SSD130X_SET_COM_SCAN_DIR_SET(ssd130x->com_invdir));
-	ret = ssd130x_write_cmd(ssd130x,  1, com_invdir);
+		      SSD130X_SET_COM_SCAN_DIR_SET(ssd13xx->com_invdir));
+	ret = ssd13xx_write_cmd(ssd13xx,  1, com_invdir);
 	if (ret < 0)
 		return ret;
 
 	/* Set multiplex ratio value */
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_MULTIPLEX_RATIO, ssd130x->height - 1);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_MULTIPLEX_RATIO, ssd13xx->height - 1);
 	if (ret < 0)
 		return ret;
 
 	/* set display offset value */
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_DISPLAY_OFFSET, ssd130x->com_offset);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_DISPLAY_OFFSET, ssd13xx->com_offset);
 	if (ret < 0)
 		return ret;
 
 	/* Set clock frequency */
-	dclk = (SSD130X_SET_CLOCK_DIV_SET(ssd130x->dclk_div - 1) |
-		SSD130X_SET_CLOCK_FREQ_SET(ssd130x->dclk_frq));
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_CLOCK_FREQ, dclk);
+	dclk = (SSD130X_SET_CLOCK_DIV_SET(ssd13xx->dclk_div - 1) |
+		SSD130X_SET_CLOCK_FREQ_SET(ssd13xx->dclk_frq));
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_CLOCK_FREQ, dclk);
 	if (ret < 0)
 		return ret;
 
 	/* Set Area Color Mode ON/OFF & Low Power Display Mode */
-	if (ssd130x->area_color_enable || ssd130x->low_power) {
+	if (ssd13xx->area_color_enable || ssd13xx->low_power) {
 		u32 mode = 0;
 
-		if (ssd130x->area_color_enable)
+		if (ssd13xx->area_color_enable)
 			mode |= SSD130X_SET_AREA_COLOR_MODE_ENABLE;
 
-		if (ssd130x->low_power)
+		if (ssd13xx->low_power)
 			mode |= SSD130X_SET_AREA_COLOR_MODE_LOW_POWER;
 
-		ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_AREA_COLOR_MODE, mode);
+		ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_AREA_COLOR_MODE, mode);
 		if (ret < 0)
 			return ret;
 	}
 
 	/* Set precharge period in number of ticks from the internal clock */
-	precharge = (SSD130X_SET_PRECHARGE_PERIOD1_SET(ssd130x->prechargep1) |
-		     SSD130X_SET_PRECHARGE_PERIOD2_SET(ssd130x->prechargep2));
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_PRECHARGE_PERIOD, precharge);
+	precharge = (SSD130X_SET_PRECHARGE_PERIOD1_SET(ssd13xx->prechargep1) |
+		     SSD130X_SET_PRECHARGE_PERIOD2_SET(ssd13xx->prechargep2));
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_PRECHARGE_PERIOD, precharge);
 	if (ret < 0)
 		return ret;
 
@@ -403,60 +403,60 @@ static int ssd130x_init(struct ssd130x_device *ssd130x)
 	 * property "solomon,com-seq". The value 0b means scan from COM0 to
 	 * COM[N - 1] while 1b means scan from COM[N - 1] to COM0.
 	 */
-	scan_mode = !ssd130x->com_seq;
+	scan_mode = !ssd13xx->com_seq;
 	compins |= (SSD130X_SET_COM_PINS_CONFIG1_SET(scan_mode) |
-		    SSD130X_SET_COM_PINS_CONFIG2_SET(ssd130x->com_lrremap));
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_COM_PINS_CONFIG, compins);
+		    SSD130X_SET_COM_PINS_CONFIG2_SET(ssd13xx->com_lrremap));
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_COM_PINS_CONFIG, compins);
 	if (ret < 0)
 		return ret;
 
 	/* Set VCOMH */
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_VCOMH, ssd130x->vcomh);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_VCOMH, ssd13xx->vcomh);
 	if (ret < 0)
 		return ret;
 
 	/* Turn on the DC-DC Charge Pump */
 	chargepump = BIT(4);
 
-	if (ssd130x->device_info->need_chargepump)
+	if (ssd13xx->device_info->need_chargepump)
 		chargepump |= BIT(2);
 
-	ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_CHARGE_PUMP, chargepump);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_CHARGE_PUMP, chargepump);
 	if (ret < 0)
 		return ret;
 
 	/* Set lookup table */
-	if (ssd130x->lookup_table_set) {
+	if (ssd13xx->lookup_table_set) {
 		int i;
 
-		ret = ssd130x_write_cmd(ssd130x, 1, SSD130X_SET_LOOKUP_TABLE);
+		ret = ssd13xx_write_cmd(ssd13xx, 1, SSD130X_SET_LOOKUP_TABLE);
 		if (ret < 0)
 			return ret;
 
-		for (i = 0; i < ARRAY_SIZE(ssd130x->lookup_table); i++) {
-			u8 val = ssd130x->lookup_table[i];
+		for (i = 0; i < ARRAY_SIZE(ssd13xx->lookup_table); i++) {
+			u8 val = ssd13xx->lookup_table[i];
 
 			if (val < 31 || val > 63)
-				dev_warn(ssd130x->dev,
+				dev_warn(ssd13xx->dev,
 					 "lookup table index %d value out of range 31 <= %d <= 63\n",
 					 i, val);
-			ret = ssd130x_write_cmd(ssd130x, 1, val);
+			ret = ssd13xx_write_cmd(ssd13xx, 1, val);
 			if (ret < 0)
 				return ret;
 		}
 	}
 
 	/* Switch to page addressing mode */
-	if (ssd130x->page_address_mode)
-		return ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_ADDRESS_MODE,
+	if (ssd13xx->page_address_mode)
+		return ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_ADDRESS_MODE,
 					 SSD130X_SET_ADDRESS_MODE_PAGE);
 
 	/* Switch to horizontal addressing mode */
-	return ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_ADDRESS_MODE,
+	return ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_ADDRESS_MODE,
 				 SSD130X_SET_ADDRESS_MODE_HORIZONTAL);
 }
 
-static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
+static int ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
 			       struct drm_rect *rect, u8 *buf,
 			       u8 *data_array)
 {
@@ -465,9 +465,9 @@ 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 = ssd13xx->device_info->page_height;
 	unsigned int pages = DIV_ROUND_UP(height, page_height);
-	struct drm_device *drm = &ssd130x->drm;
+	struct drm_device *drm = &ssd13xx->drm;
 	u32 array_idx = 0;
 	int ret, i, j, k;
 
@@ -502,13 +502,13 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 	 *  (5) A4 B4 C4 D4 E4 F4 G4 H4
 	 */
 
-	if (!ssd130x->page_address_mode) {
+	if (!ssd13xx->page_address_mode) {
 		/* Set address range for horizontal addressing mode */
-		ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset + x, width);
+		ret = ssd13xx_set_col_range(ssd13xx, ssd13xx->col_offset + x, width);
 		if (ret < 0)
 			return ret;
 
-		ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset + y / 8, pages);
+		ret = ssd13xx_set_page_range(ssd13xx, ssd13xx->page_offset + y / 8, pages);
 		if (ret < 0)
 			return ret;
 	}
@@ -517,8 +517,8 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 		int m = 8;
 
 		/* Last page may be partial */
-		if (8 * (y / 8 + i + 1) > ssd130x->height)
-			m = ssd130x->height % 8;
+		if (8 * (y / 8 + i + 1) > ssd13xx->height)
+			m = ssd13xx->height % 8;
 		for (j = 0; j < width; j++) {
 			u8 data = 0;
 
@@ -535,14 +535,14 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 		 * In page addressing mode, the start address needs to be reset,
 		 * and each page then needs to be written out separately.
 		 */
-		if (ssd130x->page_address_mode) {
-			ret = ssd130x_set_page_pos(ssd130x,
-						   ssd130x->page_offset + i,
-						   ssd130x->col_offset + x);
+		if (ssd13xx->page_address_mode) {
+			ret = ssd13xx_set_page_pos(ssd13xx,
+						   ssd13xx->page_offset + i,
+						   ssd13xx->col_offset + x);
 			if (ret < 0)
 				return ret;
 
-			ret = ssd130x_write_data(ssd130x, data_array, width);
+			ret = ssd13xx_write_data(ssd13xx, data_array, width);
 			if (ret < 0)
 				return ret;
 
@@ -551,33 +551,33 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 	}
 
 	/* Write out update in one go if we aren't using page addressing mode */
-	if (!ssd130x->page_address_mode)
-		ret = ssd130x_write_data(ssd130x, data_array, width * pages);
+	if (!ssd13xx->page_address_mode)
+		ret = ssd13xx_write_data(ssd13xx, data_array, width * pages);
 
 	return ret;
 }
 
-static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
+static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 {
-	unsigned int page_height = ssd130x->device_info->page_height;
-	unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height);
-	unsigned int width = ssd130x->width;
+	unsigned int page_height = ssd13xx->device_info->page_height;
+	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, page_height);
+	unsigned int width = ssd13xx->width;
 	int ret, i;
 
-	if (!ssd130x->page_address_mode) {
+	if (!ssd13xx->page_address_mode) {
 		memset(data_array, 0, width * pages);
 
 		/* Set address range for horizontal addressing mode */
-		ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset, width);
+		ret = ssd13xx_set_col_range(ssd13xx, ssd13xx->col_offset, width);
 		if (ret < 0)
 			return;
 
-		ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset, pages);
+		ret = ssd13xx_set_page_range(ssd13xx, ssd13xx->page_offset, pages);
 		if (ret < 0)
 			return;
 
 		/* Write out update in one go if we aren't using page addressing mode */
-		ssd130x_write_data(ssd130x, data_array, width * pages);
+		ssd13xx_write_data(ssd13xx, data_array, width * pages);
 	} else {
 		/*
 		 * In page addressing mode, the start address needs to be reset,
@@ -586,33 +586,33 @@ static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
 		memset(data_array, 0, width);
 
 		for (i = 0; i < pages; i++) {
-			ret = ssd130x_set_page_pos(ssd130x,
-						   ssd130x->page_offset + i,
-						   ssd130x->col_offset);
+			ret = ssd13xx_set_page_pos(ssd13xx,
+						   ssd13xx->page_offset + i,
+						   ssd13xx->col_offset);
 			if (ret < 0)
 				return;
 
-			ret = ssd130x_write_data(ssd130x, data_array, width);
+			ret = ssd13xx_write_data(ssd13xx, data_array, width);
 			if (ret < 0)
 				return;
 		}
 	}
 }
 
-static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb,
+static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 				const struct iosys_map *vmap,
 				struct drm_rect *rect,
 				u8 *buf, u8 *data_array)
 {
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
-	unsigned int page_height = ssd130x->device_info->page_height;
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(fb->dev);
+	unsigned int page_height = ssd13xx->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->y2 = min_t(unsigned int, round_up(rect->y2, page_height), ssd13xx->height);
 
 	dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
 
@@ -625,18 +625,18 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb,
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
-	ssd130x_update_rect(ssd130x, rect, buf, data_array);
+	ssd13xx_update_rect(ssd13xx, rect, buf, data_array);
 
 	return ret;
 }
 
-static int ssd130x_primary_plane_atomic_check(struct drm_plane *plane,
+static int ssd13xx_primary_plane_atomic_check(struct drm_plane *plane,
 					      struct drm_atomic_state *state)
 {
 	struct drm_device *drm = plane->dev;
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
-	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(plane_state);
+	struct ssd13xx_plane_state *ssd13xx_state = to_ssd13xx_plane_state(plane_state);
 	struct drm_crtc *crtc = plane_state->crtc;
 	struct drm_crtc_state *crtc_state;
 	const struct drm_format_info *fi;
@@ -658,24 +658,24 @@ static int ssd130x_primary_plane_atomic_check(struct drm_plane *plane,
 	if (!fi)
 		return -EINVAL;
 
-	pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width);
+	pitch = drm_format_info_min_pitch(fi, 0, ssd13xx->width);
 
-	ssd130x_state->buffer = kcalloc(pitch, ssd130x->height, GFP_KERNEL);
-	if (!ssd130x_state->buffer)
+	ssd13xx_state->buffer = kcalloc(pitch, ssd13xx->height, GFP_KERNEL);
+	if (!ssd13xx_state->buffer)
 		return -ENOMEM;
 
 	return 0;
 }
 
-static void ssd130x_primary_plane_atomic_update(struct drm_plane *plane,
+static void ssd13xx_primary_plane_atomic_update(struct drm_plane *plane,
 						struct drm_atomic_state *state)
 {
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
-	struct ssd130x_crtc_state *ssd130x_crtc_state =  to_ssd130x_crtc_state(crtc_state);
-	struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
+	struct ssd13xx_crtc_state *ssd13xx_crtc_state =  to_ssd13xx_crtc_state(crtc_state);
+	struct ssd13xx_plane_state *ssd13xx_plane_state = to_ssd13xx_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_device *drm = plane->dev;
@@ -693,187 +693,187 @@ static void ssd130x_primary_plane_atomic_update(struct drm_plane *plane,
 		if (!drm_rect_intersect(&dst_clip, &damage))
 			continue;
 
-		ssd130x_fb_blit_rect(fb, &shadow_plane_state->data[0], &dst_clip,
-				     ssd130x_plane_state->buffer,
-				     ssd130x_crtc_state->data_array);
+		ssd13xx_fb_blit_rect(fb, &shadow_plane_state->data[0], &dst_clip,
+				     ssd13xx_plane_state->buffer,
+				     ssd13xx_crtc_state->data_array);
 	}
 
 	drm_dev_exit(idx);
 }
 
-static void ssd130x_primary_plane_atomic_disable(struct drm_plane *plane,
+static void ssd13xx_primary_plane_atomic_disable(struct drm_plane *plane,
 						 struct drm_atomic_state *state)
 {
 	struct drm_device *drm = plane->dev;
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 	struct drm_crtc_state *crtc_state;
-	struct ssd130x_crtc_state *ssd130x_crtc_state;
+	struct ssd13xx_crtc_state *ssd13xx_crtc_state;
 	int idx;
 
 	if (!plane_state->crtc)
 		return;
 
 	crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
-	ssd130x_crtc_state = to_ssd130x_crtc_state(crtc_state);
+	ssd13xx_crtc_state = to_ssd13xx_crtc_state(crtc_state);
 
 	if (!drm_dev_enter(drm, &idx))
 		return;
 
-	ssd130x_clear_screen(ssd130x, ssd130x_crtc_state->data_array);
+	ssd13xx_clear_screen(ssd13xx, ssd13xx_crtc_state->data_array);
 
 	drm_dev_exit(idx);
 }
 
 /* Called during init to allocate the plane's atomic state. */
-static void ssd130x_primary_plane_reset(struct drm_plane *plane)
+static void ssd13xx_primary_plane_reset(struct drm_plane *plane)
 {
-	struct ssd130x_plane_state *ssd130x_state;
+	struct ssd13xx_plane_state *ssd13xx_state;
 
 	WARN_ON(plane->state);
 
-	ssd130x_state = kzalloc(sizeof(*ssd130x_state), GFP_KERNEL);
-	if (!ssd130x_state)
+	ssd13xx_state = kzalloc(sizeof(*ssd13xx_state), GFP_KERNEL);
+	if (!ssd13xx_state)
 		return;
 
-	__drm_gem_reset_shadow_plane(plane, &ssd130x_state->base);
+	__drm_gem_reset_shadow_plane(plane, &ssd13xx_state->base);
 }
 
-static struct drm_plane_state *ssd130x_primary_plane_duplicate_state(struct drm_plane *plane)
+static struct drm_plane_state *ssd13xx_primary_plane_duplicate_state(struct drm_plane *plane)
 {
 	struct drm_shadow_plane_state *new_shadow_plane_state;
-	struct ssd130x_plane_state *old_ssd130x_state;
-	struct ssd130x_plane_state *ssd130x_state;
+	struct ssd13xx_plane_state *old_ssd13xx_state;
+	struct ssd13xx_plane_state *ssd13xx_state;
 
 	if (WARN_ON(!plane->state))
 		return NULL;
 
-	old_ssd130x_state = to_ssd130x_plane_state(plane->state);
-	ssd130x_state = kmemdup(old_ssd130x_state, sizeof(*ssd130x_state), GFP_KERNEL);
-	if (!ssd130x_state)
+	old_ssd13xx_state = to_ssd13xx_plane_state(plane->state);
+	ssd13xx_state = kmemdup(old_ssd13xx_state, sizeof(*ssd13xx_state), GFP_KERNEL);
+	if (!ssd13xx_state)
 		return NULL;
 
 	/* The buffer is not duplicated and is allocated in .atomic_check */
-	ssd130x_state->buffer = NULL;
+	ssd13xx_state->buffer = NULL;
 
-	new_shadow_plane_state = &ssd130x_state->base;
+	new_shadow_plane_state = &ssd13xx_state->base;
 
 	__drm_gem_duplicate_shadow_plane_state(plane, new_shadow_plane_state);
 
 	return &new_shadow_plane_state->base;
 }
 
-static void ssd130x_primary_plane_destroy_state(struct drm_plane *plane,
+static void ssd13xx_primary_plane_destroy_state(struct drm_plane *plane,
 						struct drm_plane_state *state)
 {
-	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state);
+	struct ssd13xx_plane_state *ssd13xx_state = to_ssd13xx_plane_state(state);
 
-	kfree(ssd130x_state->buffer);
+	kfree(ssd13xx_state->buffer);
 
-	__drm_gem_destroy_shadow_plane_state(&ssd130x_state->base);
+	__drm_gem_destroy_shadow_plane_state(&ssd13xx_state->base);
 
-	kfree(ssd130x_state);
+	kfree(ssd13xx_state);
 }
 
-static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs = {
+static const struct drm_plane_helper_funcs ssd13xx_primary_plane_helper_funcs = {
 	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
-	.atomic_check = ssd130x_primary_plane_atomic_check,
-	.atomic_update = ssd130x_primary_plane_atomic_update,
-	.atomic_disable = ssd130x_primary_plane_atomic_disable,
+	.atomic_check = ssd13xx_primary_plane_atomic_check,
+	.atomic_update = ssd13xx_primary_plane_atomic_update,
+	.atomic_disable = ssd13xx_primary_plane_atomic_disable,
 };
 
-static const struct drm_plane_funcs ssd130x_primary_plane_funcs = {
+static const struct drm_plane_funcs ssd13xx_primary_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.reset = ssd130x_primary_plane_reset,
-	.atomic_duplicate_state = ssd130x_primary_plane_duplicate_state,
-	.atomic_destroy_state = ssd130x_primary_plane_destroy_state,
+	.reset = ssd13xx_primary_plane_reset,
+	.atomic_duplicate_state = ssd13xx_primary_plane_duplicate_state,
+	.atomic_destroy_state = ssd13xx_primary_plane_destroy_state,
 	.destroy = drm_plane_cleanup,
 };
 
-static enum drm_mode_status ssd130x_crtc_mode_valid(struct drm_crtc *crtc,
+static enum drm_mode_status ssd13xx_crtc_mode_valid(struct drm_crtc *crtc,
 						    const struct drm_display_mode *mode)
 {
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(crtc->dev);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(crtc->dev);
 
-	if (mode->hdisplay != ssd130x->mode.hdisplay &&
-	    mode->vdisplay != ssd130x->mode.vdisplay)
+	if (mode->hdisplay != ssd13xx->mode.hdisplay &&
+	    mode->vdisplay != ssd13xx->mode.vdisplay)
 		return MODE_ONE_SIZE;
-	else if (mode->hdisplay != ssd130x->mode.hdisplay)
+	else if (mode->hdisplay != ssd13xx->mode.hdisplay)
 		return MODE_ONE_WIDTH;
-	else if (mode->vdisplay != ssd130x->mode.vdisplay)
+	else if (mode->vdisplay != ssd13xx->mode.vdisplay)
 		return MODE_ONE_HEIGHT;
 
 	return MODE_OK;
 }
 
-static int ssd130x_crtc_atomic_check(struct drm_crtc *crtc,
+static int ssd13xx_crtc_atomic_check(struct drm_crtc *crtc,
 				     struct drm_atomic_state *state)
 {
 	struct drm_device *drm = crtc->dev;
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(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);
+	struct ssd13xx_crtc_state *ssd13xx_state = to_ssd13xx_crtc_state(crtc_state);
+	unsigned int page_height = ssd13xx->device_info->page_height;
+	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, page_height);
 	int ret;
 
 	ret = drm_crtc_helper_atomic_check(crtc, state);
 	if (ret)
 		return ret;
 
-	ssd130x_state->data_array = kmalloc(ssd130x->width * pages, GFP_KERNEL);
-	if (!ssd130x_state->data_array)
+	ssd13xx_state->data_array = kmalloc(ssd13xx->width * pages, GFP_KERNEL);
+	if (!ssd13xx_state->data_array)
 		return -ENOMEM;
 
 	return 0;
 }
 
 /* Called during init to allocate the CRTC's atomic state. */
-static void ssd130x_crtc_reset(struct drm_crtc *crtc)
+static void ssd13xx_crtc_reset(struct drm_crtc *crtc)
 {
-	struct ssd130x_crtc_state *ssd130x_state;
+	struct ssd13xx_crtc_state *ssd13xx_state;
 
 	WARN_ON(crtc->state);
 
-	ssd130x_state = kzalloc(sizeof(*ssd130x_state), GFP_KERNEL);
-	if (!ssd130x_state)
+	ssd13xx_state = kzalloc(sizeof(*ssd13xx_state), GFP_KERNEL);
+	if (!ssd13xx_state)
 		return;
 
-	__drm_atomic_helper_crtc_reset(crtc, &ssd130x_state->base);
+	__drm_atomic_helper_crtc_reset(crtc, &ssd13xx_state->base);
 }
 
-static struct drm_crtc_state *ssd130x_crtc_duplicate_state(struct drm_crtc *crtc)
+static struct drm_crtc_state *ssd13xx_crtc_duplicate_state(struct drm_crtc *crtc)
 {
-	struct ssd130x_crtc_state *old_ssd130x_state;
-	struct ssd130x_crtc_state *ssd130x_state;
+	struct ssd13xx_crtc_state *old_ssd13xx_state;
+	struct ssd13xx_crtc_state *ssd13xx_state;
 
 	if (WARN_ON(!crtc->state))
 		return NULL;
 
-	old_ssd130x_state = to_ssd130x_crtc_state(crtc->state);
-	ssd130x_state = kmemdup(old_ssd130x_state, sizeof(*ssd130x_state), GFP_KERNEL);
-	if (!ssd130x_state)
+	old_ssd13xx_state = to_ssd13xx_crtc_state(crtc->state);
+	ssd13xx_state = kmemdup(old_ssd13xx_state, sizeof(*ssd13xx_state), GFP_KERNEL);
+	if (!ssd13xx_state)
 		return NULL;
 
 	/* The buffer is not duplicated and is allocated in .atomic_check */
-	ssd130x_state->data_array = NULL;
+	ssd13xx_state->data_array = NULL;
 
-	__drm_atomic_helper_crtc_duplicate_state(crtc, &ssd130x_state->base);
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &ssd13xx_state->base);
 
-	return &ssd130x_state->base;
+	return &ssd13xx_state->base;
 }
 
-static void ssd130x_crtc_destroy_state(struct drm_crtc *crtc,
+static void ssd13xx_crtc_destroy_state(struct drm_crtc *crtc,
 				       struct drm_crtc_state *state)
 {
-	struct ssd130x_crtc_state *ssd130x_state = to_ssd130x_crtc_state(state);
+	struct ssd13xx_crtc_state *ssd13xx_state = to_ssd13xx_crtc_state(state);
 
-	kfree(ssd130x_state->data_array);
+	kfree(ssd13xx_state->data_array);
 
 	__drm_atomic_helper_crtc_destroy_state(state);
 
-	kfree(ssd130x_state);
+	kfree(ssd13xx_state);
 }
 
 /*
@@ -881,75 +881,75 @@ static void ssd130x_crtc_destroy_state(struct drm_crtc *crtc,
  * the primary plane's atomic_update function. Disabling clears
  * the screen in the primary plane's atomic_disable function.
  */
-static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs = {
-	.mode_valid = ssd130x_crtc_mode_valid,
-	.atomic_check = ssd130x_crtc_atomic_check,
+static const struct drm_crtc_helper_funcs ssd13xx_crtc_helper_funcs = {
+	.mode_valid = ssd13xx_crtc_mode_valid,
+	.atomic_check = ssd13xx_crtc_atomic_check,
 };
 
-static const struct drm_crtc_funcs ssd130x_crtc_funcs = {
-	.reset = ssd130x_crtc_reset,
+static const struct drm_crtc_funcs ssd13xx_crtc_funcs = {
+	.reset = ssd13xx_crtc_reset,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
-	.atomic_duplicate_state = ssd130x_crtc_duplicate_state,
-	.atomic_destroy_state = ssd130x_crtc_destroy_state,
+	.atomic_duplicate_state = ssd13xx_crtc_duplicate_state,
+	.atomic_destroy_state = ssd13xx_crtc_destroy_state,
 };
 
-static void ssd130x_encoder_atomic_enable(struct drm_encoder *encoder,
+static void ssd13xx_encoder_atomic_enable(struct drm_encoder *encoder,
 					  struct drm_atomic_state *state)
 {
 	struct drm_device *drm = encoder->dev;
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	int ret;
 
-	ret = ssd130x_power_on(ssd130x);
+	ret = ssd13xx_power_on(ssd13xx);
 	if (ret)
 		return;
 
-	ret = ssd130x_init(ssd130x);
+	ret = ssd13xx_init(ssd13xx);
 	if (ret)
 		goto power_off;
 
-	ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_ON);
+	ssd13xx_write_cmd(ssd13xx, 1, SSD130X_DISPLAY_ON);
 
-	backlight_enable(ssd130x->bl_dev);
+	backlight_enable(ssd13xx->bl_dev);
 
 	return;
 
 power_off:
-	ssd130x_power_off(ssd130x);
+	ssd13xx_power_off(ssd13xx);
 	return;
 }
 
-static void ssd130x_encoder_atomic_disable(struct drm_encoder *encoder,
+static void ssd13xx_encoder_atomic_disable(struct drm_encoder *encoder,
 					   struct drm_atomic_state *state)
 {
 	struct drm_device *drm = encoder->dev;
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 
-	backlight_disable(ssd130x->bl_dev);
+	backlight_disable(ssd13xx->bl_dev);
 
-	ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_OFF);
+	ssd13xx_write_cmd(ssd13xx, 1, SSD130X_DISPLAY_OFF);
 
-	ssd130x_power_off(ssd130x);
+	ssd13xx_power_off(ssd13xx);
 }
 
-static const struct drm_encoder_helper_funcs ssd130x_encoder_helper_funcs = {
-	.atomic_enable = ssd130x_encoder_atomic_enable,
-	.atomic_disable = ssd130x_encoder_atomic_disable,
+static const struct drm_encoder_helper_funcs ssd13xx_encoder_helper_funcs = {
+	.atomic_enable = ssd13xx_encoder_atomic_enable,
+	.atomic_disable = ssd13xx_encoder_atomic_disable,
 };
 
-static const struct drm_encoder_funcs ssd130x_encoder_funcs = {
+static const struct drm_encoder_funcs ssd13xx_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
-static int ssd130x_connector_get_modes(struct drm_connector *connector)
+static int ssd13xx_connector_get_modes(struct drm_connector *connector)
 {
-	struct ssd130x_device *ssd130x = drm_to_ssd130x(connector->dev);
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(connector->dev);
 	struct drm_display_mode *mode;
-	struct device *dev = ssd130x->dev;
+	struct device *dev = ssd13xx->dev;
 
-	mode = drm_mode_duplicate(connector->dev, &ssd130x->mode);
+	mode = drm_mode_duplicate(connector->dev, &ssd13xx->mode);
 	if (!mode) {
 		dev_err(dev, "Failed to duplicated mode\n");
 		return 0;
@@ -962,11 +962,11 @@ static int ssd130x_connector_get_modes(struct drm_connector *connector)
 	return 1;
 }
 
-static const struct drm_connector_helper_funcs ssd130x_connector_helper_funcs = {
-	.get_modes = ssd130x_connector_get_modes,
+static const struct drm_connector_helper_funcs ssd13xx_connector_helper_funcs = {
+	.get_modes = ssd13xx_connector_get_modes,
 };
 
-static const struct drm_connector_funcs ssd130x_connector_funcs = {
+static const struct drm_connector_funcs ssd13xx_connector_funcs = {
 	.reset = drm_atomic_helper_connector_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = drm_connector_cleanup,
@@ -974,19 +974,19 @@ static const struct drm_connector_funcs ssd130x_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static const struct drm_mode_config_funcs ssd130x_mode_config_funcs = {
+static const struct drm_mode_config_funcs ssd13xx_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create_with_dirty,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
-static const uint32_t ssd130x_formats[] = {
+static const uint32_t ssd13xx_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
 
-DEFINE_DRM_GEM_FOPS(ssd130x_fops);
+DEFINE_DRM_GEM_FOPS(ssd13xx_fops);
 
-static const struct drm_driver ssd130x_drm_driver = {
+static const struct drm_driver ssd13xx_drm_driver = {
 	DRM_GEM_SHMEM_DRIVER_OPS,
 	.name			= DRIVER_NAME,
 	.desc			= DRIVER_DESC,
@@ -994,85 +994,85 @@ static const struct drm_driver ssd130x_drm_driver = {
 	.major			= DRIVER_MAJOR,
 	.minor			= DRIVER_MINOR,
 	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
-	.fops			= &ssd130x_fops,
+	.fops			= &ssd13xx_fops,
 };
 
-static int ssd130x_update_bl(struct backlight_device *bdev)
+static int ssd13xx_update_bl(struct backlight_device *bdev)
 {
-	struct ssd130x_device *ssd130x = bl_get_data(bdev);
+	struct ssd13xx_device *ssd13xx = bl_get_data(bdev);
 	int brightness = backlight_get_brightness(bdev);
 	int ret;
 
-	ssd130x->contrast = brightness;
+	ssd13xx->contrast = brightness;
 
-	ret = ssd130x_write_cmd(ssd130x, 1, SSD130X_CONTRAST);
+	ret = ssd13xx_write_cmd(ssd13xx, 1, SSD130X_CONTRAST);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd130x_write_cmd(ssd130x, 1, ssd130x->contrast);
+	ret = ssd13xx_write_cmd(ssd13xx, 1, ssd13xx->contrast);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
 
-static const struct backlight_ops ssd130xfb_bl_ops = {
-	.update_status	= ssd130x_update_bl,
+static const struct backlight_ops ssd13xxfb_bl_ops = {
+	.update_status	= ssd13xx_update_bl,
 };
 
-static void ssd130x_parse_properties(struct ssd130x_device *ssd130x)
+static void ssd13xx_parse_properties(struct ssd13xx_device *ssd13xx)
 {
-	struct device *dev = ssd130x->dev;
+	struct device *dev = ssd13xx->dev;
 
-	if (device_property_read_u32(dev, "solomon,width", &ssd130x->width))
-		ssd130x->width = ssd130x->device_info->default_width;
+	if (device_property_read_u32(dev, "solomon,width", &ssd13xx->width))
+		ssd13xx->width = ssd13xx->device_info->default_width;
 
-	if (device_property_read_u32(dev, "solomon,height", &ssd130x->height))
-		ssd130x->height = ssd130x->device_info->default_height;
+	if (device_property_read_u32(dev, "solomon,height", &ssd13xx->height))
+		ssd13xx->height = ssd13xx->device_info->default_height;
 
-	if (device_property_read_u32(dev, "solomon,page-offset", &ssd130x->page_offset))
-		ssd130x->page_offset = 1;
+	if (device_property_read_u32(dev, "solomon,page-offset", &ssd13xx->page_offset))
+		ssd13xx->page_offset = 1;
 
-	if (device_property_read_u32(dev, "solomon,col-offset", &ssd130x->col_offset))
-		ssd130x->col_offset = 0;
+	if (device_property_read_u32(dev, "solomon,col-offset", &ssd13xx->col_offset))
+		ssd13xx->col_offset = 0;
 
-	if (device_property_read_u32(dev, "solomon,com-offset", &ssd130x->com_offset))
-		ssd130x->com_offset = 0;
+	if (device_property_read_u32(dev, "solomon,com-offset", &ssd13xx->com_offset))
+		ssd13xx->com_offset = 0;
 
-	if (device_property_read_u32(dev, "solomon,prechargep1", &ssd130x->prechargep1))
-		ssd130x->prechargep1 = 2;
+	if (device_property_read_u32(dev, "solomon,prechargep1", &ssd13xx->prechargep1))
+		ssd13xx->prechargep1 = 2;
 
-	if (device_property_read_u32(dev, "solomon,prechargep2", &ssd130x->prechargep2))
-		ssd130x->prechargep2 = 2;
+	if (device_property_read_u32(dev, "solomon,prechargep2", &ssd13xx->prechargep2))
+		ssd13xx->prechargep2 = 2;
 
 	if (!device_property_read_u8_array(dev, "solomon,lookup-table",
-					   ssd130x->lookup_table,
-					   ARRAY_SIZE(ssd130x->lookup_table)))
-		ssd130x->lookup_table_set = 1;
-
-	ssd130x->seg_remap = !device_property_read_bool(dev, "solomon,segment-no-remap");
-	ssd130x->com_seq = device_property_read_bool(dev, "solomon,com-seq");
-	ssd130x->com_lrremap = device_property_read_bool(dev, "solomon,com-lrremap");
-	ssd130x->com_invdir = device_property_read_bool(dev, "solomon,com-invdir");
-	ssd130x->area_color_enable =
+					   ssd13xx->lookup_table,
+					   ARRAY_SIZE(ssd13xx->lookup_table)))
+		ssd13xx->lookup_table_set = 1;
+
+	ssd13xx->seg_remap = !device_property_read_bool(dev, "solomon,segment-no-remap");
+	ssd13xx->com_seq = device_property_read_bool(dev, "solomon,com-seq");
+	ssd13xx->com_lrremap = device_property_read_bool(dev, "solomon,com-lrremap");
+	ssd13xx->com_invdir = device_property_read_bool(dev, "solomon,com-invdir");
+	ssd13xx->area_color_enable =
 		device_property_read_bool(dev, "solomon,area-color-enable");
-	ssd130x->low_power = device_property_read_bool(dev, "solomon,low-power");
+	ssd13xx->low_power = device_property_read_bool(dev, "solomon,low-power");
 
-	ssd130x->contrast = 127;
-	ssd130x->vcomh = ssd130x->device_info->default_vcomh;
+	ssd13xx->contrast = 127;
+	ssd13xx->vcomh = ssd13xx->device_info->default_vcomh;
 
 	/* Setup display timing */
-	if (device_property_read_u32(dev, "solomon,dclk-div", &ssd130x->dclk_div))
-		ssd130x->dclk_div = ssd130x->device_info->default_dclk_div;
-	if (device_property_read_u32(dev, "solomon,dclk-frq", &ssd130x->dclk_frq))
-		ssd130x->dclk_frq = ssd130x->device_info->default_dclk_frq;
+	if (device_property_read_u32(dev, "solomon,dclk-div", &ssd13xx->dclk_div))
+		ssd13xx->dclk_div = ssd13xx->device_info->default_dclk_div;
+	if (device_property_read_u32(dev, "solomon,dclk-frq", &ssd13xx->dclk_frq))
+		ssd13xx->dclk_frq = ssd13xx->device_info->default_dclk_frq;
 }
 
-static int ssd130x_init_modeset(struct ssd130x_device *ssd130x)
+static int ssd13xx_init_modeset(struct ssd13xx_device *ssd13xx)
 {
-	struct drm_display_mode *mode = &ssd130x->mode;
-	struct device *dev = ssd130x->dev;
-	struct drm_device *drm = &ssd130x->drm;
+	struct drm_display_mode *mode = &ssd13xx->mode;
+	struct device *dev = ssd13xx->dev;
+	struct drm_device *drm = &ssd13xx->drm;
 	unsigned long max_width, max_height;
 	struct drm_plane *primary_plane;
 	struct drm_crtc *crtc;
@@ -1092,10 +1092,10 @@ static int ssd130x_init_modeset(struct ssd130x_device *ssd130x)
 
 	mode->type = DRM_MODE_TYPE_DRIVER;
 	mode->clock = 1;
-	mode->hdisplay = mode->htotal = ssd130x->width;
-	mode->hsync_start = mode->hsync_end = ssd130x->width;
-	mode->vdisplay = mode->vtotal = ssd130x->height;
-	mode->vsync_start = mode->vsync_end = ssd130x->height;
+	mode->hdisplay = mode->htotal = ssd13xx->width;
+	mode->hsync_start = mode->hsync_end = ssd13xx->width;
+	mode->vdisplay = mode->vtotal = ssd13xx->height;
+	mode->vsync_start = mode->vsync_end = ssd13xx->height;
 	mode->width_mm = 27;
 	mode->height_mm = 27;
 
@@ -1107,60 +1107,60 @@ static int ssd130x_init_modeset(struct ssd130x_device *ssd130x)
 	drm->mode_config.min_height = mode->vdisplay;
 	drm->mode_config.max_height = max_height;
 	drm->mode_config.preferred_depth = 24;
-	drm->mode_config.funcs = &ssd130x_mode_config_funcs;
+	drm->mode_config.funcs = &ssd13xx_mode_config_funcs;
 
 	/* Primary plane */
 
-	primary_plane = &ssd130x->primary_plane;
-	ret = drm_universal_plane_init(drm, primary_plane, 0, &ssd130x_primary_plane_funcs,
-				       ssd130x_formats, ARRAY_SIZE(ssd130x_formats),
+	primary_plane = &ssd13xx->primary_plane;
+	ret = drm_universal_plane_init(drm, primary_plane, 0, &ssd13xx_primary_plane_funcs,
+				       ssd13xx_formats, ARRAY_SIZE(ssd13xx_formats),
 				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		dev_err(dev, "DRM primary plane init failed: %d\n", ret);
 		return ret;
 	}
 
-	drm_plane_helper_add(primary_plane, &ssd130x_primary_plane_helper_funcs);
+	drm_plane_helper_add(primary_plane, &ssd13xx_primary_plane_helper_funcs);
 
 	drm_plane_enable_fb_damage_clips(primary_plane);
 
 	/* CRTC */
 
-	crtc = &ssd130x->crtc;
+	crtc = &ssd13xx->crtc;
 	ret = drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
-					&ssd130x_crtc_funcs, NULL);
+					&ssd13xx_crtc_funcs, NULL);
 	if (ret) {
 		dev_err(dev, "DRM crtc init failed: %d\n", ret);
 		return ret;
 	}
 
-	drm_crtc_helper_add(crtc, &ssd130x_crtc_helper_funcs);
+	drm_crtc_helper_add(crtc, &ssd13xx_crtc_helper_funcs);
 
 	/* Encoder */
 
-	encoder = &ssd130x->encoder;
-	ret = drm_encoder_init(drm, encoder, &ssd130x_encoder_funcs,
+	encoder = &ssd13xx->encoder;
+	ret = drm_encoder_init(drm, encoder, &ssd13xx_encoder_funcs,
 			       DRM_MODE_ENCODER_NONE, NULL);
 	if (ret) {
 		dev_err(dev, "DRM encoder init failed: %d\n", ret);
 		return ret;
 	}
 
-	drm_encoder_helper_add(encoder, &ssd130x_encoder_helper_funcs);
+	drm_encoder_helper_add(encoder, &ssd13xx_encoder_helper_funcs);
 
 	encoder->possible_crtcs = drm_crtc_mask(crtc);
 
 	/* Connector */
 
-	connector = &ssd130x->connector;
-	ret = drm_connector_init(drm, connector, &ssd130x_connector_funcs,
+	connector = &ssd13xx->connector;
+	ret = drm_connector_init(drm, connector, &ssd13xx_connector_funcs,
 				 DRM_MODE_CONNECTOR_Unknown);
 	if (ret) {
 		dev_err(dev, "DRM connector init failed: %d\n", ret);
 		return ret;
 	}
 
-	drm_connector_helper_add(connector, &ssd130x_connector_helper_funcs);
+	drm_connector_helper_add(connector, &ssd13xx_connector_helper_funcs);
 
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret) {
@@ -1173,62 +1173,62 @@ static int ssd130x_init_modeset(struct ssd130x_device *ssd130x)
 	return 0;
 }
 
-static int ssd130x_get_resources(struct ssd130x_device *ssd130x)
+static int ssd13xx_get_resources(struct ssd13xx_device *ssd13xx)
 {
-	struct device *dev = ssd130x->dev;
+	struct device *dev = ssd13xx->dev;
 
-	ssd130x->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(ssd130x->reset))
-		return dev_err_probe(dev, PTR_ERR(ssd130x->reset),
+	ssd13xx->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ssd13xx->reset))
+		return dev_err_probe(dev, PTR_ERR(ssd13xx->reset),
 				     "Failed to get reset gpio\n");
 
-	ssd130x->vcc_reg = devm_regulator_get(dev, "vcc");
-	if (IS_ERR(ssd130x->vcc_reg))
-		return dev_err_probe(dev, PTR_ERR(ssd130x->vcc_reg),
+	ssd13xx->vcc_reg = devm_regulator_get(dev, "vcc");
+	if (IS_ERR(ssd13xx->vcc_reg))
+		return dev_err_probe(dev, PTR_ERR(ssd13xx->vcc_reg),
 				     "Failed to get VCC regulator\n");
 
 	return 0;
 }
 
-struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
+struct ssd13xx_device *ssd13xx_probe(struct device *dev, struct regmap *regmap)
 {
-	struct ssd130x_device *ssd130x;
+	struct ssd13xx_device *ssd13xx;
 	struct backlight_device *bl;
 	struct drm_device *drm;
 	int ret;
 
-	ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
-				     struct ssd130x_device, drm);
-	if (IS_ERR(ssd130x))
-		return ERR_PTR(dev_err_probe(dev, PTR_ERR(ssd130x),
+	ssd13xx = devm_drm_dev_alloc(dev, &ssd13xx_drm_driver,
+				     struct ssd13xx_device, drm);
+	if (IS_ERR(ssd13xx))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(ssd13xx),
 					     "Failed to allocate DRM device\n"));
 
-	drm = &ssd130x->drm;
+	drm = &ssd13xx->drm;
 
-	ssd130x->dev = dev;
-	ssd130x->regmap = regmap;
-	ssd130x->device_info = device_get_match_data(dev);
+	ssd13xx->dev = dev;
+	ssd13xx->regmap = regmap;
+	ssd13xx->device_info = device_get_match_data(dev);
 
-	if (ssd130x->device_info->page_mode_only)
-		ssd130x->page_address_mode = 1;
+	if (ssd13xx->device_info->page_mode_only)
+		ssd13xx->page_address_mode = 1;
 
-	ssd130x_parse_properties(ssd130x);
+	ssd13xx_parse_properties(ssd13xx);
 
-	ret = ssd130x_get_resources(ssd130x);
+	ret = ssd13xx_get_resources(ssd13xx);
 	if (ret)
 		return ERR_PTR(ret);
 
-	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
-					    &ssd130xfb_bl_ops, NULL);
+	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd13xx,
+					    &ssd13xxfb_bl_ops, NULL);
 	if (IS_ERR(bl))
 		return ERR_PTR(dev_err_probe(dev, PTR_ERR(bl),
 					     "Unable to register backlight device\n"));
 
-	bl->props.brightness = ssd130x->contrast;
+	bl->props.brightness = ssd13xx->contrast;
 	bl->props.max_brightness = MAX_CONTRAST;
-	ssd130x->bl_dev = bl;
+	ssd13xx->bl_dev = bl;
 
-	ret = ssd130x_init_modeset(ssd130x);
+	ret = ssd13xx_init_modeset(ssd13xx);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -1238,22 +1238,22 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 
 	drm_fbdev_generic_setup(drm, 32);
 
-	return ssd130x;
+	return ssd13xx;
 }
-EXPORT_SYMBOL_GPL(ssd130x_probe);
+EXPORT_SYMBOL_GPL(ssd13xx_probe);
 
-void ssd130x_remove(struct ssd130x_device *ssd130x)
+void ssd13xx_remove(struct ssd13xx_device *ssd13xx)
 {
-	drm_dev_unplug(&ssd130x->drm);
-	drm_atomic_helper_shutdown(&ssd130x->drm);
+	drm_dev_unplug(&ssd13xx->drm);
+	drm_atomic_helper_shutdown(&ssd13xx->drm);
 }
-EXPORT_SYMBOL_GPL(ssd130x_remove);
+EXPORT_SYMBOL_GPL(ssd13xx_remove);
 
-void ssd130x_shutdown(struct ssd130x_device *ssd130x)
+void ssd13xx_shutdown(struct ssd13xx_device *ssd13xx)
 {
-	drm_atomic_helper_shutdown(&ssd130x->drm);
+	drm_atomic_helper_shutdown(&ssd13xx->drm);
 }
-EXPORT_SYMBOL_GPL(ssd130x_shutdown);
+EXPORT_SYMBOL_GPL(ssd13xx_shutdown);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
diff --git a/drivers/gpu/drm/solomon/ssd13xx.h b/drivers/gpu/drm/solomon/ssd13xx.h
index f89ccd11cd29..e5abf23196b0 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -24,16 +24,16 @@
 #define SSD130X_DATA				0x40
 #define SSD130X_COMMAND				0x80
 
-enum ssd130x_variants {
+enum ssd13xx_variants {
 	SH1106_ID,
 	SSD1305_ID,
 	SSD1306_ID,
 	SSD1307_ID,
 	SSD1309_ID,
-	NR_SSD130X_VARIANTS
+	NR_SSD13XX_VARIANTS
 };
 
-struct ssd130x_deviceinfo {
+struct ssd13xx_deviceinfo {
 	u32 default_vcomh;
 	u32 default_dclk_div;
 	u32 default_dclk_frq;
@@ -45,7 +45,7 @@ struct ssd130x_deviceinfo {
 	bool page_mode_only;
 };
 
-struct ssd130x_device {
+struct ssd13xx_device {
 	struct drm_device drm;
 	struct device *dev;
 	struct drm_display_mode mode;
@@ -57,7 +57,7 @@ struct ssd130x_device {
 
 	struct regmap *regmap;
 
-	const struct ssd130x_deviceinfo *device_info;
+	const struct ssd13xx_deviceinfo *device_info;
 
 	unsigned page_address_mode : 1;
 	unsigned area_color_enable : 1;
@@ -91,10 +91,10 @@ struct ssd130x_device {
 	u8 page_end;
 };
 
-extern const struct ssd130x_deviceinfo ssd130x_variants[];
+extern const struct ssd13xx_deviceinfo ssd13xx_variants[];
 
-struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap);
-void ssd130x_remove(struct ssd130x_device *ssd130x);
-void ssd130x_shutdown(struct ssd130x_device *ssd130x);
+struct ssd13xx_device *ssd13xx_probe(struct device *dev, struct regmap *regmap);
+void ssd13xx_remove(struct ssd13xx_device *ssd13xx);
+void ssd13xx_shutdown(struct ssd13xx_device *ssd13xx);
 
 #endif /* __SSD13XX_H__ */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 2/8] drm/ssd13xx: Rename data structures and functions prefixes Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-11  7:39   ` Geert Uytterhoeven
  2023-10-09 18:34 ` [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch Javier Martinez Canillas
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

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 <javierm@redhat.com>
---

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

diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index 10a767fb614c..d29be17665b5 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -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 ssd13xx_deviceinfo ssd13xx_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 ssd13xx_deviceinfo ssd13xx_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 ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.need_chargepump = 1,
 		.default_width = 128,
 		.default_height = 64,
-		.page_height = 8,
 	},
 	[SSD1307_ID] = {
 		.default_vcomh = 0x20,
@@ -128,7 +127,6 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.need_pwm = 1,
 		.default_width = 128,
 		.default_height = 39,
-		.page_height = 8,
 	},
 	[SSD1309_ID] = {
 		.default_vcomh = 0x34,
@@ -136,7 +134,6 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.default_dclk_frq = 10,
 		.default_width = 128,
 		.default_height = 64,
-		.page_height = 8,
 	}
 };
 EXPORT_SYMBOL_NS_GPL(ssd13xx_variants, DRM_SSD13XX);
@@ -465,13 +462,13 @@ static int ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
 	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 = ssd13xx->device_info->page_height;
+	unsigned int page_height = SSD130X_PAGE_HEIGHT;
 	unsigned int pages = DIV_ROUND_UP(height, page_height);
 	struct drm_device *drm = &ssd13xx->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 ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
 	 */
 
 	if (!ssd13xx->page_address_mode) {
+		u8 page_start;
+
 		/* Set address range for horizontal addressing mode */
 		ret = ssd13xx_set_col_range(ssd13xx, ssd13xx->col_offset + x, width);
 		if (ret < 0)
 			return ret;
 
-		ret = ssd13xx_set_page_range(ssd13xx, ssd13xx->page_offset + y / 8, pages);
+		page_start = ssd13xx->page_offset + y / page_height;
+		ret = ssd13xx_set_page_range(ssd13xx, 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) > ssd13xx->height)
-			m = ssd13xx->height % 8;
+		if (page_height * (y / page_height + i + 1) > ssd13xx->height)
+			m = ssd13xx->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 ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
 
 static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 {
-	unsigned int page_height = ssd13xx->device_info->page_height;
-	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, page_height);
+	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
 	unsigned int width = ssd13xx->width;
 	int ret, i;
 
@@ -605,14 +606,13 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 				u8 *buf, u8 *data_array)
 {
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(fb->dev);
-	unsigned int page_height = ssd13xx->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), ssd13xx->height);
+	rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
+	rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd13xx->height);
 
 	dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
 
@@ -814,8 +814,7 @@ static int ssd13xx_crtc_atomic_check(struct drm_crtc *crtc,
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	struct ssd13xx_crtc_state *ssd13xx_state = to_ssd13xx_crtc_state(crtc_state);
-	unsigned int page_height = ssd13xx->device_info->page_height;
-	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, page_height);
+	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
 	int ret;
 
 	ret = drm_crtc_helper_atomic_check(crtc, state);
diff --git a/drivers/gpu/drm/solomon/ssd13xx.h b/drivers/gpu/drm/solomon/ssd13xx.h
index e5abf23196b0..64283935fbc1 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -39,7 +39,6 @@ struct ssd13xx_deviceinfo {
 	u32 default_dclk_frq;
 	u32 default_width;
 	u32 default_height;
-	u32 page_height;
 	bool need_pwm;
 	bool need_chargepump;
 	bool page_mode_only;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2023-10-09 18:34 ` [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-11  7:59   ` Geert Uytterhoeven
  2023-10-09 18:34 ` [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table Javier Martinez Canillas
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

Don't assume bpp of 1 and instead compute the destination pitch using the
intermediate buffer pixel format info when doing a format conversion.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/gpu/drm/solomon/ssd13xx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index d29be17665b5..9747f8656636 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -148,6 +148,8 @@ struct ssd13xx_plane_state {
 	struct drm_shadow_plane_state base;
 	/* Intermediate buffer to convert pixels from XRGB8888 to HW format */
 	u8 *buffer;
+	/* Pixel format info for the intermediate buffer */
+	const struct drm_format_info *fi;
 };
 
 static inline struct ssd13xx_crtc_state *to_ssd13xx_crtc_state(struct drm_crtc_state *state)
@@ -602,8 +604,9 @@ static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 
 static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 				const struct iosys_map *vmap,
-				struct drm_rect *rect,
-				u8 *buf, u8 *data_array)
+				struct drm_rect *rect, u8 *buf,
+				const struct drm_format_info *fi,
+				u8 *data_array)
 {
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(fb->dev);
 	struct iosys_map dst;
@@ -614,7 +617,7 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 	rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
 	rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd13xx->height);
 
-	dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
+	dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
 
 	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
 	if (ret)
@@ -664,6 +667,8 @@ static int ssd13xx_primary_plane_atomic_check(struct drm_plane *plane,
 	if (!ssd13xx_state->buffer)
 		return -ENOMEM;
 
+	ssd13xx_state->fi = fi;
+
 	return 0;
 }
 
@@ -695,6 +700,7 @@ static void ssd13xx_primary_plane_atomic_update(struct drm_plane *plane,
 
 		ssd13xx_fb_blit_rect(fb, &shadow_plane_state->data[0], &dst_clip,
 				     ssd13xx_plane_state->buffer,
+				     ssd13xx_plane_state->fi,
 				     ssd13xx_crtc_state->data_array);
 	}
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
                   ` (3 preceding siblings ...)
  2023-10-09 18:34 ` [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-11  8:09   ` Geert Uytterhoeven
  2023-10-09 18:34 ` [PATCH 6/8] drm/ssd13xx: Rename commands that are shared across chip families Javier Martinez Canillas
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

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.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/gpu/drm/solomon/ssd13xx.c | 88 ++++++++++++++++++++++---------
 drivers/gpu/drm/solomon/ssd13xx.h | 27 ++++++++++
 2 files changed, 90 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index 9747f8656636..5a426ac10c58 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -104,6 +104,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.default_width = 132,
 		.default_height = 64,
 		.page_mode_only = 1,
+		.family_id = SSD130X_FAMILY,
 	},
 	[SSD1305_ID] = {
 		.default_vcomh = 0x34,
@@ -111,6 +112,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.default_dclk_frq = 7,
 		.default_width = 132,
 		.default_height = 64,
+		.family_id = SSD130X_FAMILY,
 	},
 	[SSD1306_ID] = {
 		.default_vcomh = 0x20,
@@ -119,6 +121,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.need_chargepump = 1,
 		.default_width = 128,
 		.default_height = 64,
+		.family_id = SSD130X_FAMILY,
 	},
 	[SSD1307_ID] = {
 		.default_vcomh = 0x20,
@@ -127,6 +130,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.need_pwm = 1,
 		.default_width = 128,
 		.default_height = 39,
+		.family_id = SSD130X_FAMILY,
 	},
 	[SSD1309_ID] = {
 		.default_vcomh = 0x34,
@@ -134,6 +138,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.default_dclk_frq = 10,
 		.default_width = 128,
 		.default_height = 64,
+		.family_id = SSD130X_FAMILY,
 	}
 };
 EXPORT_SYMBOL_NS_GPL(ssd13xx_variants, DRM_SSD13XX);
@@ -148,8 +153,6 @@ struct ssd13xx_plane_state {
 	struct drm_shadow_plane_state base;
 	/* Intermediate buffer to convert pixels from XRGB8888 to HW format */
 	u8 *buffer;
-	/* Pixel format info for the intermediate buffer */
-	const struct drm_format_info *fi;
 };
 
 static inline struct ssd13xx_crtc_state *to_ssd13xx_crtc_state(struct drm_crtc_state *state)
@@ -331,7 +334,7 @@ static void ssd13xx_power_off(struct ssd13xx_device *ssd13xx)
 	regulator_disable(ssd13xx->vcc_reg);
 }
 
-static int ssd13xx_init(struct ssd13xx_device *ssd13xx)
+static int ssd130x_init(struct ssd13xx_device *ssd13xx)
 {
 	u32 precharge, dclk, com_invdir, compins, chargepump, seg_remap;
 	bool scan_mode;
@@ -455,7 +458,7 @@ static int ssd13xx_init(struct ssd13xx_device *ssd13xx)
 				 SSD130X_SET_ADDRESS_MODE_HORIZONTAL);
 }
 
-static int ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
+static int ssd130x_update_rect(struct ssd13xx_device *ssd13xx,
 			       struct drm_rect *rect, u8 *buf,
 			       u8 *data_array)
 {
@@ -561,7 +564,7 @@ static int ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
 	return ret;
 }
 
-static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
+static void ssd130x_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 {
 	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
 	unsigned int width = ssd13xx->width;
@@ -602,6 +605,15 @@ static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 	}
 }
 
+static const struct ssd13xx_funcs ssd13xx_family_funcs[] = {
+	[SSD130X_FAMILY] = {
+		.init = ssd130x_init,
+		.update_rect = ssd130x_update_rect,
+		.clear_screen = ssd130x_clear_screen,
+		.fmt_convert = drm_fb_xrgb8888_to_mono,
+	},
+};
+
 static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 				const struct iosys_map *vmap,
 				struct drm_rect *rect, u8 *buf,
@@ -609,6 +621,7 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 				u8 *data_array)
 {
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(fb->dev);
+	const struct ssd13xx_funcs *ssd13xx_funcs = ssd13xx->funcs;
 	struct iosys_map dst;
 	unsigned int dst_pitch;
 	int ret = 0;
@@ -624,13 +637,11 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 		return ret;
 
 	iosys_map_set_vaddr(&dst, buf);
-	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
+	ssd13xx_funcs->fmt_convert(&dst, &dst_pitch, vmap, fb, rect);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
-	ssd13xx_update_rect(ssd13xx, rect, buf, data_array);
-
-	return ret;
+	return ssd13xx_funcs->update_rect(ssd13xx, rect, buf, data_array);
 }
 
 static int ssd13xx_primary_plane_atomic_check(struct drm_plane *plane,
@@ -642,8 +653,6 @@ static int ssd13xx_primary_plane_atomic_check(struct drm_plane *plane,
 	struct ssd13xx_plane_state *ssd13xx_state = to_ssd13xx_plane_state(plane_state);
 	struct drm_crtc *crtc = plane_state->crtc;
 	struct drm_crtc_state *crtc_state;
-	const struct drm_format_info *fi;
-	unsigned int pitch;
 	int ret;
 
 	if (!crtc)
@@ -657,18 +666,10 @@ static int ssd13xx_primary_plane_atomic_check(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	fi = drm_format_info(DRM_FORMAT_R1);
-	if (!fi)
-		return -EINVAL;
-
-	pitch = drm_format_info_min_pitch(fi, 0, ssd13xx->width);
-
-	ssd13xx_state->buffer = kcalloc(pitch, ssd13xx->height, GFP_KERNEL);
+	ssd13xx_state->buffer = kzalloc(ssd13xx->buffer_size, GFP_KERNEL);
 	if (!ssd13xx_state->buffer)
 		return -ENOMEM;
 
-	ssd13xx_state->fi = fi;
-
 	return 0;
 }
 
@@ -684,6 +685,7 @@ static void ssd13xx_primary_plane_atomic_update(struct drm_plane *plane,
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_device *drm = plane->dev;
+	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	struct drm_rect dst_clip;
 	struct drm_rect damage;
 	int idx;
@@ -700,7 +702,7 @@ static void ssd13xx_primary_plane_atomic_update(struct drm_plane *plane,
 
 		ssd13xx_fb_blit_rect(fb, &shadow_plane_state->data[0], &dst_clip,
 				     ssd13xx_plane_state->buffer,
-				     ssd13xx_plane_state->fi,
+				     ssd13xx->buffer_fi,
 				     ssd13xx_crtc_state->data_array);
 	}
 
@@ -712,6 +714,7 @@ static void ssd13xx_primary_plane_atomic_disable(struct drm_plane *plane,
 {
 	struct drm_device *drm = plane->dev;
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
+	const struct ssd13xx_funcs *ssd13xx_funcs = ssd13xx->funcs;
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 	struct drm_crtc_state *crtc_state;
 	struct ssd13xx_crtc_state *ssd13xx_crtc_state;
@@ -726,7 +729,7 @@ static void ssd13xx_primary_plane_atomic_disable(struct drm_plane *plane,
 	if (!drm_dev_enter(drm, &idx))
 		return;
 
-	ssd13xx_clear_screen(ssd13xx, ssd13xx_crtc_state->data_array);
+	ssd13xx_funcs->clear_screen(ssd13xx, ssd13xx_crtc_state->data_array);
 
 	drm_dev_exit(idx);
 }
@@ -820,14 +823,13 @@ static int ssd13xx_crtc_atomic_check(struct drm_crtc *crtc,
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	struct ssd13xx_crtc_state *ssd13xx_state = to_ssd13xx_crtc_state(crtc_state);
-	unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
 	int ret;
 
 	ret = drm_crtc_helper_atomic_check(crtc, state);
 	if (ret)
 		return ret;
 
-	ssd13xx_state->data_array = kmalloc(ssd13xx->width * pages, GFP_KERNEL);
+	ssd13xx_state->data_array = kmalloc(ssd13xx->data_array_size, GFP_KERNEL);
 	if (!ssd13xx_state->data_array)
 		return -ENOMEM;
 
@@ -905,13 +907,14 @@ static void ssd13xx_encoder_atomic_enable(struct drm_encoder *encoder,
 {
 	struct drm_device *drm = encoder->dev;
 	struct ssd13xx_device *ssd13xx = drm_to_ssd13xx(drm);
+	const struct ssd13xx_funcs *ssd13xx_funcs = ssd13xx->funcs;
 	int ret;
 
 	ret = ssd13xx_power_on(ssd13xx);
 	if (ret)
 		return;
 
-	ret = ssd13xx_init(ssd13xx);
+	ret = ssd13xx_funcs->init(ssd13xx);
 	if (ret)
 		goto power_off;
 
@@ -1195,11 +1198,38 @@ static int ssd13xx_get_resources(struct ssd13xx_device *ssd13xx)
 	return 0;
 }
 
+static int ssd13xx_set_buffer_sizes(struct ssd13xx_device *ssd13xx,
+				    enum ssd13xx_family_ids family_id)
+{
+	const struct drm_format_info *fi;
+	unsigned int buffer_pitch;
+
+	switch (family_id) {
+	case SSD130X_FAMILY:
+		unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
+
+		ssd13xx->data_array_size = ssd13xx->width * pages;
+
+		fi = drm_format_info(DRM_FORMAT_R1);
+		break;
+	}
+
+	if (!fi)
+		return -EINVAL;
+
+	buffer_pitch = drm_format_info_min_pitch(fi, 0, ssd13xx->width);
+	ssd13xx->buffer_size = buffer_pitch * ssd13xx->height;
+	ssd13xx->buffer_fi = fi;
+
+	return 0;
+}
+
 struct ssd13xx_device *ssd13xx_probe(struct device *dev, struct regmap *regmap)
 {
 	struct ssd13xx_device *ssd13xx;
 	struct backlight_device *bl;
 	struct drm_device *drm;
+	enum ssd13xx_family_ids family_id;
 	int ret;
 
 	ssd13xx = devm_drm_dev_alloc(dev, &ssd13xx_drm_driver,
@@ -1214,11 +1244,19 @@ struct ssd13xx_device *ssd13xx_probe(struct device *dev, struct regmap *regmap)
 	ssd13xx->regmap = regmap;
 	ssd13xx->device_info = device_get_match_data(dev);
 
+	family_id = ssd13xx->device_info->family_id;
+
+	ssd13xx->funcs = &ssd13xx_family_funcs[family_id];
+
 	if (ssd13xx->device_info->page_mode_only)
 		ssd13xx->page_address_mode = 1;
 
 	ssd13xx_parse_properties(ssd13xx);
 
+	ret = ssd13xx_set_buffer_sizes(ssd13xx, family_id);
+	if (ret)
+		return ERR_PTR(ret);
+
 	ret = ssd13xx_get_resources(ssd13xx);
 	if (ret)
 		return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/solomon/ssd13xx.h b/drivers/gpu/drm/solomon/ssd13xx.h
index 64283935fbc1..e78d5ab87474 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -20,11 +20,17 @@
 #include <drm/drm_plane_helper.h>
 
 #include <linux/regmap.h>
+#include <linux/iosys-map.h>
 
 #define SSD130X_DATA				0x40
 #define SSD130X_COMMAND				0x80
 
+enum ssd13xx_family_ids {
+	SSD130X_FAMILY,
+};
+
 enum ssd13xx_variants {
+	/* ssd130x family */
 	SH1106_ID,
 	SSD1305_ID,
 	SSD1306_ID,
@@ -39,6 +45,7 @@ struct ssd13xx_deviceinfo {
 	u32 default_dclk_frq;
 	u32 default_width;
 	u32 default_height;
+	enum ssd13xx_family_ids family_id;
 	bool need_pwm;
 	bool need_chargepump;
 	bool page_mode_only;
@@ -76,6 +83,12 @@ struct ssd13xx_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;
@@ -88,6 +101,20 @@ struct ssd13xx_device {
 	u8 col_end;
 	u8 page_start;
 	u8 page_end;
+
+	/* Chip family specific operations */
+	const struct ssd13xx_funcs *funcs;
+};
+
+struct ssd13xx_funcs {
+	int (*init)(struct ssd13xx_device *ssd130x);
+	int (*update_rect)(struct ssd13xx_device *ssd13xx, struct drm_rect *rect,
+			   u8 *buf, u8 *data_array);
+	void (*clear_screen)(struct ssd13xx_device *ssd13xx,
+			     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 ssd13xx_deviceinfo ssd13xx_variants[];
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 6/8] drm/ssd13xx: Rename commands that are shared across chip families
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
                   ` (4 preceding siblings ...)
  2023-10-09 18:34 ` [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family Javier Martinez Canillas
  2023-10-09 18:34 ` [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers Javier Martinez Canillas
  7 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

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 <javierm@redhat.com>
---

 drivers/gpu/drm/solomon/ssd13xx-spi.c |  4 +--
 drivers/gpu/drm/solomon/ssd13xx.c     | 45 +++++++++++++++------------
 drivers/gpu/drm/solomon/ssd13xx.h     |  4 +--
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd13xx-spi.c b/drivers/gpu/drm/solomon/ssd13xx-spi.c
index a5ebe5475a49..2416756686cc 100644
--- a/drivers/gpu/drm/solomon/ssd13xx-spi.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-spi.c
@@ -34,10 +34,10 @@ static int ssd13xx_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 */
diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index 5a426ac10c58..b30224856518 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -44,18 +44,24 @@
 
 #define SSD130X_PAGE_HEIGHT 8
 
+/* ssd13xx commands */
+#define SSD13XX_CONTRAST			0x81
+#define SSD13XX_SET_SEG_REMAP			0xa0
+#define SSD13XX_SET_MULTIPLEX_RATIO		0xa8
+#define SSD13XX_DISPLAY_OFF			0xae
+#define SSD13XX_DISPLAY_ON			0xaf
+
+#define SSD13XX_SET_SEG_REMAP_MASK		GENMASK(0, 0)
+#define SSD13XX_SET_SEG_REMAP_SET(val)		FIELD_PREP(SSD13XX_SET_SEG_REMAP_MASK, (val))
+
+/* ssd130x commands */
 #define SSD130X_PAGE_COL_START_LOW		0x00
 #define SSD130X_PAGE_COL_START_HIGH		0x10
 #define SSD130X_SET_ADDRESS_MODE		0x20
 #define SSD130X_SET_COL_RANGE			0x21
 #define SSD130X_SET_PAGE_RANGE			0x22
-#define SSD130X_CONTRAST			0x81
 #define SSD130X_SET_LOOKUP_TABLE		0x91
 #define SSD130X_CHARGE_PUMP			0x8d
-#define SSD130X_SET_SEG_REMAP			0xa0
-#define SSD130X_DISPLAY_OFF			0xae
-#define SSD130X_SET_MULTIPLEX_RATIO		0xa8
-#define SSD130X_DISPLAY_ON			0xaf
 #define SSD130X_START_PAGE_ADDRESS		0xb0
 #define SSD130X_SET_COM_SCAN_DIR		0xc0
 #define SSD130X_SET_DISPLAY_OFFSET		0xd3
@@ -65,13 +71,12 @@
 #define SSD130X_SET_COM_PINS_CONFIG		0xda
 #define SSD130X_SET_VCOMH			0xdb
 
+/* ssd130x commands accessors */
 #define SSD130X_PAGE_COL_START_MASK		GENMASK(3, 0)
 #define SSD130X_PAGE_COL_START_HIGH_SET(val)	FIELD_PREP(SSD130X_PAGE_COL_START_MASK, (val) >> 4)
 #define SSD130X_PAGE_COL_START_LOW_SET(val)	FIELD_PREP(SSD130X_PAGE_COL_START_MASK, (val))
 #define SSD130X_START_PAGE_ADDRESS_MASK		GENMASK(2, 0)
 #define SSD130X_START_PAGE_ADDRESS_SET(val)	FIELD_PREP(SSD130X_START_PAGE_ADDRESS_MASK, (val))
-#define SSD130X_SET_SEG_REMAP_MASK		GENMASK(0, 0)
-#define SSD130X_SET_SEG_REMAP_SET(val)		FIELD_PREP(SSD130X_SET_SEG_REMAP_MASK, (val))
 #define SSD130X_SET_COM_SCAN_DIR_MASK		GENMASK(3, 3)
 #define SSD130X_SET_COM_SCAN_DIR_SET(val)	FIELD_PREP(SSD130X_SET_COM_SCAN_DIR_MASK, (val))
 #define SSD130X_SET_CLOCK_DIV_MASK		GENMASK(3, 0)
@@ -171,20 +176,20 @@ static inline struct ssd13xx_device *drm_to_ssd13xx(struct drm_device *drm)
 }
 
 /*
- * Helper to write data (SSD130X_DATA) to the device.
+ * Helper to write data (SSD13XX_DATA) to the device.
  */
 static int ssd13xx_write_data(struct ssd13xx_device *ssd13xx, u8 *values, int count)
 {
-	return regmap_bulk_write(ssd13xx->regmap, SSD130X_DATA, values, count);
+	return regmap_bulk_write(ssd13xx->regmap, SSD13XX_DATA, values, count);
 }
 
 /*
- * Helper to write command (SSD130X_COMMAND). The fist variadic argument
+ * Helper to write command (SSD13XX_COMMAND). The fist variadic argument
  * is the command to write and the following are the command options.
  *
  * Note that the ssd13xx protocol requires each command and option to be
- * written as a SSD130X_COMMAND device register value. That is why a call
- * to regmap_write(..., SSD130X_COMMAND, ...) is done for each argument.
+ * written as a SSD13XX_COMMAND device register value. That is why a call
+ * to regmap_write(..., SSD13XX_COMMAND, ...) is done for each argument.
  */
 static int ssd13xx_write_cmd(struct ssd13xx_device *ssd13xx, int count,
 			     /* u8 cmd, u8 option, ... */...)
@@ -197,7 +202,7 @@ static int ssd13xx_write_cmd(struct ssd13xx_device *ssd13xx, int count,
 
 	do {
 		value = va_arg(ap, int);
-		ret = regmap_write(ssd13xx->regmap, SSD130X_COMMAND, value);
+		ret = regmap_write(ssd13xx->regmap, SSD13XX_COMMAND, value);
 		if (ret)
 			goto out_end;
 	} while (--count);
@@ -341,13 +346,13 @@ static int ssd130x_init(struct ssd13xx_device *ssd13xx)
 	int ret;
 
 	/* Set initial contrast */
-	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_CONTRAST, ssd13xx->contrast);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD13XX_CONTRAST, ssd13xx->contrast);
 	if (ret < 0)
 		return ret;
 
 	/* Set segment re-map */
-	seg_remap = (SSD130X_SET_SEG_REMAP |
-		     SSD130X_SET_SEG_REMAP_SET(ssd13xx->seg_remap));
+	seg_remap = (SSD13XX_SET_SEG_REMAP |
+		     SSD13XX_SET_SEG_REMAP_SET(ssd13xx->seg_remap));
 	ret = ssd13xx_write_cmd(ssd13xx, 1, seg_remap);
 	if (ret < 0)
 		return ret;
@@ -360,7 +365,7 @@ static int ssd130x_init(struct ssd13xx_device *ssd13xx)
 		return ret;
 
 	/* Set multiplex ratio value */
-	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_MULTIPLEX_RATIO, ssd13xx->height - 1);
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD13XX_SET_MULTIPLEX_RATIO, ssd13xx->height - 1);
 	if (ret < 0)
 		return ret;
 
@@ -918,7 +923,7 @@ static void ssd13xx_encoder_atomic_enable(struct drm_encoder *encoder,
 	if (ret)
 		goto power_off;
 
-	ssd13xx_write_cmd(ssd13xx, 1, SSD130X_DISPLAY_ON);
+	ssd13xx_write_cmd(ssd13xx, 1, SSD13XX_DISPLAY_ON);
 
 	backlight_enable(ssd13xx->bl_dev);
 
@@ -937,7 +942,7 @@ static void ssd13xx_encoder_atomic_disable(struct drm_encoder *encoder,
 
 	backlight_disable(ssd13xx->bl_dev);
 
-	ssd13xx_write_cmd(ssd13xx, 1, SSD130X_DISPLAY_OFF);
+	ssd13xx_write_cmd(ssd13xx, 1, SSD13XX_DISPLAY_OFF);
 
 	ssd13xx_power_off(ssd13xx);
 }
@@ -1013,7 +1018,7 @@ static int ssd13xx_update_bl(struct backlight_device *bdev)
 
 	ssd13xx->contrast = brightness;
 
-	ret = ssd13xx_write_cmd(ssd13xx, 1, SSD130X_CONTRAST);
+	ret = ssd13xx_write_cmd(ssd13xx, 1, SSD13XX_CONTRAST);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/solomon/ssd13xx.h b/drivers/gpu/drm/solomon/ssd13xx.h
index e78d5ab87474..399b0c8b5680 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -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 ssd13xx_family_ids {
 	SSD130X_FAMILY,
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
                   ` (5 preceding siblings ...)
  2023-10-09 18:34 ` [PATCH 6/8] drm/ssd13xx: Rename commands that are shared across chip families Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-11  8:13   ` Geert Uytterhoeven
  2023-10-09 18:34 ` [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers Javier Martinez Canillas
  7 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Daniel Vetter, David Airlie,
	Maarten Lankhorst, dri-devel

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 <javierm@redhat.com>
---

 drivers/gpu/drm/solomon/ssd13xx-i2c.c |  13 ++
 drivers/gpu/drm/solomon/ssd13xx-spi.c |  13 ++
 drivers/gpu/drm/solomon/ssd13xx.c     | 206 +++++++++++++++++++++++++-
 drivers/gpu/drm/solomon/ssd13xx.h     |   5 +
 4 files changed, 234 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd13xx-i2c.c b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
index d9cece374331..9cf78d206c6e 100644
--- a/drivers/gpu/drm/solomon/ssd13xx-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-i2c.c
@@ -92,6 +92,19 @@ static const struct of_device_id ssd13xx_of_match[] = {
 		.compatible = "solomon,ssd1309fb-i2c",
 		.data = &ssd13xx_variants[SSD1309_ID],
 	},
+	/* ssd1302x family */
+	{
+		.compatible = "solomon,ssd1322",
+		.data = &ssd13xx_variants[SSD1322_ID],
+	},
+	{
+		.compatible = "solomon,ssd1325",
+		.data = &ssd13xx_variants[SSD1325_ID],
+	},
+	{
+		.compatible = "solomon,ssd1327",
+		.data = &ssd13xx_variants[SSD1327_ID],
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ssd13xx_of_match);
diff --git a/drivers/gpu/drm/solomon/ssd13xx-spi.c b/drivers/gpu/drm/solomon/ssd13xx-spi.c
index 2416756686cc..55162e49f037 100644
--- a/drivers/gpu/drm/solomon/ssd13xx-spi.c
+++ b/drivers/gpu/drm/solomon/ssd13xx-spi.c
@@ -129,6 +129,19 @@ static const struct of_device_id ssd13xx_of_match[] = {
 		.compatible = "solomon,ssd1309",
 		.data = &ssd13xx_variants[SSD1309_ID],
 	},
+	/* ssd1302x family */
+	{
+		.compatible = "solomon,ssd1322",
+		.data = &ssd13xx_variants[SSD1322_ID],
+	},
+	{
+		.compatible = "solomon,ssd1325",
+		.data = &ssd13xx_variants[SSD1325_ID],
+	},
+	{
+		.compatible = "solomon,ssd1327",
+		.data = &ssd13xx_variants[SSD1327_ID],
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ssd13xx_of_match);
diff --git a/drivers/gpu/drm/solomon/ssd13xx.c b/drivers/gpu/drm/solomon/ssd13xx.c
index b30224856518..bc53e7c80ffe 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.c
+++ b/drivers/gpu/drm/solomon/ssd13xx.c
@@ -99,6 +99,24 @@
 #define SSD130X_SET_AREA_COLOR_MODE_ENABLE	0x1e
 #define SSD130X_SET_AREA_COLOR_MODE_LOW_POWER	0x05
 
+/* ssd132x commands */
+#define SSD132X_SET_COL_RANGE			0x15
+#define SSD132X_SET_DEACTIVATE_SCROLL		0x2e
+#define SSD132X_SET_ROW_RANGE			0x75
+#define SSD132X_SET_DISPLAY_START		0xa1
+#define SSD132X_SET_DISPLAY_OFFSET		0xa2
+#define SSD132X_SET_DISPLAY_NORMAL		0xa4
+#define SSD132X_SET_FUNCTION_SELECT_A		0xab
+#define SSD132X_SET_PHASE_LENGTH		0xb1
+#define SSD132X_SET_CLOCK_FREQ			0xb3
+#define SSD132X_SET_GPIO			0xb5
+#define SSD132X_SET_PRECHARGE_PERIOD		0xb6
+#define SSD132X_SET_GRAY_SCALE_TABLE		0xb8
+#define SSD132X_SELECT_DEFAULT_TABLE		0xb9
+#define SSD132X_SET_PRECHARGE_VOLTAGE		0xbc
+#define SSD130X_SET_VCOMH_VOLTAGE		0xbe
+#define SSD132X_SET_FUNCTION_SELECT_B		0xd5
+
 #define MAX_CONTRAST 255
 
 const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
@@ -144,6 +162,22 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
 		.default_width = 128,
 		.default_height = 64,
 		.family_id = SSD130X_FAMILY,
+	},
+	/* ssd132x family */
+	[SSD1322_ID] = {
+		.default_width = 480,
+		.default_height = 128,
+		.family_id = SSD132X_FAMILY,
+	},
+	[SSD1325_ID] = {
+		.default_width = 128,
+		.default_height = 80,
+		.family_id = SSD132X_FAMILY,
+	},
+	[SSD1327_ID] = {
+		.default_width = 128,
+		.default_height = 128,
+		.family_id = SSD132X_FAMILY,
 	}
 };
 EXPORT_SYMBOL_NS_GPL(ssd13xx_variants, DRM_SSD13XX);
@@ -610,6 +644,156 @@ static void ssd130x_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
 	}
 }
 
+static int ssd132x_init(struct ssd13xx_device *ssd13xx)
+{
+	int ret;
+
+	/* Set initial contrast */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD13XX_CONTRAST, 0x80);
+	if (ret < 0)
+		return ret;
+
+	/* Set column start and end */
+	ret = ssd13xx_write_cmd(ssd13xx, 3, SSD132X_SET_COL_RANGE, 0x00, ssd13xx->width / 2 - 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set row start and end */
+	ret = ssd13xx_write_cmd(ssd13xx, 3, SSD132X_SET_ROW_RANGE, 0x00, ssd13xx->height - 1);
+	if (ret < 0)
+		return ret;
+	/*
+	 * Horizontal Address Increment
+	 * Re-map for Column Address, Nibble and COM
+	 * COM Split Odd Even
+	 */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD13XX_SET_SEG_REMAP, 0x53);
+	if (ret < 0)
+		return ret;
+
+	/* Set display start and offset */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_DISPLAY_START, 0x00);
+	if (ret < 0)
+		return ret;
+
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_DISPLAY_OFFSET, 0x00);
+	if (ret < 0)
+		return ret;
+
+	/* Set display mode normal */
+	ret = ssd13xx_write_cmd(ssd13xx, 1, SSD132X_SET_DISPLAY_NORMAL);
+	if (ret < 0)
+		return ret;
+
+	/* Set multiplex ratio value */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD13XX_SET_MULTIPLEX_RATIO, ssd13xx->height - 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set phase length */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_PHASE_LENGTH, 0x55);
+	if (ret < 0)
+		return ret;
+
+	/* Select default linear gray scale table */
+	ret = ssd13xx_write_cmd(ssd13xx, 1, SSD132X_SELECT_DEFAULT_TABLE);
+	if (ret < 0)
+		return ret;
+
+	/* Set clock frequency */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_CLOCK_FREQ, 0x01);
+	if (ret < 0)
+		return ret;
+
+	/* Enable internal VDD regulator */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_FUNCTION_SELECT_A, 0x1);
+	if (ret < 0)
+		return ret;
+
+	/* Set pre-charge period */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_PRECHARGE_PERIOD, 0x01);
+	if (ret < 0)
+		return ret;
+
+	/* Set pre-charge voltage */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_PRECHARGE_VOLTAGE, 0x08);
+	if (ret < 0)
+		return ret;
+
+	/* Set VCOMH voltage */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD130X_SET_VCOMH_VOLTAGE, 0x07);
+	if (ret < 0)
+		return ret;
+
+	/* Enable second pre-charge and internal VSL */
+	ret = ssd13xx_write_cmd(ssd13xx, 2, SSD132X_SET_FUNCTION_SELECT_B, 0x62);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ssd132x_update_rect(struct ssd13xx_device *ssd13xx,
+			       struct drm_rect *rect, u8 *buf,
+			       u8 *data_array)
+{
+	unsigned int x = rect->x1 / 2;
+	unsigned int y = rect->y1;
+	unsigned int width = drm_rect_width(rect);
+	unsigned int height = drm_rect_height(rect);
+	unsigned int columns = DIV_ROUND_UP(width, 2);
+	unsigned int rows = height;
+	u32 array_idx = 0;
+	int ret, i, j;
+
+	/*
+	 * 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 = ssd13xx_write_cmd(ssd13xx, 3, SSD132X_SET_COL_RANGE, x, columns - 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set row start and end */
+	ret = ssd13xx_write_cmd(ssd13xx, 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 += 2) {
+			u8 n1 = buf[i * width + j];
+			u8 n2 = buf[i * width + j + 1];
+
+			data_array[array_idx++] = (n2 << 4) | n1;
+		}
+	}
+
+	/* Write out update in one go since horizontal addressing mode is used */
+	ret = ssd13xx_write_data(ssd13xx, data_array, columns * rows);
+
+	return ret;
+}
+
+static void ssd132x_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
+{
+	memset(data_array, 0, ssd13xx->data_array_size);
+
+	/* Write out update in one go since horizontal addressing mode is used */
+	ssd13xx_write_data(ssd13xx, data_array, ssd13xx->data_array_size);
+}
+
 static const struct ssd13xx_funcs ssd13xx_family_funcs[] = {
 	[SSD130X_FAMILY] = {
 		.init = ssd130x_init,
@@ -617,6 +801,12 @@ static const struct ssd13xx_funcs ssd13xx_family_funcs[] = {
 		.clear_screen = ssd130x_clear_screen,
 		.fmt_convert = drm_fb_xrgb8888_to_mono,
 	},
+	[SSD132X_FAMILY] = {
+		.init = ssd132x_init,
+		.update_rect = ssd132x_update_rect,
+		.clear_screen = ssd132x_clear_screen,
+		.fmt_convert = drm_fb_xrgb8888_to_gray8,
+	}
 };
 
 static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
@@ -631,9 +821,12 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
 	unsigned int dst_pitch;
 	int ret = 0;
 
-	/* Align y to display page boundaries */
-	rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
-	rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd13xx->height);
+	if (ssd13xx->device_info->family_id == SSD130X_FAMILY) {
+		/* Align y to display page boundaries */
+		rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
+		rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT),
+				 ssd13xx->height);
+	}
 
 	dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
 
@@ -1217,6 +1410,13 @@ static int ssd13xx_set_buffer_sizes(struct ssd13xx_device *ssd13xx,
 
 		fi = drm_format_info(DRM_FORMAT_R1);
 		break;
+	case SSD132X_FAMILY:
+		unsigned int columns = DIV_ROUND_UP(ssd13xx->width, 2);
+		unsigned int rows = ssd13xx->height;
+
+		ssd13xx->data_array_size = columns * rows;
+
+		fi = drm_format_info(DRM_FORMAT_R8);
 	}
 
 	if (!fi)
diff --git a/drivers/gpu/drm/solomon/ssd13xx.h b/drivers/gpu/drm/solomon/ssd13xx.h
index 399b0c8b5680..58083c7e08c8 100644
--- a/drivers/gpu/drm/solomon/ssd13xx.h
+++ b/drivers/gpu/drm/solomon/ssd13xx.h
@@ -27,6 +27,7 @@
 
 enum ssd13xx_family_ids {
 	SSD130X_FAMILY,
+	SSD132X_FAMILY,
 };
 
 enum ssd13xx_variants {
@@ -36,6 +37,10 @@ enum ssd13xx_variants {
 	SSD1306_ID,
 	SSD1307_ID,
 	SSD1309_ID,
+	/* ssd132x family */
+	SSD1322_ID,
+	SSD1325_ID,
+	SSD1327_ID,
 	NR_SSD13XX_VARIANTS
 };
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
                   ` (6 preceding siblings ...)
  2023-10-09 18:34 ` [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family Javier Martinez Canillas
@ 2023-10-09 18:34 ` Javier Martinez Canillas
  2023-10-10 16:31   ` Conor Dooley
  2023-10-10 16:58   ` Rob Herring
  7 siblings, 2 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-09 18:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Thomas Zimmermann, Geert Uytterhoeven,
	Javier Martinez Canillas, Conor Dooley, Daniel Vetter,
	David Airlie, Krzysztof Kozlowski, Maarten Lankhorst,
	Rob Herring, devicetree, dri-devel

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

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 .../bindings/display/solomon,ssd132x.yaml     | 116 ++++++++++++++++++
 1 file changed, 116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
new file mode 100644
index 000000000000..b64904703a3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
@@ -0,0 +1,116 @@
+# 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 Controllers
+
+maintainers:
+  - Javier Martinez Canillas <javierm@redhat.com>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - solomon,ssd1322
+          - solomon,ssd1325
+          - solomon,ssd1327
+
+  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.
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.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>;
+
+            ssd1327_i2c: oled@3c {
+                    compatible = "solomon,ssd1327";
+                    reg = <0x3c>;
+                    reset-gpios = <&gpio2 7>;
+            };
+
+    };
+  - |
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ssd1327_spi: oled@0 {
+                    compatible = "solomon,ssd1327";
+                    reg = <0x0>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+            };
+    };
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx
  2023-10-09 18:34 ` [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx Javier Martinez Canillas
@ 2023-10-10  8:09   ` Maxime Ripard
  2023-10-10  8:38     ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Maxime Ripard @ 2023-10-10  8:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Thomas Zimmermann, Geert Uytterhoeven,
	Daniel Vetter, David Airlie, Maarten Lankhorst, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Hi,

On Mon, Oct 09, 2023 at 08:34:15PM +0200, Javier Martinez Canillas wrote:
> The driver only supports the SSD130x family of Solomon OLED controllers
> now, but will be extended to also support the SSD132x (4-bit grayscale)
> and SSD133x (16-bit RGB) controller families. Rename driver to ssd13xx.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

I'm not sure it's worth it. I understand what you want to achieve, but
this will create conflicts, affect the configuration of everyone
enabling that driver, etc.

And we have plenty of drivers that don't match all the devices they
support anyway.

Plus ....

> @@ -11,10 +11,10 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  
> -#include "ssd130x.h"
> +#include "ssd13xx.h"
>  
> -#define DRIVER_NAME	"ssd130x-i2c"
> -#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays (I2C)"
> +#define DRIVER_NAME	"ssd13xx-i2c"
> +#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
>  
>  static const struct regmap_config ssd130x_i2c_regmap_config = {
>  	.reg_bits = 8,

... We now end up with a lot of inconsistencies where some things are
called ssd130x and others ssd13xx.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx
  2023-10-10  8:09   ` Maxime Ripard
@ 2023-10-10  8:38     ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-10  8:38 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-kernel, Thomas Zimmermann, Geert Uytterhoeven,
	Daniel Vetter, David Airlie, Maarten Lankhorst, dri-devel

Maxime Ripard <mripard@kernel.org> writes:

Hello Maxime,

Thanks for the feedback.

> Hi,
>
> On Mon, Oct 09, 2023 at 08:34:15PM +0200, Javier Martinez Canillas wrote:
>> The driver only supports the SSD130x family of Solomon OLED controllers
>> now, but will be extended to also support the SSD132x (4-bit grayscale)
>> and SSD133x (16-bit RGB) controller families. Rename driver to ssd13xx.
>> 
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> I'm not sure it's worth it. I understand what you want to achieve, but
> this will create conflicts, affect the configuration of everyone
> enabling that driver, etc.
>
> And we have plenty of drivers that don't match all the devices they
> support anyway.
>

Yeah, I was on the fence and even discussed this with others. I'm OK with
dropping this patch if the agreegment is that isn't worth it to make the
name more accurate.

> Plus ....
>
>> @@ -11,10 +11,10 @@
>>  #include <linux/i2c.h>
>>  #include <linux/module.h>
>>  
>> -#include "ssd130x.h"
>> +#include "ssd13xx.h"
>>  
>> -#define DRIVER_NAME	"ssd130x-i2c"
>> -#define DRIVER_DESC	"DRM driver for Solomon SSD130x OLED displays (I2C)"
>> +#define DRIVER_NAME	"ssd13xx-i2c"
>> +#define DRIVER_DESC	"DRM driver for Solomon SSD13xx OLED displays (I2C)"
>>  
>>  static const struct regmap_config ssd130x_i2c_regmap_config = {
>>  	.reg_bits = 8,
>
> ... We now end up with a lot of inconsistencies where some things are
> called ssd130x and others ssd13xx.
>

Yes, but I fix that in patch #2.

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-09 18:34 ` [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers Javier Martinez Canillas
@ 2023-10-10 16:31   ` Conor Dooley
  2023-10-11  6:34     ` Javier Martinez Canillas
  2023-10-10 16:58   ` Rob Herring
  1 sibling, 1 reply; 25+ messages in thread
From: Conor Dooley @ 2023-10-10 16:31 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann,
	Geert Uytterhoeven, Conor Dooley, Daniel Vetter, David Airlie,
	Krzysztof Kozlowski, Maarten Lankhorst, Rob Herring, devicetree,
	dri-devel

[-- Attachment #1: Type: text/plain, Size: 4442 bytes --]

Hey,

On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
> Add a Device Tree binding schema for the OLED panels based on the Solomon
> SSD132x family of controllers.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>  .../bindings/display/solomon,ssd132x.yaml     | 116 ++++++++++++++++++
>  1 file changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> new file mode 100644
> index 000000000000..b64904703a3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> @@ -0,0 +1,116 @@
> +# 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 Controllers
> +
> +maintainers:
> +  - Javier Martinez Canillas <javierm@redhat.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - solomon,ssd1322
> +          - solomon,ssd1325
> +          - solomon,ssd1327

You don't need the oneOf here here as there is only the enum as a
possible item.
I didn't get anything else in the series, I have to ask - are these
controllers not compatible with eachother?

> +
> +  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.

You probably know better than me, operating in drm stuff, but are there
really no generic properties for the weidth/height of a display?

> +
> +  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.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.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

Unless you did it like this for clarity, 2 of these have the same
default width and 2 have the same default height. You could cut this
down to a pair of if/then/else on that basis AFAICT.
:wq

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            ssd1327_i2c: oled@3c {

This label is unused as far as I can tell. Ditto below.

Cheers,
Conor.

> +                    compatible = "solomon,ssd1327";
> +                    reg = <0x3c>;
> +                    reset-gpios = <&gpio2 7>;
> +            };
> +
> +    };
> +  - |
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            ssd1327_spi: oled@0 {
> +                    compatible = "solomon,ssd1327";
> +                    reg = <0x0>;
> +                    reset-gpios = <&gpio2 7>;
> +                    dc-gpios = <&gpio2 8>;
> +                    spi-max-frequency = <10000000>;
> +            };
> +    };
> -- 
> 2.41.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-09 18:34 ` [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers Javier Martinez Canillas
  2023-10-10 16:31   ` Conor Dooley
@ 2023-10-10 16:58   ` Rob Herring
  2023-10-11  6:39     ` Javier Martinez Canillas
  1 sibling, 1 reply; 25+ messages in thread
From: Rob Herring @ 2023-10-10 16:58 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann,
	Geert Uytterhoeven, Conor Dooley, Daniel Vetter, David Airlie,
	Krzysztof Kozlowski, Maarten Lankhorst, devicetree, dri-devel

On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
> Add a Device Tree binding schema for the OLED panels based on the Solomon
> SSD132x family of controllers.

Looks like the same binding as solomon,ssd1307fb.yaml. Why a different 
binding? Why does that binding need a slew of custom properties and here 
you don't?

> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>  .../bindings/display/solomon,ssd132x.yaml     | 116 ++++++++++++++++++
>  1 file changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> new file mode 100644
> index 000000000000..b64904703a3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> @@ -0,0 +1,116 @@
> +# 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 Controllers
> +
> +maintainers:
> +  - Javier Martinez Canillas <javierm@redhat.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - solomon,ssd1322
> +          - solomon,ssd1325
> +          - solomon,ssd1327
> +
> +  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.

Don't define the same properties twice. Either share the binding or 
split out the common properties into their own schema file.

Rob

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-10 16:31   ` Conor Dooley
@ 2023-10-11  6:34     ` Javier Martinez Canillas
  2023-10-11 15:50       ` Conor Dooley
  0 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  6:34 UTC (permalink / raw)
  To: Conor Dooley
  Cc: devicetree, Conor Dooley, Thomas Zimmermann, Krzysztof Kozlowski,
	linux-kernel, Maxime Ripard, Rob Herring, Geert Uytterhoeven,
	dri-devel

Conor Dooley <conor@kernel.org> writes:

Hello Conor,

Thanks a lot for your feedback.

> Hey,
>
> On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:

[...]

>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - enum:
>> +          - solomon,ssd1322
>> +          - solomon,ssd1325
>> +          - solomon,ssd1327
>
> You don't need the oneOf here here as there is only the enum as a
> possible item.

Indeed. I'll fix that in v2.

> I didn't get anything else in the series, I have to ask - are these
> controllers not compatible with eachother?
>

They are not, basically the difference is in the default width and height
for each controller. That's why the width and height fields are optional.

But other than the default resolution, yes the controllers are very much
the same.

>> +
>> +  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.
>
> You probably know better than me, operating in drm stuff, but are there
> really no generic properties for the weidth/height of a display?
>

There are some common properties, such as the width-mm and height-mm for
the panel-common:

Documentation/devicetree/bindings/display/panel/panel-common.yaml

But those are to describe the physical area expressed in millimeters and
the Solomon drivers (the old ssd1307fb fbdev driver and the new ssd130x
DRM driver for backward compatibility with existing DTB) express the width
and height in pixels.

That's why are Solomon controller specific properties "solomon,width" and
"solomon,height".

[...]

>> +    then:
>> +      properties:
>> +        width:
>> +          default: 128
>> +        height:
>> +          default: 128
>
> Unless you did it like this for clarity, 2 of these have the same
> default width and 2 have the same default height. You could cut this
> down to a pair of if/then/else on that basis AFAICT.
> :wq
>

Yes, this was done like that for clarity. Because is easier for someone
reading the DT binding schema to reason about resolution (width,height)
for a given SSD132x controller, rather than following the if/else logic.

>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            ssd1327_i2c: oled@3c {
>
> This label is unused as far as I can tell. Ditto below.
>

Right, I'll drop those too.

> Cheers,
> Conor.
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-10 16:58   ` Rob Herring
@ 2023-10-11  6:39     ` Javier Martinez Canillas
  2023-10-11  7:34       ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  6:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Conor Dooley, Thomas Zimmermann, Krzysztof Kozlowski,
	linux-kernel, Maxime Ripard, Geert Uytterhoeven, dri-devel

Rob Herring <robh@kernel.org> writes:

Hello Rob,

Thanks a lot for your feedback.

> On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
>> Add a Device Tree binding schema for the OLED panels based on the Solomon
>> SSD132x family of controllers.
>
> Looks like the same binding as solomon,ssd1307fb.yaml. Why a different 
> binding? Why does that binding need a slew of custom properties and here 
> you don't?
>

It's a sub-set of it. Because only the minimum required properties are
defined. But also, is a clean slate schema because the old ssd1307fb fbdev
driver only supports the Solomon SSD130x family, so there is no need to
follow the existing solomon,ssd1307fb.yaml nor the need for backward compat.

[...]

>> +  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.
>
> Don't define the same properties twice. Either share the binding or 
> split out the common properties into their own schema file.
>

Agreed. I'll do that in v2.

> Rob
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-11  6:39     ` Javier Martinez Canillas
@ 2023-10-11  7:34       ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  7:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Conor Dooley, Thomas Zimmermann, Krzysztof Kozlowski,
	linux-kernel, Maxime Ripard, Geert Uytterhoeven, dri-devel

Javier Martinez Canillas <javierm@redhat.com> writes:

> Rob Herring <robh@kernel.org> writes:
>
> Hello Rob,
>
> Thanks a lot for your feedback.
>
>> On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
>>> Add a Device Tree binding schema for the OLED panels based on the Solomon
>>> SSD132x family of controllers.
>>
>> Looks like the same binding as solomon,ssd1307fb.yaml. Why a different 
>> binding? Why does that binding need a slew of custom properties and here 
>> you don't?
>>
>
> It's a sub-set of it. Because only the minimum required properties are
> defined. But also, is a clean slate schema because the old ssd1307fb fbdev
> driver only supports the Solomon SSD130x family, so there is no need to
> follow the existing solomon,ssd1307fb.yaml nor the need for backward compat.
>

I think this answer was a little sparse, let me elaborate a bit. The Solomon
display controllers are quite flexible, so that could be used with different
OLED panels.

And the ssd1307fb binding schema defines a set of properties that are almost
a 1:1 mapping from properties to the configuration registers. This makes the
driver to support most SSD130x controller + panel configurations but at the
expense of making the binding more complicated (a slew of custom propertie
as you pointed out).

Now, as mentioned this is support for greyscale Solomon controllers (the old
fbdev driver only supports monochrome controllers) so we don't care about DT
backward compatibility.

I decided for now to keep the binding at a minimum and be more opinionated in
the driver with having what I think are sane defaults for most of the config
registers.

If there is a need to expose any of this configuration as DT properties, then
we can later added it share some of the existing solomon,ssd1307fb.yaml custom
properties and move them to a common schema.

We can always change the driver in the future anyways, but we can't do the same
with the DT binding schema since that is considered an ABI.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant
  2023-10-09 18:34 ` [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant Javier Martinez Canillas
@ 2023-10-11  7:39   ` Geert Uytterhoeven
  2023-10-11  8:33     ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2023-10-11  7:39 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> 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 <javierm@redhat.com>

> --- a/drivers/gpu/drm/solomon/ssd13xx.c
> +++ b/drivers/gpu/drm/solomon/ssd13xx.c

> @@ -465,13 +462,13 @@ static int ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
>         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 = ssd13xx->device_info->page_height;
> +       unsigned int page_height = SSD130X_PAGE_HEIGHT;
>         unsigned int pages = DIV_ROUND_UP(height, page_height);
>         struct drm_device *drm = &ssd13xx->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 ssd13xx_update_rect(struct ssd13xx_device *ssd13xx,
>          */
>
>         if (!ssd13xx->page_address_mode) {
> +               u8 page_start;
> +
>                 /* Set address range for horizontal addressing mode */
>                 ret = ssd13xx_set_col_range(ssd13xx, ssd13xx->col_offset + x, width);
>                 if (ret < 0)
>                         return ret;
>
> -               ret = ssd13xx_set_page_range(ssd13xx, ssd13xx->page_offset + y / 8, pages);
> +               page_start = ssd13xx->page_offset + y / page_height;
> +               ret = ssd13xx_set_page_range(ssd13xx, 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) > ssd13xx->height)
> -                       m = ssd13xx->height % 8;
> +               if (page_height * (y / page_height + i + 1) > ssd13xx->height)
> +                       m = ssd13xx->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;

Nit: I would use unsigned int for idx, as that matches all other
variables involved.
But given array_idx is u32, too, u32 may makes sense.

> +                               u8 byte = buf[idx];
>                                 u8 bit = (byte >> (j % 8)) & 1;
>
>                                 data |= bit << k;

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch
  2023-10-09 18:34 ` [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch Javier Martinez Canillas
@ 2023-10-11  7:59   ` Geert Uytterhoeven
  2023-10-11  8:37     ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2023-10-11  7:59 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Hi Javier,

On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> Don't assume bpp of 1 and instead compute the destination pitch using the
> intermediate buffer pixel format info when doing a format conversion.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks for your patch!

> --- a/drivers/gpu/drm/solomon/ssd13xx.c
> +++ b/drivers/gpu/drm/solomon/ssd13xx.c
> @@ -148,6 +148,8 @@ struct ssd13xx_plane_state {
>         struct drm_shadow_plane_state base;
>         /* Intermediate buffer to convert pixels from XRGB8888 to HW format */
>         u8 *buffer;
> +       /* Pixel format info for the intermediate buffer */
> +       const struct drm_format_info *fi;

This is really intermediate, as it is removed again in the next patch :-)

In fact 60% of this patch is changed again in the next patch.
So perhaps combine this with the next patch?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table
  2023-10-09 18:34 ` [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table Javier Martinez Canillas
@ 2023-10-11  8:09   ` Geert Uytterhoeven
  2023-10-11  8:48     ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2023-10-11  8:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Hi Javier,

On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> 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.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks for your patch!

> --- a/drivers/gpu/drm/solomon/ssd13xx.c
> +++ b/drivers/gpu/drm/solomon/ssd13xx.c
> @@ -104,6 +104,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
>                 .default_width = 132,
>                 .default_height = 64,
>                 .page_mode_only = 1,
> +               .family_id = SSD130X_FAMILY,

Why not store &ssd13xx_family_funcs[SSD130X_FAMILY]?

>         },
>         [SSD1305_ID] = {
>                 .default_vcomh = 0x34,

> @@ -602,6 +605,15 @@ static void ssd13xx_clear_screen(struct ssd13xx_device *ssd13xx, u8 *data_array)
>         }
>  }
>
> +static const struct ssd13xx_funcs ssd13xx_family_funcs[] = {
> +       [SSD130X_FAMILY] = {
> +               .init = ssd130x_init,
> +               .update_rect = ssd130x_update_rect,
> +               .clear_screen = ssd130x_clear_screen,
> +               .fmt_convert = drm_fb_xrgb8888_to_mono,
> +       },
> +};
> +
>  static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
>                                 const struct iosys_map *vmap,
>                                 struct drm_rect *rect, u8 *buf,

> @@ -1195,11 +1198,38 @@ static int ssd13xx_get_resources(struct ssd13xx_device *ssd13xx)
>         return 0;
>  }
>
> +static int ssd13xx_set_buffer_sizes(struct ssd13xx_device *ssd13xx,
> +                                   enum ssd13xx_family_ids family_id)
> +{
> +       const struct drm_format_info *fi;
> +       unsigned int buffer_pitch;
> +
> +       switch (family_id) {
> +       case SSD130X_FAMILY:
> +               unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
> +
> +               ssd13xx->data_array_size = ssd13xx->width * pages;
> +
> +               fi = drm_format_info(DRM_FORMAT_R1);
> +               break;
> +       }

Please don't mix ssd13xx_funcs[family_id] and switch (family_id).
The switch() could be replaced by an extra function pointer in
ssd13xx_funcs, and by storing fi in ssd13xx_funcs, too.

Note that you don't really need the full drm_format_info, the number
of bits per pixel is sufficient.  But having the drm_format_info is
nice, as fmt_convert() will need it later when adding support for
fbdev emulation using R1 or R4 ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family
  2023-10-09 18:34 ` [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family Javier Martinez Canillas
@ 2023-10-11  8:13   ` Geert Uytterhoeven
  2023-10-11  8:49     ` Javier Martinez Canillas
  0 siblings, 1 reply; 25+ messages in thread
From: Geert Uytterhoeven @ 2023-10-11  8:13 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Hi Javier,

On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> 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 <javierm@redhat.com>

Thanks for your patch!

> --- a/drivers/gpu/drm/solomon/ssd13xx.c
> +++ b/drivers/gpu/drm/solomon/ssd13xx.c

> @@ -631,9 +821,12 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
>         unsigned int dst_pitch;
>         int ret = 0;
>
> -       /* Align y to display page boundaries */
> -       rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
> -       rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd13xx->height);
> +       if (ssd13xx->device_info->family_id == SSD130X_FAMILY) {
> +               /* Align y to display page boundaries */
> +               rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
> +               rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT),
> +                                ssd13xx->height);
> +       }

Don't you need to align to page boundaries (2 pixels per page)
on SSD132X?

This should be handled through ssd13xx_funcs instead of a family check.

>
>         dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant
  2023-10-11  7:39   ` Geert Uytterhoeven
@ 2023-10-11  8:33     ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  8:33 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Geert Uytterhoeven <geert@linux-m68k.org> writes:

Hello Geert,

Thanks a lot for your feedback.

> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:

[...]

>>         u32 array_idx = 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;
>
> Nit: I would use unsigned int for idx, as that matches all other
> variables involved.
> But given array_idx is u32, too, u32 may makes sense.
>

Yes, this function logic is mostly based on ssd1307fb_update_rect() from
drivers/video/fbdev/ssd1307fb.c and that is from where the u32 array_idx
comes from.

As you said, I used u32 for idx to be consistent with that variable type.

>> +                               u8 byte = buf[idx];
>>                                 u8 bit = (byte >> (j % 8)) & 1;
>>
>>                                 data |= bit << k;
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch
  2023-10-11  7:59   ` Geert Uytterhoeven
@ 2023-10-11  8:37     ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  8:37 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Javier,
>
> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> Don't assume bpp of 1 and instead compute the destination pitch using the
>> intermediate buffer pixel format info when doing a format conversion.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Thanks for your patch!
>
>> --- a/drivers/gpu/drm/solomon/ssd13xx.c
>> +++ b/drivers/gpu/drm/solomon/ssd13xx.c
>> @@ -148,6 +148,8 @@ struct ssd13xx_plane_state {
>>         struct drm_shadow_plane_state base;
>>         /* Intermediate buffer to convert pixels from XRGB8888 to HW format */
>>         u8 *buffer;
>> +       /* Pixel format info for the intermediate buffer */
>> +       const struct drm_format_info *fi;
>
> This is really intermediate, as it is removed again in the next patch :-)
>
> In fact 60% of this patch is changed again in the next patch.
> So perhaps combine this with the next patch?
>

I actually had it like that but then thought that maybe someone would say
that should be a separate patch :) I will squash it then.

> Gr{oetje,eeting}s,
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table
  2023-10-11  8:09   ` Geert Uytterhoeven
@ 2023-10-11  8:48     ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  8:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-kernel, Maxime Ripard, Thomas Zimmermann, Daniel Vetter,
	David Airlie, Maarten Lankhorst, dri-devel

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Javier,
>
> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> 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.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Thanks for your patch!
>
>> --- a/drivers/gpu/drm/solomon/ssd13xx.c
>> +++ b/drivers/gpu/drm/solomon/ssd13xx.c
>> @@ -104,6 +104,7 @@ const struct ssd13xx_deviceinfo ssd13xx_variants[] = {
>>                 .default_width = 132,
>>                 .default_height = 64,
>>                 .page_mode_only = 1,
>> +               .family_id = SSD130X_FAMILY,
>
> Why not store &ssd13xx_family_funcs[SSD130X_FAMILY]?
>

I could do that, yeah. Originally thought that could be useful besides the
per chip functions, but I don't think that it's used for anything else...

[...]

>> +       switch (family_id) {
>> +       case SSD130X_FAMILY:
>> +               unsigned int pages = DIV_ROUND_UP(ssd13xx->height, SSD130X_PAGE_HEIGHT);
>> +
>> +               ssd13xx->data_array_size = ssd13xx->width * pages;
>> +
>> +               fi = drm_format_info(DRM_FORMAT_R1);
>> +               break;
>> +       }
>
> Please don't mix ssd13xx_funcs[family_id] and switch (family_id).
> The switch() could be replaced by an extra function pointer in
> ssd13xx_funcs, and by storing fi in ssd13xx_funcs, too.
>

Yes, I didn't want to store the format info in struct ssd13xx_funcs
because the idea was for that struct to only have chip operations.

But could do that. I wonder if should rename that struct thought? to
something that better denotes is more than function handlers.

> Note that you don't really need the full drm_format_info, the number
> of bits per pixel is sufficient.  But having the drm_format_info is
> nice, as fmt_convert() will need it later when adding support for
> fbdev emulation using R1 or R4 ;-)
>

Exactly, thinking in your patches is why I stored the full format info :)

> Gr{oetje,eeting}s,

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family
  2023-10-11  8:13   ` Geert Uytterhoeven
@ 2023-10-11  8:49     ` Javier Martinez Canillas
  0 siblings, 0 replies; 25+ messages in thread
From: Javier Martinez Canillas @ 2023-10-11  8:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Thomas Zimmermann, linux-kernel, Maxime Ripard, dri-devel

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Javier,
>
> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> 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 <javierm@redhat.com>
>
> Thanks for your patch!
>
>> --- a/drivers/gpu/drm/solomon/ssd13xx.c
>> +++ b/drivers/gpu/drm/solomon/ssd13xx.c
>
>> @@ -631,9 +821,12 @@ static int ssd13xx_fb_blit_rect(struct drm_framebuffer *fb,
>>         unsigned int dst_pitch;
>>         int ret = 0;
>>
>> -       /* Align y to display page boundaries */
>> -       rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
>> -       rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT), ssd13xx->height);
>> +       if (ssd13xx->device_info->family_id == SSD130X_FAMILY) {
>> +               /* Align y to display page boundaries */
>> +               rect->y1 = round_down(rect->y1, SSD130X_PAGE_HEIGHT);
>> +               rect->y2 = min_t(unsigned int, round_up(rect->y2, SSD130X_PAGE_HEIGHT),
>> +                                ssd13xx->height);
>> +       }
>
> Don't you need to align to page boundaries (2 pixels per page)
> on SSD132X?
>

I guess so, yes. I'll add that to v2 as well.

> This should be handled through ssd13xx_funcs instead of a family check.
>

Agreed.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers
  2023-10-11  6:34     ` Javier Martinez Canillas
@ 2023-10-11 15:50       ` Conor Dooley
  0 siblings, 0 replies; 25+ messages in thread
From: Conor Dooley @ 2023-10-11 15:50 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: devicetree, Conor Dooley, Thomas Zimmermann, Krzysztof Kozlowski,
	linux-kernel, Maxime Ripard, Rob Herring, Geert Uytterhoeven,
	dri-devel

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On Wed, Oct 11, 2023 at 08:34:29AM +0200, Javier Martinez Canillas wrote:
> Conor Dooley <conor@kernel.org> writes:
> >> +  # 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.
> >
> > You probably know better than me, operating in drm stuff, but are there
> > really no generic properties for the weidth/height of a display?
> >
> 
> There are some common properties, such as the width-mm and height-mm for
> the panel-common:
> 
> Documentation/devicetree/bindings/display/panel/panel-common.yaml
> 
> But those are to describe the physical area expressed in millimeters and
> the Solomon drivers (the old ssd1307fb fbdev driver and the new ssd130x
> DRM driver for backward compatibility with existing DTB) express the width
> and height in pixels.
> 
> That's why are Solomon controller specific properties "solomon,width" and
> "solomon,height".

Okay. Thanks for the explanation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2023-10-11 15:51 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09 18:34 [PATCH 0/8] drm/solomon: Add support for the SSD132x controller family Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 1/8] drm/solomon: Rename ssd130x driver to ssd13xx Javier Martinez Canillas
2023-10-10  8:09   ` Maxime Ripard
2023-10-10  8:38     ` Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 2/8] drm/ssd13xx: Rename data structures and functions prefixes Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 3/8] drm/ssd13xx: Replace .page_height field in device info with a constant Javier Martinez Canillas
2023-10-11  7:39   ` Geert Uytterhoeven
2023-10-11  8:33     ` Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 4/8] drm/ssd13xx: Use drm_format_info_min_pitch() to calculate the dest_pitch Javier Martinez Canillas
2023-10-11  7:59   ` Geert Uytterhoeven
2023-10-11  8:37     ` Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 5/8] drm/ssd13xx: Add a per controller family functions table Javier Martinez Canillas
2023-10-11  8:09   ` Geert Uytterhoeven
2023-10-11  8:48     ` Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 6/8] drm/ssd13xx: Rename commands that are shared across chip families Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 7/8] drm/ssd13xx: Add support for the SSD132x OLED controller family Javier Martinez Canillas
2023-10-11  8:13   ` Geert Uytterhoeven
2023-10-11  8:49     ` Javier Martinez Canillas
2023-10-09 18:34 ` [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers Javier Martinez Canillas
2023-10-10 16:31   ` Conor Dooley
2023-10-11  6:34     ` Javier Martinez Canillas
2023-10-11 15:50       ` Conor Dooley
2023-10-10 16:58   ` Rob Herring
2023-10-11  6:39     ` Javier Martinez Canillas
2023-10-11  7:34       ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).