From: Daniel Vetter <hidden> Date: 2017-07-06 12:57:32
There's a bunch of folks who're trying to make printk less
contended and faster, but there's a problem: printk uses the
console_lock, and the console lock has become the BKL for all things
fbdev/fbcon, which in turn pulled in half the drm subsystem under that
lock. That's awkward.
There reasons for that is probably just a historical accident:
- fbcon is a runtime option of fbdev, i.e. at runtime you can pick
whether your fbdev driver instances are used as kernel consoles.
Unfortunately this wasn't implemented with some module option, but
through some module loading magic: As long as you don't load
fbcon.ko, there's no fbdev console support, but loading it (in any
order wrt fbdev drivers) will create console instances for all fbdev
drivers.
- This was implemented through a notifier chain. fbcon.ko enumerates
all fbdev instances at load time and also registers itself as
listener in the fbdev notifier. The fbdev core tries to register new
fbdev instances with fbcon using the notifier.
- On top of that the modifier chain is also used at runtime by the
fbdev subsystem to e.g. control backlights for panels.
- The problem is that the notifier puts a mutex locking context
between fbdev and fbcon, which mixes up the locking contexts for
both the runtime usage and the register time usage to notify fbcon.
And at runtime fbcon (through the fbdev core) might call into the
notifier from a printk critical section while console_lock is held.
- This means console_lock must be an outer lock for the entire fbdev
subsystem, which also means it must be acquired when registering a
new framebuffer driver as the outermost lock since we might call
into fbcon (through the notifier) which would result in a locking
inversion if fbcon would acquire the console_lock from its notifier
callback (which it needs to register the console).
- console_lock can be held anywhere, since printk can be called
anywhere, and through the above story, plus drm/kms being an fbdev
driver, we pull in a shocking amount of locking hiercharchy
underneath the console_lock. Which makes cleaning up printk really
hard (not even splitting console_lock into an rwsem is all that
useful due to this).
There's various ways to address this, but the cleanest would be to
make fbcon a compile-time option, where fbdev directly calls the fbcon
register functions from register_framebuffer, or dummy static inline
versions if fbcon is disabled. Maybe augmented with a runtime knob to
disable fbcon, if that's needed (for debugging perhaps).
But this could break some users who rely on the magic "loading
fbcon.ko enables/disables fbdev framebuffers at runtime" thing, even
if that's unlikely. Hence we must be careful:
1. Create a compile-time dependency between fbcon and fbdev in the
least minimal way. This is what this patch does.
2. Wait at least 1 year to give possible users time to scream about
how we broke their setup. Unlikely, since all distros make fbcon
compile-in, and embedded platforms only compile stuff they know they
need anyway. But still.
3. Convert the notifier to direct functions calls, with dummy static
inlines if fbcon is disabled. We'll still need the fb notifier for the
other uses (like backlights), but we can probably move it into the fb
core (atm it must be built-into vmlinux).
4. Push console_lock down the call-chain, until it is down in
console_register again.
5. Finally start to clean up and rework the printk/console locking.
For context of this saga see
commit 50e244cc793d511b86adea24972f3a7264cae114
Author: Alan Cox [off-list ref]
Date: Fri Jan 25 10:28:15 2013 +1000
fb: rework locking to fix lock ordering on takeover
plus the pile of commits on top that tried to make this all work
without terminally upsetting lockdep. We've uncovered all this when
console_lock lockdep annotations where added in
commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter [off-list ref]
Date: Sat Sep 22 19:52:11 2012 +0200
console: implement lockdep support for console_lock
On the patch itself:
- Switch CONFIG_FRAMEBUFFER_CONSOLE to be a boolean, using the overall
CONFIG_FB tristate to decided whether it should be a module or
built-in.
- At first I thought I could force the build depency with just a dummy
symbol that fbcon.ko exports and fb.ko uses. But that leads to a
module depency cycle (it works fine when built-in).
Since this tight binding is the entire goal the simplest solution is
to move all the fbcon modules (and there's a bunch of optinal
source-files which are each modules of their own, for no good
reason) into the overall fb.ko core module. That's a bit more than
what I would have liked to do in this patch, but oh well.
Cc: Alan Cox <redacted>
Cc: Sergey Senozhatsky <redacted>
Cc: Linux Fbdev development list <redacted>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Daniel Vetter <redacted>
---
v2: Switch to building fbcon code into fb.ko right away because the
cheap trick leads to a module depency loop.
---
drivers/video/console/Kconfig | 2 +-
drivers/video/console/Makefile | 8 --------
drivers/video/fbdev/core/Makefile | 11 +++++++++++
drivers/video/{console => fbdev/core}/bitblit.c | 4 ----
drivers/video/{console => fbdev/core}/fbcon.c | 13 +++----------
drivers/video/{console => fbdev/core}/fbcon.h | 0
drivers/video/{console => fbdev/core}/fbcon_ccw.c | 4 ----
drivers/video/{console => fbdev/core}/fbcon_cw.c | 4 ----
drivers/video/{console => fbdev/core}/fbcon_rotate.c | 4 ----
drivers/video/{console => fbdev/core}/fbcon_rotate.h | 0
drivers/video/{console => fbdev/core}/fbcon_ud.c | 4 ----
drivers/video/fbdev/core/fbmem.c | 6 ++++++
drivers/video/{console => fbdev/core}/softcursor.c | 4 ----
drivers/video/{console => fbdev/core}/tileblit.c | 5 -----
include/linux/fbcon.h | 12 ++++++++++++
15 files changed, 33 insertions(+), 48 deletions(-)
rename drivers/video/{console => fbdev/core}/bitblit.c (98%)
rename drivers/video/{console => fbdev/core}/fbcon.c (99%)
rename drivers/video/{console => fbdev/core}/fbcon.h (100%)
rename drivers/video/{console => fbdev/core}/fbcon_ccw.c (98%)
rename drivers/video/{console => fbdev/core}/fbcon_cw.c (98%)
rename drivers/video/{console => fbdev/core}/fbcon_rotate.c (95%)
rename drivers/video/{console => fbdev/core}/fbcon_rotate.h (100%)
rename drivers/video/{console => fbdev/core}/fbcon_ud.c (98%)
rename drivers/video/{console => fbdev/core}/softcursor.c (93%)
rename drivers/video/{console => fbdev/core}/tileblit.c (96%)
create mode 100644 include/linux/fbcon.h
diff --git a/drivers/video/console/bitblit.c b/drivers/video/fbdev/core/bitblit.csimilarity index 98%rename from drivers/video/console/bitblit.crename to drivers/video/fbdev/core/bitblit.cindex dbfe4eecf12e..99f3a1c3d093 100644--- a/drivers/video/console/bitblit.c+++ b/drivers/video/fbdev/core/bitblit.c
diff --git a/drivers/video/console/fbcon.c b/drivers/video/fbdev/core/fbcon.csimilarity index 99%rename from drivers/video/console/fbcon.crename to drivers/video/fbdev/core/fbcon.cindex 12ded23f1aaf..86b3bcbd01a8 100644--- a/drivers/video/console/fbcon.c+++ b/drivers/video/fbdev/core/fbcon.c
diff --git a/drivers/video/console/fbcon.h b/drivers/video/fbdev/core/fbcon.hsimilarity index 100%rename from drivers/video/console/fbcon.hrename to drivers/video/fbdev/core/fbcon.hdiff --git a/drivers/video/console/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.csimilarity index 98%rename from drivers/video/console/fbcon_ccw.crename to drivers/video/fbdev/core/fbcon_ccw.cindex 5a3cbf6dff4d..2eeefa97bd4a 100644--- a/drivers/video/console/fbcon_ccw.c+++ b/drivers/video/fbdev/core/fbcon_ccw.c
diff --git a/drivers/video/console/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.csimilarity index 98%rename from drivers/video/console/fbcon_cw.crename to drivers/video/fbdev/core/fbcon_cw.cindex e7ee44db4e98..c321f7c59e5c 100644--- a/drivers/video/console/fbcon_cw.c+++ b/drivers/video/fbdev/core/fbcon_cw.c
diff --git a/drivers/video/console/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.csimilarity index 95%rename from drivers/video/console/fbcon_rotate.crename to drivers/video/fbdev/core/fbcon_rotate.cindex db6528f2d3f2..8a51e4d95cc5 100644--- a/drivers/video/console/fbcon_rotate.c+++ b/drivers/video/fbdev/core/fbcon_rotate.c
diff --git a/drivers/video/console/fbcon_rotate.h b/drivers/video/fbdev/core/fbcon_rotate.hsimilarity index 100%rename from drivers/video/console/fbcon_rotate.hrename to drivers/video/fbdev/core/fbcon_rotate.hdiff --git a/drivers/video/console/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.csimilarity index 98%rename from drivers/video/console/fbcon_ud.crename to drivers/video/fbdev/core/fbcon_ud.cindex 19e3714abfe8..c0b605d49cb3 100644--- a/drivers/video/console/fbcon_ud.c+++ b/drivers/video/fbdev/core/fbcon_ud.c
diff --git a/drivers/video/console/softcursor.c b/drivers/video/fbdev/core/softcursor.csimilarity index 93%rename from drivers/video/console/softcursor.crename to drivers/video/fbdev/core/softcursor.cindex 46dd8f5d2e9e..fc93f254498e 100644--- a/drivers/video/console/softcursor.c+++ b/drivers/video/fbdev/core/softcursor.c
diff --git a/drivers/video/console/tileblit.c b/drivers/video/fbdev/core/tileblit.csimilarity index 96%rename from drivers/video/console/tileblit.crename to drivers/video/fbdev/core/tileblit.cindex 15e8e1a89c45..4636a6110c27 100644--- a/drivers/video/console/tileblit.c+++ b/drivers/video/fbdev/core/tileblit.c
From: Daniel Vetter <hidden> Date: 2017-07-06 12:57:33
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
---
drivers/video/fbdev/core/fbcon.c | 2 +-
drivers/video/fbdev/core/fbmem.c | 4 ++--
include/linux/fb.h | 7 +------
3 files changed, 4 insertions(+), 9 deletions(-)
@@ -463,7 +463,7 @@ static int fb_show_logo_line(struct fb_info *info, int rotate,/* Return if the frame buffer is not mapped or suspended */if(logo=NULL||info->state!=FBINFO_STATE_RUNNING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;image.depth=8;
@@ -601,7 +601,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate)memset(&fb_logo,0,sizeof(structlogo_data));if(info->flags&FBINFO_MISC_TILEBLITTING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;if(info->fix.visual=FB_VISUAL_DIRECTCOLOR){
From: Daniel Vetter <hidden> Date: 2017-07-06 12:57:34
It's not accelarated, just system memory. Note we don't even need to
set the default flag since that's now always 0.
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Daniel Vetter <redacted>
---
drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Daniel Vetter <hidden> Date: 2017-07-06 12:57:49
- FBINFO_CAN_FORCE_OUTPUT has been a lie ever since we nerfed&removed
the entire panic handling code in our fbdev emulation. We might
restore kms panic output, but not through the bazillion of legacy
code layers called fbdev/fbcon, there's just no way to make that
work safely.
- With the module check change FBINFO_DEFAULT is always 0, so can be
removed too.
That removes another change to cargo-cult stuff in kms drivers, yay!
Signed-off-by: Daniel Vetter <redacted>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 1 -
drivers/gpu/drm/armada/armada_fbdev.c | 1 -
drivers/gpu/drm/ast/ast_fb.c | 1 -
drivers/gpu/drm/bochs/bochs_fbdev.c | 1 -
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 1 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 1 -
drivers/gpu/drm/i915/intel_fbdev.c | 1 -
drivers/gpu/drm/mgag200/mgag200_fb.c | 1 -
drivers/gpu/drm/msm/msm_fbdev.c | 1 -
drivers/gpu/drm/omapdrm/omap_fbdev.c | 1 -
drivers/gpu/drm/qxl/qxl_fb.c | 1 -
drivers/gpu/drm/radeon/radeon_fb.c | 1 -
drivers/gpu/drm/udl/udl_fb.c | 1 -
drivers/gpu/drm/virtio/virtgpu_fb.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 1 -
15 files changed, 15 deletions(-)
From: kbuild test robot <hidden> Date: 2017-07-07 23:21:45
Hi Daniel,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.12 next-20170707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Make-fbcon-a-built-time-depency-for-fbdev/20170708-063020
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
In file included from drivers/video/fbdev/matrox/matroxfb_base.h:35:0,
from drivers/video/fbdev/matrox/matroxfb_base.c:104:
drivers/video/fbdev/matrox/matroxfb_base.c: In function 'initMatrox2':
quoted
include/linux/fb.h:538:28: error: 'FBINFO_MODULE' undeclared (first use in this function)
#define FBINFO_FLAG_MODULE FBINFO_MODULE
^
quoted
drivers/video/fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of macro 'FBINFO_FLAG_MODULE'
minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~~~~~~~~~~~~~~~~~
include/linux/fb.h:538:28: note: each undeclared identifier is reported only once for each function it appears in
#define FBINFO_FLAG_MODULE FBINFO_MODULE
^
quoted
drivers/video/fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of macro 'FBINFO_FLAG_MODULE'
minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~~~~~~~~~~~~~~~~~
--
In file included from drivers/video//fbdev/matrox/matroxfb_base.h:35:0,
from drivers/video//fbdev/matrox/matroxfb_base.c:104:
drivers/video//fbdev/matrox/matroxfb_base.c: In function 'initMatrox2':
quoted
include/linux/fb.h:538:28: error: 'FBINFO_MODULE' undeclared (first use in this function)
#define FBINFO_FLAG_MODULE FBINFO_MODULE
^
drivers/video//fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of macro 'FBINFO_FLAG_MODULE'
minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~~~~~~~~~~~~~~~~~
include/linux/fb.h:538:28: note: each undeclared identifier is reported only once for each function it appears in
#define FBINFO_FLAG_MODULE FBINFO_MODULE
^
drivers/video//fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of macro 'FBINFO_FLAG_MODULE'
minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~~~~~~~~~~~~~~~~~
vim +/FBINFO_MODULE +538 include/linux/fb.h
1471ca9a Marcin Slusarz 2010-05-16 532 a->count = max_num;
1471ca9a Marcin Slusarz 2010-05-16 533 return a;
1471ca9a Marcin Slusarz 2010-05-16 534 }
1471ca9a Marcin Slusarz 2010-05-16 535
^1da177e Linus Torvalds 2005-04-16 536
^1da177e Linus Torvalds 2005-04-16 537 // This will go away
^1da177e Linus Torvalds 2005-04-16 @538 #define FBINFO_FLAG_MODULE FBINFO_MODULE
^1da177e Linus Torvalds 2005-04-16 539 #define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT
^1da177e Linus Torvalds 2005-04-16 540
^1da177e Linus Torvalds 2005-04-16 541 /* This will go away
:::::: The code at line 538 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds [off-list ref]
:::::: CC: Linus Torvalds [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Daniel Vetter <hidden> Date: 2017-07-11 09:27:03
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
---
drivers/video/fbdev/core/fbcon.c | 2 +-
drivers/video/fbdev/core/fbmem.c | 4 ++--
drivers/video/fbdev/matrox/matroxfb_base.c | 4 +---
include/linux/fb.h | 11 +----------
4 files changed, 5 insertions(+), 16 deletions(-)
@@ -463,7 +463,7 @@ static int fb_show_logo_line(struct fb_info *info, int rotate,/* Return if the frame buffer is not mapped or suspended */if(logo=NULL||info->state!=FBINFO_STATE_RUNNING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;image.depth=8;
@@ -601,7 +601,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate)memset(&fb_logo,0,sizeof(structlogo_data));if(info->flags&FBINFO_MISC_TILEBLITTING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;if(info->fix.visual=FB_VISUAL_DIRECTCOLOR){
@@ -1794,9 +1794,7 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)minfo->fbops=matroxfb_ops;minfo->fbcon.fbops=&minfo->fbops;minfo->fbcon.pseudo_palette=minfo->cmap;-/* after __init time we are like module... no logo */-minfo->fbcon.flags=hotplug?FBINFO_FLAG_MODULE:FBINFO_FLAG_DEFAULT;-minfo->fbcon.flags|=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */+minfo->fbcon.flags=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */FBINFO_HWACCEL_COPYAREA|/* We have hw-assisted bmove */FBINFO_HWACCEL_FILLRECT|/* And fillrect */FBINFO_HWACCEL_IMAGEBLIT|/* And imageblit */
@@ -400,7 +400,7 @@ struct fb_tile_ops {#endif /* CONFIG_FB_TILEBLITTING *//* FBINFO_* = fb_info.flags bit flags */-#define FBINFO_MODULE 0x0001 /* Low-level driver is a module */+#define FBINFO_DEFAULT 0#define FBINFO_HWACCEL_DISABLED 0x0002/* When FBINFO_HWACCEL_DISABLED is set:*Hardwareaccelerationisturnedoff.Softwareimplementations
@@ -533,15 +533,6 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {returna;}-#ifdef MODULE-#define FBINFO_DEFAULT FBINFO_MODULE-#else-#define FBINFO_DEFAULT 0-#endif--// This will go away-#define FBINFO_FLAG_MODULE FBINFO_MODULE-#define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT/* This will go away*fbsetcurrentlyhacksinFB_ACCELF_TEXTintovar.accel_flags
From: Daniel Vetter <hidden> Date: 2017-07-11 14:52:27
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
---
drivers/video/fbdev/core/fbcon.c | 2 +-
drivers/video/fbdev/core/fbmem.c | 4 ++--
drivers/video/fbdev/matrox/matroxfb_base.c | 4 +---
include/linux/fb.h | 10 +---------
4 files changed, 5 insertions(+), 15 deletions(-)
@@ -463,7 +463,7 @@ static int fb_show_logo_line(struct fb_info *info, int rotate,/* Return if the frame buffer is not mapped or suspended */if(logo=NULL||info->state!=FBINFO_STATE_RUNNING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;image.depth=8;
@@ -601,7 +601,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate)memset(&fb_logo,0,sizeof(structlogo_data));if(info->flags&FBINFO_MISC_TILEBLITTING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;if(info->fix.visual=FB_VISUAL_DIRECTCOLOR){
@@ -1794,9 +1794,7 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)minfo->fbops=matroxfb_ops;minfo->fbcon.fbops=&minfo->fbops;minfo->fbcon.pseudo_palette=minfo->cmap;-/* after __init time we are like module... no logo */-minfo->fbcon.flags=hotplug?FBINFO_FLAG_MODULE:FBINFO_FLAG_DEFAULT;-minfo->fbcon.flags|=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */+minfo->fbcon.flags=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */FBINFO_HWACCEL_COPYAREA|/* We have hw-assisted bmove */FBINFO_HWACCEL_FILLRECT|/* And fillrect */FBINFO_HWACCEL_IMAGEBLIT|/* And imageblit */
@@ -400,7 +400,7 @@ struct fb_tile_ops {#endif /* CONFIG_FB_TILEBLITTING *//* FBINFO_* = fb_info.flags bit flags */-#define FBINFO_MODULE 0x0001 /* Low-level driver is a module */+#define FBINFO_DEFAULT 0#define FBINFO_HWACCEL_DISABLED 0x0002/* When FBINFO_HWACCEL_DISABLED is set:*Hardwareaccelerationisturnedoff.Softwareimplementations
@@ -533,14 +533,6 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {returna;}-#ifdef MODULE-#define FBINFO_DEFAULT FBINFO_MODULE-#else-#define FBINFO_DEFAULT 0-#endif--// This will go away-#define FBINFO_FLAG_MODULE FBINFO_MODULE#define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT/* This will go away
On Thursday, July 06, 2017 02:57:32 PM Daniel Vetter wrote:
There's a bunch of folks who're trying to make printk less
contended and faster, but there's a problem: printk uses the
console_lock, and the console lock has become the BKL for all things
fbdev/fbcon, which in turn pulled in half the drm subsystem under that
lock. That's awkward.
There reasons for that is probably just a historical accident:
- fbcon is a runtime option of fbdev, i.e. at runtime you can pick
whether your fbdev driver instances are used as kernel consoles.
Unfortunately this wasn't implemented with some module option, but
through some module loading magic: As long as you don't load
fbcon.ko, there's no fbdev console support, but loading it (in any
order wrt fbdev drivers) will create console instances for all fbdev
drivers.
- This was implemented through a notifier chain. fbcon.ko enumerates
all fbdev instances at load time and also registers itself as
listener in the fbdev notifier. The fbdev core tries to register new
fbdev instances with fbcon using the notifier.
- On top of that the modifier chain is also used at runtime by the
fbdev subsystem to e.g. control backlights for panels.
- The problem is that the notifier puts a mutex locking context
between fbdev and fbcon, which mixes up the locking contexts for
both the runtime usage and the register time usage to notify fbcon.
And at runtime fbcon (through the fbdev core) might call into the
notifier from a printk critical section while console_lock is held.
- This means console_lock must be an outer lock for the entire fbdev
subsystem, which also means it must be acquired when registering a
new framebuffer driver as the outermost lock since we might call
into fbcon (through the notifier) which would result in a locking
inversion if fbcon would acquire the console_lock from its notifier
callback (which it needs to register the console).
- console_lock can be held anywhere, since printk can be called
anywhere, and through the above story, plus drm/kms being an fbdev
driver, we pull in a shocking amount of locking hiercharchy
underneath the console_lock. Which makes cleaning up printk really
hard (not even splitting console_lock into an rwsem is all that
useful due to this).
There's various ways to address this, but the cleanest would be to
make fbcon a compile-time option, where fbdev directly calls the fbcon
register functions from register_framebuffer, or dummy static inline
versions if fbcon is disabled. Maybe augmented with a runtime knob to
disable fbcon, if that's needed (for debugging perhaps).
But this could break some users who rely on the magic "loading
fbcon.ko enables/disables fbdev framebuffers at runtime" thing, even
if that's unlikely. Hence we must be careful:
1. Create a compile-time dependency between fbcon and fbdev in the
least minimal way. This is what this patch does.
2. Wait at least 1 year to give possible users time to scream about
how we broke their setup. Unlikely, since all distros make fbcon
compile-in, and embedded platforms only compile stuff they know they
need anyway. But still.
3. Convert the notifier to direct functions calls, with dummy static
inlines if fbcon is disabled. We'll still need the fb notifier for the
other uses (like backlights), but we can probably move it into the fb
core (atm it must be built-into vmlinux).
4. Push console_lock down the call-chain, until it is down in
console_register again.
5. Finally start to clean up and rework the printk/console locking.
For context of this saga see
commit 50e244cc793d511b86adea24972f3a7264cae114
Author: Alan Cox [off-list ref]
Date: Fri Jan 25 10:28:15 2013 +1000
fb: rework locking to fix lock ordering on takeover
plus the pile of commits on top that tried to make this all work
without terminally upsetting lockdep. We've uncovered all this when
console_lock lockdep annotations where added in
commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter [off-list ref]
Date: Sat Sep 22 19:52:11 2012 +0200
console: implement lockdep support for console_lock
On the patch itself:
- Switch CONFIG_FRAMEBUFFER_CONSOLE to be a boolean, using the overall
CONFIG_FB tristate to decided whether it should be a module or
built-in.
- At first I thought I could force the build depency with just a dummy
symbol that fbcon.ko exports and fb.ko uses. But that leads to a
module depency cycle (it works fine when built-in).
Since this tight binding is the entire goal the simplest solution is
to move all the fbcon modules (and there's a bunch of optinal
source-files which are each modules of their own, for no good
reason) into the overall fb.ko core module. That's a bit more than
what I would have liked to do in this patch, but oh well.
Cc: Alan Cox <redacted>
Cc: Sergey Senozhatsky <redacted>
Cc: Linux Fbdev development list <redacted>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
---
v2: Switch to building fbcon code into fb.ko right away because the
cheap trick leads to a module depency loop.
---
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: Daniel Vetter <hidden> Date: 2017-07-12 12:42:18
On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
quoted
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Do you plan to pick these two patches up yourself, or do you expect me to
merge them?
I'm always confused when the official maintainer acks something without
saying anything else ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Wednesday, July 12, 2017 02:42:14 PM Daniel Vetter wrote:
On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
quoted
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Do you plan to pick these two patches up yourself, or do you expect me to
merge them?
Since the original patchset contained DRM changes (two last patches)
depending on fbdev changes (two first patches, the patch being discussed
was the second one) I assumed that you would like to take them all
through DRM tree. If this is not what is preferred, please tell me.
I'm always confused when the official maintainer acks something without
saying anything else ...
Point taken, I will try to be more verbose next time.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: Daniel Vetter <hidden> Date: 2017-07-12 15:07:44
On Wed, Jul 12, 2017 at 2:54 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
On Wednesday, July 12, 2017 02:42:14 PM Daniel Vetter wrote:
quoted
On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
quoted
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Do you plan to pick these two patches up yourself, or do you expect me to
merge them?
Since the original patchset contained DRM changes (two last patches)
depending on fbdev changes (two first patches, the patch being discussed
was the second one) I assumed that you would like to take them all
through DRM tree. If this is not what is preferred, please tell me.
There's no direct depency between 1&2 and 3&4, the only effect of
merging them through separate trees is that the bootup logo might not
show up when it's expected, until the trees are merged together. We
could merge them through separate trees if you prefer that (I forgot
to mention that in the cover letter), but I'm fine with putting them
all into drm-misc with your ack for 4.14.
Whatever you prefer, I don't mind either way.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Wednesday, July 12, 2017 05:07:42 PM Daniel Vetter wrote:
On Wed, Jul 12, 2017 at 2:54 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Wednesday, July 12, 2017 02:42:14 PM Daniel Vetter wrote:
quoted
On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
quoted
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Do you plan to pick these two patches up yourself, or do you expect me to
merge them?
Since the original patchset contained DRM changes (two last patches)
depending on fbdev changes (two first patches, the patch being discussed
was the second one) I assumed that you would like to take them all
through DRM tree. If this is not what is preferred, please tell me.
There's no direct depency between 1&2 and 3&4, the only effect of
merging them through separate trees is that the bootup logo might not
show up when it's expected, until the trees are merged together. We
could merge them through separate trees if you prefer that (I forgot
to mention that in the cover letter), but I'm fine with putting them
all into drm-misc with your ack for 4.14.
Whatever you prefer, I don't mind either way.
Then I will merge patches 1&2 for v4.14 through fbdev tree (there are
some other changes pending touching fbdev core and I would like to avoid
conflicts between fbdev & drm-misc trees). Thanks!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: Sean Paul <hidden> Date: 2017-07-13 14:50:47
On Tue, Jul 11, 2017 at 04:52:19PM +0200, Daniel Vetter wrote:
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
@@ -463,7 +463,7 @@ static int fb_show_logo_line(struct fb_info *info, int rotate,/* Return if the frame buffer is not mapped or suspended */if(logo=NULL||info->state!=FBINFO_STATE_RUNNING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;image.depth=8;
@@ -601,7 +601,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate)memset(&fb_logo,0,sizeof(structlogo_data));if(info->flags&FBINFO_MISC_TILEBLITTING||-info->flags&FBINFO_MODULE)+info->fbops->owner)return0;if(info->fix.visual=FB_VISUAL_DIRECTCOLOR){
@@ -1794,9 +1794,7 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)minfo->fbops=matroxfb_ops;minfo->fbcon.fbops=&minfo->fbops;minfo->fbcon.pseudo_palette=minfo->cmap;-/* after __init time we are like module... no logo */-minfo->fbcon.flags=hotplug?FBINFO_FLAG_MODULE:FBINFO_FLAG_DEFAULT;-minfo->fbcon.flags|=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */+minfo->fbcon.flags=FBINFO_PARTIAL_PAN_OK|/* Prefer panning for scroll under MC viewer/edit */FBINFO_HWACCEL_COPYAREA|/* We have hw-assisted bmove */FBINFO_HWACCEL_FILLRECT|/* And fillrect */FBINFO_HWACCEL_IMAGEBLIT|/* And imageblit */
@@ -400,7 +400,7 @@ struct fb_tile_ops {#endif /* CONFIG_FB_TILEBLITTING *//* FBINFO_* = fb_info.flags bit flags */-#define FBINFO_MODULE 0x0001 /* Low-level driver is a module */+#define FBINFO_DEFAULT 0#define FBINFO_HWACCEL_DISABLED 0x0002/* When FBINFO_HWACCEL_DISABLED is set:*Hardwareaccelerationisturnedoff.Softwareimplementations
@@ -533,14 +533,6 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {returna;}-#ifdef MODULE-#define FBINFO_DEFAULT FBINFO_MODULE-#else-#define FBINFO_DEFAULT 0-#endif--// This will go away-#define FBINFO_FLAG_MODULE FBINFO_MODULE#define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT/* This will go away
--
2.13.2
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sean Paul, Software Engineer, Google / Chromium OS
From: Sean Paul <hidden> Date: 2017-07-13 14:52:05
On Thu, Jul 06, 2017 at 02:57:35PM +0200, Daniel Vetter wrote:
- FBINFO_CAN_FORCE_OUTPUT has been a lie ever since we nerfed&removed
the entire panic handling code in our fbdev emulation. We might
restore kms panic output, but not through the bazillion of legacy
code layers called fbdev/fbcon, there's just no way to make that
work safely.
- With the module check change FBINFO_DEFAULT is always 0, so can be
removed too.
That removes another change to cargo-cult stuff in kms drivers, yay!
Signed-off-by: Daniel Vetter <redacted>
From: Daniel Vetter <hidden> Date: 2017-07-19 07:39:14
On Thu, Jul 06, 2017 at 02:57:34PM +0200, Daniel Vetter wrote:
It's not accelarated, just system memory. Note we don't even need to
set the default flag since that's now always 0.
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Daniel Vetter <redacted>
Merged with Dave's irc-ack + commit message amended that qxl once had
accel, but that was removed in 2015.
-Daniel
On Wednesday, July 12, 2017 12:40:45 PM Bartlomiej Zolnierkiewicz wrote:
On Thursday, July 06, 2017 02:57:32 PM Daniel Vetter wrote:
quoted
There's a bunch of folks who're trying to make printk less
contended and faster, but there's a problem: printk uses the
console_lock, and the console lock has become the BKL for all things
fbdev/fbcon, which in turn pulled in half the drm subsystem under that
lock. That's awkward.
There reasons for that is probably just a historical accident:
- fbcon is a runtime option of fbdev, i.e. at runtime you can pick
whether your fbdev driver instances are used as kernel consoles.
Unfortunately this wasn't implemented with some module option, but
through some module loading magic: As long as you don't load
fbcon.ko, there's no fbdev console support, but loading it (in any
order wrt fbdev drivers) will create console instances for all fbdev
drivers.
- This was implemented through a notifier chain. fbcon.ko enumerates
all fbdev instances at load time and also registers itself as
listener in the fbdev notifier. The fbdev core tries to register new
fbdev instances with fbcon using the notifier.
- On top of that the modifier chain is also used at runtime by the
fbdev subsystem to e.g. control backlights for panels.
- The problem is that the notifier puts a mutex locking context
between fbdev and fbcon, which mixes up the locking contexts for
both the runtime usage and the register time usage to notify fbcon.
And at runtime fbcon (through the fbdev core) might call into the
notifier from a printk critical section while console_lock is held.
- This means console_lock must be an outer lock for the entire fbdev
subsystem, which also means it must be acquired when registering a
new framebuffer driver as the outermost lock since we might call
into fbcon (through the notifier) which would result in a locking
inversion if fbcon would acquire the console_lock from its notifier
callback (which it needs to register the console).
- console_lock can be held anywhere, since printk can be called
anywhere, and through the above story, plus drm/kms being an fbdev
driver, we pull in a shocking amount of locking hiercharchy
underneath the console_lock. Which makes cleaning up printk really
hard (not even splitting console_lock into an rwsem is all that
useful due to this).
There's various ways to address this, but the cleanest would be to
make fbcon a compile-time option, where fbdev directly calls the fbcon
register functions from register_framebuffer, or dummy static inline
versions if fbcon is disabled. Maybe augmented with a runtime knob to
disable fbcon, if that's needed (for debugging perhaps).
But this could break some users who rely on the magic "loading
fbcon.ko enables/disables fbdev framebuffers at runtime" thing, even
if that's unlikely. Hence we must be careful:
1. Create a compile-time dependency between fbcon and fbdev in the
least minimal way. This is what this patch does.
2. Wait at least 1 year to give possible users time to scream about
how we broke their setup. Unlikely, since all distros make fbcon
compile-in, and embedded platforms only compile stuff they know they
need anyway. But still.
3. Convert the notifier to direct functions calls, with dummy static
inlines if fbcon is disabled. We'll still need the fb notifier for the
other uses (like backlights), but we can probably move it into the fb
core (atm it must be built-into vmlinux).
4. Push console_lock down the call-chain, until it is down in
console_register again.
5. Finally start to clean up and rework the printk/console locking.
For context of this saga see
commit 50e244cc793d511b86adea24972f3a7264cae114
Author: Alan Cox [off-list ref]
Date: Fri Jan 25 10:28:15 2013 +1000
fb: rework locking to fix lock ordering on takeover
plus the pile of commits on top that tried to make this all work
without terminally upsetting lockdep. We've uncovered all this when
console_lock lockdep annotations where added in
commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter [off-list ref]
Date: Sat Sep 22 19:52:11 2012 +0200
console: implement lockdep support for console_lock
On the patch itself:
- Switch CONFIG_FRAMEBUFFER_CONSOLE to be a boolean, using the overall
CONFIG_FB tristate to decided whether it should be a module or
built-in.
- At first I thought I could force the build depency with just a dummy
symbol that fbcon.ko exports and fb.ko uses. But that leads to a
module depency cycle (it works fine when built-in).
Since this tight binding is the entire goal the simplest solution is
to move all the fbcon modules (and there's a bunch of optinal
source-files which are each modules of their own, for no good
reason) into the overall fb.ko core module. That's a bit more than
what I would have liked to do in this patch, but oh well.
Cc: Alan Cox <redacted>
Cc: Sergey Senozhatsky <redacted>
Cc: Linux Fbdev development list <redacted>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Daniel Vetter <redacted>
Patch queued for 4.14, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Thursday, July 13, 2017 04:01:50 PM Bartlomiej Zolnierkiewicz wrote:
On Wednesday, July 12, 2017 05:07:42 PM Daniel Vetter wrote:
quoted
On Wed, Jul 12, 2017 at 2:54 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Wednesday, July 12, 2017 02:42:14 PM Daniel Vetter wrote:
quoted
On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
quoted
Instead check info->ops->owner, which amounts to the same.
Spotted because I want to remove the pile of broken and cargo-culted
fb_info->flags assignments in drm drivers.
v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* defines
that I've failed to spot.
v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <redacted>
Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Do you plan to pick these two patches up yourself, or do you expect me to
merge them?
Since the original patchset contained DRM changes (two last patches)
depending on fbdev changes (two first patches, the patch being discussed
was the second one) I assumed that you would like to take them all
through DRM tree. If this is not what is preferred, please tell me.
There's no direct depency between 1&2 and 3&4, the only effect of
merging them through separate trees is that the bootup logo might not
show up when it's expected, until the trees are merged together. We
could merge them through separate trees if you prefer that (I forgot
to mention that in the cover letter), but I'm fine with putting them
all into drm-misc with your ack for 4.14.
Whatever you prefer, I don't mind either way.
Then I will merge patches 1&2 for v4.14 through fbdev tree (there are
some other changes pending touching fbdev core and I would like to avoid
conflicts between fbdev & drm-misc trees). Thanks!
Patch queued for 4.14, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics