Re: [PATCH v3 4/4] drm/ssd130x: Add support for the SSD133x OLED controller family
Jocelyn Falempe writes: Hello Jocelyn, Thanks a lot for your review! > On 19/12/2023 21:34, Javier Martinez Canillas wrote: >> The Solomon SSD133x controllers (such as the SSD1331) are used by RGB dot >> matrix OLED panels, add a modesetting pipeline to support the chip family. >> >> The SSD133x controllers support 256 (8-bit) and 65k (16-bit) color depths >> but only the former is implemented for now. This is because the 256 color >> depth format matches a fourcc code already present in DRM (RGB8), but the >> 65k pixel format does not match the existing RG16 fourcc code format. >> >> Instead of a R:G:B 5:6:5, the controller expects the 16-bit pixels to be >> R:G:B 6:5:6, and so a new fourcc needs to be added to support this format. > > small typo here, R:G:B 6:5:6 => that's 17 bits > Oh, tanks for pointing that out. It seems to be a typo in the SSD1331 controller datasheet itself: https://cdn-shop.adafruit.com/datasheets/SSD1331_1.2.pdf "Each pixel has 16-bit data. Three sub-pixels for color A, B and C have 6 bits, 5 bits and 6 bits respectively." I blindly copied what the datasheet said without relizing that it was 17 bits indeed! So looking again at "Table 9 - Data bus usage under different bus width and color depth mode" in the datasheet shared above, it seems the format has the same sub-pixel layout than DRM_FORMAT_RGB565. But I tested with that format and the colors were off, and the same for DRM_FORMAT_BGR565. Now, even when only using 256 colors the images are pretty decent as you can see in https://fosstodon.org/@javierm/111591985174504541 I'll reword the commit message and drop the comment about that RGB format and explain that only DRM_FORMAT_RGB332 is supported for now. > other than that, it looks good to me, feel free to add: > Reviewed-by: Jocelyn Falempe > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v3 4/4] drm/ssd130x: Add support for the SSD133x OLED controller family
On 19/12/2023 21:34, Javier Martinez Canillas wrote: The Solomon SSD133x controllers (such as the SSD1331) are used by RGB dot matrix OLED panels, add a modesetting pipeline to support the chip family. The SSD133x controllers support 256 (8-bit) and 65k (16-bit) color depths but only the former is implemented for now. This is because the 256 color depth format matches a fourcc code already present in DRM (RGB8), but the 65k pixel format does not match the existing RG16 fourcc code format. Instead of a R:G:B 5:6:5, the controller expects the 16-bit pixels to be R:G:B 6:5:6, and so a new fourcc needs to be added to support this format. small typo here, R:G:B 6:5:6 => that's 17 bits other than that, it looks good to me, feel free to add: Reviewed-by: Jocelyn Falempe Signed-off-by: Javier Martinez Canillas --- (no changes since v1) drivers/gpu/drm/solomon/ssd130x-i2c.c | 5 + drivers/gpu/drm/solomon/ssd130x-spi.c | 7 + drivers/gpu/drm/solomon/ssd130x.c | 370 ++ drivers/gpu/drm/solomon/ssd130x.h | 5 +- 4 files changed, 386 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c index f2ccab9c06d9..a047dbec4e48 100644 --- a/drivers/gpu/drm/solomon/ssd130x-i2c.c +++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c @@ -105,6 +105,11 @@ static const struct of_device_id ssd130x_of_match[] = { .compatible = "solomon,ssd1327", .data = &ssd130x_variants[SSD1327_ID], }, + /* ssd133x family */ + { + .compatible = "solomon,ssd1331", + .data = &ssd130x_variants[SSD1331_ID], + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, ssd130x_of_match); diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c index 84e035a7ab3f..84bfde31d172 100644 --- a/drivers/gpu/drm/solomon/ssd130x-spi.c +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c @@ -142,6 +142,11 @@ static const struct of_device_id ssd130x_of_match[] = { .compatible = "solomon,ssd1327", .data = &ssd130x_variants[SSD1327_ID], }, + /* ssd133x family */ + { + .compatible = "solomon,ssd1331", + .data = &ssd130x_variants[SSD1331_ID], + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, ssd130x_of_match); @@ -166,6 +171,8 @@ static const struct spi_device_id ssd130x_spi_table[] = { { "ssd1322", SSD1322_ID }, { "ssd1325", SSD1325_ID }, { "ssd1327", SSD1327_ID }, + /* ssd133x family */ + { "ssd1331", SSD1331_ID }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(spi, ssd130x_spi_table); diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index bef293922b98..447d0c7c88c6 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -119,6 +119,26 @@ #define SSD130X_SET_VCOMH_VOLTAGE 0xbe #define SSD132X_SET_FUNCTION_SELECT_B 0xd5 +/* ssd133x commands */ +#define SSD133X_SET_COL_RANGE 0x15 +#define SSD133X_SET_ROW_RANGE 0x75 +#define SSD133X_CONTRAST_A 0x81 +#define SSD133X_CONTRAST_B 0x82 +#define SSD133X_CONTRAST_C 0x83 +#define SSD133X_SET_MASTER_CURRENT 0x87 +#define SSD132X_SET_PRECHARGE_A0x8a +#define SSD132X_SET_PRECHARGE_B0x8b +#define SSD132X_SET_PRECHARGE_C0x8c +#define SSD133X_SET_DISPLAY_START 0xa1 +#define SSD133X_SET_DISPLAY_OFFSET 0xa2 +#define SSD133X_SET_DISPLAY_NORMAL 0xa4 +#define SSD133X_SET_MASTER_CONFIG 0xad +#define SSD133X_POWER_SAVE_MODE0xb0 +#define SSD133X_PHASES_PERIOD 0xb1 +#define SSD133X_SET_CLOCK_FREQ 0xb3 +#define SSD133X_SET_PRECHARGE_VOLTAGE 0xbb +#define SSD133X_SET_VCOMH_VOLTAGE 0xbe + #define MAX_CONTRAST 255 const struct ssd130x_deviceinfo ssd130x_variants[] = { @@ -180,6 +200,12 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = { .default_width = 128, .default_height = 128, .family_id = SSD132X_FAMILY, + }, + /* ssd133x family */ + [SSD1331_ID] = { + .default_width = 96, + .default_height = 64, + .family_id = SSD133X_FAMILY, } }; EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD130X); @@ -589,6 +615,117 @@ static int ssd132x_init(struct ssd130x_device *ssd130x) return 0; } +static int ssd133x_init(struct ssd130x_device *ssd130x) +{ + int ret; + + /* Set color A contrast */ + ret = ssd130x_write_cmd(ssd130x, 2, SSD133X_CONTRAST_A, 0x91); + if (ret < 0) + return ret; + + /* Set color B contrast
[PATCH v3 4/4] drm/ssd130x: Add support for the SSD133x OLED controller family
The Solomon SSD133x controllers (such as the SSD1331) are used by RGB dot matrix OLED panels, add a modesetting pipeline to support the chip family. The SSD133x controllers support 256 (8-bit) and 65k (16-bit) color depths but only the former is implemented for now. This is because the 256 color depth format matches a fourcc code already present in DRM (RGB8), but the 65k pixel format does not match the existing RG16 fourcc code format. Instead of a R:G:B 5:6:5, the controller expects the 16-bit pixels to be R:G:B 6:5:6, and so a new fourcc needs to be added to support this format. Signed-off-by: Javier Martinez Canillas --- (no changes since v1) drivers/gpu/drm/solomon/ssd130x-i2c.c | 5 + drivers/gpu/drm/solomon/ssd130x-spi.c | 7 + drivers/gpu/drm/solomon/ssd130x.c | 370 ++ drivers/gpu/drm/solomon/ssd130x.h | 5 +- 4 files changed, 386 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c index f2ccab9c06d9..a047dbec4e48 100644 --- a/drivers/gpu/drm/solomon/ssd130x-i2c.c +++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c @@ -105,6 +105,11 @@ static const struct of_device_id ssd130x_of_match[] = { .compatible = "solomon,ssd1327", .data = &ssd130x_variants[SSD1327_ID], }, + /* ssd133x family */ + { + .compatible = "solomon,ssd1331", + .data = &ssd130x_variants[SSD1331_ID], + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, ssd130x_of_match); diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c index 84e035a7ab3f..84bfde31d172 100644 --- a/drivers/gpu/drm/solomon/ssd130x-spi.c +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c @@ -142,6 +142,11 @@ static const struct of_device_id ssd130x_of_match[] = { .compatible = "solomon,ssd1327", .data = &ssd130x_variants[SSD1327_ID], }, + /* ssd133x family */ + { + .compatible = "solomon,ssd1331", + .data = &ssd130x_variants[SSD1331_ID], + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, ssd130x_of_match); @@ -166,6 +171,8 @@ static const struct spi_device_id ssd130x_spi_table[] = { { "ssd1322", SSD1322_ID }, { "ssd1325", SSD1325_ID }, { "ssd1327", SSD1327_ID }, + /* ssd133x family */ + { "ssd1331", SSD1331_ID }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(spi, ssd130x_spi_table); diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index bef293922b98..447d0c7c88c6 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -119,6 +119,26 @@ #define SSD130X_SET_VCOMH_VOLTAGE 0xbe #define SSD132X_SET_FUNCTION_SELECT_B 0xd5 +/* ssd133x commands */ +#define SSD133X_SET_COL_RANGE 0x15 +#define SSD133X_SET_ROW_RANGE 0x75 +#define SSD133X_CONTRAST_A 0x81 +#define SSD133X_CONTRAST_B 0x82 +#define SSD133X_CONTRAST_C 0x83 +#define SSD133X_SET_MASTER_CURRENT 0x87 +#define SSD132X_SET_PRECHARGE_A0x8a +#define SSD132X_SET_PRECHARGE_B0x8b +#define SSD132X_SET_PRECHARGE_C0x8c +#define SSD133X_SET_DISPLAY_START 0xa1 +#define SSD133X_SET_DISPLAY_OFFSET 0xa2 +#define SSD133X_SET_DISPLAY_NORMAL 0xa4 +#define SSD133X_SET_MASTER_CONFIG 0xad +#define SSD133X_POWER_SAVE_MODE0xb0 +#define SSD133X_PHASES_PERIOD 0xb1 +#define SSD133X_SET_CLOCK_FREQ 0xb3 +#define SSD133X_SET_PRECHARGE_VOLTAGE 0xbb +#define SSD133X_SET_VCOMH_VOLTAGE 0xbe + #define MAX_CONTRAST 255 const struct ssd130x_deviceinfo ssd130x_variants[] = { @@ -180,6 +200,12 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = { .default_width = 128, .default_height = 128, .family_id = SSD132X_FAMILY, + }, + /* ssd133x family */ + [SSD1331_ID] = { + .default_width = 96, + .default_height = 64, + .family_id = SSD133X_FAMILY, } }; EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD130X); @@ -589,6 +615,117 @@ static int ssd132x_init(struct ssd130x_device *ssd130x) return 0; } +static int ssd133x_init(struct ssd130x_device *ssd130x) +{ + int ret; + + /* Set color A contrast */ + ret = ssd130x_write_cmd(ssd130x, 2, SSD133X_CONTRAST_A, 0x91); + if (ret < 0) + return ret; + + /* Set color B contrast */ + ret = ssd130x_write_cmd(ssd130x, 2, SSD133X_CONTRAST_B, 0x50); + if (ret < 0) + return ret; + + /* Set color C contrast */ + ret = ssd130x_write_cmd(ssd130x, 2, SSD133X_CONTR