| Message ID | 6a9be31e-0bb4-d477-99e6-6cac3cd5087f@axentia.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option | expand |
On 01/07/2019 11:35 AM, Peter Rosin wrote: > On 2019-01-07 10:11, Geert Uytterhoeven wrote: >> Hi Peter, >> >> On Mon, Jan 7, 2019 at 10:03 AM Peter Rosin <peda@axentia.se> wrote: >>> On 2019-01-07 09:59, Peter Rosin wrote: >>>> On 2019-01-07 09:40, Geert Uytterhoeven wrote: >>>>> On Mon, Jan 7, 2019 at 9:26 AM Peter Rosin <peda@axentia.se> wrote: >>>>>> On 2019-01-06 10:33, Geert Uytterhoeven wrote: >>>>>>> On Mon, Nov 26, 2018 at 10:59 PM Peter Rosin <peda@axentia.se> wrote: >>>>>>>> If there are extra logos (CONFIG_FB_LOGO_EXTRA) the heights of these >>>>>>>> extra logos are not considered when centering the first logo vertically. >>>>>>>> >>>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se> >>>>>>> >>>>>>>> --- a/drivers/video/logo/Kconfig >>>>>>>> +++ b/drivers/video/logo/Kconfig >>>>>>>> @@ -10,6 +10,15 @@ menuconfig LOGO >>>>>>>> >>>>>>>> if LOGO >>>>>>>> >>>>>>>> +config FB_LOGO_CENTER >>>>>>>> + bool "Center the logo" >>>>>>>> + depends on FB=y >>>>>>>> + help >>>>>>>> + When this option is selected, the bootup logo is centered both >>>>>>>> + horizontally and vertically. If more than one logo is displayed >>>>>>>> + due to multiple CPUs, the collected line of logos is centered >>>>>>>> + as a whole. >>>>>>>> + >>>>>>> >>>>>>> Isn't a kernel command line option more suitable to configure the position >>>>>>> of the logo? >>>>>> >>>>>> That didn't occur to me previously, but it does make sense now that you >>>>>> mention it. This is on top of v5.0-rc1, and if applied before v5.0 we >>>>>> can avoid possible regressions for folks who might start to rely on >>>>>> CONFIG_FB_LOGO_CENTER if v5.0 is released w/o this. >>>>>> >>>>>> Cheers, >>>>>> Peter >>>>>> >>>>>> From de7353ab519ba9b5c9ea3f62d607bb8e94b687cc Mon Sep 17 00:00:00 2001 >>>>>> From: Peter Rosin <peda@axentia.se> >>>>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >>>>>> Cc: Jonathan Corbet <corbet@lwn.net> >>>>>> Cc: Peter Rosin <peda@axentia.se> >>>>>> Cc: dri-devel@lists.freedesktop.org >>>>>> Cc: linux-fbdev@vger.kernel.org >>>>>> Cc: linux-doc@vger.kernel.org >>>>>> To: linux-kernel@vger.kernel.org >>>>>> Date: Mon, 7 Jan 2019 08:35:26 +0100 >>>>>> Subject: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line >>>>>> option >>>>>> >>>>>> A command line option is much more flexible than a config option and >>>>>> the supporting code is small. Gets rid of #ifdefs in the code too... >>>>>> >>>>>> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> >>>>>> Signed-off-by: Peter Rosin <peda@axentia.se> >>>>> >>>>> Thanks for your patch! >>>>> >>>>>> --- a/Documentation/fb/fbcon.txt >>>>>> +++ b/Documentation/fb/fbcon.txt >>>>>> @@ -163,6 +163,12 @@ C. Boot options >>>>>> be preserved until there actually is some text is output to the console. >>>>>> This option causes fbcon to bind immediately to the fbdev device. >>>>>> >>>>>> +7. fbcon=center-logo >>>>>> + >>>>>> + When this option is selected, the bootup logo is centered both >>>>>> + horizontally and vertically. If more than one logo is displayed due to >>>>>> + multiple CPUs, the collected line of logos is centered as a whole. >>>>>> + >>>>> >>>>> What about making this more generic, than an option specific for centering? >>>>> Else people want fbcon=left-logo or fbcon=right-logo soon? >>>>> >>>>> fbcon=logo-pos:<pos> >>>>> >>>>> With <pos> being "center", "left", "top", "right", "bottom", and combinations >>>>> like "top,center"? Or is that complicating stuff too much? >>>> >>>> I'm not too keen on implementing all that when I have zero use for it. I can >>>> however rename the trigger for the existing fb_center_logo bool to >>>> >>>> fbcon=logo-pos:center >>>> >>>> and add a check for "top,left" to reset the bool to false. Then the other >>>> variants can be added later by whomever and when needed. >>>> >>>> Is that good enough? >>> >>> Ouch, I just realized that using a comma is a no-go, as that is already >>> separating fbcon suboptions, so I suggest top-left instead. >> >> Sure, sounds fine to me. I just want to avoid having a parameter system >> that complicates future extension. >> I think you can drop the check for top-left, as the bool defaults to false. > > Right. So, here's an update... > > Again, it would probably be best if this went in before 5.0 is released. > > Changes since v1: > - rename from fbcon=center-logo option to fbcon=logo-pos:center (and adjust > the help text accordingly). > > Cheers, > Peter > > From ebfb2feb7ea4298ac9c222e6ea6f9c9a92452084 Mon Sep 17 00:00:00 2001 > From: Peter Rosin <peda@axentia.se> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Peter Rosin <peda@axentia.se> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-fbdev@vger.kernel.org > Cc: linux-doc@vger.kernel.org > To: linux-kernel@vger.kernel.org > Date: Mon, 7 Jan 2019 08:35:26 +0100 > Subject: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd > line option > > A command line option is much more flexible than a config option and > the supporting code is small. Gets rid of #ifdefs in the code too... > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Peter Rosin <peda@axentia.se> Patch queued for 5.0, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 2019-01-16 17:45, Bartlomiej Zolnierkiewicz wrote: > On 01/07/2019 11:35 AM, Peter Rosin wrote: >> Right. So, here's an update... >> >> Again, it would probably be best if this went in before 5.0 is released. >> >> Changes since v1: >> - rename from fbcon=center-logo option to fbcon=logo-pos:center (and adjust >> the help text accordingly). >> >> Cheers, >> Peter >> >> From ebfb2feb7ea4298ac9c222e6ea6f9c9a92452084 Mon Sep 17 00:00:00 2001 >> From: Peter Rosin <peda@axentia.se> >> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >> Cc: Jonathan Corbet <corbet@lwn.net> >> Cc: Peter Rosin <peda@axentia.se> >> Cc: dri-devel@lists.freedesktop.org >> Cc: linux-fbdev@vger.kernel.org >> Cc: linux-doc@vger.kernel.org >> To: linux-kernel@vger.kernel.org >> Date: Mon, 7 Jan 2019 08:35:26 +0100 >> Subject: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd >> line option >> >> A command line option is much more flexible than a config option and >> the supporting code is small. Gets rid of #ifdefs in the code too... >> >> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> >> Signed-off-by: Peter Rosin <peda@axentia.se> > > Patch queued for 5.0, thanks. Hmm, I see the patch on the fbdev-for-next branch, but that sounds like stuff destined for 5.1? Maybe you simply don't have any fbdev-for-current or fbdev-fixes branch or something like that? Just checking to make sure we're on the same page... Cheers, Peter
On 01/17/2019 02:40 PM, Peter Rosin wrote: > On 2019-01-16 17:45, Bartlomiej Zolnierkiewicz wrote: >> On 01/07/2019 11:35 AM, Peter Rosin wrote: >>> Right. So, here's an update... >>> >>> Again, it would probably be best if this went in before 5.0 is released. >>> >>> Changes since v1: >>> - rename from fbcon=center-logo option to fbcon=logo-pos:center (and adjust >>> the help text accordingly). >>> >>> Cheers, >>> Peter >>> >>> From ebfb2feb7ea4298ac9c222e6ea6f9c9a92452084 Mon Sep 17 00:00:00 2001 >>> From: Peter Rosin <peda@axentia.se> >>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >>> Cc: Jonathan Corbet <corbet@lwn.net> >>> Cc: Peter Rosin <peda@axentia.se> >>> Cc: dri-devel@lists.freedesktop.org >>> Cc: linux-fbdev@vger.kernel.org >>> Cc: linux-doc@vger.kernel.org >>> To: linux-kernel@vger.kernel.org >>> Date: Mon, 7 Jan 2019 08:35:26 +0100 >>> Subject: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd >>> line option >>> >>> A command line option is much more flexible than a config option and >>> the supporting code is small. Gets rid of #ifdefs in the code too... >>> >>> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> >>> Signed-off-by: Peter Rosin <peda@axentia.se> >> >> Patch queued for 5.0, thanks. > > Hmm, I see the patch on the fbdev-for-next branch, but that sounds > like stuff destined for 5.1? Maybe you simply don't have any > fbdev-for-current or fbdev-fixes branch or something like that? Yes. the single public fbdev branch has been sufficient for now (there is no active fbdev development and patches for -rcX kernels are even more rare). OTOH there is no problem with adding -fixes branch once needed.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/Documentation/fb/fbcon.txt b/Documentation/fb/fbcon.txt index 62af30511a95..60a5ec04e8f0 100644 --- a/Documentation/fb/fbcon.txt +++ b/Documentation/fb/fbcon.txt @@ -163,6 +163,14 @@ C. Boot options be preserved until there actually is some text is output to the console. This option causes fbcon to bind immediately to the fbdev device. +7. fbcon=logo-pos:<location> + + The only possible 'location' is 'center' (without quotes), and when + given, the bootup logo is moved from the default top-left corner + location to the center of the framebuffer. If more than one logo is + displayed due to multiple CPUs, the collected line of logos is moved + as a whole. + C. Attaching, Detaching and Unloading Before going on to how to attach, detach and unload the framebuffer console, an diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 8976190b6c1f..bfa1360ec750 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -510,6 +510,13 @@ static int __init fb_console_setup(char *this_opt) continue; } #endif + + if (!strncmp(options, "logo-pos:", 9)) { + options += 9; + if (!strcmp(options, "center")) + fb_center_logo = true; + continue; + } } return 1; } diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 558ed2ed3124..cb43a2258c51 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -53,6 +53,9 @@ EXPORT_SYMBOL(registered_fb); int num_registered_fb __read_mostly; EXPORT_SYMBOL(num_registered_fb); +bool fb_center_logo __read_mostly; +EXPORT_SYMBOL(fb_center_logo); + static struct fb_info *get_fb_info(unsigned int idx) { struct fb_info *fb_info; @@ -506,8 +509,7 @@ static int fb_show_logo_line(struct fb_info *info, int rotate, fb_set_logo(info, logo, logo_new, fb_logo.depth); } -#ifdef CONFIG_FB_LOGO_CENTER - { + if (fb_center_logo) { int xres = info->var.xres; int yres = info->var.yres; @@ -520,11 +522,11 @@ static int fb_show_logo_line(struct fb_info *info, int rotate, --n; image.dx = (xres - n * (logo->width + 8) - 8) / 2; image.dy = y ?: (yres - logo->height) / 2; + } else { + image.dx = 0; + image.dy = y; } -#else - image.dx = 0; - image.dy = y; -#endif + image.width = logo->width; image.height = logo->height; @@ -684,9 +686,8 @@ int fb_prepare_logo(struct fb_info *info, int rotate) } height = fb_logo.logo->height; -#ifdef CONFIG_FB_LOGO_CENTER - height += (yres - fb_logo.logo->height) / 2; -#endif + if (fb_center_logo) + height += (yres - fb_logo.logo->height) / 2; return fb_prepare_extra_logos(info, height, yres); } diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig index 1e972c4e88b1..d1f6196c8b9a 100644 --- a/drivers/video/logo/Kconfig +++ b/drivers/video/logo/Kconfig @@ -10,15 +10,6 @@ menuconfig LOGO if LOGO -config FB_LOGO_CENTER - bool "Center the logo" - depends on FB=y - help - When this option is selected, the bootup logo is centered both - horizontally and vertically. If more than one logo is displayed - due to multiple CPUs, the collected line of logos is centered - as a whole. - config FB_LOGO_EXTRA bool depends on FB=y diff --git a/include/linux/fb.h b/include/linux/fb.h index 7cdd31a69719..f52ef0ad6781 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -653,6 +653,7 @@ extern int fb_new_modelist(struct fb_info *info); extern struct fb_info *registered_fb[FB_MAX]; extern int num_registered_fb; +extern bool fb_center_logo; extern struct class *fb_class; #define for_each_registered_fb(i) \