- Get rid of typedefs;
- Get rid of a duplicated enum type with different names for
ISP2400 and ISP2401;
- adjust indentation on the touched code.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
There's a different table for some BYT variants that depend
on something inside a FIXME ifdef.
Place this also inside it, just to shut up a clang-11 warning.
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The abstraction layer for kvfree() was removed, but there
is still a left-over code there.
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
When building with clang, multiple copies of the structures to be
initialized are passed around on the stack and copied locally, using an
insane amount of stack space:
drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
Use constantly-allocated variables plus an explicit memcpy()
to avoid that.
Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Fixes: 6dc9a2568f ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
For most warnings, the current code is OK. There are still
some issues with implicit-fallthough warnings.
Solve those and re-enable all warnings for this driver.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
This table used to be used also to translate between ia_css
abstraction and V4L2 fourcc codes.
This was removed on a past patch, but the table now contains
two fields with identical values.
Get rid of one of them.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Without that driver, there is a link failure in
ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
[drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!
Add an explicit Kconfig dependency.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
clang points out the usage of an incorrect enum type in the
list of supported image formats:
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
Checking the git history, I found a commit that disabled one such case
because it did not work. It seems likely that the incorrect enum was
part of the original problem and that the others do not work either,
or have never been tested.
Disable all the ones that cause a warning.
Fixes: cb02ae3d71 ("media: staging: atomisp: Disable custom format for now")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Some function calls pass an incorrect enum type:
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
gp_device_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion]
config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
of the expected types instead.
Fixes: a49d25364d ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
In some configurations, including this header leads to a warning:
drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility]
Make sure the struct tag is known before declaring a function
that uses it as an argument.
Fixes: 9d4fa1a16b ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
This driver is licensed under GPL 2.0, as stated inside their
headers.
Add the proper tag there. We should probably latter cleanup
the reduntant licensing text, but this could be done later,
after we get rid of other abstraction layers.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
If something gets wrong, return, instead of trying to
convert from a NULL pointer.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Atomisp compat IOCTL handling suffers from the same security issue than
the V4L2 did. Fix this for atomisp.
See more information in patch a1dfb4c48c ("media: v4l2-compat-ioctl32.c:
refactor compat ioctl32 logic").
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The struct atomisp_overlay contains overlay_start_x and overlay_start_y
fields. Instead of copying the value of the overlay_start_x field between
the two structs, the value of the overlay_start_y field of the compat
struct was copied to the overlay_start_x field of the 64-bit kernel struct
in get operation and back in put. The overlay_start_x field value was not
copied from or to the user space struct.
Fix this so that the value of overlay_start_x is copied to overlay_start_x
and the value of overlay_start_y is copied to overlay_start_y.
Also do copy blend_overlay_perc_u field only once.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The ISP firmware logic is complex, as several binaries are
contained into a single file.
Print debug messages:
- with a stack dump if binary not found;
- when a firmware is selected.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
In order to check if aren't there any memory leaks, let's
add a debug print for hmm_free().
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
It can be useful to be able to test different firmware files
at modprobe time, in order to be able to test different
variants without much efforts.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
While there's a way to list the firmware binaries in runtime,
it is worth to also print it during firmware load.
One advantage is that this code also introduces additional
checks with regards to invalid firmware types, which can be
useful to identify problems.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
In order to make easier to identify what a firmware file
contains, add more info at the firmware dump log facility.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
There are too many wrapper functions at atomisp_compat_css20.c.
Get rid of another set of such wrappers.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
When probe fails, it is possible that hmm_init() to not be
called. On such case, hmm_cleanup() will cause a WARN_ON().
Avoid it by adding an explicit check at hmm_cleanup() to
ensure that the hmm code was properly initialized.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
There is an abstraction layer there meant to convert to
the Linux standard error codes. As the driver now use
such errors everywhere. we can get rid of this.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The atomisp driver has its own error codes under the
ia_css_err.h file. On several places, those got already
replaced by standard error codes, but there are still a
lot more to be fixed.
Let's get rid of all of those, mapping them into
the already-existing set of Linux error codes.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Run checkpatch --fix-inline again, in order to get rid
of some additional issues that got introduced (or that
checkpatch can now detect).
This should help preventing receiving random cleanups,
while keeping the code on a better shape.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
There are tons of code inside atomisp_compat_css20.c, but
several of them are just trivial wrappers to other functions.
Getting rid of all of them will take some time, but let's
start getting rid of some of the trivial ones.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Use the variant which zeroes the memory when allocating,
instead of having an explicit memset.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64:
warning: implicit conversion from enumeration type 'const enum
ia_css_frame_format' to different enumeration type 'enum
atomisp_input_format' [-Wenum-conversion]
binary_supports_input_format(xcandidate, req_in_info->format));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~^~~~~~
As it turns out, binary_supports_input_format only asserts that
xcandidate is not NULL and just returns true so this call is never
actually made.
There are other functions that are called that assert info is not NULL
so this function actually serves no purpose. Remove it. It can be
brought back if needed later.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35:
warning: implicit conversion from 'unsigned long' to 'int32_t' (aka
'int') changes value from 18446744073709543424 to -8192
[-Wconstant-conversion]
return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR);
~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
XNR_BLENDING_SCALE_FACTOR is BIT(13), or 8192, which will easily fit
into a signed 32-bit integer. However, it is an unsigned long, which
means that negating it is the same as subtracting that value from
ULONG_MAX + 1, which causes it to be larger than a signed 32-bit
integer so it gets implicitly converted.
We can avoid this by using the variable isp_scale, which holds the value
of XNR_BLENDING_SCALE_FACTOR already, where the implicit conversion from
unsigned long to s32 already happened. If that were to ever overflow,
clang would warn: https://godbolt.org/z/EeSxLG
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: warning:
address of 'config->info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!&config->info) {
~ ~~~~~~~~^~~~
config cannot be NULL because it comes from an ioctl, which ensures that
the user is not giving us an invalid pointer through copy_from_user. If
config is not NULL, info cannot be NULL. Remove this check.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
../drivers/staging/media/atomisp/pci/sh_css.c:8537:14: warning: address
of 'pipe->output_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (&pipe->output_stage)
~~ ~~~~~~^~~~~~~~~~~~
../drivers/staging/media/atomisp/pci/sh_css.c:8545:14: warning: address
of 'pipe->vf_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (&pipe->vf_stage)
~~ ~~~~~~^~~~~~~~
output_stage and vf_stage are pointers in the middle of a struct, their
addresses cannot be NULL if pipe is not NULL and pipe is already checked
for NULL in this function. Simplify this if block.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1097:3: warning:
variable 'count' is incremented both in the loop header and in the loop
body [-Wfor-loop-analysis]
count++;
^
This was probably unintentional, remove it.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Clang warns:
../drivers/staging/media/atomisp/pci/sh_css_sp.c:1039:23: warning:
address of 'binary->in_frame_info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
} else if (&binary->in_frame_info) {
~~ ~~~~~~~~^~~~~~~~~~~~~
in_frame_info is not a pointer so if binary is not NULL, in_frame_info's
address cannot be NULL. Change this to an else since it will always be
evaluated as one.
While we are here, clean up this if block. The contents of both if
blocks are the same but a check against "stage == 0" is added when
ISP2401 is defined. USE_INPUT_SYSTEM_VERSION_2401 is only defined when
isp2401_system_global.h is included, which only happens when ISP2401. In
other words, USE_INPUT_SYSTEM_VERSION_2401 always requires ISP2401 to be
defined so the '#ifndef ISP2401' makes no sense. Remove that part of the
block to simplify everything.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
stage->args->delay_frames array could point to NULL frames.
What's weird is that we didn't notice this behavior with the
Intel Aero Yocto code.
Handle it, while adding a notice at the code, as this could
be due to some broken pipeline setup.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The sh_css layer adds an abstraction for kvmalloc/kvcalloc.
Get rid of them. Most of the work here was done by this
small coccinelle script:
<cocci>
@@
expression size;
@@
- sh_css_malloc(size)
+ kvmalloc(size, GFP_KERNEL)
@@
expression n;
expression size;
@@
- sh_css_calloc(n, size)
+ kvcalloc(n, size, GFP_KERNEL)
</cocci>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The frame allocation logic happens differently for userptr
or normal mmap. On a quick look, this sounded to be unbalanced,
but the logic should actually work for both cases.
Add an extra comment to reflect it.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
If something gets wrong when enabling or disabling an IRQ,
we should know better about what happened.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The hmm code is still complex and has bugs. Add a debug print
when memory gets allocated, in order to help identifying what's
happening out there.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Instead of using a hacked version of an old copy of
get_user_pages(), use pin_user_pages().
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The hmm code is partially based on a fork from 3.10 code,
and has bugs.
Add debug there to help tracking what happens there.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The kernel ABI was extended to allow pass tagged user pointers.
Untag the pointers in this function.
Fixes: d93445225c ("uaccess: add noop untagged_addr definition")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Now that we have everything in place, we can get rid of the
memory_access abstraction layer.
Now, everything related to heterogeneous memory management
(hmm) is under hmm.c & related pools.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
The mmgr alloc code returns a different type than hmm, due to
some abstraction layer.
Change the driver to use just one type to represent the
hmm memory.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
