Re: [PATCH 09/13] media: qcom: camss: Add CSID Gen3 support for SM8550
From: Depeng Shao
Date: Thu Jul 11 2024 - 07:17:35 EST
Hi Krzysztof,
On 7/10/2024 7:20 PM, Krzysztof Kozlowski wrote:
On 09/07/2024 18:06, 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 are
moved to CSID gen3. And CSID gen3 has a new register block which
is named as CSID top, it controls the output of CSID, since the
CSID can connect to Sensor Front End (SFE) or original VFE, the
register in top block is used to control the HW connection.
Co-developed-by: Yongsheng Li <quic_yon@xxxxxxxxxxx>
Signed-off-by: Yongsheng Li <quic_yon@xxxxxxxxxxx>
Signed-off-by: Depeng Shao <quic_depengs@xxxxxxxxxxx>
---
drivers/media/platform/qcom/camss/Makefile | 1 +
.../platform/qcom/camss/camss-csid-gen3.c | 445 ++++++++++++++++++
.../platform/qcom/camss/camss-csid-gen3.h | 26 +
.../media/platform/qcom/camss/camss-csid.h | 2 +
4 files changed, 474 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 e636968a1126..c336e4c1a399 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.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..17fd7c5499de
--- /dev/null
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * camss-csid-gen3.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSID (CSI Decoder) Module
+ *
+ * Copyright (c) 2024 Qualcomm Technologies, Inc.
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+#include "camss.h"
+#include "camss-csid.h"
+#include "camss-csid-gen3.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
That's a random set of indentations.
Thanks, will fixes this.
+#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
...
+
+ 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));
+}
+
such patterns and...
+ */
+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);
... here - using everywhere relaxed here is odd and looks racy. These
looks like some strict sequences.
Yes, these are some sequences to initialize the HW.
+
+ 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;
+ }
+
+
+static void csid_subdev_init(struct csid_device *csid)
+{
+ csid->testgen.modes = csid_testgen_modes;
+ csid->testgen.nmodes = CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2;
+}
+
+const struct csid_hw_ops csid_ops_gen3 = {
Isn't there a warning here?
+ .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,
+};
Your patchset does not apply at all. Tried v6.9, v6.10, next. I see some
dependency above, but that means no one can test it and no one can apply it.
Fix the warnings, I cannot verify it but I am sure you have them.
My code base is next master branch, do you mean the 'declared and not
used' warning? I don't see this warning with below two version compiler
even though I just pick this patch and pull the code the latest version.
But it indeed have this issue, these structures are declared and will be
used later in "media: qcom: camss: Add sm8550 resources" patch, will
think about how to avoid this.
aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
aarch64-linux-gnu-gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
---
CALL scripts/checksyscalls.sh
CC [M] drivers/media/platform/qcom/camss/camss-csid.o
CC [M] drivers/media/platform/qcom/camss/camss-csid-4-1.o
CC [M] drivers/media/platform/qcom/camss/camss-csid-4-7.o
CC [M] drivers/media/platform/qcom/camss/camss-csid-gen2.o
CC [M] drivers/media/platform/qcom/camss/camss-csid-gen3.o
CC [M] drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.o
CC [M] drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.o
---
89a4a25b7b56 (HEAD -> master) media: qcom: camss: Add CSID Gen3 support
for SM8550
d0cfd24496d3 media: qcom: camss: csiphy-3ph: Add Gen2 v1.2 two-phase
MIPI CSI-2 DPHY init
5795fd39a8ee dt-bindings: media: camss: Add qcom,sm8550-camss binding
Best regards,
Krzysztof
Thanks,
Depeng