Re: [PATCH v2 3/4] drm/fl2000: Add frame streaming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/fl2000: stale damage replay on same-size mode set in
  fl2000_stream_mode_set()
--

commit fb468144aa76017b626067d45248552b6ec6cb90
Author: Angel Gabriel Crispin Valdivia <angelcrispinvaldivia@gmail.com>

drm/fl2000: Add frame streaming

This commit adds frame streaming support for the FL2000 USB display
adapter, converting damaged framebuffer rectangles into a shadow buffer and
sending frames over bulk USB transfers from a pool of scatter-gather
buffers.

> diff --git a/drivers/gpu/drm/fl2000/fl2000_streaming.c b/drivers/gpu/drm/fl2000/fl2000_streaming.c
> new file mode 100644
> index 0000000000000..dad640de5ec1a
> --- /dev/null
> +++ b/drivers/gpu/drm/fl2000/fl2000_streaming.c

[ ... ]

> +static void fl2000_copy_rect(struct fl2000_stream *stream, void *dst, const struct drm_rect *r)
> +{
> +	u32 line_len = stream->width * stream->bytes_pix;
> +	size_t off, span;
> +	int x1, x2, y1, y2;
> +
> +	/*
> +	 * A rectangle can outlive the geometry it was recorded under: a mode
> +	 * change that keeps the buffer size keeps the buffers too.
> +	 */
> +	x1 = clamp(r->x1, 0, (int)stream->width);
> +	x2 = clamp(r->x2, 0, (int)stream->width);
> +	y1 = clamp(r->y1, 0, (int)stream->height);
> +	y2 = clamp(r->y2, 0, (int)stream->height);
> +	if (x1 >= x2 || y1 >= y2)
> +		return;
> +
> +	off = (size_t)x1 * stream->bytes_pix;
> +	span = (size_t)(x2 - x1) * stream->bytes_pix;
> +
> +	for (int y = y1; y < y2; y++) {
> +		size_t start = round_down((size_t)y * line_len + off, 8);
> +		size_t end = min_t(size_t, round_up((size_t)y * line_len + off + span, 8),
> +				   stream->buf_size);
> +
> +		memcpy(dst + start, stream->shadow + start, end - start);
> +	}
> +}

[ ... ]

> +int fl2000_stream_mode_set(struct fl2000_stream *stream, unsigned int width,
> +			   unsigned int height, u32 bytes_pix)
> +{
> +	int ret;
> +	unsigned int size;
> +
> +	/* Round buffer size up to multiple of 8 to meet HW expectations */
> +	size = ALIGN(width * height * bytes_pix, 8);
> +
> +	guard(mutex)(&stream->shadow_lock);
> +
> +	/* If there are buffers with same size - keep them */
> +	if (stream->buf_size != size) {
> +		/*
> +		 * Without a shadow nothing writes a frame, and an allocation that fails below
> +		 * must not leave the old, smaller one to be written with the new geometry.
> +		 */
> +		fl2000_stream_put_buffers(stream);
> +		vfree(stream->shadow);
> +		stream->shadow = NULL;
> +		stream->buf_size = 0;
> +
> +		ret = fl2000_stream_get_buffers(stream, size);
> +		if (ret)
> +			return ret;
> +
> +		/* One full frame in the device format; only the CPU ever reads it */
> +		stream->shadow = vzalloc(size);
> +		if (!stream->shadow) {
> +			fl2000_stream_put_buffers(stream);
> +			return -ENOMEM;
> +		}
> +
> +		/* A buffer carrying zero has never been built and takes the full-copy path */
> +		stream->shadow_seq++;
> +
> +		stream->buf_size = size;
> +	}
> +
> +	stream->bytes_pix = bytes_pix;
> +	stream->width = width;
> +	stream->height = height;
> +
> +	return 0;
> +}

[Severity: Medium]
When a mode change keeps the same aligned buffer size (for example,
800x600 at 3 bytes/pixel to 1200x600 at 2 bytes/pixel, or a 90-degree
rotation), can fl2000_stream_build() corrupt outgoing buffers by replaying
old damage rectangles under the new geometry?

When stream->buf_size == size, fl2000_stream_mode_set() keeps stream->shadow
and the existing buffers with their current sb->seq values.  Rectangles in
stream->damage_log from before the mode change were written using the old
stride and pixel size, but fl2000_copy_rect() replays them using the new
width and bytes_pix, copying the wrong byte ranges from stream->shadow
while marking sb->seq caught up.  Advancing stream->shadow_seq by
FL2000_DAMAGE_LOG (and clearing stream->shadow) when the geometry or format
changes would force retained buffers onto the full-copy path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-fl2000-upstream-v2-0-201f87708cb0@gmail.com?part=3