From c88b946af120029944f4f78fe156bd9361698e97 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:54:59 +0100 Subject: fbdev: Fix contact info in fb_cmdline.c Fix Daniel's email address. No functional changes. Signed-off-by: Thomas Zimmermann Cc: Daniel Vetter Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-2-tzimmermann@suse.de --- drivers/video/fbdev/core/fb_cmdline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c index 3b5bd666b952..6792010d6716 100644 --- a/drivers/video/fbdev/core/fb_cmdline.c +++ b/drivers/video/fbdev/core/fb_cmdline.c @@ -12,7 +12,7 @@ * for more details. * * Authors: - * Vetter + * Daniel Vetter */ #include #include -- cgit v1.2.3 From 73ce73c30ba9ae4d90fdfad7ebe9104001d5d851 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:00 +0100 Subject: fbdev: Transfer video= option strings to caller; clarify ownership In fb_get_options(), always duplicate the returned option string and transfer ownership of the memory to the function's caller. Until now, only the global option string got duplicated and transferred to the caller; the per-driver options were owned by fb_get_options(). In the end, it was impossible for the function's caller to detect if it had to release the string's memory buffer. Hence, all calling drivers leak the memory buffer. The leaks have existed ever since, but drivers only call fb_get_option() once as part of module initialization. So the amount of leaked memory is not significant. Fix the semantics of fb_get_option() by unconditionally transferring ownership of the memory buffer to the caller. Later patches can resolve the memory leaks in the fbdev drivers. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-3-tzimmermann@suse.de --- drivers/video/fbdev/core/fb_cmdline.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c index 6792010d6716..702b00b71870 100644 --- a/drivers/video/fbdev/core/fb_cmdline.c +++ b/drivers/video/fbdev/core/fb_cmdline.c @@ -30,13 +30,17 @@ EXPORT_SYMBOL_GPL(fb_mode_option); * (video=:) * @option: the option will be stored here * + * The caller owns the string returned in @option and is + * responsible for releasing the memory. + * * NOTE: Needed to maintain backwards compatibility */ int fb_get_options(const char *name, char **option) { - char *opt, *options = NULL; + const char *options = NULL; int retval = 0; int name_len = strlen(name), i; + char *opt; if (name_len && ofonly && strncmp(name, "offb", 4)) retval = 1; @@ -55,12 +59,16 @@ int fb_get_options(const char *name, char **option) } /* No match, pass global option */ if (!options && option && fb_mode_option) - options = kstrdup(fb_mode_option, GFP_KERNEL); + options = fb_mode_option; if (options && !strncmp(options, "off", 3)) retval = 1; - if (option) - *option = options; + if (option) { + if (options) + *option = kstrdup(options, GFP_KERNEL); + else + *option = NULL; + } return retval; } -- cgit v1.2.3 From cedaf7cddd7339a3e189a59eec35e76c6557a6d5 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:01 +0100 Subject: fbdev: Support NULL for name in option-string lookup Ignore the per-driver video options if no driver name has been specified to fb_get_option(). Return the global options in this case. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-4-tzimmermann@suse.de --- drivers/video/fbdev/core/fb_cmdline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c index 702b00b71870..cc8a88e8f308 100644 --- a/drivers/video/fbdev/core/fb_cmdline.c +++ b/drivers/video/fbdev/core/fb_cmdline.c @@ -39,13 +39,18 @@ int fb_get_options(const char *name, char **option) { const char *options = NULL; int retval = 0; - int name_len = strlen(name), i; + size_t name_len; char *opt; + if (name) + name_len = strlen(name); + if (name_len && ofonly && strncmp(name, "offb", 4)) retval = 1; if (name_len && !retval) { + unsigned int i; + for (i = 0; i < FB_MAX; i++) { if (video_options[i] == NULL) continue; -- cgit v1.2.3 From 089d924d03d5c17b05d02992f57671ccfba1c4f0 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:03 +0100 Subject: fbdev: Read video= option with fb_get_option() in modedb Get the kernel's global video= parameter with fb_get_option(). Done to unexport the internal fbdev state fb_mode_config. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-6-tzimmermann@suse.de --- drivers/video/fbdev/core/modedb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c index 6473e0dfe146..23cf8eba785d 100644 --- a/drivers/video/fbdev/core/modedb.c +++ b/drivers/video/fbdev/core/modedb.c @@ -620,6 +620,7 @@ int fb_find_mode(struct fb_var_screeninfo *var, const struct fb_videomode *default_mode, unsigned int default_bpp) { + char *mode_option_buf = NULL; int i; /* Set up defaults */ @@ -635,8 +636,10 @@ int fb_find_mode(struct fb_var_screeninfo *var, default_bpp = 8; /* Did the user specify a video mode? */ - if (!mode_option) - mode_option = fb_mode_option; + if (!mode_option) { + fb_get_options(NULL, &mode_option_buf); + mode_option = mode_option_buf; + } if (mode_option) { const char *name = mode_option; unsigned int namelen = strlen(name); @@ -715,6 +718,7 @@ int fb_find_mode(struct fb_var_screeninfo *var, res_specified = 1; } done: + kfree(mode_option_buf); if (cvt) { struct fb_videomode cvt_mode; int ret; -- cgit v1.2.3 From 6d8ad3406a699f1f88ebf976338542857d95eeb5 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:04 +0100 Subject: fbdev: Unexport fb_mode_option There are no external users of fb_mode_option. Unexport the variable and declare it static. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-7-tzimmermann@suse.de --- drivers/video/fbdev/core/fb_cmdline.c | 4 +--- include/linux/fb.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c index cc8a88e8f308..512da89ff8b5 100644 --- a/drivers/video/fbdev/core/fb_cmdline.c +++ b/drivers/video/fbdev/core/fb_cmdline.c @@ -18,11 +18,9 @@ #include static char *video_options[FB_MAX] __read_mostly; +static const char *fb_mode_option __read_mostly; static int ofonly __read_mostly; -const char *fb_mode_option; -EXPORT_SYMBOL_GPL(fb_mode_option); - /** * fb_get_options - get kernel boot parameters * @name: framebuffer name as it would appear in diff --git a/include/linux/fb.h b/include/linux/fb.h index daf336385613..054e8950b274 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -764,7 +764,6 @@ struct dmt_videomode { const struct fb_videomode *mode; }; -extern const char *fb_mode_option; extern const struct fb_videomode vesa_modes[]; extern const struct dmt_videomode dmt_modes[]; -- cgit v1.2.3 From 367221793d4796b18181021b7a39091c268bf39b Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:05 +0100 Subject: fbdev: Move option-string lookup into helper Move the lookup of the option string into an internal helper. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-8-tzimmermann@suse.de --- drivers/video/fbdev/core/fb_cmdline.c | 60 +++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 24 deletions(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c index 512da89ff8b5..f811c7b679e1 100644 --- a/drivers/video/fbdev/core/fb_cmdline.c +++ b/drivers/video/fbdev/core/fb_cmdline.c @@ -21,6 +21,36 @@ static char *video_options[FB_MAX] __read_mostly; static const char *fb_mode_option __read_mostly; static int ofonly __read_mostly; +static const char *__fb_get_options(const char *name) +{ + const char *options = NULL; + size_t name_len = 0; + + if (name) + name_len = strlen(name); + + if (name_len) { + unsigned int i; + const char *opt; + + for (i = 0; i < ARRAY_SIZE(video_options); ++i) { + if (!video_options[i]) + continue; + if (video_options[i][0] == '\0') + continue; + opt = video_options[i]; + if (!strncmp(opt, name, name_len) && opt[name_len] == ':') + options = opt + name_len + 1; + } + } + + /* No match, return global options */ + if (!options) + options = fb_mode_option; + + return options; +} + /** * fb_get_options - get kernel boot parameters * @name: framebuffer name as it would appear in @@ -35,36 +65,18 @@ static int ofonly __read_mostly; */ int fb_get_options(const char *name, char **option) { - const char *options = NULL; int retval = 0; - size_t name_len; - char *opt; + const char *options; - if (name) - name_len = strlen(name); - - if (name_len && ofonly && strncmp(name, "offb", 4)) + if (name && ofonly && strncmp(name, "offb", 4)) retval = 1; - if (name_len && !retval) { - unsigned int i; + options = __fb_get_options(name); - for (i = 0; i < FB_MAX; i++) { - if (video_options[i] == NULL) - continue; - if (!video_options[i][0]) - continue; - opt = video_options[i]; - if (!strncmp(name, opt, name_len) && - opt[name_len] == ':') - options = opt + name_len + 1; - } + if (options) { + if (!strncmp(options, "off", 3)) + retval = 1; } - /* No match, pass global option */ - if (!options && option && fb_mode_option) - options = fb_mode_option; - if (options && !strncmp(options, "off", 3)) - retval = 1; if (option) { if (options) -- cgit v1.2.3 From 93604a5ade3a021fe3daf37f8d378b12cabb26b4 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 9 Feb 2023 14:55:06 +0100 Subject: fbdev: Handle video= parameter in video/cmdline.c Handle the command-line parameter video= in video/cmdline.c. Implement the fbdev helper fb_get_options() on top. Will allows to handle the kernel parameter in DRM without fbdev dependencies. Note that __video_get_options() has the meaning of its return value inverted compared to fb_get_options(). The new helper returns true if the adapter has been enabled, and false otherwise. There is the ofonly parameter, which disables output for non-OF-based framebuffers. It is only for offb and looks like a workaround. The actual purpose it not clear to me. Use 'video=off' or 'nomodeset' instead. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230209135509.7786-9-tzimmermann@suse.de --- drivers/gpu/drm/Kconfig | 2 +- drivers/video/Kconfig | 3 + drivers/video/Makefile | 1 + drivers/video/cmdline.c | 133 ++++++++++++++++++++++++++++++++++ drivers/video/fbdev/Kconfig | 5 +- drivers/video/fbdev/core/Makefile | 3 +- drivers/video/fbdev/core/fb_cmdline.c | 93 +++--------------------- include/video/cmdline.h | 20 +++++ 8 files changed, 172 insertions(+), 88 deletions(-) create mode 100644 drivers/video/cmdline.c create mode 100644 include/video/cmdline.h (limited to 'drivers/video') diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f42d4c6a19f2..17d252dc25e2 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -10,13 +10,13 @@ menuconfig DRM depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA select DRM_PANEL_ORIENTATION_QUIRKS select HDMI - select FB_CMDLINE select I2C select DMA_SHARED_BUFFER select SYNC_FILE # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate # device and dmabuf fd. Let's make sure that is available for our userspace. select KCMP + select VIDEO_CMDLINE select VIDEO_NOMODESET help Kernel-level support for the Direct Rendering Infrastructure (DRI) diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 6d2fde6c5d11..bf05363d8906 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -11,6 +11,9 @@ config APERTURE_HELPERS Support tracking and hand-over of aperture ownership. Required by graphics drivers for firmware-provided framebuffers. +config VIDEO_CMDLINE + bool + config VIDEO_NOMODESET bool default n diff --git a/drivers/video/Makefile b/drivers/video/Makefile index a50eb528ed3c..831c9fa57a6c 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_APERTURE_HELPERS) += aperture.o obj-$(CONFIG_VGASTATE) += vgastate.o +obj-$(CONFIG_VIDEO_CMDLINE) += cmdline.o obj-$(CONFIG_VIDEO_NOMODESET) += nomodeset.o obj-$(CONFIG_HDMI) += hdmi.o diff --git a/drivers/video/cmdline.c b/drivers/video/cmdline.c new file mode 100644 index 000000000000..d3d257489c3d --- /dev/null +++ b/drivers/video/cmdline.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Based on the fbdev code in drivers/video/fbdev/core/fb_cmdline: + * + * Copyright (C) 2014 Intel Corp + * Copyright (C) 1994 Martin Schaller + * + * 2001 - Documented with DocBook + * - Brad Douglas + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of this archive + * for more details. + * + * Authors: + * Daniel Vetter + */ + +#include /* for FB_MAX */ +#include + +#include