* Re: [RFC PATCH v0] Add tw5864 driver [not found] <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net> @ 2016-01-03 3:47 ` Joe Perches 2016-01-03 13:34 ` Andrey Utkin 2016-01-03 5:38 ` Leon Romanovsky 2016-01-11 10:52 ` Hans Verkuil 2 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2016-01-03 3:47 UTC (permalink / raw) To: Andrey Utkin, Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Hans Verkuil, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman Cc: Andrey Utkin On Sun, 2016-01-03 at 03:41 +0200, Andrey Utkin wrote: > (Disclaimer up to scissors mark) > > Please be so kind to take a look at a new driver. trivial comments only: > diff --git a/drivers/staging/media/tw5864/tw5864-bs.h b/drivers/staging/media/tw5864/tw5864-bs.h [] > +static inline int bs_pos(struct bs *s) > +{ > + return (8 * (s->p - s->p_start) + 8 - s->i_left); > +} several of these have unnecessary parentheses > +static inline int bs_eof(struct bs *s) > +{ > + return (s->p >= s->p_end ? 1 : 0); > +} Maybe use bool a bit more > +/* golomb functions */ > +static inline void bs_write_ue(struct bs *s, u32 val) > +{ > + int i_size = 0; > + static const int i_size0_255[256] = { Maybe use s8 instead of int to reduce object size > + 1, 1, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, > + 5, 5, > + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, > + 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, Perhaps it'd be clearer to use gcc's ranged initializer syntax [ 0 ... 1] = 1, [ 2 ... 3] = 2, [ 4 ... 7] = 3, [ 8 ... 15] = 4, [ 16 ... 31] = 5, [ 32 ... 63] = 6, etc... or maybe just use fls > +static inline int bs_size_ue(unsigned int val) > +{ > + int i_size = 0; > + static const int i_size0_254[255] = { Same sort of thing > diff --git a/drivers/staging/media/tw5864/tw5864-config.c b/drivers/staging/media/tw5864/tw5864-config.c [] > +u8 tw_indir_readb(struct tw5864_dev *dev, u16 addr) > +{ > + int timeout = 30000; misleading name, retries would be more proper, or maybe use real timed loops. > + u32 data = 0; > + > + addr <<= 2; > + > + while ((tw_readl(TW5864_IND_CTL) >> 31) && (timeout--)) > + ; > + if (!timeout) > + dev_err(&dev->pci->dev, > + "tw_indir_writel() timeout before reading\n"); > + > + tw_writel(TW5864_IND_CTL, addr | TW5864_ENABLE); > + > + timeout = 30000; > + while ((tw_readl(TW5864_IND_CTL) >> 31) && (timeout--)) > + ; > + if (!timeout) > + dev_err(&dev->pci->dev, > + "tw_indir_writel() timeout at reading\n"); > + > + data = tw_readl(TW5864_IND_DATA); > + return data & 0xff; > +} [] > +static size_t regs_dump(struct tw5864_dev *dev, char *buf, size_t size) > +{ > + size_t count = 0; > + > + u32 reg_addr; > + u32 value; > + > + for (reg_addr = 0x0000; (count < size) && (reg_addr <= 0x2FFC); > + reg_addr += 4) { > + value = tw_readl(reg_addr); > + count += scnprintf(buf + count, size - count, > + "[0x%05x] = 0x%08x\n", reg_addr, value); > + } > + > + for (reg_addr = 0x4000; (count < size) && (reg_addr <= 0x4FFC); > + reg_addr += 4) { > + value = tw_readl(reg_addr); > + count += scnprintf(buf + count, size - count, > + "[0x%05x] = 0x%08x\n", reg_addr, value); > + } > + > + for (reg_addr = 0x8000; (count < size) && (reg_addr <= 0x180DC); > + reg_addr += 4) { > + value = tw_readl(reg_addr); > + count += scnprintf(buf + count, size - count, > + "[0x%05x] = 0x%08x\n", reg_addr, value); > + } > + > + for (reg_addr = 0x18100; (count < size) && (reg_addr <= 0x1817C); > + reg_addr += 4) { > + value = tw_readl(reg_addr); > + count += scnprintf(buf + count, size - count, > + "[0x%05x] = 0x%08x\n", reg_addr, value); > + } > + > + for (reg_addr = 0x80000; (count < size) && (reg_addr <= 0x87FFF); > + reg_addr += 4) { > + value = tw_readl(reg_addr); > + count += scnprintf(buf + count, size - count, > + "[0x%05x] = 0x%08x\n", reg_addr, value); > + } This seems a little repetitive. > diff --git a/drivers/staging/media/tw5864/tw5864-tables.h b/drivers/staging/media/tw5864/tw5864-tables.h [] > +static const u32 forward_quantization_table[QUANTIZATION_TABLE_LEN] = { u16? > + static const struct v4l2_ctrl_config tw5864_md_thresholds = { > + .ops = &tw5864_ctrl_ops, > + .id = V4L2_CID_DETECT_MD_THRESHOLD_GRID, > + .dims = {MD_CELLS_HOR, MD_CELLS_VERT}, > + .def = 14, > + /* See tw5864_md_metric_from_mvd() */ > + .max = 2 * 0x0f, > + .step = 1, > + }; odd indentation > +#ifdef DEBUG > + dev_dbg(&input->root->pci->dev, > + "input %d, frame md stats: min %u, max %u, avg %u, cells above threshold: %u\n", > + input->input_number, min, max, sum / md_cells, > + cnt_above_thresh); > +#endif unnecessary #ifdef -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-01-03 3:47 ` [RFC PATCH v0] Add tw5864 driver Joe Perches @ 2016-01-03 13:34 ` Andrey Utkin 0 siblings, 0 replies; 13+ messages in thread From: Andrey Utkin @ 2016-01-03 13:34 UTC (permalink / raw) To: Joe Perches Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Hans Verkuil, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin On Sun, Jan 3, 2016 at 5:47 AM, Joe Perches <joe@perches.com> wrote: > several of these have unnecessary parentheses Thanks, fixed. > Maybe use bool a bit more Thanks, fixed. > or maybe just use fls Thanks, fls() fit greatly, rewritten the function with compatibility testing. >> +static inline int bs_size_ue(unsigned int val) >> +{ >> + int i_size = 0; >> + static const int i_size0_254[255] = { > > Same sort of thing Dropped this procedure because it is not used. Thanks. >> diff --git a/drivers/staging/media/tw5864/tw5864-config.c b/drivers/staging/media/tw5864/tw5864-config.c > [] >> +u8 tw_indir_readb(struct tw5864_dev *dev, u16 addr) >> +{ >> + int timeout = 30000; > > misleading name, retries would be more proper, > or maybe use real timed loops. Thanks, renamed to "retries". > This seems a little repetitive. Thanks, reworked. > u16? Thanks, fixed. > odd indentation Indeed. For some mysterious reason, vim + syntastic insists on this way. Fixed. >> +#ifdef DEBUG >> + dev_dbg(&input->root->pci->dev, >> + "input %d, frame md stats: min %u, max %u, avg %u, cells above threshold: %u\n", >> + input->input_number, min, max, sum / md_cells, >> + cnt_above_thresh); >> +#endif > > unnecessary #ifdef Not quite. This debug printout works with variables which are declared in another "#ifdef DEBUG" clause. And it turns out that dev_dbg is compiled not only if DEBUG is declared, so when I remove this ifdef, I get "undefined variable" errors. It seems it is compiled if this condition is met: #if (defined DEBUG) || (defined CONFIG_DYNAMIC_DEBUG) so I can wrap my stats variables into this statement instead. But such change is not equivalent - I guess CONFIG_DYNAMIC_DEBUG is common to be enabled, so debug stats will be always calculated, even when module is not under debug. Except if I use DEFINE_DYNAMIC_DEBUG_METADATA etc. in my code. Please let me know if this can be sorted out in cleaner way. On Sun, Jan 3, 2016 at 7:38 AM, Leon Romanovsky <leon@leon.nu> wrote: > On Sun, Jan 03, 2016 at 03:41:42AM +0200, Andrey Utkin wrote: > .... >> +/* >> + * TW5864 driver - Exp-Golomb code functions >> + * >> + * Copyright (C) 2015 Bluecherry, LLC <maintainers@bluecherrydvr.com> >> + * Copyright (C) 2015 Andrey Utkin <andrey.utkin@corp.bluecherry.net> > > I doubt that you have contract with your employer which permits you to > claim copyright on the work/product. Thank you for commenting. I have previously asked my employer to review copyright statment, and he told this is fine. Now I have requrested him again with reference to your comment. -- Bluecherry developer. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver [not found] <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net> 2016-01-03 3:47 ` [RFC PATCH v0] Add tw5864 driver Joe Perches @ 2016-01-03 5:38 ` Leon Romanovsky 2016-01-11 10:52 ` Hans Verkuil 2 siblings, 0 replies; 13+ messages in thread From: Leon Romanovsky @ 2016-01-03 5:38 UTC (permalink / raw) To: Andrey Utkin Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Hans Verkuil, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin On Sun, Jan 03, 2016 at 03:41:42AM +0200, Andrey Utkin wrote: .... > +/* > + * TW5864 driver - Exp-Golomb code functions > + * > + * Copyright (C) 2015 Bluecherry, LLC <maintainers@bluecherrydvr.com> > + * Copyright (C) 2015 Andrey Utkin <andrey.utkin@corp.bluecherry.net> I doubt that you have contract with your employer which permits you to claim copyright on the work/product. Signed-of-by != copyright. ---- > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver [not found] <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net> 2016-01-03 3:47 ` [RFC PATCH v0] Add tw5864 driver Joe Perches 2016-01-03 5:38 ` Leon Romanovsky @ 2016-01-11 10:52 ` Hans Verkuil 2016-01-15 2:13 ` Andrey Utkin 2 siblings, 1 reply; 13+ messages in thread From: Hans Verkuil @ 2016-01-11 10:52 UTC (permalink / raw) To: Andrey Utkin, Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman Cc: Andrey Utkin Hi Andrey, On 01/03/2016 02:41 AM, Andrey Utkin wrote: > (Disclaimer up to scissors mark) > > Please be so kind to take a look at a new driver. > We aim to follow high standards of kernel development and possibly to get this driver in mainline kernel. > The device is multichannel video and audio capture and compression chip TW5864 of Intersil/Techwell. > > The code is in repo https://github.com/bluecherrydvr/linux , branch "tw5864", subdir drivers/staging/media/tw5864 . > This branch is regularly rebased during development, so that there's a single commit adding this driver on top of current linux-next. > Direct link to subdir: https://github.com/bluecherrydvr/linux/tree/tw5864/drivers/staging/media/tw5864 > > > Implementation status > > * H.264 streaming work stable, including concurrent work of multiple channels on same chip; > * Audio streaming functionality is not implemented, but is considered for future; > * The chip has motion detection capability, but of same sensitivity level for whole frame; this was considered quite limiting for our needs and we have implemented per-grid-cell sensitivity with a bit of heuristic processing of motion vector data exposed by hardware. Datasheet-suggested mechanism is not used currently. > > Testing status > > * Runtime tests on my test machine show that video streaming works stable. Multichannel streaming was working, but I haven't test this with latest revision lately yet. > * Runtime performance will be tested later also on few early-adopters' CCTV machines. > * checkpatch.pl -f on files reports no warnings, errors or style issues; > * checkpatch.pl on patch reports no warnings, errors or style issues; > * sparse reports nothing; > * compilation shows no warnings (gcc 4.9.3); > * compilation with EXTRA_CFLAGS=-W shows a lot of warnings from included headers (over 9000 lines of output). Seems this technique from Documentation/SubmitChecklist is not practical in this case > * Other Documentation/SubmitChecklist advises weren't thoroughly worked out. Did you also test with v4l2-compliance? Before I accept the driver I want to see the output of 'v4l2-compliance' and 'v4l2-compliance -s'. Basically there shouldn't be any failures. I did a quick scan over the source and I saw nothing big. When you post the new version I will go over it more carefully and do a proper review. Regards, Hans ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-01-11 10:52 ` Hans Verkuil @ 2016-01-15 2:13 ` Andrey Utkin 2016-02-08 9:58 ` Hans Verkuil 0 siblings, 1 reply; 13+ messages in thread From: Andrey Utkin @ 2016-01-15 2:13 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin On Mon, Jan 11, 2016 at 12:52 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > Did you also test with v4l2-compliance? Before I accept the driver I want to see the > output of 'v4l2-compliance' and 'v4l2-compliance -s'. Basically there shouldn't be > any failures. > > I did a quick scan over the source and I saw nothing big. When you post the new > version I will go over it more carefully and do a proper review. Thank you for review. You can find output of v4l2-compliance and v4l2-compliance -s tests below (v4l-utils-1.8.0-42-gf1348b4). There are some issues, I will work on them. Could you please advise how could I optimize high-volume data passing to userspace apps? In one installation with 16 boards, when all streams are set to be 30 FPS all-I-frames (workaround for another P-frames quality issue), the host struggles from regular lockups, see message immediately below (4.2.0 kernel though, DKMS package with slightly modified code to match older API). I am not fully sure it is related to my driver, but I guess memcpy-ish approach to preparing video frames may be very CPU-expensive. 30 FPS all-I-frames video stream is like 3 megabytes per second. Of course this is not very good situation anyway, and I should either fix original issue or find some workarounds, at last to lower framerate. But still, optimization of data passing using some appropriate API like scatter-gather should be done I believe. I need hints how to do that right. Thanks in advance. Jan 13 19:35:20 cctv kernel: [34202.715069] NMI watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [khugepaged:69] Jan 13 19:35:20 cctv kernel: [34202.715071] Modules linked in: ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ast ttm drm_kms_helper drm syscopyarea sysfillrect sysimgblt xfs libcrc32c ipmi_ssif joydev input_leds intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd sb_edac edac_core lpc_ich mei_me mei shpchp wmi tw5864(OE) solo6x10 videobuf2_dma_contig videobuf2_dma_sg videobuf2_memops videobuf2_core v4l2_common videodev media ipmi_si 8250_fintek ipmi_msghandler snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd acpi_power_meter acpi_pad soundcore mac_hid parport_pc ppdev lp parport hid_generic usbhid hid igb i2c_algo_bit ahci dca ptp libahci megaraid_sas pps_core Jan 13 19:35:20 cctv kernel: [34202.715105] CPU: 2 PID: 69 Comm: khugepaged Tainted: G W OEL 4.2.0-23-generic #28~14.04.1-Ubuntu Jan 13 19:35:20 cctv kernel: [34202.715106] Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0 12/17/2015 Jan 13 19:35:20 cctv kernel: [34202.715107] task: ffff88046d7a6040 ti: ffff88046d0f4000 task.ti: ffff88046d0f4000 Jan 13 19:35:20 cctv kernel: [34202.715108] RIP: 0010:[<ffffffff810f4c49>] [<ffffffff810f4c49>] smp_call_function_many+0x1f9/0x250 Jan 13 19:35:20 cctv kernel: [34202.715112] RSP: 0018:ffff88046d0f7b58 EFLAGS: 00000202 Jan 13 19:35:20 cctv kernel: [34202.715112] RAX: 0000000000000003 RBX: 0000000000000000 RCX: ffff88047fd1a870 Jan 13 19:35:20 cctv kernel: [34202.715113] RDX: 0000000000000004 RSI: 0000000000000100 RDI: ffff88047fc97788 Jan 13 19:35:20 cctv kernel: [34202.715114] RBP: ffff88046d0f7b98 R08: 00000000000000fb R09: 0000000000000000 Jan 13 19:35:20 cctv kernel: [34202.715114] R10: 0000000000000004 R11: 0000000000000000 R12: 0000000000000297 Jan 13 19:35:20 cctv kernel: [34202.715115] R13: ffff88046d0f7b08 R14: 0000000000000202 R15: 0000000000000202 Jan 13 19:35:20 cctv kernel: [34202.715116] FS: 0000000000000000(0000) GS:ffff88047fc80000(0000) knlGS:0000000000000000 Jan 13 19:35:20 cctv kernel: [34202.715117] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Jan 13 19:35:20 cctv kernel: [34202.715118] CR2: 00007fae31dc0000 CR3: 0000000001c0d000 CR4: 00000000001406e0 Jan 13 19:35:20 cctv kernel: [34202.715118] Stack: Jan 13 19:35:20 cctv kernel: [34202.715119] 0000000000017740 01ff880400000001 0000000000000009 ffffffff81fb4ec0 Jan 13 19:35:20 cctv kernel: [34202.715120] ffffffff8117d4c0 0000000000000000 0000000000000002 0000000000000100 Jan 13 19:35:20 cctv kernel: [34202.715122] ffff88046d0f7bc8 ffffffff810f4e49 0000000000000100 0000000000000000 Jan 13 19:35:20 cctv kernel: [34202.715123] Call Trace: Jan 13 19:35:20 cctv kernel: [34202.715127] [<ffffffff8117d4c0>] ? page_alloc_cpu_notify+0x50/0x50 Jan 13 19:35:20 cctv kernel: [34202.715128] [<ffffffff810f4e49>] on_each_cpu_mask+0x29/0x70 Jan 13 19:35:20 cctv kernel: [34202.715129] [<ffffffff8117b513>] drain_all_pages+0xd3/0xf0 Jan 13 19:35:20 cctv kernel: [34202.715131] [<ffffffff8117f391>] __alloc_pages_nodemask+0x631/0x990 Jan 13 19:35:20 cctv kernel: [34202.715134] [<ffffffff811d5cb0>] khugepaged_scan_mm_slot+0x540/0xf50 Jan 13 19:35:20 cctv kernel: [34202.715136] [<ffffffff817b9b85>] ? schedule_timeout+0x165/0x2a0 Jan 13 19:35:20 cctv kernel: [34202.715137] [<ffffffff811d67dd>] khugepaged+0x11d/0x440 Jan 13 19:35:20 cctv kernel: [34202.715139] [<ffffffff810b73f0>] ? prepare_to_wait_event+0xf0/0xf0 Jan 13 19:35:20 cctv kernel: [34202.715141] [<ffffffff811d66c0>] ? khugepaged_scan_mm_slot+0xf50/0xf50 Jan 13 19:35:20 cctv kernel: [34202.715143] [<ffffffff810951b2>] kthread+0xd2/0xf0 Jan 13 19:35:20 cctv kernel: [34202.715144] [<ffffffff810950e0>] ? kthread_create_on_node+0x1c0/0x1c0 Jan 13 19:35:20 cctv kernel: [34202.715146] [<ffffffff817baedf>] ret_from_fork+0x3f/0x70 Jan 13 19:35:20 cctv kernel: [34202.715147] [<ffffffff810950e0>] ? kthread_create_on_node+0x1c0/0x1c0 Jan 13 19:35:20 cctv kernel: [34202.715147] Code: 51 2c 00 3b 05 ad 82 c3 00 89 c2 0f 8d 98 fe ff ff 48 98 49 8b 4d 00 48 03 0c c5 40 ae d2 81 8b 41 18 a8 01 74 ca f3 90 8b 41 18 <a8> 01 75 f7 eb bf 0f b6 4d c8 4c 89 fa 4c 89 f6 44 89 ef e8 7f # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -d /dev/video6 Driver Info: Driver name : tw5864 Card type : TW5864 Encoder 2 Bus info : PCI:0000:06:05.0 Driver version: 4.4.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video6 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: fail: v4l2-test-buffers.cpp(386): node->s_std(std) fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL test VIDIOC_EXPBUF: OK (Not Supported) Test input 0: Total: 42, Succeeded: 41, Failed: 1, Warnings: 0 [ERR] 01:58:05root@localhost /usr/local/src/linux # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -s -d /dev/video6 Driver Info: Driver name : tw5864 Card type : TW5864 Encoder 2 Bus info : PCI:0000:06:05.0 Driver version: 4.4.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video6 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: fail: v4l2-test-buffers.cpp(386): node->s_std(std) fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL test VIDIOC_EXPBUF: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(959): ret != EINVAL test MMAP: FAIL fail: v4l2-test-buffers.cpp(1043): buf.qbuf(node) fail: v4l2-test-buffers.cpp(1087): setupUserPtr(node, q) test USERPTR: FAIL test DMABUF: Cannot test, specify --expbuf-device Total: 45, Succeeded: 42, Failed: 3, Warnings: 0 [ERR] 01:58:11root@localhost /usr/local/src/linux # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -s --expbuf-device=/dev/video6 -d /dev/video6 Driver Info: Driver name : tw5864 Card type : TW5864 Encoder 2 Bus info : PCI:0000:06:05.0 Driver version: 4.4.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video6 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: fail: v4l2-test-buffers.cpp(386): node->s_std(std) fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL test VIDIOC_EXPBUF: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(959): ret != EINVAL test MMAP: FAIL fail: v4l2-test-buffers.cpp(1043): buf.qbuf(node) fail: v4l2-test-buffers.cpp(1087): setupUserPtr(node, q) test USERPTR: FAIL fail: v4l2-test-buffers.cpp(1111): exp_q.reqbufs(expbuf_node, q.g_buffers()) fail: v4l2-test-buffers.cpp(1200): setupDmaBuf(expbuf_node, node, q, exp_q) test DMABUF: FAIL Total: 46, Succeeded: 42, Failed: 4, Warnings: 0 [ERR] -- Bluecherry developer. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-01-15 2:13 ` Andrey Utkin @ 2016-02-08 9:58 ` Hans Verkuil 2016-02-08 10:23 ` Andrey Utkin 2016-03-09 14:29 ` Andrey Utkin 0 siblings, 2 replies; 13+ messages in thread From: Hans Verkuil @ 2016-02-08 9:58 UTC (permalink / raw) To: Andrey Utkin Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin Hi Andrey, Hmm, it looks like I forgot to reply. Sorry about that. On 01/15/2016 03:13 AM, Andrey Utkin wrote: > On Mon, Jan 11, 2016 at 12:52 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> Did you also test with v4l2-compliance? Before I accept the driver I want to see the >> output of 'v4l2-compliance' and 'v4l2-compliance -s'. Basically there shouldn't be >> any failures. >> >> I did a quick scan over the source and I saw nothing big. When you post the new >> version I will go over it more carefully and do a proper review. > > Thank you for review. > You can find output of v4l2-compliance and v4l2-compliance -s tests > below (v4l-utils-1.8.0-42-gf1348b4). > There are some issues, I will work on them. > Could you please advise how could I optimize high-volume data passing > to userspace apps? > In one installation with 16 boards, when all streams are set to be 30 > FPS all-I-frames (workaround for another P-frames quality issue), the > host struggles from regular lockups, see message immediately below > (4.2.0 kernel though, DKMS package with slightly modified code to > match older API). > I am not fully sure it is related to my driver, but I guess memcpy-ish > approach to preparing video frames may be very CPU-expensive. 30 FPS > all-I-frames video stream is like 3 megabytes per second. > Of course this is not very good situation anyway, and I should either > fix original issue or find some workarounds, at last to lower > framerate. But still, optimization of data passing using some > appropriate API like scatter-gather should be done I believe. I need > hints how to do that right. > Thanks in advance. If I understand it correctly the problem is that you need to insert h264 headers in front of the data, and for that you need to do a memcpy, for now at least. We are looking into mechanisms to pass the header separately from the video frame data, but I don't dare give an ETA when that arrives in the kernel. That said, it makes no sense to use H264 with I frames only, it defeats the purpose of H264. I would concentrate on making the encoder work correcly as that will dramatically reduce the amount of data to memcpy. For SDTV inputs you can get good quality using an MPEG-2 encoder with just 1 MB/s in my experience. H264 should reduce that further. And doing a memcpy for worst-case 4*16 = 64 MB/s should not be a problem at all for modern CPUs. Frankly, even 3*64 = 192 MB/s as you have now shouldn't pose a problem. I wouldn't change the memcpy: in my experience it is very useful to get a well-formed compressed stream out of the hardware. And the overhead of having to do a memcpy is a small price to pay and on modern CPUs should be barely noticeable for SDTV inputs. I don't believe that the lockups you see are related to the memcpy as such. The trace says that a cpu is stuck for 22s, no way that is related to something like that. It looks more like a deadlock somewhere. Regarding the compliance tests: don't pass VB2_USERPTR (doesn't work well with videobuf2-dma-contig). Also add vidioc_expbuf = vb2_ioctl_expbuf for the DMABUF support. That should clear up some of the errors you see. Regards, Hans > > Jan 13 19:35:20 cctv kernel: [34202.715069] NMI watchdog: BUG: soft > lockup - CPU#2 stuck for 22s! [khugepaged:69] > Jan 13 19:35:20 cctv kernel: [34202.715071] Modules linked in: ib_iser > rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp > libiscsi_tcp libiscsi scsi_transport_iscsi ast ttm drm_kms_helper drm > syscopyarea sysfillrect sysimgblt xfs libcrc32c ipmi_ssif joydev > input_leds intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp > coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul aesni_intel > aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd sb_edac > edac_core lpc_ich mei_me mei shpchp wmi tw5864(OE) solo6x10 > videobuf2_dma_contig videobuf2_dma_sg videobuf2_memops videobuf2_core > v4l2_common videodev media ipmi_si 8250_fintek ipmi_msghandler snd_pcm > snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device > snd_timer snd acpi_power_meter acpi_pad soundcore mac_hid parport_pc > ppdev lp parport hid_generic usbhid hid igb i2c_algo_bit ahci dca ptp > libahci megaraid_sas pps_core > Jan 13 19:35:20 cctv kernel: [34202.715105] CPU: 2 PID: 69 Comm: > khugepaged Tainted: G W OEL 4.2.0-23-generic > #28~14.04.1-Ubuntu > Jan 13 19:35:20 cctv kernel: [34202.715106] Hardware name: Supermicro > Super Server/X10SRi-F, BIOS 2.0 12/17/2015 > Jan 13 19:35:20 cctv kernel: [34202.715107] task: ffff88046d7a6040 ti: > ffff88046d0f4000 task.ti: ffff88046d0f4000 > Jan 13 19:35:20 cctv kernel: [34202.715108] RIP: > 0010:[<ffffffff810f4c49>] [<ffffffff810f4c49>] > smp_call_function_many+0x1f9/0x250 > Jan 13 19:35:20 cctv kernel: [34202.715112] RSP: 0018:ffff88046d0f7b58 > EFLAGS: 00000202 > Jan 13 19:35:20 cctv kernel: [34202.715112] RAX: 0000000000000003 RBX: > 0000000000000000 RCX: ffff88047fd1a870 > Jan 13 19:35:20 cctv kernel: [34202.715113] RDX: 0000000000000004 RSI: > 0000000000000100 RDI: ffff88047fc97788 > Jan 13 19:35:20 cctv kernel: [34202.715114] RBP: ffff88046d0f7b98 R08: > 00000000000000fb R09: 0000000000000000 > Jan 13 19:35:20 cctv kernel: [34202.715114] R10: 0000000000000004 R11: > 0000000000000000 R12: 0000000000000297 > Jan 13 19:35:20 cctv kernel: [34202.715115] R13: ffff88046d0f7b08 R14: > 0000000000000202 R15: 0000000000000202 > Jan 13 19:35:20 cctv kernel: [34202.715116] FS: > 0000000000000000(0000) GS:ffff88047fc80000(0000) > knlGS:0000000000000000 > Jan 13 19:35:20 cctv kernel: [34202.715117] CS: 0010 DS: 0000 ES: > 0000 CR0: 0000000080050033 > Jan 13 19:35:20 cctv kernel: [34202.715118] CR2: 00007fae31dc0000 CR3: > 0000000001c0d000 CR4: 00000000001406e0 > Jan 13 19:35:20 cctv kernel: [34202.715118] Stack: > Jan 13 19:35:20 cctv kernel: [34202.715119] 0000000000017740 > 01ff880400000001 0000000000000009 ffffffff81fb4ec0 > Jan 13 19:35:20 cctv kernel: [34202.715120] ffffffff8117d4c0 > 0000000000000000 0000000000000002 0000000000000100 > Jan 13 19:35:20 cctv kernel: [34202.715122] ffff88046d0f7bc8 > ffffffff810f4e49 0000000000000100 0000000000000000 > Jan 13 19:35:20 cctv kernel: [34202.715123] Call Trace: > Jan 13 19:35:20 cctv kernel: [34202.715127] [<ffffffff8117d4c0>] ? > page_alloc_cpu_notify+0x50/0x50 > Jan 13 19:35:20 cctv kernel: [34202.715128] [<ffffffff810f4e49>] > on_each_cpu_mask+0x29/0x70 > Jan 13 19:35:20 cctv kernel: [34202.715129] [<ffffffff8117b513>] > drain_all_pages+0xd3/0xf0 > Jan 13 19:35:20 cctv kernel: [34202.715131] [<ffffffff8117f391>] > __alloc_pages_nodemask+0x631/0x990 > Jan 13 19:35:20 cctv kernel: [34202.715134] [<ffffffff811d5cb0>] > khugepaged_scan_mm_slot+0x540/0xf50 > Jan 13 19:35:20 cctv kernel: [34202.715136] [<ffffffff817b9b85>] ? > schedule_timeout+0x165/0x2a0 > Jan 13 19:35:20 cctv kernel: [34202.715137] [<ffffffff811d67dd>] > khugepaged+0x11d/0x440 > Jan 13 19:35:20 cctv kernel: [34202.715139] [<ffffffff810b73f0>] ? > prepare_to_wait_event+0xf0/0xf0 > Jan 13 19:35:20 cctv kernel: [34202.715141] [<ffffffff811d66c0>] ? > khugepaged_scan_mm_slot+0xf50/0xf50 > Jan 13 19:35:20 cctv kernel: [34202.715143] [<ffffffff810951b2>] > kthread+0xd2/0xf0 > Jan 13 19:35:20 cctv kernel: [34202.715144] [<ffffffff810950e0>] ? > kthread_create_on_node+0x1c0/0x1c0 > Jan 13 19:35:20 cctv kernel: [34202.715146] [<ffffffff817baedf>] > ret_from_fork+0x3f/0x70 > Jan 13 19:35:20 cctv kernel: [34202.715147] [<ffffffff810950e0>] ? > kthread_create_on_node+0x1c0/0x1c0 > Jan 13 19:35:20 cctv kernel: [34202.715147] Code: 51 2c 00 3b 05 ad 82 > c3 00 89 c2 0f 8d 98 fe ff ff 48 98 49 8b 4d 00 48 03 0c c5 40 ae d2 > 81 8b 41 18 a8 01 74 ca f3 90 8b 41 18 <a8> 01 75 f7 eb bf 0f b6 4d c8 > 4c 89 fa 4c 89 f6 44 89 ef e8 7f > > > > # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -d /dev/video6 > Driver Info: > Driver name : tw5864 > Card type : TW5864 Encoder 2 > Bus info : PCI:0000:06:05.0 > Driver version: 4.4.0 > Capabilities : 0x85200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x05200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > > Compliance test for device /dev/video6 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Test input 0: > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > fail: v4l2-test-buffers.cpp(386): node->s_std(std) > fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL > test VIDIOC_EXPBUF: OK (Not Supported) > > Test input 0: > > > Total: 42, Succeeded: 41, Failed: 1, Warnings: 0 > [ERR] > 01:58:05root@localhost /usr/local/src/linux > # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -s -d /dev/video6 > Driver Info: > Driver name : tw5864 > Card type : TW5864 Encoder 2 > Bus info : PCI:0000:06:05.0 > Driver version: 4.4.0 > Capabilities : 0x85200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x05200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > > Compliance test for device /dev/video6 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Test input 0: > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > fail: v4l2-test-buffers.cpp(386): node->s_std(std) > fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL > test VIDIOC_EXPBUF: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK > fail: v4l2-test-buffers.cpp(959): ret != EINVAL > test MMAP: FAIL > fail: v4l2-test-buffers.cpp(1043): buf.qbuf(node) > fail: v4l2-test-buffers.cpp(1087): setupUserPtr(node, q) > test USERPTR: FAIL > test DMABUF: Cannot test, specify --expbuf-device > > > Total: 45, Succeeded: 42, Failed: 3, Warnings: 0 > [ERR] > 01:58:11root@localhost /usr/local/src/linux > # /src/v4l-utils/utils/v4l2-compliance/v4l2-compliance -s > --expbuf-device=/dev/video6 -d /dev/video6 > Driver Info: > Driver name : tw5864 > Card type : TW5864 Encoder 2 > Bus info : PCI:0000:06:05.0 > Driver version: 4.4.0 > Capabilities : 0x85200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x05200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > > Compliance test for device /dev/video6 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Test input 0: > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > fail: v4l2-test-buffers.cpp(386): node->s_std(std) > fail: v4l2-test-buffers.cpp(500): testCanSetSameTimings(node) > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL > test VIDIOC_EXPBUF: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK > fail: v4l2-test-buffers.cpp(959): ret != EINVAL > test MMAP: FAIL > fail: v4l2-test-buffers.cpp(1043): buf.qbuf(node) > fail: v4l2-test-buffers.cpp(1087): setupUserPtr(node, q) > test USERPTR: FAIL > fail: v4l2-test-buffers.cpp(1111): > exp_q.reqbufs(expbuf_node, q.g_buffers()) > fail: v4l2-test-buffers.cpp(1200): > setupDmaBuf(expbuf_node, node, q, exp_q) > test DMABUF: FAIL > > > Total: 46, Succeeded: 42, Failed: 4, Warnings: 0 > [ERR] > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-02-08 9:58 ` Hans Verkuil @ 2016-02-08 10:23 ` Andrey Utkin 2016-02-08 10:29 ` Hans Verkuil 2016-03-09 14:29 ` Andrey Utkin 1 sibling, 1 reply; 13+ messages in thread From: Andrey Utkin @ 2016-02-08 10:23 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin On Mon, Feb 8, 2016 at 11:58 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > Hi Andrey, > > Hmm, it looks like I forgot to reply. Sorry about that. Thank you very much anyway. > I wouldn't change the memcpy: in my experience it is very useful to get a > well-formed compressed stream out of the hardware. And the overhead of > having to do a memcpy is a small price to pay and on modern CPUs should > be barely noticeable for SDTV inputs. So there's no usecase for scatter-gather approach, right? > I don't believe that the lockups you see are related to the memcpy as > such. The trace says that a cpu is stuck for 22s, no way that is related > to something like that. It looks more like a deadlock somewhere. There was a locking issue (lack of _irqsave) and was fixed since then. > Regarding the compliance tests: don't pass VB2_USERPTR (doesn't work well > with videobuf2-dma-contig). Also add vidioc_expbuf = vb2_ioctl_expbuf for > the DMABUF support. That should clear up some of the errors you see. Thank you! -- Bluecherry developer. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-02-08 10:23 ` Andrey Utkin @ 2016-02-08 10:29 ` Hans Verkuil 0 siblings, 0 replies; 13+ messages in thread From: Hans Verkuil @ 2016-02-08 10:29 UTC (permalink / raw) To: Andrey Utkin Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Ezequiel Garcia, Greg Kroah-Hartman, Andrey Utkin On 02/08/2016 11:23 AM, Andrey Utkin wrote: > On Mon, Feb 8, 2016 at 11:58 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> Hi Andrey, >> >> Hmm, it looks like I forgot to reply. Sorry about that. > > Thank you very much anyway. > >> I wouldn't change the memcpy: in my experience it is very useful to get a >> well-formed compressed stream out of the hardware. And the overhead of >> having to do a memcpy is a small price to pay and on modern CPUs should >> be barely noticeable for SDTV inputs. > > So there's no usecase for scatter-gather approach, right? The only advantage scatter-gather would bring is more efficient memory usage: dma-contig requires physically contiguous memory, dma-sg doesn't. If you need a lot of contiguous memory you can run into out-of-memory situations. The alternative is to build a kernel with CMA enabled and reserve memory that way. dma-sg doesn't have these problems, so that can be a good alternative, but it comes at the price of higher complexity. > >> I don't believe that the lockups you see are related to the memcpy as >> such. The trace says that a cpu is stuck for 22s, no way that is related >> to something like that. It looks more like a deadlock somewhere. > > There was a locking issue (lack of _irqsave) and was fixed since then. > >> Regarding the compliance tests: don't pass VB2_USERPTR (doesn't work well >> with videobuf2-dma-contig). Also add vidioc_expbuf = vb2_ioctl_expbuf for >> the DMABUF support. That should clear up some of the errors you see. > > Thank you! > Regards, Hans ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-02-08 9:58 ` Hans Verkuil 2016-02-08 10:23 ` Andrey Utkin @ 2016-03-09 14:29 ` Andrey Utkin 2016-03-11 8:00 ` Hans Verkuil 1 sibling, 1 reply; 13+ messages in thread From: Andrey Utkin @ 2016-03-09 14:29 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Andrey Utkin Hi Hans! Some improvements took place on the driver, including cleaner v4l2-compliance tests passing. But there's a single test failure I don't understand. In the code of v4l2-compliance, it seems like an API call CREATE_BUFS is supposed to fail with EINVAL. But in case of my driver, which simply uses vb2_ioctl_create_bufs(), the call returns 0. Please review the below report. The report is produced by fresh v4l-utils from git: v4l-utils-1.10.0-59-g1388c0a The actual code which was tested is at tag release/tw5864/1.10 of https://github.com/bluecherrydvr/linux.git , see drivers/staging/media/tw5864 . If we sort this out, I am considering resubmit the driver for merging upstream. There's also another issue with v4l2-compliance. At some moment the driver receives S_PARM command with timeperframe 0/0, by which reason I added special handling, but I guess there's something out of my knowledge which caused this and which should be fixed. # v4l2-compliance -d /dev/video6 -s Driver Info: Driver name : tw5864 Card type : TW5864 Encoder 2 Bus info : PCI:0000:06:05.0 Driver version: 4.5.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video6 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK Test input 0: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(959): ret != EINVAL test MMAP: FAIL test USERPTR: OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total: 45, Succeeded: 44, Failed: 1, Warnings: 0 [ERR] 14:14:15root@tw ~ # gdb v4l2-compliance GNU gdb (Ubuntu 7.10-1ubuntu3) 7.10 Copyright (C) 2015 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i686-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from v4l2-compliance...done. (gdb) break v4l2-test-buffers.cpp:959 Breakpoint 1 at 0x8071469: file v4l2-test-buffers.cpp, line 959. (gdb) run -d /dev/video6 -s Starting program: /usr/local/bin/v4l2-compliance -d /dev/video6 -s [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1". Driver Info: Driver name : tw5864 Card type : TW5864 Encoder 2 Bus info : PCI:0000:06:05.0 Driver version: 4.5.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video6 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 11 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK Test input 0: Streaming ioctls: test read/write: OK Breakpoint 1, testMmap (node=0xbfffd1e4, frame_count`) at v4l2-test-buffers.cpp:959 959 fail_on_test(ret != EINVAL); (gdb) print ret $1 = 0 (gdb) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-03-09 14:29 ` Andrey Utkin @ 2016-03-11 8:00 ` Hans Verkuil 2016-03-11 8:40 ` Andrey Utkin 0 siblings, 1 reply; 13+ messages in thread From: Hans Verkuil @ 2016-03-11 8:00 UTC (permalink / raw) To: Andrey Utkin Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Andrey Utkin On 03/09/2016 03:29 PM, Andrey Utkin wrote: > Hi Hans! > > Some improvements took place on the driver, including cleaner > v4l2-compliance tests passing. But there's a single test failure I > don't understand. > > In the code of v4l2-compliance, it seems like an API > call CREATE_BUFS is supposed to fail with EINVAL. But in case of my > driver, which simply uses vb2_ioctl_create_bufs(), the call returns 0. > > Please review the below report. > > The report is produced by fresh v4l-utils from git: > v4l-utils-1.10.0-59-g1388c0a > > The actual code which was tested is at tag release/tw5864/1.10 of > https://github.com/bluecherrydvr/linux.git , see > drivers/staging/media/tw5864 . If we sort this out, I am considering > resubmit the driver for merging upstream. The reason is likely to be the tw5864_queue_setup function which has not been updated to handle CREATE_BUFS support correctly. It should look like this: static int tw5864_queue_setup(struct vb2_queue *q, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]) { struct tw5864_input *dev = vb2_get_drv_priv(q); if (q->num_buffers + *num_buffers < 12) *num_buffers = 12 - q->num_buffers; alloc_ctxs[0] = dev->alloc_ctx; if (*num_planes) return sizes[0] < H264_VLC_BUF_SIZE ? -EINVAL : 0; sizes[0] = H264_VLC_BUF_SIZE; *num_planes = 1; return 0; } > > There's also another issue with v4l2-compliance. At some moment the > driver receives S_PARM command with timeperframe 0/0, by which reason I > added special handling, but I guess there's something out of my > knowledge which caused this and which should be fixed. A timeperframe value of 0/0 is valid and the driver is supposed to replace it with the default timeperframe. Which is why v4l2-compliance tests for this corner case. Regards, Hans > > > # v4l2-compliance -d /dev/video6 -s > Driver Info: > Driver name : tw5864 > Card type : TW5864 Encoder 2 > Bus info : PCI:0000:06:05.0 > Driver version: 4.5.0 > Capabilities : 0x85200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x05200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > > Compliance test for device /dev/video6 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Test input 0: > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > > Test input 0: > > Streaming ioctls: > test read/write: OK > fail: v4l2-test-buffers.cpp(959): ret != EINVAL > test MMAP: FAIL > test USERPTR: OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > > Total: 45, Succeeded: 44, Failed: 1, Warnings: 0 > [ERR] > 14:14:15root@tw ~ > # gdb v4l2-compliance > GNU gdb (Ubuntu 7.10-1ubuntu3) 7.10 > Copyright (C) 2015 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later > <http://gnu.org/licenses/gpl.html> This is free software: you are free > to change and redistribute it. There is NO WARRANTY, to the extent > permitted by law. Type "show copying" and "show warranty" for details. > This GDB was configured as "i686-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <http://www.gnu.org/software/gdb/bugs/>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > For help, type "help". > Type "apropos word" to search for commands related to "word"... > Reading symbols from v4l2-compliance...done. > (gdb) break v4l2-test-buffers.cpp:959 > Breakpoint 1 at 0x8071469: file v4l2-test-buffers.cpp, line 959. > (gdb) run -d /dev/video6 -s > Starting program: /usr/local/bin/v4l2-compliance -d /dev/video6 -s > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1". > Driver Info: > Driver name : tw5864 > Card type : TW5864 Encoder 2 > Bus info : PCI:0000:06:05.0 > Driver version: 4.5.0 > Capabilities : 0x85200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x05200001 > Video Capture > Read/Write > Streaming > Extended Pix Format > > Compliance test for device /dev/video6 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Test input 0: > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 11 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > > Test input 0: > > Streaming ioctls: > test read/write: OK > > Breakpoint 1, testMmap (node=0xbfffd1e4, frame_count`) at v4l2-test-buffers.cpp:959 > 959 fail_on_test(ret != EINVAL); > (gdb) print ret > $1 = 0 > (gdb) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-03-11 8:00 ` Hans Verkuil @ 2016-03-11 8:40 ` Andrey Utkin 2016-03-11 9:59 ` Hans Verkuil 0 siblings, 1 reply; 13+ messages in thread From: Andrey Utkin @ 2016-03-11 8:40 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Andrey Utkin On Fri, 11 Mar 2016 09:00:18 +0100 Hans Verkuil <hverkuil@xs4all.nl> wrote: > The reason is likely to be the tw5864_queue_setup function which has > not been updated to handle CREATE_BUFS support correctly. It should > look like this: > > static int tw5864_queue_setup(struct vb2_queue *q, > unsigned int *num_buffers, > unsigned int *num_planes, unsigned int > sizes[], void *alloc_ctxs[]) > { > struct tw5864_input *dev = vb2_get_drv_priv(q); > > if (q->num_buffers + *num_buffers < 12) > *num_buffers = 12 - q->num_buffers; > > alloc_ctxs[0] = dev->alloc_ctx; > if (*num_planes) > return sizes[0] < H264_VLC_BUF_SIZE ? -EINVAL : 0; > > sizes[0] = H264_VLC_BUF_SIZE; > *num_planes = 1; > > return 0; > } Thanks for suggestion, but now the failure looks this way: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(297): g_field() = V4L2_FIELD_ANY fail: v4l2-test-buffers.cpp(703): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(976): captureBufs(node, q, m2m_q, frame_count, false) test MMAP: FAIL Will check that later. If you have any suggestions, I would be very grateful. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-03-11 8:40 ` Andrey Utkin @ 2016-03-11 9:59 ` Hans Verkuil 2016-03-11 13:23 ` Andrey Utkin 0 siblings, 1 reply; 13+ messages in thread From: Hans Verkuil @ 2016-03-11 9:59 UTC (permalink / raw) To: Andrey Utkin Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Andrey Utkin On 03/11/2016 09:40 AM, Andrey Utkin wrote: > On Fri, 11 Mar 2016 09:00:18 +0100 > Hans Verkuil <hverkuil@xs4all.nl> wrote: >> The reason is likely to be the tw5864_queue_setup function which has >> not been updated to handle CREATE_BUFS support correctly. It should >> look like this: >> >> static int tw5864_queue_setup(struct vb2_queue *q, >> unsigned int *num_buffers, >> unsigned int *num_planes, unsigned int >> sizes[], void *alloc_ctxs[]) >> { >> struct tw5864_input *dev = vb2_get_drv_priv(q); >> >> if (q->num_buffers + *num_buffers < 12) >> *num_buffers = 12 - q->num_buffers; >> >> alloc_ctxs[0] = dev->alloc_ctx; >> if (*num_planes) >> return sizes[0] < H264_VLC_BUF_SIZE ? -EINVAL : 0; >> >> sizes[0] = H264_VLC_BUF_SIZE; >> *num_planes = 1; >> >> return 0; >> } > > Thanks for suggestion, but now the failure looks this way: > > Streaming ioctls: > test read/write: OK > fail: v4l2-test-buffers.cpp(297): g_field() = V4L2_FIELD_ANY While userspace may specify FIELD_ANY when setting a format, the driver should always map that to a specific field setting and should never return FIELD_ANY back to userspace. In this case, the 'field' field of the v4l2_buffer struct has FIELD_ANY which means it is not set correctly (or at all) in the driver. It's a common mistake, which is why v4l2-compliance tests for it :-) Regards, Hans > fail: v4l2-test-buffers.cpp(703): buf.check(q, last_seq) > fail: v4l2-test-buffers.cpp(976): captureBufs(node, q, m2m_q, frame_count, false) > test MMAP: FAIL > > Will check that later. If you have any suggestions, I would be very > grateful. > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v0] Add tw5864 driver 2016-03-11 9:59 ` Hans Verkuil @ 2016-03-11 13:23 ` Andrey Utkin 0 siblings, 0 replies; 13+ messages in thread From: Andrey Utkin @ 2016-03-11 13:23 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media, linux-kernel@vger.kernel.org, kernel-mentors@selenic.com, devel, kernel-janitors, Mauro Carvalho Chehab, Andrey Utkin On Fri, 11 Mar 2016 10:59:02 +0100 Hans Verkuil <hverkuil@xs4all.nl> wrote: > While userspace may specify FIELD_ANY when setting a format, the > driver should always map that to a specific field setting and should > never return FIELD_ANY back to userspace. > > In this case, the 'field' field of the v4l2_buffer struct has > FIELD_ANY which means it is not set correctly (or at all) in the > driver. > > It's a common mistake, which is why v4l2-compliance tests for it :-) Thanks for great guidance Hans, finally I have solved all issues. You can review latest state at tw5864 branch, also you can review changelog of v4l2-compliance fixing at tags tw5864_pre_1.11, tw5864_pre_1.10 of https://github.com/bluecherrydvr/linux . I will make a final internal review before submission, and try to submit the driver for inclusion. Everybody is appreciated to make any comments even before submission, the actual code to review is at tw5864 branch. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-03-11 13:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net>
2016-01-03 3:47 ` [RFC PATCH v0] Add tw5864 driver Joe Perches
2016-01-03 13:34 ` Andrey Utkin
2016-01-03 5:38 ` Leon Romanovsky
2016-01-11 10:52 ` Hans Verkuil
2016-01-15 2:13 ` Andrey Utkin
2016-02-08 9:58 ` Hans Verkuil
2016-02-08 10:23 ` Andrey Utkin
2016-02-08 10:29 ` Hans Verkuil
2016-03-09 14:29 ` Andrey Utkin
2016-03-11 8:00 ` Hans Verkuil
2016-03-11 8:40 ` Andrey Utkin
2016-03-11 9:59 ` Hans Verkuil
2016-03-11 13:23 ` Andrey Utkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox