Add monochrome framebuffer support to simple-framebuffer

drivers/video/fbdev/simplefb.c static int simplefb_probe(struct platform_device *pdev) · line 587

[Severity: Medium]
Does the bit ordering used by the cfb drawing helpers match DRM_FORMAT_R1 on little-endian systems?

In include/uapi/drm/drm_fourcc.h, DRM_FORMAT_R1 is defined with MSB-first pixel ordering ([7:0] R0:R1:R2:R3:R4:R5:R6:R7, where the leftmost pixel R0 is in bit 7).

However, in drivers/video/fbdev/core/fb_draw.h, fb_reverse_init() and the cfb drawing helpers (cfb_imageblit(), cfb_fillrect(), cfb_copyarea()) place pixel 0 in the LSB on little-endian architectures and in the MSB on big-endian architectures unless CONFIG_FB_CFB_REV_PIXELS_IN_BYTE is enabled and FB_NONSTD_REV_PIX_IN_B is set in info->var.nonstd:

drivers/video/fbdev/core/fb_draw.h:fb_reverse_init() {
	struct fb_reverse reverse;
#ifdef __LITTLE_ENDIAN
	reverse.byte = fb_be_math(info) != 0;
#else
	reverse.byte = fb_be_math(info) == 0;
#endif
#ifdef FB_REV_PIXELS_IN_BYTE
	reverse.pixel = info->var.bits_per_pixel < BITS_PER_BYTE
		&& (info->var.nonstd & FB_NONSTD_REV_PIX_IN_B);
#else
	reverse.pixel = false;
#endif
	return reverse;
}

Since simplefb leaves info->var.nonstd at 0 and does not select FB_CFB_REV_PIXELS_IN_BYTE, won't the 8 horizontal pixels within each byte be bit-reversed relative to DRM_FORMAT_R1 (and relative to big-endian simplefb) on little-endian machines?

include/linux/platform_data/simplefb.h line 18

[Severity: High]
Can adding "mono" with DRM_FORMAT_R1 to SIMPLEFB_FORMATS break display output when simpledrm is enabled?

SIMPLEFB_FORMATS is shared with drivers/gpu/drm/sysfb/simpledrm.c, where simplefb_get_validated_format() matches fmt->name and looks up fmt->fourcc:

drivers/gpu/drm/sysfb/simpledrm.c:simplefb_get_validated_format() {
	...
		if (!strcmp(format_name, fmt->name)) {
			info = drm_format_info(fmt->fourcc);
			if (!info)
				return ERR_PTR(-EINVAL);
			return info;
		}
	...
}

With this entry added, simpledrm binds to "mono" simple-framebuffer devices, acquires the aperture (evicting or blocking simplefb), and registers a primary plane advertising DRM_FORMAT_R1 and emulated DRM_FORMAT_XRGB8888.

When in-kernel clients (such as fbdev emulation via drm_client_setup() -> drm_fbdev_client_setup()) or userspace compositors commit an XRGB8888 framebuffer, drm_sysfb_plane_helper_begin_fb_access() calls drm_sysfb_get_blit_func(DRM_FORMAT_R1, DRM_FORMAT_XRGB8888):

drivers/gpu/drm/sysfb/drm_sysfb_modeset.c:drm_sysfb_get_blit_func() {
	if (src_format == dst_format) {
		return drm_sysfb_memcpy;
	} else if (src_format == DRM_FORMAT_XRGB8888) {
		switch (dst_format) {
		case DRM_FORMAT_RGB565:
			return drm_fb_xrgb8888_to_rgb565;
		...
		}
	}
	return NULL;
}

Because drm_sysfb_get_blit_func() has no conversion helper for DRM_FORMAT_R1, it returns NULL, triggering a warning and failing every XRGB8888 atomic commit with -EINVAL.

Additionally, if a userspace client commits a native DRM_FORMAT_R1 framebuffer with a non-zero horizontal damage clip offset (clip->x1 > 0), drm_sysfb_memcpy() calls drm_fb_memcpy():

drivers/gpu/drm/drm_format_helper.c:drm_fb_memcpy() {
	...
		unsigned int bpp_i = drm_format_info_bpp(format, i);
		unsigned int cpp_i = DIV_ROUND_UP(bpp_i, 8);
		size_t len_i = DIV_ROUND_UP(drm_rect_width(clip) * bpp_i, 8);

		src_i += clip_offset(clip, src->pitches[i], cpp_i);
	...
}

For 1-bpp DRM_FORMAT_R1, cpp_i evaluates to 1 byte per pixel, so clip_offset() advances src_i by clip->x1 bytes instead of bits (8 times the horizontal byte offset) while drm_sysfb_plane_helper_atomic_update() offsets dst using dst_format->cpp[0] == 0. Won't that read past the end of the source buffer on partial horizontal updates?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Documentation/devicetree/bindings/display/simple-framebuffer.yaml properties: · line 97

