[RFC,v3,00/18] Support for Tegra video capture from external sensor

Message ID 1594786855-26506-1-git-send-email-skomatineni@nvidia.com (mailing list archive)
Headers
Series Support for Tegra video capture from external sensor |

Message

Sowjanya Komatineni July 15, 2020, 4:20 a.m. UTC
  This series adds support for video capture from external camera sensor to
Tegra video driver.

Jetson TX1 has camera expansion connector and supports custom camera module
designed as per TX1 design specification.

This series also enables camera capture support for Jetson Nano which has
Raspberry PI camera header.

This series is tested with IMX219 camera sensor.

This series include,

VI I2C related fixes
- Camera sensor programming happens through VI I2C which is on host1x bus.
- These patches includes device tree and I2C driver fixes for VI I2C.

Tegra video driver updates
- TPG Vs Non-TPG based on Kconfig
- Support for external sensor video capture based on device graph from DT.
- Support for selection ioctl operations
- Tegra MIPI CSI pads calibration
- CSI T-CLK and T-HS settle time computation based on clock rates.

Host1x driver updates
- Adds API to allow creating mipi device for specific device node.
- Splits MIPI pads calibrate start and waiting for calibration to be done.

Device tree updates
- Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson TX1 DT.
- Enabled VI and CSI support in Jetson Nano DT.


Delta between patch versions:

[v3]:	Includes v2 feedback
	- Uses separate helper function for retrieving remote csi subdevice
	  and source subdevice.
	- Added check for presence of subdevice ops set/get_selection
	- dropped vb2_queue_release from driver and using
	  vb2_video_unregister_device instead of video_unregister_device.
	- video device register should happen in the last after all video
	  device related setup is done in the driver. This is being addressed
	  in below RFC patch. Once proper implementation of this is available
	  will update Tegra video driver to use split APIs and do all setup
	  prior to device register. Added this as TODO in the driver.
	  https://www.spinics.net/lists/linux-media/msg172761.html

	Note:
	Patch-0012 has compilation dependency on
	https://patchwork.kernel.org/patch/11659521/


[v2]:	Includes below changes based on v1 feedback
	- dt-binding document and the driver update for device graph to use
	  separate ports for sink endpoint and source endpoint for csi.
	- Use data-lanes endpoint property for csi.
	- Update tegra_mipi_request() to take device node pointer argument
	  rather than adding extra API.
	- Remove checking for clk pointer before clk_disable.


Sowjanya Komatineni (18):
  dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
    power-domains
  arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C
  i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
  i2c: tegra: Remove NULL pointer check before
    clk_enable/disable/prepare/unprepare
  i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
  i2c: tegra: Fix runtime resume to re-init VI I2C
  i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
  media: tegra-video: Fix channel format alignment
  media: tegra-video: Enable TPG based on kernel config
  media: tegra-video: Update format lookup to offset based
  dt-bindings: tegra: Update VI and CSI bindings with port info
  media: tegra-video: Add support for external sensor capture
  media: tegra-video: Add support for selection ioctl ops
  gpu: host1x: mipi: Update tegra_mipi_request() to be node based
  gpu: host1x: mipi: Use readl_relaxed_poll_timeout in tegra_mipi_wait
  gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
  media: tegra-video: Add CSI MIPI pads calibration
  media: tegra-video: Compute settle times based on the clock rate

 .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
 drivers/gpu/drm/tegra/dsi.c                        |   9 +-
 drivers/gpu/host1x/mipi.c                          |  37 +-
 drivers/i2c/busses/i2c-tegra.c                     | 101 +--
 drivers/staging/media/tegra-video/Kconfig          |   7 +
 drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
 drivers/staging/media/tegra-video/csi.h            |   8 +
 drivers/staging/media/tegra-video/tegra210.c       |  25 +-
 drivers/staging/media/tegra-video/vi.c             | 793 +++++++++++++++++++--
 drivers/staging/media/tegra-video/vi.h             |  25 +-
 drivers/staging/media/tegra-video/video.c          |  23 +-
 include/linux/host1x.h                             |   4 +-
 14 files changed, 1242 insertions(+), 154 deletions(-)
  

Comments

Hans Verkuil July 17, 2020, 10:54 a.m. UTC | #1
Hi Sowjanya,

On 15/07/2020 06:20, Sowjanya Komatineni wrote:
> This series adds support for video capture from external camera sensor to
> Tegra video driver.
> 
> Jetson TX1 has camera expansion connector and supports custom camera module
> designed as per TX1 design specification.
> 
> This series also enables camera capture support for Jetson Nano which has
> Raspberry PI camera header.
> 
> This series is tested with IMX219 camera sensor.
> 
> This series include,
> 
> VI I2C related fixes
> - Camera sensor programming happens through VI I2C which is on host1x bus.
> - These patches includes device tree and I2C driver fixes for VI I2C.
> 
> Tegra video driver updates
> - TPG Vs Non-TPG based on Kconfig
> - Support for external sensor video capture based on device graph from DT.
> - Support for selection ioctl operations
> - Tegra MIPI CSI pads calibration
> - CSI T-CLK and T-HS settle time computation based on clock rates.
> 
> Host1x driver updates
> - Adds API to allow creating mipi device for specific device node.
> - Splits MIPI pads calibrate start and waiting for calibration to be done.
> 
> Device tree updates
> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson TX1 DT.
> - Enabled VI and CSI support in Jetson Nano DT.

I'm doing a bit of stress testing with:

while true; do v4l2-ctl --stream-mmap --stream-count=1; done

and I see that the imx274 has often streaming failures:

[  172.025144] IMX274 8-001a: s_stream failed
[  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3132 = 870 (2 bytes)
[  179.033575] IMX274 8-001a: s_stream failed
[  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3130 = 878 (2 bytes)
[  226.533761] IMX274 8-001a: s_stream failed
[  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
[  227.037758] IMX274 8-001a: s_stream failed
[  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
[  247.033658] IMX274 8-001a: s_stream failed
[  293.025517] IMX274 8-001a: s_stream failed
[  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30e0 = 0 (2 bytes)
[  309.032969] IMX274 8-001a: s_stream failed
[  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 11d2 (3 bytes)
[  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
[  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
[  309.550243] IMX274 8-001a: s_stream failed
[  314.025561] IMX274 8-001a: s_stream failed
[  329.025586] IMX274 8-001a: s_stream failed
[  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 303a = f0c (2 bytes)
[  340.538009] IMX274 8-001a: s_stream failed
[  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
[  347.534008] IMX274 8-001a: s_stream failed
[  365.033640] IMX274 8-001a: s_stream failed
[  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3038 = c (2 bytes)
[  437.533997] IMX274 8-001a: s_stream failed
[  456.029780] IMX274 8-001a: s_stream failed
[  472.025862] IMX274 8-001a: s_stream failed
[  498.025861] IMX274 8-001a: s_stream failed
[  500.025905] IMX274 8-001a: s_stream failed

where v4l2-ctl returns:

                VIDIOC_STREAMON returned -1 (Remote I/O error)

I don't see this with the imx219.

I also see this occasionally:

[Fri Jul 17 12:51:42 2020] video4linux video1: failed to run capture start kthread: -4

Something is not stable here.

Regards,

	Hans

> 
> 
> Delta between patch versions:
> 
> [v3]:	Includes v2 feedback
> 	- Uses separate helper function for retrieving remote csi subdevice
> 	  and source subdevice.
> 	- Added check for presence of subdevice ops set/get_selection
> 	- dropped vb2_queue_release from driver and using
> 	  vb2_video_unregister_device instead of video_unregister_device.
> 	- video device register should happen in the last after all video
> 	  device related setup is done in the driver. This is being addressed
> 	  in below RFC patch. Once proper implementation of this is available
> 	  will update Tegra video driver to use split APIs and do all setup
> 	  prior to device register. Added this as TODO in the driver.
> 	  https://www.spinics.net/lists/linux-media/msg172761.html
> 
> 	Note:
> 	Patch-0012 has compilation dependency on
> 	https://patchwork.kernel.org/patch/11659521/
> 
> 
> [v2]:	Includes below changes based on v1 feedback
> 	- dt-binding document and the driver update for device graph to use
> 	  separate ports for sink endpoint and source endpoint for csi.
> 	- Use data-lanes endpoint property for csi.
> 	- Update tegra_mipi_request() to take device node pointer argument
> 	  rather than adding extra API.
> 	- Remove checking for clk pointer before clk_disable.
> 
> 
> Sowjanya Komatineni (18):
>   dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>     power-domains
>   arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C
>   i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>   i2c: tegra: Remove NULL pointer check before
>     clk_enable/disable/prepare/unprepare
>   i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>   i2c: tegra: Fix runtime resume to re-init VI I2C
>   i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>   media: tegra-video: Fix channel format alignment
>   media: tegra-video: Enable TPG based on kernel config
>   media: tegra-video: Update format lookup to offset based
>   dt-bindings: tegra: Update VI and CSI bindings with port info
>   media: tegra-video: Add support for external sensor capture
>   media: tegra-video: Add support for selection ioctl ops
>   gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>   gpu: host1x: mipi: Use readl_relaxed_poll_timeout in tegra_mipi_wait
>   gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
>   media: tegra-video: Add CSI MIPI pads calibration
>   media: tegra-video: Compute settle times based on the clock rate
> 
>  .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
>  drivers/gpu/drm/tegra/dsi.c                        |   9 +-
>  drivers/gpu/host1x/mipi.c                          |  37 +-
>  drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>  drivers/staging/media/tegra-video/Kconfig          |   7 +
>  drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>  drivers/staging/media/tegra-video/csi.h            |   8 +
>  drivers/staging/media/tegra-video/tegra210.c       |  25 +-
>  drivers/staging/media/tegra-video/vi.c             | 793 +++++++++++++++++++--
>  drivers/staging/media/tegra-video/vi.h             |  25 +-
>  drivers/staging/media/tegra-video/video.c          |  23 +-
>  include/linux/host1x.h                             |   4 +-
>  14 files changed, 1242 insertions(+), 154 deletions(-)
>
  
Sowjanya Komatineni July 17, 2020, 4:34 p.m. UTC | #2
On 7/17/20 3:54 AM, Hans Verkuil wrote:
> Hi Sowjanya,
>
> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>> This series adds support for video capture from external camera sensor to
>> Tegra video driver.
>>
>> Jetson TX1 has camera expansion connector and supports custom camera module
>> designed as per TX1 design specification.
>>
>> This series also enables camera capture support for Jetson Nano which has
>> Raspberry PI camera header.
>>
>> This series is tested with IMX219 camera sensor.
>>
>> This series include,
>>
>> VI I2C related fixes
>> - Camera sensor programming happens through VI I2C which is on host1x bus.
>> - These patches includes device tree and I2C driver fixes for VI I2C.
>>
>> Tegra video driver updates
>> - TPG Vs Non-TPG based on Kconfig
>> - Support for external sensor video capture based on device graph from DT.
>> - Support for selection ioctl operations
>> - Tegra MIPI CSI pads calibration
>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>
>> Host1x driver updates
>> - Adds API to allow creating mipi device for specific device node.
>> - Splits MIPI pads calibrate start and waiting for calibration to be done.
>>
>> Device tree updates
>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson TX1 DT.
>> - Enabled VI and CSI support in Jetson Nano DT.
> I'm doing a bit of stress testing with:
>
> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>
> and I see that the imx274 has often streaming failures:
>
> [  172.025144] IMX274 8-001a: s_stream failed
> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3132 = 870 (2 bytes)
> [  179.033575] IMX274 8-001a: s_stream failed
> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3130 = 878 (2 bytes)
> [  226.533761] IMX274 8-001a: s_stream failed
> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
> [  227.037758] IMX274 8-001a: s_stream failed
> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
> [  247.033658] IMX274 8-001a: s_stream failed
> [  293.025517] IMX274 8-001a: s_stream failed
> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30e0 = 0 (2 bytes)
> [  309.032969] IMX274 8-001a: s_stream failed
> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 11d2 (3 bytes)
> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
> [  309.550243] IMX274 8-001a: s_stream failed
> [  314.025561] IMX274 8-001a: s_stream failed
> [  329.025586] IMX274 8-001a: s_stream failed
> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 303a = f0c (2 bytes)
> [  340.538009] IMX274 8-001a: s_stream failed
> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
> [  347.534008] IMX274 8-001a: s_stream failed
> [  365.033640] IMX274 8-001a: s_stream failed
> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3038 = c (2 bytes)
> [  437.533997] IMX274 8-001a: s_stream failed
> [  456.029780] IMX274 8-001a: s_stream failed
> [  472.025862] IMX274 8-001a: s_stream failed
> [  498.025861] IMX274 8-001a: s_stream failed
> [  500.025905] IMX274 8-001a: s_stream failed
>
> where v4l2-ctl returns:
>
>                  VIDIOC_STREAMON returned -1 (Remote I/O error)
>
> I don't see this with the imx219.
>
> I also see this occasionally:
>
> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run capture start kthread: -4
>
> Something is not stable here.
>
> Regards,
>
> 	Hans

Hi Hans,

Running the same single frame continuous loop for more than 2 hours now 
and I don't see any failure.

Above failure shows i2c bulk writes to IMX274 failure due to which 
s_stream also failed.

Not sure if its due to i2c mux in the path to sensor on your module 
causing some issue when there is more i2c write traffic as we are doing 
single stream in continuous loop. Also IMX219 does not show on your side 
so something specific to IMX274 setup probably.


Regarding kthread_run failure where kthread_run() returned -EINTR during 
capture start thread, I always see this happen at the point of stopping 
the continuous single stream while loop by pressing ctrl+c after few 
loops of execution.

while true; do v4l2-ctl --stream-mmap --stream-count=1; done

when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not 
terminate immediately and probably SIGKILLed  is seen prior to complete.

Using below can help to terminate loop as well when we stop ctrl-c and 
with this I don't see any repro of EINTR error from kthread_run when run 
in infinite loop.

while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; done



>>
>> Delta between patch versions:
>>
>> [v3]:	Includes v2 feedback
>> 	- Uses separate helper function for retrieving remote csi subdevice
>> 	  and source subdevice.
>> 	- Added check for presence of subdevice ops set/get_selection
>> 	- dropped vb2_queue_release from driver and using
>> 	  vb2_video_unregister_device instead of video_unregister_device.
>> 	- video device register should happen in the last after all video
>> 	  device related setup is done in the driver. This is being addressed
>> 	  in below RFC patch. Once proper implementation of this is available
>> 	  will update Tegra video driver to use split APIs and do all setup
>> 	  prior to device register. Added this as TODO in the driver.
>> 	  https://www.spinics.net/lists/linux-media/msg172761.html
>>
>> 	Note:
>> 	Patch-0012 has compilation dependency on
>> 	https://patchwork.kernel.org/patch/11659521/
>>
>>
>> [v2]:	Includes below changes based on v1 feedback
>> 	- dt-binding document and the driver update for device graph to use
>> 	  separate ports for sink endpoint and source endpoint for csi.
>> 	- Use data-lanes endpoint property for csi.
>> 	- Update tegra_mipi_request() to take device node pointer argument
>> 	  rather than adding extra API.
>> 	- Remove checking for clk pointer before clk_disable.
>>
>>
>> Sowjanya Komatineni (18):
>>    dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>      power-domains
>>    arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C
>>    i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>    i2c: tegra: Remove NULL pointer check before
>>      clk_enable/disable/prepare/unprepare
>>    i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>    i2c: tegra: Fix runtime resume to re-init VI I2C
>>    i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>    media: tegra-video: Fix channel format alignment
>>    media: tegra-video: Enable TPG based on kernel config
>>    media: tegra-video: Update format lookup to offset based
>>    dt-bindings: tegra: Update VI and CSI bindings with port info
>>    media: tegra-video: Add support for external sensor capture
>>    media: tegra-video: Add support for selection ioctl ops
>>    gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>    gpu: host1x: mipi: Use readl_relaxed_poll_timeout in tegra_mipi_wait
>>    gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
>>    media: tegra-video: Add CSI MIPI pads calibration
>>    media: tegra-video: Compute settle times based on the clock rate
>>
>>   .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
>>   .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
>>   arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
>>   drivers/gpu/drm/tegra/dsi.c                        |   9 +-
>>   drivers/gpu/host1x/mipi.c                          |  37 +-
>>   drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>>   drivers/staging/media/tegra-video/Kconfig          |   7 +
>>   drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>>   drivers/staging/media/tegra-video/csi.h            |   8 +
>>   drivers/staging/media/tegra-video/tegra210.c       |  25 +-
>>   drivers/staging/media/tegra-video/vi.c             | 793 +++++++++++++++++++--
>>   drivers/staging/media/tegra-video/vi.h             |  25 +-
>>   drivers/staging/media/tegra-video/video.c          |  23 +-
>>   include/linux/host1x.h                             |   4 +-
>>   14 files changed, 1242 insertions(+), 154 deletions(-)
>>
  
Hans Verkuil July 17, 2020, 5:08 p.m. UTC | #3
On 17/07/2020 18:34, Sowjanya Komatineni wrote:
> 
> On 7/17/20 3:54 AM, Hans Verkuil wrote:
>> Hi Sowjanya,
>>
>> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>>> This series adds support for video capture from external camera sensor to
>>> Tegra video driver.
>>>
>>> Jetson TX1 has camera expansion connector and supports custom camera module
>>> designed as per TX1 design specification.
>>>
>>> This series also enables camera capture support for Jetson Nano which has
>>> Raspberry PI camera header.
>>>
>>> This series is tested with IMX219 camera sensor.
>>>
>>> This series include,
>>>
>>> VI I2C related fixes
>>> - Camera sensor programming happens through VI I2C which is on host1x bus.
>>> - These patches includes device tree and I2C driver fixes for VI I2C.
>>>
>>> Tegra video driver updates
>>> - TPG Vs Non-TPG based on Kconfig
>>> - Support for external sensor video capture based on device graph from DT.
>>> - Support for selection ioctl operations
>>> - Tegra MIPI CSI pads calibration
>>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>>
>>> Host1x driver updates
>>> - Adds API to allow creating mipi device for specific device node.
>>> - Splits MIPI pads calibrate start and waiting for calibration to be done.
>>>
>>> Device tree updates
>>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson TX1 DT.
>>> - Enabled VI and CSI support in Jetson Nano DT.
>> I'm doing a bit of stress testing with:
>>
>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>
>> and I see that the imx274 has often streaming failures:
>>
>> [  172.025144] IMX274 8-001a: s_stream failed
>> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3132 = 870 (2 bytes)
>> [  179.033575] IMX274 8-001a: s_stream failed
>> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3130 = 878 (2 bytes)
>> [  226.533761] IMX274 8-001a: s_stream failed
>> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>> [  227.037758] IMX274 8-001a: s_stream failed
>> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>> [  247.033658] IMX274 8-001a: s_stream failed
>> [  293.025517] IMX274 8-001a: s_stream failed
>> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30e0 = 0 (2 bytes)
>> [  309.032969] IMX274 8-001a: s_stream failed
>> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 11d2 (3 bytes)
>> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
>> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
>> [  309.550243] IMX274 8-001a: s_stream failed
>> [  314.025561] IMX274 8-001a: s_stream failed
>> [  329.025586] IMX274 8-001a: s_stream failed
>> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 303a = f0c (2 bytes)
>> [  340.538009] IMX274 8-001a: s_stream failed
>> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>> [  347.534008] IMX274 8-001a: s_stream failed
>> [  365.033640] IMX274 8-001a: s_stream failed
>> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3038 = c (2 bytes)
>> [  437.533997] IMX274 8-001a: s_stream failed
>> [  456.029780] IMX274 8-001a: s_stream failed
>> [  472.025862] IMX274 8-001a: s_stream failed
>> [  498.025861] IMX274 8-001a: s_stream failed
>> [  500.025905] IMX274 8-001a: s_stream failed
>>
>> where v4l2-ctl returns:
>>
>>                  VIDIOC_STREAMON returned -1 (Remote I/O error)
>>
>> I don't see this with the imx219.
>>
>> I also see this occasionally:
>>
>> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run capture start kthread: -4
>>
>> Something is not stable here.
>>
>> Regards,
>>
>> 	Hans
> 
> Hi Hans,
> 
> Running the same single frame continuous loop for more than 2 hours now 
> and I don't see any failure.
> 
> Above failure shows i2c bulk writes to IMX274 failure due to which 
> s_stream also failed.
> 
> Not sure if its due to i2c mux in the path to sensor on your module 
> causing some issue when there is more i2c write traffic as we are doing 
> single stream in continuous loop. Also IMX219 does not show on your side 
> so something specific to IMX274 setup probably.

I'll take a closer look next week. Good to know that it works fine for you.

> 
> 
> Regarding kthread_run failure where kthread_run() returned -EINTR during 
> capture start thread, I always see this happen at the point of stopping 
> the continuous single stream while loop by pressing ctrl+c after few 
> loops of execution.

Hmm, if this is normal behavior, then should this message be a debug message
only? Or perhaps only show the message if the error code != EINTR.

Regards,

	Hans

> 
> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
> 
> when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not 
> terminate immediately and probably SIGKILLed  is seen prior to complete.
> 
> Using below can help to terminate loop as well when we stop ctrl-c and 
> with this I don't see any repro of EINTR error from kthread_run when run 
> in infinite loop.
> 
> while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; done
> 
> 
> 
>>>
>>> Delta between patch versions:
>>>
>>> [v3]:	Includes v2 feedback
>>> 	- Uses separate helper function for retrieving remote csi subdevice
>>> 	  and source subdevice.
>>> 	- Added check for presence of subdevice ops set/get_selection
>>> 	- dropped vb2_queue_release from driver and using
>>> 	  vb2_video_unregister_device instead of video_unregister_device.
>>> 	- video device register should happen in the last after all video
>>> 	  device related setup is done in the driver. This is being addressed
>>> 	  in below RFC patch. Once proper implementation of this is available
>>> 	  will update Tegra video driver to use split APIs and do all setup
>>> 	  prior to device register. Added this as TODO in the driver.
>>> 	  https://www.spinics.net/lists/linux-media/msg172761.html
>>>
>>> 	Note:
>>> 	Patch-0012 has compilation dependency on
>>> 	https://patchwork.kernel.org/patch/11659521/
>>>
>>>
>>> [v2]:	Includes below changes based on v1 feedback
>>> 	- dt-binding document and the driver update for device graph to use
>>> 	  separate ports for sink endpoint and source endpoint for csi.
>>> 	- Use data-lanes endpoint property for csi.
>>> 	- Update tegra_mipi_request() to take device node pointer argument
>>> 	  rather than adding extra API.
>>> 	- Remove checking for clk pointer before clk_disable.
>>>
>>>
>>> Sowjanya Komatineni (18):
>>>    dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>>      power-domains
>>>    arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C
>>>    i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>>    i2c: tegra: Remove NULL pointer check before
>>>      clk_enable/disable/prepare/unprepare
>>>    i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>>    i2c: tegra: Fix runtime resume to re-init VI I2C
>>>    i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>>    media: tegra-video: Fix channel format alignment
>>>    media: tegra-video: Enable TPG based on kernel config
>>>    media: tegra-video: Update format lookup to offset based
>>>    dt-bindings: tegra: Update VI and CSI bindings with port info
>>>    media: tegra-video: Add support for external sensor capture
>>>    media: tegra-video: Add support for selection ioctl ops
>>>    gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>>    gpu: host1x: mipi: Use readl_relaxed_poll_timeout in tegra_mipi_wait
>>>    gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
>>>    media: tegra-video: Add CSI MIPI pads calibration
>>>    media: tegra-video: Compute settle times based on the clock rate
>>>
>>>   .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
>>>   .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
>>>   arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
>>>   drivers/gpu/drm/tegra/dsi.c                        |   9 +-
>>>   drivers/gpu/host1x/mipi.c                          |  37 +-
>>>   drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>>>   drivers/staging/media/tegra-video/Kconfig          |   7 +
>>>   drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>>>   drivers/staging/media/tegra-video/csi.h            |   8 +
>>>   drivers/staging/media/tegra-video/tegra210.c       |  25 +-
>>>   drivers/staging/media/tegra-video/vi.c             | 793 +++++++++++++++++++--
>>>   drivers/staging/media/tegra-video/vi.h             |  25 +-
>>>   drivers/staging/media/tegra-video/video.c          |  23 +-
>>>   include/linux/host1x.h                             |   4 +-
>>>   14 files changed, 1242 insertions(+), 154 deletions(-)
>>>
  
Sowjanya Komatineni July 17, 2020, 5:23 p.m. UTC | #4
On 7/17/20 10:08 AM, Hans Verkuil wrote:
> On 17/07/2020 18:34, Sowjanya Komatineni wrote:
>> On 7/17/20 3:54 AM, Hans Verkuil wrote:
>>> Hi Sowjanya,
>>>
>>> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>>>> This series adds support for video capture from external camera sensor to
>>>> Tegra video driver.
>>>>
>>>> Jetson TX1 has camera expansion connector and supports custom camera module
>>>> designed as per TX1 design specification.
>>>>
>>>> This series also enables camera capture support for Jetson Nano which has
>>>> Raspberry PI camera header.
>>>>
>>>> This series is tested with IMX219 camera sensor.
>>>>
>>>> This series include,
>>>>
>>>> VI I2C related fixes
>>>> - Camera sensor programming happens through VI I2C which is on host1x bus.
>>>> - These patches includes device tree and I2C driver fixes for VI I2C.
>>>>
>>>> Tegra video driver updates
>>>> - TPG Vs Non-TPG based on Kconfig
>>>> - Support for external sensor video capture based on device graph from DT.
>>>> - Support for selection ioctl operations
>>>> - Tegra MIPI CSI pads calibration
>>>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>>>
>>>> Host1x driver updates
>>>> - Adds API to allow creating mipi device for specific device node.
>>>> - Splits MIPI pads calibrate start and waiting for calibration to be done.
>>>>
>>>> Device tree updates
>>>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson TX1 DT.
>>>> - Enabled VI and CSI support in Jetson Nano DT.
>>> I'm doing a bit of stress testing with:
>>>
>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>
>>> and I see that the imx274 has often streaming failures:
>>>
>>> [  172.025144] IMX274 8-001a: s_stream failed
>>> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3132 = 870 (2 bytes)
>>> [  179.033575] IMX274 8-001a: s_stream failed
>>> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3130 = 878 (2 bytes)
>>> [  226.533761] IMX274 8-001a: s_stream failed
>>> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>>> [  227.037758] IMX274 8-001a: s_stream failed
>>> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>>> [  247.033658] IMX274 8-001a: s_stream failed
>>> [  293.025517] IMX274 8-001a: s_stream failed
>>> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30e0 = 0 (2 bytes)
>>> [  309.032969] IMX274 8-001a: s_stream failed
>>> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 11d2 (3 bytes)
>>> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
>>> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
>>> [  309.550243] IMX274 8-001a: s_stream failed
>>> [  314.025561] IMX274 8-001a: s_stream failed
>>> [  329.025586] IMX274 8-001a: s_stream failed
>>> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 303a = f0c (2 bytes)
>>> [  340.538009] IMX274 8-001a: s_stream failed
>>> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 30f6 = 107 (2 bytes)
>>> [  347.534008] IMX274 8-001a: s_stream failed
>>> [  365.033640] IMX274 8-001a: s_stream failed
>>> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write failed, 3038 = c (2 bytes)
>>> [  437.533997] IMX274 8-001a: s_stream failed
>>> [  456.029780] IMX274 8-001a: s_stream failed
>>> [  472.025862] IMX274 8-001a: s_stream failed
>>> [  498.025861] IMX274 8-001a: s_stream failed
>>> [  500.025905] IMX274 8-001a: s_stream failed
>>>
>>> where v4l2-ctl returns:
>>>
>>>                   VIDIOC_STREAMON returned -1 (Remote I/O error)
>>>
>>> I don't see this with the imx219.
>>>
>>> I also see this occasionally:
>>>
>>> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run capture start kthread: -4
>>>
>>> Something is not stable here.
>>>
>>> Regards,
>>>
>>> 	Hans
>> Hi Hans,
>>
>> Running the same single frame continuous loop for more than 2 hours now
>> and I don't see any failure.
>>
>> Above failure shows i2c bulk writes to IMX274 failure due to which
>> s_stream also failed.
>>
>> Not sure if its due to i2c mux in the path to sensor on your module
>> causing some issue when there is more i2c write traffic as we are doing
>> single stream in continuous loop. Also IMX219 does not show on your side
>> so something specific to IMX274 setup probably.
> I'll take a closer look next week. Good to know that it works fine for you.
>
>>
>> Regarding kthread_run failure where kthread_run() returned -EINTR during
>> capture start thread, I always see this happen at the point of stopping
>> the continuous single stream while loop by pressing ctrl+c after few
>> loops of execution.
> Hmm, if this is normal behavior, then should this message be a debug message
> only? Or perhaps only show the message if the error code != EINTR.

I believe its good to still show this as its reported by kthread_run -> 
kthread_create_on_node.

But not sure in real usecase we will ever use while true like this and 
we should use script to also break while loop along with v4l2-ctl 
termination when ctrl-c terminate request happens.


>
> Regards,
>
> 	Hans
>
>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>
>> when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not
>> terminate immediately and probably SIGKILLed  is seen prior to complete.
>>
>> Using below can help to terminate loop as well when we stop ctrl-c and
>> with this I don't see any repro of EINTR error from kthread_run when run
>> in infinite loop.
>>
>> while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; done
>>
>>
>>
>>>> Delta between patch versions:
>>>>
>>>> [v3]:	Includes v2 feedback
>>>> 	- Uses separate helper function for retrieving remote csi subdevice
>>>> 	  and source subdevice.
>>>> 	- Added check for presence of subdevice ops set/get_selection
>>>> 	- dropped vb2_queue_release from driver and using
>>>> 	  vb2_video_unregister_device instead of video_unregister_device.
>>>> 	- video device register should happen in the last after all video
>>>> 	  device related setup is done in the driver. This is being addressed
>>>> 	  in below RFC patch. Once proper implementation of this is available
>>>> 	  will update Tegra video driver to use split APIs and do all setup
>>>> 	  prior to device register. Added this as TODO in the driver.
>>>> 	  https://www.spinics.net/lists/linux-media/msg172761.html
>>>>
>>>> 	Note:
>>>> 	Patch-0012 has compilation dependency on
>>>> 	https://patchwork.kernel.org/patch/11659521/
>>>>
>>>>
>>>> [v2]:	Includes below changes based on v1 feedback
>>>> 	- dt-binding document and the driver update for device graph to use
>>>> 	  separate ports for sink endpoint and source endpoint for csi.
>>>> 	- Use data-lanes endpoint property for csi.
>>>> 	- Update tegra_mipi_request() to take device node pointer argument
>>>> 	  rather than adding extra API.
>>>> 	- Remove checking for clk pointer before clk_disable.
>>>>
>>>>
>>>> Sowjanya Komatineni (18):
>>>>     dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>>>       power-domains
>>>>     arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C
>>>>     i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>>>     i2c: tegra: Remove NULL pointer check before
>>>>       clk_enable/disable/prepare/unprepare
>>>>     i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>>>     i2c: tegra: Fix runtime resume to re-init VI I2C
>>>>     i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>>>     media: tegra-video: Fix channel format alignment
>>>>     media: tegra-video: Enable TPG based on kernel config
>>>>     media: tegra-video: Update format lookup to offset based
>>>>     dt-bindings: tegra: Update VI and CSI bindings with port info
>>>>     media: tegra-video: Add support for external sensor capture
>>>>     media: tegra-video: Add support for selection ioctl ops
>>>>     gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>>>     gpu: host1x: mipi: Use readl_relaxed_poll_timeout in tegra_mipi_wait
>>>>     gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
>>>>     media: tegra-video: Add CSI MIPI pads calibration
>>>>     media: tegra-video: Compute settle times based on the clock rate
>>>>
>>>>    .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
>>>>    .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
>>>>    arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
>>>>    drivers/gpu/drm/tegra/dsi.c                        |   9 +-
>>>>    drivers/gpu/host1x/mipi.c                          |  37 +-
>>>>    drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>>>>    drivers/staging/media/tegra-video/Kconfig          |   7 +
>>>>    drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>>>>    drivers/staging/media/tegra-video/csi.h            |   8 +
>>>>    drivers/staging/media/tegra-video/tegra210.c       |  25 +-
>>>>    drivers/staging/media/tegra-video/vi.c             | 793 +++++++++++++++++++--
>>>>    drivers/staging/media/tegra-video/vi.h             |  25 +-
>>>>    drivers/staging/media/tegra-video/video.c          |  23 +-
>>>>    include/linux/host1x.h                             |   4 +-
>>>>    14 files changed, 1242 insertions(+), 154 deletions(-)
>>>>
  
Sowjanya Komatineni July 17, 2020, 5:38 p.m. UTC | #5
On 7/17/20 10:23 AM, Sowjanya Komatineni wrote:
>
> On 7/17/20 10:08 AM, Hans Verkuil wrote:
>> On 17/07/2020 18:34, Sowjanya Komatineni wrote:
>>> On 7/17/20 3:54 AM, Hans Verkuil wrote:
>>>> Hi Sowjanya,
>>>>
>>>> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>>>>> This series adds support for video capture from external camera 
>>>>> sensor to
>>>>> Tegra video driver.
>>>>>
>>>>> Jetson TX1 has camera expansion connector and supports custom 
>>>>> camera module
>>>>> designed as per TX1 design specification.
>>>>>
>>>>> This series also enables camera capture support for Jetson Nano 
>>>>> which has
>>>>> Raspberry PI camera header.
>>>>>
>>>>> This series is tested with IMX219 camera sensor.
>>>>>
>>>>> This series include,
>>>>>
>>>>> VI I2C related fixes
>>>>> - Camera sensor programming happens through VI I2C which is on 
>>>>> host1x bus.
>>>>> - These patches includes device tree and I2C driver fixes for VI I2C.
>>>>>
>>>>> Tegra video driver updates
>>>>> - TPG Vs Non-TPG based on Kconfig
>>>>> - Support for external sensor video capture based on device graph 
>>>>> from DT.
>>>>> - Support for selection ioctl operations
>>>>> - Tegra MIPI CSI pads calibration
>>>>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>>>>
>>>>> Host1x driver updates
>>>>> - Adds API to allow creating mipi device for specific device node.
>>>>> - Splits MIPI pads calibrate start and waiting for calibration to 
>>>>> be done.
>>>>>
>>>>> Device tree updates
>>>>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to Jetson 
>>>>> TX1 DT.
>>>>> - Enabled VI and CSI support in Jetson Nano DT.
>>>> I'm doing a bit of stress testing with:
>>>>
>>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>>
>>>> and I see that the imx274 has often streaming failures:
>>>>
>>>> [  172.025144] IMX274 8-001a: s_stream failed
>>>> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 3132 = 870 (2 bytes)
>>>> [  179.033575] IMX274 8-001a: s_stream failed
>>>> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 3130 = 878 (2 bytes)
>>>> [  226.533761] IMX274 8-001a: s_stream failed
>>>> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 30f6 = 107 (2 bytes)
>>>> [  227.037758] IMX274 8-001a: s_stream failed
>>>> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 30f6 = 107 (2 bytes)
>>>> [  247.033658] IMX274 8-001a: s_stream failed
>>>> [  293.025517] IMX274 8-001a: s_stream failed
>>>> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 30e0 = 0 (2 bytes)
>>>> [  309.032969] IMX274 8-001a: s_stream failed
>>>> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 30f8 = 11d2 (3 bytes)
>>>> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
>>>> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
>>>> [  309.550243] IMX274 8-001a: s_stream failed
>>>> [  314.025561] IMX274 8-001a: s_stream failed
>>>> [  329.025586] IMX274 8-001a: s_stream failed
>>>> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 303a = f0c (2 bytes)
>>>> [  340.538009] IMX274 8-001a: s_stream failed
>>>> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 30f6 = 107 (2 bytes)
>>>> [  347.534008] IMX274 8-001a: s_stream failed
>>>> [  365.033640] IMX274 8-001a: s_stream failed
>>>> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>> failed, 3038 = c (2 bytes)
>>>> [  437.533997] IMX274 8-001a: s_stream failed
>>>> [  456.029780] IMX274 8-001a: s_stream failed
>>>> [  472.025862] IMX274 8-001a: s_stream failed
>>>> [  498.025861] IMX274 8-001a: s_stream failed
>>>> [  500.025905] IMX274 8-001a: s_stream failed
>>>>
>>>> where v4l2-ctl returns:
>>>>
>>>>                   VIDIOC_STREAMON returned -1 (Remote I/O error)
>>>>
>>>> I don't see this with the imx219.
>>>>
>>>> I also see this occasionally:
>>>>
>>>> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run 
>>>> capture start kthread: -4
>>>>
>>>> Something is not stable here.
>>>>
>>>> Regards,
>>>>
>>>>     Hans
>>> Hi Hans,
>>>
>>> Running the same single frame continuous loop for more than 2 hours now
>>> and I don't see any failure.
>>>
>>> Above failure shows i2c bulk writes to IMX274 failure due to which
>>> s_stream also failed.
>>>
>>> Not sure if its due to i2c mux in the path to sensor on your module
>>> causing some issue when there is more i2c write traffic as we are doing
>>> single stream in continuous loop. Also IMX219 does not show on your 
>>> side
>>> so something specific to IMX274 setup probably.
>> I'll take a closer look next week. Good to know that it works fine 
>> for you.
>>
>>>
>>> Regarding kthread_run failure where kthread_run() returned -EINTR 
>>> during
>>> capture start thread, I always see this happen at the point of stopping
>>> the continuous single stream while loop by pressing ctrl+c after few
>>> loops of execution.
>> Hmm, if this is normal behavior, then should this message be a debug 
>> message
>> only? Or perhaps only show the message if the error code != EINTR.
>
> I believe its good to still show this as its reported by kthread_run 
> -> kthread_create_on_node.
>
> But not sure in real usecase we will ever use while true like this and 
> we should use script to also break while loop along with v4l2-ctl 
> termination when ctrl-c terminate request happens.
>
Hi Hans, As this happens only during this type of case, I can update to 
show message only when error code != EINTR.

Thanks

Sowjanya

>
>>
>> Regards,
>>
>>     Hans
>>
>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>
>>> when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not
>>> terminate immediately and probably SIGKILLed  is seen prior to 
>>> complete.
>>>
>>> Using below can help to terminate loop as well when we stop ctrl-c and
>>> with this I don't see any repro of EINTR error from kthread_run when 
>>> run
>>> in infinite loop.
>>>
>>> while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; done
>>>
>>>
>>>
>>>>> Delta between patch versions:
>>>>>
>>>>> [v3]:    Includes v2 feedback
>>>>>     - Uses separate helper function for retrieving remote csi 
>>>>> subdevice
>>>>>       and source subdevice.
>>>>>     - Added check for presence of subdevice ops set/get_selection
>>>>>     - dropped vb2_queue_release from driver and using
>>>>>       vb2_video_unregister_device instead of video_unregister_device.
>>>>>     - video device register should happen in the last after all video
>>>>>       device related setup is done in the driver. This is being 
>>>>> addressed
>>>>>       in below RFC patch. Once proper implementation of this is 
>>>>> available
>>>>>       will update Tegra video driver to use split APIs and do all 
>>>>> setup
>>>>>       prior to device register. Added this as TODO in the driver.
>>>>> https://www.spinics.net/lists/linux-media/msg172761.html
>>>>>
>>>>>     Note:
>>>>>     Patch-0012 has compilation dependency on
>>>>>     https://patchwork.kernel.org/patch/11659521/
>>>>>
>>>>>
>>>>> [v2]:    Includes below changes based on v1 feedback
>>>>>     - dt-binding document and the driver update for device graph 
>>>>> to use
>>>>>       separate ports for sink endpoint and source endpoint for csi.
>>>>>     - Use data-lanes endpoint property for csi.
>>>>>     - Update tegra_mipi_request() to take device node pointer 
>>>>> argument
>>>>>       rather than adding extra API.
>>>>>     - Remove checking for clk pointer before clk_disable.
>>>>>
>>>>>
>>>>> Sowjanya Komatineni (18):
>>>>>     dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>>>>       power-domains
>>>>>     arm64: tegra: Add missing clocks and power-domains to Tegra210 
>>>>> VI I2C
>>>>>     i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>>>>     i2c: tegra: Remove NULL pointer check before
>>>>>       clk_enable/disable/prepare/unprepare
>>>>>     i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>>>>     i2c: tegra: Fix runtime resume to re-init VI I2C
>>>>>     i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>>>>     media: tegra-video: Fix channel format alignment
>>>>>     media: tegra-video: Enable TPG based on kernel config
>>>>>     media: tegra-video: Update format lookup to offset based
>>>>>     dt-bindings: tegra: Update VI and CSI bindings with port info
>>>>>     media: tegra-video: Add support for external sensor capture
>>>>>     media: tegra-video: Add support for selection ioctl ops
>>>>>     gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>>>>     gpu: host1x: mipi: Use readl_relaxed_poll_timeout in 
>>>>> tegra_mipi_wait
>>>>>     gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait
>>>>>     media: tegra-video: Add CSI MIPI pads calibration
>>>>>     media: tegra-video: Compute settle times based on the clock rate
>>>>>
>>>>>    .../display/tegra/nvidia,tegra20-host1x.txt        | 92 ++-
>>>>>    .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 19 +-
>>>>>    arch/arm64/boot/dts/nvidia/tegra210.dtsi           | 6 +
>>>>>    drivers/gpu/drm/tegra/dsi.c                        | 9 +-
>>>>>    drivers/gpu/host1x/mipi.c                          | 37 +-
>>>>>    drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>>>>>    drivers/staging/media/tegra-video/Kconfig          | 7 +
>>>>>    drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>>>>>    drivers/staging/media/tegra-video/csi.h            | 8 +
>>>>>    drivers/staging/media/tegra-video/tegra210.c       | 25 +-
>>>>>    drivers/staging/media/tegra-video/vi.c             | 793 
>>>>> +++++++++++++++++++--
>>>>>    drivers/staging/media/tegra-video/vi.h             | 25 +-
>>>>>    drivers/staging/media/tegra-video/video.c          | 23 +-
>>>>>    include/linux/host1x.h                             | 4 +-
>>>>>    14 files changed, 1242 insertions(+), 154 deletions(-)
>>>>>
  
Sowjanya Komatineni July 17, 2020, 5:48 p.m. UTC | #6
On 7/17/20 10:38 AM, Sowjanya Komatineni wrote:
>
> On 7/17/20 10:23 AM, Sowjanya Komatineni wrote:
>>
>> On 7/17/20 10:08 AM, Hans Verkuil wrote:
>>> On 17/07/2020 18:34, Sowjanya Komatineni wrote:
>>>> On 7/17/20 3:54 AM, Hans Verkuil wrote:
>>>>> Hi Sowjanya,
>>>>>
>>>>> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>>>>>> This series adds support for video capture from external camera 
>>>>>> sensor to
>>>>>> Tegra video driver.
>>>>>>
>>>>>> Jetson TX1 has camera expansion connector and supports custom 
>>>>>> camera module
>>>>>> designed as per TX1 design specification.
>>>>>>
>>>>>> This series also enables camera capture support for Jetson Nano 
>>>>>> which has
>>>>>> Raspberry PI camera header.
>>>>>>
>>>>>> This series is tested with IMX219 camera sensor.
>>>>>>
>>>>>> This series include,
>>>>>>
>>>>>> VI I2C related fixes
>>>>>> - Camera sensor programming happens through VI I2C which is on 
>>>>>> host1x bus.
>>>>>> - These patches includes device tree and I2C driver fixes for VI 
>>>>>> I2C.
>>>>>>
>>>>>> Tegra video driver updates
>>>>>> - TPG Vs Non-TPG based on Kconfig
>>>>>> - Support for external sensor video capture based on device graph 
>>>>>> from DT.
>>>>>> - Support for selection ioctl operations
>>>>>> - Tegra MIPI CSI pads calibration
>>>>>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>>>>>
>>>>>> Host1x driver updates
>>>>>> - Adds API to allow creating mipi device for specific device node.
>>>>>> - Splits MIPI pads calibrate start and waiting for calibration to 
>>>>>> be done.
>>>>>>
>>>>>> Device tree updates
>>>>>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to 
>>>>>> Jetson TX1 DT.
>>>>>> - Enabled VI and CSI support in Jetson Nano DT.
>>>>> I'm doing a bit of stress testing with:
>>>>>
>>>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>>>
>>>>> and I see that the imx274 has often streaming failures:
>>>>>
>>>>> [  172.025144] IMX274 8-001a: s_stream failed
>>>>> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 3132 = 870 (2 bytes)
>>>>> [  179.033575] IMX274 8-001a: s_stream failed
>>>>> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 3130 = 878 (2 bytes)
>>>>> [  226.533761] IMX274 8-001a: s_stream failed
>>>>> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 30f6 = 107 (2 bytes)
>>>>> [  227.037758] IMX274 8-001a: s_stream failed
>>>>> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 30f6 = 107 (2 bytes)
>>>>> [  247.033658] IMX274 8-001a: s_stream failed
>>>>> [  293.025517] IMX274 8-001a: s_stream failed
>>>>> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 30e0 = 0 (2 bytes)
>>>>> [  309.032969] IMX274 8-001a: s_stream failed
>>>>> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 30f8 = 11d2 (3 bytes)
>>>>> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
>>>>> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
>>>>> [  309.550243] IMX274 8-001a: s_stream failed
>>>>> [  314.025561] IMX274 8-001a: s_stream failed
>>>>> [  329.025586] IMX274 8-001a: s_stream failed
>>>>> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 303a = f0c (2 bytes)
>>>>> [  340.538009] IMX274 8-001a: s_stream failed
>>>>> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 30f6 = 107 (2 bytes)
>>>>> [  347.534008] IMX274 8-001a: s_stream failed
>>>>> [  365.033640] IMX274 8-001a: s_stream failed
>>>>> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>> failed, 3038 = c (2 bytes)
>>>>> [  437.533997] IMX274 8-001a: s_stream failed
>>>>> [  456.029780] IMX274 8-001a: s_stream failed
>>>>> [  472.025862] IMX274 8-001a: s_stream failed
>>>>> [  498.025861] IMX274 8-001a: s_stream failed
>>>>> [  500.025905] IMX274 8-001a: s_stream failed
>>>>>
>>>>> where v4l2-ctl returns:
>>>>>
>>>>>                   VIDIOC_STREAMON returned -1 (Remote I/O error)
>>>>>
>>>>> I don't see this with the imx219.
>>>>>
>>>>> I also see this occasionally:
>>>>>
>>>>> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run 
>>>>> capture start kthread: -4
>>>>>
>>>>> Something is not stable here.
>>>>>
>>>>> Regards,
>>>>>
>>>>>     Hans
>>>> Hi Hans,
>>>>
>>>> Running the same single frame continuous loop for more than 2 hours 
>>>> now
>>>> and I don't see any failure.
>>>>
>>>> Above failure shows i2c bulk writes to IMX274 failure due to which
>>>> s_stream also failed.
>>>>
>>>> Not sure if its due to i2c mux in the path to sensor on your module
>>>> causing some issue when there is more i2c write traffic as we are 
>>>> doing
>>>> single stream in continuous loop. Also IMX219 does not show on your 
>>>> side
>>>> so something specific to IMX274 setup probably.
>>> I'll take a closer look next week. Good to know that it works fine 
>>> for you.
>>>
>>>>
>>>> Regarding kthread_run failure where kthread_run() returned -EINTR 
>>>> during
>>>> capture start thread, I always see this happen at the point of 
>>>> stopping
>>>> the continuous single stream while loop by pressing ctrl+c after few
>>>> loops of execution.
>>> Hmm, if this is normal behavior, then should this message be a debug 
>>> message
>>> only? Or perhaps only show the message if the error code != EINTR.
>>
>> I believe its good to still show this as its reported by kthread_run 
>> -> kthread_create_on_node.
>>
>> But not sure in real usecase we will ever use while true like this 
>> and we should use script to also break while loop along with v4l2-ctl 
>> termination when ctrl-c terminate request happens.
>>
> Hi Hans, As this happens only during this type of case, I can update 
> to show message only when error code != EINTR.
>
> Thanks
>
> Sowjanya


Sorry, Was thinking to not mask debug message for -EINTR in case if it 
happens in any other valid scenarios. If you still want to mask, will 
update in next version.

>
>>
>>>
>>> Regards,
>>>
>>>     Hans
>>>
>>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>>
>>>> when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not
>>>> terminate immediately and probably SIGKILLed  is seen prior to 
>>>> complete.
>>>>
>>>> Using below can help to terminate loop as well when we stop ctrl-c and
>>>> with this I don't see any repro of EINTR error from kthread_run 
>>>> when run
>>>> in infinite loop.
>>>>
>>>> while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; 
>>>> done
>>>>
>>>>
>>>>
>>>>>> Delta between patch versions:
>>>>>>
>>>>>> [v3]:    Includes v2 feedback
>>>>>>     - Uses separate helper function for retrieving remote csi 
>>>>>> subdevice
>>>>>>       and source subdevice.
>>>>>>     - Added check for presence of subdevice ops set/get_selection
>>>>>>     - dropped vb2_queue_release from driver and using
>>>>>>       vb2_video_unregister_device instead of 
>>>>>> video_unregister_device.
>>>>>>     - video device register should happen in the last after all 
>>>>>> video
>>>>>>       device related setup is done in the driver. This is being 
>>>>>> addressed
>>>>>>       in below RFC patch. Once proper implementation of this is 
>>>>>> available
>>>>>>       will update Tegra video driver to use split APIs and do all 
>>>>>> setup
>>>>>>       prior to device register. Added this as TODO in the driver.
>>>>>> https://www.spinics.net/lists/linux-media/msg172761.html
>>>>>>
>>>>>>     Note:
>>>>>>     Patch-0012 has compilation dependency on
>>>>>>     https://patchwork.kernel.org/patch/11659521/
>>>>>>
>>>>>>
>>>>>> [v2]:    Includes below changes based on v1 feedback
>>>>>>     - dt-binding document and the driver update for device graph 
>>>>>> to use
>>>>>>       separate ports for sink endpoint and source endpoint for csi.
>>>>>>     - Use data-lanes endpoint property for csi.
>>>>>>     - Update tegra_mipi_request() to take device node pointer 
>>>>>> argument
>>>>>>       rather than adding extra API.
>>>>>>     - Remove checking for clk pointer before clk_disable.
>>>>>>
>>>>>>
>>>>>> Sowjanya Komatineni (18):
>>>>>>     dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>>>>>       power-domains
>>>>>>     arm64: tegra: Add missing clocks and power-domains to 
>>>>>> Tegra210 VI I2C
>>>>>>     i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>>>>>     i2c: tegra: Remove NULL pointer check before
>>>>>>       clk_enable/disable/prepare/unprepare
>>>>>>     i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>>>>>     i2c: tegra: Fix runtime resume to re-init VI I2C
>>>>>>     i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>>>>>     media: tegra-video: Fix channel format alignment
>>>>>>     media: tegra-video: Enable TPG based on kernel config
>>>>>>     media: tegra-video: Update format lookup to offset based
>>>>>>     dt-bindings: tegra: Update VI and CSI bindings with port info
>>>>>>     media: tegra-video: Add support for external sensor capture
>>>>>>     media: tegra-video: Add support for selection ioctl ops
>>>>>>     gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>>>>>     gpu: host1x: mipi: Use readl_relaxed_poll_timeout in 
>>>>>> tegra_mipi_wait
>>>>>>     gpu: host1x: mipi: Split tegra_mipi_calibrate and 
>>>>>> tegra_mipi_wait
>>>>>>     media: tegra-video: Add CSI MIPI pads calibration
>>>>>>     media: tegra-video: Compute settle times based on the clock rate
>>>>>>
>>>>>>    .../display/tegra/nvidia,tegra20-host1x.txt        | 92 ++-
>>>>>>    .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 19 +-
>>>>>>    arch/arm64/boot/dts/nvidia/tegra210.dtsi           | 6 +
>>>>>>    drivers/gpu/drm/tegra/dsi.c                        | 9 +-
>>>>>>    drivers/gpu/host1x/mipi.c                          | 37 +-
>>>>>>    drivers/i2c/busses/i2c-tegra.c                     | 101 +--
>>>>>>    drivers/staging/media/tegra-video/Kconfig          | 7 +
>>>>>>    drivers/staging/media/tegra-video/csi.c            | 247 ++++++-
>>>>>>    drivers/staging/media/tegra-video/csi.h            | 8 +
>>>>>>    drivers/staging/media/tegra-video/tegra210.c       | 25 +-
>>>>>>    drivers/staging/media/tegra-video/vi.c             | 793 
>>>>>> +++++++++++++++++++--
>>>>>>    drivers/staging/media/tegra-video/vi.h             | 25 +-
>>>>>>    drivers/staging/media/tegra-video/video.c          | 23 +-
>>>>>>    include/linux/host1x.h                             | 4 +-
>>>>>>    14 files changed, 1242 insertions(+), 154 deletions(-)
>>>>>>
  
Sowjanya Komatineni July 20, 2020, 4:41 a.m. UTC | #7
On 7/17/20 10:48 AM, Sowjanya Komatineni wrote:
>
> On 7/17/20 10:38 AM, Sowjanya Komatineni wrote:
>>
>> On 7/17/20 10:23 AM, Sowjanya Komatineni wrote:
>>>
>>> On 7/17/20 10:08 AM, Hans Verkuil wrote:
>>>> On 17/07/2020 18:34, Sowjanya Komatineni wrote:
>>>>> On 7/17/20 3:54 AM, Hans Verkuil wrote:
>>>>>> Hi Sowjanya,
>>>>>>
>>>>>> On 15/07/2020 06:20, Sowjanya Komatineni wrote:
>>>>>>> This series adds support for video capture from external camera 
>>>>>>> sensor to
>>>>>>> Tegra video driver.
>>>>>>>
>>>>>>> Jetson TX1 has camera expansion connector and supports custom 
>>>>>>> camera module
>>>>>>> designed as per TX1 design specification.
>>>>>>>
>>>>>>> This series also enables camera capture support for Jetson Nano 
>>>>>>> which has
>>>>>>> Raspberry PI camera header.
>>>>>>>
>>>>>>> This series is tested with IMX219 camera sensor.
>>>>>>>
>>>>>>> This series include,
>>>>>>>
>>>>>>> VI I2C related fixes
>>>>>>> - Camera sensor programming happens through VI I2C which is on 
>>>>>>> host1x bus.
>>>>>>> - These patches includes device tree and I2C driver fixes for VI 
>>>>>>> I2C.
>>>>>>>
>>>>>>> Tegra video driver updates
>>>>>>> - TPG Vs Non-TPG based on Kconfig
>>>>>>> - Support for external sensor video capture based on device 
>>>>>>> graph from DT.
>>>>>>> - Support for selection ioctl operations
>>>>>>> - Tegra MIPI CSI pads calibration
>>>>>>> - CSI T-CLK and T-HS settle time computation based on clock rates.
>>>>>>>
>>>>>>> Host1x driver updates
>>>>>>> - Adds API to allow creating mipi device for specific device node.
>>>>>>> - Splits MIPI pads calibrate start and waiting for calibration 
>>>>>>> to be done.
>>>>>>>
>>>>>>> Device tree updates
>>>>>>> - Adds camera connector 2V8, 1V8, 1V2 regulator supplies to 
>>>>>>> Jetson TX1 DT.
>>>>>>> - Enabled VI and CSI support in Jetson Nano DT.
>>>>>> I'm doing a bit of stress testing with:
>>>>>>
>>>>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>>>>
>>>>>> and I see that the imx274 has often streaming failures:
>>>>>>
>>>>>> [  172.025144] IMX274 8-001a: s_stream failed
>>>>>> [  179.025192] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 3132 = 870 (2 bytes)
>>>>>> [  179.033575] IMX274 8-001a: s_stream failed
>>>>>> [  226.525378] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 3130 = 878 (2 bytes)
>>>>>> [  226.533761] IMX274 8-001a: s_stream failed
>>>>>> [  227.029325] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 30f6 = 107 (2 bytes)
>>>>>> [  227.037758] IMX274 8-001a: s_stream failed
>>>>>> [  247.025218] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 30f6 = 107 (2 bytes)
>>>>>> [  247.033658] IMX274 8-001a: s_stream failed
>>>>>> [  293.025517] IMX274 8-001a: s_stream failed
>>>>>> [  309.024727] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 30e0 = 0 (2 bytes)
>>>>>> [  309.032969] IMX274 8-001a: s_stream failed
>>>>>> [  309.529506] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 30f8 = 11d2 (3 bytes)
>>>>>> [  309.538103] IMX274 8-001a: imx274_set_frame_length error = -121
>>>>>> [  309.544102] IMX274 8-001a: imx274_set_frame_interval error = -121
>>>>>> [  309.550243] IMX274 8-001a: s_stream failed
>>>>>> [  314.025561] IMX274 8-001a: s_stream failed
>>>>>> [  329.025586] IMX274 8-001a: s_stream failed
>>>>>> [  340.529567] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 303a = f0c (2 bytes)
>>>>>> [  340.538009] IMX274 8-001a: s_stream failed
>>>>>> [  347.525627] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 30f6 = 107 (2 bytes)
>>>>>> [  347.534008] IMX274 8-001a: s_stream failed
>>>>>> [  365.033640] IMX274 8-001a: s_stream failed
>>>>>> [  437.525788] IMX274 8-001a: imx274_write_mbreg : i2c bulk write 
>>>>>> failed, 3038 = c (2 bytes)
>>>>>> [  437.533997] IMX274 8-001a: s_stream failed
>>>>>> [  456.029780] IMX274 8-001a: s_stream failed
>>>>>> [  472.025862] IMX274 8-001a: s_stream failed
>>>>>> [  498.025861] IMX274 8-001a: s_stream failed
>>>>>> [  500.025905] IMX274 8-001a: s_stream failed
>>>>>>
>>>>>> where v4l2-ctl returns:
>>>>>>
>>>>>>                   VIDIOC_STREAMON returned -1 (Remote I/O error)
>>>>>>
>>>>>> I don't see this with the imx219.
>>>>>>
>>>>>> I also see this occasionally:
>>>>>>
>>>>>> [Fri Jul 17 12:51:42 2020] video4linux video1: failed to run 
>>>>>> capture start kthread: -4
>>>>>>
>>>>>> Something is not stable here.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>>     Hans
>>>>> Hi Hans,
>>>>>
>>>>> Running the same single frame continuous loop for more than 2 
>>>>> hours now
>>>>> and I don't see any failure.
>>>>>
>>>>> Above failure shows i2c bulk writes to IMX274 failure due to which
>>>>> s_stream also failed.
>>>>>
>>>>> Not sure if its due to i2c mux in the path to sensor on your module
>>>>> causing some issue when there is more i2c write traffic as we are 
>>>>> doing
>>>>> single stream in continuous loop. Also IMX219 does not show on 
>>>>> your side
>>>>> so something specific to IMX274 setup probably.
>>>> I'll take a closer look next week. Good to know that it works fine 
>>>> for you.
>>>>
>>>>>
>>>>> Regarding kthread_run failure where kthread_run() returned -EINTR 
>>>>> during
>>>>> capture start thread, I always see this happen at the point of 
>>>>> stopping
>>>>> the continuous single stream while loop by pressing ctrl+c after few
>>>>> loops of execution.
>>>> Hmm, if this is normal behavior, then should this message be a 
>>>> debug message
>>>> only? Or perhaps only show the message if the error code != EINTR.
>>>
>>> I believe its good to still show this as its reported by kthread_run 
>>> -> kthread_create_on_node.
>>>
>>> But not sure in real usecase we will ever use while true like this 
>>> and we should use script to also break while loop along with 
>>> v4l2-ctl termination when ctrl-c terminate request happens.
>>>
>> Hi Hans, As this happens only during this type of case, I can update 
>> to show message only when error code != EINTR.
>>
>> Thanks
>>
>> Sowjanya
>
>
> Sorry, Was thinking to not mask debug message for -EINTR in case if it 
> happens in any other valid scenarios. If you still want to mask, will 
> update in next version.
>
As we are running application that uses threads depending on when ctrl-c 
gets hit I see sigkill happens during kthread run even with break 
causing this.

Good way I see it working all the time is to use crtl-z and then kill 
application as below

Command to run

while true; do ./v4l2-ctl --stream-mmap --stream-count=1; done

To stop ,

press ctrl-z and then execute kill %%


>>
>>>
>>>>
>>>> Regards,
>>>>
>>>>     Hans
>>>>
>>>>> while true; do v4l2-ctl --stream-mmap --stream-count=1; done
>>>>>
>>>>> when we stop loop with ctrl+c, v4l2-ctl terminates but loop does not
>>>>> terminate immediately and probably SIGKILLed  is seen prior to 
>>>>> complete.
>>>>>
>>>>> Using below can help to terminate loop as well when we stop ctrl-c 
>>>>> and
>>>>> with this I don't see any repro of EINTR error from kthread_run 
>>>>> when run
>>>>> in infinite loop.
>>>>>
>>>>> while true; do ./v4l2-ctl --stream-mmap --stream-count=1 || break; 
>>>>> done
>>>>>
>>>>>
>>>>>
>>>>>>> Delta between patch versions:
>>>>>>>
>>>>>>> [v3]:    Includes v2 feedback
>>>>>>>     - Uses separate helper function for retrieving remote csi 
>>>>>>> subdevice
>>>>>>>       and source subdevice.
>>>>>>>     - Added check for presence of subdevice ops set/get_selection
>>>>>>>     - dropped vb2_queue_release from driver and using
>>>>>>>       vb2_video_unregister_device instead of 
>>>>>>> video_unregister_device.
>>>>>>>     - video device register should happen in the last after all 
>>>>>>> video
>>>>>>>       device related setup is done in the driver. This is being 
>>>>>>> addressed
>>>>>>>       in below RFC patch. Once proper implementation of this is 
>>>>>>> available
>>>>>>>       will update Tegra video driver to use split APIs and do 
>>>>>>> all setup
>>>>>>>       prior to device register. Added this as TODO in the driver.
>>>>>>> https://www.spinics.net/lists/linux-media/msg172761.html
>>>>>>>
>>>>>>>     Note:
>>>>>>>     Patch-0012 has compilation dependency on
>>>>>>>     https://patchwork.kernel.org/patch/11659521/
>>>>>>>
>>>>>>>
>>>>>>> [v2]:    Includes below changes based on v1 feedback
>>>>>>>     - dt-binding document and the driver update for device graph 
>>>>>>> to use
>>>>>>>       separate ports for sink endpoint and source endpoint for csi.
>>>>>>>     - Use data-lanes endpoint property for csi.
>>>>>>>     - Update tegra_mipi_request() to take device node pointer 
>>>>>>> argument
>>>>>>>       rather than adding extra API.
>>>>>>>     - Remove checking for clk pointer before clk_disable.
>>>>>>>
>>>>>>>
>>>>>>> Sowjanya Komatineni (18):
>>>>>>>     dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and
>>>>>>>       power-domains
>>>>>>>     arm64: tegra: Add missing clocks and power-domains to 
>>>>>>> Tegra210 VI I2C
>>>>>>>     i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
>>>>>>>     i2c: tegra: Remove NULL pointer check before
>>>>>>>       clk_enable/disable/prepare/unprepare
>>>>>>>     i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
>>>>>>>     i2c: tegra: Fix runtime resume to re-init VI I2C
>>>>>>>     i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c
>>>>>>>     media: tegra-video: Fix channel format alignment
>>>>>>>     media: tegra-video: Enable TPG based on kernel config
>>>>>>>     media: tegra-video: Update format lookup to offset based
>>>>>>>     dt-bindings: tegra: Update VI and CSI bindings with port info
>>>>>>>     media: tegra-video: Add support for external sensor capture
>>>>>>>     media: tegra-video: Add support for selection ioctl ops
>>>>>>>     gpu: host1x: mipi: Update tegra_mipi_request() to be node based
>>>>>>>     gpu: host1x: mipi: Use readl_relaxed_poll_timeout in 
>>>>>>> tegra_mipi_wait
>>>>>>>     gpu: host1x: mipi: Split tegra_mipi_calibrate and 
>>>>>>> tegra_mipi_wait
>>>>>>>     media: tegra-video: Add CSI MIPI pads calibration
>>>>>>>     media: tegra-video: Compute settle times based on the clock 
>>>>>>> rate
>>>>>>>
>>>>>>>    .../display/tegra/nvidia,tegra20-host1x.txt | 92 ++-
>>>>>>>    .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 19 +-
>>>>>>>    arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 +
>>>>>>>    drivers/gpu/drm/tegra/dsi.c | 9 +-
>>>>>>>    drivers/gpu/host1x/mipi.c | 37 +-
>>>>>>>    drivers/i2c/busses/i2c-tegra.c | 101 +--
>>>>>>>    drivers/staging/media/tegra-video/Kconfig | 7 +
>>>>>>>    drivers/staging/media/tegra-video/csi.c | 247 ++++++-
>>>>>>>    drivers/staging/media/tegra-video/csi.h | 8 +
>>>>>>>    drivers/staging/media/tegra-video/tegra210.c | 25 +-
>>>>>>>    drivers/staging/media/tegra-video/vi.c | 793 
>>>>>>> +++++++++++++++++++--
>>>>>>>    drivers/staging/media/tegra-video/vi.h | 25 +-
>>>>>>>    drivers/staging/media/tegra-video/video.c | 23 +-
>>>>>>>    include/linux/host1x.h | 4 +-
>>>>>>>    14 files changed, 1242 insertions(+), 154 deletions(-)
>>>>>>>