Arnd Bergmann [Fri, 29 May 2020 20:00:27 +0000 (22:00 +0200)]
media: staging: media: atomisp: fix stack overflow in init_pipe_defaults()
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: 2615bb5fd62d ("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>
Arnd Bergmann [Fri, 29 May 2020 20:00:30 +0000 (22:00 +0200)]
media: staging: media: atomisp: disable all custom formats
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.
Arnd Bergmann [Fri, 29 May 2020 20:00:29 +0000 (22:00 +0200)]
media: staging: media: atomisp: fix enum type mixups
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: f40413881fa2 ("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>
Arnd Bergmann [Fri, 29 May 2020 20:00:24 +0000 (22:00 +0200)]
media: staging: media: atomisp: declare 'struct device' before using it
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.
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.
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>
media: atomisp: get rid of an error abstraction layer
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.
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.
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.
media: atomisp: Avoid overflow in compute_blending
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
media: atomisp: Remove unnecessary NULL check in atomisp_param
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.
media: atomisp: Remove unnecessary NULL checks in ia_css_pipe_load_extension
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.
media: atomisp: Remove second increment of count in atomisp_subdev_probe
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++;
^
media: atomisp: Clean up if block in sh_css_sp_init_stage
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.
media: atomisp: add more comments about frame allocation
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.
Colin Ian King [Thu, 21 May 2020 19:44:32 +0000 (21:44 +0200)]
media: atomisp: fix a handful of spelling mistakes
There are several spelling mistakes in various messages and literal
strings. Fix these.
Signed-off-by: Colin Ian King <colin.king@canonical.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
In other words, we have:
MAX_NUM_VIDEO_DELAY_FRAMES = 3
MAX_NUM_DELAY_FRAMES = 2
The MAX_NUM_DELAY_FRAMES macro is used only only when allocating
memory. On all other parts, including looping over such array,
MAX_NUM_VIDEO_DELAY_FRAMES is used instead, like:
void sh_css_binary_args_reset(struct sh_css_binary_args *args)
{
unsigned int i;
...
for (i = 0; i < MAX_NUM_VIDEO_DELAY_FRAMES; i++)
args->delay_frames[i] = NULL;
Which will cause buffer overflows, with may override the next array
(tnr_frames[]).
In practice, this may not be causing real issues, as the code
checks for num_delay_frames on some parts (but not everywhere).
So, get rid of the smallest value.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Some arguments for tnf and ref settings are meant to be const, but
they're defined without such annotation. Due to that, there's an
ugly cast at sh_css_sp.c.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
media: atomisp: get rid of some old broken debug code
It sounds that someone once changed the debug level at compile
time for some testing, but forgot to remove the legacy code after
finishing debuging it.
Get rid of the dead code.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
media: atomisp: don't flood dmesg with -EAGAIN return codes
Using DQBUF on non-blocking mode will return -EAGAIN
if nothing arrives. Printing it has no value, even for debug
purposes. So, only display real return codes.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Flooding the dmesg with lots of messages per second. The same
pattern happens for all other IRQs.
Change the logic for printing just one message per IRQ and
rate-limit those, as, for debugging purposes, it is usually
interesting to know that IRQs are being received, but not
displaying every single one.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
media: s5k5baf: Replace zero-length array with flexible-array
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
media: i2c: imx219: Drop <linux/clk-provider.h> and <linux/clkdev.h>
The IMX219 camera driver is not a clock provider, but merely a clock
consumer, and thus does not need to include <linux/clk-provider.h> and
<linux/clkdev.h>.
Bingbu Cao [Mon, 11 May 2020 13:41:14 +0000 (15:41 +0200)]
media: i2c: Add ov2740 image sensor driver
OminiVision ov2740 is a 2 megapixels RAW RGB image sensor which can
deliver 1920x1080@60fps frames. This driver add the support of
vertical blanking, exposure, test pattern, digital and analog gain
control for sensor.
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> Signed-off-by: Shawn Tu <shawnx.tu@intel.com> Signed-off-by: Qiu, Tianshu <tian.shu.qiu@intel.com> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Dirk Nehring [Mon, 11 May 2020 08:29:53 +0000 (10:29 +0200)]
media: dvb-usb: Add Cinergy S2 PCIe Dual Port support
Terratec Cinergy S2 PCIe Dual is a PCIe device with two tuners that
actually contains two USB devices. The devices are visible in the
lsusb printout.
Bus 004 Device 002: ID 153b:1182 TerraTec Electronic GmbH Cinergy S2 PCIe Dual Port 2
Bus 003 Device 002: ID 153b:1181 TerraTec Electronic GmbH Cinergy S2 PCIe Dual Port 1
The devices use the Montage M88DS3000/M88TS2022 demod/tuner.
Signed-off-by: Dirk Nehring <dnehring@gmx.net> Signed-off-by: Sean Young <sean@mess.org> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Brad Love [Thu, 14 May 2020 16:40:43 +0000 (18:40 +0200)]
media: dvbdev: Fix tuner->demod media controller link
Fixes bug exposed by:
[50637b9020b: media: mc-entity.c: use WARN_ON, validate link pads]
The dvbdev incorrectly requests a tuner sink pad to connect to a demux
sink pad. The media controller failure percolates back and the dvb device
creation fails. Fix this by requesting a tuner source pad. Instead of
forcing that pad to be index zero, check if a negative integer error
is returned. A note is added that first source pad found is chosen.
Affected bridges cx231xx and em28xx printed the below warning[s]
when a variety of media controller dvb enabled devices were connected.
The warning returns an error causing all affected devices to fail DVB
device creation.
media: atomisp: don't produce errs for ignored IRQs
Depending on the ISP-specific HAS_NO_INPUT_FORMATTER macro,
some IRQs will be ignored by the driver. Yet, those keep
happening, as reported by this debug print:
[ 61.620746] atomisp-isp2 0000:00:03.0: atomisp_css_irq_enable: css irq info 0x00000004: disable.
Causing this warning:
[ 61.620749] atomisp-isp2 0000:00:03.0: atomisp_css_irq_enable:Invalid irq info.
Well, if this is a normal situation, just ignore it without
warnings.
Instead of using it only if system memory is below 2GB,
don't use it at all. The problem is that the code there is not
compatible anymore with modern Kernels:
media: atomisp: add a notice about possible leak resources
Calling acpi_bus_get_device() may end allocating resources that
aren't freed. So, add a notice about that, as, if those drivers
get out of staging, we may need some changes.
media: atomisp: disable the dynamic and reserved pools
The memory management code for atomisp is complex: it has 2
extra pools (plus some ION-specific code).
The code for those extra pools are complex, and there are even
some parts of code over there that were forked from some
mm/ code, probably from Kernel 3.10.
Let's just use a single one, in order to make the driver
simpler.
media: atomisp: change the code to properly wait for sensor
The sensor should finish its init before atomisp driver, as
otherwise the atomisp driver won't be able to talk with it.
So, we need to turn atomisp_gmin_platform into a module
again, for it to not depend on atomisp driver to finish
probing, and add some delay at atomisp to let the sensor
driver to finish probing.
Yeah, this is hacky. The real solution here would be to use
the async framework, but for now, our goal is to make the
driver to work. So, let's postpone such change to be done
later.
media: atomisp: fix a slab error due to a wrong free
The mmu mapping logic uses a different logic depending on the
RAM size: if it is lower than 2GB, it uses kmem_cache_zalloc(),
but if memory is bigger than that, it uses its own way to
allocate memory.
Yet, when freeing, it uses kmem_cache_free() for any cases.
On recent Kernels, slab tracks the memory allocated on it,
with causes those warnings:
projects / kernel.git / log