diff mbox series

[v2,3/4] media: v4l2-subdev: Store frame interval in subdev state

Message ID 20231127111703.30527-1-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series media: v4l2-subdev: Improve frame interval handling | expand

Commit Message

Laurent Pinchart Nov. 27, 2023, 11:17 a.m. UTC
Subdev states store all standard pad configuration data, except for
frame intervals. Fix it by adding interval fields in the
v4l2_subdev_pad_config and v4l2_subdev_stream_config structures, with
corresponding accessor functions and a helper function to implement the
.get_frame_interval() operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Use __v4l2_subdev_state_gen_call()
---
 drivers/media/v4l2-core/v4l2-subdev.c | 50 +++++++++++++++++++++++++++
 include/media/v4l2-subdev.h           | 43 +++++++++++++++++++++++
 2 files changed, 93 insertions(+)

Comments

Laurent Pinchart Nov. 28, 2023, 10:18 a.m. UTC | #1
Hi Hans,

On Tue, Nov 28, 2023 at 10:42:58AM +0100, Hans Verkuil wrote:
> On 27/11/2023 12:17, Laurent Pinchart wrote:
> > Subdev states store all standard pad configuration data, except for
> > frame intervals. Fix it by adding interval fields in the
> > v4l2_subdev_pad_config and v4l2_subdev_stream_config structures, with
> > corresponding accessor functions and a helper function to implement the
> > .get_frame_interval() operation.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Use __v4l2_subdev_state_gen_call()
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 50 +++++++++++++++++++++++++++
> >  include/media/v4l2-subdev.h           | 43 +++++++++++++++++++++++
> >  2 files changed, 93 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 4cbe4024ff67..559d6a5082b1 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1651,6 +1651,40 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
> >  }
> >  EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose);
> >  
> > +struct v4l2_fract *
> > +__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
> > +				 unsigned int pad, u32 stream)
> > +{
> > +	struct v4l2_subdev_stream_configs *stream_configs;
> > +	unsigned int i;
> > +
> > +	if (WARN_ON(!state))
> > +		return NULL;
> > +
> > +	lockdep_assert_held(state->lock);
> > +
> > +	if (state->pads) {
> > +		if (stream)
> > +			return NULL;
> > +
> > +		if (WARN_ON(pad >= state->sd->entity.num_pads))
> > +			pad = 0;
> 
> In the other variants (e.g. __v4l2_subdev_state_get_compose) there
> is no WARN_ON and it just returns NULL.
> 
> > +
> > +		return &state->pads[pad].interval;
> > +	}
> 
> The other variants have a lockdep_assert_held(state->lock); here.
> 
> I think this should be the same as the other variants.

I think this is because I wrote this patch before the other variants
were changed. I'll update it for the next version.

> > +
> > +	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].interval;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_interval);
> > +
> >  #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> >  
> >  static int
> > @@ -1717,6 +1751,22 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
> >  
> > +int v4l2_subdev_get_frame_interval(struct v4l2_subdev *sd,
> > +				   struct v4l2_subdev_state *state,
> > +				   struct v4l2_subdev_frame_interval *fi)
> > +{
> > +	struct v4l2_fract *interval;
> > +
> > +	interval = v4l2_subdev_state_get_interval(state, fi->pad, fi->stream);
> > +	if (!interval)
> > +		return -EINVAL;
> > +
> > +	fi->interval = *interval;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_interval);
> > +
> >  int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> >  			    struct v4l2_subdev_state *state,
> >  			    const struct v4l2_subdev_krouting *routing)
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index b2dbaa739afa..4d87e8ece872 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -681,11 +681,13 @@ struct v4l2_subdev_ir_ops {
> >   * @format: &struct v4l2_mbus_framefmt
> >   * @crop: &struct v4l2_rect to be used for crop
> >   * @compose: &struct v4l2_rect to be used for compose
> > + * @interval: frame interval
> >   */
> >  struct v4l2_subdev_pad_config {
> >  	struct v4l2_mbus_framefmt format;
> >  	struct v4l2_rect crop;
> >  	struct v4l2_rect compose;
> > +	struct v4l2_fract interval;
> >  };
> >  
> >  /**
> > @@ -697,6 +699,7 @@ struct v4l2_subdev_pad_config {
> >   * @fmt: &struct v4l2_mbus_framefmt
> >   * @crop: &struct v4l2_rect to be used for crop
> >   * @compose: &struct v4l2_rect to be used for compose
> > + * @interval: frame interval
> >   *
> >   * This structure stores configuration for a stream.
> >   */
> > @@ -708,6 +711,7 @@ struct v4l2_subdev_stream_config {
> >  	struct v4l2_mbus_framefmt fmt;
> >  	struct v4l2_rect crop;
> >  	struct v4l2_rect compose;
> > +	struct v4l2_fract interval;
> >  };
> >  
> >  /**
> > @@ -1494,6 +1498,24 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
> >  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> >  			struct v4l2_subdev_format *format);
> >  
> > +/**
> > + * v4l2_subdev_get_frame_interval() - Fill frame interval based on state
> > + * @sd: subdevice
> > + * @state: subdevice state
> > + * @fi: pointer to &struct v4l2_subdev_frame_interval
> > + *
> > + * Fill @fi->interval field based on the information in the @fi struct.
> > + *
> > + * This function can be used by the subdev drivers which support active state to
> > + * implement v4l2_subdev_pad_ops.get_frame_interval if the subdev driver does
> > + * not need to do anything special in their get_frame_interval op.
> > + *
> > + * Returns 0 on success, error value otherwise.
> > + */
> > +int v4l2_subdev_get_frame_interval(struct v4l2_subdev *sd,
> > +				   struct v4l2_subdev_state *state,
> > +				   struct v4l2_subdev_frame_interval *fi);
> > +
> >  /**
> >   * v4l2_subdev_set_routing() - Set given routing to subdev state
> >   * @sd: The subdevice
> > @@ -1539,6 +1561,27 @@ int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
> >  				     const struct v4l2_subdev_krouting *routing,
> >  				     const struct v4l2_mbus_framefmt *fmt);
> >  
> > +/**
> > + * v4l2_subdev_state_get_interval() - Get pointer to a stream frame interval
> > + * @state: subdevice state
> > + * @pad: pad id
> > + * @...: stream id (optional argument)
> > + *
> > + * This returns a pointer to the frame interval for the given pad + stream in
> > + * the subdev state.
> > + *
> > + * For stream-unaware drivers the frame interval for the corresponding pad is
> > + * returned. If the pad does not exist, NULL is returned.
> > + */
> > +#define v4l2_subdev_state_get_interval(state, pad, ...)			\
> > +	__v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad)	\
> > +		(state, pad, ##__VA_ARGS__)
> > +#define __v4l2_subdev_state_get_interval_pad(state, pad)	\
> > +	__v4l2_subdev_state_get_interval(state, pad, 0)
> > +struct v4l2_fract *
> > +__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
> > +				 unsigned int pad, u32 stream);
> > +
> >  /**
> >   * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
> >   * @routing: routing used to find the opposite side
> 
> Regards,
> 
> 	Hans
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 4cbe4024ff67..559d6a5082b1 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1651,6 +1651,40 @@  __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
 }
 EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose);
 
+struct v4l2_fract *
+__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
+				 unsigned int pad, u32 stream)
+{
+	struct v4l2_subdev_stream_configs *stream_configs;
+	unsigned int i;
+
+	if (WARN_ON(!state))
+		return NULL;
+
+	lockdep_assert_held(state->lock);
+
+	if (state->pads) {
+		if (stream)
+			return NULL;
+
+		if (WARN_ON(pad >= state->sd->entity.num_pads))
+			pad = 0;
+
+		return &state->pads[pad].interval;
+	}
+
+	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].interval;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_interval);
+
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 
 static int
@@ -1717,6 +1751,22 @@  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
 
+int v4l2_subdev_get_frame_interval(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state,
+				   struct v4l2_subdev_frame_interval *fi)
+{
+	struct v4l2_fract *interval;
+
+	interval = v4l2_subdev_state_get_interval(state, fi->pad, fi->stream);
+	if (!interval)
+		return -EINVAL;
+
+	fi->interval = *interval;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_interval);
+
 int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_state *state,
 			    const struct v4l2_subdev_krouting *routing)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index b2dbaa739afa..4d87e8ece872 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -681,11 +681,13 @@  struct v4l2_subdev_ir_ops {
  * @format: &struct v4l2_mbus_framefmt
  * @crop: &struct v4l2_rect to be used for crop
  * @compose: &struct v4l2_rect to be used for compose
+ * @interval: frame interval
  */
 struct v4l2_subdev_pad_config {
 	struct v4l2_mbus_framefmt format;
 	struct v4l2_rect crop;
 	struct v4l2_rect compose;
+	struct v4l2_fract interval;
 };
 
 /**
@@ -697,6 +699,7 @@  struct v4l2_subdev_pad_config {
  * @fmt: &struct v4l2_mbus_framefmt
  * @crop: &struct v4l2_rect to be used for crop
  * @compose: &struct v4l2_rect to be used for compose
+ * @interval: frame interval
  *
  * This structure stores configuration for a stream.
  */
@@ -708,6 +711,7 @@  struct v4l2_subdev_stream_config {
 	struct v4l2_mbus_framefmt fmt;
 	struct v4l2_rect crop;
 	struct v4l2_rect compose;
+	struct v4l2_fract interval;
 };
 
 /**
@@ -1494,6 +1498,24 @@  __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
 int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
 			struct v4l2_subdev_format *format);
 
+/**
+ * v4l2_subdev_get_frame_interval() - Fill frame interval based on state
+ * @sd: subdevice
+ * @state: subdevice state
+ * @fi: pointer to &struct v4l2_subdev_frame_interval
+ *
+ * Fill @fi->interval field based on the information in the @fi struct.
+ *
+ * This function can be used by the subdev drivers which support active state to
+ * implement v4l2_subdev_pad_ops.get_frame_interval if the subdev driver does
+ * not need to do anything special in their get_frame_interval op.
+ *
+ * Returns 0 on success, error value otherwise.
+ */
+int v4l2_subdev_get_frame_interval(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state,
+				   struct v4l2_subdev_frame_interval *fi);
+
 /**
  * v4l2_subdev_set_routing() - Set given routing to subdev state
  * @sd: The subdevice
@@ -1539,6 +1561,27 @@  int v4l2_subdev_set_routing_with_fmt(struct v4l2_subdev *sd,
 				     const struct v4l2_subdev_krouting *routing,
 				     const struct v4l2_mbus_framefmt *fmt);
 
+/**
+ * v4l2_subdev_state_get_interval() - Get pointer to a stream frame interval
+ * @state: subdevice state
+ * @pad: pad id
+ * @...: stream id (optional argument)
+ *
+ * This returns a pointer to the frame interval for the given pad + stream in
+ * the subdev state.
+ *
+ * For stream-unaware drivers the frame interval for the corresponding pad is
+ * returned. If the pad does not exist, NULL is returned.
+ */
+#define v4l2_subdev_state_get_interval(state, pad, ...)			\
+	__v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad)	\
+		(state, pad, ##__VA_ARGS__)
+#define __v4l2_subdev_state_get_interval_pad(state, pad)	\
+	__v4l2_subdev_state_get_interval(state, pad, 0)
+struct v4l2_fract *
+__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
+				 unsigned int pad, u32 stream);
+
 /**
  * v4l2_subdev_routing_find_opposite_end() - Find the opposite stream
  * @routing: routing used to find the opposite side