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:   Wed, 5 Aug 2020 09:33:52 -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/5/20 7:19 AM, Dmitry Osipenko wrote:
> 05.08.2020 17:05, Dmitry Osipenko пишет:
>> 05.08.2020 16:46, Thierry Reding пишет:
>>> On Mon, Aug 03, 2020 at 08:42:24AM -0700, Sowjanya Komatineni wrote:
>>>> With the split of MIPI calibration into tegra_mipi_calibrate() and
>>>> tegra_mipi_wait(), MIPI clock is not kept enabled till the calibration
>>>> is done.
>>>>
>>>> So, this patch skips disabling MIPI clock after triggering start of
>>>> calibration and disables it only after waiting for done status from
>>>> the calibration logic.
>>>>
>>>> This patch renames tegra_mipi_calibrate() as tegra_mipi_start_calibration()
>>>> and tegra_mipi_wait() as tegra_mipi_finish_calibration() to be inline
>>>> with their usage.
>>>>
>>>> As MIPI clock is left enabled and in case of any failures with CSI input
>>>> streaming tegra_mipi_finish_calibration() will not get invoked.
>>>> So added new API tegra_mipi_cancel_calibration() which disables MIPI clock
>>>> and consumer drivers can call this in such cases.
>>>>
>>>> Reviewed-by: Dmitry Osipenko <digetx@...il.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
>>>> ---
>>>>   drivers/gpu/drm/tegra/dsi.c |  4 ++--
>>>>   drivers/gpu/host1x/mipi.c   | 19 ++++++++++---------
>>>>   include/linux/host1x.h      |  5 +++--
>>>>   3 files changed, 15 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
>>>> index 3820e8d..a7864e9 100644
>>>> --- a/drivers/gpu/drm/tegra/dsi.c
>>>> +++ b/drivers/gpu/drm/tegra/dsi.c
>>>> @@ -694,11 +694,11 @@ static int tegra_dsi_pad_calibrate(struct tegra_dsi *dsi)
>>>>   		DSI_PAD_PREEMP_PD(0x03) | DSI_PAD_PREEMP_PU(0x3);
>>>>   	tegra_dsi_writel(dsi, value, DSI_PAD_CONTROL_3);
>>>>   
>>>> -	err = tegra_mipi_calibrate(dsi->mipi);
>>>> +	err = tegra_mipi_start_calibration(dsi->mipi);
>>>>   	if (err < 0)
>>>>   		return err;
>>>>   
>>>> -	return tegra_mipi_wait(dsi->mipi);
>>>> +	return tegra_mipi_finish_calibration(dsi->mipi);
>>>>   }
>>>>   
>>>>   static void tegra_dsi_set_timeout(struct tegra_dsi *dsi, unsigned long bclk,
>>>> diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
>>>> index e606464..b15ab6e 100644
>>>> --- a/drivers/gpu/host1x/mipi.c
>>>> +++ b/drivers/gpu/host1x/mipi.c
>>>> @@ -293,17 +293,19 @@ int tegra_mipi_disable(struct tegra_mipi_device *dev)
>>>>   }
>>>>   EXPORT_SYMBOL(tegra_mipi_disable);
>>>>   
>>>> -int tegra_mipi_wait(struct tegra_mipi_device *device)
>>>> +void tegra_mipi_cancel_calibration(struct tegra_mipi_device *device)
>>>> +{
>>>> +	clk_disable(device->mipi->clk);
>>> Do we need to do anything with the MIPI_CAL_CTRL and MIPI_CAL_STATUS
>>> registers here? We don't clear the START bit in the former when the
>>> calibration has successfully finished, but I suspect that's because
>>> the bit is self-clearing. But I wonder if we still need to clear it
>>> upon cancellation to make sure the calibration does indeed stop.
>> Apparently there is no way to explicitly stop calibration other than to
>> reset MIPI calibration block, but Sowjanya says this is unnecessary.
>>
>> Perhaps having a fixed delay before disabling clock could be enough to
>> ensure that calibration is stopped before the clock is disabled?
>>
> Actually, there is a MIPI_CAL_ACTIVE bit in the status register. Maybe
> it needs to be polled until it's unset?

Confirmed with HW design team during this patch update.

SW does not need to clear START bit and only write 1 takes effect to 
that bit.

Also, no need to have delay or do any other register settings unclear as 
its FSM and there's nothing to get stuck.

Also it goes thru small finite set of codes and by the time sensor 
programming happens for enabling streaming FSM will finish its 
calibration sequence much early and it will only wait for pads LP-11.

So, during cancel we only need disable MIPI clock.