| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Remove unused fields from struct atomisp_input_subdev:
1. frame_size is never used at all
2. sensor_index is always 0, just directly pass 0 in the single user.
Link: https://lore.kernel.org/r/20230518153733.195306-3-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The in_reset argument to atomisp_css_start() is only ever true
in atomisp_assert_recovery_work(), drop the argument and move
the special reset handlig to atomisp_assert_recovery_work().
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Remove no longer user atomisp_get_css_pipe_id() function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
atomisp_css_[start|stop]()
The css_pipe_id argument pass to atomisp_css_[start|stop]() is
not used, drop it.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
atomisp_streaming_count() is just an alias for isp->asd.streaming now,
replace it with directly checking that and remove the helper.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The ATOMISP_DEVICE_STREAMING_STOPPING pipe state comes from when we still
had continuous mode. This would be set when streaming from both capture +
preview devnodes when 1 of the 2 streams has been stopped and the driver
was waiting for the other stream to get stopped too.
With continuous mode gone the stopping state is no longer necessary and
asd->streaming can be changed to a bool.
Note that atomisp_assert_recovery_work() would still temporarily
set streaming to stopping, but it does so with the isp->mutex held and
changes streaming to either enabled or disabled before releasing
the mutex, so none of the consumers which care about the difference
ever see the stopping state.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
All switch (pipe->asd->streaming) cases in atomisp_pipe_check()
are either no-ops or never happen:
1. ATOMISP_DEVICE_STREAMING_DISABLED already is a no-op
2. The videobuf2 core guarantees that when we are streaming
vb2_is_busy() returns true. So the ATOMISP_DEVICE_STREAMING_ENABLED case
is already handled by the if above the switch (pipe->asd->streaming).
3. After recent changes pipe->asd->streaming is only ever set to
ATOMISP_DEVICE_STREAMING_STOPPING in atomisp_assert_recovery_work() and
that function holds isp->mutex and always transitions the streaming state
to ATOMISP_DEVICE_STREAMING_DISABLED or ATOMISP_DEVICE_STREAMING_ENABLED
before releasing the mutex. So atomisp_pipe_check() never sees
ATOMISP_DEVICE_STREAMING_STOPPING.
Remove the entire switch-case.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that continuous mode is gone and we only have 1 /dev/video# node,
the videobuf2 core guarantees that atomisp_[start|stop]_streaming() will
only be called one at a time.
So there is no need to check for things like the isp is already streaming
when starting, or other streams still being active while stopping.
Remove checks for these from atomisp_[start|stop]_streaming().
While at it also improve the logging a bit:
1. Remove the dev_err(isp->dev, "atomisp_reset") logged on every
stream stop (even though everything is fine.
2. Log a message when starting/stopping the sensor stream fails
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that we no longer have continuous mode we always want to
start the sensor on the first atomisp_start_streaming() call
and stop it on the first atomisp_stop_streaming() call.
Remove atomisp_sensor_start_stream() which returned the number
of streams which should be active before starting the sensor
and always start/stop the sensor directly.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
isp_timeout only ever gets set in __atomisp_css_recover() and then
immediately gets cleared again after calling atomisp_reset().
All this happens with isp->mutex held.
The only consumer of isp->isp_timeout is atomisp_stop_streaming(), which
also holds isp->mutex and which is *not* called by atomisp_reset().
Since both hold isp->mutex and since __atomisp_css_recover() clears
isp_timeout before releasing the mutex, atomisp_stop_streaming() can
never see isp_timeout being true, so just remove the flag.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that there is only 1 source-pad for an asd there is no need
to have a parameter for this in various places.
Remove the source_pad function parameter and
atomisp_sub_device.capture_pad data member.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that we have only 1 /dev/video# node for output for all different
run-modes (with only 1 run-mode at a time) using video_out_preview for
the remaining atomisp_pipe does not properly reflect that this is
*the* output pipe. Fo the following renames to fix the naming:
s/video_out_preview/video_out/
s/ATOMISP_SUBDEV_PAD_SOURCE_PREVIEW/ATOMISP_SUBDEV_PAD_SOURCE/
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Since there only is one /dev/video# node now (no more continuous mode),
there are now no longer separate main capture + view-finder pipes.
We are now always on the main pipe, so atomisp_is_vf_pipe() should
always return false now. Drop any checks using it, replacing them
with the code-path for a false return.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that we no longer support continuous mode and thus no longer support
streaming from 2 /dev/video# nodes at the same time, there is no need
to have a separate /dev/video# node (+ matching v4l2-subdev pads)
for each run-mode.
Keep the video_out_preview /dev/video0 device and remove
the video_out_video_capture / video_out_vf / video_out_capture
video-devices (atomisp_pipe-s) and also remove the matching
ATOMISP_SUBDEV_PAD_SOURCE_VIDEO / ATOMISP_SUBDEV_PAD_SOURCE_VF /
ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE source-pads.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Since we no longer support Continuous mode, setting the run_mode to
ATOMISP_RUN_MODE_CONTINUOUS_CAPTURE no longer make sense, so remove
this.
While at it, also remove ATOMISP_RUN_MODE_SDV, which was never exposed
to userspace in the first place.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The only remaining caller of atomisp_try_fmt() always passes NULL
for the res_overflow parameter. Drop it and simplify atomisp_try_fmt().
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Remove the struct atomisp_sub_device.
This was used for 2 things:
1. In dev_dbg() messages
2. To set the name of the v4l2_subdev for each struct atomisp_sub_device
Now that only 1 subdev is used neither is useful anymore. Remove the _%d
postfix from the v4l2_subdev name and remove the logging of the asd index
from the dev_dbg() messages.
In case of the atomisp_s_input() check to see if an input/sensor has
already been assigned to another subdev the entire check no longer
makes sense, so instead of changing the dev_err() message there just
drop the entire check.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
With support for depth mode gone there really is no need to support
streaming from 2 sensors at once. As discussed and acked on the list
(see Link tag) it is desirable to drop support for this since it
involves quite a lot of special handling / hacks in the code.
This initial commit limits itself to a minimum set of changes to switch
to 1 struct atomisp_sub_device / 1 stream. Further commits will actually
remove / cleanup much of the special handling.
Likewise this initial commit also deliberately skips the opportunity to
turn some multi-line statements into single-line statements, so as to
keep the diff small / easier to review.
Link: https://lore.kernel.org/linux-media/5309d845-063b-6dd9-529d-0f82654290f2@redhat.com/
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
ATOMISP_USE_YUVPP() always returns false, so remove it and remove any
code-paths which only run when it would return true.
Link: https://lore.kernel.org/r/20230221145906.8113-9-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
After the continues-mode removal the delayed-work never gets queues
remove all the related code.
Link: https://lore.kernel.org/r/20230221145906.8113-4-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Continues mode is a special mode where 2 /dev/video devices can be active
at the same time. Either the video-preview + video nodes or the
viewfinder (for still capture) + capture nodes.
For the video-preview + video-recording case modern userspace will
use a single stream multiplexed by pipewire.
The still-capture case is extra special only starting the preview
stream and then relying on a custom ATOMISP_IOC_S_CONT_CAPTURE_CONFIG
ioctl to set things up followed by a second stream on to capture
the amount of configured still pictures. While running the sensor
at full resolution all the time. This case too is better handled
with dma-buf + GPU downscaling for the view-finder rather then all this
custom special code. Besises this the ioctl expects a bunch of special
non error checked conditions to be met otherwise things will crash/hang.
The continues mode also involves a special cases all over the code
getting in the way of further cleanups and simplifying the code to
using just 1 /dev/video# node. So lets remove it and the
related custom ATOMISP_IOC_S_CONT_CAPTURE_CONFIG ioctl.
Link: https://lore.kernel.org/linux-media/ea81b17b-7d1f-a5e1-11dd-04db310e1e50@redhat.com/
Link: https://lore.kernel.org/r/20230221145906.8113-3-hdegoede@redhat.com
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Remove support for depth mode. This is a special mode where 2 streams
(from 2 different sensors) can be setup and then starting/stopping
1 will automatically also start/stop the other.
Like many of these special features I'm pretty sure that if the queue
setup is not done exactly right things will crash and there is no error
checking for this.
This seems to be for stereoscopic vision and the only known hw which
actually supports this is the Intel Aero board/SDK, all other 1000+
BYT/CHT models don't need this.
This false outside of the standard webcam use scenario which we are
trying to get working and this involves a bunch of hacks / special
exceptions all over the code, so lets remove this.
Link: https://lore.kernel.org/linux-media/ea81b17b-7d1f-a5e1-11dd-04db310e1e50@redhat.com/
Link: https://lore.kernel.org/r/20230221145906.8113-2-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The s_power callback for v4l2-subdevs has been deprecated, allow sensor
drivers without a s_power callback to work by ignoring the -ENOIOCTLCMD
return value.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The bin-factor-x and bin-factor-y ctrls are only used internally to
get a single value to pass to atomisp_css_input_set_binning_factor(),
which is supposed to tune the lens-shading correction for the binning
factor. But all sensor drivers return either 0 or 1 for this,
with 0 meaning unset and 1 meaning no-binning. Even though some modes
do actually do binning ...
Also note that the removed atomisp_get_sensor_bin_factor() would fall
back to 0 if either the x and y factor differ or if the ctrls are not
implemented (not all sensor drivers implement them).
Simply always pass 0 to atomisp_css_input_set_binning_factor().
This is part of a patch-series which tries to remove atomisp specific /
custom code from the sensor drivers, with as end goal to make the atomisp
drivers regular camera sensor drivers.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
This ioctl returns a number of fixed sensor parameters +
a number of mode-specific parameters.
With libcamera these fixed parameters are instead stored in a table
with sensor-name to parameters mappings (camera_sensor_properties.cpp);
and the variable parameters can be derived from the set fmt.
So this custom ioctl is not necessary; and it currently has no users.
Remove the ioctl and all the sensor drivers xxxx_get_intg_factor()
helpers which return this info.
This is part of a patch-series which tries to remove atomisp specific /
custom code from the sensor drivers, with as end goal to make the atomisp
drivers regular camera sensor drivers.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
This ioctl simply returns a couple of fixed sensor parameters.
With libcamera these fixed parameters are instead stored in a table
with sensor-name to parameters mappings (camera_sensor_properties.cpp),
so this custom ioctl is not necessary; and it currently has no users.
Remove the ioctl and also remove the custom v4l2-ctrls underpinning
the ioctl.
This is part of a patch-series which tries to remove atomisp specific /
custom code from the sensor drivers, with as end goal to make the atomisp
drivers regular camera sensor drivers.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Check buffer index is in range inside atomisp_qbuf_wrapper() before
using it do index pipe->frame_request_config_id[].
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The videobuf2-core expects buffers to be put back in the queued state
when the vb2 start_streaming callback fails. But the atomisp
atomisp_flush_video_pipe() would unconditionally return them to the core
in an error state.
This triggers the following warning in the videobuf2-core:
drivers/media/common/videobuf2/videobuf2-core.c:1652:
/*
* If done_list is not empty, then start_streaming() didn't call
* vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED) but STATE_ERROR or
* STATE_DONE.
*/
WARN_ON(!list_empty(&q->done_list));
Fix this by adding a state argument to atomisp_flush_video_pipe() and use
VB2_BUF_STATE_QUEUED as state when atomisp_start_streaming() fails.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Don't touch q->start_streaming_called directly, use the
vb2_start_streaming_called() function instead.
Link: https://lore.kernel.org/r/bc6c24ec-72ea-64a1-9061-311cc7339827@xs4all.nl
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Remove the unnecessary sw_contex.power_state checks:
1. atomisp_freq_scaling() and atomisp_stop_streaming() only run when the
ISP is powered up through runtime-pm, so the checks are not necessary
2. atomisp_mrfld_pre_power_down() and atomisp_runtime_resume() are only
called through the driver-core pm handling code which already
guarantees they are not called when already powered down / up.
3. atomisp_isr() also checks isp->css_initialized which only gets set
by atomisp_css_init() which runs *after* powering up the ISP and which
gets cleared by atomisp_css_uninit() *before* powering down the ISP.
So all the checks are unnecessary, remove them as well as the
sw_contex.power_state field itself.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
I managed to trigger an atomisp_css_start() error by pushing my test system
towards an OOM situation, this triggered the following WARN_ON() in
__vb2_queue_cancel() in videobuf2-core.c:
/*
* If you see this warning, then the driver isn't cleaning up properly
* after a failed start_streaming(). See the start_streaming()
* documentation in videobuf2-core.h for more information how buffers
* should be returned to vb2 in start_streaming().
*/
if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
Fix this by calling atomisp_flush_video_pipe() to return any queued buffers
back to the videobuf2-core on an atomisp_css_start() error.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Make atomisp_g_fmt_cap() default to YUV420 so that it matches with what
atomisp_try_fmt_cap() and atomisp_queue_setup() do when they need to
pick a default pixelformat.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The atomisp_try_fmt() call in atomisp_try_fmt_cap() replaces
the pixelformat passed by userspace with the sensors native pixelformat.
Which always gets replaced by V4L2_PIX_FMT_YUV420 by atomisp_adjust_fmt()
because raw sensor formats are not supported.
This needs to be fixed so that userspace which does a try_fmt call before
s_fmt does not end up always getting YUV420 even if it passed something
else into the try_fmt call.
To fix this restore the userspace requested pixelformat before
the atomisp_adjust_fmt() call. atomisp_adjust_fmt() will replace this
with V4L2_PIX_FMT_YUV420 in case an unsupported format is requested.
Note this relies on the "media: atomisp: Refactor atomisp_adjust_fmt()"
change, before that atomisp_adjust_fmt() would return -EINVAL for
unsupported pixelformats.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Refactor atomisp_adjust_fmt():
1. The block starting at "format_bridge = atomisp_get_format_bridge(...)"
and ending with "if (field == V4L2_FIELD_ANY) field = V4L2_FIELD_NONE;"
is duplicated. With only the second block:
a) Properly checking that format_bridge is not NULL; amd
b) Having the special handling for IA_CSS_FRAME_FORMAT_RAW
Remove the first block.
2. On a NULL return from atomisp_get_format_bridge(f->fmt.pix.pixelformat)
fall back to V4L2_PIX_FMT_YUV420 just like in the IA_CSS_FRAME_FORMAT_RAW
case. atomisp_adjust_fmt() is used in VIDIOC_TRY_FMT handling and that
should jusy pick a fmt rather then returning -EINVAL.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
To fix atomisp_queue_setup() sometimes failing it needs to be able to call
atomisp_set_fmt(), but atomisp_queue_setup() (VIDIOC_REQBUFS) does not get
passed a file handle by the videobuf2 core.
Partly revert commit b3be98f984d4 ("media: atomisp: Remove a couple of not
useful function wrappers") so that atomisp_set_fmt() can be used
without a file handle.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Convert atomisp to use videobuf2.
This fixes mmap not working and in general moving over to
the more modern videobuf2 is a good plan.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Use the new atomisp_flush_video_pipe() helper instead of open-coding it.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Instead of using an integer to keep count of how many buffers have
been handed over to the ISP (buffers_in_css) move buffers handed
over to the ISP to a new buffers_in_css list_head so that we can
easily loop over them.
This removes the need for atomisp_flush_video_pipe() to loop over
all buffers and then (ab)use the state to figure out if they
were handed over to the ISP.
Since the buffers are now always on a list when owned by the driver
this also allows the buffer_done path on flush vs normal completion
to be unified (both now need a list_del()) and this common code can
now be factored out into a new atomisp_buffer_done() helper.
This is a preparation patch for moving the driver over to
the videobuf2 framework.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
There is no guarantee that when we stop the pipeline all buffers owned
by the CSS are cleanly returned to the videobuf queue.
This is a problem with videobuf2 which will complain loudly when not
all buffers have been returned after the streamoff() queue op has
returned.
And this also allows removing a WARN() in the continuous mode path.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Fixes this Werror breakage:
drivers/staging/media/atomisp/pci/atomisp_ioctl.c: In function 'atomisp_streamon':
drivers/staging/media/atomisp/pci/atomisp_ioctl.c:1714:44: error: variable 'sink' set but not used [-Werror=unused-but-set-variable]
1714 | struct v4l2_mbus_framefmt *sink;
| ^~~~
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
atomisp_source_pad_to_stream_id() returns ATOMISP_INPUT_STREAM_GENERAL
unconditionally now. Drop it and directly use ATOMISP_INPUT_STREAM_GENERAL
in its callers.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The code checks a camera_caps struct in various places, but this always
points to the same const camera_caps struct.
Remove the checks, keeping the code paths which would be taken with
the fixed camera caps struct still in place and remove the camera_caps
struct itself.
Note this completely removes atomisp_pause_buffer_event() because that
only ever does something if camera_caps.sensors[0].is_slave is true and
that never is true.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Drop an unnecessary first_streamoff check from atomisp_streamoff(),
above the check there is a:
if (!first_streamoff)
goto stop_sensor;
Code block which will jump over the code with the test, so the test
is only executed when first_streamoff is true and therefor the test
is not necessary.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The __atomisp_reqbufs(), __atomisp_streamoff() are 1:1 wrappers for
the non __ prefixed functions now, drop these wrappers.
The atomisp_s_fmt_cap() wrapper is almost a 1:1 wrapper for
atomisp_set_fmt() adjust the latter to have the right function
prototype and drop the wrapper.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Set video_dev.lock to point to isp->mutex so that the core does
the locking surroundig ioctls for us and drop all the now no longer
necessary (and conflicting) locking from the ioctl handling code.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Now that __atomisp_streamoff() no longer drops isp->mutex to cancel
the watchdog timer, the streamoff_mutex is no longer necessary to
avoid multiple streamoffs racing with each other.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
During the first __atomisp_streamoff() call on an asd with only one pipe
streaming asd->streaming would get set twice:
asd->streaming = ATOMISP_DEVICE_STREAMING_STOPPING;
asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED;
Rework the code a bit so that it gets set to the correct value
right away instead of doing this in 2 steps.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
The watchdog timer code to recover from the ISP getting stuck has several
major issues:
1. There is no way to do fault injection and normally the ISP does not
get stuck, so is it is impossible to test it.
2. It in essence just stops all streams, resets the ISP and then brings
everything back up. Userspace can easily do this itself by using a
timeout on dqbuf and then closing (which causes a poweroff) +
re-opening the device. Doing this in userspace (if it ever turns out
to be necessary) greatly simplifies the kernel code and in general
will be a more robust solution.
Even just a quick look at the code finds several more issues:
3. The need to sync-cancel the timers + work on streamoff requires
isp->mutex to be dropped halfway during the ioctl opening all sorts of
races.
4. The atomisp code supports setting up 2 pipelines, streaming from
two sensors at the same time. But there is only a single wdt_work
and stopping one of the 2 streams will cancel the timers + work,
stopping the wdt even though the other stream might still be running.
5. In case atomisp_css_flush() the sync cancel is done while keeping
isp->mutex locked, causing a deadlock when racing with wdt_work which
also takes isp->mutex.
6. Even though the watchdog is purely a software/driver thing which
just checkes that new frames keep coming in, there are 2 completely
different implementations for the ISP2400/ISP2401 which is not
necessary at all.
So all in all I believe that it is better to just remove the current
watchdog implementation. Fixing all the issues with the current
implementation will be so much work, that if it turns out that we do
need something like this then doing a clean re-implementation from
scratch will be better anyways.
wdt_work was also (ab)used to reset the ISP after the firmware signalled
an fw-assert error through the irq, add a new assert_recover_work to
replace this.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
Several of the ioctl handlers all do the same checks
(isp->fatal_error and asd->streaming errors) add
an atomisp_pipe_check() helper for this.
Note this changes the vidioc_s_fmt_vid_cap and vidioc_s_input handlers
to now reject calls made while asd->streaming==STOPPING. This fixes
a possible race where one thread can make this ioctls while
vidioc_streamoff is running from another thread and it has
temporarily released isp->mutex to kill the watchdog timers / work.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
|
At probe time isp_subdev_init_entities() sets pipe->asd to a non NULL
value for all four (preview/vf/capture/capture_video) pipes by calling
atomisp_init_subdev_pipe() for all 4 pipes.
So it can never be NULL. Remove the redundant NULL checks.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
|
