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@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: >>>>>> On 2019-01-06 10:33, Geert Uytterhoeven wrote: >>>>>>> On Mon, Nov 26, 2018 at 10:59 PM Peter Rosin <peda@xxxxxxxxxx> 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@xxxxxxxxxx> >>>>>>> >>>>>>>> --- 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@xxxxxxxxxx> >>>>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> >>>>>> Cc: Jonathan Corbet <corbet@xxxxxxx> >>>>>> Cc: Peter Rosin <peda@xxxxxxxxxx> >>>>>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx >>>>>> Cc: linux-fbdev@xxxxxxxxxxxxxxx >>>>>> Cc: linux-doc@xxxxxxxxxxxxxxx >>>>>> To: linux-kernel@xxxxxxxxxxxxxxx >>>>>> 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@xxxxxxxxxxxxxx> >>>>>> Signed-off-by: Peter Rosin <peda@xxxxxxxxxx> >>>>> >>>>> 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@xxxxxxxxxx> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx> > Cc: Peter Rosin <peda@xxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: linux-fbdev@xxxxxxxxxxxxxxx > Cc: linux-doc@xxxxxxxxxxxxxxx > To: linux-kernel@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxx> > Signed-off-by: Peter Rosin <peda@xxxxxxxxxx> Patch queued for 5.0, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics