| Message ID | 20240313072516.241106-16-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
Hi Sakari, Thank you for the patch. On Wed, Mar 13, 2024 at 09:24:53AM +0200, Sakari Ailus wrote: > Return the routes set using S_ROUTING back to the user. Also reflect this > in documentation. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > .../media/v4l/vidioc-subdev-g-routing.rst | 5 +++-- > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > index 8f9aa83275c5..6eb6a59570dc 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > @@ -43,8 +43,9 @@ The routing configuration determines the flows of data inside an entity. > Drivers report their current routing tables using the > ``VIDIOC_SUBDEV_G_ROUTING`` ioctl and application may enable or disable routes > with the ``VIDIOC_SUBDEV_S_ROUTING`` ioctl, by adding or removing routes and > -setting or clearing flags of the ``flags`` field of a > -struct :c:type:`v4l2_subdev_route`. > +setting or clearing flags of the ``flags`` field of a struct > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is > called. This means that the userspace must reconfigure all streams after calling > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 486a5c3016de..a357ce318192 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -957,8 +957,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, > krouting.len_routes = routing->len_routes; > krouting.routes = routes; > > - return v4l2_subdev_call(sd, pad, set_routing, state, > + rval = v4l2_subdev_call(sd, pad, set_routing, state, > routing->which, &krouting); > + if (rval < 0) > + return rval; > + > + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, > + state->routing.routes, > + state->routing.num_routes * I think this should be capped at len_routes. > + sizeof(*state->routing.routes)); > + routing->num_routes = state->routing.num_routes; > + > + return 0; > } > > case VIDIOC_SUBDEV_G_ROUTING: { > -- > 2.39.2 >
On Wed, Mar 20, 2024 at 03:45:14AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Wed, Mar 13, 2024 at 09:24:53AM +0200, Sakari Ailus wrote: > > Return the routes set using S_ROUTING back to the user. Also reflect this > > in documentation. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > .../media/v4l/vidioc-subdev-g-routing.rst | 5 +++-- > > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- > > 2 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > index 8f9aa83275c5..6eb6a59570dc 100644 > > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > @@ -43,8 +43,9 @@ The routing configuration determines the flows of data inside an entity. > > Drivers report their current routing tables using the > > ``VIDIOC_SUBDEV_G_ROUTING`` ioctl and application may enable or disable routes > > with the ``VIDIOC_SUBDEV_S_ROUTING`` ioctl, by adding or removing routes and > > -setting or clearing flags of the ``flags`` field of a > > -struct :c:type:`v4l2_subdev_route`. > > +setting or clearing flags of the ``flags`` field of a struct > > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also > > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. > > > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is > > called. This means that the userspace must reconfigure all streams after calling > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > index 486a5c3016de..a357ce318192 100644 > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > @@ -957,8 +957,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, > > krouting.len_routes = routing->len_routes; > > krouting.routes = routes; > > > > - return v4l2_subdev_call(sd, pad, set_routing, state, > > + rval = v4l2_subdev_call(sd, pad, set_routing, state, > > routing->which, &krouting); > > + if (rval < 0) > > + return rval; > > + > > + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, > > + state->routing.routes, > > + state->routing.num_routes * > > I think this should be capped at len_routes. Good catch! > > > + sizeof(*state->routing.routes)); > > + routing->num_routes = state->routing.num_routes; > > + > > + return 0; > > } > > > > case VIDIOC_SUBDEV_G_ROUTING: { > > -- > > 2.39.2 > > > > -- > Regards, > > Laurent Pinchart
diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst index 8f9aa83275c5..6eb6a59570dc 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst @@ -43,8 +43,9 @@ The routing configuration determines the flows of data inside an entity. Drivers report their current routing tables using the ``VIDIOC_SUBDEV_G_ROUTING`` ioctl and application may enable or disable routes with the ``VIDIOC_SUBDEV_S_ROUTING`` ioctl, by adding or removing routes and -setting or clearing flags of the ``flags`` field of a -struct :c:type:`v4l2_subdev_route`. +setting or clearing flags of the ``flags`` field of a struct +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. This means that the userspace must reconfigure all streams after calling diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 486a5c3016de..a357ce318192 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -957,8 +957,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, krouting.len_routes = routing->len_routes; krouting.routes = routes; - return v4l2_subdev_call(sd, pad, set_routing, state, + rval = v4l2_subdev_call(sd, pad, set_routing, state, routing->which, &krouting); + if (rval < 0) + return rval; + + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, + state->routing.routes, + state->routing.num_routes * + sizeof(*state->routing.routes)); + routing->num_routes = state->routing.num_routes; + + return 0; } case VIDIOC_SUBDEV_G_ROUTING: {
Return the routes set using S_ROUTING back to the user. Also reflect this in documentation. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- .../media/v4l/vidioc-subdev-g-routing.rst | 5 +++-- drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-)