Messages in this thread | | | Subject | Re: [PATCH v3 07/10] media: i2c: imx290: Add RAW12 mode support | From | Andrey Konovalov <> | Date | Wed, 27 May 2020 11:42:22 +0300 |
| |
Hi Dave,
On 26.05.2020 19:05, Dave Stevenson wrote: > Hi Andrey > > Thanks for the patch. > > On Sun, 24 May 2020 at 20:26, Andrey Konovalov > <andrey.konovalov@linaro.org> wrote: >> >> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> >> IMX290 is capable of outputting frames in both Raw Bayer (packed) 10 and >> 12 bit formats. Since the driver already supports RAW10 mode, let's add >> the missing RAW12 mode as well. >> >> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> >> --- >> drivers/media/i2c/imx290.c | 36 +++++++++++++++++++++++++++++++++--- >> 1 file changed, 33 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c >> index 162c345fffac..6e70ff22bc5f 100644 >> --- a/drivers/media/i2c/imx290.c >> +++ b/drivers/media/i2c/imx290.c >> @@ -71,6 +71,7 @@ struct imx290 { >> struct clk *xclk; >> struct regmap *regmap; >> u8 nlanes; >> + u8 bpp; >> >> struct v4l2_subdev sd; >> struct v4l2_fwnode_endpoint ep; >> @@ -90,10 +91,12 @@ struct imx290 { >> >> struct imx290_pixfmt { >> u32 code; >> + u8 bpp; >> }; >> >> static const struct imx290_pixfmt imx290_formats[] = { >> - { MEDIA_BUS_FMT_SRGGB10_1X10 }, >> + { MEDIA_BUS_FMT_SRGGB10_1X10, 10 }, >> + { MEDIA_BUS_FMT_SRGGB12_1X12, 12 }, >> }; >> >> static const struct regmap_config imx290_regmap_config = { >> @@ -261,6 +264,18 @@ static const struct imx290_regval imx290_10bit_settings[] = { >> { 0x300b, 0x00}, >> }; >> >> +static const struct imx290_regval imx290_12bit_settings[] = { >> + { 0x3005, 0x01 }, >> + { 0x3046, 0x01 }, >> + { 0x3129, 0x00 }, >> + { 0x317c, 0x00 }, >> + { 0x31ec, 0x0e }, >> + { 0x3441, 0x0c }, >> + { 0x3442, 0x0c }, >> + { 0x300a, 0xf0 }, >> + { 0x300b, 0x00 }, >> +}; >> + >> /* supported link frequencies */ >> static const s64 imx290_link_freq_2lanes[] = { >> 891000000, /* 1920x1080 - 2 lane */ >> @@ -421,7 +436,12 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) >> } else { >> imx290_write_reg(imx290, IMX290_PGCTRL, 0x00); >> msleep(10); >> - imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x3c); >> + if (imx290->bpp == 10) >> + imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, >> + 0x3c); >> + else /* 12 bits per pixel */ >> + imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, >> + 0xf0); >> imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00); >> } >> break; >> @@ -496,7 +516,7 @@ static u64 imx290_calc_pixel_rate(struct imx290 *imx290) >> u8 nlanes = imx290->nlanes; >> >> /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ >> - return (link_freq * 2 * nlanes / 10); >> + return (link_freq * 2 * nlanes / imx290->bpp); > > This doesn't link on a 32bit system as it's a 64bit divide: > ERROR: "__aeabi_ldivmod" [drivers/media/i2c/imx290.ko] undefined! > It ought to be using do_div().
Nice catch, thanks! I'll fix this in the next version of the patchset.
Thanks, Andrey
> Admittedly it didn't compile before as you still had a s64 divide by > 10, but I hadn't tried that :-) > > Dave > >> } >> >> static int imx290_set_fmt(struct v4l2_subdev *sd, >> @@ -533,6 +553,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, >> } else { >> format = &imx290->current_format; >> imx290->current_mode = mode; >> + imx290->bpp = imx290_formats[i].bpp; >> >> if (imx290->link_freq) >> __v4l2_ctrl_s_ctrl(imx290->link_freq, >> @@ -577,6 +598,15 @@ static int imx290_write_current_format(struct imx290 *imx290) >> return ret; >> } >> break; >> + case MEDIA_BUS_FMT_SRGGB12_1X12: >> + ret = imx290_set_register_array(imx290, imx290_12bit_settings, >> + ARRAY_SIZE( >> + imx290_12bit_settings)); >> + if (ret < 0) { >> + dev_err(imx290->dev, "Could not set format registers\n"); >> + return ret; >> + } >> + break; >> default: >> dev_err(imx290->dev, "Unknown pixel format\n"); >> return -EINVAL; >> -- >> 2.17.1 >>
| |