Re: [PATCH 1/4] drm/tegra: dc: Don't disable display power partition

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

 




On Tue, Aug 02, 2016 at 11:34:26AM +0100, Jon Hunter wrote:
> Commit 33a8eb8d40ee ("drm/tegra: dc: Implement runtime PM") disables the
> display power partition when probing and this causes the Tegra114
> Dalmore to hang during boot.
> 
> The hang occurs when accessing the MIPI calibration registers (which are
> accessed during the configuration of the DSI interface). Ideally the
> MIPI driver should manage the power partition itself to ensure it is on
> when needed. The problem is that the legacy PMC APIs used for managing
> the power partitions do not support reference counting and so this
> cannot be easily done currently. Long-term we will migrate devices to
> use generic PM domains and such scenarios will be easy to support. For
> now fix this by removing the code to turn off the display power
> partition when probing the DC and always keep the DC on so that the
> power partition is not turned off. This is consistent with how the power
> partition was managed prior to this commit.
> 
> Please note that for earlier devices such as Tegra114 the MIPI
> calibration logic is part of the display power partition, where as for
> newer devices, such as Tegra124/210 it is part of the SOR power
> partition. Hence, in the long-term is makes more sense to handle such
> power partitions via the generic PM domain framework.
> 
> Fixes: 33a8eb8d40ee ("drm/tegra: dc: Implement runtime PM")
> 
> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> ---
> 
> Please note that the hang is only seen on Tegra114 with v4.8 if the
> patch "ARM: tegra: Correct polarity for Tegra114 PMIC interrupt" (2nd
> patch in series) is applied without this patch. Without the fix for the
> PMIC interrupt polarity the Palmas PMIC probe fails and the display
> probe also fails because the regulators are not found.
> 
>  drivers/gpu/drm/tegra/dc.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)

I don't think this fixes the problem at the root. After looking at the
code I think what you're seeing is caused by the tegra_mipi_power_up()
call that happens as part of the tegra_mipi_request() from the DSI
driver's ->probe().

Generally there shouldn't be a problem because the display controller
will always get enabled before the encoder (DSI) and hence the power
partition should be enabled when the actual calibration happens. The
fundamental problem in this case is that we're actually powering up
the MIPI calibration logic at the wrong time. So I think what we'll
want for a proper fix is to move all register accesses out of the
tegra_mipi_request() function and add tegra_mipi_enable() and
tegra_mipi_disable() functions that power up and power down the MIPI
calibration logic, respectively. That way we can move all the code
which relies on the power partition into the tegra_dsi_encoder_enable()
and tegra_dsi_encoder_disable() functions.

Perhaps an even better place to call these new functions from would be
the DSI driver's ->suspend() and ->resume() functions.

An added benefit of this will be that the MIPI calibration logic could
be powered off when DSI is disabled (provided no other user requires it
to be powered on), whereas currently it will remain powered even if the
DSI output is off, since the power on happens in ->probe().

I'll go write a patch.

Thierry

Attachment: signature.asc
Description: PGP signature


[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