Le jeu. 11 janv. 2024 à 13:04, AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> a écrit : > > Il 10/01/24 15:14, Julien Stephan ha scritto: > > From: Louis Kuo <louis.kuo@xxxxxxxxxxxx> > > > > This will add the mediatek ISP3.0 seninf (sensor interface) driver found > > on several Mediatek SoCs such as the mt8365. > > > > Then seninf module has 4 physical CSI-2 inputs. Depending on the soc they > > may not be all connected. > > > > Signed-off-by: Louis Kuo <louis.kuo@xxxxxxxxxxxx> > > Signed-off-by: Phi-bang Nguyen <pnguyen@xxxxxxxxxxxx> > > Signed-off-by: Florian Sylvestre <fsylvestre@xxxxxxxxxxxx> > > Co-developed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Co-developed-by: Julien Stephan <jstephan@xxxxxxxxxxxx> > > Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx> > > --- > > MAINTAINERS | 1 + > > drivers/media/platform/mediatek/Kconfig | 1 + > > drivers/media/platform/mediatek/Makefile | 1 + > > drivers/media/platform/mediatek/isp/Kconfig | 2 + > > drivers/media/platform/mediatek/isp/Makefile | 3 + > > .../platform/mediatek/isp/isp_30/Kconfig | 16 + > > .../platform/mediatek/isp/isp_30/Makefile | 3 + > > .../mediatek/isp/isp_30/seninf/Makefile | 5 + > > .../mediatek/isp/isp_30/seninf/mtk_seninf.c | 1488 +++++++++++++++++ > > .../isp/isp_30/seninf/mtk_seninf_reg.h | 112 ++ > > 10 files changed, 1632 insertions(+) > > create mode 100644 drivers/media/platform/mediatek/isp/Kconfig > > create mode 100644 drivers/media/platform/mediatek/isp/Makefile > > create mode 100644 drivers/media/platform/mediatek/isp/isp_30/Kconfig > > create mode 100644 drivers/media/platform/mediatek/isp/isp_30/Makefile > > create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile > > create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c > > create mode 100644 drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf_reg.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 3ea2158864e1..52d200d5e36c 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13618,6 +13618,7 @@ M: Andy Hsieh <andy.hsieh@xxxxxxxxxxxx> > > S: Supported > > F: Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml > > F: Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml > > +F: drivers/media/platform/mediatek/isp/isp_30/seninf/* > > > > MEDIATEK SMI DRIVER > > M: Yong Wu <yong.wu@xxxxxxxxxxxx> > > diff --git a/drivers/media/platform/mediatek/Kconfig b/drivers/media/platform/mediatek/Kconfig > > index 84104e2cd024..4e0a5a43f35e 100644 > > --- a/drivers/media/platform/mediatek/Kconfig > > +++ b/drivers/media/platform/mediatek/Kconfig > > @@ -7,3 +7,4 @@ source "drivers/media/platform/mediatek/mdp/Kconfig" > > source "drivers/media/platform/mediatek/vcodec/Kconfig" > > source "drivers/media/platform/mediatek/vpu/Kconfig" > > source "drivers/media/platform/mediatek/mdp3/Kconfig" > > +source "drivers/media/platform/mediatek/isp/Kconfig" > > diff --git a/drivers/media/platform/mediatek/Makefile b/drivers/media/platform/mediatek/Makefile > > index 38e6ba917fe5..695f05f525a6 100644 > > --- a/drivers/media/platform/mediatek/Makefile > > +++ b/drivers/media/platform/mediatek/Makefile > > @@ -4,3 +4,4 @@ obj-y += mdp/ > > obj-y += vcodec/ > > obj-y += vpu/ > > obj-y += mdp3/ > > +obj-y += isp/ > > diff --git a/drivers/media/platform/mediatek/isp/Kconfig b/drivers/media/platform/mediatek/isp/Kconfig > > new file mode 100644 > > index 000000000000..708b9a6660d2 > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/Kconfig > > @@ -0,0 +1,2 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +source "drivers/media/platform/mediatek/isp/isp_30/Kconfig" > > diff --git a/drivers/media/platform/mediatek/isp/Makefile b/drivers/media/platform/mediatek/isp/Makefile > > new file mode 100644 > > index 000000000000..a81ab33d0dd3 > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/Makefile > > @@ -0,0 +1,3 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > + > > +obj-y += isp_30/ > > diff --git a/drivers/media/platform/mediatek/isp/isp_30/Kconfig b/drivers/media/platform/mediatek/isp/isp_30/Kconfig > > new file mode 100644 > > index 000000000000..9791312589fb > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/isp_30/Kconfig > > @@ -0,0 +1,16 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +config MTK_SENINF30 > > + tristate "MediaTek ISP3.0 SENINF driver" > > + depends on VIDEO_V4L2_SUBDEV_API > > + depends on MEDIA_CAMERA_SUPPORT > > + depends on ARCH_MEDIATEK || COMPILE_TEST > > + depends on OF > > + select V4L2_FWNODE > > + default n > > + help > > + This driver provides a MIPI CSI-2 receiver interface to connect > > + an external camera module with MediaTek ISP3.0. It is able to handle > > + multiple cameras at the same time. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called mtk-seninf. > > diff --git a/drivers/media/platform/mediatek/isp/isp_30/Makefile b/drivers/media/platform/mediatek/isp/isp_30/Makefile > > new file mode 100644 > > index 000000000000..ac3142de4739 > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/isp_30/Makefile > > @@ -0,0 +1,3 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +obj-$(CONFIG_MTK_SENINF30) += seninf/ > > diff --git a/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile b/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile > > new file mode 100644 > > index 000000000000..f28480d6d6c3 > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/isp_30/seninf/Makefile > > @@ -0,0 +1,5 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +mtk-seninf-objs += mtk_seninf.o > > + > > +obj-$(CONFIG_MTK_SENINF30) += mtk-seninf.o > > diff --git a/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c b/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c > > new file mode 100644 > > index 000000000000..67b2c697d9ca > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/isp/isp_30/seninf/mtk_seninf.c > > @@ -0,0 +1,1488 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2022 MediaTek Inc. > > + */ > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/interrupt.h> > > +#include <linux/module.h> > > +#include <linux/of_graph.h> > > +#include <linux/of_platform.h> > > +#include <linux/phy/phy.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/videodev2.h> > > +#include <media/media-device.h> > > +#include <media/media-entity.h> > > +#include <media/v4l2-async.h> > > +#include <media/v4l2-common.h> > > +#include <media/v4l2-ctrls.h> > > +#include <media/v4l2-dev.h> > > +#include <media/v4l2-device.h> > > +#include <media/v4l2-event.h> > > +#include <media/v4l2-fwnode.h> > > +#include <media/v4l2-mc.h> > > +#include <media/v4l2-subdev.h> > > + > > +#include "mtk_seninf_reg.h" > > + > > +#define SENINF_TIMESTAMP_STEP 0x67 > > +#define SENINF_SETTLE_DELAY 0x15 > > +#define SENINF_HS_TRAIL_PARAMETER 0x8 > > + > > +#define SENINF_MAX_NUM_INPUTS 4 > > +#define SENINF_MAX_NUM_OUTPUTS 6 > > +#define SENINF_MAX_NUM_MUXES 6 > > +#define SENINF_MAX_NUM_PADS (SENINF_MAX_NUM_INPUTS + \ > > + SENINF_MAX_NUM_OUTPUTS) > > + > > +#define SENINF_DEFAULT_BUS_FMT MEDIA_BUS_FMT_SGRBG10_1X10 > > +#define SENINF_DEFAULT_WIDTH 1920 > > +#define SENINF_DEFAULT_HEIGHT 1080 > > + > > +#define SENINF_PAD_10BIT 0 > > + > > +#define SENINF_TEST_MODEL 0 > > +#define SENINF_NORMAL_MODEL 1 > > +#define SENINF_ALL_ERR_IRQ_EN 0x7f > > +#define SENINF_IRQ_CLR_SEL 0x80000000 > > + > > +#define SENINF_MIPI_SENSOR 0x8 > > + > > +#define MTK_CSI_MAX_LANES 4 > > + > > +/* Port number in the device tree. */ > > +enum mtk_seninf_port { > > + CSI_PORT_0 = 0, /* 4D1C or 2D1C */ > > + CSI_PORT_1, /* 4D1C */ > > + CSI_PORT_2, /* 4D1C */ > > + CSI_PORT_0B, /* 2D1C */ > > +}; > > + > > +enum mtk_seninf_id { > > + SENINF_1 = 0, > > + SENINF_2 = 1, > > + SENINF_3 = 2, > > + SENINF_5 = 4, > > +}; > > + > > +static const u32 port_to_seninf_id[] = { > > + [CSI_PORT_0] = SENINF_1, > > + [CSI_PORT_1] = SENINF_3, > > + [CSI_PORT_2] = SENINF_5, > > + [CSI_PORT_0B] = SENINF_2, > > +}; > > + > > +enum mtk_seninf_phy_mode { > > + SENINF_PHY_MODE_NONE, > > + SENINF_PHY_MODE_4D1C, > > + SENINF_PHY_MODE_2D1C, > > +}; > > + > > +enum mtk_seninf_format_flag { > > + MTK_SENINF_FORMAT_BAYER = BIT(0), > > + MTK_SENINF_FORMAT_DPCM = BIT(1), > > + MTK_SENINF_FORMAT_JPEG = BIT(2), > > + MTK_SENINF_FORMAT_INPUT_ONLY = BIT(3), > > +}; > > + > > +/** > > + * struct mtk_seninf_conf - Model-specific SENINF parameters > > + * @model: Model description > > + * @nb_inputs: Number of SENINF inputs > > + * @nb_muxes: Number of SENINF MUX (FIFO) instances > > + * @nb_outputs: Number of outputs (to CAM and CAMSV instances) > > + */ > > +struct mtk_seninf_conf { > > + const char *model; > > + u8 nb_inputs; > > + u8 nb_muxes; > > + u8 nb_outputs; > > +}; > > + > > +/** > > + * struct mtk_seninf_format_info - Information about media bus formats > > + * @code: V4L2 media bus code > > + * @flags: Flags describing the format, as a combination of MTK_SENINF_FORMAT_* > > + */ > > +struct mtk_seninf_format_info { > > + u32 code; > > + u32 flags; > > +}; > > + > > +/** > > + * struct mtk_seninf_input - SENINF input block > > + * @pad: DT port and media entity pad number > > + * @seninf_id: SENINF hardware instance ID > > + * @base: Memory mapped I/O based address > > + * @seninf: Back pointer to the mtk_seninf > > + * @phy: PHY connected to the input > > + * @phy_mode: PHY operation mode (NONE when the input is not connected) > > + * @bus: CSI-2 bus configuration from DT > > + * @source_sd: Source subdev connected to the input > > + */ > > +struct mtk_seninf_input { > > + enum mtk_seninf_port pad; > > + enum mtk_seninf_id seninf_id; > > + void __iomem *base; > > + struct mtk_seninf *seninf; > > + > > + struct phy *phy; > > + enum mtk_seninf_phy_mode phy_mode; > > + > > + struct v4l2_mbus_config_mipi_csi2 bus; > > + > > + struct v4l2_subdev *source_sd; > > +}; > > + > > +/** > > + * struct mtk_seninf_mux - SENINF MUX channel > > + * @pad: DT port and media entity pad number > > + * @mux_id: MUX hardware instance ID > > + * @base: Memory mapped I/O based address > > + * @seninf: Back pointer to the mtk_seninf > > + */ > > +struct mtk_seninf_mux { > > + unsigned int pad; > > + unsigned int mux_id; > > + void __iomem *base; > > + struct mtk_seninf *seninf; > > +}; > > + > > +/** > > + * struct mtk_seninf - Top-level SENINF device > > + * @dev: The (platform) device > > + * @phy: PHYs at the SENINF inputs > > + * @num_clks: Number of clocks in the clks array > > + * @clks: Clocks > > + * @base: Memory mapped I/O base address > > + * @media_dev: Media controller device > > + * @v4l2_dev: V4L2 device > > + * @subdev: V4L2 subdevice > > + * @pads: Media entity pads > > + * @notifier: V4L2 async notifier for source subdevs > > + * @ctrl_handler: V4L2 controls handler > > + * @source_format: Active format on the source pad > > + * @inputs: Array of SENINF inputs > > + * @muxes: Array of MUXes > > + * @conf: Model-specific SENINF parameters > > + * @is_testmode: Whether or not the test pattern generator is enabled > > + */ > > +struct mtk_seninf { > > + struct device *dev; > > + struct phy *phy[5]; > > + unsigned int num_clks; > > + struct clk_bulk_data *clks; > > + void __iomem *base; > > + > > + struct media_device media_dev; > > + struct v4l2_device v4l2_dev; > > + struct v4l2_subdev subdev; > > + struct media_pad pads[SENINF_MAX_NUM_PADS]; > > + struct v4l2_async_notifier notifier; > > + struct v4l2_ctrl_handler ctrl_handler; > > + > > + struct mtk_seninf_input inputs[SENINF_MAX_NUM_INPUTS]; > > + struct mtk_seninf_mux muxes[SENINF_MAX_NUM_MUXES]; > > + > > + const struct mtk_seninf_conf *conf; > > + > > + bool is_testmode; > > +}; > > + > > +inline struct mtk_seninf *sd_to_mtk_seninf(struct v4l2_subdev *sd) > > +{ > > + return container_of(sd, struct mtk_seninf, subdev); > > +} > > + > > +static inline bool mtk_seninf_pad_is_sink(struct mtk_seninf *priv, > > + unsigned int pad) > > +{ > > + return pad < priv->conf->nb_inputs; > > +} > > + > > +static inline bool mtk_seninf_pad_is_source(struct mtk_seninf *priv, > > + unsigned int pad) > > +{ > > + return !mtk_seninf_pad_is_sink(priv, pad); > > +} > > + > > +/* ----------------------------------------------------------------------------- > > + * Formats > > + */ > > + > > +static const struct mtk_seninf_format_info mtk_seninf_formats[] = { > > + { > > + .code = MEDIA_BUS_FMT_SBGGR8_1X8, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG8_1X8, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGRBG8_1X8, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB8_1X8, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGRBG10_1X10, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB10_1X10, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SBGGR10_1X10, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG10_1X10, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SBGGR12_1X12, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG12_1X12, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGRBG12_1X12, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB12_1X12, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SBGGR14_1X14, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG14_1X14, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGRBG14_1X14, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB14_1X14, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SBGGR16_1X16, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG16_1X16, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SGRBG16_1X16, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB16_1X16, > > + .flags = MTK_SENINF_FORMAT_BAYER, > > + }, { > > + .code = MEDIA_BUS_FMT_UYVY8_1X16, > > + }, { > > + .code = MEDIA_BUS_FMT_VYUY8_1X16, > > + }, { > > + .code = MEDIA_BUS_FMT_YUYV8_1X16, > > + }, { > > + .code = MEDIA_BUS_FMT_YVYU8_1X16, > > + }, { > > + .code = MEDIA_BUS_FMT_JPEG_1X8, > > + .flags = MTK_SENINF_FORMAT_JPEG, > > + }, { > > + .code = MEDIA_BUS_FMT_S5C_UYVY_JPEG_1X8, > > + .flags = MTK_SENINF_FORMAT_JPEG, > > + }, > > + /* Keep the input-only formats last. */ > > Your comment doesn't make me understand why input-only formats shall be > placed last - and makes me think that having two arrays (one for both > and one for input only) would be easier and less error prone, other than > making you able to drop the MTK_SENINF_FORMAT_INPUT_ONLY flag entirely. > > > + { > > + .code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, > > + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY, > > + }, { > > + .code = MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, > > + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY, > > + }, { > > + .code = MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, > > + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY, > > + }, { > > + .code = MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, > > + .flags = MTK_SENINF_FORMAT_DPCM | MTK_SENINF_FORMAT_INPUT_ONLY, > > + } > > +}; > > + > > +static const struct mtk_seninf_format_info *mtk_seninf_format_info(u32 code) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(mtk_seninf_formats); ++i) { > > + if (mtk_seninf_formats[i].code == code) > > + return &mtk_seninf_formats[i]; > > + } > > + > > + return NULL; > > +} > > + > > ..snip.. > > > + > > +static void mtk_seninf_input_setup_csi2(struct mtk_seninf_input *input, > > + struct v4l2_subdev_state *state) > > +{ > > + const struct mtk_seninf_format_info *fmtinfo; > > + const struct v4l2_mbus_framefmt *format; > > + unsigned int num_data_lanes = input->bus.num_data_lanes; > > + unsigned int val = 0; > > + > > + format = v4l2_subdev_state_get_stream_format(state, input->pad, 0); > > + fmtinfo = mtk_seninf_format_info(format->code); > > + > > + /* Configure timestamp */ > > + writel(SENINF_TIMESTAMP_STEP, input->base + SENINF_TG1_TM_STP); > > + > > + /* HQ */ > > + writel(0x0, input->base + SENINF_TG1_PH_CNT); > > Zero means: > - Sensor master clock: ISP_CLK > - Sensor clock polarity: Rising edge > - Sensor reset deasserted > - Sensor powered up > - Pixel clock inversion disabled > - Sensor master clock polarity disabled > - Phase counter disabled > > > + writel(0x10001, input->base + SENINF_TG1_SEN_CK); > > Unroll this one... this is the TG1 sensor clock divider. > > CLKFL GENMASK(5, 0) > CLKRS GENMASK(13, 8) > CLKCNT GENMASK(21,16) > > Like this, I don't get what you're trying to set, because you're using a fixed > sensor clock rate, meaning that only a handful of camera sensors will be usable. > > Is this 8Mhz? 16? 24? what? :-) > > Two hints: > - sensor_clk = clk_get_rate(isp_clk) / (tg1_sen_ck_clkcnt + 1); > - int mtk_seninf_set_sensor_clk(u8 rate_mhz); > > Please :-) Hi Angelo, I think I get your point about not hardcoding the sensor rate, but I am not sure how to use a mtk_seninf_set_sensor_clk(u8 rate_mhz); function. Where would it be called? How is it exposed to the user? Cheers Julien > > > + > > + /* First Enable Sensor interface and select pad (0x1a04_0200) */ > > + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_EN, 1); > > + mtk_seninf_input_update(input, SENINF_CTRL, PAD2CAM_DATA_SEL, SENINF_PAD_10BIT); > > + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_SRC_SEL, 0); > > + mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_CSI2_IP_EN, 1); > > + mtk_seninf_input_update(input, SENINF_CTRL_EXT, SENINF_NCSI2_IP_EN, 0); > > + > > + /* DPCM Enable */ > > + if (fmtinfo->flags & MTK_SENINF_FORMAT_DPCM) > > + val = SENINF_CSI2_DPCM_DI_2A_DPCM_EN; > > + else > > + val = SENINF_CSI2_DPCM_DI_30_DPCM_EN; > > + writel(val, input->base + SENINF_CSI2_DPCM); > > + > > + /* Settle delay */ > > + mtk_seninf_input_update(input, SENINF_CSI2_LNRD_TIMING, > > + DATA_SETTLE_PARAMETER, SENINF_SETTLE_DELAY); > > + > > + /* HQ */ > > + writel(0x10, input->base + SENINF_CSI2_LNRC_FSM); > > As far as I know, SENINF_CSI2_LNRC_FSM is a read-only register: this write will do > exactly nothing... > > > + > > + /* CSI2 control */ > > + val = readl(input->base + SENINF_CSI2_CTL) > > + | (FIELD_PREP(SENINF_CSI2_CTL_ED_SEL, DATA_HEADER_ORDER_DI_WCL_WCH) > > + | SENINF_CSI2_CTL_CLOCK_LANE_EN | (BIT(num_data_lanes) - 1)); > > + writel(val, input->base + SENINF_CSI2_CTL); > > + > > + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, > > + BYPASS_LANE_RESYNC, 0); > > 93 columns: fits in one line (not only this one!). > > > + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, CDPHY_SEL, 0); > > + mtk_seninf_input_update(input, SENINF_CSI2_RESYNC_MERGE_CTL, > > + CPHY_LANE_RESYNC_CNT, 3); > > + mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_MODE, 0); > > + mtk_seninf_input_update(input, SENINF_CSI2_MODE, CSR_CSI2_HEADER_LEN, 0); > > + mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_MASK_0, 0xff00); > > + mtk_seninf_input_update(input, SENINF_CSI2_DPHY_SYNC, SYNC_SEQ_PAT_0, 0x001d); > > + > > + mtk_seninf_input_update(input, SENINF_CSI2_CTL, CLOCK_HS_OPTION, 0); > > + mtk_seninf_input_update(input, SENINF_CSI2_CTL, HSRX_DET_EN, 0); > > + mtk_seninf_input_update(input, SENINF_CSI2_CTL, HS_TRAIL_EN, 1); > > + mtk_seninf_input_update(input, SENINF_CSI2_HS_TRAIL, HS_TRAIL_PARAMETER, > > + SENINF_HS_TRAIL_PARAMETER); > > + > > + /* Set debug port to output packet number */ > > + mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_EN, 1); > > + mtk_seninf_input_update(input, SENINF_CSI2_DGB_SEL, DEBUG_SEL, 0x1a); > > + > > + /* HQ */ > > + writel(0xfffffffe, input->base + SENINF_CSI2_SPARE0); > > I have no idea what this SPARE0 does, but I think that this is something that you > want to get from platform_data, as I guess this would be different on various SoCs? > > > + > > + /* Enable CSI2 IRQ mask */ > > + /* Turn on all interrupt */ > > + writel(0xffffffff, input->base + SENINF_CSI2_INT_EN); > > + /* Write clear CSI2 IRQ */ > > + writel(0xffffffff, input->base + SENINF_CSI2_INT_STATUS); > > + /* Enable CSI2 Extend IRQ mask */ > > You missed: > writel(0xffffffff, input->base + SENINF_CSI2_INT_EN_EXT); > > P.S.: #define SENINF_CSI2_INT_EN_EXT 0x0b10 > > > > + /* Turn on all interrupt */ > > /* Reset the CSI2 to commit changes */ <-- makes more sense, doesn't it? > > > + mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 1); > > + udelay(1); > > + mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 0); > > +} > > + > > +static void mtk_seninf_mux_setup(struct mtk_seninf_mux *mux, > > + struct mtk_seninf_input *input, > > + struct v4l2_subdev_state *state) > > +{ > > + const struct mtk_seninf_format_info *fmtinfo; > > + const struct v4l2_mbus_framefmt *format; > > + unsigned int pix_sel_ext; > > + unsigned int pix_sel; > > + unsigned int hs_pol = 0; > > + unsigned int vs_pol = 0; > > + unsigned int val; > > + u32 rst_mask; > > + > > + format = v4l2_subdev_state_get_stream_format(state, input->pad, 0); > > + fmtinfo = mtk_seninf_format_info(format->code); > > + > > + /* Enable mux */ > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_MUX_EN, 1); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_SRC_SEL, SENINF_MIPI_SENSOR); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, SENINF_SRC_SEL_EXT, SENINF_NORMAL_MODEL); > > + > > + pix_sel_ext = 0; > > + pix_sel = 1; > > > pixels_per_cycle = 1; > bus_width = pixels_per_cycle >> 1; > > because: 0 == 1pix/cyc, 1 == 2pix/cyc, 2 == 4pix/cyc, 3 == 8pix... etc > ...but the width of this register depends on the SoC, so you also want to set > constraints to the bus width on a per-soc basis (platform data again, or at > least leave a comment here). > > mtk_seninf_mux_update(.... PIX_SEL_EXT, bus_width); > mtk_seninf_mux_update(.... PIX_SEL, bus_width); > > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, SENINF_PIX_SEL_EXT, pix_sel_ext); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_PIX_SEL, pix_sel); > > + > > + if (fmtinfo->flags & MTK_SENINF_FORMAT_JPEG) { > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 0); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN, > > + FIFO_FLUSH_EN_JPEG_2_PIXEL_MODE); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN, > > + FIFO_PUSH_EN_JPEG_2_PIXEL_MODE); > > + } else { > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 2); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN, > > + FIFO_FLUSH_EN_NORMAL_MODE); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN, > > + FIFO_PUSH_EN_NORMAL_MODE); > > + } > > + > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_POL, hs_pol); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_VSYNC_POL, vs_pol); > > + > > + val = mtk_seninf_mux_read(mux, SENINF_MUX_CTRL); > > + rst_mask = SENINF_MUX_CTRL_SENINF_IRQ_SW_RST | SENINF_MUX_CTRL_SENINF_MUX_SW_RST; > > + > > + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, val | rst_mask); > > Are you sure that you don't need any wait between assertion and deassertion of RST? > Looks strange, but I don't really know then. > > > + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, val & ~rst_mask); > > + > > + /* HQ */ > > + mtk_seninf_mux_write(mux, SENINF_MUX_SPARE, 0xc2000); > > val = SENINF_FIFO_FULL_SEL; > > /* SPARE field meaning is unknown */ > val |= 0xc0000; > > mtk_seninf_mux_write(mux, SENINF_MUX_SPARE, val); > > > +} > > + > > +static void mtk_seninf_top_mux_setup(struct mtk_seninf *priv, > > + enum mtk_seninf_id seninf_id, > > + struct mtk_seninf_mux *mux) > > +{ > > + unsigned int val; > > + > > + /* > > + * Use the top mux (from SENINF input to MUX) to configure routing, and > > + * hardcode a 1:1 mapping from the MUX instances to the SENINF outputs. > > + */ > > + val = readl(priv->base + SENINF_TOP_MUX_CTRL) > > + & ~(0xf << (mux->mux_id * 4)); > > + val |= (seninf_id & 0xf) << (mux->mux_id * 4); > > + writel(val, priv->base + SENINF_TOP_MUX_CTRL); > > + > > + writel(0x76541010, priv->base + SENINF_TOP_CAM_MUX_CTRL); > > Each four bits of TOP_CAM_MUX_CTRL selects between seninf1 to seninf8 muxes, and > TOP_MUX_CTRL is laid out in the very same way. > > This means that if you're calculating a value for TOP_MUX_CTRL, you can do exactly > the same for TOP_CAM_MUX_CTRL. > > > +} > > + > > +static void seninf_enable_test_pattern(struct mtk_seninf *priv, > > + struct v4l2_subdev_state *state) > > +{ > > + struct mtk_seninf_input *input = &priv->inputs[CSI_PORT_0]; > > + struct mtk_seninf_mux *mux = &priv->muxes[0]; > > + const struct mtk_seninf_format_info *fmtinfo; > > + const struct v4l2_mbus_framefmt *format; > > + unsigned int val; > > + unsigned int pix_sel_ext; > > + unsigned int pix_sel; > > + unsigned int hs_pol = 0; > > + unsigned int vs_pol = 0; > > + unsigned int seninf = 0; > > + unsigned int tm_size = 0; > > + unsigned int mux_id = mux->mux_id; > > + > > + format = v4l2_subdev_state_get_stream_format(state, priv->conf->nb_inputs, 0); > > + fmtinfo = mtk_seninf_format_info(format->code); > > + > > + mtk_seninf_update(priv, SENINF_TOP_CTRL, MUX_LP_MODE, 0); > > + > > + mtk_seninf_update(priv, SENINF_TOP_CTRL, SENINF_PCLK_EN, 1); > > + mtk_seninf_update(priv, SENINF_TOP_CTRL, SENINF2_PCLK_EN, 1); > > + > > + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_EN, 1); > > + mtk_seninf_input_update(input, SENINF_CTRL, SENINF_SRC_SEL, 1); > > + mtk_seninf_input_update(input, SENINF_CTRL_EXT, > > + SENINF_TESTMDL_IP_EN, 1); > > + > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_EN, 1); > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_PAT, 0xc); > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_VSYNC, 4); > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_DUMMYPXL, 0x28); > > + > > + if (fmtinfo->flags & MTK_SENINF_FORMAT_BAYER) > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_FMT, 0x0); > > + else > > + mtk_seninf_input_update(input, SENINF_TG1_TM_CTL, TM_FMT, 0x1); > > + > > + tm_size = FIELD_PREP(SENINF_TG1_TM_SIZE_TM_LINE, format->height + 8); > > + switch (format->code) { > > + case MEDIA_BUS_FMT_UYVY8_1X16: > > + case MEDIA_BUS_FMT_VYUY8_1X16: > > + case MEDIA_BUS_FMT_YUYV8_1X16: > > + case MEDIA_BUS_FMT_YVYU8_1X16: > > + tm_size |= FIELD_PREP(SENINF_TG1_TM_SIZE_TM_PXL, format->width * 2); > > + break; > > + default: > > + tm_size |= FIELD_PREP(SENINF_TG1_TM_SIZE_TM_PXL, format->width); > > + break; > > + } > > + writel(tm_size, input->base + SENINF_TG1_TM_SIZE); > > + > > + writel(TEST_MODEL_CLK_DIVIDED_CNT, input->base + SENINF_TG1_TM_CLK); > > + writel(TIME_STAMP_DIVIDER, input->base + SENINF_TG1_TM_STP); > > + > > + /* Set top mux */ > > + val = (readl(priv->base + SENINF_TOP_MUX_CTRL) & (~(0xf << (mux_id * 4)))) | > > + ((seninf & 0xf) << (mux_id * 4)); > > + writel(val, priv->base + SENINF_TOP_MUX_CTRL); > > This is duplicated, and it is the same that you have in mtk_seninf_top_mux_setup() > > > + > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_MUX_EN, 1); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, > > + SENINF_SRC_SEL_EXT, SENINF_TEST_MODEL); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_SRC_SEL, 1); > > + > > + pix_sel_ext = 0; > > + pix_sel = 1; > > + > > This is in mtk_seninf_mux_setup(), but if you apply my suggestion, it won't be in > there anymore, so you'll call a function here to set the right value :-) > > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL_EXT, > > + SENINF_PIX_SEL_EXT, pix_sel_ext); > > + > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_PIX_SEL, pix_sel); > > + > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_PUSH_EN, 0x1f); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FLUSH_EN, 0x1b); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, FIFO_FULL_WR_EN, 2); > > + > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_POL, hs_pol); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_VSYNC_POL, vs_pol); > > + mtk_seninf_mux_update(mux, SENINF_MUX_CTRL, SENINF_HSYNC_MASK, 1); > > + > > + mtk_seninf_mux_write(mux, SENINF_MUX_INTEN, > > + SENINF_IRQ_CLR_SEL | SENINF_ALL_ERR_IRQ_EN); > > + > > + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, > > + mtk_seninf_mux_read(mux, SENINF_MUX_CTRL) | > > + SENINF_MUX_CTRL_SENINF_IRQ_SW_RST | > > + SENINF_MUX_CTRL_SENINF_MUX_SW_RST); > > + udelay(1); > > + mtk_seninf_mux_write(mux, SENINF_MUX_CTRL, > > + mtk_seninf_mux_read(mux, SENINF_MUX_CTRL) & > > + ~(SENINF_MUX_CTRL_SENINF_IRQ_SW_RST | > > + SENINF_MUX_CTRL_SENINF_MUX_SW_RST)); > > + > > + //check this > > + writel(0x76540010, priv->base + SENINF_TOP_CAM_MUX_CTRL); > > + > > + dev_dbg(priv->dev, "%s: OK\n", __func__); > > +} > > + > > Cheers, > Angelo >