On Tue, Dec 05, 2023 at 02:26:37PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Tue, Dec 05, 2023 at 02:22:43PM +0200, Sakari Ailus wrote: > > The sub-device state information access function prototypes such as > > v4l2_subdev_state_get_format() were conditional to CONFIG_MC even though > > It's CONFIG_MEDIA_CONTROLLER, not CONFIG_MC. > > > the actual implementation was not. Drivers may use the functions without > > MC. Fix this. > > > > 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> > > --- > > Not compile tested yet but it should work... > > It won't I'm afraid. The implementations of the corresponding functions > in v4l2-subdev.c depend on CONFIG_MEDIA_CONTROLLER, both explicitly > (they're guarded by an #ifdef), and implicitly (they access the subdev > entity field, which is guarded by an #ifdef in v4l2-subdev.h). If you decide to instead select MEDIA_CONTROLLER for the VIDEO_SAA6752HS, you should also address VIDEO_ADV7183 and VIDEO_TW9910. There's also the interesting case of the RJ54N1CB0C, the rj54n1cb0c.c file is not listed in drivers/media/i2c/Makefile. > > include/media/v4l2-subdev.h | 158 ++++++++++++++++++------------------ > > 1 file changed, 79 insertions(+), 79 deletions(-) > > > > 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
