Re: [PATCH] drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/gm12u320: fb offset dropped due to redundant drm_gem_fb_vmap call
- [High] drm/gm12u320: sleeping in atomic context via gm12u320_stop_fb_update
- [Medium] drm/gm12u320: redundant logic for FB swaps improperly executes for invisible planes
- [Medium] drm/tiny: commit message claims to fix EDID extension count but no code changes exist
--

commit f388ed102067844063cbe52109b60321de9bfe2f
Author: Hui Peng <benquike@gmail.com>

drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

This commit aims to fix framebuffer size, EDID extension count, and transfer
buffer bounds checks across several tiny DRM drivers.

> Fix framebuffer size, EDID extension count, and transfer buffer bounds
> checks across drivers/gpu/drm/tiny/ (bochs.c, gm12u320.c, pixpaper.c,
> and sharp-memory.c).

[Severity: Medium]
Is the commit message missing some context, or were the EDID extension
count changes left out of this patch? The diff modifies framebuffer
bounds and buffer maps, but it doesn't appear to contain any changes
related to EDID parsing or EDID extension counts.

> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4ad074337af07..9dfbfc6bb1b64 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -268,12 +268,18 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
>  	x2 = gm12u320->fb_update.rect.x2;
>  	y1 = gm12u320->fb_update.rect.y1;
>  	y2 = gm12u320->fb_update.rect.y2;
> -	vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping abstraction properly */
> +
> +	ret = drm_gem_fb_vmap(fb, &gm12u320->fb_update.src_map, NULL);

[Severity: High]
Does this redundant mapping overwrite the correct mapping provided by the
shadow plane helper and drop the framebuffer offset?

The shadow plane helpers already map the framebuffer and apply the offset
into the shadow plane state data, which gets passed to gm12u320_fb_mark_dirty()
and saved in gm12u320->fb_update.src_map.

By calling drm_gem_fb_vmap() here in gm12u320_copy_fb_to_blocks() with a NULL
data parameter, doesn't it bypass the offset calculation, causing the driver
to read from the base GEM address instead of the correct offset address?

[ ... ]

> @@ -583,8 +592,17 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
>  	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
>  	struct drm_rect rect;
>  
> -	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +	if (!state->fb) {
> +		gm12u320_stop_fb_update(to_gm12u320(pipe->crtc.dev));
> +		return;
> +	}

[Severity: High]
Can this sequence cause us to sleep in an atomic context?

gm12u320_pipe_update() implements the plane's atomic_update callback, which
runs in atomic context where sleeping is forbidden. However, this patch
introduces a call to gm12u320_stop_fb_update(), which directly calls
cancel_delayed_work_sync() and mutex_lock() - both of which can sleep.

> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect)) {
>  		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	} else if (old_state->fb != state->fb) {
> +		drm_rect_init(&rect, 0, 0, state->fb->width, state->fb->height);
> +		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	}
>  }

[Severity: Medium]
Will this fallback logic bypass visibility checks and incorrectly trigger
redundant full updates?

For visible planes, drm_atomic_helper_damage_merged() detects FB swaps and
returns true, making this else-if block dead code. But if the plane
transitions to invisible (state->visible == false), the helper returns false.
Won't this else-if block then incorrectly trigger and queue a full frame
transfer for an invisible plane?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919223513.3889412-1-benquike@gmail.com?part=1