mbox series

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

Message ID 1592358094-23459-1-git-send-email-skomatineni@nvidia.com
Headers show
Series Support for Tegra video capture from external sensor | expand

Message

Sowjanya Komatineni June 17, 2020, 1:41 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:

[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: 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: 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
  arm64: tegra: jetson-tx1: Add camera supplies
  arm64: tegra: Enable Tegra VI CSI support for Jetson Nano

 .../display/tegra/nvidia,tegra20-host1x.txt        |  92 ++-
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  19 +-
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |  41 ++
 arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts |  10 +
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |   6 +
 drivers/gpu/drm/tegra/dsi.c                        |   9 +-
 drivers/gpu/host1x/mipi.c                          |  30 +-
 drivers/i2c/busses/i2c-tegra.c                     |  39 +-
 drivers/staging/media/tegra-video/Kconfig          |   7 +
 drivers/staging/media/tegra-video/csi.c            | 245 ++++++-
 drivers/staging/media/tegra-video/csi.h            |   8 +
 drivers/staging/media/tegra-video/tegra210.c       |  25 +-
 drivers/staging/media/tegra-video/vi.c             | 770 +++++++++++++++++++--
 drivers/staging/media/tegra-video/vi.h             |  23 +-
 drivers/staging/media/tegra-video/video.c          |  23 +-
 include/linux/host1x.h                             |   4 +-
 16 files changed, 1251 insertions(+), 100 deletions(-)

Comments

Sowjanya Komatineni July 3, 2020, 5:12 p.m. UTC | #1
On 7/3/20 1:06 AM, Hans Verkuil wrote:
> On 02/07/2020 23:20, Sowjanya Komatineni wrote:

>> On 7/2/20 6:54 AM, Hans Verkuil wrote:

>>> On 17/06/2020 03:41, Sowjanya Komatineni wrote:

>>>> This patch adds selection v4l2 ioctl operations to allow configuring

>>>> a selection rectangle in the sensor through the Tegra video device

>>>> node.

>>>>

>>>> Some sensor drivers supporting crop uses try_crop rectangle from

>>>> v4l2_subdev_pad_config during try format for computing binning.

>>>>

>>>> So with selection ops support, this patch also updates try format

>>>> to use try crop rectangle either from subdev frame size enumeration

>>>> or from subdev crop boundary.

>>>>

>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

>>>> ---

>>>>    drivers/staging/media/tegra-video/vi.c | 106 +++++++++++++++++++++++++++++++++

>>>>    1 file changed, 106 insertions(+)

>>>>

>>>> diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c

>>>> index 506c263..f9eb96b 100644

>>>> --- a/drivers/staging/media/tegra-video/vi.c

>>>> +++ b/drivers/staging/media/tegra-video/vi.c

>>>> @@ -427,6 +427,13 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,

>>>>    	struct v4l2_subdev *subdev;

>>>>    	struct v4l2_subdev_format fmt;

>>>>    	struct v4l2_subdev_pad_config *pad_cfg;

>>>> +	struct v4l2_subdev_frame_size_enum fse = {

>>>> +		.which = V4L2_SUBDEV_FORMAT_TRY,

>>>> +	};

>>>> +	struct v4l2_subdev_selection sdsel = {

>>>> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

>>>> +		.target = V4L2_SEL_TGT_CROP_BOUNDS,

>>>> +	};

>>>>    	int ret;

>>>>    

>>>>    	subdev = tegra_channel_get_remote_subdev(chan, true);

>>>> @@ -449,6 +456,24 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,

>>>>    	fmt.which = V4L2_SUBDEV_FORMAT_TRY;

>>>>    	fmt.pad = 0;

>>>>    	v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code);

>>>> +

>>>> +	/*

>>>> +	 * Attempt to obtain the format size from subdev.

>>>> +	 * If not available, try to get crop boundary from subdev.

>>>> +	 */

>>>> +	fse.code = fmtinfo->code;

>>>> +	ret = v4l2_subdev_call(subdev, pad, enum_frame_size, pad_cfg, &fse);

>>>> +	if (ret) {

>>>> +		ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);

>>>> +		if (ret)

>>>> +			return -EINVAL;

>>>> +		pad_cfg->try_crop.width = sdsel.r.width;

>>>> +		pad_cfg->try_crop.height = sdsel.r.height;

>>>> +	} else {

>>>> +		pad_cfg->try_crop.width = fse.max_width;

>>>> +		pad_cfg->try_crop.height = fse.max_height;

>>>> +	}

>>>> +

>>>>    	ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt);

>>>>    	if (ret < 0)

>>>>    		return ret;

>>>> @@ -540,6 +565,85 @@ static int tegra_channel_set_subdev_active_fmt(struct tegra_vi_channel *chan)

>>>>    	return 0;

>>>>    }

>>>>    

>>>> +static int tegra_channel_g_selection(struct file *file, void *priv,

>>>> +				     struct v4l2_selection *sel)

>>>> +{

>>>> +	struct tegra_vi_channel *chan = video_drvdata(file);

>>>> +	struct v4l2_subdev *subdev;

>>>> +	struct v4l2_subdev_format fmt = {

>>>> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

>>>> +	};

>>>> +	struct v4l2_subdev_selection sdsel = {

>>>> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

>>>> +		.target = sel->target,

>>>> +	};

>>>> +	int ret;

>>>> +

>>>> +	if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG))

>>>> +		return -ENOTTY;

>>>> +

>>>> +	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)

>>>> +		return -EINVAL;

>>>> +	/*

>>>> +	 * Try the get selection operation and fallback to get format if not

>>>> +	 * implemented.

>>>> +	 */

>>>> +	subdev = tegra_channel_get_remote_subdev(chan, true);

>>>> +	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);

>>>> +	if (!ret)

>>>> +		sel->r = sdsel.r;

>>>> +	if (ret != -ENOIOCTLCMD)

>>>> +		return ret;

>>>> +

>>>> +	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

>>>> +	if (ret < 0)

>>>> +		return ret;

>>>> +

>>>> +	sel->r.left = 0;

>>>> +	sel->r.top = 0;

>>>> +	sel->r.width = fmt.format.width;

>>>> +	sel->r.height = fmt.format.height;

>>>> +

>>>> +	return 0;

>>>> +}

>>>> +

>>>> +static int tegra_channel_s_selection(struct file *file, void *fh,

>>>> +				     struct v4l2_selection *sel)

>>>> +{

>>>> +	struct tegra_vi_channel *chan = video_drvdata(file);

>>>> +	struct v4l2_subdev *subdev;

>>>> +	int ret;

>>>> +	struct v4l2_subdev_selection sdsel = {

>>>> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

>>>> +		.target = sel->target,

>>>> +		.flags = sel->flags,

>>>> +		.r = sel->r,

>>>> +	};

>>>> +

>>> This function doesn't check if the subdev actually supports set_selection.

>>> The imx219 is one such driver: it supports get_selection, but not set_selection.

>>>

>>> So this code should add these lines to fix the v4l2-compliance fail:

>>>

>>>          subdev = tegra_channel_get_remote_subdev(chan, true);

>>>

>>>          if (!v4l2_subdev_has_op(subdev, pad, set_selection))

>>>                  return -ENOTTY;

>>>

>> v4l2_subdev_call() does that check and returns -ENOIOCTLCMD when

>> specified subdev ops does not exist.

> But that test happens too late. In the v4l2-compliance test it fails in the

> sel->type test below, so it returns EINVAL instead of ENOTTY.

>

>>>> +	if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG))

>>>> +		return -ENOTTY;

> I think this test should come before the v4l2_subdev_has_op test since there

> is probably no subdev if the TPG is enabled. So:

>

> 	if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG))

> 		return -ENOTTY;

>

>          subdev = tegra_channel_get_remote_subdev(chan, true);

>          if (!v4l2_subdev_has_op(subdev, pad, set_selection))

>                  return -ENOTTY;

>

>

> Regards,

>

> 	Hans

OK Will update in v3. Thanks Hans
>>>> +

>>>> +	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)

>>>> +		return -EINVAL;

>>>> +

>>>> +	if (vb2_is_busy(&chan->queue))

>>>> +		return -EBUSY;

>>>> +

>>>> +	subdev = tegra_channel_get_remote_subdev(chan, true);

>>> And this line can be dropped.

>>>

>>> Regards,

>>>

>>> 	Hans

>>>

>>>> +	ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel);

>>>> +	if (!ret) {

>>>> +		sel->r = sdsel.r;

>>>> +		/*

>>>> +		 * Subdev active format resolution may have changed during

>>>> +		 * set selection operation. So, update channel format to

>>>> +		 * the sub-device active format.

>>>> +		 */

>>>> +		return tegra_channel_set_subdev_active_fmt(chan);

>>>> +	}

>>>> +

>>>> +	return ret;

>>>> +}

>>>> +

>>>>    static int tegra_channel_enum_input(struct file *file, void *fh,

>>>>    				    struct v4l2_input *inp)

>>>>    {

>>>> @@ -597,6 +701,8 @@ static const struct v4l2_ioctl_ops tegra_channel_ioctl_ops = {

>>>>    	.vidioc_streamoff		= vb2_ioctl_streamoff,

>>>>    	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,

>>>>    	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,

>>>> +	.vidioc_g_selection		= tegra_channel_g_selection,

>>>> +	.vidioc_s_selection		= tegra_channel_s_selection,

>>>>    };

>>>>    

>>>>    /*

>>>>
Rob Herring (Arm) July 13, 2020, 11:49 p.m. UTC | #2
On Tue, 16 Jun 2020 18:41:17 -0700, Sowjanya Komatineni wrote:
> This patch documents missing clocks and power-domains of Tegra210 VI I2C.

> 

> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

> ---

>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt    | 19 +++++++++++++------

>  1 file changed, 13 insertions(+), 6 deletions(-)

> 


Reviewed-by: Rob Herring <robh@kernel.org>