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
There are some commands that are shared between the SSD130x and SSD132x
controller families, define these as a common SSD13XX set of commands.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
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
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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@@ -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 <[email protected]>");
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 <[email protected]>
@@ -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 <[email protected]>");
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 <[email protected]>
@@ -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 <[email protected]>
@@ -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
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 <[email protected]>
---
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
Add a Device Tree binding schema for the OLED panels based on the Solomon
SSD132x family of controllers.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
.../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 <[email protected]>
+
+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
The Solomon SSD132x controllers (such as the SSD1322, SSD1325 and SSD1327)
are used by 16 grayscale dot matrix OLED panels, extend the driver to also
support this chip family.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
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
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 <[email protected]>
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
Maxime Ripard <[email protected]> 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 <[email protected]>
>
> 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
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 <[email protected]>
> ---
>
> .../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 <[email protected]>
> +
> +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
>
>
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 <[email protected]>
> ---
>
> .../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 <[email protected]>
> +
> +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
Conor Dooley <[email protected]> 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
Rob Herring <[email protected]> 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
Javier Martinez Canillas <[email protected]> writes:
> Rob Herring <[email protected]> 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
Hi Javier,
On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<[email protected]> 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 <[email protected]>
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 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Javier,
On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
<[email protected]> 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 <[email protected]>
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 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Geert Uytterhoeven <[email protected]> writes:
> Hi Javier,
>
> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <[email protected]> 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 <[email protected]>
>
> 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
Geert Uytterhoeven <[email protected]> writes:
> Hi Javier,
>
> On Mon, Oct 9, 2023 at 8:36 PM Javier Martinez Canillas
> <[email protected]> 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 <[email protected]>
>
> 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
On Wed, Oct 11, 2023 at 08:34:29AM +0200, Javier Martinez Canillas wrote:
> Conor Dooley <[email protected]> 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.