[PATCH v8 06/13] media: rockchip: add a driver for the rockchip camera interface

[Mehdi Djait]

Hi Michael,

On Wed, Jun 18, 2025 at 02:21:51PM +0200, Michael Riesch wrote:
> Hi Mehdi,
> 
> On 6/13/25 14:33, Mehdi Djait wrote:
> > Hi Michael,
> > 
> > [...]
> 
> Let's cut the long story short, shall we :-)
> 

Yes, sorry for that :)

> >> +static void rkcif_stream_fill_format(struct rkcif_stream *stream,
> >> +				     struct v4l2_pix_format_mplane *pix)
> >> +{
> >> +	const struct rkcif_output_fmt *fmt;
> >> +	u32 height, width;
> >> +
> >> +	fmt = rkcif_stream_find_output_fmt(stream, true, pix->pixelformat);
> >> +	height = clamp_t(u32, pix->height, CIF_MIN_HEIGHT, CIF_MAX_HEIGHT);
> >> +	width = clamp_t(u32, pix->width, CIF_MIN_WIDTH, CIF_MAX_WIDTH);
> >> +	v4l2_fill_pixfmt_mp(pix, fmt->fourcc, width, height);
> > 
> > The rkcif supports the SRGGB10P: the packed version of the SRGGB10.
> > 
> > When you try to capture with SRGGB10P, the following fails in
> > v4l2_fill_pixfmt_mp()
> > 
> > 	info = v4l2_format_info(pixelformat);
> > 	if (!info)
> > 		return -EINVAL;
> > 
> > The return value is not checked in rkcif_stream_fill_format() resulting
> > in a call to queue_setup returning with sizes[0] = 0
> 
> Thanks for pointing that out. I failed to realize that this helper
> function may return an error. What I can do is to implement the error
> propagation correctly. Of course this will only avoid the kernel
> warning, you still won't be able to stream.
> 
> > This will cause a kernel Warning in the vb2_core_reqbufs() because
> > plane_size = 0
> > 
> > Exactly here:
> > 
> > 	for (i = 0; i < num_planes; i++)
> > 		if (WARN_ON(!plane_sizes[i])) {
> > 			ret = -EINVAL;
> > 			goto error;
> > 		}
> > 
> > I still don't have the solution here but wanted to let you know about
> > it.
> 
> I only had a quick look at it, but apparently the compact formats are
> missing in the format list here:
> https://elixir.bootlin.com/linux/v6.16-rc2/source/drivers/media/v4l2-core/v4l2-common.c#L244
> for whatever reason.
> 
> Have you by chance tried adding a line like
> {
> 	.format = V4L2_PIX_FMT_SRGGB10P,
> 	.pixel_enc = V4L2_PIXEL_ENC_BAYER,
> 	.mem_planes = 1,
> 	.comp_planes = 1,
> 	.bpp = { 5, 0, 0, 0 },
> 	.bpp_div = { 4, 1, 1, 1 },
> 	.hdiv = 1,
> 	.vdiv = 1,
> }
> ?
> 
> 
> Maybe we can fix this, but please note that this issue should not block
> merging this patch series.
> 

Of course! The solution from the driver side is to check the return
code.

I will investigate the issue with v4l2_fill_pixfmt_mp() separately.

> Best regards,
> Michael

--
Kind Regards
Mehdi Djait