* Re: [PATCH v2] video: fbdev: fix sys_copyarea
From: Måns Rullgård @ 2015-01-30 8:49 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel
In-Reply-To: <54CB38B6.3010202@ti.com>
Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
> On 22/01/15 03:19, Mans Rullgard wrote:
>> The sys_copyarea() function performs the same operation as
>> cfb_copyarea() but using normal memory access instead of I/O
>> accessors. Since the introduction of sys_copyarea(), there
>> have been two fixes to cfb_copyarea():
>>
>> - 00a9d699 ("framebuffer: fix cfb_copyarea")
>> - 5b789da8 ("framebuffer: fix screen corruption when copying")
>>
>> This patch incorporates the fixes into sys_copyarea() as well.
>>
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>> Changed in v2:
>> - Fixed wrong first/last calculation in bitcpy_rev()
>> ---
>> drivers/video/fbdev/core/syscopyarea.c | 137 ++++++++++++++++-----------------
>> 1 file changed, 65 insertions(+), 72 deletions(-)
>
> Thanks, queued for 3.20.
>
> This makes me wonder, though, if the sys and cfb versions could be
> somehow combined...
I would have done it by #including a common template with different
macro definitions for the memory access operations.
--
Måns Rullgård
mans@mansr.com
^ permalink raw reply
* [PATCH v5 2/2] fbcon: expose cursor blink interval via sysfs
From: Scot Doyle @ 2015-01-30 8:44 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: Geert Uytterhoeven, Richard Weinberger, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1501300836140.2305@localhost.localdomain>
fbcon toggles cursor display state every 200 milliseconds when blinking.
Since users prefer different toggle intervals, expose the interval via
/sys/class/graphics/fbcon/cursor_blink_ms so that it may be customized.
Values written to the interface set the approximate time interval in
milliseconds between cursor toggles, from 1 to 32767. Since the interval
is stored internally as a number of jiffies, the millisecond value read
from the interface may not exactly match the entered value.
An outstanding blink timer is reset after a new value is entered.
If the cursor blink is disabled, either via the 'cursor_blink' boolean
setting or some other mechanism, the 'cursor_blink_ms' setting may still
be modified. The new value will be used if the blink is reactivated.
Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
---
drivers/video/console/fbcon.c | 66 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 7a2030b..f026f65 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -3495,11 +3495,77 @@ err:
return count;
}
+static ssize_t show_cursor_blink_ms(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ struct fbcon_ops *ops;
+ int idx, ms = -1;
+
+ if (fbcon_has_exited)
+ return -ENODEV;
+
+ console_lock();
+ idx = con2fb_map[fg_console];
+
+ if (idx != -1 && registered_fb[idx] != NULL) {
+ ops = ((struct fb_info *)registered_fb[idx])->fbcon_par;
+ if (ops != NULL)
+ ms = jiffies_to_msecs(ops->blink_jiffies);
+ }
+
+ console_unlock();
+ return ms < 0 ? -ENODEV : scnprintf(buf, PAGE_SIZE, "%d\n", ms);
+}
+
+static ssize_t store_cursor_blink_ms(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fb_info *info;
+ struct fbcon_ops *ops;
+ int idx;
+ short ms;
+ int r = -ENODEV;
+
+ if (fbcon_has_exited)
+ return r;
+
+ console_lock();
+ idx = con2fb_map[fg_console];
+
+ if (idx = -1 || registered_fb[idx] = NULL)
+ goto err;
+
+ info = registered_fb[idx];
+ ops = info->fbcon_par;
+
+ if (ops = NULL)
+ goto err;
+
+ if (!kstrtos16(buf, 0, &ms) && ms > 0) {
+ //ops->blink_jiffies = max_t(int, msecs_to_jiffies(ms), 1);
+ ops->blink_jiffies = msecs_to_jiffies(ms);
+ if (info->queue.func = fb_flashcursor &&
+ ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
+ fbcon_del_cursor_timer(info);
+ fbcon_add_cursor_timer(info);
+ }
+ r = count;
+ } else
+ r = -EINVAL;
+
+err:
+ console_unlock();
+ return r;
+}
+
static struct device_attribute device_attrs[] = {
__ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
__ATTR(rotate_all, S_IWUSR, NULL, store_rotate_all),
__ATTR(cursor_blink, S_IRUGO|S_IWUSR, show_cursor_blink,
store_cursor_blink),
+ __ATTR(cursor_blink_ms, S_IRUGO|S_IWUSR, show_cursor_blink_ms,
+ store_cursor_blink_ms),
};
static int fbcon_init_device(void)
--
2.1.4
^ permalink raw reply related
* [PATCH v5 1/2] fbcon: store cursor blink interval in fbcon_ops
From: Scot Doyle @ 2015-01-30 8:43 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: Geert Uytterhoeven, Richard Weinberger, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1501300836140.2305@localhost.localdomain>
fbcon toggles cursor display state every 200 milliseconds when blinking.
Since users prefer different toggle intervals, prepare to expose the
interval via sysfs by moving it to fbdev_ops and setting the default
to 200 milliseconds.
Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
---
drivers/video/console/fbcon.c | 5 +++--
drivers/video/console/fbcon.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index ea43724..7a2030b 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -405,7 +405,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
struct fbcon_ops *ops = info->fbcon_par;
queue_work(system_power_efficient_wq, &info->queue);
- mod_timer(&ops->cursor_timer, jiffies + HZ/5);
+ mod_timer(&ops->cursor_timer, jiffies + ops->blink_jiffies);
}
static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -420,7 +420,7 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
init_timer(&ops->cursor_timer);
ops->cursor_timer.function = cursor_timer_handler;
- ops->cursor_timer.expires = jiffies + HZ / 5;
+ ops->cursor_timer.expires = jiffies + ops->blink_jiffies;
ops->cursor_timer.data = (unsigned long ) info;
add_timer(&ops->cursor_timer);
ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
@@ -959,6 +959,7 @@ static const char *fbcon_startup(void)
ops->currcon = -1;
ops->graphics = 1;
ops->cur_rotate = -1;
+ ops->blink_jiffies = msecs_to_jiffies(200);
info->fbcon_par = ops;
p->con_rotate = initial_rotation;
set_blitting_type(vc, info);
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..642c4e7 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -70,6 +70,7 @@ struct fbcon_ops {
struct fb_cursor cursor_state;
struct display *p;
int currcon; /* Current VC. */
+ int blink_jiffies;
int cursor_flash;
int cursor_reset;
int blank_state;
--
2.1.4
^ permalink raw reply related
* [PATCH v5 0/2] fbcon: user-defined cursor blink interval
From: Scot Doyle @ 2015-01-30 8:40 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: Geert Uytterhoeven, Richard Weinberger, linux-fbdev, linux-kernel
Since users prefer different fbcon cursor blink intervals, allow the
interval to be set via sysfs. The current interval of 200 milliseconds
is retained as the default. Tested with intelfb.
v2: Use kstrtos16() instead of kstrtoul() and min_t() as suggested by
Geert Uytterhoeven
v3: Add error messages as suggested by Tomi Valkeinen
v4: Add rationale into the patches as suggested by Richard Weinberger
v5: Return error codes instead of logging error messages (my mistake)
Scot Doyle (2):
fbcon: store cursor blink interval in fbcon_ops
fbcon: expose cursor blink interval via sysfs
drivers/video/console/fbcon.c | 71 +++++++++++++++++++++++++++++++++++++++++--
drivers/video/console/fbcon.h | 1 +
2 files changed, 70 insertions(+), 2 deletions(-)
--
2.1.4
^ permalink raw reply
* Re: [PATCH v4 0/2] fbcon: user-defined cursor blink interval
From: Tomi Valkeinen @ 2015-01-30 8:02 UTC (permalink / raw)
To: Scot Doyle, Jean-Christophe Plagniol-Villard
Cc: Geert Uytterhoeven, Richard Weinberger, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1501262305150.2783@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 583 bytes --]
On 27/01/15 01:09, Scot Doyle wrote:
> Since users prefer different fbcon cursor blink intervals, allow the
> interval to be set via sysfs. The current interval of 200 milliseconds
> is retained as the default. Tested with intelfb.
>
> v2: Use kstrtos16() instead of kstrtoul() and min_t() as suggested by
> Geert Uytterhoeven
> v3: Add error messages as suggested by Tomi Valkeinen
When I said "return an error" I did not mean "print an error message".
You should return an error code from the show and store functions when
something went wrong.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] omapfb: Return error code when applying overlay settings fails
From: Peter Meerwald @ 2015-01-30 7:59 UTC (permalink / raw)
To: linux-omap; +Cc: tomi.valkeinen, linux-fbdev, Peter Meerwald
the check of the return code is missing, user space does not get notified
about the error condition:
omapdss OVERLAY error: overlay 2 horizontally not inside the display area (403 + 800 >= 800)
omapdss APPLY error: failed to apply settings: illegal configuration.
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
index 146b6f5..9ddfdd6 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
@@ -137,8 +137,11 @@ static int omapfb_setup_plane(struct fb_info *fbi, struct omapfb_plane_info *pi)
goto undo;
}
- if (ovl->manager)
- ovl->manager->apply(ovl->manager);
+ if (ovl->manager) {
+ r = ovl->manager->apply(ovl->manager);
+ if (r)
+ goto undo;
+ }
if (pi->enabled) {
r = ovl->enable(ovl);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2] video: fbdev: fix sys_copyarea
From: Tomi Valkeinen @ 2015-01-30 7:54 UTC (permalink / raw)
To: Mans Rullgard, Jean-Christophe Plagniol-Villard, linux-fbdev,
linux-kernel
In-Reply-To: <1421889589-16362-1-git-send-email-mans@mansr.com>
[-- Attachment #1: Type: text/plain, Size: 871 bytes --]
On 22/01/15 03:19, Mans Rullgard wrote:
> The sys_copyarea() function performs the same operation as
> cfb_copyarea() but using normal memory access instead of I/O
> accessors. Since the introduction of sys_copyarea(), there
> have been two fixes to cfb_copyarea():
>
> - 00a9d699 ("framebuffer: fix cfb_copyarea")
> - 5b789da8 ("framebuffer: fix screen corruption when copying")
>
> This patch incorporates the fixes into sys_copyarea() as well.
>
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> Changed in v2:
> - Fixed wrong first/last calculation in bitcpy_rev()
> ---
> drivers/video/fbdev/core/syscopyarea.c | 137 ++++++++++++++++-----------------
> 1 file changed, 65 insertions(+), 72 deletions(-)
Thanks, queued for 3.20.
This makes me wonder, though, if the sys and cfb versions could be
somehow combined...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video/mmpfb: allow modular build
From: Tomi Valkeinen @ 2015-01-30 7:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8150312.I9dm5QVFgr@wuerfel>
[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]
On 28/01/15 22:13, Arnd Bergmann wrote:
> The frame buffer core can be a module, which means any fb drivers
> should be able to build as modules too. This turns mmpfb into
> a tristate option to allow that and fix a possible randconfig
> build error.
>
> drivers/built-in.o: In function `modes_setup':
> :(.text+0x11b34): undefined reference to `fb_videomode_to_modelist'
> :(.text+0x11b5c): undefined reference to `fb_videomode_to_var'
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/video/fbdev/mmp/Makefile b/drivers/video/fbdev/mmp/Makefile
> index a014cb358bf8..924dd0930cc7 100644
> --- a/drivers/video/fbdev/mmp/Makefile
> +++ b/drivers/video/fbdev/mmp/Makefile
> @@ -1 +1,3 @@
> -obj-y += core.o hw/ panel/ fb/
> +obj-$(CONFIG_MMP_DISP) += mmp_disp.o hw/ panel/ fb/
> +
> +mmp_disp-y += core.o
> diff --git a/drivers/video/fbdev/mmp/fb/Kconfig b/drivers/video/fbdev/mmp/fb/Kconfig
> index 9b0141f105f5..985e1a7cd254 100644
> --- a/drivers/video/fbdev/mmp/fb/Kconfig
> +++ b/drivers/video/fbdev/mmp/fb/Kconfig
> @@ -1,7 +1,7 @@
> if MMP_DISP
>
> config MMP_FB
> - bool "fb driver for Marvell MMP Display Subsystem"
> + tristate "fb driver for Marvell MMP Display Subsystem"
> depends on FB
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
>
Thanks, queued for 3.20.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fb: via: turn gpiolib and i2c selects into dependencies
From: Tomi Valkeinen @ 2015-01-30 7:46 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <3127720.VGEDzudcPs@wuerfel>
[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]
On 28/01/15 22:12, Arnd Bergmann wrote:
> Device driver should not directly select subsystems. In this case
> we get build warnings like
>
> warning: (ARCH_REQUIRE_GPIOLIB && PINCTRL_AT91 && PINCTRL_NOMADIK && MFD_TC6393XB && FB_VIA) selects GPIOLIB which has unmet direct dependencies (ARCH_WANT_OPTIONAL_GPIOLIB || ARCH_REQUIRE_GPIOLIB)
>
> which we can avoid using the normal 'depends on' statement.
>
> Also, this patch makes it possible for DRM drivers to have a dependency
> on GPIOLIB without getting circular Kconfig dependencies.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index f2c3fb7d0399..b3dd417b4719 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1530,13 +1530,11 @@ config FB_SIS_315
>
> config FB_VIA
> tristate "VIA UniChrome (Pro) and Chrome9 display support"
> - depends on FB && PCI && X86
> + depends on FB && PCI && X86 && GPIOLIB && I2C
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> select I2C_ALGOBIT
> - select I2C
> - select GPIOLIB
> help
> This is the frame buffer device driver for Graphics chips of VIA
> UniChrome (Pro) Family (CLE266,PM800/CN400,P4M800CE/P4M800Pro/
>
Thanks, queued for 3.20.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] fbdev: ssd1307fb: return proper error code if write command fails
From: Tomi Valkeinen @ 2015-01-30 7:40 UTC (permalink / raw)
To: Lad, Prabhakar, LFBDEV, Jean-Christophe Plagniol-Villard,
Maxime Ripard
Cc: LKML
In-Reply-To: <1421348737-4606-1-git-send-email-prabhakar.csengg@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 534 bytes --]
On 15/01/15 21:05, Lad, Prabhakar wrote:
> From: Prabhakar Lad <prabhakar.csengg@gmail.com>
>
> this patch fixes ssd1307fb_ssd1306_init() function to return
> proper error codes in case of failures.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---
> Changes for v2:
> a: Added new line as per Maxime's suggestion.
>
> drivers/video/fbdev/ssd1307fb.c | 67 ++++++++++++++++++++++++++++++++---------
> 1 file changed, 53 insertions(+), 14 deletions(-)
>
Thanks, queued for 3.20.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: i915 framebuffer init too slow to find logo
From: S. Gilles @ 2015-01-29 14:59 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: S. Gilles, Jean-Christophe Plagniol-Villard, Maik Broemme,
linux-fbdev, linux-kernel, Thierry Reding
In-Reply-To: <54C9EAC3.9040509@ti.com>
On 2015-01-29T10:09:39+0200, Tomi Valkeinen wrote:
> On 29/01/15 04:32, S. Gilles wrote:
> > Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
> > logs after they have been freed, my i915 machine has no logo on boot
> > (reverting that commit brings it back on recent trees). My .config
> > builds nothing but wireless as =m, so I think this is a genuine false
> > positive (as predicted by the commit). Examining an augmented dmesg,
>
> It's not so much about modules, but when the code tries to use the
> logos. Drivers as modules might cause the use of logos to happen later,
> but that's only one possible reason.
>
> > it appears that the framebuffer setup is too slow by about 0.3s, which
> > I wouldn't really expect from this system/driver.
> >
> > Is this slowness considered worth fixing, or is this issue considered
> > too cosmetic? (Or is this just PEBKAC?)
> >
> > Possibly useful information:
> >
> > $ lspci | grep VGA
> > 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
> >
> > From various printk()s, it looks like the slow portion of
> > fb_console_init() is restore_fbdev_mode(), specifically
> > drm_mode_set_config_internal(), which takes about 0.45s, while the
> > fb_logo_late_init() call happens about 0.15s into that. I can give the
> > full details if requested.
>
> When does the driver probe() happen? Does the initialization happen
> outside of the probe(), via workqueue or such? If so, then the fix is
> valid for your case also, as the work could be ran after the logos have
> been freed.
It looks like the fix is indeed valid, since the initialization
happens without probe() in the trace: the result of putting
dump_stack() at the beginning of the relevant functions is (in far too
much detail)
...
[ 0.302391] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5+ #26
[ 0.302476] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[ 0.302561] ffff880138ffd000 ffff880139563c98 ffffffff817945db 0000000000000126
[ 0.302882] ffffffff81849d20 ffff880139563cc8 ffffffff813b7ec4 ffff880138ffd000
[ 0.303189] ffffffff81848b10 ffffffff81c3ad58 ffff880138ffd090 ffff880139563cf8
[ 0.303511] Call Trace:
[ 0.303578] [<ffffffff817945db>] dump_stack+0x45/0x57
[ 0.303677] [<ffffffff813b7ec4>] i915_pci_probe+0x1a/0x68
[ 0.303769] [<ffffffff8131c209>] pci_device_probe+0x54/0xa3
[ 0.303850] [<ffffffff8144367d>] driver_probe_device+0x99/0x1c8
[ 0.303916] [<ffffffff81443841>] __driver_attach+0x5d/0x80
[ 0.303980] [<ffffffff814437e4>] ? __device_attach+0x38/0x38
[ 0.305851] [<ffffffff81441d7c>] bus_for_each_dev+0x7b/0x85
[ 0.305914] [<ffffffff81443240>] driver_attach+0x19/0x1b
[ 0.305975] [<ffffffff81442f28>] bus_add_driver+0x109/0x1d3
[ 0.306038] [<ffffffff81443c20>] driver_register+0x8a/0xc7
[ 0.306141] [<ffffffff8131bf4b>] __pci_register_driver+0x5c/0x60
[ 0.306219] [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[ 0.306329] [<ffffffff813a5fa3>] drm_pci_init+0x4d/0xcd
[ 0.306428] [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[ 0.306505] [<ffffffff81d05356>] i915_init+0x8a/0x92
[ 0.306566] [<ffffffff81d052cc>] ? ftrace_define_fields_drm_vblank_event_delivered+0x9f/0x9f
[ 0.306650] [<ffffffff8100030d>] do_one_initcall+0xe9/0x172
[ 0.306746] [<ffffffff81ccefb1>] kernel_init_freeable+0x117/0x19f
[ 0.306827] [<ffffffff81cce7c0>] ? initcall_blacklist+0xa3/0xa3
[ 0.306926] [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
[ 0.306993] [<ffffffff8178e562>] kernel_init+0x9/0xd0
[ 0.307085] [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[ 0.307176] [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
...
[ 0.646722] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5+ #26
[ 0.646723] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[ 0.646726] ffff8800b5aff000 ffff880139563e78 ffffffff817945db 0000000000000013
[ 0.646728] ffffffff81d00a08 ffff880139563e88 ffffffff81d00a1f ffff880139563ef8
[ 0.646730] ffffffff8100030d 0000000000000000 ffffffff81b7f6b8 0000019d00070007
[ 0.646730] Call Trace:
[ 0.646733] [<ffffffff817945db>] dump_stack+0x45/0x57
[ 0.646736] [<ffffffff81d00a08>] ? fb_console_init+0x116/0x116
[ 0.646738] [<ffffffff81d00a1f>] fb_logo_late_init+0x17/0x22
[ 0.646741] [<ffffffff8100030d>] do_one_initcall+0xe9/0x172
[ 0.646744] [<ffffffff81ccefb1>] kernel_init_freeable+0x117/0x19f
[ 0.646745] [<ffffffff81cce7c0>] ? initcall_blacklist+0xa3/0xa3
[ 0.646747] [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
[ 0.646749] [<ffffffff8178e562>] kernel_init+0x9/0xd0
[ 0.646752] [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[ 0.646754] [<ffffffff8178e559>] ? rest_init+0xb6/0xb6
...
[ 1.162602] CPU: 1 PID: 6 Comm: kworker/u16:0 Not tainted 3.19.0-rc5+ #26
[ 1.162605] Hardware name: LENOVO 4286CTO/4286CTO, BIOS 8DET42WW (1.12 ) 04/01/2011
[ 1.162627] 0000000000000018 ffff8801395a3918 ffffffff817945db ffffffff81caeee0
[ 1.162634] 0000000000000018 ffff8801395a3938 ffffffff8178f5f7 0000000000000000
[ 1.162641] ffff8800b5ff0c00 ffff8801395a3968 ffffffff813321fb 0000000000000320
[ 1.162643] Call Trace:
[ 1.162653] [<ffffffff817945db>] dump_stack+0x45/0x57
[ 1.162661] [<ffffffff8178f5f7>] fb_find_logo+0xd/0x43
[ 1.162669] [<ffffffff813321fb>] fb_prepare_logo+0x87/0x12d
[ 1.162675] [<ffffffff81328fed>] fbcon_prepare_logo+0x7f/0x2e8
[ 1.162680] [<ffffffff8132b3b9>] fbcon_init+0x3d9/0x447
[ 1.162688] [<ffffffff813817de>] visual_init+0xb7/0x10d
[ 1.162695] [<ffffffff81383000>] do_bind_con_driver+0x1ab/0x2cd
[ 1.162702] [<ffffffff813835e1>] do_take_over_console+0x132/0x162
[ 1.162707] [<ffffffff813292ac>] do_fbcon_takeover+0x56/0x9a
[ 1.162712] [<ffffffff8132cb6a>] fbcon_event_notify+0x31c/0x644
[ 1.162718] [<ffffffff810bcf4f>] notifier_call_chain+0x39/0x5c
[ 1.162723] [<ffffffff810bd208>] __blocking_notifier_call_chain+0x47/0x60
[ 1.162729] [<ffffffff810bd230>] blocking_notifier_call_chain+0xf/0x11
[ 1.162735] [<ffffffff81331cc3>] fb_notifier_call_chain+0x16/0x18
[ 1.162741] [<ffffffff81333b50>] register_framebuffer+0x261/0x299
[ 1.162750] [<ffffffff8139bcfb>] drm_fb_helper_initial_config+0x26e/0x328
[ 1.162757] [<ffffffff8141da68>] intel_fbdev_initial_config+0x16/0x18
[ 1.162762] [<ffffffff810be0a0>] async_run_entry_fn+0x33/0xca
[ 1.162770] [<ffffffff810b800b>] process_one_work+0x223/0x3f9
[ 1.162775] [<ffffffff810b7f8f>] ? process_one_work+0x1a7/0x3f9
[ 1.162781] [<ffffffff810b8905>] worker_thread+0x260/0x354
[ 1.162788] [<ffffffff810b86a5>] ? cancel_delayed_work_sync+0x10/0x10
[ 1.162794] [<ffffffff810bc333>] kthread+0xe8/0xf0
[ 1.162802] [<ffffffff810bc24b>] ? kthread_create_on_node+0x1b1/0x1b1
[ 1.162810] [<ffffffff8179ceec>] ret_from_fork+0x7c/0xb0
[ 1.162817] [<ffffffff810bc24b>] ? kthread_create_on_node+0x1b1/0x1b1
...
> However, it does seem that the fix seems to cause logos to disappear for
> many people. I'd be interesting to know how many of those cases were
> working by luck, either by
>
> 1) an async work being ran fast enough, before the logos had been freed
> 2) the use of logos happening after the logos had been freed, but if no
> one had trashed the logo memory yet, it still works
In my case, it looks like #1 is the case: free_initmem() was called
~6.96s into boot on the run matching the traces above, so the initdata
looks safe barring async trickery.
> I don't care so much about the logo myself but people do seem to like
> it, so perhaps we need to change the code as Thierry suggested:
> allocating memory for the logos and keeping them in memory until someone
> uses them the first time, and then free the memory.
I'm not too concerned about the pixels myself, but I was concerned
that the initialization was happening slow enough to be caught by this
(perhaps this might have been a warning sign if setting up the console
was depending unsafely on other initdata?). It is also rather obvious,
so it's something a novice like myself can easily bisect and report.
--
S. Gilles
^ permalink raw reply
* Re: [PATCH 2/3 v3] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Vitaly Kuznetsov @ 2015-01-29 13:02 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: K. Y. Srinivasan, linux-fbdev, Haiyang Zhang, linux-kernel,
Tomi Valkeinen, devel, Jean-Christophe Plagniol-Villard
In-Reply-To: <1422527056-24929-1-git-send-email-der.herr@hofr.at>
Nicholas Mc Guire <der.herr@hofr.at> writes:
> The return type of wait_for_completion_timeout is unsigned long not
> int. This patch fixes up the declarations only.
>
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
I would be slightly better to remove ".c" from your subject like,
anyway:
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>
> v2: fixed subject line
> v3: fixed patch description as recommended by Dan Carpenter
> <dan.carpenter@oracle.com>
>
> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>
> Patch is against 3.19.0-rc5 -next-20150123
>
> drivers/video/fbdev/hyperv_fb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 4254336..807ee22 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -415,7 +415,8 @@ static int synthvid_negotiate_ver(struct hv_device *hdev, u32 ver)
> struct fb_info *info = hv_get_drvdata(hdev);
> struct hvfb_par *par = info->par;
> struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
> - int t, ret = 0;
> + int ret = 0;
> + unsigned long t;
>
> memset(msg, 0, sizeof(struct synthvid_msg));
> msg->vid_hdr.type = SYNTHVID_VERSION_REQUEST;
> @@ -488,7 +489,8 @@ static int synthvid_send_config(struct hv_device *hdev)
> struct fb_info *info = hv_get_drvdata(hdev);
> struct hvfb_par *par = info->par;
> struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
> - int t, ret = 0;
> + int ret = 0;
> + unsigned long t;
>
> /* Send VRAM location */
> memset(msg, 0, sizeof(struct synthvid_msg));
--
Vitaly
^ permalink raw reply
* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:41 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <54CA0CBA.6030900@ti.com>
On Thu, 29 Jan 2015, Tomi Valkeinen wrote:
> On 29/01/15 11:38, Nicholas Mc Guire wrote:
> > On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> >
> >> Hi,
> >>
> >> On 25/01/15 16:47, Nicholas Mc Guire wrote:
> >>> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> >>> ---
> >>>
> >>> v2: fixed subject line
> >>>
> >>> The return type of wait_for_completion_timeout is unsigned long not
> >>> int. This patch fixes up the declarations only.
> >>>
> >>> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> >>> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
> >>
> >> Why didn't you set the text above as the patch description (which is
> >> empty at the moment)?
> >>
> > basically because the one-line is sufficient to understand the patch
>
> You didn't have one line, you had no description. Patch subject is not
> patch description. In the minimal case, the description should have the
> same text as the subject, but usually it's better to have a bit more
> text in the description.
ok - was not clear about this - got it.
>
> > and the rest of the information is not relevant for the git log but only
> > for the review
> >
> > if you think it is necessary to understand the patch I'll move it and
> > resubmit.
>
> Well, a good description is not only about understanding the code in the
> patch. It may contain information like which platform/setup this issue
> happened on, are the any possible side effects, or whatever might be
> relevant for someone looking at the patch years later.
>
yup - but it seemed to me that the information on the build
config and kernel version details would not really be relevant for
this cleanup patch - so if I got your right the description line should
have gone up and the config/kernel info stays below "---".
Just resent it - hope this is correct now.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Tomi Valkeinen @ 2015-01-29 10:34 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <20150129093839.GB23666@opentech.at>
[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]
On 29/01/15 11:38, Nicholas Mc Guire wrote:
> On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
>
>> Hi,
>>
>> On 25/01/15 16:47, Nicholas Mc Guire wrote:
>>> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
>>> ---
>>>
>>> v2: fixed subject line
>>>
>>> The return type of wait_for_completion_timeout is unsigned long not
>>> int. This patch fixes up the declarations only.
>>>
>>> Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
>>> CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>>
>> Why didn't you set the text above as the patch description (which is
>> empty at the moment)?
>>
> basically because the one-line is sufficient to understand the patch
You didn't have one line, you had no description. Patch subject is not
patch description. In the minimal case, the description should have the
same text as the subject, but usually it's better to have a bit more
text in the description.
> and the rest of the information is not relevant for the git log but only
> for the review
>
> if you think it is necessary to understand the patch I'll move it and
> resubmit.
Well, a good description is not only about understanding the code in the
patch. It may contain information like which platform/setup this issue
happened on, are the any possible side effects, or whatever might be
relevant for someone looking at the patch years later.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:25 UTC (permalink / raw)
To: Dan Carpenter
Cc: Tomi Valkeinen, devel, Haiyang Zhang, linux-fbdev, linux-kernel
In-Reply-To: <20150129094704.GT6456@mwanda>
On Thu, 29 Jan 2015, Dan Carpenter wrote:
> On Thu, Jan 29, 2015 at 10:38:39AM +0100, Nicholas Mc Guire wrote:
> > On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> >
> > > Hi,
> > >
> > > On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > > > ---
> > > >
> > > > v2: fixed subject line
> > > >
> > > > The return type of wait_for_completion_timeout is unsigned long not
> > > > int. This patch fixes up the declarations only.
> > > >
>
> This line is relevant for the patch description.
>
> > > > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > > > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>
> This line is not relevant.
>
thanks - will resend it then - assumed that the one-line would do for this
patch.
thx!
hofrat
^ permalink raw reply
* [PATCH 2/3 v3] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 10:24 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: Haiyang Zhang, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
devel, linux-fbdev, linux-kernel, Nicholas Mc Guire
The return type of wait_for_completion_timeout is unsigned long not
int. This patch fixes up the declarations only.
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
v2: fixed subject line
v3: fixed patch description as recommended by Dan Carpenter
<dan.carpenter@oracle.com>
Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
Patch is against 3.19.0-rc5 -next-20150123
drivers/video/fbdev/hyperv_fb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 4254336..807ee22 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -415,7 +415,8 @@ static int synthvid_negotiate_ver(struct hv_device *hdev, u32 ver)
struct fb_info *info = hv_get_drvdata(hdev);
struct hvfb_par *par = info->par;
struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
- int t, ret = 0;
+ int ret = 0;
+ unsigned long t;
memset(msg, 0, sizeof(struct synthvid_msg));
msg->vid_hdr.type = SYNTHVID_VERSION_REQUEST;
@@ -488,7 +489,8 @@ static int synthvid_send_config(struct hv_device *hdev)
struct fb_info *info = hv_get_drvdata(hdev);
struct hvfb_par *par = info->par;
struct synthvid_msg *msg = (struct synthvid_msg *)par->init_buf;
- int t, ret = 0;
+ int ret = 0;
+ unsigned long t;
/* Send VRAM location */
memset(msg, 0, sizeof(struct synthvid_msg));
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Dan Carpenter @ 2015-01-29 9:47 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Tomi Valkeinen, devel, Haiyang Zhang, linux-fbdev, linux-kernel
In-Reply-To: <20150129093839.GB23666@opentech.at>
On Thu, Jan 29, 2015 at 10:38:39AM +0100, Nicholas Mc Guire wrote:
> On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
>
> > Hi,
> >
> > On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > > ---
> > >
> > > v2: fixed subject line
> > >
> > > The return type of wait_for_completion_timeout is unsigned long not
> > > int. This patch fixes up the declarations only.
> > >
This line is relevant for the patch description.
> > > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
This line is not relevant.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] video: treat signal like timeout as failure
From: Nicholas Mc Guire @ 2015-01-29 9:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150126125905.GI26493@n2100.arm.linux.org.uk>
On Mon, 26 Jan 2015, Russell King - ARM Linux wrote:
> On Tue, Jan 20, 2015 at 06:23:50AM +0100, Nicholas Mc Guire wrote:
> > if(!wait_for_completion_interruptible_timeout(...))
> > only handles the timeout case - this patch adds handling the
> > signal case the same as timeout and cleans up.
> >
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >
> > Only the timeout case was being handled, return of 0 in
> > wait_for_completion_interruptible_timeout, the signal case (-ERESTARTSYS)
> > was treated just like the case of successful completion, which is most
> > likely not reasonable.
> >
> > Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
> > are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
> >
> > This patch simply treats the signal case the same way as the timeout case,
> > by releasing locks and returning 0 - which might not be the right thing to
> > do - this needs a review by someone knowing the details of this driver.
> >
> > Patch is against 3.19.0-rc5 -next-20150119
> >
> > Patch was only compile-tested with exynos_defconfig
> >
> > drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c b/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > index 2358a2f..55a7a45 100644
> > --- a/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > +++ b/drivers/video/fbdev/exynos/exynos_mipi_dsi_common.c
> > @@ -157,6 +157,7 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,
> > const unsigned char *data0, unsigned int data_size)
> > {
> > unsigned int check_rx_ack = 0;
> > + long timeout;
> >
> > if (dsim->state = DSIM_STATE_ULPS) {
> > dev_err(dsim->dev, "state is ULPS.\n");
> > @@ -244,9 +245,11 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,
> > exynos_mipi_dsi_wr_tx_header(dsim, data_id, data_size & 0xff,
> > (data_size & 0xff00) >> 8);
> >
> > - if (!wait_for_completion_interruptible_timeout(&dsim_wr_comp,
> > - MIPI_FIFO_TIMEOUT)) {
> > - dev_warn(dsim->dev, "command write timeout.\n");
> > + timeout = wait_for_completion_interruptible_timeout(
> > + &dsim_wr_comp, MIPI_FIFO_TIMEOUT);
> > + if (timeout <= 0) {
> > + dev_warn(dsim->dev,
> > + "command write timed-out/interrupted.\n");
>
> This is really silly. Let's say that the program which results in
> this function called is using signals (eg, alarm() with SIGALRM, or
> asynchronous IO with SIGIO, etc).
>
> Why should having a SIGALRM raised print a kernel message? If this
> happens a lot, it will result in the kernel log being flooded with
> these messages.
>
> Signals should not be seen as exceptional conditions. For some programs,
> they are merely asynchronous events which are a normal part of the
> programs operation (eg, SIGIO, SIGALRM, etc.)
>
> Please, if you are going to handle signals, then handle them properly.
> If you're not going to handle them properly, don't use a wait that
> caters for them - use wait_for_completion_killable_timeout() which
> doesn't finish waiting on a signal unless the signal is going to result
> in the death of the program.
>
the current code would treat the signal case identical with the
completion success case - and that hardly can be the intention
so while it might not be necessary to call printk in the signal
case it should in some way be handled - if there is not need to
handle signals then it might be more resonable to use
wait_for_completion_timeout which is not interruptible.
So the key issue here is not that a signal should necessarily print
a message but that it should not be treated as the success case. The
current code will only treat timeout as an error condition and a received
signal (implying that the condition being waited for is most likely not
satisfied) as a successful completion.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH 2/3 v2] hyperv: hyperv_fb.c: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-29 9:38 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: K. Y. Srinivasan, Haiyang Zhang, devel, linux-fbdev, linux-kernel
In-Reply-To: <54C62D07.3040603@ti.com>
On Mon, 26 Jan 2015, Tomi Valkeinen wrote:
> Hi,
>
> On 25/01/15 16:47, Nicholas Mc Guire wrote:
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >
> > v2: fixed subject line
> >
> > The return type of wait_for_completion_timeout is unsigned long not
> > int. This patch fixes up the declarations only.
> >
> > Patch was compile tested only for x86_64_defconfig + CONFIG_X86_VSMP=y
> > CONFIG_HYPERV=m, CONFIG_FB_HYPERV=m
>
> Why didn't you set the text above as the patch description (which is
> empty at the moment)?
>
basically because the one-line is sufficient to understand the patch
and the rest of the information is not relevant for the git log but only
for the review
if you think it is necessary to understand the patch I'll move it and
resubmit.
thanks for your review !
hofrat
^ permalink raw reply
* Re: i915 framebuffer init too slow to find logo
From: Tomi Valkeinen @ 2015-01-29 8:09 UTC (permalink / raw)
To: S. Gilles, Jean-Christophe Plagniol-Villard, Maik Broemme
Cc: linux-fbdev, linux-kernel, Thierry Reding
In-Reply-To: <20150129023247.GA10967@number16>
[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]
On 29/01/15 04:32, S. Gilles wrote:
> Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
> logs after they have been freed, my i915 machine has no logo on boot
> (reverting that commit brings it back on recent trees). My .config
> builds nothing but wireless as =m, so I think this is a genuine false
> positive (as predicted by the commit). Examining an augmented dmesg,
It's not so much about modules, but when the code tries to use the
logos. Drivers as modules might cause the use of logos to happen later,
but that's only one possible reason.
> it appears that the framebuffer setup is too slow by about 0.3s, which
> I wouldn't really expect from this system/driver.
>
> Is this slowness considered worth fixing, or is this issue considered
> too cosmetic? (Or is this just PEBKAC?)
>
> Possibly useful information:
>
> $ lspci | grep VGA
> 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
>
> From various printk()s, it looks like the slow portion of
> fb_console_init() is restore_fbdev_mode(), specifically
> drm_mode_set_config_internal(), which takes about 0.45s, while the
> fb_logo_late_init() call happens about 0.15s into that. I can give the
> full details if requested.
When does the driver probe() happen? Does the initialization happen
outside of the probe(), via workqueue or such? If so, then the fix is
valid for your case also, as the work could be ran after the logos have
been freed.
However, it does seem that the fix seems to cause logos to disappear for
many people. I'd be interesting to know how many of those cases were
working by luck, either by
1) an async work being ran fast enough, before the logos had been freed
2) the use of logos happening after the logos had been freed, but if no
one had trashed the logo memory yet, it still works
I don't care so much about the logo myself but people do seem to like
it, so perhaps we need to change the code as Thierry suggested:
allocating memory for the logos and keeping them in memory until someone
uses them the first time, and then free the memory.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* i915 framebuffer init too slow to find logo
From: S. Gilles @ 2015-01-29 2:32 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard, Maik Broemme
Cc: linux-fbdev, linux-kernel
Since commit 92b004d1aa9f367c372511ca0330f58216b25703 : prevent use of
logs after they have been freed, my i915 machine has no logo on boot
(reverting that commit brings it back on recent trees). My .config
builds nothing but wireless as =m, so I think this is a genuine false
positive (as predicted by the commit). Examining an augmented dmesg,
it appears that the framebuffer setup is too slow by about 0.3s, which
I wouldn't really expect from this system/driver.
Is this slowness considered worth fixing, or is this issue considered
too cosmetic? (Or is this just PEBKAC?)
Possibly useful information:
$ lspci | grep VGA
00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
From various printk()s, it looks like the slow portion of
fb_console_init() is restore_fbdev_mode(), specifically
drm_mode_set_config_internal(), which takes about 0.45s, while the
fb_logo_late_init() call happens about 0.15s into that. I can give the
full details if requested.
Possibly relevant .config bits:
# Graphics support
CONFIG_AGP=y
CONFIG_AGP_INTEL=y
CONFIG_INTEL_GTT=y
CONFIG_VGA_ARB=y
CONFIG_VGA_ARB_MAX_GPUS\x16
# Direct Rendering Manager
CONFIG_DRM=y
CONFIG_DRM_KMS_HELPER=y
CONFIG_DRM_KMS_FB_HELPER=y
# I2C encoder or helper chips
CONFIG_DRM_I915=y
CONFIG_DRM_I915_KMS=y
CONFIG_DRM_I915_FBDEV=y
CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y
# Frame buffer Devices
CONFIG_FB=y
CONFIG_FIRMWARE_EDID=y
CONFIG_FB_CMDLINE=y
CONFIG_FB_CFB_FILLRECT=y
CONFIG_FB_CFB_COPYAREA=y
CONFIG_FB_CFB_IMAGEBLIT=y
# Frame buffer hardware drivers
CONFIG_BACKLIGHT_LCD_SUPPORT=y
CONFIG_LCD_CLASS_DEVICE=y
CONFIG_LCD_PLATFORM=y
CONFIG_BACKLIGHT_CLASS_DEVICE=y
CONFIG_BACKLIGHT_GENERIC=y
CONFIG_HDMI=y
# Console display driver support
CONFIG_VGA_CONSOLE=y
CONFIG_DUMMY_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
CONFIG_LOGO=y
CONFIG_LOGO_LINUX_MONO=y
CONFIG_LOGO_LINUX_VGA16=y
CONFIG_LOGO_LINUX_CLUT224=y
--
S. Gilles
^ permalink raw reply
* [PATCH] video/mmpfb: allow modular build
From: Arnd Bergmann @ 2015-01-28 20:13 UTC (permalink / raw)
To: linux-arm-kernel
The frame buffer core can be a module, which means any fb drivers
should be able to build as modules too. This turns mmpfb into
a tristate option to allow that and fix a possible randconfig
build error.
drivers/built-in.o: In function `modes_setup':
:(.text+0x11b34): undefined reference to `fb_videomode_to_modelist'
:(.text+0x11b5c): undefined reference to `fb_videomode_to_var'
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/video/fbdev/mmp/Makefile b/drivers/video/fbdev/mmp/Makefile
index a014cb358bf8..924dd0930cc7 100644
--- a/drivers/video/fbdev/mmp/Makefile
+++ b/drivers/video/fbdev/mmp/Makefile
@@ -1 +1,3 @@
-obj-y += core.o hw/ panel/ fb/
+obj-$(CONFIG_MMP_DISP) += mmp_disp.o hw/ panel/ fb/
+
+mmp_disp-y += core.o
diff --git a/drivers/video/fbdev/mmp/fb/Kconfig b/drivers/video/fbdev/mmp/fb/Kconfig
index 9b0141f105f5..985e1a7cd254 100644
--- a/drivers/video/fbdev/mmp/fb/Kconfig
+++ b/drivers/video/fbdev/mmp/fb/Kconfig
@@ -1,7 +1,7 @@
if MMP_DISP
config MMP_FB
- bool "fb driver for Marvell MMP Display Subsystem"
+ tristate "fb driver for Marvell MMP Display Subsystem"
depends on FB
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
^ permalink raw reply related
* [PATCH] fb: via: turn gpiolib and i2c selects into dependencies
From: Arnd Bergmann @ 2015-01-28 20:12 UTC (permalink / raw)
To: linux-fbdev
Device driver should not directly select subsystems. In this case
we get build warnings like
warning: (ARCH_REQUIRE_GPIOLIB && PINCTRL_AT91 && PINCTRL_NOMADIK && MFD_TC6393XB && FB_VIA) selects GPIOLIB which has unmet direct dependencies (ARCH_WANT_OPTIONAL_GPIOLIB || ARCH_REQUIRE_GPIOLIB)
which we can avoid using the normal 'depends on' statement.
Also, this patch makes it possible for DRM drivers to have a dependency
on GPIOLIB without getting circular Kconfig dependencies.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f2c3fb7d0399..b3dd417b4719 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1530,13 +1530,11 @@ config FB_SIS_315
config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
- depends on FB && PCI && X86
+ depends on FB && PCI && X86 && GPIOLIB && I2C
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select I2C_ALGOBIT
- select I2C
- select GPIOLIB
help
This is the frame buffer device driver for Graphics chips of VIA
UniChrome (Pro) Family (CLE266,PM800/CN400,P4M800CE/P4M800Pro/
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: hdmi5: remove unneeded check
From: Sudip Mukherjee @ 2015-01-27 13:42 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe Plagniol-Villard, linux-omap, linux-fbdev,
linux-kernel
In-Reply-To: <54C63606.30002@ti.com>
On Mon, Jan 26, 2015 at 02:41:42PM +0200, Tomi Valkeinen wrote:
> On 13/01/15 18:46, Sudip Mukherjee wrote:
> > prior to this check we are checking for word_length_16b and if word_length_16b
> > is false then we are returning with -EINVAL.
> > So at this point word_length_16b can only be true.
>
> True, but it looks to me the code may be extended in the future.
>
> And if it would be clear that it won't be extended in the future, then
> there's more code changes needed to reflect that (the whole
> word_length_16b can be removed, etc).
you are the author of the code, so you will know if it will be extended .. :)
Sudip
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: remove unused variables
From: Sudip Mukherjee @ 2015-01-27 13:20 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Thomas Winischhofer, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <54C63506.3020106@ti.com>
On Mon, Jan 26, 2015 at 02:37:26PM +0200, Tomi Valkeinen wrote:
> On 22/01/15 17:31, Sudip Mukherjee wrote:
> > removed some variables which were not used. Few calls to SiS_GetReg()
> > were left behind as they are reading from the hardare, removing them
> > might affect the overall functionality.
> >
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > This patch will generate checkpatch error, to fix that error we need
> > to change the style of init.c and init301.c
> >
> > drivers/video/fbdev/sis/init.c | 33 +++++----------------------------
> > drivers/video/fbdev/sis/init301.c | 10 ++--------
> > drivers/video/fbdev/sis/sis_main.c | 9 ++++-----
> > 3 files changed, 11 insertions(+), 41 deletions(-)
>
> Are you able to test this?
>
> The patch doesn't look too complex, but it's still slightly too complex
> for me to comfortably merge it without any testing.
No. It is not tested on actual hardware.
Sudip
>
> Tomi
>
>
^ permalink raw reply
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