2024-03-20 14:18:07

by Depeng Shao

[permalink] [raw]
Subject: [PATCH v2 0/8] media: qcom: camss: Add sm8550 support

V2:
- Update some commit messages
Link to V1: https://lore.kernel.org/all/[email protected]/

V1:
SM8550 is a Qualcomm flagship SoC. This series adds support to
bring up the CSIPHY, CSID, VFE/RDI interfaces in SM8550.

SM8550 provides

- 3 x VFE, 3 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE
- 3 x CSID
- 2 x CSID Lite
- 8 x CSI PHY

This series is rebased based on:
https://patchew.org/linux/[email protected]/

The changes are verified on SM8550 AIM300 board, the base dts for AIM300 is:
https://patchew.org/linux/[email protected]/

Depeng Shao (3):
media: qcom: camss: Add new params for csid_device
media: qcom: camss: Add CSID gen3 driver
media: qcom: camss: Add sm8550 support

Yongsheng Li (5):
media: qcom: camss: Add CAMSS_8550 enum
media: qcom: camss: Add subdev notify support
media: qcom: camss: Add new csiphy driver 2-1-2
media: qcom: camss: Add new VFE driver for SM8550
media: qcom: camss: Add sm8550 resources

drivers/media/platform/qcom/camss/Makefile | 3 +
.../platform/qcom/camss/camss-csid-gen3.c | 639 ++++++++++++++++++
.../platform/qcom/camss/camss-csid-gen3.h | 26 +
.../media/platform/qcom/camss/camss-csid.c | 19 +
.../media/platform/qcom/camss/camss-csid.h | 10 +
.../platform/qcom/camss/camss-csiphy-2-1-2.c | 343 ++++++++++
.../media/platform/qcom/camss/camss-csiphy.c | 1 +
.../media/platform/qcom/camss/camss-csiphy.h | 3 +
.../media/platform/qcom/camss/camss-vfe-780.c | 455 +++++++++++++
drivers/media/platform/qcom/camss/camss-vfe.c | 7 +
drivers/media/platform/qcom/camss/camss-vfe.h | 3 +
.../media/platform/qcom/camss/camss-video.c | 1 +
drivers/media/platform/qcom/camss/camss.c | 395 +++++++++++
drivers/media/platform/qcom/camss/camss.h | 8 +
14 files changed, 1913 insertions(+)
create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.c
create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.h
create mode 100644 drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c

--
2.17.1



2024-03-20 14:33:15

by Depeng Shao

[permalink] [raw]
Subject: [PATCH v2 7/8] media: qcom: camss: Add sm8550 resources

From: Yongsheng Li <[email protected]>

Add sm8550 resources
- 3 x VFE, 3 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE
- 3 x CSID
- 2 x CSID Lite
- 8 x CSI PHY

Signed-off-by: Yongsheng Li <[email protected]>
---
drivers/media/platform/qcom/camss/camss.c | 345 ++++++++++++++++++++++
1 file changed, 345 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index b57cd25bf6c7..f43c957de49e 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1233,6 +1233,337 @@ static const struct resources_icc icc_res_sc8280xp[] = {
},
};

+static const struct camss_subdev_resources csiphy_res_8550[] = {
+ /* CSIPHY0 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy0",
+ "csiphy0_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy0" },
+ .interrupt = { "csiphy0" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY1 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy1",
+ "csiphy1_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy1" },
+ .interrupt = { "csiphy1" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY2 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy2",
+ "csiphy2_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy2" },
+ .interrupt = { "csiphy2" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY3 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy3",
+ "csiphy3_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy3" },
+ .interrupt = { "csiphy3" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY4 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy4",
+ "csiphy4_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy4" },
+ .interrupt = { "csiphy4" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY5 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy5",
+ "csiphy5_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy5" },
+ .interrupt = { "csiphy5" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY6 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy6",
+ "csiphy6_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy6" },
+ .interrupt = { "csiphy6" },
+ .ops = &csiphy_ops_2_1_2,
+ },
+ /* CSIPHY7 */
+ {
+ .regulators = {},
+ .clock = {
+ "csiphy7",
+ "csiphy7_timer" },
+ .clock_rate = {
+ { 300000000, 400000000, 400000000, 400000000, 400000000 },
+ { 300000000, 400000000, 400000000, 400000000, 400000000 } },
+ .reg = { "csiphy7" },
+ .interrupt = { "csiphy7" },
+ .ops = &csiphy_ops_2_1_2,
+ }
+};
+
+static const struct camss_subdev_resources csid_res_8550[] = {
+ /* CSID0 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = {
+ "csid",
+ "csiphy_rx" },
+ .clock_rate = {
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 } },
+ .reg = { "csid0", "csid_top" },
+ .interrupt = { "csid0" },
+ .ops = &csid_ops_gen3,
+ },
+ /* CSID1 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "csid", "csiphy_rx" },
+ .clock_rate = {
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 } },
+ .reg = { "csid1", "csid_top" },
+ .interrupt = { "csid1" },
+ .ops = &csid_ops_gen3,
+ },
+ /* CSID2 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "csid", "csiphy_rx" },
+ .clock_rate = {
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 } },
+ .reg = { "csid2", "csid_top" },
+ .interrupt = { "csid2" },
+ .ops = &csid_ops_gen3,
+ },
+ /* CSID3 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "vfe_lite_csid",
+ "vfe_lite_cphy_rx" },
+ .clock_rate = {
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 } },
+ .reg = { "csid_lite0" },
+ .interrupt = { "csid_lite0" },
+ .ops = &csid_ops_gen3,
+ },
+ /* CSID4 */
+ {
+ .regulators = { "vdda-phy", "vdda-pll" },
+ .clock = { "vfe_lite_csid",
+ "vfe_lite_cphy_rx" },
+ .clock_rate = {
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 } },
+ .reg = { "csid_lite1" },
+ .interrupt = { "csid_lite1" },
+ .ops = &csid_ops_gen3,
+ }
+};
+
+static const struct camss_subdev_resources vfe_res_8550[] = {
+ /* VFE0 */
+ {
+ .regulators = {},
+ .clock = {
+ "cam_hf_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb_clk",
+ "vfe0_fast_ahb",
+ "vfe0",
+ "cpas_vfe0",
+ "camnoc_axi"},
+ .clock_rate = {
+ { 0, 0, 0, 0, 0 },
+ { 0, 0, 0, 0, 80000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 466000000, 594000000, 675000000, 785000000, 785000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 }
+ },
+ .reg = { "vfe0" },
+ .interrupt = { "vfe0" },
+ .line_num = 3,
+ .is_lite = false,
+ .has_pd = true,
+ .pd_name = "ife0",
+ .ops = &vfe_ops_780,
+ },
+ /* VFE1 */
+ {
+ .regulators = {},
+ .clock = {
+ "cam_hf_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb_clk",
+ "vfe1_fast_ahb",
+ "vfe1",
+ "cpas_vfe1",
+ "camnoc_axi"},
+ .clock_rate = {
+ { 0, 0, 0, 0, 0 },
+ { 0, 0, 0, 0, 80000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 466000000, 594000000, 675000000, 785000000, 785000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 }
+ },
+ .reg = { "vfe1" },
+ .interrupt = { "vfe1" },
+ .line_num = 3,
+ .is_lite = false,
+ .has_pd = true,
+ .pd_name = "ife1",
+ .ops = &vfe_ops_780,
+ },
+ /* VFE2 */
+ {
+ .regulators = {},
+ .clock = {
+ "cam_hf_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb_clk",
+ "vfe2_fast_ahb",
+ "vfe2",
+ "cpas_vfe2",
+ "camnoc_axi"},
+ .clock_rate = {
+ { 0, 0, 0, 0, 0 },
+ { 0, 0, 0, 0, 80000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 466000000, 594000000, 675000000, 785000000, 785000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 }
+ },
+ .reg = { "vfe2" },
+ .interrupt = { "vfe2" },
+ .line_num = 3,
+ .is_lite = false,
+ .has_pd = true,
+ .pd_name = "ife2",
+ .ops = &vfe_ops_780,
+ },
+ /* VFE3 (lite) */
+ {
+ .regulators = {},
+ .clock = {
+ "cam_hf_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb_clk",
+ "vfe_lite_ahb",
+ "vfe_lite",
+ "cpas_ife_lite",
+ "camnoc_axi"},
+ .clock_rate = {
+ { 0, 0, 0, 0, 0 },
+ { 0, 0, 0, 0, 80000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 }
+ },
+ .reg = { "vfe_lite0" },
+ .interrupt = { "vfe_lite0" },
+ .line_num = 4,
+ .is_lite = true,
+ .ops = &vfe_ops_780,
+ },
+ /* VFE4 (lite) */
+ {
+ .regulators = {},
+ .clock = {
+ "cam_hf_axi",
+ "cpas_ahb",
+ "cpas_fast_ahb_clk",
+ "vfe_lite_ahb",
+ "vfe_lite",
+ "cpas_ife_lite",
+ "camnoc_axi"},
+ .clock_rate = {
+ { 0, 0, 0, 0, 0 },
+ { 0, 0, 0, 0, 80000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 400000000, 480000000, 480000000, 480000000, 480000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 },
+ { 300000000, 300000000, 400000000, 400000000, 400000000 }
+ },
+ .reg = { "vfe_lite1" },
+ .interrupt = { "vfe_lite1" },
+ .line_num = 4,
+ .is_lite = true,
+ .ops = &vfe_ops_780,
+ },
+};
+
+static const struct resources_icc icc_res_sm8550[] = {
+ {
+ .name = "cam_ahb",
+ .icc_bw_tbl.avg = 2097152,
+ .icc_bw_tbl.peak = 2097152,
+ },
+ {
+ .name = "cam_hf_0_mnoc",
+ .icc_bw_tbl.avg = 2097152,
+ .icc_bw_tbl.peak = 2097152,
+ },
+ {
+ .name = "cam_sf_0_mnoc",
+ .icc_bw_tbl.avg = 2097152,
+ .icc_bw_tbl.peak = 2097152,
+ },
+ {
+ .name = "cam_sf_icp_mnoc",
+ .icc_bw_tbl.avg = 2097152,
+ .icc_bw_tbl.peak = 2097152,
+ },
+};
+
/*
* camss_add_clock_margin - Add margin to clock frequency rate
* @rate: Clock frequency rate
@@ -2182,6 +2513,19 @@ static const struct camss_resources sc8280xp_resources = {
.vfe_num = ARRAY_SIZE(vfe_res_sc8280xp),
};

+static const struct camss_resources sm8550_resources = {
+ .version = CAMSS_8550,
+ .pd_name = "top",
+ .csiphy_res = csiphy_res_8550,
+ .csid_res = csid_res_8550,
+ .vfe_res = vfe_res_8550,
+ .icc_res = icc_res_sm8550,
+ .icc_path_num = ARRAY_SIZE(icc_res_sm8550),
+ .csiphy_num = ARRAY_SIZE(csiphy_res_8550),
+ .csid_num = ARRAY_SIZE(csid_res_8550),
+ .vfe_num = ARRAY_SIZE(vfe_res_8550),
+};
+
static const struct of_device_id camss_dt_match[] = {
{ .compatible = "qcom,msm8916-camss", .data = &msm8916_resources },
{ .compatible = "qcom,msm8996-camss", .data = &msm8996_resources },
@@ -2189,6 +2533,7 @@ static const struct of_device_id camss_dt_match[] = {
{ .compatible = "qcom,sdm845-camss", .data = &sdm845_resources },
{ .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
{ .compatible = "qcom,sc8280xp-camss", .data = &sc8280xp_resources },
+ { .compatible = "qcom,sm8550-camss", .data = &sm8550_resources },
{ }
};

--
2.17.1


2024-03-20 14:33:50

by Depeng Shao

[permalink] [raw]
Subject: [PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550

From: Yongsheng Li <[email protected]>

Add IFE driver for SM8550, the main difference with
old HW is register offset is different, register
update, reset and buf done is moved to CSID. And
the image address support 36 bits, so need to right
shift the image address when configuring the write
master.

Signed-off-by: Yongsheng Li <[email protected]>
Co-developed-by: Depeng Shao <[email protected]>
Signed-off-by: Depeng Shao <[email protected]>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../media/platform/qcom/camss/camss-vfe-780.c | 455 ++++++++++++++++++
drivers/media/platform/qcom/camss/camss-vfe.h | 1 +
3 files changed, 457 insertions(+)
create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c

diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index c5fcd6eec0f2..ac40bbab18a3 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -18,6 +18,7 @@ qcom-camss-objs += \
camss-vfe-4-8.o \
camss-vfe-17x.o \
camss-vfe-480.o \
+ camss-vfe-780.o \
camss-vfe-gen1.o \
camss-vfe.o \
camss-video.o \
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c
new file mode 100644
index 000000000000..a78647f23c8c
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
@@ -0,0 +1,455 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-vfe-780.c
+ *
+ * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v780 (SM8550)
+ *
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+
+#include "camss.h"
+#include "camss-vfe.h"
+
+#define VFE_HW_VERSION (vfe_is_lite(vfe) ? 0x1000 : 0x0)
+
+#define VFE_IRQ_CMD (vfe_is_lite(vfe) ? 0x1038 : 0x30)
+#define IRQ_CMD_GLOBAL_CLEAR BIT(0)
+
+#define VFE_IRQ_MASK(n) ((vfe_is_lite(vfe) ? 0x1024 : 0x34) + (n) * 4)
+#define IRQ_MASK_0_BUS_TOP_IRQ (vfe_is_lite(vfe) ? BIT(0) | BIT(1) | BIT(2) : \
+ BIT(0) | BIT(4) | BIT(18))
+#define IRQ_MASK_1_BUS_TOP_IRQ(n) (vfe_is_lite(vfe) ? BIT(2 * n + 2) | BIT(2 * n + 3) : \
+ BIT(2 * n + 8) | BIT(2 * n + 9))
+#define VFE_IRQ_CLEAR(n) ((vfe_is_lite(vfe) ? 0x102C : 0x3C) + (n) * 4)
+#define VFE_IRQ_STATUS(n) ((vfe_is_lite(vfe) ? 0x101C : 0x44) + (n) * 4)
+
+#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x1200 : 0xC00)
+
+#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 + 0xdc)
+
+#define VFE_BUS_WM_CFG(n) (BUS_REG_BASE + 0x200 + (n) * 0x100)
+#define WM_CFG_EN (0)
+#define WM_VIR_FRM_EN (1)
+#define WM_CFG_MODE (16)
+#define MODE_QCOM_PLAIN (0)
+#define MODE_MIPI_RAW (1)
+#define VFE_BUS_WM_IMAGE_ADDR(n) (BUS_REG_BASE + 0x204 + (n) * 0x100)
+#define VFE_BUS_WM_FRAME_INCR(n) (BUS_REG_BASE + 0x208 + (n) * 0x100)
+#define VFE_BUS_WM_IMAGE_CFG_0(n) (BUS_REG_BASE + 0x20c + (n) * 0x100)
+#define WM_IMAGE_CFG_0_DEFAULT_WIDTH (0xFFFF)
+#define VFE_BUS_WM_IMAGE_CFG_1(n) (BUS_REG_BASE + 0x210 + (n) * 0x100)
+#define VFE_BUS_WM_IMAGE_CFG_2(n) (BUS_REG_BASE + 0x214 + (n) * 0x100)
+#define WM_IMAGE_CFG_2_DEFAULT_STRIDE (0xFFFF)
+#define VFE_BUS_WM_PACKER_CFG(n) (BUS_REG_BASE + 0x218 + (n) * 0x100)
+#define VFE_BUS_WM_HEADER_ADDR(n) (BUS_REG_BASE + 0x220 + (n) * 0x100)
+#define VFE_BUS_WM_HEADER_INCR(n) (BUS_REG_BASE + 0x224 + (n) * 0x100)
+#define VFE_BUS_WM_HEADER_CFG(n) (BUS_REG_BASE + 0x228 + (n) * 0x100)
+
+#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n) (BUS_REG_BASE + 0x230 + (n) * 0x100)
+#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n) (BUS_REG_BASE + 0x234 + (n) * 0x100)
+#define VFE_BUS_WM_FRAMEDROP_PERIOD(n) (BUS_REG_BASE + 0x238 + (n) * 0x100)
+#define VFE_BUS_WM_FRAMEDROP_PATTERN(n) (BUS_REG_BASE + 0x23c + (n) * 0x100)
+
+#define VFE_BUS_WM_MMU_PREFETCH_CFG(n) (BUS_REG_BASE + 0x260 + (n) * 0x100)
+#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n) (BUS_REG_BASE + 0x264 + (n) * 0x100)
+#define VFE_BUS_WM_SYSTEM_CACHE_CFG(n) (BUS_REG_BASE + 0x268 + (n) * 0x100)
+
+
+/* for titan 780, each bus client is hardcoded to a specific path */
+#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0 : 23) + (n))
+
+#define MAX_VFE_OUTPUT_LINES 4
+#define MAX_VFE_ACT_BUF 1
+
+static u32 vfe_hw_version(struct vfe_device *vfe)
+{
+ u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
+
+ u32 gen = (hw_version >> 28) & 0xF;
+ u32 rev = (hw_version >> 16) & 0xFFF;
+ u32 step = hw_version & 0xFFFF;
+
+ dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step);
+
+ return hw_version;
+}
+
+static void vfe_global_reset(struct vfe_device *vfe)
+{
+}
+
+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); /* map to actual WM used (from wm=RDI index) */
+
+ /* no clock gating at bus input */
+ writel_relaxed(0, vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
+
+ writel_relaxed(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL);
+
+ writel_relaxed(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8,
+ vfe->base + VFE_BUS_WM_FRAME_INCR(wm));
+ writel_relaxed((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF),
+ vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm));
+ writel_relaxed(WM_IMAGE_CFG_2_DEFAULT_STRIDE,
+ vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm));
+ writel_relaxed(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm));
+
+ /* no dropped frames, one irq per frame */
+ writel_relaxed(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm));
+ writel_relaxed(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm));
+ writel_relaxed(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm));
+ writel_relaxed(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm));
+
+ writel_relaxed(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm));
+ writel_relaxed(0xFFFFFFFF, vfe->base + VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm));
+
+ writel_relaxed(1 << WM_CFG_EN | MODE_MIPI_RAW << 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); /* map to actual WM used (from wm=RDI index) */
+ writel_relaxed(0, vfe->base + VFE_BUS_WM_CFG(wm));
+}
+
+static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr,
+ struct vfe_line *line)
+{
+ wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */
+ writel_relaxed((addr >> 8) & 0xFFFFFFFF, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
+
+ dev_dbg(vfe->camss->dev, "wm:%d, image buf addr:0x%llx\n",
+ wm, addr);
+}
+
+static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
+{
+ int port_id = line_id;
+
+ v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP, (void *)&port_id);
+}
+
+static inline void vfe_reg_update_clear(struct vfe_device *vfe,
+ enum vfe_line_id line_id)
+{
+ int port_id = line_id;
+
+ v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP_CLEAR, (void *)&port_id);
+}
+
+static void vfe_enable_irq_common(struct vfe_device *vfe, enum vfe_line_id line_id)
+{
+ int port_id = line_id;
+
+ /* enable top BUS status IRQ */
+ writel_relaxed(IRQ_MASK_0_BUS_TOP_IRQ,
+ vfe->base + VFE_IRQ_MASK(0));
+
+ writel_relaxed(IRQ_MASK_1_BUS_TOP_IRQ(port_id),
+ vfe->base + VFE_IRQ_MASK(1));
+}
+
+/*
+ * vfe_isr - VFE module interrupt handler
+ * @irq: Interrupt line
+ * @dev: VFE device
+ *
+ * Return IRQ_HANDLED on success
+ */
+static irqreturn_t vfe_isr(int irq, void *dev)
+{
+ struct vfe_device *vfe = dev;
+ u32 status;
+
+ status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
+ writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
+ writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
+
+ if (status)
+ dev_dbg(vfe->camss->dev, "Top Status_0:0x%x\n", status);
+
+ status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(1));
+ writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(1));
+ writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
+
+ if (status)
+ dev_dbg(vfe->camss->dev, "Top Status_1:0x%x\n", status);
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * vfe_halt - Trigger halt on VFE module and wait to complete
+ * @vfe: VFE device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_halt(struct vfe_device *vfe)
+{
+ /* rely on vfe_disable_output() to stop the VFE */
+ return 0;
+}
+
+static int vfe_get_output(struct vfe_line *line)
+{
+ struct vfe_device *vfe = to_vfe(line);
+ struct vfe_output *output;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vfe->output_lock, flags);
+
+ output = &line->output;
+ if (output->state > VFE_OUTPUT_RESERVED) {
+ dev_err(vfe->camss->dev, "Output is running\n");
+ goto error;
+ }
+
+ output->wm_num = 1;
+
+ /* Correspondence between VFE line number and WM number.
+ * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
+ * Note this 1:1 mapping will not work for PIX streams.
+ */
+ output->wm_idx[0] = line->id;
+ vfe->wm_output_map[line->id] = line->id;
+
+ output->drop_update_idx = 0;
+
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+
+ return 0;
+
+error:
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+ output->state = VFE_OUTPUT_OFF;
+
+ return -EINVAL;
+}
+
+static int vfe_enable_output(struct vfe_line *line)
+{
+ struct vfe_device *vfe = to_vfe(line);
+ struct vfe_output *output = &line->output;
+ unsigned long flags;
+ unsigned int i;
+
+ spin_lock_irqsave(&vfe->output_lock, flags);
+
+ vfe_reg_update_clear(vfe, line->id);
+
+ if (output->state > VFE_OUTPUT_RESERVED) {
+ dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
+ output->state);
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+ return -EINVAL;
+ }
+
+ WARN_ON(output->gen2.active_num);
+
+ output->state = VFE_OUTPUT_ON;
+
+ output->sequence = 0;
+
+ vfe_wm_start(vfe, output->wm_idx[0], line);
+
+ for (i = 0; i < MAX_VFE_ACT_BUF; i++) {
+ output->buf[i] = vfe_buf_get_pending(output);
+ if (!output->buf[i])
+ break;
+ output->gen2.active_num++;
+ vfe_wm_update(vfe, output->wm_idx[0], output->buf[i]->addr[0], line);
+
+ vfe_reg_update(vfe, line->id);
+ }
+
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+
+ return 0;
+}
+
+/*
+ * vfe_enable - Enable streaming on VFE line
+ * @line: VFE line
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_enable(struct vfe_line *line)
+{
+ struct vfe_device *vfe = to_vfe(line);
+ int ret;
+
+ mutex_lock(&vfe->stream_lock);
+
+ if (!vfe->stream_count)
+ vfe_enable_irq_common(vfe, line->id);
+
+ vfe->stream_count++;
+
+ mutex_unlock(&vfe->stream_lock);
+
+ ret = vfe_get_output(line);
+ if (ret < 0)
+ goto error_get_output;
+
+ ret = vfe_enable_output(line);
+ if (ret < 0)
+ goto error_enable_output;
+
+ vfe->was_streaming = 1;
+
+ return 0;
+
+error_enable_output:
+ vfe_put_output(line);
+
+error_get_output:
+ mutex_lock(&vfe->stream_lock);
+
+ vfe->stream_count--;
+
+ mutex_unlock(&vfe->stream_lock);
+
+ return ret;
+}
+
+/*
+ * vfe_buf_done - Process write master done interrupt
+ * @vfe: VFE Device
+ * @wm: Write master id
+ */
+static void vfe_buf_done(struct vfe_device *vfe, int wm)
+{
+ struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]];
+ struct camss_buffer *ready_buf;
+ struct vfe_output *output;
+ unsigned long flags;
+ u32 index;
+ u64 ts = ktime_get_ns();
+
+ spin_lock_irqsave(&vfe->output_lock, flags);
+
+ if (vfe->wm_output_map[wm] == VFE_LINE_NONE) {
+ dev_err_ratelimited(vfe->camss->dev,
+ "Received wm done for unmapped index\n");
+ goto out_unlock;
+ }
+ output = &vfe->line[vfe->wm_output_map[wm]].output;
+
+ ready_buf = output->buf[0];
+ if (!ready_buf) {
+ dev_err_ratelimited(vfe->camss->dev,
+ "Missing ready buf %d!\n", output->state);
+ goto out_unlock;
+ }
+
+ ready_buf->vb.vb2_buf.timestamp = ts;
+ ready_buf->vb.sequence = output->sequence++;
+
+ index = 0;
+ output->buf[0] = output->buf[1];
+ if (output->buf[0])
+ index = 1;
+
+ output->buf[index] = vfe_buf_get_pending(output);
+
+ if (output->buf[index]) {
+ vfe_wm_update(vfe, output->wm_idx[0], output->buf[index]->addr[0], line);
+ vfe_reg_update(vfe, line->id);
+ } else
+ output->gen2.active_num--;
+
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+
+ vb2_buffer_done(&ready_buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
+
+ return;
+
+out_unlock:
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+}
+
+/*
+ * vfe_queue_buffer - Add empty buffer
+ * @vid: Video device structure
+ * @buf: Buffer to be enqueued
+ *
+ * Add an empty buffer - depending on the current number of buffers it will be
+ * put in pending buffer queue or directly given to the hardware to be filled.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_queue_buffer(struct camss_video *vid,
+ struct camss_buffer *buf)
+{
+ struct vfe_line *line = container_of(vid, struct vfe_line, video_out);
+ struct vfe_device *vfe = to_vfe(line);
+ struct vfe_output *output;
+ unsigned long flags;
+
+ output = &line->output;
+
+ spin_lock_irqsave(&vfe->output_lock, flags);
+
+ if (output->state == VFE_OUTPUT_ON &&
+ output->gen2.active_num < MAX_VFE_ACT_BUF) {
+ output->buf[output->gen2.active_num++] = buf;
+ vfe_wm_update(vfe, output->wm_idx[0], buf->addr[0], line);
+ vfe_reg_update(vfe, line->id);
+ } else {
+ vfe_buf_add_pending(output, buf);
+ }
+
+ spin_unlock_irqrestore(&vfe->output_lock, flags);
+
+ return 0;
+}
+
+static const struct camss_video_ops vfe_video_ops_780 = {
+ .queue_buffer = vfe_queue_buffer,
+ .flush_buffers = vfe_flush_buffers,
+};
+
+static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
+{
+ vfe->video_ops = vfe_video_ops_780;
+}
+
+static void vfe_subdev_event(struct vfe_device *vfe, unsigned int evt_type, void *arg)
+{
+ int port_id = *(int *)arg;
+
+ switch (evt_type) {
+ case NOTIFY_BUF_DONE:
+ vfe_buf_done(vfe, port_id);
+ break;
+
+ default:
+ break;
+ }
+}
+
+const struct vfe_hw_ops vfe_ops_780 = {
+ .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,
+ .subdev_init = vfe_subdev_init,
+ .vfe_disable = vfe_disable,
+ .vfe_enable = vfe_enable,
+ .vfe_halt = vfe_halt,
+ .vfe_wm_stop = vfe_wm_stop,
+ .event = vfe_subdev_event,
+};
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index 9919fe0ff101..150ea0b31fb1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -224,6 +224,7 @@ extern const struct vfe_hw_ops vfe_ops_4_7;
extern const struct vfe_hw_ops vfe_ops_4_8;
extern const struct vfe_hw_ops vfe_ops_170;
extern const struct vfe_hw_ops vfe_ops_480;
+extern const struct vfe_hw_ops vfe_ops_780;

int vfe_get(struct vfe_device *vfe);
void vfe_put(struct vfe_device *vfe);
--
2.17.1


2024-03-20 14:33:58

by Depeng Shao

[permalink] [raw]
Subject: [PATCH v2 5/8] media: qcom: camss: Add CSID gen3 driver

The CSID in SM8550 is gen3, it has new register
offset and new functionality, the buf done irq,
register update and reset is moved to CSID gen3.

Signed-off-by: Depeng Shao <[email protected]>
Co-developed-by: Yongsheng Li <[email protected]>
Signed-off-by: Yongsheng Li <[email protected]>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../platform/qcom/camss/camss-csid-gen3.c | 639 ++++++++++++++++++
.../platform/qcom/camss/camss-csid-gen3.h | 26 +
.../media/platform/qcom/camss/camss-csid.h | 1 +
4 files changed, 667 insertions(+)
create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.c
create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.h

diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index 28eba4bf3e38..c5fcd6eec0f2 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -7,6 +7,7 @@ qcom-camss-objs += \
camss-csid-4-1.o \
camss-csid-4-7.o \
camss-csid-gen2.o \
+ camss-csid-gen3.o \
camss-csiphy-2ph-1-0.o \
camss-csiphy-3ph-1-0.o \
camss-csiphy-2-1-2.o \
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
new file mode 100644
index 000000000000..b97005f7065d
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -0,0 +1,639 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-csid-gen3.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
+ *
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+
+
+#include "camss-csid.h"
+#include "camss-csid-gen3.h"
+#include "camss.h"
+
+
+#define CSID_TOP_IO_PATH_CFG0(csid) (0x4 * (csid))
+#define OUTPUT_IFE_EN 0x100
+#define INTERNAL_CSID 1
+
+#define CSID_HW_VERSION 0x0
+#define HW_VERSION_STEPPING 0
+#define HW_VERSION_REVISION 16
+#define HW_VERSION_GENERATION 28
+
+#define CSID_RST_CFG 0xC
+#define RST_MODE 0
+#define RST_LOCATION 4
+
+#define CSID_RST_CMD 0x10
+#define SELECT_HW_RST 0
+#define SELECT_SW_RST 1
+#define SELECT_IRQ_RST 2
+
+#define CSID_CSI2_RX_IRQ_STATUS 0x9C
+#define CSID_CSI2_RX_IRQ_MASK 0xA0
+#define CSID_CSI2_RX_IRQ_CLEAR 0xA4
+#define CSID_CSI2_RX_IRQ_SET 0xA8
+
+#define CSID_CSI2_RDIN_IRQ_STATUS(rdi) (0xEC + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_IRQ_MASK(rdi) (0xF0 + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_INFO_FIFO_FULL 2
+#define CSID_CSI2_RDIN_INFO_CAMIF_EOF 3
+#define CSID_CSI2_RDIN_INFO_CAMIF_SOF 4
+#define CSID_CSI2_RDIN_INFO_INPUT_EOF 9
+#define CSID_CSI2_RDIN_INFO_INPUT_SOF 12
+#define CSID_CSI2_RDIN_ERROR_REC_FRAME_DROP 18
+#define CSID_CSI2_RDIN_ERROR_REC_OVERFLOW_IRQ 19
+#define CSID_CSI2_RDIN_ERROR_REC_CCIF_VIOLATION 20
+#define CSID_CSI2_RDIN_EPOCH0 21
+#define CSID_CSI2_RDIN_EPOCH1 22
+#define CSID_CSI2_RDIN_RUP_DONE 23
+#define CSID_CSI2_RDIN_CCIF_VIOLATION 29
+
+#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi) (0xF4 + 0x10 * (rdi))
+#define CSID_CSI2_RDIN_IRQ_SET(rdi) (0xF8 + 0x10 * (rdi))
+
+#define CSID_TOP_IRQ_STATUS 0x7C
+#define TOP_IRQ_STATUS_RESET_DONE 0
+#define CSID_TOP_IRQ_MASK 0x80
+#define CSID_TOP_IRQ_CLEAR 0x84
+#define CSID_TOP_IRQ_SET 0x88
+#define CSID_IRQ_CMD 0x14
+#define IRQ_CMD_CLEAR 0
+#define IRQ_CMD_SET 4
+
+#define CSID_BUF_DONE_IRQ_STATUS 0x8C
+#define BUF_DONE_IRQ_STATUS_RDI_OFFSET (csid_is_lite(csid) ? 1 : 14)
+#define CSID_BUF_DONE_IRQ_MASK 0x90
+#define CSID_BUF_DONE_IRQ_CLEAR 0x94
+#define CSID_BUF_DONE_IRQ_SET 0x98
+
+#define CSID_CSI2_RX_CFG0 0x200
+#define CSI2_RX_CFG0_NUM_ACTIVE_LANES 0
+#define CSI2_RX_CFG0_DL0_INPUT_SEL 4
+#define CSI2_RX_CFG0_DL1_INPUT_SEL 8
+#define CSI2_RX_CFG0_DL2_INPUT_SEL 12
+#define CSI2_RX_CFG0_DL3_INPUT_SEL 16
+#define CSI2_RX_CFG0_PHY_NUM_SEL 20
+#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
+#define CSI2_RX_CFG0_PHY_TYPE_SEL 24
+#define CSI2_RX_CFG0_TPG_MUX_EN 27
+#define CSI2_RX_CFG0_TPG_NUM_SEL 28
+
+#define CSID_CSI2_RX_CFG1 0x204
+#define CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN 0
+#define CSI2_RX_CFG1_DE_SCRAMBLE_EN 1
+#define CSI2_RX_CFG1_VC_MODE 2
+#define CSI2_RX_CFG1_COMPLETE_STREAM_EN 4
+#define CSI2_RX_CFG1_COMPLETE_STREAM_FRAME_TIMING 5
+#define CSI2_RX_CFG1_MISR_EN 6
+#define CSI2_RX_CFG1_PHY_BIST_EN 7
+#define CSI2_RX_CFG1_EPD_MODE 8
+#define CSI2_RX_CFG1_EOTP_EN 9
+#define CSI2_RX_CFG1_DYN_SWITCH_EN 10
+#define CSI2_RX_CFG1_RUP_AUP_LATCH_DIS 11
+
+#define CSID_CSI2_RX_CAPTURE_CTRL 0x208
+#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_EN 0
+#define CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_EN 1
+#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_EN 3
+#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_DT 4
+#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_VC 10
+#define CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_VC 15
+#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_DT 20
+#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_VC 26
+
+#define CSID_RDI_CFG0(rdi) (0x500 + 0x100 * (rdi))
+#define RDI_CFG0_VFR_EN 0
+#define RDI_CFG0_FRAME_ID_DEC_EN 1
+#define RDI_CFG0_RETIME_DIS 5
+#define RDI_CFG0_TIMESTAMP_EN 6
+#define RDI_CFG0_TIMESTAMP_STB_SEL 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 31
+
+#define CSID_RDI_CFG1(rdi) (0x510 + 0x100 * (rdi))
+#define RDI_CFG1_BYTE_CNTR_EN 2
+#define RDI_CFG1_DROP_H_EN 5
+#define RDI_CFG1_DROP_V_EN 6
+#define RDI_CFG1_CROP_H_EN 7
+#define RDI_CFG1_CROP_V_EN 8
+#define RDI_CFG1_MISR_EN 9
+#define RDI_CFG1_PIX_STORE 10
+#define RDI_CFG1_PLAIN_ALIGNMENT 11
+#define RDI_CFG1_PLAIN_FORMAT 12
+#define RDI_CFG1_EARLY_EOF_EN 14
+#define RDI_CFG1_PACKING_FORMAT 15
+
+#define CSID_RDI_CTRL(rdi) (0x504 + 0x100 * (rdi))
+#define RDI_CTRL_START_CMD 0
+#define RDI_CTRL_START_MODE 2
+
+#define CSID_RDI_EPOCH_IRQ_CFG(rdi) (0x52C + 0x100 * (rdi))
+
+#define CSID_RDI_FRM_DROP_PATTERN(rdi) (0x540 + 0x100 * (rdi))
+#define CSID_RDI_FRM_DROP_PERIOD(rdi) (0x544 + 0x100 * (rdi))
+#define CSID_RDI_IRQ_SUBSAMPLE_PATTERN(rdi) (0x548 + 0x100 * (rdi))
+#define CSID_RDI_IRQ_SUBSAMPLE_PERIOD(rdi) (0x54C + 0x100 * (rdi))
+#define CSID_RDI_RPP_PIX_DROP_PATTERN(rdi) (0x558 + 0x100 * (rdi))
+#define CSID_RDI_RPP_PIX_DROP_PERIOD(rdi) (0x55C + 0x100 * (rdi))
+#define CSID_RDI_RPP_LINE_DROP_PATTERN(rdi) (0x560 + 0x100 * (rdi))
+#define CSID_RDI_RPP_LINE_DROP_PERIOD(rdi) (0x564 + 0x100 * (rdi))
+
+#define CSID_RDI_RPP_HCROP(rdi) (0x550 + 0x100 * (rdi))
+#define CSID_RDI_RPP_VCROP(rdi) (0x554 + 0x100 * (rdi))
+
+#define CSID_RDI_ERROR_RECOVERY_CFG0(rdi) (0x514 + 0x100 * (rdi))
+
+#define CSID_DISCARD_FRAMES 4
+
+#define CSID_REG_UPDATE_CMD 0x18
+static inline int reg_update_rdi(struct csid_device *csid, int n)
+{
+ return BIT(n + 4) + BIT(20 + n);
+}
+
+#define REG_UPDATE_RDI reg_update_rdi
+
+static const struct csid_format csid_formats[] = {
+ {
+ MEDIA_BUS_FMT_UYVY8_1X16,
+ DATA_TYPE_YUV422_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 2,
+ },
+ {
+ MEDIA_BUS_FMT_VYUY8_1X16,
+ DATA_TYPE_YUV422_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 2,
+ },
+ {
+ MEDIA_BUS_FMT_YUYV8_1X16,
+ DATA_TYPE_YUV422_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 2,
+ },
+ {
+ MEDIA_BUS_FMT_YVYU8_1X16,
+ DATA_TYPE_YUV422_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 2,
+ },
+ {
+ MEDIA_BUS_FMT_SBGGR8_1X8,
+ DATA_TYPE_RAW_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGBRG8_1X8,
+ DATA_TYPE_RAW_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGRBG8_1X8,
+ DATA_TYPE_RAW_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SRGGB8_1X8,
+ DATA_TYPE_RAW_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SBGGR10_1X10,
+ DATA_TYPE_RAW_10BIT,
+ DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+ 10,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGBRG10_1X10,
+ DATA_TYPE_RAW_10BIT,
+ DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+ 10,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGRBG10_1X10,
+ DATA_TYPE_RAW_10BIT,
+ DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+ 10,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SRGGB10_1X10,
+ DATA_TYPE_RAW_10BIT,
+ DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+ 10,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_Y8_1X8,
+ DATA_TYPE_RAW_8BIT,
+ DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+ 8,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_Y10_1X10,
+ DATA_TYPE_RAW_10BIT,
+ DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+ 10,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SBGGR12_1X12,
+ DATA_TYPE_RAW_12BIT,
+ DECODE_FORMAT_UNCOMPRESSED_12_BIT,
+ 12,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGBRG12_1X12,
+ DATA_TYPE_RAW_12BIT,
+ DECODE_FORMAT_UNCOMPRESSED_12_BIT,
+ 12,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGRBG12_1X12,
+ DATA_TYPE_RAW_12BIT,
+ DECODE_FORMAT_UNCOMPRESSED_12_BIT,
+ 12,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SRGGB12_1X12,
+ DATA_TYPE_RAW_12BIT,
+ DECODE_FORMAT_UNCOMPRESSED_12_BIT,
+ 12,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SBGGR14_1X14,
+ DATA_TYPE_RAW_14BIT,
+ DECODE_FORMAT_UNCOMPRESSED_14_BIT,
+ 14,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGBRG14_1X14,
+ DATA_TYPE_RAW_14BIT,
+ DECODE_FORMAT_UNCOMPRESSED_14_BIT,
+ 14,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SGRBG14_1X14,
+ DATA_TYPE_RAW_14BIT,
+ DECODE_FORMAT_UNCOMPRESSED_14_BIT,
+ 14,
+ 1,
+ },
+ {
+ MEDIA_BUS_FMT_SRGGB14_1X14,
+ DATA_TYPE_RAW_14BIT,
+ DECODE_FORMAT_UNCOMPRESSED_14_BIT,
+ 14,
+ 1,
+ },
+};
+
+static void __csid_configure_rx(struct csid_device *csid,
+ struct csid_phy_config *phy, int vc)
+{
+ 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_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
+
+ val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
+ if (vc > 3)
+ val |= 1 << CSI2_RX_CFG1_VC_MODE;
+ writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
+}
+
+static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
+{
+ int val;
+
+ if (enable)
+ val = 1 << RDI_CTRL_START_CMD;
+ else
+ val = 0 << RDI_CTRL_START_CMD;
+ writel_relaxed(val, csid->base + CSID_RDI_CTRL(rdi));
+}
+
+static void __csid_configure_top(struct csid_device *csid)
+{
+ u32 val;
+
+ if (csid->top_base) {
+ val = OUTPUT_IFE_EN | INTERNAL_CSID;
+ writel_relaxed(val, csid->top_base + CSID_TOP_IO_PATH_CFG0(csid->id));
+ }
+}
+
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
+{
+ u32 val;
+ u8 lane_cnt = csid->phy.lane_cnt;
+ /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
+ struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
+ const struct csid_format *format = csid_get_fmt_entry(csid->formats, csid->nformats,
+ input_format->code);
+
+ if (!lane_cnt)
+ lane_cnt = 4;
+
+ val = 0;
+ writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(vc));
+
+ /*
+ * 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;
+
+ val = 1 << RDI_CFG0_TIMESTAMP_EN;
+ val |= 2 << RDI_CFG0_TIMESTAMP_STB_SEL;
+ /* 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_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+
+ val = 1 << RDI_CFG1_PACKING_FORMAT;
+ val |= 1 << RDI_CFG1_PIX_STORE;
+ val |= 1 << RDI_CFG1_DROP_H_EN;
+ val |= 1 << RDI_CFG1_DROP_V_EN;
+ val |= 1 << RDI_CFG1_CROP_H_EN;
+ val |= 1 << RDI_CFG1_CROP_V_EN;
+ val |= RDI_CFG1_EARLY_EOF_EN;
+
+ writel_relaxed(val, csid->base + CSID_RDI_CFG1(vc));
+
+ val = 0;
+ writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
+
+ val = 1;
+ writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
+
+ val = 0;
+ writel_relaxed(val, csid->base + CSID_RDI_CTRL(vc));
+
+ val = readl_relaxed(csid->base + CSID_RDI_CFG0(vc));
+ val |= enable << RDI_CFG0_EN;
+ writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+}
+
+static void csid_configure_stream(struct csid_device *csid, u8 enable)
+{
+ u8 i;
+
+ /* Loop through all enabled VCs and configure stream for each */
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+ if (csid->phy.en_vc & BIT(i)) {
+ __csid_configure_top(csid);
+ __csid_configure_rdi_stream(csid, enable, i);
+ __csid_configure_rx(csid, &csid->phy, i);
+ __csid_ctrl_rdi(csid, enable, i);
+ }
+}
+
+static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
+{
+ if (val > 0 && val <= csid->testgen.nmodes)
+ csid->testgen.mode = val;
+
+ return 0;
+}
+
+/*
+ * csid_hw_version - CSID hardware version query
+ * @csid: CSID device
+ *
+ * Return HW version or error
+ */
+static u32 csid_hw_version(struct csid_device *csid)
+{
+ u32 hw_version;
+ u32 hw_gen;
+ u32 hw_rev;
+ u32 hw_step;
+
+ hw_version = readl_relaxed(csid->base + CSID_HW_VERSION);
+ hw_gen = (hw_version >> HW_VERSION_GENERATION) & 0xF;
+ hw_rev = (hw_version >> HW_VERSION_REVISION) & 0xFFF;
+ hw_step = (hw_version >> HW_VERSION_STEPPING) & 0xFFFF;
+ dev_info(csid->camss->dev, "CSID HW Version = %u.%u.%u\n",
+ hw_gen, hw_rev, hw_step);
+
+ return hw_version;
+}
+
+static void csid_reg_update(struct csid_device *csid, int port_id)
+{
+ csid->reg_update |= REG_UPDATE_RDI(csid, port_id);
+ writel_relaxed(csid->reg_update, csid->base + CSID_REG_UPDATE_CMD);
+}
+
+static inline void csid_reg_update_clear(struct csid_device *csid,
+ int port_id)
+{
+ csid->reg_update &= ~REG_UPDATE_RDI(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_relaxed(csid->base + CSID_TOP_IRQ_STATUS);
+ writel_relaxed(val, csid->base + CSID_TOP_IRQ_CLEAR);
+ reset_done = val & BIT(TOP_IRQ_STATUS_RESET_DONE);
+
+ val = readl_relaxed(csid->base + CSID_CSI2_RX_IRQ_STATUS);
+ writel_relaxed(val, csid->base + CSID_CSI2_RX_IRQ_CLEAR);
+
+ buf_done_val = readl_relaxed(csid->base + CSID_BUF_DONE_IRQ_STATUS);
+ writel_relaxed(buf_done_val, csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+
+ /* Read and clear IRQ status for each enabled RDI channel */
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+ if (csid->phy.en_vc & BIT(i)) {
+ val = readl_relaxed(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i));
+ writel_relaxed(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i));
+
+ if (buf_done_val & BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i)) {
+ dev_dbg(csid->camss->dev, "RDI%d BUF DONE", i);
+ v4l2_subdev_notify(&csid->subdev, NOTIFY_BUF_DONE, (void *)&i);
+ }
+ }
+
+ val = 1 << IRQ_CMD_CLEAR;
+ writel_relaxed(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);
+
+ writel_relaxed(1, csid->base + CSID_TOP_IRQ_CLEAR);
+ writel_relaxed(1, csid->base + CSID_IRQ_CMD);
+ writel_relaxed(1, csid->base + CSID_TOP_IRQ_MASK);
+
+ for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
+ if (csid->phy.en_vc & BIT(i)) {
+ writel_relaxed(BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i),
+ csid->base + CSID_BUF_DONE_IRQ_CLEAR);
+ writel_relaxed(0x1 << IRQ_CMD_CLEAR, csid->base + CSID_IRQ_CMD);
+ writel_relaxed(BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i),
+ csid->base + CSID_BUF_DONE_IRQ_MASK);
+ }
+
+ /* preserve registers */
+ val = (0x1 << RST_LOCATION) | (0x1 << RST_MODE);
+ writel_relaxed(val, csid->base + CSID_RST_CFG);
+
+ val = (0x1 << SELECT_HW_RST) | (0x1 << SELECT_IRQ_RST);
+ writel_relaxed(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 -EIO;
+ }
+
+ return 0;
+}
+
+static u32 csid_src_pad_code(struct csid_device *csid, u32 sink_code,
+ unsigned int match_format_idx, u32 match_code)
+{
+ switch (sink_code) {
+ case MEDIA_BUS_FMT_SBGGR10_1X10:
+ {
+ u32 src_code[] = {
+ MEDIA_BUS_FMT_SBGGR10_1X10,
+ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE,
+ };
+
+ return csid_find_code(src_code, ARRAY_SIZE(src_code),
+ match_format_idx, match_code);
+ }
+ case MEDIA_BUS_FMT_Y10_1X10:
+ {
+ u32 src_code[] = {
+ MEDIA_BUS_FMT_Y10_1X10,
+ MEDIA_BUS_FMT_Y10_2X8_PADHI_LE,
+ };
+
+ return csid_find_code(src_code, ARRAY_SIZE(src_code),
+ match_format_idx, match_code);
+ }
+ default:
+ if (match_format_idx > 0)
+ return 0;
+
+ return sink_code;
+ }
+}
+
+static void csid_subdev_init(struct csid_device *csid)
+{
+ csid->formats = csid_formats;
+ csid->nformats = ARRAY_SIZE(csid_formats);
+ csid->testgen.modes = csid_testgen_modes;
+ csid->testgen.nmodes = CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2;
+}
+
+static void csid_subdev_event(struct csid_device *csid, unsigned int evt_type, void *arg)
+{
+ int evt_data = *(int *)arg;
+
+ switch (evt_type) {
+ case NOTIFY_RUP:
+ csid_reg_update(csid, evt_data);
+ break;
+ case NOTIFY_RUP_CLEAR:
+ csid_reg_update_clear(csid, evt_data);
+ break;
+ default:
+ dev_err(csid->camss->dev, "NOT Supported EVT Type\n");
+ break;
+ }
+}
+
+const struct csid_hw_ops csid_ops_gen3 = {
+ .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,
+ .event = csid_subdev_event,
+};
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.h b/drivers/media/platform/qcom/camss/camss-csid-gen3.h
new file mode 100644
index 000000000000..aa5bd5f2fec5
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * camss-csid-gen3.h
+ *
+ * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module Generation 1
+ *
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#ifndef QC_MSM_CAMSS_CSID_GEN3_H
+#define QC_MSM_CAMSS_CSID_GEN3_H
+
+#define DECODE_FORMAT_UNCOMPRESSED_8_BIT 0x1
+#define DECODE_FORMAT_UNCOMPRESSED_10_BIT 0x2
+#define DECODE_FORMAT_UNCOMPRESSED_12_BIT 0x3
+#define DECODE_FORMAT_UNCOMPRESSED_14_BIT 0x4
+#define DECODE_FORMAT_UNCOMPRESSED_16_BIT 0x5
+#define DECODE_FORMAT_UNCOMPRESSED_20_BIT 0x6
+#define DECODE_FORMAT_UNCOMPRESSED_24_BIT 0x7
+#define DECODE_FORMAT_PAYLOAD_ONLY 0xf
+
+
+#define PLAIN_FORMAT_PLAIN8 0x0 /* supports DPCM, UNCOMPRESSED_6/8_BIT */
+#define PLAIN_FORMAT_PLAIN16 0x1 /* supports DPCM, UNCOMPRESSED_10/16_BIT */
+#define PLAIN_FORMAT_PLAIN32 0x2 /* supports UNCOMPRESSED_20_BIT */
+
+#endif /* QC_MSM_CAMSS_CSID_GEN3_H */
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index ca654b007441..c1d061f91b78 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -223,6 +223,7 @@ extern const char * const csid_testgen_modes[];
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_gen2;
+extern const struct csid_hw_ops csid_ops_gen3;

/*
* csid_is_lite - Check if CSID is CSID lite.
--
2.17.1


2024-03-20 14:34:21

by Depeng Shao

[permalink] [raw]
Subject: [PATCH v2 3/8] media: qcom: camss: Add new csiphy driver 2-1-2

From: Yongsheng Li <[email protected]>

The CSIPHY register offset of SM8550 is different with
SM8250, so add a new driver for the SM8550 platform.

Signed-off-by: Yongsheng Li <[email protected]>
Co-developed-by: Depeng Shao <[email protected]>
Signed-off-by: Depeng Shao <[email protected]>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../platform/qcom/camss/camss-csiphy-2-1-2.c | 343 ++++++++++++++++++
.../media/platform/qcom/camss/camss-csiphy.h | 1 +
3 files changed, 345 insertions(+)
create mode 100644 drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c

diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index 0d4389ab312d..28eba4bf3e38 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -9,6 +9,7 @@ qcom-camss-objs += \
camss-csid-gen2.o \
camss-csiphy-2ph-1-0.o \
camss-csiphy-3ph-1-0.o \
+ camss-csiphy-2-1-2.o \
camss-csiphy.o \
camss-ispif.o \
camss-vfe-4-1.o \
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
new file mode 100644
index 000000000000..df7860d7a4f4
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-csiphy-2-1-2.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v2.0
+ *
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include "camss.h"
+#include "camss-csiphy.h"
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+
+
+#define CSIPHY_CMN_CSI_COMMON_CTRLn(n) (0x1000 + 0x4 * (n))
+#define CSIPHY_CMN_CSI_COMMON_CTRL5_CLK_ENABLE BIT(7)
+#define CSIPHY_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
+#define CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID BIT(1)
+#define CSIPHY_CMN_CSI_COMMON_CTRL7_CONTROL 0x7A
+#define CSIPHY_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET BIT(0)
+#define CSIPHY_CMN_CSI_COMMON_CTRL0_SWI_ENABLE_LANESYNC BIT(1)
+#define CSIPHY_CMN_CSI_COMMON_STATUSn(n) (0x10B0 + 0x4 * (n))
+
+#define CSIPHY_DEFAULT_PARAMS 0
+#define CSIPHY_LANE_ENABLE 1
+#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
+#define CSIPHY_SETTLE_CNT_HIGHER_BYTE 3
+#define CSIPHY_DNP_PARAMS 4
+#define CSIPHY_2PH_REGS 5
+#define CSIPHY_3PH_REGS 6
+
+struct csiphy_reg_t {
+ s32 reg_addr;
+ s32 reg_data;
+ s32 delay;
+ u32 csiphy_param_type;
+};
+
+/* 2.1.2 2PH */
+static const struct
+csiphy_reg_t lane_regs_sm8550[5][20] = {
+ {
+ {0x0E90, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0E94, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
+ {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0090, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0094, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
+ {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, 0x01, CSIPHY_DEFAULT_PARAMS},
+ {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, 0x01, CSIPHY_DEFAULT_PARAMS},
+ {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, 0x01, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0000, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0400, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ },
+ {
+ {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0800, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C00, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0094, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x005C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0060, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0064, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0494, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x045C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0460, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0464, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0894, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x085C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0860, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0864, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C94, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C5C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C60, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {0x0C64, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
+ }
+};
+
+static void csiphy_hw_version_read(struct csiphy_device *csiphy,
+ struct device *dev)
+{
+ u32 hw_version;
+
+ writel(CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID,
+ csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(6));
+
+ hw_version = readl_relaxed(csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_STATUSn(12));
+ hw_version |= readl_relaxed(csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_STATUSn(13)) << 8;
+ hw_version |= readl_relaxed(csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_STATUSn(14)) << 16;
+ hw_version |= readl_relaxed(csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_STATUSn(15)) << 24;
+
+ dev_info(dev, "CSIPHY 3PH HW Version = 0x%08x\n", hw_version);
+}
+
+/*
+ * csiphy_reset - Perform software reset on CSIPHY module
+ * @csiphy: CSIPHY device
+ */
+static void csiphy_reset(struct csiphy_device *csiphy)
+{
+ writel_relaxed(0x1, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
+ usleep_range(5000, 8000);
+ writel_relaxed(0x0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
+}
+
+static irqreturn_t csiphy_isr(int irq, void *dev)
+{
+ struct csiphy_device *csiphy = dev;
+ int i;
+
+ for (i = 0; i < 11; i++) {
+ int c = i + 22;
+ u8 val = readl_relaxed(csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_STATUSn(i));
+
+ writel_relaxed(val, csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_CTRLn(c));
+ }
+
+ writel_relaxed(0x1, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(10));
+ writel_relaxed(0x0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(10));
+
+ for (i = 22; i < 33; i++)
+ writel_relaxed(0x0, csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_CTRLn(i));
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * csiphy_settle_cnt_calc - Calculate settle count value
+ *
+ * Helper function to calculate settle count value. This is
+ * based on the CSI2 T_hs_settle parameter which in turn
+ * is calculated based on the CSI2 transmitter link frequency.
+ *
+ * Return settle count value or 0 if the CSI2 link frequency
+ * is not available
+ */
+static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
+{
+ u32 ui; /* ps */
+ u32 timer_period; /* ps */
+ u32 t_hs_prepare_max; /* ps */
+ u32 t_hs_settle; /* ps */
+ u8 settle_cnt;
+
+ if (link_freq <= 0)
+ return 0;
+
+ ui = div_u64(1000000000000LL, link_freq);
+ ui /= 2;
+ t_hs_prepare_max = 85000 + 6 * ui;
+ t_hs_settle = t_hs_prepare_max;
+
+ timer_period = div_u64(1000000000000LL, timer_clk_rate);
+ settle_cnt = t_hs_settle / timer_period - 6;
+
+ return settle_cnt;
+}
+
+static void csiphy_config_lanes(struct csiphy_device *csiphy,
+ u8 settle_cnt)
+{
+ const struct csiphy_reg_t *r;
+ int i, l, array_size;
+ u32 val;
+
+ switch (csiphy->camss->res->version) {
+ case CAMSS_8550:
+ r = &lane_regs_sm8550[0][0];
+ array_size = ARRAY_SIZE(lane_regs_sm8550[0]);
+ break;
+ default:
+ WARN(1, "unknown csi version\n");
+ return;
+ }
+
+ for (l = 0; l < 5; l++) {
+ for (i = 0; i < array_size; i++, r++) {
+ switch (r->csiphy_param_type) {
+ case CSIPHY_SETTLE_CNT_LOWER_BYTE:
+ val = settle_cnt & 0xff;
+ break;
+ case CSIPHY_DNP_PARAMS:
+ continue;
+ default:
+ val = r->reg_data;
+ break;
+ }
+ writel_relaxed(val, csiphy->base + r->reg_addr);
+
+ if (r->delay > 0)
+ udelay(r->delay);
+ }
+ }
+}
+
+static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
+{
+ u8 lane_mask;
+ int i;
+
+ for (i = 0; i < lane_cfg->num_data; i++)
+ lane_mask |= 1 << lane_cfg->data[i].pos;
+
+ return lane_mask;
+}
+
+static void csiphy_lanes_enable(struct csiphy_device *csiphy,
+ struct csiphy_config *cfg,
+ s64 link_freq, u8 lane_mask)
+{
+ struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
+ u8 settle_cnt;
+ u8 val = 0;
+ int i;
+
+ settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
+
+ val = CSIPHY_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+ for (i = 0; i < c->num_data; i++)
+ val |= BIT(c->data[i].pos * 2);
+
+ writel_relaxed(val, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(5));
+
+ val = CSIPHY_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
+ writel_relaxed(val, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(6));
+
+ val = 0x2;
+ writel_relaxed(val, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(7));
+
+ val = 0x0;
+ writel_relaxed(val, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
+
+ csiphy_config_lanes(csiphy, settle_cnt);
+
+ /* IRQ_MASK registers - disable all interrupts */
+ for (i = 11; i < 22; i++)
+ writel_relaxed(0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(i));
+}
+
+static void csiphy_lanes_disable(struct csiphy_device *csiphy,
+ struct csiphy_config *cfg)
+{
+ writel_relaxed(0, csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_CTRLn(5));
+
+ writel_relaxed(0, csiphy->base +
+ CSIPHY_CMN_CSI_COMMON_CTRLn(6));
+}
+
+const struct csiphy_hw_ops csiphy_ops_2_1_2 = {
+ .get_lane_mask = csiphy_get_lane_mask,
+ .hw_version_read = csiphy_hw_version_read,
+ .reset = csiphy_reset,
+ .lanes_enable = csiphy_lanes_enable,
+ .lanes_disable = csiphy_lanes_disable,
+ .isr = csiphy_isr,
+ .event = NULL,
+};
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index ffe1b95eea98..286fdb9f4fdf 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -98,5 +98,6 @@ void msm_csiphy_unregister_entity(struct csiphy_device *csiphy);

extern const struct csiphy_hw_ops csiphy_ops_2ph_1_0;
extern const struct csiphy_hw_ops csiphy_ops_3ph_1_0;
+extern const struct csiphy_hw_ops csiphy_ops_2_1_2;

#endif /* QC_MSM_CAMSS_CSIPHY_H */
--
2.17.1


2024-03-20 15:09:22

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] media: qcom: camss: Add sm8550 support

On 20/03/2024 14:11, Depeng Shao wrote:
> V2:
> - Update some commit messages
> Link to V1: https://lore.kernel.org/all/[email protected]/

Here's your first problem, doesn't apply.

Please rebase on - at a minimum linux-next or on top of this branch

https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/b4/camss-sc8280xp-v6?ref_type=heads

and indicate which branch you've done the work against.

deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git fetch
linux-next
remote: Enumerating objects: 1907, done.
remote: Counting objects: 100% (1860/1860), done.
remote: Compressing objects: 100% (491/491), done.
remote: Total 1907 (delta 1432), reused 1759 (delta 1367), pack-reused 47
Receiving objects: 100% (1907/1907), 1.16 MiB | 8.24 MiB/s, done.
Resolving deltas: 100% (1461/1461), completed with 263 local objects.
From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
+ 226d3c72fcde1...72fb52fb0ac44 master -> linux-next/master
(forced update)
+ 00fe0619c6bef...ad58b0c356a01 pending-fixes ->
linux-next/pending-fixes (forced update)
bf3a69c6861ff..78c3925c048c7 stable -> linux-next/stable
* [new tag] next-20240320 -> next-20240320

deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ git checkout -b
linux-next-24-03-20-sm8550 linux-next/master

Updating files: 100% (18508/18508), done.
branch 'linux-next-24-03-20-sm8550' set up to track 'linux-next/master'.
Switched to a new branch 'linux-next-24-03-20-sm8550'
deckard@sagittarius-a:~/Development/qualcomm/qlt-kernel$ b4 shazam
[email protected]
Grabbing thread from
lore.kernel.org/all/[email protected]/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 10 messages in the thread
Checking attestation on all messages, may take a moment...
---
[PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum
+ Acked-by: Bryan O'Donoghue <[email protected]>
[PATCH v2 2/8] media: qcom: camss: Add subdev notify support
[PATCH v2 3/8] media: qcom: camss: Add new csiphy driver 2-1-2
[PATCH v2 4/8] media: qcom: camss: Add new params for csid_device
[PATCH v2 5/8] media: qcom: camss: Add CSID gen3 driver
[PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550
[PATCH v2 7/8] media: qcom: camss: Add sm8550 resources
[PATCH v2 8/8] media: qcom: camss: Add sm8550 support
---
NOTE: install dkimpy for DKIM signature verification
---
Total patches: 8
---
Applying: media: qcom: camss: Add CAMSS_8550 enum
Patch failed at 0001 media: qcom: camss: Add CAMSS_8550 enum
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: patch failed: drivers/media/platform/qcom/camss/camss.h:78
error: drivers/media/platform/qcom/camss/camss.h: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch

---
bod

2024-03-20 15:22:07

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 3/8] media: qcom: camss: Add new csiphy driver 2-1-2

On 20/03/2024 14:11, Depeng Shao wrote:
> From: Yongsheng Li <[email protected]>
>
> The CSIPHY register offset of SM8550 is different with
> SM8250, so add a new driver for the SM8550 platform.
>
> Signed-off-by: Yongsheng Li <[email protected]>
> Co-developed-by: Depeng Shao <[email protected]>

You're listing yourself ? Twice with a different CD and SOB.
Shouldn't this be
Co-developed-by: Depeng Shao <[email protected]>

?

> Signed-off-by: Depeng Shao <[email protected]>
> ---
> drivers/media/platform/qcom/camss/Makefile | 1 +
> .../platform/qcom/camss/camss-csiphy-2-1-2.c | 343 ++++++++++++++++++
> .../media/platform/qcom/camss/camss-csiphy.h | 1 +
> 3 files changed, 345 insertions(+)
> create mode 100644 drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
>
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index 0d4389ab312d..28eba4bf3e38 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -9,6 +9,7 @@ qcom-camss-objs += \
> camss-csid-gen2.o \
> camss-csiphy-2ph-1-0.o \
> camss-csiphy-3ph-1-0.o \
> + camss-csiphy-2-1-2.o \

File name here is incorrect camss-csiphy-3ph-1-2.o this is a 3 phase
capable PHY right ? So both the filename and the commit title need to
reflect that.

I thought we had discussed offline using an offset instead of a new file ?

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/6dc8f49ef7c6ca69783aa02bdca81c77e66ecc0d

Then just set the offset

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/022a837257d9e1fa8070f0dfa2f683e903111aa0


> camss-csiphy.o \
> camss-ispif.o \
> camss-vfe-4-1.o \
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
> new file mode 100644
> index 000000000000..df7860d7a4f4
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * camss-csiphy-2-1-2.c
> + *
> + * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v2.0
> + *
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include "camss.h"
> +#include "camss-csiphy.h"
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +
> +
> +#define CSIPHY_CMN_CSI_COMMON_CTRLn(n) (0x1000 + 0x4 * (n))
> +#define CSIPHY_CMN_CSI_COMMON_CTRL5_CLK_ENABLE BIT(7)
> +#define CSIPHY_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
> +#define CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID BIT(1)
> +#define CSIPHY_CMN_CSI_COMMON_CTRL7_CONTROL 0x7A
> +#define CSIPHY_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET BIT(0)
> +#define CSIPHY_CMN_CSI_COMMON_CTRL0_SWI_ENABLE_LANESYNC BIT(1)
> +#define CSIPHY_CMN_CSI_COMMON_STATUSn(n) (0x10B0 + 0x4 * (n))
> +
> +#define CSIPHY_DEFAULT_PARAMS 0
> +#define CSIPHY_LANE_ENABLE 1
> +#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
> +#define CSIPHY_SETTLE_CNT_HIGHER_BYTE 3
> +#define CSIPHY_DNP_PARAMS 4
> +#define CSIPHY_2PH_REGS 5
> +#define CSIPHY_3PH_REGS 6
> +
> +struct csiphy_reg_t {
> + s32 reg_addr;
> + s32 reg_data;
> + s32 delay;
> + u32 csiphy_param_type;
> +};
> +
> +/* 2.1.2 2PH */
> +static const struct
> +csiphy_reg_t lane_regs_sm8550[5][20] = {

So per the tree I shared with you..

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/fa666c241eb530a08aa6b391e15b018296e93f66

We don't need to break this up into a multi-dimensional array

its just a straight write of values
> + {
> + {0x0E90, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0E94, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
> + {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0090, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0094, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
> + {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, 0x01, CSIPHY_DEFAULT_PARAMS},
> + {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, 0x01, CSIPHY_DEFAULT_PARAMS},
> + {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, 0x01, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0000, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0400, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
> + },
> + {
> + {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0800, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C00, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
> + {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0094, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x005C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0060, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0064, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0494, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x045C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0460, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0464, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0894, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x085C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0860, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0864, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C94, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C5C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C60, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
> + {0x0C64, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
> + }
> +};
> +
> +static void csiphy_hw_version_read(struct csiphy_device *csiphy,
> + struct device *dev)
> +{
> + u32 hw_version;
> +
> + writel(CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID,
> + csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(6));
> +
> + hw_version = readl_relaxed(csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_STATUSn(12));
> + hw_version |= readl_relaxed(csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_STATUSn(13)) << 8;
> + hw_version |= readl_relaxed(csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_STATUSn(14)) << 16;
> + hw_version |= readl_relaxed(csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_STATUSn(15)) << 24;
> +
> + dev_info(dev, "CSIPHY 3PH HW Version = 0x%08x\n", hw_version);
> +}
> +
> +/*
> + * csiphy_reset - Perform software reset on CSIPHY module
> + * @csiphy: CSIPHY device
> + */
> +static void csiphy_reset(struct csiphy_device *csiphy)
> +{
> + writel_relaxed(0x1, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
> + usleep_range(5000, 8000);
> + writel_relaxed(0x0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
> +}
> +
> +static irqreturn_t csiphy_isr(int irq, void *dev)
> +{
> + struct csiphy_device *csiphy = dev;
> + int i;
> +
> + for (i = 0; i < 11; i++) {
> + int c = i + 22;
> + u8 val = readl_relaxed(csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_STATUSn(i));
> +
> + writel_relaxed(val, csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_CTRLn(c));
> + }
> +
> + writel_relaxed(0x1, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(10));
> + writel_relaxed(0x0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(10));
> +
> + for (i = 22; i < 33; i++)
> + writel_relaxed(0x0, csiphy->base +
> + CSIPHY_CMN_CSI_COMMON_CTRLn(i));
> +
> + return IRQ_HANDLED;
> +}
> +
> +/*
> + * csiphy_settle_cnt_calc - Calculate settle count value
> + *
> + * Helper function to calculate settle count value. This is
> + * based on the CSI2 T_hs_settle parameter which in turn
> + * is calculated based on the CSI2 transmitter link frequency.
> + *
> + * Return settle count value or 0 if the CSI2 link frequency
> + * is not available
> + */
> +static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
> +{
> + u32 ui; /* ps */
> + u32 timer_period; /* ps */
> + u32 t_hs_prepare_max; /* ps */
> + u32 t_hs_settle; /* ps */
> + u8 settle_cnt;
> +
> + if (link_freq <= 0)
> + return 0;
> +
> + ui = div_u64(1000000000000LL, link_freq);
> + ui /= 2;
> + t_hs_prepare_max = 85000 + 6 * ui;
> + t_hs_settle = t_hs_prepare_max;
> +
> + timer_period = div_u64(1000000000000LL, timer_clk_rate);
> + settle_cnt = t_hs_settle / timer_period - 6;
> +
> + return settle_cnt;
> +}

Yep this is all literal copy/paste of existing code.

Please try taking

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/aa27ac8e1ffedd48c5ef6ac0f75f1f15716fe296

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/fa666c241eb530a08aa6b391e15b018296e93f66

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/e9e99dee4e723755274e6430bb43adfbf77d2a1a

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/5dc53c39b96eb22bdfe47554f047e6b63ddb25c0

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/36655b353fa71269812cff0e695341ff12042546

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/6dc8f49ef7c6ca69783aa02bdca81c77e66ecc0d

and

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/042ec64867d43e014c2b369db3b92b4f5432497f

into your tree

it works for me on sm8250 and is WIP for x1e80100

Point being we need to make an effort to unify/reuse/fix init sequences
upstream not to copy/paste from one file to another.

> +
> +static void csiphy_config_lanes(struct csiphy_device *csiphy,
> + u8 settle_cnt)
> +{
> + const struct csiphy_reg_t *r;
> + int i, l, array_size;
> + u32 val;
> +
> + switch (csiphy->camss->res->version) {
> + case CAMSS_8550:
> + r = &lane_regs_sm8550[0][0];
> + array_size = ARRAY_SIZE(lane_regs_sm8550[0]);
> + break;
> + default:
> + WARN(1, "unknown csi version\n");
> + return;
> + }
> +
> + for (l = 0; l < 5; l++) {

These hard-coded values in control loops must be done away with upstream.

---
bod

2024-03-20 15:41:26

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] media: qcom: camss: Add CSID gen3 driver

On 20/03/2024 14:11, Depeng Shao wrote:
> The CSID in SM8550 is gen3, it has new register
> offset and new functionality, the buf done irq,
> register update and reset is moved to CSID gen3.
>
> Signed-off-by: Depeng Shao <[email protected]>
> Co-developed-by: Yongsheng Li <[email protected]>
> Signed-off-by: Yongsheng Li <[email protected]>
> ---
> drivers/media/platform/qcom/camss/Makefile | 1 +
> .../platform/qcom/camss/camss-csid-gen3.c | 639 ++++++++++++++++++
> .../platform/qcom/camss/camss-csid-gen3.h | 26 +
> .../media/platform/qcom/camss/camss-csid.h | 1 +
> 4 files changed, 667 insertions(+)
> create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.c
> create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.h
>
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index 28eba4bf3e38..c5fcd6eec0f2 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -7,6 +7,7 @@ qcom-camss-objs += \
> camss-csid-4-1.o \
> camss-csid-4-7.o \
> camss-csid-gen2.o \
> + camss-csid-gen3.o \
> camss-csiphy-2ph-1-0.o \
> camss-csiphy-3ph-1-0.o \
> camss-csiphy-2-1-2.o \
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> new file mode 100644
> index 000000000000..b97005f7065d
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> @@ -0,0 +1,639 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * camss-csid-gen3.c
> + *
> + * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
> + *
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/delay.h>

Please alphanumerically sort your include list.

> +
> +
> +#include "camss-csid.h"
> +#include "camss-csid-gen3.h"
> +#include "camss.h"

and here.

> +
> +#define CSID_TOP_IO_PATH_CFG0(csid) (0x4 * (csid))
> +#define OUTPUT_IFE_EN 0x100
> +#define INTERNAL_CSID 1
> +
> +#define CSID_HW_VERSION 0x0
> +#define HW_VERSION_STEPPING 0
> +#define HW_VERSION_REVISION 16
> +#define HW_VERSION_GENERATION 28
> +
> +#define CSID_RST_CFG 0xC
> +#define RST_MODE 0
> +#define RST_LOCATION 4
> +
> +#define CSID_RST_CMD 0x10
> +#define SELECT_HW_RST 0
> +#define SELECT_SW_RST 1
> +#define SELECT_IRQ_RST 2
> +
> +#define CSID_CSI2_RX_IRQ_STATUS 0x9C
> +#define CSID_CSI2_RX_IRQ_MASK 0xA0
> +#define CSID_CSI2_RX_IRQ_CLEAR 0xA4
> +#define CSID_CSI2_RX_IRQ_SET 0xA8
> +
> +#define CSID_CSI2_RDIN_IRQ_STATUS(rdi) (0xEC + 0x10 * (rdi))
> +#define CSID_CSI2_RDIN_IRQ_MASK(rdi) (0xF0 + 0x10 * (rdi))
> +#define CSID_CSI2_RDIN_INFO_FIFO_FULL 2
> +#define CSID_CSI2_RDIN_INFO_CAMIF_EOF 3
> +#define CSID_CSI2_RDIN_INFO_CAMIF_SOF 4
> +#define CSID_CSI2_RDIN_INFO_INPUT_EOF 9
> +#define CSID_CSI2_RDIN_INFO_INPUT_SOF 12
> +#define CSID_CSI2_RDIN_ERROR_REC_FRAME_DROP 18
> +#define CSID_CSI2_RDIN_ERROR_REC_OVERFLOW_IRQ 19
> +#define CSID_CSI2_RDIN_ERROR_REC_CCIF_VIOLATION 20
> +#define CSID_CSI2_RDIN_EPOCH0 21
> +#define CSID_CSI2_RDIN_EPOCH1 22
> +#define CSID_CSI2_RDIN_RUP_DONE 23
> +#define CSID_CSI2_RDIN_CCIF_VIOLATION 29
> +
> +#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi) (0xF4 + 0x10 * (rdi))
> +#define CSID_CSI2_RDIN_IRQ_SET(rdi) (0xF8 + 0x10 * (rdi))
> +
> +#define CSID_TOP_IRQ_STATUS 0x7C
> +#define TOP_IRQ_STATUS_RESET_DONE 0
> +#define CSID_TOP_IRQ_MASK 0x80
> +#define CSID_TOP_IRQ_CLEAR 0x84
> +#define CSID_TOP_IRQ_SET 0x88
> +#define CSID_IRQ_CMD 0x14
> +#define IRQ_CMD_CLEAR 0
> +#define IRQ_CMD_SET 4
> +
> +#define CSID_BUF_DONE_IRQ_STATUS 0x8C
> +#define BUF_DONE_IRQ_STATUS_RDI_OFFSET (csid_is_lite(csid) ? 1 : 14)

Stick with hex please 0x01 : 0x0e

> +#define CSID_BUF_DONE_IRQ_MASK 0x90
> +#define CSID_BUF_DONE_IRQ_CLEAR 0x94
> +#define CSID_BUF_DONE_IRQ_SET 0x98
> +
> +#define CSID_CSI2_RX_CFG0 0x200
> +#define CSI2_RX_CFG0_NUM_ACTIVE_LANES 0
> +#define CSI2_RX_CFG0_DL0_INPUT_SEL 4
> +#define CSI2_RX_CFG0_DL1_INPUT_SEL 8
> +#define CSI2_RX_CFG0_DL2_INPUT_SEL 12
> +#define CSI2_RX_CFG0_DL3_INPUT_SEL 16
> +#define CSI2_RX_CFG0_PHY_NUM_SEL 20
> +#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
> +#define CSI2_RX_CFG0_PHY_TYPE_SEL 24
> +#define CSI2_RX_CFG0_TPG_MUX_EN 27
> +#define CSI2_RX_CFG0_TPG_NUM_SEL 28
> +
> +#define CSID_CSI2_RX_CFG1 0x204
> +#define CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN 0
> +#define CSI2_RX_CFG1_DE_SCRAMBLE_EN 1
> +#define CSI2_RX_CFG1_VC_MODE 2
> +#define CSI2_RX_CFG1_COMPLETE_STREAM_EN 4
> +#define CSI2_RX_CFG1_COMPLETE_STREAM_FRAME_TIMING 5
> +#define CSI2_RX_CFG1_MISR_EN 6
> +#define CSI2_RX_CFG1_PHY_BIST_EN 7
> +#define CSI2_RX_CFG1_EPD_MODE 8
> +#define CSI2_RX_CFG1_EOTP_EN 9
> +#define CSI2_RX_CFG1_DYN_SWITCH_EN 10
> +#define CSI2_RX_CFG1_RUP_AUP_LATCH_DIS 11
> +
> +#define CSID_CSI2_RX_CAPTURE_CTRL 0x208
> +#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_EN 0
> +#define CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_EN 1
> +#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_EN 3
> +#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_DT 4
> +#define CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_VC 10
> +#define CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_VC 15
> +#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_DT 20
> +#define CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_VC 26
> +
> +#define CSID_RDI_CFG0(rdi) (0x500 + 0x100 * (rdi))
> +#define RDI_CFG0_VFR_EN 0
> +#define RDI_CFG0_FRAME_ID_DEC_EN 1
> +#define RDI_CFG0_RETIME_DIS 5
> +#define RDI_CFG0_TIMESTAMP_EN 6
> +#define RDI_CFG0_TIMESTAMP_STB_SEL 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 31
> +
> +#define CSID_RDI_CFG1(rdi) (0x510 + 0x100 * (rdi))
> +#define RDI_CFG1_BYTE_CNTR_EN 2
> +#define RDI_CFG1_DROP_H_EN 5
> +#define RDI_CFG1_DROP_V_EN 6
> +#define RDI_CFG1_CROP_H_EN 7
> +#define RDI_CFG1_CROP_V_EN 8
> +#define RDI_CFG1_MISR_EN 9
> +#define RDI_CFG1_PIX_STORE 10
> +#define RDI_CFG1_PLAIN_ALIGNMENT 11
> +#define RDI_CFG1_PLAIN_FORMAT 12
> +#define RDI_CFG1_EARLY_EOF_EN 14
> +#define RDI_CFG1_PACKING_FORMAT 15
> +
> +#define CSID_RDI_CTRL(rdi) (0x504 + 0x100 * (rdi))
> +#define RDI_CTRL_START_CMD 0
> +#define RDI_CTRL_START_MODE 2
> +
> +#define CSID_RDI_EPOCH_IRQ_CFG(rdi) (0x52C + 0x100 * (rdi))
> +
> +#define CSID_RDI_FRM_DROP_PATTERN(rdi) (0x540 + 0x100 * (rdi))
> +#define CSID_RDI_FRM_DROP_PERIOD(rdi) (0x544 + 0x100 * (rdi))
> +#define CSID_RDI_IRQ_SUBSAMPLE_PATTERN(rdi) (0x548 + 0x100 * (rdi))
> +#define CSID_RDI_IRQ_SUBSAMPLE_PERIOD(rdi) (0x54C + 0x100 * (rdi))
> +#define CSID_RDI_RPP_PIX_DROP_PATTERN(rdi) (0x558 + 0x100 * (rdi))
> +#define CSID_RDI_RPP_PIX_DROP_PERIOD(rdi) (0x55C + 0x100 * (rdi))
> +#define CSID_RDI_RPP_LINE_DROP_PATTERN(rdi) (0x560 + 0x100 * (rdi))
> +#define CSID_RDI_RPP_LINE_DROP_PERIOD(rdi) (0x564 + 0x100 * (rdi))
> +
> +#define CSID_RDI_RPP_HCROP(rdi) (0x550 + 0x100 * (rdi))
> +#define CSID_RDI_RPP_VCROP(rdi) (0x554 + 0x100 * (rdi))
> +
> +#define CSID_RDI_ERROR_RECOVERY_CFG0(rdi) (0x514 + 0x100 * (rdi))
> +
> +#define CSID_DISCARD_FRAMES 4
> +
> +#define CSID_REG_UPDATE_CMD 0x18
> +static inline int reg_update_rdi(struct csid_device *csid, int n)
> +{
> + return BIT(n + 4) + BIT(20 + n);
> +}
> +
> +#define REG_UPDATE_RDI reg_update_rdi
> +
> +static const struct csid_format csid_formats[] = {
> + {
> + MEDIA_BUS_FMT_UYVY8_1X16,
> + DATA_TYPE_YUV422_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 2,
> + },
> + {
> + MEDIA_BUS_FMT_VYUY8_1X16,
> + DATA_TYPE_YUV422_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 2,
> + },
> + {
> + MEDIA_BUS_FMT_YUYV8_1X16,
> + DATA_TYPE_YUV422_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 2,
> + },
> + {
> + MEDIA_BUS_FMT_YVYU8_1X16,
> + DATA_TYPE_YUV422_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 2,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_Y8_1X8,
> + DATA_TYPE_RAW_8BIT,
> + DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> + 8,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_Y10_1X10,
> + DATA_TYPE_RAW_10BIT,
> + DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> + 10,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB12_1X12,
> + DATA_TYPE_RAW_12BIT,
> + DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> + 12,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SBGGR14_1X14,
> + DATA_TYPE_RAW_14BIT,
> + DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> + 14,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGBRG14_1X14,
> + DATA_TYPE_RAW_14BIT,
> + DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> + 14,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SGRBG14_1X14,
> + DATA_TYPE_RAW_14BIT,
> + DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> + 14,
> + 1,
> + },
> + {
> + MEDIA_BUS_FMT_SRGGB14_1X14,
> + DATA_TYPE_RAW_14BIT,
> + DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> + 14,
> + 1,
> + },
> +};
> +
> +static void __csid_configure_rx(struct csid_device *csid,
> + struct csid_phy_config *phy, int vc)
> +{
> + 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_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
> +
> + val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
> + if (vc > 3)
> + val |= 1 << CSI2_RX_CFG1_VC_MODE;
> + writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
> +}
> +
> +static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
> +{
> + int val;
> +
> + if (enable)
> + val = 1 << RDI_CTRL_START_CMD;
> + else
> + val = 0 << RDI_CTRL_START_CMD;
> + writel_relaxed(val, csid->base + CSID_RDI_CTRL(rdi));
> +}
> +
> +static void __csid_configure_top(struct csid_device *csid)
> +{
> + u32 val;
> +
> + if (csid->top_base) {
> + val = OUTPUT_IFE_EN | INTERNAL_CSID;
> + writel_relaxed(val, csid->top_base + CSID_TOP_IO_PATH_CFG0(csid->id));
> + }
> +}
> +
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
> +{
> + u32 val;
> + u8 lane_cnt = csid->phy.lane_cnt;
> + /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
> + struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
> + const struct csid_format *format = csid_get_fmt_entry(csid->formats, csid->nformats,
> + input_format->code);
> +
> + if (!lane_cnt)
> + lane_cnt = 4;
> +
> + val = 0;
> + writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(vc));
> +
> + /*
> + * 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;
> +
> + val = 1 << RDI_CFG0_TIMESTAMP_EN;
> + val |= 2 << RDI_CFG0_TIMESTAMP_STB_SEL;
> + /* 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_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
> +
> + val = 1 << RDI_CFG1_PACKING_FORMAT;
> + val |= 1 << RDI_CFG1_PIX_STORE;
> + val |= 1 << RDI_CFG1_DROP_H_EN;
> + val |= 1 << RDI_CFG1_DROP_V_EN;
> + val |= 1 << RDI_CFG1_CROP_H_EN;
> + val |= 1 << RDI_CFG1_CROP_V_EN;
> + val |= RDI_CFG1_EARLY_EOF_EN;
> +
> + writel_relaxed(val, csid->base + CSID_RDI_CFG1(vc));
> +
> + val = 0;
> + writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
> +
> + val = 1;
> + writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
> +
> + val = 0;
> + writel_relaxed(val, csid->base + CSID_RDI_CTRL(vc));
> +
> + val = readl_relaxed(csid->base + CSID_RDI_CFG0(vc));
> + val |= enable << RDI_CFG0_EN;
> + writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
> +}

Hmm.

So I think 90% of the code here can be moved into a shared file -
ideally instantiated in gen2.c and then reused here rather than
copy/paste from one file to the other.

Lets sync up and try to get a unified tree for x1e80100 and sm8550 for
that purpose.

I think the code you have here is slightly further along that the
CSID/VFE stuff I've been working with.

But still - reductio ad absurdum - we need to functionally decompose and
not replicate code.

> +
> +static void csid_configure_stream(struct csid_device *csid, u8 enable)
> +{
> + u8 i;
> +
> + /* Loop through all enabled VCs and configure stream for each */
> + for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
> + if (csid->phy.en_vc & BIT(i)) {
> + __csid_configure_top(csid);
> + __csid_configure_rdi_stream(csid, enable, i);
> + __csid_configure_rx(csid, &csid->phy, i);
> + __csid_ctrl_rdi(csid, enable, i);
> + }
> +}

Another example, straight copy/paste - we need to zap all of that.

---
bod

2024-03-20 15:51:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] media: qcom: camss: Add sm8550 resources

On 20/03/2024 15:11, Depeng Shao wrote:
>
> +static const struct camss_resources sm8550_resources = {
> + .version = CAMSS_8550,
> + .pd_name = "top",
> + .csiphy_res = csiphy_res_8550,
> + .csid_res = csid_res_8550,
> + .vfe_res = vfe_res_8550,
> + .icc_res = icc_res_sm8550,
> + .icc_path_num = ARRAY_SIZE(icc_res_sm8550),
> + .csiphy_num = ARRAY_SIZE(csiphy_res_8550),
> + .csid_num = ARRAY_SIZE(csid_res_8550),
> + .vfe_num = ARRAY_SIZE(vfe_res_8550),
> +};
> +
> static const struct of_device_id camss_dt_match[] = {
> { .compatible = "qcom,msm8916-camss", .data = &msm8916_resources },
> { .compatible = "qcom,msm8996-camss", .data = &msm8996_resources },
> @@ -2189,6 +2533,7 @@ static const struct of_device_id camss_dt_match[] = {
> { .compatible = "qcom,sdm845-camss", .data = &sdm845_resources },
> { .compatible = "qcom,sm8250-camss", .data = &sm8250_resources },
> { .compatible = "qcom,sc8280xp-camss", .data = &sc8280xp_resources },
> + { .compatible = "qcom,sm8550-camss", .data = &sm8550_resources },

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.

Do not ignore checkpatch warnings.

Also, because you did not post bindings, I am afraid of sneaking here
stuff which is not accepted upstream. Therefore please post DTS in
separate patchset, for public review.
> { }
> };
>

Best regards,
Krzysztof


2024-03-20 15:57:56

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550

On 20/03/2024 14:11, Depeng Shao wrote:
> From: Yongsheng Li <[email protected]>
>
> Add IFE driver for SM8550, the main difference with
> old HW is register offset is different, register
> update, reset and buf done is moved to CSID. And
> the image address support 36 bits, so need to right
> shift the image address when configuring the write
> master.
>
> Signed-off-by: Yongsheng Li <[email protected]>
> Co-developed-by: Depeng Shao <[email protected]>
> Signed-off-by: Depeng Shao <[email protected]>

Same comment with your co-developed and SOB

> ---
> drivers/media/platform/qcom/camss/Makefile | 1 +
> .../media/platform/qcom/camss/camss-vfe-780.c | 455 ++++++++++++++++++
> drivers/media/platform/qcom/camss/camss-vfe.h | 1 +
> 3 files changed, 457 insertions(+)
> create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c
>
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index c5fcd6eec0f2..ac40bbab18a3 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -18,6 +18,7 @@ qcom-camss-objs += \
> camss-vfe-4-8.o \
> camss-vfe-17x.o \
> camss-vfe-480.o \
> + camss-vfe-780.o \
> camss-vfe-gen1.o \
> camss-vfe.o \
> camss-video.o \
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c
> new file mode 100644
> index 000000000000..a78647f23c8c
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * camss-vfe-780.c
> + *
> + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v780 (SM8550)
> + *
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +
> +#include "camss.h"
> +#include "camss-vfe.h"
> +
> +#define VFE_HW_VERSION (vfe_is_lite(vfe) ? 0x1000 : 0x0)
> +
> +#define VFE_IRQ_CMD (vfe_is_lite(vfe) ? 0x1038 : 0x30)
> +#define IRQ_CMD_GLOBAL_CLEAR BIT(0)
> +
> +#define VFE_IRQ_MASK(n) ((vfe_is_lite(vfe) ? 0x1024 : 0x34) + (n) * 4)
> +#define IRQ_MASK_0_BUS_TOP_IRQ (vfe_is_lite(vfe) ? BIT(0) | BIT(1) | BIT(2) : \
> + BIT(0) | BIT(4) | BIT(18))
> +#define IRQ_MASK_1_BUS_TOP_IRQ(n) (vfe_is_lite(vfe) ? BIT(2 * n + 2) | BIT(2 * n + 3) : \
> + BIT(2 * n + 8) | BIT(2 * n + 9))
> +#define VFE_IRQ_CLEAR(n) ((vfe_is_lite(vfe) ? 0x102C : 0x3C) + (n) * 4)
> +#define VFE_IRQ_STATUS(n) ((vfe_is_lite(vfe) ? 0x101C : 0x44) + (n) * 4)
> +
> +#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x1200 : 0xC00)
> +
> +#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 + 0xdc)
> +
> +#define VFE_BUS_WM_CFG(n) (BUS_REG_BASE + 0x200 + (n) * 0x100)
> +#define WM_CFG_EN (0)
> +#define WM_VIR_FRM_EN (1)
> +#define WM_CFG_MODE (16)
> +#define MODE_QCOM_PLAIN (0)
> +#define MODE_MIPI_RAW (1)
> +#define VFE_BUS_WM_IMAGE_ADDR(n) (BUS_REG_BASE + 0x204 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAME_INCR(n) (BUS_REG_BASE + 0x208 + (n) * 0x100)
> +#define VFE_BUS_WM_IMAGE_CFG_0(n) (BUS_REG_BASE + 0x20c + (n) * 0x100)
> +#define WM_IMAGE_CFG_0_DEFAULT_WIDTH (0xFFFF)
> +#define VFE_BUS_WM_IMAGE_CFG_1(n) (BUS_REG_BASE + 0x210 + (n) * 0x100)
> +#define VFE_BUS_WM_IMAGE_CFG_2(n) (BUS_REG_BASE + 0x214 + (n) * 0x100)
> +#define WM_IMAGE_CFG_2_DEFAULT_STRIDE (0xFFFF)
> +#define VFE_BUS_WM_PACKER_CFG(n) (BUS_REG_BASE + 0x218 + (n) * 0x100)
> +#define VFE_BUS_WM_HEADER_ADDR(n) (BUS_REG_BASE + 0x220 + (n) * 0x100)
> +#define VFE_BUS_WM_HEADER_INCR(n) (BUS_REG_BASE + 0x224 + (n) * 0x100)
> +#define VFE_BUS_WM_HEADER_CFG(n) (BUS_REG_BASE + 0x228 + (n) * 0x100)
> +
> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n) (BUS_REG_BASE + 0x230 + (n) * 0x100)
> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n) (BUS_REG_BASE + 0x234 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAMEDROP_PERIOD(n) (BUS_REG_BASE + 0x238 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAMEDROP_PATTERN(n) (BUS_REG_BASE + 0x23c + (n) * 0x100)
> +
> +#define VFE_BUS_WM_MMU_PREFETCH_CFG(n) (BUS_REG_BASE + 0x260 + (n) * 0x100)
> +#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n) (BUS_REG_BASE + 0x264 + (n) * 0x100)
> +#define VFE_BUS_WM_SYSTEM_CACHE_CFG(n) (BUS_REG_BASE + 0x268 + (n) * 0x100)
> +
> +
> +/* for titan 780, each bus client is hardcoded to a specific path */
> +#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0 : 23) + (n))

No admixture of hex and decimal please.

> +
> +#define MAX_VFE_OUTPUT_LINES 4
> +#define MAX_VFE_ACT_BUF 1
> +
> +static u32 vfe_hw_version(struct vfe_device *vfe)
> +{
> + u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
> +
> + u32 gen = (hw_version >> 28) & 0xF;
> + u32 rev = (hw_version >> 16) & 0xFFF;
> + u32 step = hw_version & 0xFFFF;
> +
> + dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step);
> +
> + return hw_version;
> +}

Same comment as with CSID, its time to rationalise all of this
replicated code down to one place, instead of proliferating it further.

> +static void vfe_global_reset(struct vfe_device *vfe)
> +{
> +}
> +
> +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); /* map to actual WM used (from wm=RDI index) */
> +
> + /* no clock gating at bus input */
> + writel_relaxed(0, vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
> +
> + writel_relaxed(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL);
> +
> + writel_relaxed(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8,
> + vfe->base + VFE_BUS_WM_FRAME_INCR(wm));
> + writel_relaxed((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF),
> + vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm));
> + writel_relaxed(WM_IMAGE_CFG_2_DEFAULT_STRIDE,
> + vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm));
> + writel_relaxed(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm));
> +
> + /* no dropped frames, one irq per frame */
> + writel_relaxed(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm));
> + writel_relaxed(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm));
> + writel_relaxed(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm));
> + writel_relaxed(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm));
> +
> + writel_relaxed(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm));
> + writel_relaxed(0xFFFFFFFF, vfe->base + VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm));
> +
> + writel_relaxed(1 << WM_CFG_EN | MODE_MIPI_RAW << 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); /* map to actual WM used (from wm=RDI index) */
> + writel_relaxed(0, vfe->base + VFE_BUS_WM_CFG(wm));
> +}

vfe_wm_stop() as an example can/should live in a shared file.

As proof of concept code its fine to copy/paste between files but, to
merge we need to get rid of any replicated code - introducing
indirection/offsets as necessary.

> +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr,
> + struct vfe_line *line)
> +{
> + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */
> + writel_relaxed((addr >> 8) & 0xFFFFFFFF, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
> +
> + dev_dbg(vfe->camss->dev, "wm:%d, image buf addr:0x%llx\n",
> + wm, addr);
> +}
> +
> +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
> +{
> + int port_id = line_id;
> +
> + v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP, (void *)&port_id);
> +}
> +
> +static inline void vfe_reg_update_clear(struct vfe_device *vfe,
> + enum vfe_line_id line_id)
> +{
> + int port_id = line_id;
> +
> + v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP_CLEAR, (void *)&port_id);
> +}

I'm not sure I quite understand why we need to use this API to clear
registers inside of the address space of the same driver.

Is there not a much more direct way to write our _internal_ registers ?


> +static void vfe_enable_irq_common(struct vfe_device *vfe, enum vfe_line_id line_id)
> +{
> + int port_id = line_id;
> +
> + /* enable top BUS status IRQ */
> + writel_relaxed(IRQ_MASK_0_BUS_TOP_IRQ,
> + vfe->base + VFE_IRQ_MASK(0));
> +
> + writel_relaxed(IRQ_MASK_1_BUS_TOP_IRQ(port_id),
> + vfe->base + VFE_IRQ_MASK(1));
> +}
> +
> +/*
> + * vfe_isr - VFE module interrupt handler
> + * @irq: Interrupt line
> + * @dev: VFE device
> + *
> + * Return IRQ_HANDLED on success
> + */
> +static irqreturn_t vfe_isr(int irq, void *dev)
> +{
> + struct vfe_device *vfe = dev;
> + u32 status;
> +
> + status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
> + writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
> + writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
> +
> + if (status)
> + dev_dbg(vfe->camss->dev, "Top Status_0:0x%x\n", status);
> +
> + status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(1));
> + writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(1));
> + writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
> +
> + if (status)
> + dev_dbg(vfe->camss->dev, "Top Status_1:0x%x\n", status);
> +
> + return IRQ_HANDLED;
> +}

Why is this ISR required ? What does it do ?

Does it actually run on your reference hardware ?

If the purpose of the ISR is just to clear the status registers, why
even enable it ?

> +
> +/*
> + * vfe_halt - Trigger halt on VFE module and wait to complete
> + * @vfe: VFE device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int vfe_halt(struct vfe_device *vfe)
> +{
> + /* rely on vfe_disable_output() to stop the VFE */
> + return 0;
> +}
> +
> +static int vfe_get_output(struct vfe_line *line)
> +{
> + struct vfe_device *vfe = to_vfe(line);
> + struct vfe_output *output;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&vfe->output_lock, flags);
> +
> + output = &line->output;
> + if (output->state > VFE_OUTPUT_RESERVED) {
> + dev_err(vfe->camss->dev, "Output is running\n");
> + goto error;
> + }
> +
> + output->wm_num = 1;
> +
> + /* Correspondence between VFE line number and WM number.
> + * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3
> + * Note this 1:1 mapping will not work for PIX streams.
> + */
> + output->wm_idx[0] = line->id;
> + vfe->wm_output_map[line->id] = line->id;
> +
> + output->drop_update_idx = 0;
> +
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> +
> + return 0;
> +
> +error:
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> + output->state = VFE_OUTPUT_OFF;
> +
> + return -EINVAL;
> +}
> +
> +static int vfe_enable_output(struct vfe_line *line)
> +{
> + struct vfe_device *vfe = to_vfe(line);
> + struct vfe_output *output = &line->output;
> + unsigned long flags;
> + unsigned int i;
> +
> + spin_lock_irqsave(&vfe->output_lock, flags);
> +
> + vfe_reg_update_clear(vfe, line->id);
> +
> + if (output->state > VFE_OUTPUT_RESERVED) {
> + dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
> + output->state);
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> + return -EINVAL;
> + }
> +
> + WARN_ON(output->gen2.active_num);
> +
> + output->state = VFE_OUTPUT_ON;
> +
> + output->sequence = 0;
> +
> + vfe_wm_start(vfe, output->wm_idx[0], line);
> +
> + for (i = 0; i < MAX_VFE_ACT_BUF; i++) {
> + output->buf[i] = vfe_buf_get_pending(output);
> + if (!output->buf[i])
> + break;
> + output->gen2.active_num++;
> + vfe_wm_update(vfe, output->wm_idx[0], output->buf[i]->addr[0], line);
> +
> + vfe_reg_update(vfe, line->id);
> + }
> +
> + spin_unlock_irqrestore(&vfe->output_lock, flags);
> +
> + return 0;
> +}

So you'll see with the WIP code for x1e80100 which is I think the same
VFE - no one generation less 680 - that this code is copy/pasted.

But there's again no good reason for that. Common code needs to be
squashed down into one place.

---
bod

2024-03-20 16:12:53

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] media: qcom: camss: Add CSID gen3 driver

On 20/03/2024 15:40, Bryan O'Donoghue wrote:
> +static const struct csid_format csid_formats[] = {
> +    {
> +        MEDIA_BUS_FMT_UYVY8_1X16,
> +        DATA_TYPE_YUV422_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        2,
> +    },
> +    {
> +        MEDIA_BUS_FMT_VYUY8_1X16,
> +        DATA_TYPE_YUV422_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        2,
> +    },
> +    {
> +        MEDIA_BUS_FMT_YUYV8_1X16,
> +        DATA_TYPE_YUV422_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        2,
> +    },
> +    {
> +        MEDIA_BUS_FMT_YVYU8_1X16,
> +        DATA_TYPE_YUV422_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        2,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SBGGR8_1X8,
> +        DATA_TYPE_RAW_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGBRG8_1X8,
> +        DATA_TYPE_RAW_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGRBG8_1X8,
> +        DATA_TYPE_RAW_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SRGGB8_1X8,
> +        DATA_TYPE_RAW_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SBGGR10_1X10,
> +        DATA_TYPE_RAW_10BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> +        10,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGBRG10_1X10,
> +        DATA_TYPE_RAW_10BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> +        10,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGRBG10_1X10,
> +        DATA_TYPE_RAW_10BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> +        10,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SRGGB10_1X10,
> +        DATA_TYPE_RAW_10BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> +        10,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_Y8_1X8,
> +        DATA_TYPE_RAW_8BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
> +        8,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_Y10_1X10,
> +        DATA_TYPE_RAW_10BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
> +        10,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SBGGR12_1X12,
> +        DATA_TYPE_RAW_12BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> +        12,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGBRG12_1X12,
> +        DATA_TYPE_RAW_12BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> +        12,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGRBG12_1X12,
> +        DATA_TYPE_RAW_12BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> +        12,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SRGGB12_1X12,
> +        DATA_TYPE_RAW_12BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
> +        12,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SBGGR14_1X14,
> +        DATA_TYPE_RAW_14BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> +        14,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGBRG14_1X14,
> +        DATA_TYPE_RAW_14BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> +        14,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SGRBG14_1X14,
> +        DATA_TYPE_RAW_14BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> +        14,
> +        1,
> +    },
> +    {
> +        MEDIA_BUS_FMT_SRGGB14_1X14,
> +        DATA_TYPE_RAW_14BIT,
> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
> +        14,
> +        1,
> +    },
> +};

Also please consider including/reviewing Gjorgji's patchset which
reworks the declaration of resources.

https://lore.kernel.org/lkml/[email protected]/T/

---
bod

2024-03-25 15:29:17

by Depeng Shao

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550

Hi Bryan,

On 3/20/2024 11:57 PM, Bryan O'Donoghue wrote:
> On 20/03/2024 14:11, Depeng Shao wrote:
>> From: Yongsheng Li <[email protected]>
>>
>> Add IFE driver for SM8550, the main difference with
>> old HW is register offset is different, register
>> update, reset and buf done is moved to CSID. And
>> the image address support 36 bits, so need to right
>> shift the image address when configuring the write
>> master.
>>
>> Signed-off-by: Yongsheng Li <[email protected]>
>> Co-developed-by: Depeng Shao <[email protected]>
>> Signed-off-by: Depeng Shao <[email protected]>
>
> Same comment with your co-developed and SOB
>
>> ---
>>   drivers/media/platform/qcom/camss/Makefile    |   1 +
>>   .../media/platform/qcom/camss/camss-vfe-780.c | 455 ++++++++++++++++++
>>   drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
>>   3 files changed, 457 insertions(+)
>>   create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c
>>
>> diff --git a/drivers/media/platform/qcom/camss/Makefile
>> b/drivers/media/platform/qcom/camss/Makefile
>> index c5fcd6eec0f2..ac40bbab18a3 100644
>> --- a/drivers/media/platform/qcom/camss/Makefile
>> +++ b/drivers/media/platform/qcom/camss/Makefile
>> @@ -18,6 +18,7 @@ qcom-camss-objs += \
>>           camss-vfe-4-8.o \
>>           camss-vfe-17x.o \
>>           camss-vfe-480.o \
>> +        camss-vfe-780.o \
>>           camss-vfe-gen1.o \
>>           camss-vfe.o \
>>           camss-video.o \
>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c
>> b/drivers/media/platform/qcom/camss/camss-vfe-780.c
>> new file mode 100644
>> index 000000000000..a78647f23c8c
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
>> @@ -0,0 +1,455 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * camss-vfe-780.c
>> + *
>> + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v780
>> (SM8550)
>> + *
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +
>> +#include "camss.h"
>> +#include "camss-vfe.h"
>> +
>> +#define VFE_HW_VERSION            (vfe_is_lite(vfe) ? 0x1000 : 0x0)
>> +
>> +#define VFE_IRQ_CMD            (vfe_is_lite(vfe) ? 0x1038 : 0x30)
>> +#define     IRQ_CMD_GLOBAL_CLEAR    BIT(0)
>> +
>> +#define VFE_IRQ_MASK(n)            ((vfe_is_lite(vfe) ? 0x1024 :
>> 0x34) + (n) * 4)
>> +#define        IRQ_MASK_0_BUS_TOP_IRQ    (vfe_is_lite(vfe) ? BIT(0) |
>> BIT(1) | BIT(2) : \
>> +                        BIT(0) | BIT(4) | BIT(18))
>> +#define        IRQ_MASK_1_BUS_TOP_IRQ(n)    (vfe_is_lite(vfe) ? BIT(2
>> * n + 2) | BIT(2 * n + 3) : \
>> +                        BIT(2 * n + 8) | BIT(2 * n + 9))
>> +#define VFE_IRQ_CLEAR(n)        ((vfe_is_lite(vfe) ? 0x102C : 0x3C) +
>> (n) * 4)
>> +#define VFE_IRQ_STATUS(n)        ((vfe_is_lite(vfe) ? 0x101C : 0x44)
>> + (n) * 4)
>> +
>> +#define BUS_REG_BASE            (vfe_is_lite(vfe) ? 0x1200 : 0xC00)
>> +
>> +#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 + 0xdc)
>> +
>> +#define VFE_BUS_WM_CFG(n)        (BUS_REG_BASE + 0x200 + (n) * 0x100)
>> +#define        WM_CFG_EN            (0)
>> +#define        WM_VIR_FRM_EN        (1)
>> +#define        WM_CFG_MODE            (16)
>> +#define            MODE_QCOM_PLAIN    (0)
>> +#define            MODE_MIPI_RAW    (1)
>> +#define VFE_BUS_WM_IMAGE_ADDR(n)    (BUS_REG_BASE + 0x204 + (n) * 0x100)
>> +#define VFE_BUS_WM_FRAME_INCR(n)    (BUS_REG_BASE + 0x208 + (n) * 0x100)
>> +#define VFE_BUS_WM_IMAGE_CFG_0(n)    (BUS_REG_BASE + 0x20c + (n) *
>> 0x100)
>> +#define                WM_IMAGE_CFG_0_DEFAULT_WIDTH    (0xFFFF)
>> +#define VFE_BUS_WM_IMAGE_CFG_1(n)    (BUS_REG_BASE + 0x210 + (n) *
>> 0x100)
>> +#define VFE_BUS_WM_IMAGE_CFG_2(n)    (BUS_REG_BASE + 0x214 + (n) *
>> 0x100)
>> +#define                WM_IMAGE_CFG_2_DEFAULT_STRIDE    (0xFFFF)
>> +#define VFE_BUS_WM_PACKER_CFG(n)    (BUS_REG_BASE + 0x218 + (n) * 0x100)
>> +#define VFE_BUS_WM_HEADER_ADDR(n)    (BUS_REG_BASE + 0x220 + (n) *
>> 0x100)
>> +#define VFE_BUS_WM_HEADER_INCR(n)    (BUS_REG_BASE + 0x224 + (n) *
>> 0x100)
>> +#define VFE_BUS_WM_HEADER_CFG(n)    (BUS_REG_BASE + 0x228 + (n) * 0x100)
>> +
>> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n)    (BUS_REG_BASE + 0x230 +
>> (n) * 0x100)
>> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n)    (BUS_REG_BASE + 0x234
>> + (n) * 0x100)
>> +#define VFE_BUS_WM_FRAMEDROP_PERIOD(n)        (BUS_REG_BASE + 0x238 +
>> (n) * 0x100)
>> +#define VFE_BUS_WM_FRAMEDROP_PATTERN(n)        (BUS_REG_BASE + 0x23c
>> + (n) * 0x100)
>> +
>> +#define VFE_BUS_WM_MMU_PREFETCH_CFG(n)    (BUS_REG_BASE + 0x260 + (n)
>> * 0x100)
>> +#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n)    (BUS_REG_BASE +
>> 0x264 + (n) * 0x100)
>> +#define VFE_BUS_WM_SYSTEM_CACHE_CFG(n)    (BUS_REG_BASE + 0x268 + (n)
>> * 0x100)
>> +
>> +
>> +/* for titan 780, each bus client is hardcoded to a specific path */
>> +#define RDI_WM(n)            ((vfe_is_lite(vfe) ? 0 : 23) + (n))
>
> No admixture of hex and decimal please.
>
>> +
>> +#define MAX_VFE_OUTPUT_LINES    4
>> +#define MAX_VFE_ACT_BUF    1
>> +
>> +static u32 vfe_hw_version(struct vfe_device *vfe)
>> +{
>> +    u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
>> +
>> +    u32 gen = (hw_version >> 28) & 0xF;
>> +    u32 rev = (hw_version >> 16) & 0xFFF;
>> +    u32 step = hw_version & 0xFFFF;
>> +
>> +    dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen,
>> rev, step);
>> +
>> +    return hw_version;
>> +}
>
> Same comment as with CSID, its time to rationalise all of this
> replicated code down to one place, instead of proliferating it further.
>

I understand, but the original driver and current driver are using macro
definition to define the register offset, then we aren't able to add the
common code to one place.

We need to refactor the code to not use macro definition for the
register offset first, then we can reuse the common code.

>> +static void vfe_global_reset(struct vfe_device *vfe)
>> +{
>> +}
>> +
>> +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); /* map to actual WM used (from wm=RDI index) */
>> +
>> +    /* no clock gating at bus input */
>> +    writel_relaxed(0, vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
>> +
>> +    writel_relaxed(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL);
>> +
>> +    writel_relaxed(ALIGN(pix->plane_fmt[0].bytesperline, 16) *
>> pix->height >> 8,
>> +               vfe->base + VFE_BUS_WM_FRAME_INCR(wm));
>> +    writel_relaxed((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF),
>> +               vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm));
>> +    writel_relaxed(WM_IMAGE_CFG_2_DEFAULT_STRIDE,
>> +               vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm));
>> +    writel_relaxed(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm));
>> +
>> +    /* no dropped frames, one irq per frame */
>> +    writel_relaxed(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm));
>> +    writel_relaxed(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm));
>> +    writel_relaxed(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm));
>> +    writel_relaxed(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm));
>> +
>> +    writel_relaxed(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm));
>> +    writel_relaxed(0xFFFFFFFF, vfe->base +
>> VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm));
>> +
>> +    writel_relaxed(1 << WM_CFG_EN | MODE_MIPI_RAW << 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); /* map to actual WM used (from wm=RDI index) */
>> +    writel_relaxed(0, vfe->base + VFE_BUS_WM_CFG(wm));
>> +}
>
> vfe_wm_stop() as an example can/should live in a shared file.
>
> As proof of concept code its fine to copy/paste between files but, to
> merge we need to get rid of any replicated code - introducing
> indirection/offsets as necessary.
>

Same with above, we'd better to refactor the code to not use macro
definition for the register offset first.

>> +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr,
>> +              struct vfe_line *line)
>> +{
>> +    wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */
>> +    writel_relaxed((addr >> 8) & 0xFFFFFFFF, vfe->base +
>> VFE_BUS_WM_IMAGE_ADDR(wm));
>> +
>> +    dev_dbg(vfe->camss->dev, "wm:%d, image buf addr:0x%llx\n",
>> +        wm, addr);
>> +}
>> +
>> +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id
>> line_id)
>> +{
>> +    int port_id = line_id;
>> +
>> +    v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP, (void
>> *)&port_id);
>> +}
>> +
>> +static inline void vfe_reg_update_clear(struct vfe_device *vfe,
>> +                    enum vfe_line_id line_id)
>> +{
>> +    int port_id = line_id;
>> +
>> +    v4l2_subdev_notify(&vfe->line[line_id].subdev, NOTIFY_RUP_CLEAR,
>> (void *)&port_id);
>> +}
>
> I'm not sure I quite understand why we need to use this API to clear
> registers inside of the address space of the same driver.
>
> Is there not a much more direct way to write our _internal_ registers ?
>

Since the register update(RUP) is moved to CSID, so it isn't _internal_
registers in this hardware.

>
>> +static void vfe_enable_irq_common(struct vfe_device *vfe, enum
>> vfe_line_id line_id)
>> +{
>> +    int port_id = line_id;
>> +
>> +    /* enable top BUS status IRQ */
>> +    writel_relaxed(IRQ_MASK_0_BUS_TOP_IRQ,
>> +                vfe->base + VFE_IRQ_MASK(0));
>> +
>> +    writel_relaxed(IRQ_MASK_1_BUS_TOP_IRQ(port_id),
>> +                vfe->base + VFE_IRQ_MASK(1));
>> +}
>> +
>> +/*
>> + * vfe_isr - VFE module interrupt handler
>> + * @irq: Interrupt line
>> + * @dev: VFE device
>> + *
>> + * Return IRQ_HANDLED on success
>> + */
>> +static irqreturn_t vfe_isr(int irq, void *dev)
>> +{
>> +    struct vfe_device *vfe = dev;
>> +    u32 status;
>> +
>> +    status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(0));
>> +    writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(0));
>> +    writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
>> +
>> +    if (status)
>> +        dev_dbg(vfe->camss->dev, "Top Status_0:0x%x\n", status);
>> +
>> +    status = readl_relaxed(vfe->base + VFE_IRQ_STATUS(1));
>> +    writel_relaxed(status, vfe->base + VFE_IRQ_CLEAR(1));
>> +    writel_relaxed(IRQ_CMD_GLOBAL_CLEAR, vfe->base + VFE_IRQ_CMD);
>> +
>> +    if (status)
>> +        dev_dbg(vfe->camss->dev, "Top Status_1:0x%x\n", status);
>> +
>> +    return IRQ_HANDLED;
>> +}
>
> Why is this ISR required ? What does it do ?
>
> Does it actually run on your reference hardware ?
>
> If the purpose of the ISR is just to clear the status registers, why
> even enable it ?
>

Yeah, we can remove it, it was added for some debug info and verify the
hardware, but it doesn't affect the control in this hardware, so we can
remove it.

>> +
>> +/*
>> + * vfe_halt - Trigger halt on VFE module and wait to complete
>> + * @vfe: VFE device
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int vfe_halt(struct vfe_device *vfe)
>> +{
>> +    /* rely on vfe_disable_output() to stop the VFE */
>> +    return 0;
>> +}
>> +
>> +static int vfe_get_output(struct vfe_line *line)
>> +{
>> +    struct vfe_device *vfe = to_vfe(line);
>> +    struct vfe_output *output;
>> +    unsigned long flags;
>> +
>> +    spin_lock_irqsave(&vfe->output_lock, flags);
>> +
>> +    output = &line->output;
>> +    if (output->state > VFE_OUTPUT_RESERVED) {
>> +        dev_err(vfe->camss->dev, "Output is running\n");
>> +        goto error;
>> +    }
>> +
>> +    output->wm_num = 1;
>> +
>> +    /* Correspondence between VFE line number and WM number.
>> +     * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 ->
>> PIX/RDI3
>> +     * Note this 1:1 mapping will not work for PIX streams.
>> +     */
>> +    output->wm_idx[0] = line->id;
>> +    vfe->wm_output_map[line->id] = line->id;
>> +
>> +    output->drop_update_idx = 0;
>> +
>> +    spin_unlock_irqrestore(&vfe->output_lock, flags);
>> +
>> +    return 0;
>> +
>> +error:
>> +    spin_unlock_irqrestore(&vfe->output_lock, flags);
>> +    output->state = VFE_OUTPUT_OFF;
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +static int vfe_enable_output(struct vfe_line *line)
>> +{
>> +    struct vfe_device *vfe = to_vfe(line);
>> +    struct vfe_output *output = &line->output;
>> +    unsigned long flags;
>> +    unsigned int i;
>> +
>> +    spin_lock_irqsave(&vfe->output_lock, flags);
>> +
>> +    vfe_reg_update_clear(vfe, line->id);
>> +
>> +    if (output->state > VFE_OUTPUT_RESERVED) {
>> +        dev_err(vfe->camss->dev, "Output is not in reserved state %d\n",
>> +            output->state);
>> +        spin_unlock_irqrestore(&vfe->output_lock, flags);
>> +        return -EINVAL;
>> +    }
>> +
>> +    WARN_ON(output->gen2.active_num);
>> +
>> +    output->state = VFE_OUTPUT_ON;
>> +
>> +    output->sequence = 0;
>> +
>> +    vfe_wm_start(vfe, output->wm_idx[0], line);
>> +
>> +    for (i = 0; i < MAX_VFE_ACT_BUF; i++) {
>> +        output->buf[i] = vfe_buf_get_pending(output);
>> +        if (!output->buf[i])
>> +            break;
>> +        output->gen2.active_num++;
>> +        vfe_wm_update(vfe, output->wm_idx[0],
>> output->buf[i]->addr[0], line);
>> +
>> +        vfe_reg_update(vfe, line->id);
>> +    }
>> +
>> +    spin_unlock_irqrestore(&vfe->output_lock, flags);
>> +
>> +    return 0;
>> +}
>
> So you'll see with the WIP code for x1e80100 which is I think the same
> VFE - no one generation less 680 - that this code is copy/pasted.
>
> But there's again no good reason for that. Common code needs to be
> squashed down into one place.
>

Agree, this part can be moved to camss-vfe.c, since they don't have
register configuration, just some common code.

> ---
> bod

Thanks,
Depeng

2024-03-25 16:07:09

by Depeng Shao

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] media: qcom: camss: Add CSID gen3 driver

Hi Bryan,

On 3/20/2024 11:40 PM, Bryan O'Donoghue wrote:
> On 20/03/2024 14:11, Depeng Shao wrote:
>> The CSID in SM8550 is gen3, it has new register
>> offset and new functionality, the buf done irq,
>> register update and reset is moved to CSID gen3.
>>
>> Signed-off-by: Depeng Shao <[email protected]>
>> Co-developed-by: Yongsheng Li <[email protected]>
>> Signed-off-by: Yongsheng Li <[email protected]>
>> ---
>>   drivers/media/platform/qcom/camss/Makefile    |   1 +
>>   .../platform/qcom/camss/camss-csid-gen3.c     | 639 ++++++++++++++++++
>>   .../platform/qcom/camss/camss-csid-gen3.h     |  26 +
>>   .../media/platform/qcom/camss/camss-csid.h    |   1 +
>>   4 files changed, 667 insertions(+)
>>   create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.c
>>   create mode 100644 drivers/media/platform/qcom/camss/camss-csid-gen3.h
>>
>> diff --git a/drivers/media/platform/qcom/camss/Makefile
>> b/drivers/media/platform/qcom/camss/Makefile
>> index 28eba4bf3e38..c5fcd6eec0f2 100644
>> --- a/drivers/media/platform/qcom/camss/Makefile
>> +++ b/drivers/media/platform/qcom/camss/Makefile
>> @@ -7,6 +7,7 @@ qcom-camss-objs += \
>>           camss-csid-4-1.o \
>>           camss-csid-4-7.o \
>>           camss-csid-gen2.o \
>> +        camss-csid-gen3.o \
>>           camss-csiphy-2ph-1-0.o \
>>           camss-csiphy-3ph-1-0.o \
>>           camss-csiphy-2-1-2.o \
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> new file mode 100644
>> index 000000000000..b97005f7065d
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> @@ -0,0 +1,639 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * camss-csid-gen3.c
>> + *
>> + * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
>> + *
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +#include <linux/completion.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +#include <linux/delay.h>
>
> Please alphanumerically sort your include list.
>
>> +
>> +
>> +#include "camss-csid.h"
>> +#include "camss-csid-gen3.h"
>> +#include "camss.h"
>
> and here.
>
>> +
>> +#define CSID_TOP_IO_PATH_CFG0(csid)    (0x4 * (csid))
>> +#define        OUTPUT_IFE_EN 0x100
>> +#define        INTERNAL_CSID 1
>> +
>> +#define CSID_HW_VERSION        0x0
>> +#define        HW_VERSION_STEPPING    0
>> +#define        HW_VERSION_REVISION    16
>> +#define        HW_VERSION_GENERATION    28
>> +
>> +#define CSID_RST_CFG    0xC
>> +#define        RST_MODE        0
>> +#define        RST_LOCATION    4
>> +
>> +#define CSID_RST_CMD    0x10
>> +#define        SELECT_HW_RST    0
>> +#define        SELECT_SW_RST    1
>> +#define        SELECT_IRQ_RST    2
>> +
>> +#define CSID_CSI2_RX_IRQ_STATUS    0x9C
>> +#define CSID_CSI2_RX_IRQ_MASK    0xA0
>> +#define CSID_CSI2_RX_IRQ_CLEAR    0xA4
>> +#define CSID_CSI2_RX_IRQ_SET    0xA8
>> +
>> +#define CSID_CSI2_RDIN_IRQ_STATUS(rdi)        (0xEC + 0x10 * (rdi))
>> +#define CSID_CSI2_RDIN_IRQ_MASK(rdi)        (0xF0 + 0x10 * (rdi))
>> +#define        CSID_CSI2_RDIN_INFO_FIFO_FULL 2
>> +#define        CSID_CSI2_RDIN_INFO_CAMIF_EOF 3
>> +#define        CSID_CSI2_RDIN_INFO_CAMIF_SOF 4
>> +#define        CSID_CSI2_RDIN_INFO_INPUT_EOF 9
>> +#define        CSID_CSI2_RDIN_INFO_INPUT_SOF 12
>> +#define        CSID_CSI2_RDIN_ERROR_REC_FRAME_DROP 18
>> +#define        CSID_CSI2_RDIN_ERROR_REC_OVERFLOW_IRQ 19
>> +#define        CSID_CSI2_RDIN_ERROR_REC_CCIF_VIOLATION 20
>> +#define        CSID_CSI2_RDIN_EPOCH0 21
>> +#define        CSID_CSI2_RDIN_EPOCH1 22
>> +#define        CSID_CSI2_RDIN_RUP_DONE 23
>> +#define        CSID_CSI2_RDIN_CCIF_VIOLATION 29
>> +
>> +#define CSID_CSI2_RDIN_IRQ_CLEAR(rdi)        (0xF4 + 0x10 * (rdi))
>> +#define CSID_CSI2_RDIN_IRQ_SET(rdi)            (0xF8 + 0x10 * (rdi))
>> +
>> +#define CSID_TOP_IRQ_STATUS    0x7C
>> +#define        TOP_IRQ_STATUS_RESET_DONE 0
>> +#define CSID_TOP_IRQ_MASK    0x80
>> +#define CSID_TOP_IRQ_CLEAR    0x84
>> +#define CSID_TOP_IRQ_SET    0x88
>> +#define CSID_IRQ_CMD        0x14
>> +#define        IRQ_CMD_CLEAR    0
>> +#define        IRQ_CMD_SET    4
>> +
>> +#define CSID_BUF_DONE_IRQ_STATUS    0x8C
>> +#define        BUF_DONE_IRQ_STATUS_RDI_OFFSET (csid_is_lite(csid) ? 1
>> : 14)
>
> Stick with hex please 0x01 : 0x0e
>
>> +#define CSID_BUF_DONE_IRQ_MASK    0x90
>> +#define CSID_BUF_DONE_IRQ_CLEAR    0x94
>> +#define CSID_BUF_DONE_IRQ_SET    0x98
>> +
>> +#define CSID_CSI2_RX_CFG0    0x200
>> +#define        CSI2_RX_CFG0_NUM_ACTIVE_LANES    0
>> +#define        CSI2_RX_CFG0_DL0_INPUT_SEL    4
>> +#define        CSI2_RX_CFG0_DL1_INPUT_SEL    8
>> +#define        CSI2_RX_CFG0_DL2_INPUT_SEL    12
>> +#define        CSI2_RX_CFG0_DL3_INPUT_SEL    16
>> +#define        CSI2_RX_CFG0_PHY_NUM_SEL    20
>> +#define            CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
>> +#define        CSI2_RX_CFG0_PHY_TYPE_SEL    24
>> +#define        CSI2_RX_CFG0_TPG_MUX_EN        27
>> +#define        CSI2_RX_CFG0_TPG_NUM_SEL    28
>> +
>> +#define CSID_CSI2_RX_CFG1    0x204
>> +#define        CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN        0
>> +#define        CSI2_RX_CFG1_DE_SCRAMBLE_EN            1
>> +#define        CSI2_RX_CFG1_VC_MODE                2
>> +#define        CSI2_RX_CFG1_COMPLETE_STREAM_EN            4
>> +#define        CSI2_RX_CFG1_COMPLETE_STREAM_FRAME_TIMING    5
>> +#define        CSI2_RX_CFG1_MISR_EN                6
>> +#define        CSI2_RX_CFG1_PHY_BIST_EN            7
>> +#define        CSI2_RX_CFG1_EPD_MODE                8
>> +#define        CSI2_RX_CFG1_EOTP_EN                9
>> +#define        CSI2_RX_CFG1_DYN_SWITCH_EN            10
>> +#define        CSI2_RX_CFG1_RUP_AUP_LATCH_DIS        11
>> +
>> +#define CSID_CSI2_RX_CAPTURE_CTRL    0x208
>> +#define        CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_EN    0
>> +#define        CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_EN    1
>> +#define        CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_EN    3
>> +#define        CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_DT        4
>> +#define        CSI2_RX_CAPTURE_CTRL_LONG_PKT_CAPTURE_VC        10
>> +#define        CSI2_RX_CAPTURE_CTRL_SHORT_PKT_CAPTURE_VC    15
>> +#define        CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_DT        20
>> +#define        CSI2_RX_CAPTURE_CTRL_CPHY_PKT_CAPTURE_VC        26
>> +
>> +#define CSID_RDI_CFG0(rdi)            (0x500 + 0x100 * (rdi))
>> +#define        RDI_CFG0_VFR_EN                0
>> +#define        RDI_CFG0_FRAME_ID_DEC_EN    1
>> +#define        RDI_CFG0_RETIME_DIS            5
>> +#define        RDI_CFG0_TIMESTAMP_EN        6
>> +#define        RDI_CFG0_TIMESTAMP_STB_SEL    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                    31
>> +
>> +#define CSID_RDI_CFG1(rdi)            (0x510 + 0x100 * (rdi))
>> +#define        RDI_CFG1_BYTE_CNTR_EN    2
>> +#define        RDI_CFG1_DROP_H_EN    5
>> +#define        RDI_CFG1_DROP_V_EN    6
>> +#define        RDI_CFG1_CROP_H_EN    7
>> +#define        RDI_CFG1_CROP_V_EN    8
>> +#define        RDI_CFG1_MISR_EN    9
>> +#define        RDI_CFG1_PIX_STORE  10
>> +#define        RDI_CFG1_PLAIN_ALIGNMENT    11
>> +#define        RDI_CFG1_PLAIN_FORMAT    12
>> +#define        RDI_CFG1_EARLY_EOF_EN    14
>> +#define        RDI_CFG1_PACKING_FORMAT    15
>> +
>> +#define CSID_RDI_CTRL(rdi)            (0x504 + 0x100 * (rdi))
>> +#define        RDI_CTRL_START_CMD        0
>> +#define        RDI_CTRL_START_MODE        2
>> +
>> +#define CSID_RDI_EPOCH_IRQ_CFG(rdi) (0x52C + 0x100 * (rdi))
>> +
>> +#define CSID_RDI_FRM_DROP_PATTERN(rdi)            (0x540 + 0x100 *
>> (rdi))
>> +#define CSID_RDI_FRM_DROP_PERIOD(rdi)            (0x544 + 0x100 * (rdi))
>> +#define CSID_RDI_IRQ_SUBSAMPLE_PATTERN(rdi)        (0x548 + 0x100 *
>> (rdi))
>> +#define CSID_RDI_IRQ_SUBSAMPLE_PERIOD(rdi)        (0x54C + 0x100 *
>> (rdi))
>> +#define CSID_RDI_RPP_PIX_DROP_PATTERN(rdi)        (0x558 + 0x100 *
>> (rdi))
>> +#define CSID_RDI_RPP_PIX_DROP_PERIOD(rdi)        (0x55C + 0x100 * (rdi))
>> +#define CSID_RDI_RPP_LINE_DROP_PATTERN(rdi)        (0x560 + 0x100 *
>> (rdi))
>> +#define CSID_RDI_RPP_LINE_DROP_PERIOD(rdi)        (0x564 + 0x100 *
>> (rdi))
>> +
>> +#define CSID_RDI_RPP_HCROP(rdi)                 (0x550 + 0x100 * (rdi))
>> +#define CSID_RDI_RPP_VCROP(rdi)                 (0x554 + 0x100 * (rdi))
>> +
>> +#define CSID_RDI_ERROR_RECOVERY_CFG0(rdi)       (0x514 + 0x100 * (rdi))
>> +
>> +#define CSID_DISCARD_FRAMES 4
>> +
>> +#define CSID_REG_UPDATE_CMD        0x18
>> +static inline int reg_update_rdi(struct csid_device *csid, int n)
>> +{
>> +    return BIT(n + 4) + BIT(20 + n);
>> +}
>> +
>> +#define        REG_UPDATE_RDI        reg_update_rdi
>> +
>> +static const struct csid_format csid_formats[] = {
>> +    {
>> +        MEDIA_BUS_FMT_UYVY8_1X16,
>> +        DATA_TYPE_YUV422_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        2,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_VYUY8_1X16,
>> +        DATA_TYPE_YUV422_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        2,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_YUYV8_1X16,
>> +        DATA_TYPE_YUV422_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        2,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_YVYU8_1X16,
>> +        DATA_TYPE_YUV422_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        2,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SBGGR8_1X8,
>> +        DATA_TYPE_RAW_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGBRG8_1X8,
>> +        DATA_TYPE_RAW_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGRBG8_1X8,
>> +        DATA_TYPE_RAW_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SRGGB8_1X8,
>> +        DATA_TYPE_RAW_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SBGGR10_1X10,
>> +        DATA_TYPE_RAW_10BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
>> +        10,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGBRG10_1X10,
>> +        DATA_TYPE_RAW_10BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
>> +        10,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGRBG10_1X10,
>> +        DATA_TYPE_RAW_10BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
>> +        10,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SRGGB10_1X10,
>> +        DATA_TYPE_RAW_10BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
>> +        10,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_Y8_1X8,
>> +        DATA_TYPE_RAW_8BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_8_BIT,
>> +        8,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_Y10_1X10,
>> +        DATA_TYPE_RAW_10BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_10_BIT,
>> +        10,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SBGGR12_1X12,
>> +        DATA_TYPE_RAW_12BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
>> +        12,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGBRG12_1X12,
>> +        DATA_TYPE_RAW_12BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
>> +        12,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGRBG12_1X12,
>> +        DATA_TYPE_RAW_12BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
>> +        12,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SRGGB12_1X12,
>> +        DATA_TYPE_RAW_12BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_12_BIT,
>> +        12,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SBGGR14_1X14,
>> +        DATA_TYPE_RAW_14BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
>> +        14,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGBRG14_1X14,
>> +        DATA_TYPE_RAW_14BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
>> +        14,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SGRBG14_1X14,
>> +        DATA_TYPE_RAW_14BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
>> +        14,
>> +        1,
>> +    },
>> +    {
>> +        MEDIA_BUS_FMT_SRGGB14_1X14,
>> +        DATA_TYPE_RAW_14BIT,
>> +        DECODE_FORMAT_UNCOMPRESSED_14_BIT,
>> +        14,
>> +        1,
>> +    },
>> +};
>> +
>> +static void __csid_configure_rx(struct csid_device *csid,
>> +                struct csid_phy_config *phy, int vc)
>> +{
>> +    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_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
>> +
>> +    val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
>> +    if (vc > 3)
>> +        val |= 1 << CSI2_RX_CFG1_VC_MODE;
>> +    writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
>> +}
>> +
>> +static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8
>> rdi)
>> +{
>> +    int val;
>> +
>> +    if (enable)
>> +        val = 1 << RDI_CTRL_START_CMD;
>> +    else
>> +        val = 0 << RDI_CTRL_START_CMD;
>> +    writel_relaxed(val, csid->base + CSID_RDI_CTRL(rdi));
>> +}
>> +
>> +static void __csid_configure_top(struct csid_device *csid)
>> +{
>> +    u32 val;
>> +
>> +    if (csid->top_base) {
>> +        val = OUTPUT_IFE_EN | INTERNAL_CSID;
>> +        writel_relaxed(val, csid->top_base +
>> CSID_TOP_IO_PATH_CFG0(csid->id));
>> +    }
>> +}
>> +
>> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8
>> enable, u8 vc)
>> +{
>> +    u32 val;
>> +    u8 lane_cnt = csid->phy.lane_cnt;
>> +    /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0,
>> Pad 2 -> RDI1, etc. */
>> +    struct v4l2_mbus_framefmt *input_format =
>> &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
>> +    const struct csid_format *format =
>> csid_get_fmt_entry(csid->formats, csid->nformats,
>> +                                  input_format->code);
>> +
>> +    if (!lane_cnt)
>> +        lane_cnt = 4;
>> +
>> +    val = 0;
>> +    writel_relaxed(val, csid->base + CSID_RDI_FRM_DROP_PERIOD(vc));
>> +
>> +    /*
>> +     * 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;
>> +
>> +    val = 1 << RDI_CFG0_TIMESTAMP_EN;
>> +    val |= 2 << RDI_CFG0_TIMESTAMP_STB_SEL;
>> +    /* 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_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
>> +
>> +    val = 1 << RDI_CFG1_PACKING_FORMAT;
>> +    val |= 1 << RDI_CFG1_PIX_STORE;
>> +    val |= 1 << RDI_CFG1_DROP_H_EN;
>> +    val |= 1 << RDI_CFG1_DROP_V_EN;
>> +    val |= 1 << RDI_CFG1_CROP_H_EN;
>> +    val |= 1 << RDI_CFG1_CROP_V_EN;
>> +    val |= RDI_CFG1_EARLY_EOF_EN;
>> +
>> +    writel_relaxed(val, csid->base + CSID_RDI_CFG1(vc));
>> +
>> +    val = 0;
>> +    writel_relaxed(val, csid->base + CSID_RDI_IRQ_SUBSAMPLE_PERIOD(vc));
>> +
>> +    val = 1;
>> +    writel_relaxed(val, csid->base +
>> CSID_RDI_IRQ_SUBSAMPLE_PATTERN(vc));
>> +
>> +    val = 0;
>> +    writel_relaxed(val, csid->base + CSID_RDI_CTRL(vc));
>> +
>> +    val = readl_relaxed(csid->base + CSID_RDI_CFG0(vc));
>> +    val |=  enable << RDI_CFG0_EN;
>> +    writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
>> +}
>
> Hmm.
>
> So I think 90% of the code here can be moved into a shared file -
> ideally instantiated in gen2.c and then reused here rather than
> copy/paste from one file to the other.
>
> Lets sync up and try to get a unified tree for x1e80100 and sm8550 for
> that purpose.
>
> I think the code you have here is slightly further along that the
> CSID/VFE stuff I've been working with.
>
> But still - reductio ad absurdum - we need to functionally decompose and
> not replicate code.
>

Yes, agree, most of the code are same, but the difficult part is most of
the code is configuring the register which are defined as macro
definition, and they are different between different platform.

>> +
>> +static void csid_configure_stream(struct csid_device *csid, u8 enable)
>> +{
>> +    u8 i;
>> +
>> +    /* Loop through all enabled VCs and configure stream for each */
>> +    for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>> +        if (csid->phy.en_vc & BIT(i)) {
>> +            __csid_configure_top(csid);
>> +            __csid_configure_rdi_stream(csid, enable, i);
>> +            __csid_configure_rx(csid, &csid->phy, i);
>> +            __csid_ctrl_rdi(csid, enable, i);
>> +        }
>> +}
>
> Another example, straight copy/paste - we need to zap all of that.
>
> ---
> bod

Thanks,
Depeng

2024-03-25 16:46:05

by Depeng Shao

[permalink] [raw]
Subject: Re: [PATCH v2 3/8] media: qcom: camss: Add new csiphy driver 2-1-2

Hi Bryan,


On 3/20/2024 11:21 PM, Bryan O'Donoghue wrote:
> On 20/03/2024 14:11, Depeng Shao wrote:
>> From: Yongsheng Li <[email protected]>
>>
>> The CSIPHY register offset of SM8550 is different with
>> SM8250, so add a new driver for the SM8550 platform.
>>
>> Signed-off-by: Yongsheng Li <[email protected]>
>> Co-developed-by: Depeng Shao <[email protected]>
>
> You're listing yourself ? Twice with a different CD and SOB.
> Shouldn't this be
> Co-developed-by: Depeng Shao <[email protected]>
>
> ?
Sorry for mistake, will update it in new patch.
>
>> Signed-off-by: Depeng Shao <[email protected]>
>> ---
>>   drivers/media/platform/qcom/camss/Makefile    |   1 +
>>   .../platform/qcom/camss/camss-csiphy-2-1-2.c  | 343 ++++++++++++++++++
>>   .../media/platform/qcom/camss/camss-csiphy.h  |   1 +
>>   3 files changed, 345 insertions(+)
>>   create mode 100644
>> drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
>>
>> diff --git a/drivers/media/platform/qcom/camss/Makefile
>> b/drivers/media/platform/qcom/camss/Makefile
>> index 0d4389ab312d..28eba4bf3e38 100644
>> --- a/drivers/media/platform/qcom/camss/Makefile
>> +++ b/drivers/media/platform/qcom/camss/Makefile
>> @@ -9,6 +9,7 @@ qcom-camss-objs += \
>>           camss-csid-gen2.o \
>>           camss-csiphy-2ph-1-0.o \
>>           camss-csiphy-3ph-1-0.o \
>> +        camss-csiphy-2-1-2.o \
>
> File name here is incorrect camss-csiphy-3ph-1-2.o this is a 3 phase
> capable PHY right ? So both the filename and the commit title need to
> reflect that.
>
> I thought we had discussed offline using an offset instead of a new
> file ?
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/6dc8f49ef7c6ca69783aa02bdca81c77e66ecc0d
>
>
> Then just set the offset
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/022a837257d9e1fa8070f0dfa2f683e903111aa0
>
>
Yes, this is a good idea, but looks like the code in your x1e80100
branch hasn't ready yet, could we have a separate patch for this
optimization, then I can rebase my change based on it.


>
>>           camss-csiphy.o \
>>           camss-ispif.o \
>>           camss-vfe-4-1.o \
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
>> b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
>> new file mode 100644
>> index 000000000000..df7860d7a4f4
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-2-1-2.c
>> @@ -0,0 +1,343 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * camss-csiphy-2-1-2.c
>> + *
>> + * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v2.0
>> + *
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +
>> +#include "camss.h"
>> +#include "camss-csiphy.h"
>> +
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +
>> +
>> +#define CSIPHY_CMN_CSI_COMMON_CTRLn(n)    (0x1000 + 0x4 * (n))
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL5_CLK_ENABLE    BIT(7)
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B    BIT(0)
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL7_CONTROL    0x7A
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET    BIT(0)
>> +#define CSIPHY_CMN_CSI_COMMON_CTRL0_SWI_ENABLE_LANESYNC BIT(1)
>> +#define CSIPHY_CMN_CSI_COMMON_STATUSn(n)    (0x10B0 + 0x4 * (n))
>> +
>> +#define CSIPHY_DEFAULT_PARAMS            0
>> +#define CSIPHY_LANE_ENABLE               1
>> +#define CSIPHY_SETTLE_CNT_LOWER_BYTE     2
>> +#define CSIPHY_SETTLE_CNT_HIGHER_BYTE    3
>> +#define CSIPHY_DNP_PARAMS                4
>> +#define CSIPHY_2PH_REGS                  5
>> +#define CSIPHY_3PH_REGS                  6
>> +
>> +struct csiphy_reg_t {
>> +    s32 reg_addr;
>> +    s32 reg_data;
>> +    s32 delay;
>> +    u32 csiphy_param_type;
>> +};
>> +
>> +/* 2.1.2 2PH */
>> +static const struct
>> +csiphy_reg_t lane_regs_sm8550[5][20] = {
>
> So per the tree I shared with you..
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/fa666c241eb530a08aa6b391e15b018296e93f66
>
>
> We don't need to break this up into a multi-dimensional array
>
> its just a straight write of values


Yeah, agree, I also don't want to do like this, I just follow the old
driver's format


>> +    {
>> +        {0x0E90, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0E94, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
>> +        {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0090, 0x0f, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0094, 0x07, 0x01, CSIPHY_DEFAULT_PARAMS},
>> +        {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, 0x01, CSIPHY_DEFAULT_PARAMS},
>> +        {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, 0x01, CSIPHY_DEFAULT_PARAMS},
>> +        {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, 0x01, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> +        {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0000, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> +        {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0400, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> +        {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +    },
>> +    {
>> +        {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0800, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> +        {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C00, 0x8E, 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, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
>> +        {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0094, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x005C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0060, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0064, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0494, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x045C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0460, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0464, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0894, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x085C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0860, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0864, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C94, 0xD7, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C5C, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C60, 0xBD, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +        {0x0C64, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
>> +    }
>> +};
>> +
>> +static void csiphy_hw_version_read(struct csiphy_device *csiphy,
>> +                   struct device *dev)
>> +{
>> +    u32 hw_version;
>> +
>> +    writel(CSIPHY_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID,
>> +           csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(6));
>> +
>> +    hw_version = readl_relaxed(csiphy->base +
>> +                   CSIPHY_CMN_CSI_COMMON_STATUSn(12));
>> +    hw_version |= readl_relaxed(csiphy->base +
>> +                   CSIPHY_CMN_CSI_COMMON_STATUSn(13)) << 8;
>> +    hw_version |= readl_relaxed(csiphy->base +
>> +                   CSIPHY_CMN_CSI_COMMON_STATUSn(14)) << 16;
>> +    hw_version |= readl_relaxed(csiphy->base +
>> +                   CSIPHY_CMN_CSI_COMMON_STATUSn(15)) << 24;
>> +
>> +    dev_info(dev, "CSIPHY 3PH HW Version = 0x%08x\n", hw_version);
>> +}
>> +
>> +/*
>> + * csiphy_reset - Perform software reset on CSIPHY module
>> + * @csiphy: CSIPHY device
>> + */
>> +static void csiphy_reset(struct csiphy_device *csiphy)
>> +{
>> +    writel_relaxed(0x1, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
>> +    usleep_range(5000, 8000);
>> +    writel_relaxed(0x0, csiphy->base + CSIPHY_CMN_CSI_COMMON_CTRLn(0));
>> +}
>> +
>> +static irqreturn_t csiphy_isr(int irq, void *dev)
>> +{
>> +    struct csiphy_device *csiphy = dev;
>> +    int i;
>> +
>> +    for (i = 0; i < 11; i++) {
>> +        int c = i + 22;
>> +        u8 val = readl_relaxed(csiphy->base +
>> +                       CSIPHY_CMN_CSI_COMMON_STATUSn(i));
>> +
>> +        writel_relaxed(val, csiphy->base +
>> +                    CSIPHY_CMN_CSI_COMMON_CTRLn(c));
>> +    }
>> +
>> +    writel_relaxed(0x1, csiphy->base +
>> CSIPHY_CMN_CSI_COMMON_CTRLn(10));
>> +    writel_relaxed(0x0, csiphy->base +
>> CSIPHY_CMN_CSI_COMMON_CTRLn(10));
>> +
>> +    for (i = 22; i < 33; i++)
>> +        writel_relaxed(0x0, csiphy->base +
>> +                    CSIPHY_CMN_CSI_COMMON_CTRLn(i));
>> +
>> +    return IRQ_HANDLED;
>> +}
>> +
>> +/*
>> + * csiphy_settle_cnt_calc - Calculate settle count value
>> + *
>> + * Helper function to calculate settle count value. This is
>> + * based on the CSI2 T_hs_settle parameter which in turn
>> + * is calculated based on the CSI2 transmitter link frequency.
>> + *
>> + * Return settle count value or 0 if the CSI2 link frequency
>> + * is not available
>> + */
>> +static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
>> +{
>> +    u32 ui; /* ps */
>> +    u32 timer_period; /* ps */
>> +    u32 t_hs_prepare_max; /* ps */
>> +    u32 t_hs_settle; /* ps */
>> +    u8 settle_cnt;
>> +
>> +    if (link_freq <= 0)
>> +        return 0;
>> +
>> +    ui = div_u64(1000000000000LL, link_freq);
>> +    ui /= 2;
>> +    t_hs_prepare_max = 85000 + 6 * ui;
>> +    t_hs_settle = t_hs_prepare_max;
>> +
>> +    timer_period = div_u64(1000000000000LL, timer_clk_rate);
>> +    settle_cnt = t_hs_settle / timer_period - 6;
>> +
>> +    return settle_cnt;
>> +}
>
> Yep this is all literal copy/paste of existing code.
>
> Please try taking
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/aa27ac8e1ffedd48c5ef6ac0f75f1f15716fe296
>
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/fa666c241eb530a08aa6b391e15b018296e93f66
>
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/e9e99dee4e723755274e6430bb43adfbf77d2a1a
>
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/5dc53c39b96eb22bdfe47554f047e6b63ddb25c0
>
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/36655b353fa71269812cff0e695341ff12042546
>
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/6dc8f49ef7c6ca69783aa02bdca81c77e66ecc0d
>
>
> and
>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/042ec64867d43e014c2b369db3b92b4f5432497f
>
>
> into your tree
>
> it works for me on sm8250 and is WIP for x1e80100
>
> Point being we need to make an effort to unify/reuse/fix init
> sequences upstream not to copy/paste from one file to another.
>
I asked can we have a separate change for the csiphy optimization in
above, looks like this the separate series.

Sorry, I just want to make sure the merge order since there are many
ongoing changes, they will conflict with each other.

>> +
>> +static void csiphy_config_lanes(struct csiphy_device *csiphy,
>> +                     u8 settle_cnt)
>> +{
>> +    const struct csiphy_reg_t *r;
>> +    int i, l, array_size;
>> +    u32 val;
>> +
>> +    switch (csiphy->camss->res->version) {
>> +    case CAMSS_8550:
>> +        r = &lane_regs_sm8550[0][0];
>> +        array_size = ARRAY_SIZE(lane_regs_sm8550[0]);
>> +        break;
>> +    default:
>> +        WARN(1, "unknown csi version\n");
>> +        return;
>> +    }
>> +
>> +    for (l = 0; l < 5; l++) {
>
> These hard-coded values in control loops must be done away with upstream.
>
Won't have these code if I rebase the change based on yours.
> ---
> bod


Thanks,

Depeng


2024-04-09 13:43:38

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550

On 20/03/2024 14:11, Depeng Shao wrote:
> +#define VFE_IRQ_STATUS(n) ((vfe_is_lite(vfe) ? 0x101C : 0x44) + (n) * 4)

These defines do the right thing for the RDI however

1. Please use 'rdi' not 'n' as the parameter here and
2. Pass 'vfe' explicitly not implicitly

Right now you need a variable named 'vfe' in the scope of the macro but
that ought to be an explicit parameter passed to the macro.

---
bod

2024-04-09 16:39:01

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] media: qcom: camss: Add new VFE driver for SM8550

On 09/04/2024 14:39, Bryan O'Donoghue wrote:
> 1. Please use 'rdi' not 'n' as the parameter here and

Ignore this comment 'n' is the right value here - you've already used
'rdi' in CSID which is the appropriate place 'n' is correct here.

---
bod