Skip to content

Commit

Permalink
fbdev: Make registered_fb[] private to fbmem.c
Browse files Browse the repository at this point in the history
Well except when the olpc dcon fbdev driver is enabled, that thing
digs around in there in rather unfixable ways.

Cc oldc_dcon maintainers as fyi.

v2: I typoed the config name (0day)

Cc: kernel test robot <lkp@intel.com>
Cc: Jens Frederich <jfrederich@gmail.com>
Cc: Jon Nettleton <jon.nettleton@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-staging@lists.linux.dev
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Helge Deller <deller@gmx.de>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Cc: linux-fbdev@vger.kernel.org
Cc: Zheyu Ma <zheyuma97@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Series-cc: Thomas Zimmermann <tzimmermann@suse.de>
Series-cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Series-cc: dri-devel@lists.freedesktop.org
Series-cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Series-version: 4
Series-changes: 4
- Add patch to make registered_fb[] private.
Cover-letter:
Fix some race between sysfb device registration and drivers probe
Hello,

The patches in this series contain patches changes suggested by Daniel Vetter
to fix a race condition that exists between sysfb platform device registration
and fbdev or DRM drivers probing and registering devices.

For example, it is currently possible for sysfb to register a platform
device after a real DRM driver was registered and requested to remove the
conflicting framebuffers.

A symptom of this issue, was worked around with by commit fb561bf
("fbdev: Prevent probing generic drivers if a FB is already registered")
but that's really a hack and should be reverted.

This series attempt to fix it more properly and revert the mentioned hack.
That will also allow to make the num_registered_fb variable not visible to
drivers anymore, since that's internal to fbdev core.

This is a v4 that addresses issues pointed out in v3 by Thomas Zimmermann.

Patch #1 is just a trivial preparatory change.

Patch #2 add a sysfb_disable() helper for fbdev and DRM to use it.

Patch #3 fixes the mentioned race condition by disabling sysfb if a driver
probes and registers a device correctly.

Patch #4 is the revert patch that was posted by Daniel before but dropped
from his set and finally patch #5 is the one that makes num_registered_fb
private to fbmem.c and not allow drivers to use it anymore.

The patches were tested on a rpi4 using the following configurations:

* simpledrm and vc4 both built-in
* simpledrm built-in, vc4 as a module
* simpledrm as module, vc4 built-in
* simplefb and vc4 both built-in
* simplefb built-in, vc4 as a module
* simplefb as a module, vc4 built-in

Best regards,
Javier
END
  • Loading branch information
danvet authored and martinezjavier committed Apr 28, 2022
1 parent 3641bcc commit 4f3380c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
8 changes: 6 additions & 2 deletions drivers/video/fbdev/core/fbmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@
static DEFINE_MUTEX(registration_lock);

struct fb_info *registered_fb[FB_MAX] __read_mostly;
EXPORT_SYMBOL(registered_fb);

int num_registered_fb __read_mostly;
#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
EXPORT_SYMBOL(registered_fb);
EXPORT_SYMBOL(num_registered_fb);
#endif
#define for_each_registered_fb(i) \
for (i = 0; i < FB_MAX; i++) \
if (!registered_fb[i]) {} else

bool fb_center_logo __read_mostly;

Expand Down
7 changes: 3 additions & 4 deletions include/linux/fb.h
Original file line number Diff line number Diff line change
Expand Up @@ -623,16 +623,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var,
extern int fb_get_options(const char *name, char **option);
extern int fb_new_modelist(struct fb_info *info);

#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
extern struct fb_info *registered_fb[FB_MAX];

extern int num_registered_fb;
#endif
extern bool fb_center_logo;
extern int fb_logo_count;
extern struct class *fb_class;

#define for_each_registered_fb(i) \
for (i = 0; i < FB_MAX; i++) \
if (!registered_fb[i]) {} else

static inline void lock_fb_info(struct fb_info *info)
{
mutex_lock(&info->lock);
Expand Down

0 comments on commit 4f3380c

Please sign in to comment.