[PATCH 2/2] drm/radeon: remove bapm callbacks

[Lucas Stach]

Am Donnerstag, den 23.04.2015, 21:41 +0000 schrieb Deucher, Alexander:
> > -----Original Message-----
> > From: Lucas Stach [mailto:dev at lynxeye.de]
> > Sent: Thursday, April 23, 2015 5:28 PM
> > To: Deucher, Alexander; Koenig, Christian; David Airlie
> > Cc: dri-devel at lists.freedesktop.org
> > Subject: [PATCH 2/2] drm/radeon: remove bapm callbacks
> > 
> > Trying to disable BAPM on battery power does not fix the problematic
> > Trinity mobile parts. Fixing those probably need a more complex solution,
> > like doing a complete reinit of the DPM state. This will take more work
> > and most likely it will not be possible to map this to a single callback.
> > 
> > Even worse trying to change the BAPM state mid flight is breaking BAPM
> > on Richland mobile parts that are otherwise completely stable with BAPM
> > enabled.
> 
> Do you have any examples of this?  This was added to fix bapm
> stability on mobile parts and did help on a number of systems.  Bapm
> is disabled by default since it is so problematic on a lot of systems.
> Ripping all of this out doesn't seem like it will really help
> anything.
> 

I have a A10-5750M (Richland) that runs completely stable with BAPM
enabled. The SMC seems to do the right thing on it's own as I can see
that the CPU clock rate is constrained to the highest non-turbo clock
when on battery power while it boosts up to max-turbo when on AC and
sufficient thermal headroom is there.

Trying to disable BAPM mid flight is the only thing that is able to make
it hang. So for this machine the "fix" is clearly to blame for an
instability.

> Alex
> 
> > 
> > As there are no users of this hook anymore we can just remove the calling
> > infrastructure.
> > 
> > Signed-off-by: Lucas Stach <dev at lynxeye.de>
> > ---
> > I've did some pretty extensive tests with one of the problematic Trinity
> > mobile parts. I was able to figure out a stable config with BAPM enabled
> > for this, but unfortunately the power consumption rose quite a bit. So on
> > Trinity one has to disable BAPM while on battery power. I found a sequence
> 
> Can you elaborate on this?  This seems to contradict what the patch does.
> 
This patch rips out the BAPM callback as it is most likely not
sufficient to fix the stability problems. So to describe a bit more what
I found on a A8-4500M (Trinity):

BAPM is only stable with power gating disabled. Unfortunately disabling
power gating adds about 1.5W to the idle power consumption, enabling
BAPM adds another 8W to this, so this is clearly unacceptable on battery
power. Or maybe it is if the user explicitly requested the performance
power state.

When properly en-/disabling power gating before a BAPM transition I can
get a stable transition without hanging the GPU. I can see that it works
as CPU clock rates are constrained to non-turbo without BAPM while they
go up quite a bit (but not to max-turbo) with BAPM enabled.

Now the problem is that after switching BAPM off for the first time the
GPU is stuck at whatever power level it was at the moment of the BAPM
transition. This can be any power level, low power if the transition
happened with GPU idle or even the highest power level if the GPU was
rendering at that moment. So simply telling the SMC to switch off BAPM
is clearly not doing the right thing.

Most likely the DPM state needs to be reinitialized when switching BAPM.
The battery power state even has a flag to disable BAPM. So I think the
right thing to do when switching between AC/DC is to do a complete
re-evaluation of the power state and to do a complete DPM power state
switch if necessary. The simple BAPM on/off callback won't be of much
help for this.

Regards,
Lucas