[Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"

[O'Rourke, Tom]

On Mon, Sep 29, 2014 at 12:46:25PM -0700, Jesse Barnes wrote:
> On Mon, 29 Sep 2014 09:52:38 -0700
> Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
> 
> > On Mon, Sep 29, 2014 at 9:38 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> > > On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote:
> > >> On Mon, 29 Sep 2014 15:11:51 +0200
> > >> Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > >>
> > >> > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca.
> > >> >
> > >> > It's apparently too broken so that Rodrigo submitted a patch to add a
> > >> > config option for it. Given that the design is also ... suboptimal and
> > >> > that I've only merged this to get lead engineers and managers off my
> > >> > back for one second let's just revert this.
> > >> >
> > >> > /me puts on combat gear again
> > >> >
> > >> > It was worth a shot ...
> > >>
> > >> I thought we had a fix for the runtime PM issue this created?  And
> > >> Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess
> > >> I don't see the big issue?
> > 
> > Well, actually the issue is to stay with high busted gt frequency even on idle.
> > Or this or an incompatibility with the actual rps interface...
> > 
> > So my patch was actually a protect a feture under development with parameter.
> > 
> > >>
> > >> Or is there another bug you didn't mention in the s-o-b section you're
> > >> worried about?
> > 
> > The only issue I know is:
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=77869
> 
> Ah yeah, misread it thinking it was accidentally being enabled on
> non-bdw too or something.
> 
> But yeah I expect turbo to be broken even with your patch, since we
> won't ramp up/down the freq (or at least not properly).  The only thing
> Daisy's patch addresses is making sure we at least try to ramp if we're
> getting regular page flips.  The more complete fix (which should
> address the test failure too) would be to peroidically poll the hw busy
> state and adjust the freq based on that, independent of the flip
> frequency.  That should allow GPGPU apps, offlien media encode/decode,
> and these tests to work as they should, but it's a little more work.
> We need to make sure the timer is shut down when we go into RC6 and
> only fires frequently when the GPU is busy.
>
I object to reverting Daisy's "BDW Software Turbo" patch 
without also having an acceptable replacement.

Yes, Daisy's patch is not a complete fix.  Yes, a different 
design may have been better.  But this patch did get turbo 
working on BDW systems with regular flips (such as those 
running Android).  Taking this patch out without a 
replacement is a step in the wrong direction.

-- Tom O'Rourke