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

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 3314 bytes --]

Hi Torsten,

Le jeudi 22 octobre 2026 à 19:00 +0200, Torsten Duwe a écrit :
> 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?

Drivers only implement one queue type, MPLANE is the recommended one. There is
pixel formats for single allocation and multi-allocation that userspace can pick
from. So a driver can do MPLANE and support both NV12 and NV12M formats as an
example.

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

Should actually match what the JPEG header says, RGB jpeg exist.

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

I believe Sascha has been testing with GStreamer, the element is called
v4l2jpegdec.

Nicolas

> 
> Thanks,
> 	Torsten
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]