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. :-(
--
Sakari Ailus
