All of lore.kernel.org
 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 --]

WARNING: multiple messages have this Message-ID (diff)
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.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 #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Thread overview: 114+ 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 ` 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  3:50     ` Jesse Barnes
2011-09-21  4:45     ` [Intel-gfx] " Keith Packard
2011-09-23  2:55       ` Jesse Barnes
2011-09-23  2:55         ` Jesse Barnes
2011-09-23  5:07         ` Keith Packard [this message]
2011-09-23  5:07           ` [Intel-gfx] " Keith Packard
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-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  1:09             ` Keith Packard
2011-09-30 16:27             ` Daniel Vetter
2011-09-30 17:50               ` Keith Packard
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:09                     ` Daniel Vetter
2011-09-30 18:28                     ` Keith Packard
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  1:09             ` 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-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-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 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.