From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media@vger.kernel.org,
Alexander Stein <alexander.stein@ew.tq-group.com>
Subject: Re: [PATCH v2 11/13] media: i2c: imx290: Add support for 74.25MHz external clock
Date: Tue, 7 Feb 2023 03:50:02 +0200 [thread overview]
Message-ID: <Y+GuSqapyqV+Lp9b@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230203191811.947697-12-dave.stevenson@raspberrypi.com>
Hi Dave,
Thank you for the patch.
On Fri, Feb 03, 2023 at 07:18:09PM +0000, Dave Stevenson wrote:
> The sensor supports either a 37.125 or 74.25MHz external, but
> the driver only supported 37.125MHz.
>
> Add the relevant register configuration for either clock
> frequency option.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/imx290.c | 132 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 116 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index a74930e86a6c..045d27b4d31b 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -102,6 +102,7 @@
> #define IMX290_TCLKPREPARE IMX290_REG_16BIT(0x3452)
> #define IMX290_TLPX IMX290_REG_16BIT(0x3454)
> #define IMX290_X_OUT_SIZE IMX290_REG_16BIT(0x3472)
> +#define IMX290_INCKSEL7 IMX290_REG_8BIT(0x3480)
>
> #define IMX290_PGCTRL_REGEN BIT(0)
> #define IMX290_PGCTRL_THRU BIT(1)
> @@ -178,11 +179,29 @@ struct imx290_model_info {
> enum imx290_colour_variant colour_variant;
> };
>
> +enum imx290_clk_freq {
> + IMX290_CLK_37_125,
> + IMX290_CLK_74_25,
> + IMX290_NUM_CLK
> +};
> +
> struct imx290_regval {
> u32 reg;
> u32 val;
> };
>
> +/*
> + * Clock configuration for registers INCKSEL1 to INCKSEL6.
> + */
> +struct imx290_clk_cfg {
> + u8 incksel1;
> + u8 incksel2;
> + u8 incksel3;
> + u8 incksel4;
> + u8 incksel5;
> + u8 incksel6;
> +};
> +
> struct imx290_mode {
> u32 width;
> u32 height;
> @@ -192,6 +211,8 @@ struct imx290_mode {
>
> const struct imx290_regval *data;
> u32 data_size;
> +
> + const struct imx290_clk_cfg *clk_cfg;
> };
>
> struct imx290_csi_cfg {
> @@ -210,6 +231,7 @@ struct imx290 {
> struct device *dev;
> struct clk *xclk;
> struct regmap *regmap;
> + enum imx290_clk_freq xclk_idx;
> u8 nlanes;
> const struct imx290_model_info *model;
>
> @@ -238,7 +260,6 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
> */
>
> static const struct imx290_regval imx290_global_init_settings[] = {
> - { IMX290_EXTCK_FREQ, 0x2520 },
> { IMX290_WINWV_OB, 12 },
> { IMX290_WINPH, 0 },
> { IMX290_WINPV, 0 },
> @@ -288,7 +309,18 @@ static const struct imx290_regval imx290_global_init_settings[] = {
> { IMX290_REG_8BIT(0x33b0), 0x50 },
> { IMX290_REG_8BIT(0x33b2), 0x1a },
> { IMX290_REG_8BIT(0x33b3), 0x04 },
> - { IMX290_REG_8BIT(0x3480), 0x49 },
> +};
> +
> +#define IMX290_NUM_CLK_REGS 2
> +static const struct imx290_regval xclk_regs[][IMX290_NUM_CLK_REGS] = {
> + [IMX290_CLK_37_125] = {
> + { IMX290_EXTCK_FREQ, (37125 * 256) / 1000 },
> + { IMX290_INCKSEL7, 0x49 },
> + },
> + [IMX290_CLK_74_25] = {
> + { IMX290_EXTCK_FREQ, (74250 * 256) / 1000 },
> + { IMX290_INCKSEL7, 0x92 },
> + },
> };
>
> static const struct imx290_regval imx290_1080p_settings[] = {
> @@ -298,12 +330,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
> { IMX290_OPB_SIZE_V, 10 },
> { IMX290_X_OUT_SIZE, 1920 },
> { IMX290_Y_OUT_SIZE, 1080 },
> - { IMX290_INCKSEL1, 0x18 },
> - { IMX290_INCKSEL2, 0x03 },
> - { IMX290_INCKSEL3, 0x20 },
> - { IMX290_INCKSEL4, 0x01 },
> - { IMX290_INCKSEL5, 0x1a },
> - { IMX290_INCKSEL6, 0x1a },
> };
>
> static const struct imx290_regval imx290_720p_settings[] = {
> @@ -313,12 +339,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
> { IMX290_OPB_SIZE_V, 4 },
> { IMX290_X_OUT_SIZE, 1280 },
> { IMX290_Y_OUT_SIZE, 720 },
> - { IMX290_INCKSEL1, 0x20 },
> - { IMX290_INCKSEL2, 0x00 },
> - { IMX290_INCKSEL3, 0x20 },
> - { IMX290_INCKSEL4, 0x01 },
> - { IMX290_INCKSEL5, 0x1a },
> - { IMX290_INCKSEL6, 0x1a },
> };
>
> static const struct imx290_regval imx290_10bit_settings[] = {
> @@ -424,6 +444,48 @@ static inline int imx290_link_freqs_num(const struct imx290 *imx290)
> return ARRAY_SIZE(imx290_link_freq_4lanes);
> }
>
> +static const struct imx290_clk_cfg imx290_1080p_clock_config[] = {
> + [IMX290_CLK_37_125] = {
> + /* 37.125MHz clock config */
> + .incksel1 = 0x18,
> + .incksel2 = 0x03,
> + .incksel3 = 0x20,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1a,
> + .incksel6 = 0x1a,
> + },
> + [IMX290_CLK_74_25] = {
> + /* 74.25MHz clock config */
> + .incksel1 = 0x0c,
> + .incksel2 = 0x03,
> + .incksel3 = 0x10,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1b,
> + .incksel6 = 0x1b,
> + },
> +};
> +
> +static const struct imx290_clk_cfg imx290_720p_clock_config[] = {
> + [IMX290_CLK_37_125] = {
> + /* 37.125MHz clock config */
> + .incksel1 = 0x20,
> + .incksel2 = 0x00,
> + .incksel3 = 0x20,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1a,
> + .incksel6 = 0x1a,
> + },
> + [IMX290_CLK_74_25] = {
> + /* 74.25MHz clock config */
> + .incksel1 = 0x10,
> + .incksel2 = 0x00,
> + .incksel3 = 0x10,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1b,
> + .incksel6 = 0x1b,
> + },
> +};
> +
> /* Mode configs */
> static const struct imx290_mode imx290_modes_2lanes[] = {
> {
> @@ -434,6 +496,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
> .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> + .clk_cfg = imx290_1080p_clock_config,
> },
> {
> .width = 1280,
> @@ -443,6 +506,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
> .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> + .clk_cfg = imx290_720p_clock_config,
> },
> };
>
> @@ -455,6 +519,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
> .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> + .clk_cfg = imx290_1080p_clock_config,
> },
> {
> .width = 1280,
> @@ -464,6 +529,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
> .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> + .clk_cfg = imx290_720p_clock_config,
> },
> };
>
> @@ -587,6 +653,26 @@ static int imx290_set_register_array(struct imx290 *imx290,
> return 0;
> }
>
> +static int imx290_set_clock(struct imx290 *imx290)
> +{
> + const struct imx290_mode *mode = imx290->current_mode;
> + enum imx290_clk_freq clk_idx = imx290->xclk_idx;
> + const struct imx290_clk_cfg *clk_cfg = &mode->clk_cfg[clk_idx];
> + int ret;
> +
> + ret = imx290_set_register_array(imx290, xclk_regs[clk_idx],
> + IMX290_NUM_CLK_REGS);
> +
> + imx290_write(imx290, IMX290_INCKSEL1, clk_cfg->incksel1, &ret);
> + imx290_write(imx290, IMX290_INCKSEL2, clk_cfg->incksel2, &ret);
> + imx290_write(imx290, IMX290_INCKSEL3, clk_cfg->incksel3, &ret);
> + imx290_write(imx290, IMX290_INCKSEL4, clk_cfg->incksel4, &ret);
> + imx290_write(imx290, IMX290_INCKSEL5, clk_cfg->incksel5, &ret);
> + imx290_write(imx290, IMX290_INCKSEL6, clk_cfg->incksel6, &ret);
> +
> + return ret;
> +}
> +
> static int imx290_set_data_lanes(struct imx290 *imx290)
> {
> int ret = 0;
> @@ -891,6 +977,13 @@ static int imx290_start_streaming(struct imx290 *imx290,
> return ret;
> }
>
> + /* Set clock parameters based on mode and xclk */
> + ret = imx290_set_clock(imx290);
> + if (ret < 0) {
> + dev_err(imx290->dev, "Could not set clocks\n");
> + return ret;
> + }
> +
> /* Set data lane count */
> ret = imx290_set_data_lanes(imx290);
> if (ret < 0) {
> @@ -1290,8 +1383,15 @@ static int imx290_init_clk(struct imx290 *imx290)
> return ret;
> }
>
> - /* external clock must be 37.125 MHz */
> - if (xclk_freq != 37125000) {
> + /* external clock must be 37.125 MHz or 74.25MHz */
> + switch (xclk_freq) {
> + case 37125000:
> + imx290->xclk_idx = IMX290_CLK_37_125;
> + break;
> + case 74250000:
> + imx290->xclk_idx = IMX290_CLK_74_25;
> + break;
> + default:
> dev_err(imx290->dev, "External clock frequency %u is not supported\n",
> xclk_freq);
> return -EINVAL;
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2023-02-07 1:50 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-03 19:17 [PATCH v2 00/13] imx290: Minor fixes, support for alternate INCK, and more ctrls Dave Stevenson
2023-02-03 19:17 ` [PATCH v2 01/13] media: i2c: imx290: Match kernel coding style on whitespace Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 02/13] media: i2c: imx290: Set the colorspace fields in the format Dave Stevenson
2023-02-07 1:34 ` Laurent Pinchart
2023-02-03 19:18 ` [PATCH v2 03/13] media: i2c: imx290: Add V4L2_SUBDEV_FL_HAS_EVENTS and subscribe hooks Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 04/13] media: i2c: imx290: Fix the pixel rate at 148.5Mpix/s Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 05/13] media: i2c: imx290: Support 60fps in 2 lane operation Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 06/13] media: i2c: imx290: Use CSI timings as per datasheet Dave Stevenson
2023-02-06 7:48 ` Alexander Stein
2023-02-03 19:18 ` [PATCH v2 07/13] media: i2c: imx290: Convert V4L2_CID_HBLANK to read/write Dave Stevenson
2023-02-06 7:59 ` Alexander Stein
2023-02-03 19:18 ` [PATCH v2 08/13] media: i2c: imx290: Convert V4L2_CID_VBLANK " Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 09/13] media: i2c: imx290: VMAX is mode dependent Dave Stevenson
2023-02-07 1:46 ` Laurent Pinchart
2023-02-03 19:18 ` [PATCH v2 10/13] media: i2c: imx290: Remove duplicated write to IMX290_CTRL_07 Dave Stevenson
2023-02-03 19:18 ` [PATCH v2 11/13] media: i2c: imx290: Add support for 74.25MHz external clock Dave Stevenson
2023-02-06 8:24 ` Alexander Stein
2023-02-07 1:50 ` Laurent Pinchart [this message]
2023-02-03 19:18 ` [PATCH v2 12/13] media: i2c: imx290: Add support for H & V Flips Dave Stevenson
2023-02-07 1:52 ` Laurent Pinchart
2023-02-03 19:18 ` [PATCH v2 13/13] media: i2c: imx290: Add the error code to logs in start_streaming Dave Stevenson
2023-02-06 8:27 ` Alexander Stein
2023-02-07 1:53 ` Laurent Pinchart
2023-02-07 1:55 ` [PATCH v2 00/13] imx290: Minor fixes, support for alternate INCK, and more ctrls Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y+GuSqapyqV+Lp9b@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=alexander.stein@ew.tq-group.com \
--cc=dave.stevenson@raspberrypi.com \
--cc=linux-media@vger.kernel.org \
--cc=mani@kernel.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.