Re: [PATCH v4] media: rcar-isp: Add Renesas R-Car Image Signal Processor driver

Hej Sakari,

On 2021-07-28 20:03:37 +0300, Sakari Ailus wrote:

Hej Niklas,

On Tue, Jul 27, 2021 at 07:55:49PM +0200, Niklas Söderlund wrote:
...

quoted

quoted

quoted

quoted

quoted

+static const struct v4l2_subdev_pad_ops risp_pad_ops = {
+	.set_fmt = risp_set_pad_format,
+	.get_fmt = risp_get_pad_format,

Maybe set link_validate to v4l2_subdev_link_validate_default?

I thought about that but with the multiplexed stream issue I thought it 
best to not to add that yet. Do you think this make sens?

What would be the alternative? Without that, there's no guarantee
whatsoever what the input is. It may also enable writing user space that
breaks after fixing this in the driver.

We still still don't have any guarantees whatsoever as we can only 
describe and validate one stream.

quoted

Where in the pipeline would you have multiple streams over a single data
path?

On Gen3 platforms without an ISP we have it on the rcar-csi2 sink pad 
and on Gen3 platforms with an ISP we have it on the rcar-isp sink pad.  
In both cases it's the IP block that separates the CSI-2 buss to the 
different capture engines.

Ok. In this case the driver may be better merged to the staging tree or
labelled experimental as its UAPI isn't stable. I wonder what Hans thinks.

Marking it experimental maybe a good idea or only allowing one stream on 
the CSI-2 bus. Then we could use v4l2_subdev_link_validate_default and 
validate the link pad format, I will do so in the next version.

-- 
Kind regards,

Sakari Ailus

-- 
Regards,
Niklas Söderlund