[Intel-gfx] [PATCH 5/6] drm/i915: don't wait for power cycle when waiting for power off

[Jesse Barnes]

On Thu, 19 Dec 2013 14:29:43 -0200
Paulo Zanoni <przanoni at gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> Function ironlake_wait_panel_off should just wait for the power off
> delay, while function ironlake_wait_panel_power_cycle should wait for
> the panel cycle (that's required after we turn the panel off, before
> we enable it again).
> 
> The problem is that, currently, ironlake_wait_panel_off is waiting not
> just for the panel to be off, but also for the power cycle delay and
> the backlight off delay. This function relies on the PP_STATUS bits
> 3:0, which are not documented and not supposed to be used. A quick
> analysis of the values we get while waiting quickly shows that power
> off is reached while bits 3:0 are still 0x1, and the time it takes to
> become 0x0 is the power cycle delay.
> 
> On my system with backlight off delay of 200ms, power down delay of
> 50ms and power cycle delay of 500ms, this is what I get:
>  - Start waiting with value 0x80000008, timestamp 6.429364.
>  - Jumps to 0xa0000003, timestamp 6.431360 (time waited: 0.001996)
>  - Jumps to 0xa0000002, timestamp 6.631277 (time waited: 0.201913)
>  - Jumps to 0x08000001, timestamp 6.681258 (time waited: 0.251894)
>  - Jumps to 0x00000000, timestamp 7.192012 (time waited: 0.762648)
> 
> As you can see, ironlake_wait_panel_off is sleeping 760ms instead of
> the expected 50ms: the first 200ms matches the backlight off delay
> (which we should already have waited for!), then the 50ms for the real
> panel off delay, then the 500ms for the panel power cycle.
> 
> This patch makes is look just at bits 31 and 29:28, which will ignore
> the panel power cycle.
> 
> And just to be clear: this saves 500ms on my system every time we
> disable the panel. But we can still save 200ms more (the backlight off
> delay) on the next patches.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a6a4c4f..69d8f1c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1011,8 +1011,8 @@ static void intel_dp_mode_set(struct intel_encoder *encoder)
>  #define IDLE_ON_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | PP_SEQUENCE_STATE_MASK)
>  #define IDLE_ON_VALUE   	(PP_ON | PP_SEQUENCE_NONE | 0                     | PP_SEQUENCE_STATE_ON_IDLE)
>  
> -#define IDLE_OFF_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | PP_SEQUENCE_STATE_MASK)
> -#define IDLE_OFF_VALUE		(0     | PP_SEQUENCE_NONE | 0                     | PP_SEQUENCE_STATE_OFF_IDLE)
> +#define IDLE_OFF_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | 0)
> +#define IDLE_OFF_VALUE		(0     | PP_SEQUENCE_NONE | 0                     | 0)
>  
>  #define IDLE_CYCLE_MASK		(PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
>  #define IDLE_CYCLE_VALUE	(0     | PP_SEQUENCE_NONE | 0                     | PP_SEQUENCE_STATE_OFF_IDLE)

It would be good to note confirmation from the hw guys in the commit
msg, and maybe get some data on an LVDS panel just to be sure, but
otherwise:

Reviewed-by: Jesse Barnes <jbarnes at virtuougseek.org>