Hi Paul, On 10/25/23 10:43, Paul Kocialkowski wrote: > [...] >>> 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 >>> >>> Note that there are a few variations over time (added/removed registers), but >>> the offsets of crucial registers are always the same, so we can safely >>> assume this is the same unit in different generations. >>> >>> Since the RK3066 is the first model starting the RK30 lineup I think we can >>> safely use that for the "base" compatible to be used for e.g. the bindings >>> document, instead of px30 which is just one of the many SoCs that use this unit. >> >> Once the name of the driver is defined and adjusted in v9, I can try to >> give the series a shot on my RK3568 board. First attempts to do so >> basing on Maxime's v5 showed that with a few modifications the DVP >> feature works fine. In a subsequent step, we could discuss the inclusion >> of the MIPI CSI-2 things in order to keep the driver sufficiently general. > > Nice! I guess there will be a need to introduce a variant structure associated > to each compatible to express the differences betweens these different > generations. Indeed. If Mehdi and you suggest something, I'd be happy to review. Otherwise, I'll try to come up with something reasonable. IMHO it would make sense (as a first step) to have the clocks and the resets in this structure, as well as a sub-structure that describes the DVP. The latter consists of registers mainly, but maybe supported input/output formats and other things should go in there as well. Also, downstream code has a significant number of if (some condition including chip_id) A; else B; things that we should probably get rid of with this variant structure. As next step, a sub-structure for MIPI CSI-2 could be defined. RK356X will have one of those, RK3588 will feature even six of them. So we should add a const array to the variant structure. > Note that we will also probably need to convert the driver over to a MC-centric > approach, but this is of course outside of the scope of this series. That would absolutely make sense. What is missing, though? (I was wondering that the driver calls media_device_(un)register but no /dev/mediaX device pops up.) Best regards, Michael > > Cheers, > > Paul > >> @Mehdi: If you could Cc: me when you send out v9 it'd be much appreciated. >> >> Best regards, >> Michael >> >> [0] >> https://github.com/rockchip-linux/kernel/blob/develop-5.10/drivers/media/platform/rockchip/cif/hw.c#L968 >> >>> >>>> This version of the driver supports ONLY the parallel interface BT656 >>>> and was tested/implemented using an SDTV video decoder >>>> >>>> media_tree, base-commit: 2c1bae27df787c9535e48cc27bbd11c3c3e0a235 >>>> >>>> V7 => V8: >>>> vip/capture.c: >>>> - fixed a warning: unused variable reported by the kernel test robot >>>> >>>> V6 => V7: >>>> vip/capture.c vip/dev.c vip/dev.h >>>> - renamed all struct rk_vip_dev dev => struct rk_vip_dev vip_dev >>>> - added some error when rk_vip_get_buffer() returns NULL >>>> - removed a WARN_ON >>>> - made the irq NOT shared >>>> - dropped of_match_ptr >>>> - added the rk_vip_get_resource() function >>>> >>>> rockchip,px30-vip.yaml: >>>> - changed filename to match the compatible >>>> - dropped the mention of the other rockchip SoC in the dt-binding >>>> description and added a more detailed description of VIP >>>> - removed unused labels in the example >>>> >>>> >>>> V5 [1] => V6: >>>> vip/capture.c vip/dev.c vip/dev.h >>>> - added a video g_input_status subdev call, V4L2_IN_CAP_STD and the >>>> supported stds in rk_vip_enum_input callback >>>> - added rk_vip_g_std, rk_vip_s_std and rk_vip_querystd callbacks >>>> - added the supported video_device->tvnorms >>>> - s_std will now update the format as this depends on the standard >>>> NTSC/PAL (as suggested by Hans in [1]) >>>> - removed STD_ATSC >>>> - moved the colorimetry information to come from the subdev >>>> - removed the core s_power subdev calls >>>> - dropped cropping in rk_vip_stream struct >>>> >>>> rockchip-vip.yaml: >>>> - fixed a mistake in the name of third clock plckin -> plck >>>> - changed the reg maxItems 2 -> 1 >>>> >>>> [1] https://lore.kernel.org/linux-media/20201229161724.511102-1-maxime.chevallier@xxxxxxxxxxx/ >>>> >>>> I used v4l-utils with HEAD: commit 1ee258e5bb91a12df378e19eb255c5219d6bc36b >>>> >>>> # v4l2-compliance >>>> v4l2-compliance 1.25.0, 64 bits, 64-bit time_t >>>> >>>> Compliance test for rk_vip device /dev/video0: >>>> >>>> Driver Info: >>>> Driver name : rk_vip >>>> Card type : rk_vip >>>> Bus info : platform:ff490000.vip >>>> Driver version : 6.6.0 >>>> Capabilities : 0x84201000 >>>> Video Capture Multiplanar >>>> Streaming >>>> Extended Pix Format >>>> Device Capabilities >>>> Device Caps : 0x04201000 >>>> Video Capture Multiplanar >>>> Streaming >>>> Extended Pix Format >>>> Media Driver Info: >>>> Driver name : rk_vip >>>> Model : rk_vip >>>> Serial : >>>> Bus info : platform:ff490000.vip >>>> Media version : 6.6.0 >>>> Hardware revision: 0x00000000 (0) >>>> Driver version : 6.6.0 >>>> Interface Info: >>>> ID : 0x03000002 >>>> Type : V4L Video >>>> Entity Info: >>>> ID : 0x00000001 (1) >>>> Name : video_rkvip >>>> Function : V4L2 I/O >>>> Pad 0x01000004 : 0: Sink >>>> Link 0x02000009: from remote pad 0x1000006 of entity 'tw9900 2-0044' (Digital Video Decoder): Data, Enabled >>>> >>>> Required ioctls: >>>> test MC information (see 'Media Driver Info' above): OK >>>> test VIDIOC_QUERYCAP: OK >>>> test invalid ioctls: OK >>>> >>>> Allow for multiple opens: >>>> test second /dev/video0 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 (Not Supported) >>>> >>>> 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 >>>> 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 (Not Supported) >>>> test VIDIOC_QUERYCTRL: OK (Not Supported) >>>> test VIDIOC_G/S_CTRL: OK (Not Supported) >>>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported) >>>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported) >>>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) >>>> Standard Controls: 0 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 (Not Supported) >>>> >>>> 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 rk_vip device /dev/video0: 46, Succeeded: 46, Failed: 0, Warnings: 0 >>>> >>>> Mehdi Djait (3): >>>> media: dt-bindings: media: add bindings for Rockchip VIP >>>> media: rockchip: Add a driver for Rockhip's camera interface >>>> arm64: dts: rockchip: Add the camera interface >>>> >>>> .../bindings/media/rockchip,px30-vip.yaml | 93 ++ >>>> arch/arm64/boot/dts/rockchip/px30.dtsi | 12 + >>>> drivers/media/platform/rockchip/Kconfig | 1 + >>>> drivers/media/platform/rockchip/Makefile | 1 + >>>> drivers/media/platform/rockchip/vip/Kconfig | 14 + >>>> drivers/media/platform/rockchip/vip/Makefile | 3 + >>>> drivers/media/platform/rockchip/vip/capture.c | 1210 +++++++++++++++++ >>>> drivers/media/platform/rockchip/vip/dev.c | 346 +++++ >>>> drivers/media/platform/rockchip/vip/dev.h | 163 +++ >>>> drivers/media/platform/rockchip/vip/regs.h | 260 ++++ >>>> 10 files changed, 2103 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml >>>> create mode 100644 drivers/media/platform/rockchip/vip/Kconfig >>>> create mode 100644 drivers/media/platform/rockchip/vip/Makefile >>>> create mode 100644 drivers/media/platform/rockchip/vip/capture.c >>>> create mode 100644 drivers/media/platform/rockchip/vip/dev.c >>>> create mode 100644 drivers/media/platform/rockchip/vip/dev.h >>>> create mode 100644 drivers/media/platform/rockchip/vip/regs.h >>>> >>>> -- >>>> 2.41.0 >>>> >>> >