diff mbox series

[v4,5/9] media: v4l: subdev: Make stream argument optional in state access functions

Message ID 20231026070329.948847-6-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Unify sub-device state access functions | expand

Commit Message

Sakari Ailus Oct. 26, 2023, 7:03 a.m. UTC
The sub-device state access functions take three arguments: sub-device
state, pad and stream. The stream is not relevant for the majority of
drivers and having to specify 0 for the stream is considered a nuisance.

Provide a two-argument macros for these state access functions to cover
the needs of stream-unaware users.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 18 +++++-----
 include/media/v4l2-subdev.h           | 48 ++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 18 deletions(-)

Comments

Laurent Pinchart Oct. 26, 2023, 1:49 p.m. UTC | #1
On Thu, Oct 26, 2023 at 10:03:25AM +0300, Sakari Ailus wrote:
> The sub-device state access functions take three arguments: sub-device
> state, pad and stream. The stream is not relevant for the majority of
> drivers and having to specify 0 for the stream is considered a nuisance.
> 
> Provide a two-argument macros for these state access functions to cover
> the needs of stream-unaware users.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 18 +++++-----
>  include/media/v4l2-subdev.h           | 48 ++++++++++++++++++++++-----
>  2 files changed, 48 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 2a615836c1d4..9d4ff9b4fcec 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1671,8 +1671,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_format(struct v4l2_subdev_state *state, unsigned int pad,
> -			     u32 stream)
> +__v4l2_subdev_state_get_format_stream(struct v4l2_subdev_state *state,
> +				      unsigned int pad, u32 stream)
>  {
>  	struct v4l2_subdev_stream_configs *stream_configs;
>  	unsigned int i;
> @@ -1706,11 +1706,11 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
>  
>  	return NULL;
>  }
> -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_format);
> +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format_stream);
>  
>  struct v4l2_rect *
> -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> -			   u32 stream)
> +__v4l2_subdev_state_get_crop_stream(struct v4l2_subdev_state *state,
> +				    unsigned int pad, u32 stream)
>  {
>  	struct v4l2_subdev_stream_configs *stream_configs;
>  	unsigned int i;
> @@ -1744,11 +1744,11 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
>  
>  	return NULL;
>  }
> -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_crop);
> +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop_stream);
>  
>  struct v4l2_rect *
> -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> -			      u32 stream)
> +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> +				       unsigned int pad, u32 stream)
>  {
>  	struct v4l2_subdev_stream_configs *stream_configs;
>  	unsigned int i;
> @@ -1782,7 +1782,7 @@ v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
>  
>  	return NULL;
>  }
> -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_compose);
> +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose_stream);
>  
>  int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing,
>  					  u32 pad, u32 stream, u32 *other_pad,
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 71cb35133b4a..ebb3373b431a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1545,7 +1545,7 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
>   * v4l2_subdev_state_get_format() - Get pointer to a stream format
>   * @state: subdevice state
>   * @pad: pad id
> - * @stream: stream id
> + * @...: stream id (optional argument)
>   *
>   * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad +
>   * stream in the subdev state.
> @@ -1553,15 +1553,31 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
>   * 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(...)				\
> +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> +				       _stream, _pad)(__VA_ARGS__)
> +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\

How about renaming this macro to __v4l2_subdev_state_get_format_name ...

> +	__v4l2_subdev_state_get_format ## ARG
> +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> +				      unsigned int pad, u32 stream);

... and this function to __v4l2_subdev_state_get_format() ? That way the
macro used by drivers and the backend function will have the same name,
with a __ prefix for the function. I think it would be a bit cleaner.
Same below.

But now that I've written that, I realize you would need an additional
__v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
you think that's cleaner or not.


You could also take it one step forward, and avoid three copies of the
same name selection macro:

#define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
	__v4l2_subdev_state_get_ ## name ## ARG

