- 13 May, 2022 40 commits
-
-
Hans Verkuil authored
Using an activation counter to decide when the enable or disable the cec adapter is not the best approach and can lead to race conditions. Change this to determining the current status of the adapter, and enable or disable the adapter accordingly. It now only needs to be called whenever there is a chance that the state changes, and it can handle enabling/disabling monitoring as well if needed. This simplifies the code and it should be a more robust approach as well. Signed-off-by:
Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by:
Mauro Carvalho Chehab <mchehab@kernel.org>
-
Hans Verkuil authored
If the physical address changes (i.e. becomes invalid, then valid again) while the adapter is still claiming free logical addresses, then trigger a reconfiguration since any claimed LAs may now be stale. Signed-off-by:
-
Hans Verkuil authored
If an adapter is trying to claim a free logical address then it is in the 'is_configuring' state. If during that process the cable is disconnected (HPD goes low, which in turn invalidates the physical address), then cec_adap_unconfigure() is called, and that set the is_configuring boolean to false, even though the thread that's trying to claim an LA is still running. Don't touch the is_configuring bool in cec_adap_unconfigure(), it will eventually be cleared by the thread. By making that change the cec_config_log_addr() function also had to change: it was aborting if is_configuring became false (since that is what cec_adap_unconfigure() did), but that no longer works. Instead check if the physical address is invalid. That is a much more appropriate check anyway. This fixes a bug where the the adapter could be disabled even though the device was still configuring. This could cause POLL transmits to time out. Signed-off-by:
-
Hans Verkuil authored
If, while trying to claim a free logical address, a POLL message times out, then abort this process. A CEC_TX_STATUS_TIMEOUT should be handled the same as a CEC_TX_STATUS_ABORTED. This avoids a situation where transmits time out due to a driver or hardware bug and it takes ages before the attempt to find available free logical addresses finishes. Signed-off-by:
-
Hans Verkuil authored
The __cec_s_log_addrs() function can configure or unconfigure the adapter. The ioctl handler in cec-api.c will prevent it from being called to configure the adapter if it was already configured (or in the process of configuring). But it can still be called to unconfigure an already unconfigured adapter, and it didn't check for that. This can cause cec_activate_cnt_dec() to be called too often, causing a WARN_ON. Instead first check if adap->log_addrs.num_log_addrs == 0 and return since in that case the adapter is already unconfigured. Signed-off-by:
-
Hans Verkuil authored
It's OK to keep the pending pin events when disabling or enabling the 'adapter'. Zeroing this can cause a race condition if this happens when the pin kthread is handling a pin event and calls atomic_dec later, causing work_pin_num_events to become negative. Just leave pending events in the queue, they'll be read eventually. Signed-off-by:
-
Hans Verkuil authored
When the adap_enable callback is called the adap->lock is held. When disabling the adapter it attempts to stop the kthread that deals with receiving and transmitting messages. However, kthread_stop waits for the thread to stop, so all that time adap->lock is held. Unfortunately, the kernel thread itself can call functions that take that same lock, so a deadlock can occur. Change the logic to keep the kernel thread running and instead when disabling the adapter, just set the pin to high, go to idle and then to state OFF and disable the interrupt. Only stop the kernel thread when the adapter is deleted. This way disabling the adapter will not wait for anything and the deadlock is avoided. Signed-off-by:
-
Tetsuo Handa authored
Since usb_register_dev() from imon_init_display() from imon_probe() holds minor_rwsem while display_open() which holds driver_lock and ictx->lock is called with minor_rwsem held from usb_open(), holding driver_lock or ictx->lock when calling usb_register_dev() causes circular locking dependency problem. Since usb_deregister_dev() from imon_disconnect() holds minor_rwsem while display_open() which holds driver_lock is called with minor_rwsem held, holding driver_lock when calling usb_deregister_dev() also causes circular locking dependency problem. Sean Young explained that the problem is there are imon devices which have two usb interfaces, even though it is one device. The probe and disconnect function of both usb interfaces can run concurrently. Alan Stern responded that the driver and USB cores guarantee that when an interface is probed, both the interface and its USB device are locked. Ditto for when the disconnect callback gets run. So concurrent probing/ disconnection of multiple interfaces on the same device is not possible. Therefore, we don't need locks for handling race between imon_probe() and imon_disconnect(). But we still need to handle race between display_open() /vfd_write()/lcd_write()/display_close() and imon_disconnect(), for disconnect event can happen while file descriptors are in use. Since "struct file"->private_data is set by display_open(), vfd_write()/ lcd_write()/display_close() can assume that "struct file"->private_data is not NULL even after usb_set_intfdata(interface, NULL) was called. Replace insufficiently held driver_lock with refcount_t based management. Add a boolean flag for recording whether imon_disconnect() was already called. Use RCU for accessing this boolean flag and refcount_t. Since the boolean flag for imon_disconnect() is shared, disconnect event on either intf0 or intf1 affects both interfaces. But I assume that this change does not matter, for usually disconnect event would not happen while interfaces are in use. Link: https://syzkaller.appspot.com/bug?extid=c558267ad910fc494497Reported-by:
syzbot <syzbot+c558267ad910fc494497@syzkaller.appspotmail.com> Signed-off-by:
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Tested-by:
Sean Young <sean@mess.org> Signed-off-by:
-
Oliver Neukum authored
The point of using get/put_device() is to keep references for as long as the device may be in use. That means dropping them must be the penultimate action right before freeing the memory. Signed-off-by:
Oliver Neukum <oneukum@suse.com> Signed-off-by:
-
Oliver Neukum authored
The timer will report events for an input device. Reporting events for an unregistered device is bad. Hence the timer must be killed first. Signed-off-by:
-
Oliver Neukum authored
GFP_NOIO is fine here. Signed-off-by:
-
Hans Verkuil authored
Fix smatch warning: drivers/media/i2c/s5k6a3.c:234 __s5k6a3_power_on() warn: 'sensor->clock' from clk_prepare_enable() not released on lines: 234. Signed-off-by:
Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by:
-
Hans Verkuil authored
Fixes smatch warning: drivers/media/i2c/ccs/ccs-core.c:1676 ccs_power_on() warn: 'sensor->ext_clk' from clk_prepare_enable() not released on lines: 1606. Signed-off-by:
-
Hans Verkuil authored
Fixes smatch warning: drivers/media/pci/cx25821/cx25821-alsa.c:768 cx25821_audio_initdev() warn: 'dev->pci->irq' from request_irq() not released on lines: 768. Signed-off-by:
-
Hans Verkuil authored
Fixes smatch warning: drivers/media/common/videobuf2/videobuf2-dma-sg.c:129 vb2_dma_sg_alloc() warn: Please consider using kvcalloc instead Signed-off-by:
Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
-
Hans Verkuil authored
Fixes smatch warning: drivers/media/v4l2-core/v4l2-ctrls-core.c:1143 v4l2_ctrl_handler_init_class() warn: Please consider using kvcalloc instead Signed-off-by:
-
Hans Verkuil authored
Fixes smatch warning: drivers/media/v4l2-core/v4l2-subdev.c:978 __v4l2_subdev_state_alloc() warn: Please consider using kvcalloc instead Signed-off-by:
-
Benjamin Gaignard authored
Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2 register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT flag value. The vendor code does the same to set these values. This fixes conformance test CAINIT_G_SHARP_3. Fluster HEVC score is increase by one with this patch. Signed-off-by:
Benjamin Gaignard <benjamin.gaignard@collabora.com> Reviewed-by:
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Signed-off-by:
-
Benjamin Gaignard authored
PoC shall be int the range of -2^31 to 2^31 -1 (HEVC spec section 8.3.1 Decoding process for picture order count). The current way to know if an entry in reference picture array is free is to test if PoC = UNUSED_REF. Since UNUSED_REF is defined as '-1' that could lead to decode issue if one PoC also equal '-1'. PoC with value = '-1' exists in conformance test SLIST_B_Sony_9. Change the way unused entries are managed in reference pictures array to avoid using PoC to detect then. This patch doesn't change fluster HEVC score. Signed-off-by:
-
Benjamin Gaignard authored
Use pps->column_width_minus1[j] + 1 as value for the tile info buffer instead of pps->column_width_minus1[j + 1]. The patch fixes DBLK_E_VIXS_2, DBLK_F_VIXS_2, DBLK_G_VIXS_2, SAO_B_MediaTek_5, TILES_A_Cisco_2 and TILES_B_Cisco_1 tests in fluster. Fixes: cb5dd5a0 ("media: hantro: Introduce G2/HEVC decoder") Signed-off-by:
-
Benjamin Gaignard authored
Hantro decoder doesn't take care of the requested and aligned size of the capture buffer. Stop using the bitstream width/height and use capture frame size stored in the context to get the correct values. hantro_hevc_chroma_offset() and hantro_hevc_motion_vectors_offset() are only used in hantro_g2_hevc_dec.c so take the opportunity to move them here. fluster HEVC score goes up from 77 to 85 successful tests (over 147) with this patch. Signed-off-by:
-
Benjamin Gaignard authored
Hantro G2 post processor is able to down scale decoded frames by a factor of 2, 4 or 8. Add enum_framesizes() ops to postproc_ops structure to enumerate the possible output sizes for a given input resolution. For G2 post-processor use fsize->index (from 0 to 3) as power of 2 divisor. As described in v4l2 documentation return -EINVAL when scaling down isn't possible. fluster scores: 77/147 for HEVC 143/303 for VP9 Signed-off-by:
-
Ming Qian authored
G/S_PARM doesn't make sense for the capture queue of a stateful encoder, unless V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL is set to reserve hardware resources. Otherwise it will fail the v4l2-compliance Signed-off-by:
Ming Qian <ming.qian@nxp.com> Signed-off-by:
-
Ming Qian authored
when error occurs, driver set error flag, and driver need to wake up the poll wait Signed-off-by:
-
Eugen Hristev authored
With this, the csi2dc will take part in the media pipeline graph walk and validate the links with it's entities. Signed-off-by:
Eugen Hristev <eugen.hristev@microchip.com> Signed-off-by:
-
Eugen Hristev authored
The Microchip XISC supports a bus width of 14 bits. Add it to the supported bus widths. Signed-off-by:
Rob Herring <robh@kernel.org> Signed-off-by:
-
Eugen Hristev authored
Remove stray line from formats struct. Signed-off-by:
Jacopo Mondi <jacopo@jmondi.org> Signed-off-by:
-
Eugen Hristev authored
Compact the list array to be more readable. No other changes, only cosmetic. Signed-off-by:
Jacopo Mondi <jacopo+renesas@jmondi.org> Signed-off-by:
-
Eugen Hristev authored
The AWB workqueue runs in a kernel thread and needs to be synchronized w.r.t. the streaming status. It is possible that streaming is stopped while the AWB workq is running. In this case it is likely that the check for vb2_start_streaming_called is done at one point in time, but the AWB computations are done later, including a call to isc_update_profile, which requires streaming to be started. Thus , isc_update_profile will fail if during this operation sequence the streaming was stopped. To solve this issue, a mutex is added, that will serialize the awb work and streaming stopping, with the mention that either streaming is stopped completely including termination of the last frame is done, and after that the AWB work can check stream status and stop; either first AWB work is completed and after that the streaming can stop correctly. The awb spin lock cannot be used since this spinlock is taken in the same context and using it in the stop streaming will result in a recursion BUG. Signed-off-by:
-
Eugen Hristev authored
While this does not happen in production, this check should be done versus the mask, as checking with the YCYC value may not include some bits that may be set. It is correct and safe to check the whole mask. Fixes: 123aaf81 ("media: atmel: atmel-sama5d2-isc: fix YUYV format") Signed-off-by:
-
Eugen Hristev authored
Remove duplicate comments which are already in place before the struct definition. Signed-off-by:
-
Eugen Hristev authored
In s_fmt_vid_cap, we should check if vb2_is_busy and return EBUSY, not check if it's streaming to return the busy state. Suggested-by:
Hans Verkuil <hverkuil@xs4all.nl> Signed-off-by:
-
Eugen Hristev authored
During experiments with libcamera, it looks like vb2_is_streaming returns true before our start streaming is called. Order of operations is streamon -> queue -> start_streaming ISC would have started the DMA immediately when a buffer is being added to the vbqueue if the queue is streaming. It is more safe to start the DMA after the start streaming of the driver is called. Thus, even if vb2queue is streaming, add the buffer to the dma queue of the driver instead of actually starting the DMA process, if the start streaming has not been called yet. Tho achieve this, we have to use vb2_start_streaming_called instead of vb2_is_streaming. Signed-off-by:
-
Michael Rodin authored
The vertical subsampling factor is currently not considered in the offset calculation for plane cropping done in rpf_configure_partition. This causes a distortion (shift of the color plane) when formats with the vsub factor larger than 1 are used (e.g. NV12, see vsp1_video_formats in vsp1_pipe.c). This commit considers vsub factor for all planes except plane 0 (luminance). Drop generalization of the offset calculation to reduce the binary size. Fixes: e5ad37b6 ("[media] v4l: vsp1: Add cropping support") Signed-off-by:
Michael Rodin <mrodin@de.adit-jv.com> Signed-off-by:
LUU HOAI <hoai.luu.ub@renesas.com> Signed-off-by:
Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by:
Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Signed-off-by:
-
Laurent Pinchart authored
Use the new vb2_queue_is_busy() helper to replace the open-coded version. Signed-off-by:
-
Laurent Pinchart authored
Use the vb2_fop_release() helper to replace the open-coded version. The video->lock is assigned to the queue lock, used by vb2_fop_release(), so the only functional difference is that v4l2_fh_release() is now called before vsp1_device_put(). This should be harmless. Signed-off-by:
-
Laurent Pinchart authored
vb2 queue ownership is managed by the ioctl handler helpers (vb2_ioctl_*). There are however use cases where drivers can benefit from checking queue ownership, for instance when open-coding an ioctl handler that needs to perform additional checks before calling the corresponding vb2 operation. Expose the vb2_queue_is_busy() function in the videobuf2-v4l2.h header, and change its first argument to a struct vb2_queue pointer as the function name implies it operates on a queue, not a video_device. Signed-off-by:
-
Laurent Pinchart authored
Replace the driver-specific definitions of MIPI CSI-2 data types with macros from mipi-csi2.h. Signed-off-by:
Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Reviewed-by:
-
Laurent Pinchart authored
Replace the hardcoded MIPI CSI-2 data types with macros from mipi-csi2.h. Signed-off-by:
-
Mauro Carvalho Chehab authored
As warned by gcc 12.1: drivers/staging/media/atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c: In function 'ia_css_rmgr_acq_vbuf': drivers/staging/media/atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c:275:33: error: storing the address of local variable 'h' in '*handle' [-Werror=dangling-pointer=] 275 | *handle = &h; | ~~~~~~~~^~~~ drivers/staging/media/atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c:257:40: note: 'h' declared here 257 | struct ia_css_rmgr_vbuf_handle h; | ^ drivers/staging/media/atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c:257:40: note: 'handle' declared here cc1: all warnings being treated as errors The logic uses a temporary struct to update the handler, but, instead of copying the value to the pointer sent by the caller, it replaces it with the content with a local variable. That's wrong, and may lead the caller to use a weird value. Signed-off-by:
-
