diff mbox series

[3/5] media: adv748x: Move format to subdev state

Message ID 20211216170323.141321-4-jacopo+renesas@jmondi.org (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series media: adv748x: Add CSI-2 VC support | expand

Commit Message

Jacopo Mondi Dec. 16, 2021, 5:03 p.m. UTC 
Move format handling to the v4l2_subdev state and store it per
(pad, stream) combination.

Now that the image format is stored in the subdev state, it can be
accessed through v4l2_subdev_get_fmt() instead of open-coding it.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/adv748x/adv748x-csi2.c | 83 +++++++-----------------
 drivers/media/i2c/adv748x/adv748x.h      |  1 -
 2 files changed, 22 insertions(+), 62 deletions(-)

Comments

kernel test robot Dec. 17, 2021, 7:30 a.m. UTC | #1 
Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.16-rc5 next-20211216]
[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/Jacopo-Mondi/media-adv748x-Add-CSI-2-VC-support/20211217-010519
base:   git://linuxtv.org/media_tree.git master
config: arc-randconfig-r043-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171539.cPT19ZOz-lkp@intel.com/config)
compiler: arc-elf-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/90158bf217d9df03d83fac378198a756af229010
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jacopo-Mondi/media-adv748x-Add-CSI-2-VC-support/20211217-010519
        git checkout 90158bf217d9df03d83fac378198a756af229010
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

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/adv748x/adv748x-csi2.c: In function 'adv748x_csi2_init_cfg':
   drivers/media/i2c/adv748x/adv748x-csi2.c:146:34: error: array type has incomplete element type 'struct v4l2_subdev_route'
     146 |         struct v4l2_subdev_route routes[ADV748X_CSI2_STREAMS] = {
         |                                  ^~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c:152:34: error: 'V4L2_SUBDEV_ROUTE_FL_ACTIVE' undeclared (first use in this function); did you mean 'V4L2_SUBDEV_FORMAT_ACTIVE'?
     152 |                         .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
         |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                  V4L2_SUBDEV_FORMAT_ACTIVE
   drivers/media/i2c/adv748x/adv748x-csi2.c:152:34: note: each undeclared identifier is reported only once for each function it appears in
   drivers/media/i2c/adv748x/adv748x-csi2.c:173:37: error: storage size of 'routing' isn't known
     173 |         struct v4l2_subdev_krouting routing;
         |                                     ^~~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c:179:9: error: implicit declaration of function 'v4l2_subdev_lock_state'; did you mean 'v4l2_subdev_alloc_state'? [-Werror=implicit-function-declaration]
     179 |         v4l2_subdev_lock_state(state);
         |         ^~~~~~~~~~~~~~~~~~~~~~
         |         v4l2_subdev_alloc_state
   drivers/media/i2c/adv748x/adv748x-csi2.c:180:15: error: implicit declaration of function 'v4l2_subdev_set_routing'; did you mean 'v4l2_subdev_notify'? [-Werror=implicit-function-declaration]
     180 |         ret = v4l2_subdev_set_routing(sd, state, &routing);
         |               ^~~~~~~~~~~~~~~~~~~~~~~
         |               v4l2_subdev_notify
   drivers/media/i2c/adv748x/adv748x-csi2.c:181:9: error: implicit declaration of function 'v4l2_subdev_unlock_state'; did you mean 'v4l2_subdev_alloc_state'? [-Werror=implicit-function-declaration]
     181 |         v4l2_subdev_unlock_state(state);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
         |         v4l2_subdev_alloc_state
   drivers/media/i2c/adv748x/adv748x-csi2.c:173:37: warning: unused variable 'routing' [-Wunused-variable]
     173 |         struct v4l2_subdev_krouting routing;
         |                                     ^~~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c:146:34: warning: unused variable 'routes' [-Wunused-variable]
     146 |         struct v4l2_subdev_route routes[ADV748X_CSI2_STREAMS] = {
         |                                  ^~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c: In function 'adv748x_csi2_set_format':
>> drivers/media/i2c/adv748x/adv748x-csi2.c:198:15: error: implicit declaration of function 'v4l2_state_get_stream_format'; did you mean 'v4l2_subdev_get_try_format'? [-Werror=implicit-function-declaration]
     198 |         fmt = v4l2_state_get_stream_format(sd_state, sdformat->pad,
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |               v4l2_subdev_get_try_format
>> drivers/media/i2c/adv748x/adv748x-csi2.c:199:52: error: 'struct v4l2_subdev_format' has no member named 'stream'
     199 |                                            sdformat->stream);
         |                                                    ^~
>> drivers/media/i2c/adv748x/adv748x-csi2.c:207:15: error: implicit declaration of function 'v4l2_subdev_state_get_opposite_stream_format' [-Werror=implicit-function-declaration]
     207 |         fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state, sdformat->pad,
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c:208:68: error: 'struct v4l2_subdev_format' has no member named 'stream'
     208 |                                                            sdformat->stream);
         |                                                                    ^~
   drivers/media/i2c/adv748x/adv748x-csi2.c: At top level:
>> drivers/media/i2c/adv748x/adv748x-csi2.c:253:20: error: 'v4l2_subdev_get_fmt' undeclared here (not in a function); did you mean 'v4l2_subdev_notify'?
     253 |         .get_fmt = v4l2_subdev_get_fmt,
         |                    ^~~~~~~~~~~~~~~~~~~
         |                    v4l2_subdev_notify
   drivers/media/i2c/adv748x/adv748x-csi2.c: In function 'adv748x_csi2_init':
   drivers/media/i2c/adv748x/adv748x-csi2.c:323:29: error: 'V4L2_SUBDEV_FL_MULTIPLEXED' undeclared (first use in this function)
     323 |                             V4L2_SUBDEV_FL_MULTIPLEXED,
         |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/i2c/adv748x/adv748x-csi2.c:340:15: error: implicit declaration of function 'v4l2_subdev_init_finalize'; did you mean 'v4l2_subdev_init'? [-Werror=implicit-function-declaration]
     340 |         ret = v4l2_subdev_init_finalize(&tx->sd);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~
         |               v4l2_subdev_init
   drivers/media/i2c/adv748x/adv748x-csi2.c:357:9: error: implicit declaration of function 'v4l2_subdev_cleanup'; did you mean 'v4l2_subdev_call'? [-Werror=implicit-function-declaration]
     357 |         v4l2_subdev_cleanup(&tx->sd);
         |         ^~~~~~~~~~~~~~~~~~~
         |         v4l2_subdev_call
   cc1: some warnings being treated as errors


vim +198 drivers/media/i2c/adv748x/adv748x-csi2.c

   185	
   186	static int adv748x_csi2_set_format(struct v4l2_subdev *sd,
   187					   struct v4l2_subdev_state *sd_state,
   188					   struct v4l2_subdev_format *sdformat)
   189	{
   190		struct v4l2_mbus_framefmt *fmt;
   191		int ret = 0;
   192	
   193		/* Do not allow to set format on the multiplexed source pad. */
   194		if (sdformat->pad == ADV748X_CSI2_SOURCE)
   195			return -EINVAL;
   196	
   197		v4l2_subdev_lock_state(sd_state);
 > 198		fmt = v4l2_state_get_stream_format(sd_state, sdformat->pad,
 > 199						   sdformat->stream);
   200		if (!fmt) {
   201			ret = -EINVAL;
   202			goto out;
   203		};
   204		*fmt = sdformat->format;
   205	
   206		/* Propagate format to the other end of the route. */
 > 207		fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state, sdformat->pad,
   208								   sdformat->stream);
   209		if (!fmt) {
   210			ret = -EINVAL;
   211			goto out;
   212		}
   213		*fmt = sdformat->format;
   214	
   215	out:
   216		v4l2_subdev_unlock_state(sd_state);
   217	
   218		return ret;
   219	}
   220	
   221	static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
   222						struct v4l2_mbus_config *config)
   223	{
   224		struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
   225	
   226		if (pad != ADV748X_CSI2_SOURCE)
   227			return -EINVAL;
   228	
   229		config->type = V4L2_MBUS_CSI2_DPHY;
   230		switch (tx->active_lanes) {
   231		case 1:
   232			config->flags = V4L2_MBUS_CSI2_1_LANE;
   233			break;
   234	
   235		case 2:
   236			config->flags = V4L2_MBUS_CSI2_2_LANE;
   237			break;
   238	
   239		case 3:
   240			config->flags = V4L2_MBUS_CSI2_3_LANE;
   241			break;
   242	
   243		case 4:
   244			config->flags = V4L2_MBUS_CSI2_4_LANE;
   245			break;
   246		}
   247	
   248		return 0;
   249	}
   250	
   251	static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = {
   252		.init_cfg = adv748x_csi2_init_cfg,
 > 253		.get_fmt = v4l2_subdev_get_fmt,
   254		.set_fmt = adv748x_csi2_set_format,
   255		.get_mbus_config = adv748x_csi2_get_mbus_config,
   256	};
   257	

---
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/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 9061c34ba561..549c3cd96006 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -183,76 +183,37 @@  static int adv748x_csi2_init_cfg(struct v4l2_subdev *sd,
 	return ret;
 }
 
-static struct v4l2_mbus_framefmt *
-adv748x_csi2_get_pad_format(struct v4l2_subdev *sd,
-			    struct v4l2_subdev_state *sd_state,
-			    unsigned int pad, u32 which)
-{
-	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
-
-	if (which == V4L2_SUBDEV_FORMAT_TRY)
-		return v4l2_subdev_get_try_format(sd, sd_state, pad);
-
-	return &tx->format;
-}
-
-static int adv748x_csi2_get_format(struct v4l2_subdev *sd,
-				   struct v4l2_subdev_state *sd_state,
-				   struct v4l2_subdev_format *sdformat)
-{
-	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
-	struct adv748x_state *state = tx->state;
-	struct v4l2_mbus_framefmt *mbusformat;
-
-	mbusformat = adv748x_csi2_get_pad_format(sd, sd_state, sdformat->pad,
-						 sdformat->which);
-	if (!mbusformat)
-		return -EINVAL;
-
-	mutex_lock(&state->mutex);
-
-	sdformat->format = *mbusformat;
-
-	mutex_unlock(&state->mutex);
-
-	return 0;
-}
-
 static int adv748x_csi2_set_format(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *sd_state,
 				   struct v4l2_subdev_format *sdformat)
 {
-	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
-	struct adv748x_state *state = tx->state;
-	struct v4l2_mbus_framefmt *mbusformat;
+	struct v4l2_mbus_framefmt *fmt;
 	int ret = 0;
 
-	mbusformat = adv748x_csi2_get_pad_format(sd, sd_state, sdformat->pad,
-						 sdformat->which);
-	if (!mbusformat)
+	/* Do not allow to set format on the multiplexed source pad. */
+	if (sdformat->pad == ADV748X_CSI2_SOURCE)
 		return -EINVAL;
 
-	mutex_lock(&state->mutex);
-
-	if (sdformat->pad == ADV748X_CSI2_SOURCE) {
-		const struct v4l2_mbus_framefmt *sink_fmt;
-
-		sink_fmt = adv748x_csi2_get_pad_format(sd, sd_state,
-						       ADV748X_CSI2_SINK,
-						       sdformat->which);
-
-		if (!sink_fmt) {
-			ret = -EINVAL;
-			goto unlock;
-		}
-
-		sdformat->format = *sink_fmt;
+	v4l2_subdev_lock_state(sd_state);
+	fmt = v4l2_state_get_stream_format(sd_state, sdformat->pad,
+					   sdformat->stream);
+	if (!fmt) {
+		ret = -EINVAL;
+		goto out;
+	};
+	*fmt = sdformat->format;
+
+	/* Propagate format to the other end of the route. */
+	fmt = v4l2_subdev_state_get_opposite_stream_format(sd_state, sdformat->pad,
+							   sdformat->stream);
+	if (!fmt) {
+		ret = -EINVAL;
+		goto out;
 	}
+	*fmt = sdformat->format;
 
-	*mbusformat = sdformat->format;
-
-unlock:
-	mutex_unlock(&state->mutex);
+out:
+	v4l2_subdev_unlock_state(sd_state);
 
 	return ret;
 }
@@ -289,7 +250,7 @@  static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
 
 static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = {
 	.init_cfg = adv748x_csi2_init_cfg,
-	.get_fmt = adv748x_csi2_get_format,
+	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = adv748x_csi2_set_format,
 	.get_mbus_config = adv748x_csi2_get_mbus_config,
 };
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index d651c8390e6f..98a3b3e0642a 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -78,7 +78,6 @@  enum adv748x_csi2_pads {
 
 struct adv748x_csi2 {
 	struct adv748x_state *state;
-	struct v4l2_mbus_framefmt format;
 	unsigned int page;
 	unsigned int port;
 	unsigned int num_lanes;