Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCHv4 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Thomas Niederprüm @ 2015-03-16 17:11 UTC (permalink / raw)
  To: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt
  Cc: linux-fbdev, linux-kernel, Thomas Niederprüm
In-Reply-To: <1426525918-12745-1-git-send-email-niederp@physik.uni-kl.de>

The 130X controllers are very similar from the configuration point of view.
The configuration registers for the SSD1305/6/7 are bit identical (except the
the VHCOM register and the the default values for clock setup register). This
patch unifies the init code of the controller and adds hardware specific
properties to DT that are needed to correctly initialize the device.

The SSD130X can be wired to the OLED panel in various ways. Even for the
same controller this wiring can differ from one display module to another
and can not be probed by software. The added DT properties reflect these
hardware decisions of the display module manufacturer.
The 'com-sequential', 'com-lrremap' and 'com-invdir' values define different
possibilities for the COM signals pin configuration and readout direction
of the video memory. The 'segment-no-remap' allows the inversion of the
memory-to-pin mapping ultimately inverting the order of the controllers
output pins. The 'prechargepX' values need to be adapted according the
capacitance of the OLEDs pixel cells.

So far these hardware specific bits are hard coded in the init code, making
the driver usable only for one certain wiring of the controller. This patch
makes the driver usable with all possible hardware setups, given a valid hw
description in DT. If these values are not set in DT the default values,
as they are set in the ssd1307 init code right now, are used. This implies
that without the corresponding DT property "segment-no-remap" the segment
remap of the ssd130X controller gets activated. Even though this is not the
default behaviour according to the datasheet it maintains backward
compatibility with older DTBs.

Note that the SSD1306 does not seem to be using the configuration written to
the registers at all. Therefore this patch does not try to maintain these
values without changes in DT. For reference an example is added to the DT
bindings documentation that reproduces the configuration that is set in the
current init code.

Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
 .../devicetree/bindings/video/ssd1307fb.txt        |  21 +++
 drivers/video/fbdev/ssd1307fb.c                    | 195 ++++++++++++---------
 2 files changed, 137 insertions(+), 79 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
index 7a12542..be27562 100644
--- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -15,6 +15,16 @@ Required properties:
 
 Optional properties:
   - reset-active-low: Is the reset gpio is active on physical low?
+  - solomon,segment-no-remap: Display needs normal (non-inverted) data column
+                              to segment mapping
+  - solomon,com-sequential: Display uses sequential COM pin configuration
+  - solomon,com-lrremap: Display uses left-right COM pin remap
+  - solomon,com-invdir: Display uses inverted COM pin scan direction
+  - solomon,com-offset: Offset of the first COM pin wired to the panel
+  - solomon,prechargep1: Length of deselect period (phase 1) in clock cycles.
+  - 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
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 
@@ -26,3 +36,14 @@ ssd1307: oled@3c {
         reset-gpios = <&gpio2 7>;
         reset-active-low;
 };
+
+ssd1306: oled@3c {
+        compatible = "solomon,ssd1306fb-i2c";
+        reg = <0x3c>;
+        pwms = <&pwm 4 3000>;
+        reset-gpios = <&gpio2 7>;
+        reset-active-low;
+        solomon,com-lrremap;
+        solomon,com-invdir;
+        solomon,com-offset = <32>;
+};
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 8d34c56..9a66118 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -16,6 +16,9 @@
 #include <linux/pwm.h>
 #include <linux/delay.h>
 
+#define DEVID_SSD1306 6
+#define DEVID_SSD1307 7
+
 #define SSD1307FB_DATA			0x40
 #define SSD1307FB_COMMAND		0x80
 
@@ -38,22 +41,38 @@
 #define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
 #define	SSD1307FB_SET_VCOMH		0xdb
 
+static u_int contrast = 128;
+module_param(contrast, uint, S_IRUGO);
+
 struct ssd1307fb_par;
 
-struct ssd1307fb_ops {
-	int (*init)(struct ssd1307fb_par *);
-	int (*remove)(struct ssd1307fb_par *);
+struct ssd1307fb_deviceinfo {
+	int device_id;
+	u32 default_vcomh;
+	u32 default_dclk_div;
+	u32 default_dclk_frq;
 };
 
 struct ssd1307fb_par {
+	u32 com_invdir;
+	u32 com_lrremap;
+	u32 com_offset;
+	u32 com_seq;
+	u32 contrast;
+	u32 dclk_div;
+	u32 dclk_frq;
+	struct ssd1307fb_deviceinfo *device_info;
 	struct i2c_client *client;
 	u32 height;
 	struct fb_info *info;
-	struct ssd1307fb_ops *ops;
 	u32 page_offset;
+	u32 prechargep1;
+	u32 prechargep2;
 	struct pwm_device *pwm;
 	u32 pwm_period;
 	int reset;
+	u32 seg_remap;
+	u32 vcomh;
 	u32 width;
 };
 
@@ -254,69 +273,46 @@ static struct fb_deferred_io ssd1307fb_defio = {
 	.deferred_io	= ssd1307fb_deferred_io,
 };
 
-static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par)
+static int ssd1307fb_init(struct ssd1307fb_par *par)
 {
 	int ret;
+	u32 precharge, dclk, com_invdir, compins;
 
-	par->pwm = pwm_get(&par->client->dev, NULL);
-	if (IS_ERR(par->pwm)) {
-		dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
-		return PTR_ERR(par->pwm);
-	}
-
-	par->pwm_period = pwm_get_period(par->pwm);
-	/* Enable the PWM */
-	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
-	pwm_enable(par->pwm);
-
-	dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
-		par->pwm->pwm, par->pwm_period);
-
-	/* Map column 127 of the OLED to segment 0 */
-	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
-	if (ret < 0)
-		return ret;
-
-	/* Turn on the display */
-	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par)
-{
-	pwm_disable(par->pwm);
-	pwm_put(par->pwm);
-	return 0;
-}
+	if (par->device_info->device_id = DEVID_SSD1307) {
+		par->pwm = pwm_get(&par->client->dev, NULL);
+		if (IS_ERR(par->pwm)) {
+			dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
+			return PTR_ERR(par->pwm);
+		}
 
-static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
-	.init	= ssd1307fb_ssd1307_init,
-	.remove	= ssd1307fb_ssd1307_remove,
-};
+		par->pwm_period = pwm_get_period(par->pwm);
+		/* Enable the PWM */
+		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+		pwm_enable(par->pwm);
 
-static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
-{
-	int ret;
+		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);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x7f);
-	if (ret < 0)
-		return ret;
-
-	/* Set COM direction */
-	ret = ssd1307fb_write_cmd(par->client, 0xc8);
+	ret = ssd1307fb_write_cmd(par->client, par->contrast);
 	if (ret < 0)
 		return ret;
 
 	/* Set segment re-map */
-	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+	if (par->seg_remap) {
+		ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+		if (ret < 0)
+			return ret;
+	};
+
+	/* Set COM direction */
+	com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
+	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
 	if (ret < 0)
 		return ret;
 
@@ -334,34 +330,38 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x20);
+	ret = ssd1307fb_write_cmd(par->client, par->com_offset);
 	if (ret < 0)
 		return ret;
 
 	/* Set clock frequency */
+	dclk = (par->dclk_div & 0xf) | (par->dclk_frq & 0xf) << 4;
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_CLOCK_FREQ);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0xf0);
+	ret = ssd1307fb_write_cmd(par->client, dclk);
 	if (ret < 0)
 		return ret;
 
 	/* Set precharge period in number of ticks from the internal clock */
+	precharge = (par->prechargep1 & 0xf) | (par->prechargep2 & 0xf) << 4;
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PRECHARGE_PERIOD);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x22);
+	ret = ssd1307fb_write_cmd(par->client, precharge);
 	if (ret < 0)
 		return ret;
 
 	/* Set COM pins configuration */
+	compins = 0x02 | (!par->com_seq & 0x1) << 4
+				   | (par->com_lrremap & 0x1) << 5;
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x22);
+	ret = ssd1307fb_write_cmd(par->client, compins);
 	if (ret < 0)
 		return ret;
 
@@ -370,18 +370,20 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x49);
+	ret = ssd1307fb_write_cmd(par->client, par->vcomh);
 	if (ret < 0)
 		return ret;
 
-	/* Turn on the DC-DC Charge Pump */
-	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
-	if (ret < 0)
-		return ret;
+	if (par->device_info->device_id = DEVID_SSD1306) {
+		/* Turn on the DC-DC Charge Pump */
+		ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
+		if (ret < 0)
+			return ret;
 
-	ret = ssd1307fb_write_cmd(par->client, 0x14);
-	if (ret < 0)
-		return ret;
+		ret = ssd1307fb_write_cmd(par->client, 0x14);
+		if (ret < 0)
+			return ret;
+	};
 
 	/* Switch to horizontal addressing mode */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
@@ -393,6 +395,7 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
+    /* Set column range */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COL_RANGE);
 	if (ret < 0)
 		return ret;
@@ -405,6 +408,7 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
+    /* Set page range */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PAGE_RANGE);
 	if (ret < 0)
 		return ret;
@@ -426,18 +430,28 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
 	return 0;
 }
 
-static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
-	.init	= ssd1307fb_ssd1306_init,
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
+	.device_id  = DEVID_SSD1306,
+	.default_vcomh = 0x20,
+	.default_dclk_div = 0,
+	.default_dclk_frq = 8,
+};
+
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
+	.device_id  = DEVID_SSD1307,
+	.default_vcomh = 0x20,
+	.default_dclk_div = 1,
+	.default_dclk_frq = 12,
 };
 
 static const struct of_device_id ssd1307fb_of_match[] = {
 	{
 		.compatible = "solomon,ssd1306fb-i2c",
-		.data = (void *)&ssd1307fb_ssd1306_ops,
+		.data = (void *)&ssd1307fb_ssd1306_deviceinfo,
 	},
 	{
 		.compatible = "solomon,ssd1307fb-i2c",
-		.data = (void *)&ssd1307fb_ssd1307_ops,
+		.data = (void *)&ssd1307fb_ssd1307_deviceinfo,
 	},
 	{},
 };
@@ -468,8 +482,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	par->info = info;
 	par->client = client;
 
-	par->ops = (struct ssd1307fb_ops *)of_match_device(ssd1307fb_of_match,
-							   &client->dev)->data;
+	par->device_info = (struct ssd1307fb_deviceinfo *)of_match_device(
+			ssd1307fb_of_match, &client->dev)->data;
 
 	par->reset = of_get_named_gpio(client->dev.of_node,
 					 "reset-gpios", 0);
@@ -487,6 +501,27 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	if (of_property_read_u32(node, "solomon,page-offset", &par->page_offset))
 		par->page_offset = 1;
 
+	if (of_property_read_u32(node, "solomon,com-offset", &par->com_offset))
+		par->com_offset = 0;
+
+	if (of_property_read_u32(node, "solomon,prechargep1", &par->prechargep1))
+		par->prechargep1 = 2;
+
+	if (of_property_read_u32(node, "solomon,prechargep2", &par->prechargep2))
+		par->prechargep2 = 0;
+
+	par->seg_remap = !of_property_read_bool(node, "solomon,segment-no-remap");
+	par->com_seq = of_property_read_bool(node, "solomon,com-sequential");
+	par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap");
+	par->com_invdir = of_property_read_bool(node, "solomon,com-invdir");
+
+	par->contrast = contrast;
+	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;
+
 	vmem_size = par->width * par->height / 8;
 
 	vmem = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
@@ -539,11 +574,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	gpio_set_value(par->reset, 1);
 	udelay(4);
 
-	if (par->ops->init) {
-		ret = par->ops->init(par);
-		if (ret)
-			goto reset_oled_error;
-	}
+	ret = ssd1307fb_init(par);
+	if (ret)
+		goto reset_oled_error;
 
 	ret = register_framebuffer(info);
 	if (ret) {
@@ -556,8 +589,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	return 0;
 
 panel_init_error:
-	if (par->ops->remove)
-		par->ops->remove(par);
+	if (par->device_info->device_id = DEVID_SSD1307) {
+		pwm_disable(par->pwm);
+		pwm_put(par->pwm);
+	};
 reset_oled_error:
 	fb_deferred_io_cleanup(info);
 fb_alloc_error:
@@ -571,8 +606,10 @@ static int ssd1307fb_remove(struct i2c_client *client)
 	struct ssd1307fb_par *par = info->par;
 
 	unregister_framebuffer(info);
-	if (par->ops->remove)
-		par->ops->remove(par);
+	if (par->device_info->device_id = DEVID_SSD1307) {
+		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.3.0


^ permalink raw reply related

* [PATCHv4 03/10] of: Add Solomon Systech vendor prefix.
From: Thomas Niederprüm @ 2015-03-16 17:11 UTC (permalink / raw)
  To: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt
  Cc: linux-fbdev, linux-kernel, Thomas Niederprüm
In-Reply-To: <1426525918-12745-1-git-send-email-niederp@physik.uni-kl.de>

This patch adds the solomon prefix for Solomon Systech Limited.

Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d3f4809..933c8f5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -169,6 +169,7 @@ sitronix	Sitronix Technology Corporation
 smsc	Standard Microsystems Corporation
 snps	Synopsys, Inc.
 solidrun	SolidRun
+solomon        Solomon Systech Limited
 sony	Sony Corporation
 spansion	Spansion Inc.
 sprd	Spreadtrum Communications Inc.
-- 
2.3.0


^ permalink raw reply related

* [PATCHv4 02/10] fbdev: ssd1307fb: Allocate page aligned video memory.
From: Thomas Niederprüm @ 2015-03-16 17:11 UTC (permalink / raw)
  To: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt
  Cc: linux-fbdev, linux-kernel, Thomas Niederprüm
In-Reply-To: <1426525918-12745-1-git-send-email-niederp@physik.uni-kl.de>

Currently the videomemory is allocated by kmalloc, making it a memory
region that is not necessarily page aligend. This leads to problems
upon mmap call, where the video memory's address gets aligned to the
next page boundary. The result is that the userspace program that issued
the mmap call is not able to access the video memory from the start to
the next page boundary.

This patch changes the allocation of the video memory to use
__get_free_pages() in order to obtain memory that is aligned
to page boundaries.

Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
 drivers/video/fbdev/ssd1307fb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 61e0ce8..8d34c56 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -489,7 +489,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	vmem_size = par->width * par->height / 8;
 
-	vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
+	vmem = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+					get_order(vmem_size));
 	if (!vmem) {
 		dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
 		ret = -ENOMEM;
@@ -573,6 +574,7 @@ static int ssd1307fb_remove(struct i2c_client *client)
 	if (par->ops->remove)
 		par->ops->remove(par);
 	fb_deferred_io_cleanup(info);
+	__free_pages(__va(info->fix.smem_start), get_order(info->fix.smem_len));
 	framebuffer_release(info);
 
 	return 0;
-- 
2.3.0


^ permalink raw reply related

* [PATCHv4 01/10] fbdev: ssd1307fb: fix memory address smem_start.
From: Thomas Niederprüm @ 2015-03-16 17:11 UTC (permalink / raw)
  To: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt
  Cc: linux-fbdev, linux-kernel, Thomas Niederprüm
In-Reply-To: <1426525918-12745-1-git-send-email-niederp@physik.uni-kl.de>

the smem_start pointer of the framebuffer info struct needs to hold the
physical address rather than the logical address. Right now the logical
address returned by kmalloc is stored. This patch converts this address
to a physical address and thus fixes a driver crash on mmaping the
framebuffer memory due to an access to the wrong memory address.

Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 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 f7ed6d9..61e0ce8 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -515,7 +515,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	info->var.blue.offset = 0;
 
 	info->screen_base = (u8 __force __iomem *)vmem;
-	info->fix.smem_start = (unsigned long)vmem;
+	info->fix.smem_start = __pa(vmem);
 	info->fix.smem_len = vmem_size;
 
 	fb_deferred_io_init(info);
-- 
2.3.0


^ permalink raw reply related

* [PATCHv4 00/10] Cleanup and add support for SSD1305
From: Thomas Niederprüm @ 2015-03-16 17:11 UTC (permalink / raw)
  To: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt
  Cc: linux-fbdev, linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>

This patch series is the result of making the ssd1307fb driver work with
a Newhaven OLED display using the Solomon SSD1305 controller. To achieve
this the intialization code for the SSD1306 and the SSD1307 is merged
and based on DT configuration to reflect the various possible wirings
of the SSD130X controller (04/10). Based on these changes it is straight
forward to add support for the SSD1305 controller (06/10).

While working on the driver I realized that it was not possible to
correctly mmap the video memory from userspace since the address handed
to the userspace app is a logical one where it should be a physical one.
Patch 01/10 fixes this. Furthermore the memory reserved by kzalloc is
not page aligned while the address handed to userspace is aligned to the
next page frame. This problem is fixed by using __get_free_pages() in 02/10.

Furthermore a module parameter is added to set the delay for the
deferred io update (07/10). Also the backlight class is implemented to make
the contrast setting available in userspace (09/10).

changes since v1 (thanks to Maxime for the feedback):
- dedicated patch for fixing smem_start address
- remove page reserve upon vmalloc
- remove return value check upon display turn-off at module unload
- use a module parameter refreshrate rather than delaydivider
- allocate fbdefio dynamically
- use sysfs_create_groups to create sysfs entries
- remove contrast, vhcom and dclk properties from DT since they are
  not part of hw description. The contrast module parameter was added
  to set contrast at load time. vhcom and dclk stays at it's default
  values for now.
- add new DT properties to in tree users of ssd130X
- rebased to apply on top of linux-next

changes since v2 (thanks to Maxime again):
- free memory allocated by vmalloc on driver unload
- set default values in the init code to the ones of the existing ssd1307
  init code
- added two ACKs (Maxime Ripard)

changes since v3:
- use backlight class rather than dedicated sysfs files to set the 
  contrast (Thanks to Tomi Valkeinen)
- remove [PATCHv3 08/10] fbdev: ssd1307fb: Add module parameter bitsperpixel
- add new patch to blank the display (unreviewed)
- allocate video memory through __get_free_pages() rather than vmalloc
  (Thanks to Geert Uytterhoeven)
- minor rewordings of the commit messages

Thomas Niederprüm (10):
  fbdev: ssd1307fb: fix memory address smem_start.
  fbdev: ssd1307fb: Allocate page aligned video memory.
  of: Add Solomon Systech vendor prefix.
  fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
  ARM: mxs: fix in tree users of ssd1306
  fbdev: ssd1307fb: Add support for SSD1305
  fbdev: ssd1307fb: Add a module parameter to set the refresh rate
  fbdev: ssd1307fb: Turn off display on driver unload.
  fbdev: ssd1307fb: add backlight controls for setting the contrast
  fbdev: ssd1307fb: Add blank mode

 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 .../devicetree/bindings/video/ssd1307fb.txt        |  23 +-
 arch/arm/boot/dts/imx28-cfa10036.dts               |   3 +
 drivers/video/fbdev/Kconfig                        |   1 +
 drivers/video/fbdev/ssd1307fb.c                    | 310 +++++++++++++++------
 5 files changed, 250 insertions(+), 88 deletions(-)

-- 
2.3.0


^ permalink raw reply

* Re: [PATCH] staging:sm750fb:Fixed no space and indent warnings
From: Sudip Mukherjee @ 2015-03-16 14:20 UTC (permalink / raw)
  To: Ragavendra Nagraj; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150316002109.GA6640@localhost.localdomain>

On Sun, Mar 15, 2015 at 05:21:09PM -0700, Ragavendra Nagraj wrote:
the convention to write the subject is :
staging: sm750fb: your subject line
this is for all drivers, if you see the mails in lkml you will see
that almost everyone follows the same convention.

> 
> Signed-off-by: Ragavendra Nagraj <ragavendra.bn@gmail.com>
> ---
<snip>
>  
>  unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL)
>  {
> -    unsigned int ulPllReg = 0;
> -
> -    pPLL->inputFreq = DEFAULT_INPUT_CLOCK;
> -    pPLL->clockType = clockType;
> -
> -    switch (clockType)
> -    {
> -        case MXCLK_PLL:
> -            ulPllReg = PEEK32(MXCLK_PLL_CTRL);
> -            break;
<snip>
> +
> +	switch (clockType)
> +	{
> +	case MXCLK_PLL:
> +		ulPllReg = PEEK32(MXCLK_PLL_CTRL);
> +		break;
you changed the switch-case indention here in this function. but you
have not changed the switch-case indention in the function
setMemoryClock() or in setMasterClock() later in your patch.
but again changed the indention in the function ddk750_getVMSize().
any reason? was that intentional?

> +	case PRIMARY_PLL:
<snip>
> -
> -        /* Master Clock Control: MXCLK_PLL */
> -        POKE32(MXCLK_PLL_CTRL, formatPllReg(&pll));
> -    }
> +	if (frequency != 0)
> +		{
an extra tab came here.	
> +		/*
<snip>		   
>  #ifdef VALIDATION_CHIP
> -                        if (OD > 2)
> -                            POD = 2;
> -                        else
> -                            POD = OD;
> +			if (OD > 2)
> +				POD = 2;
> +			else
> +				POD = OD;
>  #endif
>  
> -                        pPLL->POD = POD;
> -                    }
> -                }
> -            }
> -        }
> -    }
> +			pPLL->POD = POD;
> +			}
> +		}
> +		}
	     ^^^^^^^
problem with indention with this code block.
and also in some places in your patch you changed the indention of opening brace
of the for loop.
the code was:
for ( ; ; )
{

you are making it: 
for ( ; ; )
	{

well, ultimately that opening brace of switch-case, for loops and if
will go to the previous line, so i think this extra indention will not
hurt, but I am not sure. Dan Carpenter or Joe Perches or Greg can say
if this extra indention is acceptable.
But the indention problem marked just above this will not be accepted.

regards
sudip

> +	}

^ permalink raw reply

* [PATCH] staging/sm75fb: Declare static functions as such
From: Ricardo Ribalda Delgado @ 2015-03-16 10:51 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman, linux-fbdev,
	devel, linux-kernel
  Cc: Ricardo Ribalda Delgado

This patch fixes this sparse warning

CHECK   drivers/staging/sm750fb/ddk750_swi2c.c
drivers/staging/sm750fb/ddk750_swi2c.c:223:6: warning: symbol
  'swI2CStart' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:234:6: warning: symbol
  'swI2CStop' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:252:6: warning: symbol
  'swI2CWriteByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:320:15: warning: symbol
  'swI2CReadByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:361:6: warning: symbol
  'swI2CInit_SM750LE' was not declared. Should it be static?

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/staging/sm750fb/ddk750_swi2c.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 1249759..231ec85 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -220,7 +220,7 @@ static void swI2CAck(void)
 /*
  *  This function sends the start command to the slave device
  */
-void swI2CStart(void)
+static void swI2CStart(void)
 {
     /* Start I2C */
     swI2CSDA(1);
@@ -231,7 +231,7 @@ void swI2CStart(void)
 /*
  *  This function sends the stop command to the slave device
  */
-void swI2CStop(void)
+static void swI2CStop(void)
 {
     /* Stop the I2C */
     swI2CSCL(1);
@@ -249,7 +249,7 @@ void swI2CStop(void)
  *       0   - Success
  *      -1   - Fail to write byte
  */
-long swI2CWriteByte(unsigned char data)
+static long swI2CWriteByte(unsigned char data)
 {
     unsigned char value = data;
     int i;
@@ -317,7 +317,7 @@ long swI2CWriteByte(unsigned char data)
  *  Return Value:
  *      One byte data read from the Slave device
  */
-unsigned char swI2CReadByte(unsigned char ack)
+static unsigned char swI2CReadByte(unsigned char ack)
 {
     int i;
     unsigned char data = 0;
@@ -358,10 +358,8 @@ unsigned char swI2CReadByte(unsigned char ack)
  *      -1   - Fail to initialize the i2c
  *       0   - Success
  */
-long swI2CInit_SM750LE(
-    unsigned char i2cClkGPIO,
-    unsigned char i2cDataGPIO
-)
+static long swI2CInit_SM750LE(unsigned char i2cClkGPIO,
+			      unsigned char i2cDataGPIO)
 {
     int i;
 
-- 
2.1.4


^ permalink raw reply related

* [PATCH] staging:sm750fb:Fixed no space and indent warnings
From: Ragavendra Nagraj @ 2015-03-16  0:21 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel

This patch fixes the no spaces and indent warnings identified by the
checkpath.pl script for the entire ddk750_chip.c file by using appropriate tab
spaces and indents accordingly.

Signed-off-by: Ragavendra Nagraj <ragavendra.bn@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c |  454 ++++++++++++++++-----------------
 1 file changed, 227 insertions(+), 227 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index b71169e..5c9a118 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -20,22 +20,22 @@ logical_chip_type_t getChipType()
 	physicalID = devId750;//either 0x718 or 0x750
 	physicalRev = revId750;
 
-    if (physicalID = 0x718)
-    {
-        chip = SM718;
-    }
-    else if (physicalID = 0x750)
-    {
-        chip = SM750;
+	if (physicalID = 0x718)
+	{
+		chip = SM718;
+	}
+	else if (physicalID = 0x750)
+	{
+		chip = SM750;
 		/* SM750 and SM750LE are different in their revision ID only. */
 		if (physicalRev = SM750LE_REVISION_ID){
 			chip = SM750LE;
 		}
-    }
-    else
-    {
-        chip = SM_UNKNOWN;
-    }
+	}
+	else
+	{
+		chip = SM_UNKNOWN;
+	}
 
 	return chip;
 }
@@ -43,63 +43,63 @@ logical_chip_type_t getChipType()
 
 inline unsigned int twoToPowerOfx(unsigned long x)
 {
-    unsigned long i;
-    unsigned long result = 1;
+	unsigned long i;
+	unsigned long result = 1;
 
-    for (i=1; i<=x; i++)
-        result *= 2;
-    return result;
+	for (i=1; i<=x; i++)
+		result *= 2;
+	return result;
 }
 
 inline unsigned int calcPLL(pll_value_t *pPLL)
 {
-    return (pPLL->inputFreq * pPLL->M / pPLL->N / twoToPowerOfx(pPLL->OD) / twoToPowerOfx(pPLL->POD));
+	return (pPLL->inputFreq * pPLL->M / pPLL->N / twoToPowerOfx(pPLL->OD) / twoToPowerOfx(pPLL->POD));
 }
 
 unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL)
 {
-    unsigned int ulPllReg = 0;
-
-    pPLL->inputFreq = DEFAULT_INPUT_CLOCK;
-    pPLL->clockType = clockType;
-
-    switch (clockType)
-    {
-        case MXCLK_PLL:
-            ulPllReg = PEEK32(MXCLK_PLL_CTRL);
-            break;
-        case PRIMARY_PLL:
-            ulPllReg = PEEK32(PANEL_PLL_CTRL);
-            break;
-        case SECONDARY_PLL:
-            ulPllReg = PEEK32(CRT_PLL_CTRL);
-            break;
-        case VGA0_PLL:
-            ulPllReg = PEEK32(VGA_PLL0_CTRL);
-            break;
-        case VGA1_PLL:
-            ulPllReg = PEEK32(VGA_PLL1_CTRL);
-            break;
-    }
-
-    pPLL->M = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, M);
-    pPLL->N = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, N);
-    pPLL->OD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, OD);
-    pPLL->POD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, POD);
-
-    return calcPLL(pPLL);
+	unsigned int ulPllReg = 0;
+
+	pPLL->inputFreq = DEFAULT_INPUT_CLOCK;
+	pPLL->clockType = clockType;
+
+	switch (clockType)
+	{
+	case MXCLK_PLL:
+		ulPllReg = PEEK32(MXCLK_PLL_CTRL);
+		break;
+	case PRIMARY_PLL:
+		ulPllReg = PEEK32(PANEL_PLL_CTRL);
+		break;
+	case SECONDARY_PLL:
+		ulPllReg = PEEK32(CRT_PLL_CTRL);
+		break;
+	case VGA0_PLL:
+		ulPllReg = PEEK32(VGA_PLL0_CTRL);
+		break;
+	case VGA1_PLL:
+		ulPllReg = PEEK32(VGA_PLL1_CTRL);
+		break;
+	}
+
+	pPLL->M = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, M);
+	pPLL->N = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, N);
+	pPLL->OD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, OD);
+	pPLL->POD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, POD);
+
+	return calcPLL(pPLL);
 }
 
 
 unsigned int getChipClock()
 {
-    pll_value_t pll;
+	pll_value_t pll;
 #if 1
 	if(getChipType() = SM750LE)
 		return MHz(130);
 #endif
 
-    return getPllValue(MXCLK_PLL, &pll);
+	return getPllValue(MXCLK_PLL, &pll);
 }
 
 
