| Message ID | 20220727103639.581567-16-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | v4l: routing and streams support | expand |
Moi, On Wed, Jul 27, 2022 at 01:36:24PM +0300, Tomi Valkeinen wrote: > Add a helper function to set the subdev routing. The helper can be used > from subdev driver's set_routing op to store the routing table. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++++++++++++++++ > include/media/v4l2-subdev.h | 16 ++++++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index befd00ebf381..0b841cf74c74 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1176,6 +1176,33 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, > } > EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt); > > +int v4l2_subdev_set_routing(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_krouting *routing) > +{ > + struct v4l2_subdev_krouting *dst = &state->routing; > + const struct v4l2_subdev_krouting *src = routing; > + > + lockdep_assert_held(state->lock); > + > + kfree(dst->routes); > + dst->routes = NULL; > + dst->num_routes = 0; Shouldn't you keep the old routing around until memory allocation (and anything else that can fail) has succeeded? > + > + if (src->num_routes > 0) { > + dst->routes = kmemdup(src->routes, > + src->num_routes * sizeof(*src->routes), > + GFP_KERNEL); > + if (!dst->routes) > + return -ENOMEM; > + > + dst->num_routes = src->num_routes; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing); > + > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > #endif /* CONFIG_MEDIA_CONTROLLER */ > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index decd1e1c3d39..37081a2e6697 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1407,6 +1407,22 @@ 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_set_routing() - Set given routing to subdev state > + * @sd: The subdevice > + * @state: The subdevice state > + * @routing: Routing that will be copied to subdev state > + * > + * This will release old routing table (if any) from the state, allocate > + * enough space for the given routing, and copy the routing. > + * > + * This can be used from the subdev driver's set_routing op, after validating > + * the routing. > + */ > +int v4l2_subdev_set_routing(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_krouting *routing); > + > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > #endif /* CONFIG_MEDIA_CONTROLLER */
On 01/08/2022 09:59, Sakari Ailus wrote: > Moi, > > On Wed, Jul 27, 2022 at 01:36:24PM +0300, Tomi Valkeinen wrote: >> Add a helper function to set the subdev routing. The helper can be used >> from subdev driver's set_routing op to store the routing table. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> --- >> drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++++++++++++++++ >> include/media/v4l2-subdev.h | 16 ++++++++++++++++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >> index befd00ebf381..0b841cf74c74 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -1176,6 +1176,33 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, >> } >> EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt); >> >> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd, >> + struct v4l2_subdev_state *state, >> + struct v4l2_subdev_krouting *routing) >> +{ >> + struct v4l2_subdev_krouting *dst = &state->routing; >> + const struct v4l2_subdev_krouting *src = routing; >> + >> + lockdep_assert_held(state->lock); >> + >> + kfree(dst->routes); >> + dst->routes = NULL; >> + dst->num_routes = 0; > > Shouldn't you keep the old routing around until memory allocation (and > anything else that can fail) has succeeded? Indeed, good catch! I think v4l2_subdev_init_stream_configs() also needs improving. Tomi
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index befd00ebf381..0b841cf74c74 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1176,6 +1176,33 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, } EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt); +int v4l2_subdev_set_routing(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, + struct v4l2_subdev_krouting *routing) +{ + struct v4l2_subdev_krouting *dst = &state->routing; + const struct v4l2_subdev_krouting *src = routing; + + lockdep_assert_held(state->lock); + + kfree(dst->routes); + dst->routes = NULL; + dst->num_routes = 0; + + if (src->num_routes > 0) { + dst->routes = kmemdup(src->routes, + src->num_routes * sizeof(*src->routes), + GFP_KERNEL); + if (!dst->routes) + return -ENOMEM; + + dst->num_routes = src->num_routes; + } + + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing); + #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ #endif /* CONFIG_MEDIA_CONTROLLER */ diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index decd1e1c3d39..37081a2e6697 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1407,6 +1407,22 @@ 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_set_routing() - Set given routing to subdev state + * @sd: The subdevice + * @state: The subdevice state + * @routing: Routing that will be copied to subdev state + * + * This will release old routing table (if any) from the state, allocate + * enough space for the given routing, and copy the routing. + * + * This can be used from the subdev driver's set_routing op, after validating + * the routing. + */ +int v4l2_subdev_set_routing(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, + struct v4l2_subdev_krouting *routing); + #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ #endif /* CONFIG_MEDIA_CONTROLLER */