Re: [RESEND Patch v13 2/3] media: rockchip: Add a driver for Rockchip's camera interface
From: Val Packett
Date: Wed Jun 12 2024 - 01:24:08 EST
Hi,
a couple more comments on this driver:
On Sun, Feb 11 2024 at 20:03:31 +01:00:00, Mehdi Djait
<mehdi.djait.k@xxxxxxxxx> wrote:
+static int cif_stream_start(struct cif_stream *stream)
+{
+ u32 val, fmt_type, xfer_mode = 0;
+ struct cif_device *cif_dev = stream->cifdev;
+ struct cif_remote *remote_info = &cif_dev->remote;
+ int ret;
+ u32 input_mode;
+
+ stream->frame_idx = 0;
+ stream->frame_phase = 0;
+
+ fmt_type = stream->cif_fmt_in->fmt_type;
+ input_mode = (remote_info->std == V4L2_STD_NTSC) ?
+ CIF_FORMAT_INPUT_MODE_NTSC :
+ CIF_FORMAT_INPUT_MODE_PAL;
Mode logic needs to be expanded for cameras; I'm trying to get it
working correctly,
so far managed to get some cursed selfies with the wrong pixel format
:) but either
way I could send a patch when I have it working well.
+static int subdev_notifier_complete(struct v4l2_async_notifier
*notifier)
+{
+ struct cif_device *cif_dev;
+ struct v4l2_subdev *sd;
+ int ret;
+
+ cif_dev = container_of(notifier, struct cif_device, notifier);
+ sd = cif_dev->remote.sd;
+
+ mutex_lock(&cif_dev->media_dev.graph_mutex);
+
Potential deadlock with this lock:
[ 3.438667] ======================================================
[ 3.438672] WARNING: possible circular locking dependency detected
[ 3.438677] 6.10.0-rc1-next-20240531 #151 Not tainted
[ 3.438684] ------------------------------------------------------
[ 3.438687] kworker/u8:1/25 is trying to acquire lock:
[ 3.438695] c152d41c (videodev_lock){+.+.}-{4:4}, at:
__video_register_device+0xf4/0x15b0
[ 3.438737]
[ 3.438737] but task is already holding lock:
[ 3.438740] c31981fc (&mdev->graph_mutex){+.+.}-{4:4}, at:
subdev_notifier_complete+0x20/0x80
[ 3.438765]
[ 3.438765] which lock already depends on the new lock.
[ 3.438765]
[ 3.438769]
[ 3.438769] the existing dependency chain (in reverse order) is:
[ 3.438772]
[ 3.438772] -> #1 (&mdev->graph_mutex){+.+.}-{4:4}:
[ 3.438786] lock_acquire+0x110/0x374
[ 3.438809] __mutex_lock+0xac/0xf2c
[ 3.438828] mutex_lock_nested+0x1c/0x24
[ 3.438843] media_device_register_entity+0x80/0x1e8
[ 3.438857] __video_register_device+0xab0/0x15b0
[ 3.438869] cif_register_stream_vdev+0x158/0x18c
[ 3.438880] cif_plat_probe+0x20c/0x424
[ 3.438888] platform_probe+0x5c/0xb0
[ 3.438905] really_probe+0xc8/0x2cc
[ 3.438916] __driver_probe_device+0x88/0x1a0
[ 3.438926] driver_probe_device+0x30/0x108
[ 3.438936] __driver_attach+0x94/0x184
[ 3.438946] bus_for_each_dev+0x7c/0xcc
[ 3.438955] bus_add_driver+0xcc/0x1ec
[ 3.438964] driver_register+0x7c/0x114
[ 3.438975] do_one_initcall+0x7c/0x2f8
[ 3.438989] kernel_init_freeable+0x1b0/0x20c
[ 3.439010] kernel_init+0x14/0x12c
[ 3.439024] ret_from_fork+0x14/0x28
[ 3.439033]
[ 3.439033] -> #0 (videodev_lock){+.+.}-{4:4}:
[ 3.439048] check_prev_add+0x134/0x17d8
[ 3.439065] __lock_acquire+0x17e0/0x21fc
[ 3.439080] lock_acquire+0x110/0x374
[ 3.439095] __mutex_lock+0xac/0xf2c
[ 3.439109] mutex_lock_nested+0x1c/0x24
[ 3.439123] __video_register_device+0xf4/0x15b0
[ 3.439135] __v4l2_device_register_subdev_nodes+0xd8/0x1a0
[ 3.439152] subdev_notifier_complete+0x2c/0x80
[ 3.439160] __v4l2_async_register_subdev+0xa8/0x1a0
[ 3.439176] gc0308_probe+0x654/0x6f4
[ 3.439187] i2c_device_probe+0x168/0x268
[ 3.439201] really_probe+0xc8/0x2cc
[ 3.439211] __driver_probe_device+0x88/0x1a0
[ 3.439221] driver_probe_device+0x30/0x108
[ 3.439231] __device_attach_driver+0x94/0x10c
[ 3.439241] bus_for_each_drv+0x90/0xe4
[ 3.439250] __device_attach+0xac/0x1b0
[ 3.439260] bus_probe_device+0x8c/0x90
[ 3.439269] deferred_probe_work_func+0x7c/0xac
[ 3.439279] process_one_work+0x23c/0x6bc
[ 3.439295] worker_thread+0x190/0x3d8
[ 3.439308] kthread+0xf8/0x114
[ 3.439321] ret_from_fork+0x14/0x28
[ 3.439330]
[ 3.439330] other info that might help us debug this:
[ 3.439330]
[ 3.439333] Possible unsafe locking scenario:
[ 3.439333]
[ 3.439336] CPU0 CPU1
[ 3.439339] ---- ----
[ 3.439341] lock(&mdev->graph_mutex);
[ 3.439349] lock(videodev_lock);
[ 3.439356] lock(&mdev->graph_mutex);
[ 3.439363] lock(videodev_lock);
[ 3.439370]
[ 3.439370] *** DEADLOCK ***
[ 3.439370]
[ 3.439373] 5 locks held by kworker/u8:1/25:
[ 3.439379] #0: c28106b4
((wq_completion)events_unbound){+.+.}-{0:0}, at:
process_one_work+0x1ac/0x6bc
[ 3.439408] #1: f0889f20 (deferred_probe_work){+.+.}-{0:0}, at:
process_one_work+0x1d4/0x6bc
[ 3.439436] #2: c303ec9c (&dev->mutex){....}-{4:4}, at:
__device_attach+0x30/0x1b0
[ 3.439461] #3: c152d3d0 (list_lock){+.+.}-{4:4}, at:
__v4l2_async_register_subdev+0x50/0x1a0
[ 3.439491] #4: c31981fc (&mdev->graph_mutex){+.+.}-{4:4}, at:
subdev_notifier_complete+0x20/0x80