Thread (20 messages) flat view 20 messages, 4 authors, 6d ago

Re: [PATCH v5 2/6] drm/ssd130x: Change SSD133X color format to RGB565 from RGB332

From: sashiko-bot@kernel.org
Date: 2026-08-23 11:36:37
Also in: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing drm_format_conv_state_reserve() pre-allocation forces unsafe GFP_KERNEL allocation in atomic context.
- [High] Sleeping functions called from plane atomic update callback.
--
--- Patch [2]: [PATCH v5 2/6] drm/ssd130x: Change SSD133X color format to RGB565 from RGB332 ---
commit e1ae8460e953e75a280fd3111fa7957d70e68fcc
Author: Amit Barzilai [off-list ref]

drm/ssd130x: Change SSD133X color format to RGB565 from RGB332
    
SSD133X screens were driven at 8bpp RGB332 despite supporting 16bpp RGB565.
Switch the SSD133X data path to RGB565.
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 0b0fc6fe3df23..8cf1e9c84a810 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -978,17 +990,18 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer *fb,
 				struct drm_format_conv_state *fmtcnv_state)
 {
 	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
-	const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+	const struct drm_format_info *fi;
 	unsigned int dst_pitch;
 	struct iosys_map dst;
 
+	fi = drm_format_info(DRM_FORMAT_RGB565);
 	if (!fi)
 		return -EINVAL;
 
 	dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
 
 	iosys_map_set_vaddr(&dst, data_array);
-	drm_fb_xrgb8888_to_rgb332(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+	drm_fb_xrgb8888_to_rgb565be(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
[Severity: High]
This is a pre-existing issue, but does this conversion allocate memory with
GFP_KERNEL in an atomic context?

Because ssd133x_primary_plane_atomic_check() does not pre-allocate the format
conversion state using drm_format_conv_state_reserve(), this call to
drm_fb_xrgb8888_to_rgb565be() can fall back to a dynamic GFP_KERNEL allocation
within __drm_fb_xfrm(). This leads to might_sleep() warnings since it executes
during the plane's atomic_update callback.
 
 	ssd133x_update_rect(ssd130x, rect, data_array, dst_pitch);
[Severity: High]
This isn't a bug introduced by this patch, but does this function sequence
perform sleeping operations from an atomic context?

The ssd133x_primary_plane_atomic_update() callback runs in an atomic context
where sleeping is forbidden. However, it calls drm_gem_fb_begin_cpu_access()
(which waits on dma_resv and can sleep) and then calls into this function,
which reaches ssd133x_update_rect(). The update function performs hardware
writes using regmap_bulk_write() on sleepable I2C/SPI buses.
 
 	return 0;
 }
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823111959.17029-1-amit.barzilai22@gmail.com?part=2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help