From: Heiko Stuebner <heiko@sntech.de>
To: Doug Anderson <dianders@chromium.org>
Cc: "open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
zhangqing <zhangqing@rock-chips.com>,
Caesar Wang <wxt@rock-chips.com>,
Xing Zheng <zhengxing@rock-chips.com>,
Derek Basehore <dbasehore@chromium.org>,
Shawn Lin <shawn.lin@rock-chips.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] soc: rockchip: power-domain: Don't (incorrectly) set rk3399 up/down counts
Date: Fri, 19 Aug 2016 00:20:01 +0200 [thread overview]
Message-ID: <1700555.4f5dI05O1A@phil> (raw)
In-Reply-To: <CAD=FV=W++ONg4UjjPTmzXMp9wVWjJDNyOpQW33Oc_1Y8BfTYjQ@mail.gmail.com>
Am Donnerstag, 18. August 2016, 15:08:12 CEST schrieb Doug Anderson:
> Hi,
>
> On Thu, Aug 18, 2016 at 3:03 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Hi Doug,
> >
> > Am Donnerstag, 18. August 2016, 11:56:01 CEST schrieb Douglas Anderson:
> >> On rk3288 it was important that powerdown and powerup counts for the
> >
> >> CPU/GPU in the kernel because:
> > somehow this sentence seems to miss some verb or so :-)
>
> Sigh. I guess I can't type.
>
> On rk3288 it was important that powerdown and powerup counts for the
>
> CPU/GPU be set in the kernel because:
> >> * The power on default was crazy long.
> >> * We couldn't rely on the firmware to set this up because really this
> >>
> >> wasn't the firmware's job--the kernel was the only one that really
> >> cared about bringing up / down CPUs and the GPU and doing suspend /
> >> resume (which involves bringing up / down CPUs).
> >>
> >> On newer ARM systems (like rk3399) ARM Trusted Firmware is in charge of
> >> bringing up and down the CPUs and it really should be in charge of
> >> setting all these counts right. After all ATF is in charge of suspend /
> >> resume and CPU up / down. Let's get out of the way and let ATF do its
> >> job.
> >>
> >> A few other motivations for doing this:
> >> * Depending on another configuration (PMU_24M_EN_CFG) these counts can
> >>
> >> be either in 24M or 32k cycles. Thus, though ATF isn't really so
> >> involved in bringing up the GPU, ATF should probably manage the counts
> >> for everything so it can also manage the 24M / 32k choice.
> >>
> >> * It turns out that (right now) 24M mode is broken on rk3399 and not
> >>
> >> being used. That means that the count the kernel was programming
> >> in (24) was not 1 us (which it seems was intended) but was actually
> >> .75 ms
> >>
> >> * On rk3399 there are actually 2 separate registers for setting CPU
> >>
> >> up/down time plus 1 register for GPU up/down time. The curent kernel
> >> code actually was putting the register for the "little" cores in the
> >> "CPU" slot and the register for the "big" cores in the "GPU" slot. It
> >> was never initting the GPU counts.
> >>
> >> Note: this change assumes that ATF will actually set these values at
> >> boot, as I'm proposing in <http://crosreview.com/372381>.
> >
> > I'd hope to see a link to an ATF github pull request here :-)
> > But I guess that simply needs some more discussion on your side.
>
> Caesar is going to get confirmation that the patch is OK then I think
> he'll work on the ATF pull request. Once done we can update the link
> here?
yep and I can then also update your sentence above :-)
> >> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > change itself looks good to me.
> >
> > So I guess we'll just need to wait for the counterpart to land in the ATF
> > or do you know if the poweron-defaults are somewhat sane?
>
> Power on defaults are crappy (750 ms to turn on/off a CPU), so
> non-ideal. Probably best to wait for ATF to land.
ok, so we'll wait. As I might miss it when the other side gets merged into the
ATF, can you ping here once that is done please?
Heiko
next prev parent reply other threads:[~2016-08-19 1:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-18 18:56 [PATCH] soc: rockchip: power-domain: Don't (incorrectly) set rk3399 up/down counts Douglas Anderson
2016-08-18 22:03 ` Heiko Stuebner
2016-08-18 22:08 ` Doug Anderson
2016-08-18 22:20 ` Heiko Stuebner [this message]
2016-09-26 20:18 ` Heiko Stuebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1700555.4f5dI05O1A@phil \
--to=heiko@sntech.de \
--cc=dbasehore@chromium.org \
--cc=dianders@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=shawn.lin@rock-chips.com \
--cc=wxt@rock-chips.com \
--cc=zhangqing@rock-chips.com \
--cc=zhengxing@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).