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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

I suggest to put that LLM away for a bit and rather try to get one of 
these drivers to work. Once you have that, try to make a test case for 
one of these "problems". And when that is ready, we can see how to fix it.

Best regards
Thomas

Am 20.09.26 um 00:35 schrieb Hui Peng:
> 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).
>
> Fixes: 77b8cabf3d52 ("drm/gm12u320: Move driver to drm/tiny")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index e2d957e51505..19d0af677321 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -422,6 +422,8 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   						   struct drm_atomic_commit *state)
>   {
>   	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_framebuffer *fb = new_plane_state->fb;
> +	struct bochs_device *bochs = to_bochs_device(plane->dev);
>   	struct drm_crtc *new_crtc = new_plane_state->crtc;
>   	struct drm_crtc_state *new_crtc_state = NULL;
>   	int ret;
> @@ -438,6 +440,9 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   	else if (!new_plane_state->visible)
>   		return 0;
>   
> +	if (fb && (u64)fb->pitches[0] * fb->height > bochs->fb_size)
> +		return -EINVAL;
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4ad074337af0..9dfbfc6bb1b6 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);
> +	if (ret) {
> +		GM12U320_ERR("drm_gem_fb_vmap err: %d\n", ret);
> +		goto put_fb;
> +	}
> +	vaddr = gm12u320->fb_update.src_map.vaddr;
>   
>   	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>   	if (ret) {
>   		GM12U320_ERR("drm_gem_fb_begin_cpu_access err: %d\n", ret);
> -		goto put_fb;
> +		goto vunmap_fb;
>   	}
>   
>   	src = vaddr + y1 * fb->pitches[0] + x1 * 4;
> @@ -311,6 +317,8 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
>   	}
>   
>   	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +vunmap_fb:
> +	drm_gem_fb_vunmap(fb, &gm12u320->fb_update.src_map);
>   put_fb:
>   	drm_framebuffer_put(fb);
>   	gm12u320->fb_update.fb = NULL;
> @@ -418,6 +426,7 @@ static void gm12u320_fb_mark_dirty(struct drm_framebuffer *fb,
>   	} else {
>   		struct drm_rect *rect = &gm12u320->fb_update.rect;
>   
> +		gm12u320->fb_update.src_map = *map;
>   		rect->x1 = min(rect->x1, dirty->x1);
>   		rect->y1 = min(rect->y1, dirty->y1);
>   		rect->x2 = max(rect->x2, dirty->x2);
> @@ -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;
> +	}
> +
> +	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);
> +	}
>   }
>   
>   static const struct drm_simple_display_pipe_funcs gm12u320_pipe_funcs = {
> diff --git a/drivers/gpu/drm/tiny/pixpaper.c b/drivers/gpu/drm/tiny/pixpaper.c
> index d02ac26d007c..475e92c3410e 100644
> --- a/drivers/gpu/drm/tiny/pixpaper.c
> +++ b/drivers/gpu/drm/tiny/pixpaper.c
> @@ -865,7 +865,7 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
>   	struct drm_shadow_plane_state *shadow_plane_state =
>   		to_drm_shadow_plane_state(plane_state);
>   	struct drm_crtc *crtc = plane_state->crtc;
> -	struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> +	struct pixpaper_panel *panel = to_pixpaper_panel(plane->dev);
>   
>   	struct drm_device *drm = &panel->drm;
>   	struct drm_framebuffer *fb = plane_state->fb;
> @@ -875,6 +875,9 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
>   	__le32 *src_pixels = NULL;
>   	struct pixpaper_error_ctx err = { .errno_code = 0 };
>   
> +	if (!crtc || !fb)
> +		return;
> +
>   	if (!drm_dev_enter(drm, &idx))
>   		return;
>   
> diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
> index 7efd7b567f3b..b365e7549666 100644
> --- a/drivers/gpu/drm/tiny/sharp-memory.c
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -595,6 +595,9 @@ static int sharp_memory_probe(struct spi_device *spi)
>   		smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
>   		smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
>   						  smd, "sw_vcom_signal");
> +		if (IS_ERR(smd->sw_vcom_signal))
> +			return dev_err_probe(dev, PTR_ERR(smd->sw_vcom_signal),
> +					     "Failed to start sw_vcom_signal thread\n");
>   
>   	} else if (!strcmp("external", vcom_mode_str)) {
>   		smd->vcom_mode = SHARP_MEMORY_EXTERNAL_VCOM;
> @@ -616,15 +619,21 @@ static int sharp_memory_probe(struct spi_device *spi)
>   	ret = sharp_memory_pipe_init(drm, smd, sharp_memory_formats,
>   				     ARRAY_SIZE(sharp_memory_formats),
>   				     NULL);
> -	if (ret)
> +	if (ret) {
> +		if (smd->sw_vcom_signal)
> +			kthread_stop(smd->sw_vcom_signal);
>   		return dev_err_probe(dev, ret, "Failed to initialize display pipeline.\n");
> +	}
>   
>   	drm_plane_enable_fb_damage_clips(&smd->plane);
>   	drm_mode_config_reset(drm);
>   
>   	ret = drm_dev_register(drm, 0);
> -	if (ret)
> +	if (ret) {
> +		if (smd->sw_vcom_signal)
> +			kthread_stop(smd->sw_vcom_signal);
>   		return dev_err_probe(dev, ret, "Failed to register drm device.\n");
> +	}
>   
>   	drm_client_setup(drm, NULL);
>   

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)