| Message ID | 1595883452-17343-14-git-send-email-skomatineni@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support for Tegra video capture from external sensor | expand |
27.07.2020 23:57, Sowjanya Komatineni пишет: > + /* > + * 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. > + */ > subdev = on ? src_subdev : csi_subdev; > ret = v4l2_subdev_call(subdev, video, s_stream, on); > - if (ret < 0 && ret != -ENOIOCTLCMD) > + if (ret < 0 && ret != -ENOIOCTLCMD) { I assume -ENOIOCTLCMD means that camera wasn't turned ON, so why -ENOIOCTLCMD is special? > + if (on && csi_chan->mipi) > + tegra_mipi_cancel_calibration(csi_chan->mipi); > return ret; > + } > + > + if (on && csi_chan->mipi) { Does finish_calibration() really need to be called for ret=-ENOIOCTLCMD? Shouldn't it be cancel_calibration( for the -ENOIOCTLCMD? > + ret = tegra_mipi_finish_calibration(csi_chan->mipi); > + if (ret < 0) > + dev_err(csi_chan->csi->dev, > + "MIPI calibration failed: %d\n", ret); Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? > + return ret; > + } > > return 0; > }
On 7/28/20 3:30 AM, Dmitry Osipenko wrote: > 27.07.2020 23:57, Sowjanya Komatineni пишет: >> + /* >> + * 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. >> + */ >> subdev = on ? src_subdev : csi_subdev; >> ret = v4l2_subdev_call(subdev, video, s_stream, on); >> - if (ret < 0 && ret != -ENOIOCTLCMD) >> + if (ret < 0 && ret != -ENOIOCTLCMD) { > I assume -ENOIOCTLCMD means that camera wasn't turned ON, so why > -ENOIOCTLCMD is special? No -ENOIOCTLCMD mean subdev don't have s_stream ops > >> + if (on && csi_chan->mipi) >> + tegra_mipi_cancel_calibration(csi_chan->mipi); >> return ret; >> + } >> + >> + if (on && csi_chan->mipi) { > Does finish_calibration() really need to be called for ret=-ENOIOCTLCMD? > > Shouldn't it be cancel_calibration( for the -ENOIOCTLCMD? start calibration happens during csi sensor streaming which happens prior to this point. In case if sensor subdev does not have s_stream ops, then either finish/cancel calibration should happen to disable the clock. > >> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >> + if (ret < 0) >> + dev_err(csi_chan->csi->dev, >> + "MIPI calibration failed: %d\n", ret); > Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? Not required as on error streaming fails and runtime PM will turn off power anyway. Also we only did csi subdev s_stream on and during sensor subdev s_stream on fail, actual stream dont happen and on tegra side frame capture by HW happens only when kthreads run. >> + return ret; >> + } >> >> return 0; >> }
On 7/28/20 8:59 AM, Sowjanya Komatineni wrote: > > On 7/28/20 3:30 AM, Dmitry Osipenko wrote: >> 27.07.2020 23:57, Sowjanya Komatineni пишет: >>> + /* >>> + * 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. >>> + */ >>> subdev = on ? src_subdev : csi_subdev; >>> ret = v4l2_subdev_call(subdev, video, s_stream, on); >>> - if (ret < 0 && ret != -ENOIOCTLCMD) >>> + if (ret < 0 && ret != -ENOIOCTLCMD) { >> I assume -ENOIOCTLCMD means that camera wasn't turned ON, so why >> -ENOIOCTLCMD is special? > No -ENOIOCTLCMD mean subdev don't have s_stream ops >> >>> + if (on && csi_chan->mipi) >>> + tegra_mipi_cancel_calibration(csi_chan->mipi); >>> return ret; >>> + } >>> + >>> + if (on && csi_chan->mipi) { >> Does finish_calibration() really need to be called for ret=-ENOIOCTLCMD? >> >> Shouldn't it be cancel_calibration( for the -ENOIOCTLCMD? > > start calibration happens during csi sensor streaming which happens > prior to this point. > > In case if sensor subdev does not have s_stream ops, then either > finish/cancel calibration should happen to disable the clock. For -ENOIOCTLCMD, calling finish calibration as some sensors might keep pads in LP-11 on power up and for such sensors calibration logic will apply results to pads and done bit will be set. Also avoiding additional check to specifically call cancel calibration on ENOIOCTLCMD and making it fall into finish calibration as both does disable clock except finish will wait for done bit to be set. Also, most sensor subdev have s_stream ops implemented. > >> >>> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >>> + if (ret < 0) >>> + dev_err(csi_chan->csi->dev, >>> + "MIPI calibration failed: %d\n", ret); >> Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? > > Not required as on error streaming fails and runtime PM will turn off > power anyway. > > Also we only did csi subdev s_stream on and during sensor subdev > s_stream on fail, actual stream dont happen and on tegra side frame > capture by HW happens only when kthreads run. >>> + return ret; >>> + } >>> return 0; >>> }
28.07.2020 18:59, Sowjanya Komatineni пишет: ... >>> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >>> + if (ret < 0) >>> + dev_err(csi_chan->csi->dev, >>> + "MIPI calibration failed: %d\n", ret); >> Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? > > Not required as on error streaming fails and runtime PM will turn off > power anyway. I see that camera drivers bump theirs RPM on s_stream=1, and thus, s_stream=0 should be invoked in order to balance the RPM. What am I missing? https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/media/i2c/ov2740.c#L634 > Also we only did csi subdev s_stream on and during sensor subdev > s_stream on fail, actual stream dont happen and on tegra side frame > capture by HW happens only when kthreads run. Secondly, perhaps a failed calibration isn't a very critical error? Hence just printing a warning message should be enough. Could you please make a patch that factors all ON/OFF code paths into a separate functions? It's a bit difficult to follow the combined code, especially partial changes in the patches. Thanks in advance!
On 7/29/20 4:25 PM, Dmitry Osipenko wrote: > 28.07.2020 18:59, Sowjanya Komatineni пишет: > ... >>>> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >>>> + if (ret < 0) >>>> + dev_err(csi_chan->csi->dev, >>>> + "MIPI calibration failed: %d\n", ret); >>> Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? >> Not required as on error streaming fails and runtime PM will turn off >> power anyway. > I see that camera drivers bump theirs RPM on s_stream=1, and thus, > s_stream=0 should be invoked in order to balance the RPM. What am I missing? > > https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/media/i2c/ov2740.c#L634 Sensor drivers take care of RPM put when any failure happens during s_stream. So bridge driver don't have to call v4l2_subdev_call s_stream off incase if sensor subdev stream on fails. >> Also we only did csi subdev s_stream on and during sensor subdev >> s_stream on fail, actual stream dont happen and on tegra side frame >> capture by HW happens only when kthreads run. > Secondly, perhaps a failed calibration isn't a very critical error? > Hence just printing a warning message should be enough. Using dev_err to report calibration failure. Are you suggesting to use dev_warn instead of dev_err? > > Could you please make a patch that factors all ON/OFF code paths into a > separate functions? It's a bit difficult to follow the combined code, > especially partial changes in the patches. Thanks in advance! what do you mean by partial changes in patches? Can you please be more clear? Thanks Sowjanya
On 7/29/20 4:59 PM, Sowjanya Komatineni wrote: > > On 7/29/20 4:25 PM, Dmitry Osipenko wrote: >> 28.07.2020 18:59, Sowjanya Komatineni пишет: >> ... >>>>> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >>>>> + if (ret < 0) >>>>> + dev_err(csi_chan->csi->dev, >>>>> + "MIPI calibration failed: %d\n", ret); >>>> Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? >>> Not required as on error streaming fails and runtime PM will turn off >>> power anyway. >> I see that camera drivers bump theirs RPM on s_stream=1, and thus, >> s_stream=0 should be invoked in order to balance the RPM. What am I >> missing? >> >> https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/media/i2c/ov2740.c#L634 >> > > Sensor drivers take care of RPM put when any failure happens during > s_stream. > > So bridge driver don't have to call v4l2_subdev_call s_stream off > incase if sensor subdev stream on fails. > >>> Also we only did csi subdev s_stream on and during sensor subdev >>> s_stream on fail, actual stream dont happen and on tegra side frame >>> capture by HW happens only when kthreads run. >> Secondly, perhaps a failed calibration isn't a very critical error? >> Hence just printing a warning message should be enough. > > Using dev_err to report calibration failure. Are you suggesting to use > dev_warn instead of dev_err? > >> >> Could you please make a patch that factors all ON/OFF code paths into a >> separate functions? It's a bit difficult to follow the combined code, >> especially partial changes in the patches. Thanks in advance! > > what do you mean by partial changes in patches? > > Can you please be more clear? Also please specify what ON/OFF code paths you are referring to when you say to move into separate functions? > > Thanks > > Sowjanya >
30.07.2020 03:27, Sowjanya Komatineni пишет: ... >>> Secondly, perhaps a failed calibration isn't a very critical error? >>> Hence just printing a warning message should be enough. >> >> Using dev_err to report calibration failure. Are you suggesting to use >> dev_warn instead of dev_err? I meant that failing s_stream might be unnecessary. The dev_warn should be more appropriate for a non-critical errors. >>> Could you please make a patch that factors all ON/OFF code paths into a >>> separate functions? It's a bit difficult to follow the combined code, >>> especially partial changes in the patches. Thanks in advance! >> >> what do you mean by partial changes in patches? >> >> Can you please be more clear? > > Also please specify what ON/OFF code paths you are referring to when you > say to move into separate functions? I meant to change all the code like this: set(on) { if (on) { ... return; } if (!on) ... return; } to somewhat like this: set(on) { if (on) ret = enable(); else ret = disable(); return ret; }
On 7/29/20 5:27 PM, Sowjanya Komatineni wrote: > > On 7/29/20 4:59 PM, Sowjanya Komatineni wrote: >> >> On 7/29/20 4:25 PM, Dmitry Osipenko wrote: >>> 28.07.2020 18:59, Sowjanya Komatineni пишет: >>> ... >>>>>> + ret = tegra_mipi_finish_calibration(csi_chan->mipi); >>>>>> + if (ret < 0) >>>>>> + dev_err(csi_chan->csi->dev, >>>>>> + "MIPI calibration failed: %d\n", ret); >>>>> Doesn't v4l2_subdev_call(OFF) need to be invoked here on error? >>>> Not required as on error streaming fails and runtime PM will turn off >>>> power anyway. >>> I see that camera drivers bump theirs RPM on s_stream=1, and thus, >>> s_stream=0 should be invoked in order to balance the RPM. What am I >>> missing? >>> >>> https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/media/i2c/ov2740.c#L634 >>> >> >> Sensor drivers take care of RPM put when any failure happens during >> s_stream. >> >> So bridge driver don't have to call v4l2_subdev_call s_stream off >> incase if sensor subdev stream on fails. >> >>>> Also we only did csi subdev s_stream on and during sensor subdev >>>> s_stream on fail, actual stream dont happen and on tegra side frame >>>> capture by HW happens only when kthreads run. >>> Secondly, perhaps a failed calibration isn't a very critical error? >>> Hence just printing a warning message should be enough. >> >> Using dev_err to report calibration failure. Are you suggesting to >> use dev_warn instead of dev_err? >> OK I think I understood what you meant. When v4l2_subdev_call for sensor s_stream ON fails, we dont have to do v4l2_subdev_call s_stream OFF. As sensor drivers take care of RPM put when any failure happens during s_stream ON Other case when v4l2_subdev_call for sensor s_stream ON is good, then tegra_mipi_finish_calibration fail need to call s_stream OFF for sensor. Agree as calibration errors out in this case as its not critical in this scenario, So will change dev_err to dev_warn and will not report this as error so no need to call s_stream off. >>> >>> Could you please make a patch that factors all ON/OFF code paths into a >>> separate functions? It's a bit difficult to follow the combined code, >>> especially partial changes in the patches. Thanks in advance! >> >> what do you mean by partial changes in patches? >> >> Can you please be more clear? > > Also please specify what ON/OFF code paths you are referring to when > you say to move into separate functions? > >> >> Thanks >> >> Sowjanya >>
On 7/29/20 5:43 PM, Dmitry Osipenko wrote: > 30.07.2020 03:27, Sowjanya Komatineni пишет: > ... >>>> Secondly, perhaps a failed calibration isn't a very critical error? >>>> Hence just printing a warning message should be enough. >>> Using dev_err to report calibration failure. Are you suggesting to use >>> dev_warn instead of dev_err? > I meant that failing s_stream might be unnecessary. > > The dev_warn should be more appropriate for a non-critical errors. > >>>> Could you please make a patch that factors all ON/OFF code paths into a >>>> separate functions? It's a bit difficult to follow the combined code, >>>> especially partial changes in the patches. Thanks in advance! >>> what do you mean by partial changes in patches? >>> >>> Can you please be more clear? >> Also please specify what ON/OFF code paths you are referring to when you >> say to move into separate functions? > I meant to change all the code like this: > > set(on) { > if (on) { > ... > return; > } > > if (!on) > ... > > return; > } > > to somewhat like this: > > set(on) { > if (on) > ret = enable(); > else > ret = disable(); > > return ret; > } You mean to change tegra_channel_set_stream() ?
30.07.2020 03:55, Sowjanya Komatineni пишет: > > On 7/29/20 5:52 PM, Sowjanya Komatineni wrote: >> >> On 7/29/20 5:43 PM, Dmitry Osipenko wrote: >>> 30.07.2020 03:27, Sowjanya Komatineni пишет: >>> ... >>>>>> Secondly, perhaps a failed calibration isn't a very critical error? >>>>>> Hence just printing a warning message should be enough. >>>>> Using dev_err to report calibration failure. Are you suggesting to use >>>>> dev_warn instead of dev_err? >>> I meant that failing s_stream might be unnecessary. >>> >>> The dev_warn should be more appropriate for a non-critical errors. >>> >>>>>> Could you please make a patch that factors all ON/OFF code paths >>>>>> into a >>>>>> separate functions? It's a bit difficult to follow the combined code, >>>>>> especially partial changes in the patches. Thanks in advance! >>>>> what do you mean by partial changes in patches? >>>>> >>>>> Can you please be more clear? >>>> Also please specify what ON/OFF code paths you are referring to when >>>> you >>>> say to move into separate functions? >>> I meant to change all the code like this: >>> >>> set(on) { >>> if (on) { >>> ... >>> return; >>> } >>> >>> if (!on) >>> ... >>> >>> return; >>> } >>> >>> to somewhat like this: >>> >>> set(on) { >>> if (on) >>> ret = enable(); >>> else >>> ret = disable(); >>> >>> return ret; >>> } >> >> You mean to change tegra_channel_set_stream() ? Yes, and tegra_csi_s_stream(). > changing tegra_channel_set_stream() to have like below will have > redundant calls as most of the code b/w enable and disable is same > except calling them in reverse order based on on/off and doing MIPI > calibration only during ON > > > if (on) > ret = enable() > else > ret = disable() > return ret; Readability should be more important than number of lines.
On 7/29/20 5:52 PM, Sowjanya Komatineni wrote: > > On 7/29/20 5:43 PM, Dmitry Osipenko wrote: >> 30.07.2020 03:27, Sowjanya Komatineni пишет: >> ... >>>>> Secondly, perhaps a failed calibration isn't a very critical error? >>>>> Hence just printing a warning message should be enough. >>>> Using dev_err to report calibration failure. Are you suggesting to use >>>> dev_warn instead of dev_err? >> I meant that failing s_stream might be unnecessary. >> >> The dev_warn should be more appropriate for a non-critical errors. >> >>>>> Could you please make a patch that factors all ON/OFF code paths >>>>> into a >>>>> separate functions? It's a bit difficult to follow the combined code, >>>>> especially partial changes in the patches. Thanks in advance! >>>> what do you mean by partial changes in patches? >>>> >>>> Can you please be more clear? >>> Also please specify what ON/OFF code paths you are referring to when >>> you >>> say to move into separate functions? >> I meant to change all the code like this: >> >> set(on) { >> if (on) { >> ... >> return; >> } >> >> if (!on) >> ... >> >> return; >> } >> >> to somewhat like this: >> >> set(on) { >> if (on) >> ret = enable(); >> else >> ret = disable(); >> >> return ret; >> } > > You mean to change tegra_channel_set_stream() ? changing tegra_channel_set_stream() to have like below will have redundant calls as most of the code b/w enable and disable is same except calling them in reverse order based on on/off and doing MIPI calibration only during ON if (on) ret = enable() else ret = disable() return ret; > >
On 7/29/20 5:53 PM, Dmitry Osipenko wrote: > 30.07.2020 03:55, Sowjanya Komatineni пишет: >> On 7/29/20 5:52 PM, Sowjanya Komatineni wrote: >>> On 7/29/20 5:43 PM, Dmitry Osipenko wrote: >>>> 30.07.2020 03:27, Sowjanya Komatineni пишет: >>>> ... >>>>>>> Secondly, perhaps a failed calibration isn't a very critical error? >>>>>>> Hence just printing a warning message should be enough. >>>>>> Using dev_err to report calibration failure. Are you suggesting to use >>>>>> dev_warn instead of dev_err? >>>> I meant that failing s_stream might be unnecessary. >>>> >>>> The dev_warn should be more appropriate for a non-critical errors. >>>> >>>>>>> Could you please make a patch that factors all ON/OFF code paths >>>>>>> into a >>>>>>> separate functions? It's a bit difficult to follow the combined code, >>>>>>> especially partial changes in the patches. Thanks in advance! >>>>>> what do you mean by partial changes in patches? >>>>>> >>>>>> Can you please be more clear? >>>>> Also please specify what ON/OFF code paths you are referring to when >>>>> you >>>>> say to move into separate functions? >>>> I meant to change all the code like this: >>>> >>>> set(on) { >>>> if (on) { >>>> ... >>>> return; >>>> } >>>> >>>> if (!on) >>>> ... >>>> >>>> return; >>>> } >>>> >>>> to somewhat like this: >>>> >>>> set(on) { >>>> if (on) >>>> ret = enable(); >>>> else >>>> ret = disable(); >>>> >>>> return ret; >>>> } >>> You mean to change tegra_channel_set_stream() ? > Yes, and tegra_csi_s_stream(). > >> changing tegra_channel_set_stream() to have like below will have >> redundant calls as most of the code b/w enable and disable is same >> except calling them in reverse order based on on/off and doing MIPI >> calibration only during ON >> >> >> if (on) >> ret = enable() >> else >> ret = disable() >> return ret; > Readability should be more important than number of lines. Will have v6 and add additional patch at the end to do enable/disable separately. Separating this out with additional patch before adding sensor support patch requires all patches to be updated. So I am ok either ways. Please let me know if adding additional patch at the end to split tegra_channel_set_stream() and tegra_csi_s_stream() separately is ok?
30.07.2020 04:06, Sowjanya Komatineni пишет: ... > Will have v6 and add additional patch at the end to do enable/disable > separately. > > Separating this out with additional patch before adding sensor support > patch requires all patches to be updated. > > So I am ok either ways. Please let me know if adding additional patch at > the end to split tegra_channel_set_stream() and tegra_csi_s_stream() > separately is ok? Should be okay, thanks!
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 c21dd09..752ebe8 100644 --- a/drivers/staging/media/tegra-video/csi.c +++ b/drivers/staging/media/tegra-video/csi.c @@ -252,15 +252,51 @@ static int tegra_csi_s_stream(struct v4l2_subdev *subdev, int enable) 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; + } + } + ret = csi->ops->csi_start_streaming(csi_chan); - if (ret < 0) - goto rpm_put; + if (ret < 0) { + if (csi_chan->mipi) + tegra_mipi_cancel_calibration(csi_chan->mipi); + goto disable_mipi; + } return 0; } csi->ops->csi_stop_streaming(csi_chan); +disable_mipi: + if (csi_chan->mipi) { + ret = tegra_mipi_disable(csi_chan->mipi); + if (ret < 0) + dev_err(csi->dev, + "failed to disable MIPI pads: %d\n", ret); + } + rpm_put: pm_runtime_put(csi->dev); return ret; @@ -294,6 +330,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) @@ -312,7 +349,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) @@ -475,6 +521,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 fc43629..9c01cc5 100644 --- a/drivers/staging/media/tegra-video/vi.c +++ b/drivers/staging/media/tegra-video/vi.c @@ -191,10 +191,12 @@ tegra_channel_get_remote_source_subdev(struct tegra_vi_channel *chan) int tegra_channel_set_stream(struct tegra_vi_channel *chan, bool on) { struct v4l2_subdev *subdev, *csi_subdev, *src_subdev; + struct tegra_csi_channel *csi_chan; int ret; csi_subdev = tegra_channel_get_remote_csi_subdev(chan); src_subdev = tegra_channel_get_remote_source_subdev(chan); + csi_chan = v4l2_get_subdevdata(csi_subdev); /* * Tegra CSI receiver can detect the first LP to HS transition. * So, start the CSI stream-on prior to sensor stream-on and @@ -208,10 +210,31 @@ int tegra_channel_set_stream(struct tegra_vi_channel *chan, bool on) if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) return 0; + /* + * 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. + */ subdev = on ? src_subdev : csi_subdev; ret = v4l2_subdev_call(subdev, video, s_stream, on); - if (ret < 0 && ret != -ENOIOCTLCMD) + if (ret < 0 && ret != -ENOIOCTLCMD) { + if (on && csi_chan->mipi) + tegra_mipi_cancel_calibration(csi_chan->mipi); return ret; + } + + if (on && csi_chan->mipi) { + ret = tegra_mipi_finish_calibration(csi_chan->mipi); + if (ret < 0) + dev_err(csi_chan->csi->dev, + "MIPI calibration failed: %d\n", ret); + return 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 | 55 +++++++++++++++++++++++++++++++-- drivers/staging/media/tegra-video/csi.h | 2 ++ drivers/staging/media/tegra-video/vi.c | 25 ++++++++++++++- 4 files changed, 78 insertions(+), 5 deletions(-)