| Message ID | 20231013104424.404768-4-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Unify sub-device state access functions | expand |
Hi Sakari, Thank you for the patch. On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote: > Rename the sub-devices state information access functions, removing > "_state" and "_stream" from them. This makes them shorter and so more > convenient to use. No other functions will be needed to access this > information. The new names are too generic, and thus confusing. For instance, v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm fine dropping "_stream", but I would like to keep "_state". > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++------------- > include/media/v4l2-subdev.h | 28 ++++++++++++++------------- > 2 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 7d0ce8c8aab4..a522cd8096cf 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -177,7 +177,7 @@ static int check_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, > { > if (sd->flags & V4L2_SUBDEV_FL_STREAMS) { > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > - if (!v4l2_subdev_state_get_stream_format(state, pad, stream)) > + if (!v4l2_subdev_get_format(state, pad, stream)) > return -EINVAL; > return 0; > #else > @@ -1581,8 +1581,8 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, > struct v4l2_mbus_framefmt *fmt; > > if (sd->flags & V4L2_SUBDEV_FL_STREAMS) > - fmt = v4l2_subdev_state_get_stream_format(state, format->pad, > - format->stream); > + fmt = v4l2_subdev_get_format(state, format->pad, > + format->stream); > else if (format->pad < sd->entity.num_pads && format->stream == 0) > fmt = v4l2_subdev_get_pad_format(sd, state, format->pad); > else > @@ -1678,8 +1678,8 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, > EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing_with_fmt); > > struct v4l2_mbus_framefmt * > -v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +v4l2_subdev_get_format(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1709,11 +1709,11 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_format); > +EXPORT_SYMBOL_GPL(v4l2_subdev_get_format); > > struct v4l2_rect * > -v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +v4l2_subdev_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1743,11 +1743,11 @@ v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_crop); > +EXPORT_SYMBOL_GPL(v4l2_subdev_get_crop); > > struct v4l2_rect * > -v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +v4l2_subdev_get_compose(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1777,7 +1777,7 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); > +EXPORT_SYMBOL_GPL(v4l2_subdev_get_compose); > > int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing, > u32 pad, u32 stream, u32 *other_pad, > @@ -1823,8 +1823,7 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, > if (ret) > return NULL; > > - return v4l2_subdev_state_get_stream_format(state, other_pad, > - other_stream); > + return v4l2_subdev_get_format(state, other_pad, other_stream); > } > EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 5e5499a2fb0e..a5b819a3be1c 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1542,7 +1542,7 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, > const struct v4l2_mbus_framefmt *fmt); > > /** > - * v4l2_subdev_state_get_stream_format() - Get pointer to a stream format > + * v4l2_subdev_get_format() - Get pointer to a stream format > * @state: subdevice state > * @pad: pad id > * @stream: stream id > @@ -1550,14 +1550,15 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, > * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad + > * stream in the subdev state. > * > - * If the state does not contain the given pad + stream, NULL is returned. > + * For stream-unaware drivers the format for the corresponding pad is returned. > + * If the pad does not exist, NULL is returned. > */ > struct v4l2_mbus_framefmt * > -v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +v4l2_subdev_get_format(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream); > > /** > - * v4l2_subdev_state_get_stream_crop() - Get pointer to a stream crop rectangle > + * v4l2_subdev_get_crop() - Get pointer to a stream crop rectangle > * @state: subdevice state > * @pad: pad id > * @stream: stream id > @@ -1565,15 +1566,15 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, > * This returns a pointer to crop rectangle for the given pad + stream in the > * subdev state. > * > - * If the state does not contain the given pad + stream, NULL is returned. > + * For stream-unaware drivers the crop rectangle for the corresponding pad is > + * returned. If the pad does not exist, NULL is returned. > */ > struct v4l2_rect * > -v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +v4l2_subdev_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream); > > /** > - * v4l2_subdev_state_get_stream_compose() - Get pointer to a stream compose > - * rectangle > + * v4l2_subdev_get_compose() - Get pointer to a stream compose rectangle > * @state: subdevice state > * @pad: pad id > * @stream: stream id > @@ -1581,11 +1582,12 @@ v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, > * This returns a pointer to compose rectangle for the given pad + stream in the > * subdev state. > * > - * If the state does not contain the given pad + stream, NULL is returned. > + * For stream-unaware drivers the compose rectangle for the corresponding pad is > + * returned. If the pad does not exist, NULL is returned. > */ > struct v4l2_rect * > -v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +v4l2_subdev_get_compose(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream); > > /** > * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
Hi Laurent, On Fri, Oct 13, 2023 at 02:04:39PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote: > > Rename the sub-devices state information access functions, removing > > "_state" and "_stream" from them. This makes them shorter and so more > > convenient to use. No other functions will be needed to access this > > information. > > The new names are too generic, and thus confusing. For instance, > v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm > fine dropping "_stream", but I would like to keep "_state". > My intention was actually to rename v4l2_subdev_get_fmt() later on: it's used in an ops struct, almost uniquely, so its name can be longer without it being a nuisance. I can include this in the same set. The reason for using a shorter names such as v4l2_subdev_get_format() is that they're nicer to use in the code. The function names we've added recently are often exceedingly long. There's hardly room for confusion in this case either: these functions will remain as the only interface to access information in sub-device state.
On Fri, Oct 13, 2023 at 11:09:36AM +0000, Sakari Ailus wrote: > On Fri, Oct 13, 2023 at 02:04:39PM +0300, Laurent Pinchart wrote: > > On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote: > > > Rename the sub-devices state information access functions, removing > > > "_state" and "_stream" from them. This makes them shorter and so more > > > convenient to use. No other functions will be needed to access this > > > information. > > > > The new names are too generic, and thus confusing. For instance, > > v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm > > fine dropping "_stream", but I would like to keep "_state". > > > > My intention was actually to rename v4l2_subdev_get_fmt() later on: it's > used in an ops struct, almost uniquely, so its name can be longer without > it being a nuisance. I can include this in the same set. No objection, as long as the new name is clear. > The reason for using a shorter names such as v4l2_subdev_get_format() is > that they're nicer to use in the code. The function names we've added > recently are often exceedingly long. There's hardly room for confusion in > this case either: these functions will remain as the only interface to > access information in sub-device state. I agree that long names are not nice, but too short names end up not being descriptive enough. As these functions operate on a state, I'd like to keep that information in the name to differenciate them from functions operating on the subdev, and use the same state-aware prefix for all similar functions (I expect we'll get more of them). If you can find a good short form for the v4l2_subdev_state_ prefix that we can use through the code base, that would be fine too. And before you ask v4l2_sd_st_ is not good :-)
On Fri, Oct 13, 2023 at 11:33:42AM +0000, Sakari Ailus wrote: > On Fri, Oct 13, 2023 at 02:23:53PM +0300, Laurent Pinchart wrote: > > On Fri, Oct 13, 2023 at 11:09:36AM +0000, Sakari Ailus wrote: > > > On Fri, Oct 13, 2023 at 02:04:39PM +0300, Laurent Pinchart wrote: > > > > On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote: > > > > > Rename the sub-devices state information access functions, removing > > > > > "_state" and "_stream" from them. This makes them shorter and so more > > > > > convenient to use. No other functions will be needed to access this > > > > > information. > > > > > > > > The new names are too generic, and thus confusing. For instance, > > > > v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm > > > > fine dropping "_stream", but I would like to keep "_state". > > > > > > > > > > My intention was actually to rename v4l2_subdev_get_fmt() later on: it's > > > used in an ops struct, almost uniquely, so its name can be longer without > > > it being a nuisance. I can include this in the same set. > > > > No objection, as long as the new name is clear. > > > > > The reason for using a shorter names such as v4l2_subdev_get_format() is > > > that they're nicer to use in the code. The function names we've added > > > recently are often exceedingly long. There's hardly room for confusion in > > > this case either: these functions will remain as the only interface to > > > access information in sub-device state. > > > > I agree that long names are not nice, but too short names end up not > > being descriptive enough. As these functions operate on a state, I'd > > like to keep that information in the name to differenciate them from > > functions operating on the subdev, and use the same state-aware prefix > > for all similar functions (I expect we'll get more of them). If you can > > find a good short form for the v4l2_subdev_state_ prefix that we can use > > through the code base, that would be fine too. And before you ask > > v4l2_sd_st_ is not good :-) > > What would you say about v4l2_subdev_state_format() etc.? It's a function > to obtain a pointer to the format on a given pad(/stream) and "get" > typically isn't a part of the function name in other similar cases. I like it better than v4l2_subdev_get_format() for sure, but I think the _get here still looks a bit better. v4l2_subdev_state_get_format() would already be shorter than the existing function name, do we need to shorten it even further ?
On Mon, Oct 16, 2023 at 11:26:19AM +0300, Laurent Pinchart wrote: > On Fri, Oct 13, 2023 at 11:33:42AM +0000, Sakari Ailus wrote: > > On Fri, Oct 13, 2023 at 02:23:53PM +0300, Laurent Pinchart wrote: > > > On Fri, Oct 13, 2023 at 11:09:36AM +0000, Sakari Ailus wrote: > > > > On Fri, Oct 13, 2023 at 02:04:39PM +0300, Laurent Pinchart wrote: > > > > > On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote: > > > > > > Rename the sub-devices state information access functions, removing > > > > > > "_state" and "_stream" from them. This makes them shorter and so more > > > > > > convenient to use. No other functions will be needed to access this > > > > > > information. > > > > > > > > > > The new names are too generic, and thus confusing. For instance, > > > > > v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm > > > > > fine dropping "_stream", but I would like to keep "_state". > > > > > > > > > > > > > My intention was actually to rename v4l2_subdev_get_fmt() later on: it's > > > > used in an ops struct, almost uniquely, so its name can be longer without > > > > it being a nuisance. I can include this in the same set. > > > > > > No objection, as long as the new name is clear. > > > > > > > The reason for using a shorter names such as v4l2_subdev_get_format() is > > > > that they're nicer to use in the code. The function names we've added > > > > recently are often exceedingly long. There's hardly room for confusion in > > > > this case either: these functions will remain as the only interface to > > > > access information in sub-device state. > > > > > > I agree that long names are not nice, but too short names end up not > > > being descriptive enough. As these functions operate on a state, I'd > > > like to keep that information in the name to differenciate them from > > > functions operating on the subdev, and use the same state-aware prefix > > > for all similar functions (I expect we'll get more of them). If you can > > > find a good short form for the v4l2_subdev_state_ prefix that we can use > > > through the code base, that would be fine too. And before you ask > > > v4l2_sd_st_ is not good :-) > > > > What would you say about v4l2_subdev_state_format() etc.? It's a function > > to obtain a pointer to the format on a given pad(/stream) and "get" > > typically isn't a part of the function name in other similar cases. > > I like it better than v4l2_subdev_get_format() for sure, but I think the > _get here still looks a bit better. v4l2_subdev_state_get_format() would > already be shorter than the existing function name, do we need to > shorten it even further ? As noted, we don't have "_get" as part of other similar functions. The only reason, as far as I can see, would be to keep it for historical reasons. As we've been changing the APIs otherwise a lot, I don't put much value for that. Having "get" also suggests refcounting is involved. Therefore I prefer the shorter form.
Hi Sakari,
kernel test robot noticed the following build warnings:
[auto build test WARNING on next-20231016]
[also build test WARNING on linus/master v6.6-rc6]
[cannot apply to media-tree/master rockchip/for-next sailus-media-tree/streams v6.6-rc6 v6.6-rc5 v6.6-rc4]
[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/Sakari-Ailus/media-v4l-subdev-Also-return-pads-array-information-on-stream-functions/20231017-120800
base: next-20231016
patch link: https://lore.kernel.org/r/20231013104424.404768-4-sakari.ailus%40linux.intel.com
patch subject: [PATCH 3/6] media: v4l: subdev: Rename sub-device state information access functions
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231017/202310171359.UuOwm2bc-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310171359.UuOwm2bc-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/202310171359.UuOwm2bc-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/media/i2c/ds90ub913.c: In function 'ub913_set_fmt':
drivers/media/i2c/ds90ub913.c:427:15: error: implicit declaration of function 'v4l2_subdev_state_get_stream_format'; did you mean 'v4l2_subdev_state_get_opposite_stream_format'? [-Werror=implicit-function-declaration]
427 | fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| v4l2_subdev_state_get_opposite_stream_format
>> drivers/media/i2c/ds90ub913.c:427:13: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
427 | fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
| ^
cc1: some warnings being treated as errors
--
drivers/media/i2c/ds90ub953.c: In function 'ub953_set_fmt':
drivers/media/i2c/ds90ub953.c:561:15: error: implicit declaration of function 'v4l2_subdev_state_get_stream_format'; did you mean 'v4l2_subdev_state_get_opposite_stream_format'? [-Werror=implicit-function-declaration]
561 | fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| v4l2_subdev_state_get_opposite_stream_format
>> drivers/media/i2c/ds90ub953.c:561:13: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
561 | fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
| ^
cc1: some warnings being treated as errors
--
drivers/media/i2c/ds90ub960.c: In function 'ub960_configure_ports_for_streaming':
drivers/media/i2c/ds90ub960.c:2454:23: error: implicit declaration of function 'v4l2_subdev_state_get_stream_format'; did you mean 'v4l2_subdev_state_get_opposite_stream_format'? [-Werror=implicit-function-declaration]
2454 | fmt = v4l2_subdev_state_get_stream_format(state,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| v4l2_subdev_state_get_opposite_stream_format
>> drivers/media/i2c/ds90ub960.c:2454:21: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
2454 | fmt = v4l2_subdev_state_get_stream_format(state,
| ^
drivers/media/i2c/ds90ub960.c: In function 'ub960_get_frame_desc':
drivers/media/i2c/ds90ub960.c:2845:29: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
2845 | fmt = v4l2_subdev_state_get_stream_format(state, pad,
| ^
drivers/media/i2c/ds90ub960.c: In function 'ub960_set_fmt':
drivers/media/i2c/ds90ub960.c:2894:13: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
2894 | fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
| ^
cc1: some warnings being treated as errors
vim +427 drivers/media/i2c/ds90ub913.c
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 403
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 404 static int ub913_set_fmt(struct v4l2_subdev *sd,
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 405 struct v4l2_subdev_state *state,
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 406 struct v4l2_subdev_format *format)
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 407 {
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 408 struct ub913_data *priv = sd_to_ub913(sd);
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 409 struct v4l2_mbus_framefmt *fmt;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 410 const struct ub913_format_info *finfo;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 411
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 412 if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 413 priv->enabled_source_streams)
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 414 return -EBUSY;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 415
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 416 /* Source format is fully defined by the sink format, so not settable */
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 417 if (format->pad == UB913_PAD_SOURCE)
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 418 return v4l2_subdev_get_fmt(sd, state, format);
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 419
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 420 finfo = ub913_find_format(format->format.code);
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 421 if (!finfo) {
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 422 finfo = &ub913_formats[0];
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 423 format->format.code = finfo->incode;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 424 }
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 425
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 426 /* Set sink format */
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 @427 fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 428 format->stream);
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 429 if (!fmt)
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 430 return -EINVAL;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 431
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 432 *fmt = format->format;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 433
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 434 /* Propagate to source format, and adjust the mbus code */
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 435 fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 436 format->stream);
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 437 if (!fmt)
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 438 return -EINVAL;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 439
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 440 format->format.code = finfo->outcode;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 441
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 442 *fmt = format->format;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 443
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 444 return 0;
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 445 }
c158d0d4ff1530 Tomi Valkeinen 2023-06-19 446
Hi Sakari,
kernel test robot noticed the following build warnings:
[auto build test WARNING on next-20231016]
[also build test WARNING on linus/master v6.6-rc6]
[cannot apply to media-tree/master rockchip/for-next sailus-media-tree/streams v6.6-rc6 v6.6-rc5 v6.6-rc4]
[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/Sakari-Ailus/media-v4l-subdev-Also-return-pads-array-information-on-stream-functions/20231017-120800
base: next-20231016
patch link: https://lore.kernel.org/r/20231013104424.404768-4-sakari.ailus%40linux.intel.com
patch subject: [PATCH 3/6] media: v4l: subdev: Rename sub-device state information access functions
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20231017/202310171645.8Tac7Ez4-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310171645.8Tac7Ez4-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/202310171645.8Tac7Ez4-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c: In function 'mxc_isi_crossbar_gasket_enable':
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:61:15: error: implicit declaration of function 'v4l2_subdev_state_get_stream_format'; did you mean 'v4l2_subdev_state_get_opposite_stream_format'? [-Werror=implicit-function-declaration]
61 | fmt = v4l2_subdev_state_get_stream_format(state, port, 0);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| v4l2_subdev_state_get_opposite_stream_format
>> drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:61:13: warning: assignment to 'const struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
61 | fmt = v4l2_subdev_state_get_stream_format(state, port, 0);
| ^
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c: In function 'mxc_isi_crossbar_set_fmt':
>> drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:284:18: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
284 | sink_fmt = v4l2_subdev_state_get_stream_format(state, fmt->pad,
| ^
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:299:28: warning: assignment to 'struct v4l2_mbus_framefmt *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
299 | source_fmt = v4l2_subdev_state_get_stream_format(state, route->source_pad,
| ^
cc1: some warnings being treated as errors
vim +61 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
cf21f328fcafac Laurent Pinchart 2019-01-21 26
cf21f328fcafac Laurent Pinchart 2019-01-21 27 static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
cf21f328fcafac Laurent Pinchart 2019-01-21 28 struct v4l2_subdev_state *state,
cf21f328fcafac Laurent Pinchart 2019-01-21 29 struct v4l2_subdev *remote_sd,
cf21f328fcafac Laurent Pinchart 2019-01-21 30 u32 remote_pad, unsigned int port)
cf21f328fcafac Laurent Pinchart 2019-01-21 31 {
cf21f328fcafac Laurent Pinchart 2019-01-21 32 struct mxc_isi_dev *isi = xbar->isi;
f48498ad0a4106 Guoniu.zhou 2023-06-29 33 const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
cf21f328fcafac Laurent Pinchart 2019-01-21 34 const struct v4l2_mbus_framefmt *fmt;
cf21f328fcafac Laurent Pinchart 2019-01-21 35 struct v4l2_mbus_frame_desc fd;
cf21f328fcafac Laurent Pinchart 2019-01-21 36 int ret;
cf21f328fcafac Laurent Pinchart 2019-01-21 37
f48498ad0a4106 Guoniu.zhou 2023-06-29 38 if (!gasket_ops)
cf21f328fcafac Laurent Pinchart 2019-01-21 39 return 0;
cf21f328fcafac Laurent Pinchart 2019-01-21 40
cf21f328fcafac Laurent Pinchart 2019-01-21 41 /*
cf21f328fcafac Laurent Pinchart 2019-01-21 42 * Configure and enable the gasket with the frame size and CSI-2 data
cf21f328fcafac Laurent Pinchart 2019-01-21 43 * type. For YUV422 8-bit, enable dual component mode unconditionally,
cf21f328fcafac Laurent Pinchart 2019-01-21 44 * to match the configuration of the CSIS.
cf21f328fcafac Laurent Pinchart 2019-01-21 45 */
cf21f328fcafac Laurent Pinchart 2019-01-21 46
cf21f328fcafac Laurent Pinchart 2019-01-21 47 ret = v4l2_subdev_call(remote_sd, pad, get_frame_desc, remote_pad, &fd);
cf21f328fcafac Laurent Pinchart 2019-01-21 48 if (ret) {
cf21f328fcafac Laurent Pinchart 2019-01-21 49 dev_err(isi->dev,
cf21f328fcafac Laurent Pinchart 2019-01-21 50 "failed to get frame descriptor from '%s':%u: %d\n",
cf21f328fcafac Laurent Pinchart 2019-01-21 51 remote_sd->name, remote_pad, ret);
cf21f328fcafac Laurent Pinchart 2019-01-21 52 return ret;
cf21f328fcafac Laurent Pinchart 2019-01-21 53 }
cf21f328fcafac Laurent Pinchart 2019-01-21 54
cf21f328fcafac Laurent Pinchart 2019-01-21 55 if (fd.num_entries != 1) {
cf21f328fcafac Laurent Pinchart 2019-01-21 56 dev_err(isi->dev, "invalid frame descriptor for '%s':%u\n",
cf21f328fcafac Laurent Pinchart 2019-01-21 57 remote_sd->name, remote_pad);
cf21f328fcafac Laurent Pinchart 2019-01-21 58 return -EINVAL;
cf21f328fcafac Laurent Pinchart 2019-01-21 59 }
cf21f328fcafac Laurent Pinchart 2019-01-21 60
cf21f328fcafac Laurent Pinchart 2019-01-21 @61 fmt = v4l2_subdev_state_get_stream_format(state, port, 0);
cf21f328fcafac Laurent Pinchart 2019-01-21 62 if (!fmt)
cf21f328fcafac Laurent Pinchart 2019-01-21 63 return -EINVAL;
cf21f328fcafac Laurent Pinchart 2019-01-21 64
f48498ad0a4106 Guoniu.zhou 2023-06-29 65 gasket_ops->enable(isi, &fd, fmt, port);
cf21f328fcafac Laurent Pinchart 2019-01-21 66 return 0;
cf21f328fcafac Laurent Pinchart 2019-01-21 67 }
cf21f328fcafac Laurent Pinchart 2019-01-21 68
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 7d0ce8c8aab4..a522cd8096cf 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -177,7 +177,7 @@ static int check_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, { if (sd->flags & V4L2_SUBDEV_FL_STREAMS) { #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) - if (!v4l2_subdev_state_get_stream_format(state, pad, stream)) + if (!v4l2_subdev_get_format(state, pad, stream)) return -EINVAL; return 0; #else @@ -1581,8 +1581,8 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, struct v4l2_mbus_framefmt *fmt; if (sd->flags & V4L2_SUBDEV_FL_STREAMS) - fmt = v4l2_subdev_state_get_stream_format(state, format->pad, - format->stream); + fmt = v4l2_subdev_get_format(state, format->pad, + format->stream); else if (format->pad < sd->entity.num_pads && format->stream == 0) fmt = v4l2_subdev_get_pad_format(sd, state, format->pad); else @@ -1678,8 +1678,8 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing_with_fmt); struct v4l2_mbus_framefmt * -v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +v4l2_subdev_get_format(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1709,11 +1709,11 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_format); +EXPORT_SYMBOL_GPL(v4l2_subdev_get_format); struct v4l2_rect * -v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +v4l2_subdev_get_crop(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1743,11 +1743,11 @@ v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_crop); +EXPORT_SYMBOL_GPL(v4l2_subdev_get_crop); struct v4l2_rect * -v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +v4l2_subdev_get_compose(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1777,7 +1777,7 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); +EXPORT_SYMBOL_GPL(v4l2_subdev_get_compose); int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing, u32 pad, u32 stream, u32 *other_pad, @@ -1823,8 +1823,7 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, if (ret) return NULL; - return v4l2_subdev_state_get_stream_format(state, other_pad, - other_stream); + return v4l2_subdev_get_format(state, other_pad, other_stream); } EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 5e5499a2fb0e..a5b819a3be1c 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1542,7 +1542,7 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, const struct v4l2_mbus_framefmt *fmt); /** - * v4l2_subdev_state_get_stream_format() - Get pointer to a stream format + * v4l2_subdev_get_format() - Get pointer to a stream format * @state: subdevice state * @pad: pad id * @stream: stream id @@ -1550,14 +1550,15 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd, * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad + * stream in the subdev state. * - * If the state does not contain the given pad + stream, NULL is returned. + * For stream-unaware drivers the format for the corresponding pad is returned. + * If the pad does not exist, NULL is returned. */ struct v4l2_mbus_framefmt * -v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +v4l2_subdev_get_format(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream); /** - * v4l2_subdev_state_get_stream_crop() - Get pointer to a stream crop rectangle + * v4l2_subdev_get_crop() - Get pointer to a stream crop rectangle * @state: subdevice state * @pad: pad id * @stream: stream id @@ -1565,15 +1566,15 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state, * This returns a pointer to crop rectangle for the given pad + stream in the * subdev state. * - * If the state does not contain the given pad + stream, NULL is returned. + * For stream-unaware drivers the crop rectangle for the corresponding pad is + * returned. If the pad does not exist, NULL is returned. */ struct v4l2_rect * -v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +v4l2_subdev_get_crop(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream); /** - * v4l2_subdev_state_get_stream_compose() - Get pointer to a stream compose - * rectangle + * v4l2_subdev_get_compose() - Get pointer to a stream compose rectangle * @state: subdevice state * @pad: pad id * @stream: stream id @@ -1581,11 +1582,12 @@ v4l2_subdev_state_get_stream_crop(struct v4l2_subdev_state *state, * This returns a pointer to compose rectangle for the given pad + stream in the * subdev state. * - * If the state does not contain the given pad + stream, NULL is returned. + * For stream-unaware drivers the compose rectangle for the corresponding pad is + * returned. If the pad does not exist, NULL is returned. */ struct v4l2_rect * -v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +v4l2_subdev_get_compose(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream); /** * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
Rename the sub-devices state information access functions, removing "_state" and "_stream" from them. This makes them shorter and so more convenient to use. No other functions will be needed to access this information. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++------------- include/media/v4l2-subdev.h | 28 ++++++++++++++------------- 2 files changed, 28 insertions(+), 27 deletions(-)