RE: [PATCH/RFC v1 2/9] media: i2c: Add a driver for the onsemi AR0144 camera sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

I want to add some details about communicating with the vendor. He share with me
ar0144's spec, I can see the min/max val of params such as sys_div/ pre_pll_clk_div
and pll_multiplier. 
But there is no relevant definition in the spec of ar0234, only ext_clk and pll_op_clk
have min and max val ​​defined. This makes it difficult for me to dynamically calculate
the pll based on ext_clk.

Thanks,
Dongcheng

> -----Original Message-----
> From: Bingbu Cao <bingbu.cao@xxxxxxxxxxxxxxx>
> Sent: Thursday, September 26, 2024 4:03 PM
> To: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>;
> linux-media@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Cc: Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>;
> Conor Dooley <conor+dt@xxxxxxxxxx>; Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx>; Hans Verkuil <hverkuil-cisco@xxxxxxxxx>; Yan,
> Dongcheng <dongcheng.yan@xxxxxxxxx>
> Subject: Re: [PATCH/RFC v1 2/9] media: i2c: Add a driver for the onsemi
> AR0144 camera sensor
> 
> Laurent,
> 
> On 6/30/24 10:17 PM, Laurent Pinchart wrote:
> > The AR0144 is a 1/4" 1MP CMOS camera sensor from onsemi. The driver
> > supports both the monochrome and color versions, and both the parallel
> > and MIPI CSI-2 interfaces. Due to limitations of the test platform,
> > only the CSI-2 output has been tested.
> >
> > The driver supports 8-, 10- and 12-bit output formats, analog crop and
> > binning/skipping. It exposes controls that cover the usual use cases
> > for camera sensors.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> >  MAINTAINERS                |    1 +
> >  drivers/media/i2c/Kconfig  |   11 +
> >  drivers/media/i2c/Makefile |    1 +
> >  drivers/media/i2c/ar0144.c | 1826
> > ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1839 insertions(+)
> >  create mode 100644 drivers/media/i2c/ar0144.c
> >
> ...
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * PLL
> > + */
> > +
> > +static int ar0144_pll_calculate(struct ar0144 *sensor, struct ccs_pll *pll,
> > +				unsigned int link_freq, unsigned int bpp) {
> > +	struct ccs_pll_limits limits = {
> > +		.min_ext_clk_freq_hz = 6000000,
> > +		.max_ext_clk_freq_hz = 48000000,
> > +
> > +		.vt_fr = {
> > +			.min_pre_pll_clk_div = 1,
> > +			.max_pre_pll_clk_div = 63,
> > +			.min_pll_ip_clk_freq_hz = 1000000,	/*
> min_pll_op_clk_freq_hz / max_pll_multiplier */
> > +			.max_pll_ip_clk_freq_hz = 24000000,	/*
> max_pll_op_clk_freq_hz / min_pll_multiplier */
> > +			.min_pll_multiplier = 32,
> > +			.max_pll_multiplier = 384,
> > +			.min_pll_op_clk_freq_hz = 384000000,
> > +			.max_pll_op_clk_freq_hz = 768000000,
> > +		},
> > +		.vt_bk = {
> > +			.min_sys_clk_div = 1,
> > +			.max_sys_clk_div = 16,
> > +			.min_pix_clk_div = 4,
> > +			.max_pix_clk_div = 16,
> > +			.min_pix_clk_freq_hz = 6000000,		/* No documented limit
> */
> > +			.max_pix_clk_freq_hz = 74250000,
> > +		},
> > +		.op_bk = {
> > +			.min_sys_clk_div = 1,
> > +			.max_sys_clk_div = 16,
> > +			.min_pix_clk_div = 8,
> > +			.max_pix_clk_div = 12,
> > +			.min_pix_clk_freq_hz = 6000000,		/* No documented limit
> */
> > +			.max_pix_clk_freq_hz = 74250000,
> > +		},
> > +
> > +		.min_line_length_pck_bin = 1200 + AR0144_MIN_HBLANK,	/* To
> be checked */
> > +		.min_line_length_pck = 1200 + AR0144_MIN_HBLANK,
> > +	};
> > +	unsigned int num_lanes =
> sensor->bus_cfg.bus.mipi_csi2.num_data_lanes;
> > +	int ret;
> > +
> > +	/*
> > +	 * The OP pixel clock limits depends on the number of lanes, which the
> > +	 * PLL calculator doesn't take into account despite specifying the
> > +	 * CCS_PLL_FLAG_LANE_SPEED_MODEL flag. Adjust them manually.
> > +	 */
> > +	limits.op_bk.min_pix_clk_freq_hz /= num_lanes;
> > +	limits.op_bk.max_pix_clk_freq_hz /= num_lanes;
> > +
> > +	/*
> > +	 * There are no documented constraints on the sys clock frequency, for
> > +	 * either branch. Recover them based on the PLL output clock frequency
> > +	 * and sys_clk_div limits on one hand, and the pix clock frequency and
> > +	 * the pix_clk_div limits on the other hand.
> > +	 */
> > +	limits.vt_bk.min_sys_clk_freq_hz =
> > +		max(limits.vt_fr.min_pll_op_clk_freq_hz /
> limits.vt_bk.max_sys_clk_div,
> > +		    limits.vt_bk.min_pix_clk_freq_hz *
> limits.vt_bk.min_pix_clk_div);
> > +	limits.vt_bk.max_sys_clk_freq_hz =
> > +		min(limits.vt_fr.max_pll_op_clk_freq_hz /
> limits.vt_bk.min_sys_clk_div,
> > +		    limits.vt_bk.max_pix_clk_freq_hz *
> > +limits.vt_bk.max_pix_clk_div);
> > +
> > +	limits.op_bk.min_sys_clk_freq_hz =
> > +		max(limits.vt_fr.min_pll_op_clk_freq_hz /
> limits.op_bk.max_sys_clk_div,
> > +		    limits.op_bk.min_pix_clk_freq_hz *
> limits.op_bk.min_pix_clk_div);
> > +	limits.op_bk.max_sys_clk_freq_hz =
> > +		min(limits.vt_fr.max_pll_op_clk_freq_hz /
> limits.op_bk.min_sys_clk_div,
> > +		    limits.op_bk.max_pix_clk_freq_hz *
> > +limits.op_bk.max_pix_clk_div);
> > +
> > +	memset(pll, 0, sizeof(*pll));
> > +
> > +	pll->bus_type = CCS_PLL_BUS_TYPE_CSI2_DPHY;
> > +	pll->op_lanes = num_lanes;
> > +	pll->vt_lanes = 1;
> > +	pll->csi2.lanes = num_lanes;
> > +	/*
> > +	 * As the sensor doesn't support FIFO derating, binning doesn't
> > +	 * influence the PLL configuration. Hardcode the binning factors.
> > +	 */
> > +	pll->binning_horizontal = 1;
> > +	pll->binning_vertical = 1;
> > +	pll->scale_m = 1;
> > +	pll->scale_n = 1;
> > +	pll->bits_per_pixel = bpp;
> > +	pll->flags = CCS_PLL_FLAG_LANE_SPEED_MODEL;
> > +	pll->link_freq = link_freq;
> > +	pll->ext_clk_freq_hz = clk_get_rate(sensor->clk);
> > +
> > +	ret = ccs_pll_calculate(sensor->dev, &limits, pll);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * The sensor ignores the LSB of the PLL multiplier. If the multiplier
> > +	 * is an odd value, as a workaround to avoid precision loss, multiply
> > +	 * both the pre-divider and the multiplier by 2 if this doesn't bring
> > +	 * them beyond their maximum values. This doesn't necessarily
> guarantee
> > +	 * optimum PLL parameters. Ideally the PLL calculator should handle
> > +	 * this constraint.
> > +	 */
> > +	if ((pll->vt_fr.pll_multiplier & 1) &&
> > +	    pll->vt_fr.pll_multiplier * 2 <= limits.vt_fr.max_pll_multiplier &&
> > +	    pll->vt_fr.pre_pll_clk_div * 2 <= limits.vt_fr.max_pre_pll_clk_div) {
> > +		pll->vt_fr.pll_multiplier *= 2;
> > +		pll->vt_fr.pre_pll_clk_div *= 2;
> > +	}
> > +
> > +	if (pll->vt_fr.pll_multiplier & 1)
> > +		dev_warn(sensor->dev,
> > +			 "Odd PLL multiplier, link frequency %u will not be exact\n",
> > +			 pll->link_freq);
> > +
> > +	return 0;
> > +}
> 
> Dongcheng and I are trying to calculate the AR pll like code here. But we did
> not find any datasheet or manual to refer to. Even vendor has no idea. Could
> you share which doc can help us to do that?
> 
> > +
> > +static int ar0144_pll_update(struct ar0144 *sensor,
> > +			     const struct ar0144_format_info *info) {
> > +	u64 link_freq;
> > +	int ret;
> > +
> > +	link_freq = sensor->bus_cfg.link_frequencies[sensor->link_freq->val];
> > +	ret = ar0144_pll_calculate(sensor, &sensor->pll, link_freq, info->bpp);
> > +	if (ret) {
> > +		dev_err(sensor->dev, "PLL calculations failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate,
> > +				 sensor->pll.pixel_rate_pixel_array);
> > +
> > +	return 0;
> > +}
> > +
> ...
> >
> 
> --
> Best regards,
> Bingbu Cao




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux