Link validation currently accesses invalid pointers if the link passed to
it is not between two sub-devices. This is of course a driver bug.
Ignore the error but print a warning message, as this is how it used to
work previously.
Fixes: a6b995ed03 ("media: subdev: use streams in v4l2_subdev_link_validate()")
Reported-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
The new subdev state code has a possible deadlock scenario during link
validation when the pipeline contains subdevs that support state and
that do not support state.
The current code locks the states of the subdevs on both ends of the
link when starting the link validation, locking the sink side first,
then the source. If either (or both) of the subdevs does not support
state, nothing is done for that subdev at this point, and instead the
locking is handled the old way, i.e. the subdev's ops do the locking
internally.
The issue arises when the sink doesn't support state, but source does,
so the validation code locks the source for the duration of the
validation, and then the sink is locked only when the get_fmt op is
called. So lockdep sees the source locked first, then the sink.
Later, when the streaming is started, the sink's s_stream op is called,
which probably takes the subdev's lock. The op then calls the source's
s_stream, which takes the source's lock. So, the sink is locked first,
then the source.
Note that link validation and stream starting is not done at the same
time, so an actual deadlock should never happen. However, it's still a
clear bug.
Fix this by locking the subdev states only if both subdevs support
state. In other words, we have two scenarios:
1. Both subdevs support state. Lock sink first, then source, and keep
the locks while validating the link.
2. At least one of the subdevs do not support state. Take the lock only
for the duration of the operation (get_fmt or looking at the
routing), and release after the op is done.
Obviously 1. is better, as we have a more consistent view of the states
of the subdevs during validation. 2. is how it has been so far, so it's
no worse than this used to be.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+QmuaPwR3wnBdVwACF8+vY7k4RUFAmP7M9AACgkQCF8+vY7k
4RVpxRAAjarn420frUo/YiMWuYiYtDCFmXj+toHgqsa9fcUOjxml9V+S5L0uY6tF
D6d9KCgqKf1AO2MDzB3aR1qQmPfelMoSomQjsTm6cWaMPDobxpzL2IlcspMDBxz0
PyCz4R9cCK5kwuBiQlz3dE605/t/7JXOAFEopo5tvYWNfRt9YXFbPJ/Hdttc4cqw
d6js3TN7oxHoa+t5Ox9Fq+i6MSxsMEku5RvfHVI6yUs//eWcf9H2zFfZ83vZ7+vY
L8PlRzMXlvovsFwXivtiZdSkuwFloWrqIs8btHb1/psClOUxFQhpk2B4hkUixCAn
wk9EN7eHWNdbaZha5//uPRmxUjjhIn4XAIXnfslsB7iiRn7uJtYryUnt+b+kD3Lt
dtF2i1W7nNfUd5e7YRjipTjgjtazLpeyDGvH0TqfpwK8Wn10Acj+Az1v4bjf+cc0
GC1EVUtGeJhexYzLsHSQMQZB1IgFxUw5LNKdqsrboled3yvxfgK69Yp9FQLon9vZ
R7KEDHuzt+e4Kihxq8dTp6wMV47dNrq0wpJKMjfylKhq/MPqa9uiygl1s2KlMg6n
HDJQlYbQGlzrgHDDQRhYUAgxs3JeyxAmup2eLR6dWrAqBW+ULT9S2DiggOk9xxKg
UkaCkodr3ZOhZti+oLRRAY2cRsGYgan7rKhscd0t7opO+CrfHxo=
=0XIw
-----END PGP SIGNATURE-----
Merge tag 'media/v6.3-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
Pull media updates from Mauro Carvalho Chehab:
- Removal of several VB1-only deprecated drivers: cpia2, fsl-viu, meye,
stkwebcam, tm6000, vpfe_capture and zr364xx
- saa7146 recovered from staging/deprecated. We opted to give ti a
chance, and, instead of deprecating it, the intention is to write
patches migrating it from VB1 to VB2.
- av7110 returned from staging/deprecated/ to staging/ as we're not
planning on dropping it any time soon
- media controller API has gained experimental support for G_ROUTING
and streams API. No drivers use it right now. We're planning to add
one after -rc1, giving some time to experience the API and eventually
have changes during the next development cycle
- New sensor drivers: imx296, imx415, ov8858
- Atomisp had lots of changes, specially on its sensor's interface,
making atomisp sensor drivers closer to normal sensor drivers
- media controller kAPI has gained some helpers to traverse pipelines
- uvcvideo now better support power line control
- lots of bug fixes, cleanups and driver improvements
* tag 'media/v6.3-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media: (296 commits)
media: imx-mipi-csis: Check csis_fmt validity before use
media: v4l2-subdev.c: clear stream field
media: v4l2-ctrls-api.c: move ctrl->is_new = 1 to the correct line
media: Revert "media: saa7146: deprecate hexium_gemini/orion, mxb and ttpci"
media: Revert "media: av7110: move to staging/media/deprecated/saa7146"
media: imx-pxp: convert to regmap
media: imx-pxp: Use non-threaded IRQ
media: imx-pxp: Introduce pxp_read() and pxp_write() wrappers
media: imx-pxp: Implement frame size enumeration
media: imx-pxp: Pass pixel format value to find_format()
media: imx-pxp: Add media controller support
media: imx-pxp: Don't set bus_info manually in .querycap()
media: imx-pxp: Sort headers alphabetically
media: imx-pxp: add support for i.MX7D
media: imx-pxp: make data_path_ctrl0 platform dependent
media: imx-pxp: disable LUT block
media: imx-pxp: explicitly disable unused blocks
media: imx-pxp: extract helper function to setup data path
media: imx-pxp: detect PXP version
media: dt-bindings: media: fsl-pxp: convert to yaml
...
Both userspace and kernelspace can pass structs with an uninitialized
'stream' field. Since the check_state() function checks for a non-zero
stream field, suddenly these calls will fails with -EINVAL.
So check in the wrapper functions in v4l2-subdev.c (which are used by both
the kernel and userspace API) if V4L2_SUBDEV_FL_STREAMS is set, and if not,
then zero the stream field.
Currently no drivers set V4L2_SUBDEV_FL_STREAMS, so the stream field will
always be set to 0.
This patch might well be reverted in the future when streams support is
fully enabled and we finalized the userspace API support for this feature.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Make v4l2_async_register_subdev_sensor() try to get a privacy LED
associated with the sensor and extend the call_s_stream() wrapper to
enable/disable the privacy LED if found.
This makes the core handle privacy LED control, rather then having to
duplicate this code in all the sensor drivers.
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Link: https://lore.kernel.org/r/20230127203729.10205-2-hdegoede@redhat.com
The v4l2_subdev_s_stream_helper() helper can be used by subdevs that
implement the stream-aware .enable_streams() and .disable_streams()
operations to implement .s_stream(). This is limited to subdevs that
have a single source pad.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add two new subdev pad operations, .enable_streams() and
.disable_streams(), to allow control of individual streams per pad. This
is a superset of what the video .s_stream() operation implements.
To help with handling of backward compatibility, add two wrapper
functions around those operations, and require their usage in drivers.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a helper function to translate streams between two pads of a subdev,
using the subdev's internal routing table.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a v4l2_subdev_routing_validate() helper for verifying routing for
common cases like only allowing non-overlapping 1-to-1 streams.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
v4l2_subdev_set_routing_with_fmt() is the same as
v4l2_subdev_set_routing(), but additionally initializes all the streams
with the given format.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add streams support to v4l2_subdev_get_fmt() helper function. Subdev
drivers that do not need to do anything special in their get_fmt op can
use this helper directly for v4l2_subdev_pad_ops.get_fmt.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add two helper functions to make dealing with streams easier:
v4l2_subdev_routing_find_opposite_end - given a routing table and a pad
+ stream, return the pad + stream on the opposite side of the subdev.
v4l2_subdev_state_get_opposite_stream_format - return a pointer to the
format on the pad + stream on the opposite side from the given pad +
stream.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Update v4l2_subdev_link_validate() to use routing and streams for
validation.
Instead of just looking at the format on the pad on both ends of the
link, the routing tables are used to collect all the streams going from
the source to the sink over the link, and the streams' formats on both
ends of the link are verified.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add support to manage configurations (format, crop, compose) per stream,
instead of per pad. This is accomplished with data structures that hold
an array of all subdev's stream configurations.
The number of streams can vary at runtime based on routing. Every time
the routing is changed, the stream configurations need to be
re-initialized.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a for_each_active_route() macro to replace the repeated pattern
of iterating on the active routes of a routing table.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a helper function to set the subdev routing. The helper can be used
from subdev driver's set_routing op to store the routing table.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a v4l2_subdev_has_pad_interdep() helper function which can be used
for media_entity_operations.has_pad_interdep op.
It considers two pads interdependent if there is an active route between
pad0 and pad1.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
The Streams API is an experimental feature. To use the Streams API, the
user needs to change a variable in v4l2-subdev.c and recompile the kernel.
This commit should be reverted when the Streams API is deemed ready for
production use.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add support for subdev internal routing. A route is defined as a single
stream from a sink pad to a source pad.
The userspace can configure the routing via two new ioctls,
VIDIOC_SUBDEV_G_ROUTING and VIDIOC_SUBDEV_S_ROUTING, and subdevs can
implement the functionality with v4l2_subdev_pad_ops.set_routing().
- Add sink and source streams for multiplexed links
- Copy the argument back in case of an error. This is needed to let the
caller know the number of routes.
- Expand and refine documentation.
- Make the 'routes' pointer a __u64 __user pointer so that a compat32
version of the ioctl is not required.
- Add struct v4l2_subdev_krouting to be used for subdevice operations.
- Fix typecasing warnings
- Check sink & source pad types
- Add 'which' field
- Routing to subdev state
- Dropped get_routing subdev op
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a subdev capability flag to expose to userspace if a subdev supports
multiplexed streams.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Replace custom implementation of the device_match_fwnode().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Complain in the newly added s_stream video op wrapper if disabling
streaming failed. Also return zero in this case as there's nothing the
caller can do to return the error.
This way drivers also won't need to bother with printing error messages.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add v4l2_subdev_get_fmt() helper function which implements
v4l2_subdev_pad_ops.get_fmt using active state. Subdev drivers that
support active state and do not need to do anything special in their
get_fmt op can use this helper directly for v4l2_subdev_pad_ops.get_fmt.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
It is common that media drivers call subdev ops in source subdevs, and
pass NULL as the state. This was the way to indicate that the callee
should use the callee's private active state.
E.g.:
v4l2_subdev_call(priv->source_sd, pad, get_fmt, NULL, &sd_fmt);
Now that we have a real subdev active state in the v4l2_subdev struct,
we want the caller to pass a proper state (when available). And
furthermore, the state should be locked.
This would mean changing all the callers, which is the long term goal.
To fix this issue in the short term, let's add an extra wrapper layer to
all v4l2_subdev_call_pad_wrappers which deal with states. These wrappers
handle the state == NULL case by using the locked active state instead
(when available).
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
The V4L2 subdevs have managed without centralized locking for the state
(previously pad_config), as the try-state is supposedly safe (although I
believe two TRY ioctls for the same fd would race), and the
active-state, and its locking, is managed by the drivers internally.
We now have active-state in a centralized position, and need locking.
Strictly speaking the locking is only needed for new drivers that use
the new state, as the current drivers continue behaving as they used to.
However, active-state locking is complicated by the fact that currently
the real active-state of a subdev is split into multiple parts: the new
v4l2_subdev_state, subdev control state, and subdev's internal state.
In the future all these three states should be combined into one state
(the v4l2_subdev_state), and then a single lock for the state should be
sufficient.
But to solve the current split-state situation we need to share locks
between the three states. This is accomplished by using the same lock
management as the control handler does: we use a pointer to a mutex,
allowing the driver to override the default mutex. Thus the driver can
do e.g.:
sd->state_lock = sd->ctrl_handler->lock;
before calling v4l2_subdev_init_finalize(), resulting in sharing the
same lock between the states and the controls.
The locking model for active-state is such that any subdev op that gets
the state as a parameter expects the state to be already locked by the
caller, and expects the caller to release the lock.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
At the moment when a subdev op is called, the TRY subdev state
(subdev_fh->state) is passed as a parameter even for the ACTIVE case, or
alternatively a NULL can be passed for ACTIVE case. This used to make
sense, as the ACTIVE state was handled internally by the subdev drivers.
We now have a state for the ACTIVE case in a standard place, and can
pass that also to the drivers. This patch changes the subdev ioctls to
either pass the TRY or ACTIVE state to the subdev.
Unfortunately many drivers call ops from other subdevs, and implicitly
pass NULL as the state, so this is just a partial solution. A coccinelle
spatch could perhaps be created which fixes the drivers' subdev calls.
For all current upstream drivers this doesn't matter, as they do not
expect to get a valid state for ACTIVE case. But future drivers which
support multiplexed streaming and routing will depend on getting a state
for both active and try cases.
For new drivers we can mandate that the pipelines where the drivers are
used need to pass the state properly, or preferably, not call such
subdev ops at all.
However, if an existing subdev driver is changed to support multiplexed
streams, the driver has to consider cases where its ops will be called
with NULL state. The problem can easily be solved by using the
v4l2_subdev_lock_and_get_active_state() helper, introduced in a follow
up patch.
Another follow up patch adds wrappers for pad ops dealing with subdev
state, which automate the use of
v4l2_subdev_lock_and_get_active_state() for cases where the state is
NULL.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Add a new 'active_state' field to struct v4l2_subdev to which we can
store the active state of a subdev. This will place the subdev
configuration into a known place, allowing us to use the state directly
from the v4l2 framework, thus simplifying the drivers.
Also add functions v4l2_subdev_init_finalize() and
v4l2_subdev_cleanup(), which will allocate and free the active state.
The functions are named in a generic way so that they can be also used
for other subdev initialization work.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
v4l2_subdev_alloc_state() and v4l2_subdev_free_state() are not supposed
to be used by the drivers. However, we do have a few drivers that use
those at the moment, so we need to expose these functions for the time
being.
Prefix the functions with __ to mark the functions as internal.
At the same time, rename them to v4l2_subdev_state_alloc and
v4l2_subdev_state_free to match the style used for other functions like
video_device_alloc() and media_request_alloc().
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
subdev_open() is inside #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API, which
depends on CONFIG_MEDIA_CONTROLLER, so there's no need for an extra
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
The .set_mbus_config() operation is deprecated, and nothing in the
kernel uses it. Drop it.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
code, but for some reason I ended up adding another conversion helper to
subdev_do_ioctl() as well. I must have concluded that this does not go
through the common conversion, but it has done that since the ioctl
handler was first added.
I assume this one is harmless as there should be no way to arrive here
from user space if CONFIG_COMPAT_32BIT_TIME is set, but since it is dead
code, it should just get removed.
On a 64-bit architecture, as well as a 32-bit architecture without
CONFIG_COMPAT_32BIT_TIME, handling this command is a mistake,
and the kernel should return an error.
Fixes: 1a6c0b36dd ("media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
We have 'struct v4l2_subdev_pad_config' which contains configuration for
a single pad used for the TRY functionality, and an array of those
structs is passed to various v4l2_subdev_pad_ops.
I was working on subdev internal routing between pads, and realized that
there's no way to add TRY functionality for routes, which is not pad
specific configuration. Adding a separate struct for try-route config
wouldn't work either, as e.g. set-fmt needs to know the try-route
configuration to propagate the settings.
This patch adds a new struct, 'struct v4l2_subdev_state' (which at the
moment only contains the v4l2_subdev_pad_config array) and the new
struct is used in most of the places where v4l2_subdev_pad_config was
used. All v4l2_subdev_pad_ops functions taking v4l2_subdev_pad_config
are changed to instead take v4l2_subdev_state.
The changes to drivers/media/v4l2-core/v4l2-subdev.c and
include/media/v4l2-subdev.h were written by hand, and all the driver
changes were done with the semantic patch below. The spatch needs to be
applied to a select list of directories. I used the following shell
commands to apply the spatch:
dirs="drivers/media/i2c drivers/media/platform drivers/media/usb drivers/media/test-drivers/vimc drivers/media/pci drivers/staging/media"
for dir in $dirs; do spatch -j8 --dir --include-headers --no-show-diff --in-place --sp-file v4l2-subdev-state.cocci $dir; done
Note that Coccinelle chokes on a few drivers (gcc extensions?). With
minor changes we can make Coccinelle run fine, and these changes can be
reverted after spatch. The diff for these changes is:
For drivers/media/i2c/s5k5baf.c:
@@ -1481,7 +1481,7 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd,
&s5k5baf_cis_rect,
v4l2_subdev_get_try_crop(sd, cfg, PAD_CIS),
v4l2_subdev_get_try_compose(sd, cfg, PAD_CIS),
- v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT)
+ v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT),
};
s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
return 0;
For drivers/media/platform/s3c-camif/camif-capture.c:
@@ -1230,7 +1230,7 @@ static int s3c_camif_subdev_get_fmt(struct v4l2_subdev *sd,
*mf = camif->mbus_fmt;
break;
- case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
+ case CAMIF_SD_PAD_SOURCE_C:
/* crop rectangle at camera interface input */
mf->width = camif->camif_crop.width;
mf->height = camif->camif_crop.height;
@@ -1332,7 +1332,7 @@ static int s3c_camif_subdev_set_fmt(struct v4l2_subdev *sd,
}
break;
- case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
+ case CAMIF_SD_PAD_SOURCE_C:
/* Pixel format can be only changed on the sink pad. */
mf->code = camif->mbus_fmt.code;
mf->width = crop->width;
The semantic patch is:
// <smpl>
// Change function parameter
@@
identifier func;
identifier cfg;
@@
func(...,
- struct v4l2_subdev_pad_config *cfg
+ struct v4l2_subdev_state *sd_state
, ...)
{
<...
- cfg
+ sd_state
...>
}
// Change function declaration parameter
@@
identifier func;
identifier cfg;
type T;
@@
T func(...,
- struct v4l2_subdev_pad_config *cfg
+ struct v4l2_subdev_state *sd_state
, ...);
// Change function return value
@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
*func(...)
{
...
}
// Change function declaration return value
@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
*func(...);
// Some drivers pass a local pad_cfg for a single pad to a called function. Wrap it
// inside a pad_state.
@@
identifier func;
identifier pad_cfg;
@@
func(...)
{
...
struct v4l2_subdev_pad_config pad_cfg;
+ struct v4l2_subdev_state pad_state = { .pads = &pad_cfg };
<+...
(
v4l2_subdev_call
|
sensor_call
|
isi_try_fse
|
isc_try_fse
|
saa_call_all
)
(...,
- &pad_cfg
+ &pad_state
,...)
...+>
}
// If the function uses fields from pad_config, access via state->pads
@@
identifier func;
identifier state;
@@
func(...,
struct v4l2_subdev_state *state
, ...)
{
<...
(
- state->try_fmt
+ state->pads->try_fmt
|
- state->try_crop
+ state->pads->try_crop
|
- state->try_compose
+ state->pads->try_compose
)
...>
}
// If the function accesses the filehandle, use fh->state instead
@@
struct v4l2_subdev_fh *fh;
@@
- fh->pad
+ fh->state
@@
struct v4l2_subdev_fh fh;
@@
- fh.pad
+ fh.state
// Start of vsp1 specific
@@
@@
struct vsp1_entity {
...
- struct v4l2_subdev_pad_config *config;
+ struct v4l2_subdev_state *config;
...
};
@@
symbol entity;
@@
vsp1_entity_init(...)
{
...
entity->config =
- v4l2_subdev_alloc_pad_config
+ v4l2_subdev_alloc_state
(&entity->subdev);
...
}
@@
symbol entity;
@@
vsp1_entity_destroy(...)
{
...
- v4l2_subdev_free_pad_config
+ v4l2_subdev_free_state
(entity->config);
...
}
@exists@
identifier func =~ "(^vsp1.*)|(hsit_set_format)|(sru_enum_frame_size)|(sru_set_format)|(uif_get_selection)|(uif_set_selection)|(uds_enum_frame_size)|(uds_set_format)|(brx_set_format)|(brx_get_selection)|(histo_get_selection)|(histo_set_selection)|(brx_set_selection)";
symbol config;
@@
func(...) {
...
- struct v4l2_subdev_pad_config *config;
+ struct v4l2_subdev_state *config;
...
}
// End of vsp1 specific
// Start of rcar specific
@@
identifier sd;
identifier pad_cfg;
@@
rvin_try_format(...)
{
...
- struct v4l2_subdev_pad_config *pad_cfg;
+ struct v4l2_subdev_state *sd_state;
...
- pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+ sd_state = v4l2_subdev_alloc_state(sd);
<...
- pad_cfg
+ sd_state
...>
- v4l2_subdev_free_pad_config(pad_cfg);
+ v4l2_subdev_free_state(sd_state);
...
}
// End of rcar specific
// Start of rockchip specific
@@
identifier func =~ "(rkisp1_rsz_get_pad_fmt)|(rkisp1_rsz_get_pad_crop)|(rkisp1_rsz_register)";
symbol rsz;
symbol pad_cfg;
@@
func(...)
{
+ struct v4l2_subdev_state state = { .pads = rsz->pad_cfg };
...
- rsz->pad_cfg
+ &state
...
}
@@
identifier func =~ "(rkisp1_isp_get_pad_fmt)|(rkisp1_isp_get_pad_crop)";
symbol isp;
symbol pad_cfg;
@@
func(...)
{
+ struct v4l2_subdev_state state = { .pads = isp->pad_cfg };
...
- isp->pad_cfg
+ &state
...
}
@@
symbol rkisp1;
symbol isp;
symbol pad_cfg;
@@
rkisp1_isp_register(...)
{
+ struct v4l2_subdev_state state = { .pads = rkisp1->isp.pad_cfg };
...
- rkisp1->isp.pad_cfg
+ &state
...
}
// End of rockchip specific
// Start of tegra-video specific
@@
identifier sd;
identifier pad_cfg;
@@
__tegra_channel_try_format(...)
{
...
- struct v4l2_subdev_pad_config *pad_cfg;
+ struct v4l2_subdev_state *sd_state;
...
- pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+ sd_state = v4l2_subdev_alloc_state(sd);
<...
- pad_cfg
+ sd_state
...>
- v4l2_subdev_free_pad_config(pad_cfg);
+ v4l2_subdev_free_state(sd_state);
...
}
@@
identifier sd_state;
@@
__tegra_channel_try_format(...)
{
...
struct v4l2_subdev_state *sd_state;
<...
- sd_state->try_crop
+ sd_state->pads->try_crop
...>
}
// End of tegra-video specific
// </smpl>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The existing link format validation failure debug message in media-entity.c
helped to pinpoint the point of failure but provided no additional
information what's wrong. Tell the user exactly why the validation failed.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Introduce two new pad operations to allow retrieving and configuring the
media bus parameters on a subdevice pad.
The newly introduced operations aims to replace the s/g_mbus_config video
operations, which have been on their way for deprecation since a long
time.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Add a convenience function that can be used as the .get_fwnode_pad
operation for subdevices that map port numbers and pad indexes 1:1.
The function verifies the endpoint is owned by the subdevice, and if
so returns the endpoint port number.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl
that apps can call to determine that it is indeed a V4L2 device, there
is currently no equivalent for v4l-subdev nodes. Adding this ioctl will
solve that, and it will allow utilities like v4l2-compliance to be used
with these devices as well.
SUBDEV_QUERYCAP currently returns the version and capabilities of the
subdevice. Define a capability flag to report if the subdevice is
registered in read-only mode.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
A sub-device device node can be registered in user space only if the
CONFIG_VIDEO_V4L2_SUBDEV_API Kconfig option is selected. Currently the
open/close file operations and the ioctl handler have some parts of
their implementations guarded by #if
defined(CONFIG_VIDEO_V4L2_SUBDEV_API), while they are actually not
accessible without a video device node registered to user space.
Guard the whole open, close and ioctl handler and provide stubs if the
VIDEO_V4L2_SUBDEV_API Kconfig option is not selected.
This slightly reduces the kernel size when the option is not selected
and simplifies the file ops and ioctl implementations.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Add to the V4L2 core a function to register device nodes for video
subdevices in read-only mode.
Registering a device node in read-only mode is useful to expose to
userspace the current sub-device configuration, without allowing
application to change it by using the V4L2 subdevice ioctls.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
In the case where v4l2_event_dequeue fails the structure ev is not
being filled and this garbage data from the stack is being copied
to the ev32 structure and being copied back to userspace on the
VIDIOC_DQEVENT_TIME32 ioctl. Fix this by ensuring the ev structure
is zero'd to ensure uninitialized data is not leaked back.
Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: 1a6c0b36dd ("media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The v4l2_event structure contains a 'struct timespec' member that is
defined by the user space C library, creating an ABI incompatibility
when that gets updated to a 64-bit time_t.
While passing a 32-bit time_t here would be sufficient for CLOCK_MONOTONIC
timestamps, simply redefining the structure to use the kernel's
__kernel_old_timespec would not work for any library that uses a copy
of the linux/videodev2.h header file rather than including the copy from
the latest kernel headers.
This means the kernel has to be changed to handle both versions of the
structure layout on a 32-bit architecture. The easiest way to do this
is during the copy from/to user space.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Commit a8fa55078a ("media: v4l2-subdev: Verify arguments in
v4l2_subdev_call()") and commit 374d62e7aa ("media: v4l2-subdev:
Verify v4l2_subdev_call() pad config argument") introduced a few local
functions, unfortunately with arguments of type __u32, reserved for use
in Linux uAPI. Use u32 instead.
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Currently, the v4l2 control code is a bit silent on errors.
Add debug messages on (hopefully) most of the error paths.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
sd->entity.graph_obj.mdev can be NULL when this function is called, and
that breaks existing drivers (rcar-vin, but probably others as well).
Check if sd->entity.num_pads is non-zero instead since that doesn't depend
on mdev.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reported-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Fixes: a8fa55078a ("media: v4l2-subdev: Verify arguments in v4l2_subdev_call()")
Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Extend parameter checks performed by v4l2_subdev_call() with a check for
a non-NULL pad config pointer if V4L2_SUBDEV_FORMAT_TRY format type is
requested so drivers don't need to care.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Parameters passed to check helpers are now obtained by dereferencing
unverified pointer arguments. Check validity of those pointers first.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Correctness of format type (try or active) and pad number parameters
passed to subdevice operation callbacks is now verified only for IOCTL
calls. However, those callbacks are also used by drivers, e.g., V4L2
host interfaces.
Since both subdev_do_ioctl() and drivers are using v4l2_subdev_call()
macro while calling subdevice operations, move those parameter checks
from subdev_do_ioctl() to v4l2_subdev_call() so we can avoid taking care
of those checks inside drivers.
Define a wrapper function for each operation callback in scope, then
gather those wrappers in a static v4l2_subdev_ops structure so the
v4l2_subdev_call() macro can find them easy if provided.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license version 2 as
published by the free software foundation this program is
distributed in the hope that it will be useful but without any
warranty without even the implied warranty of merchantability or
fitness for a particular purpose see the gnu general public license
for more details
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-only
has been chosen to replace the boilerplate/reference in 655 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070034.575739538@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The module ownership refcounting was done in media_entity_get/put,
but that was very confusing and it did not work either in case an
application had a v4l-subdevX device open and the module was
unbound. When the v4l-subdevX device was closed the media_entity_put
was never called and the module refcount was left one too high, making
it impossible to unload it.
Since v4l2-subdev.c was the only place where media_entity_get/put was
called, just move the functionality to v4l2-subdev.c and drop those
confusing entity functions.
Store the module in subdev_fh so module_put no longer depends on
the media_entity struct.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
