diff mbox series

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

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

Commit Message

Sakari Ailus Oct. 23, 2023, 5:44 p.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>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++-------
 include/media/v4l2-subdev.h           | 39 ++++++++++++++++++++-------
 2 files changed, 39 insertions(+), 18 deletions(-)

Comments

Laurent Pinchart Oct. 25, 2023, 10:50 a.m. UTC | #1
Hi Sakari,

On Wed, Oct 25, 2023 at 10:19:15AM +0000, Sakari Ailus wrote:
> On Wed, Oct 25, 2023 at 11:53:35AM +0200, Hans Verkuil wrote:
> > On 23/10/2023 19:44, 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>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++-------
> > >  include/media/v4l2-subdev.h           | 39 ++++++++++++++++++++-------
> > >  2 files changed, 39 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index f0f0af48730f..e35226587244 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(struct v4l2_subdev_state *state,
> > > +			       unsigned int pad, u32 stream)
> > >  {
> > >  	struct v4l2_subdev_stream_configs *stream_configs;
> > >  	unsigned int i;
> > > @@ -1702,11 +1702,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);
> > >  
> > >  struct v4l2_rect *
> > > -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			   u32 stream)
> > > +__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;
> > > @@ -1736,11 +1736,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);
> > >  
> > >  struct v4l2_rect *
> > > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > > -			      u32 stream)
> > > +__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;
> > > @@ -1770,7 +1770,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);
> > >  
> > >  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..12eb0fe746d7 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,22 @@ 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.
> > >   */
> > > +#define v4l2_subdev_state_get_format(state, pad, ...)         \
> > > +        __v4l2_subdev_state_get_format_ ## __VA_OPT__(stream) \
> > > +        (state, pad __VA_OPT__(,) __VA_ARGS__)
> > > +#define __v4l2_subdev_state_get_format_(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)
> > 
> > Hmm, 'git grep __VA_OPT__' finds no use of this in the kernel. Are you sure this
> > is safe to use? Does clang support it? Do all gcc versions that the kernel support
> > implement it?
> > 
> > A 10+ for creativity, but I'm a bit concerned about it.
> 
> :-)
> 
> I noticed the problem, too, as the lkp bot failed compiling this on GCC
> 7.5. Indeed only GCC 8 supports __VA_OPT__ but the oldest GCC version
> supported for kernel compilation is 7.1. So __VA_OPT__ isn't... an option.
> 
> I thought this could be done using variable arguments, making most error
> handling take place at runtime (for GCC versions not supporting
> __VA_OPT__), but then Laurent figured this can be done with just
> __VA_ARGS__ which is part of C99.
> 
> The code would look like this, which is only slightly less understandable
> than with __VA_OPT__:
> 
> #define v4l2_subdev_state_get_format(...)				\
> 	__v4l2_subdev_state_get_format_var(__VA_ARGS__, , _2)(__VA_ARGS__)

How about

 	__v4l2_subdev_state_get_format_var(__VA_ARGS__, _stream, _pad)(__VA_ARGS__)

? The internal macro and function names would then be more explicit.

> #define __v4l2_subdev_state_get_format_var(_1, _2, _3, NUM, ...)	\
> 	__v4l2_subdev_state_get_format ## NUM
> #define __v4l2_subdev_state_get_format_2(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);
> 
> > And if there are only a few drivers that need to give a specific stream,
> > wouldn't it be easier to just have a v4l2_subdev_state_get_format_stream
> > function and a static inline v4l2_subdev_state_get_format() that just calls
> > the _stream version with 0?
> 
> I'm trying to shorten function names here: they're overly long. Also I'd
> like to see a single set of API functions to access sub-device state
> information irrespective of how it is stored. I originally proposed just
> specifying stream zero but both Laurent and Tomi opined against it.
Sakari Ailus Oct. 25, 2023, 11:03 a.m. UTC | #2
Hi Laurent,

On Wed, Oct 25, 2023 at 01:50:55PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wed, Oct 25, 2023 at 10:19:15AM +0000, Sakari Ailus wrote:
> > On Wed, Oct 25, 2023 at 11:53:35AM +0200, Hans Verkuil wrote:
> > > On 23/10/2023 19:44, 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>
> > > > ---
> > > >  drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++-------
> > > >  include/media/v4l2-subdev.h           | 39 ++++++++++++++++++++-------
> > > >  2 files changed, 39 insertions(+), 18 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > > index f0f0af48730f..e35226587244 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(struct v4l2_subdev_state *state,
> > > > +			       unsigned int pad, u32 stream)
> > > >  {
> > > >  	struct v4l2_subdev_stream_configs *stream_configs;
> > > >  	unsigned int i;
> > > > @@ -1702,11 +1702,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);
> > > >  
> > > >  struct v4l2_rect *
> > > > -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> > > > -			   u32 stream)
> > > > +__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;
> > > > @@ -1736,11 +1736,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);
> > > >  
> > > >  struct v4l2_rect *
> > > > -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> > > > -			      u32 stream)
> > > > +__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;
> > > > @@ -1770,7 +1770,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);
> > > >  
> > > >  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..12eb0fe746d7 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,22 @@ 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.
> > > >   */
> > > > +#define v4l2_subdev_state_get_format(state, pad, ...)         \
> > > > +        __v4l2_subdev_state_get_format_ ## __VA_OPT__(stream) \
> > > > +        (state, pad __VA_OPT__(,) __VA_ARGS__)
> > > > +#define __v4l2_subdev_state_get_format_(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)
> > > 
> > > Hmm, 'git grep __VA_OPT__' finds no use of this in the kernel. Are you sure this
> > > is safe to use? Does clang support it? Do all gcc versions that the kernel support
> > > implement it?
> > > 
> > > A 10+ for creativity, but I'm a bit concerned about it.
> > 
> > :-)
> > 
> > I noticed the problem, too, as the lkp bot failed compiling this on GCC
> > 7.5. Indeed only GCC 8 supports __VA_OPT__ but the oldest GCC version
> > supported for kernel compilation is 7.1. So __VA_OPT__ isn't... an option.
> > 
> > I thought this could be done using variable arguments, making most error
> > handling take place at runtime (for GCC versions not supporting
> > __VA_OPT__), but then Laurent figured this can be done with just
> > __VA_ARGS__ which is part of C99.
> > 
> > The code would look like this, which is only slightly less understandable
> > than with __VA_OPT__:
> > 
> > #define v4l2_subdev_state_get_format(...)				\
> > 	__v4l2_subdev_state_get_format_var(__VA_ARGS__, , _2)(__VA_ARGS__)
> 
> How about
> 
>  	__v4l2_subdev_state_get_format_var(__VA_ARGS__, _stream, _pad)(__VA_ARGS__)
> 
> ? The internal macro and function names would then be more explicit.

Works for me.

I'll also rename __v4l2_subdev_state_get_format_var, removing _var.

> 
> > #define __v4l2_subdev_state_get_format_var(_1, _2, _3, NUM, ...)	\
> > 	__v4l2_subdev_state_get_format ## NUM
> > #define __v4l2_subdev_state_get_format_2(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);
> > 
> > > And if there are only a few drivers that need to give a specific stream,
> > > wouldn't it be easier to just have a v4l2_subdev_state_get_format_stream
> > > function and a static inline v4l2_subdev_state_get_format() that just calls
> > > the _stream version with 0?
> > 
> > I'm trying to shorten function names here: they're overly long. Also I'd
> > like to see a single set of API functions to access sub-device state
> > information irrespective of how it is stored. I originally proposed just
> > specifying stream zero but both Laurent and Tomi opined against it.
>
Sakari Ailus Oct. 25, 2023, 11:30 a.m. UTC | #3
Hi Hans,

On Wed, Oct 25, 2023 at 01:20:29PM +0200, Hans Verkuil wrote:
> On 25/10/2023 12:19, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > Thank you for the review.
> > 
> > On Wed, Oct 25, 2023 at 11:53:35AM +0200, Hans Verkuil wrote:
> >> On 23/10/2023 19:44, 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>
> >>> ---
> >>>  drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++-------
> >>>  include/media/v4l2-subdev.h           | 39 ++++++++++++++++++++-------
> >>>  2 files changed, 39 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> >>> index f0f0af48730f..e35226587244 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(struct v4l2_subdev_state *state,
> >>> +			       unsigned int pad, u32 stream)
> >>>  {
> >>>  	struct v4l2_subdev_stream_configs *stream_configs;
> >>>  	unsigned int i;
> >>> @@ -1702,11 +1702,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);
> >>>  
> >>>  struct v4l2_rect *
> >>> -v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
> >>> -			   u32 stream)
> >>> +__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;
> >>> @@ -1736,11 +1736,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);
> >>>  
> >>>  struct v4l2_rect *
> >>> -v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
> >>> -			      u32 stream)
> >>> +__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;
> >>> @@ -1770,7 +1770,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);
> >>>  
> >>>  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..12eb0fe746d7 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,22 @@ 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.
> >>>   */
> >>> +#define v4l2_subdev_state_get_format(state, pad, ...)         \
> >>> +        __v4l2_subdev_state_get_format_ ## __VA_OPT__(stream) \
> >>> +        (state, pad __VA_OPT__(,) __VA_ARGS__)
> >>> +#define __v4l2_subdev_state_get_format_(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)
> >>
> >> Hmm, 'git grep __VA_OPT__' finds no use of this in the kernel. Are you sure this
> >> is safe to use? Does clang support it? Do all gcc versions that the kernel support
> >> implement it?
> >>
> >> A 10+ for creativity, but I'm a bit concerned about it.
> > 
> > :-)
> > 
> > I noticed the problem, too, as the lkp bot failed compiling this on GCC
> > 7.5. Indeed only GCC 8 supports __VA_OPT__ but the oldest GCC version
> > supported for kernel compilation is 7.1. So __VA_OPT__ isn't... an option.
> > 
> > I thought this could be done using variable arguments, making most error
> > handling take place at runtime (for GCC versions not supporting
> > __VA_OPT__), but then Laurent figured this can be done with just
> > __VA_ARGS__ which is part of C99.
> > 
> > The code would look like this, which is only slightly less understandable
> > than with __VA_OPT__:
> > 
> > #define v4l2_subdev_state_get_format(...)				\
> > 	__v4l2_subdev_state_get_format_var(__VA_ARGS__, , _2)(__VA_ARGS__)
> > #define __v4l2_subdev_state_get_format_var(_1, _2, _3, NUM, ...)	\
> > 	__v4l2_subdev_state_get_format ## NUM
> > #define __v4l2_subdev_state_get_format_2(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);
> 
> Wow! OK, I guess that will work.
> 
> A strongly suggest adding a comment before this explaining what you are
> trying to achieve. It's not exactly obvious :-)

I'll add:

/*
 * 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.
 */
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f0f0af48730f..e35226587244 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(struct v4l2_subdev_state *state,
+			       unsigned int pad, u32 stream)
 {
 	struct v4l2_subdev_stream_configs *stream_configs;
 	unsigned int i;
@@ -1702,11 +1702,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);
 
 struct v4l2_rect *
-v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
-			   u32 stream)
+__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;
@@ -1736,11 +1736,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);
 
 struct v4l2_rect *
-v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
-			      u32 stream)
+__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;
@@ -1770,7 +1770,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);
 
 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..12eb0fe746d7 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,22 @@  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.
  */
+#define v4l2_subdev_state_get_format(state, pad, ...)         \
+        __v4l2_subdev_state_get_format_ ## __VA_OPT__(stream) \
+        (state, pad __VA_OPT__(,) __VA_ARGS__)
+#define __v4l2_subdev_state_get_format_(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);
+__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: stream id
+ * @...: stream id (optional argument)
  *
  * This returns a pointer to crop rectangle for the given pad + stream in the
  * subdev state.
@@ -1569,15 +1576,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(state, pad, ...)         \
+        __v4l2_subdev_state_get_crop_ ## __VA_OPT__(stream) \
+        (state, pad __VA_OPT__(,) __VA_ARGS__)
+#define __v4l2_subdev_state_get_crop_(state, pad)	\
+        __v4l2_subdev_state_get_crop(state, pad, 0)
+#define __v4l2_subdev_state_get_crop_stream(state, pad, stream)	\
+        __v4l2_subdev_state_get_crop(state, pad, stream)
 struct v4l2_rect *
-v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
-			   u32 stream);
+__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: stream id
+ * @...: stream id (optional argument)
  *
  * This returns a pointer to compose rectangle for the given pad + stream in the
  * subdev state.
@@ -1585,9 +1599,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(state, pad, ...)         \
+        __v4l2_subdev_state_get_compose_ ## __VA_OPT__(stream) \
+        (state, pad __VA_OPT__(,) __VA_ARGS__)
+#define __v4l2_subdev_state_get_compose_(state, pad)	\
+        __v4l2_subdev_state_get_compose(state, pad, 0)
+#define __v4l2_subdev_state_get_compose_stream(state, pad, stream)	\
+        __v4l2_subdev_state_get_compose(state, pad, stream)
 struct v4l2_rect *
-v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, unsigned int pad,
-			      u32 stream);
+__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
+				unsigned int pad, u32 stream);
 
 /**
  * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream