* [PATCH 07/11] fblog: allow selecting fbs via sysfs and module-parameters
From: David Herrmann @ 2012-08-12 14:53 UTC (permalink / raw)
To: linux-fbdev
Cc: Florian Tobias Schandinat, Greg Kroah-Hartman, linux-serial,
Alan Cox, linux-kernel, Geert Uytterhoeven, David Herrmann
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
fblog is mainly useful during boot, reboot, panics and maintenance. In all
cases you often want to control which monitors are used for console
output. Moreover, in multi-seat environments it is desireable to reduce
system-overhead by not drawing the console to all framebuffers. Four
mechanisms to select framebuffers for fblog are added:
1) "active" module parameter: This parameter selects whether fblog has
access to available framebuffer devices. If it is true, then fblog will
open devices following the rules described below and rendering will take
place. If it is false, new hotplugged devices will not be activated and no
more rendering to currently active devices takes place. However, active
devices will continue rendering after this is set to true again.
2) "active" sysfs attribute for each fblog object. Reading this value
returns whether a framebuffer is currently active. Writing it opens/closes
the framebuffer. This allows runtime control which fbs are used. For
instance, init can set these to 0 after bootup.
Note that a framebuffer is only active if this is 1 _and_ the "active"
module parameter is set to "true".
3) "activate_on_hotplug" module parameter: This selects whether a device
is activated by default when hotplugged. This is true by default so new
devices will be automatically activated.
4) "main_only" module parameter: This selects what devices are activated
on hotplug. This has no effect if "activate_on_hotplug" is false.
Otherwise, if this is true then only fb0 will be activated on hotplug.
This is false by default.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/fblog.c | 66 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
index 1c526c5..aed77dc 100644
--- a/drivers/video/console/fblog.c
+++ b/drivers/video/console/fblog.c
@@ -44,6 +44,9 @@ struct fblog_fb {
static DEFINE_MUTEX(fblog_registration_lock);
static struct fblog_fb *fblog_fbs[FB_MAX];
+static bool active = true;
+static bool activate_on_hotplug = true;
+static bool main_only = false;
#define to_fblog_dev(_d) container_of(_d, struct fblog_fb, dev)
@@ -63,6 +66,9 @@ static int fblog_open(struct fblog_fb *fb)
{
int ret;
+ if (!active)
+ return -EPERM;
+
mutex_lock(&fb->lock);
if (test_bit(FBLOG_KILLED, &fb->flags)) {
@@ -115,6 +121,40 @@ static void fblog_close(struct fblog_fb *fb, bool kill_dev)
mutex_unlock(&fb->lock);
}
+static ssize_t fblog_dev_active_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct fblog_fb *fb = to_fblog_dev(dev);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n",
+ !!test_bit(FBLOG_OPEN, &fb->flags));
+}
+
+static ssize_t fblog_dev_active_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct fblog_fb *fb = to_fblog_dev(dev);
+ unsigned long num;
+ int ret = 0;
+
+ num = simple_strtoul(buf, NULL, 10);
+
+ mutex_lock(&fb->info->lock);
+ if (num)
+ ret = fblog_open(fb);
+ else
+ fblog_close(fb, false);
+ mutex_unlock(&fb->info->lock);
+
+ return ret ? ret : count;
+}
+
+static DEVICE_ATTR(active, S_IRUGO | S_IWUSR | S_IWGRP, fblog_dev_active_show,
+ fblog_dev_active_store);
+
/*
* fblog framebuffer list
* The fblog_fbs[] array contains all currently registered framebuffers. If a
@@ -148,6 +188,7 @@ static void fblog_do_unregister(struct fb_info *info)
fblog_fbs[info->node] = NULL;
fblog_close(fb, true);
+ device_remove_file(&fb->dev, &dev_attr_active);
device_del(&fb->dev);
put_device(&fb->dev);
}
@@ -156,6 +197,7 @@ static void fblog_do_register(struct fb_info *info, bool force)
{
struct fblog_fb *fb;
int ret;
+ bool do_open = true;
fb = fblog_fbs[info->node];
if (fb && fb->info != info) {
@@ -186,7 +228,20 @@ static void fblog_do_register(struct fb_info *info, bool force)
return;
}
- fblog_open(fb);
+ ret = device_create_file(&fb->dev, &dev_attr_active);
+ if (ret) {
+ pr_err("fblog: cannot create sysfs entry");
+ /* do not open fb if we cannot create control file */
+ do_open = false;
+ }
+
+ if (!activate_on_hotplug)
+ do_open = false;
+ if (main_only && info->node != 0)
+ do_open = false;
+
+ if (do_open)
+ fblog_open(fb);
}
static void fblog_register(struct fb_info *info, bool force)
@@ -321,6 +376,15 @@ static void __exit fblog_exit(void)
}
}
+module_param(active, bool, S_IRUGO | S_IWUSR | S_IWGRP);
+MODULE_PARM_DESC(active, "Activate fblog rendering");
+
+module_param(activate_on_hotplug, bool, S_IRUGO | S_IWUSR | S_IWGRP);
+MODULE_PARM_DESC(activate_on_hotplug, "Activate fblog on hotplugged devices");
+
+module_param(main_only, bool, S_IRUGO);
+MODULE_PARM_DESC(main_only, "Activate fblog by default only on main devices");
+
module_init(fblog_init);
module_exit(fblog_exit);
MODULE_LICENSE("GPL");
--
1.7.11.4
^ permalink raw reply related
* [PATCH 08/11] fblog: cache framebuffer BLANK and SUSPEND states
From: David Herrmann @ 2012-08-12 14:53 UTC (permalink / raw)
To: linux-fbdev
Cc: Florian Tobias Schandinat, Greg Kroah-Hartman, linux-serial,
Alan Cox, linux-kernel, Geert Uytterhoeven, David Herrmann
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
We must cache these states so we will never draw to the framebuffer while
it is suspended or blanked.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/fblog.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
index aed77dc..8bcd0ce 100644
--- a/drivers/video/console/fblog.c
+++ b/drivers/video/console/fblog.c
@@ -33,6 +33,8 @@
enum fblog_flags {
FBLOG_KILLED,
FBLOG_OPEN,
+ FBLOG_SUSPENDED,
+ FBLOG_BLANKED,
};
struct fblog_fb {
@@ -264,6 +266,7 @@ static int fblog_event(struct notifier_block *self, unsigned long action,
struct fb_event *event = data;
struct fb_info *info = event->info;
struct fblog_fb *fb;
+ int *blank;
switch(action) {
case FB_EVENT_FB_REGISTERED:
@@ -291,6 +294,44 @@ static int fblog_event(struct notifier_block *self, unsigned long action,
if (fb)
fblog_close(fb, true);
break;
+ case FB_EVENT_SUSPEND:
+ /* This is called when the low-level display driver suspends the
+ * video system. We should not access the video system while it
+ * is suspended. This is called with the console lock held. */
+ mutex_lock(&fblog_registration_lock);
+ fb = fblog_fbs[info->node];
+ mutex_unlock(&fblog_registration_lock);
+
+ if (fb)
+ set_bit(FBLOG_SUSPENDED, &fb->flags);
+ break;
+ case FB_EVENT_RESUME:
+ /* This is called when the low-level display driver resumes
+ * operating. It is called with the console lock held. */
+ mutex_lock(&fblog_registration_lock);
+ fb = fblog_fbs[info->node];
+ mutex_unlock(&fblog_registration_lock);
+
+ if (fb)
+ clear_bit(FBLOG_SUSPENDED, &fb->flags);
+ break;
+ case FB_EVENT_BLANK:
+ /* This gets called _after_ the framebuffer was successfully
+ * blanked. The console-lock is always held while fb_blank is
+ * called and during this callback. */
+ mutex_lock(&fblog_registration_lock);
+ fb = fblog_fbs[info->node];
+ mutex_unlock(&fblog_registration_lock);
+
+ if (!fb)
+ break;
+
+ blank = (int*)event->data;
+ if (*blank = FB_BLANK_UNBLANK)
+ clear_bit(FBLOG_BLANKED, &fb->flags);
+ else
+ set_bit(FBLOG_BLANKED, &fb->flags);
+ break;
}
return 0;
--
1.7.11.4
^ permalink raw reply related
* [PATCH 09/11] fblog: register console driver
From: David Herrmann @ 2012-08-12 14:53 UTC (permalink / raw)
To: linux-fbdev
Cc: Florian Tobias Schandinat, Greg Kroah-Hartman, linux-serial,
Alan Cox, linux-kernel, Geert Uytterhoeven, David Herrmann
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
We want to print the kernel log to all FBs so we need a console driver.
This registers the driver on startup and writes all messages to all
registered fblog instances.
We cannot share a console buffer between FBs because they might have
different resolutions. Therefore, we create one buffer per object. We
destroy the buffer during close() so we do not waste memory if it is not
used.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/fblog.c | 150 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 150 insertions(+)
diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
index 8bcd0ce..2e39577 100644
--- a/drivers/video/console/fblog.c
+++ b/drivers/video/console/fblog.c
@@ -25,11 +25,34 @@
#define pr_fmt(_fmt) KBUILD_MODNAME ": " _fmt
+#include <linux/console.h>
#include <linux/device.h>
#include <linux/fb.h>
#include <linux/module.h>
#include <linux/mutex.h>
+/**
+ * struct fblog_buf: Console text buffer
+ *
+ * Each framebuffer has its own text buffer which contains all characters that
+ * are currently printed on screen. The buffers might have different sizes and
+ * can be resized during runtime. When the buffer content changes, we redraw the
+ * screen.
+ *
+ * width: Width of buffer in characters
+ * height: Height of buffer in characters
+ * lines: Array of lines
+ * pos_x: Cursor x-position
+ * pos_y: Cursor y-position
+ */
+struct fblog_buf {
+ size_t width;
+ size_t height;
+ u16 **lines;
+ size_t pos_x;
+ size_t pos_y;
+};
+
enum fblog_flags {
FBLOG_KILLED,
FBLOG_OPEN,
@@ -42,6 +65,7 @@ struct fblog_fb {
struct fb_info *info;
struct device dev;
struct mutex lock;
+ struct fblog_buf buf;
};
static DEFINE_MUTEX(fblog_registration_lock);
@@ -52,6 +76,106 @@ static bool main_only = false;
#define to_fblog_dev(_d) container_of(_d, struct fblog_fb, dev)
+static void fblog_buf_resize(struct fblog_buf *buf, size_t width,
+ size_t height)
+{
+ u16 **lines = NULL;
+ size_t i, j, minw, minh;
+
+ if (buf->height = height && buf->width = width)
+ return;
+
+ if (width && height) {
+ lines = kzalloc(height * sizeof(char *), GFP_KERNEL);
+ if (!lines)
+ return;
+
+ for (i = 0; i < height; ++i) {
+ lines[i] = kzalloc(width * sizeof(u16), GFP_KERNEL);
+ if (!lines[i]) {
+ while (i--)
+ kfree(lines[i]);
+ return;
+ }
+ }
+
+ /* copy old lines */
+ minw = min(width, buf->width);
+ minh = min(height, buf->height);
+ if (height >= buf->height)
+ i = 0;
+ else
+ i = buf->height - height;
+
+ for (j = 0; j < minh; ++i, ++j)
+ memcpy(lines[j], buf->lines[i], minw * sizeof(u16));
+ } else {
+ width = 0;
+ height = 0;
+ }
+
+ for (i = 0; i < buf->height; ++i)
+ kfree(buf->lines[i]);
+ kfree(buf->lines);
+
+ buf->lines = lines;
+ buf->width = width;
+ buf->height = height;
+}
+
+static void fblog_buf_deinit(struct fblog_buf *buf)
+{
+ fblog_buf_resize(buf, 0, 0);
+}
+
+static void fblog_buf_rotate(struct fblog_buf *buf)
+{
+ u16 *line;
+
+ if (!buf->height)
+ return;
+
+ line = buf->lines[0];
+ memset(line, 0, sizeof(u16) * buf->width);
+
+ memmove(buf->lines, &buf->lines[1], sizeof(char*) * (buf->height - 1));
+ buf->lines[buf->height - 1] = line;
+}
+
+static void fblog_buf_write(struct fblog_buf *buf, const char *str, size_t len)
+{
+ char c;
+
+ if (!buf->height)
+ return;
+
+ while (len--) {
+ c = *str++;
+
+ if (c = 0)
+ c = '?';
+
+ if (c = '\n') {
+ buf->pos_x = 0;
+ if (++buf->pos_y >= buf->height) {
+ buf->pos_y = buf->height - 1;
+ fblog_buf_rotate(buf);
+ }
+ } else {
+ if (buf->pos_x >= buf->width) {
+ buf->pos_x = 0;
+ ++buf->pos_y;
+ }
+ if (buf->pos_y >= buf->height) {
+ buf->pos_y = buf->height - 1;
+ fblog_buf_rotate(buf);
+ }
+
+ buf->lines[buf->pos_y][buf->pos_x++] = c;
+ }
+ }
+}
+
/*
* fblog_open/close()
* These functions manage access to the underlying framebuffer. While opened, we
@@ -66,6 +190,7 @@ static bool main_only = false;
static int fblog_open(struct fblog_fb *fb)
{
+ static const char init_str[] = "Framebuffer log initialized\n";
int ret;
if (!active)
@@ -93,6 +218,8 @@ static int fblog_open(struct fblog_fb *fb)
goto out_unref;
}
+ fblog_buf_resize(&fb->buf, 80, 24);
+ fblog_buf_write(&fb->buf, init_str, sizeof(init_str) - 1);
set_bit(FBLOG_OPEN, &fb->flags);
mutex_unlock(&fb->lock);
return 0;
@@ -115,6 +242,7 @@ static void fblog_close(struct fblog_fb *fb, bool kill_dev)
fb->info->fbops->fb_release(fb->info, 0);
module_put(fb->info->fbops->owner);
clear_bit(FBLOG_OPEN, &fb->flags);
+ fblog_buf_deinit(&fb->buf);
}
if (kill_dev)
@@ -379,6 +507,26 @@ static struct notifier_block fblog_notifier = {
.notifier_call = fblog_event,
};
+static void fblog_con_write(struct console *con, const char *buf,
+ unsigned int len)
+{
+ int i;
+
+ mutex_lock(&fblog_registration_lock);
+ for (i = 0; i < FB_MAX; ++i) {
+ if (fblog_fbs[i]) {
+ fblog_buf_write(&fblog_fbs[i]->buf, buf, len);
+ }
+ }
+ mutex_unlock(&fblog_registration_lock);
+}
+
+static struct console fblog_con_driver = {
+ .name = "fblog",
+ .write = fblog_con_write,
+ .flags = CON_PRINTBUFFER | CON_ENABLED,
+};
+
static int __init fblog_init(void)
{
int ret;
@@ -390,6 +538,7 @@ static int __init fblog_init(void)
}
fblog_scan();
+ register_console(&fblog_con_driver);
return 0;
}
@@ -399,6 +548,7 @@ static void __exit fblog_exit(void)
unsigned int i;
struct fb_info *info;
+ unregister_console(&fblog_con_driver);
fb_unregister_client(&fblog_notifier);
/* We scan through the whole registered_fb array here instead of
--
1.7.11.4
^ permalink raw reply related
* [PATCH 10/11] fblog: draw console to framebuffers
From: David Herrmann @ 2012-08-12 14:53 UTC (permalink / raw)
To: linux-fbdev
Cc: Florian Tobias Schandinat, Greg Kroah-Hartman, linux-serial,
Alan Cox, linux-kernel, Geert Uytterhoeven, David Herrmann
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
If not disabled or suspended, we now blit the console data to each
framebuffer. We only redraw on changes to avoid consuming too much CPU
power.
This isn't optimized for speed, currently. However, fblog is mainly used
for debugging purposes so this can be optimized later.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/fblog.c | 110 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 109 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
index 2e39577..25bb63d 100644
--- a/drivers/video/console/fblog.c
+++ b/drivers/video/console/fblog.c
@@ -28,8 +28,11 @@
#include <linux/console.h>
#include <linux/device.h>
#include <linux/fb.h>
+#include <linux/font.h>
+#include <linux/kd.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include "fbdraw.h"
/**
* struct fblog_buf: Console text buffer
@@ -66,6 +69,7 @@ struct fblog_fb {
struct device dev;
struct mutex lock;
struct fblog_buf buf;
+ struct console_font font;
};
static DEFINE_MUTEX(fblog_registration_lock);
@@ -176,6 +180,61 @@ static void fblog_buf_write(struct fblog_buf *buf, const char *str, size_t len)
}
}
+static void fblog_redraw_clear(struct fblog_fb *fb)
+{
+ struct fb_fillrect region;
+ struct fb_info *info = fb->info;
+
+ region.color = 0;
+ region.dx = 0;
+ region.dy = 0;
+ region.width = info->var.xres;
+ region.height = info->var.yres;
+ region.rop = ROP_COPY;
+
+ info->fbops->fb_fillrect(info, ®ion);
+}
+
+static void fblog_redraw(struct fblog_fb *fb)
+{
+ size_t i;
+
+ if (!active)
+ return;
+
+ mutex_lock(&fb->lock);
+ if (!test_bit(FBLOG_OPEN, &fb->flags) ||
+ test_bit(FBLOG_SUSPENDED, &fb->flags) ||
+ test_bit(FBLOG_BLANKED, &fb->flags)) {
+ mutex_unlock(&fb->lock);
+ return;
+ }
+
+ fblog_redraw_clear(fb);
+
+ for (i = 0; i < fb->buf.height; ++i) {
+ fbdraw_font(fb->info, &fb->font, false, 0, i, 7, 0, 0,
+ fb->buf.lines[i], fb->buf.width);
+ }
+
+ mutex_unlock(&fb->lock);
+}
+
+static void fblog_refresh(struct fblog_fb *fb)
+{
+ unsigned int width, height;
+
+ mutex_lock(&fb->lock);
+ if (test_bit(FBLOG_OPEN, &fb->flags)) {
+ width = fb->info->var.xres / fb->font.width;
+ height = fb->info->var.yres / fb->font.height;
+ fblog_buf_resize(&fb->buf, width, height);
+ }
+ mutex_unlock(&fb->lock);
+
+ fblog_redraw(fb);
+}
+
/*
* fblog_open/close()
* These functions manage access to the underlying framebuffer. While opened, we
@@ -192,6 +251,10 @@ static int fblog_open(struct fblog_fb *fb)
{
static const char init_str[] = "Framebuffer log initialized\n";
int ret;
+ struct fb_var_screeninfo var;
+ const struct fb_videomode *mode;
+ unsigned int width, height;
+ const struct font_desc *font;
if (!active)
return -EPERM;
@@ -208,6 +271,13 @@ static int fblog_open(struct fblog_fb *fb)
goto unlock;
}
+ font = get_default_font(var.xres, var.yres, fb->info->pixmap.blit_x,
+ fb->info->pixmap.blit_y);
+ if (!font) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+
if (!try_module_get(fb->info->fbops->owner)) {
ret = -ENODEV;
goto out_killed;
@@ -218,10 +288,22 @@ static int fblog_open(struct fblog_fb *fb)
goto out_unref;
}
- fblog_buf_resize(&fb->buf, 80, 24);
+ var = fb->info->var;
+ mode = fb_find_best_mode(&var, &fb->info->modelist);
+ var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE;
+ fb_set_var(fb->info, &var);
+
+ fb->font.width = font->width;
+ fb->font.height = font->height;
+ fb->font.data = (void*)font->data;
+
+ width = var.xres / fb->font.width;
+ height = var.yres / fb->font.height;
+ fblog_buf_resize(&fb->buf, width, height);
fblog_buf_write(&fb->buf, init_str, sizeof(init_str) - 1);
set_bit(FBLOG_OPEN, &fb->flags);
mutex_unlock(&fb->lock);
+ fblog_redraw(fb);
return 0;
out_unref:
@@ -460,6 +542,31 @@ static int fblog_event(struct notifier_block *self, unsigned long action,
else
set_bit(FBLOG_BLANKED, &fb->flags);
break;
+ case FB_EVENT_MODE_DELETE:
+ /* This is sent when a video mode is removed. The current video
+ * mode is never removed! The console lock is held while this is
+ * called. */
+ /* fallthrough */
+ case FB_EVENT_NEW_MODELIST:
+ /* This is sent when the modelist got changed. The console-lock
+ * is held and we should reset the mode. */
+ /* fallthrough */
+ case FB_EVENT_MODE_CHANGE_ALL:
+ /* This is the same as below but notifies us that the user used
+ * the FB_ACTIVATE_ALL flag when setting the video mode. */
+ /* fallthrough */
+ case FB_EVENT_MODE_CHANGE:
+ /* This is called when the _user_ changes the video mode via
+ * ioctls. It is not sent, when the kernel changes the mode
+ * internally. This callback is called inside fb_set_var() so
+ * the console lock is held. */
+ mutex_lock(&fblog_registration_lock);
+ fb = fblog_fbs[info->node];
+ mutex_unlock(&fblog_registration_lock);
+
+ if (fb)
+ fblog_refresh(fb);
+ break;
}
return 0;
@@ -516,6 +623,7 @@ static void fblog_con_write(struct console *con, const char *buf,
for (i = 0; i < FB_MAX; ++i) {
if (fblog_fbs[i]) {
fblog_buf_write(&fblog_fbs[i]->buf, buf, len);
+ fblog_redraw(fblog_fbs[i]);
}
}
mutex_unlock(&fblog_registration_lock);
--
1.7.11.4
^ permalink raw reply related
* [PATCH 11/11] MAINTAINERS: add fblog entry
From: David Herrmann @ 2012-08-12 14:53 UTC (permalink / raw)
To: linux-fbdev
Cc: Florian Tobias Schandinat, Greg Kroah-Hartman, linux-serial,
Alan Cox, linux-kernel, Geert Uytterhoeven, David Herrmann
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
Add myself as maintainer for the fblog driver to the MAINTAINERS file.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 375b1fd..40a10b54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2895,6 +2895,12 @@ F: drivers/video/
F: include/video/
F: include/linux/fb.h
+FRAMEBUFFER LOG DRIVER
+M: David Herrmann <dh.herrmann@googlemail.com>
+L: linux-fbdev@vger.kernel.org
+S: Maintained
+F: drivers/video/console/fblog.c
+
FREESCALE DMA DRIVER
M: Li Yang <leoli@freescale.com>
M: Zhang Wei <zw@zh-kernel.org>
--
1.7.11.4
^ permalink raw reply related
* [PATCH] drivers: console: font_: Change a glyph from "broken bar" to "vertical line"
From: Bjarni Ingi Gislason @ 2012-08-12 15:05 UTC (permalink / raw)
To: linux-fbdev
The code 124 (0x7C, |) is rendered as a broken line in two
fonts, instead of a continuous line. Some keyboards show a
"broken bar" on one of theirs keys, other show a (continuous)
"vertical line".
Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
linux-3.5/drivers/video/console/font_mini_4x6.c | 2 +-
linux-3.5/drivers/video/console/font_sun8x16.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff -upr -X linux-3.5/Documentation/dontdiff linux-3.5/drivers/video/console/font_mini_4x6.c devel/linux-3.5/drivers/video/console/font_mini_4x6.c
--- linux-3.5/drivers/video/console/font_mini_4x6.c 2012-07-21 20:58:29.000000000 +0000
+++ devel/linux-3.5/drivers/video/console/font_mini_4x6.c 2012-08-11 15:27:41.778918653 +0000
@@ -1092,7 +1092,7 @@ static const unsigned char fontdata_mini
/*{*/ /* Char 124: '|' */
0x44, /*= [ * ] */
0x44, /*= [ * ] */
- 0x00, /*= [ ] */
+ 0x44, /*= [ * ] */
0x44, /*= [ * ] */
0x44, /*= [ * ] */
0x00, /*= [ ] */
diff -upr -X linux-3.5/Documentation/dontdiff linux-3.5/drivers/video/console/font_sun8x16.c devel/linux-3.5/drivers/video/console/font_sun8x16.c
--- linux-3.5/drivers/video/console/font_sun8x16.c 2012-07-21 20:58:29.000000000 +0000
+++ devel/linux-3.5/drivers/video/console/font_sun8x16.c 2012-08-11 15:28:22.490299722 +0000
@@ -127,7 +127,7 @@ static const unsigned char fontdata_sun8
/*y*/ 0x00,0x00,0x00,0x00,0x00,0xc6,0xc6,0xc6,0xc6,0xc6,0xc6,0x7e,0x06,0x0c,0xf8,0x00,
/*z*/ 0x00,0x00,0x00,0x00,0x00,0xfe,0xcc,0x18,0x30,0x60,0xc6,0xfe,0x00,0x00,0x00,0x00,
/*{*/ 0x00,0x00,0x0e,0x18,0x18,0x18,0x70,0x18,0x18,0x18,0x18,0x0e,0x00,0x00,0x00,0x00,
-/*|*/ 0x00,0x00,0x18,0x18,0x18,0x18,0x00,0x18,0x18,0x18,0x18,0x18,0x00,0x00,0x00,0x00,
+/*|*/ 0x00,0x00,0x18,0x18,0x18,0x18,0x18,0x18,0x18,0x18,0x18,0x18,0x00,0x00,0x00,0x00,
/*}*/ 0x00,0x00,0x70,0x18,0x18,0x18,0x0e,0x18,0x18,0x18,0x18,0x70,0x00,0x00,0x00,0x00,
/*~*/ 0x00,0x00,0x76,0xdc,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
/* */ 0x00,0x00,0x00,0x00,0x10,0x38,0x6c,0xc6,0xc6,0xc6,0xfe,0x00,0x00,0x00,0x00,0x00,
--
Bjarni I. Gislason
^ permalink raw reply
* Re: [PATCH 00/11] fblog: Framebuffer kernel log driver v4
From: Florian Tobias Schandinat @ 2012-08-12 15:28 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, Greg Kroah-Hartman, linux-serial, Alan Cox,
linux-kernel, Geert Uytterhoeven
In-Reply-To: <1344783205-2384-1-git-send-email-dh.herrmann@googlemail.com>
On 08/12/2012 02:53 PM, David Herrmann wrote:
> Hi
>
> This is revision 4 of the fblog driver. It is a replacement for fbcon for
> systems that do not want/need CONFIG_VT. It simply prints the kernel-log to all
> connected framebuffers. Previous versions are available here:
> v3: http://thread.gmane.org/gmane.linux.kernel/1328164
> v2: http://thread.gmane.org/gmane.linux.serial/8133
> v1: http://marc.info/?l=linux-kernel&m\x133988465602225&w=2
>
> This patchset is based on linux-next.
>
> Changes from previous version:
> - Changed mailinglist to linux-fbdev instead of linux-serial
> - Added "main_only" and "activate_on_hotplug" module parameters to provide
> more fine-grained control over which framebuffers are used
>
> As always simply remove the !VT dependency if you want to test this with VTs
> enabled. Some more background information on deprecating CONFIG_VT can be found
> here:
> http://dvdhrm.wordpress.com/2012/08/12/killing-off-config_vt/
>
> I am still looking for someone who is willing to push this to linux-next through
> his or her tree. Feedback is much appreciated!
I'll take care of this. But I'm currently very busy writing a thesis, so
it could take some time. It would be great if someone would find time to
give this a complete thorough review.
Thanks,
Florian Tobias Schandinat
>
> Thanks
> David
>
> David Herrmann (11):
> fbcon: move update_attr() into separate source file
> fbcon: move bit_putcs() into separate source file
> fblog: new framebuffer kernel log dummy driver
> fbdev: export get_fb_info()/put_fb_info()
> fblog: register one fblog object per framebuffer
> fblog: open fb on registration
> fblog: allow selecting fbs via sysfs and module-parameters
> fblog: cache framebuffer BLANK and SUSPEND states
> fblog: register console driver
> fblog: draw console to framebuffers
> MAINTAINERS: add fblog entry
>
> MAINTAINERS | 6 +
> drivers/video/Kconfig | 5 +-
> drivers/video/Makefile | 2 +-
> drivers/video/console/Kconfig | 37 ++-
> drivers/video/console/Makefile | 4 +-
> drivers/video/console/bitblit.c | 149 +--------
> drivers/video/console/fbcon.h | 5 +-
> drivers/video/console/fbdraw.c | 171 ++++++++++
> drivers/video/console/fbdraw.h | 30 ++
> drivers/video/console/fblog.c | 691 ++++++++++++++++++++++++++++++++++++++++
> drivers/video/fbmem.c | 6 +-
> include/linux/fb.h | 3 +
> 12 files changed, 951 insertions(+), 158 deletions(-)
> create mode 100644 drivers/video/console/fbdraw.c
> create mode 100644 drivers/video/console/fbdraw.h
> create mode 100644 drivers/video/console/fblog.c
>
^ permalink raw reply
* Re: [PATCH 03/11] fblog: new framebuffer kernel log dummy driver
From: Ryan Mallon @ 2012-08-12 23:34 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, Florian Tobias Schandinat, Greg Kroah-Hartman,
linux-serial, Alan Cox, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1344783205-2384-4-git-send-email-dh.herrmann@googlemail.com>
On 13/08/12 00:53, David Herrmann wrote:
> Fblog displays all kernel log messages on all connected framebuffers. It
> replaces fbcon when CONFIG_VT=n is selected. Its main purpose is to debug
> boot problems by displaying the whole boot log on the screen. This patch
> provides the first dummy module-init/deinit functions.
>
> As it uses all the font and fb functions I placed it in
> drivers/video/console. However, this means that we need to move the check
> for CONFIG_VT in Makefile/Kconfig from drivers/video into
> drivers/video/console as fblog does not depend on CONFIG_VT.
>
> Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Hi David,
Couple of comments below.
~Ryan
> config VGA_CONSOLE
> bool "VGA text console" if EXPERT || !X86
> - depends on !4xx && !8xx && !SPARC && !M68K && !PARISC && !FRV && !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER)
> + depends on VT && !4xx && !8xx && !SPARC && !M68K && !PARISC && !FRV && !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER)
> default y
> help
> Saying Y here will allow you to use Linux in text mode through a
> @@ -45,7 +45,7 @@ config VGACON_SOFT_SCROLLBACK_SIZE
> screenfuls of scrollback buffer
You could just place all of the CONFIG_VT options inside an if VT and
then the new fblog options in the else case rather than modifying all of
the (already overly long) depends statements.
> +config FBLOG
> + tristate "Framebuffer Kernel Log Driver"
> + depends on !VT && FB
> + default n
Default n is not required, all options are default n unless otherwise
specified. I don't think you need the dependency on FB either, since
this file should only get included if CONFIG_FB is set?
> + help
> + This driver displays all kernel log messages on all connected
> + framebuffers. It is mutually exclusive with CONFIG_FRAMEBUFFER_CONSOLE
> + and CONFIG_VT. It was mainly created for debugging purposes when
> + CONFIG_VT is not selected but you still want kernel boot messages on
> + the screen.
Do command line options exist to specify screens to write the debug log
to? It might be a useful feature to have.
~Ryan
^ permalink raw reply
* Re: [PATCH 05/11] fblog: register one fblog object per framebuffer
From: Ryan Mallon @ 2012-08-12 23:54 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, Florian Tobias Schandinat, Greg Kroah-Hartman,
linux-serial, Alan Cox, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1344783205-2384-6-git-send-email-dh.herrmann@googlemail.com>
On 13/08/12 00:53, David Herrmann wrote:
> One fblog object is associated to each registered framebuffer. This way,
> we can draw the console to each framebuffer. When a framebuffer driver
> unregisters a framebuffer, we also unregister our fblog object. That is,
> our lifetime is coupled to the lifetime of the framebuffer. However, this
> does not mean that we are always active. On the contrary, we do not even
> own a reference to the framebuffer. We don't need it as we are notified
> _before_ the last reference is dropped.
>
> However, if other users have a reference to our object, we simply mark it
> as dead when the associated framebuffer dies and leave it alone. When the
> last reference is dropped, it will be automatically freed.
>
> Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Hi David,
Quick review below.
Thanks,
~Ryan
> ---
> drivers/video/console/fblog.c | 195 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 195 insertions(+)
>
> diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
> index fb39737..279f4d8 100644
> --- a/drivers/video/console/fblog.c
> +++ b/drivers/video/console/fblog.c
> @@ -23,15 +23,210 @@
> * all fblog instances before running other graphics applications.
> */
>
> +#define pr_fmt(_fmt) KBUILD_MODNAME ": " _fmt
> +
> +#include <linux/device.h>
> +#include <linux/fb.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +enum fblog_flags {
> + FBLOG_KILLED,
> +};
> +
> +struct fblog_fb {
> + unsigned long flags;
Are more flags added in later patches? If not, why not just have:
bool is_killed;
?
> + struct fb_info *info;
> + struct device dev;
> +};
> +
> +static DEFINE_MUTEX(fblog_registration_lock);
> +static struct fblog_fb *fblog_fbs[FB_MAX];
> +
> +#define to_fblog_dev(_d) container_of(_d, struct fblog_fb, dev)
> +
> +/*
> + * fblog framebuffer list
> + * The fblog_fbs[] array contains all currently registered framebuffers. If a
> + * framebuffer is in that list, we always must make sure that we own a reference
> + * to it. If it is added through the notifier callbacks, then this is always
> + * guaranteed.
> + * We are only interested in registered framebuffers. That is, if a driver calls
> + * unregister_framebuffer() we directly unlink it from our list. This guarantees
> + * that the associated fb_info is always valid. However, we might still have
> + * pending users so we mark it as dead so no further framebuffer actions are
> + * done. If the last user then drops a reference, the memory gets freed
> + * automatically.
> + */
> +
> +static void fblog_release(struct device *dev)
> +{
> + struct fblog_fb *fb = to_fblog_dev(dev);
> +
> + kfree(fb);
> + module_put(THIS_MODULE);
> +}
> +
> +static void fblog_do_unregister(struct fb_info *info)
> +{
> + struct fblog_fb *fb;
> +
> + fb = fblog_fbs[info->node];
> + if (!fb || fb->info != info)
> + return;
> +
> + fblog_fbs[info->node] = NULL;
> +
> + device_del(&fb->dev);
> + put_device(&fb->dev);
device_unregister?
> +}
> +
> +static void fblog_do_register(struct fb_info *info, bool force)
> +{
> + struct fblog_fb *fb;
> + int ret;
> +
> + fb = fblog_fbs[info->node];
> + if (fb && fb->info != info) {
> + if (!force)
> + return;
> +
> + fblog_do_unregister(fb->info);
> + }
> +
> + fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> + if (!fb)
> + return;
> +
> + fb->info = info;
> + __module_get(THIS_MODULE);
> + device_initialize(&fb->dev);
> + fb->dev.class = fb_class;
> + fb->dev.release = fblog_release;
> + dev_set_name(&fb->dev, "fblog%d", info->node);
> + fblog_fbs[info->node] = fb;
> +
> + ret = device_add(&fb->dev);
> + if (ret) {
> + fblog_fbs[info->node] = NULL;
> + set_bit(FBLOG_KILLED, &fb->flags);
> + put_device(&fb->dev);
kfree(fb); ?
> + return;
> + }
> +}
> +
> +static void fblog_register(struct fb_info *info, bool force)
> +{
> + mutex_lock(&fblog_registration_lock);
> + fblog_do_register(info, force);
> + mutex_unlock(&fblog_registration_lock);
> +}
> +
> +static void fblog_unregister(struct fb_info *info)
> +{
> + mutex_lock(&fblog_registration_lock);
> + fblog_do_unregister(info);
> + mutex_unlock(&fblog_registration_lock);
> +}
This locking is needlessly heavy, and could easily pushed down into the
fb_do_(un)register functions. It would also help make it clear exactly
what the lock is protecting.
> +static int fblog_event(struct notifier_block *self, unsigned long action,
> + void *data)
> +{
> + struct fb_event *event = data;
> + struct fb_info *info = event->info;
> +
> + switch(action) {
> + case FB_EVENT_FB_REGISTERED:
> + /* This is called when a low-level system driver registers a new
> + * framebuffer. The registration lock is held but the console
> + * lock might not be held when this is called. */
Nitpick:
/*
* The Linux kernel multi-line
* comment style looks like
* this.
*/
> + fblog_register(info, true);
> + break;
> + case FB_EVENT_FB_UNREGISTERED:
> + /* This is called when a low-level system driver unregisters a
> + * framebuffer. The registration lock is held but the console
> + * lock might not be held. */
> + fblog_unregister(info);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void fblog_scan(void)
> +{
> + unsigned int i;
> + struct fb_info *info, *tmp;
> +
> + for (i = 0; i < FB_MAX; ++i) {
> + info = get_fb_info(i);
> + if (!info || IS_ERR(info))
Nitpick:
if (IS_ERR_OR_NULL(info))
> + continue;
> +
> + fblog_register(info, false);
This function should really return a value to indicate if it failed.
There is no point continuing if it didn't register anything.
> + /* There is a very subtle race-condition. Even though we might
> + * own a reference to the fb, it may still get unregistered
> + * between our call from get_fb_info() and fblog_register().
> + * Therefore, we simply check whether the same fb still is
> + * registered by calling get_fb_info() again. Only if they
> + * differ we know that it got unregistered, therefore, we
> + * call fblog_unregister() with the old pointer. */
> +
> + tmp = get_fb_info(i);
> + if (tmp && !IS_ERR(tmp))
> + put_fb_info(tmp);
> + if (tmp != info)
> + fblog_unregister(info);
It would be better to fix this issue properly. Calling fblog_unregister
here also looks unsafe if the call to fblog_register above failed.
> + /* Here we either called fblog_unregister() and therefore do not
> + * need any reference to the fb, or we can be sure that the FB
> + * is registered and FB_EVENT_FB_UNREGISTERED will be called
> + * before the last reference is dropped. Hence, we can drop our
> + * reference here. */
This seems a slightly odd reasoning. Why would you not hold a reference
to something you are using?
> + put_fb_info(info);
> + }
> +}
> +
> +static struct notifier_block fblog_notifier = {
> + .notifier_call = fblog_event,
> +};
>
> static int __init fblog_init(void)
> {
> + int ret;
> +
> + ret = fb_register_client(&fblog_notifier);
> + if (ret) {
> + pr_err("cannot register framebuffer notifier\n");
> + return ret;
> + }
> +
> + fblog_scan();
> +
> return 0;
> }
>
> static void __exit fblog_exit(void)
> {
> + unsigned int i;
> + struct fb_info *info;
> +
> + fb_unregister_client(&fblog_notifier);
> +
> + /* We scan through the whole registered_fb array here instead of
> + * fblog_fbs because we need to get the device lock _before_ the
> + * fblog-registration-lock. */
> +
> + for (i = 0; i < FB_MAX; ++i) {
> + info = get_fb_info(i);
> + if (!info || IS_ERR(info))
> + continue;
> +
> + fblog_unregister(info);
Given the description of the get_fb_info/fblog_register race above, can
this unregister the wrong framebuffer?
> + put_fb_info(info);
> + }
> }
>
> module_init(fblog_init);
>
^ permalink raw reply
* Re: [PATCH 06/11] fblog: open fb on registration
From: Ryan Mallon @ 2012-08-13 0:00 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, Florian Tobias Schandinat, Greg Kroah-Hartman,
linux-serial, Alan Cox, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1344783205-2384-7-git-send-email-dh.herrmann@googlemail.com>
On 13/08/12 00:53, David Herrmann wrote:
> This opens the framebuffer upon registration so we can use it for
> drawing-operations. On unregistration we close it again.
>
> While opening/closing or accessing the fb in any other way, we must hold
> the fb-mutex. However, since the notifiers are often called with the mutex
> already held, we cannot lock it _after_ taking the
> fblog_registration_lock. Therefore, we require the caller to make sure the
> fb-mutex is held.
>
> Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Some comments below,
~Ryan
> ---
> drivers/video/console/fblog.c | 94 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
> index 279f4d8..1c526c5 100644
> --- a/drivers/video/console/fblog.c
> +++ b/drivers/video/console/fblog.c
> @@ -32,12 +32,14 @@
>
> enum fblog_flags {
> FBLOG_KILLED,
> + FBLOG_OPEN,
> };
>
> struct fblog_fb {
> unsigned long flags;
> struct fb_info *info;
> struct device dev;
> + struct mutex lock;
> };
>
> static DEFINE_MUTEX(fblog_registration_lock);
> @@ -46,6 +48,74 @@ static struct fblog_fb *fblog_fbs[FB_MAX];
> #define to_fblog_dev(_d) container_of(_d, struct fblog_fb, dev)
>
> /*
> + * fblog_open/close()
> + * These functions manage access to the underlying framebuffer. While opened, we
> + * have a valid reference to the fb and can use it for drawing operations. When
> + * the fb is unregistered, we drop our reference and close the fb so it can get
> + * deleted properly. We also mark it as dead so no further fblog_open() call
> + * will succeed.
> + * Both functions must be called with the fb->info->lock mutex held! But make
> + * sure to lock it _before_ locking the fblog-registration-lock. Otherwise, we
> + * will dead-lock with fb-registration.
> + */
> +
> +static int fblog_open(struct fblog_fb *fb)
> +{
> + int ret;
> +
> + mutex_lock(&fb->lock);
> +
> + if (test_bit(FBLOG_KILLED, &fb->flags)) {
> + ret = -ENODEV;
> + goto unlock;
> + }
> +
> + if (test_bit(FBLOG_OPEN, &fb->flags)) {
> + ret = 0;
> + goto unlock;
> + }
> +
> + if (!try_module_get(fb->info->fbops->owner)) {
> + ret = -ENODEV;
> + goto out_killed;
> + }
> +
> + if (fb->info->fbops->fb_open && fb->info->fbops->fb_open(fb->info, 0)) {
Should propagate the error here:
if (fb->info->fbops->fb_open) {
ret = fb->info->fbops->fb_open(fb->info, 0);
if (ret)
gotou out_unref;
}
> + ret = -EIO;
> + goto out_unref;
> + }
> +
> + set_bit(FBLOG_OPEN, &fb->flags);
> + mutex_unlock(&fb->lock);
> + return 0;
> +
> +out_unref:
> + module_put(fb->info->fbops->owner);
> +out_killed:
> + set_bit(FBLOG_KILLED, &fb->flags);
> +unlock:
> + mutex_unlock(&fb->lock);
> + return ret;
> +}
> +
> +static void fblog_close(struct fblog_fb *fb, bool kill_dev)
> +{
> + mutex_lock(&fb->lock);
> +
> + if (test_bit(FBLOG_OPEN, &fb->flags)) {
> + if (fb->info->fbops->fb_release)
> + fb->info->fbops->fb_release(fb->info, 0);
> + module_put(fb->info->fbops->owner);
> + clear_bit(FBLOG_OPEN, &fb->flags);
> + }
> +
> + if (kill_dev)
> + set_bit(FBLOG_KILLED, &fb->flags);
> +
> + mutex_unlock(&fb->lock);
> +}
> +
> +/*
> * fblog framebuffer list
> * The fblog_fbs[] array contains all currently registered framebuffers. If a
> * framebuffer is in that list, we always must make sure that we own a reference
> @@ -77,6 +147,7 @@ static void fblog_do_unregister(struct fb_info *info)
>
> fblog_fbs[info->node] = NULL;
>
> + fblog_close(fb, true);
> device_del(&fb->dev);
> put_device(&fb->dev);
device_unregister?
> }
> @@ -99,6 +170,7 @@ static void fblog_do_register(struct fb_info *info, bool force)
> return;
>
> fb->info = info;
> + mutex_init(&fb->lock);
> __module_get(THIS_MODULE);
> device_initialize(&fb->dev);
> fb->dev.class = fb_class;
> @@ -113,6 +185,8 @@ static void fblog_do_register(struct fb_info *info, bool force)
> put_device(&fb->dev);
> return;
> }
> +
> + fblog_open(fb);
> }
This function should really return an errno value.
>
> static void fblog_register(struct fb_info *info, bool force)
> @@ -134,6 +208,7 @@ static int fblog_event(struct notifier_block *self, unsigned long action,
> {
> struct fb_event *event = data;
> struct fb_info *info = event->info;
> + struct fblog_fb *fb;
>
> switch(action) {
> case FB_EVENT_FB_REGISTERED:
> @@ -145,8 +220,21 @@ static int fblog_event(struct notifier_block *self, unsigned long action,
> case FB_EVENT_FB_UNREGISTERED:
> /* This is called when a low-level system driver unregisters a
> * framebuffer. The registration lock is held but the console
> - * lock might not be held. */
> + * lock might not be held. The fb-lock is not held, either! */
> + mutex_lock(&info->lock);
> fblog_unregister(info);
> + mutex_unlock(&info->lock);
> + break;
> + case FB_EVENT_FB_UNBIND:
> + /* Called directly before unregistering an FB. The FB is still
> + * valid here and the registration lock is held but the console
> + * lock might not be held (really?). */
> + mutex_lock(&fblog_registration_lock);
> + fb = fblog_fbs[info->node];
> + mutex_unlock(&fblog_registration_lock);
> +
> + if (fb)
> + fblog_close(fb, true);
> break;
> }
>
> @@ -163,7 +251,9 @@ static void fblog_scan(void)
> if (!info || IS_ERR(info))
> continue;
>
> + mutex_lock(&info->lock);
> fblog_register(info, false);
> + mutex_unlock(&info->lock);
>
> /* There is a very subtle race-condition. Even though we might
> * own a reference to the fb, it may still get unregistered
> @@ -224,7 +314,9 @@ static void __exit fblog_exit(void)
> if (!info || IS_ERR(info))
> continue;
>
> + mutex_lock(&info->lock);
> fblog_unregister(info);
> + mutex_unlock(&info->lock);
> put_fb_info(info);
> }
> }
>
^ permalink raw reply
* Re: [PATCH 07/11] fblog: allow selecting fbs via sysfs and module-parameters
From: Ryan Mallon @ 2012-08-13 0:04 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, Florian Tobias Schandinat, Greg Kroah-Hartman,
linux-serial, Alan Cox, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1344783205-2384-8-git-send-email-dh.herrmann@googlemail.com>
On 13/08/12 00:53, David Herrmann wrote:
> fblog is mainly useful during boot, reboot, panics and maintenance. In all
> cases you often want to control which monitors are used for console
> output. Moreover, in multi-seat environments it is desireable to reduce
> system-overhead by not drawing the console to all framebuffers. Four
> mechanisms to select framebuffers for fblog are added:
>
> 1) "active" module parameter: This parameter selects whether fblog has
> access to available framebuffer devices. If it is true, then fblog will
> open devices following the rules described below and rendering will take
> place. If it is false, new hotplugged devices will not be activated and no
> more rendering to currently active devices takes place. However, active
> devices will continue rendering after this is set to true again.
>
> 2) "active" sysfs attribute for each fblog object. Reading this value
> returns whether a framebuffer is currently active. Writing it opens/closes
> the framebuffer. This allows runtime control which fbs are used. For
> instance, init can set these to 0 after bootup.
> Note that a framebuffer is only active if this is 1 _and_ the "active"
> module parameter is set to "true".
>
> 3) "activate_on_hotplug" module parameter: This selects whether a device
> is activated by default when hotplugged. This is true by default so new
> devices will be automatically activated.
>
> 4) "main_only" module parameter: This selects what devices are activated
> on hotplug. This has no effect if "activate_on_hotplug" is false.
> Otherwise, if this is true then only fb0 will be activated on hotplug.
> This is false by default.
>
> Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
> ---
> drivers/video/console/fblog.c | 66 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
> index 1c526c5..aed77dc 100644
> --- a/drivers/video/console/fblog.c
> +++ b/drivers/video/console/fblog.c
> @@ -44,6 +44,9 @@ struct fblog_fb {
>
> static DEFINE_MUTEX(fblog_registration_lock);
> static struct fblog_fb *fblog_fbs[FB_MAX];
> +static bool active = true;
> +static bool activate_on_hotplug = true;
> +static bool main_only = false;
>
> #define to_fblog_dev(_d) container_of(_d, struct fblog_fb, dev)
>
> @@ -63,6 +66,9 @@ static int fblog_open(struct fblog_fb *fb)
> {
> int ret;
>
> + if (!active)
> + return -EPERM;
> +
> mutex_lock(&fb->lock);
>
> if (test_bit(FBLOG_KILLED, &fb->flags)) {
> @@ -115,6 +121,40 @@ static void fblog_close(struct fblog_fb *fb, bool kill_dev)
> mutex_unlock(&fb->lock);
> }
>
> +static ssize_t fblog_dev_active_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct fblog_fb *fb = to_fblog_dev(dev);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + !!test_bit(FBLOG_OPEN, &fb->flags));
Nitpick. sprintf is okay here, %d is rarely longer than PAGE_SIZE :-).
> +}
> +
> +static ssize_t fblog_dev_active_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct fblog_fb *fb = to_fblog_dev(dev);
> + unsigned long num;
> + int ret = 0;
> +
> + num = simple_strtoul(buf, NULL, 10);
kstrtoul is preferred these days I think, it also catches errors.
> +
> + mutex_lock(&fb->info->lock);
> + if (num)
> + ret = fblog_open(fb);
> + else
> + fblog_close(fb, false);
> + mutex_unlock(&fb->info->lock);
> +
> + return ret ? ret : count;
Nitpick, you can use gcc's shortcut form of the ? operator here:
return ret ?: count;
> +}
> +
> +static DEVICE_ATTR(active, S_IRUGO | S_IWUSR | S_IWGRP, fblog_dev_active_show,
> + fblog_dev_active_store);
> +
> /*
> * fblog framebuffer list
> * The fblog_fbs[] array contains all currently registered framebuffers. If a
> @@ -148,6 +188,7 @@ static void fblog_do_unregister(struct fb_info *info)
> fblog_fbs[info->node] = NULL;
>
> fblog_close(fb, true);
> + device_remove_file(&fb->dev, &dev_attr_active);
> device_del(&fb->dev);
> put_device(&fb->dev);
> }
> @@ -156,6 +197,7 @@ static void fblog_do_register(struct fb_info *info, bool force)
> {
> struct fblog_fb *fb;
> int ret;
> + bool do_open = true;
>
> fb = fblog_fbs[info->node];
> if (fb && fb->info != info) {
> @@ -186,7 +228,20 @@ static void fblog_do_register(struct fb_info *info, bool force)
> return;
> }
>
> - fblog_open(fb);
> + ret = device_create_file(&fb->dev, &dev_attr_active);
> + if (ret) {
> + pr_err("fblog: cannot create sysfs entry");
Shouldn't need the "fblog: " prefix, since you have pr_fmt defined.
> + /* do not open fb if we cannot create control file */
> + do_open = false;
> + }
> +
> + if (!activate_on_hotplug)
> + do_open = false;
> + if (main_only && info->node != 0)
> + do_open = false;
> +
> + if (do_open)
> + fblog_open(fb);
> }
>
> static void fblog_register(struct fb_info *info, bool force)
> @@ -321,6 +376,15 @@ static void __exit fblog_exit(void)
> }
> }
>
> +module_param(active, bool, S_IRUGO | S_IWUSR | S_IWGRP);
> +MODULE_PARM_DESC(active, "Activate fblog rendering");
> +
> +module_param(activate_on_hotplug, bool, S_IRUGO | S_IWUSR | S_IWGRP);
> +MODULE_PARM_DESC(activate_on_hotplug, "Activate fblog on hotplugged devices");
> +
> +module_param(main_only, bool, S_IRUGO);
> +MODULE_PARM_DESC(main_only, "Activate fblog by default only on main devices");
> +
> module_init(fblog_init);
> module_exit(fblog_exit);
> MODULE_LICENSE("GPL");
>
^ permalink raw reply
* RE: [PATCH v4] da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-08-13 3:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344440145-27117-1-git-send-email-prakash.pm@ti.com>
Hi Sergei,
Thanks for reviewing the patch.
On Wed, Aug 08, 2012 at 21:44:55, Sergei Shtylyov wrote:
> Hello.
>
> On 08-08-2012 19:35, Manjunathappa, Prakash wrote:
>
> > LCD controller on am335x supports 24bpp raster configuration in addition
> > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > component of the data.
>
> > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> [...]
>
> > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > index 7ae9d53..1abcfa9 100644
> > --- a/drivers/video/da8xx-fb.c
> > +++ b/drivers/video/da8xx-fb.c
> [...]
> > @@ -499,6 +501,9 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > {
> > u32 reg;
> >
> > + if ((bpp > 16) && (lcd_revision = LCD_VERSION_1))
>
> Parens around operands of && not necessary.
>
I agree it is not necessary, isn't it more readable?
> > @@ -542,6 +547,12 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > reg = lcdc_read(LCD_RASTER_CTRL_REG) & ~(1 << 8);
> > if (raster_order)
> > reg |= LCD_RASTER_ORDER;
> > +
> > + if (bpp = 24)
> > + reg |= LCD_V2_TFT_24BPP_MODE;
> > + else if (bpp = 32)
> > + reg |= (LCD_V2_TFT_24BPP_MODE | LCD_V2_TFT_24BPP_UNPACK);
> > +
>
> This asks to be a *switch* statement.
>
I will move it to *switch* statement below.
> > lcdc_write(reg, LCD_RASTER_CTRL_REG);
> >
> > switch (bpp) {
> > @@ -549,6 +560,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > case 2:
> > case 4:
> > case 16:
> > + case 24:
> > + case 32:
> > par->palette_sz = 16 * 2;
> > break;
> >
> > @@ -578,13 +591,36 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> > if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
> > return 1;
> >
> > - if (info->var.bits_per_pixel = 4) {
> > - if (regno > 15)
> > - return 1;
> > + if ((info->var.bits_per_pixel > 16) && (lcd_revision = LCD_VERSION_1))
>
> Parens around operands of && not necessary.
>
Same as above, can I retain it for purpose of readability?
> > + switch (info->fix.visual) {
> > + case FB_VISUAL_TRUECOLOR:
> > + red = CNVT_TOHW(red, info->var.red.length);
> > + green = CNVT_TOHW(green, info->var.green.length);
> > + blue = CNVT_TOHW(blue, info->var.blue.length);
> > + break;
> > + case FB_VISUAL_PSEUDOCOLOR:
> > + if (info->var.bits_per_pixel = 4) {
> > + if (regno > 15)
> > + return -EINVAL;
> > +
> > + if (info->var.grayscale) {
> > + pal = regno;
> > + } else {
> > + red >>= 4;
> > + green >>= 8;
> > + blue >>= 12;
> > +
> > + pal = (red & 0x0f00);
> > + pal |= (green & 0x00f0);
> > + pal |= (blue & 0x000f);
>
> Parens not needed.
>
Will remove parens around.
> > + }
> > + if (regno = 0)
> > + pal |= 0x2000;
> > + palette[regno] = pal;
> > +
> > + } else if (info->var.bits_per_pixel = 8) {
>
> This asks to be a *switch* statement.
>
Agreed, I will replace with *switch* statement.
Thanks,
Prakash
> > @@ -842,6 +877,9 @@ static int fb_check_var(struct fb_var_screeninfo *var,
> > {
> > int err = 0;
[...]
^ permalink raw reply
* RE: [PATCH] video: move CNVT_TOHW to a common place
From: Manjunathappa, Prakash @ 2012-08-13 5:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344426866-8789-1-git-send-email-prakash.pm@ti.com>
SGkgR2VlcnQgVXl0dGVyaG9ldmVuLA0KDQpUaGFua3MgZm9yIHJldmlld2luZyB0aGUgcGF0Y2gu
DQoNCk9uIFdlZCwgQXVnIDA4LCAyMDEyIGF0IDIyOjMyOjUxLCBHZWVydCBVeXR0ZXJob2V2ZW4g
d3JvdGU6DQo+IE9uIFdlZCwgQXVnIDgsIDIwMTIgYXQgMTo1NCBQTSwgTWFuanVuYXRoYXBwYSwg
UHJha2FzaA0KPiA8cHJha2FzaC5wbUB0aS5jb20+IHdyb3RlOg0KPiA+IC0tLSBhL2luY2x1ZGUv
bGludXgvZmIuaA0KPiA+ICsrKyBiL2luY2x1ZGUvbGludXgvZmIuaA0KPiA+IEBAIC0zOTcsNiAr
Mzk3LDExIEBAIHN0cnVjdCBmYl9jdXJzb3Igew0KPiA+ICAgICAgICAgc3RydWN0IGZiX2ltYWdl
IGltYWdlOyAgLyogQ3Vyc29yIGltYWdlICovDQo+ID4gIH07DQo+ID4NCj4gPiArc3RhdGljIGlu
bGluZSBfX3UzMiBDTlZUX1RPSFcoX191MzIgdmFsLCBfX3UzMiB3aWR0aCkNCj4gDQo+ICJfX3Uz
MiIgaXMgYSBiaXQgbWlzbGVhZGluZywgYXMgInZhbCIgaXMgYWN0dWFsbHkgYW4gdW5zaWduZWQg
MTYtYml0IHZhbHVlLg0KPiANCg0KSSBhZ3JlZSwgd2lsbCBjb3JyZWN0IHRoaXMuDQoNCj4gPiAr
ew0KPiA+ICsgICAgICAgcmV0dXJuICgodmFsIDw8IHdpZHRoKSArIDB4N0ZGRiAtIHZhbCkgPj4g
MTY7DQo+ID4gK30NCj4gDQo+IEFsc28sIGlmIHlvdSBtb3ZlIGl0IHRvIGEgcHVibGljIGhlYWRl
ciwgcGVyaGFwcyB0aGUgZnVuY3Rpb24gbmVlZHMgYSBtb3JlDQo+IGRlc2NyaXB0aXZlIG5hbWU/
IFdoYXQgaXQgZG9lcyBpcyByZW1hcCBhIDE2LWJpdCB2YWx1ZSAidmFsIiBvbnRvIGEgdmFsdWUg
d2l0aA0KPiAid2lkdGgiIGJpdHMgb2YgcmVzb2x1dGlvbiwgd2hlcmUgIndpZHRoIiBtdXN0IGJl
IHNtYWxsZXIgdGhhbiAxNi4NCj4gInJlZHVjZV9yZXNvbHV0aW9uKCkiPw0KPiANCg0KSSBhZ3Jl
ZSB0aGF0IGl0IHJlZHVjZXMgdGhlIHJlc29sdXRpb24sIGJ1dCBpdCByZWR1Y2VzIHRvIG1hdGNo
IHRoZSBoYXJkd2FyZS4NClNvICJDTlZUX1RPSFciOiBjb252ZXJ0IHRvIGhhcmR3YXJlIHN1cHBv
cnRlZCByZXNvbHV0aW9uLCBpc24ndCBpdCBtb3JlIGFwcHJvcHJpYXRlPw0KDQpJIHdpbGwgYWRk
IGRlc2NyaXB0aW9uIGFzIGl0IGlzIGdvaW5nIHRvIHB1YmxpYyBoZWFkZXIgZmlsZS4NCg0KPiBB
bHNvLCBpdCBjYW4gYmVjb21lIG1vcmUgZ2VuZXJpYywgbm90IGxpbWl0aW5nIHRvIDE2LWJpdCBp
bnB1dCB2YWx1ZXMuDQo+IFRoZSAiMHg3ZmZmIiBwYXJ0IGlzICIoMSA8PCAod2lkdGggLTEpKSAt
IDEiLiBCdXQgSSdtIG5vdCBzdXJlIGl0J3MNCj4gd29ydGggdGhlIGVmZm9ydA0KPiAoYXJlIHRo
ZXJlIHBsYWNlcyB3aGVyZSB0aGlzIGlzIG5lZWRlZD8pLg0KPiANCg0KRmxvcmlhbiBUb2JpYXMg
U2NoYW5kaW5hdDogQ291bGQgeW91IHBsZWFzZSBjb21tZW50IGhlcmU/DQoNClRoYW5rcywNClBy
YWthc2gNCg0KPiBHcntvZXRqZSxlZXRpbmd9cywNCj4gDQo+ICAgICAgICAgICAgICAgICAgICAg
ICAgIEdlZXJ0DQo+IA0KPiAtLQ0KPiBHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhlcmUncyBsb3Rz
IG9mIExpbnV4IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQo+IA0KPiBJbiBw
ZXJzb25hbCBjb252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBjYWxsIG15c2Vs
ZiBhIGhhY2tlci4gQnV0DQo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlzdHMgSSBqdXN0
IHNheSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5nIGxpa2UgdGhhdC4NCj4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0KPiANCg0K
^ permalink raw reply
* [PATCH] video/mx3fb: remove stray l from debug output
From: Uwe Kleine-König @ 2012-08-13 7:44 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/video/mx3fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index c89f8a8..1dfdeb1 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -771,7 +771,7 @@ static int __set_par(struct fb_info *fbi, bool lock)
if (fbi->var.sync & FB_SYNC_SHARP_MODE)
mode = IPU_PANEL_SHARP_TFT;
- dev_dbg(fbi->device, "pixclock = %ul Hz\n",
+ dev_dbg(fbi->device, "pixclock = %u Hz\n",
(u32) (PICOS2KHZ(fbi->var.pixclock) * 1000UL));
if (sdc_init_panel(mx3fb, mode,
--
1.7.10.4
^ permalink raw reply related
* [PATCH] video:uvesafb: check the return value of kzalloc
From: Wang YanQing @ 2012-08-13 10:02 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, spock
Michal maybe forgot it merely, we should add code
to check the return value of kzalloc to make the
code more robust.
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
drivers/video/uvesafb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index b0e2a42..2f8f82d 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -659,6 +659,8 @@ static int __devinit uvesafb_vbe_getedid(struct uvesafb_ktask *task,
task->t.flags = TF_BUF_RET | TF_BUF_ESDI;
task->t.buf_len = EDID_LENGTH;
task->buf = kzalloc(EDID_LENGTH, GFP_KERNEL);
+ if (!task->buf)
+ return -ENOMEM;
err = uvesafb_exec(task);
--
1.7.11.1.116.g8228a23
^ permalink raw reply related
* [PATCH 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-13 12:10 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1344425874-28222-1-git-send-email-cmahapatra@ti.com>
All the cpu_is checks have been moved to dispc_init_features function providing
a much more generic and cleaner interface. The OMAP version and revision
specific functions and data are initialized by dispc_features structure which is
local to dispc.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 428 +++++++++++++++++++++++++--------------
1 file changed, 273 insertions(+), 155 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..11ca3f9 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -81,6 +81,23 @@ struct dispc_irq_stats {
unsigned irqs[32];
};
+struct dispc_features {
+ int hp_max;
+ int vp_max;
+ int sw_max;
+ int sw_start;
+ int fp_start;
+ int bp_start;
+ int (*calc_scaling) (enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk);
+ unsigned long (*calc_core_clk) (enum omap_channel channel,
+ u16 width, u16 height, u16 out_width, u16 out_height);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -101,6 +118,8 @@ static struct {
bool ctx_valid;
u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
+ const struct dispc_features *feat;
+
#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
spinlock_t irq_stats_lock;
struct dispc_irq_stats irq_stats;
@@ -1939,7 +1958,18 @@ static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
return core_clk;
}
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (height > out_height && width > out_width)
+ return pclk * 4;
+ else
+ return pclk * 2;
+}
+
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
u16 height, u16 out_width, u16 out_height)
{
unsigned int hf, vf;
@@ -1958,25 +1988,163 @@ static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
hf = 2;
else
hf = 1;
-
if (height > out_height)
vf = 2;
else
vf = 1;
- if (cpu_is_omap24xx()) {
- if (vf > 1 && hf > 1)
- return pclk * 4;
- else
- return pclk * 2;
- } else if (cpu_is_omap34xx()) {
- return pclk * vf * hf;
- } else {
- if (hf > 1)
- return DIV_ROUND_UP(pclk, out_width) * width;
- else
- return pclk;
+ return pclk * vf * hf;
+}
+
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (width > out_width)
+ return DIV_ROUND_UP(pclk, out_width) * width;
+ else
+ return pclk;
+}
+
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+ *five_taps = false;
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+ error = (in_width > maxsinglelinewidth || !*core_clk ||
+ *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale max input width exceeded");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+ in_width, in_height, out_width, out_height, color_mode);
+
+ error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+ in_width, in_height, out_width, out_height);
+
+ if (in_width > maxsinglelinewidth)
+ if (in_height > out_height &&
+ in_height < out_height * 2)
+ *five_taps = false;
+ if (!*five_taps)
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+
+ error = (error || in_width > maxsinglelinewidth * 2 ||
+ (in_width > maxsinglelinewidth && *five_taps) ||
+ !*core_clk || *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+ out_width, out_height)){
+ DSSERR("horizontal timing too tight\n");
+ return -EINVAL;
}
+
+ if (in_width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+
+ if (in_width > maxsinglelinewidth && *five_taps) {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ u16 in_width, in_width_max;
+ int decim_x_min = *decim_x;
+ u16 in_height = DIV_ROUND_UP(height, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ in_width_max = dispc_core_clk_rate() /
+ DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+ *decim_x = DIV_ROUND_UP(width, in_width_max);
+
+ *decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+ if (*decim_x > *x_predecim)
+ return -EINVAL;
+
+ do {
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ } while (*decim_x <= *x_predecim &&
+ in_width > maxsinglelinewidth && ++*decim_x);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
+ }
+
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width, in_height,
+ out_width, out_height);
+ return 0;
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2156,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
- const int maxsinglelinewidth - dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
const int max_decim_limit = 16;
unsigned long core_clk = 0;
- int decim_x, decim_y, error, min_factor;
- u16 in_width, in_height, in_width_max = 0;
+ int decim_x, decim_y, ret;
if (width = out_width && height = out_height)
return 0;
@@ -2017,118 +2182,17 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
- min_factor = min(decim_x, decim_y);
-
if (decim_x > *x_predecim || out_width > width * 8)
return -EINVAL;
if (decim_y > *y_predecim || out_height > height * 8)
return -EINVAL;
- if (cpu_is_omap24xx()) {
- *five_taps = false;
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- error = (in_width > maxsinglelinewidth || !core_clk ||
- core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
- error);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale max input width exceeded");
- return -EINVAL;
- }
- } else if (cpu_is_omap34xx()) {
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk_five_taps(channel, mgr_timings,
- in_width, in_height, out_width, out_height,
- color_mode);
-
- error = check_horiz_timing_omap3(channel, mgr_timings,
- pos_x, in_width, in_height, out_width,
- out_height);
-
- if (in_width > maxsinglelinewidth)
- if (in_height > out_height &&
- in_height < out_height * 2)
- *five_taps = false;
- if (!*five_taps)
- core_clk = calc_core_clk(channel, in_width,
- in_height, out_width, out_height);
- error = (error || in_width > maxsinglelinewidth * 2 ||
- (in_width > maxsinglelinewidth && *five_taps) ||
- !core_clk || core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim
- && error);
-
- if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
- height, out_width, out_height)){
- DSSERR("horizontal timing too tight\n");
- return -EINVAL;
- }
-
- if (in_width > (maxsinglelinewidth * 2)) {
- DSSERR("Cannot setup scaling");
- DSSERR("width exceeds maximum width possible");
- return -EINVAL;
- }
-
- if (in_width > maxsinglelinewidth && *five_taps) {
- DSSERR("cannot setup scaling with five taps");
- return -EINVAL;
- }
- } else {
- int decim_x_min = decim_x;
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width_max = dispc_core_clk_rate() /
- DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
- out_width);
- decim_x = DIV_ROUND_UP(width, in_width_max);
-
- decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
- if (decim_x > *x_predecim)
- return -EINVAL;
-
- do {
- in_width = DIV_ROUND_UP(width, decim_x);
- } while (decim_x <= *x_predecim &&
- in_width > maxsinglelinewidth && decim_x++);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale width exceeds max line width");
- return -EINVAL;
- }
-
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- }
+ ret = dispc.feat->calc_scaling(channel, mgr_timings, width, height,
+ out_width, out_height, color_mode, five_taps, x_predecim,
+ y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+ if (ret)
+ return ret;
DSSDBG("required core clk rate = %lu Hz\n", core_clk);
DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2604,24 +2668,13 @@ static bool _dispc_mgr_size_ok(u16 width, u16 height)
static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
int vsw, int vfp, int vbp)
{
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- if (hsw < 1 || hsw > 64 ||
- hfp < 1 || hfp > 256 ||
- hbp < 1 || hbp > 256 ||
- vsw < 1 || vsw > 64 ||
- vfp < 0 || vfp > 255 ||
- vbp < 0 || vbp > 255)
- return false;
- } else {
- if (hsw < 1 || hsw > 256 ||
- hfp < 1 || hfp > 4096 ||
- hbp < 1 || hbp > 4096 ||
- vsw < 1 || vsw > 256 ||
- vfp < 0 || vfp > 4095 ||
- vbp < 0 || vbp > 4095)
- return false;
- }
-
+ if (hsw < 1 || hsw > dispc.feat->sw_max ||
+ hfp < 1 || hfp > dispc.feat->hp_max ||
+ hbp < 1 || hbp > dispc.feat->hp_max ||
+ vsw < 1 || vsw > dispc.feat->sw_max ||
+ vfp < 0 || vfp > dispc.feat->vp_max ||
+ vbp < 0 || vbp > dispc.feat->vp_max)
+ return false;
return true;
}
@@ -2653,19 +2706,12 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
u32 timing_h, timing_v, l;
bool onoff, rf, ipc;
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
- FLD_VAL(hbp-1, 27, 20);
-
- timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
- FLD_VAL(vbp, 27, 20);
- } else {
- timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
- FLD_VAL(hbp-1, 31, 20);
-
- timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
- FLD_VAL(vbp, 31, 20);
- }
+ timing_h = FLD_VAL(hsw-1, dispc.feat->sw_start, 0) |
+ FLD_VAL(hfp-1, dispc.feat->fp_start, 8) |
+ FLD_VAL(hbp-1, dispc.feat->bp_start, 20);
+ timing_v = FLD_VAL(vsw-1, dispc.feat->sw_start, 0) |
+ FLD_VAL(vfp, dispc.feat->fp_start, 8) |
+ FLD_VAL(vbp, dispc.feat->bp_start, 20);
dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
@@ -3671,6 +3717,72 @@ static void _omap_dispc_initial_config(void)
dispc_ovl_enable_zorder_planes();
}
+static const struct __initdata dispc_features omap2_dispc_features = {
+ .hp_max = 256,
+ .vp_max = 255,
+ .sw_max = 64,
+ .sw_start = 5,
+ .fp_start = 15,
+ .bp_start = 27,
+ .calc_scaling = dispc_ovl_calc_scaling_24xx,
+ .calc_core_clk = calc_core_clk_24xx,
+};
+
+static const struct __initdata dispc_features omap3_2_1_dispc_features = {
+ .hp_max = 256,
+ .vp_max = 255,
+ .sw_max = 64,
+ .sw_start = 5,
+ .fp_start = 15,
+ .bp_start = 27,
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+};
+
+static const struct __initdata dispc_features omap3_3_0_dispc_features = {
+ .hp_max = 4096,
+ .vp_max = 4095,
+ .sw_max = 256,
+ .sw_start = 7,
+ .fp_start = 19,
+ .bp_start = 31,
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+};
+
+static const struct __initdata dispc_features omap4_dispc_features = {
+ .hp_max = 4096,
+ .vp_max = 4095,
+ .sw_max = 256,
+ .sw_start = 7,
+ .fp_start = 19,
+ .bp_start = 31,
+ .calc_scaling = dispc_ovl_calc_scaling_44xx,
+ .calc_core_clk = calc_core_clk_44xx,
+};
+
+static int __init dispc_init_features(struct device *dev)
+{
+ dispc.feat = devm_kzalloc(dev, sizeof(*dispc.feat), GFP_KERNEL);
+ if (!dispc.feat) {
+ dev_err(dev, "Failed to allocate DISPC Features\n");
+ return -ENOMEM;
+ }
+
+ if (cpu_is_omap24xx()) {
+ dispc.feat = &omap2_dispc_features;
+ } else if (cpu_is_omap34xx()) {
+ if (omap_rev() < OMAP3430_REV_ES3_0)
+ dispc.feat = &omap3_2_1_dispc_features;
+ else
+ dispc.feat = &omap3_3_0_dispc_features;
+ } else {
+ dispc.feat = &omap4_dispc_features;
+ }
+
+ return 0;
+}
+
/* DISPC HW IP initialisation */
static int __init omap_dispchw_probe(struct platform_device *pdev)
{
@@ -3725,6 +3837,10 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
dispc.dss_clk = clk;
+ r = dispc_init_features(&dispc.pdev->dev);
+ if (r)
+ return r;
+
pm_runtime_enable(&pdev->dev);
r = dispc_runtime_get();
@@ -3760,6 +3876,8 @@ static int __exit omap_dispchw_remove(struct platform_device *pdev)
clk_put(dispc.dss_clk);
+ devm_kfree(&dispc.pdev->dev, (void *)dispc.feat);
+
return 0;
}
--
1.7.10
^ permalink raw reply related
* [PATCH 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-13 12:11 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1344425899-28267-1-git-send-email-cmahapatra@ti.com>
All the cpu_is checks have been moved to dss_init_features function providing a
much more generic and cleaner interface. The OMAP version and revision specific
initializations in various functions are cleaned and the necessary data are
moved to dss_features structure which is local to dss.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dss.c | 115 ++++++++++++++++++++++++++---------------
1 file changed, 74 insertions(+), 41 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7b1c6ac..6ab236e 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -31,6 +31,7 @@
#include <linux/clk.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/dma-mapping.h>
#include <video/omapdss.h>
@@ -65,6 +66,13 @@ struct dss_reg {
static int dss_runtime_get(void);
static void dss_runtime_put(void);
+struct dss_features {
+ u16 fck_div_max;
+ int factor;
+ char *clk_name;
+ bool (*check_cinfo_fck) (void);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -83,6 +91,8 @@ static struct {
bool ctx_valid;
u32 ctx[DSS_SZ_REGS / sizeof(u32)];
+
+ const struct dss_features *feat;
} dss;
static const char * const dss_generic_clk_source_names[] = {
@@ -91,6 +101,30 @@ static const char * const dss_generic_clk_source_names[] = {
[OMAP_DSS_CLK_SRC_FCK] = "DSS_FCK",
};
+static const struct __initdata dss_features omap2_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .clk_name = NULL,
+};
+
+static const struct __initdata dss_features omap34_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .clk_name = "dpll4_m4_ck",
+};
+
+static const struct __initdata dss_features omap36_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .clk_name = "dpll4_m4_ck",
+};
+
+static const struct __initdata dss_features omap4_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .clk_name = "dpll_per_m5x2_ck",
+};
+
static inline void dss_write_reg(const struct dss_reg idx, u32 val)
{
__raw_writel(val, dss.base + idx.idx);
@@ -236,7 +270,6 @@ const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src)
return dss_generic_clk_source_names[clk_src];
}
-
void dss_dump_clocks(struct seq_file *s)
{
unsigned long dpll4_ck_rate;
@@ -259,18 +292,10 @@ void dss_dump_clocks(struct seq_file *s)
seq_printf(s, "dpll4_ck %lu\n", dpll4_ck_rate);
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- seq_printf(s, "%s (%s) = %lu / %lu = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
- else
- seq_printf(s, "%s (%s) = %lu / %lu * 2 = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
+ seq_printf(s, "%s (%s) = %lu / %lu * %d = %lu\n",
+ fclk_name, fclk_real_name, dpll4_ck_rate,
+ dpll4_ck_rate / dpll4_m4_ck_rate,
+ dss.feat->factor, fclk_rate);
} else {
seq_printf(s, "%s (%s) = %lu\n",
fclk_name, fclk_real_name,
@@ -470,7 +495,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
unsigned long fck, max_dss_fck;
- u16 fck_div, fck_div_max = 16;
+ u16 fck_div;
int match = 0;
int min_fck_per_pck;
@@ -480,9 +505,8 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
fck = clk_get_rate(dss.dss_clk);
- if (req_pck = dss.cache_req_pck &&
- ((cpu_is_omap34xx() && prate = dss.cache_prate) ||
- dss.cache_dss_cinfo.fck = fck)) {
+ if (req_pck = dss.cache_req_pck && prate = dss.cache_prate &&
+ dss.cache_dss_cinfo.fck = fck) {
DSSDBG("dispc clock info found from cache.\n");
*dss_cinfo = dss.cache_dss_cinfo;
*dispc_cinfo = dss.cache_dispc_cinfo;
@@ -519,16 +543,10 @@ retry:
goto found;
} else {
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- fck_div_max = 32;
-
- for (fck_div = fck_div_max; fck_div > 0; --fck_div) {
+ for (fck_div = dss.feat->fck_div_max; fck_div > 0; --fck_div) {
struct dispc_clock_info cur_dispc;
- if (fck_div_max = 32)
- fck = prate / fck_div;
- else
- fck = prate / fck_div * 2;
+ fck = prate / fck_div * dss.feat->factor;
if (fck > max_dss_fck)
continue;
@@ -633,22 +651,11 @@ static int dss_get_clocks(void)
dss.dss_clk = clk;
- if (cpu_is_omap34xx()) {
- clk = clk_get(NULL, "dpll4_m4_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll4_m4_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else if (cpu_is_omap44xx()) {
- clk = clk_get(NULL, "dpll_per_m5x2_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll_per_m5x2_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else { /* omap24xx */
- clk = NULL;
+ clk = clk_get(NULL, dss.feat->clk_name);
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get %s\n", dss.feat->clk_name);
+ r = PTR_ERR(clk);
+ goto err;
}
dss.dpll4_m4_ck = clk;
@@ -704,6 +711,26 @@ void dss_debug_dump_clocks(struct seq_file *s)
}
#endif
+static int __init dss_init_features(struct device *dev)
+{
+ dss.feat = devm_kzalloc(dev, sizeof(*dss.feat), GFP_KERNEL);
+ if (!dss.feat) {
+ dev_err(dev, "Failed to allocate local DSS Features\n");
+ return -ENOMEM;
+ }
+
+ if (cpu_is_omap24xx())
+ dss.feat = &omap2_dss_features;
+ else if (cpu_is_omap34xx())
+ dss.feat = &omap34_dss_features;
+ else if (cpu_is_omap3630())
+ dss.feat = &omap36_dss_features;
+ else
+ dss.feat = &omap4_dss_features;
+
+ return 0;
+}
+
/* DSS HW IP initialisation */
static int __init omap_dsshw_probe(struct platform_device *pdev)
{
@@ -750,6 +777,10 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
+ r = dss_init_features(&dss.pdev->dev);
+ if (r)
+ return r;
+
rev = dss_read_reg(DSS_REVISION);
printk(KERN_INFO "OMAP DSS rev %d.%d\n",
FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
@@ -772,6 +803,8 @@ static int __exit omap_dsshw_remove(struct platform_device *pdev)
dss_put_clocks();
+ devm_kfree(&dss.pdev->dev, (void *)dss.feat);
+
return 0;
}
--
1.7.10
^ permalink raw reply related
* re: s3fb: fix up DDC build with MTRR disabled.
From: Dan Carpenter @ 2012-08-13 16:39 UTC (permalink / raw)
To: linux-fbdev
[Sorry this is reall old. I don't think I've emailed about it before]
Hello Paul Mundt,
This is a semi-automatic email about new static checker warnings.
The patch 4f2970b9f622: "s3fb: fix up DDC build with MTRR disabled."
from Apr 20, 2011, leads to the following Smatch complaint:
drivers/video/s3fb.c:1411 s3_pci_remove()
warn: variable dereferenced before check 'info' (see line 1409)
drivers/video/s3fb.c
1408 struct fb_info *info = pci_get_drvdata(dev);
1409 struct s3fb_info __maybe_unused *par = info->par;
^^^^^^^^^
New dereference.
1410
1411 if (info) {
^^^^
Old check.
1412
1413 #ifdef CONFIG_MTRR
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] video/mx3fb: remove stray l from debug output
From: Geert Uytterhoeven @ 2012-08-13 17:09 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344843875-8174-1-git-send-email-u.kleine-koenig@pengutronix.de>
On Mon, Aug 13, 2012 at 9:44 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/video/mx3fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index c89f8a8..1dfdeb1 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -771,7 +771,7 @@ static int __set_par(struct fb_info *fbi, bool lock)
> if (fbi->var.sync & FB_SYNC_SHARP_MODE)
> mode = IPU_PANEL_SHARP_TFT;
>
> - dev_dbg(fbi->device, "pixclock = %ul Hz\n",
> + dev_dbg(fbi->device, "pixclock = %u Hz\n",
> (u32) (PICOS2KHZ(fbi->var.pixclock) * 1000UL));
I think a better fix is to change the dyslectic "%ul" to "%lu", and
drop the cast to
u32, which was probably only added to kill the compiler warning caused by the
dyslectia issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: 转发: Siliconmotion new kernel driver initial patch
From: Konrad Rzeszutek Wilk @ 2012-08-13 18:03 UTC (permalink / raw)
To: Aaron.Chen 陈俊杰
Cc: linux-fbdev, mill.chen 陈军,
Mandy.Wang 王少媚, dri-devel,
Paul.Chen 陈波, caesar.qiu 裘赛海,
Sunny.Yang 杨光
In-Reply-To: <4CE6A5494DEECD498EBCD4C5488B1AFCD04CD1@CNDC08.cn.smi.ad>
On Mon, Aug 13, 2012 at 04:56:33PM +0800, Aaron.Chen 陈俊杰 wrote:
>
> Since there is no response for the last mail. Maybe it didn't sent successfully. So I send it again. Here is the initial patch for siliconmotion kernel driver.
What's with the #ifdef 0 or #ifdef 1?
Why is there a bunch of ddkxxx something? Can those header files
be squashed together?
>
> Aaron
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 0/7] HID: picoLCD updates
From: Tejun Heo @ 2012-08-13 23:27 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-input, linux-kernel, Jiri Kosina, linux-fbdev
In-Reply-To: <20120809200947.2194e89f@neptune.home>
Hello,
On Thu, Aug 09, 2012 at 08:09:47PM +0200, Bruno Prémont wrote:
> As you are working on workqueues and related code, could you have a look
> at my usage of them in combination with db_defio?
>
> The delayed memory corruptions or system reboots after unbinding/unplugging
> the PicoLCD seem very much related to workqueue used to handle the deferred
> IO to framebuffer.
>
> I think things don't get cleaned-up as they should though I'm not sure
> where the trouble lies.
>
> For ease of reading, I'm inlineing below the framebuffer related code of
> PicoLCD (for complete driver after this patch series apply the whole series
> on top of 3.5 https://lkml.org/lkml/2012/7/30/375 )
...
> void picolcd_exit_framebuffer(struct picolcd_data *data)
> {
> struct fb_info *info = data->fb_info;
> u8 *fb_vbitmap = data->fb_vbitmap;
>
> if (!info)
> return;
>
> device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate);
> info->par = NULL;
> unregister_framebuffer(info);
> data->fb_vbitmap = NULL;
> data->fb_bitmap = NULL;
> data->fb_bpp = 0;
> data->fb_info = NULL;
> kfree(fb_vbitmap);
> }
I'm kinda shooting in the dark but who flushes / cancels
fb_info->deferred_work?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] video: exynos-mipi-dsi: Add missing static storage class specifiers
From: Donghwa Lee @ 2012-08-14 5:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1343888051-406-1-git-send-email-sachin.kamat@linaro.org>
This patch looks good to me.
Acked-by: Donghwa Lee <dh09.lee@samsung.com>
On Thursday, August 02, 2012³â 15:14, Sachin Kamat wrote:
> Fixes the following sparse warnings:
> drivers/video/exynos/exynos_mipi_dsi.c:208:22: warning:
> symbol 'exynos_mipi_dsi_find_lcd_device' was not declared. Should it be static?
> drivers/video/exynos/exynos_mipi_dsi.c:268:22: warning:
> symbol 'exynos_mipi_dsi_bind_lcd_ddi' was not declared. Should it be static?
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/video/exynos/exynos_mipi_dsi.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index 4bc2b8a..ef68228 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -205,7 +205,8 @@ int exynos_mipi_dsi_register_lcd_device(struct mipi_dsim_lcd_device *lcd_dev)
> return 0;
> }
>
> -struct mipi_dsim_ddi *exynos_mipi_dsi_find_lcd_device(struct mipi_dsim_lcd_driver *lcd_drv)
> +static struct mipi_dsim_ddi *exynos_mipi_dsi_find_lcd_device(
> + struct mipi_dsim_lcd_driver *lcd_drv)
> {
> struct mipi_dsim_ddi *dsim_ddi, *next;
> struct mipi_dsim_lcd_device *lcd_dev;
> @@ -265,7 +266,8 @@ int exynos_mipi_dsi_register_lcd_driver(struct mipi_dsim_lcd_driver *lcd_drv)
>
> }
>
> -struct mipi_dsim_ddi *exynos_mipi_dsi_bind_lcd_ddi(struct mipi_dsim_device *dsim,
> +static struct mipi_dsim_ddi *exynos_mipi_dsi_bind_lcd_ddi(
> + struct mipi_dsim_device *dsim,
> const char *name)
> {
> struct mipi_dsim_ddi *dsim_ddi, *next;
^ permalink raw reply
* Re: [PATCH 0/7] HID: picoLCD updates
From: Bruno Prémont @ 2012-08-14 6:30 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-input, linux-kernel, Jiri Kosina, linux-fbdev
In-Reply-To: <20120813232708.GF25632@google.com>
Hello Tejun,
[Tejun: sorry for duplicate, did hit "reply" instead of "reply to all"]
On Mon, 13 Aug 2012 16:27:08 Tejun Heo <tj@kernel.org> wrote:
> On Thu, Aug 09, 2012 at 08:09:47PM +0200, Bruno Prémont wrote:
> > As you are working on workqueues and related code, could you have a look
> > at my usage of them in combination with db_defio?
> >
> > The delayed memory corruptions or system reboots after unbinding/unplugging
> > the PicoLCD seem very much related to workqueue used to handle the deferred
> > IO to framebuffer.
> >
> > I think things don't get cleaned-up as they should though I'm not sure
> > where the trouble lies.
> >
> > For ease of reading, I'm inlineing below the framebuffer related code of
> > PicoLCD (for complete driver after this patch series apply the whole series
> > on top of 3.5 https://lkml.org/lkml/2012/7/30/375 )
> ...
> > void picolcd_exit_framebuffer(struct picolcd_data *data)
> > {
> > struct fb_info *info = data->fb_info;
> > u8 *fb_vbitmap = data->fb_vbitmap;
> >
> > if (!info)
> > return;
> >
> > device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate);
> > info->par = NULL;
> > unregister_framebuffer(info);
> > data->fb_vbitmap = NULL;
> > data->fb_bitmap = NULL;
> > data->fb_bpp = 0;
> > data->fb_info = NULL;
> > kfree(fb_vbitmap);
> > }
>
> I'm kinda shooting in the dark but who flushes / cancels
> fb_info->deferred_work?
fb_deferred_io_cleanup() does so and is called by destroy fbops
(when last reference to fb_info is returned):
static void picolcd_fb_destroy(struct fb_info *info)
{
/* make sure no work is deferred */
fb_deferred_io_cleanup(info);
vfree((u8 *)info->fix.smem_start);
framebuffer_release(info);
printk(KERN_DEBUG "picolcd_fb_destroy(%p)\n", info);
}
=== drivers/video/fb_defio.c ==void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct page *page;
int i;
BUG_ON(!fbdefio);
cancel_delayed_work_sync(&info->deferred_work);
/* clear out the mapping that we setup */
for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
page = fb_deferred_io_page(info, i);
page->mapping = NULL;
}
info->fbops->fb_mmap = NULL;
mutex_destroy(&fbdefio->lock);
}
Thanks,
Bruno
^ permalink raw reply
* RE: [PATCH v3] video: da8xx-fb: add 24bpp LCD configuration support
From: Manjunathappa, Prakash @ 2012-08-14 6:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1342712266-4381-1-git-send-email-prakash.pm@ti.com>
Hi,
On Mon, Jul 23, 2012 at 13:56:42, Manjunathappa, Prakash wrote:
> Hi,
>
> On Thu, Jul 19, 2012 at 21:07:46, Manjunathappa, Prakash wrote:
> > LCD controller on am335x supports 24bpp raster configuration in addition
> > to ones on da850. LCDC also supports 24bpp in unpacked format having
> > ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
> > component of the data.
> >
> > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> > ---
[...]
> > @@ -546,6 +559,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
> > return 0;
> > }
> >
> > +
> > +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
>
> since multiple FB drivers have re-defined this macro, I will move this to common place(linux/fb.h) and
> convert it as inline function.
>
This macro has lot of variations in various hardware and as I do not have bigger picture to address them
all, so for now I will be dropping the attempt of moving it to common place. This will be taken up later
point of time.
Thanks,
Prakash
>
> > static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> [...]
>
^ permalink raw reply
* [PATCH RESEND] video/udlfb: fix line counting in fb_write
From: Alexander Holler @ 2012-08-14 7:11 UTC (permalink / raw)
To: linux-kernel; +Cc: Bernie Thompson, Alexander Holler, linux-fbdev
Line 0 and 1 were both written to line 0 (on the display) and all subsequent
lines had an offset of -1. The result was that the last line on the display
was never overwritten by writes to /dev/fbN.
Cc: stable@vger.kernel.org
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
drivers/video/udlfb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index a159b63..85d8110 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -647,7 +647,7 @@ static ssize_t dlfb_ops_write(struct fb_info *info,
const char __user *buf,
result = fb_sys_write(info, buf, count, ppos);
if (result > 0) {
- int start = max((int)(offset / info->fix.line_length) - 1, 0);
+ int start = max((int)(offset / info->fix.line_length), 0);
int lines = min((u32)((result / info->fix.line_length) + 1),
(u32)info->var.yres);
-- 1.7.6.5
^ 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