On Thu, 19 Dec 2013 09:38:45 -0800 Jesse Barnes <jbar...@virtuousgeek.org> wrote:
> On Thu, 19 Dec 2013 14:29:43 -0200 > Paulo Zanoni <przan...@gmail.com> wrote: > > > From: Paulo Zanoni <paulo.r.zan...@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.zan...@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. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx