diff mbox series

[5/5] media: i2c: imx219: Simplify code handling in s_fmt

Message ID	20230704104057.149837-6-jacopo.mondi@ideasonboard.com (mailing list archive)
State	Superseded
Headers	From: Jacopo Mondi <jacopo.mondi@ideasonboard.com> To: linux-media@vger.kernel.org Cc: Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Dave Stevenson <dave.stevenson@raspberrypi.com>, Hans Verkuil <hverkuil@xs4all.nl>, Sakari Ailus <sakari.ailus@iki.fi>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>, Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> Subject: [PATCH 5/5] media: i2c: imx219: Simplify code handling in s_fmt Date: Tue, 4 Jul 2023 12:40:57 +0200 Message-Id: <20230704104057.149837-6-jacopo.mondi@ideasonboard.com> In-Reply-To: <20230704104057.149837-1-jacopo.mondi@ideasonboard.com> References: <20230704103611.149631-1-jacopo.mondi@ideasonboard.com> <20230704104057.149837-1-jacopo.mondi@ideasonboard.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	media: i2c: imx219: Use subdev active state \| [0/5] media: i2c: imx219: Use subdev active state [1/5] media: i2c: imx219: Rename mbus codes array [2/5] media: i2c: imx219: Switch from open to init_cfg [3/5] media: i2c: imx219: Use subdev active state [4/5] media: i2c: imx219: Simplify format assignment [5/5] media: i2c: imx219: Simplify code handling in s_fmt

Commit Message

Jacopo Mondi July 4, 2023, 10:40 a.m. UTC

  The imx219_set_pad_format() function adjusts the media bus code provided
through the v4l2_subdev_format parameter to a media bus code known
to be supported by the sensor.

The same exact operation is performed by the imx219_get_format_code()
function which called by imx219_update_pad_format(), which is in the
imx219_set_pad_format() call path.

Remove the duplicated operation and simplify imx219_set_pad_format().

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

Comments

kernel test robot July 4, 2023, 2:39 p.m. UTC | #1

Hi Jacopo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v6.4 next-20230704]
[cannot apply to sailus-media-tree/streams]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-i2c-imx219-Rename-mbus-codes-array/20230704-184252
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20230704104057.149837-6-jacopo.mondi%40ideasonboard.com
patch subject: [PATCH 5/5] media: i2c: imx219: Simplify code handling in s_fmt
config: i386-randconfig-r005-20230704 (https://download.01.org/0day-ci/archive/20230704/202307042233.nrNvmP4V-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230704/202307042233.nrNvmP4V-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307042233.nrNvmP4V-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/imx219.c:755:15: warning: unused variable 'i' [-Wunused-variable]
           unsigned int i;
                        ^
   1 warning generated.


vim +/i +755 drivers/media/i2c/imx219.c

1283b3b8f82b90 Dave Stevenson 2020-01-20  747  
1283b3b8f82b90 Dave Stevenson 2020-01-20  748  static int imx219_set_pad_format(struct v4l2_subdev *sd,
0d346d2a6f54f0 Tomi Valkeinen 2021-06-10  749  				 struct v4l2_subdev_state *sd_state,
1283b3b8f82b90 Dave Stevenson 2020-01-20  750  				 struct v4l2_subdev_format *fmt)
1283b3b8f82b90 Dave Stevenson 2020-01-20  751  {
1283b3b8f82b90 Dave Stevenson 2020-01-20  752  	struct imx219 *imx219 = to_imx219(sd);
1283b3b8f82b90 Dave Stevenson 2020-01-20  753  	const struct imx219_mode *mode;
1283b3b8f82b90 Dave Stevenson 2020-01-20  754  	int exposure_max, exposure_def, hblank;
22da1d56e98215 Lad Prabhakar  2020-03-10 @755  	unsigned int i;
1283b3b8f82b90 Dave Stevenson 2020-01-20  756  
1283b3b8f82b90 Dave Stevenson 2020-01-20  757  	mode = v4l2_find_nearest_size(supported_modes,
1283b3b8f82b90 Dave Stevenson 2020-01-20  758  				      ARRAY_SIZE(supported_modes),
1283b3b8f82b90 Dave Stevenson 2020-01-20  759  				      width, height,
1283b3b8f82b90 Dave Stevenson 2020-01-20  760  				      fmt->format.width, fmt->format.height);
563219f153d5f7 Jacopo Mondi   2023-07-04  761  
7471d0495584f7 Jacopo Mondi   2023-07-04  762  	imx219_update_pad_format(imx219, mode, &fmt->format, fmt->format.code);
563219f153d5f7 Jacopo Mondi   2023-07-04  763  
563219f153d5f7 Jacopo Mondi   2023-07-04  764  	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
1283b3b8f82b90 Dave Stevenson 2020-01-20  765  		imx219->mode = mode;
1283b3b8f82b90 Dave Stevenson 2020-01-20  766  		/* Update limits and set FPS to default */
1283b3b8f82b90 Dave Stevenson 2020-01-20  767  		__v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
1283b3b8f82b90 Dave Stevenson 2020-01-20  768  					 IMX219_VTS_MAX - mode->height, 1,
1283b3b8f82b90 Dave Stevenson 2020-01-20  769  					 mode->vts_def - mode->height);
1283b3b8f82b90 Dave Stevenson 2020-01-20  770  		__v4l2_ctrl_s_ctrl(imx219->vblank,
1283b3b8f82b90 Dave Stevenson 2020-01-20  771  				   mode->vts_def - mode->height);
1283b3b8f82b90 Dave Stevenson 2020-01-20  772  		/* Update max exposure while meeting expected vblanking */
1283b3b8f82b90 Dave Stevenson 2020-01-20  773  		exposure_max = mode->vts_def - 4;
1283b3b8f82b90 Dave Stevenson 2020-01-20  774  		exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
1283b3b8f82b90 Dave Stevenson 2020-01-20  775  			exposure_max : IMX219_EXPOSURE_DEFAULT;
1283b3b8f82b90 Dave Stevenson 2020-01-20  776  		__v4l2_ctrl_modify_range(imx219->exposure,
1283b3b8f82b90 Dave Stevenson 2020-01-20  777  					 imx219->exposure->minimum,
1283b3b8f82b90 Dave Stevenson 2020-01-20  778  					 exposure_max, imx219->exposure->step,
1283b3b8f82b90 Dave Stevenson 2020-01-20  779  					 exposure_def);
1283b3b8f82b90 Dave Stevenson 2020-01-20  780  		/*
1283b3b8f82b90 Dave Stevenson 2020-01-20  781  		 * Currently PPL is fixed to IMX219_PPL_DEFAULT, so hblank
1283b3b8f82b90 Dave Stevenson 2020-01-20  782  		 * depends on mode->width only, and is not changeble in any
1283b3b8f82b90 Dave Stevenson 2020-01-20  783  		 * way other than changing the mode.
1283b3b8f82b90 Dave Stevenson 2020-01-20  784  		 */
1283b3b8f82b90 Dave Stevenson 2020-01-20  785  		hblank = IMX219_PPL_DEFAULT - mode->width;
1283b3b8f82b90 Dave Stevenson 2020-01-20  786  		__v4l2_ctrl_modify_range(imx219->hblank, hblank, hblank, 1,
1283b3b8f82b90 Dave Stevenson 2020-01-20  787  					 hblank);
1283b3b8f82b90 Dave Stevenson 2020-01-20  788  	}
1283b3b8f82b90 Dave Stevenson 2020-01-20  789  
563219f153d5f7 Jacopo Mondi   2023-07-04  790  	*v4l2_subdev_get_pad_format(sd, sd_state, 0) = fmt->format;
1283b3b8f82b90 Dave Stevenson 2020-01-20  791  
1283b3b8f82b90 Dave Stevenson 2020-01-20  792  	return 0;
1283b3b8f82b90 Dave Stevenson 2020-01-20  793  }
1283b3b8f82b90 Dave Stevenson 2020-01-20  794

kernel test robot July 4, 2023, 3:41 p.m. UTC | #2

Hi Jacopo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v6.4 next-20230704]
[cannot apply to sailus-media-tree/streams]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-i2c-imx219-Rename-mbus-codes-array/20230704-184252
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20230704104057.149837-6-jacopo.mondi%40ideasonboard.com
patch subject: [PATCH 5/5] media: i2c: imx219: Simplify code handling in s_fmt
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230704/202307042337.lqiNqeJW-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230704/202307042337.lqiNqeJW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307042337.lqiNqeJW-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/media/i2c/imx219.c: In function 'imx219_set_pad_format':
>> drivers/media/i2c/imx219.c:755:22: warning: unused variable 'i' [-Wunused-variable]
     755 |         unsigned int i;
         |                      ^


vim +/i +755 drivers/media/i2c/imx219.c

1283b3b8f82b90 Dave Stevenson 2020-01-20  747  
1283b3b8f82b90 Dave Stevenson 2020-01-20  748  static int imx219_set_pad_format(struct v4l2_subdev *sd,
0d346d2a6f54f0 Tomi Valkeinen 2021-06-10  749  				 struct v4l2_subdev_state *sd_state,
1283b3b8f82b90 Dave Stevenson 2020-01-20  750  				 struct v4l2_subdev_format *fmt)
1283b3b8f82b90 Dave Stevenson 2020-01-20  751  {
1283b3b8f82b90 Dave Stevenson 2020-01-20  752  	struct imx219 *imx219 = to_imx219(sd);
1283b3b8f82b90 Dave Stevenson 2020-01-20  753  	const struct imx219_mode *mode;
1283b3b8f82b90 Dave Stevenson 2020-01-20  754  	int exposure_max, exposure_def, hblank;
22da1d56e98215 Lad Prabhakar  2020-03-10 @755  	unsigned int i;
1283b3b8f82b90 Dave Stevenson 2020-01-20  756  
1283b3b8f82b90 Dave Stevenson 2020-01-20  757  	mode = v4l2_find_nearest_size(supported_modes,
1283b3b8f82b90 Dave Stevenson 2020-01-20  758  				      ARRAY_SIZE(supported_modes),
1283b3b8f82b90 Dave Stevenson 2020-01-20  759  				      width, height,
1283b3b8f82b90 Dave Stevenson 2020-01-20  760  				      fmt->format.width, fmt->format.height);
563219f153d5f7 Jacopo Mondi   2023-07-04  761  
7471d0495584f7 Jacopo Mondi   2023-07-04  762  	imx219_update_pad_format(imx219, mode, &fmt->format, fmt->format.code);
563219f153d5f7 Jacopo Mondi   2023-07-04  763  
563219f153d5f7 Jacopo Mondi   2023-07-04  764  	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
1283b3b8f82b90 Dave Stevenson 2020-01-20  765  		imx219->mode = mode;
1283b3b8f82b90 Dave Stevenson 2020-01-20  766  		/* Update limits and set FPS to default */
1283b3b8f82b90 Dave Stevenson 2020-01-20  767  		__v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
1283b3b8f82b90 Dave Stevenson 2020-01-20  768  					 IMX219_VTS_MAX - mode->height, 1,
1283b3b8f82b90 Dave Stevenson 2020-01-20  769  					 mode->vts_def - mode->height);
1283b3b8f82b90 Dave Stevenson 2020-01-20  770  		__v4l2_ctrl_s_ctrl(imx219->vblank,
1283b3b8f82b90 Dave Stevenson 2020-01-20  771  				   mode->vts_def - mode->height);
1283b3b8f82b90 Dave Stevenson 2020-01-20  772  		/* Update max exposure while meeting expected vblanking */
1283b3b8f82b90 Dave Stevenson 2020-01-20  773  		exposure_max = mode->vts_def - 4;
1283b3b8f82b90 Dave Stevenson 2020-01-20  774  		exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
1283b3b8f82b90 Dave Stevenson 2020-01-20  775  			exposure_max : IMX219_EXPOSURE_DEFAULT;
1283b3b8f82b90 Dave Stevenson 2020-01-20  776  		__v4l2_ctrl_modify_range(imx219->exposure,
1283b3b8f82b90 Dave Stevenson 2020-01-20  777  					 imx219->exposure->minimum,
1283b3b8f82b90 Dave Stevenson 2020-01-20  778  					 exposure_max, imx219->exposure->step,
1283b3b8f82b90 Dave Stevenson 2020-01-20  779  					 exposure_def);
1283b3b8f82b90 Dave Stevenson 2020-01-20  780  		/*
1283b3b8f82b90 Dave Stevenson 2020-01-20  781  		 * Currently PPL is fixed to IMX219_PPL_DEFAULT, so hblank
1283b3b8f82b90 Dave Stevenson 2020-01-20  782  		 * depends on mode->width only, and is not changeble in any
1283b3b8f82b90 Dave Stevenson 2020-01-20  783  		 * way other than changing the mode.
1283b3b8f82b90 Dave Stevenson 2020-01-20  784  		 */
1283b3b8f82b90 Dave Stevenson 2020-01-20  785  		hblank = IMX219_PPL_DEFAULT - mode->width;
1283b3b8f82b90 Dave Stevenson 2020-01-20  786  		__v4l2_ctrl_modify_range(imx219->hblank, hblank, hblank, 1,
1283b3b8f82b90 Dave Stevenson 2020-01-20  787  					 hblank);
1283b3b8f82b90 Dave Stevenson 2020-01-20  788  	}
1283b3b8f82b90 Dave Stevenson 2020-01-20  789  
563219f153d5f7 Jacopo Mondi   2023-07-04  790  	*v4l2_subdev_get_pad_format(sd, sd_state, 0) = fmt->format;
1283b3b8f82b90 Dave Stevenson 2020-01-20  791  
1283b3b8f82b90 Dave Stevenson 2020-01-20  792  	return 0;
1283b3b8f82b90 Dave Stevenson 2020-01-20  793  }
1283b3b8f82b90 Dave Stevenson 2020-01-20  794

Laurent Pinchart July 7, 2023, 3:54 p.m. UTC | #3

Hi Jacopo,

Thank you for the patch.

On Tue, Jul 04, 2023 at 12:40:57PM +0200, Jacopo Mondi wrote:
> The imx219_set_pad_format() function adjusts the media bus code provided
> through the v4l2_subdev_format parameter to a media bus code known
> to be supported by the sensor.
> 
> The same exact operation is performed by the imx219_get_format_code()
> function which called by imx219_update_pad_format(), which is in the
> imx219_set_pad_format() call path.
> 
> Remove the duplicated operation and simplify imx219_set_pad_format().
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/i2c/imx219.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index c1246bd23b0d..37630e173040 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -754,19 +754,12 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
>  	int exposure_max, exposure_def, hblank;
>  	unsigned int i;

This variable should be dropped too.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  
> -	for (i = 0; i < ARRAY_SIZE(imx219_mbus_formats); i++)
> -		if (imx219_mbus_formats[i] == fmt->format.code)
> -			break;
> -	if (i >= ARRAY_SIZE(imx219_mbus_formats))
> -		i = 0;
> -
>  	mode = v4l2_find_nearest_size(supported_modes,
>  				      ARRAY_SIZE(supported_modes),
>  				      width, height,
>  				      fmt->format.width, fmt->format.height);
>  
> -	imx219_update_pad_format(imx219, mode, &fmt->format,
> -				 imx219_mbus_formats[i]);
> +	imx219_update_pad_format(imx219, mode, &fmt->format, fmt->format.code);
>  
>  	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
>  		imx219->mode = mode;

diff mbox series

Patch

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index c1246bd23b0d..37630e173040 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -754,19 +754,12 @@  static int imx219_set_pad_format(struct v4l2_subdev *sd,
 	int exposure_max, exposure_def, hblank;
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(imx219_mbus_formats); i++)
-		if (imx219_mbus_formats[i] == fmt->format.code)
-			break;
-	if (i >= ARRAY_SIZE(imx219_mbus_formats))
-		i = 0;
-
 	mode = v4l2_find_nearest_size(supported_modes,
 				      ARRAY_SIZE(supported_modes),
 				      width, height,
 				      fmt->format.width, fmt->format.height);
 
-	imx219_update_pad_format(imx219, mode, &fmt->format,
-				 imx219_mbus_formats[i]);
+	imx219_update_pad_format(imx219, mode, &fmt->format, fmt->format.code);
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		imx219->mode = mode;