* [PATCH v8 0/5] Add SDM670 camera subsystem
@ 2024-12-16 22:30 Richard Acayan
2024-12-16 22:30 ` [PATCH v8 1/5] dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible Richard Acayan
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
This adds support for the camera subsystem on the Snapdragon 670.
Changes since v7 (20241210233534.614520-7-mailingradian@gmail.com):
- move regulators to CSIPHY blocks (3/5)
- move clocks before interrupts (2/5, 5/5)
- sort clocks alphanumerically (2/5, 5/5)
- rename example node to generic node name (2/5)
Changes since v6 (20241011023724.614584-7-mailingradian@gmail.com):
- set unit address in node name to first address in regs (2/5, 5/5)
Changes since v5 (20241001023520.547271-9-mailingradian@gmail.com):
- sort reg and reg-names alphabetically (2/5, 5/5)
- drop CCI I2C patches since they are applied (formerly 2/7, 3/7)
Changes since v4 (20240904020448.52035-9-mailingradian@gmail.com):
- change camss interrupts to rising edge in dts (7/7)
- change IRQs to rising edge in camss dt-bindings example (4/7)
- move gcc and ahb clocks in camss dt-bindings example (4/7)
- add reviewed-by for camcc dt-bindings patch (1/7)
Changes since v3 (20240819221051.31489-7-mailingradian@gmail.com):
- add specific sdm670 compatible for camcc to dt schema and dts (1/7, 6/7)
- pick up patch from Bryan for CCI driver (3/7)
- stop assigning CCI frequency in dts (7/7)
- add maxItems for sdm670 cci clocks (2/7)
- remove empty line at top of camss dt schema (4/7)
- move regs and reg-names up in camss dt schema (4/7)
- move gcc and ahb clocks up in dts and dt schema (4/7, 7/7)
- add reviewed-by from Vladimir for CCI dt-bindings patch (2/7)
- add reviewed-by from Bryan for dts patch (7/7)
- add reviewed-by from Krzysztof for camss dt-bindings patch (4/7)
- add rewiew tags for camss driver patch (5/7)
Changes since v2 (20240813230037.84004-8-mailingradian@gmail.com):
- drop unnecessary assigned AXI clock frequency (5/5)
- drop src clocks from cci (5/5)
- add unit name, remove mmio properties from port in example dts (2/5)
- correct the reg-names order (2/5)
- add parent_dev_ops to csid (3/5)
- remove CSID clocks from VFE (3/5)
- remove AXI clock from CSIPHY (3/5)
- change subsystem part of the commit message summary (3/5)
- add reviewed-by (4/5)
Changes since v1 (20240806224219.71623-7-mailingradian@gmail.com):
- define dedicated resource structs/arrays for sdm670 (3/5)
- separate camcc device tree node into its own patch (4/5)
- specify correct dual license (2/5)
- add include directives in dt-bindings camss example (2/5)
- remove src clocks from dt-bindings (2/5)
- remove src clocks from dtsi (5/5)
- add power-domain-names to camss (5/5)
- specify power domain names (3/5)
- restrict cci-i2c clocks (1/5)
- populate a commit message with hw info (2/5)
- reword commit message (3/5)
Richard Acayan (5):
dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible
dt-bindings: media: camss: Add qcom,sdm670-camss
media: qcom: camss: add support for SDM670 camss
arm64: dts: qcom: sdm670: add camcc
arm64: dts: qcom: sdm670: add camss and cci
.../bindings/clock/qcom,sdm845-camcc.yaml | 6 +-
.../bindings/media/qcom,sdm670-camss.yaml | 318 ++++++++++++++++++
arch/arm64/boot/dts/qcom/sdm670.dtsi | 195 +++++++++++
drivers/media/platform/qcom/camss/camss.c | 191 +++++++++++
4 files changed, 709 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
--
2.47.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v8 1/5] dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
@ 2024-12-16 22:30 ` Richard Acayan
2024-12-16 22:30 ` [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss Richard Acayan
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
The camera clocks on SDM670 and SDM845 have no significant differences
that would require a change in the clock controller driver. The only
difference is the clock frequency at each level of the power domains,
which is not specified in the clock driver. There should still be a
compatible specific to the SoC, so add the compatible for SDM670 with
the SDM845 compatible as fallback.
Link: https://android.googlesource.com/kernel/msm/+/d4dc50c0a9291bd99895d4844f973421c047d267/drivers/clk/qcom/camcc-sdm845.c#2048
Suggested-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Suggested-by: Konrad Dybcio <konradybcio@kernel.org>
Link: https://lore.kernel.org/linux-arm-msm/7d26a62b-b898-4737-bd53-f49821e3b471@linaro.org
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../devicetree/bindings/clock/qcom,sdm845-camcc.yaml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sdm845-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sdm845-camcc.yaml
index 810b852ae371..fa95c3a1ba3a 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sdm845-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sdm845-camcc.yaml
@@ -20,7 +20,11 @@ allOf:
properties:
compatible:
- const: qcom,sdm845-camcc
+ oneOf:
+ - items:
+ - const: qcom,sdm670-camcc
+ - const: qcom,sdm845-camcc
+ - const: qcom,sdm845-camcc
clocks:
items:
--
2.47.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
2024-12-16 22:30 ` [PATCH v8 1/5] dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible Richard Acayan
@ 2024-12-16 22:30 ` Richard Acayan
2024-12-16 22:47 ` Bryan O'Donoghue
2024-12-17 6:20 ` Krzysztof Kozlowski
2024-12-16 22:30 ` [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss Richard Acayan
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
As found in the Pixel 3a, the Snapdragon 670 has a camera subsystem with
3 CSIDs and 3 VFEs (including 1 VFE lite). Add this camera subsystem to
the bindings.
Adapted from SC8280XP camera subsystem.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
.../bindings/media/qcom,sdm670-camss.yaml | 318 ++++++++++++++++++
1 file changed, 318 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
diff --git a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
new file mode 100644
index 000000000000..f8701a8d27fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
@@ -0,0 +1,318 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,sdm670-camss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SDM670 Camera Subsystem (CAMSS)
+
+maintainers:
+ - Richard Acayan <mailingradian@gmail.com>
+
+description:
+ The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
+
+properties:
+ compatible:
+ const: qcom,sdm670-camss
+
+ reg:
+ maxItems: 9
+
+ reg-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite
+
+ interrupts:
+ maxItems: 9
+
+ interrupt-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite
+
+ clocks:
+ maxItems: 22
+
+ clock-names:
+ items:
+ - const: camnoc_axi
+ - const: cpas_ahb
+ - const: csi0
+ - const: csi1
+ - const: csi2
+ - const: csiphy0
+ - const: csiphy0_timer
+ - const: csiphy1
+ - const: csiphy1_timer
+ - const: csiphy2
+ - const: csiphy2_timer
+ - const: gcc_camera_ahb
+ - const: gcc_camera_axi
+ - const: soc_ahb
+ - const: vfe0
+ - const: vfe0_axi
+ - const: vfe0_cphy_rx
+ - const: vfe1
+ - const: vfe1_axi
+ - const: vfe1_cphy_rx
+ - const: vfe_lite
+ - const: vfe_lite_cphy_rx
+
+ iommus:
+ maxItems: 4
+
+ power-domains:
+ items:
+ - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
+ - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
+ - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
+
+ power-domain-names:
+ items:
+ - const: ife0
+ - const: ife1
+ - const: top
+
+ vdda-phy-supply:
+ description:
+ Phandle to a regulator supply to PHY core block.
+
+ vdda-pll-supply:
+ description:
+ Phandle to 1.8V regulator supply to PHY refclk pll block.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports.
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data from CSIPHY0.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data from CSIPHY1.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@2:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data from CSIPHY2.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - iommus
+ - power-domains
+ - power-domain-names
+ - vdda-phy-supply
+ - vdda-pll-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,camcc-sdm845.h>
+ #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ camera-controller@acb3000 {
+ compatible = "qcom,sdm670-camss";
+
+ reg = <0 0x0acb3000 0 0x1000>,
+ <0 0x0acba000 0 0x1000>,
+ <0 0x0acc8000 0 0x1000>,
+ <0 0x0ac65000 0 0x1000>,
+ <0 0x0ac66000 0 0x1000>,
+ <0 0x0ac67000 0 0x1000>,
+ <0 0x0acaf000 0 0x4000>,
+ <0 0x0acb6000 0 0x4000>,
+ <0 0x0acc4000 0 0x4000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "vfe0",
+ "vfe1",
+ "vfe_lite";
+
+ interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "vfe0",
+ "vfe1",
+ "vfe_lite";
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_IFE_0_CSID_CLK>,
+ <&camcc CAM_CC_IFE_1_CSID_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY1_CLK>,
+ <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY2_CLK>,
+ <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_AHB_CLK>,
+ <&gcc GCC_CAMERA_AXI_CLK>,
+ <&camcc CAM_CC_SOC_AHB_CLK>,
+ <&camcc CAM_CC_IFE_0_CLK>,
+ <&camcc CAM_CC_IFE_0_AXI_CLK>,
+ <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_1_CLK>,
+ <&camcc CAM_CC_IFE_1_AXI_CLK>,
+ <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
+ clock-names = "camnoc_axi",
+ "cpas_ahb",
+ "csi0",
+ "csi1",
+ "csi2",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "gcc_camera_ahb",
+ "gcc_camera_axi",
+ "soc_ahb",
+ "vfe0",
+ "vfe0_axi",
+ "vfe0_cphy_rx",
+ "vfe1",
+ "vfe1_axi",
+ "vfe1_cphy_rx",
+ "vfe_lite",
+ "vfe_lite_cphy_rx";
+
+ iommus = <&apps_smmu 0x808 0x0>,
+ <&apps_smmu 0x810 0x8>,
+ <&apps_smmu 0xc08 0x0>,
+ <&apps_smmu 0xc10 0x8>;
+
+ power-domains = <&camcc IFE_0_GDSC>,
+ <&camcc IFE_1_GDSC>,
+ <&camcc TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "top";
+
+ vdda-phy-supply = <&vreg_l1a_1p225>;
+ vdda-pll-supply = <&vreg_l8a_1p8>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ csiphy_ep0: endpoint {
+ clock-lanes = <7>;
+ data-lanes = <0 1 2 3>;
+ remote-endpoint = <&front_sensor_ep>;
+ };
+ };
+ };
+ };
+ };
--
2.47.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
2024-12-16 22:30 ` [PATCH v8 1/5] dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible Richard Acayan
2024-12-16 22:30 ` [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss Richard Acayan
@ 2024-12-16 22:30 ` Richard Acayan
2024-12-16 22:41 ` Bryan O'Donoghue
2024-12-17 6:21 ` Krzysztof Kozlowski
2024-12-16 22:30 ` [PATCH v8 4/5] arm64: dts: qcom: sdm670: add camcc Richard Acayan
2024-12-16 22:30 ` [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci Richard Acayan
4 siblings, 2 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
The camera subsystem for the SDM670 the same as on SDM845 except with
3 CSIPHY ports instead of 4. Add support for the SDM670 camera
subsystem.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
.../bindings/media/qcom,sdm670-camss.yaml | 72 +++----
drivers/media/platform/qcom/camss/camss.c | 191 ++++++++++++++++++
2 files changed, 227 insertions(+), 36 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
index f8701a8d27fe..892412fb68a9 100644
--- a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
@@ -31,21 +31,6 @@ properties:
- const: vfe1
- const: vfe_lite
- interrupts:
- maxItems: 9
-
- interrupt-names:
- items:
- - const: csid0
- - const: csid1
- - const: csid2
- - const: csiphy0
- - const: csiphy1
- - const: csiphy2
- - const: vfe0
- - const: vfe1
- - const: vfe_lite
-
clocks:
maxItems: 22
@@ -74,6 +59,21 @@ properties:
- const: vfe_lite
- const: vfe_lite_cphy_rx
+ interrupts:
+ maxItems: 9
+
+ interrupt-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite
+
iommus:
maxItems: 4
@@ -177,10 +177,10 @@ required:
- compatible
- reg
- reg-names
- - interrupts
- - interrupt-names
- clocks
- clock-names
+ - interrupts
+ - interrupt-names
- iommus
- power-domains
- power-domain-names
@@ -221,25 +221,6 @@ examples:
"vfe1",
"vfe_lite";
- interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
- interrupt-names = "csid0",
- "csid1",
- "csid2",
- "csiphy0",
- "csiphy1",
- "csiphy2",
- "vfe0",
- "vfe1",
- "vfe_lite";
-
clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
<&camcc CAM_CC_CPAS_AHB_CLK>,
<&camcc CAM_CC_IFE_0_CSID_CLK>,
@@ -285,6 +266,25 @@ examples:
"vfe_lite",
"vfe_lite_cphy_rx";
+ interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "vfe0",
+ "vfe1",
+ "vfe_lite";
+
iommus = <&apps_smmu 0x808 0x0>,
<&apps_smmu 0x810 0x8>,
<&apps_smmu 0xc08 0x0>,
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 9fb31f4c18ad..aba2dbc00e82 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -738,6 +738,185 @@ static const struct camss_subdev_resources vfe_res_660[] = {
}
};
+static const struct camss_subdev_resources csiphy_res_670[] = {
+ /* CSIPHY0 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "soc_ahb", "cpas_ahb",
+ "csiphy0", "csiphy0_timer" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 19200000, 240000000, 269333333 } },
+ .reg = { "csiphy0" },
+ .interrupt = { "csiphy0" },
+ .csiphy = {
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+
+ /* CSIPHY1 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "soc_ahb", "cpas_ahb",
+ "csiphy1", "csiphy1_timer" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 19200000, 240000000, 269333333 } },
+ .reg = { "csiphy1" },
+ .interrupt = { "csiphy1" },
+ .csiphy = {
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+
+ /* CSIPHY2 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "soc_ahb", "cpas_ahb",
+ "csiphy2", "csiphy2_timer" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 19200000, 240000000, 269333333 } },
+ .reg = { "csiphy2" },
+ .interrupt = { "csiphy2" },
+ .csiphy = {
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ }
+};
+
+static const struct camss_subdev_resources csid_res_670[] = {
+ /* CSID0 */
+ {
+ .regulators = {},
+ .clock = { "cpas_ahb", "soc_ahb", "vfe0",
+ "vfe0_cphy_rx", "csi0" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 },
+ { 384000000 },
+ { 19200000, 75000000, 384000000, 538666667 } },
+ .reg = { "csid0" },
+ .interrupt = { "csid0" },
+ .csid = {
+ .hw_ops = &csid_ops_gen2,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .formats = &csid_formats_gen2
+ }
+ },
+
+ /* CSID1 */
+ {
+ .regulators = {},
+ .clock = { "cpas_ahb", "soc_ahb", "vfe1",
+ "vfe1_cphy_rx", "csi1" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 },
+ { 384000000 },
+ { 19200000, 75000000, 384000000, 538666667 } },
+ .reg = { "csid1" },
+ .interrupt = { "csid1" },
+ .csid = {
+ .hw_ops = &csid_ops_gen2,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .formats = &csid_formats_gen2
+ }
+ },
+
+ /* CSID2 */
+ {
+ .regulators = {},
+ .clock = { "cpas_ahb", "soc_ahb", "vfe_lite",
+ "vfe_lite_cphy_rx", "csi2" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 },
+ { 384000000 },
+ { 19200000, 75000000, 384000000, 538666667 } },
+ .reg = { "csid2" },
+ .interrupt = { "csid2" },
+ .csid = {
+ .is_lite = true,
+ .hw_ops = &csid_ops_gen2,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .formats = &csid_formats_gen2
+ }
+ }
+};
+
+static const struct camss_subdev_resources vfe_res_670[] = {
+ /* VFE0 */
+ {
+ .regulators = {},
+ .clock = { "camnoc_axi", "cpas_ahb", "soc_ahb",
+ "vfe0", "vfe0_axi" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 },
+ { 0 } },
+ .reg = { "vfe0" },
+ .interrupt = { "vfe0" },
+ .vfe = {
+ .line_num = 4,
+ .has_pd = true,
+ .pd_name = "ife0",
+ .hw_ops = &vfe_ops_170,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+
+ /* VFE1 */
+ {
+ .regulators = {},
+ .clock = { "camnoc_axi", "cpas_ahb", "soc_ahb",
+ "vfe1", "vfe1_axi" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 },
+ { 0 } },
+ .reg = { "vfe1" },
+ .interrupt = { "vfe1" },
+ .vfe = {
+ .line_num = 4,
+ .has_pd = true,
+ .pd_name = "ife1",
+ .hw_ops = &vfe_ops_170,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+
+ /* VFE-lite */
+ {
+ .regulators = {},
+ .clock = { "camnoc_axi", "cpas_ahb", "soc_ahb",
+ "vfe_lite" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 0 },
+ { 100000000, 320000000, 404000000, 480000000, 600000000 } },
+ .reg = { "vfe_lite" },
+ .interrupt = { "vfe_lite" },
+ .vfe = {
+ .is_lite = true,
+ .line_num = 4,
+ .hw_ops = &vfe_ops_170,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ }
+};
+
static const struct camss_subdev_resources csiphy_res_845[] = {
/* CSIPHY0 */
{
@@ -2582,6 +2761,17 @@ static const struct camss_resources sdm660_resources = {
.link_entities = camss_link_entities
};
+static const struct camss_resources sdm670_resources = {
+ .version = CAMSS_845,
+ .csiphy_res = csiphy_res_670,
+ .csid_res = csid_res_670,
+ .vfe_res = vfe_res_670,
+ .csiphy_num = ARRAY_SIZE(csiphy_res_670),
+ .csid_num = ARRAY_SIZE(csid_res_670),
+ .vfe_num = ARRAY_SIZE(vfe_res_670),
+ .link_entities = camss_link_entities
+};
+
static const struct camss_resources sdm845_resources = {
.version = CAMSS_845,
.csiphy_res = csiphy_res_845,
@@ -2627,6 +2817,7 @@ static const struct of_device_id camss_dt_match[] = {
{ .compatible = "qcom,msm8953-camss", .data = &msm8953_resources },
{ .compatible = "qcom,msm8996-camss", .data = &msm8996_resources },
{ .compatible = "qcom,sdm660-camss", .data = &sdm660_resources },
+ { .compatible = "qcom,sdm670-camss", .data = &sdm670_resources },
{ .compatible = "qcom,sdm845-camss", .data = &sdm845_resources },
{ .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
{ .compatible = "qcom,sc8280xp-camss", .data = &sc8280xp_resources },
--
2.47.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v8 4/5] arm64: dts: qcom: sdm670: add camcc
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
` (2 preceding siblings ...)
2024-12-16 22:30 ` [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss Richard Acayan
@ 2024-12-16 22:30 ` Richard Acayan
2024-12-16 22:30 ` [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci Richard Acayan
4 siblings, 0 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
The camera clock controller on SDM670 controls the clocks that drive the
camera subsystem. The clocks are the same as on SDM845. Add the camera
clock controller for SDM670.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
arch/arm64/boot/dts/qcom/sdm670.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
index c93dd06c0b7d..328096b91126 100644
--- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
@@ -1400,6 +1400,16 @@ spmi_bus: spmi@c440000 {
#interrupt-cells = <4>;
};
+ camcc: clock-controller@ad00000 {
+ compatible = "qcom,sdm670-camcc", "qcom,sdm845-camcc";
+ reg = <0 0x0ad00000 0 0x10000>;
+ clocks = <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "bi_tcxo";
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+
mdss: display-subsystem@ae00000 {
compatible = "qcom,sdm670-mdss";
reg = <0 0x0ae00000 0 0x1000>;
--
2.47.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
` (3 preceding siblings ...)
2024-12-16 22:30 ` [PATCH v8 4/5] arm64: dts: qcom: sdm670: add camcc Richard Acayan
@ 2024-12-16 22:30 ` Richard Acayan
2024-12-17 7:31 ` Vladimir Zapolskiy
4 siblings, 1 reply; 15+ messages in thread
From: Richard Acayan @ 2024-12-16 22:30 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy, Richard Acayan
Add the camera subsystem and CCI used to interface with cameras on the
Snapdragon 670.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/sdm670.dtsi | 185 +++++++++++++++++++++++++++
1 file changed, 185 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
index 328096b91126..d4e1251ada04 100644
--- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
@@ -6,6 +6,7 @@
* Copyright (c) 2022, Richard Acayan. All rights reserved.
*/
+#include <dt-bindings/clock/qcom,camcc-sdm845.h>
#include <dt-bindings/clock/qcom,dispcc-sdm845.h>
#include <dt-bindings/clock/qcom,gcc-sdm845.h>
#include <dt-bindings/clock/qcom,rpmh.h>
@@ -1168,6 +1169,34 @@ tlmm: pinctrl@3400000 {
gpio-ranges = <&tlmm 0 0 151>;
wakeup-parent = <&pdc>;
+ cci0_default: cci0-default-state {
+ pins = "gpio17", "gpio18";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ cci0_sleep: cci0-sleep-state {
+ pins = "gpio17", "gpio18";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ cci1_default: cci1-default-state {
+ pins = "gpio19", "gpio20";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ cci1_sleep: cci1-sleep-state {
+ pins = "gpio19", "gpio20";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
qup_i2c0_default: qup-i2c0-default-state {
pins = "gpio0", "gpio1";
function = "qup0";
@@ -1400,6 +1429,162 @@ spmi_bus: spmi@c440000 {
#interrupt-cells = <4>;
};
+ cci: cci@ac4a000 {
+ compatible = "qcom,sdm670-cci", "qcom,msm8996-cci";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0 0x0ac4a000 0 0x4000>;
+ interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
+ power-domains = <&camcc TITAN_TOP_GDSC>;
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+ <&camcc CAM_CC_SOC_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CCI_CLK>;
+ clock-names = "camnoc_axi",
+ "soc_ahb",
+ "cpas_ahb",
+ "cci";
+
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&cci0_default &cci1_default>;
+ pinctrl-1 = <&cci0_sleep &cci1_sleep>;
+
+ status = "disabled";
+
+ cci_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ camss: camera-controller@acb3000 {
+ compatible = "qcom,sdm670-camss";
+ reg = <0 0x0acb3000 0 0x1000>,
+ <0 0x0acba000 0 0x1000>,
+ <0 0x0acc8000 0 0x1000>,
+ <0 0x0ac65000 0 0x1000>,
+ <0 0x0ac66000 0 0x1000>,
+ <0 0x0ac67000 0 0x1000>,
+ <0 0x0acaf000 0 0x4000>,
+ <0 0x0acb6000 0 0x4000>,
+ <0 0x0acc4000 0 0x4000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "vfe0",
+ "vfe1",
+ "vfe_lite";
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_IFE_0_CSID_CLK>,
+ <&camcc CAM_CC_IFE_1_CSID_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY1_CLK>,
+ <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY2_CLK>,
+ <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_AHB_CLK>,
+ <&gcc GCC_CAMERA_AXI_CLK>,
+ <&camcc CAM_CC_SOC_AHB_CLK>,
+ <&camcc CAM_CC_IFE_0_CLK>,
+ <&camcc CAM_CC_IFE_0_AXI_CLK>,
+ <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_1_CLK>,
+ <&camcc CAM_CC_IFE_1_AXI_CLK>,
+ <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
+ clock-names = "camnoc_axi",
+ "cpas_ahb",
+ "csi0",
+ "csi1",
+ "csi2",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "gcc_camera_ahb",
+ "gcc_camera_axi",
+ "soc_ahb",
+ "vfe0",
+ "vfe0_axi",
+ "vfe0_cphy_rx",
+ "vfe1",
+ "vfe1_axi",
+ "vfe1_cphy_rx",
+ "vfe_lite",
+ "vfe_lite_cphy_rx";
+
+ interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "vfe0",
+ "vfe1",
+ "vfe_lite";
+
+ iommus = <&apps_smmu 0x808 0x0>,
+ <&apps_smmu 0x810 0x8>,
+ <&apps_smmu 0xc08 0x0>,
+ <&apps_smmu 0xc10 0x8>;
+
+ power-domains = <&camcc IFE_0_GDSC>,
+ <&camcc IFE_1_GDSC>,
+ <&camcc TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "top";
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ camss_port0: port@0 {
+ reg = <0>;
+ };
+
+ camss_port1: port@1 {
+ reg = <1>;
+ };
+
+ camss_port2: port@2 {
+ reg = <2>;
+ };
+ };
+ };
+
camcc: clock-controller@ad00000 {
compatible = "qcom,sdm670-camcc", "qcom,sdm845-camcc";
reg = <0 0x0ad00000 0 0x10000>;
--
2.47.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss
2024-12-16 22:30 ` [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss Richard Acayan
@ 2024-12-16 22:41 ` Bryan O'Donoghue
2024-12-17 23:20 ` Richard Acayan
2024-12-17 6:21 ` Krzysztof Kozlowski
1 sibling, 1 reply; 15+ messages in thread
From: Bryan O'Donoghue @ 2024-12-16 22:41 UTC (permalink / raw)
To: Richard Acayan, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio, linux-arm-msm,
linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy
On 16/12/2024 22:30, Richard Acayan wrote:
> .../bindings/media/qcom,sdm670-camss.yaml | 72 +++----
> drivers/media/platform/qcom/camss/camss.c | 191 ++++++++++++++++++
> 2 files changed, 227 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> index f8701a8d27fe..892412fb68a9 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> @@ -31,21 +31,6 @@ properties:
> - const: vfe1
> - const: vfe_lite
>
> - interrupts:
> - maxItems: 9
I think you've had an error squashing some patches.
---
bod
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss
2024-12-16 22:30 ` [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss Richard Acayan
@ 2024-12-16 22:47 ` Bryan O'Donoghue
2024-12-17 6:20 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Bryan O'Donoghue @ 2024-12-16 22:47 UTC (permalink / raw)
To: Richard Acayan, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio, linux-arm-msm,
linux-clk, devicetree, linux-media
Cc: Vladimir Zapolskiy
On 16/12/2024 22:30, Richard Acayan wrote:
> As found in the Pixel 3a, the Snapdragon 670 has a camera subsystem with
> 3 CSIDs and 3 VFEs (including 1 VFE lite). Add this camera subsystem to
> the bindings.
>
> Adapted from SC8280XP camera subsystem.
>
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> ---
> .../bindings/media/qcom,sdm670-camss.yaml | 318 ++++++++++++++++++
> 1 file changed, 318 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> new file mode 100644
> index 000000000000..f8701a8d27fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> @@ -0,0 +1,318 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,sdm670-camss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SDM670 Camera Subsystem (CAMSS)
> +
> +maintainers:
> + - Richard Acayan <mailingradian@gmail.com>
> +
> +description:
> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
> +
> +properties:
> + compatible:
> + const: qcom,sdm670-camss
> +
> + reg:
> + maxItems: 9
> +
> + reg-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: vfe0
> + - const: vfe1
> + - const: vfe_lite
> +
> + interrupts:
> + maxItems: 9
> +
> + interrupt-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: vfe0
> + - const: vfe1
> + - const: vfe_lite
> +
> + clocks:
> + maxItems: 22
> +
> + clock-names:
> + items:
> + - const: camnoc_axi
> + - const: cpas_ahb
> + - const: csi0
> + - const: csi1
> + - const: csi2
> + - const: csiphy0
> + - const: csiphy0_timer
> + - const: csiphy1
> + - const: csiphy1_timer
> + - const: csiphy2
> + - const: csiphy2_timer
> + - const: gcc_camera_ahb
> + - const: gcc_camera_axi
> + - const: soc_ahb
> + - const: vfe0
> + - const: vfe0_axi
> + - const: vfe0_cphy_rx
> + - const: vfe1
> + - const: vfe1_axi
> + - const: vfe1_cphy_rx
> + - const: vfe_lite
> + - const: vfe_lite_cphy_rx
> +
> + iommus:
> + maxItems: 4
> +
> + power-domains:
> + items:
> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
> +
> + power-domain-names:
> + items:
> + - const: ife0
> + - const: ife1
> + - const: top
> +
> + vdda-phy-supply:
> + description:
> + Phandle to a regulator supply to PHY core block.
> +
> + vdda-pll-supply:
> + description:
> + Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + description:
> + CSI input ports.
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data from CSIPHY0.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data from CSIPHY1.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> + port@2:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port for receiving CSI data from CSIPHY2.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> + - clocks
> + - clock-names
> + - iommus
> + - power-domains
> + - power-domain-names
> + - vdda-phy-supply
> + - vdda-pll-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,camcc-sdm845.h>
> + #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + camera-controller@acb3000 {
isp@
> + compatible = "qcom,sdm670-camss";
> +
> + reg = <0 0x0acb3000 0 0x1000>,
> + <0 0x0acba000 0 0x1000>,
> + <0 0x0acc8000 0 0x1000>,
> + <0 0x0ac65000 0 0x1000>,
> + <0 0x0ac66000 0 0x1000>,
> + <0 0x0ac67000 0 0x1000>,
> + <0 0x0acaf000 0 0x4000>,
> + <0 0x0acb6000 0 0x4000>,
> + <0 0x0acc4000 0 0x4000>;
> + reg-names = "csid0",
> + "csid1",
> + "csid2",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "vfe0",
> + "vfe1",
> + "vfe_lite";
> +
> + interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "csid0",
> + "csid1",
> + "csid2",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "vfe0",
> + "vfe1",
> + "vfe_lite";
> +
> + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_IFE_0_CSID_CLK>,
> + <&camcc CAM_CC_IFE_1_CSID_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
> + <&camcc CAM_CC_CSIPHY0_CLK>,
> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> + <&camcc CAM_CC_CSIPHY1_CLK>,
> + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> + <&camcc CAM_CC_CSIPHY2_CLK>,
> + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> + <&gcc GCC_CAMERA_AHB_CLK>,
> + <&gcc GCC_CAMERA_AXI_CLK>,
> + <&camcc CAM_CC_SOC_AHB_CLK>,
> + <&camcc CAM_CC_IFE_0_CLK>,
> + <&camcc CAM_CC_IFE_0_AXI_CLK>,
> + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_1_CLK>,
> + <&camcc CAM_CC_IFE_1_AXI_CLK>,
> + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
> + clock-names = "camnoc_axi",
> + "cpas_ahb",
> + "csi0",
> + "csi1",
> + "csi2",
> + "csiphy0",
> + "csiphy0_timer",
> + "csiphy1",
> + "csiphy1_timer",
> + "csiphy2",
> + "csiphy2_timer",
> + "gcc_camera_ahb",
> + "gcc_camera_axi",
> + "soc_ahb",
> + "vfe0",
> + "vfe0_axi",
> + "vfe0_cphy_rx",
> + "vfe1",
> + "vfe1_axi",
> + "vfe1_cphy_rx",
> + "vfe_lite",
> + "vfe_lite_cphy_rx";
> +
> + iommus = <&apps_smmu 0x808 0x0>,
> + <&apps_smmu 0x810 0x8>,
> + <&apps_smmu 0xc08 0x0>,
> + <&apps_smmu 0xc10 0x8>;
> +
> + power-domains = <&camcc IFE_0_GDSC>,
> + <&camcc IFE_1_GDSC>,
> + <&camcc TITAN_TOP_GDSC>;
> + power-domain-names = "ife0",
> + "ife1",
> + "top";
> +
> + vdda-phy-supply = <&vreg_l1a_1p225>;
> + vdda-pll-supply = <&vreg_l8a_1p8>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + csiphy_ep0: endpoint {
> + clock-lanes = <7>;
> + data-lanes = <0 1 2 3>;
> + remote-endpoint = <&front_sensor_ep>;
> + };
> + };
> + };
> + };
> + };
Otherwise LGTM
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss
2024-12-16 22:30 ` [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss Richard Acayan
2024-12-16 22:47 ` Bryan O'Donoghue
@ 2024-12-17 6:20 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-17 6:20 UTC (permalink / raw)
To: Richard Acayan
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media,
Vladimir Zapolskiy
On Mon, Dec 16, 2024 at 05:30:23PM -0500, Richard Acayan wrote:
> As found in the Pixel 3a, the Snapdragon 670 has a camera subsystem with
> 3 CSIDs and 3 VFEs (including 1 VFE lite). Add this camera subsystem to
> the bindings.
>
> Adapted from SC8280XP camera subsystem.
>
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> ---
> .../bindings/media/qcom,sdm670-camss.yaml | 318 ++++++++++++++++++
> 1 file changed, 318 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss
2024-12-16 22:30 ` [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss Richard Acayan
2024-12-16 22:41 ` Bryan O'Donoghue
@ 2024-12-17 6:21 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-17 6:21 UTC (permalink / raw)
To: Richard Acayan
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media,
Vladimir Zapolskiy
On Mon, Dec 16, 2024 at 05:30:24PM -0500, Richard Acayan wrote:
> The camera subsystem for the SDM670 the same as on SDM845 except with
> 3 CSIPHY ports instead of 4. Add support for the SDM670 camera
> subsystem.
>
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> .../bindings/media/qcom,sdm670-camss.yaml | 72 +++----
> drivers/media/platform/qcom/camss/camss.c | 191 ++++++++++++++++++
You cannot combine such changes. Bindings are always separate.
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.
Please drop the review/ack tags and request re-review for this patch.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci
2024-12-16 22:30 ` [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci Richard Acayan
@ 2024-12-17 7:31 ` Vladimir Zapolskiy
2024-12-17 20:25 ` Richard Acayan
0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Zapolskiy @ 2024-12-17 7:31 UTC (permalink / raw)
To: Richard Acayan, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Bryan O'Donoghue, Mauro Carvalho Chehab,
Konrad Dybcio, linux-arm-msm, linux-clk, devicetree, linux-media
Hi Richard.
On 12/17/24 00:30, Richard Acayan wrote:
> Add the camera subsystem and CCI used to interface with cameras on the
> Snapdragon 670.
>
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sdm670.dtsi | 185 +++++++++++++++++++++++++++
> 1 file changed, 185 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> index 328096b91126..d4e1251ada04 100644
> --- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> @@ -6,6 +6,7 @@
> * Copyright (c) 2022, Richard Acayan. All rights reserved.
> */
>
> +#include <dt-bindings/clock/qcom,camcc-sdm845.h>
> #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
> #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> #include <dt-bindings/clock/qcom,rpmh.h>
> @@ -1168,6 +1169,34 @@ tlmm: pinctrl@3400000 {
> gpio-ranges = <&tlmm 0 0 151>;
> wakeup-parent = <&pdc>;
>
> + cci0_default: cci0-default-state {
> + pins = "gpio17", "gpio18";
> + function = "cci_i2c";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + cci0_sleep: cci0-sleep-state {
> + pins = "gpio17", "gpio18";
> + function = "cci_i2c";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> +
> + cci1_default: cci1-default-state {
> + pins = "gpio19", "gpio20";
> + function = "cci_i2c";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + cci1_sleep: cci1-sleep-state {
> + pins = "gpio19", "gpio20";
> + function = "cci_i2c";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> +
> qup_i2c0_default: qup-i2c0-default-state {
> pins = "gpio0", "gpio1";
> function = "qup0";
> @@ -1400,6 +1429,162 @@ spmi_bus: spmi@c440000 {
> #interrupt-cells = <4>;
> };
>
> + cci: cci@ac4a000 {
> + compatible = "qcom,sdm670-cci", "qcom,msm8996-cci";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0 0x0ac4a000 0 0x4000>;
> + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> + power-domains = <&camcc TITAN_TOP_GDSC>;
> +
> + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&camcc CAM_CC_SOC_AHB_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_CCI_CLK>;
> + clock-names = "camnoc_axi",
> + "soc_ahb",
> + "cpas_ahb",
> + "cci";
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&cci0_default &cci1_default>;
> + pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> +
> + status = "disabled";
> +
> + cci_i2c0: i2c-bus@0 {
> + reg = <0>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + cci_i2c1: i2c-bus@1 {
> + reg = <1>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + camss: camera-controller@acb3000 {
Wasn't it agreed recently to have 'isp' as a generic device name for CAMSS IP?
> + compatible = "qcom,sdm670-camss";
> + reg = <0 0x0acb3000 0 0x1000>,
> + <0 0x0acba000 0 0x1000>,
> + <0 0x0acc8000 0 0x1000>,
> + <0 0x0ac65000 0 0x1000>,
> + <0 0x0ac66000 0 0x1000>,
> + <0 0x0ac67000 0 0x1000>,
> + <0 0x0acaf000 0 0x4000>,
> + <0 0x0acb6000 0 0x4000>,
> + <0 0x0acc4000 0 0x4000>;
> + reg-names = "csid0",
> + "csid1",
> + "csid2",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "vfe0",
> + "vfe1",
> + "vfe_lite";
> +
> + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_IFE_0_CSID_CLK>,
> + <&camcc CAM_CC_IFE_1_CSID_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
> + <&camcc CAM_CC_CSIPHY0_CLK>,
> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> + <&camcc CAM_CC_CSIPHY1_CLK>,
> + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> + <&camcc CAM_CC_CSIPHY2_CLK>,
> + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> + <&gcc GCC_CAMERA_AHB_CLK>,
> + <&gcc GCC_CAMERA_AXI_CLK>,
> + <&camcc CAM_CC_SOC_AHB_CLK>,
> + <&camcc CAM_CC_IFE_0_CLK>,
> + <&camcc CAM_CC_IFE_0_AXI_CLK>,
> + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_1_CLK>,
> + <&camcc CAM_CC_IFE_1_AXI_CLK>,
> + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
> + clock-names = "camnoc_axi",
> + "cpas_ahb",
> + "csi0",
> + "csi1",
> + "csi2",
> + "csiphy0",
> + "csiphy0_timer",
> + "csiphy1",
> + "csiphy1_timer",
> + "csiphy2",
> + "csiphy2_timer",
> + "gcc_camera_ahb",
> + "gcc_camera_axi",
> + "soc_ahb",
> + "vfe0",
> + "vfe0_axi",
> + "vfe0_cphy_rx",
> + "vfe1",
> + "vfe1_axi",
> + "vfe1_cphy_rx",
> + "vfe_lite",
> + "vfe_lite_cphy_rx";
> +
> + interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "csid0",
> + "csid1",
> + "csid2",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "vfe0",
> + "vfe1",
> + "vfe_lite";
> +
> + iommus = <&apps_smmu 0x808 0x0>,
> + <&apps_smmu 0x810 0x8>,
> + <&apps_smmu 0xc08 0x0>,
> + <&apps_smmu 0xc10 0x8>;
> +
> + power-domains = <&camcc IFE_0_GDSC>,
> + <&camcc IFE_1_GDSC>,
> + <&camcc TITAN_TOP_GDSC>;
> + power-domain-names = "ife0",
> + "ife1",
> + "top";
> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camss_port0: port@0 {
> + reg = <0>;
> + };
> +
> + camss_port1: port@1 {
> + reg = <1>;
> + };
> +
> + camss_port2: port@2 {
Likely labels to ports are excessive here, please remove them.
> + reg = <2>;
> + };
> + };
> + };
> +
> camcc: clock-controller@ad00000 {
> compatible = "qcom,sdm670-camcc", "qcom,sdm845-camcc";
> reg = <0 0x0ad00000 0 0x10000>;
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci
2024-12-17 7:31 ` Vladimir Zapolskiy
@ 2024-12-17 20:25 ` Richard Acayan
2024-12-17 21:34 ` Vladimir Zapolskiy
0 siblings, 1 reply; 15+ messages in thread
From: Richard Acayan @ 2024-12-17 20:25 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
On Tue, Dec 17, 2024 at 09:31:50AM +0200, Vladimir Zapolskiy wrote:
> Hi Richard.
>
> On 12/17/24 00:30, Richard Acayan wrote:
> > Add the camera subsystem and CCI used to interface with cameras on the
> > Snapdragon 670.
> >
> > Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > ---
> > arch/arm64/boot/dts/qcom/sdm670.dtsi | 185 +++++++++++++++++++++++++++
> > 1 file changed, 185 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > index 328096b91126..d4e1251ada04 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > @@ -6,6 +6,7 @@
> > * Copyright (c) 2022, Richard Acayan. All rights reserved.
> > */
> > +#include <dt-bindings/clock/qcom,camcc-sdm845.h>
> > #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
> > #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> > #include <dt-bindings/clock/qcom,rpmh.h>
> > @@ -1168,6 +1169,34 @@ tlmm: pinctrl@3400000 {
> > gpio-ranges = <&tlmm 0 0 151>;
> > wakeup-parent = <&pdc>;
> > + cci0_default: cci0-default-state {
> > + pins = "gpio17", "gpio18";
> > + function = "cci_i2c";
> > + drive-strength = <2>;
> > + bias-pull-up;
> > + };
> > +
> > + cci0_sleep: cci0-sleep-state {
> > + pins = "gpio17", "gpio18";
> > + function = "cci_i2c";
> > + drive-strength = <2>;
> > + bias-pull-down;
> > + };
> > +
> > + cci1_default: cci1-default-state {
> > + pins = "gpio19", "gpio20";
> > + function = "cci_i2c";
> > + drive-strength = <2>;
> > + bias-pull-up;
> > + };
> > +
> > + cci1_sleep: cci1-sleep-state {
> > + pins = "gpio19", "gpio20";
> > + function = "cci_i2c";
> > + drive-strength = <2>;
> > + bias-pull-down;
> > + };
> > +
> > qup_i2c0_default: qup-i2c0-default-state {
> > pins = "gpio0", "gpio1";
> > function = "qup0";
> > @@ -1400,6 +1429,162 @@ spmi_bus: spmi@c440000 {
> > #interrupt-cells = <4>;
> > };
> > + cci: cci@ac4a000 {
> > + compatible = "qcom,sdm670-cci", "qcom,msm8996-cci";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + reg = <0 0x0ac4a000 0 0x4000>;
> > + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> > + power-domains = <&camcc TITAN_TOP_GDSC>;
> > +
> > + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> > + <&camcc CAM_CC_SOC_AHB_CLK>,
> > + <&camcc CAM_CC_CPAS_AHB_CLK>,
> > + <&camcc CAM_CC_CCI_CLK>;
> > + clock-names = "camnoc_axi",
> > + "soc_ahb",
> > + "cpas_ahb",
> > + "cci";
> > +
> > + pinctrl-names = "default", "sleep";
> > + pinctrl-0 = <&cci0_default &cci1_default>;
> > + pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> > +
> > + status = "disabled";
> > +
> > + cci_i2c0: i2c-bus@0 {
> > + reg = <0>;
> > + clock-frequency = <1000000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > + cci_i2c1: i2c-bus@1 {
> > + reg = <1>;
> > + clock-frequency = <1000000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + };
> > +
> > + camss: camera-controller@acb3000 {
>
> Wasn't it agreed recently to have 'isp' as a generic device name for CAMSS IP?
Yeah, will change.
>
> > + compatible = "qcom,sdm670-camss";
> > + reg = <0 0x0acb3000 0 0x1000>,
> > + <0 0x0acba000 0 0x1000>,
> > + <0 0x0acc8000 0 0x1000>,
> > + <0 0x0ac65000 0 0x1000>,
> > + <0 0x0ac66000 0 0x1000>,
> > + <0 0x0ac67000 0 0x1000>,
> > + <0 0x0acaf000 0 0x4000>,
> > + <0 0x0acb6000 0 0x4000>,
> > + <0 0x0acc4000 0 0x4000>;
> > + reg-names = "csid0",
> > + "csid1",
> > + "csid2",
> > + "csiphy0",
> > + "csiphy1",
> > + "csiphy2",
> > + "vfe0",
> > + "vfe1",
> > + "vfe_lite";
> > +
> > + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> > + <&camcc CAM_CC_CPAS_AHB_CLK>,
> > + <&camcc CAM_CC_IFE_0_CSID_CLK>,
> > + <&camcc CAM_CC_IFE_1_CSID_CLK>,
> > + <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
> > + <&camcc CAM_CC_CSIPHY0_CLK>,
> > + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> > + <&camcc CAM_CC_CSIPHY1_CLK>,
> > + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> > + <&camcc CAM_CC_CSIPHY2_CLK>,
> > + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> > + <&gcc GCC_CAMERA_AHB_CLK>,
> > + <&gcc GCC_CAMERA_AXI_CLK>,
> > + <&camcc CAM_CC_SOC_AHB_CLK>,
> > + <&camcc CAM_CC_IFE_0_CLK>,
> > + <&camcc CAM_CC_IFE_0_AXI_CLK>,
> > + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> > + <&camcc CAM_CC_IFE_1_CLK>,
> > + <&camcc CAM_CC_IFE_1_AXI_CLK>,
> > + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> > + <&camcc CAM_CC_IFE_LITE_CLK>,
> > + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
> > + clock-names = "camnoc_axi",
> > + "cpas_ahb",
> > + "csi0",
> > + "csi1",
> > + "csi2",
> > + "csiphy0",
> > + "csiphy0_timer",
> > + "csiphy1",
> > + "csiphy1_timer",
> > + "csiphy2",
> > + "csiphy2_timer",
> > + "gcc_camera_ahb",
> > + "gcc_camera_axi",
> > + "soc_ahb",
> > + "vfe0",
> > + "vfe0_axi",
> > + "vfe0_cphy_rx",
> > + "vfe1",
> > + "vfe1_axi",
> > + "vfe1_cphy_rx",
> > + "vfe_lite",
> > + "vfe_lite_cphy_rx";
> > +
> > + interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
> > + interrupt-names = "csid0",
> > + "csid1",
> > + "csid2",
> > + "csiphy0",
> > + "csiphy1",
> > + "csiphy2",
> > + "vfe0",
> > + "vfe1",
> > + "vfe_lite";
> > +
> > + iommus = <&apps_smmu 0x808 0x0>,
> > + <&apps_smmu 0x810 0x8>,
> > + <&apps_smmu 0xc08 0x0>,
> > + <&apps_smmu 0xc10 0x8>;
> > +
> > + power-domains = <&camcc IFE_0_GDSC>,
> > + <&camcc IFE_1_GDSC>,
> > + <&camcc TITAN_TOP_GDSC>;
> > + power-domain-names = "ife0",
> > + "ife1",
> > + "top";
> > +
> > + status = "disabled";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + camss_port0: port@0 {
> > + reg = <0>;
> > + };
> > +
> > + camss_port1: port@1 {
> > + reg = <1>;
> > + };
> > +
> > + camss_port2: port@2 {
>
> Likely labels to ports are excessive here, please remove them.
How would you imagine connecting a camera to the ports, then? For MDSS,
there's a label for the endpoint (mdss_dsi0_out) which the device DTS
can then reference:
&mdss_dsi0_out {
remote-endpoint = <&panel_in>;
data-lanes = <0 1 2 3>;
};
For CAMSS, the port labels would be used like so:
&camss_port1 {
camss_endpoint1: endpoint {
clock-lanes = <7>;
data-lanes = <0 1 2 3>;
remote-endpoint = <&cam_front_endpoint>;
};
};
Without the labels, the connection might look something like:
&camss {
status = "okay";
// Modification of existing /soc/isp@acb3000/ports node
ports {
// Modification of existing /soc/isp@acb3000/ports/port@1 node
port@1 {
// New node
camss_endpoint1: endpoint {
clock-lanes = <7>;
data-lanes = <0 1 2 3>;
remote-endpoint = <&cam_front_endpoint>;
};
};
};
};
which I believe is not preferred.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci
2024-12-17 20:25 ` Richard Acayan
@ 2024-12-17 21:34 ` Vladimir Zapolskiy
2024-12-17 23:06 ` Richard Acayan
0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Zapolskiy @ 2024-12-17 21:34 UTC (permalink / raw)
To: Richard Acayan
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
On 12/17/24 22:25, Richard Acayan wrote:
> On Tue, Dec 17, 2024 at 09:31:50AM +0200, Vladimir Zapolskiy wrote:
>> Hi Richard.
>>
>> On 12/17/24 00:30, Richard Acayan wrote:
>>> Add the camera subsystem and CCI used to interface with cameras on the
>>> Snapdragon 670.
>>>
>>> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>> arch/arm64/boot/dts/qcom/sdm670.dtsi | 185 +++++++++++++++++++++++++++
>>> 1 file changed, 185 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
>>> index 328096b91126..d4e1251ada04 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
>>> @@ -6,6 +6,7 @@
>>> * Copyright (c) 2022, Richard Acayan. All rights reserved.
>>> */
>>> +#include <dt-bindings/clock/qcom,camcc-sdm845.h>
>>> #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
>>> #include <dt-bindings/clock/qcom,gcc-sdm845.h>
>>> #include <dt-bindings/clock/qcom,rpmh.h>
>>> @@ -1168,6 +1169,34 @@ tlmm: pinctrl@3400000 {
>>> gpio-ranges = <&tlmm 0 0 151>;
>>> wakeup-parent = <&pdc>;
>>> + cci0_default: cci0-default-state {
>>> + pins = "gpio17", "gpio18";
>>> + function = "cci_i2c";
>>> + drive-strength = <2>;
>>> + bias-pull-up;
>>> + };
>>> +
>>> + cci0_sleep: cci0-sleep-state {
>>> + pins = "gpio17", "gpio18";
>>> + function = "cci_i2c";
>>> + drive-strength = <2>;
>>> + bias-pull-down;
>>> + };
>>> +
>>> + cci1_default: cci1-default-state {
>>> + pins = "gpio19", "gpio20";
>>> + function = "cci_i2c";
>>> + drive-strength = <2>;
>>> + bias-pull-up;
>>> + };
>>> +
>>> + cci1_sleep: cci1-sleep-state {
>>> + pins = "gpio19", "gpio20";
>>> + function = "cci_i2c";
>>> + drive-strength = <2>;
>>> + bias-pull-down;
>>> + };
>>> +
>>> qup_i2c0_default: qup-i2c0-default-state {
>>> pins = "gpio0", "gpio1";
>>> function = "qup0";
>>> @@ -1400,6 +1429,162 @@ spmi_bus: spmi@c440000 {
>>> #interrupt-cells = <4>;
>>> };
>>> + cci: cci@ac4a000 {
>>> + compatible = "qcom,sdm670-cci", "qcom,msm8996-cci";
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + reg = <0 0x0ac4a000 0 0x4000>;
>>> + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
>>> + power-domains = <&camcc TITAN_TOP_GDSC>;
>>> +
>>> + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
>>> + <&camcc CAM_CC_SOC_AHB_CLK>,
>>> + <&camcc CAM_CC_CPAS_AHB_CLK>,
>>> + <&camcc CAM_CC_CCI_CLK>;
>>> + clock-names = "camnoc_axi",
>>> + "soc_ahb",
>>> + "cpas_ahb",
>>> + "cci";
>>> +
>>> + pinctrl-names = "default", "sleep";
>>> + pinctrl-0 = <&cci0_default &cci1_default>;
>>> + pinctrl-1 = <&cci0_sleep &cci1_sleep>;
>>> +
>>> + status = "disabled";
>>> +
>>> + cci_i2c0: i2c-bus@0 {
>>> + reg = <0>;
>>> + clock-frequency = <1000000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + };
>>> +
>>> + cci_i2c1: i2c-bus@1 {
>>> + reg = <1>;
>>> + clock-frequency = <1000000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + };
>>> + };
>>> +
>>> + camss: camera-controller@acb3000 {
>>
>> Wasn't it agreed recently to have 'isp' as a generic device name for CAMSS IP?
>
> Yeah, will change.
>
>>
>>> + compatible = "qcom,sdm670-camss";
>>> + reg = <0 0x0acb3000 0 0x1000>,
>>> + <0 0x0acba000 0 0x1000>,
>>> + <0 0x0acc8000 0 0x1000>,
>>> + <0 0x0ac65000 0 0x1000>,
>>> + <0 0x0ac66000 0 0x1000>,
>>> + <0 0x0ac67000 0 0x1000>,
>>> + <0 0x0acaf000 0 0x4000>,
>>> + <0 0x0acb6000 0 0x4000>,
>>> + <0 0x0acc4000 0 0x4000>;
>>> + reg-names = "csid0",
>>> + "csid1",
>>> + "csid2",
>>> + "csiphy0",
>>> + "csiphy1",
>>> + "csiphy2",
>>> + "vfe0",
>>> + "vfe1",
>>> + "vfe_lite";
>>> +
>>> + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
>>> + <&camcc CAM_CC_CPAS_AHB_CLK>,
>>> + <&camcc CAM_CC_IFE_0_CSID_CLK>,
>>> + <&camcc CAM_CC_IFE_1_CSID_CLK>,
>>> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
>>> + <&camcc CAM_CC_CSIPHY0_CLK>,
>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>>> + <&camcc CAM_CC_CSIPHY1_CLK>,
>>> + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
>>> + <&camcc CAM_CC_CSIPHY2_CLK>,
>>> + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
>>> + <&gcc GCC_CAMERA_AHB_CLK>,
>>> + <&gcc GCC_CAMERA_AXI_CLK>,
>>> + <&camcc CAM_CC_SOC_AHB_CLK>,
>>> + <&camcc CAM_CC_IFE_0_CLK>,
>>> + <&camcc CAM_CC_IFE_0_AXI_CLK>,
>>> + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
>>> + <&camcc CAM_CC_IFE_1_CLK>,
>>> + <&camcc CAM_CC_IFE_1_AXI_CLK>,
>>> + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
>>> + <&camcc CAM_CC_IFE_LITE_CLK>,
>>> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
>>> + clock-names = "camnoc_axi",
>>> + "cpas_ahb",
>>> + "csi0",
>>> + "csi1",
>>> + "csi2",
>>> + "csiphy0",
>>> + "csiphy0_timer",
>>> + "csiphy1",
>>> + "csiphy1_timer",
>>> + "csiphy2",
>>> + "csiphy2_timer",
>>> + "gcc_camera_ahb",
>>> + "gcc_camera_axi",
>>> + "soc_ahb",
>>> + "vfe0",
>>> + "vfe0_axi",
>>> + "vfe0_cphy_rx",
>>> + "vfe1",
>>> + "vfe1_axi",
>>> + "vfe1_cphy_rx",
>>> + "vfe_lite",
>>> + "vfe_lite_cphy_rx";
>>> +
>>> + interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
>>> + <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
>>> + interrupt-names = "csid0",
>>> + "csid1",
>>> + "csid2",
>>> + "csiphy0",
>>> + "csiphy1",
>>> + "csiphy2",
>>> + "vfe0",
>>> + "vfe1",
>>> + "vfe_lite";
>>> +
>>> + iommus = <&apps_smmu 0x808 0x0>,
>>> + <&apps_smmu 0x810 0x8>,
>>> + <&apps_smmu 0xc08 0x0>,
>>> + <&apps_smmu 0xc10 0x8>;
>>> +
>>> + power-domains = <&camcc IFE_0_GDSC>,
>>> + <&camcc IFE_1_GDSC>,
>>> + <&camcc TITAN_TOP_GDSC>;
>>> + power-domain-names = "ife0",
>>> + "ife1",
>>> + "top";
>>> +
>>> + status = "disabled";
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + camss_port0: port@0 {
>>> + reg = <0>;
>>> + };
>>> +
>>> + camss_port1: port@1 {
>>> + reg = <1>;
>>> + };
>>> +
>>> + camss_port2: port@2 {
>>
>> Likely labels to ports are excessive here, please remove them.
>
> How would you imagine connecting a camera to the ports, then? For MDSS,
> there's a label for the endpoint (mdss_dsi0_out) which the device DTS
> can then reference:
>
> &mdss_dsi0_out {
> remote-endpoint = <&panel_in>;
> data-lanes = <0 1 2 3>;
> };
>
> For CAMSS, the port labels would be used like so:
>
> &camss_port1 {
> camss_endpoint1: endpoint {
> clock-lanes = <7>;
> data-lanes = <0 1 2 3>;
> remote-endpoint = <&cam_front_endpoint>;
> };
> };
>
> Without the labels, the connection might look something like:
>
Even if you insist on moving endpoints out of &camss, then why do
you add port labels? Unavoidably you do have endpoint labels, so
it's non-obvious why the version above is better than the next one:
&camss_endpoint1 {
clock-lanes = <7>;
data-lanes = <0 1 2 3>;
remote-endpoint = <&cam_front_endpoint>;
};
Minus two lines of code, minus one label. Port labels are not needed.
> &camss {
> status = "okay";
>
> // Modification of existing /soc/isp@acb3000/ports node
> ports {
> // Modification of existing /soc/isp@acb3000/ports/port@1 node
> port@1 {
> // New node
> camss_endpoint1: endpoint {
> clock-lanes = <7>;
> data-lanes = <0 1 2 3>;
> remote-endpoint = <&cam_front_endpoint>;
> };
> };
> };
> };
>
> which I believe is not preferred.
Well, that's exactly how it's done right at the moment, in other words
it was preferred every time in the past.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci
2024-12-17 21:34 ` Vladimir Zapolskiy
@ 2024-12-17 23:06 ` Richard Acayan
0 siblings, 0 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-17 23:06 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
linux-arm-msm, linux-clk, devicetree, linux-media
On Tue, Dec 17, 2024 at 11:34:20PM +0200, Vladimir Zapolskiy wrote:
> On 12/17/24 22:25, Richard Acayan wrote:
> > On Tue, Dec 17, 2024 at 09:31:50AM +0200, Vladimir Zapolskiy wrote:
> > > Hi Richard.
> > >
> > > On 12/17/24 00:30, Richard Acayan wrote:
> > > > Add the camera subsystem and CCI used to interface with cameras on the
> > > > Snapdragon 670.
> > > >
> > > > Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> > > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > > ---
> > > > arch/arm64/boot/dts/qcom/sdm670.dtsi | 185 +++++++++++++++++++++++++++
> > > > 1 file changed, 185 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > > > index 328096b91126..d4e1251ada04 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> > > > @@ -6,6 +6,7 @@
> > > > * Copyright (c) 2022, Richard Acayan. All rights reserved.
> > > > */
> > > > +#include <dt-bindings/clock/qcom,camcc-sdm845.h>
> > > > #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
> > > > #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> > > > #include <dt-bindings/clock/qcom,rpmh.h>
> > > > @@ -1168,6 +1169,34 @@ tlmm: pinctrl@3400000 {
> > > > gpio-ranges = <&tlmm 0 0 151>;
> > > > wakeup-parent = <&pdc>;
> > > > + cci0_default: cci0-default-state {
> > > > + pins = "gpio17", "gpio18";
> > > > + function = "cci_i2c";
> > > > + drive-strength = <2>;
> > > > + bias-pull-up;
> > > > + };
> > > > +
> > > > + cci0_sleep: cci0-sleep-state {
> > > > + pins = "gpio17", "gpio18";
> > > > + function = "cci_i2c";
> > > > + drive-strength = <2>;
> > > > + bias-pull-down;
> > > > + };
> > > > +
> > > > + cci1_default: cci1-default-state {
> > > > + pins = "gpio19", "gpio20";
> > > > + function = "cci_i2c";
> > > > + drive-strength = <2>;
> > > > + bias-pull-up;
> > > > + };
> > > > +
> > > > + cci1_sleep: cci1-sleep-state {
> > > > + pins = "gpio19", "gpio20";
> > > > + function = "cci_i2c";
> > > > + drive-strength = <2>;
> > > > + bias-pull-down;
> > > > + };
> > > > +
> > > > qup_i2c0_default: qup-i2c0-default-state {
> > > > pins = "gpio0", "gpio1";
> > > > function = "qup0";
> > > > @@ -1400,6 +1429,162 @@ spmi_bus: spmi@c440000 {
> > > > #interrupt-cells = <4>;
> > > > };
> > > > + cci: cci@ac4a000 {
> > > > + compatible = "qcom,sdm670-cci", "qcom,msm8996-cci";
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + reg = <0 0x0ac4a000 0 0x4000>;
> > > > + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> > > > + power-domains = <&camcc TITAN_TOP_GDSC>;
> > > > +
> > > > + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> > > > + <&camcc CAM_CC_SOC_AHB_CLK>,
> > > > + <&camcc CAM_CC_CPAS_AHB_CLK>,
> > > > + <&camcc CAM_CC_CCI_CLK>;
> > > > + clock-names = "camnoc_axi",
> > > > + "soc_ahb",
> > > > + "cpas_ahb",
> > > > + "cci";
> > > > +
> > > > + pinctrl-names = "default", "sleep";
> > > > + pinctrl-0 = <&cci0_default &cci1_default>;
> > > > + pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> > > > +
> > > > + status = "disabled";
> > > > +
> > > > + cci_i2c0: i2c-bus@0 {
> > > > + reg = <0>;
> > > > + clock-frequency = <1000000>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > +
> > > > + cci_i2c1: i2c-bus@1 {
> > > > + reg = <1>;
> > > > + clock-frequency = <1000000>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + };
> > > > +
> > > > + camss: camera-controller@acb3000 {
> > >
> > > Wasn't it agreed recently to have 'isp' as a generic device name for CAMSS IP?
> >
> > Yeah, will change.
> >
> > >
> > > > + compatible = "qcom,sdm670-camss";
> > > > + reg = <0 0x0acb3000 0 0x1000>,
> > > > + <0 0x0acba000 0 0x1000>,
> > > > + <0 0x0acc8000 0 0x1000>,
> > > > + <0 0x0ac65000 0 0x1000>,
> > > > + <0 0x0ac66000 0 0x1000>,
> > > > + <0 0x0ac67000 0 0x1000>,
> > > > + <0 0x0acaf000 0 0x4000>,
> > > > + <0 0x0acb6000 0 0x4000>,
> > > > + <0 0x0acc4000 0 0x4000>;
> > > > + reg-names = "csid0",
> > > > + "csid1",
> > > > + "csid2",
> > > > + "csiphy0",
> > > > + "csiphy1",
> > > > + "csiphy2",
> > > > + "vfe0",
> > > > + "vfe1",
> > > > + "vfe_lite";
> > > > +
> > > > + clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> > > > + <&camcc CAM_CC_CPAS_AHB_CLK>,
> > > > + <&camcc CAM_CC_IFE_0_CSID_CLK>,
> > > > + <&camcc CAM_CC_IFE_1_CSID_CLK>,
> > > > + <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
> > > > + <&camcc CAM_CC_CSIPHY0_CLK>,
> > > > + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> > > > + <&camcc CAM_CC_CSIPHY1_CLK>,
> > > > + <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> > > > + <&camcc CAM_CC_CSIPHY2_CLK>,
> > > > + <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> > > > + <&gcc GCC_CAMERA_AHB_CLK>,
> > > > + <&gcc GCC_CAMERA_AXI_CLK>,
> > > > + <&camcc CAM_CC_SOC_AHB_CLK>,
> > > > + <&camcc CAM_CC_IFE_0_CLK>,
> > > > + <&camcc CAM_CC_IFE_0_AXI_CLK>,
> > > > + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> > > > + <&camcc CAM_CC_IFE_1_CLK>,
> > > > + <&camcc CAM_CC_IFE_1_AXI_CLK>,
> > > > + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> > > > + <&camcc CAM_CC_IFE_LITE_CLK>,
> > > > + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>;
> > > > + clock-names = "camnoc_axi",
> > > > + "cpas_ahb",
> > > > + "csi0",
> > > > + "csi1",
> > > > + "csi2",
> > > > + "csiphy0",
> > > > + "csiphy0_timer",
> > > > + "csiphy1",
> > > > + "csiphy1_timer",
> > > > + "csiphy2",
> > > > + "csiphy2_timer",
> > > > + "gcc_camera_ahb",
> > > > + "gcc_camera_axi",
> > > > + "soc_ahb",
> > > > + "vfe0",
> > > > + "vfe0_axi",
> > > > + "vfe0_cphy_rx",
> > > > + "vfe1",
> > > > + "vfe1_axi",
> > > > + "vfe1_cphy_rx",
> > > > + "vfe_lite",
> > > > + "vfe_lite_cphy_rx";
> > > > +
> > > > + interrupts = <GIC_SPI 464 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 466 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
> > > > + <GIC_SPI 469 IRQ_TYPE_EDGE_RISING>;
> > > > + interrupt-names = "csid0",
> > > > + "csid1",
> > > > + "csid2",
> > > > + "csiphy0",
> > > > + "csiphy1",
> > > > + "csiphy2",
> > > > + "vfe0",
> > > > + "vfe1",
> > > > + "vfe_lite";
> > > > +
> > > > + iommus = <&apps_smmu 0x808 0x0>,
> > > > + <&apps_smmu 0x810 0x8>,
> > > > + <&apps_smmu 0xc08 0x0>,
> > > > + <&apps_smmu 0xc10 0x8>;
> > > > +
> > > > + power-domains = <&camcc IFE_0_GDSC>,
> > > > + <&camcc IFE_1_GDSC>,
> > > > + <&camcc TITAN_TOP_GDSC>;
> > > > + power-domain-names = "ife0",
> > > > + "ife1",
> > > > + "top";
> > > > +
> > > > + status = "disabled";
> > > > +
> > > > + ports {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + camss_port0: port@0 {
> > > > + reg = <0>;
> > > > + };
> > > > +
> > > > + camss_port1: port@1 {
> > > > + reg = <1>;
> > > > + };
> > > > +
> > > > + camss_port2: port@2 {
> > >
> > > Likely labels to ports are excessive here, please remove them.
> >
> > How would you imagine connecting a camera to the ports, then? For MDSS,
> > there's a label for the endpoint (mdss_dsi0_out) which the device DTS
> > can then reference:
> >
> > &mdss_dsi0_out {
> > remote-endpoint = <&panel_in>;
> > data-lanes = <0 1 2 3>;
> > };
> >
> > For CAMSS, the port labels would be used like so:
> >
> > &camss_port1 {
> > camss_endpoint1: endpoint {
> > clock-lanes = <7>;
> > data-lanes = <0 1 2 3>;
> > remote-endpoint = <&cam_front_endpoint>;
> > };
> > };
> >
> > Without the labels, the connection might look something like:
> >
>
> Even if you insist on moving endpoints out of &camss, then why do
> you add port labels? Unavoidably you do have endpoint labels, so
> it's non-obvious why the version above is better than the next one:
>
> &camss_endpoint1 {
> clock-lanes = <7>;
> data-lanes = <0 1 2 3>;
> remote-endpoint = <&cam_front_endpoint>;
> };
>
> Minus two lines of code, minus one label. Port labels are not needed.
Thanks for having me look into this.
In DTSI:
ports {
#address-cells = <1>;
#size-cells = <0>;
camss_port0: port@0 {
reg = <0>;
camss_endpoint0: endpoint {
};
};
camss_port1: port@1 {
reg = <1>;
camss_endpoint1: endpoint {
};
};
camss_port2: port@2 {
reg = <2>;
camss_endpoint2: endpoint {
};
};
};
The example above doesn't work as-is.
[ 15.387215] qcom-camss acb3000.camera-controller: Cannot get remote parent
[ 15.387604] qcom-camss acb3000.camera-controller: probe with driver qcom-camss failed with error -22
However, the camss_of_parse_ports function has a way to make it work,
even if all endpoint nodes are present.
In DTSI:
ports {
#address-cells = <1>;
#size-cells = <0>;
camss_port0: port@0 {
reg = <0>;
camss_endpoint0: endpoint {
status = "disabled";
};
};
camss_port1: port@1 {
reg = <1>;
camss_endpoint1: endpoint {
status = "disabled";
};
};
camss_port2: port@2 {
reg = <2>;
camss_endpoint2: endpoint {
status = "disabled";
};
};
};
In DTS:
&camss_endpoint1 {
clock-lanes = <7>;
data-lanes = <0 1 2 3>;
remote-endpoint = <&cam_front_endpoint>;
status = "okay";
};
I didn't know how to make the working example.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss
2024-12-16 22:41 ` Bryan O'Donoghue
@ 2024-12-17 23:20 ` Richard Acayan
0 siblings, 0 replies; 15+ messages in thread
From: Richard Acayan @ 2024-12-17 23:20 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-media, Vladimir Zapolskiy
On Mon, Dec 16, 2024 at 10:41:52PM +0000, Bryan O'Donoghue wrote:
> On 16/12/2024 22:30, Richard Acayan wrote:
> > .../bindings/media/qcom,sdm670-camss.yaml | 72 +++----
> > drivers/media/platform/qcom/camss/camss.c | 191 ++++++++++++++++++
> > 2 files changed, 227 insertions(+), 36 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> > index f8701a8d27fe..892412fb68a9 100644
> > --- a/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> > +++ b/Documentation/devicetree/bindings/media/qcom,sdm670-camss.yaml
> > @@ -31,21 +31,6 @@ properties:
> > - const: vfe1
> > - const: vfe_lite
> > - interrupts:
> > - maxItems: 9
>
> I think you've had an error squashing some patches.
Yeah, copy-paste error... best to revert the change (and change back DTS).
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-12-17 23:20 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-16 22:30 [PATCH v8 0/5] Add SDM670 camera subsystem Richard Acayan
2024-12-16 22:30 ` [PATCH v8 1/5] dt-bindings: clock: qcom,sdm845-camcc: add sdm670 compatible Richard Acayan
2024-12-16 22:30 ` [PATCH v8 2/5] dt-bindings: media: camss: Add qcom,sdm670-camss Richard Acayan
2024-12-16 22:47 ` Bryan O'Donoghue
2024-12-17 6:20 ` Krzysztof Kozlowski
2024-12-16 22:30 ` [PATCH v8 3/5] media: qcom: camss: add support for SDM670 camss Richard Acayan
2024-12-16 22:41 ` Bryan O'Donoghue
2024-12-17 23:20 ` Richard Acayan
2024-12-17 6:21 ` Krzysztof Kozlowski
2024-12-16 22:30 ` [PATCH v8 4/5] arm64: dts: qcom: sdm670: add camcc Richard Acayan
2024-12-16 22:30 ` [PATCH v8 5/5] arm64: dts: qcom: sdm670: add camss and cci Richard Acayan
2024-12-17 7:31 ` Vladimir Zapolskiy
2024-12-17 20:25 ` Richard Acayan
2024-12-17 21:34 ` Vladimir Zapolskiy
2024-12-17 23:06 ` Richard Acayan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).