[Severity: Medium]
Should the binding description specify both the pixel polarity and the intra-byte bit ordering for mono, similar to how the other formats define their bit-to-channel mappings?

In include/linux/platform_data/simplefb.h, SIMPLEFB_FORMATS maps mono to DRM_FORMAT_R1:

include/linux/platform_data/simplefb.h:

    #define SIMPLEFB_FORMATS \
    { \
        { "mono", 1, {0, 1}, {0, 1}, {0, 1}, {0, 0}, DRM_FORMAT_R1 }, \

In include/uapi/drm/drm_fourcc.h, DRM_FORMAT_R1 is defined with the leftmost pixel in bit 7 (MSB) and the rightmost pixel in bit 0 (LSB).

However, in drivers/video/fbdev/simplefb.c, simplefb_probe() sets info->fix.visual = FB_VISUAL_MONO10 and uses FB_DEFAULT_IOMEM_OPS without setting FBINFO_BE_MATH or FB_NONSTD_REV_PIX_IN_B:

drivers/video/fbdev/simplefb.c:simplefb_probe() {
    ...
	if (params.format->bits_per_pixel == 1)
		info->fix.visual = FB_VISUAL_MONO10;
	else
		info->fix.visual = FB_VISUAL_TRUECOLOR;
    ...
}

Because fb_reverse_init() in drivers/video/fbdev/core/fb_draw.h places the leftmost pixel at bit 0 (LSB) on little-endian CPUs and at bit 7 (MSB) on big-endian CPUs unless reversed, does simplefb end up using a CPU-endian dependent pixel bit order that is reversed relative to DRM_FORMAT_R1 on little-endian systems?

Documentation/devicetree/bindings/display/simple-framebuffer.yaml properties: · line 110

[Severity: High]
How does simpledrm handle a simple-framebuffer Device Tree node with format = "mono" when CONFIG_DRM_SIMPLEDRM is enabled?

Both simplefb and simpledrm bind to simple-framebuffer nodes and validate the format property against SIMPLEFB_FORMATS:

drivers/gpu/drm/sysfb/simpledrm.c:

    simpledrm_probe() ->
        simpledrm_device_create() ->
            simplefb_get_validated_format()

When simplefb_get_validated_format() accepts mono as DRM_FORMAT_R1, simpledrm claims the framebuffer aperture and registers a DRM device whose primary plane advertises DRM_FORMAT_R1 and emulated DRM_FORMAT_XRGB8888 via drm_sysfb_build_fourcc_list().

During drm_client_setup(), drm_fbdev_client_setup() requests color_mode = 1, which maps via drm_mode_legacy_fb_format(1, 1) to DRM_FORMAT_C1 rather than DRM_FORMAT_R1, so __drm_fb_helper_find_sizes() falls back to DRM_FORMAT_XRGB8888.

When the fbdev client or an XRGB8888 userspace compositor commits a framebuffer, drm_sysfb_plane_helper_begin_fb_access() calls drm_sysfb_get_blit_func(DRM_FORMAT_R1, DRM_FORMAT_XRGB8888):

drivers/gpu/drm/sysfb/drm_sysfb_modeset.c:drm_sysfb_get_blit_func() {
	if (src_format == dst_format) {
		return drm_sysfb_memcpy;
	} else if (src_format == DRM_FORMAT_XRGB8888) {
		switch (dst_format) {
		case DRM_FORMAT_RGB565:
		...
		}
	}
	return NULL;
}

Since drm_sysfb_get_blit_func() has no case for DRM_FORMAT_R1, won't drm_sysfb_plane_helper_begin_fb_access() warn and fail every XRGB8888 atomic commit with -EINVAL?

Additionally, if a client commits a native DRM_FORMAT_R1 framebuffer with a partial damage clip where x1 > 0, drm_sysfb_plane_helper_atomic_update() calculates the destination offset using drm_fb_clip_offset():

drivers/gpu/drm/sysfb/drm_sysfb_modeset.c:drm_sysfb_plane_helper_atomic_update() {
    ...
		iosys_map_incr(&dst, drm_fb_clip_offset(dst_pitch, dst_format, &dst_clip));
		blit_to_crtc(&dst, &dst_pitch, shadow_plane_state->data, fb, &damage,
			     &shadow_plane_state->fmtcnv_state);
    ...
}

Because dst_format->cpp[0] is 0 for DRM_FORMAT_R1 while drm_fb_memcpy() uses DIV_ROUND_UP(bpp, 8) = 1 byte per pixel for the source clip offset, wouldn't partial damage updates also miscalculate horizontal offsets?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2