Re: [PATCH v6 3/5] media: platform: mediatek: isp_30: add mediatek ISP3.0 sensor interface

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

Hi, Julien:

On Mon, 2024-07-29 at 16:48 +0200, Julien Stephan wrote:
>  	 
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>  From: Louis Kuo <louis.kuo@mediatek.com>
> 
> 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@mediatek.com>
> Signed-off-by: Phi-bang Nguyen <pnguyen@baylibre.com>
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Co-developed-by: Julien Stephan <jstephan@baylibre.com>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---

[snip]

> +static int mtk_seninf_input_setup_csi2(struct mtk_seninf *priv,
> +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;
> +s64 clock_count;
> +
> +format = v4l2_subdev_state_get_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 */
> +/*
> + * Configure phase counter. 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(0x0, input->base + SENINF_TG1_PH_CNT);
> +
> +clock_count = mtk_seninf_get_clk_divider(priv, input->seninf_id, fmtinfo->bpp,
> + num_data_lanes);
> +if (clock_count < 0)
> +return clock_count;
> +
> +clock_count = FIELD_PREP(GENMASK(21, 16), clock_count) | 0x1;
> +writel(clock_count, input->base + SENINF_TG1_SEN_CK);
> +
> +/* 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);
> +
> +/* 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);
> +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);
> +
> +/* 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);
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_STATUS_EXT);
> +/* Enable CSI2 Extend IRQ mask */
> +writel(0xffffffff, input->base + SENINF_CSI2_INT_EN_EXT);

Does the IRQ means the CPU would receive an interrupt?
But this driver does not request interrupt handler, so enable IRQ seems redundant.

Regards,
CK

> +/* Reset the CSI2 to commit changes */
> +mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 1);
> +udelay(1);
> +mtk_seninf_input_update(input, SENINF_CTRL, CSI2_SW_RST, 0);
> +
> +return 0;
> +}
> +
>