| Message ID | 20181126215725.2548-3-peda@axentia.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add config option to center the bootup logo | expand |
On 11/26/2018 10:57 PM, Peter Rosin 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> Patch queued for 4.21, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi Peter, 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? Gr{oetje,eeting}s, Geert
Hi Peter, 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? Gr{oetje,eeting}s, Geert
On 2019-01-07 09:40, Geert Uytterhoeven wrote: > Hi Peter, > > 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? Cheers, Peter
On 2019-01-07 09:59, Peter Rosin wrote: > On 2019-01-07 09:40, Geert Uytterhoeven wrote: >> Hi Peter, >> >> 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. Cheers, Peter
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. Gr{oetje,eeting}s, Geert
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 43d4047f472a..fdbad029e5e6 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -502,8 +502,25 @@ 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 + { + int xres = info->var.xres; + int yres = info->var.yres; + + if (rotate == FB_ROTATE_CW || rotate == FB_ROTATE_CCW) { + xres = info->var.yres; + yres = info->var.xres; + } + + while (n && (n * (logo->width + 8) - 8 > xres)) + --n; + image.dx = (xres - n * (logo->width + 8) - 8) / 2; + image.dy = y ?: (yres - logo->height) / 2; + } +#else image.dx = 0; image.dy = y; +#endif image.width = logo->width; image.height = logo->height; @@ -600,6 +617,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate) { int depth = fb_get_color_depth(&info->var, &info->fix); unsigned int yres; + int height; memset(&fb_logo, 0, sizeof(struct logo_data)); @@ -661,7 +679,12 @@ int fb_prepare_logo(struct fb_info *info, int rotate) } } - return fb_prepare_extra_logos(info, fb_logo.logo->height, yres); + height = fb_logo.logo->height; +#ifdef CONFIG_FB_LOGO_CENTER + height += (yres - fb_logo.logo->height) / 2; +#endif + + return fb_prepare_extra_logos(info, height, yres); } int fb_show_logo(struct fb_info *info, int rotate) diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig index d1f6196c8b9a..1e972c4e88b1 100644 --- 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. + config FB_LOGO_EXTRA bool depends on FB=y
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> --- drivers/video/fbdev/core/fbmem.c | 25 ++++++++++++++++++++++++- drivers/video/logo/Kconfig | 9 +++++++++ 2 files changed, 33 insertions(+), 1 deletion(-)