Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing `fmtcnv_state` buffer reservation in `ssd133x_primary_plane_atomic_check` causes a `GFP_KERNEL` allocation in atomic context. -- commit 58733b43936278b0113cbbd363c279ece2464617 Author: Amit Barzilai <[email protected]> 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. > diff --git a/drivers/gpu/drm/solomon/ssd130x.c > b/drivers/gpu/drm/solomon/ssd130x.c > index feec4933e3e6b..cbf77a74c4fa0 100644 > --- a/drivers/gpu/drm/solomon/ssd130x.c > +++ b/drivers/gpu/drm/solomon/ssd130x.c [ ... ] > @@ -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 code cause a GFP_KERNEL allocation in atomic context? When the screen is updated, ssd133x_primary_plane_atomic_update() (which runs in atomic context) calls ssd133x_fb_blit_rect() which in turn calls drm_fb_xrgb8888_to_rgb565be(). This internally calls __drm_fb_xfrm(), which attempts a GFP_KERNEL allocation if the conversion buffer was not previously reserved: drivers/gpu/drm/drm_format_helper.c:__drm_fb_xfrm() { if (!vaddr_cached_hint) { stmp = drm_format_conv_state_reserve(state, sbuf_len, GFP_KERNEL); if (!stmp) return -ENOMEM; } } Because ssd133x_primary_plane_atomic_check() omits the drm_format_conv_state_reserve() call (unlike its ssd130x and ssd132x variants): drivers/gpu/drm/solomon/ssd130x.c:ssd133x_primary_plane_atomic_check() { ... else if (!plane_state->visible) return 0; return 0; } Will this cause the allocation to be unconditionally executed in atomic context on the first frame update? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
