mbox series

[00/22] Allwinner A31/A83T MIPI CSI-2 Support and A31 ISP Support

Message ID 20210910184147.336618-1-paul.kocialkowski@bootlin.com
Headers show
Series Allwinner A31/A83T MIPI CSI-2 Support and A31 ISP Support | expand

Message

Paul Kocialkowski Sept. 10, 2021, 6:41 p.m. UTC
This series introduces support for the Allwinner A31 and A83T MIPI CSI-2
controllers as well as the Allwinner A31 Image Signal Processor (ISP).
It follows v5 of the "Allwinner MIPI CSI-2 support for A31/V3s/A83T"
series, with the addition of ISP support for the V3. Since both aspect
are quite dependent due to changes to the sun6i-csi driver, they are
merged into this new series.

Aside from the ISP driver itself, the most outstanding change is a
complete rewrite of the CSI driver, for the specific reasons that are
exposed in the commit message introducing the new driver.

The commit message introducing the ISP driver should also contain useful
details regarding the implementation and outstanding specifics of the hardware.

This was tested on the V3s and A83T, using the following sensors:
- IMX219
- OV5648
- OV8856
- OV8865

v4l2-compliance seems pretty happy about the video nodes, see the detailed
reports below.

Thanks!

Changes since v5 of the MIPI CSI-2 series:
- D-PHY direction is no longer represented with a submode since this is
  not a runtime decision: no switching between the two submodes is
  possible and each instance of a controller will be dedicated to one
  direction only. Instead, a device-tree property is used.
  A separate compatible was considered, but it feels unfit since the
  direction does not describe the particular type of hardware
  implementation, but rather how it is used;
- Updated comments about channels based on latest information;
- Various cosmetic changes (and splitting) to the code;

-- Allwinner MIPI CSI-2 support for A31/V3s/A83T changelog

Changes since v4:
- Added patch to stop using v4l2_async_notifier_parse_fwnode_endpoints;
- Fixed checkpatch strict issues (parenthesis alignment);
- Fixed runtime PM call order and disable;
- Fixed fwnode_handle_put order;
- Brought back phy-names for A31 since it's mandatory according to the generic
  PHY binding and needed by the code;
- Added collected tags.

Changes since v3:
- Fixed single-item phys description in sun6i mipi csi-2 binding;
- Fixed variables names in macros using container_of;
- Fixed style issue with operators at the end of lines;
- Reworked source endpoint/subdev assignment in sun6i-csi to handle
  link_validate error case;
- Removed unrelated dt change in sun8i-a83t mipi csi-2 driver;
- Added collected tags.

Changes since v2:
- added Kconfig depend on PM since it's not optional;
- removed phy-names for A31 MIPI CSI-2 controller;
- removed v3s compatible in the A31 MIPI CSI-2 controller driver;
- removed A31 CSI controller single-port binding deprecation;
- removed empty dt port definitions;
- fixed minor checkpatch warnings;
- added collected tags;
- added media-ctl output in cover letter.

Changes since v1:
- reworked fwnode and media graph on the CSI controller end to have one port
  per interface, which solves the bus type representation issue;
- removed unused IRQ handlers in the MIPI CSI-2 bridges;
- avoided the use of devm_regmap_init_mmio_clk;
- deasserted reset before enabling clocks;
- fixed reported return code issues (ret |=, missing checks);
- applied requested cosmetic changes (backward goto, etc);
- switched over to runtime PM for the mipi csi-2 bridge drivers;
- selected PHY_SUN6I_MIPI_DPHY in Kconfig for sun6i-mipi-csi2;
- registered nodes with mipi csi-2 bridge subdevs;
- used V4L2 format info instead of switch/case for sun6i-csi bpp;
- fixed device-tree bindings as requested (useless properties, license);
- fixed mipi bridge dt instances names;
- added PHY API documentation about mode/power on order requirement;
- fixed clock error return code in d-phy code;
- fixed D-PHY mode check in d-phy code;
- added MAINTAINERS entries for the new drivers;
- added V4L2 compliance results;
- added various comments and rework commit mesages as requested.

-- V3 media topology

Media controller API version 5.13.0

Media device information
------------------------
driver          sun6i-isp
model           Allwinner ISP Capture Device
serial          
bus info        platform:1cb8000.isp
hw revision     0x0
driver version  5.13.0

Device topology
- entity 1: sun6i-isp-proc (3 pads, 3 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
	pad0: Sink
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		<- "sun6i-csi-bridge":1 [ENABLED]
	pad1: Sink
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		<- "sun6i-isp-params":0 [ENABLED,IMMUTABLE]
	pad2: Source
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		-> "sun6i-isp-capture":0 [ENABLED,IMMUTABLE]

- entity 5: sun6i-csi-bridge (2 pads, 3 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev1
	pad0: Sink
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		<- "sun6i-mipi-csi2":1 [ENABLED]
	pad1: Source
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		-> "sun6i-isp-proc":0 [ENABLED]
		-> "sun6i-csi-capture":0 []

- entity 10: sun6i-mipi-csi2 (2 pads, 2 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev2
	pad0: Sink
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		<- "imx219 1-0010":0 [ENABLED,IMMUTABLE]
	pad1: Source
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw]
		-> "sun6i-csi-bridge":0 [ENABLED]

- entity 13: sun6i-csi-capture (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video0
	pad0: Sink
		<- "sun6i-csi-bridge":1 []

- entity 21: sun6i-isp-capture (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video1
	pad0: Sink
		<- "sun6i-isp-proc":2 [ENABLED,IMMUTABLE]

- entity 27: sun6i-isp-params (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video2
	pad0: Source
		-> "sun6i-isp-proc":1 [ENABLED,IMMUTABLE]

- entity 33: imx219 1-0010 (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev3
	pad0: Source
		[fmt:SRGGB8_1X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range
		 crop.bounds:(8,8)/3280x2464
		 crop:(688,700)/1920x1080]
		-> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE]

-- sun6i-csi-capture v4l2-compliance run

v4l2-compliance SHA: not available, 32 bits

Compliance test for sun6i-csi device /dev/video0:

Driver Info:
	Driver name      : sun6i-csi
	Card type        : sun6i-csi-capture
	Bus info         : platform:1cb0000.camera
	Driver version   : 5.13.0
	Capabilities     : 0x84200001
		Video Capture
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04200001
		Video Capture
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : sun6i-isp
	Model            : Allwinner ISP Capture Device
	Serial           : 
	Bus info         : platform:1cb8000.isp
	Media version    : 5.13.0
	Hardware revision: 0x00000000 (0)
	Driver version   : 5.13.0
Interface Info:
	ID               : 0x0300000f
	Type             : V4L Video
Entity Info:
	ID               : 0x0000000d (13)
	Name             : sun6i-csi-capture
	Function         : V4L2 I/O
	Pad 0x0100000e   : 0: Sink
	  Link 0x02000011: from remote pad 0x1000007 of entity 'sun6i-csi-bridge': Data

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
		warn: v4l2-compliance.cpp(633): media bus_info 'platform:1cb8000.isp' differs from V4L2 bus_info 'platform:1cb0000.camera'
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video0 open: OK
		warn: v4l2-compliance.cpp(633): media bus_info 'platform:1cb8000.isp' differs from V4L2 bus_info 'platform:1cb0000.camera'
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 17 Private Controls: 0

Format ioctls (Input 0):
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for sun6i-csi device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 2

-- sun6i-isp-capture v4l2-compliance run

v4l2-compliance SHA: not available, 32 bits

Compliance test for sun6i-isp device /dev/video1:

Driver Info:
	Driver name      : sun6i-isp
	Card type        : sun6i-isp-capture
	Bus info         : platform:1cb8000.isp
	Driver version   : 5.13.0
	Capabilities     : 0x84200001
		Video Capture
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x04200001
		Video Capture
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : sun6i-isp
	Model            : Allwinner ISP Capture Device
	Serial           : 
	Bus info         : platform:1cb8000.isp
	Media version    : 5.13.0
	Hardware revision: 0x00000000 (0)
	Driver version   : 5.13.0
Interface Info:
	ID               : 0x03000017
	Type             : V4L Video
Entity Info:
	ID               : 0x00000015 (21)
	Name             : sun6i-isp-capture
	Function         : V4L2 I/O
	Pad 0x01000016   : 0: Sink, Must Connect
	  Link 0x02000019: from remote pad 0x1000004 of entity 'sun6i-isp-proc': Data, Enabled, Immutable

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video1 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 17 Private Controls: 0

Format ioctls (Input 0):
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for sun6i-isp device /dev/video1: 45, Succeeded: 45, Failed: 0, Warnings: 0

-- sun6i-isp-params v4l2-compliance run

v4l2-compliance SHA: not available, 32 bits

Compliance test for sun6i-isp device /dev/video2:

Driver Info:
	Driver name      : sun6i-isp
	Card type        : sun6i-isp-params
	Bus info         : platform:1cb8000.isp
	Driver version   : 5.13.0
	Capabilities     : 0x8c200000
		Metadata Output
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x0c200000
		Metadata Output
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : sun6i-isp
	Model            : Allwinner ISP Capture Device
	Serial           : 
	Bus info         : platform:1cb8000.isp
	Media version    : 5.13.0
	Hardware revision: 0x00000000 (0)
	Driver version   : 5.13.0
Interface Info:
	ID               : 0x0300001d
	Type             : V4L Video
Entity Info:
	ID               : 0x0000001b (27)
	Name             : sun6i-isp-params
	Function         : V4L2 I/O
	Pad 0x0100001c   : 0: Source, Must Connect
	  Link 0x0200001f: to remote pad 0x1000003 of entity 'sun6i-isp-proc': Data, Enabled, Immutable

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video2 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 17 Private Controls: 0

Format ioctls:
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK (Not Supported)

Codec ioctls:
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for sun6i-isp device /dev/video2: 45, Succeeded: 45, Failed: 0, Warnings: 0

Kévin L'hôpital (1):
  ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865

Paul Kocialkowski (21):
  clk: sunxi-ng: v3s: Make the ISP PLL clock public
  ARM: dts: sun8i: v3s: Parent the CSI module clock to the ISP PLL
  dt-bindings: sun6i-a31-mipi-dphy: Add optional direction property
  phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI
    CSI-2
  dt-bindings: media: sun6i-a31-csi: Add MIPI CSI-2 input port
  dt-bindings: media: Add Allwinner A31 MIPI CSI-2 bindings
    documentation
  media: sunxi: Add support for the A31 MIPI CSI-2 controller
  MAINTAINERS: Add entry for the Allwinner A31 MIPI CSI-2 bridge driver
  ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support
  dt-bindings: media: Add Allwinner A83T MIPI CSI-2 bindings
    documentation
  media: sunxi: Add support for the A83T MIPI CSI-2 controller
  MAINTAINERS: Add entry for the Allwinner A83T MIPI CSI-2 bridge
  ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node
  media: sunxi: Remove the sun6i-csi driver implementation
  media: sunxi: Introduce a rewritten sun6i-csi driver
  dt-bindings: media: Add Allwinner A31 ISP bindings documentation
  dt-bindings: media: sun6i-a31-csi: Add ISP output port
  soc: sunxi: mbus: Add A31 ISP compatibles to the list
  staging: media: Add support for the Allwinner A31 ISP
  MAINTAINERS: Add entry for the Allwinner A31 ISP driver
  ARM: dts: sun8i: v3s: Add support for the ISP

 .../media/allwinner,sun6i-a31-csi.yaml        |   91 +-
 .../media/allwinner,sun6i-a31-isp.yaml        |  111 ++
 .../media/allwinner,sun6i-a31-mipi-csi2.yaml  |  142 +++
 .../media/allwinner,sun8i-a83t-mipi-csi2.yaml |  133 ++
 .../phy/allwinner,sun6i-a31-mipi-dphy.yaml    |   12 +
 MAINTAINERS                                   |   25 +
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts  |  102 ++
 arch/arm/boot/dts/sun8i-a83t.dtsi             |   26 +
 arch/arm/boot/dts/sun8i-v3s.dtsi              |  108 ++
 drivers/clk/sunxi-ng/ccu-sun8i-v3s.h          |    1 -
 drivers/media/platform/sunxi/Kconfig          |    2 +
 drivers/media/platform/sunxi/Makefile         |    2 +
 .../media/platform/sunxi/sun6i-csi/Makefile   |    2 +-
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      | 1051 +++++-----------
 .../platform/sunxi/sun6i-csi/sun6i_csi.h      |  155 +--
 .../sunxi/sun6i-csi/sun6i_csi_bridge.c        |  895 ++++++++++++++
 .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   64 +
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 1094 +++++++++++++++++
 .../sunxi/sun6i-csi/sun6i_csi_capture.h       |   73 ++
 .../platform/sunxi/sun6i-csi/sun6i_csi_reg.h  |  364 +++---
 .../platform/sunxi/sun6i-csi/sun6i_video.c    |  683 ----------
 .../platform/sunxi/sun6i-csi/sun6i_video.h    |   38 -
 .../platform/sunxi/sun6i-mipi-csi2/Kconfig    |   12 +
 .../platform/sunxi/sun6i-mipi-csi2/Makefile   |    4 +
 .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   |  746 +++++++++++
 .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h   |   52 +
 .../sun6i-mipi-csi2/sun6i_mipi_csi2_reg.h     |   82 ++
 .../sunxi/sun8i-a83t-mipi-csi2/Kconfig        |   11 +
 .../sunxi/sun8i-a83t-mipi-csi2/Makefile       |    4 +
 .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c    |   72 ++
 .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h    |   39 +
 .../sun8i_a83t_mipi_csi2.c                    |  812 ++++++++++++
 .../sun8i_a83t_mipi_csi2.h                    |   55 +
 .../sun8i_a83t_mipi_csi2_reg.h                |  157 +++
 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c   |  166 ++-
 drivers/soc/sunxi/sunxi_mbus.c                |    2 +
 drivers/staging/media/sunxi/Kconfig           |    1 +
 drivers/staging/media/sunxi/Makefile          |    1 +
 drivers/staging/media/sunxi/sun6i-isp/Kconfig |   13 +
 .../staging/media/sunxi/sun6i-isp/Makefile    |    4 +
 .../staging/media/sunxi/sun6i-isp/sun6i_isp.c |  577 +++++++++
 .../staging/media/sunxi/sun6i-isp/sun6i_isp.h |   86 ++
 .../media/sunxi/sun6i-isp/sun6i_isp_capture.c |  759 ++++++++++++
 .../media/sunxi/sun6i-isp/sun6i_isp_capture.h |   79 ++
 .../media/sunxi/sun6i-isp/sun6i_isp_params.c  |  571 +++++++++
 .../media/sunxi/sun6i-isp/sun6i_isp_params.h  |   53 +
 .../media/sunxi/sun6i-isp/sun6i_isp_proc.c    |  598 +++++++++
 .../media/sunxi/sun6i-isp/sun6i_isp_proc.h    |   61 +
 .../media/sunxi/sun6i-isp/sun6i_isp_reg.h     |  275 +++++
 .../sunxi/sun6i-isp/uapi/sun6i-isp-config.h   |   43 +
 include/dt-bindings/clock/sun8i-v3s-ccu.h     |    1 +
 51 files changed, 8702 insertions(+), 1808 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-isp.yaml
 create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
 create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h
 delete mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
 delete mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig
 create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile
 create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2_reg.h
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h
 create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2_reg.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/Kconfig
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/Makefile
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.c
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_params.c
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_params.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_proc.c
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_proc.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_reg.h
 create mode 100644 drivers/staging/media/sunxi/sun6i-isp/uapi/sun6i-isp-config.h

Comments

Samuel Holland Sept. 11, 2021, 2:32 a.m. UTC | #1
On 9/10/21 1:41 PM, Paul Kocialkowski wrote:
> MIPI CSI-2 is supported on the V3s with an A31-based MIPI CSI-2 bridge
> controller. The controller uses a separate D-PHY, which is the same
> that is otherwise used for MIPI DSI, but used in Rx mode.
> 
> On the V3s, the CSI0 controller is dedicated to MIPI CSI-2 as it does
> not have access to any parallel interface pins.
> 
> Add all the necessary nodes (CSI0, MIPI CSI-2 bridge and D-PHY) to
> support the MIPI CSI-2 interface.
> 
> Note that a fwnode graph link is created between CSI0 and MIPI CSI-2
> even when no sensor is connected. This will result in a probe failure
> for the controller as long as no sensor is connected but this is fine
> since no other interface is available.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  arch/arm/boot/dts/sun8i-v3s.dtsi | 72 ++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
> index a77b63362a1d..ec7fa6459547 100644
> --- a/arch/arm/boot/dts/sun8i-v3s.dtsi
> +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
> @@ -612,6 +612,34 @@ spi0: spi@1c68000 {
>  			#size-cells = <0>;
>  		};
>  
> +		csi0: camera@1cb0000 {
> +			compatible = "allwinner,sun8i-v3s-csi";
> +			reg = <0x01cb0000 0x1000>;
> +			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_CSI>,
> +				 <&ccu CLK_CSI1_SCLK>,
> +				 <&ccu CLK_DRAM_CSI>;
> +			clock-names = "bus", "mod", "ram";
> +			resets = <&ccu RST_BUS_CSI>;
> +			status = "disabled";
> +
> +			assigned-clocks = <&ccu CLK_CSI1_SCLK>;
> +			assigned-clock-parents = <&ccu CLK_PLL_ISP>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					csi0_in_mipi_csi2: endpoint {
> +						remote-endpoint = <&mipi_csi2_out_csi0>;
> +					};
> +				};
> +			};
> +		};
> +
>  		csi1: camera@1cb4000 {
>  			compatible = "allwinner,sun8i-v3s-csi";
>  			reg = <0x01cb4000 0x3000>;

All of the new nodes should be added above this one, to maintain unit
address order.

Regards,
Samuel

> @@ -637,5 +665,49 @@ gic: interrupt-controller@1c81000 {
>  			#interrupt-cells = <3>;
>  			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>  		};
> +
> +		mipi_csi2: csi@1cb1000 {
> +			compatible = "allwinner,sun8i-v3s-mipi-csi2",
> +				     "allwinner,sun6i-a31-mipi-csi2";
> +			reg = <0x01cb1000 0x1000>;
> +			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_CSI>,
> +				 <&ccu CLK_CSI1_SCLK>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_CSI>;
> +			status = "disabled";
> +
> +			phys = <&dphy>;
> +			phy-names = "dphy";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				mipi_csi2_in: port@0 {
> +					reg = <0>;
> +				};
> +
> +				mipi_csi2_out: port@1 {
> +					reg = <1>;
> +
> +					mipi_csi2_out_csi0: endpoint {
> +						remote-endpoint = <&csi0_in_mipi_csi2>;
> +					};
> +				};
> +			};
> +		};
> +
> +		dphy: d-phy@1cb2000 {
> +			compatible = "allwinner,sun6i-a31-mipi-dphy";
> +			reg = <0x01cb2000 0x1000>;
> +			clocks = <&ccu CLK_BUS_CSI>,
> +				 <&ccu CLK_MIPI_CSI>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_CSI>;
> +			allwinner,direction = "rx";
> +			status = "disabled";
> +			#phy-cells = <0>;
> +		};
>  	};
>  };
>
Samuel Holland Sept. 11, 2021, 2:36 a.m. UTC | #2
On 9/10/21 1:41 PM, Paul Kocialkowski wrote:
> The A31 ISP sits on the mbus and requires the usual bus address
> adaptation. Add its compatibles to the list.

My understanding is that this driver only exists to work around old DT
bindings where the interconnects/interconnect-names = "dma-mem"
properties are not required (and so they are historically missing from
the device trees).

For new bindings, it would be better to use those properties and not add
to this list.

Regards,
Samuel

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/soc/sunxi/sunxi_mbus.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/soc/sunxi/sunxi_mbus.c b/drivers/soc/sunxi/sunxi_mbus.c
> index d90e4a264b6f..7f0079ea30b1 100644
> --- a/drivers/soc/sunxi/sunxi_mbus.c
> +++ b/drivers/soc/sunxi/sunxi_mbus.c
> @@ -37,6 +37,7 @@ static const char * const sunxi_mbus_devices[] = {
>  	"allwinner,sun5i-a13-video-engine",
>  	"allwinner,sun6i-a31-csi",
>  	"allwinner,sun6i-a31-display-backend",
> +	"allwinner,sun6i-a31-isp",
>  	"allwinner,sun7i-a20-csi0",
>  	"allwinner,sun7i-a20-display-backend",
>  	"allwinner,sun7i-a20-display-frontend",
> @@ -50,6 +51,7 @@ static const char * const sunxi_mbus_devices[] = {
>  	"allwinner,sun8i-h3-csi",
>  	"allwinner,sun8i-h3-video-engine",
>  	"allwinner,sun8i-v3s-csi",
> +	"allwinner,sun8i-v3s-isp",
>  	"allwinner,sun9i-a80-display-backend",
>  	"allwinner,sun50i-a64-csi",
>  	"allwinner,sun50i-a64-video-engine",
>
Chen-Yu Tsai Sept. 11, 2021, 2:53 a.m. UTC | #3
Hi,

On Sat, Sep 11, 2021 at 2:42 AM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> MIPI CSI-2 is supported on the A83T with a dedicated controller that
> covers both the protocol and D-PHY. It can be connected to the CSI
> interface as a V4L2 subdev through the fwnode graph.
>
> This is not done by default since connecting the bridge without a
> subdev attached to it will cause a failure on the CSI driver.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

I believe you tagged the wrong patch to not be merged? AFAICT it
should be the next patch that hooks up OV8865, not this one.

> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index ac97eac91349..1fa51f7ef063 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -1064,6 +1064,32 @@ csi: camera@1cb0000 {
>                         status = "disabled";
>                 };
>
> +               mipi_csi2: csi@1cb1000 {
> +                       compatible = "allwinner,sun8i-a83t-mipi-csi2";
> +                       reg = <0x01cb1000 0x1000>;
> +                       interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_CSI>,
> +                                <&ccu CLK_CSI_SCLK>,
> +                                <&ccu CLK_MIPI_CSI>,
> +                                <&ccu CLK_CSI_MISC>;
> +                       clock-names = "bus", "mod", "mipi", "misc";
> +                       resets = <&ccu RST_BUS_CSI>;
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               mipi_csi2_in: port@0 {
> +                                       reg = <0>;
> +                               };
> +
> +                               mipi_csi2_out: port@1 {
> +                                       reg = <1>;
> +                               };
> +                       };
> +               };
> +
>                 hdmi: hdmi@1ee0000 {
>                         compatible = "allwinner,sun8i-a83t-dw-hdmi";
>                         reg = <0x01ee0000 0x10000>;
> --
> 2.32.0
>
>
Paul Kocialkowski Sept. 13, 2021, 7:44 a.m. UTC | #4
Hi Samuel,

On Fri 10 Sep 21, 21:32, Samuel Holland wrote:
> On 9/10/21 1:41 PM, Paul Kocialkowski wrote:
> > MIPI CSI-2 is supported on the V3s with an A31-based MIPI CSI-2 bridge
> > controller. The controller uses a separate D-PHY, which is the same
> > that is otherwise used for MIPI DSI, but used in Rx mode.
> > 
> > On the V3s, the CSI0 controller is dedicated to MIPI CSI-2 as it does
> > not have access to any parallel interface pins.
> > 
> > Add all the necessary nodes (CSI0, MIPI CSI-2 bridge and D-PHY) to
> > support the MIPI CSI-2 interface.
> > 
> > Note that a fwnode graph link is created between CSI0 and MIPI CSI-2
> > even when no sensor is connected. This will result in a probe failure
> > for the controller as long as no sensor is connected but this is fine
> > since no other interface is available.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  arch/arm/boot/dts/sun8i-v3s.dtsi | 72 ++++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
> > index a77b63362a1d..ec7fa6459547 100644
> > --- a/arch/arm/boot/dts/sun8i-v3s.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
> > @@ -612,6 +612,34 @@ spi0: spi@1c68000 {
> >  			#size-cells = <0>;
> >  		};
> >  
> > +		csi0: camera@1cb0000 {
> > +			compatible = "allwinner,sun8i-v3s-csi";
> > +			reg = <0x01cb0000 0x1000>;
> > +			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&ccu CLK_BUS_CSI>,
> > +				 <&ccu CLK_CSI1_SCLK>,
> > +				 <&ccu CLK_DRAM_CSI>;
> > +			clock-names = "bus", "mod", "ram";
> > +			resets = <&ccu RST_BUS_CSI>;
> > +			status = "disabled";
> > +
> > +			assigned-clocks = <&ccu CLK_CSI1_SCLK>;
> > +			assigned-clock-parents = <&ccu CLK_PLL_ISP>;
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +
> > +					csi0_in_mipi_csi2: endpoint {
> > +						remote-endpoint = <&mipi_csi2_out_csi0>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> >  		csi1: camera@1cb4000 {
> >  			compatible = "allwinner,sun8i-v3s-csi";
> >  			reg = <0x01cb4000 0x3000>;
> 
> All of the new nodes should be added above this one, to maintain unit
> address order.

Good catch, this was an overlook on my side.

Thanks,

Paul

> Regards,
> Samuel
> 
> > @@ -637,5 +665,49 @@ gic: interrupt-controller@1c81000 {
> >  			#interrupt-cells = <3>;
> >  			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> >  		};
> > +
> > +		mipi_csi2: csi@1cb1000 {
> > +			compatible = "allwinner,sun8i-v3s-mipi-csi2",
> > +				     "allwinner,sun6i-a31-mipi-csi2";
> > +			reg = <0x01cb1000 0x1000>;
> > +			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&ccu CLK_BUS_CSI>,
> > +				 <&ccu CLK_CSI1_SCLK>;
> > +			clock-names = "bus", "mod";
> > +			resets = <&ccu RST_BUS_CSI>;
> > +			status = "disabled";
> > +
> > +			phys = <&dphy>;
> > +			phy-names = "dphy";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				mipi_csi2_in: port@0 {
> > +					reg = <0>;
> > +				};
> > +
> > +				mipi_csi2_out: port@1 {
> > +					reg = <1>;
> > +
> > +					mipi_csi2_out_csi0: endpoint {
> > +						remote-endpoint = <&csi0_in_mipi_csi2>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		dphy: d-phy@1cb2000 {
> > +			compatible = "allwinner,sun6i-a31-mipi-dphy";
> > +			reg = <0x01cb2000 0x1000>;
> > +			clocks = <&ccu CLK_BUS_CSI>,
> > +				 <&ccu CLK_MIPI_CSI>;
> > +			clock-names = "bus", "mod";
> > +			resets = <&ccu RST_BUS_CSI>;
> > +			allwinner,direction = "rx";
> > +			status = "disabled";
> > +			#phy-cells = <0>;
> > +		};
> >  	};
> >  };
> > 
>
Paul Kocialkowski Sept. 13, 2021, 7:45 a.m. UTC | #5
Hi Samuel,

On Fri 10 Sep 21, 21:36, Samuel Holland wrote:
> On 9/10/21 1:41 PM, Paul Kocialkowski wrote:
> > The A31 ISP sits on the mbus and requires the usual bus address
> > adaptation. Add its compatibles to the list.
> 
> My understanding is that this driver only exists to work around old DT
> bindings where the interconnects/interconnect-names = "dma-mem"
> properties are not required (and so they are historically missing from
> the device trees).
> 
> For new bindings, it would be better to use those properties and not add
> to this list.

Oh okay, I didn't really look into it and just did the same thing that was
done for the CSI controller. Thanks for the heads up!

Paul

> Regards,
> Samuel
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/soc/sunxi/sunxi_mbus.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/soc/sunxi/sunxi_mbus.c b/drivers/soc/sunxi/sunxi_mbus.c
> > index d90e4a264b6f..7f0079ea30b1 100644
> > --- a/drivers/soc/sunxi/sunxi_mbus.c
> > +++ b/drivers/soc/sunxi/sunxi_mbus.c
> > @@ -37,6 +37,7 @@ static const char * const sunxi_mbus_devices[] = {
> >  	"allwinner,sun5i-a13-video-engine",
> >  	"allwinner,sun6i-a31-csi",
> >  	"allwinner,sun6i-a31-display-backend",
> > +	"allwinner,sun6i-a31-isp",
> >  	"allwinner,sun7i-a20-csi0",
> >  	"allwinner,sun7i-a20-display-backend",
> >  	"allwinner,sun7i-a20-display-frontend",
> > @@ -50,6 +51,7 @@ static const char * const sunxi_mbus_devices[] = {
> >  	"allwinner,sun8i-h3-csi",
> >  	"allwinner,sun8i-h3-video-engine",
> >  	"allwinner,sun8i-v3s-csi",
> > +	"allwinner,sun8i-v3s-isp",
> >  	"allwinner,sun9i-a80-display-backend",
> >  	"allwinner,sun50i-a64-csi",
> >  	"allwinner,sun50i-a64-video-engine",
> > 
>
Paul Kocialkowski Sept. 13, 2021, 7:45 a.m. UTC | #6
Hi Chen-Yu,

On Sat 11 Sep 21, 10:53, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sat, Sep 11, 2021 at 2:42 AM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > MIPI CSI-2 is supported on the A83T with a dedicated controller that
> > covers both the protocol and D-PHY. It can be connected to the CSI
> > interface as a V4L2 subdev through the fwnode graph.
> >
> > This is not done by default since connecting the bridge without a
> > subdev attached to it will cause a failure on the CSI driver.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> I believe you tagged the wrong patch to not be merged? AFAICT it
> should be the next patch that hooks up OV8865, not this one.

Yes you are definitely right, this patch is good for merge and the next
one is not.

Thanks,

Paul

> > ---
> >  arch/arm/boot/dts/sun8i-a83t.dtsi | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > index ac97eac91349..1fa51f7ef063 100644
> > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > @@ -1064,6 +1064,32 @@ csi: camera@1cb0000 {
> >                         status = "disabled";
> >                 };
> >
> > +               mipi_csi2: csi@1cb1000 {
> > +                       compatible = "allwinner,sun8i-a83t-mipi-csi2";
> > +                       reg = <0x01cb1000 0x1000>;
> > +                       interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&ccu CLK_BUS_CSI>,
> > +                                <&ccu CLK_CSI_SCLK>,
> > +                                <&ccu CLK_MIPI_CSI>,
> > +                                <&ccu CLK_CSI_MISC>;
> > +                       clock-names = "bus", "mod", "mipi", "misc";
> > +                       resets = <&ccu RST_BUS_CSI>;
> > +                       status = "disabled";
> > +
> > +                       ports {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               mipi_csi2_in: port@0 {
> > +                                       reg = <0>;
> > +                               };
> > +
> > +                               mipi_csi2_out: port@1 {
> > +                                       reg = <1>;
> > +                               };
> > +                       };
> > +               };
> > +
> >                 hdmi: hdmi@1ee0000 {
> >                         compatible = "allwinner,sun8i-a83t-dw-hdmi";
> >                         reg = <0x01ee0000 0x10000>;
> > --
> > 2.32.0
> >
> >
Maxime Ripard Sept. 13, 2021, 8:17 a.m. UTC | #7
On Fri, Sep 10, 2021 at 08:41:40PM +0200, Paul Kocialkowski wrote:
> As described in the commit adding support for the new sun6i-csi driver,
> a complete rewrite was necessary to support the Allwinner A31 ISP as
> well as fix a number of issues with the current implementation.
> 
> Farewell and thanks for all the pixels!
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

For completeness, this is what the other commit log mentions:

> While adapting the sun6i-csi driver for MIPI CSI-2 support was
> possible, it became clear that adding support for the ISP required
> very heavy changes to the driver which were quite hard to break down
> into a series of subsequent changes.

> The first major difficulty comes from the lack of v4l2 subdev that
> acts a bridge, separate from the video node representing the DMA
> engine. To support the ISP, only parts of the hardware must be
> configured (excluding aspects related to the DMA output), which made
> the separation a hard requirement.

> Another significant difficulty was the specific dance that is required
> to have both the ISP and CSI device be part of the same media device.
> Because the ISP and CSI are two different hardware blocks, they have
> two distinct drivers that will each try to register their own v4l2
> and media devices, resulting in two distinct pipelines. When the ISP
> is in use, we actually want the CSI driver to register with the ISP's
> v4l2 and media devices while keeping the ability to register its own
> when the ISP is not in use. This is done by:
> 1. Having the CSI driver check whether the ISP is available, using
>    sun6i_csi_isp_detect();
> 2. If not, it can register when its own async subdevs are ready, using
>    sun6i_csi_v4l2_complete();
> 3. If so, it will register its bridge as an async subdev which will
>    be picked-up by the ISP driver (from the fwnode graph link);
> 4. When the subdev becomes bound to the ISP's v4l2 device, we can
>    then access that device (and the associated media device) to
>    complete registration of the capture video node, using
>    sun6i_csi_isp_complete();
> Besides the logic rework, other issues were identified and resolved:
> - The sync mechanism for buffer flipping was based on the frame done
>   interrupt, which is too late (next frame is already being processed).
>   This lead to requiring 3 buffers to start and writing two addresses
>   when starting. Using vsync as a sync point seems to be the correct
>   approach and allows using only two buffers without tearing;
> - Using devm_regmap_init_mmio_clk was incorrect since the reset also
>   comes into play;
> - Some register definitions were inverted compared to their actual
>   effect (which was inherited from the Allwinner documentation and
>   code): comments were added where relevant;
> - The deprecated v4l2_async_notifier_parse_fwnode_endpoints() helper
>   is no longer used by the driver;

With that being said, NAK.

Having heavy changes to a driver is completely fine, and is kind of
expected really with such a big change. Breaking all possibility of
bisection and throwing away years of stabilization and maintenance
isn't.

And all those small bug fixes you mention at the end are just that:
small bug fixes that can be done on the current driver just fine too.

Maxime
Maxime Ripard Sept. 13, 2021, 8:31 a.m. UTC | #8
On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:
> Some Allwinner platforms come with an Image Signal Processor, which
> supports various features in order to enhance and transform data
> received by image sensors into good-looking pictures. In most cases,
> the data is raw bayer, which gets internally converted to RGB and
> finally YUV, which is what the hardware produces.
> 
> This driver supports ISPs that are similar to the A31 ISP, which was
> the first standalone ISP found in Allwinner platforms. Simpler ISP
> blocks were found in the A10 and A20, where they are tied to a CSI
> controller. Newer generations of Allwinner SoCs (starting with the
> H6, H616, etc) come with a new camera subsystem and revised ISP.
> Even though these previous and next-generation ISPs are somewhat
> similar to the A31 ISP, they have enough significant differences to
> be out of the scope of this driver.
> 
> While the ISP supports many features, including 3A and many
> enhancement blocks, this implementation is limited to the following:
> - V3s (V3/S3) platform support;
> - Bayer media bus formats as input;
> - Semi-planar YUV (NV12/NV21) as output;
> - Debayering with per-component gain and offset configuration;
> - 2D noise filtering with configurable coefficients.
> 
> Since many features are missing from the associated uAPI, the driver
> is aimed to integrate staging until all features are properly
> described.

We can add new features/interfaces to a !staging driver. Why do you
think staging is required?

> On the technical side, it uses the v4l2 and media controller APIs,
> with a video node for capture, a processor subdev and a video node
> for parameters submission. A specific uAPI structure and associated
> v4l2 meta format are used to configure parameters of the supported
> modules.

This meta format needs to be documented

Maxime
Maxime Ripard Sept. 13, 2021, 8:32 a.m. UTC | #9
On Mon, Sep 13, 2021 at 09:45:22AM +0200, Paul Kocialkowski wrote:
> Hi Samuel,
> 
> On Fri 10 Sep 21, 21:36, Samuel Holland wrote:
> > On 9/10/21 1:41 PM, Paul Kocialkowski wrote:
> > > The A31 ISP sits on the mbus and requires the usual bus address
> > > adaptation. Add its compatibles to the list.
> > 
> > My understanding is that this driver only exists to work around old DT
> > bindings where the interconnects/interconnect-names = "dma-mem"
> > properties are not required (and so they are historically missing from
> > the device trees).
> > 
> > For new bindings, it would be better to use those properties and not add
> > to this list.
> 
> Oh okay, I didn't really look into it and just did the same thing that was
> done for the CSI controller. Thanks for the heads up!

This code was done to maintain backward compatibility. New DT should
indeed use the interconnects property.

Maxime
Paul Kocialkowski Sept. 14, 2021, 7:50 a.m. UTC | #10
Hi,

On Mon 13 Sep 21, 10:31, Maxime Ripard wrote:
> On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:
> > Some Allwinner platforms come with an Image Signal Processor, which
> > supports various features in order to enhance and transform data
> > received by image sensors into good-looking pictures. In most cases,
> > the data is raw bayer, which gets internally converted to RGB and
> > finally YUV, which is what the hardware produces.
> > 
> > This driver supports ISPs that are similar to the A31 ISP, which was
> > the first standalone ISP found in Allwinner platforms. Simpler ISP
> > blocks were found in the A10 and A20, where they are tied to a CSI
> > controller. Newer generations of Allwinner SoCs (starting with the
> > H6, H616, etc) come with a new camera subsystem and revised ISP.
> > Even though these previous and next-generation ISPs are somewhat
> > similar to the A31 ISP, they have enough significant differences to
> > be out of the scope of this driver.
> > 
> > While the ISP supports many features, including 3A and many
> > enhancement blocks, this implementation is limited to the following:
> > - V3s (V3/S3) platform support;
> > - Bayer media bus formats as input;
> > - Semi-planar YUV (NV12/NV21) as output;
> > - Debayering with per-component gain and offset configuration;
> > - 2D noise filtering with configurable coefficients.
> > 
> > Since many features are missing from the associated uAPI, the driver
> > is aimed to integrate staging until all features are properly
> > described.
> 
> We can add new features/interfaces to a !staging driver. Why do you
> think staging is required?

This is true for the driver but not so much for the uAPI, so it seems that
the uAPI must be added to staging in some way. Then I'm not sure it makes sense
to have a !staging driver that depends on a staging uAPI.

Besides that, I added it to staging because that's the process that was
followed by rkisp1, which is a very similar case.

> > On the technical side, it uses the v4l2 and media controller APIs,
> > with a video node for capture, a processor subdev and a video node
> > for parameters submission. A specific uAPI structure and associated
> > v4l2 meta format are used to configure parameters of the supported
> > modules.
> 
> This meta format needs to be documented

You're right, there should probably be a pixfmt-meta-sun6i-isp.rst
documentation file. I guess it should live along in the staging driver
directory for now and be destaged later.

Cheers,

Paul
Paul Kocialkowski Sept. 14, 2021, 8:04 a.m. UTC | #11
Hi,

On Mon 13 Sep 21, 10:17, Maxime Ripard wrote:
> On Fri, Sep 10, 2021 at 08:41:40PM +0200, Paul Kocialkowski wrote:
> > As described in the commit adding support for the new sun6i-csi driver,
> > a complete rewrite was necessary to support the Allwinner A31 ISP as
> > well as fix a number of issues with the current implementation.
> > 
> > Farewell and thanks for all the pixels!
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> For completeness, this is what the other commit log mentions:
> 
> > While adapting the sun6i-csi driver for MIPI CSI-2 support was
> > possible, it became clear that adding support for the ISP required
> > very heavy changes to the driver which were quite hard to break down
> > into a series of subsequent changes.
> 
> > The first major difficulty comes from the lack of v4l2 subdev that
> > acts a bridge, separate from the video node representing the DMA
> > engine. To support the ISP, only parts of the hardware must be
> > configured (excluding aspects related to the DMA output), which made
> > the separation a hard requirement.
> 
> > Another significant difficulty was the specific dance that is required
> > to have both the ISP and CSI device be part of the same media device.
> > Because the ISP and CSI are two different hardware blocks, they have
> > two distinct drivers that will each try to register their own v4l2
> > and media devices, resulting in two distinct pipelines. When the ISP
> > is in use, we actually want the CSI driver to register with the ISP's
> > v4l2 and media devices while keeping the ability to register its own
> > when the ISP is not in use. This is done by:
> > 1. Having the CSI driver check whether the ISP is available, using
> >    sun6i_csi_isp_detect();
> > 2. If not, it can register when its own async subdevs are ready, using
> >    sun6i_csi_v4l2_complete();
> > 3. If so, it will register its bridge as an async subdev which will
> >    be picked-up by the ISP driver (from the fwnode graph link);
> > 4. When the subdev becomes bound to the ISP's v4l2 device, we can
> >    then access that device (and the associated media device) to
> >    complete registration of the capture video node, using
> >    sun6i_csi_isp_complete();
> > Besides the logic rework, other issues were identified and resolved:
> > - The sync mechanism for buffer flipping was based on the frame done
> >   interrupt, which is too late (next frame is already being processed).
> >   This lead to requiring 3 buffers to start and writing two addresses
> >   when starting. Using vsync as a sync point seems to be the correct
> >   approach and allows using only two buffers without tearing;
> > - Using devm_regmap_init_mmio_clk was incorrect since the reset also
> >   comes into play;
> > - Some register definitions were inverted compared to their actual
> >   effect (which was inherited from the Allwinner documentation and
> >   code): comments were added where relevant;
> > - The deprecated v4l2_async_notifier_parse_fwnode_endpoints() helper
> >   is no longer used by the driver;
> 
> With that being said, NAK.
> 
> Having heavy changes to a driver is completely fine, and is kind of
> expected really with such a big change. Breaking all possibility of
> bisection and throwing away years of stabilization and maintenance
> isn't.
> 
> And all those small bug fixes you mention at the end are just that:
> small bug fixes that can be done on the current driver just fine too.

I understand that this looks like we're trashing all the work that was
done previously by removing the current driver and adding the new one
but the logic for deciding what to write into registers was carefully
preserved from the original driver to make sure that the works of
stabilization and maintenance are not lost.

However I would understand that my good promise on this is not enough,
so perhaps I could provide a combinatory verification that the same set
of mbus/pixel formats end up with the same thing being written into
registers.

In addition I understand that it will be necessary to split the changes
up into small commits to clarify the transition path between the two
drivers. So I will do my best to split things up.

Does that seem like an agreeable plan or do you see other things that
would be blockers?

My initial thought was that it would be much easier to review the driver as a
rewrite, but I'm not too surprised I was wrong. To be honest it was nearly
impossible to actually have the initial development happen as sequential steps
and I preferred to allocate my time on other tasks than splitting the changes
into these sequential steps.

Cheers,

Paul
Laurent Pinchart Sept. 14, 2021, 11:11 a.m. UTC | #12
Hi Paul,

On Tue, Sep 14, 2021 at 09:50:41AM +0200, Paul Kocialkowski wrote:
> On Mon 13 Sep 21, 10:31, Maxime Ripard wrote:
> > On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:
> > > Some Allwinner platforms come with an Image Signal Processor, which
> > > supports various features in order to enhance and transform data
> > > received by image sensors into good-looking pictures. In most cases,
> > > the data is raw bayer, which gets internally converted to RGB and
> > > finally YUV, which is what the hardware produces.
> > > 
> > > This driver supports ISPs that are similar to the A31 ISP, which was
> > > the first standalone ISP found in Allwinner platforms. Simpler ISP
> > > blocks were found in the A10 and A20, where they are tied to a CSI
> > > controller. Newer generations of Allwinner SoCs (starting with the
> > > H6, H616, etc) come with a new camera subsystem and revised ISP.
> > > Even though these previous and next-generation ISPs are somewhat
> > > similar to the A31 ISP, they have enough significant differences to
> > > be out of the scope of this driver.
> > > 
> > > While the ISP supports many features, including 3A and many
> > > enhancement blocks, this implementation is limited to the following:
> > > - V3s (V3/S3) platform support;
> > > - Bayer media bus formats as input;
> > > - Semi-planar YUV (NV12/NV21) as output;
> > > - Debayering with per-component gain and offset configuration;
> > > - 2D noise filtering with configurable coefficients.
> > > 
> > > Since many features are missing from the associated uAPI, the driver
> > > is aimed to integrate staging until all features are properly
> > > described.
> > 
> > We can add new features/interfaces to a !staging driver. Why do you
> > think staging is required?
> 
> This is true for the driver but not so much for the uAPI, so it seems that
> the uAPI must be added to staging in some way. Then I'm not sure it makes sense
> to have a !staging driver that depends on a staging uAPI.
> 
> Besides that, I added it to staging because that's the process that was
> followed by rkisp1, which is a very similar case.

Maxime is right in the sense that uAPI can always be extended, but it
has to be done in a backward-compatible manner, and staging is sometimes
considered as not being covered by the ABI stability requirements of the
kernel. Not everybody agrees on this, but there are clear cases where
userspace really can't expect staging ABIs to be stable (for instance
when the driver doesn't even compile).

I think there's value in having the driver in staging to facilitate
development until we consider the ABI stable, but I'm not entirely sure
if there should be another step taken to mark this ABI is not being
ready yet.

> > > On the technical side, it uses the v4l2 and media controller APIs,
> > > with a video node for capture, a processor subdev and a video node
> > > for parameters submission. A specific uAPI structure and associated
> > > v4l2 meta format are used to configure parameters of the supported
> > > modules.
> > 
> > This meta format needs to be documented
> 
> You're right, there should probably be a pixfmt-meta-sun6i-isp.rst
> documentation file. I guess it should live along in the staging driver
> directory for now and be destaged later.

Can documentation in staging be compiled ? If not I think it can go to
Documentation/
Maxime Ripard Sept. 14, 2021, 11:48 a.m. UTC | #13
On Tue, Sep 14, 2021 at 02:11:18PM +0300, Laurent Pinchart wrote:
> Hi Paul,
> 
> On Tue, Sep 14, 2021 at 09:50:41AM +0200, Paul Kocialkowski wrote:
> > On Mon 13 Sep 21, 10:31, Maxime Ripard wrote:
> > > On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:
> > > > Some Allwinner platforms come with an Image Signal Processor, which
> > > > supports various features in order to enhance and transform data
> > > > received by image sensors into good-looking pictures. In most cases,
> > > > the data is raw bayer, which gets internally converted to RGB and
> > > > finally YUV, which is what the hardware produces.
> > > > 
> > > > This driver supports ISPs that are similar to the A31 ISP, which was
> > > > the first standalone ISP found in Allwinner platforms. Simpler ISP
> > > > blocks were found in the A10 and A20, where they are tied to a CSI
> > > > controller. Newer generations of Allwinner SoCs (starting with the
> > > > H6, H616, etc) come with a new camera subsystem and revised ISP.
> > > > Even though these previous and next-generation ISPs are somewhat
> > > > similar to the A31 ISP, they have enough significant differences to
> > > > be out of the scope of this driver.
> > > > 
> > > > While the ISP supports many features, including 3A and many
> > > > enhancement blocks, this implementation is limited to the following:
> > > > - V3s (V3/S3) platform support;
> > > > - Bayer media bus formats as input;
> > > > - Semi-planar YUV (NV12/NV21) as output;
> > > > - Debayering with per-component gain and offset configuration;
> > > > - 2D noise filtering with configurable coefficients.
> > > > 
> > > > Since many features are missing from the associated uAPI, the driver
> > > > is aimed to integrate staging until all features are properly
> > > > described.
> > > 
> > > We can add new features/interfaces to a !staging driver. Why do you
> > > think staging is required?
> > 
> > This is true for the driver but not so much for the uAPI, so it seems that
> > the uAPI must be added to staging in some way. Then I'm not sure it makes sense
> > to have a !staging driver that depends on a staging uAPI.
> > 
> > Besides that, I added it to staging because that's the process that was
> > followed by rkisp1, which is a very similar case.
> 
> Maxime is right in the sense that uAPI can always be extended, but it
> has to be done in a backward-compatible manner, and staging is sometimes
> considered as not being covered by the ABI stability requirements of the
> kernel. Not everybody agrees on this, but there are clear cases where
> userspace really can't expect staging ABIs to be stable (for instance
> when the driver doesn't even compile).
> 
> I think there's value in having the driver in staging to facilitate
> development until we consider the ABI stable, but I'm not entirely sure
> if there should be another step taken to mark this ABI is not being
> ready yet.

The rule seems to be about whether or not the user-space gets broken in
the process:

https://lore.kernel.org/lkml/CAHk-=wiVi7mSrsMP=fLXQrXK_UimybW=ziLOwSzFTtoXUacWVQ@mail.gmail.com/

Something that wouldn't compile cannot generate a regression, since it
never worked in the first place. Changing the semantic of an ioctl does.

Maxime
Sakari Ailus Sept. 15, 2021, 7:51 p.m. UTC | #14
Hi Paul, Maxime,

On Tue, Sep 14, 2021 at 10:04:25AM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon 13 Sep 21, 10:17, Maxime Ripard wrote:
> > On Fri, Sep 10, 2021 at 08:41:40PM +0200, Paul Kocialkowski wrote:
> > > As described in the commit adding support for the new sun6i-csi driver,
> > > a complete rewrite was necessary to support the Allwinner A31 ISP as
> > > well as fix a number of issues with the current implementation.
> > > 
> > > Farewell and thanks for all the pixels!
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > 
> > For completeness, this is what the other commit log mentions:
> > 
> > > While adapting the sun6i-csi driver for MIPI CSI-2 support was
> > > possible, it became clear that adding support for the ISP required
> > > very heavy changes to the driver which were quite hard to break down
> > > into a series of subsequent changes.
> > 
> > > The first major difficulty comes from the lack of v4l2 subdev that
> > > acts a bridge, separate from the video node representing the DMA
> > > engine. To support the ISP, only parts of the hardware must be
> > > configured (excluding aspects related to the DMA output), which made
> > > the separation a hard requirement.
> > 
> > > Another significant difficulty was the specific dance that is required
> > > to have both the ISP and CSI device be part of the same media device.
> > > Because the ISP and CSI are two different hardware blocks, they have
> > > two distinct drivers that will each try to register their own v4l2
> > > and media devices, resulting in two distinct pipelines. When the ISP
> > > is in use, we actually want the CSI driver to register with the ISP's
> > > v4l2 and media devices while keeping the ability to register its own
> > > when the ISP is not in use. This is done by:
> > > 1. Having the CSI driver check whether the ISP is available, using
> > >    sun6i_csi_isp_detect();
> > > 2. If not, it can register when its own async subdevs are ready, using
> > >    sun6i_csi_v4l2_complete();
> > > 3. If so, it will register its bridge as an async subdev which will
> > >    be picked-up by the ISP driver (from the fwnode graph link);
> > > 4. When the subdev becomes bound to the ISP's v4l2 device, we can
> > >    then access that device (and the associated media device) to
> > >    complete registration of the capture video node, using
> > >    sun6i_csi_isp_complete();
> > > Besides the logic rework, other issues were identified and resolved:
> > > - The sync mechanism for buffer flipping was based on the frame done
> > >   interrupt, which is too late (next frame is already being processed).
> > >   This lead to requiring 3 buffers to start and writing two addresses
> > >   when starting. Using vsync as a sync point seems to be the correct
> > >   approach and allows using only two buffers without tearing;
> > > - Using devm_regmap_init_mmio_clk was incorrect since the reset also
> > >   comes into play;
> > > - Some register definitions were inverted compared to their actual
> > >   effect (which was inherited from the Allwinner documentation and
> > >   code): comments were added where relevant;
> > > - The deprecated v4l2_async_notifier_parse_fwnode_endpoints() helper
> > >   is no longer used by the driver;
> > 
> > With that being said, NAK.
> > 
> > Having heavy changes to a driver is completely fine, and is kind of
> > expected really with such a big change. Breaking all possibility of
> > bisection and throwing away years of stabilization and maintenance
> > isn't.
> > 
> > And all those small bug fixes you mention at the end are just that:
> > small bug fixes that can be done on the current driver just fine too.
> 
> I understand that this looks like we're trashing all the work that was
> done previously by removing the current driver and adding the new one
> but the logic for deciding what to write into registers was carefully
> preserved from the original driver to make sure that the works of
> stabilization and maintenance are not lost.
> 
> However I would understand that my good promise on this is not enough,
> so perhaps I could provide a combinatory verification that the same set
> of mbus/pixel formats end up with the same thing being written into
> registers.
> 
> In addition I understand that it will be necessary to split the changes
> up into small commits to clarify the transition path between the two
> drivers. So I will do my best to split things up.
> 
> Does that seem like an agreeable plan or do you see other things that
> would be blockers?

Please do refactor the patches into reviewable chunks that make sense on
their own. I'd see the result being the same driver but with additional
patches fixing bugs, doing some or more refactoring and adding new
functionality. Please use -C100 -M100 if there's a need to rename files,
and preferrably do so in separate patches.

See e.g. patches to the smiapp driver that turned it into a CCS driver:

	git log 2db8166f739e75c1269d7e8afe8da68e70098810..b24cc2a18c50e4e315abc76a86b26b4c49652f79~ -- drivers/media/i2c/smiapp
	git log drivers/media/i2c/ccs

Usually bugfixes are best put first.

> 
> My initial thought was that it would be much easier to review the driver as a
> rewrite, but I'm not too surprised I was wrong. To be honest it was nearly
> impossible to actually have the initial development happen as sequential steps
> and I preferred to allocate my time on other tasks than splitting the changes
> into these sequential steps.

This isn't really unusual when you're changing an existing driver:
sometimes you have to implement what you want to achieve in whole, and only
then figure out how to split it into something that can be reviewed. Often
the end result will look different than what you arrived with on the first
time.