Re: [PATCH 1/1] media: v4l: subdev: Always compile sub-device state access functions

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Fri, Nov 10, 2023 at 12:10:49PM +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.

What do you mean by "plain" V4L2 drivers here ?

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 204 +++++++++++++-------------
>  include/media/v4l2-subdev.h           | 128 ++++++++--------
>  2 files changed, 166 insertions(+), 166 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 57ef4fce1186..efb39172b20f 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1524,6 +1524,108 @@ 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
> @@ -1670,108 +1772,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;
> -
> -		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);
> -
>  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 b1bad946d813..33c8e5c93a3d 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1393,70 +1393,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.
> @@ -1533,6 +1469,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