arm64: rockchip: add MIPI CSI camera pipeline support for RK3576#10
arm64: rockchip: add MIPI CSI camera pipeline support for RK3576#10hz2 wants to merge 8 commits into
Conversation
|
Thanks for your contribution! As we are targeting mainline Linux kernel, we'll need to follow the mainline Linux contribution rules - in particular the part about the Developer Certificate of Origin there (i.e. please include your real name and email address in both the patch author information and the Signed-off-by: trailer to assert the DCO). Are you certain that there is no register-level difference vs. RK3588? Rockchip's downstream driver has some GRF registers used by RK3588 but not RK3576 Please also check if https://docs.kernel.org/process/coding-assistants.html applies and also if Rockchip authorship/sign-off needs to be preserved in any of the patches (depending on how much of this was copied from the downstream develop-6.1 kernel) |
hz2
force-pushed
the
hz2/rk3576-csi-camera
branch
from
5befd87 to
82192bf
Compare
|
Thanks for getting back so quickly!
added DCO, somewhat familiar with the process
RK3576 uses sys_grf for lane select at 0x14 (GRF_SOC_CON5), where RK3588 uses 0x308 (GRF_SOC_CON2). Mainline doesn't have sys_grf at all currently, only writes GRF for lane and clock enable i believe. the register layout is identical between RK3576 and RK3588 when looking at BSP source and TRM, however, sys_grf is missing from this series and will be added. for the flipper-one use case, per-PHY GRF writes should be sufficient? i'll add sys_grf support in a follow-up or fold it into this series before submitting to the lists, open to your thoughts here as well.
ack, will review and comply
no code was copied from the BSP driver, the driver changes follow mainline patterns (RK3588 entries already in-tree) and register addresses come from TRM and BSP DTS so no Rockchip sign-off should be needed here but please keep me honest. plan would be to address the sys_grf gap i mentioned and add DT binding YAML updates then submit a proper patch series to: accordingly. happy to hear if you think anything else needs to change before that. ps: wouldn't hurt to get a dev flipper one to test with 👀 |
|
FWIW, our builds didn't enable the CSI2 driver and its siblings up to now - I've just changed that. It builds fine with your patches, thank you! Do you have any RK3576 based board to test it on? It would be great to also add the necessary board DTS nodes along with sensor-specific DTSO |
great! i tried it in a nix-shell (probably should have mentioned that) previously when building locally
i do not atm (why i asked about a dev board)
agree that the sensor wiring should go in a DTSO, assuming sensor choice isn't finalized, right? happy to work on the overlay once ik what module to target |
|
will be sending a RFC to mailing list to denote i haven't tested on hardware and get feedback |
| resets = <&cru SRST_P_CSIPHY>, <&cru SRST_SCAN_CSIPHY>; | ||
| reset-names = "apb", "phy"; | ||
| rockchip,grf = <&mipidphy0_grf>; | ||
| rockchip,sys-grf = <&sys_grf>; |
There was a problem hiding this comment.
Is this part of the binding?
Please make sure all your DT changes are tested against schema with W=1
| struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); | ||
|
|
||
| priv->sys_grf_lane_sel_bit = (res && res->start == 0x2b070000) ? 2 : 1; |
There was a problem hiding this comment.
Ouch, this looks ugly. Firstly, you are not guarding this block by a particular SoC compatible but use a SoC specific MMIO address. Secondly, this should probably be encoded as a data attribute somewhere instead of hardcoding register addresses.
There was a problem hiding this comment.
Ouch, this looks ugly.
Thanks! lol
Fair enough, one thing I can't verify without hardware: does rk3576 full mode actually need the SYS_GRF_SOC_CON5 write, or is the reset default already full mode? That decides whether I drop it cleanly or rework it in place.
When it comes back I'd carry the lane select bit as a phandle arg, rockchip,sys-grf = <&sys_grf 1>, read with syscon_regmap_lookup_by_phandle_args, rather than sniffing the MMIO address
| priv->sys_grf = syscon_regmap_lookup_by_phandle(dev->of_node, | ||
| "rockchip,sys-grf"); | ||
| if (IS_ERR(priv->sys_grf)) | ||
| priv->sys_grf = NULL; |
There was a problem hiding this comment.
Will it work on RK3576 if a sys-grf is not provided? If not, this should be an explicit error path instead of covering it up with a NULL.
| csi4_in: port@0 { | ||
| reg = <0>; | ||
| }; |
There was a problem hiding this comment.
Are these supposed to be empty?
There was a problem hiding this comment.
Yes, those are intentional, they are attach points, not omissions. Each csiN port@0 is the sensor side input, left as a bare labeled node in the SoC dtsi so a board file or overlay can attach the sensor endpoint through &csiN_in.
Happy to add a comment there if it reads better.
|
I'd like to drop the sys_grf lane select from this series and bring it back as a proper follow-up. The precedent is your own upstream RK3588 CSI DPHY support: the BSP has the lane select (rk3588_grf_dphy_regs, GRF_SOC_CON2 bit 6) but the mainline rk3588 variant left it out, and the upstream rk3588 csi_dphy nodes carry only rockchip,grf, never a sys_grf. RK3576 is the same IP and the target boards all use a single 2 lane PHY in full mode, so the core port should stand on its own the same way. The one thing I cannot verify: does RK3576 full mode actually work without writing SYS_GRF_SOC_CON5? TRM Part 2 only gives the bit meaning (bit 1 = DPHY0, bit 2 = DPHY1, 0 = full, 1 = split), not the reset value, and the BSP writes it explicitly even for full mode. If POR is full mode, dropping is safe. If not, I should rework it in-series instead. If you'd rather keep it in-series, the plan is to carry the lane select bit as a DT phandle arg, rockchip,sys-grf = <&sys_grf 1>, read with syscon_regmap_lookup_by_phandle_args, so there's no MMIO address sniffing and no offset sentinel. Open to a different representation. |
alchark
force-pushed
the
flipper-devel
branch
2 times, most recently
from
2ac16f5 to
c32805c
Compare
The RK3576 has two Innosilicon MIPI CSI D-PHY instances that use the same register layout as the RK3588 variant (THS-settle at offset 0x160, calibration at 0x168, GRF at offset 0x0). Add the compatible string and driver data for these PHY instances. Signed-off-by: Jason Devers <dev.json2@gmail.com>
The RK3576 uses the same Synopsys DesignWare MIPI CSI-2 Host controller IP as the RK3568. Add the compatible string using the existing RK3568 driver data. Signed-off-by: Jason Devers <dev.json2@gmail.com>
The RK3576 Video Capture (VICAP) unit features five MIPI CSI-2 capture interfaces (compared to six on RK3588). The register layout is identical to the RK3588 variant. Add the compatible string and match data with mipi_num set to 5. Signed-off-by: Jason Devers <dev.json2@gmail.com>
Add device tree nodes for the RK3576 camera capture pipeline: - Two Innosilicon CSI D-PHY instances (csi_dphy0, csi_dphy1) - Two MIPI D-PHY GRF syscon nodes (mipidphy0_grf, mipidphy1_grf) - Five MIPI CSI-2 receiver nodes (csi0 through csi4) - Video Capture (VICAP) unit with five MIPI input ports - VICAP IOMMU The CSI hosts are connected to the VICAP ports and wired to the Innosilicon D-PHY instances. All nodes are disabled by default and must be enabled by board device trees that have camera connectors. Signed-off-by: Jason Devers <dev.json2@gmail.com>
Add rockchip,rk3576-csi-dphy to the compatible enum. The RK3576 has two Innosilicon CSI D-PHY instances and follows the same two-reset pattern (apb, phy) as the existing rk3588-csi-dphy entry. Signed-off-by: Jason Devers <dev.json2@gmail.com>
Add rockchip,rk3576-mipi-csi2 as a compatible that falls back to rockchip,rk3568-mipi-csi2, following the same pattern as the existing rk3588-mipi-csi2 entry. Signed-off-by: Jason Devers <dev.json2@gmail.com>
The RK3576 VICAP uses the same register layout and clock setup as the RK3588 one, with two differences: it has five MIPI CSI-2 input ports instead of six and no DVP parallel port. Extend the existing RK3588 binding rather than adding a separate file: add the rockchip,rk3576-vicap compatible and an allOf/if-then block that drops the DVP port@0 and the sixth MIPI port@6, and caps resets at eight, when that compatible is used. Signed-off-by: Jason Devers <dev.json2@gmail.com>
The RK3576 has two GRF blocks for its Innosilicon CSI D-PHY instances, analogous to the rk3588-csidphy-grf. Document the compatible so the rk3576 camera pipeline nodes validate. Signed-off-by: Jason Devers <dev.json2@gmail.com>
hz2
force-pushed
the
hz2/rk3576-csi-camera
branch
from
c233671 to
56c5e08
Compare
2 participants
Adds initial MIPI CSI-2 camera capture support for the RK3576 SoC, porting the existing RK3588 pipeline to the RK3576. The two SoCs share the same IP blocks (Innosilicon CSI DPHY, Synopsys DW CSI-2 Host, Rockchip VICAP) with minor differences in instance count and register base addresses.
RK3576 has 2 Inno CSI DPHY instances, 5 CSI Host controllers, and a VICAP unit with 5 MIPI input ports (vs 6 on RK3588).
Register addresses and layouts were verified against the RK3576 TRM V1.2 Part 2 and the Rockchip BSP kernel (develop-6.1 branch).
Changes:
This is still missing DT binding YAML updates and board-level DTS wiring (sensor not yet identified for Flipper One). Compile-tested with the flipperone-linux-build-scripts config, RK3576 DTBs builds.
Closes #6