Hi Sakari,
Thank you for the patch.
On Fri, Nov 17, 2023 at 12:11:36PM +0200, Sakari Ailus wrote:
> Compile sub-device state information access functions
> v4l2_subdev_state_get_{format,crop,compose} unconditionally as they are
> now also used by plain V4L2 drivers.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 252 +++++++++++++-------------
> include/media/v4l2-subdev.h | 128 ++++++-------
> 2 files changed, 190 insertions(+), 190 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 06988e3a6f10..41fe9fa5c21a 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1531,6 +1531,132 @@ 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;
> +
> + /*
> + * Set the pad to 0 on error as this is aligned with the
> + * behaviour of the pad state information access functions. The
> + * purpose of setting pad to 0 here is to avoid accessing memory
> + * outside the pads array, but still issuing warning of the
> + * invalid access while making the caller's error handling
> + * easier.
> + */
> + if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> + pad = 0;
> +
> + 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;
> +
> + /*
> + * Set the pad to 0 on error as this is aligned with the
> + * behaviour of the pad state information access functions. The
> + * purpose of setting pad to 0 here is to avoid accessing memory
> + * outside the pads array, but still issuing warning of the
> + * invalid access while making the caller's error handling
> + * easier.
> + */
> + if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> + pad = 0;
> +
> + 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;
> +
> + /*
> + * Set the pad to 0 on error as this is aligned with the
> + * behaviour of the pad state information access functions. The
> + * purpose of setting pad to 0 here is to avoid accessing memory
> + * outside the pads array, but still issuing warning of the
> + * invalid access while making the caller's error handling
> + * easier.
> + */
> + if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> + pad = 0;
> +
> + 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
> @@ -1677,132 +1803,6 @@ 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_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;
> -
> - /*
> - * Set the pad to 0 on error as this is aligned with the
> - * behaviour of the pad state information access functions. The
> - * purpose of setting pad to 0 here is to avoid accessing memory
> - * outside the pads array, but still issuing warning of the
> - * invalid access while making the caller's error handling
> - * easier.
> - */
> - if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> - pad = 0;
> -
> - 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;
> -
> - /*
> - * Set the pad to 0 on error as this is aligned with the
> - * behaviour of the pad state information access functions. The
> - * purpose of setting pad to 0 here is to avoid accessing memory
> - * outside the pads array, but still issuing warning of the
> - * invalid access while making the caller's error handling
> - * easier.
> - */
> - if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> - pad = 0;
> -
> - 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;
> -
> - /*
> - * Set the pad to 0 on error as this is aligned with the
> - * behaviour of the pad state information access functions. The
> - * purpose of setting pad to 0 here is to avoid accessing memory
> - * outside the pads array, but still issuing warning of the
> - * invalid access while making the caller's error handling
> - * easier.
> - */
> - if (WARN_ON_ONCE(pad >= state->sd->entity.num_pads))
> - pad = 0;
> -
> - 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);
> -
> int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing,
> u32 pad, u32 stream, u32 *other_pad,
> u32 *other_stream)
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index c8ac91a2cdb8..80744d87bae4 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1470,70 +1470,6 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
> return sd->active_state;
> }
>
> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> -
> -/**
> - * v4l2_subdev_get_fmt() - Fill format based on state
> - * @sd: subdevice
> - * @state: subdevice state
> - * @format: pointer to &struct v4l2_subdev_format
> - *
> - * Fill @format->format field based on the information in the @format struct.
> - *
> - * This function can be used by the subdev drivers which support active state to
> - * implement v4l2_subdev_pad_ops.get_fmt if the subdev driver does not need to
> - * do anything special in their get_fmt op.
> - *
> - * Returns 0 on success, error value otherwise.
> - */
> -int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> - struct v4l2_subdev_format *format);
> -
> -/**
> - * v4l2_subdev_set_routing() - Set given routing to subdev state
> - * @sd: The subdevice
> - * @state: The subdevice state
> - * @routing: Routing that will be copied to subdev state
> - *
> - * This will release old routing table (if any) from the state, allocate
> - * enough space for the given routing, and copy the routing.
> - *
> - * This can be used from the subdev driver's set_routing op, after validating
> - * the routing.
> - */
> -int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - const struct v4l2_subdev_krouting *routing);
> -
> -struct v4l2_subdev_route *
> -__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
> - struct v4l2_subdev_route *route);
> -
> -/**
> - * for_each_active_route - iterate on all active routes of a routing table
> - * @routing: The routing table
> - * @route: The route iterator
> - */
> -#define for_each_active_route(routing, route) \
> - for ((route) = NULL; \
> - ((route) = __v4l2_subdev_next_active_route((routing), (route)));)
> -
> -/**
> - * v4l2_subdev_set_routing_with_fmt() - Set given routing and format to subdev
> - * state
> - * @sd: The subdevice
> - * @state: The subdevice state
> - * @routing: Routing that will be copied to subdev state
> - * @fmt: Format used to initialize all the streams
> - *
> - * This is the same as v4l2_subdev_set_routing, but additionally initializes
> - * all the streams using the given format.
> - */
> -int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - const struct v4l2_subdev_krouting *routing,
> - const struct v4l2_mbus_framefmt *fmt);
> -
> /*
> * A macro to generate the macro or function name for sub-devices state access
> * wrapper macros below.
> @@ -1613,6 +1549,70 @@ 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)
> +
> +/**
> + * v4l2_subdev_get_fmt() - Fill format based on state
> + * @sd: subdevice
> + * @state: subdevice state
> + * @format: pointer to &struct v4l2_subdev_format
> + *
> + * Fill @format->format field based on the information in the @format struct.
> + *
> + * This function can be used by the subdev drivers which support active state to
> + * implement v4l2_subdev_pad_ops.get_fmt if the subdev driver does not need to
> + * do anything special in their get_fmt op.
> + *
> + * Returns 0 on success, error value otherwise.
> + */
> +int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format);
> +
> +/**
> + * v4l2_subdev_set_routing() - Set given routing to subdev state
> + * @sd: The subdevice
> + * @state: The subdevice state
> + * @routing: Routing that will be copied to subdev state
> + *
> + * This will release old routing table (if any) from the state, allocate
> + * enough space for the given routing, and copy the routing.
> + *
> + * This can be used from the subdev driver's set_routing op, after validating
> + * the routing.
> + */
> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + const struct v4l2_subdev_krouting *routing);
> +
> +struct v4l2_subdev_route *
> +__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
> + struct v4l2_subdev_route *route);
> +
> +/**
> + * for_each_active_route - iterate on all active routes of a routing table
> + * @routing: The routing table
> + * @route: The route iterator
> + */
> +#define for_each_active_route(routing, route) \
> + for ((route) = NULL; \
> + ((route) = __v4l2_subdev_next_active_route((routing), (route)));)
> +
> +/**
> + * v4l2_subdev_set_routing_with_fmt() - Set given routing and format to subdev
> + * state
> + * @sd: The subdevice
> + * @state: The subdevice state
> + * @routing: Routing that will be copied to subdev state
> + * @fmt: Format used to initialize all the streams
> + *
> + * This is the same as v4l2_subdev_set_routing, but additionally initializes
> + * all the streams using the given format.
> + */
> +int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + const struct v4l2_subdev_krouting *routing,
> + const struct v4l2_mbus_framefmt *fmt);
> +
> /**
> * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
> * @routing: routing used to find the opposite side
--
Regards,
Laurent Pinchart
