On 26 February 2016 at 09:43, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
> 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);"
Ah yes, of course.
>
>> 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.
The decision is for Thierry to make - I'm fine with the current code.
>
> 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
