Linux clock framework development
 help / color / mirror / Atom feed
* [RFC PATCH v7 0/9] Add Tegra driver for video capture
@ 2020-04-15  2:57 Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

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.

[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                  |  612 +++++++++++
 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                   | 1128 ++++++++++++++++++++
 drivers/staging/media/tegra/vi.h                   |   85 ++
 drivers/staging/media/tegra/video.c                |  151 +++
 drivers/staging/media/tegra/video.h                |   31 +
 include/dt-bindings/clock/tegra210-car.h           |    2 +-
 include/dt-bindings/reset/tegra210-car.h           |    1 +
 21 files changed, 3489 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


-- 
2.7.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 1/9] arm64: tegra: Fix sor powergate clocks and reset
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 2/9] arm64: tegra: Add reset-cells to mc Sowjanya Komatineni
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra210 device tree lists csi clock and reset under SOR powergate
node.

But Tegra210 has csicil in SOR partition and csi in VENC partition.

So, this patch includes fix for sor powergate node.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 64c46ce..d0eff92 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -796,7 +796,9 @@
 			pd_sor: sor {
 				clocks = <&tegra_car TEGRA210_CLK_SOR0>,
 					 <&tegra_car TEGRA210_CLK_SOR1>,
-					 <&tegra_car TEGRA210_CLK_CSI>,
+					 <&tegra_car TEGRA210_CLK_CILAB>,
+					 <&tegra_car TEGRA210_CLK_CILCD>,
+					 <&tegra_car TEGRA210_CLK_CILE>,
 					 <&tegra_car TEGRA210_CLK_DSIA>,
 					 <&tegra_car TEGRA210_CLK_DSIB>,
 					 <&tegra_car TEGRA210_CLK_DPAUX>,
@@ -804,7 +806,6 @@
 					 <&tegra_car TEGRA210_CLK_MIPI_CAL>;
 				resets = <&tegra_car TEGRA210_CLK_SOR0>,
 					 <&tegra_car TEGRA210_CLK_SOR1>,
-					 <&tegra_car TEGRA210_CLK_CSI>,
 					 <&tegra_car TEGRA210_CLK_DSIA>,
 					 <&tegra_car TEGRA210_CLK_DSIB>,
 					 <&tegra_car TEGRA210_CLK_DPAUX>,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 2/9] arm64: tegra: Add reset-cells to mc
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock Sowjanya Komatineni
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra210 device tree is missing reset-cells property for mc node.

This patch fixes it.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index d0eff92..5b1dfd8 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -894,6 +894,7 @@
 		interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
 
 		#iommu-cells = <1>;
+		#reset-cells = <1>;
 	};
 
 	sata@70020000 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 2/9] arm64: tegra: Add reset-cells to mc Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate Sowjanya Komatineni
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra210 uses PLLD out internally for CSI TPG.

This patch adds clk id for this CSI TPG clock from PLLD.

Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 include/dt-bindings/clock/tegra210-car.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
index 7a8f10b..d8909e0 100644
--- a/include/dt-bindings/clock/tegra210-car.h
+++ b/include/dt-bindings/clock/tegra210-car.h
@@ -351,7 +351,7 @@
 #define TEGRA210_CLK_PLL_P_OUT_XUSB 317
 #define TEGRA210_CLK_XUSB_SSP_SRC 318
 #define TEGRA210_CLK_PLL_RE_OUT1 319
-/* 320 */
+#define TEGRA210_CLK_CSI_TPG 320
 /* 321 */
 #define TEGRA210_CLK_ISP 322
 #define TEGRA210_CLK_PLL_A_OUT_ADSP 323
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
                   ` (2 preceding siblings ...)
  2020-04-15  2:57 ` [RFC PATCH v7 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra210 CSI hardware internally uses PLLD for internal test pattern
generator logic.

PLLD_BASE register in CAR has a bit CSI_CLK_SOURCE to enable PLLD
out to CSI during TPG mode.

This patch adds this CSI TPG clock gate to Tegra210 clock driver
to allow Tegra video driver to ungate CSI TPG clock during TPG mode
and gate during non TPG mode.

Acked-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index defe3b7..81a879b 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3035,6 +3035,13 @@ static __init void tegra210_periph_clk_init(void __iomem *clk_base,
 					     periph_clk_enb_refcnt);
 	clks[TEGRA210_CLK_DSIB] = clk;
 
+	/* csi_tpg */
+	clk = clk_register_gate(NULL, "csi_tpg", "pll_d",
+				CLK_SET_RATE_PARENT, clk_base + PLLD_BASE,
+				23, 0, &pll_d_lock);
+	clk_register_clkdev(clk, "csi_tpg", NULL);
+	clks[TEGRA210_CLK_CSI_TPG] = clk;
+
 	/* la */
 	clk = tegra_clk_register_periph("la", la_parents,
 			ARRAY_SIZE(la_parents), &tegra210_la, clk_base,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 5/9] dt-binding: tegra: Add VI and CSI bindings
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
                   ` (3 preceding siblings ...)
  2020-04-15  2:57 ` [RFC PATCH v7 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 7/9] MAINTAINERS: Add Tegra Video driver section Sowjanya Komatineni
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra contains VI controller which can support up to 6 MIPI CSI
camera sensors.

Each Tegra CSI port from CSI unit can be one-to-one mapper to
VI channel and can capture from an external camera sensor or
from built-in test pattern generator.

This patch adds dt-bindings for Tegra VI and CSI.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 .../display/tegra/nvidia,tegra20-host1x.txt        | 73 ++++++++++++++++++----
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index 9999255..4731921 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -40,14 +40,30 @@ of the following host1x client modules:
 
   Required properties:
   - compatible: "nvidia,tegra<chip>-vi"
-  - reg: Physical base address and length of the controller's registers.
+  - reg: Physical base address and length of the controller registers.
   - interrupts: The interrupt outputs from the controller.
-  - clocks: Must contain one entry, for the module clock.
+  - clocks: clocks: Must contain one entry, for the module clock.
     See ../clocks/clock-bindings.txt for details.
-  - resets: Must contain an entry for each entry in reset-names.
-    See ../reset/reset.txt for details.
-  - reset-names: Must include the following entries:
-    - vi
+  - Tegra20/Tegra30/Tegra114/Tegra124:
+    - resets: Must contain an entry for each entry in reset-names.
+      See ../reset/reset.txt for details.
+    - reset-names: Must include the following entries:
+      - vi
+  - Tegra210:
+    - power-domains: Must include venc powergate node as vi is in VE partition.
+  - Tegra210 has CSI part of VI sharing same host interface and register space.
+    So, VI device node should have CSI child node.
+
+    - csi: mipi csi interface to vi
+
+      Required properties:
+      - compatible: "nvidia,tegra210-csi"
+      - reg: Physical base address offset to parent and length of the controller
+        registers.
+      - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
+        See ../clocks/clock-bindings.txt for details.
+      - power-domains: Must include sor powergate node as csicil is in
+        SOR partition.
 
 - epp: encoder pre-processor
 
@@ -309,13 +325,44 @@ Example:
 			reset-names = "mpe";
 		};
 
-		vi {
-			compatible = "nvidia,tegra20-vi";
-			reg = <0x54080000 0x00040000>;
-			interrupts = <0 69 0x04>;
-			clocks = <&tegra_car TEGRA20_CLK_VI>;
-			resets = <&tegra_car 100>;
-			reset-names = "vi";
+		vi@54080000 {
+			compatible = "nvidia,tegra210-vi";
+			reg = <0x0 0x54080000 0x0 0x700>;
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
+			assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
+
+			clocks = <&tegra_car TEGRA210_CLK_VI>;
+			power-domains = <&pd_venc>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			ranges = <0x0 0x0 0x54080000 0x2000>;
+
+			csi@838 {
+				compatible = "nvidia,tegra210-csi";
+				reg = <0x838 0x1300>;
+				assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
+						  <&tegra_car TEGRA210_CLK_CILCD>,
+						  <&tegra_car TEGRA210_CLK_CILE>,
+						  <&tegra_car TEGRA210_CLK_CSI_TPG>;
+				assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
+							 <&tegra_car TEGRA210_CLK_PLL_P>,
+							 <&tegra_car TEGRA210_CLK_PLL_P>;
+				assigned-clock-rates = <102000000>,
+						       <102000000>,
+						       <102000000>,
+						       <972000000>;
+
+				clocks = <&tegra_car TEGRA210_CLK_CSI>,
+					 <&tegra_car TEGRA210_CLK_CILAB>,
+					 <&tegra_car TEGRA210_CLK_CILCD>,
+					 <&tegra_car TEGRA210_CLK_CILE>,
+					 <&tegra_car TEGRA210_CLK_CSI_TPG>;
+				clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
+				power-domains = <&pd_sor>;
+			};
 		};
 
 		epp {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 7/9] MAINTAINERS: Add Tegra Video driver section
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
                   ` (4 preceding siblings ...)
  2020-04-15  2:57 ` [RFC PATCH v7 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset Sowjanya Komatineni
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Add maintainers and mailing list entries to Tegra Video driver section.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5872577..47be371 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16608,6 +16608,16 @@ M:	Laxman Dewangan <ldewangan@nvidia.com>
 S:	Supported
 F:	drivers/spi/spi-tegra*
 
+TEGRA VIDEO DRIVER
+M:	Thierry Reding <thierry.reding@gmail.com>
+M:	Jonathan Hunter <jonathanh@nvidia.com>
+M:	Sowjanya Komatineni <skomatineni@nvidia.com>
+L:	linux-media@vger.kernel.org
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+F:	drivers/staging/media/tegra/
+
 TEGRA XUSB PADCTL DRIVER
 M:	JC Kuo <jckuo@nvidia.com>
 S:	Supported
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
                   ` (5 preceding siblings ...)
  2020-04-15  2:57 ` [RFC PATCH v7 7/9] MAINTAINERS: Add Tegra Video driver section Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
  2020-04-15  2:57 ` [RFC PATCH v7 9/9] arm64: tegra: Add Tegra VI CSI support in device tree Sowjanya Komatineni
       [not found] ` <1586919463-30542-7-git-send-email-skomatineni@nvidia.com>
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

This patch adds ID for Tegra210 VI controller reset to use with
device tree.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 include/dt-bindings/reset/tegra210-car.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/reset/tegra210-car.h b/include/dt-bindings/reset/tegra210-car.h
index 9dc84ec..8755946 100644
--- a/include/dt-bindings/reset/tegra210-car.h
+++ b/include/dt-bindings/reset/tegra210-car.h
@@ -10,5 +10,6 @@
 #define TEGRA210_RESET(x)		(7 * 32 + (x))
 #define TEGRA210_RST_DFLL_DVCO		TEGRA210_RESET(0)
 #define TEGRA210_RST_ADSP		TEGRA210_RESET(1)
+#define TEGRA210_RST_VI			20
 
 #endif	/* _DT_BINDINGS_RESET_TEGRA210_CAR_H */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v7 9/9] arm64: tegra: Add Tegra VI CSI support in device tree
  2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
                   ` (6 preceding siblings ...)
  2020-04-15  2:57 ` [RFC PATCH v7 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset Sowjanya Komatineni
@ 2020-04-15  2:57 ` Sowjanya Komatineni
       [not found] ` <1586919463-30542-7-git-send-email-skomatineni@nvidia.com>
  8 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15  2:57 UTC (permalink / raw)
  To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

Tegra210 contains VI controller for video input capture from MIPI
CSI camera sensors and also supports built-in test pattern generator.

CSI ports can be one-to-one mapped to VI channels for capturing from
an external sensor or from built-in test pattern generator.

This patch adds support for VI and CSI and enables them in Tegra210
device tree.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 10 ++++++
 arch/arm64/boot/dts/nvidia/tegra210.dtsi       | 46 +++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
index 313a4c2..b57d837 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
@@ -14,6 +14,16 @@
 			status = "okay";
 		};
 
+		vi@54080000 {
+			status = "okay";
+
+			avdd-dsi-csi-supply = <&vdd_dsi_csi>;
+
+			csi@838 {
+				status = "okay";
+			};
+		};
+
 		sor@54580000 {
 			status = "okay";
 
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 5b1dfd8..cad42a7 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -137,9 +137,44 @@
 
 		vi@54080000 {
 			compatible = "nvidia,tegra210-vi";
-			reg = <0x0 0x54080000 0x0 0x00040000>;
+			reg = <0x0 0x54080000 0x0 0x700>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
+			assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
+			assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
+
+			clocks = <&tegra_car TEGRA210_CLK_VI>;
+			power-domains = <&pd_venc>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			ranges = <0x0 0x0 0x54080000 0x2000>;
+
+			csi@838 {
+				compatible = "nvidia,tegra210-csi";
+				reg = <0x838 0x1300>;
+				status = "disabled";
+				assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
+						  <&tegra_car TEGRA210_CLK_CILCD>,
+						  <&tegra_car TEGRA210_CLK_CILE>,
+						  <&tegra_car TEGRA210_CLK_CSI_TPG>;
+				assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
+							 <&tegra_car TEGRA210_CLK_PLL_P>,
+							 <&tegra_car TEGRA210_CLK_PLL_P>;
+				assigned-clock-rates = <102000000>,
+						       <102000000>,
+						       <102000000>,
+						       <972000000>;
+
+				clocks = <&tegra_car TEGRA210_CLK_CSI>,
+					 <&tegra_car TEGRA210_CLK_CILAB>,
+					 <&tegra_car TEGRA210_CLK_CILCD>,
+					 <&tegra_car TEGRA210_CLK_CILE>,
+					 <&tegra_car TEGRA210_CLK_CSI_TPG>;
+				clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
+				power-domains = <&pd_sor>;
+			};
 		};
 
 		tsec@54100000 {
@@ -839,6 +874,15 @@
 				reset-names = "vic";
 				#power-domain-cells = <0>;
 			};
+
+			pd_venc: venc {
+				clocks = <&tegra_car TEGRA210_CLK_VI>,
+					 <&tegra_car TEGRA210_CLK_CSI>;
+				resets = <&mc TEGRA210_MC_RESET_VI>,
+					 <&tegra_car TEGRA210_RST_VI>,
+					 <&tegra_car TEGRA210_CLK_CSI>;
+				#power-domain-cells = <0>;
+			};
 		};
 
 		sdmmc1_3v3: sdmmc1-3v3 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
       [not found] ` <1586919463-30542-7-git-send-email-skomatineni@nvidia.com>
@ 2020-04-15 14:22   ` Dmitry Osipenko
  2020-04-15 16:54     ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Osipenko @ 2020-04-15 14:22 UTC (permalink / raw)
  To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel

15.04.2020 05:57, Sowjanya Komatineni пишет:
> +static int tegra_csi_remove(struct platform_device *pdev)
> +{
> +	struct tegra_csi *csi = platform_get_drvdata(pdev);
> +	int err;
> +
> +	err = host1x_client_unregister(&csi->client);
> +	if (err < 0) {
> +		dev_err(csi->dev,
> +			"failed to unregister host1x client: %d\n", err);
> +		return err;
> +	}
> +
> +	pm_runtime_disable(csi->dev);
> +	kfree(csi);

IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
sure how moving away from the resource-managed API helps here. Could you
please explain in a more details?

Have you tried to test this driver under KASAN? I suspect that you just
masked the problem, instead of fixing it.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 14:22   ` [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver Dmitry Osipenko
@ 2020-04-15 16:54     ` Sowjanya Komatineni
  2020-04-15 17:21       ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 16:54 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>> +static int tegra_csi_remove(struct platform_device *pdev)
>> +{
>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>> +     int err;
>> +
>> +     err = host1x_client_unregister(&csi->client);
>> +     if (err < 0) {
>> +             dev_err(csi->dev,
>> +                     "failed to unregister host1x client: %d\n", err);
>> +             return err;
>> +     }
>> +
>> +     pm_runtime_disable(csi->dev);
>> +     kfree(csi);
> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
> sure how moving away from the resource-managed API helps here. Could you
> please explain in a more details?
>
> Have you tried to test this driver under KASAN? I suspect that you just
> masked the problem, instead of fixing it.
Using devm_kzalloc for vi/csi structures based on prior feedback request 
to switch to use kzalloc all over this driver.

Hi Hans,

video devices lifetime is till video device nodes are released. So, v4l2 
device release callback does the release of tegra channel allocation 
which hold video device.

Below are the 3 possible cases of unbind/unload,

1. during tegra-video module unload, if v4l2 device refcnt is not 0 
which is the case when any of video device node handle is kept opened 
then unloading module will not happen and module refcnt is also non-zero 
and unloading tegra-video module reports module in use.

2. during tegra-video driver unbind, tegra-video driver removal will do 
vi/csi clients exit ops which unregisters video device allocated memory 
during release callback of v4l2 device. vi/csi structure allocation 
remains same as vi/csi driver removal will not happen in this case.


3. during direct host1x client drivers vi/csi unbind, both 
host1x_clients vi/csi gets unregistered, deletes host1x logical device 
which executes tegra-video driver removal() -> vi/csi exit() before 
vi/csi memory gets freed in vi/csi driver remove().

So, any active streaming will stop and video devices are unregistered 
during direct client driver unbind prior to freeing vi/csi memory.

Also vi/csi driver remove does explicit free vi/csi as its allocated 
with kzalloc. So not sure how using kzalloc is different to devm_kzalloc 
for vi/csi structure in terms of when vi/csi memory gets freed?

Except for channel allocation which holds video device and as video 
device life time is beyond tegra-video module unbind->vi exit(), looks 
like we can use devm_kzalloc for vi/csi.


Can you please comment if you still think we need to use kzalloc rather 
than devm_kzalloc for vi/csi structure allocation?

Thanks

Sowjanya


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 16:54     ` Sowjanya Komatineni
@ 2020-04-15 17:21       ` Sowjanya Komatineni
  2020-04-15 17:47         ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 17:21 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 9:54 AM, Sowjanya Komatineni wrote:
>
> On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>>> +static int tegra_csi_remove(struct platform_device *pdev)
>>> +{
>>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>>> +     int err;
>>> +
>>> +     err = host1x_client_unregister(&csi->client);
>>> +     if (err < 0) {
>>> +             dev_err(csi->dev,
>>> +                     "failed to unregister host1x client: %d\n", err);
>>> +             return err;
>>> +     }
>>> +
>>> +     pm_runtime_disable(csi->dev);
>>> +     kfree(csi);
>> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
>> sure how moving away from the resource-managed API helps here. Could you
>> please explain in a more details?
>>
>> Have you tried to test this driver under KASAN? I suspect that you just
>> masked the problem, instead of fixing it.
> Using devm_kzalloc for vi/csi structures based on prior feedback 
> request to switch to use kzalloc all over this driver.
>
> Hi Hans,
>
> video devices lifetime is till video device nodes are released. So, 
> v4l2 device release callback does the release of tegra channel 
> allocation which hold video device.
>
> Below are the 3 possible cases of unbind/unload,
>
> 1. during tegra-video module unload, if v4l2 device refcnt is not 0 
> which is the case when any of video device node handle is kept opened 
> then unloading module will not happen and module refcnt is also 
> non-zero and unloading tegra-video module reports module in use.
>
> 2. during tegra-video driver unbind, tegra-video driver removal will 
> do vi/csi clients exit ops which unregisters video device allocated 
> memory during release callback of v4l2 device. vi/csi structure 
> allocation remains same as vi/csi driver removal will not happen in 
> this case.
>
>
> 3. during direct host1x client drivers vi/csi unbind, both 
> host1x_clients vi/csi gets unregistered, deletes host1x logical device 
> which executes tegra-video driver removal() -> vi/csi exit() before 
> vi/csi memory gets freed in vi/csi driver remove().
>
> So, any active streaming will stop and video devices are unregistered 
> during direct client driver unbind prior to freeing vi/csi memory.
>
> Also vi/csi driver remove does explicit free vi/csi as its allocated 
> with kzalloc. So not sure how using kzalloc is different to 
> devm_kzalloc for vi/csi structure in terms of when vi/csi memory gets 
> freed?
>
> Except for channel allocation which holds video device and as video 
> device life time is beyond tegra-video module unbind->vi exit(), looks 
> like we can use devm_kzalloc for vi/csi.
>
>
> Can you please comment if you still think we need to use kzalloc 
> rather than devm_kzalloc for vi/csi structure allocation?
>
> Thanks
>
> Sowjanya
>
One more case is when video device node is kept opened with v4l2-ctl 
sleep (rather than streaming), where it will keep device node open for 
specified time and if direct vi client driver unbind happens then vi 
driver remove() will free vi memory before v4l2 device release happens.

But I don't see any crash or errors with this case.

Also if we allow direct client driver unbind, then vi structure memory 
lifetime should also be till v4l2 device release happens.

But we can free vi in v4l2 device release callback as in case when 
device node is not kept opened, video device release happens immediate 
and we cant free vi that early.

Hans/Thierry, Can you please comment on this case?

Thanks

Sowjanya


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 17:21       ` Sowjanya Komatineni
@ 2020-04-15 17:47         ` Sowjanya Komatineni
  2020-04-15 17:48           ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 17:47 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 10:21 AM, Sowjanya Komatineni wrote:
>
> On 4/15/20 9:54 AM, Sowjanya Komatineni wrote:
>>
>> On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>>>> +static int tegra_csi_remove(struct platform_device *pdev)
>>>> +{
>>>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>>>> +     int err;
>>>> +
>>>> +     err = host1x_client_unregister(&csi->client);
>>>> +     if (err < 0) {
>>>> +             dev_err(csi->dev,
>>>> +                     "failed to unregister host1x client: %d\n", 
>>>> err);
>>>> +             return err;
>>>> +     }
>>>> +
>>>> +     pm_runtime_disable(csi->dev);
>>>> +     kfree(csi);
>>> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
>>> sure how moving away from the resource-managed API helps here. Could 
>>> you
>>> please explain in a more details?
>>>
>>> Have you tried to test this driver under KASAN? I suspect that you just
>>> masked the problem, instead of fixing it.
>> Using devm_kzalloc for vi/csi structures based on prior feedback 
>> request to switch to use kzalloc all over this driver.
>>
>> Hi Hans,
>>
>> video devices lifetime is till video device nodes are released. So, 
>> v4l2 device release callback does the release of tegra channel 
>> allocation which hold video device.
>>
>> Below are the 3 possible cases of unbind/unload,
>>
>> 1. during tegra-video module unload, if v4l2 device refcnt is not 0 
>> which is the case when any of video device node handle is kept opened 
>> then unloading module will not happen and module refcnt is also 
>> non-zero and unloading tegra-video module reports module in use.
>>
>> 2. during tegra-video driver unbind, tegra-video driver removal will 
>> do vi/csi clients exit ops which unregisters video device allocated 
>> memory during release callback of v4l2 device. vi/csi structure 
>> allocation remains same as vi/csi driver removal will not happen in 
>> this case.
>>
>>
>> 3. during direct host1x client drivers vi/csi unbind, both 
>> host1x_clients vi/csi gets unregistered, deletes host1x logical 
>> device which executes tegra-video driver removal() -> vi/csi exit() 
>> before vi/csi memory gets freed in vi/csi driver remove().
>>
>> So, any active streaming will stop and video devices are unregistered 
>> during direct client driver unbind prior to freeing vi/csi memory.
>>
>> Also vi/csi driver remove does explicit free vi/csi as its allocated 
>> with kzalloc. So not sure how using kzalloc is different to 
>> devm_kzalloc for vi/csi structure in terms of when vi/csi memory gets 
>> freed?
>>
>> Except for channel allocation which holds video device and as video 
>> device life time is beyond tegra-video module unbind->vi exit(), 
>> looks like we can use devm_kzalloc for vi/csi.
>>
>>
>> Can you please comment if you still think we need to use kzalloc 
>> rather than devm_kzalloc for vi/csi structure allocation?
>>
>> Thanks
>>
>> Sowjanya
>>
> One more case is when video device node is kept opened with v4l2-ctl 
> sleep (rather than streaming), where it will keep device node open for 
> specified time and if direct vi client driver unbind happens then vi 
> driver remove() will free vi memory before v4l2 device release happens.
>
> But I don't see any crash or errors with this case.
>
> Also if we allow direct client driver unbind, then vi structure memory 
> lifetime should also be till v4l2 device release happens.
>
> But we can free vi in v4l2 device release callback as in case when 
> device node is not kept opened, video device release happens immediate 
> and we cant free vi that early.

typo fix:

But we can't free vi structure memory allocation in v4l2 device release 
callback as in case when device node is not kept opened, device release 
happens immediate and we can't free vi structure memory that early.

>
> Hans/Thierry, Can you please comment on this case?
>
> Thanks
>
> Sowjanya
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 17:47         ` Sowjanya Komatineni
@ 2020-04-15 17:48           ` Sowjanya Komatineni
  2020-04-15 18:39             ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 17:48 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 10:47 AM, Sowjanya Komatineni wrote:
>
> On 4/15/20 10:21 AM, Sowjanya Komatineni wrote:
>>
>> On 4/15/20 9:54 AM, Sowjanya Komatineni wrote:
>>>
>>> On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>>>>> +static int tegra_csi_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>>>>> +     int err;
>>>>> +
>>>>> +     err = host1x_client_unregister(&csi->client);
>>>>> +     if (err < 0) {
>>>>> +             dev_err(csi->dev,
>>>>> +                     "failed to unregister host1x client: %d\n", 
>>>>> err);
>>>>> +             return err;
>>>>> +     }
>>>>> +
>>>>> +     pm_runtime_disable(csi->dev);
>>>>> +     kfree(csi);
>>>> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
>>>> sure how moving away from the resource-managed API helps here. 
>>>> Could you
>>>> please explain in a more details?
>>>>
>>>> Have you tried to test this driver under KASAN? I suspect that you 
>>>> just
>>>> masked the problem, instead of fixing it.
>>> Using devm_kzalloc for vi/csi structures based on prior feedback 
>>> request to switch to use kzalloc all over this driver.
>>>
>>> Hi Hans,
>>>
>>> video devices lifetime is till video device nodes are released. So, 
>>> v4l2 device release callback does the release of tegra channel 
>>> allocation which hold video device.
>>>
>>> Below are the 3 possible cases of unbind/unload,
>>>
>>> 1. during tegra-video module unload, if v4l2 device refcnt is not 0 
>>> which is the case when any of video device node handle is kept 
>>> opened then unloading module will not happen and module refcnt is 
>>> also non-zero and unloading tegra-video module reports module in use.
>>>
>>> 2. during tegra-video driver unbind, tegra-video driver removal will 
>>> do vi/csi clients exit ops which unregisters video device allocated 
>>> memory during release callback of v4l2 device. vi/csi structure 
>>> allocation remains same as vi/csi driver removal will not happen in 
>>> this case.
>>>
>>>
>>> 3. during direct host1x client drivers vi/csi unbind, both 
>>> host1x_clients vi/csi gets unregistered, deletes host1x logical 
>>> device which executes tegra-video driver removal() -> vi/csi exit() 
>>> before vi/csi memory gets freed in vi/csi driver remove().
>>>
>>> So, any active streaming will stop and video devices are 
>>> unregistered during direct client driver unbind prior to freeing 
>>> vi/csi memory.
>>>
>>> Also vi/csi driver remove does explicit free vi/csi as its allocated 
>>> with kzalloc. So not sure how using kzalloc is different to 
>>> devm_kzalloc for vi/csi structure in terms of when vi/csi memory 
>>> gets freed?
>>>
>>> Except for channel allocation which holds video device and as video 
>>> device life time is beyond tegra-video module unbind->vi exit(), 
>>> looks like we can use devm_kzalloc for vi/csi.
>>>
>>>
>>> Can you please comment if you still think we need to use kzalloc 
>>> rather than devm_kzalloc for vi/csi structure allocation?
>>>
>>> Thanks
>>>
>>> Sowjanya
>>>
>> One more case is when video device node is kept opened with v4l2-ctl 
>> sleep (rather than streaming), where it will keep device node open 
>> for specified time and if direct vi client driver unbind happens then 
>> vi driver remove() will free vi memory before v4l2 device release 
>> happens.
>>
>> But I don't see any crash or errors with this case.
>>
>> Also if we allow direct client driver unbind, then vi structure 
>> memory lifetime should also be till v4l2 device release happens.
>>
>> But we can free vi in v4l2 device release callback as in case when 
>> device node is not kept opened, video device release happens 
>> immediate and we cant free vi that early.
>
> typo fix:
>
> But we can't free vi structure memory allocation in v4l2 device 
> release callback as in case when device node is not kept opened, 
> device release happens immediate and we can't free vi structure memory 
> that early.
>
>>
>> Hans/Thierry, Can you please comment on this case?
>>
>> Thanks
>>
>> Sowjanya
>>
Also, Can you please help explain on cases where we do/need direct 
host1x clients vi/csi drivers unbind?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 17:48           ` Sowjanya Komatineni
@ 2020-04-15 18:39             ` Sowjanya Komatineni
  2020-04-15 18:53               ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 18:39 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 10:48 AM, Sowjanya Komatineni wrote:
>
> On 4/15/20 10:47 AM, Sowjanya Komatineni wrote:
>>
>> On 4/15/20 10:21 AM, Sowjanya Komatineni wrote:
>>>
>>> On 4/15/20 9:54 AM, Sowjanya Komatineni wrote:
>>>>
>>>> On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>>>>>> +static int tegra_csi_remove(struct platform_device *pdev)
>>>>>> +{
>>>>>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>>>>>> +     int err;
>>>>>> +
>>>>>> +     err = host1x_client_unregister(&csi->client);
>>>>>> +     if (err < 0) {
>>>>>> +             dev_err(csi->dev,
>>>>>> +                     "failed to unregister host1x client: %d\n", 
>>>>>> err);
>>>>>> +             return err;
>>>>>> +     }
>>>>>> +
>>>>>> +     pm_runtime_disable(csi->dev);
>>>>>> +     kfree(csi);
>>>>> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
>>>>> sure how moving away from the resource-managed API helps here. 
>>>>> Could you
>>>>> please explain in a more details?
>>>>>
>>>>> Have you tried to test this driver under KASAN? I suspect that you 
>>>>> just
>>>>> masked the problem, instead of fixing it.
>>>> Using devm_kzalloc for vi/csi structures based on prior feedback 
>>>> request to switch to use kzalloc all over this driver.
>>>>
>>>> Hi Hans,
>>>>
>>>> video devices lifetime is till video device nodes are released. So, 
>>>> v4l2 device release callback does the release of tegra channel 
>>>> allocation which hold video device.
>>>>
>>>> Below are the 3 possible cases of unbind/unload,
>>>>
>>>> 1. during tegra-video module unload, if v4l2 device refcnt is not 0 
>>>> which is the case when any of video device node handle is kept 
>>>> opened then unloading module will not happen and module refcnt is 
>>>> also non-zero and unloading tegra-video module reports module in use.
v4l2 device is associated with host1x device where during 
v4l2_device_register get_device causes refcnt of tegra video host1x 
device to increase and prevents allowing module unload/load till v4l2 
device release happens.


>>>> 2. during tegra-video driver unbind, tegra-video driver removal 
>>>> will do vi/csi clients exit ops which unregisters video device 
>>>> allocated memory during release callback of v4l2 device. vi/csi 
>>>> structure allocation remains same as vi/csi driver removal will not 
>>>> happen in this case.
>>>>
>>>>
>>>> 3. during direct host1x client drivers vi/csi unbind, both 
>>>> host1x_clients vi/csi gets unregistered, deletes host1x logical 
>>>> device which executes tegra-video driver removal() -> vi/csi exit() 
>>>> before vi/csi memory gets freed in vi/csi driver remove().
>>>>
>>>> So, any active streaming will stop and video devices are 
>>>> unregistered during direct client driver unbind prior to freeing 
>>>> vi/csi memory.
>>>>
>>>> Also vi/csi driver remove does explicit free vi/csi as its 
>>>> allocated with kzalloc. So not sure how using kzalloc is different 
>>>> to devm_kzalloc for vi/csi structure in terms of when vi/csi memory 
>>>> gets freed?
>>>>
>>>> Except for channel allocation which holds video device and as video 
>>>> device life time is beyond tegra-video module unbind->vi exit(), 
>>>> looks like we can use devm_kzalloc for vi/csi.
>>>>
>>>>
>>>> Can you please comment if you still think we need to use kzalloc 
>>>> rather than devm_kzalloc for vi/csi structure allocation?
>>>>
>>>> Thanks
>>>>
>>>> Sowjanya
>>>>
>>> One more case is when video device node is kept opened with v4l2-ctl 
>>> sleep (rather than streaming), where it will keep device node open 
>>> for specified time and if direct vi client driver unbind happens 
>>> then vi driver remove() will free vi memory before v4l2 device 
>>> release happens.
>>>
>>> But I don't see any crash or errors with this case.

In the above case, channels allocated memory release may not happen in 
this case as list head pointer will be gone when vi memory is freed 
during direct client unbind and by the time v4l2 device release callback 
gets executed vi channels list head is gone.

Also, freeing vi structure memory can't be done in v4l2 device release 
callback either.

>>>
>>> Also if we allow direct client driver unbind, then vi structure 
>>> memory lifetime should also be till v4l2 device release happens.
>>>
>>> But we can free vi in v4l2 device release callback as in case when 
>>> device node is not kept opened, video device release happens 
>>> immediate and we cant free vi that early.
>>
>> typo fix:
>>
>> But we can't free vi structure memory allocation in v4l2 device 
>> release callback as in case when device node is not kept opened, 
>> device release happens immediate and we can't free vi structure 
>> memory that early.
>>

>>> Hans/Thierry, Can you please comment on this case?
>>>
>>> Thanks
>>>
>>> Sowjanya
>>>
> Also, Can you please help explain on cases where we do/need direct 
> host1x clients vi/csi drivers unbind?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 18:39             ` Sowjanya Komatineni
@ 2020-04-15 18:53               ` Sowjanya Komatineni
  2020-04-15 19:21                 ` Dmitry Osipenko
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 18:53 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
	sakari.ailus, helen.koike
  Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
	linux-kernel


On 4/15/20 11:39 AM, Sowjanya Komatineni wrote:
>
> On 4/15/20 10:48 AM, Sowjanya Komatineni wrote:
>>
>> On 4/15/20 10:47 AM, Sowjanya Komatineni wrote:
>>>
>>> On 4/15/20 10:21 AM, Sowjanya Komatineni wrote:
>>>>
>>>> On 4/15/20 9:54 AM, Sowjanya Komatineni wrote:
>>>>>
>>>>> On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>>>>>>> +static int tegra_csi_remove(struct platform_device *pdev)
>>>>>>> +{
>>>>>>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>>>>>>> +     int err;
>>>>>>> +
>>>>>>> +     err = host1x_client_unregister(&csi->client);
>>>>>>> +     if (err < 0) {
>>>>>>> +             dev_err(csi->dev,
>>>>>>> +                     "failed to unregister host1x client: 
>>>>>>> %d\n", err);
>>>>>>> +             return err;
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     pm_runtime_disable(csi->dev);
>>>>>>> +     kfree(csi);
>>>>>> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
>>>>>> sure how moving away from the resource-managed API helps here. 
>>>>>> Could you
>>>>>> please explain in a more details?
>>>>>>
>>>>>> Have you tried to test this driver under KASAN? I suspect that 
>>>>>> you just
>>>>>> masked the problem, instead of fixing it.
Tested with kmemleak scan and did not see any memory leaks
>>>>> Using devm_kzalloc for vi/csi structures based on prior feedback 
>>>>> request to switch to use kzalloc all over this driver.
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> video devices lifetime is till video device nodes are released. 
>>>>> So, v4l2 device release callback does the release of tegra channel 
>>>>> allocation which hold video device.
>>>>>
>>>>> Below are the 3 possible cases of unbind/unload,
>>>>>
>>>>> 1. during tegra-video module unload, if v4l2 device refcnt is not 
>>>>> 0 which is the case when any of video device node handle is kept 
>>>>> opened then unloading module will not happen and module refcnt is 
>>>>> also non-zero and unloading tegra-video module reports module in use.
> v4l2 device is associated with host1x device where during 
> v4l2_device_register get_device causes refcnt of tegra video host1x 
> device to increase and prevents allowing module unload/load till v4l2 
> device release happens.
>
>
>>>>> 2. during tegra-video driver unbind, tegra-video driver removal 
>>>>> will do vi/csi clients exit ops which unregisters video device 
>>>>> allocated memory during release callback of v4l2 device. vi/csi 
>>>>> structure allocation remains same as vi/csi driver removal will 
>>>>> not happen in this case.
>>>>>
>>>>>
>>>>> 3. during direct host1x client drivers vi/csi unbind, both 
>>>>> host1x_clients vi/csi gets unregistered, deletes host1x logical 
>>>>> device which executes tegra-video driver removal() -> vi/csi 
>>>>> exit() before vi/csi memory gets freed in vi/csi driver remove().
>>>>>
>>>>> So, any active streaming will stop and video devices are 
>>>>> unregistered during direct client driver unbind prior to freeing 
>>>>> vi/csi memory.
>>>>>
>>>>> Also vi/csi driver remove does explicit free vi/csi as its 
>>>>> allocated with kzalloc. So not sure how using kzalloc is different 
>>>>> to devm_kzalloc for vi/csi structure in terms of when vi/csi 
>>>>> memory gets freed?
>>>>>
>>>>> Except for channel allocation which holds video device and as 
>>>>> video device life time is beyond tegra-video module unbind->vi 
>>>>> exit(), looks like we can use devm_kzalloc for vi/csi.
>>>>>
>>>>>
>>>>> Can you please comment if you still think we need to use kzalloc 
>>>>> rather than devm_kzalloc for vi/csi structure allocation?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Sowjanya
>>>>>
>>>> One more case is when video device node is kept opened with 
>>>> v4l2-ctl sleep (rather than streaming), where it will keep device 
>>>> node open for specified time and if direct vi client driver unbind 
>>>> happens then vi driver remove() will free vi memory before v4l2 
>>>> device release happens.
>>>>
>>>> But I don't see any crash or errors with this case.
>
> In the above case, channels allocated memory release may not happen in 
> this case as list head pointer will be gone when vi memory is freed 
> during direct client unbind and by the time v4l2 device release 
> callback gets executed vi channels list head is gone.
>
> Also, freeing vi structure memory can't be done in v4l2 device release 
> callback either.
>
>>>>
>>>> Also if we allow direct client driver unbind, then vi structure 
>>>> memory lifetime should also be till v4l2 device release happens.
>>>>
>>>> But we can free vi in v4l2 device release callback as in case when 
>>>> device node is not kept opened, video device release happens 
>>>> immediate and we cant free vi that early.
>>>
>>> typo fix:
>>>
>>> But we can't free vi structure memory allocation in v4l2 device 
>>> release callback as in case when device node is not kept opened, 
>>> device release happens immediate and we can't free vi structure 
>>> memory that early.
>>>
>
>>>> Hans/Thierry, Can you please comment on this case?
>>>>
>>>> Thanks
>>>>
>>>> Sowjanya
>>>>
>> Also, Can you please help explain on cases where we do/need direct 
>> host1x clients vi/csi drivers unbind?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 18:53               ` Sowjanya Komatineni
@ 2020-04-15 19:21                 ` Dmitry Osipenko
  2020-04-15 19:51                   ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Osipenko @ 2020-04-15 19:21 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
	helen.koike, sboyd, linux-media, devicetree, linux-clk,
	linux-tegra, linux-kernel

15.04.2020 21:53, Sowjanya Komatineni пишет:
...
>>>>>>> Have you tried to test this driver under KASAN? I suspect that
>>>>>>> you just
>>>>>>> masked the problem, instead of fixing it.
> Tested with kmemleak scan and did not see any memory leaks

You should get use-after-free and not memleak.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 19:21                 ` Dmitry Osipenko
@ 2020-04-15 19:51                   ` Sowjanya Komatineni
  2020-04-15 23:08                     ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 19:51 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
	helen.koike, sboyd, linux-media, devicetree, linux-clk,
	linux-tegra, linux-kernel


On 4/15/20 12:21 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 15.04.2020 21:53, Sowjanya Komatineni пишет:
> ...
>>>>>>>> Have you tried to test this driver under KASAN? I suspect that
>>>>>>>> you just
>>>>>>>> masked the problem, instead of fixing it.
>> Tested with kmemleak scan and did not see any memory leaks
> You should get use-after-free and not memleak.
I don't see use-after-free bugs during the testing.

But as mentioned when direct vi/csi client driver unbind happens while 
video device node is kept opened, vi driver remove will free vi 
structure memory but actual video device memory which is part of 
channels remains but list head gets lost when vi structure is freed.

So, when device node is released and executes release callback as list 
head is lost it can't free allocated channels which is not good.

This happens only with direct host1x client vi/csi driver unbind.

Need to find better place to free host1x client driver data structure to 
allow direct client driver unbind->bind.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 19:51                   ` Sowjanya Komatineni
@ 2020-04-15 23:08                     ` Sowjanya Komatineni
  2020-04-15 23:28                       ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 23:08 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
	helen.koike, sboyd, linux-media, devicetree, linux-clk,
	linux-tegra, linux-kernel

With minor change of not using vi reference after 
host1x_client_unregister and freeing vi during v4l2 device release works.

For csi, we can use devm_kzalloc for now untill we decide later if we 
want to expose async subdev nodes during sensor support.

Will have this fix in v8 with a comment in vi_remove to make sure not to 
use vi reference after host1x_client_unregister.

Will test more and will release v8 with above fix to allow direct host1x 
client driver unbind.

Thanks

sowjanya


On 4/15/20 12:51 PM, Sowjanya Komatineni wrote:
>
> On 4/15/20 12:21 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 15.04.2020 21:53, Sowjanya Komatineni пишет:
>> ...
>>>>>>>>> Have you tried to test this driver under KASAN? I suspect that
>>>>>>>>> you just
>>>>>>>>> masked the problem, instead of fixing it.
>>> Tested with kmemleak scan and did not see any memory leaks
>> You should get use-after-free and not memleak.
> I don't see use-after-free bugs during the testing.
>
> But as mentioned when direct vi/csi client driver unbind happens while 
> video device node is kept opened, vi driver remove will free vi 
> structure memory but actual video device memory which is part of 
> channels remains but list head gets lost when vi structure is freed.
>
> So, when device node is released and executes release callback as list 
> head is lost it can't free allocated channels which is not good.
>
> This happens only with direct host1x client vi/csi driver unbind.
>
> Need to find better place to free host1x client driver data structure 
> to allow direct client driver unbind->bind.
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 23:08                     ` Sowjanya Komatineni
@ 2020-04-15 23:28                       ` Sowjanya Komatineni
  2020-04-16 15:12                         ` Sowjanya Komatineni
  0 siblings, 1 reply; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-15 23:28 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
	helen.koike, sboyd, linux-media, devicetree, linux-clk,
	linux-tegra, linux-kernel

Sorry please ignore.

We can't free vi during v4l2 device release as when no device nodes are 
opened, vi free happens right away during host1x_video_remove.

With this tegra-video driver unbind ->bind will not work as vi memory 
allocated during vi_probe gets freed during v4l2 device release so 
during bind init() callback execution will crash as vi got freed while 
vi driver is still bound to device.

Will wait for Hans/Thierry comments as I see dependency depending on 
where unbind/bind happens.


On 4/15/20 4:08 PM, Sowjanya Komatineni wrote:
> With minor change of not using vi reference after 
> host1x_client_unregister and freeing vi during v4l2 device release works.
>
> For csi, we can use devm_kzalloc for now untill we decide later if we 
> want to expose async subdev nodes during sensor support.
>
> Will have this fix in v8 with a comment in vi_remove to make sure not 
> to use vi reference after host1x_client_unregister.
>
> Will test more and will release v8 with above fix to allow direct 
> host1x client driver unbind.
>
> Thanks
>
> sowjanya
>
>
> On 4/15/20 12:51 PM, Sowjanya Komatineni wrote:
>>
>> On 4/15/20 12:21 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 15.04.2020 21:53, Sowjanya Komatineni пишет:
>>> ...
>>>>>>>>>> Have you tried to test this driver under KASAN? I suspect that
>>>>>>>>>> you just
>>>>>>>>>> masked the problem, instead of fixing it.
>>>> Tested with kmemleak scan and did not see any memory leaks
>>> You should get use-after-free and not memleak.
>> I don't see use-after-free bugs during the testing.
>>
>> But as mentioned when direct vi/csi client driver unbind happens 
>> while video device node is kept opened, vi driver remove will free vi 
>> structure memory but actual video device memory which is part of 
>> channels remains but list head gets lost when vi structure is freed.
>>
>> So, when device node is released and executes release callback as 
>> list head is lost it can't free allocated channels which is not good.
>>
>> This happens only with direct host1x client vi/csi driver unbind.
>>
>> Need to find better place to free host1x client driver data structure 
>> to allow direct client driver unbind->bind.
>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver
  2020-04-15 23:28                       ` Sowjanya Komatineni
@ 2020-04-16 15:12                         ` Sowjanya Komatineni
  0 siblings, 0 replies; 21+ messages in thread
From: Sowjanya Komatineni @ 2020-04-16 15:12 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
	helen.koike, sboyd, linux-media, devicetree, linux-clk,
	linux-tegra, linux-kernel

tegra-video module unload->load and tegra-video driver unbind->bind are 
good.

Will have v8 to switch to use devm_kzalloc for vi/csi and will revisit 
direct host1x client driver unbind->bind later.

Thanks

Sowjanya


On 4/15/20 4:28 PM, Sowjanya Komatineni wrote:
> Sorry please ignore.
>
> We can't free vi during v4l2 device release as when no device nodes 
> are opened, vi free happens right away during host1x_video_remove.
>
> With this tegra-video driver unbind ->bind will not work as vi memory 
> allocated during vi_probe gets freed during v4l2 device release so 
> during bind init() callback execution will crash as vi got freed while 
> vi driver is still bound to device.
>
> Will wait for Hans/Thierry comments as I see dependency depending on 
> where unbind/bind happens.
>
>
> On 4/15/20 4:08 PM, Sowjanya Komatineni wrote:
>> With minor change of not using vi reference after 
>> host1x_client_unregister and freeing vi during v4l2 device release 
>> works.
>>
>> For csi, we can use devm_kzalloc for now untill we decide later if we 
>> want to expose async subdev nodes during sensor support.
>>
>> Will have this fix in v8 with a comment in vi_remove to make sure not 
>> to use vi reference after host1x_client_unregister.
>>
>> Will test more and will release v8 with above fix to allow direct 
>> host1x client driver unbind.
>>
>> Thanks
>>
>> sowjanya
>>
>>
>> On 4/15/20 12:51 PM, Sowjanya Komatineni wrote:
>>>
>>> On 4/15/20 12:21 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 15.04.2020 21:53, Sowjanya Komatineni пишет:
>>>> ...
>>>>>>>>>>> Have you tried to test this driver under KASAN? I suspect that
>>>>>>>>>>> you just
>>>>>>>>>>> masked the problem, instead of fixing it.
>>>>> Tested with kmemleak scan and did not see any memory leaks
>>>> You should get use-after-free and not memleak.
>>> I don't see use-after-free bugs during the testing.
>>>
>>> But as mentioned when direct vi/csi client driver unbind happens 
>>> while video device node is kept opened, vi driver remove will free 
>>> vi structure memory but actual video device memory which is part of 
>>> channels remains but list head gets lost when vi structure is freed.
>>>
>>> So, when device node is released and executes release callback as 
>>> list head is lost it can't free allocated channels which is not good.
>>>
>>> This happens only with direct host1x client vi/csi driver unbind.
>>>
>>> Need to find better place to free host1x client driver data 
>>> structure to allow direct client driver unbind->bind.
>>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-04-16 15:14 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-15  2:57 [RFC PATCH v7 0/9] Add Tegra driver for video capture Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 2/9] arm64: tegra: Add reset-cells to mc Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 7/9] MAINTAINERS: Add Tegra Video driver section Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset Sowjanya Komatineni
2020-04-15  2:57 ` [RFC PATCH v7 9/9] arm64: tegra: Add Tegra VI CSI support in device tree Sowjanya Komatineni
     [not found] ` <1586919463-30542-7-git-send-email-skomatineni@nvidia.com>
2020-04-15 14:22   ` [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver Dmitry Osipenko
2020-04-15 16:54     ` Sowjanya Komatineni
2020-04-15 17:21       ` Sowjanya Komatineni
2020-04-15 17:47         ` Sowjanya Komatineni
2020-04-15 17:48           ` Sowjanya Komatineni
2020-04-15 18:39             ` Sowjanya Komatineni
2020-04-15 18:53               ` Sowjanya Komatineni
2020-04-15 19:21                 ` Dmitry Osipenko
2020-04-15 19:51                   ` Sowjanya Komatineni
2020-04-15 23:08                     ` Sowjanya Komatineni
2020-04-15 23:28                       ` Sowjanya Komatineni
2020-04-16 15:12                         ` Sowjanya Komatineni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox