Messages in this thread | | | Subject | Re: [PATCH v8 08/10] gpu: host1x: mipi: Keep MIPI clock enabled till calibration is done | From | Sowjanya Komatineni <> | Date | Wed, 5 Aug 2020 10:29:16 -0700 |
| |
On 8/5/20 10:04 AM, Sowjanya Komatineni wrote: > > On 8/5/20 9:57 AM, Dmitry Osipenko wrote: >> 05.08.2020 19:50, Sowjanya Komatineni пишет: >>> On 8/5/20 9:47 AM, Dmitry Osipenko wrote: >>>> 05.08.2020 19:33, Sowjanya Komatineni пишет: >>>>> 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@gmail.com> >>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.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. >>>>> >>>> But there is no guarantee that cancel_calibration() couldn't be >>>> invoked >>>> in the middle of the calibration process, hence FSM could freeze in an >>>> intermediate state if it's running on the disabled MIPI clock, this >>>> doesn't sound good. >>> Actual calibration logic uses UART_FST_CAL clock which is always >>> enabled >> What enables the UART_FST_CAL clock? I don't see this clock used >> anywhere. > > UART_FST_MIPI_CAL is shared with uart and is always enabled all the time. > > I don't see mipi driver handling this and I think that's because this > clock is enabled all the time as its used for UART as well. Probably > thierry can comment on this clock. > > Also regarding cancel calibration, as FSM goes thru only finite > sequence codes by the time csi stream and sensor stream happens which > is where we check for calibration to complete for sure calibration > will be finished and calibration logic will only wait for pads to be > in LP-11 to apply results. So nothing special needed during cancel > except to turn clock off to balance its usage count. > > 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.
| |