Re: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

Re: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `memory-region` alternative is completely broken due to conflicting top-level `required` constraints and a missing property definition.
--

--- Patch [1]: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo ---
commit f24f24c36e68703fb2282044b3ecdd6fe92fa5c4
Author: Max Pedraza <maximpedraza@gmail.com>

dt-bindings: display: add a device tree supplied boot logo

Embedded products frequently need a product specific boot logo. This commit
adds a binding for a "linux,boot-logo-clut224" node to allow the boot logo
to be described by the device tree instead of baked into the kernel. The
image can live in the device tree directly or in a reserved memory region.

> diff --git a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml

[ ... ]

> +required:
> +  - compatible
> +  - width
> +  - height
> +  - clut
> +  - data

[Severity: Medium]
Does this global required block conflict with the oneOf block below?

Since width, height, clut, and data are unconditionally required here, any
node attempting to use the memory-region alternative will fail schema
validation because it lacks these globally required properties.

> +
> +# The image either lives in the device tree or in a reserved memory region,
> +# never both.
> +oneOf:
> +  - required:
> +      - width
> +      - height
> +      - clut
> +      - data
> +  - required:
> +      - memory-region
> +
> +additionalProperties: false

[Severity: Medium]
Can this schema successfully validate a node using a memory-region?

Since memory-region is missing from the main properties block, setting
additionalProperties to false here will cause validation to reject any node
that includes it, completely preventing this configuration alternative from
being used.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804225617.264861-1-maximpedraza@gmail.com?part=1

Re: [PATCH v2 3/6] fbdev: honour the device tree boot logo placement properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unvalidated device tree coordinates cause out-of-bounds memory accesses in the framebuffer.
- [High] DT logo placement logic ignores SMP logo count and extra logos, breaking centering and overlapping extra logos.
- [High] fb_logo_dt_reserve() calculates logical console space reservations using the incorrect physical axis during screen rotation.
- [High] Overriding logo rotation via device tree breaks the reservation coordinate space contract with fbcon.
- [Medium] TOC-TOU initialization race on `read_done` static flag in `fb_logo_dt_read`.
--

--- Patch [3]: [PATCH v2 3/6] fbdev: honour the device tree boot logo placement properties ---
commit bd67300c4f145f9b35c28ceac495f14e402f271d
Author: Max Pedraza <maximpedraza@gmail.com>

fbdev: honour the device tree boot logo placement properties

A logo supplied by the device tree describes a specific product, so where
it lands on the panel matters in a way it does not for the built-in Tux.
Honour the optional "logo-position", "logo-offset", "logo-centered" and
"logo-rotation" properties of the "linux,boot-logo-clut224" node.

> diff --git a/drivers/video/fbdev/core/fb_logo.c b/drivers/video/fbdev/core/fb_logo.c
> --- a/drivers/video/fbdev/core/fb_logo.c
> +++ b/drivers/video/fbdev/core/fb_logo.c
> @@ -2,12 +2,178 @@
>  
>  #include <linux/fb.h>
>  #include <linux/linux_logo.h>
> +#include <linux/of.h>

[ ... ]

