| Message ID | 20230731-upstream_csi-v8-1-fb7d3661c2c9@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | CSI2RX support on J721E and AM62 | expand |
On 31/07/2023 11:29, Jai Luthra wrote: > For link validation on video device drivers, it may be required to > match the formats set on the source subdev with the formats set on the > video device. > > Export the existing v4l2_subdev_link_validate_get_format() helper so it > can be reused by such drivers. > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Jai Luthra <j-luthra@ti.com> > --- > New in v8 > > drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++---- > include/media/v4l2-subdev.h | 12 ++++++++++++ > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 217b8019fb9b..0d3b5ff5cacc 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1130,10 +1130,9 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, > } > EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default); > > -static int > -v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, > - struct v4l2_subdev_format *fmt, > - bool states_locked) > +int v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, > + struct v4l2_subdev_format *fmt, > + bool states_locked) > { > struct v4l2_subdev_state *state; > struct v4l2_subdev *sd; > @@ -1165,6 +1164,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, > > return ret; > } > +EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_get_format); > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index a012741cc876..ef7007f46889 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1301,6 +1301,18 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, > struct v4l2_subdev_format *source_fmt, > struct v4l2_subdev_format *sink_fmt); > > +/** > + * v4l2_subdev_link_validate_get_format - get format for media link validation > + * > + * @pad: pad id > + * @stream: stream id > + * @fmt: pointer to &struct v4l2_subdev_format > + * @states_locked: is the subdev state already locked > + */ > +int v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, > + struct v4l2_subdev_format *fmt, > + bool states_locked); > + > /** > * v4l2_subdev_link_validate - validates a media link > * > I don't know about this one... The v4l2_subdev_link_validate_get_format() is a bit of an internal function, especially with the relatively-new "states_locked" parameter. If it's made public, at least the "states_locked" should be renamed to "state_locked" (well, it could perhaps be renamed anyway). But just looking at the function doc above, I think the developer's question would be "what does this do?". What does "get format for media link validation" mean? Is it similar to v4l2_subdev_state_get_stream_format() or v4l2_subdev_get_pad_format()? What does states_locked do? How do you use this with multi-stream support? Do you lock the source, then validate all streams (with the help of this function), and then unlock? If yes, then all the states_locked stuff is extra, and all the function really does is v4l2_subdev_call(sd, pad, get_fmt, state, fmt). And if so, would it be clearer to just do that, instead of hiding it behind v4l2_subdev_link_validate_get_format()? Tomi
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 217b8019fb9b..0d3b5ff5cacc 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1130,10 +1130,9 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, } EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default); -static int -v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, - struct v4l2_subdev_format *fmt, - bool states_locked) +int v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, + struct v4l2_subdev_format *fmt, + bool states_locked) { struct v4l2_subdev_state *state; struct v4l2_subdev *sd; @@ -1165,6 +1164,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, return ret; } +EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_get_format); #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index a012741cc876..ef7007f46889 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1301,6 +1301,18 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, struct v4l2_subdev_format *source_fmt, struct v4l2_subdev_format *sink_fmt); +/** + * v4l2_subdev_link_validate_get_format - get format for media link validation + * + * @pad: pad id + * @stream: stream id + * @fmt: pointer to &struct v4l2_subdev_format + * @states_locked: is the subdev state already locked + */ +int v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream, + struct v4l2_subdev_format *fmt, + bool states_locked); + /** * v4l2_subdev_link_validate - validates a media link *
For link validation on video device drivers, it may be required to match the formats set on the source subdev with the formats set on the video device. Export the existing v4l2_subdev_link_validate_get_format() helper so it can be reused by such drivers. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jai Luthra <j-luthra@ti.com> --- New in v8 drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++---- include/media/v4l2-subdev.h | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-)