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 16:32:13 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sowjanya Komatineni <skomatineni@...dia.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

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