Re: [PATCH] drm/solomon: remove unneeded variables in blit functions

[sashiko-bot@kernel.org]

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 <ikerpedrosam@gmail.com>

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/202606301409.I0ctsf41-lkp@intel.com/

> 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/20260709-fix-ssd130x-v1-1-1272cb3dc85e@gmail.com?part=1