On 06/04/2022 16:51, Hans Verkuil wrote:
On 24/03/2022 09:00, Tomi Valkeinen wrote:
Add v4l2_subdev_get_fmt() helper function which implements
v4l2_subdev_pad_ops.get_fmt using active state. Subdev drivers that
support active state and do not need to do anything special in their
get_fmt op can use this helper directly for v4l2_subdev_pad_ops.get_fmt.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++++++++++++++
include/media/v4l2-subdev.h | 17 +++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index d8d1c9ef4dc4..cbc5ebff0656 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1029,6 +1029,24 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd)
}
EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup);
+int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format)
+{
+ struct v4l2_mbus_framefmt *fmt;
+
+ if (format->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ fmt = v4l2_subdev_get_try_format(sd, state, format->pad);
Let's use the new v4l2_subdev_get_pad_format helper here.
Right.
+ if (!fmt)
+ return -EINVAL;
+
+ format->format = *fmt;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
+
#endif /* CONFIG_MEDIA_CONTROLLER */
void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 700ce376b22c..491bdbb1670c 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1300,6 +1300,23 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
return sd->active_state;
}
+/**
+ * v4l2_subdev_get_fmt() - Fill format based on state
+ * @sd: subdevice
+ * @state: subdevice state
+ * @format: pointer to &struct v4l2_subdev_format
+ *
+ * Fill @format->format field based on the information in the @format struct.
+ *
+ * This function can be used by the subdev drivers which support active state to
+ * implement v4l2_subdev_pad_ops.get_fmt if the subdev driver does not need to
+ * do anything special in their get_fmt op.
+ *
+ * Returns 0 on success, error value otherwise.
+ */
+int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format);
My main concern here is the function name: perhaps a prefix like
v4l2_subdev_pad_op_get_fmt (or perhaps just _op_ without 'pad') makes
it easier to see that this is a pad op helper.
The function can and will be used also in other places. E.g. driver's
set_fmt may use it to allow only setting the sink pad:
if (format->pad == MY_SOURCE_PAD)
return v4l2_subdev_get_fmt(sd, state, format);
That's perhaps the only other use for the function, as it takes struct
v4l2_subdev_format as a parameter, and I think usually that struct is
only used with the set_fmt/get_fmt pad ops.
So, I guess "v4l2_subdev_pad_op_get_fmt" is suitable even in the example
use case above. Then again, we do have other similar helper funcs
without any extra prefixes, e.g. media_entity_operations:
.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
.link_validate = v4l2_subdev_link_validate,
.has_route = v4l2_subdev_has_route
Tomi
