Re: [PATCH] media: v4l2-subdev: Document that routing support depends on streams

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux