Hi Sakari, Thank you for the patch. On Tue, Dec 05, 2023 at 03:00:01PM +0200, Sakari Ailus wrote: > The sub-device state information access function were only available if > CONFIG_MEDIA_CONTROLLER was defined whereas the functions themselves were > used by non-MC-enabled drivers, too. Fix this by moving the definitions > out of CONFIG_MEDIA_CONTROLLER dependent parts. Also make the MC dependent > features conditional to CONFIG_MEDIA_CONTROLLER. > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202312051913.e5iif8Qz-lkp@xxxxxxxxx/ > Fixes: bc0e8d91feec ("media: v4l: subdev: Switch to stream-aware state functions") > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > --- > Thanks to Laurent for review. > > since v1: > > - I thought the c file had already this fixed. Also fixed that now and > changed the commit message accordingly. > > drivers/media/v4l2-core/v4l2-subdev.c | 216 ++++++++++++++------------ > include/media/v4l2-subdev.h | 158 +++++++++---------- > 2 files changed, 193 insertions(+), 181 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 4fbefe4cd714..30746a7df259 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1533,108 +1533,6 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd) > } > EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); > > -struct v4l2_mbus_framefmt * > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > -{ > - struct v4l2_subdev_stream_configs *stream_configs; > - unsigned int i; > - > - if (WARN_ON_ONCE(!state)) > - return NULL; > - > - if (state->pads) { > - if (stream) > - return NULL; > - > - if (pad >= state->sd->entity.num_pads) > - return NULL; > - > - return &state->pads[pad].format; > - } > - > - lockdep_assert_held(state->lock); > - > - stream_configs = &state->stream_configs; > - > - for (i = 0; i < stream_configs->num_configs; ++i) { > - if (stream_configs->configs[i].pad == pad && > - stream_configs->configs[i].stream == stream) > - return &stream_configs->configs[i].fmt; > - } > - > - return NULL; > -} > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format); > - > -struct v4l2_rect * > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > - u32 stream) > -{ > - struct v4l2_subdev_stream_configs *stream_configs; > - unsigned int i; > - > - if (WARN_ON_ONCE(!state)) > - return NULL; > - > - if (state->pads) { > - if (stream) > - return NULL; > - > - if (pad >= state->sd->entity.num_pads) > - return NULL; > - > - return &state->pads[pad].crop; > - } > - > - lockdep_assert_held(state->lock); > - > - stream_configs = &state->stream_configs; > - > - for (i = 0; i < stream_configs->num_configs; ++i) { > - if (stream_configs->configs[i].pad == pad && > - stream_configs->configs[i].stream == stream) > - return &stream_configs->configs[i].crop; > - } > - > - return NULL; > -} > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop); > - > -struct v4l2_rect * > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > -{ > - struct v4l2_subdev_stream_configs *stream_configs; > - unsigned int i; > - > - if (WARN_ON_ONCE(!state)) > - return NULL; > - > - if (state->pads) { > - if (stream) > - return NULL; > - > - if (pad >= state->sd->entity.num_pads) > - return NULL; > - > - return &state->pads[pad].compose; > - } > - > - lockdep_assert_held(state->lock); > - > - stream_configs = &state->stream_configs; > - > - for (i = 0; i < stream_configs->num_configs; ++i) { > - if (stream_configs->configs[i].pad == pad && > - stream_configs->configs[i].stream == stream) > - return &stream_configs->configs[i].compose; > - } > - > - return NULL; > -} > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose); > - > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > static int > @@ -2272,6 +2170,120 @@ EXPORT_SYMBOL_GPL(v4l2_subdev_s_stream_helper); > > #endif /* CONFIG_MEDIA_CONTROLLER */ > > +struct v4l2_mbus_framefmt * > +__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > +{ > + struct v4l2_subdev_stream_configs *stream_configs; > + unsigned int i; > + > + if (WARN_ON_ONCE(!state)) > + return NULL; > + > + if (state->pads) { > + if (stream) > + return NULL; > + > + if (pad >= state->sd->entity.num_pads) This won't work, v4l2_subdev has no entity field when CONFIG_MEDIA_CONTROLLER is not defined. Please compile-test further versions of this patch. Now, for an attempt to review v3 before you post it: dropping the #if defined(CONFIG_MEDIA_CONTROLLER) around the entity field of v4l2_subdev won't work either, as the saa6752hs driver doesn't initialize the entity, so num_fields will be at best memset to 0, at worst random. This check will then always fail. > + return NULL; > + > + return &state->pads[pad].format; > + } > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + > + lockdep_assert_held(state->lock); > + > + stream_configs = &state->stream_configs; > + > + for (i = 0; i < stream_configs->num_configs; ++i) { > + if (stream_configs->configs[i].pad == pad && > + stream_configs->configs[i].stream == stream) > + return &stream_configs->configs[i].fmt; > + } > + > +#endif /* CONFIG_MEDIA_CONTROLLER */ > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format); > + > +struct v4l2_rect * > +__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream) > +{ > + struct v4l2_subdev_stream_configs *stream_configs; > + unsigned int i; > + > + if (WARN_ON_ONCE(!state)) > + return NULL; > + > + if (state->pads) { > + if (stream) > + return NULL; > + > + if (pad >= state->sd->entity.num_pads) > + return NULL; > + > + return &state->pads[pad].crop; > + } > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + > + lockdep_assert_held(state->lock); > + > + stream_configs = &state->stream_configs; > + > + for (i = 0; i < stream_configs->num_configs; ++i) { > + if (stream_configs->configs[i].pad == pad && > + stream_configs->configs[i].stream == stream) > + return &stream_configs->configs[i].crop; > + } > + > +#endif /* CONFIG_MEDIA_CONTROLLER */ > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop); > + > +struct v4l2_rect * > +__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > +{ > + struct v4l2_subdev_stream_configs *stream_configs; > + unsigned int i; > + > + if (WARN_ON_ONCE(!state)) > + return NULL; > + > + if (state->pads) { > + if (stream) > + return NULL; > + > + if (pad >= state->sd->entity.num_pads) > + return NULL; > + > + return &state->pads[pad].compose; > + } > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + > + lockdep_assert_held(state->lock); > + > + stream_configs = &state->stream_configs; > + > + for (i = 0; i < stream_configs->num_configs; ++i) { > + if (stream_configs->configs[i].pad == pad && > + stream_configs->configs[i].stream == stream) > + return &stream_configs->configs[i].compose; > + } > + > +#endif /* CONFIG_MEDIA_CONTROLLER */ > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose); > + > void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops) > { > INIT_LIST_HEAD(&sd->list); > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 8b08f6640dee..0099e177980e 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1186,6 +1186,85 @@ static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd) > return sd->host_priv; > } > > +/* > + * A macro to generate the macro or function name for sub-devices state access > + * wrapper macros below. > + */ > +#define __v4l2_subdev_state_gen_call(NAME, _1, ARG, ...) \ > + __v4l2_subdev_state_get_ ## NAME ## ARG > + > +/** > + * v4l2_subdev_state_get_format() - Get pointer to a stream format > + * @state: subdevice state > + * @pad: pad id > + * @...: stream id (optional argument) > + * > + * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad + > + * stream in the subdev state. > + * > + * For stream-unaware drivers the format for the corresponding pad is returned. > + * If the pad does not exist, NULL is returned. > + */ > +/* > + * Wrap v4l2_subdev_state_get_format(), allowing the function to be called with > + * two or three arguments. The purpose of the __v4l2_subdev_state_get_format() > + * macro below is to come up with the name of the function or macro to call, > + * using the last two arguments (_stream and _pad). The selected function or > + * macro is then called using the arguments specified by the caller. A similar > + * arrangement is used for v4l2_subdev_state_crop() and > + * v4l2_subdev_state_compose() below. > + */ > +#define v4l2_subdev_state_get_format(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > + (state, pad, ##__VA_ARGS__) > +#define __v4l2_subdev_state_get_format_pad(state, pad) \ > + __v4l2_subdev_state_get_format(state, pad, 0) > +struct v4l2_mbus_framefmt * > +__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream); > + > +/** > + * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle > + * @state: subdevice state > + * @pad: pad id > + * @...: stream id (optional argument) > + * > + * This returns a pointer to crop rectangle for the given pad + stream in the > + * subdev state. > + * > + * For stream-unaware drivers the crop rectangle for the corresponding pad is > + * returned. If the pad does not exist, NULL is returned. > + */ > +#define v4l2_subdev_state_get_crop(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > + (state, pad, ##__VA_ARGS__) > +#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > + __v4l2_subdev_state_get_crop(state, pad, 0) > +struct v4l2_rect * > +__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream); > + > +/** > + * v4l2_subdev_state_get_compose() - Get pointer to a stream compose rectangle > + * @state: subdevice state > + * @pad: pad id > + * @...: stream id (optional argument) > + * > + * This returns a pointer to compose rectangle for the given pad + stream in the > + * subdev state. > + * > + * For stream-unaware drivers the compose rectangle for the corresponding pad is > + * returned. If the pad does not exist, NULL is returned. > + */ > +#define v4l2_subdev_state_get_compose(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > + (state, pad, ##__VA_ARGS__) > +#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > + __v4l2_subdev_state_get_compose(state, pad, 0) > +struct v4l2_rect * > +__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream); > + > #ifdef CONFIG_MEDIA_CONTROLLER > > /** > @@ -1394,85 +1473,6 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) > return sd->active_state; > } > > -/* > - * A macro to generate the macro or function name for sub-devices state access > - * wrapper macros below. > - */ > -#define __v4l2_subdev_state_gen_call(NAME, _1, ARG, ...) \ > - __v4l2_subdev_state_get_ ## NAME ## ARG > - > -/** > - * v4l2_subdev_state_get_format() - Get pointer to a stream format > - * @state: subdevice state > - * @pad: pad id > - * @...: stream id (optional argument) > - * > - * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad + > - * stream in the subdev state. > - * > - * For stream-unaware drivers the format for the corresponding pad is returned. > - * If the pad does not exist, NULL is returned. > - */ > -/* > - * Wrap v4l2_subdev_state_get_format(), allowing the function to be called with > - * two or three arguments. The purpose of the __v4l2_subdev_state_get_format() > - * macro below is to come up with the name of the function or macro to call, > - * using the last two arguments (_stream and _pad). The selected function or > - * macro is then called using the arguments specified by the caller. A similar > - * arrangement is used for v4l2_subdev_state_crop() and > - * v4l2_subdev_state_compose() below. > - */ > -#define v4l2_subdev_state_get_format(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > - (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_format_pad(state, pad) \ > - __v4l2_subdev_state_get_format(state, pad, 0) > -struct v4l2_mbus_framefmt * > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > - > -/** > - * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle > - * @state: subdevice state > - * @pad: pad id > - * @...: stream id (optional argument) > - * > - * This returns a pointer to crop rectangle for the given pad + stream in the > - * subdev state. > - * > - * For stream-unaware drivers the crop rectangle for the corresponding pad is > - * returned. If the pad does not exist, NULL is returned. > - */ > -#define v4l2_subdev_state_get_crop(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > - (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > - __v4l2_subdev_state_get_crop(state, pad, 0) > -struct v4l2_rect * > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > - u32 stream); > - > -/** > - * v4l2_subdev_state_get_compose() - Get pointer to a stream compose rectangle > - * @state: subdevice state > - * @pad: pad id > - * @...: stream id (optional argument) > - * > - * This returns a pointer to compose rectangle for the given pad + stream in the > - * subdev state. > - * > - * For stream-unaware drivers the compose rectangle for the corresponding pad is > - * returned. If the pad does not exist, NULL is returned. > - */ > -#define v4l2_subdev_state_get_compose(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > - (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > - __v4l2_subdev_state_get_compose(state, pad, 0) > -struct v4l2_rect * > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > - > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > /** -- Regards, Laurent Pinchart
