mbox series

[RFC,v9,0/9] Add Tegra driver for video capture

Message ID 1587536339-4030-1-git-send-email-skomatineni@nvidia.com (mailing list archive)
Headers show
Series Add Tegra driver for video capture | expand

Message

Sowjanya Komatineni April 22, 2020, 6:18 a.m. UTC 
This series adds Tegra210 VI and CSI driver for built-in test pattern
generator (TPG) capture.

Tegra210 supports max 6 channels on VI and 6 ports on CSI where each
CSI port is one-to-one mapped to VI channel for video capture.

This series has TPG support only where it creates hard media links
between CSI subdevice and VI video device without device graphs.

v4l2-compliance results are available below the patch diff.

[v9]:	Includes,
	- small fix to explicitly check for both vi and csi channels
	  availability before TPG setup and cleanup so in the cases
	  of later Tegras where CSI is not child to VI and if either
	  of the platform drivers are not registered, TPG setup will be
	  skipped.

[v8]:	Includes,
	- minor change to use device managed allocation fo vi and csi for now.
	  May need to change back to non device managed allocation later when
	  support for direct host1x client driver unbind and bind is added.

[v7]:	Includes,
	- v6 feedback
	- moved registering v4l2 nodes and creating tpg media links to happen
	  after both host1x client inits so during direct host1x client driver
	  unbind and bind order of client unregister/register will not impact.
	- All channels resources and freeing happens during v4l2 device release
	  callback.
	- module unload/load works with below host1x bus driver fix.
	  http://patchwork.ozlabs.org/patch/1268191/

[v6]:	Includes,
	- v5 feedback
	- fix for csi_tpg clock parent
	- fix to free channel resources in video device release callback
	  for registered video devices as resource might still be in use
	  when application holds handle to it during driver unbind.
	- added blanking intervals based on resolution and bpp for csi
	  internal tpg.
	- added implementation for subdev pad ops enum_frame_size and
	  enum_frame_interval.

[v5]:	Includes,
	- v4 feedback
	- fix for venc powergate mc reset order.
	- fix to have unbind and bind work during v4l2-ctl sleep and streaming.

[v4]:	Includes,
	- v3 feedback changes and some improvements
	- Fixes tegra_channel_buffer struct to use v4l2 buffer as first
	  member. This also fixes crash of unable to handle kernel write
	  to read-only memory.
	- Uses separate host1x sync ids for frame start and memory write
	  ack as single sync id for both can cause sync loss between exact
	  frame start and memory write ack events.
	- Uses client managed host1x syncpoints.
	- Includes fix to increment syncpoint counter to match cached value
	  to synchronize in case of timeouts or missed hardware triggers.
	- Frame start and memory write ack syncpoint FIFO's are of size 2.
	  So, updated capture logic to avoid adding more than 2 sync point
	  condition requests to FIFOs to avoid overflow.
	- Implemented PM ops for runtime suspend and resume along with generic
	  power domains to allow proper power gate and ungate sequencing along
	  with MC VI flush during power gate.
	- Fixed Tegra210 device tree sor power domain clocks.
	- Added missing reset-cells to mc node.

[v3]:	Includes,
	- video device node handling set/get formats of all devices
	  in the pipeline.
	- Removed subdev nodes.
	- Fixed frame sync timeout issue due to CSI clocks not properly
	  set for corresponding blocks.
	- uses minimum 3 buffers to be queued to fixed memory race between
	  DMA writes and userspace reads causing kernel hang reporting
	  kernel write to read-only memory.
	- Improved capture threads and done threads to avoid possible
	  race conditions and added recovery in case of frame sync timeout.
	- Passes all the V4L compliance tests.

[v2]:	Includes,
	- v0 feedback
	- Merged files to have Tegra specific separately
	- Moved CSI device as child to VI as Tegra210 CSI is
	  part of VI sharing same host interface and register
	  space.
	- Added link_validate for format validation.
	- Fixes for passing v4l2-compliance for media, video,
	  and subdevices.

[v1]:	Includes,
	- Adds CSI TPG clock to Tegra210 clock driver
	- Host1x video driver with VI and CSI clients.
	- Support for Tegra210 only.
	- VI CSI TPG support with hard media links in driver.
	- Video formats supported by Tegra210 VI
	- CSI TPG supported video formats



Sowjanya Komatineni (9):
  arm64: tegra: Fix sor powergate clocks and reset
  arm64: tegra: Add reset-cells to mc
  dt-bindings: clock: tegra: Add clk id for CSI TPG clock
  clk: tegra: Add Tegra210 CSI TPG clock gate
  dt-binding: tegra: Add VI and CSI bindings
  media: tegra: Add Tegra210 Video input driver
  MAINTAINERS: Add Tegra Video driver section
  dt-bindings: reset: Add ID for Tegra210 VI reset
  arm64: tegra: Add Tegra VI CSI support in device tree

 .../display/tegra/nvidia,tegra20-host1x.txt        |   73 +-
 MAINTAINERS                                        |   10 +
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   10 +
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   52 +-
 drivers/clk/tegra/clk-tegra210.c                   |    7 +
 drivers/staging/media/Kconfig                      |    2 +
 drivers/staging/media/Makefile                     |    1 +
 drivers/staging/media/tegra/Kconfig                |   13 +
 drivers/staging/media/tegra/Makefile               |    8 +
 drivers/staging/media/tegra/TODO                   |   10 +
 drivers/staging/media/tegra/common.h               |  262 +++++
 drivers/staging/media/tegra/csi.c                  |  606 +++++++++++
 drivers/staging/media/tegra/csi.h                  |  149 +++
 drivers/staging/media/tegra/tegra210.c             |  709 ++++++++++++
 drivers/staging/media/tegra/tegra210.h             |  190 ++++
 drivers/staging/media/tegra/vi.c                   | 1132 ++++++++++++++++++++
 drivers/staging/media/tegra/vi.h                   |   83 ++
 drivers/staging/media/tegra/video.c                |  153 +++
 drivers/staging/media/tegra/video.h                |   34 +
 include/dt-bindings/clock/tegra210-car.h           |    2 +-
 include/dt-bindings/reset/tegra210-car.h           |    1 +
 21 files changed, 3490 insertions(+), 17 deletions(-)
 create mode 100644 drivers/staging/media/tegra/Kconfig
 create mode 100644 drivers/staging/media/tegra/Makefile
 create mode 100644 drivers/staging/media/tegra/TODO
 create mode 100644 drivers/staging/media/tegra/common.h
 create mode 100644 drivers/staging/media/tegra/csi.c
 create mode 100644 drivers/staging/media/tegra/csi.h
 create mode 100644 drivers/staging/media/tegra/tegra210.c
 create mode 100644 drivers/staging/media/tegra/tegra210.h
 create mode 100644 drivers/staging/media/tegra/vi.c
 create mode 100644 drivers/staging/media/tegra/vi.h
 create mode 100644 drivers/staging/media/tegra/video.c
 create mode 100644 drivers/staging/media/tegra/video.h


v4l2-compliance SHA: 81e45d957c4db39397f893100b3d2729ef39b052, 32 bits, 32-bit time_t

Compliance test for tegra-video device /dev/media0:

Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0

Required ioctls:
        test MEDIA_IOC_DEVICE_INFO: OK

Allow for multiple opens:
        test second /dev/media0 open: OK
        test MEDIA_IOC_DEVICE_INFO: OK
        test for unlimited opens: OK

Media Controller ioctls:
        test MEDIA_IOC_G_TOPOLOGY: OK
        Entities: 12 Interfaces: 6 Pads: 12 Links: 12
        test MEDIA_IOC_ENUM_ENTITIES/LINKS: OK
        test MEDIA_IOC_SETUP_LINK: OK
        test invalid ioctls: OK

Total for tegra-video device /dev/media0: 8, Succeeded: 8, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video0:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-0
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x03000003
        Type             : V4L Video
Entity Info:
        ID               : 0x00000001 (1)
        Name             : 54080000.vi-output-0
        Function         : V4L2 I/O
        Pad 0x01000002   : 0: Sink
          Link 0x02000007: from remote pad 0x1000006 of entity 'tpg-0': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video0 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video0: 53, Succeeded: 53, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video1:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-1
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x0300000b
        Type             : V4L Video
Entity Info:
        ID               : 0x00000009 (9)
        Name             : 54080000.vi-output-1
        Function         : V4L2 I/O
        Pad 0x0100000a   : 0: Sink
          Link 0x0200000f: from remote pad 0x100000e of entity 'tpg-1': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video1 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video1: 53, Succeeded: 53, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video2:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-2
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x03000013
        Type             : V4L Video
Entity Info:
        ID               : 0x00000011 (17)
        Name             : 54080000.vi-output-2
        Function         : V4L2 I/O
        Pad 0x01000012   : 0: Sink
          Link 0x02000017: from remote pad 0x1000016 of entity 'tpg-2': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video2 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video2: 53, Succeeded: 53, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video3:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-3
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x0300001b
        Type             : V4L Video
Entity Info:
        ID               : 0x00000019 (25)
        Name             : 54080000.vi-output-3
        Function         : V4L2 I/O
        Pad 0x0100001a   : 0: Sink
          Link 0x0200001f: from remote pad 0x100001e of entity 'tpg-3': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video3 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video3: 53, Succeeded: 53, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video4:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-4
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x03000023
        Type             : V4L Video
Entity Info:
        ID               : 0x00000021 (33)
        Name             : 54080000.vi-output-4
        Function         : V4L2 I/O
        Pad 0x01000022   : 0: Sink
          Link 0x02000027: from remote pad 0x1000026 of entity 'tpg-4': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video4 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video4: 53, Succeeded: 53, Failed: 0, Warnings: 0
--------------------------------------------------------------------------------
Compliance test for tegra-video device /dev/video5:

Driver Info:
        Driver name      : tegra-video
        Card type        : 54080000.vi-output-5
        Bus info         : platform:54080000.vi
        Driver version   : 5.6.0
        Capabilities     : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : tegra-video
        Model            : NVIDIA Tegra Video Input Device
        Serial           : 
        Bus info         : platform:54080000.vi
        Media version    : 5.6.0
        Hardware revision: 0x00000003 (3)
        Driver version   : 5.6.0
Interface Info:
        ID               : 0x0300002b
        Type             : V4L Video
Entity Info:
        ID               : 0x00000029 (41)
        Name             : 54080000.vi-output-5
        Function         : V4L2 I/O
        Pad 0x0100002a   : 0: Sink
          Link 0x0200002f: from remote pad 0x100002e of entity 'tpg-5': Data, Enabled

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video5 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

        test invalid ioctls: OK
Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

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 (Not Supported)
        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)

Control ioctls (Input 0):
        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: 2 Private Controls: 0

Format ioctls (Input 0):
        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 (Input 0):
        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 (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
        test read/write: OK
        test blocking wait: OK
        test MMAP (no poll): OK                           
        test MMAP (select): OK                            
        test MMAP (epoll): OK                             
        test USERPTR (no poll): OK (Not Supported)
        test USERPTR (select): OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device

Total for tegra-video device /dev/video5: 53, Succeeded: 53, Failed: 0, Warnings: 0

Grand Total for tegra-video device /dev/media0: 326, Succeeded: 326, Failed: 0, Warnings: 0

Comments

Hans Verkuil April 23, 2020, 7:48 a.m. UTC | #1 
On 22/04/2020 08:18, Sowjanya Komatineni wrote:
> Tegra210 contains a powerful Video Input (VI) hardware controller
> which can support up to 6 MIPI CSI camera sensors.
> 
> Each Tegra CSI port can be one-to-one mapped to VI channel and can
> capture from an external camera sensor connected to CSI or from
> built-in test pattern generator.
> 
> Tegra210 supports built-in test pattern generator from CSI to VI.
> 
> This patch adds a v4l2 capture driver with media interface for
> Tegra210 built-in CSI to VI test pattern generator.
> 
> This patch includes TPG support only and all the video pipeline
> configuration happens through the video device node.
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/staging/media/Kconfig          |    2 +
>  drivers/staging/media/Makefile         |    1 +
>  drivers/staging/media/tegra/Kconfig    |   13 +
>  drivers/staging/media/tegra/Makefile   |    8 +
>  drivers/staging/media/tegra/TODO       |   10 +
>  drivers/staging/media/tegra/common.h   |  262 ++++++++
>  drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>  drivers/staging/media/tegra/csi.h      |  149 +++++
>  drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>  drivers/staging/media/tegra/tegra210.h |  190 ++++++
>  drivers/staging/media/tegra/vi.c       | 1132 ++++++++++++++++++++++++++++++++
>  drivers/staging/media/tegra/vi.h       |   83 +++
>  drivers/staging/media/tegra/video.c    |  153 +++++
>  drivers/staging/media/tegra/video.h    |   34 +
>  14 files changed, 3352 insertions(+)
>  create mode 100644 drivers/staging/media/tegra/Kconfig
>  create mode 100644 drivers/staging/media/tegra/Makefile
>  create mode 100644 drivers/staging/media/tegra/TODO
>  create mode 100644 drivers/staging/media/tegra/common.h
>  create mode 100644 drivers/staging/media/tegra/csi.c
>  create mode 100644 drivers/staging/media/tegra/csi.h
>  create mode 100644 drivers/staging/media/tegra/tegra210.c
>  create mode 100644 drivers/staging/media/tegra/tegra210.h
>  create mode 100644 drivers/staging/media/tegra/vi.c
>  create mode 100644 drivers/staging/media/tegra/vi.h
>  create mode 100644 drivers/staging/media/tegra/video.c
>  create mode 100644 drivers/staging/media/tegra/video.h

With 'make menuconfig' I get this:

scripts/kconfig/mconf  Kconfig

WARNING: unmet direct dependencies detected for TEGRA_HOST1X
  Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM && COMPILE_TEST [=y])
  Selected by [y]:
  - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])

This is an x86_64 build with COMPILE_TEST set. I can provide my full .config if you need it.

CONFIG_TEGRA_HOST1X=y
CONFIG_VIDEO_TEGRA=y

Regards,

	Hans
Sowjanya Komatineni April 23, 2020, 4:50 p.m. UTC | #2 
On 4/23/20 12:48 AM, Hans Verkuil wrote:
> External email: Use caution opening links or attachments
>
>
> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>> Tegra210 contains a powerful Video Input (VI) hardware controller
>> which can support up to 6 MIPI CSI camera sensors.
>>
>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>> capture from an external camera sensor connected to CSI or from
>> built-in test pattern generator.
>>
>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>
>> This patch adds a v4l2 capture driver with media interface for
>> Tegra210 built-in CSI to VI test pattern generator.
>>
>> This patch includes TPG support only and all the video pipeline
>> configuration happens through the video device node.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>   drivers/staging/media/Kconfig          |    2 +
>>   drivers/staging/media/Makefile         |    1 +
>>   drivers/staging/media/tegra/Kconfig    |   13 +
>>   drivers/staging/media/tegra/Makefile   |    8 +
>>   drivers/staging/media/tegra/TODO       |   10 +
>>   drivers/staging/media/tegra/common.h   |  262 ++++++++
>>   drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>>   drivers/staging/media/tegra/csi.h      |  149 +++++
>>   drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>>   drivers/staging/media/tegra/tegra210.h |  190 ++++++
>>   drivers/staging/media/tegra/vi.c       | 1132 ++++++++++++++++++++++++++++++++
>>   drivers/staging/media/tegra/vi.h       |   83 +++
>>   drivers/staging/media/tegra/video.c    |  153 +++++
>>   drivers/staging/media/tegra/video.h    |   34 +
>>   14 files changed, 3352 insertions(+)
>>   create mode 100644 drivers/staging/media/tegra/Kconfig
>>   create mode 100644 drivers/staging/media/tegra/Makefile
>>   create mode 100644 drivers/staging/media/tegra/TODO
>>   create mode 100644 drivers/staging/media/tegra/common.h
>>   create mode 100644 drivers/staging/media/tegra/csi.c
>>   create mode 100644 drivers/staging/media/tegra/csi.h
>>   create mode 100644 drivers/staging/media/tegra/tegra210.c
>>   create mode 100644 drivers/staging/media/tegra/tegra210.h
>>   create mode 100644 drivers/staging/media/tegra/vi.c
>>   create mode 100644 drivers/staging/media/tegra/vi.h
>>   create mode 100644 drivers/staging/media/tegra/video.c
>>   create mode 100644 drivers/staging/media/tegra/video.h
> With 'make menuconfig' I get this:
>
> scripts/kconfig/mconf  Kconfig
>
> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>    Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM && COMPILE_TEST [=y])
>    Selected by [y]:
>    - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>
> This is an x86_64 build with COMPILE_TEST set. I can provide my full .config if you need it.
>
> CONFIG_TEGRA_HOST1X=y
> CONFIG_VIDEO_TEGRA=y
>
> Regards,
>
>          Hans

Hi Hans,

In v7, changed Kconfig to remove ARM. But looks like we should limit

TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on 
x86_64 shows above warning.

We should limit compile to ARM for CONFIG_VIDEO_TEGRA.

Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST 
like I had in previous version. Sorry about this.


Also, I see some changes went into latest linux-next staging media 
Kconfig, So, will have my patches on top of today's linux-next.

Thanks

Sowjanya
Dmitry Osipenko April 23, 2020, 10:55 p.m. UTC | #3 
23.04.2020 19:50, Sowjanya Komatineni пишет:
> 
> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>> which can support up to 6 MIPI CSI camera sensors.
>>>
>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>> capture from an external camera sensor connected to CSI or from
>>> built-in test pattern generator.
>>>
>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>
>>> This patch adds a v4l2 capture driver with media interface for
>>> Tegra210 built-in CSI to VI test pattern generator.
>>>
>>> This patch includes TPG support only and all the video pipeline
>>> configuration happens through the video device node.
>>>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>>   drivers/staging/media/Kconfig          |    2 +
>>>   drivers/staging/media/Makefile         |    1 +
>>>   drivers/staging/media/tegra/Kconfig    |   13 +
>>>   drivers/staging/media/tegra/Makefile   |    8 +
>>>   drivers/staging/media/tegra/TODO       |   10 +
>>>   drivers/staging/media/tegra/common.h   |  262 ++++++++
>>>   drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>>>   drivers/staging/media/tegra/csi.h      |  149 +++++
>>>   drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>>>   drivers/staging/media/tegra/tegra210.h |  190 ++++++
>>>   drivers/staging/media/tegra/vi.c       | 1132
>>> ++++++++++++++++++++++++++++++++
>>>   drivers/staging/media/tegra/vi.h       |   83 +++
>>>   drivers/staging/media/tegra/video.c    |  153 +++++
>>>   drivers/staging/media/tegra/video.h    |   34 +
>>>   14 files changed, 3352 insertions(+)
>>>   create mode 100644 drivers/staging/media/tegra/Kconfig
>>>   create mode 100644 drivers/staging/media/tegra/Makefile
>>>   create mode 100644 drivers/staging/media/tegra/TODO
>>>   create mode 100644 drivers/staging/media/tegra/common.h
>>>   create mode 100644 drivers/staging/media/tegra/csi.c
>>>   create mode 100644 drivers/staging/media/tegra/csi.h
>>>   create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>   create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>   create mode 100644 drivers/staging/media/tegra/vi.c
>>>   create mode 100644 drivers/staging/media/tegra/vi.h
>>>   create mode 100644 drivers/staging/media/tegra/video.c
>>>   create mode 100644 drivers/staging/media/tegra/video.h
>> With 'make menuconfig' I get this:
>>
>> scripts/kconfig/mconf  Kconfig
>>
>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>    Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>> COMPILE_TEST [=y])
>>    Selected by [y]:
>>    - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>
>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>> .config if you need it.
>>
>> CONFIG_TEGRA_HOST1X=y
>> CONFIG_VIDEO_TEGRA=y
>>
>> Regards,
>>
>>          Hans
> 
> Hi Hans,
> 
> In v7, changed Kconfig to remove ARM. But looks like we should limit
> 
> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
> x86_64 shows above warning.
> 
> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
> 
> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
> like I had in previous version. Sorry about this.
> 
> 
> Also, I see some changes went into latest linux-next staging media
> Kconfig, So, will have my patches on top of today's linux-next.

VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.

depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
Sowjanya Komatineni April 23, 2020, 10:59 p.m. UTC | #4 
On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>
>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>> capture from an external camera sensor connected to CSI or from
>>>> built-in test pattern generator.
>>>>
>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>
>>>> This patch adds a v4l2 capture driver with media interface for
>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>
>>>> This patch includes TPG support only and all the video pipeline
>>>> configuration happens through the video device node.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>    drivers/staging/media/Kconfig          |    2 +
>>>>    drivers/staging/media/Makefile         |    1 +
>>>>    drivers/staging/media/tegra/Kconfig    |   13 +
>>>>    drivers/staging/media/tegra/Makefile   |    8 +
>>>>    drivers/staging/media/tegra/TODO       |   10 +
>>>>    drivers/staging/media/tegra/common.h   |  262 ++++++++
>>>>    drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>>>>    drivers/staging/media/tegra/csi.h      |  149 +++++
>>>>    drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>>>>    drivers/staging/media/tegra/tegra210.h |  190 ++++++
>>>>    drivers/staging/media/tegra/vi.c       | 1132
>>>> ++++++++++++++++++++++++++++++++
>>>>    drivers/staging/media/tegra/vi.h       |   83 +++
>>>>    drivers/staging/media/tegra/video.c    |  153 +++++
>>>>    drivers/staging/media/tegra/video.h    |   34 +
>>>>    14 files changed, 3352 insertions(+)
>>>>    create mode 100644 drivers/staging/media/tegra/Kconfig
>>>>    create mode 100644 drivers/staging/media/tegra/Makefile
>>>>    create mode 100644 drivers/staging/media/tegra/TODO
>>>>    create mode 100644 drivers/staging/media/tegra/common.h
>>>>    create mode 100644 drivers/staging/media/tegra/csi.c
>>>>    create mode 100644 drivers/staging/media/tegra/csi.h
>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>>    create mode 100644 drivers/staging/media/tegra/vi.c
>>>>    create mode 100644 drivers/staging/media/tegra/vi.h
>>>>    create mode 100644 drivers/staging/media/tegra/video.c
>>>>    create mode 100644 drivers/staging/media/tegra/video.h
>>> With 'make menuconfig' I get this:
>>>
>>> scripts/kconfig/mconf  Kconfig
>>>
>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>>     Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>> COMPILE_TEST [=y])
>>>     Selected by [y]:
>>>     - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>
>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>> .config if you need it.
>>>
>>> CONFIG_TEGRA_HOST1X=y
>>> CONFIG_VIDEO_TEGRA=y
>>>
>>> Regards,
>>>
>>>           Hans
>> Hi Hans,
>>
>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>
>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>> x86_64 shows above warning.
>>
>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>
>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>> like I had in previous version. Sorry about this.
>>
>>
>> Also, I see some changes went into latest linux-next staging media
>> Kconfig, So, will have my patches on top of today's linux-next.
> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>
> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST

Was selecting it to auto-select Tegra host1x when video_tegra is enabled.

Yes, can use depends on
Dmitry Osipenko April 23, 2020, 11:03 p.m. UTC | #5 
24.04.2020 01:59, Sowjanya Komatineni пишет:
> 
> On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>>
>>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>>> capture from an external camera sensor connected to CSI or from
>>>>> built-in test pattern generator.
>>>>>
>>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>>
>>>>> This patch adds a v4l2 capture driver with media interface for
>>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>>
>>>>> This patch includes TPG support only and all the video pipeline
>>>>> configuration happens through the video device node.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>>    drivers/staging/media/Kconfig          |    2 +
>>>>>    drivers/staging/media/Makefile         |    1 +
>>>>>    drivers/staging/media/tegra/Kconfig    |   13 +
>>>>>    drivers/staging/media/tegra/Makefile   |    8 +
>>>>>    drivers/staging/media/tegra/TODO       |   10 +
>>>>>    drivers/staging/media/tegra/common.h   |  262 ++++++++
>>>>>    drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>>>>>    drivers/staging/media/tegra/csi.h      |  149 +++++
>>>>>    drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>>>>>    drivers/staging/media/tegra/tegra210.h |  190 ++++++
>>>>>    drivers/staging/media/tegra/vi.c       | 1132
>>>>> ++++++++++++++++++++++++++++++++
>>>>>    drivers/staging/media/tegra/vi.h       |   83 +++
>>>>>    drivers/staging/media/tegra/video.c    |  153 +++++
>>>>>    drivers/staging/media/tegra/video.h    |   34 +
>>>>>    14 files changed, 3352 insertions(+)
>>>>>    create mode 100644 drivers/staging/media/tegra/Kconfig
>>>>>    create mode 100644 drivers/staging/media/tegra/Makefile
>>>>>    create mode 100644 drivers/staging/media/tegra/TODO
>>>>>    create mode 100644 drivers/staging/media/tegra/common.h
>>>>>    create mode 100644 drivers/staging/media/tegra/csi.c
>>>>>    create mode 100644 drivers/staging/media/tegra/csi.h
>>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>>>    create mode 100644 drivers/staging/media/tegra/vi.c
>>>>>    create mode 100644 drivers/staging/media/tegra/vi.h
>>>>>    create mode 100644 drivers/staging/media/tegra/video.c
>>>>>    create mode 100644 drivers/staging/media/tegra/video.h
>>>> With 'make menuconfig' I get this:
>>>>
>>>> scripts/kconfig/mconf  Kconfig
>>>>
>>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>>>     Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>>> COMPILE_TEST [=y])
>>>>     Selected by [y]:
>>>>     - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>>
>>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>>> .config if you need it.
>>>>
>>>> CONFIG_TEGRA_HOST1X=y
>>>> CONFIG_VIDEO_TEGRA=y
>>>>
>>>> Regards,
>>>>
>>>>           Hans
>>> Hi Hans,
>>>
>>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>>
>>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>>> x86_64 shows above warning.
>>>
>>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>>
>>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>>> like I had in previous version. Sorry about this.
>>>
>>>
>>> Also, I see some changes went into latest linux-next staging media
>>> Kconfig, So, will have my patches on top of today's linux-next.
>> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>>
>> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
> 
> Was selecting it to auto-select Tegra host1x when video_tegra is enabled.
> 
> Yes, can use depends on
> 

BTW, perhaps ARCH_TEGRA and COMPILE_TEST aren't needed since
TEGRA_HOST1X already depends on them, so just:

depends on TEGRA_HOST1X
Dmitry Osipenko April 23, 2020, 11:14 p.m. UTC | #6 
24.04.2020 02:03, Dmitry Osipenko пишет:
> 24.04.2020 01:59, Sowjanya Komatineni пишет:
>>
>> On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>>>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>>>
>>>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>>>> capture from an external camera sensor connected to CSI or from
>>>>>> built-in test pattern generator.
>>>>>>
>>>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>>>
>>>>>> This patch adds a v4l2 capture driver with media interface for
>>>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>>>
>>>>>> This patch includes TPG support only and all the video pipeline
>>>>>> configuration happens through the video device node.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>>    drivers/staging/media/Kconfig          |    2 +
>>>>>>    drivers/staging/media/Makefile         |    1 +
>>>>>>    drivers/staging/media/tegra/Kconfig    |   13 +
>>>>>>    drivers/staging/media/tegra/Makefile   |    8 +
>>>>>>    drivers/staging/media/tegra/TODO       |   10 +
>>>>>>    drivers/staging/media/tegra/common.h   |  262 ++++++++
>>>>>>    drivers/staging/media/tegra/csi.c      |  606 +++++++++++++++++
>>>>>>    drivers/staging/media/tegra/csi.h      |  149 +++++
>>>>>>    drivers/staging/media/tegra/tegra210.c |  709 ++++++++++++++++++++
>>>>>>    drivers/staging/media/tegra/tegra210.h |  190 ++++++
>>>>>>    drivers/staging/media/tegra/vi.c       | 1132
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>    drivers/staging/media/tegra/vi.h       |   83 +++
>>>>>>    drivers/staging/media/tegra/video.c    |  153 +++++
>>>>>>    drivers/staging/media/tegra/video.h    |   34 +
>>>>>>    14 files changed, 3352 insertions(+)
>>>>>>    create mode 100644 drivers/staging/media/tegra/Kconfig
>>>>>>    create mode 100644 drivers/staging/media/tegra/Makefile
>>>>>>    create mode 100644 drivers/staging/media/tegra/TODO
>>>>>>    create mode 100644 drivers/staging/media/tegra/common.h
>>>>>>    create mode 100644 drivers/staging/media/tegra/csi.c
>>>>>>    create mode 100644 drivers/staging/media/tegra/csi.h
>>>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>>>>    create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>>>>    create mode 100644 drivers/staging/media/tegra/vi.c
>>>>>>    create mode 100644 drivers/staging/media/tegra/vi.h
>>>>>>    create mode 100644 drivers/staging/media/tegra/video.c
>>>>>>    create mode 100644 drivers/staging/media/tegra/video.h
>>>>> With 'make menuconfig' I get this:
>>>>>
>>>>> scripts/kconfig/mconf  Kconfig
>>>>>
>>>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>>>>     Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>>>> COMPILE_TEST [=y])
>>>>>     Selected by [y]:
>>>>>     - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>>>
>>>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>>>> .config if you need it.
>>>>>
>>>>> CONFIG_TEGRA_HOST1X=y
>>>>> CONFIG_VIDEO_TEGRA=y
>>>>>
>>>>> Regards,
>>>>>
>>>>>           Hans
>>>> Hi Hans,
>>>>
>>>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>>>
>>>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>>>> x86_64 shows above warning.
>>>>
>>>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>>>
>>>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>>>> like I had in previous version. Sorry about this.
>>>>
>>>>
>>>> Also, I see some changes went into latest linux-next staging media
>>>> Kconfig, So, will have my patches on top of today's linux-next.
>>> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>>>
>>> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
>>
>> Was selecting it to auto-select Tegra host1x when video_tegra is enabled.
>>
>> Yes, can use depends on
>>
> 
> BTW, perhaps ARCH_TEGRA and COMPILE_TEST aren't needed since
> TEGRA_HOST1X already depends on them, so just:
> 
> depends on TEGRA_HOST1X
> 

Although, I guess TEGRA_HOST1X isn't really needed for compile-testing,
and thus, it won't hurt to drop that dependency for testing:

depends on TEGRA_HOST1X || COMPILE_TEST
Dmitry Osipenko April 23, 2020, 11:16 p.m. UTC | #7 
22.04.2020 09:18, Sowjanya Komatineni пишет:
> +static int chan_capture_kthread_start(void *data)
> +{
> +	struct tegra_vi_channel *chan = data;
> +	struct tegra_channel_buffer *buf;
> +	int err = 0;
> +
> +	set_freezable();
> +
> +	while (1) {
> +		try_to_freeze();
> +
> +		wait_event_interruptible(chan->start_wait,
> +					 !list_empty(&chan->capture) ||
> +					 kthread_should_stop());
> +
> +		if (kthread_should_stop())
> +			break;
> +
> +		/*
> +		 * Source is not streaming if error is non-zero.
> +		 * So, do not dequeue buffers on capture error.
> +		 */
> +		if (err)
> +			continue;

This will result in an endless loop, I suppose it wasn't the intention.
Sowjanya Komatineni April 23, 2020, 11:19 p.m. UTC | #8 
On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>> +static int chan_capture_kthread_start(void *data)
>> +{
>> +     struct tegra_vi_channel *chan = data;
>> +     struct tegra_channel_buffer *buf;
>> +     int err = 0;
>> +
>> +     set_freezable();
>> +
>> +     while (1) {
>> +             try_to_freeze();
>> +
>> +             wait_event_interruptible(chan->start_wait,
>> +                                      !list_empty(&chan->capture) ||
>> +                                      kthread_should_stop());
>> +
>> +             if (kthread_should_stop())
>> +                     break;
>> +
>> +             /*
>> +              * Source is not streaming if error is non-zero.
>> +              * So, do not dequeue buffers on capture error.
>> +              */
>> +             if (err)
>> +                     continue;
> This will result in an endless loop, I suppose it wasn't the intention.

no it will not. on error we report vb2_queue_error which will do 
streaming stop request.

So thread will be stopped on streaming stop request thru kthread stop signal
Sowjanya Komatineni April 23, 2020, 11:20 p.m. UTC | #9 
On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>
> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>> +static int chan_capture_kthread_start(void *data)
>>> +{
>>> +     struct tegra_vi_channel *chan = data;
>>> +     struct tegra_channel_buffer *buf;
>>> +     int err = 0;
>>> +
>>> +     set_freezable();
>>> +
>>> +     while (1) {
>>> +             try_to_freeze();
>>> +
>>> +             wait_event_interruptible(chan->start_wait,
>>> + !list_empty(&chan->capture) ||
>>> +                                      kthread_should_stop());
>>> +
>>> +             if (kthread_should_stop())
>>> +                     break;
>>> +
>>> +             /*
>>> +              * Source is not streaming if error is non-zero.
>>> +              * So, do not dequeue buffers on capture error.
>>> +              */
>>> +             if (err)
>>> +                     continue;
>> This will result in an endless loop, I suppose it wasn't the intention.
>
> no it will not. on error we report vb2_queue_error which will do 
> streaming stop request.
>
> So thread will be stopped on streaming stop request thru kthread stop 
> signal
To be clear on error it reports vb2 queue error and waits for stop 
streaming to happen
Dmitry Osipenko April 23, 2020, 11:25 p.m. UTC | #10 
24.04.2020 02:20, Sowjanya Komatineni пишет:
> 
> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>
>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>> +static int chan_capture_kthread_start(void *data)
>>>> +{
>>>> +     struct tegra_vi_channel *chan = data;
>>>> +     struct tegra_channel_buffer *buf;
>>>> +     int err = 0;
>>>> +
>>>> +     set_freezable();
>>>> +
>>>> +     while (1) {
>>>> +             try_to_freeze();
>>>> +
>>>> +             wait_event_interruptible(chan->start_wait,
>>>> + !list_empty(&chan->capture) ||
>>>> +                                      kthread_should_stop());
>>>> +
>>>> +             if (kthread_should_stop())
>>>> +                     break;
>>>> +
>>>> +             /*
>>>> +              * Source is not streaming if error is non-zero.
>>>> +              * So, do not dequeue buffers on capture error.
>>>> +              */
>>>> +             if (err)
>>>> +                     continue;
>>> This will result in an endless loop, I suppose it wasn't the intention.
>>
>> no it will not. on error we report vb2_queue_error which will do
>> streaming stop request.
>>
>> So thread will be stopped on streaming stop request thru kthread stop
>> signal
> To be clear on error it reports vb2 queue error and waits for stop
> streaming to happen

If thread should exit on error, then it should do it on the actual
error. Otherwise it looks very error-prone.
Sowjanya Komatineni April 23, 2020, 11:50 p.m. UTC | #11 
On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>> +static int chan_capture_kthread_start(void *data)
>>>>> +{
>>>>> +     struct tegra_vi_channel *chan = data;
>>>>> +     struct tegra_channel_buffer *buf;
>>>>> +     int err = 0;
>>>>> +
>>>>> +     set_freezable();
>>>>> +
>>>>> +     while (1) {
>>>>> +             try_to_freeze();
>>>>> +
>>>>> +             wait_event_interruptible(chan->start_wait,
>>>>> + !list_empty(&chan->capture) ||
>>>>> +                                      kthread_should_stop());
>>>>> +
>>>>> +             if (kthread_should_stop())
>>>>> +                     break;
>>>>> +
>>>>> +             /*
>>>>> +              * Source is not streaming if error is non-zero.
>>>>> +              * So, do not dequeue buffers on capture error.
>>>>> +              */
>>>>> +             if (err)
>>>>> +                     continue;
>>>> This will result in an endless loop, I suppose it wasn't the intention.
>>> no it will not. on error we report vb2_queue_error which will do
>>> streaming stop request.
>>>
>>> So thread will be stopped on streaming stop request thru kthread stop
>>> signal
>> To be clear on error it reports vb2 queue error and waits for stop
>> streaming to happen
> If thread should exit on error, then it should do it on the actual
> error. Otherwise it looks very error-prone.
When v4l2 drivers indicate fatal error through vb2_queue_error, queue 
error flag  is set and wakes up all processes waiting on queue along 
with polling reporting  EPOLLERR and also reporting error for queuing 
and dequeuing buffers. Stream stop will surely happen which stops the 
thread.
Dmitry Osipenko April 24, 2020, 12:42 a.m. UTC | #12 
24.04.2020 02:50, Sowjanya Komatineni пишет:
> 
> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>> +{
>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>> +     int err = 0;
>>>>>> +
>>>>>> +     set_freezable();
>>>>>> +
>>>>>> +     while (1) {
>>>>>> +             try_to_freeze();
>>>>>> +
>>>>>> +             wait_event_interruptible(chan->start_wait,
>>>>>> + !list_empty(&chan->capture) ||
>>>>>> +                                      kthread_should_stop());
>>>>>> +
>>>>>> +             if (kthread_should_stop())
>>>>>> +                     break;
>>>>>> +
>>>>>> +             /*
>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>> +              */
>>>>>> +             if (err)
>>>>>> +                     continue;
>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>> intention.
>>>> no it will not. on error we report vb2_queue_error which will do
>>>> streaming stop request.
>>>>
>>>> So thread will be stopped on streaming stop request thru kthread stop
>>>> signal
>>> To be clear on error it reports vb2 queue error and waits for stop
>>> streaming to happen
>> If thread should exit on error, then it should do it on the actual
>> error. Otherwise it looks very error-prone.
> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
> error flag  is set and wakes up all processes waiting on queue along
> with polling reporting  EPOLLERR and also reporting error for queuing
> and dequeuing buffers. Stream stop will surely happen which stops the
> thread.

This doesn't explain what is the point of continuing to loop instead of
exiting immediately on error.
Sowjanya Komatineni April 24, 2020, 12:51 a.m. UTC | #13 
On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>> +{
>>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>>> +     int err = 0;
>>>>>>> +
>>>>>>> +     set_freezable();
>>>>>>> +
>>>>>>> +     while (1) {
>>>>>>> +             try_to_freeze();
>>>>>>> +
>>>>>>> +             wait_event_interruptible(chan->start_wait,
>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>> +                                      kthread_should_stop());
>>>>>>> +
>>>>>>> +             if (kthread_should_stop())
>>>>>>> +                     break;
>>>>>>> +
>>>>>>> +             /*
>>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>>> +              */
>>>>>>> +             if (err)
>>>>>>> +                     continue;
>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>> intention.
>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>> streaming stop request.
>>>>>
>>>>> So thread will be stopped on streaming stop request thru kthread stop
>>>>> signal
>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>> streaming to happen
>>> If thread should exit on error, then it should do it on the actual
>>> error. Otherwise it looks very error-prone.
>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>> error flag  is set and wakes up all processes waiting on queue along
>> with polling reporting  EPOLLERR and also reporting error for queuing
>> and dequeuing buffers. Stream stop will surely happen which stops the
>> thread.
> This doesn't explain what is the point of continuing to loop instead of
> exiting immediately on error.

We are using 2 threads and when capture start error happens, we can stop 
capture_start thread immediately but capture_finish thread will still 
run for any outstanding buffers.

So, as it makes no diff stopping both threads during stream stop which 
will definitely happen on error and when we don't dequeue buffers
Sowjanya Komatineni April 24, 2020, 1:08 a.m. UTC | #14 
On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>
> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>> External email: Use caution opening links or attachments
>>>>>>>
>>>>>>>
>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>> +{
>>>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>>>> +     int err = 0;
>>>>>>>> +
>>>>>>>> +     set_freezable();
>>>>>>>> +
>>>>>>>> +     while (1) {
>>>>>>>> +             try_to_freeze();
>>>>>>>> +
>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>> + kthread_should_stop());
>>>>>>>> +
>>>>>>>> +             if (kthread_should_stop())
>>>>>>>> +                     break;
>>>>>>>> +
>>>>>>>> +             /*
>>>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>>>> +              */
>>>>>>>> +             if (err)
>>>>>>>> +                     continue;
>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>> intention.
>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>> streaming stop request.
>>>>>>
>>>>>> So thread will be stopped on streaming stop request thru kthread 
>>>>>> stop
>>>>>> signal
>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>> streaming to happen
>>>> If thread should exit on error, then it should do it on the actual
>>>> error. Otherwise it looks very error-prone.
>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>> error flag  is set and wakes up all processes waiting on queue along
>>> with polling reporting  EPOLLERR and also reporting error for queuing
>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>> thread.
>> This doesn't explain what is the point of continuing to loop instead of
>> exiting immediately on error.
>
> We are using 2 threads and when capture start error happens, we can 
> stop capture_start thread immediately but capture_finish thread will 
> still run for any outstanding buffers.
>
> So, as it makes no diff stopping both threads during stream stop which 
> will definitely happen on error and when we don't dequeue buffers
>
Also there will be an issue if we break on error immediately during 
stop_streaming -> kthread_stop()

As stop streaming can happen any time, we do kthread_stop and in case of 
error if we stop thread and on stop streaming kthread_stop might crash 
as kthread_stop can only be called on running thread
Dmitry Osipenko April 24, 2020, 2:09 a.m. UTC | #15 
24.04.2020 04:08, Sowjanya Komatineni пишет:
> 
> On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>>
>> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>
>>>>>>>>
>>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>>> +{
>>>>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>>>>> +     int err = 0;
>>>>>>>>> +
>>>>>>>>> +     set_freezable();
>>>>>>>>> +
>>>>>>>>> +     while (1) {
>>>>>>>>> +             try_to_freeze();
>>>>>>>>> +
>>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>>> + kthread_should_stop());
>>>>>>>>> +
>>>>>>>>> +             if (kthread_should_stop())
>>>>>>>>> +                     break;
>>>>>>>>> +
>>>>>>>>> +             /*
>>>>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>>>>> +              */
>>>>>>>>> +             if (err)
>>>>>>>>> +                     continue;
>>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>>> intention.
>>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>>> streaming stop request.
>>>>>>>
>>>>>>> So thread will be stopped on streaming stop request thru kthread
>>>>>>> stop
>>>>>>> signal
>>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>>> streaming to happen
>>>>> If thread should exit on error, then it should do it on the actual
>>>>> error. Otherwise it looks very error-prone.
>>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>>> error flag  is set and wakes up all processes waiting on queue along
>>>> with polling reporting  EPOLLERR and also reporting error for queuing
>>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>>> thread.
>>> This doesn't explain what is the point of continuing to loop instead of
>>> exiting immediately on error.
>>
>> We are using 2 threads and when capture start error happens, we can
>> stop capture_start thread immediately but capture_finish thread will
>> still run for any outstanding buffers.
>>
>> So, as it makes no diff stopping both threads during stream stop which
>> will definitely happen on error and when we don't dequeue buffers
>>
> Also there will be an issue if we break on error immediately during
> stop_streaming -> kthread_stop()
> 
> As stop streaming can happen any time, we do kthread_stop and in case of
> error if we stop thread and on stop streaming kthread_stop might crash
> as kthread_stop can only be called on running thread
> 

This a better explanation, but still it's not good that there could be a
busy loop within the thread.

Should be better to sleep if error is set.

wait_event_interruptible(chan->start_wait,
			(!err && !list_empty(&chan->capture)) ||
			kthread_should_stop());
Sowjanya Komatineni April 24, 2020, 2:12 a.m. UTC | #16 
On 4/23/20 7:09 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 04:08, Sowjanya Komatineni пишет:
>> On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>>> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>>>> +{
>>>>>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>>>>>> +     int err = 0;
>>>>>>>>>> +
>>>>>>>>>> +     set_freezable();
>>>>>>>>>> +
>>>>>>>>>> +     while (1) {
>>>>>>>>>> +             try_to_freeze();
>>>>>>>>>> +
>>>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>>>> + kthread_should_stop());
>>>>>>>>>> +
>>>>>>>>>> +             if (kthread_should_stop())
>>>>>>>>>> +                     break;
>>>>>>>>>> +
>>>>>>>>>> +             /*
>>>>>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>>>>>> +              */
>>>>>>>>>> +             if (err)
>>>>>>>>>> +                     continue;
>>>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>>>> intention.
>>>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>>>> streaming stop request.
>>>>>>>>
>>>>>>>> So thread will be stopped on streaming stop request thru kthread
>>>>>>>> stop
>>>>>>>> signal
>>>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>>>> streaming to happen
>>>>>> If thread should exit on error, then it should do it on the actual
>>>>>> error. Otherwise it looks very error-prone.
>>>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>>>> error flag  is set and wakes up all processes waiting on queue along
>>>>> with polling reporting  EPOLLERR and also reporting error for queuing
>>>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>>>> thread.
>>>> This doesn't explain what is the point of continuing to loop instead of
>>>> exiting immediately on error.
>>> We are using 2 threads and when capture start error happens, we can
>>> stop capture_start thread immediately but capture_finish thread will
>>> still run for any outstanding buffers.
>>>
>>> So, as it makes no diff stopping both threads during stream stop which
>>> will definitely happen on error and when we don't dequeue buffers
>>>
>> Also there will be an issue if we break on error immediately during
>> stop_streaming -> kthread_stop()
>>
>> As stop streaming can happen any time, we do kthread_stop and in case of
>> error if we stop thread and on stop streaming kthread_stop might crash
>> as kthread_stop can only be called on running thread
>>
> This a better explanation, but still it's not good that there could be a
> busy loop within the thread.
>
> Should be better to sleep if error is set.
>
> wait_event_interruptible(chan->start_wait,
>                          (!err && !list_empty(&chan->capture)) ||
>                          kthread_should_stop());
ok, will add err to wait condition to let it sleep