Re: [RFC PATCH v5 13/14] media: tegra-video: Add CSI MIPI pads calibration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux