[PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets

Wenmeng Liu posted 3 patches 3 weeks, 6 days ago
[PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
Posted by Wenmeng Liu 3 weeks, 6 days ago
Add support for TPG found on LeMans, Monaco, Hamoa.

Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/Makefile         |   1 +
 drivers/media/platform/qcom/camss/camss-csid-680.c |  14 ++
 .../media/platform/qcom/camss/camss-csid-gen3.c    |  14 ++
 drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 257 +++++++++++++++++++++
 drivers/media/platform/qcom/camss/camss.c          | 128 ++++++++++
 5 files changed, 414 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
index d355e67c25700ac061b878543c32ed8defc03ad0..e8996dacf1771d13ec1936c9bebc0e71566898ef 100644
--- a/drivers/media/platform/qcom/camss/Makefile
+++ b/drivers/media/platform/qcom/camss/Makefile
@@ -28,5 +28,6 @@ qcom-camss-objs += \
 		camss-video.o \
 		camss-format.o \
 		camss-tpg.o \
+		camss-tpg-gen1.o \
 
 obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 3ad3a174bcfb8c0d319930d0010df92308cb5ae4..a5da35cae2eb9acf642795c0a91db58d845f211c 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -103,6 +103,8 @@
 #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_NUM_EN				BIT(27)
+#define		CSI2_RX_CFG0_TPG_NUM_SEL			GENMASK(29, 28)
 
 #define CSID_CSI2_RX_CFG1					0x204
 #define		CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN		BIT(0)
@@ -185,11 +187,23 @@ static void __csid_configure_rx(struct csid_device *csid,
 				struct csid_phy_config *phy, int vc)
 {
 	u32 val;
+	struct camss *camss;
+	struct tpg_device *tpg;
 
+	camss = csid->camss;
 	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;
 
+	if (camss->tpg) {
+		tpg = &camss->tpg[phy->csiphy_id];
+
+		if (csid->tpg_linked && tpg->testgen.mode > 0) {
+			val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
+			val |= CSI2_RX_CFG0_TPG_NUM_EN;
+		}
+	}
+
 	writel(val, csid->base + CSID_CSI2_RX_CFG0);
 
 	val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 664245cf6eb0cac662b02f8b920cd1c72db0aeb2..5f9eb533723f2864df64fd6c63e2682fed4a12ae 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -66,6 +66,8 @@
 #define		CSI2_RX_CFG0_VC_MODE		3
 #define		CSI2_RX_CFG0_DL0_INPUT_SEL	4
 #define		CSI2_RX_CFG0_PHY_NUM_SEL	20
+#define		CSI2_RX_CFG0_TPG_NUM_EN		BIT(27)
+#define		CSI2_RX_CFG0_TPG_NUM_SEL	GENMASK(29, 28)
 
 #define CSID_CSI2_RX_CFG1		0x204
 #define		CSI2_RX_CFG1_ECC_CORRECTION_EN	BIT(0)
@@ -109,11 +111,23 @@ static void __csid_configure_rx(struct csid_device *csid,
 				struct csid_phy_config *phy, int vc)
 {
 	int val;
+	struct camss *camss;
+	struct tpg_device *tpg;
 
+	camss = csid->camss;
 	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;
 
+	if (camss->tpg) {
+		tpg = &camss->tpg[phy->csiphy_id];
+
+		if (csid->tpg_linked && tpg->testgen.mode > 0) {
+			val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
+			val |= CSI2_RX_CFG0_TPG_NUM_EN;
+		}
+	}
+
 	writel(val, csid->base + CSID_CSI2_RX_CFG0);
 
 	val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
new file mode 100644
index 0000000000000000000000000000000000000000..d7ef7a1709648406dc59c210d355851397980769
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *
+ * Qualcomm MSM Camera Subsystem - TPG (Test Patter Generator) Module
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+#include "camss-tpg.h"
+#include "camss.h"
+
+#define TPG_HW_VERSION		0x0
+# define HW_VERSION_STEPPING		GENMASK(15, 0)
+# define HW_VERSION_REVISION		GENMASK(27, 16)
+# define HW_VERSION_GENERATION		GENMASK(31, 28)
+
+#define TPG_HW_VER(gen, rev, step) \
+	(((u32)(gen) << 28) | ((u32)(rev) << 16) | (u32)(step))
+
+#define TPG_HW_VER_2_0_0                TPG_HW_VER(2, 0, 0)
+#define TPG_HW_VER_2_1_0                TPG_HW_VER(2, 1, 0)
+
+#define TPG_HW_STATUS		0x4
+
+#define TPG_VC_n_GAIN_CFG(n)		(0x60 + (n) * 0x60)
+
+#define TPG_CTRL		0x64
+# define TPG_CTRL_TEST_EN		BIT(0)
+# define TPG_CTRL_PHY_SEL		BIT(3)
+# define TPG_CTRL_NUM_ACTIVE_LANES	GENMASK(5, 4)
+# define TPG_CTRL_VC_DT_PATTERN_ID	GENMASK(8, 6)
+# define TPG_CTRL_OVERLAP_SHDR_EN	BIT(10)
+# define TPG_CTRL_NUM_ACTIVE_VC		GENMASK(31, 30)
+#  define NUM_ACTIVE_VC_0_ENABLED		0
+#  define NUM_ACTIVE_VC_0_1_ENABLED		1
+#  define NUM_ACTIVE_VC_0_1_2_ENABLED		2
+#  define NUM_ACTIVE_VC_0_1_3_ENABLED		3
+
+#define TPG_VC_n_CFG0(n)	(0x68 + (n) * 0x60)
+# define TPG_VC_n_CFG0_VC_NUM			GENMASK(4, 0)
+# define TPG_VC_n_CFG0_NUM_ACTIVE_DT		GENMASK(9, 8)
+#  define NUM_ACTIVE_SLOTS_0_ENABLED			0
+#  define NUM_ACTIVE_SLOTS_0_1_ENABLED			1
+#  define NUM_ACTIVE_SLOTS_0_1_2_ENABLED		2
+#  define NUM_ACTIVE_SLOTS_0_1_3_ENABLED		3
+# define TPG_VC_n_CFG0_NUM_BATCH		GENMASK(15, 12)
+# define TPG_VC_n_CFG0_NUM_FRAMES		GENMASK(31, 16)
+
+#define TPG_VC_n_LSFR_SEED(n)	(0x6C + (n) * 0x60)
+
+#define TPG_VC_n_HBI_CFG(n)	(0x70 + (n) * 0x60)
+
+#define TPG_VC_n_VBI_CFG(n)	(0x74 + (n) * 0x60)
+
+#define TPG_VC_n_COLOR_BARS_CFG(n)		(0x78 + (n) * 0x60)
+# define TPG_VC_n_COLOR_BARS_CFG_PIX_PATTERN		GENMASK(2, 0)
+# define TPG_VC_n_COLOR_BARS_CFG_QCFA_EN		BIT(3)
+# define TPG_VC_n_COLOR_BARS_CFG_SPLIT_EN		BIT(4)
+# define TPG_VC_n_COLOR_BARS_CFG_NOISE_EN		BIT(5)
+# define TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD		GENMASK(13, 8)
+# define TPG_VC_n_COLOR_BARS_CFG_XCFA_EN		BIT(16)
+# define TPG_VC_n_COLOR_BARS_CFG_SIZE_X			GENMASK(26, 24)
+# define TPG_VC_n_COLOR_BARS_CFG_SIZE_Y			GENMASK(30, 28)
+
+#define TPG_VC_m_DT_n_CFG_0(m, n)		(0x7C + (m) * 0x60 + (n) * 0xC)
+# define TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT	GENMASK(15, 0)
+# define TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH	GENMASK(31, 16)
+
+#define TPG_VC_m_DT_n_CFG_1(m, n)		(0x80 + (m) * 0x60 + (n) * 0xC)
+# define TPG_VC_m_DT_n_CFG_1_DATA_TYPE		GENMASK(5, 0)
+# define TPG_VC_m_DT_n_CFG_1_ECC_XOR_MASK	GENMASK(13, 8)
+# define TPG_VC_m_DT_n_CFG_1_CRC_XOR_MASK	GENMASK(31, 16)
+
+#define TPG_VC_m_DT_n_CFG_2(m, n)		(0x84 + (m) * 0x60 + (n) * 0xC)
+# define TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE		GENMASK(3, 0)
+/* v2.0.0: USER[19:4], ENC[23:20] */
+# define TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD		GENMASK(19, 4)
+# define TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT			GENMASK(23, 20)
+/* v2.1.0: USER[27:4], ENC[31:28] */
+# define TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD	GENMASK(27, 4)
+# define TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT			GENMASK(31, 28)
+
+#define TPG_VC_n_COLOR_BAR_CFA_COLOR0(n)	(0xB0 + (n) * 0x60)
+#define TPG_VC_n_COLOR_BAR_CFA_COLOR1(n)	(0xB4 + (n) * 0x60)
+#define TPG_VC_n_COLOR_BAR_CFA_COLOR2(n)	(0xB8 + (n) * 0x60)
+#define TPG_VC_n_COLOR_BAR_CFA_COLOR3(n)	(0xBC + (n) * 0x60)
+
+/* Line offset between VC(n) and VC(n-1), n form 1 to 3 */
+#define TPG_VC_n_SHDR_CFG	(0x84 + (n) * 0x60)
+
+#define TPG_CLEAR		0x1F4
+
+#define TPG_HBI_PCT_DEFAULT			545	/* 545% */
+#define TPG_VBI_PCT_DEFAULT			10	/* 10% */
+#define PERCENT_BASE				100
+#define BITS_PER_BYTE				8
+
+/* Default user-specified payload for TPG test generator.
+ * Keep consistent with CSID TPG default: 0xBE.
+ */
+#define TPG_USER_SPECIFIED_PAYLOAD_DEFAULT	0xBE
+#define TPG_LFSR_SEED_DEFAULT			0x12345678
+#define TPG_COLOR_BARS_CFG_STANDARD \
+	FIELD_PREP(TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD, 0xA)
+
+static int tpg_stream_on(struct tpg_device *tpg)
+{
+	struct tpg_testgen_config *tg = &tpg->testgen;
+	struct v4l2_mbus_framefmt *input_format;
+	const struct tpg_format_info *format;
+	u8 lane_cnt = tpg->res->lane_cnt;
+	u8 dt_cnt = 0;
+	u8 i;
+	u32 val;
+
+	/* Loop through all enabled VCs and configure stream for each */
+	for (i = 0; i < tpg->res->vc_cnt; i++) {
+		input_format = &tpg->fmt[MSM_TPG_PAD_SRC + i];
+		format = tpg_get_fmt_entry(tpg,
+					   tpg->res->formats->formats,
+					   tpg->res->formats->nformats,
+					   input_format->code);
+		if (IS_ERR(format))
+			return -EINVAL;
+
+		val = FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT, input_format->height & 0xffff) |
+		      FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH, input_format->width & 0xffff);
+		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_0(i, dt_cnt));
+
+		val = FIELD_PREP(TPG_VC_m_DT_n_CFG_1_DATA_TYPE, format->data_type);
+		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_1(i, dt_cnt));
+
+		if (tpg->hw_version == TPG_HW_VER_2_0_0) {
+			val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg->mode - 1) |
+				FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
+					   TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
+				FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
+					   format->encode_format);
+		} else if (tpg->hw_version >= TPG_HW_VER_2_1_0) {
+			val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg->mode - 1) |
+				FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
+					   TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
+				FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
+					   format->encode_format);
+		}
+		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_2(i, dt_cnt));
+
+		writel(TPG_COLOR_BARS_CFG_STANDARD, tpg->base + TPG_VC_n_COLOR_BARS_CFG(i));
+
+		val = DIV_ROUND_UP(input_format->width * format->bpp * TPG_HBI_PCT_DEFAULT,
+				   BITS_PER_BYTE * lane_cnt * PERCENT_BASE);
+		writel(val, tpg->base + TPG_VC_n_HBI_CFG(i));
+		val = input_format->height * TPG_VBI_PCT_DEFAULT / PERCENT_BASE;
+		writel(val, tpg->base + TPG_VC_n_VBI_CFG(i));
+
+		writel(TPG_LFSR_SEED_DEFAULT, tpg->base + TPG_VC_n_LSFR_SEED(i));
+
+		/* configure one DT, infinite frames */
+		val = FIELD_PREP(TPG_VC_n_CFG0_VC_NUM, i) |
+		      FIELD_PREP(TPG_VC_n_CFG0_NUM_FRAMES, 0);
+		writel(val, tpg->base + TPG_VC_n_CFG0(i));
+	}
+
+	val = FIELD_PREP(TPG_CTRL_TEST_EN, 1) |
+		  FIELD_PREP(TPG_CTRL_PHY_SEL, 0) |
+		  FIELD_PREP(TPG_CTRL_NUM_ACTIVE_LANES, lane_cnt - 1) |
+		  FIELD_PREP(TPG_CTRL_VC_DT_PATTERN_ID, 0) |
+		  FIELD_PREP(TPG_CTRL_NUM_ACTIVE_VC, tpg->res->vc_cnt - 1);
+	writel(val, tpg->base + TPG_CTRL);
+
+	return 0;
+}
+
+static void tpg_stream_off(struct tpg_device *tpg)
+{
+	writel(0, tpg->base + TPG_CTRL);
+	writel(1, tpg->base + TPG_CLEAR);
+}
+
+static int tpg_configure_stream(struct tpg_device *tpg, u8 enable)
+{
+	int ret = 0;
+
+	if (enable)
+		ret = tpg_stream_on(tpg);
+	else
+		tpg_stream_off(tpg);
+
+	return ret;
+}
+
+static int tpg_configure_testgen_pattern(struct tpg_device *tpg, s32 val)
+{
+	if (val >= 0 && val <= TPG_PAYLOAD_MODE_COLOR_BARS)
+		tpg->testgen.mode = val;
+
+	return 0;
+}
+
+/*
+ * tpg_hw_version - tpg hardware version query
+ * @tpg: tpg device
+ *
+ * Return HW version or error
+ */
+static u32 tpg_hw_version(struct tpg_device *tpg)
+{
+	u32 hw_version;
+	u32 hw_gen;
+	u32 hw_rev;
+	u32 hw_step;
+
+	hw_version = readl(tpg->base + TPG_HW_VERSION);
+	hw_gen = FIELD_GET(HW_VERSION_GENERATION, hw_version);
+	hw_rev = FIELD_GET(HW_VERSION_REVISION, hw_version);
+	hw_step = FIELD_GET(HW_VERSION_STEPPING, hw_version);
+
+	tpg->hw_version = hw_version;
+
+	dev_dbg_once(tpg->camss->dev, "tpg HW Version = %u.%u.%u\n",
+		     hw_gen, hw_rev, hw_step);
+
+	return hw_version;
+}
+
+/*
+ * tpg_reset - Trigger reset on tpg module and wait to complete
+ * @tpg: tpg device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int tpg_reset(struct tpg_device *tpg)
+{
+	writel(0, tpg->base + TPG_CTRL);
+	writel(1, tpg->base + TPG_CLEAR);
+
+	return 0;
+}
+
+static void tpg_subdev_init(struct tpg_device *tpg)
+{
+	tpg->testgen.modes = testgen_payload_modes;
+	tpg->testgen.nmodes = TPG_PAYLOAD_MODE_NUM_SUPPORTED_GEN1;
+}
+
+const struct tpg_hw_ops tpg_ops_gen1 = {
+	.configure_stream = tpg_configure_stream,
+	.configure_testgen_pattern = tpg_configure_testgen_pattern,
+	.hw_version = tpg_hw_version,
+	.reset = tpg_reset,
+	.subdev_init = tpg_subdev_init,
+};
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 43fdcb9af101ef34b118035ca9c68757b66118df..5cddf1bc09f97c2c61f907939bb54663d8eab3d4 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3199,6 +3199,65 @@ static const struct camss_subdev_resources csiphy_res_8775p[] = {
 	},
 };
 
+static const struct camss_subdev_resources tpg_res_8775p[] = {
+	/* TPG0 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "tpg0" },
+		.interrupt = { "tpg0" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+
+	/* TPG1 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "tpg1" },
+		.interrupt = { "tpg1" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+
+	/* TPG2 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "tpg2" },
+		.interrupt = { "tpg2" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+};
+
 static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID0 */
 	{
@@ -3595,6 +3654,62 @@ static const struct camss_subdev_resources csiphy_res_x1e80100[] = {
 	},
 };
 
+static const struct camss_subdev_resources tpg_res_x1e80100[] = {
+	/* TPG0 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "csitpg0" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+
+	/* TPG1 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "csitpg1" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+
+	/* TPG2 */
+	{
+		.regulators = {  },
+		.clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
+		.clock_rate = {
+			{ 400000000 },
+			{ 0 },
+			{ 400000000 },
+		},
+		.reg = { "csitpg2" },
+		.tpg = {
+			.lane_cnt = 4,
+			.vc_cnt = 1,
+			.formats = &tpg_formats_gen1,
+			.hw_ops = &tpg_ops_gen1
+		}
+	},
+};
+
 static const struct camss_subdev_resources csid_res_x1e80100[] = {
 	/* CSID0 */
 	{
@@ -4674,6 +4789,13 @@ static int camss_probe(struct platform_device *pdev)
 	if (!camss->csiphy)
 		return -ENOMEM;
 
+	if (camss->res->tpg_num > 0) {
+		camss->tpg = devm_kcalloc(dev, camss->res->tpg_num,
+					  sizeof(*camss->tpg), GFP_KERNEL);
+		if (!camss->tpg)
+			return -ENOMEM;
+	}
+
 	camss->csid = devm_kcalloc(dev, camss->res->csid_num, sizeof(*camss->csid),
 				   GFP_KERNEL);
 	if (!camss->csid)
@@ -4863,11 +4985,13 @@ static const struct camss_resources qcs8300_resources = {
 	.version = CAMSS_8300,
 	.pd_name = "top",
 	.csiphy_res = csiphy_res_8300,
+	.tpg_res = tpg_res_8775p,
 	.csid_res = csid_res_8775p,
 	.csid_wrapper_res = &csid_wrapper_res_sm8550,
 	.vfe_res = vfe_res_8775p,
 	.icc_res = icc_res_qcs8300,
 	.csiphy_num = ARRAY_SIZE(csiphy_res_8300),
+	.tpg_num = ARRAY_SIZE(tpg_res_8775p),
 	.csid_num = ARRAY_SIZE(csid_res_8775p),
 	.vfe_num = ARRAY_SIZE(vfe_res_8775p),
 	.icc_path_num = ARRAY_SIZE(icc_res_qcs8300),
@@ -4877,11 +5001,13 @@ static const struct camss_resources sa8775p_resources = {
 	.version = CAMSS_8775P,
 	.pd_name = "top",
 	.csiphy_res = csiphy_res_8775p,
+	.tpg_res = tpg_res_8775p,
 	.csid_res = csid_res_8775p,
 	.csid_wrapper_res = &csid_wrapper_res_sm8550,
 	.vfe_res = vfe_res_8775p,
 	.icc_res = icc_res_sa8775p,
 	.csiphy_num = ARRAY_SIZE(csiphy_res_8775p),
+	.tpg_num = ARRAY_SIZE(tpg_res_8775p),
 	.csid_num = ARRAY_SIZE(csid_res_8775p),
 	.vfe_num = ARRAY_SIZE(vfe_res_8775p),
 	.icc_path_num = ARRAY_SIZE(icc_res_sa8775p),
@@ -4992,11 +5118,13 @@ static const struct camss_resources x1e80100_resources = {
 	.pd_name = "top",
 	.csiphy_res = csiphy_res_x1e80100,
 	.csid_res = csid_res_x1e80100,
+	.tpg_res = tpg_res_x1e80100,
 	.vfe_res = vfe_res_x1e80100,
 	.csid_wrapper_res = &csid_wrapper_res_x1e80100,
 	.icc_res = icc_res_x1e80100,
 	.icc_path_num = ARRAY_SIZE(icc_res_x1e80100),
 	.csiphy_num = ARRAY_SIZE(csiphy_res_x1e80100),
+	.tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
 	.csid_num = ARRAY_SIZE(csid_res_x1e80100),
 	.vfe_num = ARRAY_SIZE(vfe_res_x1e80100),
 };

-- 
2.34.1
Re: [PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
Posted by Vijay Kumar Tumati 3 weeks, 2 days ago
Hi Wenmeng,

On 1/13/2026 1:03 AM, Wenmeng Liu wrote:
> Add support for TPG found on LeMans, Monaco, Hamoa.
>
> Signed-off-by: Wenmeng Liu<wenmeng.liu@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/Makefile         |   1 +
>   drivers/media/platform/qcom/camss/camss-csid-680.c |  14 ++
>   .../media/platform/qcom/camss/camss-csid-gen3.c    |  14 ++
>   drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 257 +++++++++++++++++++++
>   drivers/media/platform/qcom/camss/camss.c          | 128 ++++++++++
>   5 files changed, 414 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index d355e67c25700ac061b878543c32ed8defc03ad0..e8996dacf1771d13ec1936c9bebc0e71566898ef 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -28,5 +28,6 @@ qcom-camss-objs += \
>   		camss-video.o \
>   		camss-format.o \
>   		camss-tpg.o \
> +		camss-tpg-gen1.o \
>   
>   obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
> index 3ad3a174bcfb8c0d319930d0010df92308cb5ae4..a5da35cae2eb9acf642795c0a91db58d845f211c 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
> @@ -103,6 +103,8 @@
>   #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_NUM_EN				BIT(27)
> +#define		CSI2_RX_CFG0_TPG_NUM_SEL			GENMASK(29, 28)
>   
>   #define CSID_CSI2_RX_CFG1					0x204
>   #define		CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN		BIT(0)
> @@ -185,11 +187,23 @@ static void __csid_configure_rx(struct csid_device *csid,
>   				struct csid_phy_config *phy, int vc)
>   {
>   	u32 val;
> +	struct camss *camss;
> +	struct tpg_device *tpg;
>   
> +	camss = csid->camss;
>   	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;
"phy_num_sel" and "tpg_num_sel" can be in if-else. They both are not 
required at once.
>   
> +	if (camss->tpg) {
> +		tpg = &camss->tpg[phy->csiphy_id];
> +
> +		if (csid->tpg_linked && tpg->testgen.mode > 0) {
If the tpg is linked and the mode is not valid, shouldn't you be 
throwing error?
> +			val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
> +			val |= CSI2_RX_CFG0_TPG_NUM_EN;
Can we rename this to CSI2_RX_CFG0_TPG_MUX_EN?
> +		}
> +	}
> +
>   	writel(val, csid->base + CSID_CSI2_RX_CFG0);
>   
>   	val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> index 664245cf6eb0cac662b02f8b920cd1c72db0aeb2..5f9eb533723f2864df64fd6c63e2682fed4a12ae 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
> @@ -66,6 +66,8 @@
>   #define		CSI2_RX_CFG0_VC_MODE		3
>   #define		CSI2_RX_CFG0_DL0_INPUT_SEL	4
>   #define		CSI2_RX_CFG0_PHY_NUM_SEL	20
> +#define		CSI2_RX_CFG0_TPG_NUM_EN		BIT(27)
> +#define		CSI2_RX_CFG0_TPG_NUM_SEL	GENMASK(29, 28)
>   
>   #define CSID_CSI2_RX_CFG1		0x204
>   #define		CSI2_RX_CFG1_ECC_CORRECTION_EN	BIT(0)
> @@ -109,11 +111,23 @@ static void __csid_configure_rx(struct csid_device *csid,
>   				struct csid_phy_config *phy, int vc)
Same as above.
>   {
>   	int val;
> +	struct camss *camss;
> +	struct tpg_device *tpg;
>   
> +	camss = csid->camss;
>   	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;
>   
> +	if (camss->tpg) {
> +		tpg = &camss->tpg[phy->csiphy_id];
> +
> +		if (csid->tpg_linked && tpg->testgen.mode > 0) {
> +			val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
> +			val |= CSI2_RX_CFG0_TPG_NUM_EN;
> +		}
> +	}
> +
>   	writel(val, csid->base + CSID_CSI2_RX_CFG0);
>   
>   	val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
> diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d7ef7a1709648406dc59c210d355851397980769
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
> @@ -0,0 +1,257 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *
> + * Qualcomm MSM Camera Subsystem - TPG (Test Patter Generator) Module
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/completion.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +
> +#include "camss-tpg.h"
> +#include "camss.h"
> +
> +#define TPG_HW_VERSION		0x0
> +# define HW_VERSION_STEPPING		GENMASK(15, 0)
> +# define HW_VERSION_REVISION		GENMASK(27, 16)
> +# define HW_VERSION_GENERATION		GENMASK(31, 28)
> +
> +#define TPG_HW_VER(gen, rev, step) \
> +	(((u32)(gen) << 28) | ((u32)(rev) << 16) | (u32)(step))
> +
> +#define TPG_HW_VER_2_0_0                TPG_HW_VER(2, 0, 0)
> +#define TPG_HW_VER_2_1_0                TPG_HW_VER(2, 1, 0)
> +
> +#define TPG_HW_STATUS		0x4
> +
> +#define TPG_VC_n_GAIN_CFG(n)		(0x60 + (n) * 0x60)
I know why this is here but it may be is better to group this with VC 
based registers. In fact, can you please segregate these macros into sub 
sections with headings like "TPG global registers", "TPG VC based 
registers", "TPG DT based registers" etc. Just for better readability.
> +
> +#define TPG_CTRL		0x64
> +# define TPG_CTRL_TEST_EN		BIT(0)
> +# define TPG_CTRL_PHY_SEL		BIT(3)
> +# define TPG_CTRL_NUM_ACTIVE_LANES	GENMASK(5, 4)
> +# define TPG_CTRL_VC_DT_PATTERN_ID	GENMASK(8, 6)
> +# define TPG_CTRL_OVERLAP_SHDR_EN	BIT(10)
> +# define TPG_CTRL_NUM_ACTIVE_VC		GENMASK(31, 30)
> +#  define NUM_ACTIVE_VC_0_ENABLED		0
> +#  define NUM_ACTIVE_VC_0_1_ENABLED		1
> +#  define NUM_ACTIVE_VC_0_1_2_ENABLED		2
> +#  define NUM_ACTIVE_VC_0_1_3_ENABLED		3
> +
> +#define TPG_VC_n_CFG0(n)	(0x68 + (n) * 0x60)
> +# define TPG_VC_n_CFG0_VC_NUM			GENMASK(4, 0)
> +# define TPG_VC_n_CFG0_NUM_ACTIVE_DT		GENMASK(9, 8)
> +#  define NUM_ACTIVE_SLOTS_0_ENABLED			0
> +#  define NUM_ACTIVE_SLOTS_0_1_ENABLED			1
> +#  define NUM_ACTIVE_SLOTS_0_1_2_ENABLED		2
> +#  define NUM_ACTIVE_SLOTS_0_1_3_ENABLED		3
s/NUM_ACTIVE_SLOTS/DT/?, if you really need these macros. Similarly for 
VCs enabled.
> +# define TPG_VC_n_CFG0_NUM_BATCH		GENMASK(15, 12)
> +# define TPG_VC_n_CFG0_NUM_FRAMES		GENMASK(31, 16)
> +
> +#define TPG_VC_n_LSFR_SEED(n)	(0x6C + (n) * 0x60)
> +
> +#define TPG_VC_n_HBI_CFG(n)	(0x70 + (n) * 0x60)
> +
> +#define TPG_VC_n_VBI_CFG(n)	(0x74 + (n) * 0x60)
> +
> +#define TPG_VC_n_COLOR_BARS_CFG(n)		(0x78 + (n) * 0x60)
> +# define TPG_VC_n_COLOR_BARS_CFG_PIX_PATTERN		GENMASK(2, 0)
> +# define TPG_VC_n_COLOR_BARS_CFG_QCFA_EN		BIT(3)
> +# define TPG_VC_n_COLOR_BARS_CFG_SPLIT_EN		BIT(4)
> +# define TPG_VC_n_COLOR_BARS_CFG_NOISE_EN		BIT(5)
> +# define TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD		GENMASK(13, 8)
> +# define TPG_VC_n_COLOR_BARS_CFG_XCFA_EN		BIT(16)
> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_X			GENMASK(26, 24)
> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_Y			GENMASK(30, 28)
> +
> +#define TPG_VC_m_DT_n_CFG_0(m, n)		(0x7C + (m) * 0x60 + (n) * 0xC)
> +# define TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT	GENMASK(15, 0)
> +# define TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH	GENMASK(31, 16)
> +
> +#define TPG_VC_m_DT_n_CFG_1(m, n)		(0x80 + (m) * 0x60 + (n) * 0xC)
> +# define TPG_VC_m_DT_n_CFG_1_DATA_TYPE		GENMASK(5, 0)
> +# define TPG_VC_m_DT_n_CFG_1_ECC_XOR_MASK	GENMASK(13, 8)
> +# define TPG_VC_m_DT_n_CFG_1_CRC_XOR_MASK	GENMASK(31, 16)
> +
> +#define TPG_VC_m_DT_n_CFG_2(m, n)		(0x84 + (m) * 0x60 + (n) * 0xC)
> +# define TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE		GENMASK(3, 0)
> +/* v2.0.0: USER[19:4], ENC[23:20] */
> +# define TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD		GENMASK(19, 4)
> +# define TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT			GENMASK(23, 20)
For better readability, can you make these TPG_V2_0_*?
> +/* v2.1.0: USER[27:4], ENC[31:28] */
> +# define TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD	GENMASK(27, 4)
> +# define TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT			GENMASK(31, 28)
> +
> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR0(n)	(0xB0 + (n) * 0x60)
> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR1(n)	(0xB4 + (n) * 0x60)
> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR2(n)	(0xB8 + (n) * 0x60)
> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR3(n)	(0xBC + (n) * 0x60)
> +
> +/* Line offset between VC(n) and VC(n-1), n form 1 to 3 */
> +#define TPG_VC_n_SHDR_CFG	(0x84 + (n) * 0x60)
> +
> +#define TPG_CLEAR		0x1F4
> +
> +#define TPG_HBI_PCT_DEFAULT			545	/* 545% */
> +#define TPG_VBI_PCT_DEFAULT			10	/* 10% */
> +#define PERCENT_BASE				100
> +#define BITS_PER_BYTE				8
> +
> +/* Default user-specified payload for TPG test generator.
> + * Keep consistent with CSID TPG default: 0xBE.
> + */
> +#define TPG_USER_SPECIFIED_PAYLOAD_DEFAULT	0xBE
> +#define TPG_LFSR_SEED_DEFAULT			0x12345678
> +#define TPG_COLOR_BARS_CFG_STANDARD \
> +	FIELD_PREP(TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD, 0xA)
> +
> +static int tpg_stream_on(struct tpg_device *tpg)
Add function headers? For this  and a few other below.
> +{
> +	struct tpg_testgen_config *tg = &tpg->testgen;
> +	struct v4l2_mbus_framefmt *input_format;
> +	const struct tpg_format_info *format;
> +	u8 lane_cnt = tpg->res->lane_cnt;
> +	u8 dt_cnt = 0;
> +	u8 i;
> +	u32 val;
> +
> +	/* Loop through all enabled VCs and configure stream for each */
> +	for (i = 0; i < tpg->res->vc_cnt; i++) {
Here as well, can we segregate the code to global, VC based and DT based 
configs with some comments?
> +		input_format = &tpg->fmt[MSM_TPG_PAD_SRC + i];
> +		format = tpg_get_fmt_entry(tpg,
> +					   tpg->res->formats->formats,
> +					   tpg->res->formats->nformats,
> +					   input_format->code);
> +		if (IS_ERR(format))
> +			return -EINVAL;
> +
> +		val = FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT, input_format->height & 0xffff) |
> +		      FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH, input_format->width & 0xffff);
> +		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_0(i, dt_cnt));
> +
> +		val = FIELD_PREP(TPG_VC_m_DT_n_CFG_1_DATA_TYPE, format->data_type);
> +		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_1(i, dt_cnt));
> +
> +		if (tpg->hw_version == TPG_HW_VER_2_0_0) {
> +			val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg->mode - 1) |
> +				FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
> +					   TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
> +				FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
> +					   format->encode_format);
> +		} else if (tpg->hw_version >= TPG_HW_VER_2_1_0) {
> +			val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg->mode - 1) |
> +				FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
> +					   TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
> +				FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
> +					   format->encode_format);
> +		}
> +		writel(val, tpg->base + TPG_VC_m_DT_n_CFG_2(i, dt_cnt));
> +
> +		writel(TPG_COLOR_BARS_CFG_STANDARD, tpg->base + TPG_VC_n_COLOR_BARS_CFG(i));
> +
> +		val = DIV_ROUND_UP(input_format->width * format->bpp * TPG_HBI_PCT_DEFAULT,
> +				   BITS_PER_BYTE * lane_cnt * PERCENT_BASE);
> +		writel(val, tpg->base + TPG_VC_n_HBI_CFG(i));
> +		val = input_format->height * TPG_VBI_PCT_DEFAULT / PERCENT_BASE;
> +		writel(val, tpg->base + TPG_VC_n_VBI_CFG(i));
> +
> +		writel(TPG_LFSR_SEED_DEFAULT, tpg->base + TPG_VC_n_LSFR_SEED(i));
> +
> +		/* configure one DT, infinite frames */
Although this driver is not supporting more than one DT in a VC right 
now, is there a way we can make the API generic enough to receive #DTs 
in each VS and their dimensions?
> +		val = FIELD_PREP(TPG_VC_n_CFG0_VC_NUM, i) |
> +		      FIELD_PREP(TPG_VC_n_CFG0_NUM_FRAMES, 0);
> +		writel(val, tpg->base + TPG_VC_n_CFG0(i));
> +	}
> +
> +	val = FIELD_PREP(TPG_CTRL_TEST_EN, 1) |
> +		  FIELD_PREP(TPG_CTRL_PHY_SEL, 0) |
Same here, is there a way to make the API generic to receive CPHY / DPHY 
mode required?
> +		  FIELD_PREP(TPG_CTRL_NUM_ACTIVE_LANES, lane_cnt - 1) |
> +		  FIELD_PREP(TPG_CTRL_VC_DT_PATTERN_ID, 0) |
You are assuming frame interleaved mode always. It may be is a good 
start but a bunch of functionality is missing here. Just please think of 
the scalability of the API even though the driver support is limited at 
this point.
> +		  FIELD_PREP(TPG_CTRL_NUM_ACTIVE_VC, tpg->res->vc_cnt - 1);
> +	writel(val, tpg->base + TPG_CTRL);
> +
> +	return 0;
> +}
> +
> +static void tpg_stream_off(struct tpg_device *tpg)
> +{
> +	writel(0, tpg->base + TPG_CTRL);
> +	writel(1, tpg->base + TPG_CLEAR);
Why not just reuse the reset function?
> +}
> +
> +static int tpg_configure_stream(struct tpg_device *tpg, u8 enable)
> +{
> +	int ret = 0;
> +
> +	if (enable)
> +		ret = tpg_stream_on(tpg);
> +	else
> +		tpg_stream_off(tpg);
> +
> +	return ret;
> +}
> +
> +static int tpg_configure_testgen_pattern(struct tpg_device *tpg, s32 val)
> +{
> +	if (val >= 0 && val <= TPG_PAYLOAD_MODE_COLOR_BARS)
> +		tpg->testgen.mode = val;
> +
> +	return 0;
> +}
> +
> +/*
> + * tpg_hw_version - tpg hardware version query
> + * @tpg: tpg device
> + *
> + * Return HW version or error
> + */
> +static u32 tpg_hw_version(struct tpg_device *tpg)
> +{
> +	u32 hw_version;
> +	u32 hw_gen;
> +	u32 hw_rev;
> +	u32 hw_step;
> +
> +	hw_version = readl(tpg->base + TPG_HW_VERSION);
> +	hw_gen = FIELD_GET(HW_VERSION_GENERATION, hw_version);
> +	hw_rev = FIELD_GET(HW_VERSION_REVISION, hw_version);
> +	hw_step = FIELD_GET(HW_VERSION_STEPPING, hw_version);
> +
> +	tpg->hw_version = hw_version;
> +
> +	dev_dbg_once(tpg->camss->dev, "tpg HW Version = %u.%u.%u\n",
> +		     hw_gen, hw_rev, hw_step);
> +
> +	return hw_version;
> +}
> +
> +/*
> + * tpg_reset - Trigger reset on tpg module and wait to complete
Doesn't seem like there is any wait here, right? Also, do you want to 
clear the IRQs in reset?
> + * @tpg: tpg device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int tpg_reset(struct tpg_device *tpg)
> +{
> +	writel(0, tpg->base + TPG_CTRL);
> +	writel(1, tpg->base + TPG_CLEAR);
> +
> +	return 0;
> +}
> +
> +static void tpg_subdev_init(struct tpg_device *tpg)
> +{
> +	tpg->testgen.modes = testgen_payload_modes;
> +	tpg->testgen.nmodes = TPG_PAYLOAD_MODE_NUM_SUPPORTED_GEN1;
> +}
> +
> +const struct tpg_hw_ops tpg_ops_gen1 = {
> +	.configure_stream = tpg_configure_stream,
> +	.configure_testgen_pattern = tpg_configure_testgen_pattern,
> +	.hw_version = tpg_hw_version,
> +	.reset = tpg_reset,
> +	.subdev_init = tpg_subdev_init,
> +};
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 43fdcb9af101ef34b118035ca9c68757b66118df..5cddf1bc09f97c2c61f907939bb54663d8eab3d4 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -3199,6 +3199,65 @@ static const struct camss_subdev_resources csiphy_res_8775p[] = {
>   	},
>   };
>   
> +static const struct camss_subdev_resources tpg_res_8775p[] = {
> +	/* TPG0 */
> +	{
> +		.regulators = {  },
> +		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
Why should TPG need camnoc_rt_axi clk?
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "tpg0" },
> +		.interrupt = { "tpg0" },
> +		.tpg = {
> +			.lane_cnt = 4,
> +			.vc_cnt = 1,
> +			.formats = &tpg_formats_gen1,
> +			.hw_ops = &tpg_ops_gen1
> +		}
> +	},
> +
> +	/* TPG1 */
> +	{
> +		.regulators = {  },
> +		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "tpg1" },
> +		.interrupt = { "tpg1" },
> +		.tpg = {
> +			.lane_cnt = 4,
> +			.vc_cnt = 1,
> +			.formats = &tpg_formats_gen1,
> +			.hw_ops = &tpg_ops_gen1
> +		}
> +	},
> +
> +	/* TPG2 */
> +	{
> +		.regulators = {  },
> +		.clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "tpg2" },
> +		.interrupt = { "tpg2" }, + .tpg = { + .lane_cnt = 4, + .vc_cnt = 1, + .formats = 
> &tpg_formats_gen1, + .hw_ops = &tpg_ops_gen1 + } + }, +}; + static 
> const struct camss_subdev_resources csid_res_8775p[] = { /* CSID0 */ { 
> @@ -3595,6 +3654,62 @@ static const struct camss_subdev_resources 
> csiphy_res_x1e80100[] = { }, }; +static const struct 
> camss_subdev_resources tpg_res_x1e80100[] = { + /* TPG0 */ + { + 
> .regulators = { }, + .clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "csitpg0" },
> +		.tpg = {
> +			.lane_cnt = 4,
> +			.vc_cnt = 1,
> +			.formats = &tpg_formats_gen1,
> +			.hw_ops = &tpg_ops_gen1
> +		}
> +	},
> +
> +	/* TPG1 */
> +	{
> +		.regulators = {  },
> +		.clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "csitpg1" },
> +		.tpg = {
> +			.lane_cnt = 4,
> +			.vc_cnt = 1,
> +			.formats = &tpg_formats_gen1,
> +			.hw_ops = &tpg_ops_gen1
> +		}
> +	},
> +
> +	/* TPG2 */
> +	{
> +		.regulators = {  },
> +		.clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
> +		.clock_rate = {
> +			{ 400000000 },
> +			{ 0 },
> +			{ 400000000 },
> +		},
> +		.reg = { "csitpg2" },
> +		.tpg = {
> +			.lane_cnt = 4,
> +			.vc_cnt = 1,
> +			.formats = &tpg_formats_gen1,
> +			.hw_ops = &tpg_ops_gen1
> +		}
> +	},
> +};
> +
>   static const struct camss_subdev_resources csid_res_x1e80100[] = {
>   	/* CSID0 */
>   	{
> @@ -4674,6 +4789,13 @@ static int camss_probe(struct platform_device *pdev)
>   	if (!camss->csiphy)
>   		return -ENOMEM;
>   
> +	if (camss->res->tpg_num > 0) {
> +		camss->tpg = devm_kcalloc(dev, camss->res->tpg_num,
> +					  sizeof(*camss->tpg), GFP_KERNEL);
> +		if (!camss->tpg)
> +			return -ENOMEM;
> +	}
> +
>   	camss->csid = devm_kcalloc(dev, camss->res->csid_num, sizeof(*camss->csid),
>   				   GFP_KERNEL);
>   	if (!camss->csid)
> @@ -4863,11 +4985,13 @@ static const struct camss_resources qcs8300_resources = {
>   	.version = CAMSS_8300,
>   	.pd_name = "top", .csiphy_res = csiphy_res_8300, + .tpg_res = tpg_res_8775p, 
> .csid_res = csid_res_8775p, .csid_wrapper_res = 
> &csid_wrapper_res_sm8550, .vfe_res = vfe_res_8775p, .icc_res = 
> icc_res_qcs8300, .csiphy_num = ARRAY_SIZE(csiphy_res_8300), + .tpg_num 
> = ARRAY_SIZE(tpg_res_8775p), .csid_num = ARRAY_SIZE(csid_res_8775p), 
> .vfe_num = ARRAY_SIZE(vfe_res_8775p), .icc_path_num = 
> ARRAY_SIZE(icc_res_qcs8300), @@ -4877,11 +5001,13 @@ static const 
> struct camss_resources sa8775p_resources = { .version = CAMSS_8775P, 
> .pd_name = "top", .csiphy_res = csiphy_res_8775p, + .tpg_res = tpg_res_8775p, 
> .csid_res = csid_res_8775p, .csid_wrapper_res = 
> &csid_wrapper_res_sm8550, .vfe_res = vfe_res_8775p, .icc_res = 
> icc_res_sa8775p, .csiphy_num = ARRAY_SIZE(csiphy_res_8775p), + 
> .tpg_num = ARRAY_SIZE(tpg_res_8775p), .csid_num = 
> ARRAY_SIZE(csid_res_8775p), .vfe_num = ARRAY_SIZE(vfe_res_8775p), 
> .icc_path_num = ARRAY_SIZE(icc_res_sa8775p), @@ -4992,11 +5118,13 @@ 
> static const struct camss_resources x1e80100_resources = { .pd_name = "top",
>   	.csiphy_res = csiphy_res_x1e80100,
>   	.csid_res = csid_res_x1e80100,
> +	.tpg_res = tpg_res_x1e80100,
>   	.vfe_res = vfe_res_x1e80100,
>   	.csid_wrapper_res = &csid_wrapper_res_x1e80100,
>   	.icc_res = icc_res_x1e80100,
>   	.icc_path_num = ARRAY_SIZE(icc_res_x1e80100),
>   	.csiphy_num = ARRAY_SIZE(csiphy_res_x1e80100),
> +	.tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
>   	.csid_num = ARRAY_SIZE(csid_res_x1e80100),
>   	.vfe_num = ARRAY_SIZE(vfe_res_x1e80100),
>   };

Thanks,

Vijay.

Re: [PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
Posted by Wenmeng Liu 3 weeks ago
Hi Vijay,

On 1/17/2026 3:32 AM, Vijay Kumar Tumati wrote:
> Hi Wenmeng,
> 
> On 1/13/2026 1:03 AM, Wenmeng Liu wrote:
>> Add support for TPG found on LeMans, Monaco, Hamoa.
>>
>> Signed-off-by: Wenmeng Liu<wenmeng.liu@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/camss/Makefile         |   1 +
>>   drivers/media/platform/qcom/camss/camss-csid-680.c |  14 ++
>>   .../media/platform/qcom/camss/camss-csid-gen3.c    |  14 ++
>>   drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 257 +++++++++++ 
>> ++++++++++
>>   drivers/media/platform/qcom/camss/camss.c          | 128 ++++++++++
>>   5 files changed, 414 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/ 
>> media/platform/qcom/camss/Makefile
>> index 
>> d355e67c25700ac061b878543c32ed8defc03ad0..e8996dacf1771d13ec1936c9bebc0e71566898ef 100644
>> --- a/drivers/media/platform/qcom/camss/Makefile
>> +++ b/drivers/media/platform/qcom/camss/Makefile
>> @@ -28,5 +28,6 @@ qcom-camss-objs += \
>>           camss-video.o \
>>           camss-format.o \
>>           camss-tpg.o \
>> +        camss-tpg-gen1.o \
>>   obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/ 
>> drivers/media/platform/qcom/camss/camss-csid-680.c
>> index 
>> 3ad3a174bcfb8c0d319930d0010df92308cb5ae4..a5da35cae2eb9acf642795c0a91db58d845f211c 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
>> @@ -103,6 +103,8 @@
>>   #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_NUM_EN                BIT(27)
>> +#define        CSI2_RX_CFG0_TPG_NUM_SEL            GENMASK(29, 28)
>>   #define CSID_CSI2_RX_CFG1                    0x204
>>   #define        CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN        BIT(0)
>> @@ -185,11 +187,23 @@ static void __csid_configure_rx(struct 
>> csid_device *csid,
>>                   struct csid_phy_config *phy, int vc)
>>   {
>>       u32 val;
>> +    struct camss *camss;
>> +    struct tpg_device *tpg;
>> +    camss = csid->camss;
>>       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;
> "phy_num_sel" and "tpg_num_sel" can be in if-else. They both are not 
> required at once.

ACK

>> +    if (camss->tpg) {
>> +        tpg = &camss->tpg[phy->csiphy_id];
>> +
>> +        if (csid->tpg_linked && tpg->testgen.mode > 0) {
> If the tpg is linked and the mode is not valid, shouldn't you be 
> throwing error?

ACK
>> +            val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy- 
>> >csiphy_id + 1);
>> +            val |= CSI2_RX_CFG0_TPG_NUM_EN;
> Can we rename this to CSI2_RX_CFG0_TPG_MUX_EN?

ACK

>> +        }
>> +    }
>> +
>>       writel(val, csid->base + CSID_CSI2_RX_CFG0);
>>       val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/ 
>> drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> index 
>> 664245cf6eb0cac662b02f8b920cd1c72db0aeb2..5f9eb533723f2864df64fd6c63e2682fed4a12ae 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> @@ -66,6 +66,8 @@
>>   #define        CSI2_RX_CFG0_VC_MODE        3
>>   #define        CSI2_RX_CFG0_DL0_INPUT_SEL    4
>>   #define        CSI2_RX_CFG0_PHY_NUM_SEL    20
>> +#define        CSI2_RX_CFG0_TPG_NUM_EN        BIT(27)
>> +#define        CSI2_RX_CFG0_TPG_NUM_SEL    GENMASK(29, 28)
>>   #define CSID_CSI2_RX_CFG1        0x204
>>   #define        CSI2_RX_CFG1_ECC_CORRECTION_EN    BIT(0)
>> @@ -109,11 +111,23 @@ static void __csid_configure_rx(struct 
>> csid_device *csid,
>>                   struct csid_phy_config *phy, int vc)
> Same as above.

ACK

>>   {
>>       int val;
>> +    struct camss *camss;
>> +    struct tpg_device *tpg;
>> +    camss = csid->camss;
>>       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;
>> +    if (camss->tpg) {
>> +        tpg = &camss->tpg[phy->csiphy_id];
>> +
>> +        if (csid->tpg_linked && tpg->testgen.mode > 0) {
>> +            val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy- 
>> >csiphy_id + 1);
>> +            val |= CSI2_RX_CFG0_TPG_NUM_EN;
>> +        }
>> +    }
>> +
>>       writel(val, csid->base + CSID_CSI2_RX_CFG0);
>>       val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
>> diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/ 
>> drivers/media/platform/qcom/camss/camss-tpg-gen1.c
>> new file mode 100644
>> index 
>> 0000000000000000000000000000000000000000..d7ef7a1709648406dc59c210d355851397980769
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
>> @@ -0,0 +1,257 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *
>> + * Qualcomm MSM Camera Subsystem - TPG (Test Patter Generator) Module
>> + *
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +#include <linux/bitfield.h>
>> +#include <linux/completion.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +
>> +#include "camss-tpg.h"
>> +#include "camss.h"
>> +
>> +#define TPG_HW_VERSION        0x0
>> +# define HW_VERSION_STEPPING        GENMASK(15, 0)
>> +# define HW_VERSION_REVISION        GENMASK(27, 16)
>> +# define HW_VERSION_GENERATION        GENMASK(31, 28)
>> +
>> +#define TPG_HW_VER(gen, rev, step) \
>> +    (((u32)(gen) << 28) | ((u32)(rev) << 16) | (u32)(step))
>> +
>> +#define TPG_HW_VER_2_0_0                TPG_HW_VER(2, 0, 0)
>> +#define TPG_HW_VER_2_1_0                TPG_HW_VER(2, 1, 0)
>> +
>> +#define TPG_HW_STATUS        0x4
>> +
>> +#define TPG_VC_n_GAIN_CFG(n)        (0x60 + (n) * 0x60)
> I know why this is here but it may be is better to group this with VC 
> based registers. In fact, can you please segregate these macros into sub 
> sections with headings like "TPG global registers", "TPG VC based 
> registers", "TPG DT based registers" etc. Just for better readability.

ACK, registers are arranged according to their addresses, and I will 
mark the global registers.

>> +
>> +#define TPG_CTRL        0x64
>> +# define TPG_CTRL_TEST_EN        BIT(0)
>> +# define TPG_CTRL_PHY_SEL        BIT(3)
>> +# define TPG_CTRL_NUM_ACTIVE_LANES    GENMASK(5, 4)
>> +# define TPG_CTRL_VC_DT_PATTERN_ID    GENMASK(8, 6)
>> +# define TPG_CTRL_OVERLAP_SHDR_EN    BIT(10)
>> +# define TPG_CTRL_NUM_ACTIVE_VC        GENMASK(31, 30)
>> +#  define NUM_ACTIVE_VC_0_ENABLED        0
>> +#  define NUM_ACTIVE_VC_0_1_ENABLED        1
>> +#  define NUM_ACTIVE_VC_0_1_2_ENABLED        2
>> +#  define NUM_ACTIVE_VC_0_1_3_ENABLED        3
>> +
>> +#define TPG_VC_n_CFG0(n)    (0x68 + (n) * 0x60)
>> +# define TPG_VC_n_CFG0_VC_NUM            GENMASK(4, 0)
>> +# define TPG_VC_n_CFG0_NUM_ACTIVE_DT        GENMASK(9, 8)
>> +#  define NUM_ACTIVE_SLOTS_0_ENABLED            0
>> +#  define NUM_ACTIVE_SLOTS_0_1_ENABLED            1
>> +#  define NUM_ACTIVE_SLOTS_0_1_2_ENABLED        2
>> +#  define NUM_ACTIVE_SLOTS_0_1_3_ENABLED        3
> s/NUM_ACTIVE_SLOTS/DT/?, if you really need these macros. Similarly for 
> VCs enabled.

ACK

>> +# define TPG_VC_n_CFG0_NUM_BATCH        GENMASK(15, 12)
>> +# define TPG_VC_n_CFG0_NUM_FRAMES        GENMASK(31, 16)
>> +
>> +#define TPG_VC_n_LSFR_SEED(n)    (0x6C + (n) * 0x60)
>> +
>> +#define TPG_VC_n_HBI_CFG(n)    (0x70 + (n) * 0x60)
>> +
>> +#define TPG_VC_n_VBI_CFG(n)    (0x74 + (n) * 0x60)
>> +
>> +#define TPG_VC_n_COLOR_BARS_CFG(n)        (0x78 + (n) * 0x60)
>> +# define TPG_VC_n_COLOR_BARS_CFG_PIX_PATTERN        GENMASK(2, 0)
>> +# define TPG_VC_n_COLOR_BARS_CFG_QCFA_EN        BIT(3)
>> +# define TPG_VC_n_COLOR_BARS_CFG_SPLIT_EN        BIT(4)
>> +# define TPG_VC_n_COLOR_BARS_CFG_NOISE_EN        BIT(5)
>> +# define TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD        GENMASK(13, 8)
>> +# define TPG_VC_n_COLOR_BARS_CFG_XCFA_EN        BIT(16)
>> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_X            GENMASK(26, 24)
>> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_Y            GENMASK(30, 28)
>> +
>> +#define TPG_VC_m_DT_n_CFG_0(m, n)        (0x7C + (m) * 0x60 + (n) * 0xC)
>> +# define TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT    GENMASK(15, 0)
>> +# define TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH    GENMASK(31, 16)
>> +
>> +#define TPG_VC_m_DT_n_CFG_1(m, n)        (0x80 + (m) * 0x60 + (n) * 0xC)
>> +# define TPG_VC_m_DT_n_CFG_1_DATA_TYPE        GENMASK(5, 0)
>> +# define TPG_VC_m_DT_n_CFG_1_ECC_XOR_MASK    GENMASK(13, 8)
>> +# define TPG_VC_m_DT_n_CFG_1_CRC_XOR_MASK    GENMASK(31, 16)
>> +
>> +#define TPG_VC_m_DT_n_CFG_2(m, n)        (0x84 + (m) * 0x60 + (n) * 0xC)
>> +# define TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE        GENMASK(3, 0)
>> +/* v2.0.0: USER[19:4], ENC[23:20] */
>> +# define TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD        
>> GENMASK(19, 4)
>> +# define TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT            GENMASK(23, 20)
> For better readability, can you make these TPG_V2_0_*?
>> +/* v2.1.0: USER[27:4], ENC[31:28] */
>> +# define TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD    
>> GENMASK(27, 4)
>> +# define TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT            
>> GENMASK(31, 28)
>> +
>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR0(n)    (0xB0 + (n) * 0x60)
>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR1(n)    (0xB4 + (n) * 0x60)
>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR2(n)    (0xB8 + (n) * 0x60)
>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR3(n)    (0xBC + (n) * 0x60)
>> +
>> +/* Line offset between VC(n) and VC(n-1), n form 1 to 3 */
>> +#define TPG_VC_n_SHDR_CFG    (0x84 + (n) * 0x60)
>> +
>> +#define TPG_CLEAR        0x1F4
>> +
>> +#define TPG_HBI_PCT_DEFAULT            545    /* 545% */
>> +#define TPG_VBI_PCT_DEFAULT            10    /* 10% */
>> +#define PERCENT_BASE                100
>> +#define BITS_PER_BYTE                8
>> +
>> +/* Default user-specified payload for TPG test generator.
>> + * Keep consistent with CSID TPG default: 0xBE.
>> + */
>> +#define TPG_USER_SPECIFIED_PAYLOAD_DEFAULT    0xBE
>> +#define TPG_LFSR_SEED_DEFAULT            0x12345678
>> +#define TPG_COLOR_BARS_CFG_STANDARD \
>> +    FIELD_PREP(TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD, 0xA)
>> +
>> +static int tpg_stream_on(struct tpg_device *tpg)
> Add function headers? For this  and a few other below.

I received a comment asking me to remove the comments,
if a function is not called in multiple places or its purpose cannot be 
directly understood, adding extra documentation is useless.

>> +{
>> +    struct tpg_testgen_config *tg = &tpg->testgen;
>> +    struct v4l2_mbus_framefmt *input_format;
>> +    const struct tpg_format_info *format;
>> +    u8 lane_cnt = tpg->res->lane_cnt;
>> +    u8 dt_cnt = 0;
>> +    u8 i;
>> +    u32 val;
>> +
>> +    /* Loop through all enabled VCs and configure stream for each */
>> +    for (i = 0; i < tpg->res->vc_cnt; i++) {
> Here as well, can we segregate the code to global, VC based and DT based 
> configs with some comments?

This loop all contains configurations related to VC/DT.
Will add some comments for this.

>> +        input_format = &tpg->fmt[MSM_TPG_PAD_SRC + i];
>> +        format = tpg_get_fmt_entry(tpg,
>> +                       tpg->res->formats->formats,
>> +                       tpg->res->formats->nformats,
>> +                       input_format->code);
>> +        if (IS_ERR(format))
>> +            return -EINVAL;
>> +
>> +        val = FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT, 
>> input_format->height & 0xffff) |
>> +              FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH, 
>> input_format->width & 0xffff);
>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_0(i, dt_cnt));
>> +
>> +        val = FIELD_PREP(TPG_VC_m_DT_n_CFG_1_DATA_TYPE, format- 
>> >data_type);
>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_1(i, dt_cnt));
>> +
>> +        if (tpg->hw_version == TPG_HW_VER_2_0_0) {
>> +            val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg- 
>> >mode - 1) |
>> +                
>> FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
>> +                       TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
>> +                FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
>> +                       format->encode_format);
>> +        } else if (tpg->hw_version >= TPG_HW_VER_2_1_0) {
>> +            val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg- 
>> >mode - 1) |
>> +                
>> FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
>> +                       TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
>> +                FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
>> +                       format->encode_format);
>> +        }
>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_2(i, dt_cnt));
>> +
>> +        writel(TPG_COLOR_BARS_CFG_STANDARD, tpg->base + 
>> TPG_VC_n_COLOR_BARS_CFG(i));
>> +
>> +        val = DIV_ROUND_UP(input_format->width * format->bpp * 
>> TPG_HBI_PCT_DEFAULT,
>> +                   BITS_PER_BYTE * lane_cnt * PERCENT_BASE);
>> +        writel(val, tpg->base + TPG_VC_n_HBI_CFG(i));
>> +        val = input_format->height * TPG_VBI_PCT_DEFAULT / PERCENT_BASE;
>> +        writel(val, tpg->base + TPG_VC_n_VBI_CFG(i));
>> +
>> +        writel(TPG_LFSR_SEED_DEFAULT, tpg->base + 
>> TPG_VC_n_LSFR_SEED(i));
>> +
>> +        /* configure one DT, infinite frames */
> Although this driver is not supporting more than one DT in a VC right 
> now, is there a way we can make the API generic enough to receive #DTs 
> in each VS and their dimensions?

ACK

>> +        val = FIELD_PREP(TPG_VC_n_CFG0_VC_NUM, i) |
>> +              FIELD_PREP(TPG_VC_n_CFG0_NUM_FRAMES, 0);
>> +        writel(val, tpg->base + TPG_VC_n_CFG0(i));
>> +    }
>> +
>> +    val = FIELD_PREP(TPG_CTRL_TEST_EN, 1) |
>> +          FIELD_PREP(TPG_CTRL_PHY_SEL, 0) |
> Same here, is there a way to make the API generic to receive CPHY / DPHY 
> mode required?

ACK

>> +          FIELD_PREP(TPG_CTRL_NUM_ACTIVE_LANES, lane_cnt - 1) |
>> +          FIELD_PREP(TPG_CTRL_VC_DT_PATTERN_ID, 0) |
> You are assuming frame interleaved mode always. It may be is a good 
> start but a bunch of functionality is missing here. Just please think of 
> the scalability of the API even though the driver support is limited at 
> this point.

ACK

>> +          FIELD_PREP(TPG_CTRL_NUM_ACTIVE_VC, tpg->res->vc_cnt - 1);
>> +    writel(val, tpg->base + TPG_CTRL);
>> +
>> +    return 0;
>> +}
>> +
>> +static void tpg_stream_off(struct tpg_device *tpg)
>> +{
>> +    writel(0, tpg->base + TPG_CTRL);
>> +    writel(1, tpg->base + TPG_CLEAR);
> Why not just reuse the reset function?

ACK

>> +}
>> +
>> +static int tpg_configure_stream(struct tpg_device *tpg, u8 enable)
>> +{
>> +    int ret = 0;
>> +
>> +    if (enable)
>> +        ret = tpg_stream_on(tpg);
>> +    else
>> +        tpg_stream_off(tpg);
>> +
>> +    return ret;
>> +}
>> +
>> +static int tpg_configure_testgen_pattern(struct tpg_device *tpg, s32 
>> val)
>> +{
>> +    if (val >= 0 && val <= TPG_PAYLOAD_MODE_COLOR_BARS)
>> +        tpg->testgen.mode = val;
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * tpg_hw_version - tpg hardware version query
>> + * @tpg: tpg device
>> + *
>> + * Return HW version or error
>> + */
>> +static u32 tpg_hw_version(struct tpg_device *tpg)
>> +{
>> +    u32 hw_version;
>> +    u32 hw_gen;
>> +    u32 hw_rev;
>> +    u32 hw_step;
>> +
>> +    hw_version = readl(tpg->base + TPG_HW_VERSION);
>> +    hw_gen = FIELD_GET(HW_VERSION_GENERATION, hw_version);
>> +    hw_rev = FIELD_GET(HW_VERSION_REVISION, hw_version);
>> +    hw_step = FIELD_GET(HW_VERSION_STEPPING, hw_version);
>> +
>> +    tpg->hw_version = hw_version;
>> +
>> +    dev_dbg_once(tpg->camss->dev, "tpg HW Version = %u.%u.%u\n",
>> +             hw_gen, hw_rev, hw_step);
>> +
>> +    return hw_version;
>> +}
>> +
>> +/*
>> + * tpg_reset - Trigger reset on tpg module and wait to complete
> Doesn't seem like there is any wait here, right?

Do you have any suggestions on this? I noticed that there is no delay 
downstream either.

> Also, do you want to 
> clear the IRQs in reset?

Following the maintainer's suggestion, I removed IRQ support, but we do 
need to add IRQ clearing.

>> + * @tpg: tpg device
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int tpg_reset(struct tpg_device *tpg)
>> +{
>> +    writel(0, tpg->base + TPG_CTRL);
>> +    writel(1, tpg->base + TPG_CLEAR);
>> +
>> +    return 0;
>> +}
>> +
>> +static void tpg_subdev_init(struct tpg_device *tpg)
>> +{
>> +    tpg->testgen.modes = testgen_payload_modes;
>> +    tpg->testgen.nmodes = TPG_PAYLOAD_MODE_NUM_SUPPORTED_GEN1;
>> +}
>> +
>> +const struct tpg_hw_ops tpg_ops_gen1 = {
>> +    .configure_stream = tpg_configure_stream,
>> +    .configure_testgen_pattern = tpg_configure_testgen_pattern,
>> +    .hw_version = tpg_hw_version,
>> +    .reset = tpg_reset,
>> +    .subdev_init = tpg_subdev_init,
>> +};
>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/ 
>> media/platform/qcom/camss/camss.c
>> index 
>> 43fdcb9af101ef34b118035ca9c68757b66118df..5cddf1bc09f97c2c61f907939bb54663d8eab3d4 100644
>> --- a/drivers/media/platform/qcom/camss/camss.c
>> +++ b/drivers/media/platform/qcom/camss/camss.c
>> @@ -3199,6 +3199,65 @@ static const struct camss_subdev_resources 
>> csiphy_res_8775p[] = {
>>       },
>>   };
>> +static const struct camss_subdev_resources tpg_res_8775p[] = {
>> +    /* TPG0 */
>> +    {
>> +        .regulators = {  },
>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
> Why should TPG need camnoc_rt_axi clk?

As tested ,TPG can`t streaming without camnoc_rt_axi clk.
For Pixel path, some platform can stream without camnoc_rt_axi clk but 
TPG not.

>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "tpg0" },
>> +        .interrupt = { "tpg0" },
>> +        .tpg = {
>> +            .lane_cnt = 4,
>> +            .vc_cnt = 1,
>> +            .formats = &tpg_formats_gen1,
>> +            .hw_ops = &tpg_ops_gen1
>> +        }
>> +    },
>> +
>> +    /* TPG1 */
>> +    {
>> +        .regulators = {  },
>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "tpg1" },
>> +        .interrupt = { "tpg1" },
>> +        .tpg = {
>> +            .lane_cnt = 4,
>> +            .vc_cnt = 1,
>> +            .formats = &tpg_formats_gen1,
>> +            .hw_ops = &tpg_ops_gen1
>> +        }
>> +    },
>> +
>> +    /* TPG2 */
>> +    {
>> +        .regulators = {  },
>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "tpg2" },
>> +        .interrupt = { "tpg2" }, + .tpg = { + .lane_cnt = 4, 
>> + .vc_cnt = 1, + .formats = &tpg_formats_gen1, + .hw_ops = 
>> &tpg_ops_gen1 + } + }, +}; + static const struct 
>> camss_subdev_resources csid_res_8775p[] = { /* CSID0 */ { @@ -3595,6 
>> +3654,62 @@ static const struct camss_subdev_resources 
>> csiphy_res_x1e80100[] = { }, }; +static const struct 
>> camss_subdev_resources tpg_res_x1e80100[] = { + /* TPG0 */ + 
>> { + .regulators = { }, + .clock = { "camnoc_rt_axi", "cpas_ahb", 
>> "csid_csiphy_rx" },
>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "csitpg0" },
>> +        .tpg = {
>> +            .lane_cnt = 4,
>> +            .vc_cnt = 1,
>> +            .formats = &tpg_formats_gen1,
>> +            .hw_ops = &tpg_ops_gen1
>> +        }
>> +    },
>> +
>> +    /* TPG1 */
>> +    {
>> +        .regulators = {  },
>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "csitpg1" },
>> +        .tpg = {
>> +            .lane_cnt = 4,
>> +            .vc_cnt = 1,
>> +            .formats = &tpg_formats_gen1,
>> +            .hw_ops = &tpg_ops_gen1
>> +        }
>> +    },
>> +
>> +    /* TPG2 */
>> +    {
>> +        .regulators = {  },
>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
>> +        .clock_rate = {
>> +            { 400000000 },
>> +            { 0 },
>> +            { 400000000 },
>> +        },
>> +        .reg = { "csitpg2" },
>> +        .tpg = {
>> +            .lane_cnt = 4,
>> +            .vc_cnt = 1,
>> +            .formats = &tpg_formats_gen1,
>> +            .hw_ops = &tpg_ops_gen1
>> +        }
>> +    },
>> +};
>> +
>>   static const struct camss_subdev_resources csid_res_x1e80100[] = {
>>       /* CSID0 */
>>       {
>> @@ -4674,6 +4789,13 @@ static int camss_probe(struct platform_device 
>> *pdev)
>>       if (!camss->csiphy)
>>           return -ENOMEM;
>> +    if (camss->res->tpg_num > 0) {
>> +        camss->tpg = devm_kcalloc(dev, camss->res->tpg_num,
>> +                      sizeof(*camss->tpg), GFP_KERNEL);
>> +        if (!camss->tpg)
>> +            return -ENOMEM;
>> +    }
>> +
>>       camss->csid = devm_kcalloc(dev, camss->res->csid_num, 
>> sizeof(*camss->csid),
>>                      GFP_KERNEL);
>>       if (!camss->csid)
>> @@ -4863,11 +4985,13 @@ static const struct camss_resources 
>> qcs8300_resources = {
>>       .version = CAMSS_8300,
>>       .pd_name = "top", .csiphy_res = csiphy_res_8300, + .tpg_res = 
>> tpg_res_8775p, .csid_res = csid_res_8775p, .csid_wrapper_res = 
>> &csid_wrapper_res_sm8550, .vfe_res = vfe_res_8775p, .icc_res = 
>> icc_res_qcs8300, .csiphy_num = ARRAY_SIZE(csiphy_res_8300), + .tpg_num 
>> = ARRAY_SIZE(tpg_res_8775p), .csid_num = 
>> ARRAY_SIZE(csid_res_8775p), .vfe_num = 
>> ARRAY_SIZE(vfe_res_8775p), .icc_path_num = 
>> ARRAY_SIZE(icc_res_qcs8300), @@ -4877,11 +5001,13 @@ static const 
>> struct camss_resources sa8775p_resources = { .version = 
>> CAMSS_8775P, .pd_name = "top", .csiphy_res = csiphy_res_8775p, 
>> + .tpg_res = tpg_res_8775p, .csid_res = 
>> csid_res_8775p, .csid_wrapper_res = &csid_wrapper_res_sm8550, .vfe_res 
>> = vfe_res_8775p, .icc_res = icc_res_sa8775p, .csiphy_num = 
>> ARRAY_SIZE(csiphy_res_8775p), + .tpg_num = 
>> ARRAY_SIZE(tpg_res_8775p), .csid_num = 
>> ARRAY_SIZE(csid_res_8775p), .vfe_num = 
>> ARRAY_SIZE(vfe_res_8775p), .icc_path_num = 
>> ARRAY_SIZE(icc_res_sa8775p), @@ -4992,11 +5118,13 @@ static const 
>> struct camss_resources x1e80100_resources = { .pd_name = "top",
>>       .csiphy_res = csiphy_res_x1e80100,
>>       .csid_res = csid_res_x1e80100,
>> +    .tpg_res = tpg_res_x1e80100,
>>       .vfe_res = vfe_res_x1e80100,
>>       .csid_wrapper_res = &csid_wrapper_res_x1e80100,
>>       .icc_res = icc_res_x1e80100,
>>       .icc_path_num = ARRAY_SIZE(icc_res_x1e80100),
>>       .csiphy_num = ARRAY_SIZE(csiphy_res_x1e80100),
>> +    .tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
>>       .csid_num = ARRAY_SIZE(csid_res_x1e80100),
>>       .vfe_num = ARRAY_SIZE(vfe_res_x1e80100),
>>   };
> 
> Thanks,
> 
> Vijay.
> 

Thanks,
Wenmeng
Re: [PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
Posted by Vijay Kumar Tumati 2 weeks, 4 days ago

On 1/18/2026 7:29 PM, Wenmeng Liu wrote:
> 
> Hi Vijay,
> 
> On 1/17/2026 3:32 AM, Vijay Kumar Tumati wrote:
>> Hi Wenmeng,
>>
>> On 1/13/2026 1:03 AM, Wenmeng Liu wrote:
>>> Add support for TPG found on LeMans, Monaco, Hamoa.
>>>
>>> Signed-off-by: Wenmeng Liu<wenmeng.liu@oss.qualcomm.com>
>>> ---
>>>   drivers/media/platform/qcom/camss/Makefile         |   1 +
>>>   drivers/media/platform/qcom/camss/camss-csid-680.c |  14 ++
>>>   .../media/platform/qcom/camss/camss-csid-gen3.c    |  14 ++
>>>   drivers/media/platform/qcom/camss/camss-tpg-gen1.c | 257 ++++++++++ 
>>> + ++++++++++
>>>   drivers/media/platform/qcom/camss/camss.c          | 128 ++++++++++
>>>   5 files changed, 414 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/ 
>>> media/platform/qcom/camss/Makefile
>>> index 
>>> d355e67c25700ac061b878543c32ed8defc03ad0..e8996dacf1771d13ec1936c9bebc0e71566898ef 100644
>>> --- a/drivers/media/platform/qcom/camss/Makefile
>>> +++ b/drivers/media/platform/qcom/camss/Makefile
>>> @@ -28,5 +28,6 @@ qcom-camss-objs += \
>>>           camss-video.o \
>>>           camss-format.o \
>>>           camss-tpg.o \
>>> +        camss-tpg-gen1.o \
>>>   obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
>>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/ 
>>> drivers/media/platform/qcom/camss/camss-csid-680.c
>>> index 
>>> 3ad3a174bcfb8c0d319930d0010df92308cb5ae4..a5da35cae2eb9acf642795c0a91db58d845f211c 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
>>> @@ -103,6 +103,8 @@
>>>   #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_NUM_EN                BIT(27)
>>> +#define        CSI2_RX_CFG0_TPG_NUM_SEL            GENMASK(29, 28)
>>>   #define CSID_CSI2_RX_CFG1                    0x204
>>>   #define        CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN        BIT(0)
>>> @@ -185,11 +187,23 @@ static void __csid_configure_rx(struct 
>>> csid_device *csid,
>>>                   struct csid_phy_config *phy, int vc)
>>>   {
>>>       u32 val;
>>> +    struct camss *camss;
>>> +    struct tpg_device *tpg;
>>> +    camss = csid->camss;
>>>       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;
>> "phy_num_sel" and "tpg_num_sel" can be in if-else. They both are not 
>> required at once.
> 
> ACK
> 
>>> +    if (camss->tpg) {
>>> +        tpg = &camss->tpg[phy->csiphy_id];
>>> +
>>> +        if (csid->tpg_linked && tpg->testgen.mode > 0) {
>> If the tpg is linked and the mode is not valid, shouldn't you be 
>> throwing error?
> 
> ACK
>>> +            val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy- 
>>> >csiphy_id + 1);
>>> +            val |= CSI2_RX_CFG0_TPG_NUM_EN;
>> Can we rename this to CSI2_RX_CFG0_TPG_MUX_EN?
> 
> ACK
> 
>>> +        }
>>> +    }
>>> +
>>>       writel(val, csid->base + CSID_CSI2_RX_CFG0);
>>>       val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
>>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/ 
>>> drivers/media/platform/qcom/camss/camss-csid-gen3.c
>>> index 
>>> 664245cf6eb0cac662b02f8b920cd1c72db0aeb2..5f9eb533723f2864df64fd6c63e2682fed4a12ae 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>>> @@ -66,6 +66,8 @@
>>>   #define        CSI2_RX_CFG0_VC_MODE        3
>>>   #define        CSI2_RX_CFG0_DL0_INPUT_SEL    4
>>>   #define        CSI2_RX_CFG0_PHY_NUM_SEL    20
>>> +#define        CSI2_RX_CFG0_TPG_NUM_EN        BIT(27)
>>> +#define        CSI2_RX_CFG0_TPG_NUM_SEL    GENMASK(29, 28)
>>>   #define CSID_CSI2_RX_CFG1        0x204
>>>   #define        CSI2_RX_CFG1_ECC_CORRECTION_EN    BIT(0)
>>> @@ -109,11 +111,23 @@ static void __csid_configure_rx(struct 
>>> csid_device *csid,
>>>                   struct csid_phy_config *phy, int vc)
>> Same as above.
> 
> ACK
> 
>>>   {
>>>       int val;
>>> +    struct camss *camss;
>>> +    struct tpg_device *tpg;
>>> +    camss = csid->camss;
>>>       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;
>>> +    if (camss->tpg) {
>>> +        tpg = &camss->tpg[phy->csiphy_id];
>>> +
>>> +        if (csid->tpg_linked && tpg->testgen.mode > 0) {
>>> +            val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy- 
>>> >csiphy_id + 1);
>>> +            val |= CSI2_RX_CFG0_TPG_NUM_EN;
>>> +        }
>>> +    }
>>> +
>>>       writel(val, csid->base + CSID_CSI2_RX_CFG0);
>>>       val = CSI2_RX_CFG1_ECC_CORRECTION_EN;
>>> diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/ 
>>> drivers/media/platform/qcom/camss/camss-tpg-gen1.c
>>> new file mode 100644
>>> index 
>>> 0000000000000000000000000000000000000000..d7ef7a1709648406dc59c210d355851397980769
>>> --- /dev/null
>>> +++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
>>> @@ -0,0 +1,257 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + *
>>> + * Qualcomm MSM Camera Subsystem - TPG (Test Patter Generator) Module
>>> + *
>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +#include <linux/bitfield.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/of.h>
>>> +
>>> +#include "camss-tpg.h"
>>> +#include "camss.h"
>>> +
>>> +#define TPG_HW_VERSION        0x0
>>> +# define HW_VERSION_STEPPING        GENMASK(15, 0)
>>> +# define HW_VERSION_REVISION        GENMASK(27, 16)
>>> +# define HW_VERSION_GENERATION        GENMASK(31, 28)
>>> +
>>> +#define TPG_HW_VER(gen, rev, step) \
>>> +    (((u32)(gen) << 28) | ((u32)(rev) << 16) | (u32)(step))
>>> +
>>> +#define TPG_HW_VER_2_0_0                TPG_HW_VER(2, 0, 0)
>>> +#define TPG_HW_VER_2_1_0                TPG_HW_VER(2, 1, 0)
>>> +
>>> +#define TPG_HW_STATUS        0x4
>>> +
>>> +#define TPG_VC_n_GAIN_CFG(n)        (0x60 + (n) * 0x60)
>> I know why this is here but it may be is better to group this with VC 
>> based registers. In fact, can you please segregate these macros into 
>> sub sections with headings like "TPG global registers", "TPG VC based 
>> registers", "TPG DT based registers" etc. Just for better readability.
> 
> ACK, registers are arranged according to their addresses, and I will 
> mark the global registers.
> 
>>> +
>>> +#define TPG_CTRL        0x64
>>> +# define TPG_CTRL_TEST_EN        BIT(0)
>>> +# define TPG_CTRL_PHY_SEL        BIT(3)
>>> +# define TPG_CTRL_NUM_ACTIVE_LANES    GENMASK(5, 4)
>>> +# define TPG_CTRL_VC_DT_PATTERN_ID    GENMASK(8, 6)
>>> +# define TPG_CTRL_OVERLAP_SHDR_EN    BIT(10)
>>> +# define TPG_CTRL_NUM_ACTIVE_VC        GENMASK(31, 30)
>>> +#  define NUM_ACTIVE_VC_0_ENABLED        0
>>> +#  define NUM_ACTIVE_VC_0_1_ENABLED        1
>>> +#  define NUM_ACTIVE_VC_0_1_2_ENABLED        2
>>> +#  define NUM_ACTIVE_VC_0_1_3_ENABLED        3
>>> +
>>> +#define TPG_VC_n_CFG0(n)    (0x68 + (n) * 0x60)
>>> +# define TPG_VC_n_CFG0_VC_NUM            GENMASK(4, 0)
>>> +# define TPG_VC_n_CFG0_NUM_ACTIVE_DT        GENMASK(9, 8)
>>> +#  define NUM_ACTIVE_SLOTS_0_ENABLED            0
>>> +#  define NUM_ACTIVE_SLOTS_0_1_ENABLED            1
>>> +#  define NUM_ACTIVE_SLOTS_0_1_2_ENABLED        2
>>> +#  define NUM_ACTIVE_SLOTS_0_1_3_ENABLED        3
>> s/NUM_ACTIVE_SLOTS/DT/?, if you really need these macros. Similarly 
>> for VCs enabled.
> 
> ACK
> 
>>> +# define TPG_VC_n_CFG0_NUM_BATCH        GENMASK(15, 12)
>>> +# define TPG_VC_n_CFG0_NUM_FRAMES        GENMASK(31, 16)
>>> +
>>> +#define TPG_VC_n_LSFR_SEED(n)    (0x6C + (n) * 0x60)
>>> +
>>> +#define TPG_VC_n_HBI_CFG(n)    (0x70 + (n) * 0x60)
>>> +
>>> +#define TPG_VC_n_VBI_CFG(n)    (0x74 + (n) * 0x60)
>>> +
>>> +#define TPG_VC_n_COLOR_BARS_CFG(n)        (0x78 + (n) * 0x60)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_PIX_PATTERN        GENMASK(2, 0)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_QCFA_EN        BIT(3)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_SPLIT_EN        BIT(4)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_NOISE_EN        BIT(5)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD        GENMASK(13, 8)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_XCFA_EN        BIT(16)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_X            GENMASK(26, 24)
>>> +# define TPG_VC_n_COLOR_BARS_CFG_SIZE_Y            GENMASK(30, 28)
>>> +
>>> +#define TPG_VC_m_DT_n_CFG_0(m, n)        (0x7C + (m) * 0x60 + (n) * 
>>> 0xC)
>>> +# define TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT    GENMASK(15, 0)
>>> +# define TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH    GENMASK(31, 16)
>>> +
>>> +#define TPG_VC_m_DT_n_CFG_1(m, n)        (0x80 + (m) * 0x60 + (n) * 
>>> 0xC)
>>> +# define TPG_VC_m_DT_n_CFG_1_DATA_TYPE        GENMASK(5, 0)
>>> +# define TPG_VC_m_DT_n_CFG_1_ECC_XOR_MASK    GENMASK(13, 8)
>>> +# define TPG_VC_m_DT_n_CFG_1_CRC_XOR_MASK    GENMASK(31, 16)
>>> +
>>> +#define TPG_VC_m_DT_n_CFG_2(m, n)        (0x84 + (m) * 0x60 + (n) * 
>>> 0xC)
>>> +# define TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE        GENMASK(3, 0)
>>> +/* v2.0.0: USER[19:4], ENC[23:20] */
>>> +# define TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD GENMASK(19, 4)
>>> +# define TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT            GENMASK(23, 
>>> 20)
>> For better readability, can you make these TPG_V2_0_*?
>>> +/* v2.1.0: USER[27:4], ENC[31:28] */
>>> +# define TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD GENMASK(27, 4)
>>> +# define TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT GENMASK(31, 28)
>>> +
>>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR0(n)    (0xB0 + (n) * 0x60)
>>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR1(n)    (0xB4 + (n) * 0x60)
>>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR2(n)    (0xB8 + (n) * 0x60)
>>> +#define TPG_VC_n_COLOR_BAR_CFA_COLOR3(n)    (0xBC + (n) * 0x60)
>>> +
>>> +/* Line offset between VC(n) and VC(n-1), n form 1 to 3 */
>>> +#define TPG_VC_n_SHDR_CFG    (0x84 + (n) * 0x60)
>>> +
>>> +#define TPG_CLEAR        0x1F4
>>> +
>>> +#define TPG_HBI_PCT_DEFAULT            545    /* 545% */
>>> +#define TPG_VBI_PCT_DEFAULT            10    /* 10% */
>>> +#define PERCENT_BASE                100
>>> +#define BITS_PER_BYTE                8
>>> +
>>> +/* Default user-specified payload for TPG test generator.
>>> + * Keep consistent with CSID TPG default: 0xBE.
>>> + */
>>> +#define TPG_USER_SPECIFIED_PAYLOAD_DEFAULT    0xBE
>>> +#define TPG_LFSR_SEED_DEFAULT            0x12345678
>>> +#define TPG_COLOR_BARS_CFG_STANDARD \
>>> +    FIELD_PREP(TPG_VC_n_COLOR_BARS_CFG_ROTATE_PERIOD, 0xA)
>>> +
>>> +static int tpg_stream_on(struct tpg_device *tpg)
>> Add function headers? For this  and a few other below.
> 
> I received a comment asking me to remove the comments,
> if a function is not called in multiple places or its purpose cannot be 
> directly understood, adding extra documentation is useless.
> 
>>> +{
>>> +    struct tpg_testgen_config *tg = &tpg->testgen;
>>> +    struct v4l2_mbus_framefmt *input_format;
>>> +    const struct tpg_format_info *format;
>>> +    u8 lane_cnt = tpg->res->lane_cnt;
>>> +    u8 dt_cnt = 0;
>>> +    u8 i;
>>> +    u32 val;
>>> +
>>> +    /* Loop through all enabled VCs and configure stream for each */
>>> +    for (i = 0; i < tpg->res->vc_cnt; i++) {
>> Here as well, can we segregate the code to global, VC based and DT 
>> based configs with some comments?
> 
> This loop all contains configurations related to VC/DT.
> Will add some comments for this.
> 
>>> +        input_format = &tpg->fmt[MSM_TPG_PAD_SRC + i];
>>> +        format = tpg_get_fmt_entry(tpg,
>>> +                       tpg->res->formats->formats,
>>> +                       tpg->res->formats->nformats,
>>> +                       input_format->code);
>>> +        if (IS_ERR(format))
>>> +            return -EINVAL;
>>> +
>>> +        val = FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_HEIGHT, 
>>> input_format->height & 0xffff) |
>>> +              FIELD_PREP(TPG_VC_m_DT_n_CFG_0_FRAME_WIDTH, 
>>> input_format->width & 0xffff);
>>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_0(i, dt_cnt));
>>> +
>>> +        val = FIELD_PREP(TPG_VC_m_DT_n_CFG_1_DATA_TYPE, format- 
>>> >data_type);
>>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_1(i, dt_cnt));
>>> +
>>> +        if (tpg->hw_version == TPG_HW_VER_2_0_0) {
>>> +            val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg- 
>>> >mode - 1) |
>>> + FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
>>> +                       TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
>>> +                FIELD_PREP(TPG_V2_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
>>> +                       format->encode_format);
>>> +        } else if (tpg->hw_version >= TPG_HW_VER_2_1_0) {
>>> +            val = FIELD_PREP(TPG_VC_m_DT_n_CFG_2_PAYLOAD_MODE, tg- 
>>> >mode - 1) |
>>> + FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_USER_SPECIFIED_PAYLOAD,
>>> +                       TPG_USER_SPECIFIED_PAYLOAD_DEFAULT) |
>>> +                FIELD_PREP(TPG_V2_1_VC_m_DT_n_CFG_2_ENCODE_FORMAT,
>>> +                       format->encode_format);
>>> +        }
>>> +        writel(val, tpg->base + TPG_VC_m_DT_n_CFG_2(i, dt_cnt));
>>> +
>>> +        writel(TPG_COLOR_BARS_CFG_STANDARD, tpg->base + 
>>> TPG_VC_n_COLOR_BARS_CFG(i));
>>> +
>>> +        val = DIV_ROUND_UP(input_format->width * format->bpp * 
>>> TPG_HBI_PCT_DEFAULT,
>>> +                   BITS_PER_BYTE * lane_cnt * PERCENT_BASE);
>>> +        writel(val, tpg->base + TPG_VC_n_HBI_CFG(i));
>>> +        val = input_format->height * TPG_VBI_PCT_DEFAULT / 
>>> PERCENT_BASE;
>>> +        writel(val, tpg->base + TPG_VC_n_VBI_CFG(i));
>>> +
>>> +        writel(TPG_LFSR_SEED_DEFAULT, tpg->base + 
>>> TPG_VC_n_LSFR_SEED(i));
>>> +
>>> +        /* configure one DT, infinite frames */
>> Although this driver is not supporting more than one DT in a VC right 
>> now, is there a way we can make the API generic enough to receive #DTs 
>> in each VS and their dimensions?
> 
> ACK
> 
>>> +        val = FIELD_PREP(TPG_VC_n_CFG0_VC_NUM, i) |
>>> +              FIELD_PREP(TPG_VC_n_CFG0_NUM_FRAMES, 0);
>>> +        writel(val, tpg->base + TPG_VC_n_CFG0(i));
>>> +    }
>>> +
>>> +    val = FIELD_PREP(TPG_CTRL_TEST_EN, 1) |
>>> +          FIELD_PREP(TPG_CTRL_PHY_SEL, 0) |
>> Same here, is there a way to make the API generic to receive CPHY / 
>> DPHY mode required?
> 
> ACK
> 
>>> +          FIELD_PREP(TPG_CTRL_NUM_ACTIVE_LANES, lane_cnt - 1) |
>>> +          FIELD_PREP(TPG_CTRL_VC_DT_PATTERN_ID, 0) |
>> You are assuming frame interleaved mode always. It may be is a good 
>> start but a bunch of functionality is missing here. Just please think 
>> of the scalability of the API even though the driver support is 
>> limited at this point.
> 
> ACK
> 
>>> +          FIELD_PREP(TPG_CTRL_NUM_ACTIVE_VC, tpg->res->vc_cnt - 1);
>>> +    writel(val, tpg->base + TPG_CTRL);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void tpg_stream_off(struct tpg_device *tpg)
>>> +{
>>> +    writel(0, tpg->base + TPG_CTRL);
>>> +    writel(1, tpg->base + TPG_CLEAR);
>> Why not just reuse the reset function?
> 
> ACK
> 
>>> +}
>>> +
>>> +static int tpg_configure_stream(struct tpg_device *tpg, u8 enable)
>>> +{
>>> +    int ret = 0;
>>> +
>>> +    if (enable)
>>> +        ret = tpg_stream_on(tpg);
>>> +    else
>>> +        tpg_stream_off(tpg);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int tpg_configure_testgen_pattern(struct tpg_device *tpg, s32 
>>> val)
>>> +{
>>> +    if (val >= 0 && val <= TPG_PAYLOAD_MODE_COLOR_BARS)
>>> +        tpg->testgen.mode = val;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/*
>>> + * tpg_hw_version - tpg hardware version query
>>> + * @tpg: tpg device
>>> + *
>>> + * Return HW version or error
>>> + */
>>> +static u32 tpg_hw_version(struct tpg_device *tpg)
>>> +{
>>> +    u32 hw_version;
>>> +    u32 hw_gen;
>>> +    u32 hw_rev;
>>> +    u32 hw_step;
>>> +
>>> +    hw_version = readl(tpg->base + TPG_HW_VERSION);
>>> +    hw_gen = FIELD_GET(HW_VERSION_GENERATION, hw_version);
>>> +    hw_rev = FIELD_GET(HW_VERSION_REVISION, hw_version);
>>> +    hw_step = FIELD_GET(HW_VERSION_STEPPING, hw_version);
>>> +
>>> +    tpg->hw_version = hw_version;
>>> +
>>> +    dev_dbg_once(tpg->camss->dev, "tpg HW Version = %u.%u.%u\n",
>>> +             hw_gen, hw_rev, hw_step);
>>> +
>>> +    return hw_version;
>>> +}
>>> +
>>> +/*
>>> + * tpg_reset - Trigger reset on tpg module and wait to complete
>> Doesn't seem like there is any wait here, right?
> 
> Do you have any suggestions on this? I noticed that there is no delay 
> downstream either.
> 
>> Also, do you want to clear the IRQs in reset?
> 
> Following the maintainer's suggestion, I removed IRQ support, but we do 
> need to add IRQ clearing.
> 
>>> + * @tpg: tpg device
>>> + *
>>> + * Return 0 on success or a negative error code otherwise
>>> + */
>>> +static int tpg_reset(struct tpg_device *tpg)
>>> +{
>>> +    writel(0, tpg->base + TPG_CTRL);
>>> +    writel(1, tpg->base + TPG_CLEAR);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void tpg_subdev_init(struct tpg_device *tpg)
>>> +{
>>> +    tpg->testgen.modes = testgen_payload_modes;
>>> +    tpg->testgen.nmodes = TPG_PAYLOAD_MODE_NUM_SUPPORTED_GEN1;
>>> +}
>>> +
>>> +const struct tpg_hw_ops tpg_ops_gen1 = {
>>> +    .configure_stream = tpg_configure_stream,
>>> +    .configure_testgen_pattern = tpg_configure_testgen_pattern,
>>> +    .hw_version = tpg_hw_version,
>>> +    .reset = tpg_reset,
>>> +    .subdev_init = tpg_subdev_init,
>>> +};
>>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/ 
>>> media/platform/qcom/camss/camss.c
>>> index 
>>> 43fdcb9af101ef34b118035ca9c68757b66118df..5cddf1bc09f97c2c61f907939bb54663d8eab3d4 100644
>>> --- a/drivers/media/platform/qcom/camss/camss.c
>>> +++ b/drivers/media/platform/qcom/camss/camss.c
>>> @@ -3199,6 +3199,65 @@ static const struct camss_subdev_resources 
>>> csiphy_res_8775p[] = {
>>>       },
>>>   };
>>> +static const struct camss_subdev_resources tpg_res_8775p[] = {
>>> +    /* TPG0 */
>>> +    {
>>> +        .regulators = {  },
>>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
>> Why should TPG need camnoc_rt_axi clk?
> 
> As tested ,TPG can`t streaming without camnoc_rt_axi clk.
> For Pixel path, some platform can stream without camnoc_rt_axi clk but 
> TPG not.
> 
It is not the expected behavior. There is no path from TPG to CAMNOC / 
DDR. TPG only drives the pattern data from the internal engine to CSID 
and you will only need CAMNOC_RT_AXI clock when the data is written from 
IFE WM to the DDR over CAMNOC, which anyway will be enabled from the IFE 
device. We can check this further together. Thanks.
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "tpg0" },
>>> +        .interrupt = { "tpg0" },
>>> +        .tpg = {
>>> +            .lane_cnt = 4,
>>> +            .vc_cnt = 1,
>>> +            .formats = &tpg_formats_gen1,
>>> +            .hw_ops = &tpg_ops_gen1
>>> +        }
>>> +    },
>>> +
>>> +    /* TPG1 */
>>> +    {
>>> +        .regulators = {  },
>>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "tpg1" },
>>> +        .interrupt = { "tpg1" },
>>> +        .tpg = {
>>> +            .lane_cnt = 4,
>>> +            .vc_cnt = 1,
>>> +            .formats = &tpg_formats_gen1,
>>> +            .hw_ops = &tpg_ops_gen1
>>> +        }
>>> +    },
>>> +
>>> +    /* TPG2 */
>>> +    {
>>> +        .regulators = {  },
>>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csiphy_rx" },
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "tpg2" },
>>> +        .interrupt = { "tpg2" }, + .tpg = { + .lane_cnt = 4, 
>>> + .vc_cnt = 1, + .formats = &tpg_formats_gen1, + .hw_ops = 
>>> &tpg_ops_gen1 + } + }, +}; + static const struct 
>>> camss_subdev_resources csid_res_8775p[] = { /* CSID0 */ { @@ -3595,6 
>>> +3654,62 @@ static const struct camss_subdev_resources 
>>> csiphy_res_x1e80100[] = { }, }; +static const struct 
>>> camss_subdev_resources tpg_res_x1e80100[] = { + /* TPG0 */ + 
>>> { + .regulators = { }, + .clock = { "camnoc_rt_axi", "cpas_ahb", 
>>> "csid_csiphy_rx" },
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "csitpg0" },
>>> +        .tpg = {
>>> +            .lane_cnt = 4,
>>> +            .vc_cnt = 1,
>>> +            .formats = &tpg_formats_gen1,
>>> +            .hw_ops = &tpg_ops_gen1
>>> +        }
>>> +    },
>>> +
>>> +    /* TPG1 */
>>> +    {
>>> +        .regulators = {  },
>>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "csitpg1" },
>>> +        .tpg = {
>>> +            .lane_cnt = 4,
>>> +            .vc_cnt = 1,
>>> +            .formats = &tpg_formats_gen1,
>>> +            .hw_ops = &tpg_ops_gen1
>>> +        }
>>> +    },
>>> +
>>> +    /* TPG2 */
>>> +    {
>>> +        .regulators = {  },
>>> +        .clock = { "camnoc_rt_axi", "cpas_ahb", "csid_csiphy_rx" },
>>> +        .clock_rate = {
>>> +            { 400000000 },
>>> +            { 0 },
>>> +            { 400000000 },
>>> +        },
>>> +        .reg = { "csitpg2" },
>>> +        .tpg = {
>>> +            .lane_cnt = 4,
>>> +            .vc_cnt = 1,
>>> +            .formats = &tpg_formats_gen1,
>>> +            .hw_ops = &tpg_ops_gen1
>>> +        }
>>> +    },
>>> +};
>>> +
>>>   static const struct camss_subdev_resources csid_res_x1e80100[] = {
>>>       /* CSID0 */
>>>       {
>>> @@ -4674,6 +4789,13 @@ static int camss_probe(struct platform_device 
>>> *pdev)
>>>       if (!camss->csiphy)
>>>           return -ENOMEM;
>>> +    if (camss->res->tpg_num > 0) {
>>> +        camss->tpg = devm_kcalloc(dev, camss->res->tpg_num,
>>> +                      sizeof(*camss->tpg), GFP_KERNEL);
>>> +        if (!camss->tpg)
>>> +            return -ENOMEM;
>>> +    }
>>> +
>>>       camss->csid = devm_kcalloc(dev, camss->res->csid_num, 
>>> sizeof(*camss->csid),
>>>                      GFP_KERNEL);
>>>       if (!camss->csid)
>>> @@ -4863,11 +4985,13 @@ static const struct camss_resources 
>>> qcs8300_resources = {
>>>       .version = CAMSS_8300,
>>>       .pd_name = "top", .csiphy_res = csiphy_res_8300, + .tpg_res = 
>>> tpg_res_8775p, .csid_res = csid_res_8775p, .csid_wrapper_res = 
>>> &csid_wrapper_res_sm8550, .vfe_res = vfe_res_8775p, .icc_res = 
>>> icc_res_qcs8300, .csiphy_num = ARRAY_SIZE(csiphy_res_8300), 
>>> + .tpg_num = ARRAY_SIZE(tpg_res_8775p), .csid_num = 
>>> ARRAY_SIZE(csid_res_8775p), .vfe_num = 
>>> ARRAY_SIZE(vfe_res_8775p), .icc_path_num = 
>>> ARRAY_SIZE(icc_res_qcs8300), @@ -4877,11 +5001,13 @@ static const 
>>> struct camss_resources sa8775p_resources = { .version = 
>>> CAMSS_8775P, .pd_name = "top", .csiphy_res = csiphy_res_8775p, 
>>> + .tpg_res = tpg_res_8775p, .csid_res = 
>>> csid_res_8775p, .csid_wrapper_res = 
>>> &csid_wrapper_res_sm8550, .vfe_res = vfe_res_8775p, .icc_res = 
>>> icc_res_sa8775p, .csiphy_num = ARRAY_SIZE(csiphy_res_8775p), 
>>> + .tpg_num = ARRAY_SIZE(tpg_res_8775p), .csid_num = 
>>> ARRAY_SIZE(csid_res_8775p), .vfe_num = 
>>> ARRAY_SIZE(vfe_res_8775p), .icc_path_num = 
>>> ARRAY_SIZE(icc_res_sa8775p), @@ -4992,11 +5118,13 @@ static const 
>>> struct camss_resources x1e80100_resources = { .pd_name = "top",
>>>       .csiphy_res = csiphy_res_x1e80100,
>>>       .csid_res = csid_res_x1e80100,
>>> +    .tpg_res = tpg_res_x1e80100,
>>>       .vfe_res = vfe_res_x1e80100,
>>>       .csid_wrapper_res = &csid_wrapper_res_x1e80100,
>>>       .icc_res = icc_res_x1e80100,
>>>       .icc_path_num = ARRAY_SIZE(icc_res_x1e80100),
>>>       .csiphy_num = ARRAY_SIZE(csiphy_res_x1e80100),
>>> +    .tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
>>>       .csid_num = ARRAY_SIZE(csid_res_x1e80100),
>>>       .vfe_num = ARRAY_SIZE(vfe_res_x1e80100),
>>>   };
>>
>> Thanks,
>>
>> Vijay.
>>
> 
> Thanks,
> Wenmeng

Re: [PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
Posted by kernel test robot 3 weeks, 3 days ago
Hi Wenmeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on f417b7ffcbef7d76b0d8860518f50dae0e7e5eda]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenmeng-Liu/media-qcom-camss-Add-common-TPG-support/20260113-171032
base:   f417b7ffcbef7d76b0d8860518f50dae0e7e5eda
patch link:    https://lore.kernel.org/r/20260113-camss_tpg-v8-3-fa2cb186a018%40oss.qualcomm.com
patch subject: [PATCH v8 3/3] media: qcom: camss: tpg: Add TPG support for multiple targets
config: parisc-randconfig-002-20260115 (https://download.01.org/0day-ci/archive/20260115/202601152315.fC7ckH9z-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260115/202601152315.fC7ckH9z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601152315.fC7ckH9z-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/media/platform/qcom/camss/camss-csid-680.c: In function '__csid_configure_rx':
>> drivers/media/platform/qcom/camss/camss-csid-680.c:202:32: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     202 |                         val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
         |                                ^~~~~~~~~~
   cc1: some warnings being treated as errors
--
   drivers/media/platform/qcom/camss/camss-csid-gen3.c: In function '__csid_configure_rx':
>> drivers/media/platform/qcom/camss/camss-csid-gen3.c:126:32: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     126 |                         val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
         |                                ^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/FIELD_PREP +202 drivers/media/platform/qcom/camss/camss-csid-680.c

   185	
   186	static void __csid_configure_rx(struct csid_device *csid,
   187					struct csid_phy_config *phy, int vc)
   188	{
   189		u32 val;
   190		struct camss *camss;
   191		struct tpg_device *tpg;
   192	
   193		camss = csid->camss;
   194		val = (phy->lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
   195		val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
   196		val |= (phy->csiphy_id + CSI2_RX_CFG0_PHY_SEL_BASE_IDX) << CSI2_RX_CFG0_PHY_NUM_SEL;
   197	
   198		if (camss->tpg) {
   199			tpg = &camss->tpg[phy->csiphy_id];
   200	
   201			if (csid->tpg_linked && tpg->testgen.mode > 0) {
 > 202				val |= FIELD_PREP(CSI2_RX_CFG0_TPG_NUM_SEL, phy->csiphy_id + 1);
   203				val |= CSI2_RX_CFG0_TPG_NUM_EN;
   204			}
   205		}
   206	
   207		writel(val, csid->base + CSID_CSI2_RX_CFG0);
   208	
   209		val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
   210		if (vc > 3)
   211			val |= CSI2_RX_CFG1_VC_MODE;
   212		writel(val, csid->base + CSID_CSI2_RX_CFG1);
   213	}
   214	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki