[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3597b850-5ce6-0e88-8f1f-e16bad5f75ef@linaro.org>
Date: Wed, 27 May 2020 11:42:22 +0300
From: Andrey Konovalov <andrey.konovalov@...aro.org>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
Sakari Ailus <sakari.ailus@....fi>,
manivannan.sadhasivam@...aro.org,
Linux Media Mailing List <linux-media@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, c.barrett@...mos.com,
a.brela@...mos.com, Peter Griffin <peter.griffin@...aro.org>
Subject: Re: [PATCH v3 07/10] media: i2c: imx290: Add RAW12 mode support
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@...aro.org> wrote:
>>
>> From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.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@...aro.org>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov@...aro.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
>>
Powered by blists - more mailing lists