mbox series

[v2,0/6] media: rockchip: add a driver for the rockchip camera interface (cif)

Message ID 20241217-v6-8-topic-rk3568-vicap-v2-0-b1d488fcc0d3@wolfvision.net
Headers show
Series media: rockchip: add a driver for the rockchip camera interface (cif) | expand

Message

Michael Riesch Dec. 17, 2024, 3:55 p.m. UTC
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
 - 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@linux.intel.com>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: Théo Lebrun <theo.lebrun@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Heiko Stuebner <heiko@sntech.de>
To: Kever Yang <kever.yang@rock-chips.com>
To: Nicolas Dufresne <nicolas@ndufresne.ca>
To: Sebastian Fricke <sebastian.fricke@collabora.com>
To: Alexander Shiyan <eagle.alexander923@gmail.com>
To: Val Packett <val@packett.cool>
To: Rob Herring <robh@kernel.org>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-media@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Michael Riesch <michael.riesch@wolfvision.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

Best regards,

Comments

Rob Herring Dec. 30, 2024, 8:08 p.m. UTC | #1
On Tue, Dec 17, 2024 at 04:55:15PM +0100, Michael Riesch wrote:
> Add documentation for the Rockchip RK3568 Video Capture (VICAP) unit.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  .../bindings/media/rockchip,rk3568-vicap.yaml      | 168 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 169 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
> new file mode 100644
> index 000000000000..ef7b14ca6879
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/rockchip,rk3568-vicap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3568 Video Capture (VICAP)
> +
> +maintainers:
> +  - Michael Riesch <michael.riesch@wolfvision.net>
> +
> +description:
> +  The Rockchip RK3568 Video Capture (VICAP) block features a digital video
> +  port (DVP, a parallel video interface) and a MIPI CSI-2 port. It receives
> +  the data from camera sensors, video decoders, or other companion ICs and
> +  transfers it into system main memory by AXI bus.
> +
> +properties:
> +  compatible:
> +    const: rockchip,rk3568-vicap
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: ACLK
> +      - description: HCLK
> +      - description: DCLK
> +      - description: ICLK
> +
> +  clock-names:
> +    items:
> +      - const: aclk
> +      - const: hclk
> +      - const: dclk
> +      - const: iclk
> +
> +  rockchip,cif-clk-delaynum:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 127
> +    description:
> +      Delay the DVP path clock input to align the sampling phase, only valid
> +      in dual edge sampling mode.
> +
> +  iommus:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: ARST
> +      - description: HRST
> +      - description: DRST
> +      - description: PRST
> +      - description: IRST
> +
> +  reset-names:
> +    items:
> +      - const: arst
> +      - const: hrst
> +      - const: drst
> +      - const: prst
> +      - const: irst
> +
> +  rockchip,grf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to general register file used for video input block control.
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description: input port on the parallel interface

What about the CSI-2 interface?

> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              bus-type:
> +                enum: [5, 6]
> +
> +            required:
> +              - bus-type
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3568-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/power/rk3568-power.h>
> +    #include <dt-bindings/media/video-interfaces.h>
> +
> +    parent {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        vicap: video-capture@fdfe0000 {
> +            compatible = "rockchip,rk3568-vicap";
> +            reg = <0x0 0xfdfe0000 0x0 0x200>;
> +            interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
> +            assigned-clocks = <&cru DCLK_VICAP>;
> +            assigned-clock-rates = <300000000>;
> +            clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>,
> +                     <&cru DCLK_VICAP>, <&cru ICLK_VICAP_G>;
> +            clock-names = "aclk", "hclk", "dclk", "iclk";
> +            iommus = <&vicap_mmu>;
> +            power-domains = <&power RK3568_PD_VI>;
> +            resets = <&cru SRST_A_VICAP>, <&cru SRST_H_VICAP>,
> +                     <&cru SRST_D_VICAP>, <&cru SRST_P_VICAP>,
> +                     <&cru SRST_I_VICAP>;
> +            reset-names = "arst", "hrst", "drst", "prst", "irst";
> +            rockchip,grf = <&grf>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                vicap_dvp: port@0 {
> +                    reg = <0>;
> +
> +                    vicap_dvp_input: endpoint {
> +                        bus-type = <MEDIA_BUS_TYPE_BT656>;
> +                        bus-width = <16>;
> +                        pclk-sample = <MEDIA_PCLK_SAMPLE_DUAL_EDGE>;
> +                        remote-endpoint = <&it6801_output>;
> +                    };
> +                };
> +
> +                vicap_mipi: port@1 {
> +                    reg = <1>;
> +                };
> +            };
> +        };
> +
> +        vicap_mmu: iommu@fdfe0800 {
> +            compatible = "rockchip,rk3568-iommu";

Not part of this binding, so drop this node.

> +            reg = <0x0 0xfdfe0800 0x0 0x100>;
> +            interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>;
> +            clock-names = "aclk", "iface";
> +            #iommu-cells = <0>;
> +            power-domains = <&power RK3568_PD_VI>;
> +            rockchip,disable-mmu-reset;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1138c8858bc7..8dbeb2927a08 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20223,6 +20223,7 @@ M:	Michael Riesch <michael.riesch@wolfvision.net>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
> +F:	Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
>  
>  ROCKCHIP CRYPTO DRIVERS
>  M:	Corentin Labbe <clabbe@baylibre.com>
> 
> -- 
> 2.34.1
>
Michael Riesch Jan. 7, 2025, 11:07 a.m. UTC | #2
Hi Rob,

On 12/30/24 21:08, Rob Herring wrote:
> On Tue, Dec 17, 2024 at 04:55:15PM +0100, Michael Riesch wrote:
> [...]
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description: input port on the parallel interface
> 
> What about the CSI-2 interface?

If it is OK to add it to binding already although there is nothing in
the driver code that uses it, I will be happy to add it in v3.

Otherwise, I'll add it together with the actual MIPI CSI-2 support.

> 
>> [...]
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/rk3568-cru.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/power/rk3568-power.h>
>> +    #include <dt-bindings/media/video-interfaces.h>
>> +
>> +    parent {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        vicap: video-capture@fdfe0000 {
>> +            compatible = "rockchip,rk3568-vicap";
>> +            reg = <0x0 0xfdfe0000 0x0 0x200>;
>> +            interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
>> +            assigned-clocks = <&cru DCLK_VICAP>;
>> +            assigned-clock-rates = <300000000>;
>> +            clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>,
>> +                     <&cru DCLK_VICAP>, <&cru ICLK_VICAP_G>;
>> +            clock-names = "aclk", "hclk", "dclk", "iclk";
>> +            iommus = <&vicap_mmu>;
>> +            power-domains = <&power RK3568_PD_VI>;
>> +            resets = <&cru SRST_A_VICAP>, <&cru SRST_H_VICAP>,
>> +                     <&cru SRST_D_VICAP>, <&cru SRST_P_VICAP>,
>> +                     <&cru SRST_I_VICAP>;
>> +            reset-names = "arst", "hrst", "drst", "prst", "irst";
>> +            rockchip,grf = <&grf>;
>> +
>> +            ports {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                vicap_dvp: port@0 {
>> +                    reg = <0>;
>> +
>> +                    vicap_dvp_input: endpoint {
>> +                        bus-type = <MEDIA_BUS_TYPE_BT656>;
>> +                        bus-width = <16>;
>> +                        pclk-sample = <MEDIA_PCLK_SAMPLE_DUAL_EDGE>;
>> +                        remote-endpoint = <&it6801_output>;
>> +                    };
>> +                };
>> +
>> +                vicap_mipi: port@1 {
>> +                    reg = <1>;
>> +                };
>> +            };
>> +        };
>> +
>> +        vicap_mmu: iommu@fdfe0800 {
>> +            compatible = "rockchip,rk3568-iommu";
> 
> Not part of this binding, so drop this node.

Ack, will remove in v3.

Thanks and regards,
Michael

> 
>> +            reg = <0x0 0xfdfe0800 0x0 0x100>;
>> +            interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
>> +            clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>;
>> +            clock-names = "aclk", "iface";
>> +            #iommu-cells = <0>;
>> +            power-domains = <&power RK3568_PD_VI>;
>> +            rockchip,disable-mmu-reset;
>> +        };
>> +    };
>> +...
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1138c8858bc7..8dbeb2927a08 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -20223,6 +20223,7 @@ M:	Michael Riesch <michael.riesch@wolfvision.net>
>>  L:	linux-media@vger.kernel.org
>>  S:	Maintained
>>  F:	Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
>> +F:	Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
>>  
>>  ROCKCHIP CRYPTO DRIVERS
>>  M:	Corentin Labbe <clabbe@baylibre.com>
>>
>> -- 
>> 2.34.1
>>
Laurent Pinchart Jan. 9, 2025, 5:12 p.m. UTC | #3
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@linux.intel.com>
> To: Maxime Chevallier <maxime.chevallier@bootlin.com>
> To: Théo Lebrun <theo.lebrun@bootlin.com>
> To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> To: Sakari Ailus <sakari.ailus@iki.fi>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Rob Herring <robh+dt@kernel.org>
> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Heiko Stuebner <heiko@sntech.de>
> To: Kever Yang <kever.yang@rock-chips.com>
> To: Nicolas Dufresne <nicolas@ndufresne.ca>
> To: Sebastian Fricke <sebastian.fricke@collabora.com>
> To: Alexander Shiyan <eagle.alexander923@gmail.com>
> To: Val Packett <val@packett.cool>
> To: Rob Herring <robh@kernel.org>
> To: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: linux-media@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.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
Michael Riesch Jan. 10, 2025, 8:48 a.m. UTC | #4
Hi Laurent,

On 1/9/25 18:12, Laurent Pinchart wrote:
> Hi Michael,
> 
> On Tue, Dec 17, 2024 at 04:55:12PM +0100, Michael Riesch wrote:
>> [...]
>> 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 ?

Well you are not the only one to be puzzled by this HW :-) I started to
bring up the MIPI CSI-2 part and can describe the situation as I
understand it. No claim for correctness or completeness, though.

Indeed, the RK3568 MIPI CSI-2 PHY (TRM Chapter 28) has 4 data lanes and
2 clock lanes. And indeed it is possible to attach one device (1x4) or
two devices (2x2) to it. In the latter case the PHY is operated in split
mode (and 1x4 would be the default full mode, then). The mode can be set
in a GRF register.

The RK3568 features a MIPI CSI-2 host controller (TRM Chapter 27) that
(apparently) is connected to the RK3568 VICAP but has its registers in a
separate memory region. Right now I am trying to clean up the downstream
V4L2 subdevice driver for this host controller, which shall be linked to
the PHY using the "phys" DT property, and linked to the VICAP using DT
endpoints. In the end, the media graph could look like

  CC1 (V4L2 subdev) --> RK3568 VICAP DVP (V4L2 device)

  CC2 (V4L2 subdev) --> RK3568 MIPI CSI-2 HOST (V4L2 subdev) -->
  RK3568 VICAP MIPI (V4L2 device)

where CC denotes a companion chip, such as an image sensor or a video
decoder. Note that there are two disjoint subgraphs in this topology.

As you already pointed out, the RK3568 ISP features its integrated MIPI
CSI-2 host controller (as e.g. the RK3399 ISP does). This host
controller is represented by a V4L2 subdevice as well, and the
connection to the (same) MIPI PHY is established similarly. The
difference may be that this controller has its registers within the ISP
register block and is thus tightly coupled to the ISP (and the rkisp1
driver).

Now I guess that in split mode the ISP MIPI path works completely
independent and could be represented either by a separate media graph or
by another disjoint subgraph in the same media graph.
In split mode we need to define which host controller should handle
which set of lanes (there are two sets, clock 1 + data 1 + data 2 as
well as clock 2 + data 3 + data 4). Right now I am not sure how this
should be accomplished, but then the split mode can also wait a bit.

However, as you pointed out in our IRC discussion, it is mentioned in
TRM Chapter 12 that the RK3568 ISP can also process the DVP input. This
still needs some verification, the rest of the paragraph is pure
speculation. But maybe on simply needs to set the correct bits in
CTRL_0000_VI_DPCL. Now this would mean that there should be one large
media graph the includes the RK3568 VICAP as well as the RK3568 ISP,
i.e., something along the lines

  CC0 (V4L2 subdev) --> RK3568 ISP MIPI CSI-2 HOST (V4L2 subdev)
                            |
                            v
                     RK3568 ISP MUX (V4L2 subdev) --> RK3568 ISP...
                            ^
                            |
  CC1 (V4L2 subdev) --> MAGIC V4L2 subdev? -->
  RK3568 VICAP DVP (V4L2 device)

  CC2 (V4L2 subdev) --> RK3568 MIPI CSI-2 HOST (V4L2 subdev) -->
  RK3568 VICAP MIPI (V4L2 device)

where the MAGIC V4L2 subdev should be a fan out of some sort?!

Now to answer your question: I guess it is going to be one media graph
for RK3568 VICAP and RK3588 ISP. This shall be in perfect alignment with
the RK3588, where VICAP and ISPs are clearly connected.

Is this something we need to take into account right now, though? Or can
we complete the work on the VICAP first and then start adding the ISP?

Hope that helps!

Regards,
Michael

> 
>>  - 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@linux.intel.com>
>> To: Maxime Chevallier <maxime.chevallier@bootlin.com>
>> To: Théo Lebrun <theo.lebrun@bootlin.com>
>> To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> To: Sakari Ailus <sakari.ailus@iki.fi>
>> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> To: Mauro Carvalho Chehab <mchehab@kernel.org>
>> To: Rob Herring <robh+dt@kernel.org>
>> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
>> To: Conor Dooley <conor+dt@kernel.org>
>> To: Heiko Stuebner <heiko@sntech.de>
>> To: Kever Yang <kever.yang@rock-chips.com>
>> To: Nicolas Dufresne <nicolas@ndufresne.ca>
>> To: Sebastian Fricke <sebastian.fricke@collabora.com>
>> To: Alexander Shiyan <eagle.alexander923@gmail.com>
>> To: Val Packett <val@packett.cool>
>> To: Rob Herring <robh@kernel.org>
>> To: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: linux-media@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-rockchip@lists.infradead.org
>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.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
>
Mehdi Djait Jan. 15, 2025, 10:55 a.m. UTC | #5
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, ...).

I wanted to CC Paul on this and add this really GOOD summary he provided
while reviewing v6 of the CIF driver: Feel free to add (or not) some of
the infos from it.

----------------------------------------------------------------------------------
So I have spent a bit of time trying to figure out the history of this unit
and in which platform in was used. The takeaway is that the earliest Rockchip
SoC that uses it is the RK3066 (2012) and the latest SoC is the RK3566/RK3568
(2020). Earlier SoCs (RK29) do mention VIP but seems quite clear that this is
a whole different unit and the recent RK3588 (2021) has a new VICAP_DVP unit
(mixed with VICAP_MIPI) which also seems significantly different.

Over the course of the existence of this unit, it is most often referred to
as CIF. Since this is also the name for the driver in the Rockchip tree,
I feel like it is best to use CIF as the mainline terminology instead of VIP.
Note that the unit is also called VICAP in RK3566/RK3568.

Here is the detail of my research on the concerned chips. The + at the beginning
of the line indicate support in Rockchip's 4.4 tree:

- RK3566/RK3568 (2020): CIF pins + VICAP terminology
+ RK1808 (2019): CIF pins + VIP registers + VIP_MIPI registers
+ PX30 (2017): VIP pins + VIP registers
+ RK3328 (2017): CIF pins + VIP terminology
- RK3326 (2017): CIF pins + VIP terminology
- RK3399 (2016): CIF pins
- RK3368 (2015): CIF pins
- PX2 (2014-11): CIF pins + CIF registers
+ RK3126/RK3128 (2014-10): CIF pins + registers
+ RK3288 (2014-05): CIF pins + VIP terminology
- RK3026 (2013): CIF pins + CIF registers
- RK3168/RK3188/PX3 (2012): CIF pins + CIF registers
- RK3066 (2012): CIF pins + CIF registers
----------------------------------------------------------------------------------

I also CC'd Sebastian Reichel, as he might be interested in this, he is tracking 
the upstream status of the RK3588: 
https://gitlab.collabora.com/hardware-enablement/rockchip-3588/notes-for-rockchip-3588/-/blob/main/mainline-status.md?ref_type=heads

--
Kind Regards
Mehdi Djait