[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150106111538.GB31830@ulmo.nvidia.com>
Date: Tue, 6 Jan 2015 12:15:40 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Vince Hsu <vinceh@...dia.com>
Cc: Peter De Schrijver <pdeschrijver@...dia.com>,
Lucas Stach <dev@...xeye.de>, swarren@...dotorg.org,
gnurou@...il.com, bskeggs@...hat.com, martin.peres@...e.fr,
seven@...rod-online.com, samuel.pitoiset@...il.com,
nouveau@...ts.freedesktop.org, linux-tegra@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/11] ARM: tegra: add function to control the GPU rail
clamp
On Tue, Jan 06, 2015 at 10:11:41AM +0800, Vince Hsu wrote:
>
> On 01/05/2015 11:09 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Thu, Dec 25, 2014 at 10:28:08AM +0800, Vince Hsu wrote:
> >>On 12/24/2014 09:16 PM, Lucas Stach wrote:
> >>>Am Dienstag, den 23.12.2014, 18:39 +0800 schrieb Vince Hsu:
> >>>>The Tegra124 and later Tegra SoCs have a sepatate rail gating register
> >>>>to enable/disable the clamp. The original function
> >>>>tegra_powergate_remove_clamping() is not sufficient for the enable
> >>>>function. So add a new function which is dedicated to the GPU rail
> >>>>gating. Also don't refer to the powergate ID since the GPU ID makes no
> >>>>sense here.
> >>>>
> >>>>Signed-off-by: Vince Hsu <vinceh@...dia.com>
> >>>To be honest I don't see the point of this patch.
> >>>You are bloating the PMC interface by introducing another exported
> >>>function that does nothing different than what the current function
> >>>already does.
> >>>
> >>>If you need a way to assert the clamp I would have expected you to
> >>>introduce a common function to do this for all power partitions.
> >>I thought about adding an tegra_powergate_assert_clamping(), but that
> >>doesn't make sense to all the power partitions except GPU. Note the
> >>difference in TRM. Any suggestion for the common function?
> >I don't think extending the powergate API is useful at this point. We've
> >long had an open TODO item to replace this with a generic API. I did
> >some prototyping a while ago to use generic power domains for this, that
> >way all the details and dependencies between the partitions could be
> >properly modeled.
> >
> >Can you take a look at my staging/powergate branch here:
> >
> > https://github.com/thierryreding/linux/commits/staging/powergate
> >
> >and see if you can use that instead? The idea is to completely hide the
> >details of power partitions from drivers and use runtime PM instead.
> You generic power domains work is exactly what we want for powergate
> eventually. :) I recall we used your prototyping in somewhere internal tree.
> We have to add more to complete it though, e.g. power domain dependency, mc
> flush, and clamping register difference like this patch does.
>
> But I have a question here. Since the GK20A is not powered on/off by the PMC
> except the clamping control, and GK20A has its own power rail, do we really
> want to hide the power sequence in the generic powergate code? We still have
> to control the voltage level in the GK20A driver through the regulator
> framework. It seems weird to me if we put the regulator_{enable|disable}
> somewhere other than the GK20A driver.
I think we want both. The power domain to control the power partition
and the regulator in the gk20a driver for the voltage control.
Thierry
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists