On 26/02/16 16:06, Mathieu Poirier wrote:
> On 15 February 2016 at 05:38, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>> For Tegra124 and Tegra210, the GPU partition cannot be toggled on and
>> off via the APBDEV_PMC_PWRGATE_TOGGLE_0 register. For these devices, the
>> partition is simply powered up and down via an external regulator.
>> For these devices, there is a separate register for controlling the
>> signal clamping of the partition and this is described in the PMC SoC
>> data by the "has_gpu_clamp" variable. Use this variable to determine if
>> the GPU partition can be controlled via the APBDEV_PMC_PWRGATE_TOGGLE_0
>> register and ensure that no one can incorrectly try to toggle the GPU
>> partition via the APBDEV_PMC_PWRGATE_TOGGLE_0 register.
>>
>> Furthermore, we cannot use the APBDEV_PMC_PWRGATE_STATUS_0 register to
>> determine if the GPU partition is powered for Tegra124 and Tegra210.
>> However, if the GPU partition is powered, then the signal clamp for the
>> GPU partition should be removed and so use bit 0 of the
>> APBDEV_PMC_GPU_RG_CNTRL_0 register to determine if the clamp has been
>> removed (bit[0] = 0) and the GPU partition is powered.
>>
>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
>> ---
>>
>> Changes v1-v2:
>> - Drop the has_gpu_toggle and use has_gpu_clamps as it is not necessary
>> to have two variables.
>> - Correct the register used for reading the clamps status for the GPU
>> partition
>>
>> drivers/soc/tegra/pmc.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 8e358dbffaed..e4fd40fa27e8 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -176,7 +176,10 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
>>
>> static inline bool tegra_powergate_state(int id)
>> {
>> - return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;
>> + if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
>> + return (tegra_pmc_readl(GPU_RG_CNTRL) & 0x1) == 0;
>> + else
>> + return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;
>
> From what I deduce power is on if bit 0 of register GPU_RG_CNTRL is
> equal to 0. In the other case power is on if bit 'id' of register
> PWRGATE_STATUS is equal to 1. If I'm right this is highly confusing
> and some comments would be appreciated. Changing
>
> "return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;"
>
> for
>
> "return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) == 1;"
Yes, it is a bit confusing, but the above does not work. You are
comparing a bit with 1. What you need is:
"return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) == BIT(id);"
> would also help.
>
>> }
>>
>> static inline bool tegra_powergate_is_valid(int id)
>> @@ -191,6 +194,9 @@ static inline bool tegra_powergate_is_valid(int id)
>> */
>> static int tegra_powergate_set(unsigned int id, bool new_state)
>> {
>> + if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
>> + return -EINVAL;
>> +
>> mutex_lock(&pmc->powergates_lock);
>>
>> if (tegra_powergate_state(id) == new_state) {
>> --
>> 2.1.4
>
> With the above in mind:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
Thanks. Thierry has queued this version and so if you feel strongly we
could update in a follow-up.
Jon
--
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
