- 08 Feb, 2023 40 commits
-
-
Hans de Goede authored
Looking at csi2_link_setup(), this function can never work, it does a switch-case like this: switch (local->index | is_media_entity_v4l2_subdev(remote->entity)) with cases like this: case ATOMISP_SUBDEV_PAD_SOURCE | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN where ATOMISP_SUBDEV_PAD_SOURCE matches an index (0-1) and MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN is 0x00020000, but is_media_entity_v4l2_subdev(remote->entity) does not return MEDIA_ENT_F_* values, it return a bool, so 0 or 1 which means that non of the cases can ever match the input value. Looking at the rest of the function all it ever does (if it would actually hit one of the cases) is set the atomisp_mipi_csi2_device struct's output member. And checking the rest of the atomisp code that member is never read. Also userspace does not actually setup media-controller links when using the atomisp /dev/video$ nodes since all the links are fixed. So csi2_link_setup() never runs. Remove the unnecessary and broken csi2_link_setup() function and also remove the unused atomisp_mipi_csi2_device struct's output member. Signed-off-by:
Hans de Goede <hdegoede@redhat.com> Signed-off-by:
Mauro Carvalho Chehab <mchehab@kernel.org>
-
Hans de Goede authored
Looking at isp_subdev_link_setup(), this function can never work, it does a switch-case like this: switch (local->index | is_media_entity_v4l2_subdev(remote->entity)) with cases like this: case ATOMISP_SUBDEV_PAD_SINK | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN where ATOMISP_SUBDEV_PAD_SINK matches an index (0-4) and MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN is 0x00020000, but is_media_entity_v4l2_subdev(remote->entity) does not return MEDIA_ENT_F_* values, it return a bool, so 0 or 1 which means that non of the cases can ever match the input value. Looking at the rest of the function all it ever does (if it would actually hit one of the cases) is set the atomisp_sub_device struct's input member. And checking the rest of the atomisp code that member is never read. Also userspace does not actually setup media-controller links when using the atomisp /dev/video$ nodes since all the links are fixed. So isp_subdev_link_setup() never runs. Remove the unnecessary and broken isp_subdev_link_setup() function and also remove the unused atomisp_sub_device struct's input member. Reviewed-by:
Andy Shevchenko <andy@kernel.org> Signed-off-by:
-
Hans de Goede authored
The device core will call ACPI to turn the device (i2c_client) for a sensor on / put it in D0 before calling its probe() method. This takes a reference on all of the ACPI power-resources belonging to the device. Since the atomisp_gmin_platform code disables ACPI power-resource management and does its own pm, this reference never gets released. This is a problem for ACPI power-resources which are shared with other devices since those now never get turned off again (nor back on again). Explicitly put the device in D3 before disabling the ACPI power-resource management. Note that atomisp_register_i2c_module() runs near the end of the sensor driver's probe() function, after the driver is done with probing the hw. So the power-resouces (the same resources as directly controlled by the atomisp platform code) getting turned off (a second time, as they are already off) is not a problem. Reviewed-by:
-
Hans de Goede authored
atomisp_set_fmt() first does: v4l2_fill_mbus_format(&vformat.format, ...); vformat.format.height += padding_h; vformat.format.width += padding_w; ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, pad, set_fmt, NULL, &vformat); if (ret) return ret; f->fmt.pix.width = vformat.format.width - padding_w; f->fmt.pix.height = vformat.format.height - padding_h; this happens with the original padding w/h = 16 values and then later on it calls: ret = atomisp_set_fmt_to_snr(vdev, &s_fmt, f->fmt.pix.pixelformat, padding_w, padding_h, dvs_env_w, dvs_env_h); Which repeats the above structure. If at that point padding w/h are changed to 12 then it will now request a different output-size of the sensor driver. The sensor drivers so far have actually been ignoring this since they use v4l2_find_nearest_size() on a fixed resolution list and the nearest resolution will be the one from the earlier calls where padding w/h was 16. But there really is no reason for sensor drivers to use a fixed resolution list. They make lower resolutions using cropping so they can make any resolution as long as width/height are even numbers. Dropping the fixed-resolution list limit from sensors on BYT results in trying to start streaming failing because the resolution set to the sensor now no longer matches with the resolution used during the initial part of the configuration done by atomisp_set_fmt(). Drop the BYT specific overriding of the padding_w/h to 12, so that the padding in the first and second s_fmt calls made to the sensor matches, to fix stream start failing when the fixed resolution list is dropped. Reviewed-by: -
Hans de Goede authored
ffmt is a local variable pointing to a substruct of another local variable which really just makes the code harder to read / follow, so drop it. Signed-off-by:
-
Hans de Goede authored
The DSDT of all Windows BYT / CHT devices which I have seen has proper ACPI powermagement for the clk and regulators used by the sensors. So there is no need for the whole custom atomisp_gmin custom code to disable the ACPI pm and directly poke at the PMIC for this. Add new atomisp_register_sensor_no_gmin() + atomisp_unregister_subdev() helpers which allow registering a sensor with the atomisp code without using any of the atomisp_gmin power-management code. Reviewed-by:
-
Hans de Goede authored
atomisp_gmin_find_subdev() can be used to lookup a subdev given its i2c-adapter + i2c-client-address. But the only caller of it reads this from the intel_v4l2_subdev_table struct and that same struct already contains a pointer to the v4l2_subdev. So this function is not necessary, drop it and modify its only caller to directly take the subdev from the intel_v4l2_subdev_table struct. Also drop struct intel_v4l2_subdev_i2c_board_info since that now no longer is used. Reviewed-by:
-
Hans de Goede authored
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:
-
Hans de Goede authored
The atomisp driver creates 8 /dev/video# device nodes. 4 nodes (preview / video / viewfinder / capture) for each of 2 possible streams aka atomisp-sub-device-s (asd-s). Both streams start with asd->input_curr set to 0 (to the first sensor), opening + releasing a file-handle on one of the nodes of an asd, while streaming from the other asd causes the sensor to get turned off, leading to the stream failing. The atomisp-code already tracks which asd "owns" a specific sensor, use this to only turn the sensor off if it is owned by the asd. Reviewed-by:
-
Hans de Goede authored
The memory for all of struct atomisp_video_pipe is kzalloc()-ed in atomisp_subdev_init() so there is no need to memset parts of struct atomisp_video_pipe to 0. Reviewed-by:
-
Hans de Goede authored
atomisp_init_pipe() does 3 things: 1. Init a bunch of list-heads / locks 2. Init the vb_queue for the videodev (aka pipe) 3. zero the per-frame parameters related variables of the pipe 1. and 2. really should not be done at file-open time, but once at probe. Currently the code is getting away with doing this on every videodev-open because only 1 open is allowed at a time. 1. is already done at probe time by atomisp_init_subdev_pipe(), move 2. to atomisp_init_subdev_pipe() so that it is also done once at probe. As for 3. The per-frame parameters can only be set from a qbuf ioctl, which can only happen after a reqbufs ioctl and atomisp_buf_cleanup already zeros the per-frame parameters when the buffers are released, so 3. is not necessary at all. Reviewed-by:
-
Hans de Goede authored
Make atomisp behave like any other drivers and have it load the firmware at probe time (as it was already doing by default). The deferred firmware loading support needlessly complicates the v4l2_file_operations.open callback (atomisp_open()), getting in the way of allowing multiple opens like a normal v4l2 device would. Reviewed-by:
-
Hans de Goede authored
Remove the no longer used bin_factor_x, bin_factor_y and bin_mode members from the resolution info inside various atomisp camera sensor drivers. Reviewed-by:
-
Hans de Goede authored
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:
-
Hans de Goede authored
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:
-
Hans de Goede authored
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:
-
Hans de Goede authored
On BYT on poweron/runtime-resume the code is doing: 1. Do nothing 2. msleep(10) 3. Start actual poweron sequence Since the runtime resume can happen at any moment, waiting 10ms after it does not really make any sense. According to both the comment and to: https://github.com/intel/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0341-atomisp-WA-sleep-10ms-when-power-up-ISP-on-byt.patch Which is the patch which originally added this this was added as a workaround for a single test failing on a single model tablet/laptop. So lets just drop this. Reviewed-by:
-
Hans de Goede authored
punit_ddr_dvfs_enable() is only used on CHT devices and there dmesg gets filled with: "DDR DVFS, door bell is not cleared within 3ms" messages, so clearly the doorbell checking is not working. This check was added by: https://github.com/intel/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0340-atomisp-add-door-bell-for-ddr-dvfs-on-cht.patch Which commit message says: "PUNIT interface added to check Req_ACK of freq status". This suggests that the doorbell mechanism may only be available with certain PUNIT fw versions and it seems that many CHT devices do not have this fw version; that or the doorbell mechanism is not working for other reasons. Revert cam-0340-atomisp-add-door-bell-for-ddr-dvfs-on-cht.patch, replacing the doorbell check with a msleep(20) this fixes dmesg getting filled with error messages. Signed-off-by:
-
Hans de Goede authored
These are clearly debug messages, printing these all the time is not useful. Silence these by simply removing them altogether. Reviewed-by:
-
Hans de Goede authored
The atomisp does not use standard PCI power-management through the PCI config space. Instead this driver directly tells the P-Unit to disable the ISP over the IOSF. The standard PCI subsystem pm_ops will try to access the config space before (resume) / after (suspend) this driver has turned the ISP on / off, resulting in the following errors: Unable to change power state from D0 to D3hot, device inaccessible Unable to change power state from D3cold to D0, device inaccessible Getting logged into dmesg a whole bunch of time during boot as well as every time the camera is used. To avoid these errors use a custom pm_domain instead of standard driver pm-callbacks so that all the PCI subsys suspend / resume handling is skipped and call pci_save_state() / pci_restore_state() ourselves. Signed-off-by:
-
Hans de Goede authored
Remove the atomisp_sw_contex struct, it has only 1 member: running_freq, instead store running_freq directly. While at it also change running_freq from an int to an unsigned int, all values stored in it are unsigned and it is compared to the also unsigned new_freq variable. Reviewed-by:
-
Hans de Goede authored
The Crystal Cove PMIC used on some BYT/CHT devices has different revisions when paired with Bay Trail (BYT) vs Cherry Trail (CHT) SoCs. The current hardcoded values are only valid for CHT devices, change the code so that it uses the correct register values on both BYT and CHT. Reviewed-by:
-
Hans de Goede authored
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 authored
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:
-
Hans de Goede authored
Calling v4l2_ctrl_s_ctrl(asd->run_mode, pipe->default_run_mode) when the stream is already active (through another /dev/video# node) causes the stream to stop. Move the call to set the default run-mode so that it is only done on the first open of one of the 4 /dev/video# nodes of one of the 2 streams (atomisp-sub-devices / asd-s). Fixes: 2c45e343 ("media: atomisp: set per-device's default mode") Reviewed-by:
-
Hans de Goede authored
If set_fmt() fails make queue_setup() actually return the error instead of returning 0. This fixes the following oops on set_fmt() failures: [ 1060.378662] ------------[ cut here ]------------ [ 1060.378805] WARNING: CPU: 0 PID: 2080 at drivers/media/common/videobuf2/videobuf2-core.c:840 vb2_core_reqbufs+0x3f7/0x430 [videobuf2_common] ... [ 1060.381414] RIP: 0010:vb2_core_reqbufs+0x3f7/0x430 [videobuf2_common] ... [ 1060.382066] vb2_ioctl_reqbufs+0x9d/0xe0 [videobuf2_v4l2] [ 1060.382181] __video_do_ioctl+0x18e/0x3c0 [videodev] Reviewed-by:
-
Hans Verkuil authored
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.nlSigned-off-by:
Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by:
-
Arnd Bergmann authored
The recent conversion missed the Kconfig bit, so it can now end up in a link error on randconfig builds: ld.lld: error: undefined symbol: vb2_vmalloc_memops >>> referenced by atomisp_fops.c >>> drivers/staging/media/atomisp/pci/atomisp_fops.o:(atomisp_open) in archive vmlinux.a Link: https://lore.kernel.org/r/20230104082212.3770415-1-arnd@kernel.org Fixes: cb48ae89 ("media: atomisp: Convert to videobuf2") Signed-off-by:
Arnd Bergmann <arnd@arndb.de> Tested-by:
-
Duoming Zhou authored
When the ene device is detaching, function ene_remove() will be called. But there is no function to cancel tx_sim_timer in ene_remove(), the timer handler ene_tx_irqsim() could race with ene_remove(). As a result, the UAF bugs could happen, the process is shown below. (cleanup routine) | (timer routine) | mod_timer(&dev->tx_sim_timer, ..) ene_remove() | (wait a time) | ene_tx_irqsim() | dev->hw_lock //USE | ene_tx_sample(dev) //USE Fix by adding del_timer_sync(&dev->tx_sim_timer) in ene_remove(), The tx_sim_timer could stop before ene device is deallocated. What's more, The rc_unregister_device() and del_timer_sync() should be called first in ene_remove() and the deallocated functions such as free_irq(), release_region() and so on should be called behind them. Because the rc_unregister_device() is well synchronized. Otherwise, race conditions may happen. The situations that may lead to race conditions are shown below. Firstly, the rx receiver is disabled with ene_rx_disable() before rc_unregister_device() in ene_remove(), which means it can be enabled again if a process opens /dev/lirc0 between ene_rx_disable() and rc_unregister_device(). Secondly, the irqaction descriptor is freed by free_irq() before the rc device is unregistered, which means irqaction descriptor may be accessed again after it is deallocated. Thirdly, the timer can call ene_tx_sample() that can write to the io ports, which means the io ports could be accessed again after they are deallocated by release_region(). Therefore, the rc_unregister_device() and del_timer_sync() should be called first in ene_remove(). Suggested by: Sean Young <sean@mess.org> Fixes: 9ea53b74 ("V4L/DVB: STAGING: remove lirc_ene0100 driver") Signed-off-by:Duoming Zhou <duoming@zju.edu.cn> Signed-off-by:
Sean Young <sean@mess.org> Signed-off-by:
-
Li Jun authored
In case runtime PM is enabled, do runtime PM clean up to remove cpu latency qos request, otherwise driver removal may have below kernel dump: [ 19.463299] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000048 [ 19.472161] Mem abort info: [ 19.474985] ESR = 0x0000000096000004 [ 19.478754] EC = 0x25: DABT (current EL), IL = 32 bits [ 19.484081] SET = 0, FnV = 0 [ 19.487149] EA = 0, S1PTW = 0 [ 19.490361] FSC = 0x04: level 0 translation fault [ 19.495256] Data abort info: [ 19.498149] ISV = 0, ISS = 0x00000004 [ 19.501997] CM = 0, WnR = 0 [ 19.504977] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000049f81000 [ 19.511432] [0000000000000048] pgd=0000000000000000, p4d=0000000000000000 [ 19.518245] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP [ 19.524520] Modules linked in: gpio_ir_recv(+) rc_core [last unloaded: rc_core] [ 19.531845] CPU: 0 PID: 445 Comm: insmod Not tainted 6.2.0-rc1-00028-g2c397a46d47c #72 [ 19.531854] Hardware name: FSL i.MX8MM EVK board (DT) [ 19.531859] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 19.551777] pc : cpu_latency_qos_remove_request+0x20/0x110 [ 19.557277] lr : gpio_ir_recv_runtime_suspend+0x18/0x30 [gpio_ir_recv] [ 19.557294] sp : ffff800008ce3740 [ 19.557297] x29: ffff800008ce3740 x28: 0000000000000000 x27: ffff800008ce3d50 [ 19.574270] x26: ffffc7e3e9cea100 x25: 00000000000f4240 x24: ffffc7e3f9ef0e30 [ 19.574284] x23: 0000000000000000 x22: ffff0061803820f4 x21: 0000000000000008 [ 19.574296] x20: ffffc7e3fa75df30 x19: 0000000000000020 x18: ffffffffffffffff [ 19.588570] x17: 0000000000000000 x16: ffffc7e3f9efab70 x15: ffffffffffffffff [ 19.595712] x14: ffff800008ce37b8 x13: ffff800008ce37aa x12: 0000000000000001 [ 19.602853] x11: 0000000000000001 x10: ffffcbe3ec0dff87 x9 : 0000000000000008 [ 19.609991] x8 : 0101010101010101 x7 : 0000000000000000 x6 : 000000000f0bfe9f [ 19.624261] x5 : 00ffffffffffffff x4 : 0025ab8e00000000 x3 : ffff006180382010 [ 19.631405] x2 : ffffc7e3e9ce8030 x1 : ffffc7e3fc3eb810 x0 : 0000000000000020 [ 19.638548] Call trace: [ 19.640995] cpu_latency_qos_remove_request+0x20/0x110 [ 19.646142] gpio_ir_recv_runtime_suspend+0x18/0x30 [gpio_ir_recv] [ 19.652339] pm_generic_runtime_suspend+0x2c/0x44 [ 19.657055] __rpm_callback+0x48/0x1dc [ 19.660807] rpm_callback+0x6c/0x80 [ 19.664301] rpm_suspend+0x10c/0x640 [ 19.667880] rpm_idle+0x250/0x2d0 [ 19.671198] update_autosuspend+0x38/0xe0 [ 19.675213] pm_runtime_set_autosuspend_delay+0x40/0x60 [ 19.680442] gpio_ir_recv_probe+0x1b4/0x21c [gpio_ir_recv] [ 19.685941] platform_probe+0x68/0xc0 [ 19.689610] really_probe+0xc0/0x3dc [ 19.693189] __driver_probe_device+0x7c/0x190 [ 19.697550] driver_probe_device+0x3c/0x110 [ 19.701739] __driver_attach+0xf4/0x200 [ 19.705578] bus_for_each_dev+0x70/0xd0 [ 19.709417] driver_attach+0x24/0x30 [ 19.712998] bus_add_driver+0x17c/0x240 [ 19.716834] driver_register+0x78/0x130 [ 19.720676] __platform_driver_register+0x28/0x34 [ 19.725386] gpio_ir_recv_driver_init+0x20/0x1000 [gpio_ir_recv] [ 19.731404] do_one_initcall+0x44/0x2ac [ 19.735243] do_init_module+0x48/0x1d0 [ 19.739003] load_module+0x19fc/0x2034 [ 19.742759] __do_sys_finit_module+0xac/0x12c [ 19.747124] __arm64_sys_finit_module+0x20/0x30 [ 19.751664] invoke_syscall+0x48/0x114 [ 19.755420] el0_svc_common.constprop.0+0xcc/0xec [ 19.760132] do_el0_svc+0x38/0xb0 [ 19.763456] el0_svc+0x2c/0x84 [ 19.766516] el0t_64_sync_handler+0xf4/0x120 [ 19.770789] el0t_64_sync+0x190/0x194 [ 19.774460] Code: 910003fd a90153f3 aa0003f3 91204021 (f9401400) [ 19.780556] ---[ end trace 0000000000000000 ]--- Signed-off-by:
Li Jun <jun.li@nxp.com> Signed-off-by:
-
Krzysztof Kozlowski authored
Convert ST STIH4xx HDMI CEC bindings to DT schema. Signed-off-by:
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by:
Rob Herring <robh@kernel.org> Signed-off-by:
-
Krzysztof Kozlowski authored
Convert NVIDIA Tegra HDMI CEC bindings to DT schema. Signed-off-by:
Thierry Reding <treding@nvidia.com> Signed-off-by:
-
Krzysztof Kozlowski authored
Convert HDMI CEC GPIO bindings to DT schema. Signed-off-by:
-
Krzysztof Kozlowski authored
Convert Samsung S5P HDMI CEC adapter bindings to DT schema. Signed-off-by:
-
Krzysztof Kozlowski authored
Reference common HDMI CEC adapter properties to simplify the binding and have only one place of definition for common properties. The common CEC binding expects also node name to be 'cec'. Signed-off-by:
-
Krzysztof Kozlowski authored
Reference common HDMI CEC adapter properties to simplify the binding and have only one place of definition for common properties. Signed-off-by:
Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by:
-
Krzysztof Kozlowski authored
Convert common HDMI CEC adapter bindings to DT schema. Signed-off-by:
-
Krzysztof Kozlowski authored
Move st,stm32-cec.yaml bindings to cec subfolder and drop unneeded "bindings" in the title. Signed-off-by:
-
Krzysztof Kozlowski authored
Move amlogic,meson-gx-ao-cec.yaml bindings to cec subfolder and drop unneeded quotes. Signed-off-by:
-
Dong Chuanjian authored
When the pointer variable is judged to be null, null is returned directly. [hverkuil: fix two checkpatch warnings] Signed-off-by:
Dong Chuanjian <chuanjian@nfschina.com> Acked-by:
Nicolas Dufresne <nicolas.dufresne@collabora.com> Fixes: d3f756ad ("media: v4l2: Trace calculated p/b0/b1 initial reflist") Signed-off-by:
-
