[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 09:38:45 -0800
Jesse Barnes <jbarnes at virtuousgeek.org> wrote:

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

For the LVDS bit we'd need to apply similar changes on the LVDS side,
but I think that may already be pretty fast, so you can ignore that bit.