#define v4l2_subdev_state_get_format(...)				\
	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
				      _stream, _pad)(__VA_ARGS__)
#define __v4l2_subdev_state_get_format_pad(state, pad)			\
	__v4l2_subdev_state_get_format(state, pad, 0)
#define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
	__v4l2_subdev_state_get_format(state, pad, stream)

struct v4l2_mbus_framefmt *
__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state,
			       unsigned int pad, u32 stream);

This can be done regardless of whether or not we want to drop the
_stream suffix from the backend functions.

>  
>  /**
>   * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle
>   * @state: subdevice state
>   * @pad: pad id
> - * @stream: stream id
> + * @...: stream id (optional argument)
>   *
>   * This returns a pointer to crop rectangle for the given pad + stream in the
>   * subdev state.
> @@ -1569,15 +1585,22 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
>   * 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(...)					\
> +	__v4l2_subdev_state_get_crop(__VA_ARGS__,			\
> +				     _stream, _pad)(__VA_ARGS__)
> +#define __v4l2_subdev_state_get_crop(_1, _2, _3, ARG, ...)	\
> +	__v4l2_subdev_state_get_crop ## ARG
> +#define __v4l2_subdev_state_get_crop_pad(state, pad)		\
> +	__v4l2_subdev_state_get_crop_stream(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_crop_stream(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: stream id
> + * @...: stream id (optional argument)
>   *
>   * This returns a pointer to compose rectangle for the given pad + stream in the
>   * subdev state.
> @@ -1585,9 +1608,16 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
>   * 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(...)				\
> +	__v4l2_subdev_state_get_compose(__VA_ARGS__,		\
> +					_stream, _pad)(__VA_ARGS__)
> +#define __v4l2_subdev_state_get_compose(_1, _2, _3, ARG, ...)	\
> +	__v4l2_subdev_state_get_compose ## ARG
> +#define __v4l2_subdev_state_get_compose_pad(state, pad)		\
> +	__v4l2_subdev_state_get_compose_stream(state, pad, 0)
>  struct v4l2_rect *
> -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> -			      u32 stream);
> +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> +				       unsigned int pad, u32 stream);
>  
>  /**
>   * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
Sakari Ailus Oct. 27, 2023, 9:06 a.m. UTC | #2
Hi Laurent,

On Thu, Oct 26, 2023 at 04:49:04PM +0300, Laurent Pinchart wrote:
> On Thu, Oct 26, 2023 at 10:03:25AM +0300, Sakari Ailus wrote:
> > The sub-device state access functions take three arguments: sub-device
> > state, pad and stream. The stream is not relevant for the majority of
> > drivers and having to specify 0 for the stream is considered a nuisance.
> > 
> > Provide a two-argument macros for these state access functions to cover
> > the needs of stream-unaware users.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 18 +++++-----
> >  include/media/v4l2-subdev.h           | 48 ++++++++++++++++++++++-----
> >  2 files changed, 48 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 2a615836c1d4..9d4ff9b4fcec 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1671,8 +1671,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_format(struct v4l2_subdev_state *state, unsigned int pad,
> > -			     u32 stream)
> > +__v4l2_subdev_state_get_format_stream(struct v4l2_subdev_state *state,
> > +				      unsigned int pad, u32 stream)
> >  {
> >  	struct v4l2_subdev_stream_configs *stream_configs;
> >  	unsigned int i;
> > @@ -1706,11 +1706,11 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
> >  
> >  	return NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_format);
> > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format_stream);
> >  
> >  struct v4l2_rect *
> > -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > -			   u32 stream)
> > +__v4l2_subdev_state_get_crop_stream(struct v4l2_subdev_state *state,
> > +				    unsigned int pad, u32 stream)
> >  {
> >  	struct v4l2_subdev_stream_configs *stream_configs;
> >  	unsigned int i;
> > @@ -1744,11 +1744,11 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> >  
> >  	return NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_crop);
> > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop_stream);
> >  
> >  struct v4l2_rect *
> > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > -			      u32 stream)
> > +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> > +				       unsigned int pad, u32 stream)
> >  {
> >  	struct v4l2_subdev_stream_configs *stream_configs;
> >  	unsigned int i;
> > @@ -1782,7 +1782,7 @@ v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> >  
> >  	return NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_compose);
> > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose_stream);
> >  
> >  int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing,
> >  					  u32 pad, u32 stream, u32 *other_pad,
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 71cb35133b4a..ebb3373b431a 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -1545,7 +1545,7 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> >   * v4l2_subdev_state_get_format() - Get pointer to a stream format
> >   * @state: subdevice state
> >   * @pad: pad id
> > - * @stream: stream id
> > + * @...: stream id (optional argument)
> >   *
> >   * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad +
> >   * stream in the subdev state.
> > @@ -1553,15 +1553,31 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> >   * 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(...)				\
> > +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> > +				       _stream, _pad)(__VA_ARGS__)
> > +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
> 
> How about renaming this macro to __v4l2_subdev_state_get_format_name ...
> 
> > +	__v4l2_subdev_state_get_format ## ARG
> > +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> > +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> > +				      unsigned int pad, u32 stream);
> 
> ... and this function to __v4l2_subdev_state_get_format() ? That way the
> macro used by drivers and the backend function will have the same name,
> with a __ prefix for the function. I think it would be a bit cleaner.
> Same below.
> 
> But now that I've written that, I realize you would need an additional
> __v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
> you think that's cleaner or not.
> 
> 
> You could also take it one step forward, and avoid three copies of the
> same name selection macro:
> 
> #define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
> 	__v4l2_subdev_state_get_ ## name ## ARG
> 
> #define v4l2_subdev_state_get_format(...)				\
> 	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
> 				      _stream, _pad)(__VA_ARGS__)

This seems like a good idea. How about calling it
__v4l2_subdev_state_gen_call()? It better describes what it does I think.

> #define __v4l2_subdev_state_get_format_pad(state, pad)			\
> 	__v4l2_subdev_state_get_format(state, pad, 0)
> #define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
> 	__v4l2_subdev_state_get_format(state, pad, stream)

This one isn't needed.

> 
> struct v4l2_mbus_framefmt *
> __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state,
> 			       unsigned int pad, u32 stream);
> 
> This can be done regardless of whether or not we want to drop the
> _stream suffix from the backend functions.
> 
> >  
> >  /**
> >   * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle
> >   * @state: subdevice state
> >   * @pad: pad id
> > - * @stream: stream id
> > + * @...: stream id (optional argument)
> >   *
> >   * This returns a pointer to crop rectangle for the given pad + stream in the
> >   * subdev state.
> > @@ -1569,15 +1585,22 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
> >   * 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(...)					\
> > +	__v4l2_subdev_state_get_crop(__VA_ARGS__,			\
> > +				     _stream, _pad)(__VA_ARGS__)
> > +#define __v4l2_subdev_state_get_crop(_1, _2, _3, ARG, ...)	\
> > +	__v4l2_subdev_state_get_crop ## ARG
> > +#define __v4l2_subdev_state_get_crop_pad(state, pad)		\
> > +	__v4l2_subdev_state_get_crop_stream(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_crop_stream(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: stream id
> > + * @...: stream id (optional argument)
> >   *
> >   * This returns a pointer to compose rectangle for the given pad + stream in the
> >   * subdev state.
> > @@ -1585,9 +1608,16 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> >   * 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(...)				\
> > +	__v4l2_subdev_state_get_compose(__VA_ARGS__,		\
> > +					_stream, _pad)(__VA_ARGS__)
> > +#define __v4l2_subdev_state_get_compose(_1, _2, _3, ARG, ...)	\
> > +	__v4l2_subdev_state_get_compose ## ARG
> > +#define __v4l2_subdev_state_get_compose_pad(state, pad)		\
> > +	__v4l2_subdev_state_get_compose_stream(state, pad, 0)
> >  struct v4l2_rect *
> > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > -			      u32 stream);
> > +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> > +				       unsigned int pad, u32 stream);
> >  
> >  /**
> >   * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
Laurent Pinchart Oct. 27, 2023, 9:10 a.m. UTC | #3
On Fri, Oct 27, 2023 at 09:06:17AM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> On Thu, Oct 26, 2023 at 04:49:04PM +0300, Laurent Pinchart wrote:
> > On Thu, Oct 26, 2023 at 10:03:25AM +0300, Sakari Ailus wrote:
> > > The sub-device state access functions take three arguments: sub-device
> > > state, pad and stream. The stream is not relevant for the majority of
> > > drivers and having to specify 0 for the stream is considered a nuisance.
> > > 
> > > Provide a two-argument macros for these state access functions to cover
> > > the needs of stream-unaware users.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-subdev.c | 18 +++++-----
> > >  include/media/v4l2-subdev.h           | 48 ++++++++++++++++++++++-----
> > >  2 files changed, 48 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index 2a615836c1d4..9d4ff9b4fcec 100644
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -1671,8 +1671,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_format(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			     u32 stream)
> > > +__v4l2_subdev_state_get_format_stream(struct v4l2_subdev_state *state,
> > > +				      unsigned int pad, u32 stream)
> > >  {
> > >  	struct v4l2_subdev_stream_configs *stream_configs;
> > >  	unsigned int i;
> > > @@ -1706,11 +1706,11 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
> > >  
> > >  	return NULL;
> > >  }
> > > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_format);
> > > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format_stream);
> > >  
> > >  struct v4l2_rect *
> > > -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			   u32 stream)
> > > +__v4l2_subdev_state_get_crop_stream(struct v4l2_subdev_state *state,
> > > +				    unsigned int pad, u32 stream)
> > >  {
> > >  	struct v4l2_subdev_stream_configs *stream_configs;
> > >  	unsigned int i;
> > > @@ -1744,11 +1744,11 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > >  
> > >  	return NULL;
> > >  }
> > > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_crop);
> > > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop_stream);
> > >  
> > >  struct v4l2_rect *
> > > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			      u32 stream)
> > > +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> > > +				       unsigned int pad, u32 stream)
> > >  {
> > >  	struct v4l2_subdev_stream_configs *stream_configs;
> > >  	unsigned int i;
> > > @@ -1782,7 +1782,7 @@ v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > >  
> > >  	return NULL;
> > >  }
> > > -EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_compose);
> > > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose_stream);
> > >  
> > >  int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing,
> > >  					  u32 pad, u32 stream, u32 *other_pad,
> > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > > index 71cb35133b4a..ebb3373b431a 100644
> > > --- a/include/media/v4l2-subdev.h
> > > +++ b/include/media/v4l2-subdev.h
> > > @@ -1545,7 +1545,7 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> > >   * v4l2_subdev_state_get_format() - Get pointer to a stream format
> > >   * @state: subdevice state
> > >   * @pad: pad id
> > > - * @stream: stream id
> > > + * @...: stream id (optional argument)
> > >   *
> > >   * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad +
> > >   * stream in the subdev state.
> > > @@ -1553,15 +1553,31 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> > >   * 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(...)				\
> > > +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> > > +				       _stream, _pad)(__VA_ARGS__)
> > > +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
> > 
> > How about renaming this macro to __v4l2_subdev_state_get_format_name ...
> > 
> > > +	__v4l2_subdev_state_get_format ## ARG
> > > +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> > > +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> > > +				      unsigned int pad, u32 stream);
> > 
> > ... and this function to __v4l2_subdev_state_get_format() ? That way the
> > macro used by drivers and the backend function will have the same name,
> > with a __ prefix for the function. I think it would be a bit cleaner.
> > Same below.
> > 
> > But now that I've written that, I realize you would need an additional
> > __v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
> > you think that's cleaner or not.
> > 
> > 
> > You could also take it one step forward, and avoid three copies of the
> > same name selection macro:
> > 
> > #define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
> > 	__v4l2_subdev_state_get_ ## name ## ARG
> > 
> > #define v4l2_subdev_state_get_format(...)				\
> > 	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
> > 				      _stream, _pad)(__VA_ARGS__)
> 
> This seems like a good idea. How about calling it
> __v4l2_subdev_state_gen_call()? It better describes what it does I think.

Works for me. It's internal anyway.

> > #define __v4l2_subdev_state_get_format_pad(state, pad)			\
> > 	__v4l2_subdev_state_get_format(state, pad, 0)
> > #define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
> > 	__v4l2_subdev_state_get_format(state, pad, stream)
> 
> This one isn't needed.

You could indeed drop it if you remove the _stream argument to the
__v4l2_subdev_state_get_macro() macro above. I thought it would be nice
to keep it though, to make it more explicit, but I don't mind much.

> > struct v4l2_mbus_framefmt *
> > __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state,
> > 			       unsigned int pad, u32 stream);
> > 
> > This can be done regardless of whether or not we want to drop the
> > _stream suffix from the backend functions.
> > 
> > >  /**
> > >   * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle
> > >   * @state: subdevice state
> > >   * @pad: pad id
> > > - * @stream: stream id
> > > + * @...: stream id (optional argument)
> > >   *
> > >   * This returns a pointer to crop rectangle for the given pad + stream in the
> > >   * subdev state.
> > > @@ -1569,15 +1585,22 @@ v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
> > >   * 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(...)					\
> > > +	__v4l2_subdev_state_get_crop(__VA_ARGS__,			\
> > > +				     _stream, _pad)(__VA_ARGS__)
> > > +#define __v4l2_subdev_state_get_crop(_1, _2, _3, ARG, ...)	\
> > > +	__v4l2_subdev_state_get_crop ## ARG
> > > +#define __v4l2_subdev_state_get_crop_pad(state, pad)		\
> > > +	__v4l2_subdev_state_get_crop_stream(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_crop_stream(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: stream id
> > > + * @...: stream id (optional argument)
> > >   *
> > >   * This returns a pointer to compose rectangle for the given pad + stream in the
> > >   * subdev state.
> > > @@ -1585,9 +1608,16 @@ v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > >   * 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(...)				\
> > > +	__v4l2_subdev_state_get_compose(__VA_ARGS__,		\
> > > +					_stream, _pad)(__VA_ARGS__)
> > > +#define __v4l2_subdev_state_get_compose(_1, _2, _3, ARG, ...)	\
> > > +	__v4l2_subdev_state_get_compose ## ARG
> > > +#define __v4l2_subdev_state_get_compose_pad(state, pad)		\
> > > +	__v4l2_subdev_state_get_compose_stream(state, pad, 0)
> > >  struct v4l2_rect *
> > > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			      u32 stream);
> > > +__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
> > > +				       unsigned int pad, u32 stream);
> > >  
> > >  /**
> > >   * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
Sakari Ailus Oct. 27, 2023, 9:12 a.m. UTC | #4
On Fri, Oct 27, 2023 at 12:10:13PM +0300, Laurent Pinchart wrote:
> > > > +/*
> > > > + * 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(...)				\
> > > > +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> > > > +				       _stream, _pad)(__VA_ARGS__)
> > > > +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
> > > 
> > > How about renaming this macro to __v4l2_subdev_state_get_format_name ...
> > > 
> > > > +	__v4l2_subdev_state_get_format ## ARG
> > > > +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> > > > +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> > > > +				      unsigned int pad, u32 stream);
> > > 
> > > ... and this function to __v4l2_subdev_state_get_format() ? That way the
> > > macro used by drivers and the backend function will have the same name,
> > > with a __ prefix for the function. I think it would be a bit cleaner.
> > > Same below.
> > > 
> > > But now that I've written that, I realize you would need an additional
> > > __v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
> > > you think that's cleaner or not.
> > > 
> > > 
> > > You could also take it one step forward, and avoid three copies of the
> > > same name selection macro:
> > > 
> > > #define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
> > > 	__v4l2_subdev_state_get_ ## name ## ARG
> > > 
> > > #define v4l2_subdev_state_get_format(...)				\
> > > 	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
> > > 				      _stream, _pad)(__VA_ARGS__)
> > 
> > This seems like a good idea. How about calling it
> > __v4l2_subdev_state_gen_call()? It better describes what it does I think.
> 
> Works for me. It's internal anyway.
> 
> > > #define __v4l2_subdev_state_get_format_pad(state, pad)			\
> > > 	__v4l2_subdev_state_get_format(state, pad, 0)
> > > #define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
> > > 	__v4l2_subdev_state_get_format(state, pad, stream)
> > 
> > This one isn't needed.
> 
> You could indeed drop it if you remove the _stream argument to the
> __v4l2_subdev_state_get_macro() macro above. I thought it would be nice
> to keep it though, to make it more explicit, but I don't mind much.

This is certainly up to interpretation but I generally prefer fewer
wrappers. :-)

We can also make the function __v4l2_subdev_state_get_format() by just
removing "_stream" from the macro call --- empty arguments are allowed.
Laurent Pinchart Oct. 27, 2023, 9:14 a.m. UTC | #5
On Fri, Oct 27, 2023 at 09:12:14AM +0000, Sakari Ailus wrote:
> On Fri, Oct 27, 2023 at 12:10:13PM +0300, Laurent Pinchart wrote:
> > > > > +/*
> > > > > + * 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(...)				\
> > > > > +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> > > > > +				       _stream, _pad)(__VA_ARGS__)
> > > > > +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
> > > > 
> > > > How about renaming this macro to __v4l2_subdev_state_get_format_name ...
> > > > 
> > > > > +	__v4l2_subdev_state_get_format ## ARG
> > > > > +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> > > > > +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> > > > > +				      unsigned int pad, u32 stream);
> > > > 
> > > > ... and this function to __v4l2_subdev_state_get_format() ? That way the
> > > > macro used by drivers and the backend function will have the same name,
> > > > with a __ prefix for the function. I think it would be a bit cleaner.
> > > > Same below.
> > > > 
> > > > But now that I've written that, I realize you would need an additional
> > > > __v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
> > > > you think that's cleaner or not.
> > > > 
> > > > 
> > > > You could also take it one step forward, and avoid three copies of the
> > > > same name selection macro:
> > > > 
> > > > #define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
> > > > 	__v4l2_subdev_state_get_ ## name ## ARG
> > > > 
> > > > #define v4l2_subdev_state_get_format(...)				\
> > > > 	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
> > > > 				      _stream, _pad)(__VA_ARGS__)
> > > 
> > > This seems like a good idea. How about calling it
> > > __v4l2_subdev_state_gen_call()? It better describes what it does I think.
> > 
> > Works for me. It's internal anyway.
> > 
> > > > #define __v4l2_subdev_state_get_format_pad(state, pad)			\
> > > > 	__v4l2_subdev_state_get_format(state, pad, 0)
> > > > #define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
> > > > 	__v4l2_subdev_state_get_format(state, pad, stream)
> > > 
> > > This one isn't needed.
> > 
> > You could indeed drop it if you remove the _stream argument to the
> > __v4l2_subdev_state_get_macro() macro above. I thought it would be nice
> > to keep it though, to make it more explicit, but I don't mind much.
> 
> This is certainly up to interpretation but I generally prefer fewer
> wrappers. :-)

Up to you.

> We can also make the function __v4l2_subdev_state_get_format() by just
> removing "_stream" from the macro call --- empty arguments are allowed.

Yes, that's what I meant.
Sakari Ailus Oct. 27, 2023, 9:21 a.m. UTC | #6
On Fri, Oct 27, 2023 at 12:14:27PM +0300, Laurent Pinchart wrote:
> On Fri, Oct 27, 2023 at 09:12:14AM +0000, Sakari Ailus wrote:
> > On Fri, Oct 27, 2023 at 12:10:13PM +0300, Laurent Pinchart wrote:
> > > > > > +/*
> > > > > > + * 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(...)				\
> > > > > > +	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
> > > > > > +				       _stream, _pad)(__VA_ARGS__)
> > > > > > +#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
> > > > > 
> > > > > How about renaming this macro to __v4l2_subdev_state_get_format_name ...
> > > > > 
> > > > > > +	__v4l2_subdev_state_get_format ## ARG
> > > > > > +#define __v4l2_subdev_state_get_format_pad(state, pad)		\
> > > > > > +	__v4l2_subdev_state_get_format_stream(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_format_stream(struct v4l2_subdev_state *state,
> > > > > > +				      unsigned int pad, u32 stream);
> > > > > 
> > > > > ... and this function to __v4l2_subdev_state_get_format() ? That way the
> > > > > macro used by drivers and the backend function will have the same name,
> > > > > with a __ prefix for the function. I think it would be a bit cleaner.
> > > > > Same below.
> > > > > 
> > > > > But now that I've written that, I realize you would need an additional
> > > > > __v4l2_subdev_state_get_format_stream() macro. I'll let you decide if
> > > > > you think that's cleaner or not.
> > > > > 
> > > > > 
> > > > > You could also take it one step forward, and avoid three copies of the
> > > > > same name selection macro:
> > > > > 
> > > > > #define __v4l2_subdev_state_get_macro(name, _1, _2, _3, ARG, ...)	\
> > > > > 	__v4l2_subdev_state_get_ ## name ## ARG
> > > > > 
> > > > > #define v4l2_subdev_state_get_format(...)				\
> > > > > 	__v4l2_subdev_state_get_macro(format, __VA_ARGS__,		\
> > > > > 				      _stream, _pad)(__VA_ARGS__)
> > > > 
> > > > This seems like a good idea. How about calling it
> > > > __v4l2_subdev_state_gen_call()? It better describes what it does I think.
> > > 
> > > Works for me. It's internal anyway.
> > > 
> > > > > #define __v4l2_subdev_state_get_format_pad(state, pad)			\
> > > > > 	__v4l2_subdev_state_get_format(state, pad, 0)
> > > > > #define __v4l2_subdev_state_get_format_stream(state, pad, stream)	\
> > > > > 	__v4l2_subdev_state_get_format(state, pad, stream)
> > > > 
> > > > This one isn't needed.
> > > 
> > > You could indeed drop it if you remove the _stream argument to the
> > > __v4l2_subdev_state_get_macro() macro above. I thought it would be nice
> > > to keep it though, to make it more explicit, but I don't mind much.
> > 
> > This is certainly up to interpretation but I generally prefer fewer
> > wrappers. :-)
> 
> Up to you.
> 
> > We can also make the function __v4l2_subdev_state_get_format() by just
> > removing "_stream" from the macro call --- empty arguments are allowed.
> 
> Yes, that's what I meant.

Great, I meant the same thing! ;-)

The result becomes thus:

/*
 * 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, _2, _3, ARG, ...)	\
	__v4l2_subdev_state_get_ ## NAME ## ARG

...

#define v4l2_subdev_state_get_format(...)				\
	__v4l2_subdev_state_gen_call(format, __VA_ARGS__, , _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);

Etc. I quite like it actually, compared how it was in the beginning.
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 2a615836c1d4..9d4ff9b4fcec 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1671,8 +1671,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_format(struct v4l2_subdev_state *state, unsigned int pad,
-			     u32 stream)
+__v4l2_subdev_state_get_format_stream(struct v4l2_subdev_state *state,
+				      unsigned int pad, u32 stream)
 {
 	struct v4l2_subdev_stream_configs *stream_configs;
 	unsigned int i;
@@ -1706,11 +1706,11 @@  v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_format);
+EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format_stream);
 
 struct v4l2_rect *
-v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
-			   u32 stream)
+__v4l2_subdev_state_get_crop_stream(struct v4l2_subdev_state *state,
+				    unsigned int pad, u32 stream)
 {
 	struct v4l2_subdev_stream_configs *stream_configs;
 	unsigned int i;
@@ -1744,11 +1744,11 @@  v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_crop);
+EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop_stream);
 
 struct v4l2_rect *
-v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
-			      u32 stream)
+__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
+				       unsigned int pad, u32 stream)
 {
 	struct v4l2_subdev_stream_configs *stream_configs;
 	unsigned int i;
@@ -1782,7 +1782,7 @@  v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_compose);
+EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose_stream);
 
 int v4l2_subdev_routing_find_opposite_end(const struct v4l2_subdev_krouting *routing,
 					  u32 pad, u32 stream, u32 *other_pad,
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 71cb35133b4a..ebb3373b431a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1545,7 +1545,7 @@  int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
  * v4l2_subdev_state_get_format() - Get pointer to a stream format
  * @state: subdevice state
  * @pad: pad id
- * @stream: stream id
+ * @...: stream id (optional argument)
  *
  * This returns a pointer to &struct v4l2_mbus_framefmt for the given pad +
  * stream in the subdev state.
@@ -1553,15 +1553,31 @@  int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
  * 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(...)				\
+	__v4l2_subdev_state_get_format(__VA_ARGS__,			\
+				       _stream, _pad)(__VA_ARGS__)
+#define __v4l2_subdev_state_get_format(_1, _2, _3, ARG, ...)	\
+	__v4l2_subdev_state_get_format ## ARG
+#define __v4l2_subdev_state_get_format_pad(state, pad)		\
+	__v4l2_subdev_state_get_format_stream(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_format_stream(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: stream id
+ * @...: stream id (optional argument)
  *
  * This returns a pointer to crop rectangle for the given pad + stream in the
  * subdev state.
@@ -1569,15 +1585,22 @@  v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, unsigned int pad,
  * 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(...)					\
+	__v4l2_subdev_state_get_crop(__VA_ARGS__,			\
+				     _stream, _pad)(__VA_ARGS__)
+#define __v4l2_subdev_state_get_crop(_1, _2, _3, ARG, ...)	\
+	__v4l2_subdev_state_get_crop ## ARG
+#define __v4l2_subdev_state_get_crop_pad(state, pad)		\
+	__v4l2_subdev_state_get_crop_stream(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_crop_stream(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: stream id
+ * @...: stream id (optional argument)
  *
  * This returns a pointer to compose rectangle for the given pad + stream in the
  * subdev state.
@@ -1585,9 +1608,16 @@  v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
  * 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(...)				\
+	__v4l2_subdev_state_get_compose(__VA_ARGS__,		\
+					_stream, _pad)(__VA_ARGS__)
+#define __v4l2_subdev_state_get_compose(_1, _2, _3, ARG, ...)	\
+	__v4l2_subdev_state_get_compose ## ARG
+#define __v4l2_subdev_state_get_compose_pad(state, pad)		\
+	__v4l2_subdev_state_get_compose_stream(state, pad, 0)
 struct v4l2_rect *
-v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
-			      u32 stream);
+__v4l2_subdev_state_get_compose_stream(struct v4l2_subdev_state *state,
+				       unsigned int pad, u32 stream);
 
 /**
  * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream