* [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 02931c7..be91dfc 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -762,6 +762,11 @@ static int ssd1307fb_remove(struct i2c_client *client)
{
struct fb_info *info = i2c_get_clientdata(client);
struct ssd1307fb_par *par = info->par;
+ int ret = 0;
+
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_OFF);
+ if (ret < 0)
+ return ret;
unregister_framebuffer(info);
if (par->device_info->device_id = DEVID_SSD1307) {
--
2.1.1
^ permalink raw reply related
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Maxime Ripard @ 2015-02-07 10:42 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-3-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 14496 bytes --]
Hi,
On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patches unifies the init code for the ssd130X chips and
> adds device tree bindings to describe the hardware configuration
> of the used controller. This gets rid of the magic bit values
> used in the init code so far.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> .../devicetree/bindings/video/ssd1307fb.txt | 11 +
> drivers/video/fbdev/ssd1307fb.c | 243 ++++++++++++++-------
> 2 files changed, 174 insertions(+), 80 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> index 7a12542..1230f68 100644
> --- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> @@ -15,6 +15,17 @@ Required properties:
>
> Optional properties:
> - reset-active-low: Is the reset gpio is active on physical low?
> + - solomon,segment-remap: Invert the order of data column to segment mapping
> + - solomon,offset: Map the display start line to one of COM0 - COM63
> + - solomon,contrast: Set initial contrast of the display
> + - solomon,prechargep1: Set the duration of the precharge period phase1
> + - solomon,prechargep2: Set the duration of the precharge period phase2
> + - solomon,com-alt: Enable/disable alternate COM pin configuration
> + - solomon,com-lrremap: Enable/disable left-right remap of COM pins
> + - solomon,com-invdir: Invert COM scan direction
> + - solomon,vcomh: Set VCOMH regulator voltage
> + - solomon,dclk-div: Set display clock divider
> + - solomon,dclk-frq: Set display clock frequency
I'm sorry, but this is the wrong approach, for at least two reasons:
you broke all existing users of that driver, which is a clear no-go,
and the DT itself should not contain any direct mapping of the
registers.
>
> [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 3d6611f..4f435aa 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
>
> @@ -40,20 +43,33 @@
>
> 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_alt;
> + u32 com_invdir;
> + u32 com_lrremap;
> + 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 offset;
> u32 page_offset;
> + u32 prechargep1;
> + u32 prechargep2;
> struct pwm_device *pwm;
> u32 pwm_period;
> int reset;
> + u32 seg_remap;
> + u32 vcomh;
> u32 width;
> };
>
> @@ -254,127 +270,151 @@ 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);
> - 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;
>
> /* Set multiplex ratio value */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_MULTIPLEX_RATIO);
> - ret = ret & ssd1307fb_write_cmd(par->client, par->height - 1);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->height - 1);
> if (ret < 0)
> return ret;
>
> /* set display offset value */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_DISPLAY_OFFSET);
> - ret = ssd1307fb_write_cmd(par->client, 0x20);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->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);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0xf0);
> if (ret < 0)
> return ret;
>
> - /* Set precharge period in number of ticks from the internal clock */
> + 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);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, precharge);
> if (ret < 0)
> return ret;
>
> /* Set COM pins configuration */
> + compins = 0x02 | (par->com_alt & 0x1) << 4
> + | (par->com_lrremap & 0x1) << 5;
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, compins);
> if (ret < 0)
> return ret;
>
> /* Set VCOMH */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_VCOMH);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x49);
> if (ret < 0)
> return ret;
>
> - /* Turn on the DC-DC Charge Pump */
> - ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x14);
> + ret = ssd1307fb_write_cmd(par->client, par->vcomh);
> 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;
> + };
> +
> /* Switch to horizontal addressing mode */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
> - ret = ret & ssd1307fb_write_cmd(par->client,
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client,
> SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
> if (ret < 0)
> return ret;
>
> + /* Set column range */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COL_RANGE);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> - ret = ret & ssd1307fb_write_cmd(par->client, par->width - 1);
> if (ret < 0)
> return ret;
>
> + ret = ssd1307fb_write_cmd(par->client, 0x0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->width - 1);
> + if (ret < 0)
> + return ret;
> +
> + /* Set page range */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PAGE_RANGE);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> - ret = ret & ssd1307fb_write_cmd(par->client,
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, 0x0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client,
> par->page_offset + (par->height / 8) - 1);
> if (ret < 0)
> return ret;
> @@ -387,18 +427,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,
> },
> {},
> };
> @@ -429,8 +479,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);
> @@ -449,8 +499,40 @@ static int ssd1307fb_probe(struct i2c_client *client,
> par->page_offset = 1;
>
> vmem_size = par->width * par->height / 8;
> + if (of_property_read_u32(node, "solomon,segment-remap", &par->seg_remap))
> + par->seg_remap = 0;
> +
> + if (of_property_read_u32(node, "solomon,offset", &par->offset))
> + par->offset = 0;
> +
> + if (of_property_read_u32(node, "solomon,contrast", &par->contrast))
> + par->contrast = 128;
> +
> + 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 = 2;
> +
> + if (of_property_read_u32(node, "solomon,com-alt", &par->com_alt))
> + par->com_alt = 1;
> +
> + if (of_property_read_u32(node, "solomon,com-lrremap", &par->com_lrremap))
> + par->com_lrremap = 0;
> +
> + if (of_property_read_u32(node, "solomon,com-invdir", &par->com_invdir))
> + par->com_invdir = 0;
> +
> + if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
> + par->vcomh = par->device_info->default_vcomh;
>
> vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> + 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;
> +
> if (!vmem) {
> dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
> ret = -ENOMEM;
> @@ -499,11 +581,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) {
> @@ -516,8 +596,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:
> @@ -531,11 +613,12 @@ 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);
> framebuffer_release(info);
> -
> return 0;
> }
>
> --
> 2.1.1
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Maxime Ripard @ 2015-02-07 11:18 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-5-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]
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.
It looks like there's numerous fbdev drivers using this (especially
since you copy pasted that code, without mentionning it).
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.
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 43 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 01cfb46..945ded9 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -11,6 +11,7 @@
> #include <linux/i2c.h>
> #include <linux/fb.h>
> #include <linux/uaccess.h>
> +#include <linux/vmalloc.h>
> #include <linux/of_device.h>
> #include <linux/of_gpio.h>
> #include <linux/pwm.h>
> @@ -93,6 +94,43 @@ static struct fb_var_screeninfo ssd1307fb_var = {
> .bits_per_pixel = 1,
> };
>
> +static void *rvmalloc(unsigned long size)
> +{
> + void *mem;
> + unsigned long adr;
> +
> + size = PAGE_ALIGN(size);
Isn't vmalloc already taking care of that?
> + mem = vmalloc_32(size);
Why the 32 bits variant?
> + if (!mem)
> + return NULL;
> +
> + memset(mem, 0, size); /* Clear the ram out, no junk to the user */
> + adr = (unsigned long) mem;
> + while (size > 0) {
> + SetPageReserved(vmalloc_to_page((void *)adr));
I'm not really sure it makes sense to mark all pages reserved if we're
not even sure we're going to use mmap.
And why do you need to mark these pages reserved in the first place?
> + adr += PAGE_SIZE;
> + size -= PAGE_SIZE;
> + }
> +
> + return mem;
> +}
> +
> +static void rvfree(void *mem, unsigned long size)
> +{
> + unsigned long adr;
> +
> + if (!mem)
> + return;
> +
> + adr = (unsigned long) mem;
> + while ((long) size > 0) {
> + ClearPageReserved(vmalloc_to_page((void *)adr));
> + adr += PAGE_SIZE;
> + size -= PAGE_SIZE;
> + }
> + vfree(mem);
> +}
> +
> static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
> {
> struct ssd1307fb_array *array;
> @@ -538,13 +576,13 @@ static int ssd1307fb_probe(struct i2c_client *client,
> if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
> par->vcomh = par->device_info->default_vcomh;
>
> - vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> 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 = rvmalloc(vmem_size);
> if (!vmem) {
> dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
> ret = -ENOMEM;
> @@ -570,7 +608,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);
Why are you changing from virtual to physical address here?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Maxime Ripard @ 2015-02-07 11:20 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-6-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
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?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io.
From: Maxime Ripard @ 2015-02-07 11:26 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-7-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]
On Fri, Feb 06, 2015 at 11:28:12PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patch adds the module parameter "delaydivider" to set delay for the
> deferred io. Effectively this is setting the refresh rate for mmap access
> to the framebuffer. The delay for the deferred io is HZ/delaydivider.
So this is actually a refresh rate?
Maybe you could expose it as such, and pass a frequency in Hz as an
argument.
Exposing the divider directly has some issues, since the bootloader
that set the parameter won't know the HZ value, you'll end up with
different rates for different configurations, without any way to do
something about it.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 1d81877..b38315d 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -44,10 +44,14 @@
> #define SSD1307FB_SET_VCOMH 0xdb
>
> #define BITSPERPIXEL 1
> +#define DELAYDIVIDER 20
>
> static u_int bitsperpixel = BITSPERPIXEL;
> module_param(bitsperpixel, uint, 0);
>
> +static u_int delaydivider = DELAYDIVIDER;
> +module_param(delaydivider, uint, 0);
> +
You're breaking the existing behaviour.
> struct ssd1307fb_par;
>
> struct ssd1307fb_deviceinfo {
> @@ -312,7 +316,7 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
> }
>
> static struct fb_deferred_io ssd1307fb_defio = {
> - .delay = HZ,
> + .delay = HZ/DELAYDIVIDER,
> .deferred_io = ssd1307fb_deferred_io,
> };
>
> @@ -601,6 +605,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
> info->fix = ssd1307fb_fix;
> info->fix.line_length = par->width * bitsperpixel / 8;
> info->fbdefio = &ssd1307fb_defio;
> + info->fbdefio->delay = HZ/delaydivider;
That won't work with multiple instances of the same driver
unfortunately.
>
> info->var = ssd1307fb_var;
> info->var.bits_per_pixel = bitsperpixel;
> --
> 2.1.1
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace.
From: Maxime Ripard @ 2015-02-07 11:43 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-8-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 5142 bytes --]
On Fri, Feb 06, 2015 at 11:28:13PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patch adds sysfs handles to enable userspace control over the display
> contrast as well as the dim mode. The handles are available as "contrast"
> and "dim" in the framebuffers sysfs domain.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 88 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 87 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index b38315d..02931c7 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -33,6 +33,7 @@
> #define SSD1307FB_CONTRAST 0x81
> #define SSD1307FB_CHARGE_PUMP 0x8d
> #define SSD1307FB_SEG_REMAP_ON 0xa1
> +#define SSD1307FB_DISPLAY_DIM 0xac
> #define SSD1307FB_DISPLAY_OFF 0xae
> #define SSD1307FB_SET_MULTIPLEX_RATIO 0xa8
> #define SSD1307FB_DISPLAY_ON 0xaf
> @@ -43,6 +44,9 @@
> #define SSD1307FB_SET_COM_PINS_CONFIG 0xda
> #define SSD1307FB_SET_VCOMH 0xdb
>
> +#define MIN_CONTRAST 0
> +#define MAX_CONTRAST 255
> +
> #define BITSPERPIXEL 1
> #define DELAYDIVIDER 20
>
> @@ -69,6 +73,7 @@ struct ssd1307fb_par {
> u32 dclk_div;
> u32 dclk_frq;
> struct ssd1307fb_deviceinfo *device_info;
> + u32 dim;
> struct i2c_client *client;
> u32 height;
> struct fb_info *info;
> @@ -515,6 +520,79 @@ static const struct of_device_id ssd1307fb_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
>
> +static ssize_t show_contrast(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", par->contrast);
> +}
> +
> +static ssize_t store_contrast(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> + unsigned long contrastval;
> + int ret;
> +
> + ret = kstrtoul(buf, 0, &contrastval);
> + if (ret < 0)
> + return ret;
> +
> + par->contrast = max(min(contrastval,
> + (ulong)MAX_CONTRAST), (ulong)MIN_CONTRAST);
> +
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> + ret = ret & ssd1307fb_write_cmd(par->client, par->contrast);
> + if (ret < 0)
> + return ret;
> +
> + return count;
> +}
> +
> +
> +static ssize_t show_dim(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", par->dim);
> +}
> +
> +static ssize_t store_dim(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> + unsigned long dimval;
> + int ret;
> +
> + ret = kstrtoul(buf, 0, &dimval);
> + if (ret < 0)
> + return ret;
> +
> + par->dim = max(min(dimval, (ulong)1), (ulong)0);
> + if (par->dim)
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_DIM);
> + else
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
> + if (ret < 0)
> + return ret;
> +
> + return count;
> +}
> +
> +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?
(oh, and btw, every sysfs file should be documented in
Documentation/ABI)
> static int ssd1307fb_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -523,7 +601,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
> u32 vmem_size;
> struct ssd1307fb_par *par;
> u8 *vmem;
> - int ret;
> + int ret, i;
>
> if (!node) {
> dev_err(&client->dev, "No device tree data found!\n");
> @@ -650,6 +728,14 @@ static int ssd1307fb_probe(struct i2c_client *client,
> goto reset_oled_error;
>
> ret = register_framebuffer(info);
> +
> + for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
> + ret = device_create_file(info->dev, &device_attrs[i]);
> +
> + if (ret) {
> + dev_err(&client->dev, "Couldn't register sysfs nodes\n");
> + }
> +
sysfs_create_groups does pretty much that already.
And don't forget to remove these files in the .remove()
> if (ret) {
> dev_err(&client->dev, "Couldn't register the framebuffer\n");
> goto panel_init_error;
> --
> 2.1.1
>
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload.
From: Maxime Ripard @ 2015-02-07 11:45 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-9-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
On Fri, Feb 06, 2015 at 11:28:14PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
A commit log is always nice :)
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 02931c7..be91dfc 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -762,6 +762,11 @@ static int ssd1307fb_remove(struct i2c_client *client)
> {
> struct fb_info *info = i2c_get_clientdata(client);
> struct ssd1307fb_par *par = info->par;
> + int ret = 0;
> +
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_OFF);
> + if (ret < 0)
> + return ret;
I don't think we really care about the return value here.
It might be even worse actually, since you'll end up in a intermediate
state, where you won't have freed everything, but your remove method
has been called still.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Thomas Niederprüm @ 2015-02-07 14:59 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207104225.GM2079@lukather>
Am Sat, 7 Feb 2015 11:42:25 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> Hi,
>
> On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
> wrote:
> > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >
> > This patches unifies the init code for the ssd130X chips and
> > adds device tree bindings to describe the hardware configuration
> > of the used controller. This gets rid of the magic bit values
> > used in the init code so far.
> >
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> > .../devicetree/bindings/video/ssd1307fb.txt | 11 +
> > drivers/video/fbdev/ssd1307fb.c | 243
> > ++++++++++++++------- 2 files changed, 174 insertions(+), 80
> > deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > 7a12542..1230f68 100644 ---
> > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> > +15,17 @@ Required properties:
> > Optional properties:
> > - reset-active-low: Is the reset gpio is active on physical low?
> > + - solomon,segment-remap: Invert the order of data column to
> > segment mapping
> > + - solomon,offset: Map the display start line to one of COM0 -
> > COM63
> > + - solomon,contrast: Set initial contrast of the display
> > + - solomon,prechargep1: Set the duration of the precharge period
> > phase1
> > + - solomon,prechargep2: Set the duration of the precharge period
> > phase2
> > + - solomon,com-alt: Enable/disable alternate COM pin configuration
> > + - solomon,com-lrremap: Enable/disable left-right remap of COM
> > pins
> > + - solomon,com-invdir: Invert COM scan direction
> > + - solomon,vcomh: Set VCOMH regulator voltage
> > + - solomon,dclk-div: Set display clock divider
> > + - solomon,dclk-frq: Set display clock frequency
>
> I'm sorry, but this is the wrong approach, for at least two reasons:
> you broke all existing users of that driver, which is a clear no-go,
Unfortunately this is true. The problem is that the SSD130X controllers
allow for a very versatile wiring of the display to the controller.
It's over to the manufacturer of the OLED module (disp+controller) to
decide how it's actually wired and during device initialization the
driver has to take care to configure the SSD130X controller according
to that wiring. If the driver fails to do so you will end up having
your display showing garbage. Unfortunately the current sate of the
initialization code of the ssd1307fb driver is not very flexible in that
respect. Taking a look at the initialization code for the ssd1306 shows
that it was written with one very special display module in mind. Most
of the magic bit values set there are non-default values according to
the datasheet. The result is that the driver works with that one
particular display module but many other (differently wired) display
modules using a ssd1306 controller won't work without changing the
hardcoded magic bit values.
My idea here was to set all configuration to the default values (as
given in the datasheet) unless it is overwritten by DT. Of course,
without a change in DT, this breaks the driver for all existing users.
The only alternative would be to set the current values as default.
Somehow this feels wrong to me as these values look arbitrary when you
don't know what exact display module they were set for. But if you
insist, I will change the default values.
> and the DT itself should not contain any direct mapping of the
> registers.
>
I think I don't get what you mean here. Is it because I do no sanity
checks of the numbers set in DT? I was just looking for a way to hand
over the information about the wiring of display to the driver. How
would you propose to solve this?
>
> >
> > [0]: Documentation/devicetree/bindings/pwm/pwm.txt
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 3d6611f..4f435aa 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
> >
> > @@ -40,20 +43,33 @@
> >
> > 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_alt;
> > + u32 com_invdir;
> > + u32 com_lrremap;
> > + 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 offset;
> > u32 page_offset;
> > + u32 prechargep1;
> > + u32 prechargep2;
> > struct pwm_device *pwm;
> > u32 pwm_period;
> > int reset;
> > + u32 seg_remap;
> > + u32 vcomh;
> > u32 width;
> > };
> >
> > @@ -254,127 +270,151 @@ 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);
> > - 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;
> >
> > /* Set multiplex ratio value */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_MULTIPLEX_RATIO);
> > - ret = ret & ssd1307fb_write_cmd(par->client, par->height -
> > 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, par->height - 1);
> > if (ret < 0)
> > return ret;
> >
> > /* set display offset value */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_DISPLAY_OFFSET);
> > - ret = ssd1307fb_write_cmd(par->client, 0x20);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, par->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);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0xf0);
> > if (ret < 0)
> > return ret;
> >
> > - /* Set precharge period in number of ticks from the
> > internal clock */
> > + 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);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, precharge);
> > if (ret < 0)
> > return ret;
> >
> > /* Set COM pins configuration */
> > + compins = 0x02 | (par->com_alt & 0x1) << 4
> > + | (par->com_lrremap & 0x1) << 5;
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_COM_PINS_CONFIG);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, compins);
> > if (ret < 0)
> > return ret;
> >
> > /* Set VCOMH */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_VCOMH);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x49);
> > if (ret < 0)
> > return ret;
> >
> > - /* Turn on the DC-DC Charge Pump */
> > - ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_CHARGE_PUMP);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x14);
> > + ret = ssd1307fb_write_cmd(par->client, par->vcomh);
> > 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;
> > + };
> > +
> > /* Switch to horizontal addressing mode */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_ADDRESS_MODE);
> > - ret = ret & ssd1307fb_write_cmd(par->client,
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
> > if (ret < 0)
> > return ret;
> >
> > + /* Set column range */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_COL_RANGE);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> > - ret = ret & ssd1307fb_write_cmd(par->client, par->width -
> > 1); if (ret < 0)
> > return ret;
> >
> > + ret = ssd1307fb_write_cmd(par->client, 0x0);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, par->width - 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Set page range */
> > ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_PAGE_RANGE);
> > - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> > - ret = ret & ssd1307fb_write_cmd(par->client,
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client, 0x0);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ssd1307fb_write_cmd(par->client,
> > par->page_offset +
> > (par->height / 8) - 1); if (ret < 0)
> > return ret;
> > @@ -387,18 +427,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,
> > },
> > {},
> > };
> > @@ -429,8 +479,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);
> > @@ -449,8 +499,40 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, par->page_offset = 1;
> >
> > vmem_size = par->width * par->height / 8;
> > + if (of_property_read_u32(node, "solomon,segment-remap",
> > &par->seg_remap))
> > + par->seg_remap = 0;
> > +
> > + if (of_property_read_u32(node, "solomon,offset",
> > &par->offset))
> > + par->offset = 0;
> > +
> > + if (of_property_read_u32(node, "solomon,contrast",
> > &par->contrast))
> > + par->contrast = 128;
> > +
> > + 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 = 2;
> > +
> > + if (of_property_read_u32(node, "solomon,com-alt",
> > &par->com_alt))
> > + par->com_alt = 1;
> > +
> > + if (of_property_read_u32(node, "solomon,com-lrremap",
> > &par->com_lrremap))
> > + par->com_lrremap = 0;
> > +
> > + if (of_property_read_u32(node, "solomon,com-invdir",
> > &par->com_invdir))
> > + par->com_invdir = 0;
> > +
> > + if (of_property_read_u32(node, "solomon,vcomh",
> > &par->vcomh))
> > + par->vcomh = par->device_info->default_vcomh;
> >
> > vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> > + 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; +
> > if (!vmem) {
> > dev_err(&client->dev, "Couldn't allocate graphical
> > memory.\n"); ret = -ENOMEM;
> > @@ -499,11 +581,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) {
> > @@ -516,8 +596,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:
> > @@ -531,11 +613,12 @@ 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);
> > framebuffer_release(info);
> > -
> > return 0;
> > }
> >
> > --
> > 2.1.1
> >
>
^ permalink raw reply
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Noralf Trønnes @ 2015-02-07 15:19 UTC (permalink / raw)
To: Thomas Niederprüm, Maxime Ripard
Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207155933.4f1d0998@maestro.intranet>
Hi,
Den 07.02.2015 15:59, skrev Thomas Niederprüm:
> Am Sat, 7 Feb 2015 11:42:25 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
>> Hi,
>>
>> On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
>> wrote:
>>> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>>>
>>> This patches unifies the init code for the ssd130X chips and
>>> adds device tree bindings to describe the hardware configuration
>>> of the used controller. This gets rid of the magic bit values
>>> used in the init code so far.
>>>
>>> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
>>> ---
>>> .../devicetree/bindings/video/ssd1307fb.txt | 11 +
>>> drivers/video/fbdev/ssd1307fb.c | 243
>>> ++++++++++++++------- 2 files changed, 174 insertions(+), 80
>>> deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
>>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
>>> 7a12542..1230f68 100644 ---
>>> a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
>>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
>>> +15,17 @@ Required properties:
>>> Optional properties:
>>> - reset-active-low: Is the reset gpio is active on physical low?
>>> + - solomon,segment-remap: Invert the order of data column to
>>> segment mapping
>>> + - solomon,offset: Map the display start line to one of COM0 -
>>> COM63
>>> + - solomon,contrast: Set initial contrast of the display
>>> + - solomon,prechargep1: Set the duration of the precharge period
>>> phase1
>>> + - solomon,prechargep2: Set the duration of the precharge period
>>> phase2
>>> + - solomon,com-alt: Enable/disable alternate COM pin configuration
>>> + - solomon,com-lrremap: Enable/disable left-right remap of COM
>>> pins
>>> + - solomon,com-invdir: Invert COM scan direction
>>> + - solomon,vcomh: Set VCOMH regulator voltage
>>> + - solomon,dclk-div: Set display clock divider
>>> + - solomon,dclk-frq: Set display clock frequency
>> I'm sorry, but this is the wrong approach, for at least two reasons:
>> you broke all existing users of that driver, which is a clear no-go,
> Unfortunately this is true. The problem is that the SSD130X controllers
> allow for a very versatile wiring of the display to the controller.
> It's over to the manufacturer of the OLED module (disp+controller) to
> decide how it's actually wired and during device initialization the
> driver has to take care to configure the SSD130X controller according
> to that wiring. If the driver fails to do so you will end up having
> your display showing garbage. Unfortunately the current sate of the
> initialization code of the ssd1307fb driver is not very flexible in that
> respect. Taking a look at the initialization code for the ssd1306 shows
> that it was written with one very special display module in mind. Most
> of the magic bit values set there are non-default values according to
> the datasheet. The result is that the driver works with that one
> particular display module but many other (differently wired) display
> modules using a ssd1306 controller won't work without changing the
> hardcoded magic bit values.
>
> My idea here was to set all configuration to the default values (as
> given in the datasheet) unless it is overwritten by DT. Of course,
> without a change in DT, this breaks the driver for all existing users.
> The only alternative would be to set the current values as default.
> Somehow this feels wrong to me as these values look arbitrary when you
> don't know what exact display module they were set for. But if you
> insist, I will change the default values.
>
>> and the DT itself should not contain any direct mapping of the
>> registers.
>>
> I think I don't get what you mean here. Is it because I do no sanity
> checks of the numbers set in DT? I was just looking for a way to hand
> over the information about the wiring of display to the driver. How
> would you propose to solve this?
I have the exact same challenge with the staging/fbtft drivers.
I have asked about this on the DT list a couple of days ago, but no
answer yet:
Can I do register initialization from Device Tree?
http://www.spinics.net/lists/devicetree/msg68174.html
Regards,
Noralf Trønnes
^ permalink raw reply
* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-02-07 15:35 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207111821.GN2079@lukather>
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 ;)
>
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> > drivers/video/fbdev/ssd1307fb.c | 43
> > +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41
> > insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 01cfb46..945ded9 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -11,6 +11,7 @@
> > #include <linux/i2c.h>
> > #include <linux/fb.h>
> > #include <linux/uaccess.h>
> > +#include <linux/vmalloc.h>
> > #include <linux/of_device.h>
> > #include <linux/of_gpio.h>
> > #include <linux/pwm.h>
> > @@ -93,6 +94,43 @@ static struct fb_var_screeninfo ssd1307fb_var = {
> > .bits_per_pixel = 1,
> > };
> >
> > +static void *rvmalloc(unsigned long size)
> > +{
> > + void *mem;
> > + unsigned long adr;
> > +
> > + size = PAGE_ALIGN(size);
>
> Isn't vmalloc already taking care of that?
>
> > + mem = vmalloc_32(size);
>
> Why the 32 bits variant?
>
> > + if (!mem)
> > + return NULL;
> > +
> > + memset(mem, 0, size); /* Clear the ram out, no junk to the
> > user */
> > + adr = (unsigned long) mem;
> > + while (size > 0) {
> > + SetPageReserved(vmalloc_to_page((void *)adr));
>
> I'm not really sure it makes sense to mark all pages reserved if we're
> not even sure we're going to use mmap.
>
> And why do you need to mark these pages reserved in the first place?
>
> > + adr += PAGE_SIZE;
> > + size -= PAGE_SIZE;
> > + }
> > +
> > + return mem;
> > +}
> > +
> > +static void rvfree(void *mem, unsigned long size)
> > +{
> > + unsigned long adr;
> > +
> > + if (!mem)
> > + return;
> > +
> > + adr = (unsigned long) mem;
> > + while ((long) size > 0) {
> > + ClearPageReserved(vmalloc_to_page((void *)adr));
> > + adr += PAGE_SIZE;
> > + size -= PAGE_SIZE;
> > + }
> > + vfree(mem);
> > +}
> > +
> > static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8
> > type) {
> > struct ssd1307fb_array *array;
> > @@ -538,13 +576,13 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, if (of_property_read_u32(node, "solomon,vcomh",
> > &par->vcomh)) par->vcomh = par->device_info->default_vcomh;
> >
> > - vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> > 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 = rvmalloc(vmem_size);
> > if (!vmem) {
> > dev_err(&client->dev, "Couldn't allocate graphical
> > memory.\n"); ret = -ENOMEM;
> > @@ -570,7 +608,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);
>
> Why are you changing from virtual to physical address here?
Oh, good catch! This is still a residual of my attempts to get this
working with kmalloc'ed memory. In the current state the driver is
presenting a completely wrong memory address upon mmap. As reported in
[0] info->fix.smem_start has to hold the physical address of the video
memory if it was allocated using kmalloc. Correcting this let me run
into the problem that the kmalloc'ed memory was not page aligned but,
the memory address handed to userspace mmap was aligned to the next
full page, resulting in an inaccessable display region. At that point I
just copied the vmalloc approach from the vfb driver.
[0] http://stackoverflow.com/questions/22285151/kernel-panic-using-deferred-io-on-kmalloced-buffer
>
> Maxime
>
^ permalink raw reply
* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Thomas Niederprüm @ 2015-02-07 16:05 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207112043.GO2079@lukather>
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.
> Maxime
>
^ permalink raw reply
* Re: [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io.
From: Thomas Niederprüm @ 2015-02-07 16:12 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207112627.GP2079@lukather>
Am Sat, 7 Feb 2015 12:26:27 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> On Fri, Feb 06, 2015 at 11:28:12PM +0100, niederp@physik.uni-kl.de
> wrote:
> > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >
> > This patch adds the module parameter "delaydivider" to set delay
> > for the deferred io. Effectively this is setting the refresh rate
> > for mmap access to the framebuffer. The delay for the deferred io
> > is HZ/delaydivider.
>
> So this is actually a refresh rate?
>
> Maybe you could expose it as such, and pass a frequency in Hz as an
> argument.
Good idea! I'll try to do it that way.
>
> Exposing the divider directly has some issues, since the bootloader
> that set the parameter won't know the HZ value, you'll end up with
> different rates for different configurations, without any way to do
> something about it.
>
> >
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> > drivers/video/fbdev/ssd1307fb.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 1d81877..b38315d 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -44,10 +44,14 @@
> > #define SSD1307FB_SET_VCOMH 0xdb
> >
> > #define BITSPERPIXEL 1
> > +#define DELAYDIVIDER 20
> >
> > static u_int bitsperpixel = BITSPERPIXEL;
> > module_param(bitsperpixel, uint, 0);
> >
> > +static u_int delaydivider = DELAYDIVIDER;
> > +module_param(delaydivider, uint, 0);
> > +
>
> You're breaking the existing behaviour.
True! I'll try to keep the existing behaviour when rewriting this to
use the refresh rate.
>
> > struct ssd1307fb_par;
> >
> > struct ssd1307fb_deviceinfo {
> > @@ -312,7 +316,7 @@ static void ssd1307fb_deferred_io(struct
> > fb_info *info, }
> >
> > static struct fb_deferred_io ssd1307fb_defio = {
> > - .delay = HZ,
> > + .delay = HZ/DELAYDIVIDER,
> > .deferred_io = ssd1307fb_deferred_io,
> > };
> >
> > @@ -601,6 +605,7 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, info->fix = ssd1307fb_fix;
> > info->fix.line_length = par->width * bitsperpixel / 8;
> > info->fbdefio = &ssd1307fb_defio;
> > + info->fbdefio->delay = HZ/delaydivider;
>
> That won't work with multiple instances of the same driver
> unfortunately.
Could you please elaborate why? I'm not seeing it...
>
> >
> > info->var = ssd1307fb_var;
> > info->var.bits_per_pixel = bitsperpixel;
> > --
> > 2.1.1
> >
>
^ 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-02-07 16:42 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207114329.GQ2079@lukather>
Am Sat, 7 Feb 2015 12:43:29 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> On Fri, Feb 06, 2015 at 11:28:13PM +0100, niederp@physik.uni-kl.de
> wrote:
> > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >
> > This patch adds sysfs handles to enable userspace control over the
> > display contrast as well as the dim mode. The handles are available
> > as "contrast" and "dim" in the framebuffers sysfs domain.
> >
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> > drivers/video/fbdev/ssd1307fb.c | 88
> > ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87
> > insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index b38315d..02931c7 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -33,6 +33,7 @@
> > #define SSD1307FB_CONTRAST 0x81
> > #define SSD1307FB_CHARGE_PUMP 0x8d
> > #define SSD1307FB_SEG_REMAP_ON 0xa1
> > +#define SSD1307FB_DISPLAY_DIM 0xac
> > #define SSD1307FB_DISPLAY_OFF 0xae
> > #define SSD1307FB_SET_MULTIPLEX_RATIO 0xa8
> > #define SSD1307FB_DISPLAY_ON 0xaf
> > @@ -43,6 +44,9 @@
> > #define SSD1307FB_SET_COM_PINS_CONFIG 0xda
> > #define SSD1307FB_SET_VCOMH 0xdb
> >
> > +#define MIN_CONTRAST 0
> > +#define MAX_CONTRAST 255
> > +
> > #define BITSPERPIXEL 1
> > #define DELAYDIVIDER 20
> >
> > @@ -69,6 +73,7 @@ struct ssd1307fb_par {
> > u32 dclk_div;
> > u32 dclk_frq;
> > struct ssd1307fb_deviceinfo *device_info;
> > + u32 dim;
> > struct i2c_client *client;
> > u32 height;
> > struct fb_info *info;
> > @@ -515,6 +520,79 @@ static const struct of_device_id
> > ssd1307fb_of_match[] = { };
> > MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
> >
> > +static ssize_t show_contrast(struct device *device,
> > + struct device_attribute *attr, char
> > *buf) +{
> > + struct fb_info *info = dev_get_drvdata(device);
> > + struct ssd1307fb_par *par = info->par;
> > +
> > + return snprintf(buf, PAGE_SIZE, "%d\n", par->contrast);
> > +}
> > +
> > +static ssize_t store_contrast(struct device *device,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct fb_info *info = dev_get_drvdata(device);
> > + struct ssd1307fb_par *par = info->par;
> > + unsigned long contrastval;
> > + int ret;
> > +
> > + ret = kstrtoul(buf, 0, &contrastval);
> > + if (ret < 0)
> > + return ret;
> > +
> > + par->contrast = max(min(contrastval,
> > + (ulong)MAX_CONTRAST), (ulong)MIN_CONTRAST);
> > +
> > + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> > + ret = ret & ssd1307fb_write_cmd(par->client,
> > par->contrast);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return count;
> > +}
> > +
> > +
> > +static ssize_t show_dim(struct device *device,
> > + struct device_attribute *attr, char
> > *buf) +{
> > + struct fb_info *info = dev_get_drvdata(device);
> > + struct ssd1307fb_par *par = info->par;
> > +
> > + return snprintf(buf, PAGE_SIZE, "%d\n", par->dim);
> > +}
> > +
> > +static ssize_t store_dim(struct device *device,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct fb_info *info = dev_get_drvdata(device);
> > + struct ssd1307fb_par *par = info->par;
> > + unsigned long dimval;
> > + int ret;
> > +
> > + ret = kstrtoul(buf, 0, &dimval);
> > + if (ret < 0)
> > + return ret;
> > +
> > + par->dim = max(min(dimval, (ulong)1), (ulong)0);
> > + if (par->dim)
> > + ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_DIM);
> > + else
> > + ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_ON);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return count;
> > +}
> > +
> > +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.
>
> (oh, and btw, every sysfs file should be documented in
> Documentation/ABI)
>
> > static int ssd1307fb_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > @@ -523,7 +601,7 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, u32 vmem_size;
> > struct ssd1307fb_par *par;
> > u8 *vmem;
> > - int ret;
> > + int ret, i;
> >
> > if (!node) {
> > dev_err(&client->dev, "No device tree data
> > found!\n"); @@ -650,6 +728,14 @@ static int ssd1307fb_probe(struct
> > i2c_client *client, goto reset_oled_error;
> >
> > ret = register_framebuffer(info);
> > +
> > + for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
> > + ret = device_create_file(info->dev,
> > &device_attrs[i]); +
> > + if (ret) {
> > + dev_err(&client->dev, "Couldn't register sysfs
> > nodes\n");
> > + }
> > +
>
> sysfs_create_groups does pretty much that already.
I'll have a look at it.
>
> And don't forget to remove these files in the .remove()
Good point! :)
>
> > if (ret) {
> > dev_err(&client->dev, "Couldn't register the
> > framebuffer\n"); goto panel_init_error;
> > --
> > 2.1.1
> >
>
> Maxime
>
^ permalink raw reply
* Re: [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload.
From: Thomas Niederprüm @ 2015-02-07 16:49 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207114534.GR2079@lukather>
Am Sat, 7 Feb 2015 12:45:34 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> On Fri, Feb 06, 2015 at 11:28:14PM +0100, niederp@physik.uni-kl.de
> wrote:
> > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >
>
> A commit log is always nice :)
Will be added.
>
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> > drivers/video/fbdev/ssd1307fb.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 02931c7..be91dfc 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -762,6 +762,11 @@ static int ssd1307fb_remove(struct i2c_client
> > *client) {
> > struct fb_info *info = i2c_get_clientdata(client);
> > struct ssd1307fb_par *par = info->par;
> > + int ret = 0;
> > +
> > + ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_OFF);
> > + if (ret < 0)
> > + return ret;
>
> I don't think we really care about the return value here.
>
> It might be even worse actually, since you'll end up in a intermediate
> state, where you won't have freed everything, but your remove method
> has been called still.
I agree. I will remove the check for the return statement.
>
> Maxime
>
^ permalink raw reply
* [PATCH 1/4] tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
From: Ondrej Zary @ 2015-02-07 20:53 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
When the kernel is compiled with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE), tridentfb
hangs the machine upon load with Blade3D cards unless acceleration is disabled.
This is caused by memcpy() which copies data byte-by-byte (rep movsb) when
compiled with -Os. The card does not like that - it requires 32-bit access.
Use iowrite_32() instead.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/tridentfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7ed9a22..7429713 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -226,7 +226,7 @@ static void blade_image_blit(struct tridentfb_par *par, const char *data,
writemmr(par, DST1, point(x, y));
writemmr(par, DST2, point(x + w - 1, y + h - 1));
- memcpy(par->io_virt + 0x10000, data, 4 * size);
+ iowrite32_rep(par->io_virt + 0x10000, data, size);
}
static void blade_copy_rect(struct tridentfb_par *par,
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 2/4] tridentfb: Fix set_lwidth on TGUI9440 and CYBER9320
From: Ondrej Zary @ 2015-02-07 20:53 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1423342417-26069-1-git-send-email-linux@rainbow-software.org>
According to X.Org driver, chips older than TGUI9660 have only 1 width bit
in AddColReg. Touching the 2nd one causes I2C/DDC to fail on TGUI9440.
Set only 1 bit of width in AddColReg on TGUI9440 and CYBER9320.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/tridentfb.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7429713..01b43e9 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -673,8 +673,14 @@ static int get_nativex(struct tridentfb_par *par)
static inline void set_lwidth(struct tridentfb_par *par, int width)
{
write3X4(par, VGA_CRTC_OFFSET, width & 0xFF);
- write3X4(par, AddColReg,
- (read3X4(par, AddColReg) & 0xCF) | ((width & 0x300) >> 4));
+ /* chips older than TGUI9660 have only 1 width bit in AddColReg */
+ /* touching the other one breaks I2C/DDC */
+ if (par->chip_id = TGUI9440 || par->chip_id = CYBER9320)
+ write3X4(par, AddColReg,
+ (read3X4(par, AddColReg) & 0xEF) | ((width & 0x100) >> 4));
+ else
+ write3X4(par, AddColReg,
+ (read3X4(par, AddColReg) & 0xCF) | ((width & 0x300) >> 4));
}
/* For resolutions smaller than FP resolution stretch */
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 3/4] fb_ddc: Allow I2C adapters without SCL read capability
From: Ondrej Zary @ 2015-02-07 20:53 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1423342417-26069-1-git-send-email-linux@rainbow-software.org>
i2c-algo-bit allows I2C adapters without SCL read capability to work but
fb_ddc_read fails to work on them.
Fix fb_ddc_read to work with I2C adapters not capable of reading SCL.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/core/fb_ddc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_ddc.c b/drivers/video/fbdev/core/fb_ddc.c
index 94322cc..22c694a 100644
--- a/drivers/video/fbdev/core/fb_ddc.c
+++ b/drivers/video/fbdev/core/fb_ddc.c
@@ -69,10 +69,11 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
algo_data->setscl(algo_data->data, 1);
for (j = 0; j < 5; j++) {
msleep(10);
- if (algo_data->getscl(algo_data->data))
+ if (algo_data->getscl &&
+ algo_data->getscl(algo_data->data))
break;
}
- if (j = 5)
+ if (algo_data->getscl && j = 5)
continue;
algo_data->setsda(algo_data->data, 0);
@@ -91,7 +92,8 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
algo_data->setscl(algo_data->data, 1);
for (j = 0; j < 10; j++) {
msleep(10);
- if (algo_data->getscl(algo_data->data))
+ if (algo_data->getscl &&
+ algo_data->getscl(algo_data->data))
break;
}
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 4/4] tridentfb: Add DDC support
From: Ondrej Zary @ 2015-02-07 20:53 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1423342417-26069-1-git-send-email-linux@rainbow-software.org>
Add DDC support for Trident cards.
Tested on TGUI9440, TGUI9680, 3DImage 9750, Blade3D 9880 and Blade XP.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/Kconfig | 9 ++
drivers/video/fbdev/tridentfb.c | 192 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 196 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 4916c97..08a7a04 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1682,6 +1682,15 @@ config FB_TRIDENT
To compile this driver as a module, choose M here: the
module will be called tridentfb.
+config FB_TRIDENT_DDC
+ bool "DDC for Trident support"
+ depends on FB_TRIDENT
+ select FB_DDC
+ select FB_MODE_HELPERS
+ default y
+ help
+ Say Y here if you want DDC support for your Trident graphics card.
+
config FB_ARK
tristate "ARK 2000PV support"
depends on FB && PCI
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 01b43e9..59471a0 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -25,6 +25,9 @@
#include <video/vga.h>
#include <video/trident.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
struct tridentfb_par {
void __iomem *io_virt; /* iospace virtual memory address */
u32 pseudo_pal[16];
@@ -40,6 +43,11 @@ struct tridentfb_par {
(struct tridentfb_par *par, const char*,
u32, u32, u32, u32, u32, u32);
unsigned char eng_oper; /* engine operation... */
+#ifdef CONFIG_FB_TRIDENT_DDC
+ bool ddc_registered;
+ struct i2c_adapter ddc_adapter;
+ struct i2c_algo_bit_data ddc_algo;
+#endif
};
static struct fb_fix_screeninfo tridentfb_fix = {
@@ -53,7 +61,7 @@ static struct fb_fix_screeninfo tridentfb_fix = {
/* defaults which are normally overriden by user values */
/* video mode */
-static char *mode_option = "640x480-8@60";
+static char *mode_option;
static int bpp = 8;
static int noaccel;
@@ -174,6 +182,124 @@ static inline u32 readmmr(struct tridentfb_par *par, u16 r)
return fb_readl(par->io_virt + r);
}
+#ifdef CONFIG_FB_TRIDENT_DDC
+
+#define DDC_SDA_TGUI BIT(0)
+#define DDC_SCL_TGUI BIT(1)
+#define DDC_SCL_DRIVE_TGUI BIT(2)
+#define DDC_SDA_DRIVE_TGUI BIT(3)
+#define DDC_MASK_TGUI (DDC_SCL_DRIVE_TGUI | DDC_SDA_DRIVE_TGUI)
+
+static void tridentfb_ddc_setscl_tgui(void *data, int val)
+{
+ struct tridentfb_par *par = data;
+ u8 reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK_TGUI;
+
+ if (val)
+ reg &= ~DDC_SCL_DRIVE_TGUI; /* disable drive - don't drive hi */
+ else
+ reg |= DDC_SCL_DRIVE_TGUI; /* drive low */
+
+ vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static void tridentfb_ddc_setsda_tgui(void *data, int val)
+{
+ struct tridentfb_par *par = data;
+ u8 reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK_TGUI;
+
+ if (val)
+ reg &= ~DDC_SDA_DRIVE_TGUI; /* disable drive - don't drive hi */
+ else
+ reg |= DDC_SDA_DRIVE_TGUI; /* drive low */
+
+ vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static int tridentfb_ddc_getsda_tgui(void *data)
+{
+ struct tridentfb_par *par = data;
+
+ return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SDA_TGUI);
+}
+
+#define DDC_SDA_IN BIT(0)
+#define DDC_SCL_OUT BIT(1)
+#define DDC_SDA_OUT BIT(3)
+#define DDC_SCL_IN BIT(6)
+#define DDC_MASK (DDC_SCL_OUT | DDC_SDA_OUT)
+
+static void tridentfb_ddc_setscl(void *data, int val)
+{
+ struct tridentfb_par *par = data;
+ unsigned char reg;
+
+ reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK;
+ if (val)
+ reg |= DDC_SCL_OUT;
+ else
+ reg &= ~DDC_SCL_OUT;
+ vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static void tridentfb_ddc_setsda(void *data, int val)
+{
+ struct tridentfb_par *par = data;
+ unsigned char reg;
+
+ reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK;
+ if (!val)
+ reg |= DDC_SDA_OUT;
+ else
+ reg &= ~DDC_SDA_OUT;
+ vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static int tridentfb_ddc_getscl(void *data)
+{
+ struct tridentfb_par *par = data;
+
+ return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SCL_IN);
+}
+
+static int tridentfb_ddc_getsda(void *data)
+{
+ struct tridentfb_par *par = data;
+
+ return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SDA_IN);
+}
+
+static int tridentfb_setup_ddc_bus(struct fb_info *info)
+{
+ struct tridentfb_par *par = info->par;
+
+ strlcpy(par->ddc_adapter.name, info->fix.id,
+ sizeof(par->ddc_adapter.name));
+ par->ddc_adapter.owner = THIS_MODULE;
+ par->ddc_adapter.class = I2C_CLASS_DDC;
+ par->ddc_adapter.algo_data = &par->ddc_algo;
+ par->ddc_adapter.dev.parent = info->device;
+ if (is_oldclock(par->chip_id)) { /* not sure if this check is OK */
+ par->ddc_algo.setsda = tridentfb_ddc_setsda_tgui;
+ par->ddc_algo.setscl = tridentfb_ddc_setscl_tgui;
+ par->ddc_algo.getsda = tridentfb_ddc_getsda_tgui;
+ /* no getscl */
+ } else {
+ par->ddc_algo.setsda = tridentfb_ddc_setsda;
+ par->ddc_algo.setscl = tridentfb_ddc_setscl;
+ par->ddc_algo.getsda = tridentfb_ddc_getsda;
+ par->ddc_algo.getscl = tridentfb_ddc_getscl;
+ }
+ par->ddc_algo.udelay = 10;
+ par->ddc_algo.timeout = 20;
+ par->ddc_algo.data = par;
+
+ i2c_set_adapdata(&par->ddc_adapter, par);
+
+ return i2c_bit_add_bus(&par->ddc_adapter);
+}
+#endif /* CONFIG_FB_TRIDENT_DDC */
+
/*
* Blade specific acceleration.
*/
@@ -1346,6 +1472,7 @@ static int trident_pci_probe(struct pci_dev *dev,
struct tridentfb_par *default_par;
int chip3D;
int chip_id;
+ bool found = false;
err = pci_enable_device(dev);
if (err)
@@ -1499,6 +1626,7 @@ static int trident_pci_probe(struct pci_dev *dev,
info->pixmap.scan_align = 1;
info->pixmap.access_align = 32;
info->pixmap.flags = FB_PIXMAP_SYSTEM;
+ info->var.bits_per_pixel = 8;
if (default_par->image_blit) {
info->flags |= FBINFO_HWACCEL_IMAGEBLIT;
@@ -1511,11 +1639,57 @@ static int trident_pci_probe(struct pci_dev *dev,
info->pixmap.scan_align = 1;
}
- if (!fb_find_mode(&info->var, info,
- mode_option, NULL, 0, NULL, bpp)) {
- err = -EINVAL;
- goto out_unmap2;
+#ifdef CONFIG_FB_TRIDENT_DDC
+ if (tridentfb_setup_ddc_bus(info) = 0) {
+ u8 *edid = fb_ddc_read(&default_par->ddc_adapter);
+
+ default_par->ddc_registered = true;
+ if (edid) {
+ fb_edid_to_monspecs(edid, &info->monspecs);
+ kfree(edid);
+ if (!info->monspecs.modedb)
+ dev_err(info->device, "error getting mode database\n");
+ else {
+ const struct fb_videomode *m;
+
+ fb_videomode_to_modelist(info->monspecs.modedb,
+ info->monspecs.modedb_len,
+ &info->modelist);
+ m = fb_find_best_display(&info->monspecs,
+ &info->modelist);
+ if (m) {
+ fb_videomode_to_var(&info->var, m);
+ /* fill all other info->var's fields */
+ if (tridentfb_check_var(&info->var,
+ info) = 0)
+ found = true;
+ }
+ }
+ }
}
+#endif
+ if (!mode_option && !found)
+ mode_option = "640x480-8@60";
+
+ /* Prepare startup mode */
+ if (mode_option) {
+ err = fb_find_mode(&info->var, info, mode_option,
+ info->monspecs.modedb,
+ info->monspecs.modedb_len,
+ NULL, info->var.bits_per_pixel);
+ if (!err || err = 4) {
+ err = -EINVAL;
+ dev_err(info->device, "mode %s not found\n",
+ mode_option);
+ fb_destroy_modedb(info->monspecs.modedb);
+ info->monspecs.modedb = NULL;
+ goto out_unmap2;
+ }
+ }
+
+ fb_destroy_modedb(info->monspecs.modedb);
+ info->monspecs.modedb = NULL;
+
err = fb_alloc_cmap(&info->cmap, 256, 0);
if (err < 0)
goto out_unmap2;
@@ -1536,6 +1710,10 @@ static int trident_pci_probe(struct pci_dev *dev,
return 0;
out_unmap2:
+#ifdef CONFIG_FB_TRIDENT_DDC
+ if (default_par->ddc_registered)
+ i2c_del_adapter(&default_par->ddc_adapter);
+#endif
kfree(info->pixmap.addr);
if (info->screen_base)
iounmap(info->screen_base);
@@ -1555,6 +1733,10 @@ static void trident_pci_remove(struct pci_dev *dev)
struct tridentfb_par *par = info->par;
unregister_framebuffer(info);
+#ifdef CONFIG_FB_TRIDENT_DDC
+ if (par->ddc_registered)
+ i2c_del_adapter(&par->ddc_adapter);
+#endif
iounmap(par->io_virt);
iounmap(info->screen_base);
release_mem_region(tridentfb_fix.smem_start, tridentfb_fix.smem_len);
--
Ondrej Zary
^ permalink raw reply related
* Re: [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace.
From: Maxime Ripard @ 2015-02-09 8:52 UTC (permalink / raw)
To: tomi.valkeinen, Thomas Niederprüm
Cc: linux-fbdev, plagnioj, linux-kernel
In-Reply-To: <20150207174244.25987977@maestro.intranet>
[-- Attachment #1: Type: text/plain, Size: 954 bytes --]
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?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io.
From: Maxime Ripard @ 2015-02-09 9:03 UTC (permalink / raw)
To: Thomas Niederprüm
Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207171211.773dc996@maestro.intranet>
[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]
On Sat, Feb 07, 2015 at 05:12:11PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 12:26:27 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
> > On Fri, Feb 06, 2015 at 11:28:12PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > >
> > > This patch adds the module parameter "delaydivider" to set delay
> > > for the deferred io. Effectively this is setting the refresh rate
> > > for mmap access to the framebuffer. The delay for the deferred io
> > > is HZ/delaydivider.
> >
> > So this is actually a refresh rate?
> >
> > Maybe you could expose it as such, and pass a frequency in Hz as an
> > argument.
>
> Good idea! I'll try to do it that way.
>
> >
> > Exposing the divider directly has some issues, since the bootloader
> > that set the parameter won't know the HZ value, you'll end up with
> > different rates for different configurations, without any way to do
> > something about it.
> >
> > >
> > > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > ---
> > > drivers/video/fbdev/ssd1307fb.c | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > > b/drivers/video/fbdev/ssd1307fb.c index 1d81877..b38315d 100644
> > > --- a/drivers/video/fbdev/ssd1307fb.c
> > > +++ b/drivers/video/fbdev/ssd1307fb.c
> > > @@ -44,10 +44,14 @@
> > > #define SSD1307FB_SET_VCOMH 0xdb
> > >
> > > #define BITSPERPIXEL 1
> > > +#define DELAYDIVIDER 20
> > >
> > > static u_int bitsperpixel = BITSPERPIXEL;
> > > module_param(bitsperpixel, uint, 0);
> > >
> > > +static u_int delaydivider = DELAYDIVIDER;
> > > +module_param(delaydivider, uint, 0);
> > > +
> >
> > You're breaking the existing behaviour.
>
> True! I'll try to keep the existing behaviour when rewriting this to
> use the refresh rate.
>
> >
> > > struct ssd1307fb_par;
> > >
> > > struct ssd1307fb_deviceinfo {
> > > @@ -312,7 +316,7 @@ static void ssd1307fb_deferred_io(struct
> > > fb_info *info, }
> > >
> > > static struct fb_deferred_io ssd1307fb_defio = {
> > > - .delay = HZ,
> > > + .delay = HZ/DELAYDIVIDER,
> > > .deferred_io = ssd1307fb_deferred_io,
> > > };
> > >
> > > @@ -601,6 +605,7 @@ static int ssd1307fb_probe(struct i2c_client
> > > *client, info->fix = ssd1307fb_fix;
> > > info->fix.line_length = par->width * bitsperpixel / 8;
> > > info->fbdefio = &ssd1307fb_defio;
> > > + info->fbdefio->delay = HZ/delaydivider;
> >
> > That won't work with multiple instances of the same driver
> > unfortunately.
>
> Could you please elaborate why? I'm not seeing it...
On a general basis, because the structure is shared by all the
instances of the driver, so it's usually not such a good idea to mix
the static declaration of the structure and the dynamic one.
From a more fundamental point of view, because this parameter will
most likely be different from one instance to another?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Thomas Niederprüm @ 2015-02-09 10:37 UTC (permalink / raw)
To: Noralf Trønnes
Cc: Maxime Ripard, linux-fbdev, plagnioj, tomi.valkeinen,
linux-kernel
In-Reply-To: <54D62D09.8080404@tronnes.org>
Am Samstag, den 07.02.2015, 16:19 +0100 schrieb Noralf Trønnes:
> Hi,
>
> Den 07.02.2015 15:59, skrev Thomas Niederprüm:
> > Am Sat, 7 Feb 2015 11:42:25 +0100
> > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> >
> >> Hi,
> >>
> >> On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
> >> wrote:
> >>> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >>>
> >>> This patches unifies the init code for the ssd130X chips and
> >>> adds device tree bindings to describe the hardware configuration
> >>> of the used controller. This gets rid of the magic bit values
> >>> used in the init code so far.
> >>>
> >>> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >>> ---
> >>> .../devicetree/bindings/video/ssd1307fb.txt | 11 +
> >>> drivers/video/fbdev/ssd1307fb.c | 243
> >>> ++++++++++++++------- 2 files changed, 174 insertions(+), 80
> >>> deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> >>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> >>> 7a12542..1230f68 100644 ---
> >>> a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> >>> b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> >>> +15,17 @@ Required properties:
> >>> Optional properties:
> >>> - reset-active-low: Is the reset gpio is active on physical low?
> >>> + - solomon,segment-remap: Invert the order of data column to
> >>> segment mapping
> >>> + - solomon,offset: Map the display start line to one of COM0 -
> >>> COM63
> >>> + - solomon,contrast: Set initial contrast of the display
> >>> + - solomon,prechargep1: Set the duration of the precharge period
> >>> phase1
> >>> + - solomon,prechargep2: Set the duration of the precharge period
> >>> phase2
> >>> + - solomon,com-alt: Enable/disable alternate COM pin configuration
> >>> + - solomon,com-lrremap: Enable/disable left-right remap of COM
> >>> pins
> >>> + - solomon,com-invdir: Invert COM scan direction
> >>> + - solomon,vcomh: Set VCOMH regulator voltage
> >>> + - solomon,dclk-div: Set display clock divider
> >>> + - solomon,dclk-frq: Set display clock frequency
> >> I'm sorry, but this is the wrong approach, for at least two reasons:
> >> you broke all existing users of that driver, which is a clear no-go,
> > Unfortunately this is true. The problem is that the SSD130X controllers
> > allow for a very versatile wiring of the display to the controller.
> > It's over to the manufacturer of the OLED module (disp+controller) to
> > decide how it's actually wired and during device initialization the
> > driver has to take care to configure the SSD130X controller according
> > to that wiring. If the driver fails to do so you will end up having
> > your display showing garbage. Unfortunately the current sate of the
> > initialization code of the ssd1307fb driver is not very flexible in that
> > respect. Taking a look at the initialization code for the ssd1306 shows
> > that it was written with one very special display module in mind. Most
> > of the magic bit values set there are non-default values according to
> > the datasheet. The result is that the driver works with that one
> > particular display module but many other (differently wired) display
> > modules using a ssd1306 controller won't work without changing the
> > hardcoded magic bit values.
> >
> > My idea here was to set all configuration to the default values (as
> > given in the datasheet) unless it is overwritten by DT. Of course,
> > without a change in DT, this breaks the driver for all existing users.
> > The only alternative would be to set the current values as default.
> > Somehow this feels wrong to me as these values look arbitrary when you
> > don't know what exact display module they were set for. But if you
> > insist, I will change the default values.
> >
> >> and the DT itself should not contain any direct mapping of the
> >> registers.
> >>
> > I think I don't get what you mean here. Is it because I do no sanity
> > checks of the numbers set in DT? I was just looking for a way to hand
> > over the information about the wiring of display to the driver. How
> > would you propose to solve this?
>
> I have the exact same challenge with the staging/fbtft drivers.
Even though we are facing the same problem here, yours is much harder
than mine, since I have much more knowledge about the controller.
Therefor I have no need to completely expose the registers for
initialization. I just define all configurable options that the
controller has in DT and let the driver take care to write the
corresponding register values during initialization. Of course this
needs the exact knowledge of the configuration options of the controller
as well as register addresses of these options in the driver. So I have
the fear that this approach does not scale for a driver handling
different controllers.
> I have asked about this on the DT list a couple of days ago, but no
> answer yet:
>
> Can I do register initialization from Device Tree?
> http://www.spinics.net/lists/devicetree/msg68174.html
>
>
> Regards,
> Noralf Trønnes
>
--
Thomas Niederprüm
TU Kaiserslautern
FB Physik (AG Ott)
Erwin-Schrödinger-Str. 46/431
67663 Kaiserslautern
Tel.: 0631 205 2387
Fax: 0631 205 3906
^ permalink raw reply
* [PATCH] video: fbdev: make of_device_id array const
From: Sanjeev Sharma @ 2015-02-09 11:14 UTC (permalink / raw)
To: kernel, plagnioj
Cc: tomi.valkeinen, linux-fbdev, linux-arm-kernel, linux-kernel,
Sanjeev Sharma, Sanjeev Sharma
Make of_device_id array const.
Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
---
drivers/video/fbdev/imxfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
index 3b6a3c8..84d1d29 100644
--- a/drivers/video/fbdev/imxfb.c
+++ b/drivers/video/fbdev/imxfb.c
@@ -183,7 +183,7 @@ static struct platform_device_id imxfb_devtype[] = {
};
MODULE_DEVICE_TABLE(platform, imxfb_devtype);
-static struct of_device_id imxfb_of_dev_id[] = {
+static const struct of_device_id imxfb_of_dev_id[] = {
{
.compatible = "fsl,imx1-fb",
.data = &imxfb_devtype[IMX1_FB],
--
1.7.11.7
^ permalink raw reply related
* fbdev: fb_defio not buildable as module
From: harald @ 2015-02-09 11:38 UTC (permalink / raw)
To: linux-fbdev
Hi *,
in drivers/video/fbdev/Kconfig we have
| menuconfig FB
| tristate "Support for frame buffer devices"
| select FB_CMDLINE
| ---help---
and
| config FB_DEFERRED_IO
| bool
| depends on FB
but in drivers/video/fbdev/core/Makefile we have the line
| obj-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
AFAICS the combination of this leads to fb_defio always
linked into the kernel image, even if the framebuffer core
is only built as a module. This doesn't make much sense and
I couldn't find any reason why this would be necessary.
The above code is there in one way or the other since the
initial version of fb_defio.c - I wonder what was the
intended behaviour or actually what is considered the right
way today?
TIA,
Harald
^ permalink raw reply
* Re: [PATCH] video: fbdev: make of_device_id array const
From: Uwe Kleine-König @ 2015-02-09 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1423479747-17899-1-git-send-email-sanjeev_sharma@mentor.com>
On Mon, Feb 09, 2015 at 04:32:27PM +0530, Sanjeev Sharma wrote:
> Make of_device_id array const.
>
> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] Trivial patch in efifb.c to solve common indent issue and indent error
From: Parmeshwr Prasad @ 2015-02-09 12:55 UTC (permalink / raw)
To: pjones-H+wXaHxf7aLQT0dZR+AlfA, plagnioj-sclMFOaUSTBWk0Htik3J/w,
tomi.valkeinen-l0cyMroinI0
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA
Hi All,
Please review this patch.
this patch is aimed to solve some indentation issue. It has also solved
three trivial error in efifb.c file.
And I have also changed printk with pr_err, pr_info ... at respective places.
From c49139fac1d15fe2da80d06e2a79eb8be7c079a7 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
Date: Mon, 9 Feb 2015 07:33:59 -0500
Subject: [PATCH] Trival patch: improved indentation, and removed some ERROR
from code
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 158 +++++++++++++++++++++++---------------------
1 file changed, 84 insertions(+), 74 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..5c03035 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -17,29 +17,28 @@
#include <video/vga.h>
#include <asm/sysfb.h>
-static bool request_mem_succeeded = false;
+static bool request_mem_succeeded;
static struct fb_var_screeninfo efifb_defined = {
- .activate = FB_ACTIVATE_NOW,
- .height = -1,
- .width = -1,
- .right_margin = 32,
- .upper_margin = 16,
- .lower_margin = 4,
- .vsync_len = 4,
- .vmode = FB_VMODE_NONINTERLACED,
+ .activate = FB_ACTIVATE_NOW,
+ .height = -1,
+ .width = -1,
+ .right_margin = 32,
+ .upper_margin = 16,
+ .lower_margin = 4,
+ .vsync_len = 4,
+ .vmode = FB_VMODE_NONINTERLACED,
};
static struct fb_fix_screeninfo efifb_fix = {
- .id = "EFI VGA",
- .type = FB_TYPE_PACKED_PIXELS,
- .accel = FB_ACCEL_NONE,
- .visual = FB_VISUAL_TRUECOLOR,
+ .id = "EFI VGA",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .accel = FB_ACCEL_NONE,
+ .visual = FB_VISUAL_TRUECOLOR,
};
static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
- unsigned blue, unsigned transp,
- struct fb_info *info)
+ unsigned blue, unsigned transp, struct fb_info *info)
{
/*
* Set a single color register. The values supplied are
@@ -52,13 +51,13 @@ static int efifb_setcolreg(unsigned regno, unsigned red,
unsigned green,
return 1;
if (regno < 16) {
- red >>= 8;
+ red >>= 8;
green >>= 8;
- blue >>= 8;
- ((u32 *)(info->pseudo_palette))[regno] - (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
+ blue >>= 8;
+ ((u32 *) (info->pseudo_palette))[regno] + (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset);
}
return 0;
}
@@ -74,12 +73,12 @@ static void efifb_destroy(struct fb_info *info)
}
static struct fb_ops efifb_ops = {
- .owner = THIS_MODULE,
- .fb_destroy = efifb_destroy,
- .fb_setcolreg = efifb_setcolreg,
- .fb_fillrect = cfb_fillrect,
- .fb_copyarea = cfb_copyarea,
- .fb_imageblit = cfb_imageblit,
+ .owner = THIS_MODULE,
+ .fb_destroy = efifb_destroy,
+ .fb_setcolreg = efifb_setcolreg,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
};
static int efifb_setup(char *options)
@@ -89,25 +88,35 @@ static int efifb_setup(char *options)
if (options && *options) {
while ((this_opt = strsep(&options, ",")) != NULL) {
- if (!*this_opt) continue;
+ if (!*this_opt)
+ continue;
for (i = 0; i < M_UNKNOWN; i++) {
if (efifb_dmi_list[i].base != 0 &&
- !strcmp(this_opt,
efifb_dmi_list[i].optname)) {
- screen_info.lfb_base efifb_dmi_list[i].base;
- screen_info.lfb_linelength efifb_dmi_list[i].stride;
- screen_info.lfb_width efifb_dmi_list[i].width;
- screen_info.lfb_height efifb_dmi_list[i].height;
+ !strcmp(this_opt,
+ efifb_dmi_list[i].optname)) {
+ screen_info.lfb_base + efifb_dmi_list[i].base;
+ screen_info.lfb_linelength + efifb_dmi_list[i].stride;
+ screen_info.lfb_width + efifb_dmi_list[i].width;
+ screen_info.lfb_height + efifb_dmi_list[i].height;
}
}
if (!strncmp(this_opt, "base:", 5))
- screen_info.lfb_base simple_strtoul(this_opt+5, NULL, 0);
+ screen_info.lfb_base + simple_strtoul(this_opt + 5, NULL, 0);
else if (!strncmp(this_opt, "stride:", 7))
- screen_info.lfb_linelength simple_strtoul(this_opt+7, NULL, 0) * 4;
+ screen_info.lfb_linelength + simple_strtoul(this_opt + 7, NULL, 0) * 4;
else if (!strncmp(this_opt, "height:", 7))
- screen_info.lfb_height simple_strtoul(this_opt+7, NULL, 0);
+ screen_info.lfb_height + simple_strtoul(this_opt + 7, NULL, 0);
else if (!strncmp(this_opt, "width:", 6))
- screen_info.lfb_width simple_strtoul(this_opt+6, NULL, 0);
+ screen_info.lfb_width + simple_strtoul(this_opt + 6, NULL, 0);
}
}
@@ -142,10 +151,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_err("efifb: invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("efifb: probing for efifb\n");
/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -181,7 +190,7 @@ static int efifb_probe(struct platform_device *dev)
* use for efifb. With modern cards it is no
* option to simply use size_total as that
* wastes plenty of kernel address space. */
- size_remap = size_vmode * 2;
+ size_remap = size_vmode * 2;
if (size_remap > size_total)
size_remap = size_total;
if (size_remap % PAGE_SIZE)
@@ -193,14 +202,14 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
+ pr_warn(
"efifb: cannot reserve video memory at 0x%lx\n",
- efifb_fix.smem_start);
+ efifb_fix.smem_start);
}
info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -216,46 +225,46 @@ static int efifb_probe(struct platform_device *dev)
info->apertures->ranges[0].base = efifb_fix.smem_start;
info->apertures->ranges[0].size = size_remap;
- info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
+ info->screen_base + ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
- efifb_fix.smem_len, efifb_fix.smem_start);
+ pr_err("efifb: abort, cannot ioremap video memory "
+ "0x%x @ 0x%lx\n",
+ efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
+ pr_info("efifb: framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
efifb_fix.smem_start, info->screen_base,
- size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ size_remap / 1024, size_total / 1024);
+ pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);
efifb_defined.xres_virtual = efifb_defined.xres;
- efifb_defined.yres_virtual = efifb_fix.smem_len /
- efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length;
+ pr_info("efifb: scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;
/* some dummy values for timing to make fbset happy */
- efifb_defined.pixclock = 10000000 / efifb_defined.xres *
- 1000 / efifb_defined.yres;
- efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
- efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
-
- efifb_defined.red.offset = screen_info.red_pos;
- efifb_defined.red.length = screen_info.red_size;
- efifb_defined.green.offset = screen_info.green_pos;
- efifb_defined.green.length = screen_info.green_size;
- efifb_defined.blue.offset = screen_info.blue_pos;
- efifb_defined.blue.length = screen_info.blue_size;
+ efifb_defined.pixclock = 10000000 / efifb_defined.xres *
+ 1000 / efifb_defined.yres;
+ efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
+ efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
+
+ efifb_defined.red.offset = screen_info.red_pos;
+ efifb_defined.red.length = screen_info.red_size;
+ efifb_defined.green.offset = screen_info.green_pos;
+ efifb_defined.green.length = screen_info.green_size;
+ efifb_defined.blue.offset = screen_info.blue_pos;
+ efifb_defined.blue.length = screen_info.blue_size;
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;
- printk(KERN_INFO "efifb: %s: "
+ pr_info("efifb: %s: "
"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
"Truecolor",
screen_info.rsvd_size,
@@ -264,10 +273,9 @@ static int efifb_probe(struct platform_device *dev)
screen_info.blue_size,
screen_info.rsvd_pos,
screen_info.red_pos,
- screen_info.green_pos,
- screen_info.blue_pos);
+ screen_info.green_pos, screen_info.blue_pos);
- efifb_fix.ypanstep = 0;
+ efifb_fix.ypanstep = 0;
efifb_fix.ywrapstep = 0;
info->fbops = &efifb_ops;
@@ -275,12 +283,14 @@ static int efifb_probe(struct platform_device *dev)
info->fix = efifb_fix;
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
- if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ err = fb_alloc_cmap(&info->cmap, 256, 0);
+ if ((err < 0) {
+ pr_err("efifb: cannot allocate colormap\n");
goto err_unmap;
}
- if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ err = register_framebuffer(info);
+ if (err < 0) {
+ pr_err("efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
@@ -310,8 +320,8 @@ static int efifb_remove(struct platform_device *pdev)
static struct platform_driver efifb_driver = {
.driver = {
- .name = "efi-framebuffer",
- },
+ .name = "efi-framebuffer",
+ },
.probe = efifb_probe,
.remove = efifb_remove,
};
--
1.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox