- 15 Nov, 2021 40 commits
-
-
Mauro Carvalho Chehab authored
Simplify the code by getting rid of this function, as it ends being just a single line of code. Link: https://lore.kernel.org/linux-media/80ea920d14379124ba92aab2e6a6d12a92d79b2b.1636544620.git.mchehab+huawei@kernel.orgSigned-off-by:
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
-
Mauro Carvalho Chehab authored
The open() fops support two types of devices: "acc" and normal ones. the acc works on a different way, using a different pipe struct. Not sure yet if it would make sense to setup a run_mode there. Also, As default_run_mode exists only on normal modes, we can simplify the logic to check if the device is in normal mode. That solves this warning: ../drivers/staging/media/atomisp/pci/atomisp_fops.c:904 atomisp_open() warn: variable dereferenced before check 'asd' (see line 807) Signed-off-by: -
Mauro Carvalho Chehab authored
The asd->isp was referenced before checking if asd is not NULL. This fixes this warning: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:5548 atomisp_set_fmt_to_snr() warn: variable dereferenced before check 'asd' (see line 5540) While here, avoid getting the pipe pointer twice. Signed-off-by: -
Mauro Carvalho Chehab authored
During atomisp register, udev tries to open all devices. For some, pipe is NULL, at least during register time, causing the driver to try to access a NULL pointer. So, add an extra check to avoid such condition. Link: https://lore.kernel.org/linux-media/6406265f2d5f3791d5c7cbd1364186217f19524c.1636364423.git.mchehab+huawei@kernel.orgSigned-off-by:
-
Dan Carpenter authored
The "power" pointer is not initialized on the else path and that would lead to an Oops. Link: https://lore.kernel.org/linux-media/20211012082150.GA31086@kili Fixes: c30f4cb2 ("media: atomisp: Refactor PMIC detection to a separate function") Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by:
Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Signed-off-by:
-
Mauro Carvalho Chehab authored
The vts value should be set before being checked, as otherwise a warning will arise: drivers/staging/media/atomisp/i2c/atomisp-ov2680.c: In function 'ov2680_set_fmt': drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:873:33: warning: 'vts' may be used uninitialized [-Wmaybe-uninitialized] 873 | if (dev->exposure > vts - OV2680_INTEGRATION_TIME_MARGIN) Reported-by:
Hans Verkuil <hverkuil@xs4all.nl> Reviewed-by:
Hans de Goede <hdegoede@redhat.com> Fixes: 62b984359b6f ("media: atomisp-ov2680: Fix ov2680_set_fmt() messing up high exposure settings") Signed-off-by:
-
Mauro Carvalho Chehab authored
As the settings are only applied when the device is powered on, it should return 0 when the device is not powered. Not doing that causes a warning: drivers/staging/media/atomisp/i2c/atomisp-ov2680.c: In function 'ov2680_ioctl': drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:390:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized] 390 | return ov2680_set_exposure(sd, coarse_itg, analog_gain, digital_gain); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:359:13: note: 'ret' was declared here 359 | int ret; | ^~~ Reported-by:
-
Hans de Goede authored
For exposure settings > (lines_per_frame - vts_margin) the VTS register needs to be programmed to (exposure + vts_margin) rather then being set to lines_per_frame. The res->regs register array was clobbering this higher setting causing high exposure settings to not work. Fix this by letting ov2680_set_fmt() calculate the vts value, instead of hardcoding it. This is the last in a series of fixes which fixes exposure and gain settings not working, with this everything works, so drop the comment that it does not work. Link: https://lore.kernel.org/linux-media/20211107171549.267583-12-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Now that we restore the default or last user set exposure setting on power_up() there is no need for the registers written by ov2680_set_fmt() to write to the exposure register. Not doing so fixes the exposure always being reset to the value from the res->regs array after a set_fmt(). Link: https://lore.kernel.org/linux-media/20211107171549.267583-11-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
The shift << 16 of the value in the code path for 16 bit values is bogus, put_unaligned_be16() takes the lower 16 bits which will not always be 0. This was causing __ov2680_set_exposure() to always set the OV2680_AGC and OV2680_TIMING_VTS registers to 0. Link: https://lore.kernel.org/linux-media/20211107171549.267583-10-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Switch to ov2680_read_reg() to read all 24 bits in one go; and the exposure value sits in bits 4-19 of the 24 bit exposure register, so we need to shift the read value by 4 to report the correct value. Link: https://lore.kernel.org/linux-media/20211107171549.267583-9-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Some ov2680 registers like exposure are 24 bit, ov2680_read_reg() already mostly supports this, we just need to change the return type from u16 to u32. Link: https://lore.kernel.org/linux-media/20211107171549.267583-8-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Save/restore exposure and gain over sensor power-down and don't write them to the sensor when ov2680_set_exposure() is called while the sensor is off. Link: https://lore.kernel.org/linux-media/20211107171549.267583-7-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Move ov2680_init_registers() call to power_up(), so that we also init the registers on code-paths which do not call ov2680_s_power() like running camorama. Link: https://lore.kernel.org/linux-media/20211107171549.267583-6-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
The ov2680_res and N_RES global variables are just hardcoded as aliases for ov2680_res_preview and N_RES_PREVIEW, remove them. Link: https://lore.kernel.org/linux-media/20211107171549.267583-5-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
ov2680_s_power() is the only caller of ov2680_init(), push the input_lock taking from ov2680_init() up into ov2680_s_power(), this way the new power_on bool is also protected by it. Link: https://lore.kernel.org/linux-media/20211107171549.267583-4-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Add a power_on bool to track if the power is on, and make power_up() a no-op if the power is already on. This also removes a power_down() call from ov2680_s_config() since that is a no-op now, this is ok because s_config() is only called once on probe and the sensor is off at boot. Besides avoiding to the work in power_up() multiple times this patch is also a preparation for switching to the clk and regulator frameworks which keep an enable count, so there we must call enable() and disable() only once per power-cycle. Link: https://lore.kernel.org/linux-media/20211107171549.267583-3-hdegoede@redhat.comSigned-off-by:
-
Hans de Goede authored
Remove a couple of variables which where either completely unused, or only ever got a value assigned to them but were never read. Link: https://lore.kernel.org/linux-media/20211107171549.267583-2-hdegoede@redhat.comSigned-off-by:
-
Mauro Carvalho Chehab authored
The atomisp currenyl registers 5 pairs of devices each one for one different run_mode, plus one for "ACC". The only one that behaves like a normal V4L2 device is the preview one. The others are doing weird things, and perhaps are using some proprietary extensions to the API. Change the device order to start with the preview one, e. g: /dev/video0: ATOMISP ISP PREVIEW output /dev/video1: ATOMISP ISP CAPTURE output /dev/video2: ATOMISP ISP VIEWFINDER output /dev/video3: ATOMISP ISP VIDEO output /dev/video4: ATOMISP ACC This way, a normal V4L2 application will get the right device, as the first one will be the one they should use. Signed-off-by: -
Mauro Carvalho Chehab authored
The atomisp driver originally used the s_parm command to initialize the run_mode type to the driver. So, before start setting up the streaming, s_parm should be called. So, even having 5 "normal" video devices, one meant to be used for each type, the run_mode was actually selected when s_parm is called. Without setting the run mode, applications that don't call VIDIOC_SET_PARM with a custom atomisp parameters won't work, as the pipeline won't be set: atomisp-isp2 0000:00:03.0: can't create streams atomisp-isp2 0000:00:03.0: __get_frame_info 1600x1200 (padded to 0) returned -22 However, commit 8a7c5594 ("media: v4l2-ioctl: clear fields in s_parm") broke support for it, with a good reason, as drivers shoudn't be extending the API for their own purposes. So, as an step to allow generic apps to use this driver, put the device's run_mode in preview after open. After this patch, using v4l2grab starts to work on preview mode (/dev/video2): $ v4l2grab -f YUYV -x 1600 -y 1200 -d /dev/video2 -n 1 -u $ feh out000.pnm So, let's just setup the default run_mode that each video devnode should assume, setting it at open() time. Reported-by:
Tsuchiya Yuto <kitakar@gmail.com> Signed-off-by:
-
Mauro Carvalho Chehab authored
All ISP2401 devices use the new input system. So, get rid of the remaining definitions, replacing them by runtime checks for BYT/CHT when applicable. Signed-off-by: -
Mauro Carvalho Chehab authored
Now that the pipeline config functions can return errors, change ia_css_dma_configure_from_info() and callers in order for them to return errors at pipelines instead of using assert(). Signed-off-by: -
Mauro Carvalho Chehab authored
Those functions can internally break, but, as they don't return errors, internally there are some assert() calls, which is bad, as it hangs the driver. So, add return codes there, in preparation for removing such assert() calls. Signed-off-by: -
Mauro Carvalho Chehab authored
If something gets wrong while setup a pipeline, return an error code. Signed-off-by: -
Mauro Carvalho Chehab authored
Propagate the lower lever ia_css config errors to the next level. Signed-off-by: -
Mauro Carvalho Chehab authored
The auto-generated code inside ia_css_isp_configs() is more complex than it should be. Also, it doesn't return any errors. However, the functions called by it can mis-configure the pipelines, but, as there's no way to return errors, it internally calls the assert() macro. So, add a return parameter to each routine there, in order to prepare the code to be more robust. Signed-off-by: -
Mauro Carvalho Chehab authored
The contents of ia_css_isp_params.c is almost identical for 2400 and 2401. The only difference is that, on 2400, there are some duplicated assignments. So, drop it, unifying this file. While here, simplify the Makefile's logic by dropping an unused define. Signed-off-by: -
Mauro Carvalho Chehab authored
Both 2400 and 2401 have this file, but they're identical. So, drop one of them. Signed-off-by: -
Mauro Carvalho Chehab authored
Both 2400 and 2401 have this file, but they're identical. So, drop one of them. Signed-off-by: -
Mauro Carvalho Chehab authored
This avoids an OOPS when VIDIOC_*_PRIORITY calls are used. [ 90.820011] BUG: kernel NULL pointer dereference, address: 0000000000000020 [ 90.820021] #PF: supervisor read access in kernel mode [ 90.820026] #PF: error_code(0x0000) - not-present page [ 90.820030] PGD 1221bc067 P4D 1221bc067 PUD 1221bd067 PMD 0 [ 90.820042] Oops: 0000 [#1] SMP [ 90.820048] CPU: 0 PID: 2007 Comm: qv4l2 Tainted: G C 5.15.0-rc4+ #77 [ 90.820055] Hardware name: ASUSTeK COMPUTER INC. T101HA/T101HA, BIOS T101HA.306 04/23/2019 [ 90.820059] RIP: 0010:v4l2_prio_change+0xf/0x40 [videodev] [ 90.820096] Code: 48 8b 47 20 8b 40 44 25 ff ff 0f 00 48 8b 04 c5 20 58 44 c0 c3 66 0f 1f 44 00 00 0f 1f 44 00 00 8d 42 ff 83 f8 02 77 21 31 c0 <39> 16 74 20 8d 02 f0 ff 04 87 8b 06 89 c1 83 e9 01 83 f9 02 77 04 [ 90.820103] RSP: 0018:ffffb348c142fd70 EFLAGS: 00010246 [ 90.820109] RAX: 0000000000000000 RBX: ffff95575fbe2760 RCX: ffffb348c142fe48 [ 90.820114] RDX: 0000000000000002 RSI: 0000000000000020 RDI: ffff955740996088 [ 90.820118] RBP: 0000000040045644 R08: ffffffffc070da80 R09: 0000000000004000 [ 90.820122] R10: 0000000000000000 R11: 0000000000000000 R12: ffff955766725300 [ 90.820126] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000044 [ 90.820131] FS: 00007f848526b900(0000) GS:ffff9557bbe00000(0000) knlGS:0000000000000000 [ 90.820137] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 90.820141] CR2: 0000000000000020 CR3: 00000001221bf000 CR4: 00000000001006f0 [ 90.820146] Call Trace: [ 90.820155] __video_do_ioctl+0x37d/0x3b0 [videodev] [ 90.820189] video_usercopy+0x188/0x7a0 [videodev] [ 90.820218] ? v4l_print_control+0x20/0x20 [videodev] [ 90.820247] ? swake_up_one+0x39/0x70 [ 90.820258] ? rcu_core+0xff/0x2e0 [ 90.820267] ? sched_clock_cpu+0x9/0xa0 [ 90.820275] ? irqtime_account_irq+0x38/0xb0 [ 90.820282] v4l2_ioctl+0x46/0x50 [videodev] Signed-off-by: -
Mauro Carvalho Chehab authored
Instead of reinventing the wheel, use v4l2_find_nearest_size() in order to get the closest resolution. This should address a bug where the wrong resolution was selected. Signed-off-by: -
Mauro Carvalho Chehab authored
Instead of reinventing the wheel, use v4l2_find_nearest_size() in order to get the closest resolution. This should address a bug where the wrong resolution was selected. Signed-off-by: -
Mauro Carvalho Chehab authored
Instead of reinventing the wheel, use v4l2_find_nearest_size() in order to get the closest resolution. This should address a bug where the wrong resolution was selected. Signed-off-by: -
Mauro Carvalho Chehab authored
Instead of reinventing the wheel, use v4l2_find_nearest_size() in order to get the closest resolution. This should address a bug where the wrong resolution was selected. Signed-off-by: -
Mauro Carvalho Chehab authored
Instead of reinventing the wheel, use v4l2_find_nearest_size() in order to get the closest resolution. This should address a bug where the wrong resolution was selected. Signed-off-by: -
Mauro Carvalho Chehab authored
The g_fmt logic is currently broken, as it is not returning the same imagesize as previoulsy calculated by s_fmt. Fix it by just re-using whatever it was defined at s_fmt, if this was called before. Signed-off-by: -
Mauro Carvalho Chehab authored
The internal try_fmt logic is not meant to provide everything that the V4L2 API should provide. Also, it doesn't decrement the pads that are used only internally by the driver, but aren't part of the device's output. Fix it. Signed-off-by: -
Mauro Carvalho Chehab authored
move atomisp_g_fmt_cap() for it to be after try_fmt, as we'll re-use try_fmt there. No functional changes. Signed-off-by: -
Mauro Carvalho Chehab authored
Currently, the enum lists the sensor's native format as a supported format. However, trying to setup a pipeline using it doesn't work. So, exclude such formats from the enum. Signed-off-by: -
Mauro Carvalho Chehab authored
There are several issues on S_FMT implementation: - it doesn't properly handle pad_h/pad_w; - it reports a wrong visible size to userspace; - it allows setting the format to a raw mode, which currently causes the pipeline to break. Address such issues, for it to start working with generic apps. Signed-off-by:
-
