Hi Michael,

On Wed, Mar 25, 2026 at 09:04:57AM +0100, Michael Riesch wrote:

Hi Mehdi,

On 3/17/26 14:28, Mehdi Djait wrote:

quoted

Hi Michael,

On Tue, Mar 17, 2026 at 02:21:20PM +0100, Michael Riesch wrote:

quoted

Hi Mehdi,

On 3/17/26 14:08, Mehdi Djait wrote:

quoted

Hi Michael,

Thank you for this nice patch!

On Tue, Mar 17, 2026 at 10:32:21AM +0100, Michael Riesch via B4 Relay wrote:

quoted

From: Michael Riesch <michael.riesch@collabora.com>

The RK3588 Video Capture (VICAP) unit features a Digital Video Port
(DVP) and six MIPI CSI-2 capture interfaces. Add initial support
for this variant to the rkcif driver and enable the MIPI CSI-2
capture interfaces.

Signed-off-by: Michael Riesch <michael.riesch@collabora.com>

[...]

quoted

 static inline unsigned int rkcif_mipi_get_reg(struct rkcif_interface *interface,
 					      unsigned int index)
 {

@@ -631,6 +765,8 @@ static int rkcif_mipi_start_streaming(struct rkcif_stream *stream)
 	rkcif_mipi_stream_write(stream, RKCIF_MIPI_CTRL1, ctrl1);
 	rkcif_mipi_stream_write(stream, RKCIF_MIPI_CTRL0, ctrl0);
 
+	rkcif_mipi_write(interface, RKCIF_MIPI_CTRL, RKCIF_MIPI_CTRL_CAP_EN);
+

while this is the correct solution for rk3588, for the rk3568 vicap this
will write 0x1 to the VICAP_MIPI_CTRL : 0x00A0 which will enable the water line.

nice catch ;-) However, the TRM (at least my version) claims that this
bit has a reset value of 0x1, so the bit in question should be already
set in the first place. Thus I decided to *not* make variant specific
code paths.

Yes, the reset value is indeed 0x1

quoted

Do you see problems in your setup?

No problems, it works as expected, I was just confused to see the
mipi capture enable added with this rk3588 patch and not before.

I just find it a bit confusing but if a nicer solution is too much
hassle we can leave it like this.

I agree that this is (while correct) not the nicest way. I am still
bringing up the remaining features of the RK3588 VICAP (MUX + TOISP +
SCALER) and it looks like I need to do some refactoring anyway to
support them. When I do that, I shall rewrite this part. For the time
being it would be great to have this merged in order to provide initial
mainline support for this unit.

That sounds good.

How about adding a TODO comment ?

--
Kind Regards
Mehdi Djait