Re: [PATCH 14/21] drm/i915: Correct eDP panel power sequencing delaycomputations

From: Daniel Vetter
Date: Sat Oct 01 2011 - 05:58:37 EST

On Fri, Sep 30, 2011 at 08:31:13PM -0700, Keith Packard wrote:
> On Fri, 30 Sep 2011 20:16:44 +0200, Daniel Vetter <daniel@xxxxxxxx> wrote:
> > Awesome patch description and the code agrees with what I've cross-checked
> > on bspec. The only fear I have is that we currently ignore the backlight
> > on/off timings and some panel probably relies on use waiting for backlight
> > on/off + panel on/off in total. But that's material for another patch.
> Ok, I've just read through the VBIOS source code to figure out how it
> converts the edp_power_seq table into the panel delay register settings.
> intel_bios.h says:
> struct edp_power_seq {
> u16 t3;
> u16 t7;
> u16 t9;
> u16 t10;
> u16 t12;
> } __attribute__ ((packed));
> The VBIOS source code says that these five words are:
> Data Code
> DW xxxx T1+T2 T1_T2 (Power Up Delay)
> DW yyyy T3 T5 (Power-On to backlight On)
> DW zzzz T4 T6 (Backlight off to power off)
> DW wwww T5 T3 (power down delay)
> DW tttt T7 T4 (power cycle delay)
> ('Data' is how the initialized data is labeled, 'Code' is how the
> struct is defined and used. Yes, they appear to be different).
> And, reading the code that programs the eDP registers
> PP_ON_DELAYS = (T1_T2 << 16) | T5
> PP_OFF_DELAYS = (T3 << 16) | T6
> PP_DIVISOR = T4 / 1000 + 1
> The BSPEC/BIOS names don't agree with the eDP spec. Here's my plan:
> Data Code eDP spec intel_dp name What I'm using this for:
> T1+T2 T1_T2 T1+T3 panel_power_up_delay VCC on to AUX channel
> T3 T5 T6+T8 backlight_on_delay Link training to backlight on
> T4 T6 T9 backlight_off_delay Backlight off to video off
> T5 T3 T10 panel_power_down_delay Video off to VCC off
> T7 T4 T11+T12 panel_power_cycle_delay VCC off to VCC on
> It's trivial to pull the values back out of the PP registers, so I'll do
> that and use the maximum of the two instead of preferring one or the
> other. Waiting too long shouldn't ever hurt.
> As far as the edp_power_seq register goes, I'd love to know where those
> names came from. They don't quite match my interpretation, in
> particular, the 't7' is almost certainly not the eDP T7 value, which is
> the delay from valid video data on the link to a correct display,
> which isn't a useful value as it doesn't impact the source at all.
> Ok, so a bit more whacking of code and I'll post an updated patch that
> uses all of these values as above.

Sounds like a plan. On further bspec review I've noticed that the
backlight power sequencing values exist since the introduction of on-chip
panel power/lvds output control for the i855gm. Currently we ignore them
for lvds panels, too. I'm hoping a bit that this causes the occasional
"backlight doesn't light up after dpms on/out of resume" issue I sometimes
see on about all my laptops here.

So while you bang your head against this, can you add the backlight power
sequencing delays for lvds panels, too? The lvds panel power sequencing
should be imo safe - we just msleep-spin with wait_for until the power
sequencer has completed the request.

Daniel Vetter
Mail: daniel@xxxxxxxx
Mobile: +41 (0)79 365 57 48
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at