diff mbox series

media: imx258: add vblank control to support wide frame rate range

Message ID 1634280771-656-1-git-send-email-bingbu.cao@intel.com (mailing list archive)
State New, archived
Headers show
Series media: imx258: add vblank control to support wide frame rate range | expand

Commit Message

Bingbu Cao Oct. 15, 2021, 6:52 a.m. UTC 
Current imx258 driver enable the automatic frame length tracking control
by default and did not support VBLANK change, it's always working at 30fps.
However, in reality we need a wider frame rate range from 15 to 30.
This patch disable the automatic frame length tracking control and enable
the v4l2 VBLANK control to allow user changing frame rate per requirement.

Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/i2c/imx258.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

kernel test robot Oct. 15, 2021, 9 a.m. UTC | #1 
Hi Bingbu,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.15-rc5 next-20211013]
[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/Bingbu-Cao/media-imx258-add-vblank-control-to-support-wide-frame-rate-range/20211015-150254
base:   git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r042-20211014 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project acb3b187c4c88650a6a717a1bcb234d27d0d7f54)
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 riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/8775f7beb99fa27ed3d3f64b289d9727963ca0c3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bingbu-Cao/media-imx258-add-vblank-control-to-support-wide-frame-rate-range/20211015-150254
        git checkout 8775f7beb99fa27ed3d3f64b289d9727963ca0c3
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/media/i2c/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/i2c/imx258.c:766:3: error: 'break' statement not in loop or switch statement
                   break;
                   ^
   1 error generated.


vim +/break +766 drivers/media/i2c/imx258.c

   751	
   752	static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
   753	{
   754		struct imx258 *imx258 =
   755			container_of(ctrl->handler, struct imx258, ctrl_handler);
   756		struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
   757		s64 max;
   758		int ret = 0;
   759	
   760		if (ctrl->id == V4L2_CID_VBLANK) {
   761			/* Update max exposure to meet expected vblanking */
   762			max = imx258->cur_mode->height + ctrl->val - 10;
   763			__v4l2_ctrl_modify_range(imx258->exposure,
   764						 imx258->exposure->minimum,
   765						 max, imx258->exposure->step, max);
 > 766			break;
   767		}
   768	
   769		/*
   770		 * Applying V4L2 control value only happens
   771		 * when power is up for streaming
   772		 */
   773		if (pm_runtime_get_if_in_use(&client->dev) == 0)
   774			return 0;
   775	
   776		switch (ctrl->id) {
   777		case V4L2_CID_ANALOGUE_GAIN:
   778			ret = imx258_write_reg(imx258, IMX258_REG_ANALOG_GAIN,
   779					IMX258_REG_VALUE_16BIT,
   780					ctrl->val);
   781			break;
   782		case V4L2_CID_EXPOSURE:
   783			ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
   784					IMX258_REG_VALUE_16BIT,
   785					ctrl->val);
   786			break;
   787		case V4L2_CID_VBLANK:
   788			ret = imx258_write_reg(imx258, IMX258_REG_FLL, 2,
   789					       imx258->cur_mode->height + ctrl->val);
   790			break;
   791		case V4L2_CID_DIGITAL_GAIN:
   792			ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
   793					ctrl->val);
   794			break;
   795		case V4L2_CID_TEST_PATTERN:
   796			ret = imx258_write_reg(imx258, IMX258_REG_TEST_PATTERN,
   797					IMX258_REG_VALUE_16BIT,
   798					ctrl->val);
   799			ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
   800					IMX258_REG_VALUE_08BIT,
   801					!ctrl->val ? REG_CONFIG_MIRROR_FLIP :
   802					REG_CONFIG_FLIP_TEST_PATTERN);
   803			break;
   804		case V4L2_CID_WIDE_DYNAMIC_RANGE:
   805			if (!ctrl->val) {
   806				ret = imx258_write_reg(imx258, IMX258_REG_HDR,
   807						       IMX258_REG_VALUE_08BIT,
   808						       IMX258_HDR_RATIO_MIN);
   809			} else {
   810				ret = imx258_write_reg(imx258, IMX258_REG_HDR,
   811						       IMX258_REG_VALUE_08BIT,
   812						       IMX258_HDR_ON);
   813				if (ret)
   814					break;
   815				ret = imx258_write_reg(imx258, IMX258_REG_HDR_RATIO,
   816						       IMX258_REG_VALUE_08BIT,
   817						       BIT(IMX258_HDR_RATIO_MAX));
   818			}
   819			break;
   820		default:
   821			dev_info(&client->dev,
   822				 "ctrl(id:0x%x,val:0x%x) is not handled\n",
   823				 ctrl->id, ctrl->val);
   824			ret = -EINVAL;
   825			break;
   826		}
   827	
   828		pm_runtime_put(&client->dev);
   829	
   830		return ret;
   831	}
   832	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Oct. 15, 2021, 9:11 a.m. UTC | #2 
Hi Bingbu,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.15-rc5 next-20211013]
[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/Bingbu-Cao/media-imx258-add-vblank-control-to-support-wide-frame-rate-range/20211015-150254
base:   git://linuxtv.org/media_tree.git master
config: alpha-randconfig-r014-20211014 (attached as .config)
compiler: alpha-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/8775f7beb99fa27ed3d3f64b289d9727963ca0c3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bingbu-Cao/media-imx258-add-vblank-control-to-support-wide-frame-rate-range/20211015-150254
        git checkout 8775f7beb99fa27ed3d3f64b289d9727963ca0c3
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/media/i2c/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/media/i2c/imx258.c: In function 'imx258_set_ctrl':
>> drivers/media/i2c/imx258.c:766:17: error: break statement not within loop or switch
     766 |                 break;
         |                 ^~~~~


vim +766 drivers/media/i2c/imx258.c

   751	
   752	static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
   753	{
   754		struct imx258 *imx258 =
   755			container_of(ctrl->handler, struct imx258, ctrl_handler);
   756		struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
   757		s64 max;
   758		int ret = 0;
   759	
   760		if (ctrl->id == V4L2_CID_VBLANK) {
   761			/* Update max exposure to meet expected vblanking */
   762			max = imx258->cur_mode->height + ctrl->val - 10;
   763			__v4l2_ctrl_modify_range(imx258->exposure,
   764						 imx258->exposure->minimum,
   765						 max, imx258->exposure->step, max);
 > 766			break;
   767		}
   768	
   769		/*
   770		 * Applying V4L2 control value only happens
   771		 * when power is up for streaming
   772		 */
   773		if (pm_runtime_get_if_in_use(&client->dev) == 0)
   774			return 0;
   775	
   776		switch (ctrl->id) {
   777		case V4L2_CID_ANALOGUE_GAIN:
   778			ret = imx258_write_reg(imx258, IMX258_REG_ANALOG_GAIN,
   779					IMX258_REG_VALUE_16BIT,
   780					ctrl->val);
   781			break;
   782		case V4L2_CID_EXPOSURE:
   783			ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
   784					IMX258_REG_VALUE_16BIT,
   785					ctrl->val);
   786			break;
   787		case V4L2_CID_VBLANK:
   788			ret = imx258_write_reg(imx258, IMX258_REG_FLL, 2,
   789					       imx258->cur_mode->height + ctrl->val);
   790			break;
   791		case V4L2_CID_DIGITAL_GAIN:
   792			ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
   793					ctrl->val);
   794			break;
   795		case V4L2_CID_TEST_PATTERN:
   796			ret = imx258_write_reg(imx258, IMX258_REG_TEST_PATTERN,
   797					IMX258_REG_VALUE_16BIT,
   798					ctrl->val);
   799			ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
   800					IMX258_REG_VALUE_08BIT,
   801					!ctrl->val ? REG_CONFIG_MIRROR_FLIP :
   802					REG_CONFIG_FLIP_TEST_PATTERN);
   803			break;
   804		case V4L2_CID_WIDE_DYNAMIC_RANGE:
   805			if (!ctrl->val) {
   806				ret = imx258_write_reg(imx258, IMX258_REG_HDR,
   807						       IMX258_REG_VALUE_08BIT,
   808						       IMX258_HDR_RATIO_MIN);
   809			} else {
   810				ret = imx258_write_reg(imx258, IMX258_REG_HDR,
   811						       IMX258_REG_VALUE_08BIT,
   812						       IMX258_HDR_ON);
   813				if (ret)
   814					break;
   815				ret = imx258_write_reg(imx258, IMX258_REG_HDR_RATIO,
   816						       IMX258_REG_VALUE_08BIT,
   817						       BIT(IMX258_HDR_RATIO_MAX));
   818			}
   819			break;
   820		default:
   821			dev_info(&client->dev,
   822				 "ctrl(id:0x%x,val:0x%x) is not handled\n",
   823				 ctrl->id, ctrl->val);
   824			ret = -EINVAL;
   825			break;
   826		}
   827	
   828		pm_runtime_put(&client->dev);
   829	
   830		return ret;
   831	}
   832	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 81cdf37216ca..106034ebc1b6 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -29,6 +29,7 @@ 
 #define IMX258_VTS_MAX			0xffff
 
 /*Frame Length Line*/
+#define IMX258_REG_FLL			0x0340
 #define IMX258_FLL_MIN			0x08a6
 #define IMX258_FLL_MAX			0xffff
 #define IMX258_FLL_STEP			1
@@ -241,7 +242,7 @@  static const struct imx258_reg mode_4208x3118_regs[] = {
 	{ 0x034D, 0x70 },
 	{ 0x034E, 0x0C },
 	{ 0x034F, 0x30 },
-	{ 0x0350, 0x01 },
+	{ 0x0350, 0x00 },
 	{ 0x0202, 0x0C },
 	{ 0x0203, 0x46 },
 	{ 0x0204, 0x00 },
@@ -360,7 +361,7 @@  static const struct imx258_reg mode_2104_1560_regs[] = {
 	{ 0x034D, 0x38 },
 	{ 0x034E, 0x06 },
 	{ 0x034F, 0x18 },
-	{ 0x0350, 0x01 },
+	{ 0x0350, 0x00 },
 	{ 0x0202, 0x06 },
 	{ 0x0203, 0x2E },
 	{ 0x0204, 0x00 },
@@ -479,7 +480,7 @@  static const struct imx258_reg mode_1048_780_regs[] = {
 	{ 0x034D, 0x18 },
 	{ 0x034E, 0x03 },
 	{ 0x034F, 0x0C },
-	{ 0x0350, 0x01 },
+	{ 0x0350, 0x00 },
 	{ 0x0202, 0x03 },
 	{ 0x0203, 0x42 },
 	{ 0x0204, 0x00 },
@@ -753,8 +754,18 @@  static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
 	struct imx258 *imx258 =
 		container_of(ctrl->handler, struct imx258, ctrl_handler);
 	struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
+	s64 max;
 	int ret = 0;
 
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		/* Update max exposure to meet expected vblanking */
+		max = imx258->cur_mode->height + ctrl->val - 10;
+		__v4l2_ctrl_modify_range(imx258->exposure,
+					 imx258->exposure->minimum,
+					 max, imx258->exposure->step, max);
+		break;
+	}
+
 	/*
 	 * Applying V4L2 control value only happens
 	 * when power is up for streaming
@@ -773,6 +784,10 @@  static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
 				IMX258_REG_VALUE_16BIT,
 				ctrl->val);
 		break;
+	case V4L2_CID_VBLANK:
+		ret = imx258_write_reg(imx258, IMX258_REG_FLL, 2,
+				       imx258->cur_mode->height + ctrl->val);
+		break;
 	case V4L2_CID_DIGITAL_GAIN:
 		ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
 				ctrl->val);
@@ -1189,9 +1204,6 @@  static int imx258_init_controls(struct imx258 *imx258)
 				IMX258_VTS_MAX - imx258->cur_mode->height, 1,
 				vblank_def);
 
-	if (imx258->vblank)
-		imx258->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
 	imx258->hblank = v4l2_ctrl_new_std(
 				ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_HBLANK,
 				IMX258_PPL_DEFAULT - imx258->cur_mode->width,