[PATCH] drm/i915: unconditionally unlock panel regs

[PATCH] drm/i915: unconditionally unlock panel regs

[Jesse Barnes]

In the panel_on function we skip everything if the panel is already
powered up.  However, if it's powered up but not unlocked, subsequent
register writes may fail.

So unlock the regs regardless of the panel state to allow other mode
setting programming to occur normally.

[Still waiting to hear from bug reporters & testers on this one, but
it does get my Vaio working again, with or without the eDP VBT patch
reverted.]

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f74a706..129d47a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -818,6 +818,11 @@ static bool ironlake_edp_panel_on (struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp, idle_on_mask = PP_ON | PP_SEQUENCE_STATE_ON_IDLE;
 
+	pp = I915_READ(PCH_PP_CONTROL);
+	pp |= PANEL_UNLOCK_REGS | POWER_TARGET_ON;
+	I915_WRITE(PCH_PP_CONTROL, pp);
+	POSTING_READ(PCH_PP_CONTROL);
+
 	if (I915_READ(PCH_PP_STATUS) & PP_ON)
 		return true;

Re: [PATCH] drm/i915: unconditionally unlock panel regs

[Chris Wilson]

On Tue, 14 Dec 2010 12:17:59 -0800, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> In the panel_on function we skip everything if the panel is already
> powered up.  However, if it's powered up but not unlocked, subsequent
> register writes may fail.
> 
> So unlock the regs regardless of the panel state to allow other mode
> setting programming to occur normally.

That looks to be consistent with other [ab]users of PP_CONTROL. Can you
tidy up the function to remove the duplicate code afterwards?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: unconditionally unlock panel regs

[Jesse Barnes]

On Tue, 14 Dec 2010 20:45:14 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, 14 Dec 2010 12:17:59 -0800, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > In the panel_on function we skip everything if the panel is already
> > powered up.  However, if it's powered up but not unlocked, subsequent
> > register writes may fail.
> > 
> > So unlock the regs regardless of the panel state to allow other mode
> > setting programming to occur normally.
> 
> That looks to be consistent with other [ab]users of PP_CONTROL. Can you
> tidy up the function to remove the duplicate code afterwards?
> -Chris

Sure, her eyou go.

-- 
Jesse Barnes, Intel Open Source Technology Center

>From 78b4ed4faf42b62de6d8a23dda8120c410488c82 Mon Sep 17 00:00:00 2001
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Tue, 14 Dec 2010 13:14:07 -0800
Subject: [PATCH] drm/i915: unconditionally unlock panel regs

In the panel_on function we skip everything if the panel is already
powered up.  However, if it's powered up but not unlocked, subsequent
register writes may fail.

So unlock the regs regardless of the panel state to allow other mode
setting programming to occur normally.

[Still waiting to hear from bug reporters & testers on this one, but
it does get my Vaio working again, with or without the eDP VBT patch
reverted.]

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f74a706..ddfd585 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -818,17 +818,21 @@ static bool ironlake_edp_panel_on (struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp, idle_on_mask = PP_ON | PP_SEQUENCE_STATE_ON_IDLE;
 
+	/* Always unlock the protected regs so other writes work */
+	pp = I915_READ(PCH_PP_CONTROL);
+	pp |= PANEL_UNLOCK_REGS;
+	I915_WRITE(PCH_PP_CONTROL, pp);
+	POSTING_READ(PCH_PP_CONTROL);
+
 	if (I915_READ(PCH_PP_STATUS) & PP_ON)
 		return true;
 
-	pp = I915_READ(PCH_PP_CONTROL);
-
 	/* ILK workaround: disable reset around power sequence */
 	pp &= ~PANEL_POWER_RESET;
 	I915_WRITE(PCH_PP_CONTROL, pp);
 	POSTING_READ(PCH_PP_CONTROL);
 
-	pp |= PANEL_UNLOCK_REGS | POWER_TARGET_ON;
+	pp |= POWER_TARGET_ON;
 	I915_WRITE(PCH_PP_CONTROL, pp);
 	POSTING_READ(PCH_PP_CONTROL);
 
-- 
1.7.0.4

Re: drm/i915: unconditionally unlock panel regs

[Jan-Hendrik Zab]

On 14/12/10 12:17 -0800, Jesse Barnes wrote:
> In the panel_on function we skip everything if the panel is already
> powered up.  However, if it's powered up but not unlocked, subsequent
> register writes may fail.
> 
> So unlock the regs regardless of the panel state to allow other mode
> setting programming to occur normally.
> 
> [Still waiting to hear from bug reporters & testers on this one, but
> it does get my Vaio working again, with or without the eDP VBT patch
> reverted.]

My Vaio VPCZ1 works with your patch in addition to the eDP VBT patch
(or rather the revert reverted). It does not work without the eDP VBT
patch.

The above is true for drm-intel-next and drm-intel-fixes.

	-jhz

-- 
http://jhz.name/blog

Re: drm/i915: unconditionally unlock panel regs

[Chris Wilson]

On Tue, 14 Dec 2010 23:15:28 +0100, Jan-Hendrik Zab <jan@jhz.name> wrote:
> My Vaio VPCZ1 works with your patch in addition to the eDP VBT patch
> (or rather the revert reverted). It does not work without the eDP VBT
> patch.
> 
> The above is true for drm-intel-next and drm-intel-fixes.

This kills the hope of a quick fix for .37. Hopefully, Jesse will find a
way to make all eDP laptops behave; I believe he is just waiting for a
couple of more test machines.

How many of these troublesome machines use SSC and so might be affected by
448f53a1ede54eb854d036abf54573281412d650 (drm/i915/bios: Reverse order of
100/120 Mhz SSC clocks)?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: unconditionally unlock panel regs

[Jim Gettys]

On 12/14/2010 04:15 PM, Jesse Barnes wrote:
> On Tue, 14 Dec 2010 20:45:14 +0000
> Chris Wilson<chris@chris-wilson.co.uk>  wrote:
>
>> >  On Tue, 14 Dec 2010 12:17:59 -0800, Jesse Barnes<jbarnes@virtuousgeek.org>  wrote:
>>> >  >  In the panel_on function we skip everything if the panel is already
>>> >  >  powered up.  However, if it's powered up but not unlocked, subsequent
>>> >  >  register writes may fail.
>>> >  >
>>> >  >  So unlock the regs regardless of the panel state to allow other mode
>>> >  >  setting programming to occur normally.
>> >
>> >  That looks to be consistent with other [ab]users of PP_CONTROL. Can you
>> >  tidy up the function to remove the duplicate code afterwards?
>> >  -Chris
> Sure, her eyou go.

Unfortunately, this patch does not seem to fix my intermittent boot 
problem on the HP 2540p.  I sort of wonder if Airlied and I have 
different BIOS's; I got mine very early on, and have not updated its 
BIOS.  This hypothesis would indicate some bit in some register isn't 
getting set.

                           - Jim

>
> -- Jesse Barnes, Intel Open Source Technology Center >From
> 78b4ed4faf42b62de6d8a23dda8120c410488c82 Mon Sep 17 00:00:00 2001 From:
> Jesse Barnes <jbarnes@virtuousgeek.org> Date: Tue, 14 Dec 2010 13:14:07
> -0800 Subject: [PATCH] drm/i915: unconditionally unlock panel regs In
> the panel_on function we skip everything if the panel is already powered
> up. However, if it's powered up but not unlocked, subsequent register
> writes may fail. So unlock the regs regardless of the panel state to
> allow other mode setting programming to occur normally. [Still waiting
> to hear from bug reporters & testers on this one, but it does get my
> Vaio working again, with or without the eDP VBT patch reverted.]
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> ---
> drivers/gpu/drm/i915/intel_dp.c | 10 +++++++--- 1 files changed, 7
> insertions(+), 3 deletions(-) diff --git
> a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f74a706..ddfd585 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++
> b/drivers/gpu/drm/i915/intel_dp.c @@ -818,17 +818,21 @@ static bool
> ironlake_edp_panel_on (struct intel_dp *intel_dp) struct
> drm_i915_private *dev_priv = dev->dev_private; u32 pp, idle_on_mask =
> PP_ON | PP_SEQUENCE_STATE_ON_IDLE; + /* Always unlock the protected regs
> so other writes work */ + pp = I915_READ(PCH_PP_CONTROL); + pp |=
> PANEL_UNLOCK_REGS; + I915_WRITE(PCH_PP_CONTROL, pp); +
> POSTING_READ(PCH_PP_CONTROL); + if (I915_READ(PCH_PP_STATUS) & PP_ON)
> return true; - pp = I915_READ(PCH_PP_CONTROL); - /* ILK workaround:
> disable reset around power sequence */ pp &= ~PANEL_POWER_RESET;
> I915_WRITE(PCH_PP_CONTROL, pp); POSTING_READ(PCH_PP_CONTROL); - pp |=
> PANEL_UNLOCK_REGS | POWER_TARGET_ON; + pp |= POWER_TARGET_ON;
> I915_WRITE(PCH_PP_CONTROL, pp); POSTING_READ(PCH_PP_CONTROL);
> -- 1.7.0.4 _______________________________________________ Intel-gfx
> mailing list Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx