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