linux-kernel - Re: [PATCH v8 08/10] gpu: host1x: mipi: Keep MIPI clock enabled till calibration is done

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Aug 2020 08:59:56 -0700
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>,
        Thierry Reding <thierry.reding@...il.com>
CC:     <jonathanh@...dia.com>, <frankc@...dia.com>, <hverkuil@...all.nl>,
        <sakari.ailus@....fi>, <robh+dt@...nel.org>,
        <helen.koike@...labora.com>, <gregkh@...uxfoundation.org>,
        <linux-media@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 08/10] gpu: host1x: mipi: Keep MIPI clock enabled till
 calibration is done


On 8/6/20 6:32 AM, Dmitry Osipenko wrote:
> 06.08.2020 03:47, Sowjanya Komatineni пишет:
>> On 8/5/20 11:06 AM, Sowjanya Komatineni wrote:
>>> On 8/5/20 10:46 AM, Sowjanya Komatineni wrote:
>>>> On 8/5/20 10:34 AM, Dmitry Osipenko wrote:
>>>>> 05.08.2020 20:29, Sowjanya Komatineni пишет:
>>>>> ...
>>>>>> UART_FST_MIPI_CAL is the clock used for calibration logic which is FSM
>>>>>> that goes thru sequence codes and when done waits for pads to be in
>>>>>> LP-11 to apply results.
>>>>>>
>>>>>> MIPI_CLK is controller gate clock which is also need to be kept
>>>>>> enabled
>>>>>> as incase if it sees LP-11 it updates registers so its recommended to
>>>>>> have this clock enabled.
>>>>>>
>>>>>> We can cancel_calibration() in CSI only when csi/sensor stream on
>>>>>> fails
>>>>>> and in which case there will be no LP-11 so we can unconditionally
>>>>>> disable MIPI_CLK.
>>>>>>
>>>>> There is no guarantee that the fail comes before the LP-11. For
>>>>> example,
>>>>> some odd camera driver may have a complicated enable sequence which may
>>>>> fail after enabling the hardware streaming.
>>>> MIPI_CLK to keep enable is for calibration logic to update results,
>>>> but like I said calibration logic uses UART_FST_MIPI_CAL clock. So
>>>> even in case if fail happens from sensor after having pads in LP-11
>>>> then, calibration logic will still be running but result update will
>>>> not happen with clock disabled. But HW will not stuck as this is
>>>> confirmed from HW designer.
>>> If LP-11 happens from sensor stream (followed by fail) and by that
>>> time if calibration FSM is done and if calibration logic sees LP-11
>>> then results will be applied to pads.
>>>
>>> We did start of calibration before CSI stream so by the time we do
>>> sensor stream enable, calibration logic might have done with FSM and
>>> waiting for LP-11
>>>
>>> Also if we see any special case, we always can use
>>> finish_calibration() instead of cancel_calibration() as well.
> Why not to do it right now?

> Then the code could look like this:
>
> src_subdev = tegra_channel_get_remote_source_subdev(chan);
> ret = v4l2_subdev_call(src_subdev, video, s_stream, true);
> err = tegra_mipi_finish_calibration(csi_chan->mipi);
>
> if (ret < 0 && ret != -ENOIOCTLCMD)
> 	goto err_disable_csi_stream;
>
> if (err < 0)
> 	dev_warn(csi_chan->csi->dev,
> 		 "MIPI calibration failed: %d\n", err);
>
>>> finish_calibration() has extra 250ms wait time polling done bit and we
>>> can ignore its return code during fail pathway.
>>>
>> Confirmed from HW designer, calibration FSM to finish takes worst case
>> 72uS so by the time it gets to sensor stream it will be done its
>> sequence and will be waiting for DONE bit.
>>
>> So disabling MIPI CAL clock on sensor stream fails is safe.
>
> 72us is quite a lot of time, what will happen if LP-11 happens before
> FSM finished calibration?
>
> Maybe the finish_calibration() needs to split into two parts:
>
>   1. wait for CAL_STATUS_ACTIVE before enabling sensor
>   2. wait for CAL_STATUS_DONE after enabling sensor

I don't think we need to split for active and done. Active will be 1 as 
long as other pads are in calibration as well.

We cant use active status check for specific pads under calibration. 
This is common bit for all pads.

Unfortunately HW don't have separate status indicating when sequence is 
done to indicate its waiting for LP11.


To avoid all this, will remove cancel_calibration() totally and use same 
finish calibration even in case of stream failure then.