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

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

 



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 could do similarly for events when the sub-device has a control handler.
> 
> A subdev could generate non-control events too. In most cases I suppose
> it would still create a control handler, but I don't think we should
> require that.

I suggested this mainly as there are tonnes of drivers that expose controls
(and thus control events) but do not have the events flag set. Doing this
automatically for almost all drivers would reduce bugs.

> 
> > The device node should probably exist in almost all cases, but I'm not sure
> > right now whether there is a reasonable test for it.

-- 
Regards,

Sakari Ailus



[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