[PATCH v7 00/14] Rockchip ISP1 Driver

[Helen Koike <helen.koike-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>]

Hello,

I'm re-sending a new version of ISP(Camera) v4l2 driver for rockchip
rk3399 SoC.

It is not perfect yet (see known issues below), but I'm sending in case
some other people already wants to start playing with it.
I believe de main design is ok (please let me know if
it is not) and it would be great to get some reviews already.

This patchset is also available at:
https://gitlab.collabora.com/koike/linux/tree/rockchip/isp/v7

Libcamera patched to work with this version:
https://gitlab.collabora.com/koike/libcamera
(I'll also sent it to the libcamera dev mailing list)

I tested on the rockpi 4 with a rpi v1.3 sensor and also with the
Scarlet Chromebook.
Images from the Scarlet are a bit dark and green for some reason, but I
believe it's a problem in the sensor's drivers as images from the
rockpi looks ok.

The main differences from previous version are (in a macro pov):
----------------------------------------------------------------
- dphy specific code migrated to drivers/phy
- design change: droped the subdevice for the interface. Now, in the
media topology, the sensors connect directly to the ISP1.
- v4l2-compliance fixes
- dropped rk3288 (as I'm not testing it)
- dropped txrx dphy support (as I'm not testing it and it requires a bit
more work to support dsi too)
- interrupts and hw config fixes
- minor bug fixes and cleanups
- I added myself in the MAINTAINERS, as I'm not sure if previous people
still wants to maintain it, please let me know if I should keep the old
names there.

Known issues:
-------------
- Reloading the module doesn't work (there is some missing cleanup when
unloading)
- When capturing in bayer format, changing the size doesn't seem to
affect the image.
- crop needs more tests
- v4l2-compliance error:
        fail: v4l2-test-controls.cpp(824): subscribe event for control 'Image Processing Controls' failed
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
It seems that if controls are supported, v4l2-compliance says that
controls of type 'Image Processing Controls' are mandatory, is this
correct?
- It seems there are still some issues with interrupts, but I couldn't
isolate them yet.

Reasoning for the design change:
--------------------------------
In the previous version, the isp subdevice call the mipidphy_g_mbus
from rkisp1.c, so it can get
informations from the sensor such as the type of mbus (V4L2_MBUS_BT656,
V4L2_MBUS_PARALLEL or V4L2_MBUS_CSI2_CPHY), the number of csi2 lanes,
flags (V4L2_MBUS_PCLK_SAMPLE_RISING, V4L2_MBUS_VSYNC_ACTIVE_LOW or
V4L2_MBUS_HSYNC_ACTIVE_LOW). And the isp driver uses those info to configure
the hardware properly.

These information come from the DT node of the sensor. And the current
implementation is propagating this information from the sensor to the isp
through this mipidphy_g_mbus_config() (thus the hack)

* 1st attempt to solve this hack) Separating the interface code from the isp.

With the topology:

isp -> csi2 -> sensor

I was trying to migrate the CSI2 hardware configuration to the csi2 subdevice code.
But the problem I found was that in the DT, the csi2 regs is in the middle of the isp1
regs, and declaring the same "regs = <>" in both nodes (isp0 and csi2) is no good.

* 2nd attempt) flatening the DT
So instead of having two DT nodes (isp0 and csi2), we can have a single node, similar
to omap3isp.
And we can have a property, "rk,phy-type" that defines the interface (csi2, bt656 or parallel).

But, now my question is: can the isp be connected to multiple interfaces at a
time? If yes, then this is not a good solution (as we won't be able to describe
multiple interfaces in the DT node).

* 3rd attemp - WIP) get rid of the interface subdevice (chosen design)
Is there a reason to have the topology like:

isp -> interface - - -> sensor1
        |   |-------->  sensor2
        | - - - - - - > sensor3

(only one sensor can be active at a time)

?

Would it be ok if I just hide the interface from the topology? Like:

isp - - - - - -> sensor1
| |----------->  sensor2
|- - - - - - - > sensor3

Like this, I could cleanup a bunch of v4l2 code from the interface node,
the isp would know the active sensor (and its configs), and it can
configure everything itself.

I don't really see a big reason to expose the interface (csi2,
bt656 or parallel) in the topology.
Unless I'm missing something.

Previous changelog:
-------------------

changes in V6:
  - add mipi txrx phy support
  - remove bool and enum from uapi header
  - add buf_prepare op
  - correct some spelling problems
  - return all queued buffers when starting stream failed

changes in V5: Sync with local changes,
  - fix the SP height limit
  - speed up the second stream capture
  - the second stream can't force sync for rsz when start/stop streaming
  - add frame id to param vb2 buf
  - enable luminance maximum threshold

changes in V4:
  - fix some bugs during development
  - move quantization settings to rkisp1 subdev
  - correct some spelling problems
  - describe ports in dt-binding documents

changes in V3:
  - add some comments
  - fix wrong use of v4l2_async_subdev_notifier_register
  - optimize two paths capture at a time
  - remove compose
  - re-struct headers
  - add a tmp wiki page: http://opensource.rock-chips.com/wiki_Rockchip-isp1

changes in V2:
  mipi-phy:
    - use async probing
    - make it be a child device of the GRF
  isp:
    - add dummy buffer
    - change the way to get bus configuration, which make it possible to
            add parallel sensor support in the future(without mipi-phy driver).

------------------

Changes in v7:
- s/IPU3/RK_ISP1
- s/correspond/corresponding
- s/use/uses
- s/docuemnt/document
- Fix checkpatch errors (lines over 80 and SPDX)
- Add TODO to improve docs
- Migrate dphy specific code from
drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
to drivers/phy/rockchip/phy-rockchip-dphy.c
- Drop support for rk3288
- Drop support for dphy txrx
- code styling and checkpatch fixes
- fixed warning because of unknown entity type
- fixed v4l2-compliance errors regarding rkisp1 formats, try formats
and default values
- fix typo riksp1/rkisp1
- redesign: remove mipi/csi subdevice, sensors connect directly to the
isp subdevice in the media topology now. As a consequence, remove the
hack in mipidphy_g_mbus_config() where information from the sensor was
being propagated through the topology.
- From the old dphy:
        * cache get_remote_sensor() in s_stream
        * use V4L2_CID_PIXEL_RATE instead of V4L2_CID_LINK_FREQ
- Replace stream state with a boolean
- code styling and checkpatch fixes
- fix stop_stream (return after calling stop, do not reenable the stream)
- fix rkisp1_isp_sd_get_selection when V4L2_SUBDEV_FORMAT_TRY is set
- fix get format in output (isp_sd->out_fmt.mbus_code was being ignored)
- s/intput/input
- remove #define sd_to_isp_sd(_sd), add a static inline as it will be
reused by the capture
- s/strlcpy/strscpy
- sort out the locks in isp stats
- code styling and checkpatch fixes
- s/strlcpy/strscpy
- s/strcpy/strscpy
- fix config lsc error
LSC data table size is 17x17, but when configuring data to ISP,
should be aligned to 18x17. That means every last data of last
line should be filled with 0, and not filled with the data of
next line.
- Update new ISP parameters immediately
For those sub modules that have shadow registers in core isp, the
new programing parameters would not be active if both
CIF_ISP_CTRL_ISP_CFG_UPD_PERMANENT and CFG_UPD are not set. Now
we configure CFG_UPD to force update the shadow registers when new
ISP parameters are configured.
- fix some ISP parameters config error
Some ISP parameter config functions may override the old enable
bit value, because the enable bits of these modules are in the
same registers with parameters. So we should save the old enable
bits firstly.
- code styling and checkpatch fixes
- s/strlcpy/strscpy
- Fix v4l2-compliance issues:
        * remove input ioctls
media api can be used to define the topology, this input api is not
required. Besides it, if an input is enumerated, v4l2-compliance is not
happy with G_FMT returning the default colorspace instead of something
more specific.
        * return the pixelformat to the userspace
G_/S_/TRY_ FORMAT should return a valid pixelformat to the user, even if
the user gave an invalid one
        * add missing default colorspace and ycbcr
        * fix wrong pixformat in mp_fmts[] table
        * add buf type check in s_/g_selection
        * queue_setup - check sizes
        * normalize bus_info name
        * fix field any v4l2-compliance -s complain - set field none
        when streaming
- Fix compiling error: s/vidioc_enum_fmt_vid_cap_mplane/vidioc_enum_fmt_vid_cap
- Replace stream state with a boolean
The rkisp1_state enum consists only of 3 entries, where 1 is completely
unused and the other two respectively mean not streaming or streaming.
Replace it with a boolean called "streaming".
- Simplify MI interrupt handling
Rather than adding unnecessary indirection, just use stream index to
handle MI interrupt enable/disable/clear, since the stream index matches
the order of bits now, thanks to previous patch. While at it, remove
some dead code.
- code styling and checkpatch fixes
- add link_validate: don't allow a link with bayer/non-bayer mismatch
- VIDEO_ROCKCHIP_ISP1 selects VIDEOBUF2_VMALLOC
- add PHY_ROCKCHIP_DPHY as a dependency for VIDEO_ROCKCHIP_ISP1
- Fix compilation and runtime errors due to bitrotting
The code has bit-rotten since March 2018, fix compilation errors.
The new V4L2 async notifier API requires notifiers to be initialized by
a call to v4l2_async_notifier_init() before being used, do so.
- Add missing module device table
- use clk_bulk framework
- add missing notifiers cleanups
- s/strlcpy/strscpy
- normalize bus_info name
- fix s_stream error path, stream_cnt wans't being decremented properly
- use devm_platform_ioremap_resource() helper
- s/deice/device
- redesign: remove mipi/csi subdevice, sensors connect directly to the
isp subdevice in the media topology now.
- remove "saved_state" member from rkisp1_stream struct
- Reverse the order of MIs
- Simplify MI interrupt handling
Rather than adding unnecessary indirection, just use stream index to
handle MI interrupt enable/disable/clear, since the stream index matches
the order of bits now, thanks to previous patch. While at it, remove
some dead code.
- code styling and checkpatch fixes
- update document with new design and tested example
- updated doc with new design and tested example
- add phy properties
- add ports
- add phy-cells

