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 12/08/16 14:46, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> 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().

No it doesn't. It is more of a bandaid. I think that this issue has
always been there but no exposed until the move to RPM for the DC. I
can't say I was too happy with it!

> 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.

Great! Thanks.
Jon

-- 
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux