| Message ID | 20241009111551.27052-1-Shu-hsiang.Yang@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add MediaTek ISP7 camera system driver | expand |
Il 09/10/24 13:15, Shu-hsiang Yang ha scritto: > Introduces support for the sensor interface in the MediaTek SoC, > with the focus on CSI and stream control. The key functionalities > include parameter control, metering and maintaining status information, > interrupt handling, and debugging. These features ensure effective > management and debugging of the camera sensor interface hardware. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- > .../isp_7x/camsys/mtk_csi_phy_2_0/Makefile | 5 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h | 911 ++++++ > .../mtk_cam-seninf-csi0-cphy.h | 69 + > .../mtk_cam-seninf-csi0-dphy.h | 139 + > .../mtk_cam-seninf-hw_phy_2_0.c | 2879 +++++++++++++++++ > .../mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h | 257 ++ > .../mtk_cam-seninf-seninf1-csi2.h | 415 +++ > .../mtk_cam-seninf-seninf1-mux.h | 147 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-seninf1.h | 47 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-tg1.h | 49 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-top-ctrl.h | 99 + > 11 files changed, 5017 insertions(+) > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/Makefile The PHY driver should go to the PHY directory as a PHY driver - or at least part of this mtk_csi_phy_2_0 driver should go there. I see that this is tightly integrated with the rest of the code in seninf, but there seems to be many functions that are just handling a "real" PHY. > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-csi0-cphy.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-csi0-dphy.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1-csi2.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1-mux.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-tg1.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-top-ctrl.h > > diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/Makefile b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/Makefile > new file mode 100644 > index 000000000000..e00b8d3904a9 > --- /dev/null > +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2022 MediaTek Inc. > + > +mtk-cam-isp-objs += \ > + mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.o > diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h > new file mode 100644 > index 000000000000..ec3c621d742a > --- /dev/null > +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h > @@ -0,0 +1,911 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2022 MediaTek Inc. > + */ > + > +#ifndef __SENINF_CAM_MUX_H__ > +#define __SENINF_CAM_MUX_H__ > + > +#define SENINF_CAM_MUX_CTRL_0 0x0000 > +#define RG_SENINF_CAM_MUX0_SRC_SEL_SHIFT 0 > +#define RG_SENINF_CAM_MUX0_SRC_SEL_MASK (0xf << 0) For all definitions: drop _SHIFT and use the GENMASK(x,y) macro. #define RG_SENINF_CAM_MUX0_SRC_SEL GENMASK(7, 0) #define RG_SENINF_CAM_MUX1_SRC_SEL GENMASK(11, 8) etc. > +#define RG_SENINF_CAM_MUX1_SRC_SEL_SHIFT 8 > +#define RG_SENINF_CAM_MUX1_SRC_SEL_MASK (0xf << 8) > +#define RG_SENINF_CAM_MUX2_SRC_SEL_SHIFT 16 > +#define RG_SENINF_CAM_MUX2_SRC_SEL_MASK (0xf << 16) > +#define RG_SENINF_CAM_MUX3_SRC_SEL_SHIFT 24 > +#define RG_SENINF_CAM_MUX3_SRC_SEL_MASK (0xf << 24) ..snip.. > diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.c b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.c > new file mode 100644 > index 000000000000..f24d8a056d0e > --- /dev/null > +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.c > @@ -0,0 +1,2879 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2022 MediaTek Inc. > + > +#include <linux/module.h> > +#include <linux/delay.h> > + > +#include "../mtk_cam-seninf.h" > +#include "../mtk_cam-seninf-hw.h" > +#include "../mtk_cam-seninf-regs.h" > +#include "mtk_cam-seninf-top-ctrl.h" > +#include "mtk_cam-seninf-seninf1-mux.h" > +#include "mtk_cam-seninf-seninf1.h" > +#include "mtk_cam-seninf-seninf1-csi2.h" > +#include "mtk_cam-seninf-tg1.h" > +#include "mtk_cam-seninf-cammux.h" > +#include "mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h" > +#include "mtk_cam-seninf-csi0-cphy.h" > +#include "mtk_cam-seninf-csi0-dphy.h" > +#include "../kd_imgsensor_define_v4l2.h" > + > +/* seninf cfg default, dts may override */ dts may override: for which reason? Why should DT override that? For which usecase? > +static struct mtk_cam_seninf_cfg _seninf_cfg = { > + .mux_num = 8, > + .seninf_num = 4, > + .cam_mux_num = 11, > + .pref_mux_num = 11, > +}; > + > +struct mtk_cam_seninf_cfg *g_seninf_cfg = &_seninf_cfg; That's unused. Drop. > + > +static inline void mtk_cam_seninf_set_di_ch_ctrl(void __iomem *pseninf, > + unsigned int stream_id, > + struct seninf_vc *vc) > +{ > + if (stream_id > 7) No magic numbers, please. #define SOMETHING 7 > + return; > + > + SENINF_BITS(pseninf, SENINF_CSI2_S0_DI_CTRL + (stream_id << 0x2), Is it me, or is SENINF_BITS() not defined?! > + RG_CSI2_DT_SEL, vc->dt); > + SENINF_BITS(pseninf, SENINF_CSI2_S0_DI_CTRL + (stream_id << 0x2), > + RG_CSI2_VC_SEL, vc->vc); > + SENINF_BITS(pseninf, SENINF_CSI2_S0_DI_CTRL + (stream_id << 0x2), > + RG_CSI2_DT_INTERLEAVE_MODE, 1); > + SENINF_BITS(pseninf, SENINF_CSI2_S0_DI_CTRL + (stream_id << 0x2), > + RG_CSI2_VC_INTERLEAVE_EN, 1); > + ..snip.. > +} > + > +int mtk_cam_seninf_init_iomem(struct seninf_ctx *ctx, void __iomem *if_base, > + void __iomem *ana_base) > +{ > + u32 i; > + > + ctx->reg_ana_csi_rx[CSI_PORT_0] = Please don't use "a = b = c" assignments. In this case, that impacts on human readability. > + ctx->reg_ana_csi_rx[CSI_PORT_0A] = ana_base + 0; > + ctx->reg_ana_csi_rx[CSI_PORT_0B] = ana_base + 0x1000; Again, no magic numbers please. #define SOMETHING 0x1000 > + > + ctx->reg_ana_csi_rx[CSI_PORT_1] = > + ctx->reg_ana_csi_rx[CSI_PORT_1A] = ana_base + 0x4000; > + ctx->reg_ana_csi_rx[CSI_PORT_1B] = ana_base + 0x5000; ..snip.. > + return 0; > +} > + > +int mtk_cam_seninf_init_port(struct seninf_ctx *ctx, int port) > +{ > + u32 port_num; > + > + if (port >= CSI_PORT_0A) > + port_num = (port - CSI_PORT_0) >> 1; I think that you really want to use bitfield.h macros to simplify this. > + else > + port_num = port; > + > + ctx->port = port; > + ctx->port_num = port_num; > + ctx->port_a = CSI_PORT_0A + (port_num << 1); > + ctx->port_b = ctx->port_a + 1; > + ctx->is_4d1c = (port == port_num); > + > + switch (port) { > + case CSI_PORT_0: > + ctx->seninf_idx = SENINF_1; Is the CSI port to SENINF mapping supposed to be static and unchangeable? If yes, then you want to add that into an array indexed by CSI_PORT_xx, so that you end up doing ctx->seninf_idx = seninf_csi[port]; > + break; > + case CSI_PORT_0A: > + ctx->seninf_idx = SENINF_1; > + break; > + case CSI_PORT_0B: > + ctx->seninf_idx = SENINF_2; > + break; > + case CSI_PORT_1: > + ctx->seninf_idx = SENINF_3; > + break; > + case CSI_PORT_1A: > + ctx->seninf_idx = SENINF_3; > + break; > + case CSI_PORT_1B: > + ctx->seninf_idx = SENINF_4; > + break; > + case CSI_PORT_2: > + ctx->seninf_idx = SENINF_5; > + break; > + case CSI_PORT_2A: > + ctx->seninf_idx = SENINF_5; > + break; > + case CSI_PORT_2B: > + ctx->seninf_idx = SENINF_6; > + break; > + case CSI_PORT_3: > + ctx->seninf_idx = SENINF_7; > + break; > + case CSI_PORT_3A: > + ctx->seninf_idx = SENINF_7; > + break; > + case CSI_PORT_3B: > + ctx->seninf_idx = SENINF_8; > + break; > + default: > + dev_dbg(ctx->dev, "invalid port %d\n", port); > + return -EINVAL; > + } > + > + return 0; > +} > + > +int mtk_cam_seninf_is_cammux_used(struct seninf_ctx *ctx, int cam_mux) > +{ > + void __iomem *seninf_cam_mux = ctx->reg_if_cam_mux; > + u32 temp = SENINF_READ_REG(seninf_cam_mux, SENINF_CAM_MUX_EN); Please, just use readl() or regmap_read() (preferrably the latter, and preferrably also define regmap reg fields). > + > + if (cam_mux >= _seninf_cfg.cam_mux_num) { > + dev_dbg(ctx->dev, > + "%s err cam_mux %d >= SENINF_CAM_MUX_NUM %d\n", > + __func__, cam_mux, _seninf_cfg.cam_mux_num); That shouldn't ever happen, should it. In that case, it's dev_err(), or even a WARN(), if necessary. > + > + return 0; > + } > + > + return !!(temp & (1 << cam_mux)); > +} > + > +int mtk_cam_seninf_cammux(struct seninf_ctx *ctx, int cam_mux) > +{ > + void __iomem *seninf_cam_mux = ctx->reg_if_cam_mux; > + u32 temp; > + > + if (cam_mux >= _seninf_cfg.cam_mux_num) { This check is duplicated across many functions... you want to do something such that it either never happens or it gets commonized. > + dev_dbg(ctx->dev, > + "%s err cam_mux %d >= SENINF_CAM_MUX_NUM %d\n", > + __func__, cam_mux, _seninf_cfg.cam_mux_num); > + > + return 0; You return 0, so there's no problem if this is the wrong cam_mux?!?!? > + } > + > + temp = SENINF_READ_REG(seninf_cam_mux, SENINF_CAM_MUX_EN); > + SENINF_WRITE_REG(seninf_cam_mux, SENINF_CAM_MUX_EN, > + temp | (1 << cam_mux)); BIT(cam_mux) > + > + SENINF_WRITE_REG(seninf_cam_mux, SENINF_CAM_MUX_IRQ_STATUS, > + 3 << (cam_mux * 2)); /* clr irq */ > + > + dev_dbg(ctx->dev, "cam_mux %d EN 0x%x IRQ_EN 0x%x IRQ_STATUS 0x%x\n", > + cam_mux, SENINF_READ_REG(seninf_cam_mux, SENINF_CAM_MUX_EN), > + SENINF_READ_REG(seninf_cam_mux, SENINF_CAM_MUX_IRQ_EN), > + SENINF_READ_REG(seninf_cam_mux, SENINF_CAM_MUX_IRQ_STATUS)); > + > + return 0; > +} > + > + > +int mtk_cam_seninf_set_vc(struct seninf_ctx *ctx, u32 seninf_idx, > + struct seninf_vcinfo *vcinfo) > +{ > + void __iomem *seninf_csi2 = ctx->reg_if_csi2[seninf_idx]; > + int i; > + struct seninf_vc *vc; > + > + if (!vcinfo || !vcinfo->cnt) > + return 0; Does !vcinfo mean that Virtual Channel should not be set? Please add a comment explaining that. > + > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S0_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S1_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S2_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S3_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S4_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S5_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S6_DI_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_S7_DI_CTRL, 0); > + > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_CH0_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_CH1_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_CH2_CTRL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_CH3_CTRL, 0); > + > + for (i = 0; i < vcinfo->cnt; i++) { > + vc = &vcinfo->vc[i]; > + > + /* General Long Packet Data Types: 0x10-0x17 */ > + if (vc->dt >= 0x10 && vc->dt <= 0x17) { > + SENINF_BITS(seninf_csi2, SENINF_CSI2_OPT, > + RG_CSI2_GENERIC_LONG_PACKET_EN, 1); > + } > + > + mtk_cam_seninf_set_di_ch_ctrl(seninf_csi2, i, vc); > + } > + > + dev_dbg(ctx->dev, "DI_CTRL 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n", > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S0_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S1_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S2_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S3_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S4_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S5_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S6_DI_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_S7_DI_CTRL)); > + > + dev_dbg(ctx->dev, "CH_CTRL 0x%x 0x%x 0x%x 0x%x\n", > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_CH0_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_CH1_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_CH2_CTRL), > + SENINF_READ_REG(seninf_csi2, SENINF_CSI2_CH3_CTRL)); > + > + return 0; > +} > + ..snip.. > + > +static int csirx_phyA_power_on(struct seninf_ctx *ctx, u32 port_idx, int en) Please, lower case on all functions. csirx_phy_a_power_on(). Also, you want to split this function in two: one should be csirx_phy_a_power_off() and one for power_on(). static void csirx_phy_a_power_off(struct seninf_ctx *ctx, u32 port_idx) { > +{ > + void __iomem *base = ctx->reg_ana_csi_rx[port_idx]; > + > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_L0_T0AB_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_L1_T1AB_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_L2_T1BC_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_XX_T0BC_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_XX_T0CA_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_8, RG_CSI0_XX_T1CA_EQ_OS_CAL_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, RG_CSI0_BG_LPF_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, RG_CSI0_BG_CORE_EN, 0); > + usleep_range(200, 300); > + } static int csirx_phy_a_power_on(struct seninf_ctx *ctx, u32 port_idx) <-- NO int en { void *iomem base = ..... /* Power it off first to ... reset, I believe? */ csirx_phy_a_power_off(); power on sequence here return 0; } > + if (en) { > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_BG_CORE_EN, 1); > + usleep_range(30, 40); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_BG_LPF_EN, 1); > + udelay(1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_L0_T0AB_EQ_OS_CAL_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_L1_T1AB_EQ_OS_CAL_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_L2_T1BC_EQ_OS_CAL_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_XX_T0BC_EQ_OS_CAL_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_XX_T0CA_EQ_OS_CAL_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_8, > + RG_CSI0_XX_T1CA_EQ_OS_CAL_EN, 1); > + udelay(1); > + } > + > + return 0; > +} > + > +static int apply_efuse_data(struct seninf_ctx *ctx) > +{ void __iomem *base; u32 m_csi_efuse = ctx->m_csi_efuse; u32 port; int ret; > + int ret = 0; > + void __iomem *base; > + u32 port; > + u32 m_csi_efuse = ctx->m_csi_efuse; > + > + if (m_csi_efuse == 0) { > + dev_dbg(ctx->dev, "No efuse data. Returned.\n"); > + return -1; > + } ..snip.. > + > + return ret; return 0; > +} > + > +static int csirx_phyA_init(struct seninf_ctx *ctx) > +{ > + u32 i, port; > + void __iomem *base; > + > + port = ctx->port; > + for (i = 0; i <= ctx->is_4d1c; i++) { Move the setup in a different function, then call it with something(port); if (is_4d1c) something(port_b); > + port = i ? ctx->port_b : ctx->port; > + base = ctx->reg_ana_csi_rx[port]; > + SENINF_BITS(base, CDPHY_RX_ANA_1, > + RG_CSI0_BG_LPRX_VTL_SEL, 0x4); > + SENINF_BITS(base, CDPHY_RX_ANA_1, > + RG_CSI0_BG_LPRX_VTH_SEL, 0x4); > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_BG_ALP_RX_VTL_SEL, 0x4); > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_BG_ALP_RX_VTH_SEL, 0x4); > + SENINF_BITS(base, CDPHY_RX_ANA_1, > + RG_CSI0_BG_VREF_SEL, 0x8); > + SENINF_BITS(base, CDPHY_RX_ANA_1, > + RG_CSI0_CDPHY_EQ_DES_VREF_SEL, 0x2); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_9, > + RG_CSI0_RESERVE, 0x3003); > + SENINF_BITS(base, CDPHY_RX_ANA_SETTING_0, > + CSR_CSI_RST_MODE, 0x2); > + > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_L0P_T0A_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_L0N_T0B_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_3, > + RG_CSI0_L1P_T0C_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_3, > + RG_CSI0_L1N_T1A_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_4, > + RG_CSI0_L2P_T1B_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_4, > + RG_CSI0_L2N_T1C_HSRT_CODE, 0x10); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_T0_CDR_FIRST_EDGE_EN, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_T1_CDR_FIRST_EDGE_EN, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_CPHY_T0_CDR_SELF_CAL_EN, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_2, > + RG_CSI0_CPHY_T1_CDR_SELF_CAL_EN, 0x0); > + > + SENINF_BITS(base, CDPHY_RX_ANA_6, > + RG_CSI0_CPHY_T0_CDR_CK_DELAY, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_7, > + RG_CSI0_CPHY_T1_CDR_CK_DELAY, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_6, > + RG_CSI0_CPHY_T0_CDR_AB_WIDTH, 0x9); > + SENINF_BITS(base, CDPHY_RX_ANA_6, > + RG_CSI0_CPHY_T0_CDR_BC_WIDTH, 0x9); > + SENINF_BITS(base, CDPHY_RX_ANA_6, > + RG_CSI0_CPHY_T0_CDR_CA_WIDTH, 0x9); > + SENINF_BITS(base, CDPHY_RX_ANA_7, > + RG_CSI0_CPHY_T1_CDR_AB_WIDTH, 0x9); > + SENINF_BITS(base, CDPHY_RX_ANA_7, > + RG_CSI0_CPHY_T1_CDR_BC_WIDTH, 0x9); > + SENINF_BITS(base, CDPHY_RX_ANA_7, > + RG_CSI0_CPHY_T1_CDR_CA_WIDTH, 0x9); > + > + dev_dbg(ctx->dev, "port:%d CDPHY_RX_ANA_0(0x%x)\n", > + port, SENINF_READ_REG(base, CDPHY_RX_ANA_0)); > + } > + > + apply_efuse_data(ctx); > + > + return 0; > +} > + > +static int csirx_dphy_init(struct seninf_ctx *ctx) > +{ > + void __iomem *base = ctx->reg_ana_dphy_top[ctx->port]; > + int settle_delay_dt, settle_delay_ck, hs_trail, hs_trail_en; > + int bit_per_pixel; u8 bits_per_pixel; > + u64 data_rate; > + > + settle_delay_dt = ctx->is_cphy ? ctx->core->cphy_settle_delay_dt : > + ctx->core->dphy_settle_delay_dt; Please set settle_delay_ck and hs_trail here. > + > + SENINF_BITS(base, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_CDPHY_RX_LD0_TRIO0_HS_SETTLE_PARAMETER, > + settle_delay_dt); > + SENINF_BITS(base, DPHY_RX_DATA_LANE1_HS_PARAMETER, > + RG_CDPHY_RX_LD1_TRIO1_HS_SETTLE_PARAMETER, > + settle_delay_dt); > + SENINF_BITS(base, DPHY_RX_DATA_LANE2_HS_PARAMETER, > + RG_CDPHY_RX_LD2_TRIO2_HS_SETTLE_PARAMETER, > + settle_delay_dt); > + SENINF_BITS(base, DPHY_RX_DATA_LANE3_HS_PARAMETER, > + RG_CDPHY_RX_LD3_TRIO3_HS_SETTLE_PARAMETER, > + settle_delay_dt); > + > + settle_delay_ck = ctx->core->settle_delay_ck; > + > + SENINF_BITS(base, DPHY_RX_CLOCK_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LC0_HS_SETTLE_PARAMETER, > + settle_delay_ck); > + SENINF_BITS(base, DPHY_RX_CLOCK_LANE1_HS_PARAMETER, > + RG_DPHY_RX_LC1_HS_SETTLE_PARAMETER, > + settle_delay_ck); > + > + SENINF_BITS(base, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_CDPHY_RX_LD0_TRIO0_HS_PREPARE_PARAMETER, 2); > + SENINF_BITS(base, DPHY_RX_DATA_LANE1_HS_PARAMETER, > + RG_CDPHY_RX_LD1_TRIO1_HS_PREPARE_PARAMETER, 2); > + SENINF_BITS(base, DPHY_RX_DATA_LANE2_HS_PARAMETER, > + RG_CDPHY_RX_LD2_TRIO2_HS_PREPARE_PARAMETER, 2); > + SENINF_BITS(base, DPHY_RX_DATA_LANE3_HS_PARAMETER, > + RG_CDPHY_RX_LD3_TRIO3_HS_PREPARE_PARAMETER, 2); > + > + hs_trail = ctx->hs_trail_parameter; > + > + SENINF_BITS(base, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LD0_HS_TRAIL_PARAMETER, hs_trail); > + SENINF_BITS(base, DPHY_RX_DATA_LANE1_HS_PARAMETER, > + RG_DPHY_RX_LD1_HS_TRAIL_PARAMETER, hs_trail); > + SENINF_BITS(base, DPHY_RX_DATA_LANE2_HS_PARAMETER, > + RG_DPHY_RX_LD2_HS_TRAIL_PARAMETER, hs_trail); > + SENINF_BITS(base, DPHY_RX_DATA_LANE3_HS_PARAMETER, > + RG_DPHY_RX_LD3_HS_TRAIL_PARAMETER, hs_trail); > + > + if (!ctx->is_cphy) { > + bit_per_pixel = 10; data_rate = ctx->customized_pixel_rate ? ctx->customized_pixel_rate : ctx->mipi_pixel_rate; data_rate *= bits_per_pixel; > + if (ctx->customized_pixel_rate != 0) > + data_rate = ctx->customized_pixel_rate * bit_per_pixel; > + else > + data_rate = ctx->mipi_pixel_rate * bit_per_pixel; > + > + do_div(data_rate, ctx->num_data_lanes); > + hs_trail_en = data_rate < 1400000000; > + SENINF_BITS(base, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LD0_HS_TRAIL_EN, hs_trail_en); > + SENINF_BITS(base, DPHY_RX_DATA_LANE1_HS_PARAMETER, > + RG_DPHY_RX_LD1_HS_TRAIL_EN, hs_trail_en); > + SENINF_BITS(base, DPHY_RX_DATA_LANE2_HS_PARAMETER, > + RG_DPHY_RX_LD2_HS_TRAIL_EN, hs_trail_en); > + SENINF_BITS(base, DPHY_RX_DATA_LANE3_HS_PARAMETER, > + RG_DPHY_RX_LD3_HS_TRAIL_EN, hs_trail_en); > + } > + > + return 0; > +} > + > +static int csirx_cphy_init(struct seninf_ctx *ctx) > +{ > + void __iomem *base = ctx->reg_ana_cphy_top[ctx->port]; > + > + SENINF_BITS(base, CPHY_RX_DETECT_CTRL_POST, > + RG_CPHY_RX_DATA_VALID_POST_EN, 1); > + > + return 0; > +} > + > +static int csirx_phy_init(struct seninf_ctx *ctx) > +{ > + csirx_phyA_init(ctx); > + > + csirx_dphy_init(ctx); > + csirx_cphy_init(ctx); > + > + return 0; > +} > + > +static int csirx_seninf_csi2_setting(struct seninf_ctx *ctx) > +{ > + void __iomem *seninf_csi2 = ctx->reg_if_csi2[ctx->seninf_idx]; > + int csi_en; > + > + SENINF_BITS(seninf_csi2, SENINF_CSI2_DBG_CTRL, > + RG_CSI2_DBG_PACKET_CNT_EN, 1); > + > + /* lane/trio count */ > + SENINF_BITS(seninf_csi2, SENINF_CSI2_RESYNC_MERGE_CTRL, > + RG_CSI2_RESYNC_CYCLE_CNT_OPT, 1); > + > + csi_en = (1 << ctx->num_data_lanes) - 1; csi_en = BIT(ctx->num_data_lanes) - 1; or csi_en = GENMASK(ctx->num_data_lanes - 1, 0); > + > + if (!ctx->is_cphy) { > + SENINF_BITS(seninf_csi2, SENINF_CSI2_OPT, RG_CSI2_CPHY_SEL, 0); > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_EN, csi_en); > + SENINF_BITS(seninf_csi2, SENINF_CSI2_HDR_MODE_0, > + RG_CSI2_HEADER_MODE, 0); > + SENINF_BITS(seninf_csi2, SENINF_CSI2_HDR_MODE_0, > + RG_CSI2_HEADER_LEN, 0); > + } else { > + u8 map_hdr_len[] = {0, 1, 2, 4, 5}; u8 map_hdr_len[] = { 0, 1, 2, 4, 5 }; > + > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_EN, csi_en); > + SENINF_BITS(seninf_csi2, SENINF_CSI2_OPT, > + RG_CSI2_CPHY_SEL, 1); > + SENINF_BITS(seninf_csi2, SENINF_CSI2_HDR_MODE_0, > + RG_CSI2_HEADER_MODE, 2); > + SENINF_BITS(seninf_csi2, SENINF_CSI2_HDR_MODE_0, > + RG_CSI2_HEADER_LEN, > + map_hdr_len[ctx->num_data_lanes]); > + } > + > + return 0; > +} > + > +static int csirx_seninf_setting(struct seninf_ctx *ctx) > +{ > + void __iomem *seninf = ctx->reg_if_ctrl[ctx->seninf_idx]; > + > + /* enable/disable seninf csi2 */ > + SENINF_BITS(seninf, SENINF_CSI2_CTRL, RG_SENINF_CSI2_EN, 1); > + > + /* enable/disable seninf, enable after csi2, testmdl is done */ > + SENINF_BITS(seninf, SENINF_CTRL, SENINF_EN, 1); > + > + return 0; > +} > + > +static int csirx_seninf_top_setting(struct seninf_ctx *ctx) > +{ ..snip.. > + > + /* port operation mode */ > + switch (ctx->port) { > + case CSI_PORT_0: > + case CSI_PORT_0A: > + case CSI_PORT_0B: reg = TOP_PHY_CTRL_CSI0; field_cphy = PHY_SENINF_MUX0_CPHY_EN; field_dphy = PHY_SENINF_MUX0_DPHY_EN; break; case CSI_PORT_1: case CSI_PORT_1A: case CSI_PORT_1B: reg = TOP_PHY_CTRL_CSI1; field_cphy = PHY_SENINF_MUX1_CPHY_EN; field_dphy = PHY_SENINF_MUX1_DPHY_EN; break; .... etc } regmap_write_bits(regmap, reg, field_cphy, ctx->is_cphy); regmap_write_bits(regmap, reg, field_dphy, !ctx->is_cphy); > + if (!ctx->is_cphy) { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI0, > + PHY_SENINF_MUX0_CPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI0, > + PHY_SENINF_MUX0_DPHY_EN, 1); > + } else { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI0, > + PHY_SENINF_MUX0_DPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI0, > + PHY_SENINF_MUX0_CPHY_EN, 1); > + } > + break; > + case CSI_PORT_1: > + case CSI_PORT_1A: > + case CSI_PORT_1B: > + if (!ctx->is_cphy) { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI1, > + PHY_SENINF_MUX1_CPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI1, > + PHY_SENINF_MUX1_DPHY_EN, 1); > + } else { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI1, > + PHY_SENINF_MUX1_DPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI1, > + PHY_SENINF_MUX1_CPHY_EN, 1); > + } > + break; > + case CSI_PORT_2: > + case CSI_PORT_2A: > + case CSI_PORT_2B: > + if (!ctx->is_cphy) { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI2, > + PHY_SENINF_MUX2_CPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI2, > + PHY_SENINF_MUX2_DPHY_EN, 1); > + } else { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI2, > + PHY_SENINF_MUX2_DPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI2, > + PHY_SENINF_MUX2_CPHY_EN, 1); > + } > + break; > + case CSI_PORT_3: > + case CSI_PORT_3A: > + case CSI_PORT_3B: > + if (!ctx->is_cphy) { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI3, > + PHY_SENINF_MUX3_CPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI3, > + PHY_SENINF_MUX3_DPHY_EN, 1); > + } else { > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI3, > + PHY_SENINF_MUX3_DPHY_EN, 0); > + SENINF_BITS(seninf_top, SENINF_TOP_PHY_CTRL_CSI3, > + PHY_SENINF_MUX3_CPHY_EN, 1); > + } > + break; > + default: > + break; So if this is called with a CSI_PORT that is out of range, we're not erroring out?!?! > + } > + > + return 0; > +} > + > +static int csirx_phyA_setting(struct seninf_ctx *ctx) > +{ > + void __iomem *base, *baseA, *baseB; > + > + base = ctx->reg_ana_csi_rx[ctx->port]; > + baseA = ctx->reg_ana_csi_rx[ctx->port_a]; > + baseB = ctx->reg_ana_csi_rx[ctx->port_b]; > + > + if (!ctx->is_cphy) { /* dphy */ For this huge register write sequence, you want to split it in at least two functions to improve human readability and reduce indentation. > + if (ctx->is_4d1c) { > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 0); > + /* clear clk sel first */ > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 0); > + > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKSEL, 1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKSEL, 1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKSEL, 1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKSEL, 1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKSEL, 1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKSEL, 1); > + > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 0); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 0); > + > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + } else { > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 0); > + /* clear clk sel first */ > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 0); > + > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKSEL, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKSEL, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKSEL, 0); > + > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L0_CKMODE_EN, 0); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L1_CKMODE_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_DPHY_L2_CKMODE_EN, 0); > + > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + } > + } else { /* cphy */ > + if (ctx->is_4d1c) { > + SENINF_BITS(baseA, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 1); > + SENINF_BITS(baseB, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 1); > + > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x3); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x0); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x3); > + SENINF_BITS(baseA, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + } else { > + SENINF_BITS(base, CDPHY_RX_ANA_0, > + RG_CSI0_CPHY_EN, 1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_BW, 0x3); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_IS, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_LATCH_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG0_EN, 0x1); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_DG1_EN, 0x0); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR0, 0x3); > + SENINF_BITS(base, CDPHY_RX_ANA_5, > + RG_CSI0_CDPHY_EQ_SR1, 0x0); > + } > + } > + > + /* phyA power on */ > + if (ctx->is_4d1c) { > + csirx_phyA_power_on(ctx, ctx->port_a, 1); > + csirx_phyA_power_on(ctx, ctx->port_b, 1); > + } else { > + csirx_phyA_power_on(ctx, ctx->port, 1); > + } > + > + return 0; > +} > + > +static int csirx_dphy_setting(struct seninf_ctx *ctx) > +{ > + void __iomem *base = ctx->reg_ana_dphy_top[ctx->port]; > + > + if (ctx->is_4d1c) { > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD3_SEL, 4); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD2_SEL, 0); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD1_SEL, 3); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD0_SEL, 1); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LC0_SEL, 2); > + > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD0_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD1_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD2_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD3_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LC0_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LC1_EN, 0); > + } else { > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD3_SEL, 5); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD2_SEL, 3); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD1_SEL, 2); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LD0_SEL, 0); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LC1_SEL, 4); > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, RG_DPHY_RX_LC0_SEL, 1); > + > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD0_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD1_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD2_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LD3_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LC0_EN, 1); > + SENINF_BITS(base, DPHY_RX_LANE_EN, DPHY_RX_LC1_EN, 1); > + } > + > + SENINF_BITS(base, DPHY_RX_LANE_SELECT, DPHY_RX_CK_DATA_MUX_EN, 1); > + > + return 0; > +} > + > +static int csirx_cphy_setting(struct seninf_ctx *ctx) > +{ > + void __iomem *base = ctx->reg_ana_cphy_top[ctx->port]; u32 reg_lprx[] = { CPHY_RX_TR0_LPRX_EN, CPHY_RX_TR1_LPRX_EN, ...TR2, ...TR3 }; > + > + switch (ctx->port) { > + case CSI_PORT_0: > + case CSI_PORT_1: > + case CSI_PORT_2: > + case CSI_PORT_3: > + case CSI_PORT_0A: > + case CSI_PORT_1A: > + case CSI_PORT_2A: > + case CSI_PORT_3A: for (i = 0; i < ctx->num_data_lanes; i++) regmap_write_bits(regmap, CPHY_RX_CTRL, reg_lprx[i], 1); for (; i < ARRAY_SIZE(reg_lprx); i++) regmap_write_bits(regmap, CPHY_RX_CTRL, reg_lprx[i], 0); > + if (ctx->num_data_lanes == 3) { > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR0_LPRX_EN, 1); > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR1_LPRX_EN, 1); > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR2_LPRX_EN, 1); > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR3_LPRX_EN, 0); > + } else if (ctx->num_data_lanes == 2) { > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR0_LPRX_EN, 1); > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR1_LPRX_EN, 1); > + } else { > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR0_LPRX_EN, 1); > + } > + break; > + case CSI_PORT_0B: > + case CSI_PORT_1B: > + case CSI_PORT_2B: > + case CSI_PORT_3B: for (i = 0; i < ctx->num_data_lanes; i++) { regmap_write_bits(regmap, CPHY_RX_CTRL, reg_lprx[i], 0); regmap_write_bits(regmap, CPHY_RX_CTRL, reg_lprx[i+2], 1); } > + if (ctx->num_data_lanes == 2) { > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR2_LPRX_EN, 1); > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR3_LPRX_EN, 1); > + } else { > + SENINF_BITS(base, CPHY_RX_CTRL, CPHY_RX_TR2_LPRX_EN, 1); > + } > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int csirx_phy_setting(struct seninf_ctx *ctx) > +{ > + csirx_phyA_setting(ctx); > + Please, if (ctx->is_cphy) x() else y() > + if (!ctx->is_cphy) > + csirx_dphy_setting(ctx); > + else > + csirx_cphy_setting(ctx); > + > + return 0; > +} > + > +int mtk_cam_seninf_set_csi_mipi(struct seninf_ctx *ctx) > +{ > + csirx_phy_init(ctx); > + > + /* seninf csi2 */ > + csirx_seninf_csi2_setting(ctx); > + > + /* seninf */ > + csirx_seninf_setting(ctx); > + > + /* seninf top */ > + csirx_seninf_top_setting(ctx); > + > + /* phy */ > + csirx_phy_setting(ctx); > + > + return 0; > +} > + > +int mtk_cam_seninf_poweroff(struct seninf_ctx *ctx) > +{ > + void __iomem *seninf_csi2; > + > + seninf_csi2 = ctx->reg_if_csi2[ctx->seninf_idx]; > + > + SENINF_WRITE_REG(seninf_csi2, SENINF_CSI2_EN, 0x0); > + > + if (ctx->is_4d1c) { > + csirx_phyA_power_on(ctx, ctx->port_a, 0); > + csirx_phyA_power_on(ctx, ctx->port_b, 0); > + } else { > + csirx_phyA_power_on(ctx, ctx->port, 0); > + } > + > + return 0; > +} > + > +int mtk_cam_seninf_reset(struct seninf_ctx *ctx, u32 seninf_idx) > +{ > + int i; > + void __iomem *seninf_mux; > + void __iomem *seninf = ctx->reg_if_ctrl[seninf_idx]; > + > + SENINF_BITS(seninf, SENINF_CSI2_CTRL, SENINF_CSI2_SW_RST, 1); What about adding a reset controller that includes SENINF_CSI(x)_SW_RST? Also, why are you resetting only CSI2 and not the others? > + udelay(1); > + SENINF_BITS(seninf, SENINF_CSI2_CTRL, SENINF_CSI2_SW_RST, 0); > + > + dev_dbg(ctx->dev, "reset seninf %d\n", seninf_idx); > + > + for (i = SENINF_MUX1; i < _seninf_cfg.mux_num; i++) > + if (mtk_cam_seninf_get_top_mux_ctrl(ctx, i) == seninf_idx && > + mtk_cam_seninf_is_mux_used(ctx, i)) { > + seninf_mux = ctx->reg_if_mux[i]; > + SENINF_BITS(seninf_mux, SENINF_MUX_CTRL_0, > + SENINF_MUX_SW_RST, 1); > + udelay(1); > + SENINF_BITS(seninf_mux, SENINF_MUX_CTRL_0, > + SENINF_MUX_SW_RST, 0); > + dev_dbg(ctx->dev, "reset mux %d\n", i); > + } > + > + return 0; > +} > + > +int mtk_cam_seninf_set_idle(struct seninf_ctx *ctx) > +{ > + int i; > + struct seninf_vcinfo *vcinfo = &ctx->vcinfo; > + struct seninf_vc *vc; > + > + for (i = 0; i < vcinfo->cnt; i++) { > + vc = &vcinfo->vc[i]; > + if (vc->enable) { > + mtk_cam_seninf_disable_mux(ctx, vc->mux); > + mtk_cam_seninf_disable_cammux(ctx, vc->cam); > + ctx->pad2cam[vc->out_pad] = 0xff; > + } > + } > + > + return 0; > +} > + > +int mtk_cam_seninf_get_mux_meter(struct seninf_ctx *ctx, u32 mux, > + struct mtk_cam_seninf_mux_meter *meter) > +{ > + void __iomem *seninf_mux; > + s64 hv, hb, vv, vb, w, h; > + u64 mipi_pixel_rate, vb_in_us, hb_in_us, line_time_in_us; > + u32 res; > + > + seninf_mux = ctx->reg_if_mux[mux]; > + > + SENINF_BITS(seninf_mux, SENINF_MUX_FRAME_SIZE_MON_CTRL, > + RG_SENINF_MUX_FRAME_SIZE_MON_EN, 1); > + > + hv = SENINF_READ_REG(seninf_mux, SENINF_MUX_FRAME_SIZE_MON_H_VALID); > + hb = SENINF_READ_REG(seninf_mux, SENINF_MUX_FRAME_SIZE_MON_H_BLANK); > + vv = SENINF_READ_REG(seninf_mux, SENINF_MUX_FRAME_SIZE_MON_V_VALID); > + vb = SENINF_READ_REG(seninf_mux, SENINF_MUX_FRAME_SIZE_MON_V_BLANK); > + res = SENINF_READ_REG(seninf_mux, SENINF_MUX_SIZE); > + > + w = res & 0xffff; Another case of bitfield.h macros open-coded, or missed opportunity to just use regmap to simplify this. > + h = res >> 16; And same here, of course. > + > + if (ctx->fps_n && ctx->fps_d) { > + mipi_pixel_rate = w * ctx->fps_n * (vv + vb); > + do_div(mipi_pixel_rate, ctx->fps_d); Use div_s64() for safe 64-bits division. > + do_div(mipi_pixel_rate, hv); > + > + vb_in_us = vb * ctx->fps_d * 1000000; > + do_div(vb_in_us, vv + vb); > + do_div(vb_in_us, ctx->fps_n); > + > + hb_in_us = hb * ctx->fps_d * 1000000; > + do_div(hb_in_us, vv + vb); > + do_div(hb_in_us, ctx->fps_n); > + > + line_time_in_us = (hv + hb) * ctx->fps_d * 1000000; > + do_div(line_time_in_us, vv + vb); > + do_div(line_time_in_us, ctx->fps_n); > + > + meter->mipi_pixel_rate = mipi_pixel_rate; > + meter->vb_in_us = vb_in_us; > + meter->hb_in_us = hb_in_us; > + meter->line_time_in_us = line_time_in_us; > + } else { > + meter->mipi_pixel_rate = -1; > + meter->vb_in_us = -1; > + meter->hb_in_us = -1; > + meter->line_time_in_us = -1; > + } > + > + meter->width = w; > + meter->height = h; > + > + meter->h_valid = hv; > + meter->h_blank = hb; > + meter->v_valid = vv; > + meter->v_blank = vb; > + > + return 0; > +} > + > +ssize_t mtk_cam_seninf_show_status(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int i, len; > + struct seninf_core *core; struct seninf_core *core = dev_get_drvdata(dev); > + struct seninf_ctx *ctx; > + struct seninf_vc *vc; > + struct media_link *link; > + struct media_pad *pad; > + struct mtk_cam_seninf_mux_meter meter; > + void __iomem *csi2, *pmux; > + void __iomem *rx, *pcammux; int i, len = 0; > + > + core = dev_get_drvdata(dev); > + len = 0; > + > + mutex_lock(&core->mutex); > + > + list_for_each_entry(ctx, &core->list, list) { > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\n[%s] port %d intf %d test %d cphy %d lanes %d\n", > + ctx->subdev.name, ctx->port, ctx->seninf_idx, > + ctx->is_test_model, ctx->is_cphy, > + ctx->num_data_lanes); > + > + pad = &ctx->pads[PAD_SINK]; > + list_for_each_entry(link, &pad->entity->links, list) { > + if (link->sink == pad) { > + len += snprintf(buf + len, PAGE_SIZE - len, > + "source %s flags 0x%lx\n", > + link->source->entity->name, > + link->flags); > + } > + } > + > + if (!ctx->streaming) > + continue; > + > + csi2 = ctx->reg_if_csi2[ctx->seninf_idx]; > + rx = ctx->reg_ana_dphy_top[ctx->port]; > + > + len += snprintf(buf + len, PAGE_SIZE - len, > + "csi2 irq_stat 0x%08x\n", > + SENINF_READ_REG(csi2, SENINF_CSI2_IRQ_STATUS)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "csi2 line_frame_num 0x%08x\n", > + SENINF_READ_REG(csi2, SENINF_CSI2_LINE_FRAME_NUM)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "csi2 packet_status 0x%08x\n", > + SENINF_READ_REG(csi2, SENINF_CSI2_PACKET_STATUS)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "csi2 packet_cnt_status 0x%08x\n", > + SENINF_READ_REG(csi2, SENINF_CSI2_PACKET_CNT_STATUS)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "rx-ana settle ck 0x%02x dt 0x%02x\n", > + SENINF_READ_BITS(rx, DPHY_RX_CLOCK_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LC0_HS_SETTLE_PARAMETER), > + SENINF_READ_BITS(rx, > + DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_CDPHY_RX_LD0_TRIO0_HS_SETTLE_PARAMETER)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "rx-ana trail en %u param 0x%02x\n", > + SENINF_READ_BITS(rx, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LD0_HS_TRAIL_EN), > + SENINF_READ_BITS(rx, DPHY_RX_DATA_LANE0_HS_PARAMETER, > + RG_DPHY_RX_LD0_HS_TRAIL_PARAMETER)); > + > + len += snprintf(buf + len, PAGE_SIZE - len, > + "data_not_enough_cnt : <%d>", > + ctx->data_not_enough_cnt); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "err_lane_resync_cnt : <%d>", > + ctx->err_lane_resync_cnt); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "crc_err_cnt : <%d>", ctx->crc_err_flag); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "ecc_err_double_cnt : <%d>", > + ctx->ecc_err_double_cnt); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "ecc_err_corrected_cnt : <%d>\n", > + ctx->ecc_err_corrected_cnt); > + > + for (i = 0; i < ctx->vcinfo.cnt; i++) { > + vc = &ctx->vcinfo.vc[i]; > + pmux = ctx->reg_if_mux[vc->mux]; > + pcammux = ctx->reg_if_cam_mux; > + > + len += snprintf(buf + len, PAGE_SIZE - len, > + "[%d] vc 0x%x dt 0x%x mux %d cam %d\n", > + i, vc->vc, vc->dt, vc->mux, vc->cam); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tmux[%d] en %d src %d irq_stat 0x%x\n", > + vc->mux, > + mtk_cam_seninf_is_mux_used(ctx, vc->mux), > + mtk_cam_seninf_get_top_mux_ctrl(ctx, vc->mux), > + SENINF_READ_REG(pmux, SENINF_MUX_IRQ_STATUS)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\t\tfifo_overrun_cnt : <%d>\n", > + ctx->fifo_overrun_cnt); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tcam[%d] en %d src %d exp 0x%x res 0x%x irq_stat 0x%x\n", > + vc->cam, > + mtk_cam_seninf_is_cammux_used(ctx, vc->cam), > + mtk_cam_seninf_get_cammux_ctrl(ctx, vc->cam), > + mtk_cam_seninf_get_cammux_exp(ctx, vc->cam), > + mtk_cam_seninf_get_cammux_res(ctx, vc->cam), > + SENINF_READ_REG(pcammux, > + SENINF_CAM_MUX_IRQ_STATUS)); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\t\tsize_err_cnt : <%d>\n", > + ctx->size_err_cnt); > + > + if (vc->feature == VC_RAW_DATA || > + vc->feature == VC_STAGGER_NE || > + vc->feature == VC_STAGGER_ME || > + vc->feature == VC_STAGGER_SE) { > + mtk_cam_seninf_get_mux_meter(ctx, vc->mux, > + &meter); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\t--- mux meter ---\n"); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\twidth %d height %d\n", > + meter.width, meter.height); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\th_valid %d, h_blank %d\n", > + meter.h_valid, meter.h_blank); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tv_valid %d, v_blank %d\n", > + meter.v_valid, meter.v_blank); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tmipi_pixel_rate %lld\n", > + meter.mipi_pixel_rate); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tv_blank %lld us\n", > + meter.vb_in_us); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\th_blank %lld us\n", > + meter.hb_in_us); > + len += snprintf(buf + len, PAGE_SIZE - len, > + "\tline_time %lld us\n", > + meter.line_time_in_us); > + } > + } > + } > + > + mutex_unlock(&core->mutex); > + > + return len; > +} > + > +#define SENINF_DRV_DEBUG_MAX_DELAY 400 > + > +static inline void > +mtk_cam_seninf_clear_matched_cam_mux_irq(struct seninf_ctx *ctx, > + u32 cam_mux_idx, Why is this a u32 param?.... > + u32 vc_idx, > + s32 enabled) > +{ > + u8 used_cammux; ...if this is a u8 local variable? (cam_mux_idx should be u8 instead - same for vc_idx) > + > + if (cam_mux_idx >= SENINF_CAM_MUX_NUM) { > + dev_info(ctx->dev, "unsupport cam_mux(%u)", cam_mux_idx); > + return; > + } > + if (vc_idx >= SENINF_VC_MAXCNT) { > + dev_info(ctx->dev, "unsupport vc_idx(%u)", vc_idx); > + return; > + } > + > + used_cammux = ctx->vcinfo.vc[vc_idx].cam; > + if (used_cammux == cam_mux_idx && > + enabled & (1 << cam_mux_idx)) { > + dev_dbg(ctx->dev, > + "before clear cam mux%u recSize = 0x%x, irq = 0x%x", > + cam_mux_idx, > + SENINF_READ_REG(ctx->reg_if_cam_mux, > + SENINF_CAM_MUX0_CHK_RES + (0x10 * cam_mux_idx)), > + SENINF_READ_REG(ctx->reg_if_cam_mux, > + SENINF_CAM_MUX_IRQ_STATUS)); > + > + SENINF_WRITE_REG(ctx->reg_if_cam_mux, > + SENINF_CAM_MUX_IRQ_STATUS, > + 3 << (cam_mux_idx * 2)); > + } > +} > + > +static inline void mtk_cam_seninf_check_matched_cam_mux(struct seninf_ctx *ctx, > + u32 cam_mux_idx, > + u32 vc_idx, > + s32 enabled, > + s32 irq_status) > +{ > + u8 used_cammux; > + > + if (cam_mux_idx >= SENINF_CAM_MUX_NUM) { > + dev_info(ctx->dev, "unsupport cam_mux(%u)", cam_mux_idx); > + return; > + } > + if (vc_idx >= SENINF_VC_MAXCNT) { > + dev_info(ctx->dev, "unsupport vc_idx(%u)", vc_idx); > + return; > + } > + > + used_cammux = ctx->vcinfo.vc[vc_idx].cam; > + > + if (used_cammux == cam_mux_idx && enabled & (1 << cam_mux_idx)) { > + int rec_size = SENINF_READ_REG(ctx->reg_if_cam_mux, > + SENINF_CAM_MUX0_CHK_RES + (0x10 * cam_mux_idx)); > + int exp_size = SENINF_READ_REG(ctx->reg_if_cam_mux, > + SENINF_CAM_MUX0_CHK_CTL_1 + (0x10 * cam_mux_idx)); > + if (rec_size != exp_size) { > + dev_dbg(ctx->dev, > + "cam mux%u size mismatch, (rec, exp) = (0x%x, 0x%x)", > + cam_mux_idx, rec_size, exp_size); > + } > + if ((irq_status & > + (3 << (cam_mux_idx * 2))) != 0) { > + dev_dbg(ctx->dev, > + "cam mux%u size mismatch!, irq = 0x%x", > + cam_mux_idx, irq_status); > + } > + } > +} > + ..snip.. > + > +#define SBUF 256 > +int mtk_cam_seninf_irq_handler(int irq, void *data) > +{ > + struct seninf_core *core = (struct seninf_core *)data; > + unsigned long flags; /* for mipi err detection */ > + struct seninf_ctx *ctx; > + struct seninf_vc *vc; > + void __iomem *csi2, *pmux, *seninf_cam_mux; > + int i; > + unsigned int csi_irq_ro; > + unsigned int mux_irq_ro; > + unsigned int cam_irq_exp_ro; > + unsigned int cam_irq_res_ro; > + char seninf_log[SBUF]; > + unsigned int wcnt = 0; > + > + spin_lock_irqsave(&core->spinlock_irq, flags); > + > + /* debug for set_reg case: REG_KEY_CSI_IRQ_EN */ > + if (core->csi_irq_en_flag) { > + list_for_each_entry(ctx, &core->list, list) { > + csi2 = ctx->reg_if_csi2[ctx->seninf_idx]; > + csi_irq_ro = > + SENINF_READ_REG(csi2, SENINF_CSI2_IRQ_STATUS); > + > + if (csi_irq_ro) { > + SENINF_WRITE_REG(csi2, SENINF_CSI2_IRQ_STATUS, > + 0xFFFFFFFF); > + } > + > + if (csi_irq_ro & (0x1 << RO_CSI2_ECC_ERR_CORRECTED_IRQ_SHIFT)) > + ctx->ecc_err_corrected_cnt++; > + if (csi_irq_ro & (0x1 << RO_CSI2_ECC_ERR_DOUBLE_IRQ_SHIFT)) > + ctx->ecc_err_double_cnt++; > + if (csi_irq_ro & (0x1 << RO_CSI2_CRC_ERR_IRQ_SHIFT)) > + ctx->crc_err_cnt++; > + if (csi_irq_ro & (0x1 << RO_CSI2_ERR_LANE_RESYNC_IRQ_SHIFT)) > + ctx->err_lane_resync_cnt++; > + if (csi_irq_ro & (0x1 << RO_CSI2_RECEIVE_DATA_NOT_ENOUGH_IRQ_SHIFT)) > + ctx->data_not_enough_cnt++; > + > + for (i = 0; i < ctx->vcinfo.cnt; i++) { > + vc = &ctx->vcinfo.vc[i]; > + pmux = ctx->reg_if_mux[vc->mux]; > + seninf_cam_mux = ctx->reg_if_cam_mux; > + > + mux_irq_ro = SENINF_READ_REG(pmux, > + SENINF_MUX_IRQ_STATUS); > + > + cam_irq_exp_ro = SENINF_READ_REG(seninf_cam_mux, > + SENINF_CAM_MUX0_CHK_CTL_1 + > + (0x10 * (vc->cam))); > + > + cam_irq_res_ro = SENINF_READ_REG(seninf_cam_mux, > + SENINF_CAM_MUX0_CHK_RES + > + (0x10 * (vc->cam))); > + > + if (mux_irq_ro) > + SENINF_WRITE_REG(pmux, > + SENINF_MUX_IRQ_STATUS, > + 0xFFFFFFFF); > + > + if (cam_irq_res_ro != cam_irq_exp_ro) > + SENINF_WRITE_REG(seninf_cam_mux, > + SENINF_CAM_MUX0_CHK_RES + > + (0x10 * (vc->cam)), > + 0xFFFFFFFF); > + > + if (mux_irq_ro & (0x1 << 0)) > + ctx->fifo_overrun_cnt++; > + > + if (cam_irq_res_ro != cam_irq_exp_ro) > + ctx->size_err_cnt++; > + } > + > + /* dump status counter: debug for electrical signal */ > + if (ctx->data_not_enough_cnt >= core->detection_cnt || > + ctx->err_lane_resync_cnt >= core->detection_cnt || > + ctx->crc_err_cnt >= core->detection_cnt || > + ctx->ecc_err_double_cnt >= core->detection_cnt || > + ctx->ecc_err_corrected_cnt >= core->detection_cnt || > + ctx->fifo_overrun_cnt >= core->detection_cnt || > + ctx->size_err_cnt >= core->detection_cnt) { > + /* disable all interrupts */ > + SENINF_WRITE_REG(csi2, SENINF_CSI2_IRQ_EN, 0x80000000); > + > + if (ctx->data_not_enough_cnt >= core->detection_cnt) > + ctx->data_not_enough_flag = 1; > + if (ctx->err_lane_resync_cnt >= core->detection_cnt) > + ctx->err_lane_resync_flag = 1; > + if (ctx->crc_err_cnt >= core->detection_cnt) > + ctx->crc_err_flag = 1; > + if (ctx->ecc_err_double_cnt >= core->detection_cnt) > + ctx->ecc_err_double_flag = 1; > + if (ctx->ecc_err_corrected_cnt >= core->detection_cnt) > + ctx->ecc_err_corrected_flag = 1; > + if (ctx->fifo_overrun_cnt >= core->detection_cnt) > + ctx->fifo_overrun_flag = 1; > + if (ctx->size_err_cnt >= core->detection_cnt) > + ctx->size_err_flag = 1; > + > + wcnt = snprintf(seninf_log, SBUF, "info: %s", __func__); You're not even printing seninf_log, so either print it (no, please don't) or just drop it. > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " data_not_enough_count: %d", > + ctx->data_not_enough_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " err_lane_resync_count: %d", > + ctx->err_lane_resync_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " crc_err_count: %d", > + ctx->crc_err_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " ecc_err_double_count: %d", > + ctx->ecc_err_double_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " ecc_err_corrected_count: %d", > + ctx->ecc_err_corrected_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " fifo_overrun_count: %d", > + ctx->fifo_overrun_cnt); > + wcnt += snprintf(seninf_log + wcnt, SBUF - wcnt, > + " size_err_count: %d", > + ctx->size_err_cnt); > + } > + } > + } > + > + spin_unlock_irqrestore(&core->spinlock_irq, flags); > + > + return 0; > +} > + > +int mtk_cam_seninf_set_sw_cfg_busy(struct seninf_ctx *ctx, bool enable, > + int index) > +{ > + void __iomem *seninf_cam_mux = ctx->reg_if_cam_mux; mask = index ? RG_SENINF_CAM_MUX_DYN_SWITCH_BSY1 : RG_SENINF_CAM_MUX_DYN_SWITCH_BSY0 > + > + if (index == 0) > + SENINF_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_CTRL, > + RG_SENINF_CAM_MUX_DYN_SWITCH_BSY0, enable); > + else > + SENINF_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_CTRL, > + RG_SENINF_CAM_MUX_DYN_SWITCH_BSY1, enable); > + return 0; > +} > + > +int mtk_cam_seninf_set_cam_mux_dyn_en(struct seninf_ctx *ctx, bool enable, > + int cam_mux, int index) > +{ > + void __iomem *seninf_cam_mux = ctx->reg_if_cam_mux; > + u32 tmp = 0; > + > + if (index == 0) { > + tmp = SENINF_READ_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_EN, > + RG_SENINF_CAM_MUX_DYN_SWITCH_EN0); regmap_set_bits() will definitely help here. > + if (enable) > + tmp = tmp | (1 << cam_mux); > + else > + tmp = tmp & ~(1 << cam_mux); > + > + SENINF_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_EN, > + RG_SENINF_CAM_MUX_DYN_SWITCH_EN0, tmp); > + } else { > + tmp = SENINF_READ_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_EN, > + RG_SENINF_CAM_MUX_DYN_SWITCH_EN1); > + if (enable) > + tmp = tmp | (1 << cam_mux); > + else > + tmp = tmp & ~(1 << cam_mux); > + > + SENINF_BITS(seninf_cam_mux, SENINF_CAM_MUX_DYN_EN, > + RG_SENINF_CAM_MUX_DYN_SWITCH_EN1, tmp); > + } > + > + return 0; > +} > + There is surely more to say on this driver, and it's far from being near to upstream quality. Please start with addressing these comments on the entire series, then we can go on with further reviews. Regards, Angelo
Il 09/10/24 13:15, Shu-hsiang Yang ha scritto: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.c | 5359 +++++++++++++++++ > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.h | 325 + > .../isp/isp_7x/camsys/mtk_cam-raw_debug.c | 403 ++ > .../isp/isp_7x/camsys/mtk_cam-raw_debug.h | 39 + > .../isp_7x/camsys/mtk_camera-v4l2-controls.h | 65 + > 5 files changed, 6191 insertions(+) > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw_debug.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw_debug.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_camera-v4l2-controls.h > > diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c > new file mode 100644 > index 000000000000..c025f53c952d > --- /dev/null > +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c > @@ -0,0 +1,5359 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (c) 2022 MediaTek Inc. > + > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/iopoll.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/vmalloc.h> > +#include <linux/videodev2.h> > +#include <linux/suspend.h> > +#include <linux/rtc.h> > + > +#include <media/v4l2-device.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-ioctl.h> > +#include <media/v4l2-subdev.h> > + > +#include <soc/mediatek/smi.h> > + > +#include "mtk_cam.h" > +#include "mtk_cam-feature.h" > +#include "mtk_cam-raw.h" > + > +#include "mtk_cam-regs-mt8188.h" > + > +#include "mtk_cam-video.h" > +#include "mtk_cam-seninf-if.h" > +#include "mtk_camera-v4l2-controls.h" > + > +#include "mtk_cam-dmadbg.h" > +#include "mtk_cam-raw_debug.h" > + > +static unsigned int debug_raw; > +module_param(debug_raw, uint, 0644); > +MODULE_PARM_DESC(debug_raw, "activates debug info"); > + > +static int debug_raw_num = -1; > +module_param(debug_raw_num, int, 0644); > +MODULE_PARM_DESC(debug_raw_num, "debug: num of used raw devices"); > + > +static int debug_pixel_mode = -1; > +module_param(debug_pixel_mode, int, 0644); > +MODULE_PARM_DESC(debug_pixel_mode, "debug: pixel mode"); > + > +static int debug_clk_idx = -1; > +module_param(debug_clk_idx, int, 0644); > +MODULE_PARM_DESC(debug_clk_idx, "debug: clk idx"); > + > +static int debug_dump_fbc; > +module_param(debug_dump_fbc, int, 0644); > +MODULE_PARM_DESC(debug_dump_fbc, "debug: dump fbc"); > + In addition to the first review that I gave you on patch [02/10]: please drop all those module parameters. If you want debug switches, use debugfs instead. Regards, Angelo
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces support for the sensor interface in the MediaTek SoC, > with the focus on CSI and stream control. The key functionalities > include parameter control, metering and maintaining status information, > interrupt handling, and debugging. These features ensure effective > management and debugging of the camera sensor interface hardware. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- > .../isp_7x/camsys/mtk_csi_phy_2_0/Makefile | 5 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h | 911 ++++++ > .../mtk_cam-seninf-csi0-cphy.h | 69 + > .../mtk_cam-seninf-csi0-dphy.h | 139 + > .../mtk_cam-seninf-hw_phy_2_0.c | 2879 +++++++++++++++++ > .../mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h | 257 ++ > .../mtk_cam-seninf-seninf1-csi2.h | 415 +++ > .../mtk_cam-seninf-seninf1-mux.h | 147 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-seninf1.h | 47 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-tg1.h | 49 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-top-ctrl.h | 99 + Move the phy part to phy/mediatek/ folder. You could refer to phy/mediatek/phy-mtk-mipi-csi-0-5.c Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.c | 5359 +++++++++++++++++ > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.h | 325 + > .../isp/isp_7x/camsys/mtk_cam-raw_debug.c | 403 ++ > .../isp/isp_7x/camsys/mtk_cam-raw_debug.h | 39 + > .../isp_7x/camsys/mtk_camera-v4l2-controls.h | 65 + Without debug, this driver could still work, so separate debug function to an independent patch. It seems yuv is an independent function. If so, separate yuv function to an independent patch. Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces support for the sensor interface in the MediaTek SoC, > with the focus on CSI and stream control. The key functionalities > include parameter control, metering and maintaining status information, > interrupt handling, and debugging. These features ensure effective > management and debugging of the camera sensor interface hardware. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +int mtk_cam_seninf_debug(struct seninf_ctx *ctx) > +{ Without debug, this driver could still work. So separate debug function to an independent patch. Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +static void set_fifo_threshold(void __iomem *dma_base) > +{ > + u32 fifo_size = 0; > + > + fifo_size = readl_relaxed(dma_base + DMA_OFFSET_CON0) & 0xFFF; > + writel_relaxed((0x1 << 28) | > + ((fifo_size * 1 / 4) & 0xFFF) << 16 | > + ((fifo_size * 1 / 8) & 0xFFF), > + dma_base + DMA_OFFSET_CON1); I can not find definition of DMA_OFFSET_CON1 in this patch, but I find it in [6/10] patch. Move the definition to this patch. And when you add this file, make sure you could build it successfully. Regards, CK > + writel_relaxed((0x1 << 28) | > + ((fifo_size * 1 / 2) & 0xFFF) << 16 | > + ((fifo_size * 3 / 8) & 0xFFF), > + dma_base + DMA_OFFSET_CON2); > + writel_relaxed((0x1 << 31) | > + ((fifo_size * 2 / 3) & 0xFFF) << 16 | > + ((fifo_size * 13 / 24) & 0xFFF), > + dma_base + DMA_OFFSET_CON3); > + writel_relaxed((0x1 << 31) | > + ((fifo_size * 3 / 8) & 0xFFF) << 16 | > + ((fifo_size * 1 / 4) & 0xFFF), > + dma_base + DMA_OFFSET_CON4); > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +static irqreturn_t mtk_thread_irq_raw(int irq, void *data) > +{ > + struct mtk_raw_device *raw_dev = (struct mtk_raw_device *)data; > + struct mtk_camsys_irq_info irq_info; > + > + if (unlikely(atomic_cmpxchg(&raw_dev->is_fifo_overflow, 1, 0))) > + dev_info(raw_dev->dev, "msg fifo overflow\n"); > + > + while (kfifo_len(&raw_dev->msg_fifo) >= sizeof(irq_info)) { > + int len = kfifo_out(&raw_dev->msg_fifo, &irq_info, sizeof(irq_info)); > + > + WARN_ON(len != sizeof(irq_info)); > + > + dev_dbg(raw_dev->dev, "ts=%llu irq_type %d, req:%d/%d\n", > + irq_info.ts_ns, irq_info.irq_type, > + irq_info.frame_idx_inner, irq_info.frame_idx); > + > + /* error case */ > + if (unlikely(irq_info.irq_type == (1 << CAMSYS_IRQ_ERROR))) { > + raw_handle_error(raw_dev, &irq_info); > + continue; > + } > + > + /* normal case */ > + /* inform interrupt information to camsys controller */ > + mtk_camsys_isr_event(raw_dev->cam, CAMSYS_ENGINE_RAW, > + raw_dev->id, &irq_info); mtk_camsys_isr_event() is not defined in this patch but defined in [8/10] patch. This is weird to use a function defined in future. Define this function in this patch or previous patch. Regards, CK > + } > + > + return IRQ_HANDLED; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +/* feature mask to categorize all raw functions */ > +#define MTK_CAM_FEATURE_HDR_MASK 0x0000000F > +#define MTK_CAM_FEATURE_SUBSAMPLE_MASK 0x000000F0 > +#define MTK_CAM_FEATURE_OFFLINE_M2M_MASK 0x00000100 > +#define MTK_CAM_FEATURE_PURE_OFFLINE_M2M_MASK 0x00000200 It seems that M2M is not basic function. It's an advanced function, so separate M2M related code to an independent patch. Make the first patch as simple as possible. Regards, CK > + > +enum raw_function_id { > + /* bit [0~3] hdr */ > + /* bit [4~7] fps */ > + /* bit [8~9] m2m */ > + OFFLINE_M2M = (1 << 8), > + PURE_OFFLINE_M2M = (1 << 9), > + RAW_FUNCTION_END = 0xF0000000, > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* > + * V 4 L 2 M E T A B U F F E R L A Y O U T > + */ > + > +/* > + * struct mtk_cam_uapi_meta_raw_stats_cfg > + * > + * @ae_awb_enable: To indicate if AE and AWB should be enblaed or not. If > + * it is 1, it means that we enable the following parts of > + * hardware: > + * (1) AE/AWB > + * (2) aao > + * (3) aaho > + * @af_enable: To indicate if AF should be enabled or not. If it is 1, > + * it means that the AF and afo is enabled. > + * @dgn_enable: To indicate if dgn module should be enabled or not. > + * @flk_enable: If it is 1, it means flk and flko is enable. If ie is 0, > + * both flk and flko is disabled. > + * @tsf_enable: If it is 1, it means tsfs and tsfso is enable. If ie is 0, > + * both tsfs and tsfso is disabled. > + * @wb_enable: To indicate if wb module should be enabled or not. > + * @pde_enable: To indicate if pde module should be enabled or not. > + * @ae_param: AE Statistic window config > + * @awb_param: AWB statistic configuration control > + * @dgn_param: DGN settings > + * @flk_param: Flicker statistic configuration > + * @tsf_param: tsf statistic configuration > + * @wb_param: WB settings > + * @pde_param: pde settings > + */ > +struct mtk_cam_uapi_meta_raw_stats_cfg { struct mtk_cam_uapi_meta_raw_stats_cfg{} is useless, so drop it. Regards, CK > + s8 ae_awb_enable; > + s8 af_enable; > + s8 dgn_enable; > + s8 flk_enable; > + s8 tsf_enable; > + s8 wb_enable; > + s8 pde_enable; > + u8 rsv; > + > + struct mtk_cam_uapi_ae_param ae_param; > + struct mtk_cam_uapi_awb_param awb_param; > + struct mtk_cam_uapi_af_param af_param; > + struct mtk_cam_uapi_dgn_param dgn_param; > + struct mtk_cam_uapi_flk_param flk_param; > + struct mtk_cam_uapi_tsf_param tsf_param; > + struct mtk_cam_uapi_wb_param wb_param; > + struct mtk_cam_uapi_pde_param pde_param; > + > + struct mtk_cam_uapi_prot { > + /* The following top control are generated by script */ > + u8 drzh2n_r1_tuning_enable; > + u8 drzh2n_r2_tuning_enable; > + u8 drzh2n_r3_tuning_enable; > + u8 drzh2n_r4_tuning_enable; > + u8 drzh2n_r5_tuning_enable; > + u8 drzh2n_r6_tuning_enable; > + u8 drzh2n_r7_tuning_enable; > + u8 drzh2n_r8_tuning_enable; > + u8 drzs4n_r1_tuning_enable; > + u8 drzs4n_r2_tuning_enable; > + u8 drzs4n_r3_tuning_enable; > + u8 dm_tuning_enable; > + u8 drzs8t_r1_tuning_enable; > + u8 drzs8t_r2_tuning_enable; > + u8 ggm_r1_tuning_enable; > + u8 ggm_r2_tuning_enable; > + u8 ggm_r3_tuning_enable; > + u8 bpc_r1_enable; > + u8 bpc_r2_enable; > + u8 ccm_r1_enable; > + u8 ccm_r2_enable; > + u8 ccm_r3_enable; > + u8 fus_enable; > + u8 g2c_r1_enable; > + u8 g2c_r2_enable; > + u8 g2c_r3_enable; > + u8 hlr_enable; > + u8 lsc_enable; > + u8 ltm_enable; > + u8 ltms_enable; > + u8 obc_r1_enable; > + u8 obc_r2_enable; > + u8 tcy_r1_enable; > + u8 tcy_r2_enable; > + u8 tcy_r3_enable; > + u8 tncs_r1_enable; > + > + struct mtk_cam_uapi_ccm_param_prot ccm_r1_param; > + struct mtk_cam_uapi_ccm_param_prot ccm_r2_param; > + struct mtk_cam_uapi_ccm_param_prot ccm_r3_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r1_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r2_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r3_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r4_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r5_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r6_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r7_param; > + struct mtk_cam_uapi_drzh2n_param_prot drzh2n_r8_param; > + struct mtk_cam_uapi_drzs4n_param_prot drzs4n_r1_param; > + struct mtk_cam_uapi_drzs4n_param_prot drzs4n_r2_param; > + struct mtk_cam_uapi_drzs4n_param_prot drzs4n_r3_param; > + struct mtk_cam_uapi_tncs_param_prot tncs_param; > + /* script generation done */ > + struct mtk_cam_uapi_drzs8t_param_prot drzs8t_r1_param; > + struct mtk_cam_uapi_drzs8t_param_prot drzs8t_r2_param; > + struct mtk_cam_uapi_awb_param_prot awb_param; > + struct mtk_cam_uapi_bpc_param_prot bpc_param; > + struct mtk_cam_uapi_lsc_param_prot lsc_param; > + struct mtk_cam_uapi_slk_param_prot slk_param; > + struct mtk_cam_uapi_wb_param_prot wb_param; > + struct mtk_cam_uapi_ltms_param_prot ltms_param; > + struct mtk_cam_uapi_yuvo_param_prot yuvo_r2_param; > + struct mtk_cam_uapi_yuvo_param_prot yuvo_r4_param; > + /* The following module stuctures are generated by script */ > + struct mtk_cam_uapi_regmap_raw_bpc bpc_r1; > + struct mtk_cam_uapi_regmap_raw_bpc bpc_r2; > + struct mtk_cam_uapi_regmap_raw_ccm ccm_r1; > + struct mtk_cam_uapi_regmap_raw_ccm ccm_r2; > + struct mtk_cam_uapi_regmap_raw_ccm ccm_r3; > + struct mtk_cam_uapi_regmap_raw_dm dm_r1; > + u8 rsv1[116]; > + struct mtk_cam_uapi_regmap_raw_g2c g2c_r1; > + struct mtk_cam_uapi_regmap_raw_g2c g2c_r2; > + struct mtk_cam_uapi_regmap_raw_g2c g2c_r3; > + struct mtk_cam_uapi_regmap_raw_ggm ggm_r1; > + struct mtk_cam_uapi_regmap_raw_ggm ggm_r2; > + struct mtk_cam_uapi_regmap_raw_ggm ggm_r3; > + u8 rsv2[68]; > + struct mtk_cam_uapi_regmap_raw_lsc lsc_r1; > + struct mtk_cam_uapi_regmap_raw_ltm ltm_r1; > + struct mtk_cam_uapi_regmap_raw_ltms ltms_r1; > + struct mtk_cam_uapi_regmap_raw_obc obc_r1; > + struct mtk_cam_uapi_regmap_raw_obc obc_r2; > + u8 rsv3[1420]; > + struct mtk_cam_uapi_regmap_raw_tsfs tsfs_r1; > + u8 rsv4[50080]; > + /* script generation done */ > + } __packed prot; > +} __packed; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +static int mtk_cam_fill_img_buf(struct mtkcam_ipi_img_output *img_out, > + const struct v4l2_format *f, dma_addr_t daddr) > +{ > + u32 pixelformat = f->fmt.pix_mp.pixelformat; > + u32 width = f->fmt.pix_mp.width; > + u32 height = f->fmt.pix_mp.height; > + const struct v4l2_plane_pix_format *plane = &f->fmt.pix_mp.plane_fmt[0]; > + u32 stride = plane->bytesperline; > + u32 aligned_width; > + unsigned int addr_offset = 0; > + int i; > + > + if (is_mtk_format(pixelformat)) { > + const struct mtk_format_info *info; > + > + info = mtk_format_info(pixelformat); > + if (!info) > + return -EINVAL; > + > + aligned_width = stride / info->bpp[0]; > + if (info->mem_planes == 1) { > + if (is_yuv_ufo(pixelformat)) { > + aligned_width = ALIGN(width, 64); > + img_out->buf[0][0].iova = daddr; > + img_out->fmt.stride[0] = aligned_width * info->bit_r_num > + / info->bit_r_den; > + img_out->buf[0][0].size = img_out->fmt.stride[0] * height; > + img_out->buf[0][0].size += img_out->fmt.stride[0] * height / 2; > + img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height; > + img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height > + / 2; > + img_out->buf[0][0].size += sizeof(struct ufbc_buffer_header); > + > + pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n", > + 0, img_out->fmt.stride[0], img_out->buf[0][0].size, > + (unsigned long)img_out->buf[0][0].iova); > + } else if (is_raw_ufo(pixelformat)) { > + aligned_width = ALIGN(width, 64); > + img_out->buf[0][0].iova = daddr; > + img_out->fmt.stride[0] = aligned_width * info->bit_r_num / > + info->bit_r_den; > + img_out->buf[0][0].size = img_out->fmt.stride[0] * height; > + img_out->buf[0][0].size += ALIGN((aligned_width / 64), 8) * height; > + img_out->buf[0][0].size += sizeof(struct ufbc_buffer_header); > + > + pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n", > + 0, img_out->fmt.stride[0], > + img_out->buf[0][0].size, > + (unsigned long)img_out->buf[0][0].iova); > + } else { > + for (i = 0; i < info->comp_planes; i++) { > + unsigned int hdiv = (i == 0) ? 1 : info->hdiv; > + unsigned int vdiv = (i == 0) ? 1 : info->vdiv; > + > + img_out->buf[0][i].iova = daddr + addr_offset; > + img_out->fmt.stride[i] = info->bpp[i] * > + DIV_ROUND_UP(aligned_width, hdiv); > + img_out->buf[0][i].size = img_out->fmt.stride[i] > + * DIV_ROUND_UP(height, vdiv); > + addr_offset += img_out->buf[0][i].size; > + > + pr_debug("plane:%d stride:%d plane_size:%d addr:0x%lx\n", > + i, img_out->fmt.stride[i], > + img_out->buf[0][i].size, > + (unsigned long)img_out->buf[0][i].iova); > + } > + } > + } else { > + pr_debug("do not support non contiguous mplane\n"); > + } > + } else { > + const struct v4l2_format_info *info; > + > + info = v4l2_format_info(pixelformat); > + if (!info) > + return -EINVAL; > + > + aligned_width = stride / info->bpp[0]; > + if (info->mem_planes == 1) { > + for (i = 0; i < info->comp_planes; i++) { This for-loop is identical with the for-loop in is_mtk_format-is-true block. So the checking of is_mtk_format is redundant. Regards, CK > + unsigned int hdiv = (i == 0) ? 1 : info->hdiv; > + unsigned int vdiv = (i == 0) ? 1 : info->vdiv; > + > + img_out->buf[0][i].iova = daddr + addr_offset; > + img_out->fmt.stride[i] = info->bpp[i] * > + DIV_ROUND_UP(aligned_width, hdiv); > + img_out->buf[0][i].size = img_out->fmt.stride[i] > + * DIV_ROUND_UP(height, vdiv); > + addr_offset += img_out->buf[0][i].size; > + > + pr_debug("stride:%d plane_size:%d addr:0x%lx\n", > + img_out->fmt.stride[i], > + img_out->buf[0][i].size, > + (unsigned long)img_out->buf[0][i].iova); > + } > + } else { > + pr_debug("do not support non contiguous mplane\n"); > + } > + } > + > + return 0; > +} > +
Hi, Le mercredi 09 octobre 2024 à 19:15 +0800, Shu-hsiang Yang a écrit : > Based on linux-next/master, tag: next-20241008 > > The patch set adds the MediaTek ISP7.x camera system hardware driver. > > This driver sets up ISP hardware, handles interrupts, and initializes > V4L2 device nodes and functions. Moreover, implement V4L2 standard > video driver that utilizes media framework APIs. It also connects > the sensors and ISP, bridging with the seninf interface. Communicate > with SCP co-processor to compose ISP registers in the firmware. Thanks for this work. If I read you correctly, this will depends on an scp firmware update. Its probably early at rev1, but we appreciate if you can cross- reference your associated linux-firmware submission (or document which MT flavour of SCP firmware) contains this support already. For ISP drivers reviewers, we have the experience with SCP that the IPC is not versioned, making it difficult to maintain backward compatibility. Won't matter until this is merged, but be aware that unless this SCP IPC design issue has been fixed, any change to the -if headers means a modification to the SCP has been made, and often we have had to ask MTK to revisit the firmware code to unbreak backward compatibility. If this specific part of the SCP IPC is versioned, please let us know. I don't want this to be alarming message, just something we need to collectively be aware of not to break userspace, which may not update their firmwares in lock step with the kernel. regards, Nicolas p.s. for those new to MTK architecture, the SCP firmware provides services to everything multimedia in the platform, including CODECs, color converter and scalers, etc. > > These patches include CSI received data from sensors, sensor interface > bridge, raw/YUV image pre-processing, ISP utility and ISP control parts. > > Thank you for reviewing these patches. > > Shu-hsiang Yang (10): > dt-bindings: media: mediatek: add camsys device > media: platform: mediatek: add seninf controller > media: platform: mediatek: add isp_7x seninf unit > media: platform: mediatek: add isp_7x cam-raw unit > media: platform: mediatek: add isp_7x camsys unit > media: platform: mediatek: add isp_7x utility > media: platform: mediatek: add isp_7x video ops > media: platform: mediatek: add isp_7x state ctrl > media: platform: mediatek: add isp_7x build config > uapi: linux: add mediatek isp_7x camsys user api > > .../media/mediatek/mediatek,cam-raw.yaml | 169 + > .../media/mediatek/mediatek,cam-yuv.yaml | 148 + > .../media/mediatek/mediatek,camisp.yaml | 71 + > .../media/mediatek/mediatek,seninf-core.yaml | 106 + > .../media/mediatek/mediatek,seninf.yaml | 88 + > drivers/media/platform/mediatek/Kconfig | 1 + > drivers/media/platform/mediatek/Makefile | 2 + > drivers/media/platform/mediatek/isp/Kconfig | 21 + > .../platform/mediatek/isp/isp_7x/Makefile | 7 + > .../mediatek/isp/isp_7x/camsys/Makefile | 16 + > .../isp_7x/camsys/kd_imgsensor_define_v4l2.h | 87 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c | 1797 ++++++ > .../mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.h | 140 + > .../isp/isp_7x/camsys/mtk_cam-debug.c | 1271 ++++ > .../isp/isp_7x/camsys/mtk_cam-debug.h | 273 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-defs.h | 168 + > .../isp/isp_7x/camsys/mtk_cam-dmadbg.h | 721 +++ > .../isp/isp_7x/camsys/mtk_cam-feature.c | 40 + > .../isp/isp_7x/camsys/mtk_cam-feature.h | 26 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-fmt.h | 87 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-ipi.h | 233 + > .../isp/isp_7x/camsys/mtk_cam-meta-mt8188.h | 2436 ++++++++ > .../isp/isp_7x/camsys/mtk_cam-plat-util.c | 207 + > .../isp/isp_7x/camsys/mtk_cam-plat-util.h | 16 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-pool.c | 393 ++ > .../mediatek/isp/isp_7x/camsys/mtk_cam-pool.h | 28 + > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.c | 5359 +++++++++++++++++ > .../mediatek/isp/isp_7x/camsys/mtk_cam-raw.h | 325 + > .../isp/isp_7x/camsys/mtk_cam-raw_debug.c | 403 ++ > .../isp/isp_7x/camsys/mtk_cam-raw_debug.h | 39 + > .../isp/isp_7x/camsys/mtk_cam-regs-mt8188.h | 382 ++ > .../isp/isp_7x/camsys/mtk_cam-seninf-def.h | 193 + > .../isp/isp_7x/camsys/mtk_cam-seninf-drv.c | 1741 ++++++ > .../isp/isp_7x/camsys/mtk_cam-seninf-drv.h | 16 + > .../isp/isp_7x/camsys/mtk_cam-seninf-hw.h | 120 + > .../isp/isp_7x/camsys/mtk_cam-seninf-if.h | 28 + > .../isp/isp_7x/camsys/mtk_cam-seninf-regs.h | 40 + > .../isp/isp_7x/camsys/mtk_cam-seninf-route.c | 356 ++ > .../isp/isp_7x/camsys/mtk_cam-seninf-route.h | 23 + > .../isp/isp_7x/camsys/mtk_cam-seninf.h | 170 + > .../isp/isp_7x/camsys/mtk_cam-timesync.c | 125 + > .../isp/isp_7x/camsys/mtk_cam-timesync.h | 12 + > .../isp/isp_7x/camsys/mtk_cam-ufbc-def.h | 59 + > .../isp/isp_7x/camsys/mtk_cam-video.c | 1817 ++++++ > .../isp/isp_7x/camsys/mtk_cam-video.h | 224 + > .../mediatek/isp/isp_7x/camsys/mtk_cam.c | 4168 +++++++++++++ > .../mediatek/isp/isp_7x/camsys/mtk_cam.h | 733 +++ > .../isp_7x/camsys/mtk_camera-v4l2-controls.h | 65 + > .../isp_7x/camsys/mtk_csi_phy_2_0/Makefile | 5 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h | 911 +++ > .../mtk_cam-seninf-csi0-cphy.h | 69 + > .../mtk_cam-seninf-csi0-dphy.h | 139 + > .../mtk_cam-seninf-hw_phy_2_0.c | 2879 +++++++++ > .../mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h | 257 + > .../mtk_cam-seninf-seninf1-csi2.h | 415 ++ > .../mtk_cam-seninf-seninf1-mux.h | 147 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-seninf1.h | 47 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-tg1.h | 49 + > .../mtk_csi_phy_2_0/mtk_cam-seninf-top-ctrl.h | 99 + > include/uapi/linux/mtkisp_camsys.h | 227 + > 60 files changed, 30194 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/mediatek/mediatek,cam-raw.yaml > create mode 100644 Documentation/devicetree/bindings/media/mediatek/mediatek,cam-yuv.yaml > create mode 100644 Documentation/devicetree/bindings/media/mediatek/mediatek,camisp.yaml > create mode 100644 Documentation/devicetree/bindings/media/mediatek/mediatek,seninf-core.yaml > create mode 100644 Documentation/devicetree/bindings/media/mediatek/mediatek,seninf.yaml > create mode 100644 drivers/media/platform/mediatek/isp/Kconfig > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/Makefile > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/Makefile > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/kd_imgsensor_define_v4l2.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ctrl.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-debug.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-debug.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-defs.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-dmadbg.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-feature.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-feature.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-fmt.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ipi.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-meta-mt8188.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-plat-util.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-plat-util.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-pool.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-pool.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw_debug.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-raw_debug.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-regs-mt8188.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-def.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-drv.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-drv.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-hw.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-if.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-regs.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-route.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf-route.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-seninf.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-timesync.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-timesync.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-ufbc-def.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-video.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam-video.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_camera-v4l2-controls.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/Makefile > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-cammux.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-csi0-cphy.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-csi0-dphy.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-hw_phy_2_0.c > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-mipi-rx-ana-cdphy-csi0a.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1-csi2.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1-mux.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-seninf1.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-tg1.h > create mode 100644 drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_csi_phy_2_0/mtk_cam-seninf-top-ctrl.h > create mode 100644 include/uapi/linux/mtkisp_camsys.h >
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces state management and debugging mechanisms for the MediaTek > ISP7.x camsys platform. State management establishes control over ISP > operations and events, defining distinct states for request handling, > sensor control, and frame synchronization, ensuring event processing. > The debugging mechanism ensures stable operation and timely data > collection during anomalies. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static bool mtk_cam_req_frame_sync_start(struct mtk_cam_request *req) > +{ > + /* All ctx with sensor is in ready state */ > + struct mtk_cam_device *cam = > + container_of(req->req.mdev, struct mtk_cam_device, media_dev); > + struct mtk_cam_ctx *ctx; > + struct mtk_cam_ctx *sync_ctx[MTKCAM_SUBDEV_MAX]; > + int i; > + u32 ctx_cnt = 0, synced_cnt = 0; > + bool ret = false; > + > + /* pick out the used ctxs */ > + for (i = 0; i < cam->max_stream_num; i++) { > + if (!(1 << i & req->ctx_used)) > + continue; > + > + sync_ctx[ctx_cnt] = &cam->ctxs[i]; > + ctx_cnt++; > + } > + > + mutex_lock(&req->fs.op_lock); > + if (ctx_cnt > 1) { > + /* multi sensor case */ multi sensor is advanced function, so separate multi sensor related code to a multi sensor patch. Regards, CK > + req->fs.on_cnt++; > + /* not first time */ > + if (req->fs.on_cnt != 1) > + goto EXIT; > + > + for (i = 0; i < ctx_cnt; i++) { > + ctx = sync_ctx[i]; > + spin_lock(&ctx->streaming_lock); > + if (!ctx->streaming) { > + spin_unlock(&ctx->streaming_lock); > + dev_info(cam->dev, > + "%s: ctx(%d): is streamed off\n", > + __func__, ctx->stream_id); > + continue; > + } > + spin_unlock(&ctx->streaming_lock); > + > + /* update sensor frame sync */ > + if (ctx->synced) > + synced_cnt++; > + } > + > + /* > + * the prepared sensor is no enough, skip > + * frame sync set failed or stream off > + */ > + if (synced_cnt < 2) { > + mtk_cam_fs_reset(&req->fs); > + dev_info(cam->dev, "%s:%s: sensor is not ready\n", > + __func__, req->req.debug_str); > + goto EXIT; > + } > + > + dev_dbg(cam->dev, "%s:%s:fs_sync_frame(1): ctxs: 0x%x\n", > + __func__, req->req.debug_str, req->ctx_used); > + > + ret = true; > + goto EXIT; > + } > + /* single sensor case: unsupported sensor hardware sync */ > + > +EXIT: > + dev_dbg(cam->dev, "%s:%s:target/on/off(%d/%d/%d)\n", __func__, > + req->req.debug_str, req->fs.target, req->fs.on_cnt, > + req->fs.off_cnt); > + mutex_unlock(&req->fs.op_lock); > + return ret; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static int mtk_cam_probe(struct platform_device *pdev) > +{ > + struct mtk_cam_device *cam_dev; > + struct device *dev = &pdev->dev; > + struct resource *res; > + int ret; > + unsigned int i; > + > + dev_dbg(dev, "camsys | start %s\n", __func__); > + > + /* initialize structure */ > + cam_dev = devm_kzalloc(dev, sizeof(*cam_dev), GFP_KERNEL); > + if (!cam_dev) > + return -ENOMEM; > + > + if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34))) { > + dev_err(dev, "%s: No suitable DMA available\n", __func__); > + return -EIO; > + } > + > + if (!dev->dma_parms) { > + dev->dma_parms = > + devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL); > + if (!dev->dma_parms) > + return -ENOMEM; > + } > + > + dma_set_max_seg_size(dev, UINT_MAX); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(dev, "failed to get mem\n"); > + return -ENODEV; > + } > + > + cam_dev->base = devm_ioremap_resource(dev, res); I can not find any where to write register of this device. If so, I think we need not to probe this driver. The rest software control can be setup by other driver. Regards, CK > + if (IS_ERR(cam_dev->base)) { > + dev_err(dev, "failed to map register base\n"); > + return PTR_ERR(cam_dev->base); > + } > + > + cam_dev->dev = dev; > + dev_set_drvdata(dev, cam_dev); > + > + cam_dev->composer_cnt = 0; > + cam_dev->num_seninf_devices = 0; > + > + cam_dev->max_stream_num = MTKCAM_SUBDEV_MAX; > + cam_dev->ctxs = devm_kcalloc(dev, cam_dev->max_stream_num, > + sizeof(*cam_dev->ctxs), GFP_KERNEL); > + if (!cam_dev->ctxs) > + return -ENOMEM; > + > + cam_dev->streaming_ctx = 0; > + for (i = 0; i < cam_dev->max_stream_num; i++) > + mtk_cam_ctx_init(cam_dev->ctxs + i, cam_dev, i); > + > + cam_dev->running_job_count = 0; > + spin_lock_init(&cam_dev->pending_job_lock); > + spin_lock_init(&cam_dev->running_job_lock); > + INIT_LIST_HEAD(&cam_dev->pending_job_list); > + INIT_LIST_HEAD(&cam_dev->running_job_list); > + > + cam_dev->dma_processing_count = 0; > + spin_lock_init(&cam_dev->dma_pending_lock); > + spin_lock_init(&cam_dev->dma_processing_lock); > + INIT_LIST_HEAD(&cam_dev->dma_pending); > + INIT_LIST_HEAD(&cam_dev->dma_processing); > + > + mutex_init(&cam_dev->queue_lock); > + > + pm_runtime_enable(dev); > + > + ret = mtk_cam_of_rproc(cam_dev, pdev); > + if (ret) > + goto fail_destroy_mutex; > + > + ret = register_sub_drivers(dev); > + if (ret) { > + dev_err(dev, "fail to register_sub_drivers\n"); > + goto fail_destroy_mutex; > + } > + > + /* register mtk_cam as all isp subdev async parent */ > + cam_dev->notifier.ops = &mtk_cam_async_nf_ops; > + v4l2_async_nf_init(&cam_dev->notifier, &cam_dev->v4l2_dev); > + ret = mtk_cam_async_subdev_add(dev); /* wait all isp sub drivers */ > + if (ret) { > + dev_err(dev, "%s failed mtk_cam_async_subdev_add\n", __func__); > + goto fail_unregister_sub_drivers; > + } > + > + ret = v4l2_async_nf_register(&cam_dev->notifier); > + if (ret) { > + dev_err(dev, "%s async_nf_register ret:%d\n", __func__, ret); > + v4l2_async_nf_cleanup(&cam_dev->notifier); > + goto fail_unregister_sub_drivers; > + } > + > + ret = mtk_cam_debug_fs_init(cam_dev); > + if (ret < 0) > + goto fail_unregister_async_nf; > + > + dev_info(dev, "camsys | [%s] success\n", __func__); > + > + return 0; > + > +fail_unregister_async_nf: > + v4l2_async_nf_unregister(&cam_dev->notifier); > + > +fail_unregister_sub_drivers: > + unregister_sub_drivers(dev); > + > +fail_destroy_mutex: > + mutex_destroy(&cam_dev->queue_lock); > + > + return ret; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces support for the sensor interface in the MediaTek SoC, > with the focus on CSI and stream control. The key functionalities > include parameter control, metering and maintaining status information, > interrupt handling, and debugging. These features ensure effective > management and debugging of the camera sensor interface hardware. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +int mtk_cam_seninf_init_iomem(struct seninf_ctx *ctx, void __iomem *if_base, > + void __iomem *ana_base) > +{ This function is defined in this patch but only used by patch [3/10]. There are many function defined in this patch but used only in patch [3/10]. I think this patch and patch [3/10] should be merged into one patch which is a complete sensor interface driver. To prevent this patch to be too big, separate advanced function to another patches. Regards, CK > + u32 i; > + > + ctx->reg_ana_csi_rx[CSI_PORT_0] = > + ctx->reg_ana_csi_rx[CSI_PORT_0A] = ana_base + 0; > + ctx->reg_ana_csi_rx[CSI_PORT_0B] = ana_base + 0x1000; > + > + ctx->reg_ana_csi_rx[CSI_PORT_1] = > + ctx->reg_ana_csi_rx[CSI_PORT_1A] = ana_base + 0x4000; > + ctx->reg_ana_csi_rx[CSI_PORT_1B] = ana_base + 0x5000; > + > + ctx->reg_ana_csi_rx[CSI_PORT_2] = > + ctx->reg_ana_csi_rx[CSI_PORT_2A] = ana_base + 0x8000; > + ctx->reg_ana_csi_rx[CSI_PORT_2B] = ana_base + 0x9000; > + > + ctx->reg_ana_csi_rx[CSI_PORT_3] = > + ctx->reg_ana_csi_rx[CSI_PORT_3A] = ana_base + 0xc000; > + ctx->reg_ana_csi_rx[CSI_PORT_3B] = ana_base + 0xd000; > + > + ctx->reg_ana_dphy_top[CSI_PORT_0A] = > + ctx->reg_ana_dphy_top[CSI_PORT_0B] = > + ctx->reg_ana_dphy_top[CSI_PORT_0] = ana_base + 0x2000; > + > + ctx->reg_ana_dphy_top[CSI_PORT_1A] = > + ctx->reg_ana_dphy_top[CSI_PORT_1B] = > + ctx->reg_ana_dphy_top[CSI_PORT_1] = ana_base + 0x6000; > + > + ctx->reg_ana_dphy_top[CSI_PORT_2A] = > + ctx->reg_ana_dphy_top[CSI_PORT_2B] = > + ctx->reg_ana_dphy_top[CSI_PORT_2] = ana_base + 0xa000; > + > + ctx->reg_ana_dphy_top[CSI_PORT_3A] = > + ctx->reg_ana_dphy_top[CSI_PORT_3B] = > + ctx->reg_ana_dphy_top[CSI_PORT_3] = ana_base + 0xe000; > + > + ctx->reg_ana_cphy_top[CSI_PORT_0A] = > + ctx->reg_ana_cphy_top[CSI_PORT_0B] = > + ctx->reg_ana_cphy_top[CSI_PORT_0] = ana_base + 0x3000; > + > + ctx->reg_ana_cphy_top[CSI_PORT_1A] = > + ctx->reg_ana_cphy_top[CSI_PORT_1B] = > + ctx->reg_ana_cphy_top[CSI_PORT_1] = ana_base + 0x7000; > + > + ctx->reg_ana_cphy_top[CSI_PORT_2A] = > + ctx->reg_ana_cphy_top[CSI_PORT_2B] = > + ctx->reg_ana_cphy_top[CSI_PORT_2] = ana_base + 0xb000; > + > + ctx->reg_ana_cphy_top[CSI_PORT_3A] = > + ctx->reg_ana_cphy_top[CSI_PORT_3B] = > + ctx->reg_ana_cphy_top[CSI_PORT_3] = ana_base + 0xf000; > + > + ctx->reg_if_top = if_base; > + > + for (i = SENINF_1; i < _seninf_cfg.seninf_num; i++) { > + ctx->reg_if_ctrl[i] = if_base + 0x0200 + (0x1000 * i); > + ctx->reg_if_tg[i] = if_base + 0x0F00 + (0x1000 * i); > + ctx->reg_if_csi2[i] = if_base + 0x0a00 + (0x1000 * i); > + } > + > + for (i = SENINF_MUX1; i < _seninf_cfg.mux_num; i++) > + ctx->reg_if_mux[i] = if_base + 0x0d00 + (0x1000 * i); > + > + ctx->reg_if_cam_mux = if_base + 0x0400; > + > + return 0; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* > + * A U T O W H I T E B A L A N C E > + */ > + > +/* Maximum blocks that MediaTek AWB supports */ > +#define MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM (10) > + > +/* > + * struct mtk_cam_uapi_awb_param - parameters for AWB configurtion > + * > + * @stat_en: AWB stat enable > + * @windownum_x: Number of horizontal AWB windows > + * @windownum_y: Number of vertical AWB windows > + * @lowthreshold_r: Low threshold of R > + * @lowthreshold_g: Low threshold of G > + * @lowthreshold_b: Low threshold of B > + * @highthreshold_r: High threshold of R > + * @highthreshold_g: High threshold of G > + * @highthreshold_b: High threshold of B > + * @lightsrc_lowthreshold_r: Low threshold of R for light source estimation > + * @lightsrc_lowthreshold_g: Low threshold of G for light source estimation > + * @lightsrc_lowthreshold_b: Low threshold of B for light source estimation > + * @lightsrc_highthreshold_r: High threshold of R for light source estimation > + * @lightsrc_highthreshold_g: High threshold of G for light source estimation > + * @lightsrc_highthreshold_b: High threshold of B for light source estimation > + * @pregainlimit_r: Maximum limit clipping for R color > + * @pregainlimit_g: Maximum limit clipping for G color > + * @pregainlimit_b: Maximum limit clipping for B color > + * @pregain_r: unit module compensation gain for R color > + * @pregain_g: unit module compensation gain for G color > + * @pregain_b: unit module compensation gain for B color > + * @valid_datawidth: valid bits of statistic data > + * @hdr_support_en: support HDR mode > + * @stat_mode: Output format select <1>sum mode <0>average mode > + * @error_ratio: Programmable error pixel count by AWB window size > + * (base : 256) > + * @awbxv_win_r: light area of right bound, the size is defined in > + * MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM > + * @awbxv_win_l: light area of left bound the size is defined in > + * MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM > + * @awbxv_win_d: light area of lower bound the size is defined in > + * MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM > + * @awbxv_win_u: light area of upper bound the size is defined in > + * MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM > + * @pregain2_r: white balance gain of R color > + * @pregain2_g: white balance gain of G color > + * @pregain2_b: white balance gain of B color > + */ > +struct mtk_cam_uapi_awb_param { > + u32 stat_en; > + u32 windownum_x; windownum_x is useless, so drop it. Regards, CK > + u32 windownum_y; > + u32 lowthreshold_r; > + u32 lowthreshold_g; > + u32 lowthreshold_b; > + u32 highthreshold_r; > + u32 highthreshold_g; > + u32 highthreshold_b; > + u32 lightsrc_lowthreshold_r; > + u32 lightsrc_lowthreshold_g; > + u32 lightsrc_lowthreshold_b; > + u32 lightsrc_highthreshold_r; > + u32 lightsrc_highthreshold_g; > + u32 lightsrc_highthreshold_b; > + u32 pregainlimit_r; > + u32 pregainlimit_g; > + u32 pregainlimit_b; > + u32 pregain_r; > + u32 pregain_g; > + u32 pregain_b; > + u32 valid_datawidth; > + u32 hdr_support_en; > + u32 stat_mode; > + u32 format_shift; > + u32 error_ratio; > + u32 postgain_r; > + u32 postgain_g; > + u32 postgain_b; > + u32 postgain2_hi_r; > + u32 postgain2_hi_g; > + u32 postgain2_hi_b; > + u32 postgain2_med_r; > + u32 postgain2_med_g; > + u32 postgain2_med_b; > + u32 postgain2_low_r; > + u32 postgain2_low_g; > + u32 postgain2_low_b; > + s32 awbxv_win_r[MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM]; > + s32 awbxv_win_l[MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM]; > + s32 awbxv_win_d[MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM]; > + s32 awbxv_win_u[MTK_CAM_UAPI_AWB_MAX_LIGHT_AREA_NUM]; > + u32 csc_ccm[9]; > + u32 acc; > + u32 med_region[4]; > + u32 low_region[4]; > + u32 pregain2_r; > + u32 pregain2_g; > + u32 pregain2_b; > +} __packed; > +
On 09/10/2024 13:15, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> ... > + > +static int mtk_yuv_of_probe(struct platform_device *pdev, > + struct mtk_yuv_device *drvdata) > +{ > + struct device *dev = &pdev->dev; > + struct resource *res; > + int irq, ret; > + int n_clks; > + > + ret = of_property_read_u32(dev->of_node, "mediatek,cam-id", > + &drvdata->id); > + if (ret) { > + dev_dbg(dev, "missing camid property\n"); Debug? Or error? > + return ret; > + } > + > + if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34))) { > + dev_err(dev, "%s: No suitable DMA available\n", __func__); > + return -EIO; > + } > + > + if (!dev->dma_parms) { > + dev->dma_parms = > + devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL); > + if (!dev->dma_parms) > + return -ENOMEM; > + } > + > + dma_set_max_seg_size(dev, UINT_MAX); > + > + /* base outer register */ > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base"); > + if (!res) { > + dev_err(dev, "failed to get mem\n"); > + return -ENODEV; > + } > + > + drvdata->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(drvdata->base)) { > + dev_dbg(dev, "failed to map register base\n"); Dbg? What? > + return PTR_ERR(drvdata->base); > + } > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(dev, "failed to get irq\n"); > + return -ENODEV; > + } > + > + ret = devm_request_irq(dev, irq, mtk_irq_yuv, 0, > + dev_name(dev), drvdata); > + if (ret) { > + dev_err(dev, "failed to request irq=%d\n", irq); > + return ret; > + } > + dev_dbg(dev, "registered irq=%d\n", irq); Drop > + > + n_clks = devm_clk_bulk_get_all(dev, &drvdata->clk_b); > + if (n_clks < 0) { > + dev_err(dev, "failed to devm_clk_bulk_get_all=%d\n", n_clks); Syntax is: return dev_err_probe() > + return n_clks; > + } > + > + drvdata->num_clks = n_clks; > + dev_info(dev, "clk_num:%d\n", drvdata->num_clks); Drop > + > +#ifdef CONFIG_PM_SLEEP > + drvdata->pm_notifier.notifier_call = yuv_pm_notifier; > + ret = register_pm_notifier(&drvdata->pm_notifier); > + if (ret) { > + dev_err(dev, "failed to register notifier block.\n"); > + return ret; > + } > +#endif > + > + return 0; > +} > + > +static int mtk_yuv_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_yuv_device *drvdata; > + struct v4l2_subdev *sd; > + int ret; > + > + dev_dbg(dev, "camsys | start %s\n", __func__); NAK > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->dev = dev; > + dev_set_drvdata(dev, drvdata); > + > + ret = mtk_yuv_of_probe(pdev, drvdata); > + if (ret) { > + dev_info(dev, "mtk_yuv_of_probe failed\n"); NAK. Neither info nor proper for memory allocation errors. Drop. > + return ret; > + } > + > + /* register yuv as mtk_cam async child */ > + sd = &drvdata->subdev; > + v4l2_subdev_init(sd, &mtk_raw_subdev_ops); > + sd->internal_ops = &mtk_raw_subdev_internal_ops; > + snprintf(sd->name, sizeof(sd->name), "%s", > + of_node_full_name(dev->of_node)); > + sd->dev = dev; > + sd->owner = THIS_MODULE; > + > + ret = v4l2_async_register_subdev(sd); > + if (ret) { > + dev_err(dev, "%s failed on async_register_subdev\n", __func__); > + return ret; > + } > + > + pm_runtime_enable(dev); > + > + dev_info(dev, "camsys | [%s] success\n", __func__); NAK > + > + return 0; > +} > + > +static void mtk_yuv_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_yuv_device *yuv_dev = dev_get_drvdata(dev); > + struct v4l2_subdev *sd = &yuv_dev->subdev; > + > + dev_dbg(dev, "camsys | start %s\n", __func__); NAK > + > + unregister_pm_notifier(&yuv_dev->pm_notifier); > + > + pm_runtime_disable(dev); > + > + v4l2_async_unregister_subdev(sd); > +} > + > +/* driver for yuv part */ > +static int mtk_yuv_runtime_suspend(struct device *dev) > +{ > + struct mtk_yuv_device *drvdata = dev_get_drvdata(dev); > + > + dev_info(dev, "%s:disable clock\n", __func__); NAK > + > + clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clk_b); > + > + return 0; > +} > + > +static int mtk_yuv_runtime_resume(struct device *dev) > +{ > + struct mtk_yuv_device *drvdata = dev_get_drvdata(dev); > + int ret; > + > + dev_info(dev, "%s:enable clock\n", __func__); NAK. Not even dev_dbg. Please don't ever post such code. > + > + ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clk_b); > + if (ret) { > + dev_info(dev, "failed at clk_bulk_prepare_enable, ret = %d\n", ret); > + clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clk_b); > + return ret; > + } > + > + return 0; > +} > + > +static const struct dev_pm_ops mtk_yuv_pm_ops = { > + SET_RUNTIME_PM_OPS(mtk_yuv_runtime_suspend, mtk_yuv_runtime_resume, > + NULL) > +}; > + > +static const struct of_device_id mtk_yuv_of_ids[] = { > + {.compatible = "mediatek,cam-yuv",}, > + {} > +}; > +MODULE_DEVICE_TABLE(of, mtk_yuv_of_ids); > + > +struct platform_driver mtk_cam_yuv_driver = { > + .probe = mtk_yuv_probe, > + .remove = mtk_yuv_remove, > + .driver = { > + .name = "mtk-cam yuv", > + .of_match_table = of_match_ptr(mtk_yuv_of_ids), Drop of_match_ptr(), you will have here warnings. > + .pm = &mtk_yuv_pm_ops, > + } > +}; ... > diff --git a/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_camera-v4l2-controls.h b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_camera-v4l2-controls.h > new file mode 100644 > index 000000000000..b775e6c30aa1 > --- /dev/null > +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_camera-v4l2-controls.h > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2022 MediaTek Inc. > + */ > + > +#ifndef __MTK_CAMERA_V4l2_CONTROLS_H > +#define __MTK_CAMERA_V4l2_CONTROLS_H > + > +#include <linux/videodev2.h> > +#include <linux/v4l2-controls.h> > +#include <linux/mtkisp_camsys.h> How these headers are used here? I don't see. Don't include unrelated stuff in your files. > + > +/* Allowed value of V4L2_CID_MTK_CAM_RAW_PATH_SELECT */ > +#define V4L2_MTK_CAM_RAW_PATH_SELECT_BPC 1 > +#define V4L2_MTK_CAM_RAW_PATH_SELECT_FUS 3 > +#define V4L2_MTK_CAM_RAW_PATH_SELECT_DGN 4 > +#define V4L2_MTK_CAM_RAW_PATH_SELECT_LSC 5 > +#define V4L2_MTK_CAM_RAW_PATH_SELECT_LTM 7 > + > +#define V4L2_MBUS_FRAMEFMT_PAD_ENABLE BIT(1) Best regards, Krzysztof
On 09/10/2024 13:15, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> ... > + > +static int mtk_cam_probe(struct platform_device *pdev) > +{ > + struct mtk_cam_device *cam_dev; > + struct device *dev = &pdev->dev; > + struct resource *res; > + int ret; > + unsigned int i; > + > + dev_dbg(dev, "camsys | start %s\n", __func__); NAK. Same issues. > + > + /* initialize structure */ > + cam_dev = devm_kzalloc(dev, sizeof(*cam_dev), GFP_KERNEL); > + if (!cam_dev) > + return -ENOMEM; > + > + if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34))) { > + dev_err(dev, "%s: No suitable DMA available\n", __func__); > + return -EIO; > + } > + > + if (!dev->dma_parms) { > + dev->dma_parms = > + devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL); > + if (!dev->dma_parms) > + return -ENOMEM; > + } > + > + dma_set_max_seg_size(dev, UINT_MAX); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(dev, "failed to get mem\n"); > + return -ENODEV; > + } > + > + cam_dev->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(cam_dev->base)) { > + dev_err(dev, "failed to map register base\n"); > + return PTR_ERR(cam_dev->base); > + } > + > + cam_dev->dev = dev; > + dev_set_drvdata(dev, cam_dev); > + > + cam_dev->composer_cnt = 0; > + cam_dev->num_seninf_devices = 0; > + > + cam_dev->max_stream_num = MTKCAM_SUBDEV_MAX; > + cam_dev->ctxs = devm_kcalloc(dev, cam_dev->max_stream_num, > + sizeof(*cam_dev->ctxs), GFP_KERNEL); > + if (!cam_dev->ctxs) > + return -ENOMEM; > + > + cam_dev->streaming_ctx = 0; > + for (i = 0; i < cam_dev->max_stream_num; i++) > + mtk_cam_ctx_init(cam_dev->ctxs + i, cam_dev, i); > + > + cam_dev->running_job_count = 0; > + spin_lock_init(&cam_dev->pending_job_lock); > + spin_lock_init(&cam_dev->running_job_lock); > + INIT_LIST_HEAD(&cam_dev->pending_job_list); > + INIT_LIST_HEAD(&cam_dev->running_job_list); > + > + cam_dev->dma_processing_count = 0; > + spin_lock_init(&cam_dev->dma_pending_lock); > + spin_lock_init(&cam_dev->dma_processing_lock); > + INIT_LIST_HEAD(&cam_dev->dma_pending); > + INIT_LIST_HEAD(&cam_dev->dma_processing); > + > + mutex_init(&cam_dev->queue_lock); > + > + pm_runtime_enable(dev); > + > + ret = mtk_cam_of_rproc(cam_dev, pdev); > + if (ret) > + goto fail_destroy_mutex; > + > + ret = register_sub_drivers(dev); > + if (ret) { > + dev_err(dev, "fail to register_sub_drivers\n"); > + goto fail_destroy_mutex; > + } > + > + /* register mtk_cam as all isp subdev async parent */ > + cam_dev->notifier.ops = &mtk_cam_async_nf_ops; > + v4l2_async_nf_init(&cam_dev->notifier, &cam_dev->v4l2_dev); > + ret = mtk_cam_async_subdev_add(dev); /* wait all isp sub drivers */ > + if (ret) { > + dev_err(dev, "%s failed mtk_cam_async_subdev_add\n", __func__); > + goto fail_unregister_sub_drivers; > + } > + > + ret = v4l2_async_nf_register(&cam_dev->notifier); > + if (ret) { > + dev_err(dev, "%s async_nf_register ret:%d\n", __func__, ret); > + v4l2_async_nf_cleanup(&cam_dev->notifier); > + goto fail_unregister_sub_drivers; > + } > + > + ret = mtk_cam_debug_fs_init(cam_dev); > + if (ret < 0) > + goto fail_unregister_async_nf; > + > + dev_info(dev, "camsys | [%s] success\n", __func__); NAK. Same issues. > + > + return 0; > + > +fail_unregister_async_nf: > + v4l2_async_nf_unregister(&cam_dev->notifier); > + > +fail_unregister_sub_drivers: > + unregister_sub_drivers(dev); > + > +fail_destroy_mutex: > + mutex_destroy(&cam_dev->queue_lock); > + > + return ret; > +} > + > +static void mtk_cam_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_cam_device *cam_dev = dev_get_drvdata(dev); > + > + pm_runtime_disable(dev); > + > + mtk_cam_debug_fs_deinit(cam_dev); > + > + v4l2_async_nf_unregister(&cam_dev->notifier); > + > + unregister_sub_drivers(dev); > + > + mutex_destroy(&cam_dev->queue_lock); > +} > + > +static const struct dev_pm_ops mtk_cam_pm_ops = { > + SET_RUNTIME_PM_OPS(mtk_cam_runtime_suspend, mtk_cam_runtime_resume, > + NULL) > +}; > + > +static struct platform_driver mtk_cam_driver = { > + .probe = mtk_cam_probe, > + .remove = mtk_cam_remove, > + .driver = { > + .name = "mtk-cam", > + .of_match_table = of_match_ptr(mtk_cam_of_ids), Same issues as in previous patch. All my comments apply to all your patches in this thread. > + .pm = &mtk_cam_pm_ops, > + } > +}; Best regards, Krzysztof
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static bool mtk_cam_raw_resource_calc(struct mtk_cam_device *cam, > + struct mtk_cam_resource_config *res, > + s64 pixel_rate, int res_plan, > + int in_w, int in_h, int *out_w, int *out_h) > +{ > + int res_step_type = 0; > + int tgo_pxl_mode = 1; > + int pixel_mode[MTK_CAMSYS_RES_STEP_NUM] = {0}; > + int bin_temp = 0, frz_temp = 0, hwn_temp = 0; > + int bin_en = 0, frz_en = 0, twin_en = 0, clk_cur = 0; > + int idx = 0, clk_res = 0, idx_res = 0; > + bool res_found = false; > + int lb_chk_res = -1; > + int frz_ratio = 100; > + int p; > + > + res->res_plan = res_plan; > + res->pixel_rate = pixel_rate; > + /* test pattern */ > + if (res->pixel_rate == 0) > + res->pixel_rate = 450 * MHz; > + dev_dbg(cam->dev, > + "[Res] PR = %lld, w/h=%d/%d HWN(%d)/BIN(%d)/FRZ(%d),Plan:%d\n", > + res->pixel_rate, in_w, in_h, > + res->hwn_limit_max, res->bin_limit, res->frz_limit, res->res_plan); > + > + memcpy(res->res_strategy, raw_resource_strategy_plan + res->res_plan, > + MTK_CAMSYS_RES_STEP_NUM * sizeof(int)); > + res->bin_enable = 0; > + res->raw_num_used = 1; > + res->frz_enable = 0; > + res->frz_ratio = frz_ratio; > + for (idx = 0; idx < MTK_CAMSYS_RES_STEP_NUM ; idx++) { > + res_step_type = res->res_strategy[idx] & MTK_CAMSYS_RES_IDXMASK; > + switch (res_step_type) { > + case MTK_CAMSYS_RES_BIN_TAG: > + bin_temp = res->res_strategy[idx] - E_RES_BIN_S; > + if (bin_temp <= res->bin_limit) > + bin_en = bin_temp; > + if (bin_en && frz_en) > + frz_en = 0; > + break; > + case MTK_CAMSYS_RES_FRZ_TAG: > + frz_temp = res->res_strategy[idx] - E_RES_FRZ_S; > + if (res->frz_limit < 100) > + frz_en = frz_temp; > + break; > + case MTK_CAMSYS_RES_HWN_TAG: > + hwn_temp = res->res_strategy[idx] - E_RES_HWN_S; > + if (hwn_temp + 1 <= res->hwn_limit_max) > + twin_en = hwn_temp; > + break; > + case MTK_CAMSYS_RES_CLK_TAG: > + clk_cur = res->res_strategy[idx] - E_RES_CLK_S; > + break; > + default: > + break; > + } > + > + /* 1 for force bin on */ > + if (res->bin_limit >= 1) > + bin_en = 1; > + > + if (res->hwn_limit_min > 1) > + twin_en = 1; > + > + /* max line buffer check*/ > + lb_chk_res = mtk_raw_linebuf_chk(twin_en, res->bin_limit & BIN_ON, > + frz_en, res->bin_limit & QBND_ON, > + is_cbn_en(res->bin_limit), > + in_w, &frz_ratio); > + /* frz ratio*/ > + if (res_step_type == MTK_CAMSYS_RES_FRZ_TAG) { > + res->frz_ratio = res->frz_limit < FRZ_PXLMODE_THRES ? > + res->frz_limit : FRZ_PXLMODE_THRES; > + } > + /*try 1-pixel mode first*/ > + for (p = 1; p <= MTK_CAMSYS_PROC_DEFAULT_PIXELMODE; p++) { MTK_CAMSYS_PROC_DEFAULT_PIXELMODE is 1, so this for-loop could be dropped. > + tgo_pxl_mode = mtk_raw_pixelmode_calc(p, twin_en, > + bin_en, frz_en, > + res->frz_ratio); mtk_raw_pixelmode_calc() would always return 1, so mtk_raw_pixelmode_calc() could be dropped. Regards, CK > + /** > + * isp throughput along resource strategy > + * (compared with pixel rate) > + */ > + pixel_mode[idx] = tgo_pxl_mode; > + > + /* only support 1-pixel mode */ > + if (p == 1 && lb_chk_res == LB_CHECK_OK) { > + if (!res_found) { > + res->bin_enable = bin_en; > + res->frz_enable = frz_en; > + res->raw_num_used = twin_en + 1; > + clk_res = clk_cur; > + idx_res = idx; > + res_found = true; > + break; > + } > + } > + } > + dev_dbg(cam->dev, > + "Res-%d B/F/H/C=%d/%d/%d/%d -> %d/%d/%d/%d (%d)(%d)\n", > + idx, bin_temp, frz_temp, hwn_temp, clk_cur, bin_en, > + frz_en, twin_en, clk_cur, lb_chk_res, pixel_mode[idx]); > + } > + > + tgo_pxl_mode = pixel_mode[idx_res]; > + switch (tgo_pxl_mode) { > + case 1: > + res->tgo_pxl_mode = 0; > + break; > + case 2: > + res->tgo_pxl_mode = 1; > + break; > + case 4: > + res->tgo_pxl_mode = 2; > + break; > + case 8: > + res->tgo_pxl_mode = 3; > + break; > + default: > + break; > + } > + > + mtk_raw_update_debug_param(cam, res, &clk_res); > + > + if (res_found) { > + dev_info(cam->dev, > + "Res-end:%d BIN/FRZ/HWN/CLK/pxl=%d/%d(%d)/%d/%d/%d, clk:%d\n", > + idx_res, res->bin_enable, res->frz_enable, res->frz_ratio, > + res->raw_num_used, clk_res, res->tgo_pxl_mode, > + res->clk_target); > + } else { > + dev_dbg(cam->dev, "[%s] Error resource result\n", __func__); > + } > + if (res->bin_enable) { > + *out_w = in_w >> 1; > + *out_h = in_h >> 1; > + } else if (res->frz_enable) { > + *out_w = in_w * res->frz_ratio / 100; > + *out_h = in_h * res->frz_ratio / 100; > + } else { > + *out_w = in_w; > + *out_h = in_h; > + } > + > + return res_found; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > sizeof(u32)) > + > +enum MTK_CAMSYS_RES_STEP { > + E_RES_BASIC, > + E_RES_BIN_S = MTK_CAMSYS_RES_BIN_TAG, > + E_RES_BIN0 = E_RES_BIN_S, > + E_RES_BIN1, > + E_RES_BIN_E, > + E_RES_FRZ_S = MTK_CAMSYS_RES_FRZ_TAG, > + E_RES_FRZ0 = E_RES_FRZ_S, > + E_RES_FRZ1, > + E_RES_FRZ_E, > + E_RES_HWN_S = MTK_CAMSYS_RES_HWN_TAG, > + E_RES_HWN0 = E_RES_HWN_S, > + E_RES_HWN1, > + E_RES_HWN2, > + E_RES_HWN_E, > + E_RES_CLK_S = MTK_CAMSYS_RES_CLK_TAG, > + E_RES_CLK0 = E_RES_CLK_S, > + E_RES_CLK1, > + E_RES_CLK2, > + E_RES_CLK3, > + E_RES_CLK_E, E_RES_CLK_E is useless, so drop it. Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static void mtk_cam_config_raw_path(struct mtk_cam_device *cam, > + struct mtkcam_ipi_frame_param *frame_param, > + struct mtk_cam_buffer *buf) > +{ > + struct mtk_cam_video_device *node; > + struct mtk_raw_pipeline *raw_pipeline; > + > + node = mtk_cam_vbq_to_vdev(buf->vbb.vb2_buf.vb2_queue); > + raw_pipeline = mtk_cam_dev_get_raw_pipeline(cam, node->uid.pipe_id); > + if (!raw_pipeline) { > + dev_err(cam->dev, "%s: cannot find raw_pipeline, pipe_id:%d\n", > + __func__, node->uid.pipe_id); > + return; > + } > + > + if (raw_pipeline->res_config.raw_path == V4L2_MTK_CAM_RAW_PATH_SELECT_BPC) > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_AFTER_BPC; > + else if (raw_pipeline->res_config.raw_path == V4L2_MTK_CAM_RAW_PATH_SELECT_FUS) > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_AFTER_FUS; > + else if (raw_pipeline->res_config.raw_path == V4L2_MTK_CAM_RAW_PATH_SELECT_DGN) > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_AFTER_DGN; > + else if (raw_pipeline->res_config.raw_path == V4L2_MTK_CAM_RAW_PATH_SELECT_LSC) > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_AFTER_LSC; > + else if (raw_pipeline->res_config.raw_path == V4L2_MTK_CAM_RAW_PATH_SELECT_LTM) > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_AFTER_LTM; > + else > + /* un-processed raw frame */ > + frame_param->raw_param.imgo_path_sel = MTKCAM_IPI_IMGO_UNPROCESSED; This patch is too big, so let this patch support basic function first. Let this patch support only MTKCAM_IPI_IMGO_UNPROCESSED, and add other path by other patch. Maybe one patch for one path. Regards, CK > + > + dev_dbg(cam->dev, "%s: node:%d fd:%d idx:%d raw_path(%d) ipi imgo_path_sel(%d))\n", > + __func__, node->desc.id, buf->vbb.request_fd, buf->vbb.vb2_buf.index, > + raw_pipeline->res_config.raw_path, frame_param->raw_param.imgo_path_sel); > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static int mtk_cam_raw_try_res_ctrl(struct mtk_raw_pipeline *pipeline, > + struct mtk_cam_resource *res_user, > + struct mtk_cam_resource_config *res_cfg, > + struct v4l2_mbus_framefmt *sink_fmt) > +{ > + s64 prate = 0; > + int width, height; > + struct device *dev = pipeline->raw->devs[pipeline->id]; > + > + res_cfg->bin_limit = res_user->raw_res.bin; /* 1: force bin on */ > + res_cfg->frz_limit = 0; > + res_cfg->hwn_limit_max = res_user->raw_res.raw_max; > + res_cfg->hwn_limit_min = res_user->raw_res.raw_min; > + res_cfg->hblank = res_user->sensor_res.hblank; > + res_cfg->vblank = res_user->sensor_res.vblank; > + res_cfg->sensor_pixel_rate = res_user->sensor_res.pixel_rate; > + res_cfg->res_plan = res_user->raw_res.strategy; > + res_cfg->raw_feature = res_user->raw_res.feature; > + res_cfg->raw_path = res_user->raw_res.path_sel; > + > + if (res_user->sensor_res.cust_pixel_rate) > + prate = res_user->sensor_res.cust_pixel_rate; > + else if (mtk_cam_feature_is_pure_m2m(res_cfg->raw_feature)) > + prate = mtk_cam_calc_pure_m2m_pixelrate(sink_fmt->width, > + sink_fmt->height, > + &res_cfg->interval); > + else > + prate = mtk_cam_seninf_calc_pixelrate(pipeline->raw->cam_dev, > + sink_fmt->width, > + sink_fmt->height, > + res_user->sensor_res.hblank, > + res_user->sensor_res.vblank, > + res_user->sensor_res.interval.denominator, > + res_user->sensor_res.interval.numerator, > + res_user->sensor_res.pixel_rate); > + > + mtk_cam_raw_resource_calc(dev_get_drvdata(pipeline->raw->cam_dev), > + res_cfg, prate, res_cfg->res_plan, > + sink_fmt->width, sink_fmt->height, > + &width, &height); If the basic function support only imgo unprocessed, does the unprocessed path imply the resource could be fixed? I mean it's not necessary to calculate resource when imgo unprocessed path. If so, remove resource calculation related code in this basic patch and add it back in advance function patch. Regards, CK > + > + if (res_user->raw_res.bin && !res_cfg->bin_enable) { > + dev_err(dev, > + "%s:pipe(%d): res calc failed on fource bin: user(%d)/bin_enable(%d)\n", > + __func__, pipeline->id, res_user->raw_res.bin, > + res_cfg->bin_enable); > + return -EINVAL; > + } > + > + if (res_cfg->raw_num_used > res_user->raw_res.raw_max || > + res_cfg->raw_num_used < res_user->raw_res.raw_min) { > + dev_err(dev, > + "%s:pipe(%d): res calc failed on raw used: user(%d/%d)/raw_num_used(%d)\n", > + __func__, pipeline->id, res_user->raw_res.raw_max, > + res_user->raw_res.raw_min, res_cfg->raw_num_used); > + } > + > + res_user->raw_res.pixel_mode = res_cfg->tgo_pxl_mode; > + res_user->raw_res.raw_used = res_cfg->raw_num_used; > + if (res_cfg->bin_limit == BIN_AUTO) > + res_user->raw_res.bin = res_cfg->bin_enable; > + else > + res_user->raw_res.bin = res_cfg->bin_limit; > + > + dev_info(dev, > + "%s:pipe(%d): res calc result: raw_used(%d)/bin(%d)/pixelmode(%d)/strategy(%d)\n", > + __func__, pipeline->id, res_user->raw_res.raw_used, > + res_user->raw_res.bin, res_user->raw_res.pixel_mode, > + res_user->raw_res.strategy); > + > + /** > + * Other output not reveal to user: > + * res_cfg->res_strategy[MTK_CAMSYS_RES_STEP_NUM]; > + * res_cfg->clk_target; > + * res_cfg->frz_enable; > + * res_cfg->frz_ratio; > + * res_cfg->tgo_pxl_mode; > + */ > + if (width != sink_fmt->width || height != sink_fmt->height) { > + dev_info(dev, > + "%s:pipe(%d): size adjust info: raw: sink(%d,%d) res:(%d,%d)\n", > + __func__, pipeline->id, sink_fmt->width, > + sink_fmt->height, width, height); > + } > + > + return 0; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static const > +char *capture_queue_names[RAW_PIPELINE_NUM][MTK_RAW_TOTAL_CAPTURE_QUEUES] = { > + {"mtk-cam raw-0 main-stream", Let the basic function support only one capture queue and add additional capture queue by other patch. Maybe one patch for one queue. The basic function support only "mtk-cam raw-0 main-stream" queue. Regards, CK > + "mtk-cam raw-0 yuvo-1", "mtk-cam raw-0 yuvo-2", > + "mtk-cam raw-0 yuvo-3", "mtk-cam raw-0 yuvo-4", > + "mtk-cam raw-0 yuvo-5", > + "mtk-cam raw-0 drzs4no-1", "mtk-cam raw-0 drzs4no-2", "mtk-cam raw-0 drzs4no-3", > + "mtk-cam raw-0 rzh1n2to-1", "mtk-cam raw-0 rzh1n2to-2", "mtk-cam raw-0 rzh1n2to-3", > + "mtk-cam raw-0 partial-meta-0", "mtk-cam raw-0 partial-meta-1", > + "mtk-cam raw-0 partial-meta-2", > + }, > + {"mtk-cam raw-1 main-stream", > + "mtk-cam raw-1 yuvo-1", "mtk-cam raw-1 yuvo-2", > + "mtk-cam raw-1 yuvo-3", "mtk-cam raw-1 yuvo-4", > + "mtk-cam raw-1 yuvo-5", > + "mtk-cam raw-1 drzs4no-1", "mtk-cam raw-1 drzs4no-2", "mtk-cam raw-1 drzs4no-3", > + "mtk-cam raw-1 rzh1n2to-1", "mtk-cam raw-1 rzh1n2to-2", "mtk-cam raw-1 rzh1n2to-3", > + "mtk-cam raw-1 partial-meta-0", "mtk-cam raw-1 partial-meta-1", > + "mtk-cam raw-1 partial-meta-2", > + }, > + > + {"mtk-cam raw-2 main-stream", > + "mtk-cam raw-2 yuvo-1", "mtk-cam raw-2 yuvo-2", > + "mtk-cam raw-2 yuvo-3", "mtk-cam raw-2 yuvo-4", > + "mtk-cam raw-2 yuvo-5", > + "mtk-cam raw-2 drzs4no-1", "mtk-cam raw-2 drzs4no-2", "mtk-cam raw-2 drzs4no-3", > + "mtk-cam raw-2 rzh1n2to-1", "mtk-cam raw-2 rzh1n2to-2", "mtk-cam raw-2 rzh1n2to-3", > + "mtk-cam raw-2 partial-meta-0", "mtk-cam raw-2 partial-meta-1", > + "mtk-cam raw-2 partial-meta-2", > + }, > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces support for the sensor interface in the MediaTek SoC, > with the focus on CSI and stream control. The key functionalities > include parameter control, metering and maintaining status information, > interrupt handling, and debugging. These features ensure effective > management and debugging of the camera sensor interface hardware. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +#define SENINF_CAM_MUX0_CHK_CTL_1 0x0104 > +#define RG_SENINF_CAM_MUX0_EXP_HSIZE_SHIFT 0 > +#define RG_SENINF_CAM_MUX0_EXP_HSIZE_MASK (0xffff << 0) > +#define RG_SENINF_CAM_MUX0_EXP_VSIZE_SHIFT 16 > +#define RG_SENINF_CAM_MUX0_EXP_VSIZE_MASK (0xffff << 16) #define SENINF_CAM_MUX_CHK_CTL_1(n) (0x0104 + 0x10 * n) #define RG_SENINF_CAM_MUX_EXP_HSIZE_SHIFT 0 #define RG_SENINF_CAM_MUX_EXP_HSIZE_MASK (0xffff << 0) #define RG_SENINF_CAM_MUX_EXP_VSIZE_SHIFT 16 #define RG_SENINF_CAM_MUX_EXP_VSIZE_MASK (0xffff << 16) > +int mtk_cam_seninf_set_cammux_src(struct seninf_ctx *ctx, int src, > + int target, int exp_hsize, int exp_vsize) > +{ > + void __iomem *seninf_cam_mux_base = ctx->reg_if_cam_mux; > + > [snip] > + > + switch (target) { > + case SENINF_CAM_MUX0: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_0, > + RG_SENINF_CAM_MUX0_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX0_CHK_CTL_1, > + RG_SENINF_CAM_MUX0_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX0_CHK_CTL_1, > + RG_SENINF_CAM_MUX0_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX1: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_0, > + RG_SENINF_CAM_MUX1_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX1_CHK_CTL_1, > + RG_SENINF_CAM_MUX1_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX1_CHK_CTL_1, > + RG_SENINF_CAM_MUX1_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX2: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_0, > + RG_SENINF_CAM_MUX2_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX2_CHK_CTL_1, > + RG_SENINF_CAM_MUX2_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX2_CHK_CTL_1, > + RG_SENINF_CAM_MUX2_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX3: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_0, > + RG_SENINF_CAM_MUX3_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX3_CHK_CTL_1, > + RG_SENINF_CAM_MUX3_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX3_CHK_CTL_1, > + RG_SENINF_CAM_MUX3_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX4: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_1, > + RG_SENINF_CAM_MUX4_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX4_CHK_CTL_1, > + RG_SENINF_CAM_MUX4_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX4_CHK_CTL_1, > + RG_SENINF_CAM_MUX4_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX5: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_1, > + RG_SENINF_CAM_MUX5_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX5_CHK_CTL_1, > + RG_SENINF_CAM_MUX5_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX5_CHK_CTL_1, > + RG_SENINF_CAM_MUX5_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX6: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_1, > + RG_SENINF_CAM_MUX6_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX6_CHK_CTL_1, > + RG_SENINF_CAM_MUX6_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX6_CHK_CTL_1, > + RG_SENINF_CAM_MUX6_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX7: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_1, > + RG_SENINF_CAM_MUX7_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX7_CHK_CTL_1, > + RG_SENINF_CAM_MUX7_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX7_CHK_CTL_1, > + RG_SENINF_CAM_MUX7_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX8: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_2, > + RG_SENINF_CAM_MUX8_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX8_CHK_CTL_1, > + RG_SENINF_CAM_MUX8_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX8_CHK_CTL_1, > + RG_SENINF_CAM_MUX8_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX9: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_2, > + RG_SENINF_CAM_MUX9_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX9_CHK_CTL_1, > + RG_SENINF_CAM_MUX9_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX9_CHK_CTL_1, > + RG_SENINF_CAM_MUX9_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX10: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_2, > + RG_SENINF_CAM_MUX10_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX10_CHK_CTL_1, > + RG_SENINF_CAM_MUX10_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX10_CHK_CTL_1, > + RG_SENINF_CAM_MUX10_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX11: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_2, > + RG_SENINF_CAM_MUX11_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX11_CHK_CTL_1, > + RG_SENINF_CAM_MUX11_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX11_CHK_CTL_1, > + RG_SENINF_CAM_MUX11_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX12: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_3, > + RG_SENINF_CAM_MUX12_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX12_CHK_CTL_1, > + RG_SENINF_CAM_MUX12_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX12_CHK_CTL_1, > + RG_SENINF_CAM_MUX12_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX13: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_3, > + RG_SENINF_CAM_MUX13_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX13_CHK_CTL_1, > + RG_SENINF_CAM_MUX13_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX13_CHK_CTL_1, > + RG_SENINF_CAM_MUX13_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX14: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_3, > + RG_SENINF_CAM_MUX14_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX14_CHK_CTL_1, > + RG_SENINF_CAM_MUX14_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX14_CHK_CTL_1, > + RG_SENINF_CAM_MUX14_EXP_VSIZE, exp_vsize); > + break; > + case SENINF_CAM_MUX15: > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CTRL_3, > + RG_SENINF_CAM_MUX15_SRC_SEL, src); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX15_CHK_CTL_1, > + RG_SENINF_CAM_MUX15_EXP_HSIZE, exp_hsize); > + SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX15_CHK_CTL_1, > + RG_SENINF_CAM_MUX15_EXP_VSIZE, exp_vsize); > + break; > + default: > + dev_dbg(ctx->dev, "invalid src %d target %d", src, target); > + return -EINVAL; > + } Replace the switch like this: SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CHK_CTL_1(target), RG_SENINF_CAM_MUX_EXP_HSIZE, exp_hsize); SENINF_BITS(seninf_cam_mux_base, SENINF_CAM_MUX_CHK_CTL_1(target), RG_SENINF_CAM_MUX_EXP_VSIZE, exp_vsize); Regards, CK > + > + return 0; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* > + * struct mtkcam_ipi_hw_buffer - DMA buffer for CAM DMA device. > + * > + * @iova: DMA address for CAM DMA device. isp7_1: u64. > + * @size: buffer size. > + */ > +struct mtkcam_ipi_hw_buffer { > + u64 iova; > + u32 size; > +} __packed; > + mtkcam_ipi_hw_buffer is useless, so drop it. Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces state management and debugging mechanisms for the MediaTek > ISP7.x camsys platform. State management establishes control over ISP > operations and events, defining distinct states for request handling, > sensor control, and frame synchronization, ensuring event processing. > The debugging mechanism ensures stable operation and timely data > collection during anomalies. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +int mtk_camsys_isr_event(struct mtk_cam_device *cam, > + enum MTK_CAMSYS_ENGINE_TYPE engine_type, > + unsigned int engine_id, > + struct mtk_camsys_irq_info *irq_info) > +{ > + int ret = 0; > + > + switch (engine_type) { > + case CAMSYS_ENGINE_RAW: > + ret = mtk_camsys_event_handle_raw(cam, engine_id, irq_info); > + break; > + case CAMSYS_ENGINE_SENINF: > + if (irq_info->irq_type & (1 << CAMSYS_IRQ_FRAME_DROP)) CAMSYS_IRQ_FRAME_DROP is never set, so drop it. Regards, CK > + dev_info(cam->dev, > + "MTK_CAMSYS_ENGINE_SENINF_TAG engine:%d type:0x%x\n", > + engine_id, irq_info->irq_type); > + break; > + default: > + break; > + } > + > + return ret; > +} > +
On Mon, 2024-10-28 at 13:20 +0800, CK Hu wrote: > Hi, Shu-hsiang: > > On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > > Introduces state management and debugging mechanisms for the MediaTek > > ISP7.x camsys platform. State management establishes control over ISP > > operations and events, defining distinct states for request handling, > > sensor control, and frame synchronization, ensuring event processing. > > The debugging mechanism ensures stable operation and timely data > > collection during anomalies. > > > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > > --- > > [snip] > > > +int mtk_camsys_isr_event(struct mtk_cam_device *cam, > > + enum MTK_CAMSYS_ENGINE_TYPE engine_type, > > + unsigned int engine_id, > > + struct mtk_camsys_irq_info *irq_info) > > +{ > > + int ret = 0; > > + > > + switch (engine_type) { > > + case CAMSYS_ENGINE_RAW: > > + ret = mtk_camsys_event_handle_raw(cam, engine_id, irq_info); > > + break; > > + case CAMSYS_ENGINE_SENINF: CAMSYS_ENGINE_SENINF is also never set, so drop it. Regards, CK > > + if (irq_info->irq_type & (1 << CAMSYS_IRQ_FRAME_DROP)) > > CAMSYS_IRQ_FRAME_DROP is never set, so drop it. > > Regards, > CK > > > + dev_info(cam->dev, > > + "MTK_CAMSYS_ENGINE_SENINF_TAG engine:%d type:0x%x\n", > > + engine_id, irq_info->irq_type); > > + break; > > + default: > > + break; > > + } > > + > > + return ret; > > +} > > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces state management and debugging mechanisms for the MediaTek > ISP7.x camsys platform. State management establishes control over ISP > operations and events, defining distinct states for request handling, > sensor control, and frame synchronization, ensuring event processing. > The debugging mechanism ensures stable operation and timely data > collection during anomalies. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +enum MTK_CAMSYS_ENGINE_TYPE { > + CAMSYS_ENGINE_RAW, > + CAMSYS_ENGINE_MRAW, > + CAMSYS_ENGINE_CAMSV, > + CAMSYS_ENGINE_SENINF, Only CAMSYS_ENGINE_RAW is used, so it's not necessary to define MTK_CAMSYS_ENGINE_TYPE because only one type. CAMSYS_ENGINE_RAW could also be dropped because only this type so it's not necessary to define it. Regards, CK > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +static const struct mtk_cam_format_desc yuv_out_group2_fmts[] = { > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, I don't know why you define width and height here. All width/height of yuv_out_group2_fmts are the same. And the max width/height is define in capture_queues[].frmsizes. So the width/height could be dropped here. Regards, CK > + .pixelformat = V4L2_PIX_FMT_NV12, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_NV21, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_NV12_10, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_NV21_10, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_MTISP_NV12_10P, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_MTISP_NV21_10P, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_NV12_12, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_NV21_12, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_MTISP_NV12_12P, > + }, > + }, > + { > + .vfmt.fmt.pix_mp = { > + .width = YUV_GROUP2_MAX_WIDTH, > + .height = YUV_GROUP2_MAX_HEIGHT, > + .pixelformat = V4L2_PIX_FMT_MTISP_NV21_12P, > + }, > + } > +}; > + [snip] > +#define MTK_RAW_TOTAL_CAPTURE_QUEUES 15 > +static const struct > +mtk_cam_dev_node_desc capture_queues[] = { > [snip] > + { > + .id = MTK_RAW_YUVO_2_OUT, > + .name = "yuvo 2", > + .cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE, > + .buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > + .link_flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE, > + .image = true, > + .smem_alloc = false, > + .dma_port = MTKCAM_IPI_RAW_YUVO_2, > + .fmts = yuv_out_group2_fmts, > + .num_fmts = ARRAY_SIZE(yuv_out_group2_fmts), > + .default_fmt_idx = 0, > + .pad_ops = &source_pad_ops_yuv, > + .ioctl_ops = &mtk_cam_v4l2_vcap_ioctl_ops, > + .frmsizes = &(struct v4l2_frmsizeenum) { > + .index = 0, > + .type = V4L2_FRMSIZE_TYPE_CONTINUOUS, > + .stepwise = { > + .max_width = YUV_GROUP2_MAX_WIDTH, > + .min_width = IMG_MIN_WIDTH, > + .max_height = YUV_GROUP2_MAX_HEIGHT, > + .min_height = IMG_MIN_HEIGHT, > + .step_height = 1, > + .step_width = 1, > + }, > + }, > + }, > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +/** > + * MediaTek Universal Frame Buffer Compression (UFBC) buffer header > + * Caller must follow the bit stream buffer size and length table buffer size. > + * > + * Header Size > + * Fixed size of 4096 bytes. Caller SHOULD NOT edit it. > + * > + * Bit Stream Size > + * Bit stream width must be aligned to 64 pixel. > + */ > + > +struct ufbc_buf_header { > + /** Describe image resolution, unit in pixel. */ > + u32 width; > + /** Describe image resolution, unit in pixel. */ > + u32 height; > + /** Describe UFBC data plane count, UFBC supports maximum 2 planes. */ > + u32 plane_count; > + /** Describe the original image data bits per pixel of the given plane. */ > + u32 bits_per_pixel[3]; > + > + /** > + * Describe the offset of the given plane bit stream data in bytes, > + * including header size. > + */ > + u32 bitstream_offset[3]; > + /** Describe the bit stream data size in bytes of the given plane. */ > + u32 bitstream_size[3]; > + /** Describe the encoded data size in bytes of the given plane. */ > + u32 bitstream_data_size[3]; > + > + /** > + * Describe the offset of length table of the given plane, including > + * header size. > + */ > + u32 table_offset[3]; > + /** Describe the length table size of the given plane */ > + u32 table_size[3]; > + /** Describe the total buffer size, including buffer header. */ > + u32 buffer_size; > +}; > + > +struct ufbc_buffer_header { > + union { > + struct ufbc_buf_header header; 'struct ufbc_buf_header' is never used, so drop it. Regards, CK > + u8 bits[4096]; > + }; > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* > + * Bit Feild of LTMS_BLKHIST_INT: LTMS_BLKHIST_INT1 > + * MTK_CAM_LTMS_BLKHIST_INT_LTMS_BLKHIST_INT1: [0, 13] > + * block histogram interval 1 > + */ > +#define MTK_CAM_LTMS_BLKHIST_INT_LTMS_BLKHIST_INT1_MASK 0x00003fff > +#define MTK_CAM_LTMS_BLKHIST_INT_LTMS_BLKHIST_INT1_SHIFT 0 > + MTK_CAM_LTMS_BLKHIST_INT_LTMS_BLKHIST_INT1_MASK and MTK_CAM_LTMS_BLKHIST_INT_LTMS_BLKHIST_INT1_SHIFT are never used, so drop them. Regards, CK
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +void mtk_cam_buf_try_queue(struct mtk_cam_ctx *ctx) > +{ > + struct mtk_cam_device *cam; > + struct mtk_cam_buffer *buf, *buf_prev; > + struct mtkcam_ipi_event event; > + struct mtkcam_ipi_session_cookie *session = &event.cookie; > + struct mtkcam_ipi_frame_info *frame_info = &event.frame_data; > + struct mtkcam_ipi_frame_param *frame_param; > + struct mtkcam_ipi_frame_param *frame_data; > + struct mtk_cam_working_buf_entry *buf_entry; > + struct list_head equeue_list; > + unsigned int processing_cnt, enque_cnt; > + > + cam = ctx->cam; > + if (!cam->streaming_ctx) { > + dev_info(cam->dev, "streams are off\n"); > + return; > + } > + > + INIT_LIST_HEAD(&equeue_list); > + > + spin_lock(&cam->dma_processing_lock); > + processing_cnt = cam->dma_processing_count; > + spin_unlock(&cam->dma_processing_lock); > + > + enque_cnt = 0; > + spin_lock(&cam->dma_pending_lock); > + list_for_each_entry_safe(buf, buf_prev, &cam->dma_pending, list) { > + if (processing_cnt + enque_cnt >= MTK_CAM_MAX_PROCESSING_BUFS) { > + dev_dbg(cam->dev, > + "processing bufs are full, buf cnt(%d)\n", > + processing_cnt); > + break; > + } > + dev_dbg(cam->dev, "%s buf cnt(%d)\n", > + __func__, processing_cnt + enque_cnt); > + > + enque_cnt++; > + list_del(&buf->list); > + list_add_tail(&buf->list, &equeue_list); > + } > + spin_unlock(&cam->dma_pending_lock); > + > + if (!enque_cnt) > + return; > + > + frame_param = kzalloc(sizeof(*frame_param), GFP_KERNEL); > + if (!frame_param) > + return; > + > + list_for_each_entry_safe(buf, buf_prev, &equeue_list, list) { > + if (buf->state.estate == E_BUF_STATE_COMPOSED) This would never happened. buf in equeue_list is moved from cam->dma_pending and buf's state is E_BUF_STATE_QUEUED. So drop this checking. Regards, CK > + continue; > + > + memset(&event, 0, sizeof(event)); > + event.cmd_id = CAM_CMD_FRAME; > + session->session_id = ctx->stream_id; > + /* prepare working buffer */ > + buf_entry = mtk_cam_working_buf_get(ctx); > + if (!buf_entry) { > + dev_info(cam->dev, > + "%s: No CQ buf availablle: enqueued_frame_seq_no:%d\n", > + __func__, atomic_read(&ctx->enqueued_frame_seq_no)); > + WARN_ON(1); > + goto EXIT; > + } > + > + spin_lock(&ctx->using_buffer_list.lock); > + list_add_tail(&buf_entry->list_entry, &ctx->using_buffer_list.list); > + ctx->using_buffer_list.cnt++; > + spin_unlock(&ctx->using_buffer_list.lock); > + > + spin_lock(&cam->dma_processing_lock); > + list_del(&buf->list); > + list_add_tail(&buf->list, &cam->dma_processing); > + cam->dma_processing_count++; > + spin_unlock(&cam->dma_processing_lock); > + > + /* Prepare rp message */ > + frame_info->cur_msgbuf_offset = > + buf_entry->msg_buffer.va - > + cam->ctxs[session->session_id].buf_pool.msg_buf_va; > + frame_info->cur_msgbuf_size = buf_entry->msg_buffer.size; > + frame_data = (struct mtkcam_ipi_frame_param *)buf_entry->msg_buffer.va; > + session->frame_no = atomic_inc_return(&ctx->enqueued_frame_seq_no); > + > + if (mtk_cam_buf_config(cam, frame_param, buf)) { > + dev_err(cam->dev, "%s: Buffer config failed\n", __func__); > + continue; > + } > + memcpy(frame_data, frame_param, sizeof(*frame_param)); > + frame_data->cur_workbuf_offset = > + buf_entry->buffer.iova - > + cam->ctxs[session->session_id].buf_pool.working_buf_iova; > + frame_data->cur_workbuf_size = buf_entry->buffer.size; > + > + if (ctx->pipe->res_config.bin_limit == BIN_AUTO) > + frame_data->raw_param.bin_flag = ctx->pipe->res_config.bin_enable; > + else > + frame_data->raw_param.bin_flag = ctx->pipe->res_config.bin_limit; > + > + scp_ipi_send(cam->scp, SCP_IPI_ISP_FRAME, &event, > + sizeof(event), MTK_CAM_IPI_SEND_TIMEOUT); > + buf->state.estate = E_BUF_STATE_COMPOSED; > + } > +EXIT: > + kfree(frame_param); > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +struct mtk_cam_device { > + struct device *dev; > + > + struct v4l2_device v4l2_dev; > + struct v4l2_async_notifier notifier; > + struct media_device media_dev; > + void __iomem *base; > + > + struct mtk_scp *scp; > + struct device *smem_dev; > + phandle rproc_phandle; > + struct rproc *rproc_handle; > + > + unsigned int composer_cnt; > + > + unsigned int num_seninf_devices; > + unsigned int num_raw_devices; > + unsigned int num_larb_devices; > + > + /* raw_pipe controller subdev */ > + struct mtk_raw raw; > + struct mutex queue_lock; /* protect queue request */ > + > + unsigned int max_stream_num; > + unsigned int streaming_ctx; > + unsigned int streaming_pipe; > + struct mtk_cam_ctx *ctxs; > + > + /* request related */ > + struct list_head pending_job_list; > + spinlock_t pending_job_lock; /* protect pending_job_list */ > + struct list_head running_job_list; > + unsigned int running_job_count; > + spinlock_t running_job_lock; /* protect running_job_list */ > + > + /* standard v4l2 buffer control */ > + struct list_head dma_pending; > + spinlock_t dma_pending_lock; /* protect dma_pending_list */ > + struct list_head dma_processing; > + spinlock_t dma_processing_lock; /* protect dma_processing_list and dma_processing_count */ > + unsigned int dma_processing_count; I would like scp-related variable has the scp prefix. struct list_head scp_dma_processing; spinlock_t scp_dma_processing_lock; unsigned int scp_dma_processing_count; So it's easy to understand that scp_dma_processing_count's max value is 2. One buffer is currently doing dma, and another one is waiting for dma. Both buffer are queued in scp. Regards, CK > + > + struct mtk_cam_debug_fs *debug_fs; > + struct workqueue_struct *debug_wq; > + struct workqueue_struct *debug_exception_wq; > + wait_queue_head_t debug_exception_waitq; > +}; > +
Hi, Shu-hsiang: On Tue, 2024-10-29 at 15:03 +0800, CK Hu wrote: > Hi, Shu-hsiang: > > On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > > The driver maintains the camera system, including sub-device management, > > DMA operations, and integration with the V4L2 framework. It handles > > request stream data, buffer management, and MediaTek-specific features, > > and pipeline management, streaming control, error handling mechanism. > > Additionally, it aggregates sub-drivers for the camera interface, raw > > and yuv pipelines. > > > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > > --- > > [snip] > > > +struct mtk_cam_device { > > + struct device *dev; > > + > > + struct v4l2_device v4l2_dev; > > + struct v4l2_async_notifier notifier; > > + struct media_device media_dev; > > + void __iomem *base; > > + > > + struct mtk_scp *scp; > > + struct device *smem_dev; > > + phandle rproc_phandle; > > + struct rproc *rproc_handle; > > + > > + unsigned int composer_cnt; > > + > > + unsigned int num_seninf_devices; > > + unsigned int num_raw_devices; > > + unsigned int num_larb_devices; > > + > > + /* raw_pipe controller subdev */ > > + struct mtk_raw raw; > > + struct mutex queue_lock; /* protect queue request */ > > + > > + unsigned int max_stream_num; > > + unsigned int streaming_ctx; > > + unsigned int streaming_pipe; > > + struct mtk_cam_ctx *ctxs; > > + > > + /* request related */ > > + struct list_head pending_job_list; > > + spinlock_t pending_job_lock; /* protect pending_job_list */ > > + struct list_head running_job_list; > > + unsigned int running_job_count; > > + spinlock_t running_job_lock; /* protect running_job_list */ > > + > > + /* standard v4l2 buffer control */ > > + struct list_head dma_pending; > > + spinlock_t dma_pending_lock; /* protect dma_pending_list */ > > + struct list_head dma_processing; > > + spinlock_t dma_processing_lock; /* protect dma_processing_list and dma_processing_count */ > > + unsigned int dma_processing_count; > > I would like scp-related variable has the scp prefix. > > struct list_head scp_dma_processing; > spinlock_t scp_dma_processing_lock; > unsigned int scp_dma_processing_count; > > So it's easy to understand that scp_dma_processing_count's max value is 2. > One buffer is currently doing dma, and another one is waiting for dma. Both buffer are queued in scp. Forget previous comment. After review the buffer control, I think the buffer list should be simplified. dma_pending, dma_processing, using_buffer_list, composed_buffer_list, processing_buffer_list could be merge into one buf_list. The buffer in buf_list has different status. In init, the buffer is queued into driver and status is waiting. buf_list-> buf0(waiting)-> buf1(waiting)-> buf2(waiting)-> buf3(waiting)-> buf4(waiting) In mtk_cam_buf_try_queue(), use scp to generate cq buffer content of buf0 and buf1, so the status is scp_generate_cq. buf_list-> buf0(scp_generate_cq)-> buf1(scp_generate_cq)-> buf2(waiting)-> buf3(waiting)-> buf4(waiting) So the buf_entry is bound to buf0 and buf1, it's not necessary have using_buffer_list, composed_buffer_list, processing_buffer_list to manage buf_entry. In isp_composer_handler_ack(), scp has finish generating cq buffer content, so the status is cq_ready. In the meantime, use scp to generate cq buffer content of buf2. buf_list-> buf0(cq_ready)-> buf1(scp_generate_cq)-> buf2(scp_generate_cq)-> buf3(waiting)-> buf4(waiting) In mtk_camsys_raw_frame_start(), apply cq buffer to hardware, so the status is cq_apply buf_list-> buf0(cq_apply)-> buf1(scp_generate_cq)-> buf2(scp_generate_cq)-> buf3(waiting)-> buf4(waiting) In mtk_camsys_frame_done(), hardware has finished writing video into buffer, so the status is done buf_list-> buf0(done)-> buf1(scp_generate_cq)-> buf2(scp_generate_cq)-> buf3(waiting)-> buf4(waiting) In this design, just need one buf_list with status. The code would be more simple. Simple code would has less bug. Maybe you could drop so many debug utility. Regards, CK > > Regards, > CK > > > + > > + struct mtk_cam_debug_fs *debug_fs; > > + struct workqueue_struct *debug_wq; > > + struct workqueue_struct *debug_exception_wq; > > + wait_queue_head_t debug_exception_waitq; > > +}; > > + > >
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +struct mtk_cam_device { > + struct device *dev; > + > + struct v4l2_device v4l2_dev; > + struct v4l2_async_notifier notifier; > + struct media_device media_dev; > + void __iomem *base; > + > + struct mtk_scp *scp; > + struct device *smem_dev; > + phandle rproc_phandle; rproc_phandle is useless, so drop it. > + struct rproc *rproc_handle; > + > + unsigned int composer_cnt; > + > + unsigned int num_seninf_devices; > + unsigned int num_raw_devices; > + unsigned int num_larb_devices; num_larb_devices is useless, so drop it. Regards, CK > + > + /* raw_pipe controller subdev */ > + struct mtk_raw raw; > + struct mutex queue_lock; /* protect queue request */ > + > + unsigned int max_stream_num; > + unsigned int streaming_ctx; > + unsigned int streaming_pipe; > + struct mtk_cam_ctx *ctxs; > + > + /* request related */ > + struct list_head pending_job_list; > + spinlock_t pending_job_lock; /* protect pending_job_list */ > + struct list_head running_job_list; > + unsigned int running_job_count; > + spinlock_t running_job_lock; /* protect running_job_list */ > + > + /* standard v4l2 buffer control */ > + struct list_head dma_pending; > + spinlock_t dma_pending_lock; /* protect dma_pending_list */ > + struct list_head dma_processing; > + spinlock_t dma_processing_lock; /* protect dma_processing_list and dma_processing_count */ > + unsigned int dma_processing_count; > + > + struct mtk_cam_debug_fs *debug_fs; > + struct workqueue_struct *debug_wq; > + struct workqueue_struct *debug_exception_wq; > + wait_queue_head_t debug_exception_waitq; > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static __maybe_unused struct dma_debug_item dbg_PDI_R1[] = { > + {0x00000014, "pdi_r1 32(hex) 0000"}, You print this register value, but how to use this value to debug? What's the error value? Each error value means what kind of error? Add comment about how to use this value to debug. If no one knows how to use this value, the debug method is redundant. > + {0x00000114, "pdi_r1 state_checksum"}, Ditto. > + {0x00000214, "pdi_r1 line_pix_cnt_tmp"}, Ditto. > + {0x00000314, "pdi_r1 line_pix_cnt"}, Ditto. > + {0x00000414, "pdi_r1 important_status"}, Ditto. > + {0x00000514, "pdi_r1 cmd_data_cnt"}, Ditto. > + {0x00000614, "pdi_r1 tilex_byte_cnt"}, Ditto. > + {0x00000714, "pdi_r1 tiley_cnt"}, Ditto. > + {0x00000814, "pdi_r1 burst_line_cnt"}, Ditto. Regards, CK > +}; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +void mtk_cam_dump_req_rdy_status(struct device *dev, > + void __iomem *base, void __iomem *yuvbase) > +{ This function is useless, so drop it. Regards, CK > + dev_dbg_ratelimited(dev, > + "REQ RAW/2/3 DMA/2:%08x/%08x/%08x/%08x/%08x\n", > + readl_relaxed(base + REG_CTL_RAW_MOD_REQ_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD2_REQ_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD3_REQ_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD5_REQ_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD6_REQ_STAT)); > + dev_dbg_ratelimited(dev, > + "RDY RAW/2/3 DMA/2:%08x/%08x/%08x/%08x/%08x\n", > + readl_relaxed(base + REG_CTL_RAW_MOD_RDY_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD2_RDY_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD3_RDY_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD5_RDY_STAT), > + readl_relaxed(base + REG_CTL_RAW_MOD6_RDY_STAT)); > + dev_dbg_ratelimited(dev, > + "REQ YUV/2/3/4 WDMA:%08x/%08x/%08x/%08x/%08x\n", > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD_REQ_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD2_REQ_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD3_REQ_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD4_REQ_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD5_REQ_STAT)); > + dev_dbg_ratelimited(dev, > + "RDY YUV/2/3/4 WDMA:%08x/%08x/%08x/%08x/%08x\n", > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD_RDY_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD2_RDY_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD3_RDY_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD4_RDY_STAT), > + readl_relaxed(yuvbase + REG_CTL_RAW_MOD5_RDY_STAT)); > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +struct mtk_cam_request_stream_data * > +mtk_cam_get_req_s_data(struct mtk_cam_ctx *ctx, unsigned int pipe_id, > + unsigned int frame_seq_no) > + > +{ > + struct mtk_cam_device *cam = ctx->cam; > + struct mtk_cam_request *req, *req_prev; > + struct mtk_cam_request_stream_data *req_stream_data; > + int i; > + > + spin_lock(&cam->running_job_lock); > + list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list) { > + if (req->pipe_used & (1 << pipe_id)) { > + for (i = 0; i < req->p_data[pipe_id].s_data_num; i++) { > + req_stream_data = &req->p_data[pipe_id].s_data[i]; > + if (req_stream_data->frame_seq_no == frame_seq_no) { > + spin_unlock(&cam->running_job_lock); > + return req_stream_data; > + } > + } > + } > + } > + spin_unlock(&cam->running_job_lock); > + > + return NULL; > +} > + > +struct mtk_cam_request *mtk_cam_get_req(struct mtk_cam_ctx *ctx, > + unsigned int frame_seq_no) > +{ > + struct mtk_cam_request_stream_data *req_stream_data; > + > + req_stream_data = mtk_cam_get_req_s_data(ctx, ctx->stream_id, frame_seq_no); In some place, it is called 'stream_id'. In some place, it is called 'pipe_id'. It's easy to confuse us that stream_id and pipe_id are different and the code readability is bad. Use the unique name so that we would not get confused. Regards, CK > + if (!req_stream_data) > + return NULL; > + > + return req_stream_data->req; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static bool mtk_raw_fmt_get_res(struct v4l2_subdev *sd, > + struct v4l2_subdev_format *fmt, > + struct mtk_cam_resource *res) > +{ > + void *user_ptr; > + u64 addr; > + > + addr = ((u64)fmt->reserved[1] << 32) | fmt->reserved[2]; The callstack to this function is: subdev_do_ioctl() -> mtk_raw_set_fmt() -> mtk_raw_try_pad_fmt() -> mtk_raw_set_src_pad_fmt() -> mtk_raw_fmt_get_res() In subdev_do_ioctl() [1], fmt->reserved[] would be cleared to zero. I don't know why you could get a non-zero addr value? [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/v4l2-subdev.c?h=v6.12-rc6#n753 Regards, CK > + user_ptr = (void *)addr; > + if (!user_ptr) { > + dev_info(sd->v4l2_dev->dev, "%s: mtk_cam_resource is null\n", > + __func__); > + return false; > + } > + > + if (copy_from_user(res, (void __user *)user_ptr, sizeof(*res))) { > + dev_info(sd->v4l2_dev->dev, > + "%s: copy_from_user failedm user_ptr:%p\n", > + __func__, user_ptr); > + return false; > + } > + > + dev_dbg(sd->v4l2_dev->dev, > + "%s:sensor:%d/%d/%lld/%d/%d, raw:%lld/%d/%d/%d/%d/%d/%d/%d/%lld\n", > + __func__, > + res->sensor_res.hblank, res->sensor_res.vblank, > + res->sensor_res.pixel_rate, res->sensor_res.interval.denominator, > + res->sensor_res.interval.numerator, > + res->raw_res.feature, res->raw_res.bin, res->raw_res.path_sel, > + res->raw_res.raw_max, res->raw_res.raw_min, res->raw_res.raw_used, > + res->raw_res.strategy, res->raw_res.pixel_mode, > + res->raw_res.throughput); > + > + return res; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the ISP pipeline driver for the MediaTek ISP raw and yuv > modules. Key functionalities include data processing, V4L2 integration, > resource management, debug support, and various control operations. > Additionally, IRQ handling, platform device management, and MediaTek > ISP DMA format support are also included. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +static int mtk_raw_of_probe(struct platform_device *pdev, > + struct mtk_raw_device *raw) > +{ > + struct device *dev = &pdev->dev; > + struct resource *res; > + int ret; > + int n_clks; > + > + ret = of_property_read_u32(dev->of_node, "mediatek,cam-id", &raw->id); You don't need raw->id. I think each raw could work independently. Each raw has its own 'struct mtk_raw_device' context data. It's not necessary to know which one it is. When you use dev_dbg() or dev_err(), it would show register base so you could identify which raw print this message. Regards, CK > + if (ret) { > + dev_dbg(dev, "missing camid property\n"); > + return ret; > + } > + > + if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34))) { > + dev_err(dev, "%s: No suitable DMA available\n", __func__); > + return -EIO; > + } > + > + if (!dev->dma_parms) { > + dev->dma_parms = > + devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL); > + if (!dev->dma_parms) > + return -ENOMEM; > + } > + > + dma_set_max_seg_size(dev, UINT_MAX); > + > + /* base outer register */ > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base"); > + if (!res) { > + dev_err(dev, "failed to get mem\n"); > + return -ENODEV; > + } > + > + raw->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(raw->base)) { > + dev_err(dev, "failed to map register base\n"); > + return PTR_ERR(raw->base); > + } > + > + /* base inner register */ > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "inner_base"); > + if (!res) { > + dev_err(dev, "failed to get mem\n"); > + return -ENODEV; > + } > + > + raw->base_inner = devm_ioremap_resource(dev, res); > + if (IS_ERR(raw->base_inner)) { > + dev_err(dev, "failed to map register inner base\n"); > + return PTR_ERR(raw->base_inner); > + } > + > + /* will be assigned later */ > + raw->yuv_base = NULL; > + > + raw->irq = platform_get_irq(pdev, 0); > + if (raw->irq < 0) { > + dev_err(dev, "failed to get irq\n"); > + return -ENODEV; > + } > + > + ret = devm_request_threaded_irq(dev, raw->irq, > + mtk_irq_raw, mtk_thread_irq_raw, > + 0, dev_name(dev), raw); > + if (ret) { > + dev_err(dev, "failed to request irq=%d\n", raw->irq); > + return ret; > + } > + dev_dbg(dev, "registered irq=%d\n", raw->irq); > + > + disable_irq(raw->irq); > + > + n_clks = devm_clk_bulk_get_all(dev, &raw->clk_b); > + if (n_clks < 0) { > + dev_err(dev, "failed to devm_clk_bulk_get_all=%d\n", n_clks); > + return n_clks; > + } > + > + raw->num_clks = n_clks; > + dev_info(dev, "clk_num:%d\n", raw->num_clks); > + > +#ifdef CONFIG_PM_SLEEP > + raw->pm_notifier.notifier_call = raw_pm_notifier; > + ret = register_pm_notifier(&raw->pm_notifier); > + if (ret) { > + dev_info(dev, "failed to register notifier block.\n"); > + return ret; > + } > +#endif > + return 0; > +} > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* RAW input trigger ctrl*/ > +#define RAWI_R2_TRIG BIT(0) > +#define RAWI_R3_TRIG BIT(1) > +#define RAWI_R4_TRIG BIT(2) RAWI_R4_TRIG is not used, so drop it. Regards, CK > +#define RAWI_R5_TRIG BIT(3) > +#define RAWI_R6_TRIG BIT(4) > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* Control flags of CAM_CMD_CONFIG */ > +#define MTK_CAM_IPI_CONFIG_TYPE_INIT 0x0001 > +#define MTK_CAM_IPI_CONFIG_TYPE_INPUT_CHANGE 0x0002 > +#define MTK_CAM_IPI_CONFIG_TYPE_EXEC_TWICE 0x0004 MTK_CAM_IPI_CONFIG_TYPE_EXEC_TWICE is useless, so drop it. > +#define MTK_CAM_IPI_CONFIG_TYPE_SMVR_PREVIEW 0x0008 Ditto for MTK_CAM_IPI_CONFIG_TYPE_SMVR_PREVIEW. Regards, CK > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > + > +#define MTK_CAM_IPI_VERSION_MAJOR (0) MTK_CAM_IPI_VERSION_MAJOR is useless, so drop it. > +#define MTK_CAM_IPI_VERSION_MINOR (1) Ditto. Regards, CK > + >
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* > + * Bit Feild of LTM_GAIN_MAP: LTM_TRAN_WGT_TYPE > + * MTK_CAM_LTM_GAIN_MAP_LTM_TRAN_WGT_TYPE: [12, 13] > + * type of tran weight > + */ > +#define MTK_CAM_LTM_GAIN_MAP_LTM_TRAN_WGT_TYPE_MASK 0x00003000 MTK_CAM_LTM_GAIN_MAP_LTM_TRAN_WGT_TYPE_MASK is useless, so drop it. Regards, CK > +#define MTK_CAM_LTM_GAIN_MAP_LTM_TRAN_WGT_TYPE_SHIFT 12 > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +struct mtkcam_ipi_hw_mapping { > + u8 pipe_id; /* ref. to mtkcam_pipe_subdev */ > + u16 dev_mask; /* ref. to mtkcam_pipe_dev */ > + u8 exp_order; exp_order is useless, so drop it. Regards, CK > +} __packed; > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* CAMSYS_RAW 0x1a03 */ > +#define OFFSET_OBC_R1_R_SUM_L 0x1178 > +#define OFFSET_OBC_R1_R_SUM_H 0x117c > +#define OFFSET_OBC_R1_B_SUM_L 0x1180 > +#define OFFSET_OBC_R1_B_SUM_H 0x1184 > +#define OFFSET_OBC_R1_GR_SUM_L 0x1188 > +#define OFFSET_OBC_R1_GR_SUM_H 0x118c > +#define OFFSET_OBC_R1_GB_SUM_L 0x1190 > +#define OFFSET_OBC_R1_GB_SUM_H 0x1194 > +#define OFFSET_OBC_R1_ACT_WIN_X 0x1198 OFFSET_OBC_R1_ACT_WIN_X is useless, so drop it. Regards, CK > +#define OFFSET_OBC_R1_ACT_WIN_Y 0x119c > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces utility files for the MediaTek ISP7.x camsys driver. These > utilities provide essential platform definitions, debugging tools, and > management functions to support ISP operations and SCP communication. > Key functionalities include: > 1.Hardware pipeline and register definitions for managing image > processing modules. > 2.DMA debugging utilities and buffer management functions. > 3.Definitions of supported image formats for proper data handling. > 4.IPI and SCP communication structures for module state management and > configuring ISP. > 5.Metadata parameters for configuring image processing. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* CAM DMA done status */ > +#define IMGO_DONE_ST BIT(0) IMGO_DONE_ST is useless, so drop it. Regards, CK > +#define AFO_DONE_ST BIT(8) > +#define CQI_R1_DONE_ST BIT(15) > +
Hi, Shu-hsiang: On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote: > Introduces the top media device driver for the MediaTek ISP7X CAMSYS. > The driver maintains the camera system, including sub-device management, > DMA operations, and integration with the V4L2 framework. It handles > request stream data, buffer management, and MediaTek-specific features, > and pipeline management, streaming control, error handling mechanism. > Additionally, it aggregates sub-drivers for the camera interface, raw > and yuv pipelines. > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com> > --- [snip] > +/* flags of mtk_cam_request */ > +#define MTK_CAM_REQ_FLAG_SENINF_CHANGED BIT(0) MTK_CAM_REQ_FLAG_SENINF_CHANGED is never used, so drop it. > +#define MTK_CAM_REQ_FLAG_SENINF_IMMEDIATE_UPDATE BIT(1) > + > +/* flags of mtk_cam_request_stream_data */ > +#define MTK_CAM_REQ_S_DATA_FLAG_TG_FLASH BIT(0) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_META1_INDEPENDENT BIT(1) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_SINK_FMT_UPDATE BIT(2) > +/* Apply sensor mode and the timing is 1 vsync before */ > +#define MTK_CAM_REQ_S_DATA_FLAG_SENSOR_MODE_UPDATE_T1 BIT(3) MTK_CAM_REQ_S_DATA_FLAG_SENSOR_MODE_UPDATE_T1 is useless, so drop it. Regards, CK > + > +#define MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_EN BIT(4) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_RAW_HDL_EN BIT(5) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_COMPLETE BIT(6) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_RAW_HDL_COMPLETE BIT(7) > + > +#define MTK_CAM_REQ_S_DATA_FLAG_SENSOR_HDL_DELAYED BIT(8) > +