On 04/09/2023 09:35, Sakari Ailus wrote:
On Mon, Sep 04, 2023 at 08:33:37AM +0300, Tomi Valkeinen wrote:
On 02/09/2023 15:10, Sakari Ailus wrote:
Hi Laurent,
On Mon, Aug 21, 2023 at 01:56:04AM +0300, Laurent Pinchart wrote:
Hi Sakari,
On Fri, Aug 18, 2023 at 04:18:41PM +0000, Sakari Ailus wrote:
On Fri, Aug 18, 2023 at 06:55:18PM +0300, Laurent Pinchart wrote:
Routing support, through the subdev .set_routing() operation, requires
the subdev to support streams. This is however not clearly documented
anywhere. Fix it by expanding the operation's documentation to indicate
that subdevs must set the V4L2_SUBDEV_FL_STREAMS flag.
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
include/media/v4l2-subdev.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index b325df0d54d6..0b04ed1994b6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -822,8 +822,9 @@ struct v4l2_subdev_state {
* operation shall fail if the pad index it has been called on
* is not valid or in case of unrecoverable failures.
*
- * @set_routing: enable or disable data connection routes described in the
- * subdevice routing table.
+ * @set_routing: Enable or disable data connection routes described in the
+ * subdevice routing table. Subdevs that implement this operation
+ * must set the V4L2_SUBDEV_FL_STREAMS flag.
Could we set the flag in the core when this op exists for a sub-device?
That won't work in all cases, as a driver could expose immutable routes
by creating them in the .init_cfg() function, without implementing
.set_routing().
Another option would be to check if the drivers has created routes after
the .init_cfg() called (indirectly) from v4l2_subdev_init_finalize(). It
may be a bit fragile though.
If you have either, then the sub-device does support streams. As otherwise,
there are no streams exposed to the user space. Right?
We need to know the existence of V4L2_SUBDEV_FL_STREAMS flag before calling
init_cfg, in __v4l2_subdev_state_alloc.
Good point. Still, it'd be great to get rid of such flags. They are
incorrectly set in so many places. :-(
Hmm, well, if we want, we could return an error at the end of
__v4l2_subdev_init_finalize, if V4L2_SUBDEV_FL_STREAMS is set, but there
is no .set_routing and .init_cfg did not set up routing, OR if
V4L2_SUBDEV_FL_STREAMS is not set, but .set_routing exists. We could
also check for V4L2_SUBDEV_FL_STREAMS in v4l2_subdev_set_routing, and
return an error if it's not set.
I think if a subdev erroneously does not set V4L2_SUBDEV_FL_STREAMS, the
driver won't get very far. So it's not an issue that can go hidden for
very long.
Tomi
