Re: [PATCH] drm/i915: Enabling RC6 immediately during init/resume

Daniel Vetter
2015-09-02T08:30:51Z

On Mon, Aug 31, 2015 at 05:08:36PM +0530, Salonie, Namrta wrote:
> Hi Chris, Daniel.
>
> Thanks for your inputs.
> I agree that we need to amend the patch. Will do following changes.
> 1. RPM ref count is not needed with immediate enabling of RC6, I will remove
> that.
> 2. I will extend this to other GEN as well.
>
> This was one of the set of optimization we implemented for BYT Android. All
> of these
> gave improvement of ~5mW for 30minutes for Active Idle WLAN KPI. And about
> ~5mW for other airplane, wifi, radio suspend scenarios.
>
> The other optimizations included :-
> 1. Reduction of autosuspend delay to 500ms from 10ms (On BYT, display D3

I guess you meant 10 s runtime pm autosuspend default?

> should happen in suspend as Punit initiates S0iX flow only considering
> Display D3). Because of this reduction Display D3 will happen immediately:
> This can be controlled by user mode in android. However shall we bring this
> value for Linux as well?

I have a patch for that (including enabling runtime pm by default), but
it's blocked because atm runtime pm is broken in upstream.

> 2. Deferring RC6 disabling from early_resume callback to resume callback to
> reduce the delay for which the wells had to stay ON – We verified the HDMI
> case and it worked without issues.

I think the big trouble there is that right now we don't handle power well
references correctly in the suspend/resume code at all - we just
force-enable them all. Definitely something we want to fix, but will be a
lot of work to make sure it works everywhere. I'd like to see an overall
approach to this though since I fear if we just move around individual
rpm references (like rc6 or specific power wells) the end-result will be a
really complicated and fragile design. Suspend/resume is already one of
the most fragile parts of the driver as-is.

> 3. During resume, perform modeset based on the DPMS state, so that Display
> remains Off for the intermediate wake ups where no DPMS ON/OFF happens.

That /should/ be how it's supposed to work. I.e. for dpms off, we should
not try to enable things again. Might have been broken, but with latest
atomic it really should work correctly.

> Also, can we port the optimizations 2 & 3 to the upstream kernel?

Sure, sounds like some good. Usual caveat applies though since upstream
needs to work everywhere, so probably some more work is needed to make
sure the patches don't break anything.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation