[PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[Peter Rosin]

On 2019-01-06 10:33, Geert Uytterhoeven wrote:
> 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?

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>
---
 Documentation/fb/fbcon.txt       |  6 ++++++
 drivers/video/fbdev/core/fbcon.c |  5 +++++
 drivers/video/fbdev/core/fbmem.c | 19 ++++++++++---------
 drivers/video/logo/Kconfig       |  9 ---------
 include/linux/fb.h               |  1 +
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/Documentation/fb/fbcon.txt b/Documentation/fb/fbcon.txt
index 62af30511a95..fb5fa0a8d553 100644
--- 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.
+
 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..552cfee63d76 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -510,6 +510,11 @@ static int __init fb_console_setup(char *this_opt)
 			continue;
 		}
 #endif
+
+		if (!strcmp(options, "center-logo")) {
+			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)		\
-- 
2.11.0

Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[Peter Rosin]

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

Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[Peter Rosin]

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

Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option

[Peter Rosin]

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>
---
 Documentation/fb/fbcon.txt       |  8 ++++++++
 drivers/video/fbdev/core/fbcon.c |  7 +++++++
 drivers/video/fbdev/core/fbmem.c | 19 ++++++++++---------
 drivers/video/logo/Kconfig       |  9 ---------
 include/linux/fb.h               |  1 +
 5 files changed, 26 insertions(+), 18 deletions(-)

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)		\
-- 
2.11.0

Re: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option

[Bartlomiej Zolnierkiewicz]

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

Re: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option

[Peter Rosin]

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

Re: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option

[Bartlomiej Zolnierkiewicz]

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