2020-07-24 15:08:55

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH 0/2] media: i2c: ov772x: Enable BT656 mode and test pattern support

Hi All,

This patch series adds support for BT656 mode in the ov772x sensor
and also enables color bar test pattern control.

Cheers,
Prabhakar

Lad Prabhakar (2):
media: i2c: ov772x: Add support for BT656 mode
media: i2c: ov772x: Add test pattern control

drivers/media/i2c/ov772x.c | 48 +++++++++++++++++++++++++++++++++++++++++++---
include/media/i2c/ov772x.h | 1 +
2 files changed, 46 insertions(+), 3 deletions(-)

--
2.7.4


2020-07-24 15:08:59

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH 1/2] media: i2c: ov772x: Add support for BT656 mode

Add support to read the bus-type and enable BT656 mode if needed.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/media/i2c/ov772x.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 2cc6a67..3b7dfba 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -31,6 +31,7 @@
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
#include <media/v4l2-image-sizes.h>
#include <media/v4l2-subdev.h>

@@ -434,6 +435,7 @@ struct ov772x_priv {
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
+ struct v4l2_fwnode_endpoint ep;
};

/*
@@ -585,7 +587,6 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
enable ? 0 : SOFT_SLEEP_MODE);
if (ret)
goto done;
-
if (enable) {
dev_dbg(&client->dev, "format %d, win %s\n",
priv->cfmt->code, priv->win->name);
@@ -1104,7 +1105,10 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;

/* COM7: Sensor resolution and output format control. */
- ret = regmap_write(priv->regmap, COM7, win->com7_bit | cfmt->com7);
+ val = win->com7_bit | cfmt->com7;
+ if (priv->ep.bus_type == V4L2_MBUS_BT656)
+ val |= ITU656_ON_OFF;
+ ret = regmap_write(priv->regmap, COM7, val);
if (ret < 0)
goto ov772x_set_fmt_error;

@@ -1354,6 +1358,7 @@ static const struct v4l2_subdev_ops ov772x_subdev_ops = {

static int ov772x_probe(struct i2c_client *client)
{
+ struct fwnode_handle *endpoint;
struct ov772x_priv *priv;
int ret;
static const struct regmap_config ov772x_regmap_config = {
@@ -1415,6 +1420,20 @@ static int ov772x_probe(struct i2c_client *client)
goto error_clk_put;
}

+ endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
+ NULL);
+ if (!endpoint) {
+ dev_err(&client->dev, "endpoint node not found\n");
+ goto error_clk_put;
+ }
+
+ ret = v4l2_fwnode_endpoint_parse(endpoint, &priv->ep);
+ fwnode_handle_put(endpoint);
+ if (ret) {
+ dev_err(&client->dev, "Could not parse endpoint\n");
+ goto error_clk_put;
+ }
+
ret = ov772x_video_probe(priv);
if (ret < 0)
goto error_gpio_put;
--
2.7.4

2020-07-24 15:10:03

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH 2/2] media: i2c: ov772x: Add test pattern control

Add support for test pattern control supported by the sensor.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
drivers/media/i2c/ov772x.c | 25 ++++++++++++++++++++++++-
include/media/i2c/ov772x.h | 1 +
2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 3b7dfba..bdee2a8 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -227,7 +227,7 @@

/* COM3 */
#define SWAP_MASK (SWAP_RGB | SWAP_YUV | SWAP_ML)
-#define IMG_MASK (VFLIP_IMG | HFLIP_IMG)
+#define IMG_MASK (VFLIP_IMG | HFLIP_IMG | SCOLOR_TEST)

#define VFLIP_IMG 0x80 /* Vertical flip image ON/OFF selection */
#define HFLIP_IMG 0x40 /* Horizontal mirror image ON/OFF selection */
@@ -425,6 +425,7 @@ struct ov772x_priv {
const struct ov772x_win_size *win;
struct v4l2_ctrl *vflip_ctrl;
struct v4l2_ctrl *hflip_ctrl;
+ unsigned int test_pattern;
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
struct v4l2_ctrl *band_filter_ctrl;
unsigned int fps;
@@ -540,6 +541,11 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
},
};

+static const char * const ov772x_test_pattern_menu[] = {
+ "Disabled",
+ "Vertical Color Bar Type 1",
+};
+
/*
* frame rate settings lists
*/
@@ -754,6 +760,13 @@ static int ov772x_s_frame_interval(struct v4l2_subdev *sd,
return ret;
}

+static int ov772x_enable_test_pattern(struct ov772x_priv *priv, u32 pattern)
+{
+ priv->test_pattern = pattern;
+ return regmap_update_bits(priv->regmap, COM3, SCOLOR_TEST,
+ pattern ? SCOLOR_TEST : 0x00);
+}
+
static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct ov772x_priv *priv = container_of(ctrl->handler,
@@ -801,6 +814,8 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
}

return ret;
+ case V4L2_CID_TEST_PATTERN:
+ return ov772x_enable_test_pattern(priv, ctrl->val);
}

return -EINVAL;
@@ -1095,10 +1110,14 @@ static int ov772x_set_params(struct ov772x_priv *priv,
val |= VFLIP_IMG;
if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val |= HFLIP_IMG;
+ if (priv->info && (priv->info->flags & OV772X_FLAG_TEST_PATTERN))
+ val |= SCOLOR_TEST;
if (priv->vflip_ctrl->val)
val ^= VFLIP_IMG;
if (priv->hflip_ctrl->val)
val ^= HFLIP_IMG;
+ if (priv->test_pattern)
+ val ^= SCOLOR_TEST;

ret = regmap_update_bits(priv->regmap, COM3, SWAP_MASK | IMG_MASK, val);
if (ret < 0)
@@ -1399,6 +1418,10 @@ static int ov772x_probe(struct i2c_client *client)
priv->band_filter_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
V4L2_CID_BAND_STOP_FILTER,
0, 256, 1, 0);
+ v4l2_ctrl_new_std_menu_items(&priv->hdl, &ov772x_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(ov772x_test_pattern_menu) - 1,
+ 0, 0, ov772x_test_pattern_menu);
priv->subdev.ctrl_handler = &priv->hdl;
if (priv->hdl.error) {
ret = priv->hdl.error;
diff --git a/include/media/i2c/ov772x.h b/include/media/i2c/ov772x.h
index a1702d4..65e6f8d 100644
--- a/include/media/i2c/ov772x.h
+++ b/include/media/i2c/ov772x.h
@@ -12,6 +12,7 @@
/* for flags */
#define OV772X_FLAG_VFLIP (1 << 0) /* Vertical flip image */
#define OV772X_FLAG_HFLIP (1 << 1) /* Horizontal flip image */
+#define OV772X_FLAG_TEST_PATTERN (1 << 2) /* Test pattern */

/*
* for Edge ctrl
--
2.7.4

2020-07-25 07:54:48

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] media: i2c: ov772x: Add support for BT656 mode

Hi Lad,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.8-rc6 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/media-i2c-ov772x-Enable-BT656-mode-and-test-pattern-support/20200724-231016
base: git://linuxtv.org/media_tree.git master
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1d09ecf36175f7910ffedd6d497c07b5c74c22fb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/ov772x.c:1425:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!endpoint) {
^~~~~~~~~
drivers/media/i2c/ov772x.c:1471:9: note: uninitialized use occurs here
return ret;
^~~
drivers/media/i2c/ov772x.c:1425:2: note: remove the 'if' if its condition is always false
if (!endpoint) {
^~~~~~~~~~~~~~~~
drivers/media/i2c/ov772x.c:1363:11: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
1 warning generated.

vim +1425 drivers/media/i2c/ov772x.c

1354
1355 /*
1356 * i2c_driver function
1357 */
1358
1359 static int ov772x_probe(struct i2c_client *client)
1360 {
1361 struct fwnode_handle *endpoint;
1362 struct ov772x_priv *priv;
1363 int ret;
1364 static const struct regmap_config ov772x_regmap_config = {
1365 .reg_bits = 8,
1366 .val_bits = 8,
1367 .max_register = DSPAUTO,
1368 };
1369
1370 if (!client->dev.of_node && !client->dev.platform_data) {
1371 dev_err(&client->dev,
1372 "Missing ov772x platform data for non-DT device\n");
1373 return -EINVAL;
1374 }
1375
1376 priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
1377 if (!priv)
1378 return -ENOMEM;
1379
1380 priv->regmap = devm_regmap_init_sccb(client, &ov772x_regmap_config);
1381 if (IS_ERR(priv->regmap)) {
1382 dev_err(&client->dev, "Failed to allocate register map\n");
1383 return PTR_ERR(priv->regmap);
1384 }
1385
1386 priv->info = client->dev.platform_data;
1387 mutex_init(&priv->lock);
1388
1389 v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops);
1390 priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
1391 V4L2_SUBDEV_FL_HAS_EVENTS;
1392 v4l2_ctrl_handler_init(&priv->hdl, 3);
1393 /* Use our mutex for the controls */
1394 priv->hdl.lock = &priv->lock;
1395 priv->vflip_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1396 V4L2_CID_VFLIP, 0, 1, 1, 0);
1397 priv->hflip_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1398 V4L2_CID_HFLIP, 0, 1, 1, 0);
1399 priv->band_filter_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1400 V4L2_CID_BAND_STOP_FILTER,
1401 0, 256, 1, 0);
1402 priv->subdev.ctrl_handler = &priv->hdl;
1403 if (priv->hdl.error) {
1404 ret = priv->hdl.error;
1405 goto error_mutex_destroy;
1406 }
1407
1408 priv->clk = clk_get(&client->dev, NULL);
1409 if (IS_ERR(priv->clk)) {
1410 dev_err(&client->dev, "Unable to get xclk clock\n");
1411 ret = PTR_ERR(priv->clk);
1412 goto error_ctrl_free;
1413 }
1414
1415 priv->pwdn_gpio = gpiod_get_optional(&client->dev, "powerdown",
1416 GPIOD_OUT_LOW);
1417 if (IS_ERR(priv->pwdn_gpio)) {
1418 dev_info(&client->dev, "Unable to get GPIO \"powerdown\"");
1419 ret = PTR_ERR(priv->pwdn_gpio);
1420 goto error_clk_put;
1421 }
1422
1423 endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
1424 NULL);
> 1425 if (!endpoint) {
1426 dev_err(&client->dev, "endpoint node not found\n");
1427 goto error_clk_put;
1428 }
1429
1430 ret = v4l2_fwnode_endpoint_parse(endpoint, &priv->ep);
1431 fwnode_handle_put(endpoint);
1432 if (ret) {
1433 dev_err(&client->dev, "Could not parse endpoint\n");
1434 goto error_clk_put;
1435 }
1436
1437 ret = ov772x_video_probe(priv);
1438 if (ret < 0)
1439 goto error_gpio_put;
1440

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.15 kB)
.config.gz (73.58 kB)
Download all attachments

2020-07-31 16:05:26

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH 1/2] media: i2c: ov772x: Add support for BT656 mode

Hi Prabhakar,

On Fri, Jul 24, 2020 at 04:08:15PM +0100, Lad Prabhakar wrote:
> Add support to read the bus-type and enable BT656 mode if needed.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>

The DT bindings should also be changed.

The default should be parallel, I guess, since the type hasn't been
documented. Parsing should be also updated so the driver can set meaningful
defaults for the flags --- this btw. also applies to the corresponding
ov5640 patch.

--
Sakari Ailus

2020-08-01 08:55:35

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH 1/2] media: i2c: ov772x: Add support for BT656 mode

Hi Sakari,

Thank you for the review.

On Fri, Jul 31, 2020 at 5:03 PM Sakari Ailus <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jul 24, 2020 at 04:08:15PM +0100, Lad Prabhakar wrote:
> > Add support to read the bus-type and enable BT656 mode if needed.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Biju Das <[email protected]>
>
> The DT bindings should also be changed.
>
I'll get this done in V2.

> The default should be parallel, I guess, since the type hasn't been
> documented. Parsing should be also updated so the driver can set meaningful
> defaults for the flags --- this btw. also applies to the corresponding
> ov5640 patch.
>
Agreed, will do.

Cheers,
Prabhakar
> --
> Sakari Ailus