linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Keith Packard <keithp@keithp.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@redhat.com>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel
Date: Thu, 22 Sep 2011 22:07:00 -0700	[thread overview]
Message-ID: <yunaa9v26ff.fsf@aiko.keithp.com> (raw)
In-Reply-To: <20110923082513.735b64fe@jbarnes-x220>

[-- Attachment #1: Type: text/plain, Size: 3048 bytes --]

On Fri, 23 Sep 2011 08:25:13 +0530, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yeah that sounds good.  (2) and (3) are ok cleanups, but it would be
> best if they were a separate patch just in case the subtle timing
> change breaks the panel power sequencing state machine.

Ok, I'll split things up into tiny patches then; easier to review, and
easier to try different combinations.

> No I think:
>   1) VDD AUX override on
>   2) PPS on
>   3) (delay)
>   4) VDD AUX override off
> is safe, I'm just worried about the timing of step (3).

Ok, that seems like a nice plan to me -- just doing the PPS on sequence,
and then setting VDD AUX off after PPS completes. That seems easy, and
avoids leaving VDD AUX once the panel is completely running.

> To fix both PCH eDP and CPU eDP in the past, I did have a version that
> only used full PPS and not VDD AUX override.  So it is possible, but
> VDD AUX is a little cleaner since it allows us to keep the registers
> locked potentially (theoretically we only actually want to unlock to
> handle CPU eDP PLL enable/disable bugs and flicker-free panel fitter
> downscaling).

I really don't care about leaving registers locked; we're not running
DOS anymore. Just doesn't make any sense to me.

> But since we unlock unconditionally now, using full PPS would be ok.

> Though we will have to shut it down still; PPS on to get AUX data and
> EDID, then off while we program the mode and train DP, then PPS on
> again.  So I'm not sure it would save much.

VDD AUX on or PPS has to be done to run the training anyways.

> Yeah, I'm liking your delayed work much better now...  Bring up the
> panel early and then just modify the shutdown timeout at various points
> to make sure it stays up from module_init all the way through the final
> mode set (so an initial timeout of 2s or so would probably be
> sufficient).

The question is whether to do PPS or use VDD AUX on to start
with.

> Another potential optimization is to start trying AUX & i2c
> transactions right after we apply VDD AUX override.  The panel will
> come up much faster than the T* values imply most of the time (varies
> by panel).  And polling the bits can get us into the actual panel
> poking code much faster.

While that's possible, it wasn't true on the MBA; it took almost the
full T3 interval after VDD AUX was on before the EDID came back.

> But I think the bottom line is to fix the EDID read (make sure the
> panel is on) and the i2c stuff.  Everything else is just tasty
> gravy. :)

Right, that is what fixed the MBA for me.

> Also I think the change to prefer EDID over VBT is correct; afaik eDP
> panels are required to have an EDID, so trusting that data over some
> potentially untested VBT data is the right way to go.

Especially when VBT comes from a BIOS emulation mess which just lies.

I'll try to get a new sequence done tomorrow with these
suggestions. Hope your Indian adventures are going well.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2011-09-23  5:07 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-19 22:21 drm/i915: eDP cleanup patch series -- fixes SNB MacBook Air Keith Packard
2011-09-19 22:21 ` [PATCH 1/9] drm/i915: Enable digital port hotplug on PCH systems Keith Packard
2011-09-19 22:21 ` [PATCH 2/9] drm/i915: Remove extra 300ms delay during eDP mode setting Keith Packard
2011-09-19 22:21 ` [PATCH 3/9] drm/i915: Only use VBT panel mode on eDP if no EDID is found Keith Packard
2011-09-19 22:21 ` [PATCH 4/9] drm/i915: Check eDP power when doing aux channel communications Keith Packard
2011-09-19 22:21 ` [PATCH 5/9] drm/i915: Unlock PCH_PP_CONTROL always Keith Packard
2011-09-19 22:22 ` [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel Keith Packard
2011-09-21  3:50   ` [Intel-gfx] " Jesse Barnes
2011-09-21  4:45     ` Keith Packard
2011-09-23  2:55       ` Jesse Barnes
2011-09-23  5:07         ` Keith Packard [this message]
2011-09-19 22:22 ` [PATCH 7/9] drm/i915: Correct eDP panel power sequencing delay computations Keith Packard
2011-09-19 22:22 ` [PATCH 8/9] drm/i915: Move eDP panel fixed mode from dev_priv to intel_dp Keith Packard
2011-09-19 22:22 ` [PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously Keith Packard
2011-09-21  4:17   ` [Intel-gfx] " Jesse Barnes
2011-09-21  4:51     ` Keith Packard
2011-09-23  3:22       ` Jesse Barnes
2011-09-30  1:09         ` [PATCH 00/24] MacBook Air patch sequence (v2) Keith Packard
2011-09-30  1:09           ` [PATCH 01/21] drm/i915: Enable digital port hotplug on PCH systems Keith Packard
2011-09-30 16:17             ` [Intel-gfx] " Daniel Vetter
2011-10-03 20:34               ` Jesse Barnes
2011-09-30  1:09           ` [PATCH 02/21] drm/i915: Shut down PCH interrupts during irq_uninstall Keith Packard
2011-09-30 16:20             ` Daniel Vetter
2011-09-30 17:44               ` Keith Packard
2011-09-30 17:56                 ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 03/21] drm/i915: Remove extra 300ms delay during eDP mode setting Keith Packard
2011-09-30 16:27             ` Daniel Vetter
2011-09-30 17:50               ` Keith Packard
2011-09-30 17:58                 ` Daniel Vetter
2011-09-30 18:09                   ` Daniel Vetter
2011-09-30 18:28                     ` Keith Packard
2011-09-30  1:09           ` [PATCH 04/21] drm/i915: Only use VBT panel mode on eDP if no EDID is found Keith Packard
2011-09-30 16:32             ` Daniel Vetter
2011-09-30 17:58               ` Keith Packard
2011-10-03 20:42                 ` [Intel-gfx] " Jesse Barnes
2011-09-30  1:09           ` [PATCH 05/21] drm/i915: Check eDP power when doing aux channel communications Keith Packard
2011-09-30 17:02             ` [Intel-gfx] " Daniel Vetter
2011-09-30 18:01               ` Keith Packard
2011-09-30 18:11                 ` Daniel Vetter
2011-09-30 18:23                 ` Chris Wilson
2011-10-03 20:48             ` Jesse Barnes
2011-09-30  1:09           ` [PATCH 06/21] drm/i915: Unlock PCH_PP_CONTROL always Keith Packard
2011-09-30 17:09             ` [Intel-gfx] " Daniel Vetter
2011-09-30 18:01               ` Keith Packard
2011-09-30 23:14               ` Keith Packard
2011-10-01  9:35                 ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 07/21] drm/i915: Check for eDP inside intel_edp_panel_vdd_on/off Keith Packard
2011-09-30 17:13             ` [Intel-gfx] " Daniel Vetter
2011-09-30 18:02               ` Keith Packard
2011-09-30  1:09           ` [PATCH 08/21] drm/i915: Turn force VDD back off when panel running in intel_dp_dpms Keith Packard
2011-09-30 17:15             ` [Intel-gfx] " Daniel Vetter
2011-09-30  1:09           ` [PATCH 09/21] drm/i915: Delay DP i2c initialization until panel power timings are computed Keith Packard
2011-09-30 17:25             ` [Intel-gfx] " Daniel Vetter
2011-09-30  1:09           ` [PATCH 10/21] drm/i915: Wrap DP EDID fetch functions to enable eDP panel power Keith Packard
2011-09-30 17:32             ` Daniel Vetter
2011-10-03 20:59             ` [Intel-gfx] " Jesse Barnes
2011-09-30  1:09           ` [PATCH 11/21] drm/i915: Enable eDP panel power during I2C initialization sequence Keith Packard
2011-09-30 17:26             ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 12/21] drm/i915: Ensure eDP powered up during DP_SET_POWER operation in dp_prepare Keith Packard
2011-09-30 17:45             ` Daniel Vetter
2011-09-30 18:30               ` Keith Packard
2011-09-30  1:09           ` [PATCH 13/21] drm/i915: Place long delays after each eDP VDD operation Keith Packard
2011-09-30 18:01             ` [Intel-gfx] " Daniel Vetter
2011-09-30 18:31               ` Keith Packard
2011-09-30  1:09           ` [PATCH 14/21] drm/i915: Correct eDP panel power sequencing delay computations Keith Packard
2011-09-30 18:16             ` Daniel Vetter
2011-09-30 18:33               ` Keith Packard
2011-10-01  3:31               ` Keith Packard
2011-10-01  9:59                 ` Daniel Vetter
2011-10-01 19:01                   ` Keith Packard
2011-10-03 10:10                     ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 15/21] drm/i915: Move eDP panel fixed mode from dev_priv to intel_dp Keith Packard
2011-09-30 18:20             ` [Intel-gfx] " Daniel Vetter
2011-09-30  1:09           ` [PATCH 16/21] drm/i915: edp_panel_on does not need to return a bool Keith Packard
2011-09-30 18:21             ` Daniel Vetter
2011-10-03 21:03             ` [Intel-gfx] " Jesse Barnes
2011-09-30  1:09           ` [PATCH 17/21] drm/i915: Create helper functions to determine eDP power state Keith Packard
2011-09-30 18:26             ` [Intel-gfx] " Daniel Vetter
2011-09-30  1:09           ` [PATCH 18/21] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously Keith Packard
2011-09-30 10:31             ` Chris Wilson
2011-09-30 18:06               ` Keith Packard
2011-09-30 18:47             ` Daniel Vetter
2011-09-30 20:56               ` Keith Packard
2011-09-30 22:01                 ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 19/21] drm/i915: Asynchronous eDP panel power off Keith Packard
2011-09-30 18:55             ` Daniel Vetter
2011-09-30 20:57               ` Keith Packard
2011-10-12 14:41             ` Dave Airlie
2011-10-12 16:43               ` Keith Packard
2011-09-30  1:09           ` [PATCH 20/21] drm/i915: Restrict ILK-specific eDP power hack to ILK Keith Packard
2011-09-30 18:57             ` Daniel Vetter
2011-09-30  1:09           ` [PATCH 21/21] drm/i915: No need to wait for eDP power off delay if panel is on Keith Packard
2011-09-30 19:01             ` Daniel Vetter
2011-09-30  3:33           ` [PATCH 00/24] MacBook Air patch sequence (v2) Greg KH
2011-09-30  8:58             ` Keith Packard
2011-09-30 13:57               ` Greg KH
2011-09-30 18:10                 ` Keith Packard
2011-09-30 13:20             ` Ted Ts'o
2011-09-30 18:17               ` Keith Packard
2011-10-03 21:06                 ` [Intel-gfx] " Jesse Barnes
2011-10-11  8:04           ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yunaa9v26ff.fsf@aiko.keithp.com \
    --to=keithp@keithp.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).