Helen Koike (1):
  MAINTAINERS: add entry for Rockchip ISP1 driver

Jacob Chen (9):
  media: doc: add document for rkisp1 meta buffer format
  media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
  media: rkisp1: add Rockchip ISP1 subdev driver
  media: rkisp1: add ISP1 statistics driver
  media: rkisp1: add ISP1 params driver
  media: rkisp1: add capture device driver
  media: rkisp1: add rockchip isp1 core driver
  dt-bindings: Document the Rockchip ISP1 bindings
  dt-bindings: Document the Rockchip MIPI RX D-PHY bindings

Jeffy Chen (1):
  media: rkisp1: Add user space ABI definitions

Shunqian Zheng (3):
  media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format
  arm64: dts: rockchip: add isp0 node for rk3399
  arm64: dts: rockchip: add rx0 mipi-phy for rk3399

 .../bindings/media/rockchip-isp1.txt          |   71 +
 .../bindings/media/rockchip-mipi-dphy.txt     |   38 +
 Documentation/media/uapi/v4l/meta-formats.rst |    2 +
 .../uapi/v4l/pixfmt-meta-rkisp1-params.rst    |   20 +
 .../uapi/v4l/pixfmt-meta-rkisp1-stat.rst      |   18 +
 MAINTAINERS                                   |    8 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |   36 +
 drivers/media/platform/Kconfig                |   12 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/rockchip/isp1/Makefile |    7 +
 .../media/platform/rockchip/isp1/capture.c    | 1754 +++++++++++++++++
 .../media/platform/rockchip/isp1/capture.h    |  164 ++
 drivers/media/platform/rockchip/isp1/common.h |  101 +
 drivers/media/platform/rockchip/isp1/dev.c    |  675 +++++++
 drivers/media/platform/rockchip/isp1/dev.h    |   97 +
 .../media/platform/rockchip/isp1/isp_params.c | 1604 +++++++++++++++
 .../media/platform/rockchip/isp1/isp_params.h |   50 +
 .../media/platform/rockchip/isp1/isp_stats.c  |  508 +++++
 .../media/platform/rockchip/isp1/isp_stats.h  |   60 +
 drivers/media/platform/rockchip/isp1/regs.c   |  223 +++
 drivers/media/platform/rockchip/isp1/regs.h   | 1525 ++++++++++++++
 drivers/media/platform/rockchip/isp1/rkisp1.c | 1286 ++++++++++++
 drivers/media/platform/rockchip/isp1/rkisp1.h |  111 ++
 drivers/media/v4l2-core/v4l2-ioctl.c          |    2 +
 drivers/phy/rockchip/Kconfig                  |    8 +
 drivers/phy/rockchip/Makefile                 |    1 +
 drivers/phy/rockchip/phy-rockchip-dphy.c      |  412 ++++
 include/uapi/linux/rkisp1-config.h            |  816 ++++++++
 include/uapi/linux/videodev2.h                |    4 +
 29 files changed, 9614 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-params.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-rkisp1-stat.rst
 create mode 100644 drivers/media/platform/rockchip/isp1/Makefile
 create mode 100644 drivers/media/platform/rockchip/isp1/capture.c
 create mode 100644 drivers/media/platform/rockchip/isp1/capture.h
 create mode 100644 drivers/media/platform/rockchip/isp1/common.h
 create mode 100644 drivers/media/platform/rockchip/isp1/dev.c
 create mode 100644 drivers/media/platform/rockchip/isp1/dev.h
 create mode 100644 drivers/media/platform/rockchip/isp1/isp_params.c
 create mode 100644 drivers/media/platform/rockchip/isp1/isp_params.h
 create mode 100644 drivers/media/platform/rockchip/isp1/isp_stats.c
 create mode 100644 drivers/media/platform/rockchip/isp1/isp_stats.h
 create mode 100644 drivers/media/platform/rockchip/isp1/regs.c
 create mode 100644 drivers/media/platform/rockchip/isp1/regs.h
 create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.c
 create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.h
 create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
 create mode 100644 include/uapi/linux/rkisp1-config.h

-- 
2.20.1