Hi

Am 23.09.26 um 22:10 schrieb Max Pedraza:
fb_prepare_logo() works out how many rows to keep clear for the logo and
fb_show_logo_line() works out where to draw it, and both of them open code
the same decision: centred if fb_center_logo, top left otherwise. The two
calculations have to agree, because fbcon erases whatever falls outside the
rows that were reserved, and nothing makes them.

Give the position a small structure of its own, with -1 on an axis meaning
centre on that axis, and have both callers ask for it. fb_center_logo
becomes {-1, -1} rather than a second path, and the arithmetic lives in one
function that both use, so they cannot end up disagreeing.

The structure and logo_place_axis() go in linux_logo.h rather than in the
frame buffer code, since where a logo goes is a property of the logo, not
of the one thing that draws it today.

No, the position is not a property of the logo. The code in linux_logo.h is only about the image itself.

In the case of fbdev, the policy for drawin the the image should be part of fbcon.

Best regards
Thomas


logo_place_axis() also clamps, which the open coded version did not: the
result is now always a position at which the logo lies entirely on screen.
That is not reachable today, since the only positions are the two the
console offers, but it stops being something the next caller has to
remember.

No functional change intended.

Signed-off-by: Max Pedraza <[email protected]>
---
  drivers/video/fbdev/core/fb_logo.c | 58 +++++++++++++++++++-----------
  include/linux/linux_logo.h         | 29 +++++++++++++++
  2 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/core/fb_logo.c 
b/drivers/video/fbdev/core/fb_logo.c
index 0bab8352b6..5ec9f9554f 100644
--- a/drivers/video/fbdev/core/fb_logo.c
+++ b/drivers/video/fbdev/core/fb_logo.c
@@ -8,6 +8,17 @@
  bool fb_center_logo __read_mostly;
  int fb_logo_count __read_mostly = -1;
+/*
+ * The placement in effect, as asked for on the console command line.
+ */
+static const struct logo_placement *fb_logo_placement(void)
+{
+       static const struct logo_placement centred = { .x = -1, .y = -1 };
+       static const struct logo_placement top_left = { };
+
+       return fb_center_logo ? &centred : &top_left;
+}
+
  static inline unsigned int safe_shift(unsigned int d, int n)
  {
        return n < 0 ? d >> -n : d << n;
@@ -281,7 +292,11 @@ static int fb_show_logo_line(struct fb_info *info, int 
rotate,
  {
        u32 *palette = NULL, *saved_pseudo_palette = NULL;
        unsigned char *logo_new = NULL, *logo_rotate = NULL;
+       const struct logo_placement *p;
+       unsigned int xres = info->var.xres;
+       unsigned int yres = info->var.yres;
        struct fb_image image;
+       unsigned int block;
/* Return if the frame buffer is not mapped or suspended */
        if (logo == NULL || info->state != FBINFO_STATE_RUNNING ||
@@ -322,26 +337,22 @@ static int fb_show_logo_line(struct fb_info *info, int 
rotate,
                fb_set_logo(info, logo, logo_new, fb_logo.depth);
        }
- if (fb_center_logo) {
-               int xres = info->var.xres;
-               int yres = info->var.yres;
+       image.width = logo->width;
+       image.height = logo->height;
- if (rotate == FB_ROTATE_CW || rotate == FB_ROTATE_CCW) {
-                       xres = info->var.yres;
-                       yres = info->var.xres;
-               }
+       if (rotate == FB_ROTATE_CW || rotate == FB_ROTATE_CCW)
+               swap(xres, yres);
- 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;
-       }
+       while (n && (n * (logo->width + 8) - 8 > xres))
+               --n;
- image.width = logo->width;
-       image.height = logo->height;
+       /* The copies are drawn in a row, so they are centred as one block */
+       block = n ? n * (logo->width + 8) - 8 : logo->width;
+
+       p = fb_logo_placement();
+       image.dx = logo_place_axis(p->x, xres, block);
+       /* A stacked logo goes where the caller put it */
+       image.dy = y ? y : logo_place_axis(p->y, yres, image.height);
if (rotate) {
                logo_rotate = kmalloc_array(logo->width, logo->height,
@@ -418,6 +429,7 @@ static int fb_show_extra_logos(struct fb_info *info, int y, 
int rotate)
  int fb_prepare_logo(struct fb_info *info, int rotate)
  {
        int depth = fb_get_color_depth(&info->var, &info->fix);
+       const struct logo_placement *p;
        unsigned int yres;
        int height;
@@ -480,9 +492,15 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
                }
        }
- height = fb_logo.logo->height;
-       if (fb_center_logo)
-               height += (yres - fb_logo.logo->height) / 2;
+       /*
+        * fbcon only leaves the first @height rows of the screen alone, so a
+        * logo placed further down would be drawn and then immediately
+        * cleared. Ask the same placement fb_show_logo_line() will use, so
+        * that the two cannot disagree.
+        */
+       p = fb_logo_placement();
+       height = logo_place_axis(p->y, yres, fb_logo.logo->height) +
+                fb_logo.logo->height;
  #ifdef CONFIG_FB_LOGO_EXTRA
        height = fb_prepare_extra_logos(info, height, yres);
  #endif
diff --git a/include/linux/linux_logo.h b/include/linux/linux_logo.h
index 1e727a2cb4..b3b15d3800 100644
--- a/include/linux/linux_logo.h
+++ b/include/linux/linux_logo.h
@@ -13,6 +13,8 @@
   */
#include <linux/init.h>
+#include <linux/minmax.h>
+#include <linux/types.h>
#define LINUX_LOGO_MONO 1 /* monochrome black/white */
@@ -36,6 +38,33 @@ extern const struct linux_logo logo_linux_clut224;
  extern const struct linux_logo logo_spe_clut224;
extern const struct linux_logo *fb_find_logo(int depth);
+
+/*
+ * Where a boot logo goes. A coordinate of -1 centres the logo on that axis.
+ * Whatever draws the logo describes its placement this way and computes it
+ * with logo_place_axis(), so that no two places can end up disagreeing about
+ * where the logo is.
+ */
+struct logo_placement {
+       s32 x, y;
+};
+
+/*
+ * Place a logo along one axis. @pos is the coordinate asked for, or -1 to
+ * centre on that axis. The result is clamped so that the logo always lies
+ * entirely within the screen: the drawing code does not clip, so asking for
+ * more than that would otherwise scribble past the end of the frame buffer.
+ */
+static inline int logo_place_axis(s32 pos, unsigned int span, unsigned int 
size)
+{
+       int last = (int)span - (int)size;
+
+       if (size > span)
+               return 0;
+
+       return pos == -1 ? last / 2 : clamp(pos, 0, last);
+}
+
  #ifdef CONFIG_FB_LOGO_EXTRA
  extern void fb_append_extra_logo(const struct linux_logo *logo,
                                 unsigned int n);

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)


Reply via email to