| Message ID | 20231024005130.28026-4-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: v4l2-subdev: Improve frame interval handling | expand |
On 24/10/2023 03:51, 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> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 44 +++++++++++++++++++++ > include/media/v4l2-subdev.h | 57 +++++++++++++++++++++++++++ > 2 files changed, 101 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index c45d60a87701..518f2f35c68d 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1618,6 +1618,29 @@ 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; > + > + if (sd->flags & V4L2_SUBDEV_FL_STREAMS) > + interval = v4l2_subdev_state_get_stream_interval(state, fi->pad, > + fi->stream); > + else if (fi->pad < sd->entity.num_pads && fi->stream == 0) > + interval = v4l2_subdev_get_pad_interval(sd, state, fi->pad); > + else > + interval = NULL; > + > + 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) > @@ -1761,6 +1784,27 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > } > EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); > > +struct v4l2_fract * > +v4l2_subdev_state_get_stream_interval(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > +{ > + struct v4l2_subdev_stream_configs *stream_configs; > + unsigned int i; > + > + 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].interval; > + } > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_interval); > + > 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 962b546d0e3b..363f9a8a084c 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; > }; > > /** > @@ -1199,6 +1203,26 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, > return &state->pads[pad].compose; > } > > +/** > + * v4l2_subdev_get_pad_interval - ancillary routine to get > + * &struct v4l2_subdev_pad_config->interval > + * > + * @sd: pointer to &struct v4l2_subdev > + * @state: pointer to &struct v4l2_subdev_state. > + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. > + */ > +static inline struct v4l2_fract * > +v4l2_subdev_get_pad_interval(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + unsigned int pad) > +{ > + if (WARN_ON(!state)) > + return NULL; > + if (WARN_ON(pad >= sd->entity.num_pads)) > + pad = 0; > + return &state->pads[pad].interval; > +} > + > /* > * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been > * renamed > @@ -1489,6 +1513,24 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) > 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. What would the "something special" be? Can we instead just say that the framework will return the frame interval from the state for drivers that support active state (similar to get-routing)? Or perhaps that's not a wise thing to do for old operations... Tomi
Hi Tomi, On Tue, Oct 24, 2023 at 10:21:18AM +0300, Tomi Valkeinen wrote: > On 24/10/2023 03:51, 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> > > --- > > drivers/media/v4l2-core/v4l2-subdev.c | 44 +++++++++++++++++++++ > > include/media/v4l2-subdev.h | 57 +++++++++++++++++++++++++++ > > 2 files changed, 101 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > index c45d60a87701..518f2f35c68d 100644 > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > @@ -1618,6 +1618,29 @@ 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; > > + > > + if (sd->flags & V4L2_SUBDEV_FL_STREAMS) > > + interval = v4l2_subdev_state_get_stream_interval(state, fi->pad, > > + fi->stream); > > + else if (fi->pad < sd->entity.num_pads && fi->stream == 0) > > + interval = v4l2_subdev_get_pad_interval(sd, state, fi->pad); > > + else > > + interval = NULL; > > + > > + 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) > > @@ -1761,6 +1784,27 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > > } > > EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); > > > > +struct v4l2_fract * > > +v4l2_subdev_state_get_stream_interval(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream) > > +{ > > + struct v4l2_subdev_stream_configs *stream_configs; > > + unsigned int i; > > + > > + 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].interval; > > + } > > + > > + return NULL; > > +} > > +EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_interval); > > + > > 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 962b546d0e3b..363f9a8a084c 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; > > }; > > > > /** > > @@ -1199,6 +1203,26 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, > > return &state->pads[pad].compose; > > } > > > > +/** > > + * v4l2_subdev_get_pad_interval - ancillary routine to get > > + * &struct v4l2_subdev_pad_config->interval > > + * > > + * @sd: pointer to &struct v4l2_subdev > > + * @state: pointer to &struct v4l2_subdev_state. > > + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. > > + */ > > +static inline struct v4l2_fract * > > +v4l2_subdev_get_pad_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state, > > + unsigned int pad) > > +{ > > + if (WARN_ON(!state)) > > + return NULL; > > + if (WARN_ON(pad >= sd->entity.num_pads)) > > + pad = 0; > > + return &state->pads[pad].interval; > > +} > > + > > /* > > * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been > > * renamed > > @@ -1489,6 +1513,24 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) > > 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. > > What would the "something special" be? Can we instead just say that the > framework will return the frame interval from the state for drivers that > support active state (similar to get-routing)? Or perhaps that's not a > wise thing to do for old operations... I've copied that text from v4l2_subdev_get_fmt(). I'm fine changing it, if both say the same. If you send a patch to improve the documentation of v4l2_subdev_get_fmt(), I'll update this patch :-)
On 24/10/2023 10:38, Laurent Pinchart wrote: > Hi Tomi, > > On Tue, Oct 24, 2023 at 10:21:18AM +0300, Tomi Valkeinen wrote: >> On 24/10/2023 03:51, 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> >>> --- >>> drivers/media/v4l2-core/v4l2-subdev.c | 44 +++++++++++++++++++++ >>> include/media/v4l2-subdev.h | 57 +++++++++++++++++++++++++++ >>> 2 files changed, 101 insertions(+) >>> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>> index c45d60a87701..518f2f35c68d 100644 >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>> @@ -1618,6 +1618,29 @@ 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; >>> + >>> + if (sd->flags & V4L2_SUBDEV_FL_STREAMS) >>> + interval = v4l2_subdev_state_get_stream_interval(state, fi->pad, >>> + fi->stream); >>> + else if (fi->pad < sd->entity.num_pads && fi->stream == 0) >>> + interval = v4l2_subdev_get_pad_interval(sd, state, fi->pad); >>> + else >>> + interval = NULL; >>> + >>> + 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) >>> @@ -1761,6 +1784,27 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, >>> } >>> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); >>> >>> +struct v4l2_fract * >>> +v4l2_subdev_state_get_stream_interval(struct v4l2_subdev_state *state, >>> + unsigned int pad, u32 stream) >>> +{ >>> + struct v4l2_subdev_stream_configs *stream_configs; >>> + unsigned int i; >>> + >>> + 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].interval; >>> + } >>> + >>> + return NULL; >>> +} >>> +EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_interval); >>> + >>> 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 962b546d0e3b..363f9a8a084c 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; >>> }; >>> >>> /** >>> @@ -1199,6 +1203,26 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, >>> return &state->pads[pad].compose; >>> } >>> >>> +/** >>> + * v4l2_subdev_get_pad_interval - ancillary routine to get >>> + * &struct v4l2_subdev_pad_config->interval >>> + * >>> + * @sd: pointer to &struct v4l2_subdev >>> + * @state: pointer to &struct v4l2_subdev_state. >>> + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. >>> + */ >>> +static inline struct v4l2_fract * >>> +v4l2_subdev_get_pad_interval(struct v4l2_subdev *sd, >>> + struct v4l2_subdev_state *state, >>> + unsigned int pad) >>> +{ >>> + if (WARN_ON(!state)) >>> + return NULL; >>> + if (WARN_ON(pad >= sd->entity.num_pads)) >>> + pad = 0; >>> + return &state->pads[pad].interval; >>> +} >>> + >>> /* >>> * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been >>> * renamed >>> @@ -1489,6 +1513,24 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) >>> 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. >> >> What would the "something special" be? Can we instead just say that the >> framework will return the frame interval from the state for drivers that >> support active state (similar to get-routing)? Or perhaps that's not a >> wise thing to do for old operations... > > I've copied that text from v4l2_subdev_get_fmt(). I'm fine changing it, > if both say the same. If you send a patch to improve the documentation > of v4l2_subdev_get_fmt(), I'll update this patch :-) My point wasn't the docs, but changing the framework to return the frame interval directly from the state. Anyway, I was mostly just thinking out loud. Not part of this series anyway. Tomi
Hi Tomi, On Tue, Oct 24, 2023 at 10:40:29AM +0300, Tomi Valkeinen wrote: > On 24/10/2023 10:38, Laurent Pinchart wrote: > > On Tue, Oct 24, 2023 at 10:21:18AM +0300, Tomi Valkeinen wrote: > >> On 24/10/2023 03:51, 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> > >>> --- > >>> drivers/media/v4l2-core/v4l2-subdev.c | 44 +++++++++++++++++++++ > >>> include/media/v4l2-subdev.h | 57 +++++++++++++++++++++++++++ > >>> 2 files changed, 101 insertions(+) > >>> > >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > >>> index c45d60a87701..518f2f35c68d 100644 > >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c > >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > >>> @@ -1618,6 +1618,29 @@ 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; > >>> + > >>> + if (sd->flags & V4L2_SUBDEV_FL_STREAMS) > >>> + interval = v4l2_subdev_state_get_stream_interval(state, fi->pad, > >>> + fi->stream); > >>> + else if (fi->pad < sd->entity.num_pads && fi->stream == 0) > >>> + interval = v4l2_subdev_get_pad_interval(sd, state, fi->pad); > >>> + else > >>> + interval = NULL; > >>> + > >>> + 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) > >>> @@ -1761,6 +1784,27 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, > >>> } > >>> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); > >>> > >>> +struct v4l2_fract * > >>> +v4l2_subdev_state_get_stream_interval(struct v4l2_subdev_state *state, > >>> + unsigned int pad, u32 stream) > >>> +{ > >>> + struct v4l2_subdev_stream_configs *stream_configs; > >>> + unsigned int i; > >>> + > >>> + 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].interval; > >>> + } > >>> + > >>> + return NULL; > >>> +} > >>> +EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_interval); > >>> + > >>> 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 962b546d0e3b..363f9a8a084c 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; > >>> }; > >>> > >>> /** > >>> @@ -1199,6 +1203,26 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, > >>> return &state->pads[pad].compose; > >>> } > >>> > >>> +/** > >>> + * v4l2_subdev_get_pad_interval - ancillary routine to get > >>> + * &struct v4l2_subdev_pad_config->interval > >>> + * > >>> + * @sd: pointer to &struct v4l2_subdev > >>> + * @state: pointer to &struct v4l2_subdev_state. > >>> + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. > >>> + */ > >>> +static inline struct v4l2_fract * > >>> +v4l2_subdev_get_pad_interval(struct v4l2_subdev *sd, > >>> + struct v4l2_subdev_state *state, > >>> + unsigned int pad) > >>> +{ > >>> + if (WARN_ON(!state)) > >>> + return NULL; > >>> + if (WARN_ON(pad >= sd->entity.num_pads)) > >>> + pad = 0; > >>> + return &state->pads[pad].interval; > >>> +} > >>> + > >>> /* > >>> * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been > >>> * renamed > >>> @@ -1489,6 +1513,24 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) > >>> 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. > >> > >> What would the "something special" be? Can we instead just say that the > >> framework will return the frame interval from the state for drivers that > >> support active state (similar to get-routing)? Or perhaps that's not a > >> wise thing to do for old operations... > > > > I've copied that text from v4l2_subdev_get_fmt(). I'm fine changing it, > > if both say the same. If you send a patch to improve the documentation > > of v4l2_subdev_get_fmt(), I'll update this patch :-) > > My point wasn't the docs, but changing the framework to return the frame > interval directly from the state. We don't do that for the .get_fmt() operation. I'm also fine considering this, but I'd like to address both operations together, for consistency. > Anyway, I was mostly just thinking out loud. Not part of this series anyway.
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index c45d60a87701..518f2f35c68d 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1618,6 +1618,29 @@ 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; + + if (sd->flags & V4L2_SUBDEV_FL_STREAMS) + interval = v4l2_subdev_state_get_stream_interval(state, fi->pad, + fi->stream); + else if (fi->pad < sd->entity.num_pads && fi->stream == 0) + interval = v4l2_subdev_get_pad_interval(sd, state, fi->pad); + else + interval = NULL; + + 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) @@ -1761,6 +1784,27 @@ v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, } EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_compose); +struct v4l2_fract * +v4l2_subdev_state_get_stream_interval(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream) +{ + struct v4l2_subdev_stream_configs *stream_configs; + unsigned int i; + + 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].interval; + } + + return NULL; +} +EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_stream_interval); + 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 962b546d0e3b..363f9a8a084c 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; }; /** @@ -1199,6 +1203,26 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd, return &state->pads[pad].compose; } +/** + * v4l2_subdev_get_pad_interval - ancillary routine to get + * &struct v4l2_subdev_pad_config->interval + * + * @sd: pointer to &struct v4l2_subdev + * @state: pointer to &struct v4l2_subdev_state. + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. + */ +static inline struct v4l2_fract * +v4l2_subdev_get_pad_interval(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, + unsigned int pad) +{ + if (WARN_ON(!state)) + return NULL; + if (WARN_ON(pad >= sd->entity.num_pads)) + pad = 0; + return &state->pads[pad].interval; +} + /* * Temprary helpers until uses of v4l2_subdev_get_try_* functions have been * renamed @@ -1489,6 +1513,24 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd) 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 @@ -1580,6 +1622,21 @@ struct v4l2_rect * v4l2_subdev_state_get_stream_compose(struct v4l2_subdev_state *state, unsigned int pad, u32 stream); +/** + * v4l2_subdev_state_get_stream_interval() - Get pointer to a stream frame interval + * @state: subdevice state + * @pad: pad id + * @stream: stream id + * + * This returns a pointer to the frame interval for the given pad + stream in + * the subdev state. + * + * If the state does not contain the given pad + stream, NULL is returned. + */ +struct v4l2_fract * +v4l2_subdev_state_get_stream_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
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> --- drivers/media/v4l2-core/v4l2-subdev.c | 44 +++++++++++++++++++++ include/media/v4l2-subdev.h | 57 +++++++++++++++++++++++++++ 2 files changed, 101 insertions(+)