linux-kernel - Re: [Intel-gfx] [PATCH 5/7] drm/i915: Make DP prepare/commit consistent with DP dpms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111103154125.1fdaff5c@jbarnes-desktop>
Date:	Thu, 3 Nov 2011 15:41:25 -0700
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Keith Packard <keithp@...thp.com>
Cc:	intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 5/7] drm/i915: Make DP prepare/commit
 consistent with DP dpms

On Thu, 03 Nov 2011 15:30:57 -0700
Keith Packard <keithp@...thp.com> wrote:

> On Thu, 3 Nov 2011 13:00:11 -0700, Jesse Barnes <jbarnes@...tuousgeek.org> wrote:
> 
> > A few comments on this one (also, is it strictly required to fix your
> > bug)?
> 
> I think so; the panel definitely was not happy when I turned the link
> off while the panel power was on. Having the mode setting and DPMS paths
> doing things in different orders definitely doesn't seem reasonable
> though. I nearly managed to share code between the two paths, but there
> are subtle differences in requirements and so I didn't bother.
> 
> > Ok so you're making sure the panel has power to down the link, I think
> > that's fine.
> 
> No, I'm turning the panel off *before* turning off the link.  The panel
> goes nuts if you down the link before turning its power off; lots of
> technicolor.

Except for VDD??  That does come on... and shouldn't be any different
than a full power sequence as far as link training etc go...

> Downing the link doesn't require any communication with the panel, so
> there's no issue with losing connectivity at this point:
> 
> 	ironlake_edp_backlight_off(intel_dp);
> 	ironlake_edp_panel_off(intel_dp);               <= panel off
> 
> 	/* Wake up the sink first */
> 	ironlake_edp_panel_vdd_on(intel_dp);
> 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> 	intel_dp_link_down(intel_dp);                   <= link down
> 	ironlake_edp_panel_vdd_off(intel_dp, false);
> 
> > But here it looks like you're shutting it off, then downing the link?
> > Should this be reordered?
> 
> Nope, it's in the same order:
> 
> 	ironlake_edp_backlight_off(intel_dp);
> 	ironlake_edp_panel_off(intel_dp);               <= panel off
> 
> 	ironlake_edp_panel_vdd_on(intel_dp);
> 	intel_dp_sink_dpms(intel_dp, mode);
> 	intel_dp_link_down(intel_dp);                   <= link down
> 	ironlake_edp_panel_vdd_off(intel_dp, false);
> 
> The only difference in these two code paths is that dp_prepare turns the
> sink to DPMS_ON, while dp_dpms turns the sink to DPMS_OFF.

Oh missed the vdd on, which is in this path too...  So I'm still
confused by the panel off, vdd on sequence, but at least they're
consistent.

-- 
Jesse Barnes, Intel Open Source Technology Center

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)