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

Pre-existing issues:
- [High] The format conversion state buffer is drastically under-allocated (or 
not allocated at all) in `atomic_check`, forcing `GFP_KERNEL` reallocation 
during the `atomic_update` phase.
--

--- Patch [1]: [PATCH] drm/solomon: remove unneeded variables in blit functions 
---
commit 8b406f12f83f8bdc420f079608108a65e9772103
Author: Iker Pedrosa <[email protected]>

drm/solomon: remove unneeded variables in blit functions

Remove unneeded 'ret' variables in ssd130x_fb_blit_rect(),
ssd132x_fb_blit_rect(), and ssd133x_fb_blit_rect() functions.
These functions initialize ret to 0 and return it unchanged,
so return 0 directly instead.

Closes: 
https://lore.kernel.org/oe-kbuild-all/[email protected]/

> diff --git a/drivers/gpu/drm/solomon/ssd130x.c 
> b/drivers/gpu/drm/solomon/ssd130x.c
> index 4907be694aecb..0940eae7a2e49 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
[ ... ]
> @@ -960,7 +957,7 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer 
> *fb,
>  
>       ssd133x_update_rect(ssd130x, rect, data_array, dst_pitch);
>  
> -     return ret;
> +     return 0;
>  }
>  
>  static int ssd130x_primary_plane_atomic_check(struct drm_plane *plane,

[Severity: High]
This isn't a bug introduced by this patch, but does the format conversion
state buffer get drastically under-allocated (or not allocated at all) in
the atomic_check functions, forcing a GFP_KERNEL reallocation during the
atomic_update phase?

Looking at ssd130x_primary_plane_atomic_check(), it reserves memory based on
the much smaller destination format pitch instead of the XRGB8888 source
format requirement:

        if (plane_state->fb->format != fi) {
                void *buf;

                /* format conversion necessary; reserve buffer */
                buf = 
drm_format_conv_state_reserve(&shadow_plane_state->fmtcnv_state,
                                                    pitch, GFP_KERNEL);
                if (!buf)
                        return -ENOMEM;
        }

Later, during the non-blocking atomic_update phase, conversion helpers like
drm_fb_xrgb8888_to_mono() request a much larger buffer:

        src32 = drm_format_conv_state_reserve(state, len_src32 + linepixels, 
GFP_KERNEL);

Since the newly requested size exceeds the previously reserved size, won't
this force drm_format_conv_state_reserve() to execute a sleeping krealloc
with GFP_KERNEL in atomic context?

This same pattern appears in ssd132x_primary_plane_atomic_check(), and
ssd133x_primary_plane_atomic_check() completely omits calling
drm_format_conv_state_reserve() which guarantees a 0 size reserve.

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

Reply via email to