[Intel-gfx] [PATCH] drm/i915: Enabling RC6 immediately during init/resume

[Salonie, Namrta]

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 
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?

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.
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.

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

Thanks,
Namrta

On 8/25/2015 8:02 PM, Daniel Vetter wrote:
> On Fri, Aug 21, 2015 at 01:41:26PM +0100, Chris Wilson wrote:
>> On Sat, Aug 22, 2015 at 02:19:48AM +0530, Namrta Salonie wrote:
>>> Since RC6 enabling does not involve PCU communication overhead,
>>> it can be enabled immediately during the resume time.
>>> This will help save additional power & meet power requirements
>>> for active Idle KPI where power is evaluated over
>>> number of transitions of suspend/resume.
>>>
>>> Signed-off-by: Namrta Salonie <namrta.salonie at intel.com>
>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>
>> You can pull out gen9 rc6 as well, and apply a similar transformation to
>> gen6-8. So instead of putting the if-chain in
>> intel_enable_gt_powersave(), add intel_enable_rc6() and start placing
>> the ready functions there.
>>
>> Reviewing the comments we only need the rpm lock until after rc6
>> enabling and as you keep that wakelock, you are not getting the full
>> improvement you seek. If you keep refactoring the remaining two rc6
>> functions, you can then drop the wakelock.
>
> Since this seems to not have much of a benefit due to the missing removal
> of the wakelock I wonder how this was tested ... Next patch should have
> (relative, we're not allowed to publish absolute) performance data
> attached, e.g. "Over 100 suspend/resume cycles with 5s of idle time in
> between each suspend/resume time this reduce in a reduction of $number
> $unit."
>
> Without this this patch is just unjustified tuning and I won't take it.
> -Daniel
>