Hi Vikram,
On Mon Nov 4, 2024 at 10:08 AM CET, Vikram Sharma wrote:
> Hi Luca,
>
> Thanks for your review and your efforts to validate tpg for this
> Please find my comments inline.
>
> Regards,
> Vikram
>
> On 11/1/2024 8:57 PM, Luca Weiss wrote:
> > On Wed Oct 30, 2024 at 11:53 AM CET, Vikram Sharma wrote:
> >> SC7280 is a Qualcomm SoC. This series adds support to bring up the CSIPHY,
> >> CSID, VFE/RDI interfaces in SC7280.
> >>
> >> SC7280 provides
> >>
> >> - 3 x VFE, 3 RDI per VFE
> >> - 2 x VFE Lite, 4 RDI per VFE
> >> - 3 x CSID
> >> - 2 x CSID Lite
> >> - 5 x CSI PHY
> >>
> >> The changes are verified on SC7280 qcs6490-rb3gen2 board, with attached vision mezzanine
> >> the base dts for qcs6490-rb3gen2 is:
> >> https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@xxxxxxxxxxx/
> > Hi Vikram!
> >
> > Two things:
> >
> > You use the property "power-domains-names" in both bindings and dtsi but
> > this property is never parsed in the kernel. This should be
> > "power-domain-names"
> Thanks for this catch. I will update this in next version.
> >
> > Second, I still can't get the test pattern to work on my QCM6490-based
> > phone (Fairphone 5). Could you please try if the commands as per [0]
> > work on your board?
> I verified TPG with this change. but only mode 8 and 9 are working for TPG.
> Rest of the modes are getting stuck. To debug this I have verified the
> register dump for all the TPG registers in all the modes.
> as expected the register programming is similar for all the modes just
> mode field is getting updated.
> We are discussing this issue with TPG hardware team. Will keep you posted.
>
> Steps I used to validate TPG:
>
> echo "* Reset pipeline"
> media-ctl-d**/dev/media0**--reset
> yavta --list /dev/v4l-subdev5
>
> # /dev/v4l-subdev5 represent CSID0 can be different as well.
> yavta --no-query -w '0x009f0903*8*' /dev/v4l-subdev5
>
> media-ctl -d/dev/media0*-*V '"msm_csid0":0[fmt:SRGGB10/4056x3040]'
>
> media-ctl-d/dev/media0**-V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]'
>
> media-ctl-d/dev/media0**-l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
>
> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0
Thanks for checking, I can confirm I see the same behavior on my board,
test pattern 8 and 9 work as expected.
This seems to be some issue with the clocks since taking the clock
definitions from older sc7280 camss patches (never sent to the list)
test patterns 1-7 work properly.
Please see attached patch on top of yours with which it works. I haven't
dug into it more to find out which exact clock is causing this, but this
should be a good starting point for you & the hardware team.
Regards
Luca
>
> >
> > [0]https://lore.kernel.org/linux-arm-msm/c912f2da-519c-4bdc-a5cb-e19c3aa63ea8@xxxxxxxxxx/
> >
> > Regards
> > Luca
> >
> >> Changes in V4:
> >> - V3 had 8 patches and V4 is reduced to 6.
> >> - Removed [Patch v3 2/8] as binding change is not required for dtso.
> >> - Removed [Patch v3 3/8] as the fix is already taken care in latest
> >> kernel tip.
> >> - Updated alignment for dtsi and dt-bindings.
> >> - Adding qcs6490-rb3gen2-vision-mezzanine as overlay.
> >> - Link to v3:https://lore.kernel.org/linux-arm-msm/20241011140932.1744124-1-quic_vikramsa@xxxxxxxxxxx/
> >>
> >> Changes in V3:
> >> - Added missed subject line for cover letter of V2.
> >> - Updated Alignment, indentation and properties order.
> >> - edit commit text for [PATCH 02/10] and [PATCH 03/10].
> >> - Refactor camss_link_entities.
> >> - Removed camcc enablement changes as it already done.
> >> - Link to v2:https://lore.kernel.org/linux-arm-msm/20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-v1-0-b18ddcd7d9df@xxxxxxxxxxx/
> >>
> >> Changes in V2:
> >> - Improved indentation/formatting.
> >> - Removed _src clocks and misleading code comments.
> >> - Added name fields for power domains and csid register offset in DTSI.
> >> - Dropped minItems field from YAML file.
> >> - Listed changes in alphabetical order.
> >> - Updated description and commit text to reflect changes
> >> - Changed the compatible string from imx412 to imx577.
> >> - Added board-specific enablement changes in the newly created vision
> >> board DTSI file.
> >> - Fixed bug encountered during testing.
> >> - Moved logically independent changes to a new/seprate patch.
> >> - Removed cci0 as no sensor is on this port and MCLK2, which was a
> >> copy-paste error from the RB5 board reference.
> >> - Added power rails, referencing the RB5 board.
> >> - Discarded Patch 5/6 completely (not required).
> >> - Removed unused enums.
> >> - Link to v1:https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@xxxxxxxxxxx/
> >>
> >> Suresh Vankadara (1):
> >> media: qcom: camss: Add support for camss driver on SC7280
> >>
> >> Vikram Sharma (5):
> >> media: dt-bindings: media: camss: Add qcom,sc7280-camss binding
> >> media: qcom: camss: Sort CAMSS version enums and compatible strings
> >> media: qcom: camss: Restructure camss_link_entities
> >> arm64: dts: qcom: sc7280: Add support for camss
> >> arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Add vision
> >> mezzanine
> >>
> >> .../bindings/media/qcom,sc7280-camss.yaml | 439 +++++++++++++++
> >> arch/arm64/boot/dts/qcom/Makefile | 4 +
> >> .../qcs6490-rb3gen2-vision-mezzanine.dtso | 73 +++
> >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 208 ++++++++
> >> .../media/platform/qcom/camss/camss-csid.c | 1 -
> >> .../qcom/camss/camss-csiphy-3ph-1-0.c | 13 +-
> >> .../media/platform/qcom/camss/camss-csiphy.c | 5 +
> >> .../media/platform/qcom/camss/camss-csiphy.h | 1 +
> >> drivers/media/platform/qcom/camss/camss-vfe.c | 8 +-
> >> drivers/media/platform/qcom/camss/camss.c | 500 ++++++++++++++++--
> >> drivers/media/platform/qcom/camss/camss.h | 1 +
> >> 11 files changed, 1190 insertions(+), 63 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/media/qcom,sc7280-camss.yaml
> >> create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 01f18080a798..69955d5f2e73 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4422,12 +4422,12 @@ cci1_i2c1: i2c-bus@1 {
camss: camss@acaf000 {
compatible = "qcom,sc7280-camss";
- clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
- <&camcc CAM_CC_IFE_0_CSID_CLK>,
- <&camcc CAM_CC_IFE_1_CSID_CLK>,
- <&camcc CAM_CC_IFE_2_CSID_CLK>,
- <&camcc CAM_CC_IFE_LITE_0_CSID_CLK>,
- <&camcc CAM_CC_IFE_LITE_1_CSID_CLK>,
+ clocks = <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_CLK_SRC>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CPHY_RX_CLK_SRC>,
<&camcc CAM_CC_CSIPHY0_CLK>,
<&camcc CAM_CC_CSI0PHYTIMER_CLK>,
<&camcc CAM_CC_CSIPHY1_CLK>,
@@ -4438,29 +4438,32 @@ camss: camss@acaf000 {
<&camcc CAM_CC_CSI3PHYTIMER_CLK>,
<&camcc CAM_CC_CSIPHY4_CLK>,
<&camcc CAM_CC_CSI4PHYTIMER_CLK>,
- <&gcc GCC_CAMERA_AHB_CLK>,
- <&gcc GCC_CAMERA_HF_AXI_CLK>,
- <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_ICP_AHB_CLK>,
+ <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
<&camcc CAM_CC_IFE_0_AXI_CLK>,
<&camcc CAM_CC_IFE_0_CLK>,
<&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_0_CSID_CLK>,
<&camcc CAM_CC_IFE_1_AXI_CLK>,
<&camcc CAM_CC_IFE_1_CLK>,
<&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_1_CSID_CLK>,
<&camcc CAM_CC_IFE_2_AXI_CLK>,
<&camcc CAM_CC_IFE_2_CLK>,
<&camcc CAM_CC_IFE_2_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_2_CSID_CLK>,
<&camcc CAM_CC_IFE_LITE_0_CLK>,
<&camcc CAM_CC_IFE_LITE_0_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_0_CSID_CLK>,
<&camcc CAM_CC_IFE_LITE_1_CLK>,
- <&camcc CAM_CC_IFE_LITE_1_CPHY_RX_CLK>;
-
- clock-names = "camnoc_axi",
- "csi0",
- "csi1",
- "csi2",
- "csi3",
- "csi4",
+ <&camcc CAM_CC_IFE_LITE_1_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_1_CSID_CLK>;
+ clock-names = "cam_hf_axi",
+ "cam_sf_axi",
+ "camnoc_axi",
+ "camnoc_axi_src",
+ "cpas_ahb",
+ "cphy_rx_src",
"csiphy0",
"csiphy0_timer",
"csiphy1",
@@ -4471,22 +4474,26 @@ camss: camss@acaf000 {
"csiphy3_timer",
"csiphy4",
"csiphy4_timer",
- "gcc_camera_ahb",
- "gcc_camera_axi",
- "soc_ahb",
+ "icp_ahb",
+ "slow_ahb_src",
"vfe0_axi",
"vfe0",
"vfe0_cphy_rx",
+ "vfe0_csid",
"vfe1_axi",
"vfe1",
"vfe1_cphy_rx",
+ "vfe1_csid",
"vfe2_axi",
"vfe2",
"vfe2_cphy_rx",
- "vfe0_lite",
- "vfe0_lite_cphy_rx",
- "vfe1_lite",
- "vfe1_lite_cphy_rx";
+ "vfe2_csid",
+ "vfe_lite0",
+ "vfe_lite0_cphy_rx",
+ "vfe_lite0_csid",
+ "vfe_lite1",
+ "vfe_lite1_cphy_rx",
+ "vfe_lite1_csid";
interconnects = <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_CAMERA_CFG 0>,
<&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 319281bcc7a5..cb8921c65196 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1484,11 +1484,9 @@ static const struct camss_subdev_resources csiphy_res_7280[] = {
/* CSIPHY0 */
{
.regulators = {},
- .clock = { "csiphy0", "csiphy0_timer"},
- .clock_rate = {
- { 300000000 },
- { 300000000 }
- },
+ .clock = { "csiphy0", "csiphy0_timer" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 300000000 } },
.reg = { "csiphy0" },
.interrupt = { "csiphy0" },
.csiphy = {
@@ -1499,11 +1497,9 @@ static const struct camss_subdev_resources csiphy_res_7280[] = {
/* CSIPHY1 */
{
.regulators = {},
- .clock = { "csiphy1", "csiphy1_timer"},
- .clock_rate = {
- { 300000000 },
- { 300000000 }
- },
+ .clock = { "csiphy1", "csiphy1_timer" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 300000000 } },
.reg = { "csiphy1" },
.interrupt = { "csiphy1" },
.csiphy = {
@@ -1514,11 +1510,9 @@ static const struct camss_subdev_resources csiphy_res_7280[] = {
/* CSIPHY2 */
{
.regulators = {},
- .clock = { "csiphy2", "csiphy2_timer"},
- .clock_rate = {
- { 300000000 },
- { 300000000 }
- },
+ .clock = { "csiphy2", "csiphy2_timer" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 300000000 } },
.reg = { "csiphy2" },
.interrupt = { "csiphy2" },
.csiphy = {
@@ -1529,11 +1523,9 @@ static const struct camss_subdev_resources csiphy_res_7280[] = {
/* CSIPHY3 */
{
.regulators = {},
- .clock = { "csiphy3", "csiphy3_timer"},
- .clock_rate = {
- { 300000000 },
- { 300000000 }
- },
+ .clock = { "csiphy3", "csiphy3_timer" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 300000000 } },
.reg = { "csiphy3" },
.interrupt = { "csiphy3" },
.csiphy = {
@@ -1544,11 +1536,9 @@ static const struct camss_subdev_resources csiphy_res_7280[] = {
/* CSIPHY4 */
{
.regulators = {},
- .clock = { "csiphy4", "csiphy4_timer"},
- .clock_rate = {
- { 300000000 },
- { 300000000 }
- },
+ .clock = { "csiphy4", "csiphy4_timer" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 300000000 } },
.reg = { "csiphy4" },
.interrupt = { "csiphy4" },
.csiphy = {
@@ -1563,13 +1553,10 @@ static const struct camss_subdev_resources csid_res_7280[] = {
{
.regulators = { "vdda-phy", "vdda-pll" },
- .clock = { "csi0", "vfe0_cphy_rx", "vfe0", "soc_ahb"},
- .clock_rate = {
- { 300000000, 0, 380000000, 0},
- { 400000000, 0, 510000000, 0},
- { 400000000, 0, 637000000, 0},
- { 400000000, 0, 760000000, 0}
- },
+ .clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 } },
.reg = { "csid0" },
.interrupt = { "csid0" },
@@ -1584,13 +1571,10 @@ static const struct camss_subdev_resources csid_res_7280[] = {
{
.regulators = { "vdda-phy", "vdda-pll" },
- .clock = { "csi1", "vfe1_cphy_rx", "vfe1", "soc_ahb"},
- .clock_rate = {
- { 300000000, 0, 380000000, 0},
- { 400000000, 0, 510000000, 0},
- { 400000000, 0, 637000000, 0},
- { 400000000, 0, 760000000, 0}
- },
+ .clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 } },
.reg = { "csid1" },
.interrupt = { "csid1" },
@@ -1605,13 +1589,10 @@ static const struct camss_subdev_resources csid_res_7280[] = {
{
.regulators = { "vdda-phy", "vdda-pll" },
- .clock = { "csi2", "vfe2_cphy_rx", "vfe2", "soc_ahb"},
- .clock_rate = {
- { 300000000, 0, 380000000, 0},
- { 400000000, 0, 510000000, 0},
- { 400000000, 0, 637000000, 0},
- { 400000000, 0, 760000000, 0}
- },
+ .clock = { "vfe2_csid", "vfe2_cphy_rx", "vfe2" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 } },
.reg = { "csid2" },
.interrupt = { "csid2" },
@@ -1626,13 +1607,10 @@ static const struct camss_subdev_resources csid_res_7280[] = {
{
.regulators = { "vdda-phy", "vdda-pll" },
- .clock = { "csi3", "vfe0_lite_cphy_rx", "vfe0_lite", "soc_ahb"},
- .clock_rate = {
- { 300000000, 0, 320000000, 0},
- { 400000000, 0, 400000000, 0},
- { 400000000, 0, 480000000, 0},
- { 400000000, 0, 600000000, 0}
- },
+ .clock = { "vfe_lite0_csid", "vfe_lite0_cphy_rx", "vfe_lite0" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 0 },
+ { 320000000, 400000000, 480000000, 600000000 } },
.reg = { "csid_lite0" },
.interrupt = { "csid_lite0" },
@@ -1647,13 +1625,10 @@ static const struct camss_subdev_resources csid_res_7280[] = {
{
.regulators = { "vdda-phy", "vdda-pll" },
- .clock = { "csi3", "vfe0_lite_cphy_rx", "vfe0_lite", "soc_ahb"},
- .clock_rate = {
- { 300000000, 0, 320000000, 0},
- { 400000000, 0, 400000000, 0},
- { 400000000, 0, 480000000, 0},
- { 400000000, 0, 600000000, 0}
- },
+ .clock = { "vfe_lite1_csid", "vfe_lite1_cphy_rx", "vfe_lite1" },
+ .clock_rate = { { 300000000, 400000000 },
+ { 0 },
+ { 320000000, 400000000, 480000000, 600000000 } },
.reg = { "csid_lite1" },
.interrupt = { "csid_lite1" },
@@ -1671,15 +1646,16 @@ static const struct camss_subdev_resources vfe_res_7280[] = {
{
.regulators = {},
- .clock = { "vfe0", "vfe0_axi", "soc_ahb",
- "camnoc_axi", "gcc_camera_axi"},
- .clock_rate = {
- { 380000000, 0, 80000000, 150000000, 0},
- { 510000000, 0, 80000000, 240000000, 0},
- { 637000000, 0, 80000000, 320000000, 0},
- { 760000000, 0, 80000000, 400000000, 0},
- { 760000000, 0, 80000000, 480000000, 0},
- },
+ .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ "camnoc_axi", "icp_ahb", "vfe0", "vfe0_axi", "cam_hf_axi" },
+ .clock_rate = { { 150000000, 240000000, 320000000, 400000000, 480000000 },
+ { 80000000 },
+ { 80000000 },
+ { 0 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 },
+ { 0 },
+ { 0 } },
.reg = { "vfe0" },
.interrupt = { "vfe0" },
@@ -1697,15 +1673,16 @@ static const struct camss_subdev_resources vfe_res_7280[] = {
{
.regulators = {},
- .clock = { "vfe1", "vfe1_axi", "soc_ahb",
- "camnoc_axi", "gcc_camera_axi"},
- .clock_rate = {
- { 380000000, 0, 80000000, 150000000, 0},
- { 510000000, 0, 80000000, 240000000, 0},
- { 637000000, 0, 80000000, 320000000, 0},
- { 760000000, 0, 80000000, 400000000, 0},
- { 760000000, 0, 80000000, 480000000, 0},
- },
+ .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ "camnoc_axi", "icp_ahb", "vfe1", "vfe1_axi", "cam_hf_axi" },
+ .clock_rate = { { 150000000, 240000000, 320000000, 400000000, 480000000 },
+ { 80000000 },
+ { 80000000 },
+ { 0 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 },
+ { 0 },
+ { 0 } },
.reg = { "vfe1" },
.interrupt = { "vfe1" },
@@ -1723,15 +1700,16 @@ static const struct camss_subdev_resources vfe_res_7280[] = {
{
.regulators = {},
- .clock = { "vfe2", "vfe2_axi", "soc_ahb",
- "camnoc_axi", "gcc_camera_axi"},
- .clock_rate = {
- { 380000000, 0, 80000000, 150000000, 0},
- { 510000000, 0, 80000000, 240000000, 0},
- { 637000000, 0, 80000000, 320000000, 0},
- { 760000000, 0, 80000000, 400000000, 0},
- { 760000000, 0, 80000000, 480000000, 0},
- },
+ .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ "camnoc_axi", "icp_ahb", "vfe2", "vfe2_axi", "cam_hf_axi" },
+ .clock_rate = { { 150000000, 240000000, 320000000, 400000000, 480000000 },
+ { 80000000 },
+ { 80000000 },
+ { 0 },
+ { 0 },
+ { 380000000, 510000000, 637000000, 760000000 },
+ { 0 },
+ { 0 } },
.reg = { "vfe2" },
.interrupt = { "vfe2" },
@@ -1747,14 +1725,15 @@ static const struct camss_subdev_resources vfe_res_7280[] = {
},
/* VFE3 (lite) */
{
- .clock = { "vfe0_lite", "soc_ahb",
- "camnoc_axi", "gcc_camera_axi"},
- .clock_rate = {
- { 320000000, 80000000, 150000000, 0},
- { 400000000, 80000000, 240000000, 0},
- { 480000000, 80000000, 320000000, 0},
- { 600000000, 80000000, 400000000, 0},
- },
+ .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ "camnoc_axi", "icp_ahb", "vfe_lite0", "cam_hf_axi" },
+ .clock_rate = { { 150000000, 240000000, 320000000, 400000000, 480000000 },
+ { 80000000 },
+ { 80000000 },
+ { 0 },
+ { 0 },
+ { 320000000, 400000000, 480000000, 600000000 },
+ { 0 } },
.regulators = {},
.reg = { "vfe_lite0" },
@@ -1769,14 +1748,15 @@ static const struct camss_subdev_resources vfe_res_7280[] = {
},
/* VFE4 (lite) */
{
- .clock = { "vfe1_lite", "soc_ahb",
- "camnoc_axi", "gcc_camera_axi"},
- .clock_rate = {
- { 320000000, 80000000, 150000000, 0},
- { 400000000, 80000000, 240000000, 0},
- { 480000000, 80000000, 320000000, 0},
- { 600000000, 80000000, 400000000, 0},
- },
+ .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ "camnoc_axi", "icp_ahb", "vfe_lite1", "cam_hf_axi" },
+ .clock_rate = { { 150000000, 240000000, 320000000, 400000000, 480000000 },
+ { 80000000 },
+ { 80000000 },
+ { 0 },
+ { 0 },
+ { 320000000, 400000000, 480000000, 600000000 },
+ { 0 } },
.regulators = {},
.reg = { "vfe_lite1" },