@@ -110,75 +110,75 @@ unsigned int getChipClock()
  */
 void setChipClock(unsigned int frequency)
 {
-    pll_value_t pll;
-    unsigned int ulActualMxClk;
+	pll_value_t pll;
+	unsigned int ulActualMxClk;
 #if 1
 		/* Cheok_0509: For SM750LE, the chip clock is fixed. Nothing to set. */
 		if (getChipType() = SM750LE)
 			return;
 #endif
 
-    if (frequency != 0)
-    {
-        /*
-         * Set up PLL, a structure to hold the value to be set in clocks.
-         */
-        pll.inputFreq = DEFAULT_INPUT_CLOCK; /* Defined in CLOCK.H */
-        pll.clockType = MXCLK_PLL;
-
-        /*
-         * Call calcPllValue() to fill up the other fields for PLL structure.
-         * Sometime, the chip cannot set up the exact clock required by User.
-         * Return value from calcPllValue() gives the actual possible clock.
-         */
-        ulActualMxClk = calcPllValue(frequency, &pll);
-
-        /* Master Clock Control: MXCLK_PLL */
-        POKE32(MXCLK_PLL_CTRL, formatPllReg(&pll));
-    }
+	if (frequency != 0)
+		{
+		/*
+		* Set up PLL, a structure to hold the value to be set in clocks.
+		*/
+		pll.inputFreq = DEFAULT_INPUT_CLOCK; /* Defined in CLOCK.H */
+		pll.clockType = MXCLK_PLL;
+
+		/*
+		* Call calcPllValue() to fill up the other fields for PLL structure.
+		* Sometime, the chip cannot set up the exact clock required by User.
+		* Return value from calcPllValue() gives the actual possible clock.
+		*/
+		ulActualMxClk = calcPllValue(frequency, &pll);
+
+		/* Master Clock Control: MXCLK_PLL */
+		POKE32(MXCLK_PLL_CTRL, formatPllReg(&pll));
+	}
 }
 
 
 
 void setMemoryClock(unsigned int frequency)
 {
-    unsigned int ulReg, divisor;
+	unsigned int ulReg, divisor;
  #if 1
 	/* Cheok_0509: For SM750LE, the memory clock is fixed. Nothing to set. */
 	if (getChipType() = SM750LE)
 		return;
 #endif
-    if (frequency != 0)
-    {
-        /* Set the frequency to the maximum frequency that the DDR Memory can take
-           which is 336MHz. */
-        if (frequency > MHz(336))
-            frequency = MHz(336);
-
-        /* Calculate the divisor */
-        divisor = (unsigned int) roundedDiv(getChipClock(), frequency);
-
-        /* Set the corresponding divisor in the register. */
-        ulReg = PEEK32(CURRENT_GATE);
-        switch(divisor)
-        {
-            default:
-            case 1:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_1);
-                break;
-            case 2:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_2);
-                break;
-            case 3:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_3);
-                break;
-            case 4:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_4);
-                break;
-        }
-
-        setCurrentGate(ulReg);
-    }
+	if (frequency != 0)
+		{
+		/* Set the frequency to the maximum frequency that the DDR Memory can take
+		which is 336MHz. */
+		if (frequency > MHz(336))
+			frequency = MHz(336);
+
+		/* Calculate the divisor */
+		divisor = (unsigned int) roundedDiv(getChipClock(), frequency);
+
+		/* Set the corresponding divisor in the register. */
+		ulReg = PEEK32(CURRENT_GATE);
+		switch(divisor)
+		{
+			default:
+			case 1:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_1);
+				break;
+			case 2:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_2);
+				break;
+			case 3:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_3);
+				break;
+			case 4:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, M2XCLK, DIV_4);
+				break;
+		}
+
+		setCurrentGate(ulReg);
+		}
 }
 
 
@@ -192,43 +192,43 @@ void setMemoryClock(unsigned int frequency)
  */
 void setMasterClock(unsigned int frequency)
 {
-    unsigned int ulReg, divisor;
+	unsigned int ulReg, divisor;
   #if 1
 	/* Cheok_0509: For SM750LE, the memory clock is fixed. Nothing to set. */
 	if (getChipType() = SM750LE)
 		return;
 #endif
-    if (frequency != 0)
-    {
-        /* Set the frequency to the maximum frequency that the SM750 engine can
-           run, which is about 190 MHz. */
-        if (frequency > MHz(190))
-            frequency = MHz(190);
-
-        /* Calculate the divisor */
-        divisor = (unsigned int) roundedDiv(getChipClock(), frequency);
-
-        /* Set the corresponding divisor in the register. */
-        ulReg = PEEK32(CURRENT_GATE);
-        switch(divisor)
-        {
-            default:
-            case 3:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_3);
-                break;
-            case 4:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_4);
-                break;
-            case 6:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_6);
-                break;
-            case 8:
-                ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_8);
-                break;
-        }
-
-        setCurrentGate(ulReg);
-    }
+	if (frequency != 0)
+		{
+		/* Set the frequency to the maximum frequency that the SM750 engine can
+		run, which is about 190 MHz. */
+		if (frequency > MHz(190))
+			frequency = MHz(190);
+
+		/* Calculate the divisor */
+		divisor = (unsigned int) roundedDiv(getChipClock(), frequency);
+
+		/* Set the corresponding divisor in the register. */
+		ulReg = PEEK32(CURRENT_GATE);
+		switch(divisor)
+		{
+			default:
+			case 3:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_3);
+				break;
+			case 4:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_4);
+				break;
+			case 6:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_6);
+				break;
+			case 8:
+				ulReg = FIELD_SET(ulReg, CURRENT_GATE, MCLK, DIV_8);
+				break;
+		}
+
+		setCurrentGate(ulReg);
+		}
 }
 
 
@@ -249,11 +249,11 @@ unsigned int ddk750_getVMSize()
 	/* get frame buffer size from GPIO */
 	reg = FIELD_GET(PEEK32(MISC_CTRL),MISC_CTRL,LOCALMEM_SIZE);
 	switch(reg){
-        case MISC_CTRL_LOCALMEM_SIZE_8M:  data = MB(8);  break; /* 8  Mega byte */
-        case MISC_CTRL_LOCALMEM_SIZE_16M: data = MB(16); break; /* 16 Mega byte */
-        case MISC_CTRL_LOCALMEM_SIZE_32M: data = MB(32); break; /* 32 Mega byte */
-        case MISC_CTRL_LOCALMEM_SIZE_64M: data = MB(64); break; /* 64 Mega byte */
-		default: data = 0;break;
+	case MISC_CTRL_LOCALMEM_SIZE_8M:  data = MB(8);  break; /* 8  Mega byte */
+	case MISC_CTRL_LOCALMEM_SIZE_16M: data = MB(16); break; /* 16 Mega byte */
+	case MISC_CTRL_LOCALMEM_SIZE_32M: data = MB(32); break; /* 32 Mega byte */
+	case MISC_CTRL_LOCALMEM_SIZE_64M: data = MB(64); break; /* 64 Mega byte */
+	default: data = 0;break;
 	}
 	return data;
 
@@ -391,10 +391,10 @@ int ddk750_initHw(initchip_param_t * pInitParam)
 
 unsigned int absDiff(unsigned int a, unsigned int b)
 {
-    if ( a > b )
-        return(a - b);
-    else
-        return(b - a);
+	if ( a > b )
+		return(a - b);
+	else
+	return(b - a);
 }
 
 #endif
@@ -435,7 +435,7 @@ unsigned int calcPllValue(unsigned int request_orig,pll_value_t *pll)
 							{3,0,3,8},
 							};
 
-	/* 	as sm750 register definition, N located in 2,15 and M located in 1,255	*/
+	/* as sm750 register definition, N located in 2,15 and M located in 1,255	*/
 	int N,M,X,d;
 	int xcnt;
 	int miniDiff;
@@ -446,11 +446,11 @@ unsigned int calcPllValue(unsigned int request_orig,pll_value_t *pll)
 
 #if 1
 	if (getChipType() = SM750LE)
-    {
-        /* SM750LE don't have prgrammable PLL and M/N values to work on.
-           Just return the requested clock. */
-        return request_orig;
-    }
+	{
+		/* SM750LE don't have prgrammable PLL and M/N values to work on.
+		Just return the requested clock. */
+		return request_orig;
+	}
 #endif
 
 	ret = 0;
