[PATCH 0/6] video: ssd1307fb: Support more displays

[PATCH 0/6] video: ssd1307fb: Support more displays

[Marko Kohtala]

The kernel driver for ssd1307fb did not allow for all proper
initialization for a Densitron 128x36 display. The trend in the driver
has been to add devicetree properties for the controller initialization
and these patches continue on that trend.

There also were some sparse and Coccinelle errors.

A small bug causing scrolling on display updates with nonzero page_offset
was a bit surprising. It would seem the driver has only been used with
page_offset set to zero. Bug has been there since commit
301bc0675b677a98475187050d56cd2b39ff0acf ("video: ssd1307fb: Make use of
horizontal addressing mode").

Marko Kohtala (6):
  video: ssd1307fb: Use screen_buffer instead of screen_base
  video: ssd1307fb: Remove unneeded semicolons
  video: ssd1307fb: Start page range at page_offset
  video: ssd1307fb: Handle width and height that are not multiple of 8
  dt-bindings: display: ssd1307fb: Add initialization properties
  video: ssd1307fb: Add devicetree configuration of display setup

 .../devicetree/bindings/display/ssd1307fb.txt |  10 ++
 drivers/video/fbdev/ssd1307fb.c               | 130 ++++++++++++------
 2 files changed, 101 insertions(+), 39 deletions(-)

-- 
2.17.1

[PATCH 1/6] video: ssd1307fb: Use screen_buffer instead of screen_base

[Marko Kohtala]

sparse reported incorrect type due to different address spaces.
The screen_base is __iomem, but the memory is not from a device so we can
use screen_buffer instead and avoid some type casts.

Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 drivers/video/fbdev/ssd1307fb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3b361bc9feb8..6c2980331ffd 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -150,7 +150,7 @@ static inline int ssd1307fb_write_cmd(struct i2c_client *client, u8 cmd)
 static void ssd1307fb_update_display(struct ssd1307fb_par *par)
 {
 	struct ssd1307fb_array *array;
-	u8 *vmem = par->info->screen_base;
+	u8 *vmem = par->info->screen_buffer;
 	int i, j, k;
 
 	array = ssd1307fb_alloc_array(par->width * par->height / 8,
@@ -213,7 +213,7 @@ static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
 	struct ssd1307fb_par *par = info->par;
 	unsigned long total_size;
 	unsigned long p = *ppos;
-	u8 __iomem *dst;
+	void *dst;
 
 	total_size = info->fix.smem_len;
 
@@ -226,7 +226,7 @@ static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
 	if (!count)
 		return -EINVAL;
 
-	dst = (void __force *) (info->screen_base + p);
+	dst = info->screen_buffer + p;
 
 	if (copy_from_user(dst, buf, count))
 		return -EFAULT;
@@ -547,7 +547,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	struct fb_deferred_io *ssd1307fb_defio;
 	u32 vmem_size;
 	struct ssd1307fb_par *par;
-	u8 *vmem;
+	void *vmem;
 	int ret;
 
 	if (!node) {
@@ -657,7 +657,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	info->var.blue.length = 1;
 	info->var.blue.offset = 0;
 
-	info->screen_base = (u8 __force __iomem *)vmem;
+	info->screen_buffer = vmem;
 	info->fix.smem_start = __pa(vmem);
 	info->fix.smem_len = vmem_size;
 
-- 
2.17.1

[PATCH 2/6] video: ssd1307fb: Remove unneeded semicolons

[Marko Kohtala]

coccicheck reported unneeded semicolons. Remove them.

Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 drivers/video/fbdev/ssd1307fb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 6c2980331ffd..9ab00e0dadc7 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -313,7 +313,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 
 		dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
 			par->pwm->pwm, par->pwm_period);
-	};
+	}
 
 	/* Set initial contrast */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
@@ -329,7 +329,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
 		if (ret < 0)
 			return ret;
-	};
+	}
 
 	/* Set COM direction */
 	com_invdir = 0xc0 | (par->com_invdir & 0x1) << 3;
@@ -716,7 +716,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	if (par->device_info->need_pwm) {
 		pwm_disable(par->pwm);
 		pwm_put(par->pwm);
-	};
+	}
 regulator_enable_error:
 	if (par->vbat_reg)
 		regulator_disable(par->vbat_reg);
@@ -740,7 +740,7 @@ static int ssd1307fb_remove(struct i2c_client *client)
 	if (par->device_info->need_pwm) {
 		pwm_disable(par->pwm);
 		pwm_put(par->pwm);
-	};
+	}
 	fb_deferred_io_cleanup(info);
 	__free_pages(__va(info->fix.smem_start), get_order(info->fix.smem_len));
 	framebuffer_release(info);
-- 
2.17.1

[PATCH 3/6] video: ssd1307fb: Start page range at page_offset

[Marko Kohtala]

The page_offset was only applied to the end of the page range. This caused
the display updates to cause a scrolling effect on the display because the
amount of data written to the display did not match the range display
expected.

Fixes: 301bc0675b67 ("video: ssd1307fb: Make use of horizontal addressing mode")
Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 drivers/video/fbdev/ssd1307fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 9ab00e0dadc7..86f2b79f3ed5 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -433,7 +433,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x0);
+	ret = ssd1307fb_write_cmd(par->client, par->page_offset);
 	if (ret < 0)
 		return ret;
 
-- 
2.17.1

[PATCH 4/6] video: ssd1307fb: Handle width and height that are not multiple of 8

[Marko Kohtala]

Some displays have dimensions that are not multiple of eight, for example
height of 36, but the driver divided the dimensions by 8. Defining display
to the next multiple of 8 is not good as then the display registers get
configured to dimensions that do not match. This contradicts intructions
by some display manufacturers.

Use DIV_ROUND_UP to multiple of 8 when needed so correct values can be
used.

The ssd1307fb_update_display bit reordering receives a simplification in
the process.

Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 drivers/video/fbdev/ssd1307fb.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 86f2b79f3ed5..4f4a1b99d17d 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -151,10 +151,11 @@ static void ssd1307fb_update_display(struct ssd1307fb_par *par)
 {
 	struct ssd1307fb_array *array;
 	u8 *vmem = par->info->screen_buffer;
+	unsigned int line_length = par->info->fix.line_length;
+	unsigned int pages = DIV_ROUND_UP(par->height, 8);
 	int i, j, k;
 
-	array = ssd1307fb_alloc_array(par->width * par->height / 8,
-				      SSD1307FB_DATA);
+	array = ssd1307fb_alloc_array(par->width * pages, SSD1307FB_DATA);
 	if (!array)
 		return;
 
@@ -187,22 +188,24 @@ static void ssd1307fb_update_display(struct ssd1307fb_par *par)
 	 *  (5) A4 B4 C4 D4 E4 F4 G4 H4
 	 */
 
-	for (i = 0; i < (par->height / 8); i++) {
+	for (i = 0; i < pages; i++) {
 		for (j = 0; j < par->width; j++) {
+			int m = 8;
 			u32 array_idx = i * par->width + j;
 			array->data[array_idx] = 0;
-			for (k = 0; k < 8; k++) {
-				u32 page_length = par->width * i;
-				u32 index = page_length + (par->width * k + j) / 8;
-				u8 byte = *(vmem + index);
-				u8 bit = byte & (1 << (j % 8));
-				bit = bit >> (j % 8);
+			/* Last page may be partial */
+			if (i + 1 = pages && par->height % 8)
+				m = par->height % 8;
+			for (k = 0; k < m; k++) {
+				u8 byte = vmem[(8 * i + k) * line_length +
+					       j / 8];
+				u8 bit = (byte >> (j % 8)) & 1;
 				array->data[array_idx] |= bit << k;
 			}
 		}
 	}
 
-	ssd1307fb_write_array(par->client, array, par->width * par->height / 8);
+	ssd1307fb_write_array(par->client, array, par->width * pages);
 	kfree(array);
 }
 
@@ -438,7 +441,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		return ret;
 
 	ret = ssd1307fb_write_cmd(par->client,
-				  par->page_offset + (par->height / 8) - 1);
+				  par->page_offset +
+				  DIV_ROUND_UP(par->height, 8) - 1);
 	if (ret < 0)
 		return ret;
 
@@ -618,7 +622,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	par->dclk_div = par->device_info->default_dclk_div;
 	par->dclk_frq = par->device_info->default_dclk_frq;
 
-	vmem_size = par->width * par->height / 8;
+	vmem_size = DIV_ROUND_UP(par->width, 8) * par->height;
 
 	vmem = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 					get_order(vmem_size));
@@ -641,7 +645,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	info->fbops = &ssd1307fb_ops;
 	info->fix = ssd1307fb_fix;
-	info->fix.line_length = par->width / 8;
+	info->fix.line_length = DIV_ROUND_UP(par->width, 8);
 	info->fbdefio = ssd1307fb_defio;
 
 	info->var = ssd1307fb_var;
-- 
2.17.1

[PATCH 5/6] dt-bindings: display: ssd1307fb: Add initialization properties

[Marko Kohtala]

Document new bindings for adapting ssd1307fb driver to new displays.

Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 .../devicetree/bindings/display/ssd1307fb.txt          | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index b67f8caa212c..27333b9551b3 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -27,6 +27,15 @@ Optional properties:
   - solomon,prechargep2: Length of precharge period (phase 2) in clock cycles.
                          This needs to be the higher, the higher the capacitance
                          of the OLED's pixels is
+  - solomon,dclk-div: Clock divisor 1 to 16
+  - solomon,dclk-frq: Clock frequency 0 to 15, higher value means higher
+                      frequency
+  - solomon,lookup-table: 8 bit value array of current drive pulse widths for
+                          BANK0, and colors A, B, and C. Each value in range
+                          of 31 to 63 for pulse widths of 32 to 64. Color D
+                          is always width 64.
+  - solomon,area-color-enable: Display uses color mode
+  - solomon,low-power. Display runs in low power mode
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 
@@ -46,4 +55,5 @@ ssd1306: oled@3c {
         solomon,com-lrremap;
         solomon,com-invdir;
         solomon,com-offset = <32>;
+        solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
 };
-- 
2.17.1

Re: [PATCH 5/6] dt-bindings: display: ssd1307fb: Add initialization properties

[Rob Herring]

On Tue, 18 Jun 2019 10:41:10 +0300, Marko Kohtala wrote:
> Document new bindings for adapting ssd1307fb driver to new displays.
> 
> Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
> ---
>  .../devicetree/bindings/display/ssd1307fb.txt          | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 6/6] video: ssd1307fb: Add devicetree configuration of display setup

[Marko Kohtala]

Various displays have differences that only mean initializing the display
driver IC with different fixed register values. Defining these in
devicetree offers easier way to adapt the driver to new displays than
requiring a patch to the kernel.

This adds devicetree properties needed to make the initialization match
the example setup as offered by Densitron for their 128x36 display.

It also makes some old one bit parameter handling a little cleaner.

Signed-off-by: Marko Kohtala <marko.kohtala@okoko.fi>
---
 drivers/video/fbdev/ssd1307fb.c | 80 ++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 4f4a1b99d17d..fca1e91d03d9 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -29,6 +29,7 @@
 #define SSD1307FB_SET_COL_RANGE		0x21
 #define SSD1307FB_SET_PAGE_RANGE	0x22
 #define SSD1307FB_CONTRAST		0x81
+#define SSD1307FB_SET_LOOKUP_TABLE	0x91
 #define	SSD1307FB_CHARGE_PUMP		0x8d
 #define SSD1307FB_SEG_REMAP_ON		0xa1
 #define SSD1307FB_DISPLAY_OFF		0xae
@@ -37,6 +38,7 @@
 #define SSD1307FB_START_PAGE_ADDRESS	0xb0
 #define SSD1307FB_SET_DISPLAY_OFFSET	0xd3
 #define	SSD1307FB_SET_CLOCK_FREQ	0xd5
+#define	SSD1307FB_SET_AREA_COLOR_MODE	0xd8
 #define	SSD1307FB_SET_PRECHARGE_PERIOD	0xd9
 #define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
 #define	SSD1307FB_SET_VCOMH		0xdb
@@ -59,10 +61,14 @@ struct ssd1307fb_deviceinfo {
 };
 
 struct ssd1307fb_par {
-	u32 com_invdir;
-	u32 com_lrremap;
+	unsigned area_color_enable : 1;
+	unsigned com_invdir : 1;
+	unsigned com_lrremap : 1;
+	unsigned com_seq : 1;
+	unsigned lookup_table_set : 1;
+	unsigned low_power : 1;
+	unsigned seg_remap : 1;
 	u32 com_offset;
-	u32 com_seq;
 	u32 contrast;
 	u32 dclk_div;
 	u32 dclk_frq;
@@ -70,6 +76,7 @@ struct ssd1307fb_par {
 	struct i2c_client *client;
 	u32 height;
 	struct fb_info *info;
+	u8 lookup_table[4];
 	u32 page_offset;
 	u32 prechargep1;
 	u32 prechargep2;
@@ -77,7 +84,6 @@ struct ssd1307fb_par {
 	u32 pwm_period;
 	struct gpio_desc *reset;
 	struct regulator *vbat_reg;
-	u32 seg_remap;
 	u32 vcomh;
 	u32 width;
 };
@@ -99,6 +105,9 @@ static const struct fb_fix_screeninfo ssd1307fb_fix = {
 
 static const struct fb_var_screeninfo ssd1307fb_var = {
 	.bits_per_pixel	= 1,
+	.red = { .length = 1 },
+	.green = { .length = 1 },
+	.blue = { .length = 1 },
 };
 
 static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
@@ -335,7 +344,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	}
 
 	/* Set COM direction */
-	com_invdir = 0xc0 | (par->com_invdir & 0x1) << 3;
+	com_invdir = 0xc0 | par->com_invdir << 3;
 	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
 	if (ret < 0)
 		return ret;
@@ -368,6 +377,22 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
+	/* Set Set Area Color Mode ON/OFF & Low Power Display Mode */
+	if (par->area_color_enable || par->low_power) {
+		u32 mode;
+
+		ret = ssd1307fb_write_cmd(par->client,
+					  SSD1307FB_SET_AREA_COLOR_MODE);
+		if (ret < 0)
+			return ret;
+
+		mode = (par->area_color_enable ? 0x30 : 0) |
+			(par->low_power ? 5 : 0);
+		ret = ssd1307fb_write_cmd(par->client, mode);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* Set precharge period in number of ticks from the internal clock */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PRECHARGE_PERIOD);
 	if (ret < 0)
@@ -383,8 +408,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
-	compins = 0x02 | !(par->com_seq & 0x1) << 4
-				   | (par->com_lrremap & 0x1) << 5;
+	compins = 0x02 | !par->com_seq << 4 | par->com_lrremap << 5;
 	ret = ssd1307fb_write_cmd(par->client, compins);
 	if (ret < 0)
 		return ret;
@@ -408,6 +432,28 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
+	/* Set lookup table */
+	if (par->lookup_table_set) {
+		int i;
+
+		ret = ssd1307fb_write_cmd(par->client,
+					  SSD1307FB_SET_LOOKUP_TABLE);
+		if (ret < 0)
+			return ret;
+
+		for (i = 0; i < ARRAY_SIZE(par->lookup_table); ++i) {
+			u8 val = par->lookup_table[i];
+
+			if (val < 31 || val > 63)
+				dev_warn(&par->client->dev,
+					"lookup table index %d value out of range 31 <= %d <= 63\n",
+					i, val);
+			ret = ssd1307fb_write_cmd(par->client, val);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
 	/* Switch to horizontal addressing mode */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
 	if (ret < 0)
@@ -610,17 +656,26 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	if (of_property_read_u32(node, "solomon,prechargep2", &par->prechargep2))
 		par->prechargep2 = 2;
 
+	if (!of_property_read_u8_array(node, "solomon,lookup-table",
+		par->lookup_table, ARRAY_SIZE(par->lookup_table)))
+		par->lookup_table_set = 1;
+
 	par->seg_remap = !of_property_read_bool(node, "solomon,segment-no-remap");
 	par->com_seq = of_property_read_bool(node, "solomon,com-seq");
 	par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap");
 	par->com_invdir = of_property_read_bool(node, "solomon,com-invdir");
+	par->area_color_enable +		of_property_read_bool(node, "solomon,area-color-enable");
+	par->low_power = of_property_read_bool(node, "solomon,low-power");
 
 	par->contrast = 127;
 	par->vcomh = par->device_info->default_vcomh;
 
 	/* Setup display timing */
-	par->dclk_div = par->device_info->default_dclk_div;
-	par->dclk_frq = par->device_info->default_dclk_frq;
+	if (of_property_read_u32(node, "solomon,dclk-div", &par->dclk_div))
+		par->dclk_div = par->device_info->default_dclk_div;
+	if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
+		par->dclk_frq = par->device_info->default_dclk_frq;
 
 	vmem_size = DIV_ROUND_UP(par->width, 8) * par->height;
 
@@ -654,13 +709,6 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	info->var.yres = par->height;
 	info->var.yres_virtual = par->height;
 
-	info->var.red.length = 1;
-	info->var.red.offset = 0;
-	info->var.green.length = 1;
-	info->var.green.offset = 0;
-	info->var.blue.length = 1;
-	info->var.blue.offset = 0;
-
 	info->screen_buffer = vmem;
 	info->fix.smem_start = __pa(vmem);
 	info->fix.smem_len = vmem_size;
-- 
2.17.1

Re: [PATCH 0/6] video: ssd1307fb: Support more displays

[Bartlomiej Zolnierkiewicz]

On 6/18/19 9:41 AM, Marko Kohtala wrote:
> The kernel driver for ssd1307fb did not allow for all proper
> initialization for a Densitron 128x36 display. The trend in the driver
> has been to add devicetree properties for the controller initialization
> and these patches continue on that trend.
> 
> There also were some sparse and Coccinelle errors.
> 
> A small bug causing scrolling on display updates with nonzero page_offset
> was a bit surprising. It would seem the driver has only been used with
> page_offset set to zero. Bug has been there since commit
> 301bc0675b677a98475187050d56cd2b39ff0acf ("video: ssd1307fb: Make use of
> horizontal addressing mode").
> 
> Marko Kohtala (6):
>   video: ssd1307fb: Use screen_buffer instead of screen_base
>   video: ssd1307fb: Remove unneeded semicolons
>   video: ssd1307fb: Start page range at page_offset
>   video: ssd1307fb: Handle width and height that are not multiple of 8
>   dt-bindings: display: ssd1307fb: Add initialization properties
>   video: ssd1307fb: Add devicetree configuration of display setup
> 
>  .../devicetree/bindings/display/ssd1307fb.txt |  10 ++
>  drivers/video/fbdev/ssd1307fb.c               | 130 ++++++++++++------
>  2 files changed, 101 insertions(+), 39 deletions(-)
Patch series queued for v5.4, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics