Re: [PATCH v7 2/4] media: rockchip: Add JPEG decoder driver

[Torsten Duwe <duwe@lst.de>]

Hi Sascha,

first: thank you for driving this :)

On Thu, 22 Oct 2026 13:06:40 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Add a driver for the JPEG hardware decoder Rockchip integrates into a
> number of its SoCs. 

[...]

> +static int rkjpegd_enum_fmt_vid_out(struct file *file, void *priv,
> +				    struct v4l2_fmtdesc *f)
> +{
> +	if (f->index)
> +		return -EINVAL;
> +
> +	f->pixelformat = V4L2_PIX_FMT_JPEG;
> +	f->flags |= V4L2_FMT_FLAG_DYN_RESOLUTION;
> +
> +	return 0;
> +}
> +
> +static int rkjpegd_enum_framesizes(struct file *file, void *priv,
> +				   struct v4l2_frmsizeenum *fsize)
> +{
> +	/* The decoded format follows the bitstream, nothing to enumerate. */
> +	if (fsize->pixel_format == V4L2_PIX_FMT_NV12 ||
> +	    fsize->pixel_format == V4L2_PIX_FMT_GREY)
> +		return -ENOTTY;
> +
> +	if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
> +		return -EINVAL;
> +
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise.min_width = RKJPEGD_MIN_WIDTH;
> +	fsize->stepwise.max_width = RKJPEGD_MAX_SIZE;
> +	fsize->stepwise.step_width = RKJPEGD_CODED_STEP;
> +	fsize->stepwise.min_height = RKJPEGD_MIN_HEIGHT;
> +	fsize->stepwise.max_height = RKJPEGD_MAX_SIZE;
> +	fsize->stepwise.step_height = RKJPEGD_CODED_STEP;
> +
> +	return 0;
> +}

Can it do JPEG, MJPEG or maybe both? AFAICT the difference is minimal,
so maybe try to be more permissive?

[...]

> +static const struct v4l2_ioctl_ops rkjpegd_ioctl_ops = {
> +	.vidioc_querycap = rkjpegd_querycap,
> +	.vidioc_enum_framesizes = rkjpegd_enum_framesizes,
> +
> +	.vidioc_enum_fmt_vid_cap = rkjpegd_enum_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap_mplane = rkjpegd_g_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap_mplane = rkjpegd_try_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap_mplane = rkjpegd_s_fmt_vid_cap,
> +
> +	.vidioc_enum_fmt_vid_out = rkjpegd_enum_fmt_vid_out,
> +	.vidioc_g_fmt_vid_out_mplane = rkjpegd_g_fmt_vid_out,
> +	.vidioc_try_fmt_vid_out_mplane = rkjpegd_try_fmt_vid_out,
> +	.vidioc_s_fmt_vid_out_mplane = rkjpegd_s_fmt_vid_out,
> 

You are populating only the *_out_mplane members and not the single
plane ones? So a greyscale JPEG needs to go _mplane with 1 plane?

And for color, the colorspace on the decoded side should be
V4L2_COLORSPACE_JPEG, not sRGB, I think.

Would you mind to give a hint about your test setup?

Thanks,
	Torsten