Messages in this thread | | | Subject | Re: [RFC PATCH v5 13/14] media: tegra-video: Add CSI MIPI pads calibration | From | Sowjanya Komatineni <> | Date | Wed, 29 Jul 2020 17:47:23 -0700 |
| |
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 >>
| |