| Message ID | 1596186169-18729-10-git-send-email-skomatineni@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support for Tegra video capture from external sensor | expand |
31.07.2020 12:02, Sowjanya Komatineni пишет: ... > @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) > return ret; > } > > + if (csi_chan->mipi) { > + ret = tegra_mipi_enable(csi_chan->mipi); > + if (ret < 0) { > + dev_err(csi->dev, > + "failed to enable MIPI pads: %d\n", ret); > + goto rpm_put; > + } > + > + /* > + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to > + * be calibrated after power on. > + * So, trigger the calibration start here and results will > + * be latched and applied to the pads when link is in LP11 > + * state during start of sensor streaming. > + */ > + ret = tegra_mipi_start_calibration(csi_chan->mipi); > + if (ret < 0) { > + dev_err(csi->dev, > + "failed to start MIPI calibration: %d\n", ret); > + goto disable_mipi; > + } What would happen if CSI stream is enabled and then immediately disabled without enabling camera sensor? > + } > + ... > static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) > { > struct v4l2_subdev *csi_subdev, *src_subdev; > + struct tegra_csi_channel *csi_chan; > int ret; > > /* > @@ -206,13 +207,30 @@ static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) > if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) > return 0; > > + csi_chan = v4l2_get_subdevdata(csi_subdev); > + /* > + * TRM has incorrectly documented to wait for done status from > + * calibration logic after CSI interface power on. > + * As per the design, calibration results are latched and applied > + * to the pads only when the link is in LP11 state which will happen > + * during the sensor stream-on. > + * CSI subdev stream-on triggers start of MIPI pads calibration. > + * Wait for calibration to finish here after sensor subdev stream-on > + * and in case of sensor stream-on failure, cancel the calibration. > + */ > src_subdev = tegra_channel_get_remote_source_subdev(chan); Is it possible to move the start_calibration() here? > ret = v4l2_subdev_call(src_subdev, video, s_stream, true); > if (ret < 0 && ret != -ENOIOCTLCMD) { > + tegra_mipi_cancel_calibration(csi_chan->mipi); > v4l2_subdev_call(csi_subdev, video, s_stream, false); > return ret; > } > > + ret = tegra_mipi_finish_calibration(csi_chan->mipi); > + if (ret < 0) > + dev_warn(csi_chan->csi->dev, > + "MIPI calibration failed: %d\n", ret); > + > return 0; > } > >
On 7/31/20 4:39 AM, Dmitry Osipenko wrote: > 31.07.2020 12:02, Sowjanya Komatineni пишет: > ... >> @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) >> return ret; >> } >> >> + if (csi_chan->mipi) { >> + ret = tegra_mipi_enable(csi_chan->mipi); >> + if (ret < 0) { >> + dev_err(csi->dev, >> + "failed to enable MIPI pads: %d\n", ret); >> + goto rpm_put; >> + } >> + >> + /* >> + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to >> + * be calibrated after power on. >> + * So, trigger the calibration start here and results will >> + * be latched and applied to the pads when link is in LP11 >> + * state during start of sensor streaming. >> + */ >> + ret = tegra_mipi_start_calibration(csi_chan->mipi); >> + if (ret < 0) { >> + dev_err(csi->dev, >> + "failed to start MIPI calibration: %d\n", ret); >> + goto disable_mipi; >> + } > What would happen if CSI stream is enabled and then immediately disabled > without enabling camera sensor? Nothing will happen as during stream enable csi receiver is kept ready. But actual capture will not happen during that point. > >> + } >> + > ... >> static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) >> { >> struct v4l2_subdev *csi_subdev, *src_subdev; >> + struct tegra_csi_channel *csi_chan; >> int ret; >> >> /* >> @@ -206,13 +207,30 @@ static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) >> if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) >> return 0; >> >> + csi_chan = v4l2_get_subdevdata(csi_subdev); >> + /* >> + * TRM has incorrectly documented to wait for done status from >> + * calibration logic after CSI interface power on. >> + * As per the design, calibration results are latched and applied >> + * to the pads only when the link is in LP11 state which will happen >> + * during the sensor stream-on. >> + * CSI subdev stream-on triggers start of MIPI pads calibration. >> + * Wait for calibration to finish here after sensor subdev stream-on >> + * and in case of sensor stream-on failure, cancel the calibration. >> + */ >> src_subdev = tegra_channel_get_remote_source_subdev(chan); > Is it possible to move the start_calibration() here? I think we can do start here as well I guess but currently I am following steps order as per design document. This is the reason I have updated in above comment as well. > >> ret = v4l2_subdev_call(src_subdev, video, s_stream, true); >> if (ret < 0 && ret != -ENOIOCTLCMD) { >> + tegra_mipi_cancel_calibration(csi_chan->mipi); >> v4l2_subdev_call(csi_subdev, video, s_stream, false); >> return ret; >> } >> >> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >> + if (ret < 0) >> + dev_warn(csi_chan->csi->dev, >> + "MIPI calibration failed: %d\n", ret); >> + >> return 0; >> } >> >>
31.07.2020 18:46, Sowjanya Komatineni пишет: > > On 7/31/20 4:39 AM, Dmitry Osipenko wrote: >> 31.07.2020 12:02, Sowjanya Komatineni пишет: >> ... >>> @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct >>> v4l2_subdev *subdev) >>> return ret; >>> } >>> + if (csi_chan->mipi) { >>> + ret = tegra_mipi_enable(csi_chan->mipi); >>> + if (ret < 0) { >>> + dev_err(csi->dev, >>> + "failed to enable MIPI pads: %d\n", ret); >>> + goto rpm_put; >>> + } >>> + >>> + /* >>> + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to >>> + * be calibrated after power on. >>> + * So, trigger the calibration start here and results will >>> + * be latched and applied to the pads when link is in LP11 >>> + * state during start of sensor streaming. >>> + */ >>> + ret = tegra_mipi_start_calibration(csi_chan->mipi); >>> + if (ret < 0) { >>> + dev_err(csi->dev, >>> + "failed to start MIPI calibration: %d\n", ret); >>> + goto disable_mipi; >>> + } >> What would happen if CSI stream is enabled and then immediately disabled >> without enabling camera sensor? > > Nothing will happen as during stream enable csi receiver is kept ready. > > But actual capture will not happen during that point. Could you please show how the full call chain looks like? It's not clear to me what keeps CSI stream "ready".
On 7/31/20 9:14 AM, Dmitry Osipenko wrote: > 31.07.2020 18:46, Sowjanya Komatineni пишет: >> On 7/31/20 4:39 AM, Dmitry Osipenko wrote: >>> 31.07.2020 12:02, Sowjanya Komatineni пишет: >>> ... >>>> @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct >>>> v4l2_subdev *subdev) >>>> return ret; >>>> } >>>> + if (csi_chan->mipi) { >>>> + ret = tegra_mipi_enable(csi_chan->mipi); >>>> + if (ret < 0) { >>>> + dev_err(csi->dev, >>>> + "failed to enable MIPI pads: %d\n", ret); >>>> + goto rpm_put; >>>> + } >>>> + >>>> + /* >>>> + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to >>>> + * be calibrated after power on. >>>> + * So, trigger the calibration start here and results will >>>> + * be latched and applied to the pads when link is in LP11 >>>> + * state during start of sensor streaming. >>>> + */ >>>> + ret = tegra_mipi_start_calibration(csi_chan->mipi); >>>> + if (ret < 0) { >>>> + dev_err(csi->dev, >>>> + "failed to start MIPI calibration: %d\n", ret); >>>> + goto disable_mipi; >>>> + } >>> What would happen if CSI stream is enabled and then immediately disabled >>> without enabling camera sensor? >> Nothing will happen as during stream enable csi receiver is kept ready. >> >> But actual capture will not happen during that point. > Could you please show how the full call chain looks like? It's not clear > to me what keeps CSI stream "ready". VI is the main video input (video device) and on streaming it starts stream of CSI subdev prior to stream of Sensor. HW path, sensor stream (CSI TX) -> CSI stream (RX) During CSI stream on, CSI PHY receiver is enabled to start receiving the data but internally capture assembled to active state will happen only when Tegra VI single shot is issues where VI thru pixel parser gets captures data into the memory
31.07.2020 19:29, Sowjanya Komatineni пишет: > > On 7/31/20 9:14 AM, Dmitry Osipenko wrote: >> 31.07.2020 18:46, Sowjanya Komatineni пишет: >>> On 7/31/20 4:39 AM, Dmitry Osipenko wrote: >>>> 31.07.2020 12:02, Sowjanya Komatineni пишет: >>>> ... >>>>> @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct >>>>> v4l2_subdev *subdev) >>>>> return ret; >>>>> } >>>>> + if (csi_chan->mipi) { >>>>> + ret = tegra_mipi_enable(csi_chan->mipi); >>>>> + if (ret < 0) { >>>>> + dev_err(csi->dev, >>>>> + "failed to enable MIPI pads: %d\n", ret); >>>>> + goto rpm_put; >>>>> + } >>>>> + >>>>> + /* >>>>> + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to >>>>> + * be calibrated after power on. >>>>> + * So, trigger the calibration start here and results will >>>>> + * be latched and applied to the pads when link is in LP11 >>>>> + * state during start of sensor streaming. >>>>> + */ >>>>> + ret = tegra_mipi_start_calibration(csi_chan->mipi); >>>>> + if (ret < 0) { >>>>> + dev_err(csi->dev, >>>>> + "failed to start MIPI calibration: %d\n", ret); >>>>> + goto disable_mipi; >>>>> + } >>>> What would happen if CSI stream is enabled and then immediately >>>> disabled >>>> without enabling camera sensor? >>> Nothing will happen as during stream enable csi receiver is kept ready. >>> >>> But actual capture will not happen during that point. >> Could you please show how the full call chain looks like? It's not clear >> to me what keeps CSI stream "ready". > > VI is the main video input (video device) and on streaming it starts > stream of CSI subdev prior to stream of Sensor. > > HW path, sensor stream (CSI TX) -> CSI stream (RX) > > During CSI stream on, CSI PHY receiver is enabled to start receiving the > data but internally capture assembled to active state will happen only > when Tegra VI single shot is issues where VI thru pixel parser gets > captures data into the memory Alright, I see now. Will be great if you could change this hunk: { ret = v4l2_subdev_call(src_subdev, video, s_stream, true); if (ret < 0 && ret != -ENOIOCTLCMD) { tegra_mipi_cancel_calibration(csi_chan->mipi); v4l2_subdev_call(csi_subdev, video, s_stream, false); return ret; } } to look like this: { err = v4l2_subdev_call(src_subdev, video, s_stream, true); if (err < 0 && err != -ENOIOCTLCMD) goto err_disable_csi_stream; ... return 0; err_disable_csi_stream: tegra_mipi_cancel_calibration(csi_chan->mipi); v4l2_subdev_call(csi_subdev, video, s_stream, false); return err; } It should make code a bit easier to read and follow. Otherwise this patch looks good to me, thanks.
On 7/31/20 1:42 PM, Dmitry Osipenko wrote: > 31.07.2020 19:29, Sowjanya Komatineni пишет: >> On 7/31/20 9:14 AM, Dmitry Osipenko wrote: >>> 31.07.2020 18:46, Sowjanya Komatineni пишет: >>>> On 7/31/20 4:39 AM, Dmitry Osipenko wrote: >>>>> 31.07.2020 12:02, Sowjanya Komatineni пишет: >>>>> ... >>>>>> @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct >>>>>> v4l2_subdev *subdev) >>>>>> return ret; >>>>>> } >>>>>> + if (csi_chan->mipi) { >>>>>> + ret = tegra_mipi_enable(csi_chan->mipi); >>>>>> + if (ret < 0) { >>>>>> + dev_err(csi->dev, >>>>>> + "failed to enable MIPI pads: %d\n", ret); >>>>>> + goto rpm_put; >>>>>> + } >>>>>> + >>>>>> + /* >>>>>> + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to >>>>>> + * be calibrated after power on. >>>>>> + * So, trigger the calibration start here and results will >>>>>> + * be latched and applied to the pads when link is in LP11 >>>>>> + * state during start of sensor streaming. >>>>>> + */ >>>>>> + ret = tegra_mipi_start_calibration(csi_chan->mipi); >>>>>> + if (ret < 0) { >>>>>> + dev_err(csi->dev, >>>>>> + "failed to start MIPI calibration: %d\n", ret); >>>>>> + goto disable_mipi; >>>>>> + } >>>>> What would happen if CSI stream is enabled and then immediately >>>>> disabled >>>>> without enabling camera sensor? >>>> Nothing will happen as during stream enable csi receiver is kept ready. >>>> >>>> But actual capture will not happen during that point. >>> Could you please show how the full call chain looks like? It's not clear >>> to me what keeps CSI stream "ready". >> VI is the main video input (video device) and on streaming it starts >> stream of CSI subdev prior to stream of Sensor. >> >> HW path, sensor stream (CSI TX) -> CSI stream (RX) >> >> During CSI stream on, CSI PHY receiver is enabled to start receiving the >> data but internally capture assembled to active state will happen only >> when Tegra VI single shot is issues where VI thru pixel parser gets >> captures data into the memory > Alright, I see now. > > Will be great if you could change this hunk: > > { > ret = v4l2_subdev_call(src_subdev, video, s_stream, true); > if (ret < 0 && ret != -ENOIOCTLCMD) { > tegra_mipi_cancel_calibration(csi_chan->mipi); > v4l2_subdev_call(csi_subdev, video, s_stream, false); > return ret; > } > } > > to look like this: > > { > err = v4l2_subdev_call(src_subdev, video, s_stream, true); > if (err < 0 && err != -ENOIOCTLCMD) > goto err_disable_csi_stream; > ... > return 0; > > err_disable_csi_stream: > tegra_mipi_cancel_calibration(csi_chan->mipi); > > v4l2_subdev_call(csi_subdev, video, s_stream, false); > > return err; > } > > > It should make code a bit easier to read and follow. > > Otherwise this patch looks good to me, thanks. Thanks Dmitry. Will send v7 now with this minor fix and would like to close on this soon. Sowjanya
diff --git a/drivers/staging/media/tegra-video/TODO b/drivers/staging/media/tegra-video/TODO index 97a19b4..98d3c7d 100644 --- a/drivers/staging/media/tegra-video/TODO +++ b/drivers/staging/media/tegra-video/TODO @@ -1,5 +1,4 @@ TODO list -* Add Tegra CSI MIPI pads calibration. * Add MIPI clock Settle time computation based on the data rate. * Add support for Ganged mode. * Add RAW10 packed video format support to Tegra210 video formats. diff --git a/drivers/staging/media/tegra-video/csi.c b/drivers/staging/media/tegra-video/csi.c index f308e8c..ea0f941 100644 --- a/drivers/staging/media/tegra-video/csi.c +++ b/drivers/staging/media/tegra-video/csi.c @@ -240,7 +240,7 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) struct tegra_vi_channel *chan = v4l2_get_subdev_hostdata(subdev); struct tegra_csi_channel *csi_chan = to_csi_chan(subdev); struct tegra_csi *csi = csi_chan->csi; - int ret; + int ret, err; ret = pm_runtime_get_sync(csi->dev); if (ret < 0) { @@ -249,13 +249,47 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev) return ret; } + if (csi_chan->mipi) { + ret = tegra_mipi_enable(csi_chan->mipi); + if (ret < 0) { + dev_err(csi->dev, + "failed to enable MIPI pads: %d\n", ret); + goto rpm_put; + } + + /* + * CSI MIPI pads PULLUP, PULLDN and TERM impedances need to + * be calibrated after power on. + * So, trigger the calibration start here and results will + * be latched and applied to the pads when link is in LP11 + * state during start of sensor streaming. + */ + ret = tegra_mipi_start_calibration(csi_chan->mipi); + if (ret < 0) { + dev_err(csi->dev, + "failed to start MIPI calibration: %d\n", ret); + goto disable_mipi; + } + } + csi_chan->pg_mode = chan->pg_mode; ret = csi->ops->csi_start_streaming(csi_chan); if (ret < 0) - goto rpm_put; + goto cancel_calibration; return 0; +cancel_calibration: + if (csi_chan->mipi) + tegra_mipi_cancel_calibration(csi_chan->mipi); +disable_mipi: + if (csi_chan->mipi) { + err = tegra_mipi_disable(csi_chan->mipi); + if (err < 0) + dev_err(csi->dev, + "failed to disable MIPI pads: %d\n", err); + } + rpm_put: pm_runtime_put(csi->dev); return ret; @@ -265,9 +299,17 @@ static int tegra_csi_disable_stream(struct v4l2_subdev *subdev) { struct tegra_csi_channel *csi_chan = to_csi_chan(subdev); struct tegra_csi *csi = csi_chan->csi; + int err; csi->ops->csi_stop_streaming(csi_chan); + if (csi_chan->mipi) { + err = tegra_mipi_disable(csi_chan->mipi); + if (err < 0) + dev_err(csi->dev, + "failed to disable MIPI pads: %d\n", err); + } + pm_runtime_put(csi->dev); return 0; @@ -313,6 +355,7 @@ static int tegra_csi_channel_alloc(struct tegra_csi *csi, unsigned int num_pads) { struct tegra_csi_channel *chan; + int ret = 0; chan = kzalloc(sizeof(*chan), GFP_KERNEL); if (!chan) @@ -331,7 +374,16 @@ static int tegra_csi_channel_alloc(struct tegra_csi *csi, chan->pads[0].flags = MEDIA_PAD_FL_SOURCE; } - return 0; + if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) + return 0; + + chan->mipi = tegra_mipi_request(csi->dev, node); + if (IS_ERR(chan->mipi)) { + ret = PTR_ERR(chan->mipi); + dev_err(csi->dev, "failed to get mipi device: %d\n", ret); + } + + return ret; } static int tegra_csi_tpg_channels_alloc(struct tegra_csi *csi) @@ -494,6 +546,9 @@ static void tegra_csi_channels_cleanup(struct tegra_csi *csi) struct tegra_csi_channel *chan, *tmp; list_for_each_entry_safe(chan, tmp, &csi->csi_chans, list) { + if (chan->mipi) + tegra_mipi_free(chan->mipi); + subdev = &chan->subdev; if (subdev->dev) { if (!IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) diff --git a/drivers/staging/media/tegra-video/csi.h b/drivers/staging/media/tegra-video/csi.h index 78a5110..0d50fc3 100644 --- a/drivers/staging/media/tegra-video/csi.h +++ b/drivers/staging/media/tegra-video/csi.h @@ -50,6 +50,7 @@ struct tegra_csi; * @framerate: active framerate for TPG * @h_blank: horizontal blanking for TPG active format * @v_blank: vertical blanking for TPG active format + * @mipi: mipi device for corresponding csi channel pads */ struct tegra_csi_channel { struct list_head list; @@ -65,6 +66,7 @@ struct tegra_csi_channel { unsigned int framerate; unsigned int h_blank; unsigned int v_blank; + struct tegra_mipi_device *mipi; }; /** diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c index 6d0aed0..1bd84eb 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -191,6 +191,7 @@ tegra_channel_get_remote_source_subdev(struct tegra_vi_channel *chan) static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) { struct v4l2_subdev *csi_subdev, *src_subdev; + struct tegra_csi_channel *csi_chan; int ret; /* @@ -206,13 +207,30 @@ static int tegra_channel_enable_stream(struct tegra_vi_channel *chan) if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) return 0; + csi_chan = v4l2_get_subdevdata(csi_subdev); + /* + * TRM has incorrectly documented to wait for done status from + * calibration logic after CSI interface power on. + * As per the design, calibration results are latched and applied + * to the pads only when the link is in LP11 state which will happen + * during the sensor stream-on. + * CSI subdev stream-on triggers start of MIPI pads calibration. + * Wait for calibration to finish here after sensor subdev stream-on + * and in case of sensor stream-on failure, cancel the calibration. + */ src_subdev = tegra_channel_get_remote_source_subdev(chan); ret = v4l2_subdev_call(src_subdev, video, s_stream, true); if (ret < 0 && ret != -ENOIOCTLCMD) { + tegra_mipi_cancel_calibration(csi_chan->mipi); v4l2_subdev_call(csi_subdev, video, s_stream, false); return ret; } + ret = tegra_mipi_finish_calibration(csi_chan->mipi); + if (ret < 0) + dev_warn(csi_chan->csi->dev, + "MIPI calibration failed: %d\n", ret); + return 0; }
CSI MIPI pads need to be enabled and calibrated for capturing from the external sensor or transmitter. MIPI CAL unit calibrates MIPI pads pull-up, pull-down and termination impedances. Calibration is done by co-work of MIPI BIAS pad and MIPI CAL control unit. Triggering calibration start can happen any time after MIPI pads are enabled but calibration results will be latched and applied to MIPI pads by MIPI CAL unit only when the link is in LP11 state and then calibration status register gets updated. This patch enables CSI MIPI pads and calibrates them during streaming. Tegra CSI receiver is able to catch the very first clock transition. So, CSI receiver is always enabled prior to sensor streaming and trigger of calibration start is done during CSI subdev streaming and status of calibration is verified after sensor stream on. Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> --- drivers/staging/media/tegra-video/TODO | 1 - drivers/staging/media/tegra-video/csi.c | 61 +++++++++++++++++++++++++++++++-- drivers/staging/media/tegra-video/csi.h | 2 ++ drivers/staging/media/tegra-video/vi.c | 18 ++++++++++ 4 files changed, 78 insertions(+), 4 deletions(-)