From 009e12561369e2fcb992af47001a06f016e9d6b5 Mon Sep 17 00:00:00 2001 From: Paweł Anikiel Date: Fri, 5 Apr 2024 14:13:56 +0000 Subject: media: v4l2-subdev: Add pad versions of dv timing subdev calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, subdev dv timing calls (i.e. g/s/query_dv_timings) are video ops without a pad argument. This is a problem if the subdevice can have different dv timings for each pad (e.g. a DisplayPort receiver with multiple virtual channels). To solve this, change these calls to include a pad argument, and put them into pad ops. Keep the old ones temporarily to make the switch easier. Signed-off-by: Paweł Anikiel Signed-off-by: Hans Verkuil --- include/media/v4l2-subdev.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'include/media/v4l2-subdev.h') diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index a9e6b8146279..a5213411ef2b 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -791,6 +791,14 @@ struct v4l2_subdev_state { * * @set_edid: callback for VIDIOC_SUBDEV_S_EDID() ioctl handler code. * + * @s_dv_timings: Set custom dv timings in the sub device. This is used + * when sub device is capable of setting detailed timing information + * in the hardware to generate/detect the video signal. + * + * @g_dv_timings: Get custom dv timings in the sub device. + * + * @query_dv_timings: callback for VIDIOC_QUERY_DV_TIMINGS() ioctl handler code. + * * @dv_timings_cap: callback for VIDIOC_SUBDEV_DV_TIMINGS_CAP() ioctl handler * code. * @@ -864,6 +872,12 @@ struct v4l2_subdev_pad_ops { struct v4l2_subdev_frame_interval *interval); int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_edid *edid); int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_edid *edid); + int (*s_dv_timings)(struct v4l2_subdev *sd, unsigned int pad, + struct v4l2_dv_timings *timings); + int (*g_dv_timings)(struct v4l2_subdev *sd, unsigned int pad, + struct v4l2_dv_timings *timings); + int (*query_dv_timings)(struct v4l2_subdev *sd, unsigned int pad, + struct v4l2_dv_timings *timings); int (*dv_timings_cap)(struct v4l2_subdev *sd, struct v4l2_dv_timings_cap *cap); int (*enum_dv_timings)(struct v4l2_subdev *sd, -- cgit v1.2.3-59-g8ed1b From d8c9a6e204f1466d228428174ce6d40ccdfb583e Mon Sep 17 00:00:00 2001 From: Paweł Anikiel Date: Fri, 5 Apr 2024 14:14:11 +0000 Subject: media: v4l2-subdev: Remove non-pad dv timing callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the conversion of dv timing calls to use a pad argument is done, remove the old callbacks. Update the subdev ioctl handlers to use the new callbacks. Signed-off-by: Paweł Anikiel Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-subdev.c | 6 +++--- include/media/v4l2-subdev.h | 14 -------------- 2 files changed, 3 insertions(+), 17 deletions(-) (limited to 'include/media/v4l2-subdev.h') diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 07759cdd0844..6572667fd5c4 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -906,16 +906,16 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, } case VIDIOC_SUBDEV_QUERY_DV_TIMINGS: - return v4l2_subdev_call(sd, video, query_dv_timings, arg); + return v4l2_subdev_call(sd, pad, query_dv_timings, 0, arg); case VIDIOC_SUBDEV_G_DV_TIMINGS: - return v4l2_subdev_call(sd, video, g_dv_timings, arg); + return v4l2_subdev_call(sd, pad, g_dv_timings, 0, arg); case VIDIOC_SUBDEV_S_DV_TIMINGS: if (ro_subdev) return -EPERM; - return v4l2_subdev_call(sd, video, s_dv_timings, arg); + return v4l2_subdev_call(sd, pad, s_dv_timings, 0, arg); case VIDIOC_SUBDEV_G_STD: return v4l2_subdev_call(sd, video, g_std, arg); diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index a5213411ef2b..1af16b16f0bf 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -452,14 +452,6 @@ enum v4l2_subdev_pre_streamon_flags { * * @g_pixelaspect: callback to return the pixelaspect ratio. * - * @s_dv_timings: Set custom dv timings in the sub device. This is used - * when sub device is capable of setting detailed timing information - * in the hardware to generate/detect the video signal. - * - * @g_dv_timings: Get custom dv timings in the sub device. - * - * @query_dv_timings: callback for VIDIOC_QUERY_DV_TIMINGS() ioctl handler code. - * * @s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev * can adjust @size to a lower value and must not write more data to the * buffer starting at @data than the original value of @size. @@ -490,12 +482,6 @@ struct v4l2_subdev_video_ops { int (*g_input_status)(struct v4l2_subdev *sd, u32 *status); int (*s_stream)(struct v4l2_subdev *sd, int enable); int (*g_pixelaspect)(struct v4l2_subdev *sd, struct v4l2_fract *aspect); - int (*s_dv_timings)(struct v4l2_subdev *sd, - struct v4l2_dv_timings *timings); - int (*g_dv_timings)(struct v4l2_subdev *sd, - struct v4l2_dv_timings *timings); - int (*query_dv_timings)(struct v4l2_subdev *sd, - struct v4l2_dv_timings *timings); int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf, unsigned int *size); int (*pre_streamon)(struct v4l2_subdev *sd, u32 flags); -- cgit v1.2.3-59-g8ed1b From 72364b91ce022ded4aa541c62f51c10a65fb3498 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Wed, 6 Sep 2023 10:56:14 +0300 Subject: media: v4l: subdev: Add a function to lock two sub-device states, use it Add two new functions, v4l2_subdev_lock_states() and v4l2_subdev_unclock_states(), to acquire and release the state of two sub-devices. They differ from calling v4l2_subdev_{un,}lock_state() so that if the two states share the same lock, the lock is acquired only once. Also use the new functions in v4l2_subdev_link_validate(). Signed-off-by: Sakari Ailus Reviewed-by: Julien Massot Reviewed-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++------- include/media/v4l2-subdev.h | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) (limited to 'include/media/v4l2-subdev.h') diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 2d67ec54569a..779583447eac 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1437,17 +1437,13 @@ int v4l2_subdev_link_validate(struct media_link *link) states_locked = sink_state && source_state; - if (states_locked) { - v4l2_subdev_lock_state(sink_state); - v4l2_subdev_lock_state(source_state); - } + if (states_locked) + v4l2_subdev_lock_states(sink_state, source_state); ret = v4l2_subdev_link_validate_locked(link, states_locked); - if (states_locked) { - v4l2_subdev_unlock_state(sink_state); - v4l2_subdev_unlock_state(source_state); - } + if (states_locked) + v4l2_subdev_unlock_states(sink_state, source_state); return ret; } diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1af16b16f0bf..e22c50ce7e05 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1724,6 +1724,46 @@ static inline void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state) mutex_unlock(state->lock); } +/** + * v4l2_subdev_lock_states - Lock two sub-device states + * @state1: One subdevice state + * @state2: The other subdevice state + * + * Locks the state of two sub-devices. + * + * The states must be unlocked with v4l2_subdev_unlock_states() after use. + * + * This differs from calling v4l2_subdev_lock_state() on both states so that if + * the states share the same lock, the lock is acquired only once (so no + * deadlock occurs). The caller is responsible for ensuring the locks will + * always be acquired in the same order. + */ +static inline void v4l2_subdev_lock_states(struct v4l2_subdev_state *state1, + struct v4l2_subdev_state *state2) +{ + mutex_lock(state1->lock); + if (state1->lock != state2->lock) + mutex_lock(state2->lock); +} + +/** + * v4l2_subdev_unlock_states() - Unlock two sub-device states + * @state1: One subdevice state + * @state2: The other subdevice state + * + * Unlocks the state of two sub-devices. + * + * This differs from calling v4l2_subdev_unlock_state() on both states so that + * if the states share the same lock, the lock is released only once. + */ +static inline void v4l2_subdev_unlock_states(struct v4l2_subdev_state *state1, + struct v4l2_subdev_state *state2) +{ + mutex_unlock(state1->lock); + if (state1->lock != state2->lock) + mutex_unlock(state2->lock); +} + /** * v4l2_subdev_get_unlocked_active_state() - Checks that the active subdev state * is unlocked and returns it -- cgit v1.2.3-59-g8ed1b From 83a22a07cd9d51b7ffd18a8904e1857061eda28f Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Thu, 31 Aug 2023 14:56:28 +0300 Subject: media: v4l: subdev: Add len_routes field to struct v4l2_subdev_routing The len_routes field is used to tell the size of the routes array in struct v4l2_subdev_routing. This way the number of routes returned from S_ROUTING IOCTL may be larger than the number of routes provided, in case there are more routes returned by the driver. Note that this uAPI is still disabled in the code, so this change can safely be done. Anyone who manually patched the code to enable this uAPI must update their code. The patch also increases the number of reserved fields in struct v4l2_subdev_routing. Signed-off-by: Sakari Ailus Reviewed-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- .../media/v4l/vidioc-subdev-g-routing.rst | 46 +++++++++++++++------- drivers/media/v4l2-core/v4l2-ioctl.c | 4 +- drivers/media/v4l2-core/v4l2-subdev.c | 12 +++--- include/media/v4l2-subdev.h | 2 + include/uapi/linux/v4l2-subdev.h | 10 +++-- 5 files changed, 49 insertions(+), 25 deletions(-) (limited to 'include/media/v4l2-subdev.h') 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 26b5004bfe6d..cbd9370006b6 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst @@ -43,23 +43,38 @@ 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`. -All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. This -means that the userspace must reconfigure all streams after calling the ioctl -with e.g. ``VIDIOC_SUBDEV_S_FMT``. +All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. +This means that the userspace must reconfigure all stream formats and selections +after calling the ioctl with e.g. ``VIDIOC_SUBDEV_S_FMT``. Only subdevices which have both sink and source pads can support routing. -When inspecting routes through ``VIDIOC_SUBDEV_G_ROUTING`` and the application -provided ``num_routes`` is not big enough to contain all the available routes -the subdevice exposes, drivers return the ENOSPC error code and adjust the -value of the ``num_routes`` field. Application should then reserve enough memory -for all the route entries and call ``VIDIOC_SUBDEV_G_ROUTING`` again. - -On a successful ``VIDIOC_SUBDEV_G_ROUTING`` call the driver updates the -``num_routes`` field to reflect the actual number of routes returned. +The ``len_routes`` field indicates the number of routes that can fit in the +``routes`` array allocated by userspace. It is set by applications for both +ioctls to indicate how many routes the kernel can return, and is never modified +by the kernel. + +The ``num_routes`` field indicates the number of routes in the routing +table. For ``VIDIOC_SUBDEV_S_ROUTING``, it is set by userspace to the number of +routes that the application stored in the ``routes`` array. For both ioctls, it +is returned by the kernel and indicates how many routes are stored in the +subdevice routing table. This may be smaller or larger than the value of +``num_routes`` set by the application for ``VIDIOC_SUBDEV_S_ROUTING``, as +drivers may adjust the requested routing table. + +The kernel can return a ``num_routes`` value larger than ``len_routes`` from +both ioctls. This indicates thare are more routes in the routing table than fits +the ``routes`` array. In this case, the ``routes`` array is filled by the kernel +with the first ``len_routes`` entries of the subdevice routing table. This is +not considered to be an error, and the ioctl call succeeds. If the applications +wants to retrieve the missing routes, it can issue a new +``VIDIOC_SUBDEV_G_ROUTING`` call with a large enough ``routes`` array. + +``VIDIOC_SUBDEV_S_ROUTING`` may return more routes than the user provided in +``num_routes`` field due to e.g. hardware properties. .. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}| @@ -74,6 +89,9 @@ On a successful ``VIDIOC_SUBDEV_G_ROUTING`` call the driver updates the - ``which`` - Routing table to be accessed, from enum :ref:`v4l2_subdev_format_whence `. + * - __u32 + - ``len_routes`` + - The length of the array (as in memory reserved for the array) * - struct :c:type:`v4l2_subdev_route` - ``routes[]`` - Array of struct :c:type:`v4l2_subdev_route` entries @@ -81,7 +99,7 @@ On a successful ``VIDIOC_SUBDEV_G_ROUTING`` call the driver updates the - ``num_routes`` - Number of entries of the routes array * - __u32 - - ``reserved``\ [5] + - ``reserved``\ [11] - Reserved for future extensions. Applications and drivers must set the array to zero. diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 0260acef97d2..aab671fae45b 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3201,13 +3201,13 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, case VIDIOC_SUBDEV_S_ROUTING: { struct v4l2_subdev_routing *routing = parg; - if (routing->num_routes > 256) + if (routing->len_routes > 256) return -E2BIG; *user_ptr = u64_to_user_ptr(routing->routes); *kernel_ptr = (void **)&routing->routes; *array_size = sizeof(struct v4l2_subdev_route) - * routing->num_routes; + * routing->len_routes; ret = 1; break; } diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 779583447eac..b565f202df67 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -960,14 +960,10 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, krouting = &state->routing; - if (routing->num_routes < krouting->num_routes) { - routing->num_routes = krouting->num_routes; - return -ENOSPC; - } - memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, krouting->routes, - krouting->num_routes * sizeof(*krouting->routes)); + min(krouting->num_routes, routing->len_routes) * + sizeof(*krouting->routes)); routing->num_routes = krouting->num_routes; return 0; @@ -989,6 +985,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev) return -EPERM; + if (routing->num_routes > routing->len_routes) + return -EINVAL; + memset(routing->reserved, 0, sizeof(routing->reserved)); for (i = 0; i < routing->num_routes; ++i) { @@ -1015,6 +1014,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, } krouting.num_routes = routing->num_routes; + krouting.len_routes = routing->len_routes; krouting.routes = routes; return v4l2_subdev_call(sd, pad, set_routing, state, diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index e22c50ce7e05..e30c463d90e5 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -714,12 +714,14 @@ struct v4l2_subdev_stream_configs { /** * struct v4l2_subdev_krouting - subdev routing table * + * @len_routes: length of routes array, in routes * @num_routes: number of routes * @routes: &struct v4l2_subdev_route * * This structure contains the routing table for a subdev. */ struct v4l2_subdev_krouting { + unsigned int len_routes; unsigned int num_routes; struct v4l2_subdev_route *routes; }; diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 81a24bd38003..2347e266cf75 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -228,15 +228,19 @@ struct v4l2_subdev_route { * struct v4l2_subdev_routing - Subdev routing information * * @which: configuration type (from enum v4l2_subdev_format_whence) - * @num_routes: the total number of routes in the routes array + * @len_routes: the length of the routes array, in routes; set by the user, not + * modified by the kernel * @routes: pointer to the routes array + * @num_routes: the total number of routes, possibly more than fits in the + * routes array * @reserved: drivers and applications must zero this array */ struct v4l2_subdev_routing { __u32 which; - __u32 num_routes; + __u32 len_routes; __u64 routes; - __u32 reserved[6]; + __u32 num_routes; + __u32 reserved[11]; }; /* -- cgit v1.2.3-59-g8ed1b