Message-ID: <20250109171232.GA17638@pendragon.ideasonboard.com>
Date: Thu, 9 Jan 2025 19:12:32 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Michael Riesch <michael.riesch@...fvision.net>
Cc: Mehdi Djait <mehdi.djait@...ux.intel.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Théo Lebrun <theo.lebrun@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiko Stuebner <heiko@...ech.de>,
	Kever Yang <kever.yang@...k-chips.com>,
	Nicolas Dufresne <nicolas@...fresne.ca>,
	Sebastian Fricke <sebastian.fricke@...labora.com>,
	Alexander Shiyan <eagle.alexander923@...il.com>,
	Val Packett <val@...kett.cool>, Rob Herring <robh@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	linux-media@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org,
	Mehdi Djait <mehdi.djait@...tlin.com>,
	Gerald Loacker <gerald.loacker@...fvision.net>
Subject: Re: [PATCH v2 0/6] media: rockchip: add a driver for the rockchip
 camera interface (cif)

Hi Michael,

On Tue, Dec 17, 2024 at 04:55:12PM +0100, Michael Riesch wrote:
> Habidere,
> 
> TL;DR:
> 
> This series introduces support for the Rockchip Camera Interface (CIF),
> which can be found (in the form of variants that differ significantly) in
> different Rockchip SoCs in general, and for the Rockchip RK3568 Video
> Capture (VICAP) variant in particular.
> 
> The patches are functional and have been tested successfully on a
> custom RK3568 board including the ITE Tech. IT6801 HDMI receiver as
> attached subdevice. The IT6801 driver still needs some loving care but
> shall be submitted as well at some point.
> 
> The long story (gather 'round children):
> 
> The Rockchip Camera Interface (CIF) is featured in many Rockchip SoCs
> in different variations. For example, the PX30 Video Input Processor (VIP)
> is able to receive video data via the Digital Video Port (DVP, a parallel
> data interface and transfer it into system memory using a double-buffering
> mechanism called ping-pong mode. The RK3568 Video Capture (VICAP) unit,
> on the other hand, features a DVP and a MIPI CSI-2 receiver that can
> receive video data independently (both using the ping-pong scheme).
> The different variants may have additional features, such as scaling
> and/or cropping.
> Finally, the RK3588 VICAP unit constitutes an essential piece of the camera
> interface with one DVP, six MIPI CSI-2 receivers, scale/crop units, and
> different data path multiplexers (to system memory, to ISP, ...).
> 
> This submission bases on the work of several Bootlin developers who have
> been tirelessly submitting support for the PX30 Video Input Processor (VIP)
> block for inclusion in mainline. This process has been going on for several
> years now, with Maxime Chevallier working on the topic up to v5 [0] and
> Mehdi Djait taking over until v13 [1].
> In the review feedback on v13 a major rework with a media controller
> centric driver as a goal was requested. This motivated me to take over
> (with no clue about the MC framework whatsoever, though).
> 
> I decided to merge Mehdi's v13 with my v1 of the RK3568 VICAP support [2]
> and refactor the whole thing. The resulting v2 of the series now adds a
> basic media controller centric V4L2 driver for the Rockchip CIF with
>  - support for the PX30 VIP (not tested, though, due to the lack of HW)
>  - support for the RK3568 VICAP DVP
>  - abstraction for the ping-pong scheme to allow for future extensions 
> 
> However, several features are not yet addressed, such as
>  - support for the RK3568 MIPI CSI-2 receiver

We've discussed this previously on IRC, but I don't think the issue has
been raised on the list.

I'm puzzled by how this will work. As far as I understand, the RK3568
has a CSI-2 receiver with 4 data lanes and 2 clock lanes. I assume this
is used to support both 2x2 lanes and 1x4 lanes. Both the VICAP and ISP
sections of the TRM list CSI2 RX registers, but it's not clear how the
components are all connected. Does the ISP need to be part of the same
media graph ?

>  - support for the RK3588 variant
>  - support for the scaling/cropping units that can be found in some
>    variants
>  - support for capturing different virtual channels (up to four IDs
>    possible)
> This needs to be in the scope of future work.
> 
> Finally, please forgive me if I forgot to address reviewer comments from
> the previous iterations. Between v1 and v13 they have seen significant
> feedback including renaming the complete driver twice (from rkcif to vip
> and back to cif) and I am pretty sure that I was not able to gather
> everything.
> 
> Looking forward to your comments!
> 
> [0] https://lore.kernel.org/linux-media/20201229161724.511102-1-maxime.chevallier@bootlin.com/
> [1] https://lore.kernel.org/linux-media/cover.1707677804.git.mehdi.djait.k@gmail.com/
> [2] https://lore.kernel.org/all/20240220-v6-8-topic-rk3568-vicap-v1-0-2680a1fa640b@wolfvision.net/
> 
> To: Mehdi Djait <mehdi.djait@...ux.intel.com>
> To: Maxime Chevallier <maxime.chevallier@...tlin.com>
> To: Théo Lebrun <theo.lebrun@...tlin.com>
> To: Thomas Petazzoni <thomas.petazzoni@...tlin.com>
> To: Sakari Ailus <sakari.ailus@....fi>
> To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> To: Mauro Carvalho Chehab <mchehab@...nel.org>
> To: Rob Herring <robh+dt@...nel.org>
> To: Krzysztof Kozlowski <krzk+dt@...nel.org>
> To: Conor Dooley <conor+dt@...nel.org>
> To: Heiko Stuebner <heiko@...ech.de>
> To: Kever Yang <kever.yang@...k-chips.com>
> To: Nicolas Dufresne <nicolas@...fresne.ca>
> To: Sebastian Fricke <sebastian.fricke@...labora.com>
> To: Alexander Shiyan <eagle.alexander923@...il.com>
> To: Val Packett <val@...kett.cool>
> To: Rob Herring <robh@...nel.org>
> To: Philipp Zabel <p.zabel@...gutronix.de>
> Cc: linux-media@...r.kernel.org
> Cc: devicetree@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-rockchip@...ts.infradead.org
> Signed-off-by: Michael Riesch <michael.riesch@...fvision.net>
> 
> Changes in v2:
> - merged with Mehdi's v13
> - refactored the complete driver towards a media controller centric driver
> - abstracted the generic ping-pong stream (can be used for DVP as well as for CSI-2)
> - switched to MPLANE API
> - added support for notifications
> - Link to v1: https://lore.kernel.org/r/20240220-v6-8-topic-rk3568-vicap-v1-0-2680a1fa640b@wolfvision.net
> 
> ---
> Mehdi Djait (2):
>       media: dt-bindings: media: add bindings for rockchip px30 vip
>       arm64: dts: rockchip: add the vip node to px30
> 
> Michael Riesch (4):
>       media: dt-bindings: media: video-interfaces: add defines for sampling modes
>       media: dt-bindings: media: add bindings for rockchip rk3568 vicap
>       media: rockchip: add a driver for the rockchip camera interface (cif)
>       arm64: dts: rockchip: add vicap node to rk356x
> 
>  .../bindings/media/rockchip,px30-vip.yaml          | 123 ++++
>  .../bindings/media/rockchip,rk3568-vicap.yaml      | 168 +++++
>  MAINTAINERS                                        |   9 +
>  arch/arm64/boot/dts/rockchip/px30.dtsi             |  12 +
>  arch/arm64/boot/dts/rockchip/rk356x-base.dtsi      |  44 ++
>  drivers/media/platform/rockchip/Kconfig            |   1 +
>  drivers/media/platform/rockchip/Makefile           |   1 +
>  drivers/media/platform/rockchip/cif/Kconfig        |  15 +
>  drivers/media/platform/rockchip/cif/Makefile       |   3 +
>  .../media/platform/rockchip/cif/cif-capture-dvp.c  | 794 +++++++++++++++++++++
>  .../media/platform/rockchip/cif/cif-capture-dvp.h  |  24 +
>  drivers/media/platform/rockchip/cif/cif-common.h   | 163 +++++
>  drivers/media/platform/rockchip/cif/cif-dev.c      | 405 +++++++++++
>  drivers/media/platform/rockchip/cif/cif-regs.h     | 132 ++++
>  drivers/media/platform/rockchip/cif/cif-stream.c   | 676 ++++++++++++++++++
>  drivers/media/platform/rockchip/cif/cif-stream.h   |  24 +
>  include/dt-bindings/media/video-interfaces.h       |   4 +
>  17 files changed, 2598 insertions(+)
> ---
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> change-id: 20240220-v6-8-topic-rk3568-vicap-b9b3f9925f44

-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists