| Message ID | 20240416193319.778192-17-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
Hi Sakari, Thank you for the patch. On Tue, Apr 16, 2024 at 10:32:49PM +0300, 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 | 7 ++----- > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- > 2 files changed, 13 insertions(+), 6 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 27eb4c82a0e1..88df7bf80b00 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > @@ -44,7 +44,8 @@ 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`. > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. :c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, ``VIDIOC_SUBDEV_S_ROUTING`` also returns the routes back to the user. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. > This means that the userspace must reconfigure all stream formats and selections > @@ -157,10 +158,6 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set > appropriately. The generic error codes are described at the > :ref:`Generic Error Codes <gen-errors>` chapter. > > -ENOSPC > - The application provided ``num_routes`` is not big enough to contain > - all the available routes the subdevice exposes. > - > EINVAL > The sink or source pad identifiers reference a non-existing pad or reference > pads of different types (ie. the sink_pad identifiers refers to a source > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 904378007a79..36b2f78cd551 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -959,8 +959,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, > + min(state->routing.num_routes, routing->len_routes) * > + sizeof(*state->routing.routes)); > + routing->num_routes = state->routing.num_routes; > + > + return 0; > } > > case VIDIOC_SUBDEV_G_ROUTING: {
Hi Laurent, On Sat, Apr 20, 2024 at 01:55:52AM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Tue, Apr 16, 2024 at 10:32:49PM +0300, 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 | 7 ++----- > > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- > > 2 files changed, 13 insertions(+), 6 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 27eb4c82a0e1..88df7bf80b00 100644 > > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > @@ -44,7 +44,8 @@ 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`. > > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also > > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. > > :c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, > ``VIDIOC_SUBDEV_S_ROUTING`` also returns the routes back to the user. The intention here is to say that S_ROUTING returns the routes back to the user, just as G_ROUTING does. Moving "also" after S_ROUTING makes the meaning of the sentence unclear. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! > > > > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. > > This means that the userspace must reconfigure all stream formats and selections > > @@ -157,10 +158,6 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set > > appropriately. The generic error codes are described at the > > :ref:`Generic Error Codes <gen-errors>` chapter. > > > > -ENOSPC > > - The application provided ``num_routes`` is not big enough to contain > > - all the available routes the subdevice exposes. > > - > > EINVAL > > The sink or source pad identifiers reference a non-existing pad or reference > > pads of different types (ie. the sink_pad identifiers refers to a source > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > index 904378007a79..36b2f78cd551 100644 > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > @@ -959,8 +959,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, > > + min(state->routing.num_routes, routing->len_routes) * > > + sizeof(*state->routing.routes)); > > + routing->num_routes = state->routing.num_routes; > > + > > + return 0; > > } > > > > case VIDIOC_SUBDEV_G_ROUTING: { >
On Tue, Apr 23, 2024 at 10:49:11AM +0000, Sakari Ailus wrote: > On Sat, Apr 20, 2024 at 01:55:52AM +0300, Laurent Pinchart wrote: > > On Tue, Apr 16, 2024 at 10:32:49PM +0300, 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 | 7 ++----- > > > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- > > > 2 files changed, 13 insertions(+), 6 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 27eb4c82a0e1..88df7bf80b00 100644 > > > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst > > > @@ -44,7 +44,8 @@ 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`. > > > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also > > > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user. > > > > :c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, > > ``VIDIOC_SUBDEV_S_ROUTING`` also returns the routes back to the user. > > The intention here is to say that S_ROUTING returns the routes back to the > user, just as G_ROUTING does. Moving "also" after S_ROUTING makes the > meaning of the sentence unclear. Keeping it before makes it sound non-native English to me :-) But I'm not a native speaker. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks! > > > > > > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. > > > This means that the userspace must reconfigure all stream formats and selections > > > @@ -157,10 +158,6 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set > > > appropriately. The generic error codes are described at the > > > :ref:`Generic Error Codes <gen-errors>` chapter. > > > > > > -ENOSPC > > > - The application provided ``num_routes`` is not big enough to contain > > > - all the available routes the subdevice exposes. > > > - > > > EINVAL > > > The sink or source pad identifiers reference a non-existing pad or reference > > > pads of different types (ie. the sink_pad identifiers refers to a source > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > > index 904378007a79..36b2f78cd551 100644 > > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > > @@ -959,8 +959,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, > > > + min(state->routing.num_routes, routing->len_routes) * > > > + sizeof(*state->routing.routes)); > > > + routing->num_routes = state->routing.num_routes; > > > + > > > + return 0; > > > } > > > > > > case VIDIOC_SUBDEV_G_ROUTING: {
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 27eb4c82a0e1..88df7bf80b00 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst @@ -44,7 +44,8 @@ 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`. +: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 stream formats and selections @@ -157,10 +158,6 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set appropriately. The generic error codes are described at the :ref:`Generic Error Codes <gen-errors>` chapter. -ENOSPC - The application provided ``num_routes`` is not big enough to contain - all the available routes the subdevice exposes. - EINVAL The sink or source pad identifiers reference a non-existing pad or reference pads of different types (ie. the sink_pad identifiers refers to a source diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 904378007a79..36b2f78cd551 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -959,8 +959,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, + min(state->routing.num_routes, routing->len_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 | 7 ++----- drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++- 2 files changed, 13 insertions(+), 6 deletions(-)