@@ -487,7 +487,7 @@ unsigned int calcPllValue(unsigned int request_orig,pll_value_t *pll)
 			{
 				unsigned int diff;
 				tmpClock = pll->inputFreq *M / N / X;
-                diff = absDiff(tmpClock,request_orig);
+				diff = absDiff(tmpClock,request_orig);
 				if(diff < miniDiff)
 				{
 					pll->M = M;
@@ -510,104 +510,104 @@ unsigned int ulRequestClk, /* Required pixel clock in Hz unit */
 pll_value_t *pPLL           /* Structure to hold the value to be set in PLL */
 )
 {
-    unsigned int M, N, OD, POD = 0, diff, pllClk, odPower, podPower;
-    unsigned int bestDiff = 0xffffffff; /* biggest 32 bit unsigned number */
+	unsigned int M, N, OD, POD = 0, diff, pllClk, odPower, podPower;
+	unsigned int bestDiff = 0xffffffff; /* biggest 32 bit unsigned number */
 	unsigned int ret;
     /* Init PLL structure to know states */
-    pPLL->M = 0;
-    pPLL->N = 0;
-    pPLL->OD = 0;
-    pPLL->POD = 0;
+	pPLL->M = 0;
+	pPLL->N = 0;
+	pPLL->OD = 0;
+	pPLL->POD = 0;
 
     /* Sanity check: None at the moment */
 
     /* Convert everything in Khz range in order to avoid calculation overflow */
-    pPLL->inputFreq /= 1000;
-    ulRequestClk /= 1000;
+	pPLL->inputFreq /= 1000;
+	ulRequestClk /= 1000;
 
 #ifndef VALIDATION_CHIP
     /* The maximum of post divider is 8. */
-    for (POD=0; POD<=3; POD++)
+	for (POD=0; POD<=3; POD++)
 #endif
-    {
+		{
 
 #ifndef VALIDATION_CHIP
-        /* MXCLK_PLL does not have post divider. */
-        if ((POD > 0) && (pPLL->clockType = MXCLK_PLL))
-            break;
+	/* MXCLK_PLL does not have post divider. */
+	if ((POD > 0) && (pPLL->clockType = MXCLK_PLL))
+		break;
 #endif
 
-        /* Work out 2 to the power of POD */
-        podPower = twoToPowerOfx(POD);
+	/* Work out 2 to the power of POD */
+	podPower = twoToPowerOfx(POD);
 
-        /* OD has only 2 bits [15:14] and its value must between 0 to 3 */
-        for (OD=0; OD<=3; OD++)
-        {
-            /* Work out 2 to the power of OD */
-            odPower = twoToPowerOfx(OD);
+	/* OD has only 2 bits [15:14] and its value must between 0 to 3 */
+	for (OD=0; OD<=3; OD++)
+		{
+		/* Work out 2 to the power of OD */
+		odPower = twoToPowerOfx(OD);
 
 #ifdef VALIDATION_CHIP
-            if (odPower > 4)
-                podPower = 4;
-            else
-                podPower = odPower;
+	if (odPower > 4)
+		podPower = 4;
+	else
+		podPower = odPower;
 #endif
 
-            /* N has 4 bits [11:8] and its value must between 2 and 15.
-               The N = 1 will behave differently --> Result is not correct. */
-            for (N=2; N<\x15; N++)
-            {
-                /* The formula for PLL is ulRequestClk = inputFreq * M / N / (2^OD)
-                   In the following steps, we try to work out a best M value given the others are known.
-                   To avoid decimal calculation, we use 1000 as multiplier for up to 3 decimal places of accuracy.
-                */
-                M = ulRequestClk * N * odPower * 1000 / pPLL->inputFreq;
-                M = roundedDiv(M, 1000);
-
-                /* M field has only 8 bits, reject value bigger than 8 bits */
-                if (M < 256)
-                {
-                    /* Calculate the actual clock for a given M & N */
-                    pllClk = pPLL->inputFreq * M / N / odPower / podPower;
-
-                    /* How much are we different from the requirement */
-                    diff = absDiff(pllClk, ulRequestClk);
-
-                    if (diff < bestDiff)
-                    {
-                        bestDiff = diff;
-
-                        /* Store M and N values */
-                        pPLL->M  = M;
-                        pPLL->N  = N;
-                        pPLL->OD = OD;
+		/* N has 4 bits [11:8] and its value must between 2 and 15.
+		The N = 1 will behave differently --> Result is not correct. */
+	for (N=2; N<\x15; N++)
+		{
+		/* The formula for PLL is ulRequestClk = inputFreq * M / N / (2^OD)
+		In the following steps, we try to work out a best M value given the others are known.
+		To avoid decimal calculation, we use 1000 as multiplier for up to 3 decimal places of accuracy.
+		*/
+		M = ulRequestClk * N * odPower * 1000 / pPLL->inputFreq;
+		M = roundedDiv(M, 1000);
+
+		/* M field has only 8 bits, reject value bigger than 8 bits */
+		if (M < 256)
+		{
+			/* Calculate the actual clock for a given M & N */
+			pllClk = pPLL->inputFreq * M / N / odPower / podPower;
+
+			/* How much are we different from the requirement */
+			diff = absDiff(pllClk, ulRequestClk);
+
+			if (diff < bestDiff)
+			{
+				bestDiff = diff;
+
+				/* Store M and N values */
+				pPLL->M  = M;
+				pPLL->N  = N;
+				pPLL->OD = OD;
 
 #ifdef VALIDATION_CHIP
-                        if (OD > 2)
-                            POD = 2;
-                        else
-                            POD = OD;
+			if (OD > 2)
+				POD = 2;
+			else
+				POD = OD;
 #endif
 
-                        pPLL->POD = POD;
-                    }
-                }
-            }
-        }
-    }
+			pPLL->POD = POD;
+			}
+		}
+		}
+	}
+	}
 
     /* Restore input frequency from Khz to hz unit */
 //    pPLL->inputFreq *= 1000;
-    ulRequestClk *= 1000;
-    pPLL->inputFreq = DEFAULT_INPUT_CLOCK; /* Default reference clock */
+	ulRequestClk *= 1000;
+	pPLL->inputFreq = DEFAULT_INPUT_CLOCK; /* Default reference clock */
 
     /* Output debug information */
-    //DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Requested Frequency = %d\n", ulRequestClk));
-    //DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Input CLK = %dHz, M=%d, N=%d, OD=%d, POD=%d\n", pPLL->inputFreq, pPLL->M, pPLL->N, pPLL->OD, pPLL->POD));
+	//DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Requested Frequency = %d\n", ulRequestClk));
+	//DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Input CLK = %dHz, M=%d, N=%d, OD=%d, POD=%d\n", pPLL->inputFreq, pPLL->M, pPLL->N, pPLL->OD, pPLL->POD));
 
     /* Return actual frequency that the PLL can set */
 	ret = calcPLL(pPLL);
-    return ret;
+	return ret;
 }
 
 
@@ -616,24 +616,24 @@ pll_value_t *pPLL           /* Structure to hold the value to be set in PLL */
 
 unsigned int formatPllReg(pll_value_t *pPLL)
 {
-    unsigned int ulPllReg = 0;
+	unsigned int ulPllReg = 0;
 
     /* Note that all PLL's have the same format. Here, we just use Panel PLL parameter
        to work out the bit fields in the register.
        On returning a 32 bit number, the value can be applied to any PLL in the calling function.
     */
-    ulPllReg -        FIELD_SET(  0, PANEL_PLL_CTRL, BYPASS, OFF)
-      | FIELD_SET(  0, PANEL_PLL_CTRL, POWER,  ON)
-      | FIELD_SET(  0, PANEL_PLL_CTRL, INPUT,  OSC)
+	ulPllReg +	FIELD_SET(  0, PANEL_PLL_CTRL, BYPASS, OFF)
+	| FIELD_SET(  0, PANEL_PLL_CTRL, POWER,  ON)
+	| FIELD_SET(  0, PANEL_PLL_CTRL, INPUT,  OSC)
 #ifndef VALIDATION_CHIP
-      | FIELD_VALUE(0, PANEL_PLL_CTRL, POD,    pPLL->POD)
+	| FIELD_VALUE(0, PANEL_PLL_CTRL, POD,    pPLL->POD)
 #endif
-      | FIELD_VALUE(0, PANEL_PLL_CTRL, OD,     pPLL->OD)
-      | FIELD_VALUE(0, PANEL_PLL_CTRL, N,      pPLL->N)
-      | FIELD_VALUE(0, PANEL_PLL_CTRL, M,      pPLL->M);
+	| FIELD_VALUE(0, PANEL_PLL_CTRL, OD,     pPLL->OD)
+	| FIELD_VALUE(0, PANEL_PLL_CTRL, N,      pPLL->N)
+	| FIELD_VALUE(0, PANEL_PLL_CTRL, M,      pPLL->M);
 
-    return(ulPllReg);
+	return(ulPllReg);
 }
 
 
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH] video: fbdev: sh_mobile_lcdcfb: Fix ROP3 sysfs attribute parsing
From: Geert Uytterhoeven @ 2015-03-15  9:32 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1426347519-2541-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

On Sat, Mar 14, 2015 at 4:38 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The ROP3 attribute is expressed as an integer in the 0-255 range. Remove
> the wrong conversion to boolean when parsing it.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-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

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Geert Uytterhoeven @ 2015-03-14 22:02 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: Tomi Valkeinen, Maxime Ripard, Linux Fbdev development list,
	Jean-Christophe PLAGNIOL-VILLARD, linux-kernel@vger.kernel.org
In-Reply-To: <20150313223128.2a3a682e@maestro.intranet>

On Fri, Mar 13, 2015 at 10:31 PM, Thomas Niederprüm
<niederp@physik.uni-kl.de> wrote:
> Am Tue, 10 Mar 2015 13:28:25 +0200
> schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>> Also, isn't doing __pa() for the memory returned by vmalloc plain
>> wrong?
>
>> What was the crash about when using kmalloc? It would be good to fix
>> defio, as I don't see why it should not work with kmalloced memory.
>
> The main challenge here is that the memory handed to userspace upon
> mmap call needs to be page aligned. The memory returned by kmalloc has
> no such alignment, but the pointer presented to the userspace program
> gets aligned to next page boundary. It's not clear to me whether there
> is an easy way to obtain page aligned kmalloc memory. Memory
> allocated by vmalloc on the other hand is always aligned to page
> boundaries. This is why I chose to go for vmalloc.

__get_free_pages()?

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

* Re: [PATCH 1/2] vt: add cursor blink interval escape sequence
From: Scot Doyle @ 2015-03-14 17:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Michael Kerrisk, Pavel Machek, Geert Uytterhoeven,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.LNX.2.11.1502271911380.4248@localhost>

On Fri, 27 Feb 2015, Scot Doyle wrote:
> Add an escape sequence to specify the current console's cursor blink
> interval. The interval is specified as a number of milliseconds until
> the next cursor display state toggle, from 50 to 65535. /proc/loadavg
> did not show a difference with a one msec interval, but the lower
> bound is set to 50 msecs since slower hardware wasn't tested.
> 
> Store the interval in the vc_data structure for later access by fbcon,
> initializing the value to fbcon's current hardcoded value of 200 msecs.
> 
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>

Hi Greg, sorry about your backlog. Is it too soon for a ping?

> ---
>  drivers/tty/vt/vt.c            | 9 +++++++++
>  include/linux/console_struct.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 6e00572..ab1f173 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -135,6 +135,7 @@ const struct consw *conswitchp;
>   */
>  #define DEFAULT_BELL_PITCH	750
>  #define DEFAULT_BELL_DURATION	(HZ/8)
> +#define DEFAULT_CURSOR_BLINK_MS	200
>  
>  struct vc vc_cons [MAX_NR_CONSOLES];
>  
> @@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
>  		case 15: /* activate the previous console */
>  			set_console(last_console);
>  			break;
> +		case 16: /* set cursor blink duration in msec */
> +			if (vc->vc_npar >= 1 && vc->vc_par[1] >= 50 &&
> +					vc->vc_par[1] <= USHRT_MAX)
> +				vc->vc_cur_blink_ms = vc->vc_par[1];
> +			else
> +				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
> +			break;
>  	}
>  }
>  
> @@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
>  
>  	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
>  	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
> +	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
>  
>  	gotoxy(vc, 0, 0);
>  	save_cur(vc);
> diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
> index e859c98..e329ee2 100644
> --- a/include/linux/console_struct.h
> +++ b/include/linux/console_struct.h
> @@ -104,6 +104,7 @@ struct vc_data {
>  	unsigned int    vc_resize_user;         /* resize request from user */
>  	unsigned int	vc_bell_pitch;		/* Console bell pitch */
>  	unsigned int	vc_bell_duration;	/* Console bell duration */
> +	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
>  	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
>  	struct uni_pagedir *vc_uni_pagedir;
>  	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */
> -- 
> 2.3.0
> 

^ permalink raw reply

* [PATCH] video: fbdev: sh_mobile_lcdcfb: Fix ROP3 sysfs attribute parsing
From: Laurent Pinchart @ 2015-03-14 15:38 UTC (permalink / raw)
  To: linux-fbdev

The ROP3 attribute is expressed as an integer in the 0-255 range. Remove
the wrong conversion to boolean when parsing it.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index d3013cd9f976..23421ec1c4e4 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1461,7 +1461,7 @@ overlay_rop3_store(struct device *dev, struct device_attribute *attr,
 	unsigned int rop3;
 	char *endp;
 
-	rop3 = !!simple_strtoul(buf, &endp, 10);
+	rop3 = simple_strtoul(buf, &endp, 10);
 	if (isspace(*endp))
 		endp++;
 
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related

* Re: [PATCH] staging: sm750fb: braces, indents, spaces fix
From: Greg KH @ 2015-03-14  8:39 UTC (permalink / raw)
  To: Ragavendra
  Cc: Dan Carpenter, Sudip Mukherjee, teddy.wang, linux-fbdev, devel,
	linux-kernel
In-Reply-To: <CAPBgb4=np7Bs=sMTZHpZZ1Ef7uriUDYjjz6Y7+49Vs1rOVo59w@mail.gmail.com>

On Fri, Mar 13, 2015 at 09:01:48AM -0700, Ragavendra wrote:
> BN is my initial, should it be expanded?

Yes.


^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-03-13 21:31 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Maxime Ripard, linux-fbdev, plagnioj, linux-kernel
In-Reply-To: <54FED559.2030506@ti.com>

Am Tue, 10 Mar 2015 13:28:25 +0200
schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:

> On 14/02/15 16:22, Thomas Niederprüm wrote:
> > Am Thu, 12 Feb 2015 16:11:21 +0100
> > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > 
> >> On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> >>> Am Sat, 7 Feb 2015 12:18:21 +0100
> >>> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> >>>
> >>>> Hi,
> >>>>
> >>>> On Fri, Feb 06, 2015 at 11:28:10PM +0100,
> >>>> niederp@physik.uni-kl.de wrote:
> >>>>> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >>>>>
> >>>>> It makes sense to use vmalloc to allocate the video buffer
> >>>>> since it has to be page aligned memory for using it with mmap.
> >>>>
> >>>> Please wrap your commit log at 80 chars.
> >>>
> >>> I'll try to do so in future, sorry for that.
> >>>
> >>>>
> >>>> It looks like there's numerous fbdev drivers using this
> >>>> (especially since you copy pasted that code, without mentionning
> >>>> it).
> >>>
> >>> Yes, I should have mentioned that in the commit message. As
> >>> implicitly indicated in the cover letter the rvmalloc() and
> >>> rvfree() are copy pasted from the vfb driver. Honestly, I didn't
> >>> give this one too much thought. It seemed a viable solution to the
> >>> mmap problem. For a bit more history on that, see my comment
> >>> below.
> >>>
> >>>>
> >>>> That should be turned into an allocator so that drivers all get
> >>>> this right.
> >>>>
> >>>>> Also deffered io seems buggy in combination with kmalloc'ed
> >>>>> memory (crash on unloading the module).
> >>>>
> >>>> And maybe that's the real issue to fix.
> >>>
> >>> The problem is solved by using vmalloc ;)
> >>
> >> Yep, but why do you need to mark the reserved pages?
> >>
> >> ...
> > 
> > As far as I understood mmaped memory is marked as userspace memory
> > in the page table and is therefore subject to swapping. The pages
> > are marked reserved to make clear that this memory can not be
> > swapped and thus lock the pages in memory. See discussions [0,1,2]. 
> 
> Why is it a problem if it is swapped? Only CPU uses the memory, as far
> as I can see.

It seems to be no problem at all. I was copying the allocation code
from the vfb driver. The memory is no longer marked as reserved from
v2 on.

> Also, isn't doing __pa() for the memory returned by vmalloc plain
> wrong?

> What was the crash about when using kmalloc? It would be good to fix
> defio, as I don't see why it should not work with kmalloced memory.

The main challenge here is that the memory handed to userspace upon
mmap call needs to be page aligned. The memory returned by kmalloc has
no such alignment, but the pointer presented to the userspace program
gets aligned to next page boundary. It's not clear to me whether there
is an easy way to obtain page aligned kmalloc memory. Memory
allocated by vmalloc on the other hand is always aligned to page
boundaries. This is why I chose to go for vmalloc.

Thomas

^ permalink raw reply

* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Thomas Niederprüm @ 2015-03-13 19:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Maxime Ripard, linux-fbdev, plagnioj, linux-kernel
In-Reply-To: <54FECB5D.1020908@ti.com>

Am Tue, 10 Mar 2015 12:45:49 +0200
schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:

> On 14/02/15 17:54, Maxime Ripard wrote:
> > On Sat, Feb 07, 2015 at 05:05:03PM +0100, Thomas Niederprüm wrote:
> >> Am Sat, 7 Feb 2015 12:20:43 +0100
> >> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> >>
> >>> On Fri, Feb 06, 2015 at 11:28:11PM +0100, niederp@physik.uni-kl.de
> >>> wrote:
> >>>> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >>>>
> >>>> This patch adds a module parameter 'bitsperpixel' to adjust the
> >>>> colordepth of the framebuffer. All values >1 will result in
> >>>> memory map of the requested color depth. However only the MSB of
> >>>> each pixel will be sent to the device. The framebuffer
> >>>> identifies itself as a grayscale display with the specified
> >>>> depth.
> >>>
> >>> I'm not sure this is the right thing to do.
> >>>
> >>> The bits per pixel for this display is rightfully defined, used
> >>> and reported to the userspace, why would you want to change that?
> >>
> >> You are right of course. The display is 1bpp and it reports to be 1
> >> bpp. The problem is that there is almost no userspace library that
> >> can handle 1 bit framebuffers correctly. So it is nice if the
> >> framebuffer (optionally) can expose itself as 8 bits per pixel
> >> grayscale to the userspace program. As an example this allows to
> >> run DirectFB on the framebuffer, which is not possible out of the
> >> box for 1bpp.
> >>
> >> Also note that if do not set the module parameter at load time
> >> the framebuffer will be 1bpp. So you have to actively set that
> >> module parameter to make the framebuffer pretend to be more than
> >> 1bpp.
> >>
> >> In any case I don't cling to that patch, I just thought it was a
> >> nice feature.
> > 
> > I'd say that the right fix would be to patch DirectFB, instead of
> > faking that in the kernel.
> > 
> > But again, that's probably Tomi's call, not mine.
> 
> Right, I'm not thrilled =). I don't think it's a good idea to lie to
> the userspace (except when fixing regressions).

Ok, since Maxime and you agree that this is not desirable I will drop
that patch in v4.

Thomas

^ permalink raw reply

* Re: [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace.
From: Thomas Niederprüm @ 2015-03-13 19:21 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Maxime Ripard, linux-fbdev, plagnioj, linux-kernel
In-Reply-To: <54FECC2F.8080804@ti.com>

Am Tue, 10 Mar 2015 12:49:19 +0200
schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:

> On 09/02/15 10:52, Maxime Ripard wrote:
> > On Sat, Feb 07, 2015 at 05:42:44PM +0100, Thomas Niederprüm wrote:
> >>>> +static struct device_attribute device_attrs[] = {
> >>>> +	__ATTR(contrast, S_IRUGO|S_IWUSR, show_contrast,
> >>>> store_contrast),
> >>>> +	__ATTR(dim, S_IRUGO|S_IWUSR, show_dim, store_dim),
> >>>> +
> >>>> +};
> >>>> +
> >>>
> >>> I would have thought this was something accessible through the
> >>> framebuffer ioctl.
> >>>
> >>> Apparently it's not, at least for the contrast, so maybe it
> >>> should be added there, instead of doing it for a single driver?
> >>
> >> I think the contrast setting for an OLED display is much like the
> >> backlight setting for LCD panel. Since there is also no ioctl to
> >> set the backlight of an LCD I wonder if the contrast of an OLED
> >> should have one.
> > 
> > It's too much of framebuffer interface debate for me here. Tomi?
> 
> We have backlight and contrast already in backlight-class and
> lcd-class (drivers/video/backlight/backlight.c and
> drivers/video/backlight/lcd.c). Are those something that could be
> used here instead of custom sysfs files?

I just gave the backlight-class a try and it works like a charm. I
will include it in v4 and drop the sysfs handles instead. Thanks for the
hint!

Thomas


^ permalink raw reply

* Re: fbdev: sh_mobile_lcdc: Implement overlays support
From: Geert Uytterhoeven @ 2015-03-13 15:45 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150313100737.GA1984@mwanda>

On Fri, Mar 13, 2015 at 11:07 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> The patch c5deac3c9b22: "fbdev: sh_mobile_lcdc: Implement overlays
> support" from Dec 12, 2011, leads to the following static checker
> warning:
>
>         drivers/video/fbdev/sh_mobile_lcdcfb.c:1471 overlay_rop3_store()
>         warn: bool comparison is always 'false'
>
> drivers/video/fbdev/sh_mobile_lcdcfb.c
>   1455  static ssize_t
>   1456  overlay_rop3_store(struct device *dev, struct device_attribute *attr,
>   1457                      const char *buf, size_t count)
>   1458  {
>   1459          struct fb_info *info = dev_get_drvdata(dev);
>   1460          struct sh_mobile_lcdc_overlay *ovl = info->par;
>   1461          unsigned int rop3;
>   1462          char *endp;
>   1463
>   1464          rop3 = !!simple_strtoul(buf, &endp, 10);
>                        ^^
> rop3 is true/false.
>
>   1465          if (isspace(*endp))
>   1466                  endp++;
>   1467
>   1468          if (endp - buf != count)
>   1469                  return -EINVAL;
>   1470
>   1471          if (rop3 > 255)
>                     ^^^^^^^^^^
> This condition is never true.  Should we just delete it?

Documentation/ABI/testing/sysfs-devices-platform-sh_mobile_lcdc_fb:
"Values range from 0 to 255".

So it looks like the "!!" should be dropped instead.

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

* Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation
From: Archit Taneja @ 2015-03-13 11:12 UTC (permalink / raw)
  To: treding, p.zabel, benjamin.gaignard, daniel
  Cc: linux-fbdev, linux-arm-msm, linux-kernel, dri-devel,
	tomi.valkeinen
In-Reply-To: <20150313090651.GZ3800@phenom.ffwll.local>



On 03/13/2015 02:36 PM, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>
>>
>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>
>>>>
>>>> On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>>>>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>>>>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>>>>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>>>>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>>>>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>>>>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>>>>>> index 151a050..38f83a0 100644
>>>>>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>>>>>>>>>>   	help
>>>>>>>>>>   	  FBDEV helpers for KMS drivers.
>>>>>>>>>>
>>>>>>>>>> +config DRM_FBDEV_EMULATION
>>>>>>>>>> +	bool "Enable legacy fbdev support for your modesetting driver"
>>>>>>>>>> +	depends on DRM
>>>>>>>>>> +	select DRM_KMS_HELPER
>>>>>>>>>> +	select DRM_KMS_FB_HELPER
>>>>>>>>>> +	select FB_SYS_FILLRECT
>>>>>>>>>> +	select FB_SYS_COPYAREA
>>>>>>>>>> +	select FB_SYS_IMAGEBLIT
>>>>>>>>>> +	select FB_SYS_FOPS
>>>>>>>>>> +	select FB_CFB_FILLRECT
>>>>>>>>>> +	select FB_CFB_COPYAREA
>>>>>>>>>> +	select FB_CFB_IMAGEBLIT
>>>>>>>>>> +	default y
>>>>>>>>>> +	help
>>>>>>>>>> +	  Choose this option if you have a need for the legacy fbdev
>>>>>>>>>> +	  support. Note that this support also provide the linux console
>>>>>>>>>> +	  support on top of your modesetting driver.
>>>>>>>>>
>>>>>>>>> Maybe clarify that for linux console support you also need
>>>>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>>>>>>>>
>>>>>>>> DRM_KMS_FB_HELPER selects that for us, right?
>>>>>>>
>>>>>>> Hm right I've missed that. Reminds me that you need one more patch at the
>>>>>>> end to remove all the various select DRM_KMS_FB_HELPER from all drm
>>>>>>> drivers. Otherwise this knob here won't work by default if you e.g. select
>>>>>>> radeon. In general we can't mix explicit options with menu entries with a
>>>>>>> select.
>>>>>>
>>>>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having
>>>>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
>>>>>> their respective xyz_fbdev.c files.
>>>>>
>>>>> But with the stubbed out functions that should work, right? Why doesn't
>>>>> it?
>>>>
>>>> There are still calls to functions from fb core like fb_set_suspend and
>>>> register_framebuffer which aren't covered by the drm fb helper functions.
>>>
>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>> fbdev isn't enabled. Is there anything else?
>>
>> There are a handful of fb core functions which are called by drm drivers:
>>
>> fb_alloc_cmap/fb_dealloc_cmap
>>
>> fb_sys_read/fb_sys_write
>>
>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>> remove_conflicting_framebuffers
>>
>> fb_set_suspend
>>
>> fb_deferred_io_init/fb_deferred_io_cleanup
>>
>> framebuffer_alloc/framebuffer_release
>
> Hm yeah that's somewhat annoying indeed. What about the following:
> 1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>
> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>
> #if defined(CONFIG_FB)
> #include <linux/fb.h>
> #else
>
> /* static inline stubs for all the fb stuff used by kms drivers */
> #endif
>
> Imo this makes sense since kms drivers really have a bit a special
> situation with fbdev. They're not full-blown fbdev drivers and can be
> useful fully without fbdev.
>
> What do you think?

This looks good! I'll give it a try.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: related to frame buffer rotation to 180 degree
From: Geert Uytterhoeven @ 2015-03-13 10:12 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <5502A2A9.4080204@deltaembedded.in>

On Fri, Mar 13, 2015 at 9:41 AM, Jaymin Dabhi
<jaymin.dabhi@deltaembedded.in> wrote:
> I using i/MX6Q processor with Linux Kernel 3.0.35 version with 3.5 inch RGB
> type ORTUSTECH LCD display.
>
> I want to rotate my LCD screen to 180 degree from current position.
> I tried out to pass digits by use of echo to fb0 and fbcon in
> /sys/class/graphics/fb0 and fbcon but still nothing happen, I can do mirror
> my display but not rotate.

You do have FRAMEBUFFER_CONSOLE_ROTATION=y in your .config?

Note that rotation is only done for text console output.
Graphical applications have to take care of rotation theirselves.

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

* re: fbdev: sh_mobile_lcdc: Implement overlays support
From: Dan Carpenter @ 2015-03-13 10:07 UTC (permalink / raw)
  To: linux-fbdev

Hello Laurent Pinchart,

The patch c5deac3c9b22: "fbdev: sh_mobile_lcdc: Implement overlays
support" from Dec 12, 2011, leads to the following static checker
warning:

	drivers/video/fbdev/sh_mobile_lcdcfb.c:1471 overlay_rop3_store()
	warn: bool comparison is always 'false'

drivers/video/fbdev/sh_mobile_lcdcfb.c
  1455  static ssize_t
  1456  overlay_rop3_store(struct device *dev, struct device_attribute *attr,
  1457                      const char *buf, size_t count)
  1458  {
  1459          struct fb_info *info = dev_get_drvdata(dev);
  1460          struct sh_mobile_lcdc_overlay *ovl = info->par;
  1461          unsigned int rop3;
  1462          char *endp;
  1463  
  1464          rop3 = !!simple_strtoul(buf, &endp, 10);
                       ^^
rop3 is true/false.

  1465          if (isspace(*endp))
  1466                  endp++;
  1467  
  1468          if (endp - buf != count)
  1469                  return -EINVAL;
  1470  
  1471          if (rop3 > 255)
                    ^^^^^^^^^^
This condition is never true.  Should we just delete it?

  1472                  return -EINVAL;
  1473  
  1474          if (ovl->rop3 != rop3) {
  1475                  ovl->rop3 = rop3;
  1476  
  1477                  if (ovl->mode = LCDC_OVERLAY_ROP3 && ovl->enabled)
  1478                          sh_mobile_lcdc_overlay_setup(ovl);
  1479          }
  1480  
  1481          return count;
  1482  }

regards,
dan carpenter

^ permalink raw reply

* Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation
From: Jani Nikula @ 2015-03-13  9:46 UTC (permalink / raw)
  To: Archit Taneja
  Cc: linux-kernel, linux-arm-msm, daniel, robdclark, airlied, treding,
	p.zabel, benjamin.gaignard, dri-devel, linux-fbdev,
	tomi.valkeinen
In-Reply-To: <20150313090651.GZ3800@phenom.ffwll.local>

On Fri, 13 Mar 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> #if defined(CONFIG_FB)
> #include <linux/fb.h>
> #else

Side note, #if IS_ENABLED(CONFIG_FB)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply

* Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation
From: Daniel Vetter @ 2015-03-13  9:06 UTC (permalink / raw)
  To: Archit Taneja
  Cc: benjamin.gaignard, linux-arm-msm, linux-kernel, linux-fbdev,
	tomi.valkeinen, dri-devel, treding
In-Reply-To: <550282C3.10903@codeaurora.org>

On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
> 
> 
> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
> >On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
> >>
> >>
> >>On 03/10/2015 05:47 PM, Daniel Vetter wrote:
> >>>On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
> >>>>On 03/10/2015 03:35 PM, Daniel Vetter wrote:
> >>>>>On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
> >>>>>>On 03/10/2015 03:17 PM, Daniel Vetter wrote:
> >>>>>>>On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
> >>>>>>>>diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >>>>>>>>index 151a050..38f83a0 100644
> >>>>>>>>--- a/drivers/gpu/drm/Kconfig
> >>>>>>>>+++ b/drivers/gpu/drm/Kconfig
> >>>>>>>>@@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
> >>>>>>>>  	help
> >>>>>>>>  	  FBDEV helpers for KMS drivers.
> >>>>>>>>
> >>>>>>>>+config DRM_FBDEV_EMULATION
> >>>>>>>>+	bool "Enable legacy fbdev support for your modesetting driver"
> >>>>>>>>+	depends on DRM
> >>>>>>>>+	select DRM_KMS_HELPER
> >>>>>>>>+	select DRM_KMS_FB_HELPER
> >>>>>>>>+	select FB_SYS_FILLRECT
> >>>>>>>>+	select FB_SYS_COPYAREA
> >>>>>>>>+	select FB_SYS_IMAGEBLIT
> >>>>>>>>+	select FB_SYS_FOPS
> >>>>>>>>+	select FB_CFB_FILLRECT
> >>>>>>>>+	select FB_CFB_COPYAREA
> >>>>>>>>+	select FB_CFB_IMAGEBLIT
> >>>>>>>>+	default y
> >>>>>>>>+	help
> >>>>>>>>+	  Choose this option if you have a need for the legacy fbdev
> >>>>>>>>+	  support. Note that this support also provide the linux console
> >>>>>>>>+	  support on top of your modesetting driver.
> >>>>>>>
> >>>>>>>Maybe clarify that for linux console support you also need
> >>>>>>>CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
> >>>>>>
> >>>>>>DRM_KMS_FB_HELPER selects that for us, right?
> >>>>>
> >>>>>Hm right I've missed that. Reminds me that you need one more patch at the
> >>>>>end to remove all the various select DRM_KMS_FB_HELPER from all drm
> >>>>>drivers. Otherwise this knob here won't work by default if you e.g. select
> >>>>>radeon. In general we can't mix explicit options with menu entries with a
> >>>>>select.
> >>>>
> >>>>I was trying that out. Removing DRM_KMS_FB_HELPER and having
> >>>>DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
> >>>>their respective xyz_fbdev.c files.
> >>>
> >>>But with the stubbed out functions that should work, right? Why doesn't
> >>>it?
> >>
> >>There are still calls to functions from fb core like fb_set_suspend and
> >>register_framebuffer which aren't covered by the drm fb helper functions.
> >
> >Hm, sounds like we need another patch to stub out fb_set_suspend when
> >fbdev isn't enabled. Is there anything else?
> 
> There are a handful of fb core functions which are called by drm drivers:
> 
> fb_alloc_cmap/fb_dealloc_cmap
> 
> fb_sys_read/fb_sys_write
> 
> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
> remove_conflicting_framebuffers
> 
> fb_set_suspend
> 
> fb_deferred_io_init/fb_deferred_io_cleanup
> 
> framebuffer_alloc/framebuffer_release

Hm yeah that's somewhat annoying indeed. What about the following:
1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h

2. Then we add stubs for these functions in drm_fb_helper.h, like this

#if defined(CONFIG_FB)
#include <linux/fb.h>
#else

/* static inline stubs for all the fb stuff used by kms drivers */
#endif

Imo this makes sense since kms drivers really have a bit a special
situation with fbdev. They're not full-blown fbdev drivers and can be
useful fully without fbdev.

What do you think?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* related to frame buffer rotation to 180 degree
From: Jaymin Dabhi @ 2015-03-13  8:53 UTC (permalink / raw)
  To: linux-fbdev

Hello,

I using i/MX6Q processor with Linux Kernel 3.0.35 version with 3.5 inch 
RGB type ORTUSTECH LCD display.

I want to rotate my LCD screen to 180 degree from current position.
I tried out to pass digits by use of echo to fb0 and fbcon in 
/sys/class/graphics/fb0 and fbcon but still nothing happen, I can do 
mirror my display but not rotate.

I write full details, what I had done for rotation at below link, please 
refer it once for full details

https://community.freescale.com/thread/346492

Help me about this to solve this issue, it will benefit me a lot.

-- 
Thanks and Regards,

JAYMIN DABHI
Embedded Engineer

Delta Embedded Solutions Pvt. Ltd.
BAT House, 720/10 Navi Peth,
Plot-985, off LBS Road,
Pune - 411030.  India.

Tel: +91 (20) 2453 0811
www.deltaembedded.in

This email message is for the sole use of the intended recipient(s) and
may contain confidential and privileged information.Any unauthorized
review, use, disclosure or distribution is prohibited.If you are not
the intended recipient, please contact the sender by reply email and
destroy all copies of the original message.


DISCLAIMER
This email message including any attachments is for the sole use of the intended recipient(s) and may contain confidential and privileged information. 
Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. 

The opinions / views expressed in the mail or its attachments are those of the sender, and do not necessarily reflect the opinions of the sender's organization.

Although we have taken every reasonable care to ensure the mail is free of viruses / computer threats, the responsibility of ensuring security lies with the recipient. The sender, sending organization and ISP cannot accept responsibility or liability for any damage arising from the use of this email or its attachments.



^ permalink raw reply

* Re: [PATCH] staging: sm750fb: braces, indents, spaces fix
From: Dan Carpenter @ 2015-03-13  7:45 UTC (permalink / raw)
  To: Ragavendra BN
  Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel
In-Reply-To: <20150313061127.GA30820@localhost.localdomain>

On Thu, Mar 12, 2015 at 11:11:27PM -0700, Ragavendra BN wrote:
> This patch removes the braces for the single line if statement. It fixes
> the indent positions correctly. It fixes the spaces appropriately making the code give no
> warnings by the checpath.pl script check. Please accept.
                                            ^^^^^^^^^^^^^^
Don't put this in the changelog.

> 
> Signed-off-by: Ragavendra BN <ragavendra.bn@gmail.com>

Is your name *really* BN?

regards,
dan carpenter


^ permalink raw reply

* Re: [RFC 1/6] drm: Add top level Kconfig option for DRM fbdev emulation
From: Archit Taneja @ 2015-03-13  6:37 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, Jani Nikula, daniel
  Cc: linux-fbdev, dri-devel, tomi.valkeinen, benjamin.gaignard,
	treding
In-Reply-To: <20150311151731.GJ3800@phenom.ffwll.local>



On 03/11/2015 08:47 PM, Daniel Vetter wrote:
> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>
>>
>> On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>>>> index 151a050..38f83a0 100644
>>>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>>>>>>>>   	help
>>>>>>>>   	  FBDEV helpers for KMS drivers.
>>>>>>>>
>>>>>>>> +config DRM_FBDEV_EMULATION
>>>>>>>> +	bool "Enable legacy fbdev support for your modesetting driver"
>>>>>>>> +	depends on DRM
>>>>>>>> +	select DRM_KMS_HELPER
>>>>>>>> +	select DRM_KMS_FB_HELPER
>>>>>>>> +	select FB_SYS_FILLRECT
>>>>>>>> +	select FB_SYS_COPYAREA
>>>>>>>> +	select FB_SYS_IMAGEBLIT
>>>>>>>> +	select FB_SYS_FOPS
>>>>>>>> +	select FB_CFB_FILLRECT
>>>>>>>> +	select FB_CFB_COPYAREA
>>>>>>>> +	select FB_CFB_IMAGEBLIT
>>>>>>>> +	default y
>>>>>>>> +	help
>>>>>>>> +	  Choose this option if you have a need for the legacy fbdev
>>>>>>>> +	  support. Note that this support also provide the linux console
>>>>>>>> +	  support on top of your modesetting driver.
>>>>>>>
>>>>>>> Maybe clarify that for linux console support you also need
>>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>>>>>>
>>>>>> DRM_KMS_FB_HELPER selects that for us, right?
>>>>>
>>>>> Hm right I've missed that. Reminds me that you need one more patch at the
>>>>> end to remove all the various select DRM_KMS_FB_HELPER from all drm
>>>>> drivers. Otherwise this knob here won't work by default if you e.g. select
>>>>> radeon. In general we can't mix explicit options with menu entries with a
>>>>> select.
>>>>
>>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having
>>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
>>>> their respective xyz_fbdev.c files.
>>>
>>> But with the stubbed out functions that should work, right? Why doesn't
>>> it?
>>
>> There are still calls to functions from fb core like fb_set_suspend and
>> register_framebuffer which aren't covered by the drm fb helper functions.
>
> Hm, sounds like we need another patch to stub out fb_set_suspend when
> fbdev isn't enabled. Is there anything else?

There are a handful of fb core functions which are called by drm drivers:

fb_alloc_cmap/fb_dealloc_cmap

fb_sys_read/fb_sys_write

register_framebuffer/unregister_framebuffer/unlink_framebuffer/
remove_conflicting_framebuffers

fb_set_suspend

fb_deferred_io_init/fb_deferred_io_cleanup

framebuffer_alloc/framebuffer_release


>
>>>> Are you saying that we should remove DRM_KMS_FB_HELPER for such drivers and
>>>> replace them with 'select DRM_FBDEV_EMULATION'?
>>>>
>>>> Another option would be to provide #ifdef DRM_FBDEV_EMULATION wrap-arounds
>>>> for fb related function calls/declarations in each driver, something that's
>>>> already done for i915 and msm drivers.
>>>
>>> The problem with the patch as-is the massive amounts of selects the FB
>>> helper still has. We need to get rid of them so that when you disable
>>> fbdev emulation you can indeed disable fbcon and the entire fbdev
>>> subsystem. I've thought that remove the hidden symbol DRM_KMS_FB_HELPER
>>> and moving all the selects to DRM_FBDEV_EMULATION should work out? If that
>>> doesn't work we need to look again how to better stub things out I think.
>>
>> That's true. Also, as Jani pointed out, maybe it isn't the best idea to make
>> every kms driver select DRM_FBDEV_EMULATION?
>>
>> Maybe the drivers that are currently built with fbdev emulation by default
>> can add a 'depends on DRM_FBDEV_EMULATION'? Since the config defaults to y,
>> the drivers should run as is. Later, we could take up each driver and build
>> the fb stuff only when DRM_FBDEV_EMULATION is set, like how we do for i915
>> and msm?
>
> Yeah we definitely can't mix select with a user-visible option. I think we
> just need to fix up the remaining functions and create stubs for them if
> needed and then drop all the selects. Well in DRM_FBDEV_EMULATION we
> should still select for fbcon since otherwise tons of bug reports about
> "where is my fbcon with kms?".

I'll give this a try. Although, it would be a better idea to make the 
drivers not call these at all when fbdev emulation isn't asked for. With 
the stubs, if someone does try to use the driver with 
DRM_FBDEV_EMULATION not set, the worst that'll happen would be that the 
driver fails to probe. Which isn't so bad.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox