Hans de Goede [Sun, 12 Jun 2022 16:05:54 +0000 (17:05 +0100)]
media: atomisp: revert "don't pass a pointer to a local variable"
The gcc is warning about returning a pointer to a local variable
is a false positive.
The type of handle is "struct ia_css_rmgr_vbuf_handle **" and
"h.vptr" is left to NULL, so the "if ((*handle)->vptr == 0x0)"
check always succeeds when the "*handle = &h;" statement which
gcc warns about executes. Leading to this statement being executed:
rmgr_pop_handle(pool, handle);
If that succeeds, then *handle has been set to point to one of
the pre-allocated array of handles, so it no longer points to h.
If that fails the following statement will be executed:
/* Note that handle will change to an internally maintained one */
ia_css_rmgr_refcount_retain_vbuf(handle);
Which allocated a new handle from the array of pre-allocated handles
and then makes *handle point to this. So the address of h is actually
never returned.
The fix for the false-postive compiler warning actually breaks the code,
the new:
**handle = h;
is part of a "if (pool->copy_on_write) { ... }" which means that the
handle where *handle points to should be treated read-only, IOW
**handle must never be set, instead *handle must be set to point to
a new handle (with a copy of the contents of the old handle).
The old code correctly did this and the new fixed code gets this wrong.
Note there is another patch in this series, which fixes the warning
in another way.
media: [PATCH] pci: atomisp_cmd: fix three missing checks on list iterator
The three bugs are here:
__func__, s3a_buf->s3a_data->exp_id);
__func__, md_buf->metadata->exp_id);
__func__, dis_buf->dis_data->exp_id);
The list iterator 's3a_buf/md_buf/dis_buf' will point to a bogus
position containing HEAD if the list is empty or no element is found.
This case must be checked before any use of the iterator, otherwise
it will lead to a invalid memory access.
To fix this bug, add an check. Use a new variable '*_iter' as the
list iterator, while use the old variable '*_buf' as a dedicated
pointer to point to the found element.
Fabio M. De Francesco [Wed, 13 Apr 2022 22:55:31 +0000 (23:55 +0100)]
media: staging: media: atomisp: Use kmap_local_page() in hmm_store()
The use of kmap() is being deprecated in favor of kmap_local_page()
where it is feasible. The same is true for kmap_atomic().
In file pci/hmm/hmm.c, function hmm_store() test if we are in atomic
context and, if so, it calls kmap_atomic(), if not, it calls kmap().
First of all, in_atomic() shouldn't be used in drivers. This macro
cannot always detect atomic context; in particular, it cannot know
about held spinlocks in non-preemptible kernels.
Notwithstanding what it is said above, this code doesn't need to care
whether or not it is executing in atomic context. It can simply use
kmap_local_page() / kunmap_local() that can instead do the mapping /
unmapping regardless of the context.
With kmap_local_page(), the mapping is per thread, CPU local and not
globally visible. Therefore, hmm_store()() is a function where the use
of kmap_local_page() in place of both kmap() and kmap_atomic() is
correctly suited.
Convert the calls of kmap() / kunmap() and kmap_atomic() /
kunmap_atomic() to kmap_local_page() / kunmap_local() and drop the
unnecessary tests which test if the code is in atomic context.
Fabio M. De Francesco [Wed, 13 Apr 2022 21:22:10 +0000 (22:22 +0100)]
media: staging: media: atomisp: Use kmap_local_page() in hmm_set()
The use of kmap() is being deprecated in favor of kmap_local_page()
where it is feasible. In file pci/hmm/hmm.c, function hmm_set() calls
kmap() / kunmap() where kmap_local_page() can instead do the mapping.
With kmap_local_page(), the mapping is per thread, CPU local and not
globally visible. Therefore, hmm_set()() is a function where the use
of kmap_local_page() in place of kmap() is correctly suited.
Convert the calls of kmap() / kunmap() to kmap_local_page() /
kunmap_local().
Fabio M. De Francesco [Fri, 8 Apr 2022 22:31:29 +0000 (23:31 +0100)]
media: staging: media: atomisp: Convert kmap() to kmap_local_page()
The use of kmap() is being deprecated in favor of kmap_local_page() where
it is feasible. With kmap_local_page(), the mapping is per thread, CPU
local and not globally visible.
load_and_flush_by_kmap() is a function where the use of kmap_local_page()
in place of kmap() is correctly suited.
Convert load_and_flush_by_kmap() from kmap() to kmap_local_page().
Tom Rix [Sat, 26 Mar 2022 19:18:53 +0000 (19:18 +0000)]
media: staging: atomisp: rework reading the id and revision values
Clang static analysis reports this representative issue
atomisp-ov2722.c:920:3: warning: 3rd function call
argument is an uninitialized value
dev_err(&client->dev, "sensor_id_high = 0x%x\n", high);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
high and low are only set when ov2722_read_reg() is successful.
Reporting the high value when there is an error is not
meaningful. The later read for low is not checked. high
and low are or-ed together and checked against a non zero
value.
Remove the unneeded error reporting for high. Initialize
high and low to 0 and use the id check to determine if
the reads were successful
The later read for revision is not checked. If it
fails the old high value will be used and the revision
will be misreported.
Since the revision is only reported and not checked or
stored it is not necessary to return if the read with
successful. This makes the ret variable unnecessary
so remove it.
Hans de Goede [Wed, 15 Jun 2022 20:50:36 +0000 (21:50 +0100)]
media: atomisp: remove force argument from __destroy_[stream[s]|pipe[s]]()
The force argument to the __destroy_pipe[s]() and __destroy_stream[s]()
functions is always true. Remove the argument and remove the code necessary
to handle the false case.
Hans de Goede [Wed, 15 Jun 2022 20:50:23 +0000 (21:50 +0100)]
media: atomisp: drop unused ATOMISP_MAP_FLAG_* flags
Drop the ATOMISP_MAP_FLAG_CACHED flag, it is never set anywhere;
also drop the matching "cached" parameter to hmm[_bo]_alloc which
value was derived form the never set flag.
Drop the ATOMISP_MAP_FLAG_NOFLUSH, it is not used anywhere.
These ioctls allow userspace to load custom programs into the ISP, which:
a) Seems dangerous
b) Cannot be used by opensource userspace since there is no FOSS code to
create such programs
b) These seem to be unused even by the Android closed source camera code
(they don't show up in a strace of the camera app)
So removing these seems be a good idea. Another reason to remove these is
that atomisp_acc_map() is the only user of the userptr functionality in
hmm_alloc(), so it gets in the way of further cleanups / simplification
of the hmm code.
The comment documenting hmm_bo_allocated() was copied (and not modified)
from the comment documenting hmm_bo_alloc(), so there are 2 copies
of the hmm_bo_alloc() documentation.
Remove the copy of the comment above the hmm_bo_allocated() prototype.
Hans de Goede [Wed, 15 Jun 2022 20:50:07 +0000 (21:50 +0100)]
media: atomisp: remove dynamic and reserved pool code
There are no callers of this code atm; and looking at the atomisp
memory-management code if anything we want to make it simpler and
not re-introduce use of these pools, so remove the pool code.
Hans de Goede [Wed, 15 Jun 2022 20:49:58 +0000 (21:49 +0100)]
media: atomisp: remove the unused RAW_BUF_STRIDE macro
I noticed that the RAW_BUF_STRIDE macro is using the removed
SH_CSS_BINARY_ID_POST_ISP define, which should be a problem except that
the RAW_BUF_STRIDE macro itself is not used at all, remove it.
Krzysztof Hałasa [Tue, 1 Mar 2022 08:41:38 +0000 (08:41 +0000)]
media: On Semi AR0521 sensor driver
The driver has been extensively tested in an i.MX6-based system.
AR0521 is a 5.7 mm x 4.3 mm, 5 MPix RGGB MIPI/HiSPi BSI CMOS sensor
from On Semiconductor.
Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Jacopo Mondi [Fri, 13 May 2022 14:14:16 +0000 (15:14 +0100)]
media: ov5640: Move format mux config in format
The image format produced by the sensor is controlled by two registers,
whose values computation is open coded in ov5640_set_framefmt().
As we have a list of formats already, move the OV5640_REG_FORMAT_CONTROL00
and OV5640_REG_ISP_FORMAT_MUX_CTRL register values to the static list
of formats instead of open coding it.
Jacopo Mondi [Fri, 13 May 2022 14:14:12 +0000 (15:14 +0100)]
media: ov5640: Add BGR888 format
Add support for BGR888 image format.
No existing media bus codes describe exactly the way data is transferred
on the CSI-2 bus. This is not a new issue, the CSI-2 YUV422 8-bit format
is described by MEDIA_BUS_FMT_UYVY8_1X16 which is an arbitrary
convention and not an exact match. Use the MEDIA_BUS_FMT_BGR888_1X24 to
follow the same convention, based on the order in which bits are
transmitted over the CSI-2 bus when producing images in RGB24 format.
Jacopo Mondi [Fri, 13 May 2022 14:14:06 +0000 (15:14 +0100)]
media: ov5640: Remove frame rate check from find_mode()
The current implementation of ov5640_find_mode() fails if the
frame rate programmed with s_frame_interval doesn't match the
maximum frame rate for the current mode.
This causes issues when moving from one mode with higher FPS to another
one which only supports a lower FPS range with tools like media-ctl.
It also forces users that do not use s_frame_interval(), but rather
configure blankings explicitly, to adjust the programmed FPS range to
avoid failures.
For this reason, remove the FPS check from ov5640_find_mode() and only
perform it at s_frame_interval() time.
Hugues Fruchet [Fri, 13 May 2022 14:14:05 +0000 (15:14 +0100)]
media: ov5640: Adjust vblank with s_frame_interval
Adjust the vertical blanking when s_frame_interval is used.
s_frame_interval allows to set a fixed frame rate, which gets stored as
the sensor's current one. When a new mode is applied, the current frame
rate is reset to the mode's default one. In order to correctly report
the currently configured vertical blanking for s_frame_interval users,
verify that the desired frame rate has not been changed. If that's the
case, compute the correct blanking value and reflect it through the
corresponding v4l2 control.
Jacopo Mondi [Fri, 13 May 2022 14:14:02 +0000 (15:14 +0100)]
media: ov5640: Remove ov5640_mode_init_data
The ov5640_mode_init_data is a fictional sensor more which is used to
program the initial sensor settings.
It is only used to initialize the sensor and can be replaced
it with a throw-away mode which just wraps the register table.
Also rename the register table to drop the format from the name to make
it clear an actual sensor mode has to be applied after the initial
programming.
Jacopo Mondi [Fri, 13 May 2022 14:13:58 +0000 (15:13 +0100)]
media: ov5640: Split DVP and CSI-2 timings
Separate the timings for the DVP mode from the timings for the CSI-2
mode in the ov5640 modes definition.
The CSI-2 timings will deviate from the DVP ones as the clock tree is
calculated differently.
In CSI-2 mode change the analog crop rectangles as the OV5640 pixel
array is composed as:
- vertically: 16 dummy columns, 2592 valid ones and 16 dummy columns for
a total of 2624 columns
- horizontally: 8 optical black lines, 6 dummy ones, 1944 valid and 6
dummies for a total of 1964 lines
Adjust the analog crop rectangle in CSI-2 mode to:
- Skip the first 16 dummy columns
- Skip the first 14 black/dummy lines
- Pass the whole valid pixel array size to the ISP for all modes except
1024x768, 720p and 1080p which are obtained by cropping the valid pixel
array.
Tested in RGB565, UYVY and RGB888 modes in CSI-2 mode.
Jacopo Mondi [Fri, 13 May 2022 14:13:56 +0000 (15:13 +0100)]
media: ov5640: Rework timings programming
The current definition of a sensor mode defines timings as follows:
- hact, vact: Visible width and height
- htot, vtot: Total sizes including blankings
This makes difficult to clearly separate the visible sizes from the
blankings and to make the vertical blanking programmable.
Rework the sensor modes sizes definition to:
- Report the analog crop sizes
- Report the visible crop size
- Report the total pixels per line as HBLANK is fixed
- Report the VBLANK value to make it programmable
Also modify the ov5640_set_timings() function to program all the
windowing registers are remove them from the per-mode register-value
tables.
Do not change the timing values from the ones reported in the register
tables to maintain bisectability.
Jacopo Mondi [Fri, 13 May 2022 14:13:55 +0000 (15:13 +0100)]
media: ov5640: Rework CSI-2 clock tree
Re-work the ov5640_set_mipi_pclk() function to calculate the
PLL configuration using the pixel_rate and link_freq values set at
s_fmt time.
Rework the DVP clock mode settings to calculate the pixel clock
internally and remove the assumption on the 16bpp format.
Tested in MIPI CSI-2 mode with 1 and 2 data lanes with:
- all the sensor supported resolutions in UYVY, RGB565 and MJPEG formats.
- resolutions >= 1280x720 in RAW Bayer format.
- resolutions < 1280x720 in RGB888 format.
Jacopo Mondi [Fri, 13 May 2022 14:13:54 +0000 (15:13 +0100)]
media: ov5640: Update pixel_rate and link_freq
After having set a new format re-calculate the pixel_rate and link_freq
control values and update them when in MIPI mode.
Take into account the limitation of the link frequency having to be
strictly smaller than 1GHz when computing the desired link_freq, and
adjust the resulting pixel_rate acounting for the clock tree
configuration.
Paul Kocialkowski [Wed, 25 May 2022 19:03:00 +0000 (20:03 +0100)]
media: sunxi: Add support for the A83T MIPI CSI-2 controller
The A83T supports MIPI CSI-2 with a composite controller, covering
both the protocol logic and the D-PHY implementation. This controller
seems to be found on the A83T only and probably was abandoned since.
This implementation splits the protocol and D-PHY registers and
uses the PHY framework internally. The D-PHY is not registered as a
standalone PHY driver since it cannot be used with any other
controller.
There are a few notable points about the controller:
- The initialisation sequence involes writing specific magic init
values that do not seem to make any particular sense given the
concerned register fields;
- Interrupts appear to be hitting regardless of the interrupt mask
registers, which can cause a serious flood when transmission errors
occur.
Only 8-bit and 10-bit Bayer formats are currently supported.
While up to 4 internal channels to the CSI controller exist, only one
is currently supported by this implementation.
This work is based on the first version of the driver submitted by
Kévin L'hôpital, which was adapted to mainline from the Allwinner BSP.
This version integrates MIPI CSI-2 support as a standalone V4L2 subdev
instead of merging it in the sun6i-csi driver.
It was tested on a Banana Pi M3 board with an OV8865 sensor in a 4-lane
configuration.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Acked-by: Maxime Ripard <mripard@kernel.org> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Paul Kocialkowski [Wed, 25 May 2022 19:02:57 +0000 (20:02 +0100)]
media: sunxi: Add support for the A31 MIPI CSI-2 controller
The A31 MIPI CSI-2 controller is a dedicated MIPI CSI-2 bridge
found on Allwinner SoCs such as the A31 and V3/V3s.
It is a standalone block, connected to the CSI controller on one side
and to the MIPI D-PHY block on the other. It has a dedicated address
space, interrupt line and clock.
It is represented as a V4L2 subdev to the CSI controller and takes a
MIPI CSI-2 sensor as its own subdev, all using the fwnode graph and
media controller API.
Only 8-bit and 10-bit Bayer formats are currently supported.
While up to 4 internal channels to the CSI controller exist, only one
is currently supported by this implementation.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Acked-by: Maxime Ripard <mripard@kernel.org> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Paul Kocialkowski [Wed, 25 May 2022 19:02:55 +0000 (20:02 +0100)]
media: dt-bindings: media: sun6i-a31-csi: Add MIPI CSI-2 input port
The A31 CSI controller supports two distinct input interfaces:
parallel and an external MIPI CSI-2 bridge. The parallel interface
is often connected to a set of hardware pins while the MIPI CSI-2
bridge is an internal FIFO-ish link. As a result, these two inputs
are distinguished as two different ports.
Note that only one of the two may be present on a controller instance.
For example, the V3s has one controller dedicated to MIPI-CSI2 and one
dedicated to parallel.
Update the binding with an explicit ports node that holds two distinct
port nodes: one for parallel input and one for MIPI CSI-2.
This is backward-compatible with the single-port approach that was
previously taken for representing the parallel interface port, which
stays enumerated as fwnode port 0.
Note that additional ports may be added in the future, especially to
support feeding the CSI controller's output to the ISP.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Reviewed-by: Rob Herring <robh@kernel.org> Acked-by: Maxime Ripard <mripard@kernel.org> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Nicolas Frattaroli [Sun, 12 Jun 2022 15:53:45 +0000 (16:53 +0100)]
media: hantro: Add support for RK356x encoder
The RK3566 and RK3568 SoCs come with a small Hantro instance which is
solely dedicated to encoding. This patch adds the necessary structs to
the Hantro driver to allow the JPEG encoder of it to function.
Through some sleuthing through the vendor's MPP source code and after
closer inspection of the TRM, it was determined that the hardware likely
supports VP8 and H.264 as well.
Nicolas Frattaroli [Sun, 12 Jun 2022 15:53:44 +0000 (16:53 +0100)]
media: dt-binding: media: Add rk3568-vepu binding
The RK3568 and RK3566 have a Hantro VPU node solely dedicated to
encoding. This patch adds a new binding to describe it, as it
does not really fit the rockchip-vpu binding, since there is no
decoder.
Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com> Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Ming Qian [Tue, 28 Jun 2022 05:19:52 +0000 (06:19 +0100)]
media: amphion: release core lock before reset vpu core
In reset vpu core, driver will wait for a response event,
but if there are still some events unhandled,
they will be handled first, driver may acquire core lock for that.
So if we do reset in core lock, it may led to reset timeout.
The mdp_ipi_comm structure defines a command that is either
PROCESS (start processing) or DEINIT (destroy instance); we
are using this one to send PROCESS or DEINIT commands from Linux
to an MDP instance through a VPU write but, while the first wants
us to stay 4-bytes aligned, the VPU instead requires an 8-bytes
data alignment.
Keeping in mind that these commands are executed immediately
after sending them (hence not chained with others before the
VPU/MDP "actually" start executing), it is fine to simply add
a padding of 4 bytes to this structure: this keeps the same
performance as before, as we're still stack-allocating it,
while avoiding hackery inside of mtk-vpu to ensure alignment
bringing a definitely bigger performance impact.
Fixes: c8eb2d7e8202 ("[media] media: Add Mediatek MDP Driver") Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Reviewed-by: Houlong Wei <houlong.wei@mediatek.com> Reviewed-by: Irui Wang <irui.wang@mediatek.com> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>