> +static void fb_logo_dt_read(void)
> +{
> +	struct fb_logo_dt_placement *p = &fb_logo_dt;
> +	static bool read_done;
> +	struct device_node *np;
> +	const char *rotation;
> +	u32 val[2];
> +	int rot;
> +
> +	if (read_done)
> +		return;
> +
> +	read_done = true;
> +
> +	np = of_find_compatible_node(NULL, NULL, "linux,boot-logo-clut224");

[Severity: Medium]
Could this lead to a race condition if called concurrently?

By setting read_done to true before the fb_logo_dt structure is actually
populated, could another thread see read_done as true and proceed to use an
uninitialized structure?

[ ... ]

> +/* Top edge of the logo, in the coordinate space the caller works in */
> +static int fb_logo_dt_top(unsigned int yres, unsigned int logo_height)
> +{
> +	struct fb_logo_dt_placement *p = &fb_logo_dt;
> +	int top;
> +
> +	if (p->centered)
> +		top = ((int)yres - (int)logo_height) / 2;
> +	else if (p->has_position)
> +		top = p->y;
> +	else
> +		top = 0;
> +
> +	return max(top + p->offset_y, 0);
> +}

[Severity: High]
When the screen is hardware-rotated (for example, CW or CCW), does returning
the physical Y coordinate here result in a broken reservation geometry?

fb_prepare_logo() needs the reservation height in the logical coordinate space
of the console. When rotated, the console's logical vertical axis maps to the
hardware's horizontal X-axis, but this logic unconditionally uses the physical
Y position (p->y).

> +static void fb_logo_dt_place(struct fb_info *info, struct fb_image *image)
> +{
> +	struct fb_logo_dt_placement *p = &fb_logo_dt;
> +	int dx;
> +
> +	fb_logo_dt_read();
> +
> +	if (!p->valid)
> +		return;
> +
> +	if (p->centered)
> +		dx = ((int)info->var.xres - (int)image->width) / 2;
> +	else if (p->has_position)
> +		dx = p->x;
> +	else
> +		dx = image->dx;
> +
> +	image->dx = max(dx + p->offset_x, 0);
> +	image->dy = fb_logo_dt_top(info->var.yres, image->height);
> +}

[Severity: High]
Does this logic break the centering of SMP logos and the vertical stacking of
extra logos?

When rendering multiple logos, fb_show_logo_line() calculates a block-centered
X coordinate for the SMP penguins and an accumulated Y coordinate to stack any
extra logos.

By calculating dx using only a single image->width, this seems to break SMP
block centering. Additionally, overwriting image->dy with a static position
forces all subsequent extra logos to render at the exact same vertical position
causing them to overlap.

[Severity: High]
Can these unvalidated device tree coordinates cause an out-of-bounds memory
access in the framebuffer?

fb_logo_dt_place() applies the device tree positions directly to image->dx
and image->dy without clamping them to the actual screen dimensions.

If a large offset is provided, would fb_do_show_logo() process the unvalidated
Y coordinate? For example, in the FB_ROTATE_UR case:

    for (x = 0;
         x < num && image->dx + image->width <= info->var.xres;
         x++) {
            info->fbops->fb_imageblit(info, image);

The loop checks the X bounds but lacks checks for image->dy + image->height
against info->var.yres, potentially passing arbitrarily large Y coordinates
to the driver's fb_imageblit callback.

[ ... ]

> @@ -423,6 +592,8 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
>  
>  	memset(&fb_logo, 0, sizeof(struct logo_data));
>  
> +	rotate = fb_logo_dt_rotation(rotate);
> +
>  	if (info->flags & FBINFO_MISC_TILEBLITTING ||
>  	    info->fbops->owner || !fb_logo_count)
>  		return 0;

[Severity: High]
Does overriding the caller's rotation parameter here break the coordinate space
agreement with the fbcon subsystem?

The fbcon subsystem calls fb_prepare_logo() with its own console rotation state
and expects the returned reservation height to scale accurately against its
logical vertical axis. If fb_logo_dt_rotation() overrides this with a
differing rotation from the device tree, will fbcon incorrectly interpret the
resulting reservation height against its differing logical axis?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804225617.264861-1-maximpedraza@gmail.com?part=3

Re: [PATCH v2 4/6] dt-bindings: display: allow the boot logo in a reserved memory region

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Arbitrary boot logo placement via device tree allows out-of-bounds memory writes in framebuffer drivers.
- [Medium] The YAML schema enforces contradictory constraints by unconditionally requiring inline image properties, making the new `memory-region` property impossible to use without failing validation.
- [Medium] The kernel driver lacks any C implementation to parse or map the newly added `memory-region` property.
- [Medium] Using `kmalloc` for potentially large image allocations will fail for sizes exceeding `KMALLOC_MAX_SIZE`.
--

--- Patch [4]: [PATCH v2 4/6] dt-bindings: display: allow the boot logo in a reserved memory region ---
commit 53263fda0e15a2f2d945eb9940e2236f6120727d
Author: Max Pedraza <maximpedraza@gmail.com>

dt-bindings: display: allow the boot logo in a reserved memory region

This commit extends the boot logo device tree binding to support loading
the image from a reserved memory region. It adds a memory-region property
as a mutually exclusive alternative to providing the image data inline.

> Carrying the image in the device tree ties it to the device tree, but the
> image and where it goes on screen are independent axes of variation. One
> board sold to several customers wants several device trees that differ in
> the logo; one customer with several products built on that board wants the
> same logo placed differently on each panel. The second case would otherwise
> mean duplicating the same image into every device tree.

[Severity: Medium]
Can allocating these large images fail?  Using kmalloc() for potentially
large image allocations up to the 32MB LOGO_DT_MAX_PIXELS limit will fail
for sizes exceeding KMALLOC_MAX_SIZE:

drivers/video/logo/logo.c:logo_dt_parse() {
	...
	data = kmalloc(npixels, GFP_KERNEL);
	if (!data) {
		ret = -ENOMEM;
		goto err_free_clut;
	}
	...
}

> 
> Let the node point at a reserved memory region filled in by the bootloader
> instead, so one image can be shared by device trees that differ only in
> placement. The region starts with a small header carrying a magic number
> and the geometry, so the kernel can tell a logo from an empty or stale
> region and bounds check everything against the reservation.

[Severity: Medium]
Is the parsing logic for this feature missing from this patch series?  The
kernel driver lacks any C implementation to parse or map the newly added
memory-region property.

In drivers/video/logo/logo.c:logo_dt_parse(), it unconditionally checks
for the inline properties and returns an error if they are absent, with no
code to handle the memory-region property:

drivers/video/logo/logo.c:logo_dt_parse() {
	...
	ret = of_property_read_u32(np, "width", &width);
	if (ret)
		return ret;
	...
}

[Severity: High]
Are the placement coordinates actually bounds-checked?  Arbitrary boot
logo placement via device tree allows out-of-bounds memory writes in
framebuffer drivers.

In fb_logo_dt_place(), unvalidated coordinates from the DT are assigned
to image->dy:

drivers/video/fbdev/core/fb_logo.c:fb_logo_dt_place() {
	...
	image->dx = max(dx + p->offset_x, 0);
	image->dy = fb_logo_dt_top(info->var.yres, image->height);
}

In fb_do_show_logo(), the bounds checks are completely missing for the Y
axis depending on rotation:

drivers/video/fbdev/core/fb_logo.c:fb_do_show_logo() {
	...
	if (rotate == FB_ROTATE_UR) {
		for (x = 0;
		     x < num && image->dx + image->width <= info->var.xres;
		     x++) {
			info->fbops->fb_imageblit(info, image);
	...
}

When fb_imageblit() is called, the driver computes an address using dy,
which will write far outside the framebuffer limits.

[ ... ]

> diff --git a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
> index a6a2069648359..7aec0cc2d3776 100644
> --- a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
> +++ b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
> @@ -59,6 +59,23 @@ properties:
>        index into the colour lookup table. The property length must be equal to
>        width multiplied by height.
>  
> +  memory-region:
> +    maxItems: 1

[Severity: Medium]
Will this property always fail validation?  The top-level required list
permanently mandates width, height, clut, and data.  If a device
tree node is added using only memory-region, it fails the global
required check:

Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml:
required:
  - compatible
  - width
  - height
  - clut
  - data

If a node provides both, it fails the oneOf constraint since the subschemas
become mutually exclusive.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804225617.264861-1-maximpedraza@gmail.com?part=4

Re: [PATCH v2 2/6] video: logo: allow the boot logo to come from the device tree

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Premature freeing of device tree boot logo memory creates a race condition with asynchronous framebuffer probes, leading to a NULL pointer dereference.
- [Medium] Unbounded kmalloc() allocation for device tree boot logo triggers a kernel warning and fails for sizes exceeding KMALLOC_MAX_SIZE.
- [Medium] Silent fallback to searching the device tree root node if the /chosen node does not exist.
--

--- Patch [2]: [PATCH v2 2/6] video: logo: allow the boot logo to come from the device tree ---
commit f460a2fda249375abd332c49e9ef32c7cd3a3f7b
Author: Max Pedraza <maximpedraza@gmail.com>

video: logo: allow the boot logo to come from the device tree

Add CONFIG_LOGO_DT_CLUT224, which makes fb_find_logo() look for a node
compatible with "linux,boot-logo-clut224" under /chosen before falling
back to the logos built into the kernel image.

> diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
> index 91535f8848da9..7f8b04ecfa987 100644
> --- a/drivers/video/logo/logo.c
> +++ b/drivers/video/logo/logo.c

[ ... ]

> +static int logo_dt_parse(struct device_node *np)
> +{
> +	unsigned int clutsize, npixels, i;
> +	unsigned char *clut, *data;
> +	u32 width, height;
> +	int len, ret;
> +
> +	ret = of_property_read_u32(np, "width", &width);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "height", &height);
> +	if (ret)
> +		return ret;
> +
> +	if (!width || !height || (u64)width * height > LOGO_DT_MAX_PIXELS)
> +		return -EINVAL;
> +
> +	npixels = width * height;
> +
> +	len = of_property_count_u8_elems(np, "clut");
> +	if (len < 3 || len % 3)
> +		return -EINVAL;
> +
> +	clutsize = len / 3;
> +	if (clutsize > LOGO_DT_MAX_CLUT)
> +		return -EINVAL;
> +
> +	ret = of_property_count_u8_elems(np, "data");
> +	if (ret < 0)
> +		return ret;
> +	if ((unsigned int)ret != npixels)
> +		return -EINVAL;
> +
> +	clut = kmalloc(len, GFP_KERNEL);
> +	if (!clut)
> +		return -ENOMEM;
> +
> +	data = kmalloc(npixels, GFP_KERNEL);

[Severity: Medium]
Could this allocation trigger a kernel warning (WARN_ON_ONCE_GFP) and fail if
the device tree provides a valid boot logo larger than KMALLOC_MAX_SIZE?
LOGO_DT_MAX_PIXELS is 32MB, but kmalloc() without __GFP_NOWARN will warn if
it exceeds the maximum contiguous allocation size. Does this code need to use
kvmalloc() instead, or restrict the size limit to KMALLOC_MAX_SIZE?

> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto err_free_clut;
> +	}
> +
> +	ret = of_property_read_u8_array(np, "clut", clut, len);

[ ... ]

> +static const struct linux_logo *logo_dt_find(void)
> +{
> +	static bool probed;
> +	struct device_node *np;
> +	int ret;
> +
> +	if (probed)
> +		return logo_dt_data ? &logo_dt_clut224 : NULL;
> +
> +	probed = true;
> +
> +	np = of_get_compatible_child(of_chosen, LOGO_DT_COMPATIBLE);

[Severity: Medium]
If the device tree lacks a /chosen node, of_chosen will be NULL. In that
scenario, wouldn't of_get_compatible_child() silently fall back to iterating
the children of the root node (of_root)? This seems to bypass the strict
constraint mentioned in the commit message that the node must be located under
/chosen.

> +	if (!np)
> +		return NULL;
> +
> +	if (of_device_is_available(np)) {
> +		ret = logo_dt_parse(np);
> +		if (ret)
> +			pr_warn("logo: ignoring malformed %pOF node (%d)\n",
> +				np, ret);
> +	}
> +
> +	of_node_put(np);

[ ... ]

> @@ -32,6 +184,7 @@ static bool logos_freed;
>  static int __init fb_logo_late_init(void)
>  {
>  	logos_freed = true;
> +	logo_dt_free();

[Severity: High]
Can this introduce a race condition resulting in a NULL pointer dereference
or use-after-free for framebuffer drivers that use async probing?

fb_logo_late_init() executes as a late_initcall_sync during do_initcalls(),
which happens before async_synchronize_full() guarantees async probes are
complete. If an async probe caches the logo pointer via fb_find_logo(), and
then fb_logo_late_init() frees the memory and clears the pointers, the async
probe might dereference the freed memory later in fb_show_logo().

>  	return 0;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804225617.264861-1-maximpedraza@gmail.com?part=2

[PATCH v2 0/6] Boot logo supplied by the device tree

[Max Pedraza]

This series lets the logo be described by the device tree instead: a node
compatible with "linux,boot-logo-clut224" under /chosen supplies the image in
the same paletted format the built-in CLUT224 logos already use, and the kernel
prefers it over the built-in ones when it is present and enabled. If the
node is absent or disabled, nothing changes.

The image can come from the node itself (patches 1-2) or from a reserved
memory region the bootloader filled in (patches 4-5), because the image and
its placement are independent axes of variation. One board sold to several
customers wants several device trees differing in the logo. One customer
with several products built on that board, with different panels, wants the
same logo placed differently on each: there the image belongs in a shared
binary and only the placement belongs in the device tree. Patch 3 adds that
placement.

We have been carrying a cruder version of this downstream on an AM335x
product since 2020, across a handful of board revisions, and it has removed
a real maintenance burden for us. This is an attempt to find out whether
something along these lines is wanted upstream, and if so in what shape --
hence RFC.

The v1 discussion turned on the obvious objection, that a bitmap is not
hardware and the device tree is not where it belongs. Geert pointed out that
configuration which is not hardware description goes under /chosen, and that
Open Firmware, which the device tree descends from, already carried a boot
logo in that spirit as the oem-logo variable under /options. The node has
moved to /chosen accordingly, which is also where simple-framebuffer nodes
live, for the same reason: they describe what firmware handed over rather
than what the hardware is.

Patches 4 and 5 are separable; the series is useful without them if the
reserved memory path is thought to be one mechanism too many.

Not included, though it exists in our downstream version: a hook in
drm_fb_helper so the logo is drawn when CONFIG_FRAMEBUFFER_CONSOLE is
disabled, which is the common case for a product that only wants a splash
screen. Today fb_show_logo() is reached from fbcon alone. That is a DRM
question rather than an fbdev one and deserves a separate posting.

Notes on the binding:

  - The palette size is derived from the length of the "clut" property
    instead of being a separate property, so it cannot disagree with the
    palette actually supplied.
  - "data" holds plain palette indices. The 32 entry offset the frame
    buffer layer reserves for the console is an implementation detail and
    is applied by the kernel.
  - "logo-position" and "logo-offset" are spelled with the prefix because
    plain "position" and "offset" are already used elsewhere in the tree
    with an incompatible type, which dtschema rejects.
  - "logo-centered" overlaps with the existing fb_center_logo, but it is
    per device tree rather than per fbcon command line, and it composes
    with "logo-offset". That combination is what panels with a partially
    visible area need, where the usable region is not the centre of the
    mode; the hardware this came from is an 800x480 panel of which only the
    bottom 320 rows are visible.
  - The reserved memory path takes a "memory-region" phandle rather than a
    bare address. The reservation is what makes the memory safe to read at
    all, and it is what gives the kernel a size to bounds check against.

The byte arrays are not written by hand: patch 6 adds ppmtodtlogo, a host
tool along the lines of the existing pnmtologo -- plain C, no dependencies,
no quantization of its own -- that turns a PPM image into the node or into
the memory region blob. It is what produced everything tested below.

Testing: build tested for arm with CONFIG_LOGO_DT_CLUT224 both enabled and
disabled, W=1 clean, checkpatch --strict clean, dt_binding_check clean. The
schema was also checked against deliberately malformed nodes, including
supplying both image sources at once and neither.

Boot tested under qemu-system-arm -M versatilepb with PL111 and fbcon, at
16bpp, both ways round. A 160x120 logo placed with "logo-centered" plus a
"logo-offset" of <0 120> lands at exactly the expected coordinates, and every
pixel matches the source image once the source is truncated to RGB565.
Supplying the same image through a reserved region instead produces a screen
whose logo area is identical. Blobs with a bad magic, a geometry larger than
the reservation and an out of range pixel are each rejected with a warning
and fall back to the built-in logo, with no crash.

That boot test earned its keep: the first version of patch 3 drew the logo
correctly and then had it erased, because fb_prepare_logo() still reserved
only the logo's own height while the logo itself had been moved further
down. Nothing in the build or the static checks catches that.

Also boot tested on real hardware with this exact tree: an AM335x board
(tilcdc) with an 800x480 panel, running the master this series is based on.
The product logo comes up where expected both ways, carried in the device
tree and taken from a bootloader-loaded reserved memory region, with the node
under /chosen and nothing reported in either case. That system
builds without CONFIG_FRAMEBUFFER_CONSOLE, so the drawing there goes through
the separate drm_fb_helper hook mentioned above as not included; the parsing,
validation and placement exercised are those of this series.

Changes since v1:
  - Rebased onto Linus' master; v1 was based on the v6.19.14 stable release
    and did not apply anywhere useful. Sorry for the noise.
  - The node moved under /chosen, per Geert's observation that this is where
    non-hardware configuration belongs. The binding and the tool follow.
  - The binding notes that a product logo is normally covered by trademark or
    copyright of its owner and that such a device tree is expected to be kept
    with the product, after Ulrich raised the licensing angle.
  - Dropped the second binding example: with the node under /chosen both
    examples define /chosen/logo and dtc merges them, which then fails the
    oneOf. The reserved memory form is documented on the property itself.

Max Pedraza (6):
  dt-bindings: display: add a device tree supplied boot logo
  video: logo: allow the boot logo to come from the device tree
  fbdev: honour the device tree boot logo placement properties
  dt-bindings: display: allow the boot logo in a reserved memory region
  video: logo: allow the boot logo to come from a reserved memory region
  video: logo: add ppmtodtlogo host tool

 .../display/linux,boot-logo-clut224.yaml      | 152 +++++++
 MAINTAINERS                                   |   1 +
 drivers/video/fbdev/core/fb_logo.c            | 174 ++++++++
 drivers/video/logo/Kconfig                    |  13 +
 drivers/video/logo/Makefile                   |   6 +-
 drivers/video/logo/logo.c                     | 256 +++++++++++
 drivers/video/logo/ppmtodtlogo.c              | 402 ++++++++++++++++++
 7 files changed, 1003 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
 create mode 100644 drivers/video/logo/ppmtodtlogo.c

-- 
2.39.5

[PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

[Max Pedraza]

Embedded products frequently need a product specific boot logo. Today the
only way to get one is to replace one of the logo_*_clut224.ppm files in
the kernel source tree, which bakes the image into the kernel image: a
change of branding, or a second product sharing the same board support,
requires a separate kernel build.

Add a binding for a "linux,boot-logo-clut224" node, which carries the logo
in the same paletted format the built-in CLUT224 logos already use, plus a
few optional properties describing where on the screen it is drawn.

The palette size is derived from the length of the "clut" property rather
than spelled out separately, and "data" holds plain palette indices; the
32 entry offset the frame buffer layer reserves for the console is an
implementation detail and is applied by the kernel, not by the binding.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 .../display/linux,boot-logo-clut224.yaml      | 131 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml

diff --git a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
new file mode 100644
index 000000000..a6a206964
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/linux,boot-logo-clut224.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Boot logo supplied by the device tree
+
+maintainers:
+  - Max Pedraza <maximpedraza@gmail.com>
+
+description: |
+  Embedded systems commonly need a product specific boot logo. Today that
+  means pointing CONFIG_LOGO_LINUX_CLUT224_FILE at a different image, which
+  bakes it into the kernel image: changing the logo means building and
+  deploying a new kernel.
+
+  This node lets the boot logo be described by the device tree instead, so that
+  a single kernel image can serve several products, or several revisions of the
+  same product, that only differ in branding.
+
+  Since a logo is configuration rather than a description of the hardware, the
+  node lives under /chosen, next to the other things firmware hands to the
+  operating system. Open Firmware, which the device tree descends from, carried
+  a boot logo in the same spirit as the oem-logo variable under /options.
+
+  The image is stored in the same paletted format the in-kernel CLUT224 logos
+  use: a palette of at most 224 RGB entries plus one palette index per pixel.
+
+properties:
+  compatible:
+    const: linux,boot-logo-clut224
+
+  width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Logo width in pixels.
+    minimum: 1
+    maximum: 65535
+
+  height:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Logo height in pixels.
+    minimum: 1
+    maximum: 65535
+
+  clut:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description:
+      Colour lookup table, as consecutive red, green and blue bytes per entry.
+      The number of entries is derived from the property length and must not
+      exceed 224.
+    minItems: 3
+    maxItems: 672
+
+  data:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description:
+      One byte per pixel, left to right and top to bottom, each byte being an
+      index into the colour lookup table. The property length must be equal to
+      width multiplied by height.
+
+  logo-position:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      X and Y coordinates, in pixels, of the top left corner of the logo.
+      Defaults to the top left corner of the screen.
+    items:
+      - description: X coordinate
+      - description: Y coordinate
+
+  logo-offset:
+    $ref: /schemas/types.yaml#/definitions/int32-array
+    description:
+      X and Y displacement, in pixels, applied after the logo has been placed.
+      Mostly useful together with logo-centered.
+    items:
+      - description: X displacement
+      - description: Y displacement
+
+  logo-centered:
+    type: boolean
+    description:
+      Centre the logo on the screen, overriding logo-position.
+
+  logo-rotation:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Rotation applied to the logo before it is drawn.
+    enum: [none, cw, ccw, ud]
+    default: none
+
+required:
+  - compatible
+  - width
+  - height
+  - clut
+  - data
+
+# The image either lives in the device tree or in a reserved memory region,
+# never both.
+oneOf:
+  - required:
+      - width
+      - height
+      - clut
+      - data
+  - required:
+      - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    // A 4x2 logo using three colours, centred on the screen.
+    / {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        chosen {
+            logo {
+                compatible = "linux,boot-logo-clut224";
+                width = <4>;
+                height = <2>;
+                clut = /bits/ 8 <0xff 0x00 0x00
+                                 0x00 0xff 0x00
+                                 0x00 0x00 0xff>;
+                data = /bits/ 8 <0x00 0x01 0x01 0x00
+                                 0x02 0x00 0x00 0x02>;
+                logo-centered;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 5114e6db7..8cf5163b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10269,6 +10269,7 @@ L:	dri-devel@lists.freedesktop.org
 S:	Maintained
 Q:	http://patchwork.kernel.org/project/linux-fbdev/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git
+F:	Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
 F:	Documentation/fb/
 F:	drivers/video/
 F:	include/linux/fb.h
-- 
2.39.5

[PATCH v2 2/6] video: logo: allow the boot logo to come from the device tree

[Max Pedraza]

Add CONFIG_LOGO_DT_CLUT224, which makes fb_find_logo() look for a node
compatible with "linux,boot-logo-clut224" under /chosen before falling
back to the logos built into the kernel image.

The image is validated before it is used: the palette must have at most
224 entries, the pixel data length must match the geometry, and every
pixel must reference an entry that exists. A malformed node is reported
and ignored rather than drawn, so a bad device tree cannot take the
display down with it.

The image is copied out of the device tree so that the 32 entry offset
the frame buffer layer reserves for the console can be applied to the
pixels, and the copy is released from fb_logo_late_init() alongside the
built-in logos.

The node lives under /chosen because a logo is configuration handed over
by firmware rather than a description of the hardware, which is also
where simple-framebuffer nodes live for the same reason.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 drivers/video/logo/Kconfig |  12 +++
 drivers/video/logo/logo.c  | 160 +++++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+)

diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig
index cda15b958..215afa7ef 100644
--- a/drivers/video/logo/Kconfig
+++ b/drivers/video/logo/Kconfig
@@ -76,4 +76,16 @@ config LOGO_LINUX_CLUT224_FILE
 
 	    magick source_image -compress none -colors 224 destination.ppm
 
+config LOGO_DT_CLUT224
+	bool "224-color logo supplied by the device tree"
+	depends on OF
+	help
+	  Look for a boot logo in the device tree, in a node compatible with
+	  "linux,boot-logo-clut224" under /chosen, instead of using one of
+	  the logos built into the kernel image. This allows a single kernel
+	  image to be used by several products that only differ in branding.
+
+	  If no such node is present, or it is disabled, the built-in logo
+	  selected above is used, so saying Y here is safe.
+
 endif # LOGO
diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
index 91535f884..7f8b04ecf 100644
--- a/drivers/video/logo/logo.c
+++ b/drivers/video/logo/logo.c
@@ -11,6 +11,9 @@
  */
 
 #include <linux/linux_logo.h>
+#include <linux/of.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
 #include <linux/stddef.h>
 #include <linux/module.h>
 
@@ -22,6 +25,155 @@ static bool nologo;
 module_param(nologo, bool, 0);
 MODULE_PARM_DESC(nologo, "Disables startup logo");
 
+#ifdef CONFIG_LOGO_DT_CLUT224
+
+#define LOGO_DT_COMPATIBLE	"linux,boot-logo-clut224"
+#define LOGO_DT_MAX_CLUT	224
+/*
+ * The first 32 palette entries are reserved for the console, so the logo
+ * colours start at index 32. That is an implementation detail of the frame
+ * buffer layer rather than a property of the image, so the device tree stores
+ * plain indices and the offset is applied here.
+ */
+#define LOGO_DT_CLUT_OFFSET	32
+/* Sanity limit on the image size, a device tree is not a good place for more */
+#define LOGO_DT_MAX_PIXELS	SZ_32M
+
+static struct linux_logo logo_dt_clut224 = {
+	.type		= LINUX_LOGO_CLUT224,
+};
+
+static unsigned char *logo_dt_clut;
+static unsigned char *logo_dt_data;
+
+static int logo_dt_parse(struct device_node *np)
+{
+	unsigned int clutsize, npixels, i;
+	unsigned char *clut, *data;
+	u32 width, height;
+	int len, ret;
+
+	ret = of_property_read_u32(np, "width", &width);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "height", &height);
+	if (ret)
+		return ret;
+
+	if (!width || !height || (u64)width * height > LOGO_DT_MAX_PIXELS)
+		return -EINVAL;
+
+	npixels = width * height;
+
+	len = of_property_count_u8_elems(np, "clut");
+	if (len < 3 || len % 3)
+		return -EINVAL;
+
+	clutsize = len / 3;
+	if (clutsize > LOGO_DT_MAX_CLUT)
+		return -EINVAL;
+
+	ret = of_property_count_u8_elems(np, "data");
+	if (ret < 0)
+		return ret;
+	if ((unsigned int)ret != npixels)
+		return -EINVAL;
+
+	clut = kmalloc(len, GFP_KERNEL);
+	if (!clut)
+		return -ENOMEM;
+
+	data = kmalloc(npixels, GFP_KERNEL);
+	if (!data) {
+		ret = -ENOMEM;
+		goto err_free_clut;
+	}
+
+	ret = of_property_read_u8_array(np, "clut", clut, len);
+	if (ret)
+		goto err_free_data;
+
+	ret = of_property_read_u8_array(np, "data", data, npixels);
+	if (ret)
+		goto err_free_data;
+
+	for (i = 0; i < npixels; i++) {
+		if (data[i] >= clutsize) {
+			ret = -ERANGE;
+			goto err_free_data;
+		}
+		data[i] += LOGO_DT_CLUT_OFFSET;
+	}
+
+	logo_dt_clut = clut;
+	logo_dt_data = data;
+
+	logo_dt_clut224.width = width;
+	logo_dt_clut224.height = height;
+	logo_dt_clut224.clutsize = clutsize;
+	logo_dt_clut224.clut = clut;
+	logo_dt_clut224.data = data;
+
+	return 0;
+
+err_free_data:
+	kfree(data);
+err_free_clut:
+	kfree(clut);
+	return ret;
+}
+
+static const struct linux_logo *logo_dt_find(void)
+{
+	static bool probed;
+	struct device_node *np;
+	int ret;
+
+	if (probed)
+		return logo_dt_data ? &logo_dt_clut224 : NULL;
+
+	probed = true;
+
+	np = of_get_compatible_child(of_chosen, LOGO_DT_COMPATIBLE);
+	if (!np)
+		return NULL;
+
+	if (of_device_is_available(np)) {
+		ret = logo_dt_parse(np);
+		if (ret)
+			pr_warn("logo: ignoring malformed %pOF node (%d)\n",
+				np, ret);
+	}
+
+	of_node_put(np);
+
+	return logo_dt_data ? &logo_dt_clut224 : NULL;
+}
+
+static void logo_dt_free(void)
+{
+	logo_dt_clut224.clut = NULL;
+	logo_dt_clut224.data = NULL;
+
+	kfree(logo_dt_clut);
+	logo_dt_clut = NULL;
+
+	kfree(logo_dt_data);
+	logo_dt_data = NULL;
+}
+
+#else /* !CONFIG_LOGO_DT_CLUT224 */
+
+static inline const struct linux_logo *logo_dt_find(void)
+{
+	return NULL;
+}
+
+static inline void logo_dt_free(void) { }
+
+#endif /* CONFIG_LOGO_DT_CLUT224 */
+
 /*
  * Logos are located in the initdata, and will be freed in kernel_init.
  * Use late_init to mark the logos as freed to prevent any further use.
@@ -32,6 +184,7 @@ static bool logos_freed;
 static int __init fb_logo_late_init(void)
 {
 	logos_freed = true;
+	logo_dt_free();
 	return 0;
 }
 
@@ -48,6 +201,13 @@ const struct linux_logo * __ref fb_find_logo(int depth)
 	if (nologo || logos_freed)
 		return NULL;
 
+	/* A logo supplied by the device tree wins over the built-in ones */
+	if (depth >= 8) {
+		logo = logo_dt_find();
+		if (logo)
+			return logo;
+	}
+
 #ifdef CONFIG_LOGO_LINUX_MONO
 	if (depth >= 1)
 		logo = &logo_linux_mono;
-- 
2.39.5

[PATCH v2 3/6] fbdev: honour the device tree boot logo placement properties

[Max Pedraza]

A logo supplied by the device tree describes a specific product, so where
it lands on the panel matters in a way it does not for the built-in Tux.
Honour the optional "logo-position", "logo-offset", "logo-centered" and
"logo-rotation" properties of the "linux,boot-logo-clut224" node.

The rotation is folded into the rotate argument at the entry points, so
fb_prepare_logo() reserves the right amount of space and fb_do_show_logo()
tiles in the same direction the image was rotated in.

Placement is applied after fb_rotate_logo(), since a rotation swaps the
image width and height and centring needs the final dimensions.

fb_prepare_logo() has to grow its reservation to cover wherever the logo
ends up, the way fb_center_logo already does: fbcon only leaves the first
n rows alone, so a logo placed further down is drawn and then immediately
cleared by the console.

"logo-centered" is deliberately close to the existing fb_center_logo, but
it is per device tree rather than per fbcon command line, and it composes
with "logo-offset" so that a logo can be centred and then nudged. That
combination is what panels with a partially visible area need, where the
usable region is not the centre of the mode.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 drivers/video/fbdev/core/fb_logo.c | 174 +++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/drivers/video/fbdev/core/fb_logo.c b/drivers/video/fbdev/core/fb_logo.c
index 0bab8352b..bfc1b72ce 100644
--- a/drivers/video/fbdev/core/fb_logo.c
+++ b/drivers/video/fbdev/core/fb_logo.c
@@ -2,12 +2,178 @@
 
 #include <linux/fb.h>
 #include <linux/linux_logo.h>
+#include <linux/of.h>
 
 #include "fb_internal.h"
 
 bool fb_center_logo __read_mostly;
 int fb_logo_count __read_mostly = -1;
 
+#ifdef CONFIG_LOGO_DT_CLUT224
+
+/*
+ * Placement of a logo supplied by the device tree. The image itself is parsed
+ * by drivers/video/logo/logo.c, here we only care about where it goes.
+ */
+struct fb_logo_dt_placement {
+	bool valid;
+	bool centered;
+	bool has_position;
+	int rotation;		/* FB_ROTATE_*, or -1 when unspecified */
+	u32 x, y;
+	s32 offset_x, offset_y;
+};
+
+static struct fb_logo_dt_placement fb_logo_dt;
+
+static int fb_logo_dt_parse_rotation(const char *rotation)
+{
+	if (!strcmp(rotation, "none"))
+		return FB_ROTATE_UR;
+	if (!strcmp(rotation, "cw"))
+		return FB_ROTATE_CW;
+	if (!strcmp(rotation, "ud"))
+		return FB_ROTATE_UD;
+	if (!strcmp(rotation, "ccw"))
+		return FB_ROTATE_CCW;
+
+	return -EINVAL;
+}
+
+static void fb_logo_dt_read(void)
+{
+	struct fb_logo_dt_placement *p = &fb_logo_dt;
+	static bool read_done;
+	struct device_node *np;
+	const char *rotation;
+	u32 val[2];
+	int rot;
+
+	if (read_done)
+		return;
+
+	read_done = true;
+
+	np = of_find_compatible_node(NULL, NULL, "linux,boot-logo-clut224");
+	if (!np)
+		return;
+
+	if (!of_device_is_available(np))
+		goto out;
+
+	p->valid = true;
+	p->rotation = -1;
+	p->centered = of_property_read_bool(np, "logo-centered");
+
+	if (!of_property_read_u32_array(np, "logo-position", val, 2)) {
+		p->has_position = true;
+		p->x = val[0];
+		p->y = val[1];
+	}
+
+	if (!of_property_read_u32_array(np, "logo-offset", val, 2)) {
+		p->offset_x = (s32)val[0];
+		p->offset_y = (s32)val[1];
+	}
+
+	if (!of_property_read_string(np, "logo-rotation", &rotation)) {
+		rot = fb_logo_dt_parse_rotation(rotation);
+		if (rot < 0)
+			pr_warn("fb: %pOF: unknown logo-rotation \"%s\"\n",
+				np, rotation);
+		else
+			p->rotation = rot;
+	}
+
+out:
+	of_node_put(np);
+}
+
+static int fb_logo_dt_rotation(int rotate)
+{
+	fb_logo_dt_read();
+
+	if (fb_logo_dt.valid && fb_logo_dt.rotation >= 0)
+		return fb_logo_dt.rotation;
+
+	return rotate;
+}
+
+/* Top edge of the logo, in the coordinate space the caller works in */
+static int fb_logo_dt_top(unsigned int yres, unsigned int logo_height)
+{
+	struct fb_logo_dt_placement *p = &fb_logo_dt;
+	int top;
+
+	if (p->centered)
+		top = ((int)yres - (int)logo_height) / 2;
+	else if (p->has_position)
+		top = p->y;
+	else
+		top = 0;
+
+	return max(top + p->offset_y, 0);
+}
+
+static void fb_logo_dt_place(struct fb_info *info, struct fb_image *image)
+{
+	struct fb_logo_dt_placement *p = &fb_logo_dt;
+	int dx;
+
+	fb_logo_dt_read();
+
+	if (!p->valid)
+		return;
+
+	if (p->centered)
+		dx = ((int)info->var.xres - (int)image->width) / 2;
+	else if (p->has_position)
+		dx = p->x;
+	else
+		dx = image->dx;
+
+	image->dx = max(dx + p->offset_x, 0);
+	image->dy = fb_logo_dt_top(info->var.yres, image->height);
+}
+
+/*
+ * fbcon only leaves the first @height rows of the screen alone, so a logo
+ * placed further down would be drawn and then immediately cleared. Grow the
+ * reservation to cover wherever the logo actually ends up, the same way
+ * fb_center_logo does.
+ */
+static int fb_logo_dt_reserve(unsigned int yres, unsigned int logo_height,
+			      int height)
+{
+	fb_logo_dt_read();
+
+	if (!fb_logo_dt.valid)
+		return height;
+
+	return max(height, fb_logo_dt_top(yres, logo_height) +
+			   (int)logo_height);
+}
+
+#else /* !CONFIG_LOGO_DT_CLUT224 */
+
+static inline int fb_logo_dt_rotation(int rotate)
+{
+	return rotate;
+}
+
+static inline void fb_logo_dt_place(struct fb_info *info,
+				    struct fb_image *image)
+{
+}
+
+static inline int fb_logo_dt_reserve(unsigned int yres,
+				     unsigned int logo_height, int height)
+{
+	return height;
+}
+
+#endif /* CONFIG_LOGO_DT_CLUT224 */
+
 static inline unsigned int safe_shift(unsigned int d, int n)
 {
 	return n < 0 ? d >> -n : d << n;
@@ -350,6 +516,9 @@ static int fb_show_logo_line(struct fb_info *info, int rotate,
 			fb_rotate_logo(info, logo_rotate, &image, rotate);
 	}
 
+	/* Done last, image dimensions are only final after the rotation */
+	fb_logo_dt_place(info, &image);
+
 	fb_do_show_logo(info, &image, rotate, n);
 
 	kfree(palette);
@@ -423,6 +592,8 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
 
 	memset(&fb_logo, 0, sizeof(struct logo_data));
 
+	rotate = fb_logo_dt_rotation(rotate);
+
 	if (info->flags & FBINFO_MISC_TILEBLITTING ||
 	    info->fbops->owner || !fb_logo_count)
 		return 0;
@@ -483,6 +654,7 @@ 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;
+	height = fb_logo_dt_reserve(yres, fb_logo.logo->height, height);
 #ifdef CONFIG_FB_LOGO_EXTRA
 	height = fb_prepare_extra_logos(info, height, yres);
 #endif
@@ -498,6 +670,8 @@ int fb_show_logo(struct fb_info *info, int rotate)
 	if (!fb_logo_count)
 		return 0;
 
+	rotate = fb_logo_dt_rotation(rotate);
+
 	count = fb_logo_count < 0 ? num_online_cpus() : fb_logo_count;
 	y = fb_show_logo_line(info, rotate, fb_logo.logo, 0, count);
 #ifdef CONFIG_FB_LOGO_EXTRA
-- 
2.39.5

[PATCH v2 4/6] dt-bindings: display: allow the boot logo in a reserved memory region

[Max Pedraza]

Carrying the image in the device tree ties it to the device tree, but the
image and where it goes on screen are independent axes of variation. One
board sold to several customers wants several device trees that differ in
the logo; one customer with several products built on that board wants the
same logo placed differently on each panel. The second case would otherwise
mean duplicating the same image into every device tree.

Let the node point at a reserved memory region filled in by the bootloader
instead, so one image can be shared by device trees that differ only in
placement. The region starts with a small header carrying a magic number
and the geometry, so the kernel can tell a logo from an empty or stale
region and bounds check everything against the reservation.

A phandle to a declared region is used rather than a bare address: the
reservation is what makes the memory safe to read and what gives the kernel
a size to validate against. The two ways of supplying the image are
mutually exclusive.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 .../display/linux,boot-logo-clut224.yaml        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
index a6a206964..7aec0cc2d 100644
--- a/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
+++ b/Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
@@ -59,6 +59,23 @@ properties:
       index into the colour lookup table. The property length must be equal to
       width multiplied by height.
 
+  memory-region:
+    maxItems: 1
+    description: |
+      Reserved memory region holding the logo, as an alternative to carrying
+      it in the width, height, clut and data properties. The bootloader is
+      expected to have placed the image there before starting the kernel.
+
+      This lets one image be shared by several device trees that differ only
+      in where the logo goes, which is what a family of products built on the
+      same board but with different panels needs.
+
+      The region starts with a header of four little endian 32 bit words:
+      the magic number 0x4f474f4c ("LOGO"), the width, the height and the
+      number of palette entries. The palette follows, as consecutive red,
+      green and blue bytes per entry, and then one byte per pixel, each an
+      index into that palette.
+
   logo-position:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     description:
-- 
2.39.5

[PATCH v2 5/6] video: logo: allow the boot logo to come from a reserved memory region

[Max Pedraza]

Take the image from the region a "memory-region" phandle points at, when
the node has one, instead of from the width, height, clut and data
properties.

Only a declared region is accepted, never a bare physical address. That
matters for more than tidiness: the region is reserved before the allocator
starts, so the logo code can never be pointed at memory the kernel is using
for something else, and a size is known, so every field can be bounds
checked. The region is mapped with memremap(), which copes with a no-map
reservation and fails cleanly instead of handing back a bogus pointer the
way phys_to_virt() on an arbitrary address would.

The image is validated and copied out rather than used in place: the magic
number has to match, the geometry has to be sane, the palette and pixels
have to fit inside the reserved region, and every pixel has to reference an
existing palette entry. Anything else is reported and ignored, falling back
to the built-in logo. Copying also means nothing that happens to the region
afterwards can affect what has already been validated, and it keeps the
pixel data in the same shape both paths produce.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 drivers/video/logo/logo.c | 171 ++++++++++++++++++++++++++++++--------
 1 file changed, 135 insertions(+), 36 deletions(-)

diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
index 7f8b04ecf..66bcb37e7 100644
--- a/drivers/video/logo/logo.c
+++ b/drivers/video/logo/logo.c
@@ -10,8 +10,10 @@
  *  Copyright (C) 2003 Geert Uytterhoeven <geert@linux-m68k.org>
  */
 
+#include <linux/io.h>
 #include <linux/linux_logo.h>
 #include <linux/of.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/stddef.h>
@@ -38,6 +40,15 @@ MODULE_PARM_DESC(nologo, "Disables startup logo");
 #define LOGO_DT_CLUT_OFFSET	32
 /* Sanity limit on the image size, a device tree is not a good place for more */
 #define LOGO_DT_MAX_PIXELS	SZ_32M
+/* "LOGO", little endian, at the start of a handed over memory region */
+#define LOGO_DT_MAGIC		0x4f474f4c
+
+struct logo_dt_header {
+	__le32 magic;
+	__le32 width;
+	__le32 height;
+	__le32 clutsize;
+};
 
 static struct linux_logo logo_dt_clut224 = {
 	.type		= LINUX_LOGO_CLUT224,
@@ -46,58 +57,41 @@ static struct linux_logo logo_dt_clut224 = {
 static unsigned char *logo_dt_clut;
 static unsigned char *logo_dt_data;
 
-static int logo_dt_parse(struct device_node *np)
+/* Reject geometries that cannot describe a sane image before using them */
+static int logo_dt_check_geometry(u32 width, u32 height, u32 clutsize)
 {
-	unsigned int clutsize, npixels, i;
-	unsigned char *clut, *data;
-	u32 width, height;
-	int len, ret;
-
-	ret = of_property_read_u32(np, "width", &width);
-	if (ret)
-		return ret;
-
-	ret = of_property_read_u32(np, "height", &height);
-	if (ret)
-		return ret;
-
 	if (!width || !height || (u64)width * height > LOGO_DT_MAX_PIXELS)
 		return -EINVAL;
 
-	npixels = width * height;
-
-	len = of_property_count_u8_elems(np, "clut");
-	if (len < 3 || len % 3)
+	if (!clutsize || clutsize > LOGO_DT_MAX_CLUT)
 		return -EINVAL;
 
-	clutsize = len / 3;
-	if (clutsize > LOGO_DT_MAX_CLUT)
-		return -EINVAL;
+	return 0;
+}
 
-	ret = of_property_count_u8_elems(np, "data");
-	if (ret < 0)
-		return ret;
-	if ((unsigned int)ret != npixels)
-		return -EINVAL;
+/*
+ * Take a private copy of an image that has already been range checked, so
+ * that nothing else can change it under us, and shift the pixels into the
+ * palette slots the frame buffer layer leaves to the logo.
+ */
+static int logo_dt_store(u32 width, u32 height, u32 clutsize,
+			 const u8 *clut_src, const u8 *data_src)
+{
+	unsigned int npixels = width * height;
+	unsigned char *clut, *data;
+	unsigned int i;
+	int ret;
 
-	clut = kmalloc(len, GFP_KERNEL);
+	clut = kmemdup(clut_src, clutsize * 3, GFP_KERNEL);
 	if (!clut)
 		return -ENOMEM;
 
-	data = kmalloc(npixels, GFP_KERNEL);
+	data = kmemdup(data_src, npixels, GFP_KERNEL);
 	if (!data) {
 		ret = -ENOMEM;
 		goto err_free_clut;
 	}
 
-	ret = of_property_read_u8_array(np, "clut", clut, len);
-	if (ret)
-		goto err_free_data;
-
-	ret = of_property_read_u8_array(np, "data", data, npixels);
-	if (ret)
-		goto err_free_data;
-
 	for (i = 0; i < npixels; i++) {
 		if (data[i] >= clutsize) {
 			ret = -ERANGE;
@@ -124,6 +118,111 @@ static int logo_dt_parse(struct device_node *np)
 	return ret;
 }
 
+static int logo_dt_parse_properties(struct device_node *np)
+{
+	const u8 *clut, *data;
+	u32 width, height;
+	int len, ret;
+
+	ret = of_property_read_u32(np, "width", &width);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "height", &height);
+	if (ret)
+		return ret;
+
+	len = of_property_count_u8_elems(np, "clut");
+	if (len < 3 || len % 3)
+		return -EINVAL;
+
+	ret = logo_dt_check_geometry(width, height, len / 3);
+	if (ret)
+		return ret;
+
+	if (of_property_count_u8_elems(np, "data") != width * height)
+		return -EINVAL;
+
+	clut = of_get_property(np, "clut", NULL);
+	data = of_get_property(np, "data", NULL);
+	if (!clut || !data)
+		return -EINVAL;
+
+	return logo_dt_store(width, height, len / 3, clut, data);
+}
+
+/*
+ * Image handed over by the bootloader in a reserved memory region. Only a
+ * region the device tree declared is accepted, never a bare address, so the
+ * kernel can never be pointed at memory it is using for something else, and
+ * so that a size is known and every access can be bounds checked.
+ */
+static int logo_dt_parse_memory_region(struct device_node *np)
+{
+	u32 width, height, clutsize;
+	const struct logo_dt_header *hdr;
+	struct device_node *mem_np;
+	struct reserved_mem *rmem;
+	size_t clutlen, datalen;
+	const u8 *payload;
+	void *mem;
+	int ret;
+
+	mem_np = of_parse_phandle(np, "memory-region", 0);
+	if (!mem_np)
+		return -ENOENT;
+
+	rmem = of_reserved_mem_lookup(mem_np);
+	of_node_put(mem_np);
+	if (!rmem)
+		return -EINVAL;
+
+	if (rmem->size < sizeof(*hdr))
+		return -EINVAL;
+
+	mem = memremap(rmem->base, rmem->size, MEMREMAP_WB);
+	if (!mem)
+		return -ENOMEM;
+
+	hdr = mem;
+	if (le32_to_cpu(hdr->magic) != LOGO_DT_MAGIC) {
+		ret = -EINVAL;
+		goto out_unmap;
+	}
+
+	width = le32_to_cpu(hdr->width);
+	height = le32_to_cpu(hdr->height);
+	clutsize = le32_to_cpu(hdr->clutsize);
+
+	ret = logo_dt_check_geometry(width, height, clutsize);
+	if (ret)
+		goto out_unmap;
+
+	clutlen = (size_t)clutsize * 3;
+	datalen = (size_t)width * height;
+
+	/* Everything the header promises has to fit inside the region */
+	if (sizeof(*hdr) + clutlen + datalen > rmem->size) {
+		ret = -EINVAL;
+		goto out_unmap;
+	}
+
+	payload = (const u8 *)(hdr + 1);
+	ret = logo_dt_store(width, height, clutsize, payload, payload + clutlen);
+
+out_unmap:
+	memunmap(mem);
+	return ret;
+}
+
+static int logo_dt_parse(struct device_node *np)
+{
+	if (of_property_present(np, "memory-region"))
+		return logo_dt_parse_memory_region(np);
+
+	return logo_dt_parse_properties(np);
+}
+
 static const struct linux_logo *logo_dt_find(void)
 {
 	static bool probed;
-- 
2.39.5

[PATCH v2 6/6] video: logo: add ppmtodtlogo host tool

[Max Pedraza]

The byte arrays a "linux,boot-logo-clut224" node carries are not meant to
be written by hand. Add a host tool that converts a PPM image into the
node, along the lines of the existing pnmtologo: plain C, no dependencies,
and no quantization of its own -- the image must already use at most 224
distinct colours, and the tool points at ImageMagick when it does not.

Three output flavours:

  ppmtodtlogo logo.ppm                  a complete overlay
  ppmtodtlogo -t dtsi logo.ppm          a bare node, for inclusion
  ppmtodtlogo -t bin -o logo.bin ...    the blob for a reserved memory
                                        region, header included

The devicetree outputs also carry the optional placement properties,
commented out unless requested on the command line, so the generated file
documents what can be tuned without regenerating the image.

The tool is built when CONFIG_LOGO_DT_CLUT224 is enabled but is not used
by the kernel build itself.

Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
---
 drivers/video/logo/Makefile      |   6 +-
 drivers/video/logo/ppmtodtlogo.c | 408 +++++++++++++++++++++++++++++++
 2 files changed, 413 insertions(+), 1 deletion(-)
 create mode 100644 drivers/video/logo/ppmtodtlogo.c

diff --git a/drivers/video/logo/Makefile b/drivers/video/logo/Makefile
index 937b37d3b..49954272e 100644
--- a/drivers/video/logo/Makefile
+++ b/drivers/video/logo/Makefile
@@ -10,7 +10,11 @@ obj-$(CONFIG_SPU_BASE)			+= logo_spe_clut224.o
 
 # How to generate logo's
 
-hostprogs := pnmtologo
+hostprogs := pnmtologo ppmtodtlogo
+
+# Not used by the build itself: converts a user's image into the devicetree
+# node or memory blob the "linux,boot-logo-clut224" binding consumes.
+always-$(CONFIG_LOGO_DT_CLUT224) += ppmtodtlogo
 
 # Create commands like "pnmtologo -t mono -n logo_linux_mono -o ..."
 quiet_cmd_logo = LOGO    $@
diff --git a/drivers/video/logo/ppmtodtlogo.c b/drivers/video/logo/ppmtodtlogo.c
new file mode 100644
index 000000000..a52c61cec
--- /dev/null
+++ b/drivers/video/logo/ppmtodtlogo.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Convert a PPM image into a devicetree boot logo node, or into the binary
+ * blob the "linux,boot-logo-clut224" binding reads from a reserved memory
+ * region.
+ *
+ * Like pnmtologo, this tool does not quantize: the image must already use
+ * at most 224 distinct colours. Reduce it first if needed, for instance:
+ *
+ *	magick logo.png -colors 224 logo.ppm
+ *
+ * The node is emitted under /chosen, where the binding expects it: a logo is
+ * configuration handed over by firmware rather than a description of the
+ * hardware.
+ *
+ * The devicetree output stores plain palette indices; the 32 entry offset
+ * the frame buffer layer reserves for the console is applied by the kernel.
+ */
+
+#include <ctype.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#define MAX_CLUT_COLORS	224
+#define MAX_PIXELS	(32U * 1024 * 1024)
+#define BLOB_MAGIC	0x4f474f4cU	/* "LOGO", little endian */
+#define BYTES_PER_LINE	12
+
+static const char *programname;
+static const char *filename;
+static const char *outputname;
+static FILE *out;
+
+enum output_type {
+	OUTPUT_DTS,	/* complete overlay */
+	OUTPUT_DTSI,	/* bare node, for inclusion */
+	OUTPUT_BIN,	/* blob for a reserved memory region */
+};
+
+static enum output_type output_type = OUTPUT_DTS;
+
+/* Placement options, emitted into the node */
+static int opt_centered;
+static char *opt_position;
+static char *opt_offset;
+static const char *opt_rotation;
+
+struct color {
+	unsigned char red;
+	unsigned char green;
+	unsigned char blue;
+};
+
+static unsigned int logo_width;
+static unsigned int logo_height;
+static unsigned char *logo_data;
+static struct color logo_clut[MAX_CLUT_COLORS];
+static unsigned int logo_clutsize;
+
+static void die(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	exit(1);
+}
+
+static void usage(void)
+{
+	die("Usage: %s [options] <filename>\n"
+	    "\n"
+	    "Convert a PPM image into a \"linux,boot-logo-clut224\" node.\n"
+	    "The image must use at most %d distinct colours.\n"
+	    "\n"
+	    "    -o <output>     write to file instead of stdout\n"
+	    "    -t <type>       dts (default), dtsi or bin\n"
+	    "    -c              centre the logo (logo-centered)\n"
+	    "    -p <x>,<y>      logo-position\n"
+	    "    -f <dx>,<dy>    logo-offset\n"
+	    "    -r <rotation>   logo-rotation: cw, ccw, ud or none\n"
+	    "    -h              this help\n",
+	    programname, MAX_CLUT_COLORS);
+}
+
+static unsigned int get_number(FILE *fp)
+{
+	int c;
+	unsigned int val;
+
+	/* Skip leading whitespace */
+	do {
+		c = fgetc(fp);
+		if (c == EOF)
+			die("%s: end of file\n", filename);
+		if (c == '#') {
+			/* Ignore comments 'till end of line */
+			do {
+				c = fgetc(fp);
+				if (c == EOF)
+					die("%s: end of file\n", filename);
+			} while (c != '\n');
+		}
+	} while (isspace(c));
+
+	if (!isdigit(c))
+		die("%s: expected a number\n", filename);
+
+	/* Parse decimal number */
+	val = 0;
+	while (isdigit(c)) {
+		val = 10 * val + c - '0';
+		c = fgetc(fp);
+		if (c == EOF)
+			break;
+	}
+	return val;
+}
+
+static unsigned char get_byte(FILE *fp)
+{
+	int c = fgetc(fp);
+
+	if (c == EOF)
+		die("%s: end of file\n", filename);
+	return c;
+}
+
+static unsigned int find_clut_entry(struct color color)
+{
+	unsigned int i;
+
+	for (i = 0; i < logo_clutsize; i++)
+		if (logo_clut[i].red == color.red &&
+		    logo_clut[i].green == color.green &&
+		    logo_clut[i].blue == color.blue)
+			return i;
+
+	if (logo_clutsize == MAX_CLUT_COLORS)
+		die("%s: more than %d colors, reduce the image first, e.g.\n"
+		    "    magick %s -colors %d out.ppm\n",
+		    filename, MAX_CLUT_COLORS, filename, MAX_CLUT_COLORS);
+
+	logo_clut[logo_clutsize] = color;
+	return logo_clutsize++;
+}
+
+static void read_image(void)
+{
+	unsigned int i, npixels, maxval;
+	int magic, raw;
+	FILE *fp;
+
+	fp = fopen(filename, "rb");
+	if (!fp)
+		die("Cannot open file %s: %s\n", filename, strerror(errno));
+
+	if (fgetc(fp) != 'P')
+		die("%s is not a PPM file\n", filename);
+
+	magic = fgetc(fp);
+	switch (magic) {
+	case '3':
+		raw = 0;
+		break;
+	case '6':
+		raw = 1;
+		break;
+	default:
+		die("%s is not a PPM file (only P3 and P6 are supported)\n",
+		    filename);
+	}
+
+	logo_width = get_number(fp);
+	logo_height = get_number(fp);
+	maxval = get_number(fp);
+	if (maxval != 255)
+		die("%s: maximum color value must be 255\n", filename);
+
+	if (!logo_width || !logo_height)
+		die("%s: zero sized image\n", filename);
+	if ((unsigned long long)logo_width * logo_height > MAX_PIXELS)
+		die("%s: image too large\n", filename);
+
+	npixels = logo_width * logo_height;
+	logo_data = malloc(npixels);
+	if (!logo_data)
+		die("%s\n", strerror(errno));
+
+	for (i = 0; i < npixels; i++) {
+		struct color color;
+
+		if (raw) {
+			color.red = get_byte(fp);
+			color.green = get_byte(fp);
+			color.blue = get_byte(fp);
+		} else {
+			color.red = get_number(fp);
+			color.green = get_number(fp);
+			color.blue = get_number(fp);
+		}
+		logo_data[i] = find_clut_entry(color);
+	}
+
+	fclose(fp);
+}
+
+static void write_bytes(const unsigned char *data, unsigned int len,
+			const char *indent)
+{
+	unsigned int i;
+
+	for (i = 0; i < len; i++) {
+		if (i % BYTES_PER_LINE == 0)
+			fprintf(out, "%s%s", i ? "\n" : "", indent);
+		else
+			fputc(' ', out);
+		fprintf(out, "0x%02x", data[i]);
+	}
+}
+
+static void write_placement(const char *indent)
+{
+	fprintf(out, "%s/* Placement. logo-centered and logo-position are exclusive;\n",
+		indent);
+	fprintf(out, "%s * logo-offset is added after either of the two. */\n",
+		indent);
+
+	fprintf(out, "%s%slogo-centered;\n", indent, opt_centered ? "" : "// ");
+	fprintf(out, "%s%slogo-position = <%s>;\n", indent,
+		opt_position ? "" : "// ", opt_position ? opt_position : "0 0");
+	fprintf(out, "%s%slogo-offset = <%s>;\n", indent,
+		opt_offset ? "" : "// ", opt_offset ? opt_offset : "0 0");
+	fprintf(out, "%s%slogo-rotation = \"%s\";\t/* cw, ccw, ud, none */\n",
+		indent, opt_rotation ? "" : "// ",
+		opt_rotation ? opt_rotation : "ccw");
+}
+
+static void write_node(const char *indent)
+{
+	char subindent[16];
+
+	snprintf(subindent, sizeof(subindent), "%s\t\t", indent);
+
+	fprintf(out, "%scompatible = \"linux,boot-logo-clut224\";\n", indent);
+	fprintf(out, "\n");
+	write_placement(indent);
+	fprintf(out, "\n");
+	fprintf(out, "%swidth = <%u>;\n", indent, logo_width);
+	fprintf(out, "%sheight = <%u>;\n", indent, logo_height);
+	fprintf(out, "\n");
+	fprintf(out, "%sclut = /bits/ 8 <", indent);
+	write_bytes((const unsigned char *)logo_clut, logo_clutsize * 3,
+		    subindent);
+	fprintf(out, ">;\n");
+	fprintf(out, "\n");
+	fprintf(out, "%sdata = /bits/ 8 <", indent);
+	write_bytes(logo_data, logo_width * logo_height, subindent);
+	fprintf(out, ">;\n");
+}
+
+static void write_header_comment(void)
+{
+	fprintf(out, "/*\n");
+	fprintf(out, " * Boot logo generated by ppmtodtlogo from %s\n",
+		filename);
+	fprintf(out, " * %ux%u pixels, %u colours.\n", logo_width, logo_height,
+		logo_clutsize);
+	fprintf(out, " */\n");
+}
+
+static void write_dts(void)
+{
+	fprintf(out, "/dts-v1/;\n/plugin/;\n\n");
+	write_header_comment();
+	fprintf(out, "\n");
+	fprintf(out, "/ {\n");
+	fprintf(out, "\tfragment@101 {\n");
+	fprintf(out, "\t\ttarget-path = \"/chosen\";\n");
+	fprintf(out, "\n");
+	fprintf(out, "\t\t__overlay__ {\n");
+	fprintf(out, "\t\t\tlogo {\n");
+	write_node("\t\t\t\t");
+	fprintf(out, "\t\t\t};\n");
+	fprintf(out, "\t\t};\n");
+	fprintf(out, "\t};\n");
+	fprintf(out, "};\n");
+}
+
+static void write_dtsi(void)
+{
+	write_header_comment();
+	fprintf(out, "\n");
+	fprintf(out, "chosen {\n");
+	fprintf(out, "\tlogo {\n");
+	write_node("\t\t");
+	fprintf(out, "\t};\n");
+	fprintf(out, "};\n");
+}
+
+static void put_le32(unsigned int val)
+{
+	fputc(val & 0xff, out);
+	fputc((val >> 8) & 0xff, out);
+	fputc((val >> 16) & 0xff, out);
+	fputc((val >> 24) & 0xff, out);
+}
+
+static void write_bin(void)
+{
+	put_le32(BLOB_MAGIC);
+	put_le32(logo_width);
+	put_le32(logo_height);
+	put_le32(logo_clutsize);
+	fwrite(logo_clut, 3, logo_clutsize, out);
+	fwrite(logo_data, 1, logo_width * logo_height, out);
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+	char *p;
+
+	programname = argv[0];
+
+	while ((opt = getopt(argc, argv, "o:t:cp:f:r:h")) != -1) {
+		switch (opt) {
+		case 'o':
+			outputname = optarg;
+			break;
+		case 't':
+			if (!strcmp(optarg, "dts"))
+				output_type = OUTPUT_DTS;
+			else if (!strcmp(optarg, "dtsi"))
+				output_type = OUTPUT_DTSI;
+			else if (!strcmp(optarg, "bin"))
+				output_type = OUTPUT_BIN;
+			else
+				usage();
+			break;
+		case 'c':
+			opt_centered = 1;
+			break;
+		case 'p':
+			opt_position = optarg;
+			break;
+		case 'f':
+			opt_offset = optarg;
+			break;
+		case 'r':
+			if (strcmp(optarg, "cw") && strcmp(optarg, "ccw") &&
+			    strcmp(optarg, "ud") && strcmp(optarg, "none"))
+				usage();
+			opt_rotation = optarg;
+			break;
+		default:
+			usage();
+		}
+	}
+	if (optind != argc - 1)
+		usage();
+	filename = argv[optind];
+
+	/* "10,20" and "10 20" are both accepted for -p and -f */
+	for (p = opt_position; p && *p; p++)
+		if (*p == ',')
+			*p = ' ';
+	for (p = opt_offset; p && *p; p++)
+		if (*p == ',')
+			*p = ' ';
+
+	read_image();
+
+	if (outputname) {
+		out = fopen(outputname,
+			    output_type == OUTPUT_BIN ? "wb" : "w");
+		if (!out)
+			die("Cannot create file %s: %s\n", outputname,
+			    strerror(errno));
+	} else {
+		out = stdout;
+	}
+
+	switch (output_type) {
+	case OUTPUT_DTS:
+		write_dts();
+		break;
+	case OUTPUT_DTSI:
+		write_dtsi();
+		break;
+	case OUTPUT_BIN:
+		write_bin();
+		break;
+	}
+
+	if (outputname)
+		fclose(out);
+
+	fprintf(stderr, "%s: %ux%u pixels, %u colours\n", filename, logo_width,
+		logo_height, logo_clutsize);
+
+	return 0;
+}
-- 
2.39.5

Re: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

[Màxim Pedraza Padilla]

El mar, 4 ago 2026 a las 23:18, Helge Deller (<deller@gmx.de>) escribió:
> Instead of adding logo-centered, couldn't use a value of -1 into
> logo-position.x to mark the logo centered in X-Axis (and .y=-1 for
> y-axis)?
>
> I don't know if this is better or not though....

It is better, and for one more reason than you give.

The binding as posted says logo-centered centres the logo "overriding
logo-position". That is two properties where one silently wins over the
other, which the schema cannot express and which leaves a device tree
setting both with no defined meaning. Folding it into logo-position
removes the contradiction rather than documenting it.

The per-axis part is not just a tidier spelling either. A boolean can
only centre both axes or neither, so "centred horizontally, at the top"
currently has to be written as logo-centered plus a logo-offset computed
from the logo height. That is impossible for the reserved memory form,
where the height lives in the blob and the device tree does not know it.
With -1 per axis it is just <(-1) 0>.

So v3 drops logo-centered and logo-position becomes an int32-array with
a minimum of -1, where -1 on an axis means centre on that axis and any
other negative value is rejected by the schema.

logo-offset stays, for the same reason the per-axis case needs it: our
own panel is 800x480 with only the bottom 320 rows visible, so the logo
is centred and then displaced, and that displacement cannot be folded
into an absolute position when the logo size comes from the blob.

Thanks, this is a clear simplification.

Max

Re: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

[Rob Herring (Arm)]

On Wed, 05 Aug 2026 00:56:12 +0200, Max Pedraza wrote:
> Embedded products frequently need a product specific boot logo. Today the
> only way to get one is to replace one of the logo_*_clut224.ppm files in
> the kernel source tree, which bakes the image into the kernel image: a
> change of branding, or a second product sharing the same board support,
> requires a separate kernel build.
> 
> Add a binding for a "linux,boot-logo-clut224" node, which carries the logo
> in the same paletted format the built-in CLUT224 logos already use, plus a
> few optional properties describing where on the screen it is drawn.
> 
> The palette size is derived from the length of the "clut" property rather
> than spelled out separately, and "data" holds plain palette indices; the
> 32 entry offset the frame buffer layer reserves for the console is an
> implementation detail and is applied by the kernel, not by the binding.
> 
> Signed-off-by: Max Pedraza <maximpedraza@gmail.com>
> ---
>  .../display/linux,boot-logo-clut224.yaml      | 131 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/linux,boot-logo-clut224.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/display/linux,boot-logo-clut224.example.dtb: /: 'compatible' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml
Documentation/devicetree/bindings/display/linux,boot-logo-clut224.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml
Documentation/devicetree/bindings/display/linux,boot-logo-clut224.example.dtb: chosen: 'logo' does not match any of the regexes: '^framebuffer', '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/chosen.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260804225617.264861-2-maximpedraza@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 0/6] Boot logo supplied by the device tree

[Rob Herring]

On Wed, Aug 05, 2026 at 12:56:11AM +0200, Max Pedraza wrote:
> This series lets the logo be described by the device tree instead: a node
> compatible with "linux,boot-logo-clut224" under /chosen supplies the image in
> the same paletted format the built-in CLUT224 logos already use, and the kernel
> prefers it over the built-in ones when it is present and enabled. If the
> node is absent or disabled, nothing changes.

Why is this linux specific? Don't people want to do a splash screen in 
u-boot or other firmware?

> 
> The image can come from the node itself (patches 1-2) or from a reserved
> memory region the bootloader filled in (patches 4-5), because the image and
> its placement are independent axes of variation. One board sold to several
> customers wants several device trees differing in the logo. One customer
> with several products built on that board, with different panels, wants the
> same logo placed differently on each: there the image belongs in a shared
> binary and only the placement belongs in the device tree. Patch 3 adds that
> placement.
> 
> We have been carrying a cruder version of this downstream on an AM335x
> product since 2020, across a handful of board revisions, and it has removed
> a real maintenance burden for us. This is an attempt to find out whether
> something along these lines is wanted upstream, and if so in what shape --
> hence RFC.
> 
> The v1 discussion turned on the obvious objection, that a bitmap is not
> hardware and the device tree is not where it belongs. Geert pointed out that
> configuration which is not hardware description goes under /chosen, and that
> Open Firmware, which the device tree descends from, already carried a boot
> logo in that spirit as the oem-logo variable under /options. The node has
> moved to /chosen accordingly, which is also where simple-framebuffer nodes
> live, for the same reason: they describe what firmware handed over rather
> than what the hardware is.

What does /options/oem-logo look like? Why can't that be used?

> Patches 4 and 5 are separable; the series is useful without them if the
> reserved memory path is thought to be one mechanism too many.

If you are going to put it in memory, why not just draw it into the 
simple-framebuffer?

Rob

Re: [PATCH v2 1/6] dt-bindings: display: add a device tree supplied boot logo

[Màxim Pedraza Padilla]

> Documentation/.../linux,boot-logo-clut224.example.dtb: /: 'compatible'
> is a required property
> Documentation/.../linux,boot-logo-clut224.example.dtb: /: 'model' is a
> required property

Fixed in v3: the example now declares compatible and model on the root
node, the way the simple-framebuffer example does.

> Documentation/.../linux,boot-logo-clut224.example.dtb: chosen: 'logo'
> does not match any of the regexes: '^framebuffer', '^pinctrl-[0-9]+$'

This one cannot be fixed in the binding: chosen.yaml allows only
^framebuffer under /chosen, so any node this binding describes is
rejected wherever it appears. I have opened a pull request against
dt-schema adding '^logo$' alongside it:

  https://github.com/devicetree-org/dt-schema/pull/204

It mirrors how framebuffer is handled: the entry is just "true", with the
contents validated by this binding through the compatible string. The
pattern is anchored because the node carries no reg and so has no unit
address, and the binding pins the name with $nodename: const: logo. If the
naming changes as a result of your other question, the pull request follows
it; it is one line.

With that applied, a full 'make dt_binding_check' is clean for this
schema. Without it, the third error above stands, so this series depends
on that pull request.

I did not see any of the three locally because I had been running with
DT_SCHEMA_FILES set, which leaves root-node.yaml and chosen.yaml out of
the processed schema entirely. Your message says exactly that; I should
have read it before rather than after. Running it unset now.

> The base for the series is generally the latest rc1. A different
> dependency should be noted in *this* patch.

Noted. The base is Linus' master, currently v7.2-rc6, at Helge's request:
v1 went out based on a stable release and did not apply to any tree he
uses, so he asked for git head. v3 carries a base-commit: trailer from
git format-patch --base= so the exact commit is recorded in the series.
Happy to rebase onto rc1 if you would rather have that.

Max

Re: [PATCH v2 0/6] Boot logo supplied by the device tree

[Màxim Pedraza Padilla]

El mié, 5 ago 2026 a las 16:09, Rob Herring (<robh@kernel.org>) escribió:
> Why is this linux specific? Don't people want to do a splash screen in
> u-boot or other firmware?

You are right, and I had not questioned the name. Looking at chosen.yaml,
the split is clear: the "linux," properties are kernel-internal structures
(initrd, kexec handover, the UEFI memory map), while anything a second
implementation could reasonably consume has no prefix at all -- bootargs,
bootsource, stdout-path, kaslr-seed. A boot logo is squarely in the second
group, and simple-framebuffer, which this sits next to, carries no vendor
prefix either.

There is a second Linux-ism in there that your question made me notice:
the 224 in the format. That is 256 minus the 32 palette entries fbcon
reserves for the console, which means nothing outside Linux. The 32 entry
offset itself is already applied by the kernel rather than by the binding,
so it is only the limit that leaks.

Rather than guess at the shape you would want, I would rather ask, since
both the name and the format follow from your answer.

On the name: dropping the prefix would give something like
"boot-logo-clut224", or "boot-logo" with the format named by a property.
Is a bare, unprefixed compatible acceptable here, the way
simple-framebuffer is, or would you rather see this described some other
way entirely?

On the format, two options that I can see:

  a) Keep a palette, but allow the full 256 entries, so that the limit
     Linux applies is a kernel limitation rather than part of the binding.

  b) Reuse the simple-framebuffer vocabulary, format = "r5g6b5" and so on,
     with raw pixels.

I lean towards (a), because the palette is what makes carrying the image
in the device tree viable at all: our own 800x480 logo is 17 KiB paletted
and 768 KiB raw. But (b) reuses an existing vocabulary instead of adding
one, and if you prefer it the reserved memory form still covers our case,
so the in-tree image could go entirely.

I will follow whichever you think is right; I would just rather not
respin the binding twice.

> What does /options/oem-logo look like? Why can't that be used?

Two reasons, and the second is the real one.

It is fixed at 64x64 monochrome (IEEE 1275-1994), stored in NVRAM. As a
precedent for "firmware may carry a logo" it is exactly right, which is
why the binding cites it, but it cannot express a product logo on a
modern panel.

More importantly, /options is the wrong node for something the kernel
reads. Its own schema in dt-schema says it is "for passing data into and
between firmware components" and that "it is ignored by operating
systems", while /chosen is firmware to OS. So the oem-logo precedent
argues for the idea and /chosen for the location, which is where Geert
pointed the node in v2.

> If you are going to put it in memory, why not just draw it into the
> simple-framebuffer?

I did not know about simplefb, and that is worth saying plainly: this
started as a patch against 4.19 for a product, simpledrm did not exist
then (5.14), and I never went looking for what the alternative would be
today. So thank you for the question.

I spent today testing it on the board rather than arguing about it. The
result is that it does not work here, and I could not make it work:

  - Added a simple-framebuffer node under /chosen pointing at U-Boot's
    framebuffer, with the region declared in reserved-memory. simpledrm
    binds cleanly at 0.62s and registers fb0. Nothing is visible.

  - The panel is dark for that entire window. The backlight is a
    pwm-backlight owned by the panel, and it is drm_panel_enable() that
    calls backlight_enable() (drm_panel.c), which only happens at the
    native driver's modeset, around 2.2s. So for as long as
    simple-framebuffer is the only driver, there is no light.

  - Forced the backlight on from probe with a local hack to pwm_bl.c, so
    the panel is lit from about 1s. The image is still not there.

  - Disabled the LCDC target module in the device tree entirely and booted
    with clk_ignore_unused, so that nothing in Linux touches the
    controller and it keeps whatever U-Boot left. Still nothing.

  - Repeated all of it with simplefb instead of simpledrm. Same result.

  - Read back the framebuffer memory through fb0 after boot: all zeros.

So the pixels are gone by the time Linux can show them, and I have not
found what clears them. I can say what it is not: not the choice of
driver, not the modeset, and not the backlight. Somewhere between U-Boot
jumping to the kernel and simpledrm probing, this platform stops
displaying that memory.

One smaller thing I noticed on the way: tilcdc came up as card1 and
simpledrm stayed on card0, with no conflicting-framebuffer removal
between them. Two DRM devices with the real one second is not a state I
would want to ship either.

There is also a case against this that does not depend on our hardware at
all. U-Boot's Falcon mode boots the kernel straight from SPL, skipping
U-Boot proper, and display initialisation normally lives in U-Boot proper
rather than in SPL. On a Falcon boot there is therefore no firmware splash
to hand over and nothing for a simple-framebuffer node to point at. That is
not an obscure corner either: Falcon mode exists to shorten boot time,
which is the same reason one cares about how early the logo appears in the
first place. A logo the kernel can draw itself works the same way in both
cases.

I am not claiming the experimental result above generalises. It may well be
specific to AM335x, or to how our U-Boot hands over. But on the hardware
this series exists for, a firmware-drawn splash does not survive into the
kernel, and the logo has to be drawn again once the native driver owns the
display. That is the gap the series fills, and the reserved memory region
is what gives the kernel the pixels to do it with.

Max