* [PATCH 1/9] dt-bindings: media: add the Qualcomm SM8750 camera subsystem
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
@ 2026-10-28 3:40 ` Qianhui Xu
2026-10-28 3:52 ` sashiko-bot
2026-10-28 3:40 ` [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750 Qianhui Xu
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:40 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
SM8750 has six CSIPHY instances, two full and two lite CSIDs, and two
full plus one lite VFEs behind one camera subsystem. Document the node
with the register ranges, clocks, interconnects, power domains and the
sub-block layout of that SoC.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../bindings/media/qcom,sm8750-camss.yaml | 435 ++++++++++++++++++
1 file changed, 435 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
new file mode 100644
index 000000000..e0e0722a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
@@ -0,0 +1,435 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,sm8750-camss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8750 Camera Subsystem (CAMSS)
+
+maintainers:
+ - Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
+
+description:
+ The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
+
+properties:
+ compatible:
+ const: qcom,sm8750-camss
+
+ dma-coherent: true
+
+ reg:
+ maxItems: 19
+
+ reg-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_lite0
+ - const: csid_lite1
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy3
+ - const: csiphy4
+ - const: csiphy5
+ - const: csitpg0
+ - const: csitpg1
+ - const: csitpg2
+ - const: vfe0
+ - const: vfe1
+ - const: vfe2
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ clocks:
+ maxItems: 35
+
+ clock-names:
+ items:
+ - const: camnoc_nrt_axi
+ - const: camnoc_rt_axi
+ - const: cpas_ahb
+ - const: cpas_fast_ahb
+ - const: cpas_vfe0
+ - const: cpas_vfe1
+ - const: cpas_vfe2
+ - const: cpas_vfe_lite
+ - const: csid
+ - const: csid_csiphy_rx
+ - const: csiphy0
+ - const: csiphy0_timer
+ - const: csiphy1
+ - const: csiphy1_timer
+ - const: csiphy2
+ - const: csiphy2_timer
+ - const: csiphy3
+ - const: csiphy3_timer
+ - const: csiphy4
+ - const: csiphy4_timer
+ - const: csiphy5
+ - const: csiphy5_timer
+ - const: gcc_axi_hf
+ - const: gcc_axi_sf
+ - const: vfe0
+ - const: vfe0_fast_ahb
+ - const: vfe1
+ - const: vfe1_fast_ahb
+ - const: vfe2
+ - const: vfe2_fast_ahb
+ - const: vfe_lite
+ - const: vfe_lite_ahb
+ - const: vfe_lite_cphy_rx
+ - const: vfe_lite_csid
+ - const: qdss_debug_xo
+
+ interrupts:
+ maxItems: 16
+
+ interrupt-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_lite0
+ - const: csid_lite1
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy3
+ - const: csiphy4
+ - const: csiphy5
+ - const: vfe0
+ - const: vfe1
+ - const: vfe2
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ interconnects:
+ maxItems: 4
+
+ interconnect-names:
+ items:
+ - const: ahb
+ - const: hf_mnoc
+ - const: sf_mnoc
+ - const: sf_icp_mnoc
+
+ iommus:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description:
+ IFE0 GDSC - Global Distributed Switch Controller for IFE0.
+ - description:
+ IFE1 GDSC - Global Distributed Switch Controller for IFE1.
+ - description:
+ IFE2 GDSC - Global Distributed Switch Controller for IFE2.
+ - description:
+ Titan GDSC - Global Distributed Switch Controller for the entire camss.
+
+ power-domain-names:
+ items:
+ - const: ife0
+ - const: ife1
+ - const: ife2
+ - const: top
+
+ vdd-csiphy0-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY0 core block.
+
+ vdd-csiphy0-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY0 pll block.
+
+ vdd-csiphy1-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY1 core block.
+
+ vdd-csiphy1-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY1 pll block.
+
+ vdd-csiphy2-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY2 core block.
+
+ vdd-csiphy2-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY2 pll block.
+
+ vdd-csiphy3-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY3 core block.
+
+ vdd-csiphy3-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY3 pll block.
+
+ vdd-csiphy4-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY4 core block.
+
+ vdd-csiphy4-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY4 pll block.
+
+ vdd-csiphy5-0p9-supply:
+ description:
+ Phandle to a 0.9V regulator supply to CSIPHY5 core block.
+
+ vdd-csiphy5-1p2-supply:
+ description:
+ Phandle to a 1.2V regulator supply to CSIPHY5 pll block.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports.
+
+ patternProperties:
+ "^port@[0-5]$":
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input ports for receiving CSI data on CSIPHY 0-5.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ bus-type:
+ enum:
+ - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
+ - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
+
+ required:
+ - data-lanes
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - interconnects
+ - interconnect-names
+ - iommus
+ - power-domains
+ - power-domain-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,sm8750-gcc.h>
+ #include <dt-bindings/clock/qcom,sm8750-camcc.h>
+ #include <dt-bindings/interconnect/qcom,icc.h>
+ #include <dt-bindings/interconnect/qcom,sm8750-rpmh.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ isp@ad27000 {
+ compatible = "qcom,sm8750-camss";
+
+ reg = <0x0 0x0ad27000 0x0 0x2b00>,
+ <0x0 0x0ad2a000 0x0 0x2b00>,
+ <0x0 0x0ad2d000 0x0 0x2b00>,
+ <0x0 0x0ad6d000 0x0 0xa00>,
+ <0x0 0x0ad72000 0x0 0xa00>,
+ <0x0 0x0ada9000 0x0 0x2000>,
+ <0x0 0x0adab000 0x0 0x2000>,
+ <0x0 0x0adad000 0x0 0x2000>,
+ <0x0 0x0adaf000 0x0 0x2000>,
+ <0x0 0x0adb1000 0x0 0x2000>,
+ <0x0 0x0adb3000 0x0 0x2000>,
+ <0x0 0x0ad8b000 0x0 0x400>,
+ <0x0 0x0ad8c000 0x0 0x400>,
+ <0x0 0x0ad8d000 0x0 0x400>,
+ <0x0 0x0ac86000 0x0 0x10000>,
+ <0x0 0x0ac96000 0x0 0x10000>,
+ <0x0 0x0aca6000 0x0 0x10000>,
+ <0x0 0x0ad6e000 0x0 0x3000>,
+ <0x0 0x0ad73000 0x0 0x3000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "csitpg0",
+ "csitpg1",
+ "csitpg2",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
+ <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_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>,
+ <&camcc CAM_CC_CSIPHY3_CLK>,
+ <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY5_CLK>,
+ <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>;
+ clock-names = "camnoc_nrt_axi",
+ "camnoc_rt_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe2",
+ "cpas_vfe_lite",
+ "csid",
+ "csid_csiphy_rx",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy3",
+ "csiphy3_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "csiphy5",
+ "csiphy5_timer",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe2",
+ "vfe2_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid",
+ "qdss_debug_xo";
+
+ interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 376 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 448 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+ <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_NRT_ICP_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "ahb",
+ "hf_mnoc",
+ "sf_mnoc",
+ "sf_icp_mnoc";
+
+ iommus = <&apps_smmu 0x1c00 0x00>;
+
+ power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
+ <&camcc CAM_CC_TFE_1_GDSC>,
+ <&camcc CAM_CC_TFE_2_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "ife2",
+ "top";
+
+ vdd-csiphy0-0p9-supply = <&vreg_0p9_supply>;
+ vdd-csiphy0-1p2-supply = <&vreg_1p2_supply>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ csiphy_ep0: endpoint {
+ data-lanes = <0 1>;
+ remote-endpoint = <&sensor_ep>;
+ };
+ };
+ };
+ };
+ };
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/9] dt-bindings: media: add the Qualcomm SM8750 camera subsystem
2026-10-28 3:40 ` [PATCH 1/9] dt-bindings: media: add the Qualcomm SM8750 camera subsystem Qianhui Xu
@ 2026-10-28 3:52 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-28 3:52 UTC (permalink / raw)
To: Qianhui Xu; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: media: commit message sub-block counts mismatch binding schema
--
commit 73bfcb41d44e2c6c144a357d1065fa300a499b3f
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
dt-bindings: media: add the Qualcomm SM8750 camera subsystem
This commit adds the device tree binding schema for the Qualcomm SM8750
camera subsystem (CAMSS).
> dt-bindings: media: add the Qualcomm SM8750 camera subsystem
>
> SM8750 has six CSIPHY instances, two full and two lite CSIDs, and two
> full plus one lite VFEs behind one camera subsystem. Document the node
> with the register ranges, clocks, interconnects, power domains and the
> sub-block layout of that SoC.
>
> Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
> new file mode 100644
> index 0000000000000..e0e0722a838c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml
> @@ -0,0 +1,435 @@
[ ... ]
> + reg:
> + maxItems: 19
> +
> + reg-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csid_lite0
> + - const: csid_lite1
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: csiphy3
> + - const: csiphy4
> + - const: csiphy5
> + - const: csitpg0
> + - const: csitpg1
> + - const: csitpg2
> + - const: vfe0
> + - const: vfe1
> + - const: vfe2
> + - const: vfe_lite0
> + - const: vfe_lite1
[Severity: Medium]
Are the sub-block instance counts in the commit message accurate?
The commit message describes SM8750 as having "two full and two lite
CSIDs, and two full plus one lite VFEs".
However, the binding schema in
Documentation/devicetree/bindings/media/qcom,sm8750-camss.yaml defines
and requires:
- three full CSIDs (csid0, csid1, csid2) and two lite CSIDs (csid_lite0,
csid_lite1) in reg-names and interrupt-names
- three full VFEs (vfe0, vfe1, vfe2) and two lite VFEs (vfe_lite0,
vfe_lite1) across reg-names, clock-names (cpas_vfe0..2, vfe0..2, and
vfe0..2_fast_ahb), interrupt-names, power-domains, and
power-domain-names (ife0..2)
- three CSITPG instances (csitpg0, csitpg1, csitpg2) in reg-names
Should the commit message be updated to match the three full CSIDs and
three full plus two lite VFEs defined in the schema?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028034105.326535-1-xiaqinghuan@mail.ustc.edu.cn?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
2026-10-28 3:40 ` [PATCH 1/9] dt-bindings: media: add the Qualcomm SM8750 camera subsystem Qianhui Xu
@ 2026-10-28 3:40 ` Qianhui Xu
2026-10-29 7:53 ` Shuai Liu
2026-10-28 3:40 ` [PATCH 3/9] media: camss: csid: unwind the power-on path through a single error label Qianhui Xu
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:40 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
SM8750 uses the CSIPHY 2.4.0 CSI PHY. The block keeps the register layout
already handled by csiphy_ops_3ph_1_0 (four data lanes at a 0x400 stride,
clock lane at 0xc00, common block at +0x1000), so the generation-2 lane
programming sequence applies unchanged, but the per-lane values differ from
the 2.3.0 block used on SM8550/SM8650:
- the lane register 0x0000 default is 0x8c instead of 0x8e,
- every lane gains the four SKEW_CAL registers
(0x0094/0x005c/0x0060/0x0064),
- the common status block sits at 0x138 instead of 0xb0.
Add lane_regs_sm8750[] with those values, report CAMSS_8750 as a
generation-2 PHY, and select the new table together with the 0x138
common status offset in csiphy_init().
The register map is the one the CSIPHY 2.4.0 vendor driver uses for its
two-phase variant (csiphy_2ph_v2_4_0_reg, 103 entries): address, mask and
parameter type are identical for all 103 entries, and the data byte is
identical for 99. The four remaining entries are the SKEW_CAL data bytes
of the four lanes (0x005c and its per-lane copies), which are 0x00 here,
i.e. the vendor value. This SoC wires the PHY in two-phase mode; the
three-phase variant of the same PHY (csiphy_3ph_v2_4_0_reg, 72 entries)
uses a different address layout, so its table cannot be reused. The
0x138 common status offset is where the vendor parameter block of this
generation points its status register window (0x1000 + 0x138).
These settings are the ones the CSIPHY runs with on a Lenovo TB322FC
(SM8750), where both the rear s5kjns and the front gc08a8 sensor stream
at full resolution and 30 fps. Enabling the SKEW_CAL data byte of the
four lanes (0x005c = 0x54) instead of leaving it at the vendor value was
also measured on that device: 90 frames with each setting, no error
reported, 30.26 fps and 53.81 vs 53.78 frame time deviation, i.e. no
measurable difference, so the vendor value is kept.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../qcom/camss/camss-csiphy-3ph-1-0.c | 124 ++++++++++++++++++
drivers/media/platform/qcom/camss/camss.h | 1 +
2 files changed, 125 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index dac8d2ecf..9cf22ff17 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -804,6 +804,123 @@ csiphy_lane_regs lane_regs_sm8650[] = {
{0x0c10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
};
+/* GEN2 2.4.0 2PH 4 lane DPHY mode */
+static const struct
+csiphy_lane_regs lane_regs_sm8750[] = {
+ /* LN 0 */
+ {0x0094, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0090, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0094, 0x07, 0xD1, CSIPHY_DEFAULT_PARAMS},
+ {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0000, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x001C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0020, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0008, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+ {0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+ {0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+ /* LN 2 */
+ {0x0494, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x04A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0490, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0498, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0494, 0x07, 0xD1, CSIPHY_DEFAULT_PARAMS},
+ {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0400, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0434, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x041C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0420, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0408, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0494, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+ {0x045C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x0460, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+ {0x0464, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+ /* LN 4 */
+ {0x0894, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x08A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0890, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0898, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0894, 0x07, 0xD1, CSIPHY_DEFAULT_PARAMS},
+ {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0800, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0838, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x082C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0834, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x081C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0814, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x083C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0804, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0820, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0808, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0894, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+ {0x085C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x0860, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+ {0x0864, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+ /* LN 6 */
+ {0x0C94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0CA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C94, 0x07, 0xD1, CSIPHY_DEFAULT_PARAMS},
+ {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C00, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C38, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C94, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+ {0x0C5C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x0C60, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+ {0x0C64, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+ /* LN CLK */
+ {0x0E94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0EA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E94, 0x07, 0xD1, CSIPHY_DEFAULT_PARAMS},
+ {0x0E30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E28, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E00, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E0C, 0xFF, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E38, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+};
+
/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
static const struct
csiphy_lane_regs lane_regs_x1e80100[] = {
@@ -1139,6 +1256,7 @@ static bool csiphy_is_gen2(u32 version)
case CAMSS_845:
case CAMSS_8550:
case CAMSS_8650:
+ case CAMSS_8750:
case CAMSS_8775P:
case CAMSS_X1E80100:
ret = true;
@@ -1259,6 +1377,12 @@ static int csiphy_init(struct csiphy_device *csiphy)
regs->lane_regs = &lane_regs_sa8775p[0];
regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
break;
+ case CAMSS_8750:
+ regs->lane_regs = &lane_regs_sm8750[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8750);
+ regs->offset = 0x1000;
+ regs->common_status_offset = 0x138;
+ break;
default:
break;
}
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 93d691c8a..ef45eea00 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -95,6 +95,7 @@ enum camss_version {
CAMSS_845,
CAMSS_8550,
CAMSS_8650,
+ CAMSS_8750,
CAMSS_8775P,
CAMSS_X1E80100,
};
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750
2026-10-28 3:40 ` [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750 Qianhui Xu
@ 2026-10-29 7:53 ` Shuai Liu
0 siblings, 0 replies; 20+ messages in thread
From: Shuai Liu @ 2026-10-29 7:53 UTC (permalink / raw)
To: linux-media, linux-arm-msm
Cc: Shuai Liu, Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
Hangxiang Ma, Qianhui Xu, devicetree, linux-kernel
Hi Qianhui,
Reviewing the series (built for x86_64 with COMPILE_TEST, checkpatch
clean, no functional testing - no hardware here).
> The register map is the one the CSIPHY 2.4.0 vendor driver uses for its
> two-phase variant (csiphy_2ph_v2_4_0_reg, 103 entries): address, mask
> and parameter type are identical for all 103 entries, and the data byte
> is identical for 99. The four remaining entries are the SKEW_CAL data
> bytes of the four lanes (0x005c and its per-lane copies), which are
> 0x00 here, i.e. the vendor value.
The commit message and the table contradict each other. The four
SKEW_CAL entries added by this patch program 0x54:
{0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
{0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
{0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
{0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},
while the commit message states twice that the 0x005c data byte is
"0x00 here, i.e. the vendor value" and that "the vendor value is kept".
As posted the correction does exist, but it is buried in patch 4/9,
which flips exactly these four entries back to 0x00 - so the final tree
matches the prose, but patch 2 as applied does not, and readers of
patch 2 alone cannot tell which value is intended.
Please make patch 2 self-consistent: if the vendor value (0x00) is what
ships, program it directly here; if 0x54 is intended, fix the commit
message instead. Either way the csiphy hunk in patch 4/9 should go
away (see my reply there).
Thanks,
Shuai Liu
(This review was prepared with the assistance of an AI coding tool;
all findings were verified against the patched source.)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/9] media: camss: csid: unwind the power-on path through a single error label
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
2026-10-28 3:40 ` [PATCH 1/9] dt-bindings: media: add the Qualcomm SM8750 camera subsystem Qianhui Xu
2026-10-28 3:40 ` [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750 Qianhui Xu
@ 2026-10-28 3:40 ` Qianhui Xu
2026-10-28 3:41 ` [PATCH 4/9] media: camss: csid: add the 980 generation Qianhui Xu
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:40 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
csid_set_power() repeats the same cleanup sequence in every error branch
and gets it wrong in the first one: when pm_runtime_resume_and_get()
fails, the parent device reference taken by parent_dev_ops->get() is
never dropped, so a failing power-on leaks it and the matching VFE stays
powered.
Replace the four open-coded unwind blocks with gotos to a shared label
chain and route the pm_runtime failure through the parent put as well.
No functional change on the success path.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../media/platform/qcom/camss/camss-csid.c | 45 +++++++++----------
1 file changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 48459b46a..592c1dfc9 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -696,44 +696,28 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- return ret;
+ goto put_parent;
ret = regulator_bulk_enable(csid->num_supplies,
csid->supplies);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
- return ret;
- }
+ if (ret < 0)
+ goto put_pm;
ret = csid_set_clock_rates(csid);
- if (ret < 0) {
- regulator_bulk_disable(csid->num_supplies,
- csid->supplies);
- pm_runtime_put_sync(dev);
- return ret;
- }
+ if (ret < 0)
+ goto disable_regulators;
ret = camss_enable_clocks(csid->nclocks, csid->clock, dev);
- if (ret < 0) {
- regulator_bulk_disable(csid->num_supplies,
- csid->supplies);
- pm_runtime_put_sync(dev);
- return ret;
- }
+ if (ret < 0)
+ goto disable_regulators;
csid->phy.need_vc_update = true;
enable_irq(csid->irq);
ret = csid->res->hw_ops->reset(csid);
- if (ret < 0) {
- disable_irq(csid->irq);
- camss_disable_clocks(csid->nclocks, csid->clock);
- regulator_bulk_disable(csid->num_supplies,
- csid->supplies);
- pm_runtime_put_sync(dev);
- return ret;
- }
+ if (ret < 0)
+ goto disable_clocks;
csid->res->hw_ops->hw_version(csid);
} else {
@@ -746,6 +730,17 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
}
return ret;
+
+disable_clocks:
+ disable_irq(csid->irq);
+ camss_disable_clocks(csid->nclocks, csid->clock);
+disable_regulators:
+ regulator_bulk_disable(csid->num_supplies, csid->supplies);
+put_pm:
+ pm_runtime_put_sync(dev);
+put_parent:
+ csid->res->parent_dev_ops->put(camss, csid->id);
+ return ret;
}
/*
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/9] media: camss: csid: add the 980 generation
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (2 preceding siblings ...)
2026-10-28 3:40 ` [PATCH 3/9] media: camss: csid: unwind the power-on path through a single error label Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 4:03 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
2026-10-28 3:41 ` [PATCH 5/9] media: camss: csid: report frame starts from CSID Qianhui Xu
` (5 subsequent siblings)
9 siblings, 2 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
The CSID found on SM8750 (call it generation 980) keeps the capture
interrupt reporting of the 680 generation, but moves the register update
completion to per-stream interrupt bits and adds separate start-of-frame
and end-of-frame timestamp registers per RDI stream.
Add camss-csid-980.c with the register maps for both the full and the
lite variants. The full variant splits the register update into separate
RUP and AUP commands that need an extra trigger write, while the lite
variant keeps the combined RUP/AUP command of the previous generations,
so struct csid_device gains dedicated rup_update and aup_update fields
next to the existing combined reg_update.
Only the register map, the RDI stream configuration and the buffer-done
reporting are described here; frame start reporting is added on top of it
by a separate change.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../platform/qcom/camss/camss-csid-980.c | 443 ++++++++++++++++++
.../media/platform/qcom/camss/camss-csid.h | 3 +
.../qcom/camss/camss-csiphy-3ph-1-0.c | 8 +-
drivers/media/platform/qcom/camss/camss-vfe.c | 15 +-
drivers/media/platform/qcom/camss/camss-vfe.h | 2 +
6 files changed, 464 insertions(+), 8 deletions(-)
create mode 100644 drivers/media/platform/qcom/camss/camss-csid-980.c
diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index 27898b3cc..348da1ccc 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -8,6 +8,7 @@ qcom-camss-objs += \
camss-csid-4-7.o \
camss-csid-340.o \
camss-csid-680.o \
+ camss-csid-980.o \
camss-csid-gen2.o \
camss-csid-gen3.o \
camss-csiphy.o \
diff --git a/drivers/media/platform/qcom/camss/camss-csid-980.c b/drivers/media/platform/qcom/camss/camss-csid-980.c
new file mode 100644
index 000000000..45f3b3786
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csid-980.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-csid-980.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include "camss.h"
+#include "camss-csid.h"
+#include "camss-csid-gen3.h"
+
+/* Reset and Command Registers */
+#define CSID_RST_CFG 0xC
+#define RST_MODE BIT(0)
+#define RST_LOCATION BIT(4)
+
+/* Reset and Command Registers */
+#define CSID_RST_CMD 0x10
+#define SELECT_HW_RST BIT(0)
+#define SELECT_IRQ_RST BIT(2)
+#define CSID_IRQ_CMD 0x14
+#define IRQ_CMD_CLEAR BIT(0)
+
+/* Register Update Commands, RUP/AUP */
+#define CSID_RUP_CMD 0x18
+#define CSID_AUP_CMD 0x1C
+#define CSID_RUP_AUP_RDI(rdi) (BIT(8) << (rdi))
+#define CSID_RUP_AUP_CMD 0x20
+#define RUP_SET BIT(0)
+
+#define CSID_LITE_RUP_AUP_CMD 0x18
+#define CSID_LITE_RUP_RDI(rdi) (BIT(4) << (rdi))
+#define CSID_LITE_AUP_RDI(rdi) (BIT(20) << (rdi))
+
+/* Top level interrupt registers */
+#define CSID_TOP_IRQ_STATUS (csid_is_lite(csid) ? 0x7C : 0x84)
+#define CSID_TOP_IRQ_MASK (csid_is_lite(csid) ? 0x80 : 0x88)
+#define CSID_TOP_IRQ_CLEAR (csid_is_lite(csid) ? 0x84 : 0x8C)
+#define INFO_RST_DONE BIT(0)
+#define BUF_DONE_IRQ_STATUS BIT(csid_is_lite(csid) ? 13 : 3)
+
+/* Buffer done interrupt registers */
+#define CSID_BUF_DONE_IRQ_STATUS (csid_is_lite(csid) ? 0x8C : 0xA4)
+#define BUF_DONE_IRQ_STATUS_RDI_OFFSET (csid_is_lite(csid) ? 1 : 16)
+#define CSID_BUF_DONE_IRQ_MASK (csid_is_lite(csid) ? 0x90 : 0xA8)
+#define CSID_BUF_DONE_IRQ_CLEAR (csid_is_lite(csid) ? 0x94 : 0xAC)
+
+/* CSI2 RX Configuration */
+#define CSID_CSI2_RX_CFG0 (csid_is_lite(csid) ? 0x200 : 0x400)
+#define CSI2_RX_CFG0_NUM_ACTIVE_LANES 0
+#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
+#define CSI2_RX_CFG0_DL0_INPUT_SEL 4
+#define CSI2_RX_CFG0_PHY_NUM_SEL 20
+#define CSID_CSI2_RX_CFG1 (csid_is_lite(csid) ? 0x204 : 0x404)
+#define CSI2_RX_CFG1_ECC_CORRECTION_EN BIT(0)
+#define CSI2_RX_CFG1_VC_MODE BIT(2)
+
+#define MSM_CSID_MAX_SRC_STREAMS_980 (csid_is_lite(csid) ? 4 : 5)
+
+#define CSID_RDI_CFG0(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x500 + 0x100 * _rdi : \
+ 0xE00 + 0x200 * _rdi; \
+ })
+#define RDI_CFG0_RETIME_BS BIT(5)
+#define RDI_CFG0_TIMESTAMP_EN BIT(6)
+#define RDI_CFG0_TIMESTAMP_STB_SEL BIT(8)
+#define RDI_CFG0_DECODE_FORMAT 12
+#define RDI_CFG0_DT 16
+#define RDI_CFG0_VC 22
+#define RDI_CFG0_DT_ID 27
+#define RDI_CFG0_EN BIT(31)
+
+/* RDI Control and Configuration */
+#define CSID_RDI_CTRL(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x504 + 0x100 * _rdi : \
+ 0xE04 + 0x200 * _rdi; \
+ })
+#define RDI_CTRL_START_CMD BIT(0)
+
+#define CSID_RDI_CFG1(rdi) \
+ ({ \
+ __typeof__(rdi) _rdi = (rdi); \
+ csid_is_lite(csid) ? 0x510 + 0x100 * _rdi : \
+ 0xE10 + 0x200 * _rdi; \
+ })
+#define RDI_CFG1_PACKING_FORMAT_MIPI BIT(15)
+
+/* RDI Pixel Store Configuration */
+#define CSID_RDI_PIX_STORE_CFG0(rdi) (0xE14 + 0x200 * (rdi))
+#define RDI_PIX_STORE_CFG0_EN BIT(0)
+#define RDI_PIX_STORE_CFG0_MIN_HBI 1
+
+/* RDI IRQ Status in wrapper */
+#define CSID_CSI2_RDIN_IRQ_STATUS(rdi) \
+ ((csid_is_lite(csid) ? 0xEC : 0x114) + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_IRQ_MASK(rdi) \
+ ((csid_is_lite(csid) ? 0xF0 : 0x118) + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi) \
+ ((csid_is_lite(csid) ? 0xF4 : 0x11C) + 0x10 * (rdi))
+#define INFO_RUP_DONE BIT(23)
+#define CSID_RDI_TOP_IRQ(rdi) BIT((csid_is_lite(csid) ? 8 : 16) + (rdi))
+#define CSID_RDI_IRQ_PATTERN(rdi) \
+ ((csid_is_lite(csid) ? 0x548 : 0xE50) + (csid_is_lite(csid) ? 0x100 : 0x200) * (rdi))
+#define CSID_RDI_IRQ_PERIOD(rdi) (CSID_RDI_IRQ_PATTERN(rdi) + 4)
+#define CSID_RDI_SOF_TS(rdi) \
+ ((csid_is_lite(csid) ? 0x594 : 0xEA0) + (csid_is_lite(csid) ? 0x100 : 0x200) * (rdi))
+#define CSID_RDI_EOF_TS(rdi) (CSID_RDI_SOF_TS(rdi) + 0x10)
+
+static void __csid_full_aup_rup_trigger(struct csid_device *csid)
+{
+ /* trigger SET in combined register */
+ writel(RUP_SET, csid->base + CSID_RUP_AUP_CMD);
+}
+
+static void __csid_aup_update(struct csid_device *csid, int port_id)
+{
+ if (csid_is_lite(csid)) {
+ /* CSID Lites in v980 follow the legacy way of a combined RUP
+ * and AUP commands without an explicit SET register.
+ */
+ csid->reg_update |= CSID_LITE_AUP_RDI(port_id);
+ writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
+ } else {
+ csid->aup_update |= CSID_RUP_AUP_RDI(port_id);
+ writel(csid->aup_update, csid->base + CSID_AUP_CMD);
+
+ /* CSID Fulls in v980 split AUP and RUP commands, which requires
+ * additional SET operation to make registers modification take
+ * effect.
+ */
+ __csid_full_aup_rup_trigger(csid);
+ }
+}
+
+static void __csid_rup_update(struct csid_device *csid, int port_id)
+{
+ if (csid_is_lite(csid)) {
+ /* CSID Lites in v980 follow the legacy way of a combined RUP
+ * and AUP commands without an explicit SET register.
+ */
+ csid->reg_update |= CSID_LITE_RUP_RDI(port_id);
+ writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
+ } else {
+ csid->rup_update |= CSID_RUP_AUP_RDI(port_id);
+ writel(csid->rup_update, csid->base + CSID_RUP_CMD);
+
+ /* CSID Fulls in v980 split AUP and RUP commands, which requires
+ * additional SET operation to make registers modification take
+ * effect.
+ */
+ __csid_full_aup_rup_trigger(csid);
+ }
+}
+
+static void __csid_aup_rup_clear(struct csid_device *csid, int port_id)
+{
+ /* Hardware clears the registers upon consuming the settings */
+ if (csid_is_lite(csid)) {
+ csid->reg_update &= ~CSID_LITE_RUP_RDI(port_id);
+ csid->reg_update &= ~CSID_LITE_AUP_RDI(port_id);
+ } else {
+ csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id);
+ csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id);
+ }
+}
+
+static void __csid_configure_rx(struct csid_device *csid,
+ struct csid_phy_config *phy)
+{
+ int val;
+
+ val = (phy->lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
+ val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
+ val |= (phy->csiphy_id + CSI2_RX_CFG0_PHY_SEL_BASE_IDX)
+ << CSI2_RX_CFG0_PHY_NUM_SEL;
+ writel(val, csid->base + CSID_CSI2_RX_CFG0);
+
+ val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
+ writel(val, csid->base + CSID_CSI2_RX_CFG1);
+}
+
+static void __csid_configure_rx_vc(struct csid_device *csid, int vc)
+{
+ int val;
+
+ if (vc > 3) {
+ val = readl(csid->base + CSID_CSI2_RX_CFG1);
+ val |= CSI2_RX_CFG1_VC_MODE;
+ writel(val, csid->base + CSID_CSI2_RX_CFG1);
+ }
+}
+
+static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
+{
+ int val = 0;
+ u32 rdi_ctrl_offset = CSID_RDI_CTRL(rdi);
+
+ if (enable)
+ val = RDI_CTRL_START_CMD;
+
+ writel(val, csid->base + rdi_ctrl_offset);
+}
+
+static void __csid_configure_rdi_pix_store(struct csid_device *csid, u8 rdi)
+{
+ u32 val;
+
+ /*
+ * Configure pixel store to allow absorption of hblanking or idle time.
+ * This helps with horizontal crop and prevents line buffer conflicts.
+ * Reset state is 0x8 which has MIN_HBI=4, we keep the default MIN_HBI
+ * and just enable the pixel store functionality.
+ */
+ val = (4 << RDI_PIX_STORE_CFG0_MIN_HBI) | RDI_PIX_STORE_CFG0_EN;
+ writel(val, csid->base + CSID_RDI_PIX_STORE_CFG0(rdi));
+}
+
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
+{
+ u32 val;
+
+ /* Source pads matching RDI channels on hardware.
+ * E.g. Pad 1 -> RDI0, Pad 2 -> RDI1, etc.
+ */
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
+ const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
+ csid->res->formats->nformats,
+ input_format->code);
+
+ /*
+ * DT_ID is a two bit bitfield that is concatenated with
+ * the four least significant bits of the five bit VC
+ * bitfield to generate an internal CID value.
+ *
+ * CSID_RDI_CFG0(vc)
+ * DT_ID : 28:27
+ * VC : 26:22
+ * DT : 21:16
+ *
+ * CID : VC 3:0 << 2 | DT_ID 1:0
+ */
+ u8 dt_id = vc & 0x03;
+ u32 rdi_cfg0_offset = CSID_RDI_CFG0(port);
+ u32 rdi_cfg1_offset = CSID_RDI_CFG1(port);
+ u32 rdi_ctrl_offset = CSID_RDI_CTRL(port);
+
+ val = RDI_CFG0_TIMESTAMP_EN;
+ val |= RDI_CFG0_TIMESTAMP_STB_SEL;
+ val |= RDI_CFG0_RETIME_BS;
+
+ /* note: for non-RDI path, this should be format->decode_format */
+ val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
+ val |= vc << RDI_CFG0_VC;
+ val |= format->data_type << RDI_CFG0_DT;
+ val |= dt_id << RDI_CFG0_DT_ID;
+ writel(val, csid->base + rdi_cfg0_offset);
+
+ val = RDI_CFG1_PACKING_FORMAT_MIPI;
+ writel(val, csid->base + rdi_cfg1_offset);
+ writel(1, csid->base + CSID_RDI_IRQ_PATTERN(port));
+ writel(0, csid->base + CSID_RDI_IRQ_PERIOD(port));
+
+ /* Configure pixel store using dedicated register in 980 */
+ if (!csid_is_lite(csid))
+ __csid_configure_rdi_pix_store(csid, port);
+
+ val = 0;
+ writel(val, csid->base + rdi_ctrl_offset);
+
+ val = readl(csid->base + rdi_cfg0_offset);
+
+ if (enable)
+ val |= RDI_CFG0_EN;
+
+ writel(val, csid->base + rdi_cfg0_offset);
+}
+
+static void csid_configure_stream(struct csid_device *csid, u8 enable)
+{
+ u8 i, k;
+
+ __csid_configure_rx(csid, &csid->phy);
+
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ u32 mask = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
+
+ writel(mask, csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
+ mask = readl(csid->base + CSID_TOP_IRQ_MASK);
+ mask = enable ? mask | CSID_RDI_TOP_IRQ(i) : mask & ~CSID_RDI_TOP_IRQ(i);
+ writel(mask, csid->base + CSID_TOP_IRQ_MASK);
+ writel(IRQ_CMD_CLEAR, csid->base + CSID_IRQ_CMD);
+ __csid_configure_rdi_stream(csid, enable, i, 0);
+ __csid_configure_rx_vc(csid, 0);
+
+ for (k = 0; enable && k < CAMSS_INIT_BUF_COUNT; k++) {
+ __csid_aup_update(csid, i);
+ __csid_rup_update(csid, i);
+ }
+
+ __csid_ctrl_rdi(csid, enable, i);
+ }
+ }
+}
+
+static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
+{
+ return 0;
+}
+
+static void csid_subdev_reg_update(struct csid_device *csid, int port_id,
+ bool clear)
+{
+ if (clear)
+ __csid_aup_rup_clear(csid, port_id);
+ else
+ __csid_aup_update(csid, port_id);
+}
+
+/**
+ * csid_isr - CSID module interrupt service routine
+ * @irq: Interrupt line
+ * @dev: CSID device
+ *
+ * Return IRQ_HANDLED on success
+ */
+static irqreturn_t csid_isr(int irq, void *dev)
+{
+ struct csid_device *csid = dev;
+ u32 val, buf_done_val;
+ u8 reset_done;
+ int i;
+
+ val = readl(csid->base + CSID_TOP_IRQ_STATUS);
+ writel(val, csid->base + CSID_TOP_IRQ_CLEAR);
+
+ reset_done = val & INFO_RST_DONE;
+
+ buf_done_val = readl(csid->base + CSID_BUF_DONE_IRQ_STATUS);
+ writel(buf_done_val, csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ bool done = buf_done_val & BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i);
+
+ val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
+ writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
+
+ if (val & INFO_RUP_DONE)
+ csid_subdev_reg_update(csid, i, true);
+
+ if (done)
+ camss_buf_done(csid->camss, csid->id, i);
+ }
+ }
+
+ val = IRQ_CMD_CLEAR;
+ writel(val, csid->base + CSID_IRQ_CMD);
+
+ if (reset_done)
+ complete(&csid->reset_complete);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * csid_reset - Trigger reset on CSID module and wait to complete
+ * @csid: CSID device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csid_reset(struct csid_device *csid)
+{
+ unsigned long time;
+ u32 val;
+ int i;
+
+ reinit_completion(&csid->reset_complete);
+
+ val = INFO_RST_DONE | BUF_DONE_IRQ_STATUS;
+ writel(val, csid->base + CSID_TOP_IRQ_CLEAR);
+ writel(val, csid->base + CSID_TOP_IRQ_MASK);
+
+ val = 0;
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
+ if (csid->phy.en_vc & BIT(i)) {
+ /*
+ * Only need to clear buf done IRQ status here,
+ * RUP done IRQ status will be cleared once isr
+ * strobe generated by CSID_RST_CMD
+ */
+ val |= BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i);
+ }
+ }
+ writel(val, csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+ writel(val, csid->base + CSID_BUF_DONE_IRQ_MASK);
+
+ /* Clear all IRQ status with CLEAR bits set */
+ val = IRQ_CMD_CLEAR;
+ writel(val, csid->base + CSID_IRQ_CMD);
+
+ val = RST_LOCATION | RST_MODE;
+ writel(val, csid->base + CSID_RST_CFG);
+
+ val = SELECT_HW_RST | SELECT_IRQ_RST;
+ writel(val, csid->base + CSID_RST_CMD);
+
+ time = wait_for_completion_timeout(&csid->reset_complete,
+ msecs_to_jiffies(CSID_RESET_TIMEOUT_MS));
+
+ if (!time) {
+ dev_err(csid->camss->dev, "CSID reset timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static void csid_subdev_init(struct csid_device *csid)
+{
+ csid->testgen.nmodes = CSID_PAYLOAD_MODE_DISABLED;
+}
+
+const struct csid_hw_ops csid_ops_980 = {
+ .configure_stream = csid_configure_stream,
+ .configure_testgen_pattern = csid_configure_testgen_pattern,
+ .hw_version = csid_hw_version,
+ .isr = csid_isr,
+ .reset = csid_reset,
+ .src_pad_code = csid_src_pad_code,
+ .subdev_init = csid_subdev_init,
+ .reg_update = csid_subdev_reg_update,
+};
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 5296b10f6..5d07cffb6 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -155,6 +155,8 @@ struct csid_device {
u32 irq;
char irq_name[30];
u32 reg_update;
+ u32 rup_update;
+ u32 aup_update;
struct camss_clock *clock;
int nclocks;
struct regulator_bulk_data *supplies;
@@ -216,6 +218,7 @@ extern const struct csid_hw_ops csid_ops_4_1;
extern const struct csid_hw_ops csid_ops_4_7;
extern const struct csid_hw_ops csid_ops_340;
extern const struct csid_hw_ops csid_ops_680;
+extern const struct csid_hw_ops csid_ops_980;
extern const struct csid_hw_ops csid_ops_gen2;
extern const struct csid_hw_ops csid_ops_gen3;
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index 9cf22ff17..999f92fbe 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -826,7 +826,7 @@ csiphy_lane_regs lane_regs_sm8750[] = {
{0x0008, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
- {0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x005C, 0x00, 0x00, CSIPHY_SKEW_CAL},
{0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
{0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},
@@ -849,7 +849,7 @@ csiphy_lane_regs lane_regs_sm8750[] = {
{0x0408, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0494, 0xD7, 0x00, CSIPHY_SKEW_CAL},
- {0x045C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x045C, 0x00, 0x00, CSIPHY_SKEW_CAL},
{0x0460, 0xFD, 0x00, CSIPHY_SKEW_CAL},
{0x0464, 0x7F, 0x00, CSIPHY_SKEW_CAL},
@@ -872,7 +872,7 @@ csiphy_lane_regs lane_regs_sm8750[] = {
{0x0808, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0894, 0xD7, 0x00, CSIPHY_SKEW_CAL},
- {0x085C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x085C, 0x00, 0x00, CSIPHY_SKEW_CAL},
{0x0860, 0xFD, 0x00, CSIPHY_SKEW_CAL},
{0x0864, 0x7F, 0x00, CSIPHY_SKEW_CAL},
@@ -895,7 +895,7 @@ csiphy_lane_regs lane_regs_sm8750[] = {
{0x0C08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0C94, 0xD7, 0x00, CSIPHY_SKEW_CAL},
- {0x0C5C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+ {0x0C5C, 0x00, 0x00, CSIPHY_SKEW_CAL},
{0x0C60, 0xFD, 0x00, CSIPHY_SKEW_CAL},
{0x0C64, 0x7F, 0x00, CSIPHY_SKEW_CAL},
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 319d19158..d1b66033a 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <media/media-entity.h>
#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
#include <media/v4l2-subdev.h>
#include "camss-vfe.h"
@@ -352,6 +353,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
case CAMSS_845:
case CAMSS_8550:
case CAMSS_8650:
+ case CAMSS_8750:
case CAMSS_8775P:
case CAMSS_X1E80100:
switch (sink_code) {
@@ -449,13 +451,14 @@ u32 vfe_hw_version(struct vfe_device *vfe)
*/
void vfe_buf_done(struct vfe_device *vfe, int wm)
{
- struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]];
+ struct vfe_line *line;
const struct vfe_hw_ops *ops = vfe->res->hw_ops;
struct camss_buffer *ready_buf;
struct vfe_output *output;
unsigned long flags;
u32 index;
u64 ts = ktime_get_ns();
+ enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
spin_lock_irqsave(&vfe->output_lock, flags);
@@ -464,7 +467,8 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
"Received wm done for unmapped index\n");
goto out_unlock;
}
- output = &vfe->line[vfe->wm_output_map[wm]].output;
+ line = &vfe->line[vfe->wm_output_map[wm]];
+ output = &line->output;
ready_buf = output->buf[0];
if (!ready_buf) {
@@ -494,7 +498,7 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
spin_unlock_irqrestore(&vfe->output_lock, flags);
- vb2_buffer_done(&ready_buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
+ vb2_buffer_done(&ready_buf->vb.vb2_buf, state);
return;
@@ -552,7 +556,9 @@ int vfe_enable_output_v2(struct vfe_line *line)
output->gen2.active_num++;
ops->vfe_wm_update(vfe, output->wm_idx[0],
output->buf[i]->addr[0], line);
- ops->reg_update(vfe, line->id);
+
+ if (!vfe->res->reg_update_after_csid_config)
+ ops->reg_update(vfe, line->id);
}
spin_unlock_irqrestore(&vfe->output_lock, flags);
@@ -2012,6 +2018,7 @@ static int vfe_bpl_align_rdi(struct vfe_device *vfe)
case CAMSS_845:
case CAMSS_8550:
case CAMSS_8650:
+ case CAMSS_8750:
case CAMSS_8775P:
case CAMSS_X1E80100:
ret = 16;
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index ae9dad353..c402ef170 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -133,6 +133,7 @@ struct vfe_isr_ops {
struct vfe_subdev_resources {
bool is_lite;
+ bool reg_update_after_csid_config;
u8 line_num;
bool has_pd;
char *pd_name;
@@ -249,6 +250,7 @@ extern const struct vfe_hw_ops vfe_ops_340;
extern const struct vfe_hw_ops vfe_ops_480;
extern const struct vfe_hw_ops vfe_ops_680;
extern const struct vfe_hw_ops vfe_ops_gen3;
+extern const struct vfe_hw_ops vfe_ops_gen4;
int vfe_get(struct vfe_device *vfe);
void vfe_put(struct vfe_device *vfe);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] media: camss: csid: add the 980 generation
2026-10-28 3:41 ` [PATCH 4/9] media: camss: csid: add the 980 generation Qianhui Xu
@ 2026-10-28 4:03 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-28 4:03 UTC (permalink / raw)
To: Qianhui Xu; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] media: camss: csid: INFO_RUP_DONE left masked in csid_configure_stream()
- [High] media: camss: csid: wrong dt_id in __csid_configure_rdi_stream()
- [Medium] media: camss: csid-980: race on reg_update, rup_update, and aup_update
--
commit 9e06166c718bdeac203de00c1a26a347c6114d7a
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
media: camss: csid: add the 980 generation
This commit adds support for the CSID 980 generation found on SM8750,
including register maps for full and lite variants, split RUP and AUP
register update commands, RDI stream configuration, and buffer-done
interrupt handling.
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-980.c b/drivers/media/platform/qcom/camss/camss-csid-980.c
> new file mode 100644
> index 0000000000000..45f3b37867040
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-csid-980.c
[ ... ]
> +static void __csid_aup_update(struct csid_device *csid, int port_id)
> +{
> + if (csid_is_lite(csid)) {
> + /* CSID Lites in v980 follow the legacy way of a combined RUP
> + * and AUP commands without an explicit SET register.
> + */
> + csid->reg_update |= CSID_LITE_AUP_RDI(port_id);
> + writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
> + } else {
> + csid->aup_update |= CSID_RUP_AUP_RDI(port_id);
> + writel(csid->aup_update, csid->base + CSID_AUP_CMD);
> +
> + /* CSID Fulls in v980 split AUP and RUP commands, which requires
> + * additional SET operation to make registers modification take
> + * effect.
> + */
> + __csid_full_aup_rup_trigger(csid);
> + }
> +}
> +
> +static void __csid_rup_update(struct csid_device *csid, int port_id)
> +{
> + if (csid_is_lite(csid)) {
> + /* CSID Lites in v980 follow the legacy way of a combined RUP
> + * and AUP commands without an explicit SET register.
> + */
> + csid->reg_update |= CSID_LITE_RUP_RDI(port_id);
> + writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
> + } else {
> + csid->rup_update |= CSID_RUP_AUP_RDI(port_id);
> + writel(csid->rup_update, csid->base + CSID_RUP_CMD);
> +
> + /* CSID Fulls in v980 split AUP and RUP commands, which requires
> + * additional SET operation to make registers modification take
> + * effect.
> + */
> + __csid_full_aup_rup_trigger(csid);
> + }
> +}
> +
> +static void __csid_aup_rup_clear(struct csid_device *csid, int port_id)
> +{
> + /* Hardware clears the registers upon consuming the settings */
> + if (csid_is_lite(csid)) {
> + csid->reg_update &= ~CSID_LITE_RUP_RDI(port_id);
> + csid->reg_update &= ~CSID_LITE_AUP_RDI(port_id);
> + } else {
> + csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id);
> + csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id);
> + }
> +}
[Severity: Medium]
Can concurrent updates to csid->reg_update, csid->rup_update, and
csid->aup_update race with each other?
In csid_isr(), when INFO_RUP_DONE is set, the interrupt handler calls
csid_subdev_reg_update(csid, i, true) -> __csid_aup_rup_clear() in hardirq
context without holding vfe->output_lock or any CSID lock:
if (val & INFO_RUP_DONE)
csid_subdev_reg_update(csid, i, true);
Meanwhile, on another CPU, vfe_queue_buffer_v2() (holding only
vfe->output_lock) or csid_configure_stream() (holding no spinlock while
csid->irq is enabled) can call __csid_aup_update() or __csid_rup_update().
Because both paths perform non-atomic read-modify-write operations on
csid->reg_update, csid->aup_update, and csid->rup_update before writing the
full mask to CSID_LITE_RUP_AUP_CMD, CSID_AUP_CMD, or CSID_RUP_CMD, could this
clobber newly set update bits or re-write stale RDI bits to the command
registers?
[ ... ]
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
> +{
> + u32 val;
> +
> + /* Source pads matching RDI channels on hardware.
> + * E.g. Pad 1 -> RDI0, Pad 2 -> RDI1, etc.
> + */
> + struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
> + const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> + csid->res->formats->nformats,
> + input_format->code);
> +
> + /*
> + * DT_ID is a two bit bitfield that is concatenated with
> + * the four least significant bits of the five bit VC
> + * bitfield to generate an internal CID value.
> + *
> + * CSID_RDI_CFG0(vc)
> + * DT_ID : 28:27
> + * VC : 26:22
> + * DT : 21:16
> + *
> + * CID : VC 3:0 << 2 | DT_ID 1:0
> + */
> + u8 dt_id = vc & 0x03;
[Severity: High]
Should dt_id be computed as port & 0x03 instead of vc & 0x03 here?
In csid_configure_stream(), __csid_configure_rdi_stream(csid, enable, i, 0)
is always called with vc = 0 for every port i. Calculating dt_id from vc
programs dt_id = 0 and an identical internal CID = 0 in CSID_RDI_CFG0(port)
for all RDI ports (RDI0 through RDI3), which breaks streaming on non-zero RDI
ports just like the issue fixed for CSID gen3 in commit ad136e52634d ("media:
qcom: camss: Fix RDI streaming for CSID GEN3").
[ ... ]
> +static void csid_configure_stream(struct csid_device *csid, u8 enable)
> +{
> + u8 i, k;
> +
> + __csid_configure_rx(csid, &csid->phy);
> +
> + for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
> + if (csid->phy.en_vc & BIT(i)) {
> + u32 mask = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
> +
> + writel(mask, csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
[Severity: High]
Does csid_configure_stream() leave INFO_RUP_DONE masked in
CSID_CSI2_RDIN_IRQ_MASK(i)?
Here, CSID_CSI2_RDIN_IRQ_MASK(i) is read into mask and written back unchanged
without setting INFO_RUP_DONE, and csid_reset() does not unmask INFO_RUP_DONE
in CSID_CSI2_RDIN_IRQ_MASK(i) either (even after follow-up commit
7457e598955f ("media: camss: csid: report frame starts from CSID"), which
only adds INFO_CAMIF_SOF to mask).
In csid_isr(), the handler checks for INFO_RUP_DONE to clear the software
update state:
val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
if (val & INFO_RUP_DONE)
csid_subdev_reg_update(csid, i, true);
Without INFO_RUP_DONE unmasked in CSID_CSI2_RDIN_IRQ_MASK(i), won't
csid_subdev_reg_update(csid, i, true) never be called, leaving
CSID_LITE_RUP_RDI(i) permanently set in csid->reg_update on CSID Lite (so
every subsequent per-frame __csid_aup_update() call in vfe_buf_done()
re-asserts RUP alongside AUP) and leaving stale bits set in csid->aup_update,
csid->rup_update, and csid->reg_update across streams?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028034105.326535-1-xiaqinghuan@mail.ustc.edu.cn?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] media: camss: csid: add the 980 generation
2026-10-28 3:41 ` [PATCH 4/9] media: camss: csid: add the 980 generation Qianhui Xu
2026-10-28 4:03 ` sashiko-bot
@ 2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: Shuai Liu @ 2026-10-29 7:54 UTC (permalink / raw)
To: linux-media, linux-arm-msm
Cc: Shuai Liu, Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
Hangxiang Ma, Qianhui Xu, devicetree, linux-kernel
Hi Qianhui,
The csid-980 register map and RDI configuration itself look sane to me
(the vc=0 stream programming matches what gen2/gen3 already do, and the
split RUP/AUP handling for the full variant is clearly commented).
> Only the register map, the RDI stream configuration and the
> buffer-done reporting are described here; frame start reporting is
> added on top of it by a separate change.
The commit message does not match the patch content, though. As
posted, this patch touches six files, not three:
1. drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c is modified
to flip the four SKEW_CAL data bytes (0x005c and per-lane copies)
from 0x54 to 0x00. That is a correction to patch 2/9 and belongs
there - patch 2 should simply program the intended value, otherwise
the series carries a value its own commit message calls wrong.
2. camss-vfe.c and camss-vfe.h carry groundwork for patches 5/9 and
8/9, none of it described by this commit message:
- the v4l2-event.h include and the hoisting of `line` plus the new
`state` variable in vfe_buf_done() (needed by 5/9);
- the new reg_update_after_csid_config flag in struct
vfe_subdev_resources and the corresponding gating of the
reg_update() call in vfe_enable_output_v2();
- the CAMSS_8750 cases in vfe_src_pad_code() and
vfe_bpl_align_rdi(), and the extern declaration of
vfe_ops_gen4 (all needed by 8/9).
Please fold the csiphy fix into patch 2, the vfe_buf_done/event
groundwork into patch 5, and the gen4 enablement into patch 8 (or
describe the split here if you prefer to keep it), so that each patch
stands on its own.
Thanks,
Shuai Liu
(This review was prepared with the assistance of an AI coding tool;
all findings were verified against the patched source.)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/9] media: camss: csid: report frame starts from CSID
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (3 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 4/9] media: camss: csid: add the 980 generation Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 4:01 ` sashiko-bot
2026-10-28 3:41 ` [PATCH 6/9] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook Qianhui Xu
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
The CSID of generation 980 moves the capture interrupts away from the VFE:
that VFE has no useful interrupt sources, and the CSID reports RDI buffer
done, register update completion and CAMIF start-of-frame per CSI2 stream
instead.
A buffer is only complete once its frame started, and the sequence number
of a frame is defined by its start: turn the CSID start-of-frame interrupt
into the frame start source. camss_frame_start() forwards it to the
paired VFE, which raises V4L2_EVENT_FRAME_SYNC on the video node,
increments the sequence only there, and marks buffers done before their
frame started as erroneous. A start-of-frame may also follow an as-yet
unhandled completion, so the end-of-frame and start-of-frame timestamps are
compared and the completion reported first in that case.
Hardware generations that still report frame starts through the VFE keep
the previous behaviour, as the new vfe_hw_ops callback is only provided by
VFE implementations that need it.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../platform/qcom/camss/camss-csid-980.c | 25 +++++++++
drivers/media/platform/qcom/camss/camss-vfe.c | 54 ++++++++++++++++++-
drivers/media/platform/qcom/camss/camss-vfe.h | 3 ++
drivers/media/platform/qcom/camss/camss.c | 12 +++++
drivers/media/platform/qcom/camss/camss.h | 1 +
5 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-980.c b/drivers/media/platform/qcom/camss/camss-csid-980.c
index 45f3b3786..8acc1efea 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-980.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-980.c
@@ -108,6 +108,7 @@
#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi) \
((csid_is_lite(csid) ? 0xF4 : 0x11C) + 0x10 * (rdi))
#define INFO_RUP_DONE BIT(23)
+#define INFO_CAMIF_SOF BIT(4)
#define CSID_RDI_TOP_IRQ(rdi) BIT((csid_is_lite(csid) ? 8 : 16) + (rdi))
#define CSID_RDI_IRQ_PATTERN(rdi) \
((csid_is_lite(csid) ? 0x548 : 0xE50) + (csid_is_lite(csid) ? 0x100 : 0x200) * (rdi))
@@ -295,6 +296,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
if (csid->phy.en_vc & BIT(i)) {
u32 mask = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
+ writel(INFO_CAMIF_SOF, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
+ mask = enable ? mask | INFO_CAMIF_SOF : mask & ~INFO_CAMIF_SOF;
writel(mask, csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
mask = readl(csid->base + CSID_TOP_IRQ_MASK);
mask = enable ? mask | CSID_RDI_TOP_IRQ(i) : mask & ~CSID_RDI_TOP_IRQ(i);
@@ -327,6 +330,18 @@ static void csid_subdev_reg_update(struct csid_device *csid, int port_id,
__csid_aup_update(csid, port_id);
}
+static u64 csid_read_timestamp(struct csid_device *csid, u32 offset)
+{
+ u32 hi, lo;
+
+ do {
+ hi = readl(csid->base + offset + 4);
+ lo = readl(csid->base + offset);
+ } while (hi != readl(csid->base + offset + 4));
+
+ return (u64)hi << 32 | lo;
+}
+
/**
* csid_isr - CSID module interrupt service routine
* @irq: Interrupt line
@@ -359,6 +374,16 @@ static irqreturn_t csid_isr(int irq, void *dev)
if (val & INFO_RUP_DONE)
csid_subdev_reg_update(csid, i, true);
+ if (val & INFO_CAMIF_SOF) {
+ /* A new SOF may follow an as-yet unhandled completion. */
+ if (done && csid_read_timestamp(csid, CSID_RDI_EOF_TS(i)) <
+ csid_read_timestamp(csid, CSID_RDI_SOF_TS(i))) {
+ camss_buf_done(csid->camss, csid->id, i);
+ done = false;
+ }
+ camss_frame_start(csid->camss, csid->id, i);
+ }
+
if (done)
camss_buf_done(csid->camss, csid->id, i);
}
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index d1b66033a..f42fc56e7 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -444,6 +444,34 @@ u32 vfe_hw_version(struct vfe_device *vfe)
return hw_version;
}
+void vfe_frame_start(struct vfe_device *vfe, int wm)
+{
+ struct v4l2_event event = { .type = V4L2_EVENT_FRAME_SYNC };
+ struct vfe_output *output;
+ struct vfe_line *line;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vfe->output_lock, flags);
+ if (vfe->wm_output_map[wm] == VFE_LINE_NONE)
+ goto out_unlock;
+
+ line = &vfe->line[vfe->wm_output_map[wm]];
+ output = &line->output;
+ if (output->state != VFE_OUTPUT_ON)
+ goto out_unlock;
+
+ /* Count frames even when there is no buffer available for capture. */
+ event.u.frame_sync.frame_sequence = output->sequence++;
+ output->frame_started = true;
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+
+ v4l2_event_queue(line->subdev.devnode, &event);
+ return;
+
+out_unlock:
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+}
+
/*
* vfe_buf_done - Process write master done interrupt
* @vfe: VFE Device
@@ -478,7 +506,13 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
}
ready_buf->vb.vb2_buf.timestamp = ts;
- ready_buf->vb.sequence = output->sequence++;
+ if (ops->frame_start) {
+ ready_buf->vb.sequence = output->sequence - 1;
+ if (!output->frame_started)
+ state = VB2_BUF_STATE_ERROR;
+ } else {
+ ready_buf->vb.sequence = output->sequence++;
+ }
index = 0;
output->buf[0] = output->buf[1];
@@ -544,6 +578,7 @@ int vfe_enable_output_v2(struct vfe_line *line)
output->state = VFE_OUTPUT_ON;
output->sequence = 0;
+ output->frame_started = false;
output->wait_reg_update = 0;
reinit_completion(&output->reg_update);
@@ -1972,8 +2007,23 @@ static int vfe_link_setup(struct media_entity *entity,
return 0;
}
+static int vfe_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+ struct v4l2_event_subscription *sub)
+{
+ struct vfe_line *line = v4l2_get_subdevdata(sd);
+ struct vfe_device *vfe = to_vfe(line);
+
+ if (!vfe->res->hw_ops->frame_start ||
+ sub->type != V4L2_EVENT_FRAME_SYNC || sub->id)
+ return -EINVAL;
+
+ return v4l2_event_subscribe(fh, sub, 16, NULL);
+}
+
static const struct v4l2_subdev_core_ops vfe_core_ops = {
.s_power = vfe_set_power,
+ .subscribe_event = vfe_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
};
static const struct v4l2_subdev_video_ops vfe_video_ops = {
@@ -2080,6 +2130,8 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
v4l2_subdev_init(sd, &vfe_v4l2_ops);
sd->internal_ops = &vfe_v4l2_internal_ops;
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ if (vfe->res->hw_ops->frame_start)
+ sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
if (i == VFE_LINE_PIX && vfe->res->is_lite == false)
snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d_%s",
MSM_VFE_NAME, vfe->id, "pix");
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index c402ef170..621eee07d 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -77,6 +77,7 @@ struct vfe_output {
};
enum vfe_output_state state;
unsigned int sequence;
+ bool frame_started;
int wait_reg_update;
struct completion sof;
@@ -118,6 +119,7 @@ struct vfe_hw_ops {
struct vfe_line *line);
void (*vfe_wm_stop)(struct vfe_device *vfe, u8 wm);
void (*vfe_buf_done)(struct vfe_device *vfe, int port_id);
+ void (*frame_start)(struct vfe_device *vfe, int port_id);
void (*vfe_wm_update)(struct vfe_device *vfe, u8 wm, u32 addr,
struct vfe_line *line);
};
@@ -286,6 +288,7 @@ int vfe_enable_v2(struct vfe_line *line);
* @wm: Write master id
*/
void vfe_buf_done(struct vfe_device *vfe, int wm);
+void vfe_frame_start(struct vfe_device *vfe, int wm);
/*
* vfe_get_output_v2 - Get vfe output line
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388..92d536986 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -5044,6 +5044,18 @@ void camss_buf_done(struct camss *camss, int hw_id, int port_id)
}
}
+void camss_frame_start(struct camss *camss, int hw_id, int port_id)
+{
+ struct vfe_device *vfe;
+
+ if (hw_id < 0 || hw_id >= camss->res->vfe_num || port_id < 0)
+ return;
+
+ vfe = &camss->vfe[hw_id];
+ if (port_id < vfe->res->line_num && vfe->res->hw_ops->frame_start)
+ vfe->res->hw_ops->frame_start(vfe, port_id);
+}
+
/*
* camss_register_entities - Register subdev nodes and create links
* @camss: CAMSS device
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index ef45eea00..6a17ec027 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -178,6 +178,7 @@ int camss_vfe_get(struct camss *camss, int id);
void camss_vfe_put(struct camss *camss, int id);
void camss_delete(struct camss *camss);
void camss_buf_done(struct camss *camss, int hw_id, int port_id);
+void camss_frame_start(struct camss *camss, int hw_id, int port_id);
void camss_reg_update(struct camss *camss, int hw_id,
int port_id, bool is_clear);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] media: camss: csid: report frame starts from CSID
2026-10-28 3:41 ` [PATCH 5/9] media: camss: csid: report frame starts from CSID Qianhui Xu
@ 2026-10-28 4:01 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-28 4:01 UTC (permalink / raw)
To: Qianhui Xu; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] media: camss: vfe_buf_done() never resets frame_started for next frame
- [Medium] media: camss: vfe: inconsistent event ioctls when frame_start is unset
--
commit 7457e598955f906dac687d34057f24b9b8d1d505
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
media: camss: csid: report frame starts from CSID
This commit handles CSID generation 980 start-of-frame interrupts and
forwards them to the paired VFE via camss_frame_start(). The VFE queues a
V4L2_EVENT_FRAME_SYNC event on the subdev node, tracks frame sequence numbers
at frame start, and checks whether a frame started before marking a completed
buffer as done.
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index d1b66033a096d..f42fc56e7dfc9 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -444,6 +444,34 @@ u32 vfe_hw_version(struct vfe_device *vfe)
> return hw_version;
> }
>
> +void vfe_frame_start(struct vfe_device *vfe, int wm)
> +{
> + struct v4l2_event event = { .type = V4L2_EVENT_FRAME_SYNC };
> + struct vfe_output *output;
> + struct vfe_line *line;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&vfe->output_lock, flags);
> + if (vfe->wm_output_map[wm] == VFE_LINE_NONE)
> + goto out_unlock;
> +
> + line = &vfe->line[vfe->wm_output_map[wm]];
> + output = &line->output;
> + if (output->state != VFE_OUTPUT_ON)
> + goto out_unlock;
> +
> + /* Count frames even when there is no buffer available for capture. */
> + event.u.frame_sync.frame_sequence = output->sequence++;
> + output->frame_started = true;
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> +
> + v4l2_event_queue(line->subdev.devnode, &event);
> + return;
> +
> +out_unlock:
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> +}
> +
> /*
> * vfe_buf_done - Process write master done interrupt
> * @vfe: VFE Device
> @@ -478,7 +506,13 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
> }
>
> ready_buf->vb.vb2_buf.timestamp = ts;
> - ready_buf->vb.sequence = output->sequence++;
> + if (ops->frame_start) {
> + ready_buf->vb.sequence = output->sequence - 1;
> + if (!output->frame_started)
> + state = VB2_BUF_STATE_ERROR;
> + } else {
> + ready_buf->vb.sequence = output->sequence++;
> + }
[Severity: Medium]
Should vfe_buf_done() reset output->frame_started to false after consuming
it for the current buffer?
In vfe_enable_output_v2(), output->sequence is initialized to 0 and
output->frame_started is set to false. When csid_isr() -> camss_frame_start()
-> vfe_frame_start() runs on the first start-of-frame interrupt, it sets
output->frame_started = true.
Because vfe_buf_done() never clears output->frame_started back to false,
output->frame_started remains true for the rest of the streaming session. On
subsequent frames, if a buffer completion interrupt arrives before that
frame's start-of-frame interrupt, !output->frame_started will not trigger,
leaving state as VB2_BUF_STATE_DONE with a duplicate sequence number
(output->sequence - 1).
Also, if a buffer completes before the very first start-of-frame interrupt
while output->sequence is still 0, does ready_buf->vb.sequence =
output->sequence - 1 underflow unsigned int to UINT_MAX?
[ ... ]
> @@ -1972,8 +2007,23 @@ static int vfe_link_setup(struct media_entity *entity,
> return 0;
> }
>
> +static int vfe_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub)
> +{
> + struct vfe_line *line = v4l2_get_subdevdata(sd);
> + struct vfe_device *vfe = to_vfe(line);
> +
> + if (!vfe->res->hw_ops->frame_start ||
> + sub->type != V4L2_EVENT_FRAME_SYNC || sub->id)
> + return -EINVAL;
> +
> + return v4l2_event_subscribe(fh, sub, 16, NULL);
> +}
> +
> static const struct vfe_subdev_core_ops vfe_core_ops = {
> .s_power = vfe_set_power,
> + .subscribe_event = vfe_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> };
>
> static const struct v4l2_subdev_video_ops vfe_video_ops = {
> @@ -2080,6 +2130,8 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
> v4l2_subdev_init(sd, &vfe_v4l2_ops);
> sd->internal_ops = &vfe_v4l2_internal_ops;
> sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + if (vfe->res->hw_ops->frame_start)
> + sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
[Severity: Medium]
On VFE hardware generations where vfe->res->hw_ops->frame_start is NULL,
msm_vfe_register_entities() does not set V4L2_SUBDEV_FL_HAS_EVENTS on the
subdevice, but vfe_core_ops still wires up .subscribe_event and
.unsubscribe_event unconditionally.
In drivers/media/v4l2-core/v4l2-subdev.c:subdev_do_ioctl(), the core checks
for those callbacks before checking V4L2_SUBDEV_FL_HAS_EVENTS:
case VIDIOC_DQEVENT:
if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
return -ENOIOCTLCMD;
return v4l2_event_dequeue(vfh, arg, file->f_flags & O_NONBLOCK);
case VIDIOC_SUBSCRIBE_EVENT:
if (v4l2_subdev_has_op(sd, core, subscribe_event))
return v4l2_subdev_call(sd, core, subscribe_event,
vfh, arg);
...
return -ENOIOCTLCMD;
case VIDIOC_UNSUBSCRIBE_EVENT:
if (v4l2_subdev_has_op(sd, core, unsubscribe_event))
return v4l2_subdev_call(sd, core, unsubscribe_event,
vfh, arg);
if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS)
return v4l2_event_subdev_unsubscribe(sd, vfh, arg);
return -ENOIOCTLCMD;
When vfe->res->hw_ops->frame_start is NULL, VIDIOC_DQEVENT returns
-ENOIOCTLCMD (-ENOTTY), whereas VIDIOC_SUBSCRIBE_EVENT calls
vfe_subscribe_event() and returns -EINVAL, and VIDIOC_UNSUBSCRIBE_EVENT calls
v4l2_event_subdev_unsubscribe() and returns 0.
Should vfe_subscribe_event() return -ENOIOCTLCMD when
!vfe->res->hw_ops->frame_start, and could .unsubscribe_event be omitted from
vfe_core_ops so subdev_do_ioctl() falls back to
v4l2_event_subdev_unsubscribe() only when V4L2_SUBDEV_FL_HAS_EVENTS is set?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028034105.326535-1-xiaqinghuan@mail.ustc.edu.cn?part=5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/9] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (4 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 5/9] media: camss: csid: report frame starts from CSID Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 3:41 ` [PATCH 7/9] media: camss: video: stop the subdevices already started when a start fails Qianhui Xu
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
The write master of the newer VFE generations can only be programmed with
a buffer whose payload is physically contiguous, because the frame
increment registers describe one linear address range. Check that the DMA
mapping of every plane is contiguous for at least sizeimage bytes and
reject the buffer otherwise, instead of silently writing past the end of
the first scatterlist entry.
Also let the SoC code add its own alignment requirements to a queued
buffer through a new prepare_buffer() video operation, which runs after
the generic checks.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../media/platform/qcom/camss/camss-video.c | 18 +++++++++++++++++-
.../media/platform/qcom/camss/camss-video.h | 1 +
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f1..027cf024f 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -155,7 +155,10 @@ static int video_buf_init(struct vb2_buffer *vb)
const struct v4l2_pix_format_mplane *format =
&video->active_fmt.fmt.pix_mp;
struct sg_table *sgt;
- unsigned int i;
+ struct scatterlist *sg;
+ dma_addr_t end;
+ unsigned int i, j;
+ size_t contiguous;
for (i = 0; i < format->num_planes; i++) {
sgt = vb2_dma_sg_plane_desc(vb, i);
@@ -163,6 +166,16 @@ static int video_buf_init(struct vb2_buffer *vb)
return -EFAULT;
buffer->addr[i] = sg_dma_address(sgt->sgl);
+ end = buffer->addr[i];
+ contiguous = 0;
+ for_each_sgtable_dma_sg(sgt, sg, j) {
+ if (sg_dma_address(sg) != end)
+ break;
+ contiguous += sg_dma_len(sg);
+ end += sg_dma_len(sg);
+ }
+ if (contiguous < format->plane_fmt[i].sizeimage)
+ return -EINVAL;
}
if (format->pixelformat == V4L2_PIX_FMT_NV12 ||
@@ -193,6 +206,9 @@ static int video_buf_prepare(struct vb2_buffer *vb)
vbuf->field = V4L2_FIELD_NONE;
+ if (video->ops->prepare_buffer)
+ return video->ops->prepare_buffer(video, vb);
+
return 0;
}
diff --git a/drivers/media/platform/qcom/camss/camss-video.h b/drivers/media/platform/qcom/camss/camss-video.h
index d3e56e240..af046ecb7 100644
--- a/drivers/media/platform/qcom/camss/camss-video.h
+++ b/drivers/media/platform/qcom/camss/camss-video.h
@@ -28,6 +28,7 @@ struct camss_buffer {
struct camss_video;
struct camss_video_ops {
+ int (*prepare_buffer)(struct camss_video *vid, struct vb2_buffer *vb);
int (*queue_buffer)(struct camss_video *vid, struct camss_buffer *buf);
int (*flush_buffers)(struct camss_video *vid,
enum vb2_buffer_state state);
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 7/9] media: camss: video: stop the subdevices already started when a start fails
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (5 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 6/9] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 3:41 ` [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem Qianhui Xu
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
When one subdevice fails to start, the ones already started further up the
pipeline were left streaming, and the next start would try to start them
again. Factor the teardown loop out of video_stop_streaming() into
video_stop_subdevices() and run it from the error path, stopping at the
subdevice that failed.
While at it, keep stopping the remaining subdevices when one of them fails
to stop instead of returning early: the pipeline is being torn down, so a
failure there must not leave the rest running.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../media/platform/qcom/camss/camss-video.c | 60 +++++++++----------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 027cf024f..ee9ffd043 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -265,11 +265,37 @@ static int video_prepare_streaming(struct vb2_queue *q)
return ret;
}
+/* Stop the started subdevices, excluding the one that failed to start. */
+static void video_stop_subdevices(struct camss_video *video,
+ struct media_entity *stop)
+{
+ struct media_entity *entity = &video->vdev.entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ while (entity != stop) {
+ pad = &entity->pads[0];
+ if (!(pad->flags & MEDIA_PAD_FL_SINK))
+ break;
+ pad = media_pad_remote_pad_first(pad);
+ if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
+ break;
+ entity = pad->entity;
+ if (entity == stop)
+ break;
+ subdev = media_entity_to_v4l2_subdev(entity);
+ ret = v4l2_subdev_call(subdev, video, s_stream, 0);
+ if (ret && ret != -ENOIOCTLCMD)
+ dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
+ }
+}
+
static int video_start_streaming(struct vb2_queue *q, unsigned int count)
{
struct camss_video *video = vb2_get_drv_priv(q);
struct video_device *vdev = &video->vdev;
- struct media_entity *entity;
+ struct media_entity *entity = &vdev->entity;
struct media_pad *pad;
struct v4l2_subdev *subdev;
int ret;
@@ -284,7 +310,6 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
if (ret < 0)
goto error;
- entity = &vdev->entity;
while (1) {
pad = &entity->pads[0];
if (!(pad->flags & MEDIA_PAD_FL_SINK))
@@ -305,6 +330,7 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
return 0;
error:
+ video_stop_subdevices(video, entity);
video_device_pipeline_stop(vdev);
flush_buffers:
@@ -316,35 +342,9 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
static void video_stop_streaming(struct vb2_queue *q)
{
struct camss_video *video = vb2_get_drv_priv(q);
- struct video_device *vdev = &video->vdev;
- struct media_entity *entity;
- struct media_pad *pad;
- struct v4l2_subdev *subdev;
- int ret;
-
- entity = &vdev->entity;
- while (1) {
- pad = &entity->pads[0];
- if (!(pad->flags & MEDIA_PAD_FL_SINK))
- break;
-
- pad = media_pad_remote_pad_first(pad);
- if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
- break;
-
- entity = pad->entity;
- subdev = media_entity_to_v4l2_subdev(entity);
-
- ret = v4l2_subdev_call(subdev, video, s_stream, 0);
-
- if (ret) {
- dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
- return;
- }
- }
-
- video_device_pipeline_stop(vdev);
+ video_stop_subdevices(video, NULL);
+ video_device_pipeline_stop(&video->vdev);
video->ops->flush_buffers(video, VB2_BUF_STATE_ERROR);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (6 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 7/9] media: camss: video: stop the subdevices already started when a start fails Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 4:00 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
2026-10-28 3:41 ` [PATCH 9/9] arm64: dts: qcom: sm8750: add the " Qianhui Xu
2026-10-29 9:37 ` [PATCH 0/9] media: camss: add SM8750 support Bryan O'Donoghue
9 siblings, 2 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
The VFE on SM8750 (gen4) keeps the write master interface, but the
registers move to a new base and the frame based interrupts are gone:
buffer done, register update completion and start of frame all come from
the CSID (see the 980 CSID support). Implement the block with the
existing vfe_buf_done()/vfe_frame_start() helpers, and require the DMA
address and the frame size to be 256-byte aligned, as the write master
takes both in 256-byte units.
Instantiate the sub-blocks for SM8750: six CSIPHY instances, two full
and two lite CSIDs, two full and one lite VFE, the interconnect paths,
the top-level resources and the compatible string.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../platform/qcom/camss/camss-vfe-gen4.c | 212 +++++++++++
drivers/media/platform/qcom/camss/camss.c | 357 ++++++++++++++++++
3 files changed, 570 insertions(+)
create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-gen4.c
diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index 348da1ccc..1ac787164 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -28,6 +28,7 @@ qcom-camss-objs += \
camss-vfe-680.o \
camss-vfe-gen1.o \
camss-vfe-gen3.o \
+ camss-vfe-gen4.o \
camss-vfe-vbif.o \
camss-video.o
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
new file mode 100644
index 000000000..2ee82898f
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-vfe-gen4.c
+ *
+ * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module gen4
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/interrupt.h>
+#include <linux/io.h>
+
+#include "camss.h"
+#include "camss-vfe.h"
+
+#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x200 : 0x800)
+
+#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08)
+#define WM_CGC_OVERRIDE_ALL (0x7FFFFFF)
+
+#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0x128)
+
+#define VFE_BUS_WM_CFG(n) (BUS_REG_BASE + 0x500 + (n) * 0x100)
+#define WM_CFG_EN BIT(0)
+#define WM_CFG_MODE BIT(16)
+#define VFE_BUS_WM_IMAGE_ADDR(n) (BUS_REG_BASE + 0x504 + (n) * 0x100)
+#define VFE_BUS_WM_FRAME_INCR(n) (BUS_REG_BASE + 0x508 + (n) * 0x100)
+#define VFE_BUS_WM_IMAGE_CFG_0(n) (BUS_REG_BASE + 0x50C + (n) * 0x100)
+#define WM_IMAGE_CFG_0_DEFAULT_WIDTH (0xFFFF)
+#define VFE_BUS_WM_IMAGE_CFG_2(n) (BUS_REG_BASE + 0x514 + (n) * 0x100)
+#define WM_IMAGE_CFG_2_DEFAULT_STRIDE (0xFFFF)
+#define VFE_BUS_WM_PACKER_CFG(n) (BUS_REG_BASE + 0x518 + (n) * 0x100)
+
+#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n) (BUS_REG_BASE + 0x530 + (n) * 0x100)
+#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n) (BUS_REG_BASE + 0x534 + (n) * 0x100)
+
+/* VFE lite has no such registers */
+#define VFE_BUS_WM_FRAMEDROP_PERIOD(n) (BUS_REG_BASE + 0x538 + (n) * 0x100)
+#define VFE_BUS_WM_FRAMEDROP_PATTERN(n) (BUS_REG_BASE + 0x53C + (n) * 0x100)
+
+#define VFE_BUS_WM_MMU_PREFETCH_CFG(n) (BUS_REG_BASE + 0x560 + (n) * 0x100)
+#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n) (BUS_REG_BASE + 0x564 + (n) * 0x100)
+
+/*
+ * IFE write master client IDs
+ *
+ * VIDEO_FULL 0
+ * VIDEO_DC4_Y 1
+ * VIDEO_DC4_C 2
+ * VIDEO_DC16_Y 3
+ * VIDEO_DC16_C 4
+ * DISPLAY_DS2_Y 5
+ * DISPLAY_DS2_C 6
+ * FD_Y 7
+ * FD_C 8
+ * IR_OUT 9
+ * STATS_AEC_BG 10
+ * STATS_AEC_BHIST 11
+ * STATS_TINTLESS_BG 12
+ * STATS_AWB_BG 13
+ * STATS_AWB_BFW 14
+ * STATS_AF_BHIST 15
+ * STATS_ALSC_BG 16
+ * STATS_FLICKER_BAYERRS 17
+ * STATS_TMC_BHIST 18
+ * PDAF_0 19
+ * PDAF_1 20
+ * PDAF_2 21
+ * PDAF_3 22
+ * RDI0 23
+ * RDI1 24
+ * RDI2 25
+ * RDI3 26
+ * RDI4 27
+ *
+ * IFE Lite write master client IDs
+ *
+ * RDI0 0
+ * RDI1 1
+ * RDI2 2
+ * RDI3 3
+ * GAMMA 4
+ * STATES_BE 5
+ */
+#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0x0 : 0x17) + (n))
+
+static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
+{
+ struct v4l2_pix_format_mplane *pix =
+ &line->video_out.active_fmt.fmt.pix_mp;
+
+ wm = RDI_WM(wm);
+
+ /* no clock gating at bus input */
+ writel(WM_CGC_OVERRIDE_ALL, vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
+
+ writel(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL);
+
+ writel(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8,
+ vfe->base + VFE_BUS_WM_FRAME_INCR(wm));
+ writel((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF),
+ vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm));
+ writel(WM_IMAGE_CFG_2_DEFAULT_STRIDE,
+ vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm));
+ writel(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm));
+
+ /* no dropped frames, one irq per frame */
+ if (!vfe_is_lite(vfe)) {
+ writel(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm));
+ writel(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm));
+ }
+
+ writel(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm));
+ writel(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm));
+
+ writel(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm));
+ writel(0xFFFFFFFF, vfe->base + VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm));
+
+ writel(WM_CFG_EN | WM_CFG_MODE, vfe->base + VFE_BUS_WM_CFG(wm));
+}
+
+static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
+{
+ wm = RDI_WM(wm);
+ writel(0, vfe->base + VFE_BUS_WM_CFG(wm));
+}
+
+static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
+ struct vfe_line *line)
+{
+ wm = RDI_WM(wm);
+ writel(addr >> 8, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
+}
+
+static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
+{
+ int port_id = line_id;
+
+ camss_reg_update(vfe->camss, vfe->id, port_id, false);
+}
+
+static inline void vfe_reg_update_clear(struct vfe_device *vfe,
+ enum vfe_line_id line_id)
+{
+ int port_id = line_id;
+
+ camss_reg_update(vfe->camss, vfe->id, port_id, true);
+}
+
+static int vfe_prepare_buffer(struct camss_video *video, struct vb2_buffer *vb)
+{
+ struct camss_buffer *buf = container_of(to_vb2_v4l2_buffer(vb),
+ struct camss_buffer, vb);
+ struct v4l2_pix_format_mplane *pix = &video->active_fmt.fmt.pix_mp;
+
+ /* Image addresses and frame increments are expressed in 256-byte units. */
+ if (!IS_ALIGNED(buf->addr[0], 256) ||
+ !IS_ALIGNED(pix->plane_fmt[0].bytesperline * pix->height, 256))
+ return -EINVAL;
+
+ return 0;
+}
+
+static const struct camss_video_ops vfe_video_ops_gen4 = {
+ .prepare_buffer = vfe_prepare_buffer,
+ .queue_buffer = vfe_queue_buffer_v2,
+ .flush_buffers = vfe_flush_buffers,
+};
+
+static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
+{
+ unsigned int i;
+
+ vfe->video_ops = vfe_video_ops_gen4;
+ for (i = 0; i < vfe->res->line_num; i++)
+ vfe->line[i].video_out.vb2_q.min_queued_buffers = CAMSS_INIT_BUF_COUNT;
+}
+
+static void vfe_global_reset(struct vfe_device *vfe)
+{
+ vfe_isr_reset_ack(vfe);
+}
+
+static irqreturn_t vfe_isr(int irq, void *dev)
+{
+ /* nop */
+ return IRQ_HANDLED;
+}
+
+static int vfe_halt(struct vfe_device *vfe)
+{
+ /* rely on vfe_disable_output() to stop the VFE */
+ return 0;
+}
+
+const struct vfe_hw_ops vfe_ops_gen4 = {
+ .global_reset = vfe_global_reset,
+ .hw_version = vfe_hw_version,
+ .isr = vfe_isr,
+ .pm_domain_off = vfe_pm_domain_off,
+ .pm_domain_on = vfe_pm_domain_on,
+ .reg_update = vfe_reg_update,
+ .reg_update_clear = vfe_reg_update_clear,
+ .subdev_init = vfe_subdev_init,
+ .vfe_disable = vfe_disable,
+ .vfe_enable = vfe_enable_v2,
+ .vfe_halt = vfe_halt,
+ .vfe_wm_start = vfe_wm_start,
+ .vfe_wm_stop = vfe_wm_stop,
+ .vfe_buf_done = vfe_buf_done,
+ .frame_start = vfe_frame_start,
+ .vfe_wm_update = vfe_wm_update,
+};
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 92d536986..b70fbff0f 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4187,6 +4187,349 @@ static const struct resources_icc icc_res_sa8775p[] = {
},
};
+static const struct camss_subdev_resources csiphy_res_8750[] = {
+ /* CSIPHY0 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy0-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy0-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy0", "csiphy0_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy0" },
+ .interrupt = { "csiphy0" },
+ .csiphy = {
+ .id = 0,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+ /* CSIPHY1 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy1-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy1-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy1", "csiphy1_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy1" },
+ .interrupt = { "csiphy1" },
+ .csiphy = {
+ .id = 1,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+ /* CSIPHY2 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy2-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy2-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy2", "csiphy2_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy2" },
+ .interrupt = { "csiphy2" },
+ .csiphy = {
+ .id = 2,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+ /* CSIPHY3 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy3-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy3-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy3", "csiphy3_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy3" },
+ .interrupt = { "csiphy3" },
+ .csiphy = {
+ .id = 3,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+ /* CSIPHY4 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy4-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy4-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy4", "csiphy4_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy4" },
+ .interrupt = { "csiphy4" },
+ .csiphy = {
+ .id = 4,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+ /* CSIPHY5 */
+ {
+ .regulators = {
+ { .supply = "vdd-csiphy5-0p9", .init_load_uA = 148000 },
+ { .supply = "vdd-csiphy5-1p2", .init_load_uA = 14660 }
+ },
+ .clock = { "csiphy5", "csiphy5_timer",
+ "cpas_ahb", "cpas_fast_ahb" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "csiphy5" },
+ .interrupt = { "csiphy5" },
+ .csiphy = {
+ .id = 5,
+ .hw_ops = &csiphy_ops_3ph_1_0,
+ .formats = &csiphy_formats_sdm845
+ }
+ },
+};
+
+static const struct camss_subdev_resources csid_res_8750[] = {
+ /* CSID0 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid0" },
+ .interrupt = { "csid0" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID1 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid1" },
+ .interrupt = { "csid1" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID2 */
+ {
+ .clock = { "csid", "csid_csiphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid2" },
+ .interrupt = { "csid2" },
+ .csid = {
+ .is_lite = false,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID_LITE0 */
+ {
+ .clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid_lite0" },
+ .interrupt = { "csid_lite0" },
+ .csid = {
+ .is_lite = true,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ },
+ /* CSID_LITE1 */
+ {
+ .clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
+ .clock_rate = { { 400000000, 480000000 },
+ { 400000000, 480000000 } },
+ .reg = { "csid_lite1" },
+ .interrupt = { "csid_lite1" },
+ .csid = {
+ .is_lite = true,
+ .parent_dev_ops = &vfe_parent_dev_ops,
+ .hw_ops = &csid_ops_980,
+ .formats = &csid_formats_gen2
+ }
+ }
+};
+
+static const struct camss_subdev_resources vfe_res_8750[] = {
+ /* VFE0 - TFE Full */
+ {
+ .clock = { "gcc_axi_hf", "vfe0_fast_ahb", "vfe0",
+ "cpas_vfe0", "cpas_vfe1", "cpas_vfe2",
+ "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 360280000, 480000000, 630000000, 716000000,
+ 833000000 },
+ { 0 },
+ { 0 },
+ { 0 },
+ { 200000000, 300000000, 400000000, 480000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "vfe0" },
+ .interrupt = { "vfe0" },
+ .vfe = {
+ .line_num = 3,
+ .is_lite = false,
+ .reg_update_after_csid_config = true,
+ .has_pd = true,
+ .pd_name = "ife0",
+ .hw_ops = &vfe_ops_gen4,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+ /* VFE1 - TFE Full */
+ {
+ .clock = { "gcc_axi_hf", "vfe1_fast_ahb", "vfe1",
+ "cpas_vfe0", "cpas_vfe1", "cpas_vfe2",
+ "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 360280000, 480000000, 630000000, 716000000,
+ 833000000 },
+ { 0 },
+ { 0 },
+ { 0 },
+ { 200000000, 300000000, 400000000, 480000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "vfe1" },
+ .interrupt = { "vfe1" },
+ .vfe = {
+ .line_num = 3,
+ .is_lite = false,
+ .reg_update_after_csid_config = true,
+ .has_pd = true,
+ .pd_name = "ife1",
+ .hw_ops = &vfe_ops_gen4,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+ /* VFE2 - TFE Full */
+ {
+ .clock = { "gcc_axi_hf", "vfe2_fast_ahb", "vfe2",
+ "cpas_vfe0", "cpas_vfe1", "cpas_vfe2",
+ "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 360280000, 480000000, 630000000, 716000000,
+ 833000000 },
+ { 0 },
+ { 0 },
+ { 0 },
+ { 200000000, 300000000, 400000000, 480000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "vfe2" },
+ .interrupt = { "vfe2" },
+ .vfe = {
+ .line_num = 3,
+ .is_lite = false,
+ .reg_update_after_csid_config = true,
+ .has_pd = true,
+ .pd_name = "ife2",
+ .hw_ops = &vfe_ops_gen4,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+ /* VFE_LITE0 */
+ {
+ .clock = { "gcc_axi_hf", "vfe_lite_ahb", "vfe_lite",
+ "cpas_vfe_lite", "camnoc_rt_axi",
+ "camnoc_nrt_axi", "qdss_debug_xo" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 266666667, 400000000, 480000000 },
+ { 0 },
+ { 200000000, 300000000, 400000000, 480000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "vfe_lite0" },
+ .interrupt = { "vfe_lite0" },
+ .vfe = {
+ .line_num = 4,
+ .is_lite = true,
+ .reg_update_after_csid_config = true,
+ .hw_ops = &vfe_ops_gen4,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ },
+ /* VFE_LITE1 */
+ {
+ .clock = { "gcc_axi_hf", "vfe_lite_ahb", "vfe_lite",
+ "cpas_vfe_lite", "camnoc_rt_axi",
+ "camnoc_nrt_axi", "qdss_debug_xo" },
+ .clock_rate = { { 0 },
+ { 0 },
+ { 266666667, 400000000, 480000000 },
+ { 0 },
+ { 200000000, 300000000, 400000000, 480000000 },
+ { 0 },
+ { 0 } },
+ .reg = { "vfe_lite1" },
+ .interrupt = { "vfe_lite1" },
+ .vfe = {
+ .line_num = 4,
+ .is_lite = true,
+ .reg_update_after_csid_config = true,
+ .hw_ops = &vfe_ops_gen4,
+ .formats_rdi = &vfe_formats_rdi_845,
+ .formats_pix = &vfe_formats_pix_845
+ }
+ }
+};
+
+static const struct resources_icc icc_res_sm8750[] = {
+ {
+ .name = "ahb",
+ .icc_bw_tbl.avg = 150000,
+ .icc_bw_tbl.peak = 300000,
+ },
+ /* Based on 4096 x 3072 30 FPS 2496 Mbps mode */
+ {
+ .name = "hf_mnoc",
+ .icc_bw_tbl.avg = 471860,
+ .icc_bw_tbl.peak = 925857,
+ },
+};
+
static const struct camss_subdev_resources csiphy_res_x1e80100[] = {
/* CSIPHY0 */
{
@@ -5735,6 +6078,19 @@ static const struct camss_resources sm8650_resources = {
.vfe_num = ARRAY_SIZE(vfe_res_sm8650),
};
+static const struct camss_resources sm8750_resources = {
+ .version = CAMSS_8750,
+ .pd_name = "top",
+ .csiphy_res = csiphy_res_8750,
+ .csid_res = csid_res_8750,
+ .vfe_res = vfe_res_8750,
+ .icc_res = icc_res_sm8750,
+ .csiphy_num = ARRAY_SIZE(csiphy_res_8750),
+ .csid_num = ARRAY_SIZE(csid_res_8750),
+ .vfe_num = ARRAY_SIZE(vfe_res_8750),
+ .icc_path_num = ARRAY_SIZE(icc_res_sm8750),
+};
+
static const struct camss_resources x1e80100_resources = {
.version = CAMSS_X1E80100,
.pd_name = "top",
@@ -5769,6 +6125,7 @@ static const struct of_device_id camss_dt_match[] = {
{ .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
{ .compatible = "qcom,sm8550-camss", .data = &sm8550_resources },
{ .compatible = "qcom,sm8650-camss", .data = &sm8650_resources },
+ { .compatible = "qcom,sm8750-camss", .data = &sm8750_resources },
{ .compatible = "qcom,x1e80100-camss", .data = &x1e80100_resources },
{ }
};
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem
2026-10-28 3:41 ` [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem Qianhui Xu
@ 2026-10-28 4:00 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-28 4:00 UTC (permalink / raw)
To: Qianhui Xu; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: camss: race on CSID reg_update state between vfe_reg_update() and csid_isr()
--
commit 9a87e4e329c131d366318a8a449031db90e2ea2c
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
media: camss: vfe: add the gen4 block and SM8750 camera subsystem
This commit adds support for the gen4 VFE block, where buffer done, register
update completion, and start of frame events come from the 980 CSID, and
requires 256-byte alignment for DMA addresses and frame sizes. It also
instantiates the CSIPHY, CSID, VFE, interconnect, and top-level resources for
SM8750.
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
> new file mode 100644
> index 0000000000000..2ee82898f37dc
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
> @@ -0,0 +1,212 @@
[ ... ]
> +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
> +{
> + int port_id = line_id;
> +
> + camss_reg_update(vfe->camss, vfe->id, port_id, false);
> +}
> +
> +static inline void vfe_reg_update_clear(struct vfe_device *vfe,
> + enum vfe_line_id line_id)
> +{
> + int port_id = line_id;
> +
> + camss_reg_update(vfe->camss, vfe->id, port_id, true);
> +}
[Severity: Medium]
Can vfe_reg_update() race with csid_isr() on csid->aup_update,
csid->rup_update, and csid->reg_update?
When an active stream has fewer than two buffers in hardware
(output->gen2.active_num < 2) and userspace queues a buffer via VIDIOC_QBUF,
vfe_queue_buffer_v2() acquires vfe->output_lock and calls into CSID 980:
vfe_queue_buffer_v2()
vfe_reg_update()
camss_reg_update(..., false)
csid_subdev_reg_update(..., false)
__csid_aup_update()
In drivers/media/platform/qcom/camss/camss-csid-980.c, __csid_aup_update()
modifies csid->reg_update or csid->aup_update and then re-reads the field to
write the command register:
if (csid_is_lite(csid)) {
csid->reg_update |= CSID_LITE_AUP_RDI(port_id);
writel(csid->reg_update, csid->base + CSID_LITE_RUP_AUP_CMD);
} else {
csid->aup_update |= CSID_RUP_AUP_RDI(port_id);
writel(csid->aup_update, csid->base + CSID_AUP_CMD);
__csid_full_aup_rup_trigger(csid);
}
Concurrently on another CPU, csid_isr() in camss-csid-980.c handles
INFO_RUP_DONE without holding vfe->output_lock or any CSID lock:
csid_isr()
if (val & INFO_RUP_DONE)
csid_subdev_reg_update(csid, i, true)
__csid_aup_rup_clear()
where __csid_aup_rup_clear() performs a non-atomic read-modify-write:
if (csid_is_lite(csid)) {
csid->reg_update &= ~CSID_LITE_RUP_RDI(port_id);
csid->reg_update &= ~CSID_LITE_AUP_RDI(port_id);
} else {
csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id);
csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id);
}
If __csid_aup_rup_clear() clears the bit between the |= and the writel() in
__csid_aup_update() (or races across RDI ports on the same CSID when
vfe_enable_output_v2() or vfe_queue_buffer_v2() updates one RDI line while
csid_isr() clears another), won't the writel() drop the pending AUP command
or write a stale RDI port update bit, stalling frame capture on that stream?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028034105.326535-1-xiaqinghuan@mail.ustc.edu.cn?part=8
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem
2026-10-28 3:41 ` [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem Qianhui Xu
2026-10-28 4:00 ` sashiko-bot
@ 2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: Shuai Liu @ 2026-10-29 7:54 UTC (permalink / raw)
To: linux-media, linux-arm-msm
Cc: Shuai Liu, Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
Hangxiang Ma, Qianhui Xu, devicetree, linux-kernel
Hi Qianhui,
The gen4 write master programming itself matches the gen3 conventions
(the FRAME_INCR >> 8 formula, the 256-byte unit addressing, RDI_WM()
offsets), so only three things stand out:
1. The interconnect table does not match the binding
The binding in patch 1 and the DT in patch 9 declare four interconnect
paths for sm8750-camss:
ahb, hf_mnoc, sf_mnoc, sf_icp_mnoc
but icc_res_sm8750[] only contains two entries (ahb, hf_mnoc), and
camss requests paths by the names in this table, so the sf_mnoc and
sf_icp_mnoc paths never receive any bandwidth vote. For reference,
sm8650 keeps the binding and the driver in sync: both declare exactly
ahb and hf_mnoc.
Please either add the missing entries with real bandwidth numbers, or
reduce the binding and DTS to the two paths the driver actually votes
on. As posted, the extra DT paths are dead weight and the bandwidth
they are supposed to guarantee is never requested.
2. The commit message miscounts the instances
The message says "two full and two lite CSIDs" and "two full and one
lite VFEs", but the binding, the DTS and this patch's own resource
tables all describe three full CSIDs (csid0/1/2) plus two lite
(csid_lite0/1), and three full VFEs (vfe0/1/2) plus two lite
(vfe_lite0/1). Patch 1/9's message has the same numbers. Please fix
the commit messages.
3. vfe_prepare_buffer() rejects formats at buffer-queue time
The 256-byte alignment of pix->plane_fmt[0].bytesperline * pix->height
is a property of the negotiated format, not of a particular buffer.
Format negotiation does not guarantee it: camss_format aligns
bytesperline per format (5 bytes for RAW10) and height is not aligned
at all, so plenty of resolutions pass try_fmt/s_fmt and then fail every
QBUF with -EINVAL and no explanation.
Suggest enforcing the constraint where the format is chosen: either
round bytesperline up to a 256-byte alignment for gen4 (which makes the
payload 256-aligned for any height), or reject unaligned formats in
try_fmt/s_fmt with a clear message, and keep only the per-buffer
address alignment check in prepare_buffer().
Thanks,
Shuai Liu
(This review was prepared with the assistance of an AI coding tool;
all findings were verified against the patched source.)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 9/9] arm64: dts: qcom: sm8750: add the camera subsystem
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (7 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 8/9] media: camss: vfe: add the gen4 block and SM8750 camera subsystem Qianhui Xu
@ 2026-10-28 3:41 ` Qianhui Xu
2026-10-28 4:06 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
2026-10-29 9:37 ` [PATCH 0/9] media: camss: add SM8750 support Bryan O'Donoghue
9 siblings, 2 replies; 20+ messages in thread
From: Qianhui Xu @ 2026-10-28 3:41 UTC (permalink / raw)
To: 'Bryan O'Donoghue', Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
Describe the camera subsystem as the single platform device the camss
driver binds to: the SM8750 block has four CSID, six CSIPHY, three VFE and
two VFE lite instances, all behind one register window, one clock and one
power domain list, with one interrupt per instance.
The node is left disabled; boards enable it and describe their sensors and
CSIPHY ports on top.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
arch/arm64/boot/dts/qcom/sm8750.dtsi | 179 +++++++++++++++++++++++++++
1 file changed, 179 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index dd738d13d..12dcf2f33 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3495,6 +3495,185 @@ dispcc: clock-controller@af00000 {
#power-domain-cells = <1>;
};
+ camss: isp@ad27000 {
+ compatible = "qcom,sm8750-camss";
+
+ reg = <0x0 0x0ad27000 0x0 0x2b00>,
+ <0x0 0x0ad2a000 0x0 0x2b00>,
+ <0x0 0x0ad2d000 0x0 0x2b00>,
+ <0x0 0x0ad6d000 0x0 0xa00>,
+ <0x0 0x0ad72000 0x0 0xa00>,
+ <0x0 0x0ada9000 0x0 0x2000>,
+ <0x0 0x0adab000 0x0 0x2000>,
+ <0x0 0x0adad000 0x0 0x2000>,
+ <0x0 0x0adaf000 0x0 0x2000>,
+ <0x0 0x0adb1000 0x0 0x2000>,
+ <0x0 0x0adb3000 0x0 0x2000>,
+ <0x0 0x0ad8b000 0x0 0x400>,
+ <0x0 0x0ad8c000 0x0 0x400>,
+ <0x0 0x0ad8d000 0x0 0x400>,
+ <0x0 0x0ac86000 0x0 0x10000>,
+ <0x0 0x0ac96000 0x0 0x10000>,
+ <0x0 0x0aca6000 0x0 0x10000>,
+ <0x0 0x0ad6e000 0x0 0x3000>,
+ <0x0 0x0ad73000 0x0 0x3000>;
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "csitpg0",
+ "csitpg1",
+ "csitpg2",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ clocks = <&camcc CAM_CC_CAMNOC_NRT_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_AXI_CLK>,
+ <&camcc CAM_CC_CAM_TOP_AHB_CLK>,
+ <&camcc CAM_CC_CAM_TOP_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_CAMNOC_RT_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_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>,
+ <&camcc CAM_CC_CSIPHY3_CLK>,
+ <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY5_CLK>,
+ <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_0_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_1_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_CLK>,
+ <&camcc CAM_CC_TFE_2_MAIN_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+ <&camcc CAM_CC_QDSS_DEBUG_XO_CLK>;
+ clock-names = "camnoc_nrt_axi",
+ "camnoc_rt_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe2",
+ "cpas_vfe_lite",
+ "csid",
+ "csid_csiphy_rx",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy3",
+ "csiphy3_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "csiphy5",
+ "csiphy5_timer",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe2",
+ "vfe2_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid",
+ "qdss_debug_xo";
+
+ interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 376 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 448 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 457 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "vfe0",
+ "vfe1",
+ "vfe2",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+ <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_CAMNOC_NRT_ICP_SF QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "ahb",
+ "hf_mnoc",
+ "sf_mnoc",
+ "sf_icp_mnoc";
+
+ iommus = <&apps_smmu 0x1c00 0x00>;
+
+ power-domains = <&camcc CAM_CC_TFE_0_GDSC>,
+ <&camcc CAM_CC_TFE_1_GDSC>,
+ <&camcc CAM_CC_TFE_2_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+ power-domain-names = "ife0",
+ "ife1",
+ "ife2",
+ "top";
+
+ dma-coherent;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
camcc: clock-controller@ade0000 {
compatible = "qcom,sm8750-camcc";
reg = <0x0 0xade0000 0x0 0x20000>;
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 9/9] arm64: dts: qcom: sm8750: add the camera subsystem
2026-10-28 3:41 ` [PATCH 9/9] arm64: dts: qcom: sm8750: add the " Qianhui Xu
@ 2026-10-28 4:06 ` sashiko-bot
2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-28 4:06 UTC (permalink / raw)
To: Qianhui Xu; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: camss: csid_set_stream() skips configure_stream() on stop and restart
--
commit d503f6b5ea6cdbb52ee98c8873d6ffd4c3dc7c21
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
arm64: dts: qcom: sm8750: add the camera subsystem
This commit adds the camera subsystem (camss) device tree node for the
Qualcomm SM8750 SoC, describing the CSID, CSIPHY, and VFE instances along
with their registers, clocks, interrupts, interconnects, and power domains.
> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index dd738d13df8e0..12dcf2f33854a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -3495,6 +3495,185 @@ dispcc: clock-controller@af00000 {
> #power-domain-cells = <1>;
> };
>
> + camss: isp@ad27000 {
> + compatible = "qcom,sm8750-camss";
[Severity: High]
Can stopping a stream or unwinding a failed stream start on SM8750 leave the
CSID RDI stream enabled with interrupts unmasked, and skip stream
configuration on a subsequent start?
In drivers/media/platform/qcom/camss/camss-csid.c, csid_set_stream() gates
csid->res->hw_ops->configure_stream() on csid->phy.need_vc_update and clears
that flag on the first call:
drivers/media/platform/qcom/camss/camss-csid.c:csid_set_stream() {
...
if (csid->phy.need_vc_update) {
csid->res->hw_ops->configure_stream(csid, enable);
csid->phy.need_vc_update = false;
}
return 0;
}
Because csid->phy.need_vc_update is only set in csid_set_power() (on power-up)
and csid_link_setup(), it is already false when streaming stops via
video_stop_streaming() -> video_stop_subdevices() or when
video_start_streaming() unwinds already-started subdevices on error:
drivers/media/platform/qcom/camss/camss-video.c:video_start_streaming() {
...
error:
video_stop_subdevices(video, entity);
video_device_pipeline_stop(vdev);
...
}
When v4l2_subdev_call(subdev, video, s_stream, 0) invokes
csid_set_stream(sd, 0), csid_configure_stream(csid, 0) in
drivers/media/platform/qcom/camss/camss-csid-980.c is skipped:
drivers/media/platform/qcom/camss/camss-csid-980.c:csid_configure_stream() {
u8 i, k;
__csid_configure_rx(csid, &csid->phy);
for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_980; i++) {
if (csid->phy.en_vc & BIT(i)) {
u32 mask = readl(csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
writel(INFO_CAMIF_SOF, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
mask = enable ? mask | INFO_CAMIF_SOF : mask & ~INFO_CAMIF_SOF;
writel(mask, csid->base + CSID_CSI2_RDIN_IRQ_MASK(i));
mask = readl(csid->base + CSID_TOP_IRQ_MASK);
mask = enable ? mask | CSID_RDI_TOP_IRQ(i) : mask & ~CSID_RDI_TOP_IRQ(i);
writel(mask, csid->base + CSID_TOP_IRQ_MASK);
writel(IRQ_CMD_CLEAR, csid->base + CSID_IRQ_CMD);
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_configure_rx_vc(csid, 0);
for (k = 0; enable && k < CAMSS_INIT_BUF_COUNT; k++) {
__csid_aup_update(csid, i);
__csid_rup_update(csid, i);
}
__csid_ctrl_rdi(csid, enable, i);
}
}
}
As a result, INFO_CAMIF_SOF in CSID_CSI2_RDIN_IRQ_MASK(i) and
CSID_RDI_TOP_IRQ(i) in CSID_TOP_IRQ_MASK are not masked, and
__csid_ctrl_rdi(csid, 0, i) is never called to stop the RDI stream.
In addition, if streaming is restarted without an intervening CSID power
cycle or link change (for example, if video_start_streaming() fails in CSIPHY
or sensor s_stream(1) after CSID s_stream(1) has already run when
VIDIOC_STREAMON was called before min_queued_buffers were queued and a later
VIDIOC_QBUF triggered vb2_start_streaming(), or while another active VFE line
keeps the CSID entity powered), csid->phy.need_vc_update remains false.
On that subsequent start attempt, vfe_enable_output_v2() in
drivers/media/platform/qcom/camss/camss-vfe.c skips ops->reg_update() because
vfe->res->reg_update_after_csid_config is true on SM8750:
drivers/media/platform/qcom/camss/camss-vfe.c:vfe_enable_output_v2() {
...
if (!vfe->res->reg_update_after_csid_config)
ops->reg_update(vfe, line->id);
}
}
And because csid_set_stream(sd, 1) also skips csid_configure_stream(csid, 1),
the initial CAMSS_INIT_BUF_COUNT calls to __csid_aup_update() and
__csid_rup_update() are never issued, leaving the initial buffer addresses
unlatched and stalling capture.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028034105.326535-1-xiaqinghuan@mail.ustc.edu.cn?part=9
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 9/9] arm64: dts: qcom: sm8750: add the camera subsystem
2026-10-28 3:41 ` [PATCH 9/9] arm64: dts: qcom: sm8750: add the " Qianhui Xu
2026-10-28 4:06 ` sashiko-bot
@ 2026-10-29 7:54 ` Shuai Liu
1 sibling, 0 replies; 20+ messages in thread
From: Shuai Liu @ 2026-10-29 7:54 UTC (permalink / raw)
To: linux-media, linux-arm-msm
Cc: Shuai Liu, Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
Hangxiang Ma, Qianhui Xu, devicetree, linux-kernel
Hi Qianhui,
The node itself matches the binding from patch 1: 19 register ranges,
35 clocks, 16 interrupts, four interconnects and four power domains all
line up with qcom,sm8750-camss.yaml, so from a schema point of view
this looks right.
One nit in the commit message:
> the SM8750 block has four CSID, six CSIPHY, three VFE and
> two VFE lite instances
The node describes five CSIDs (csid0, csid1, csid2, csid_lite0,
csid_lite1), not four - three full plus two lite. The VFE counts
(three full, two lite) are correct. Patch 1/9 and 8/9 carry the same
miscount in their commit messages.
Minor: the node declares the camera subsystem as "isp@ad27000". The
existing qcom camss nodes use "camss@" or "isp@" inconsistently, so
this is fine, just noting it for the record since the binding example
matches.
Thanks,
Shuai Liu
(This review was prepared with the assistance of an AI coding tool;
all findings were verified against the patched source.)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/9] media: camss: add SM8750 support
2026-10-28 3:40 [PATCH 0/9] media: camss: add SM8750 support Qianhui Xu
` (8 preceding siblings ...)
2026-10-28 3:41 ` [PATCH 9/9] arm64: dts: qcom: sm8750: add the " Qianhui Xu
@ 2026-10-29 9:37 ` Bryan O'Donoghue
9 siblings, 0 replies; 20+ messages in thread
From: Bryan O'Donoghue @ 2026-10-29 9:37 UTC (permalink / raw)
To: Qianhui Xu, Mauro Carvalho Chehab
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Hangxiang Ma, linux-media, linux-arm-msm,
devicetree, linux-kernel
Hi.
Thanks for your series but, I'd like to proceed with Hanxiang's
contribution.
If there are further fixes you'd like to submit on top of that enabling
series, please feel free to do so.
---
bod
^ permalink raw reply [flat|nested] 20+ messages in thread