drm/i915: Disabling unused outputs on CPT is broken

drm/i915: Disabling unused outputs on CPT is broken

[Keith Packard]

Here's a fun sequence:

$ xrandr --output LVDS1 --auto --crtc 1
$ sudo chvt 1

(then switch back to X)

The LVDS goes black. Why? The LVDS is *disabled*. Turns out that the
cause was that after the mode was all nicely set,
intel_disable_pch_ports went and turned it back off.

Well, a couple of weeks ago, we found that intel_disable_pch_ports was
doing the same thing to DP ports, and the cause was that the way to
tell which port a DP output was running from is different on CPT that
on every other chip. We fixed that, but didn't (foolishly) review the
non-DP code.

Oddly, the non-DP code had essentially the same bug:

 [PATCH 3/4] drm/i915: Fix PCH port pipe select in CPT disable paths

Also in this sequence is some code that validates the LVDS panel power
sequencing which discovered that the LVDS panel wasn't being disabled
correctly because the LVDS registers were locked in the DPMS
function. Instead of trying to get the LVDS locking right, I've just
unlocked the registers in the lvds initialization code and left them
unlocked after that:

 [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence
 [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

Finally, I removed an unused argument to dp_pipe_enable while I was
poking around in that code:

 [PATCH 4/4] drm/i915: Remove unused 'reg' argument to dp_pipe_enable

--
keith.packard@intel.com

[PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Keith Packard]

During mode setting, check to make sure the panel power sequencing has
completed before doing further operations on the device. This
uncovered errors with DPMS not turning the device off as it was left locked.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 2e8ddfc..6985e42 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -72,14 +72,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 ctl_reg, lvds_reg;
+	u32 ctl_reg, lvds_reg, stat_reg;
 
 	if (HAS_PCH_SPLIT(dev)) {
 		ctl_reg = PCH_PP_CONTROL;
 		lvds_reg = PCH_LVDS;
+		stat_reg = PCH_PP_STATUS;
 	} else {
 		ctl_reg = PP_CONTROL;
 		lvds_reg = LVDS;
+		stat_reg = PP_STATUS;
 	}
 
 	I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
@@ -105,6 +107,8 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
 	POSTING_READ(lvds_reg);
+	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
+		DRM_ERROR("timed out waiting for panel to power on\n");
 
 	intel_panel_enable_backlight(dev);
 }
@@ -113,19 +117,24 @@ static void intel_lvds_disable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 ctl_reg, lvds_reg;
+	u32 ctl_reg, lvds_reg, stat_reg;
 
 	if (HAS_PCH_SPLIT(dev)) {
 		ctl_reg = PCH_PP_CONTROL;
 		lvds_reg = PCH_LVDS;
+		stat_reg = PCH_PP_STATUS;
 	} else {
 		ctl_reg = PP_CONTROL;
 		lvds_reg = LVDS;
+		stat_reg = PP_STATUS;
 	}
 
 	intel_panel_disable_backlight(dev);
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
+	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
+		DRM_ERROR("timed out waiting for panel to power off status 0x%08x control 0x%08x\n",
+			  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
 
 	if (intel_lvds->pfit_control) {
 		if (wait_for((I915_READ(PP_STATUS) & PP_ON) == 0, 1000))
-- 
1.7.5.4

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Jesse Barnes]

On Sat,  6 Aug 2011 10:54:05 -0700
Keith Packard <keithp@keithp.com> wrote:

> During mode setting, check to make sure the panel power sequencing has
> completed before doing further operations on the device. This
> uncovered errors with DPMS not turning the device off as it was left locked.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 2e8ddfc..6985e42 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -72,14 +72,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  {
>  	struct drm_device *dev = intel_lvds->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 ctl_reg, lvds_reg;
> +	u32 ctl_reg, lvds_reg, stat_reg;
>  
>  	if (HAS_PCH_SPLIT(dev)) {
>  		ctl_reg = PCH_PP_CONTROL;
>  		lvds_reg = PCH_LVDS;
> +		stat_reg = PCH_PP_STATUS;
>  	} else {
>  		ctl_reg = PP_CONTROL;
>  		lvds_reg = LVDS;
> +		stat_reg = PP_STATUS;
>  	}
>  
>  	I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
> @@ -105,6 +107,8 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  
>  	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
>  	POSTING_READ(lvds_reg);
> +	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
> +		DRM_ERROR("timed out waiting for panel to power on\n");

Looks like maybe we want a small function for this...

>  	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
> +	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
> +		DRM_ERROR("timed out waiting for panel to power off status 0x%08x control 0x%08x\n",
> +			  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));

...to catch places like this where the wrong register gets used. :)


-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Keith Packard]

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

On Mon, 8 Aug 2011 09:27:19 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> ...to catch places like this where the wrong register gets used. :)

Ouch! There are only two places we *should* have these loops, one when
turning it off, another when turning it on. There's a couple of loops
which just need to be removed.

Here's a replacement patch:

From 436f2b19cf17c43e4d5ad55b47aeb3660c2af9b9 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 6 Aug 2011 10:30:45 -0700
Subject: [PATCH] drm/i915: Wait for LVDS panel power sequence

During mode setting, check to make sure the panel power sequencing has
completed before doing further operations on the device. This
uncovered errors with DPMS not turning the device off as it was left locked.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 2e8ddfc..6318828 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -72,14 +72,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 ctl_reg, lvds_reg;
+	u32 ctl_reg, lvds_reg, stat_reg;
 
 	if (HAS_PCH_SPLIT(dev)) {
 		ctl_reg = PCH_PP_CONTROL;
 		lvds_reg = PCH_LVDS;
+		stat_reg = PCH_PP_STATUS;
 	} else {
 		ctl_reg = PP_CONTROL;
 		lvds_reg = LVDS;
+		stat_reg = PP_STATUS;
 	}
 
 	I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
@@ -94,17 +96,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 		DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
 			      intel_lvds->pfit_control,
 			      intel_lvds->pfit_pgm_ratios);
-		if (wait_for((I915_READ(PP_STATUS) & PP_ON) == 0, 1000)) {
-			DRM_ERROR("timed out waiting for panel to power off\n");
-		} else {
-			I915_WRITE(PFIT_PGM_RATIOS, intel_lvds->pfit_pgm_ratios);
-			I915_WRITE(PFIT_CONTROL, intel_lvds->pfit_control);
-			intel_lvds->pfit_dirty = false;
-		}
+
+		I915_WRITE(PFIT_PGM_RATIOS, intel_lvds->pfit_pgm_ratios);
+		I915_WRITE(PFIT_CONTROL, intel_lvds->pfit_control);
+		intel_lvds->pfit_dirty = false;
 	}
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
 	POSTING_READ(lvds_reg);
+	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
+		DRM_ERROR("timed out waiting for panel to power on\n");
 
 	intel_panel_enable_backlight(dev);
 }
@@ -113,24 +114,25 @@ static void intel_lvds_disable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 ctl_reg, lvds_reg;
+	u32 ctl_reg, lvds_reg, stat_reg;
 
 	if (HAS_PCH_SPLIT(dev)) {
 		ctl_reg = PCH_PP_CONTROL;
 		lvds_reg = PCH_LVDS;
+		stat_reg = PCH_PP_STATUS;
 	} else {
 		ctl_reg = PP_CONTROL;
 		lvds_reg = LVDS;
+		stat_reg = PP_STATUS;
 	}
 
 	intel_panel_disable_backlight(dev);
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
+	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
+		DRM_ERROR("timed out waiting for panel to power off\n");
 
 	if (intel_lvds->pfit_control) {
-		if (wait_for((I915_READ(PP_STATUS) & PP_ON) == 0, 1000))
-			DRM_ERROR("timed out waiting for panel to power off\n");
-
 		I915_WRITE(PFIT_CONTROL, 0);
 		intel_lvds->pfit_dirty = true;
 	}
-- 
1.7.5.4

-- 
keith.packard@intel.com

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

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Jesse Barnes]

On Mon, 08 Aug 2011 11:40:06 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 8 Aug 2011 09:27:19 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > ...to catch places like this where the wrong register gets used. :)
> 
> Ouch! There are only two places we *should* have these loops, one when
> turning it off, another when turning it on. There's a couple of loops
> which just need to be removed.
> 

Yeah looks better.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

There's no reason to relock them; it just makes operations more
complex. This fixes DPMS where the panel registers were locked making
the disable not work.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   46 +++++++++++++++---------------------
 1 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6985e42..cc85618 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -414,46 +414,32 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
 	/* We try to do the minimum that is necessary in order to unlock
 	 * the registers for mode setting.
 	 *
-	 * On Ironlake, this is quite simple as we just set the unlock key
-	 * and ignore all subtleties. (This may cause some issues...)
+	 * On Ironlake, this is quite simple as we just set the unlock
+	 * key in the driver init code and ignore all subtleties.
+	 * (This may cause some issues...)
 	 *
 	 * Prior to Ironlake, we must disable the pipe if we want to adjust
 	 * the panel fitter. However at all other times we can just reset
 	 * the registers regardless.
 	 */
 
-	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(PCH_PP_CONTROL,
-			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
-	} else if (intel_lvds->pfit_dirty) {
-		I915_WRITE(PP_CONTROL,
-			   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
-			   & ~POWER_TARGET_ON);
-	} else {
-		I915_WRITE(PP_CONTROL,
-			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
+	if (!HAS_PCH_SPLIT(dev)) {
+		if (intel_lvds->pfit_dirty) {
+			I915_WRITE(PP_CONTROL,
+				   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
+				   & ~POWER_TARGET_ON);
+		} else {
+			I915_WRITE(PP_CONTROL,
+				   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
+		}
 	}
+	intel_lvds_disable(intel_lvds);
 }
 
 static void intel_lvds_commit(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* Undo any unlocking done in prepare to prevent accidental
-	 * adjustment of the registers.
-	 */
-	if (HAS_PCH_SPLIT(dev)) {
-		u32 val = I915_READ(PCH_PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PCH_PP_CONTROL, val & 0x3);
-	} else {
-		u32 val = I915_READ(PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PP_CONTROL, val & 0x3);
-	}
-
 	/* Always do a full power on as we do not know what state
 	 * we were left in.
 	 */
@@ -1049,6 +1035,12 @@ out:
 		pwm = I915_READ(BLC_PWM_PCH_CTL1);
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PCH_PP_CONTROL,
+			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
 	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
 	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {
-- 
1.7.5.4

Re: [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Jesse Barnes]

On Sat,  6 Aug 2011 10:54:06 -0700
Keith Packard <keithp@keithp.com> wrote:

> There's no reason to relock them; it just makes operations more
> complex. This fixes DPMS where the panel registers were locked making
> the disable not work.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Yep, looks fine.  The only think we might want to sprinkle about are
checks for panel off so we can avoid visible corruption if we whack
timing or fb stuff while the panel is on.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 518 bytes --]

On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

Yeah, could do. Would be nice to somehow get the LVDS code ripped out of
the middle of intel_display.c; everything in intel_lvds.c is nicely
bracketed by prepare/commit, which turn the panel off and back on.

-- 
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

Re: [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 590 bytes --]

On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

So, I'd like to know if we could unlock the panel registers on pre-PCH
hardware as well at init time; that way I could remove the unlock code
From intel_lvds_prepare too. Should it be possible to set the
PANEL_UNLOCK_REGS bits for pre-PCH hardware without having the target off?

-- 
keith.packard@intel.com

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

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Jesse Barnes]

On Mon, 08 Aug 2011 11:42:56 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Yep, looks fine.  The only think we might want to sprinkle about are
> > checks for panel off so we can avoid visible corruption if we whack
> > timing or fb stuff while the panel is on.
> 
> So, I'd like to know if we could unlock the panel registers on pre-PCH
> hardware as well at init time; that way I could remove the unlock code
> From intel_lvds_prepare too. Should it be possible to set the
> PANEL_UNLOCK_REGS bits for pre-PCH hardware without having the target off?

Yep, it's safe and possible to do on pre-PCH as well.  For panel
fitting we do need to do an actual power cycle when going from
non-native back to native iirc, but we can still leave them unlocked so
we don't have to worry about the lock/unlock sequence everywhere.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 4188 bytes --]

On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yep, it's safe and possible to do on pre-PCH as well.  For panel
> fitting we do need to do an actual power cycle when going from
> non-native back to native iirc, but we can still leave them unlocked so
> we don't have to worry about the lock/unlock sequence everywhere.

Hidden in the unlock patch was a call to intel_lvds_disable from
intel_lvds_prepare -- that *always* turns off the LVDS for mode
setting. Do we care enough about LVDS mode setting performance that we
should try leave the optimization in place that doesn't turn off the
backlight when switching between modes?

Here's a replacement which unlocks the regs at init time for all
generations. This also includes the unconditional call to
intel_lvds_disable in the _prepare function. I could back that out if
you like.

From c0f946bf41e49d9a10bcc0e4ae18a481bb8cdab3 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 6 Aug 2011 10:33:12 -0700
Subject: [PATCH 2/5] drm/i915: Leave LVDS registers unlocked

There's no reason to relock them; it just makes operations more
complex. This fixes DPMS where the panel registers were locked making
the disable not work.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   53 ++++++++++--------------------------
 1 files changed, 15 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6318828..03a9e6d 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -400,53 +400,17 @@ out:
 
 static void intel_lvds_prepare(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* We try to do the minimum that is necessary in order to unlock
-	 * the registers for mode setting.
-	 *
-	 * On Ironlake, this is quite simple as we just set the unlock key
-	 * and ignore all subtleties. (This may cause some issues...)
-	 *
-	 * Prior to Ironlake, we must disable the pipe if we want to adjust
-	 * the panel fitter. However at all other times we can just reset
-	 * the registers regardless.
+	/* Safest to just always power off for mode setting.
 	 */
-
-	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(PCH_PP_CONTROL,
-			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
-	} else if (intel_lvds->pfit_dirty) {
-		I915_WRITE(PP_CONTROL,
-			   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
-			   & ~POWER_TARGET_ON);
-	} else {
-		I915_WRITE(PP_CONTROL,
-			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
-	}
+	intel_lvds_disable(intel_lvds);
 }
 
 static void intel_lvds_commit(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* Undo any unlocking done in prepare to prevent accidental
-	 * adjustment of the registers.
-	 */
-	if (HAS_PCH_SPLIT(dev)) {
-		u32 val = I915_READ(PCH_PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PCH_PP_CONTROL, val & 0x3);
-	} else {
-		u32 val = I915_READ(PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PP_CONTROL, val & 0x3);
-	}
-
 	/* Always do a full power on as we do not know what state
 	 * we were left in.
 	 */
@@ -1042,6 +1006,19 @@ out:
 		pwm = I915_READ(BLC_PWM_PCH_CTL1);
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PCH_PP_CONTROL,
+			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
+	} else {
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PP_CONTROL,
+			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
 	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
 	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {
-- 
1.7.5.4



-- 
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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Jesse Barnes]

On Mon, 08 Aug 2011 12:53:31 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Yep, it's safe and possible to do on pre-PCH as well.  For panel
> > fitting we do need to do an actual power cycle when going from
> > non-native back to native iirc, but we can still leave them unlocked so
> > we don't have to worry about the lock/unlock sequence everywhere.
> 
> Hidden in the unlock patch was a call to intel_lvds_disable from
> intel_lvds_prepare -- that *always* turns off the LVDS for mode
> setting. Do we care enough about LVDS mode setting performance that we
> should try leave the optimization in place that doesn't turn off the
> backlight when switching between modes?

We hate flicker right?  But generally yes it's safer to just turn it
off all the time.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

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

On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 08 Aug 2011 12:53:31 -0700
> Keith Packard <keithp@keithp.com> wrote:
> 
> > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> > > Yep, it's safe and possible to do on pre-PCH as well.  For panel
> > > fitting we do need to do an actual power cycle when going from
> > > non-native back to native iirc, but we can still leave them unlocked so
> > > we don't have to worry about the lock/unlock sequence everywhere.
> > 
> > Hidden in the unlock patch was a call to intel_lvds_disable from
> > intel_lvds_prepare -- that *always* turns off the LVDS for mode
> > setting. Do we care enough about LVDS mode setting performance that we
> > should try leave the optimization in place that doesn't turn off the
> > backlight when switching between modes?
> 
> We hate flicker right?  But generally yes it's safer to just turn it
> off all the time.

I'll leave the optimization in place then; it's been there for a while
so at least it shouldn't cause any regressions.

How about this? Has the advantage of not lying in the commit message
anymore.

From 092719152aa5a235d6678798a34dc784d5cec2ad Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 6 Aug 2011 10:33:12 -0700
Subject: [PATCH 2/5] drm/i915: Leave LVDS registers unlocked

There's no reason to relock them; it just makes operations more
complex. This fixes DPMS where the panel registers were locked making
the disable not work.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   51 +++++++++++-------------------------
 1 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6318828..8b521a2 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -400,53 +400,21 @@ out:
 
 static void intel_lvds_prepare(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* We try to do the minimum that is necessary in order to unlock
-	 * the registers for mode setting.
-	 *
-	 * On Ironlake, this is quite simple as we just set the unlock key
-	 * and ignore all subtleties. (This may cause some issues...)
-	 *
+	/*
 	 * Prior to Ironlake, we must disable the pipe if we want to adjust
 	 * the panel fitter. However at all other times we can just reset
 	 * the registers regardless.
 	 */
-
-	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(PCH_PP_CONTROL,
-			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
-	} else if (intel_lvds->pfit_dirty) {
-		I915_WRITE(PP_CONTROL,
-			   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
-			   & ~POWER_TARGET_ON);
-	} else {
-		I915_WRITE(PP_CONTROL,
-			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
-	}
+	if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty)
+		intel_lvds_disable(intel_lvds);
 }
 
 static void intel_lvds_commit(struct drm_encoder *encoder)
 {
-	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/* Undo any unlocking done in prepare to prevent accidental
-	 * adjustment of the registers.
-	 */
-	if (HAS_PCH_SPLIT(dev)) {
-		u32 val = I915_READ(PCH_PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PCH_PP_CONTROL, val & 0x3);
-	} else {
-		u32 val = I915_READ(PP_CONTROL);
-		if ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS)
-			I915_WRITE(PP_CONTROL, val & 0x3);
-	}
-
 	/* Always do a full power on as we do not know what state
 	 * we were left in.
 	 */
@@ -1042,6 +1010,19 @@ out:
 		pwm = I915_READ(BLC_PWM_PCH_CTL1);
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PCH_PP_CONTROL,
+			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
+	} else {
+		/*
+		 * Unlock registers and just
+		 * leave them unlocked
+		 */
+		I915_WRITE(PP_CONTROL,
+			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
 	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
 	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {
-- 
1.7.5.4



-- 
keith.packard@intel.com

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Jesse Barnes]

On Mon, 08 Aug 2011 13:24:12 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Mon, 08 Aug 2011 12:53:31 -0700
> > Keith Packard <keithp@keithp.com> wrote:
> > 
> > > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > 
> > > > Yep, it's safe and possible to do on pre-PCH as well.  For panel
> > > > fitting we do need to do an actual power cycle when going from
> > > > non-native back to native iirc, but we can still leave them unlocked so
> > > > we don't have to worry about the lock/unlock sequence everywhere.
> > > 
> > > Hidden in the unlock patch was a call to intel_lvds_disable from
> > > intel_lvds_prepare -- that *always* turns off the LVDS for mode
> > > setting. Do we care enough about LVDS mode setting performance that we
> > > should try leave the optimization in place that doesn't turn off the
> > > backlight when switching between modes?
> > 
> > We hate flicker right?  But generally yes it's safer to just turn it
> > off all the time.
> 
> I'll leave the optimization in place then; it's been there for a while
> so at least it shouldn't cause any regressions.
> 
> How about this? Has the advantage of not lying in the commit message
> anymore.
> 
> From 092719152aa5a235d6678798a34dc784d5cec2ad Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@keithp.com>
> Date: Sat, 6 Aug 2011 10:33:12 -0700
> Subject: [PATCH 2/5] drm/i915: Leave LVDS registers unlocked
> 
> There's no reason to relock them; it just makes operations more
> complex. This fixes DPMS where the panel registers were locked making
> the disable not work.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Yeah looks like a nice improvement.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 3/4] drm/i915: Fix PCH port pipe select in CPT disable paths

[Keith Packard]

CPT pipe select is different from previous generations (using two bits
instead of one). All of the paths from intel_disable_pch_ports were
not making this distinction.

Mode setting with pipe A turned off would then also force all outputs
on pipe B to get turned off as the disable code would mistakenly
decide that all of these outputs were on pipe A and turn them off.

This is an extension of the CPT DP disable fix (why didn't I fix this then?)

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   13 ++-----
 drivers/gpu/drm/i915/intel_display.c |   60 ++++++++++++++++++++++++++++++---
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d1331f7..5baaef4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1318,6 +1318,7 @@
 #define   ADPA_PIPE_SELECT_MASK	(1<<30)
 #define   ADPA_PIPE_A_SELECT	0
 #define   ADPA_PIPE_B_SELECT	(1<<30)
+#define   ADPA_PIPE_SELECT(pipe) ((pipe) << 30)
 #define   ADPA_USE_VGA_HVPOLARITY (1<<15)
 #define   ADPA_SETS_HVPOLARITY	0
 #define   ADPA_VSYNC_CNTL_DISABLE (1<<11)
@@ -1460,6 +1461,7 @@
 /* Selects pipe B for LVDS data.  Must be set on pre-965. */
 #define   LVDS_PIPEB_SELECT		(1 << 30)
 #define   LVDS_PIPE_MASK		(1 << 30)
+#define   LVDS_PIPE(pipe)		((pipe) << 30)
 /* LVDS dithering flag on 965/g4x platform */
 #define   LVDS_ENABLE_DITHER		(1 << 25)
 /* LVDS sync polarity flags. Set to invert (i.e. negative) */
@@ -1499,9 +1501,6 @@
 #define   LVDS_B0B3_POWER_DOWN		(0 << 2)
 #define   LVDS_B0B3_POWER_UP		(3 << 2)
 
-#define LVDS_PIPE_ENABLED(V, P) \
-	(((V) & (LVDS_PIPE_MASK | LVDS_PORT_EN)) == ((P) << 30 | LVDS_PORT_EN))
-
 /* Video Data Island Packet control */
 #define VIDEO_DIP_DATA		0x61178
 #define VIDEO_DIP_CTL		0x61170
@@ -3256,14 +3255,12 @@
 #define  ADPA_CRT_HOTPLUG_VOLREF_475MV  (1<<17)
 #define  ADPA_CRT_HOTPLUG_FORCE_TRIGGER (1<<16)
 
-#define ADPA_PIPE_ENABLED(V, P) \
-	(((V) & (ADPA_TRANS_SELECT_MASK | ADPA_DAC_ENABLE)) == ((P) << 30 | ADPA_DAC_ENABLE))
-
 /* or SDVOB */
 #define HDMIB   0xe1140
 #define  PORT_ENABLE    (1 << 31)
 #define  TRANSCODER_A   (0)
 #define  TRANSCODER_B   (1 << 30)
+#define  TRANSCODER(pipe)	((pipe) << 30)
 #define  TRANSCODER_MASK   (1 << 30)
 #define  COLOR_FORMAT_8bpc      (0)
 #define  COLOR_FORMAT_12bpc     (3 << 26)
@@ -3280,9 +3277,6 @@
 #define  HSYNC_ACTIVE_HIGH      (1 << 3)
 #define  PORT_DETECTED          (1 << 2)
 
-#define HDMI_PIPE_ENABLED(V, P) \
-	(((V) & (TRANSCODER_MASK | PORT_ENABLE)) == ((P) << 30 | PORT_ENABLE))
-
 /* PCH SDVOB multiplex with HDMIB */
 #define PCH_SDVOB	HDMIB
 
@@ -3349,6 +3343,7 @@
 #define  PORT_TRANS_B_SEL_CPT	(1<<29)
 #define  PORT_TRANS_C_SEL_CPT	(2<<29)
 #define  PORT_TRANS_SEL_MASK	(3<<29)
+#define  PORT_TRANS_SEL_CPT(pipe)	((pipe) << 29)
 
 #define TRANS_DP_CTL_A		0xe0300
 #define TRANS_DP_CTL_B		0xe1300
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35364e6..4c4c903 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -998,6 +998,53 @@ static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
 	return true;
 }
 
+static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
+			      enum pipe pipe, u32 val)
+{
+	if ((val & PORT_ENABLE) == 0)
+		return false;
+
+	if (HAS_PCH_CPT(dev_priv->dev)) {
+		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
+			return false;
+	} else {
+		if ((val & TRANSCODER_MASK) != TRANSCODER(pipe))
+			return false;
+	}
+	return true;
+}
+
+static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
+			      enum pipe pipe, u32 val)
+{
+	if ((val & LVDS_PORT_EN) == 0)
+		return false;
+
+	if (HAS_PCH_CPT(dev_priv->dev)) {
+		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
+			return false;
+	} else {
+		if ((val & LVDS_PIPE_MASK) != LVDS_PIPE(pipe))
+			return false;
+	}
+	return true;
+}
+
+static bool adpa_pipe_enabled(struct drm_i915_private *dev_priv,
+			      enum pipe pipe, u32 val)
+{
+	if ((val & ADPA_DAC_ENABLE) == 0)
+		return false;
+	if (HAS_PCH_CPT(dev_priv->dev)) {
+		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
+			return false;
+	} else {
+		if ((val & ADPA_PIPE_SELECT_MASK) != ADPA_PIPE_SELECT(pipe))
+			return false;
+	}
+	return true;
+}
+
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe, int reg, u32 port_sel)
 {
@@ -1011,7 +1058,7 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
 				     enum pipe pipe, int reg)
 {
 	u32 val = I915_READ(reg);
-	WARN(HDMI_PIPE_ENABLED(val, pipe),
+	WARN(hdmi_pipe_enabled(dev_priv, val, pipe),
 	     "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
 	     reg, pipe_name(pipe));
 }
@@ -1028,13 +1075,13 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 
 	reg = PCH_ADPA;
 	val = I915_READ(reg);
-	WARN(ADPA_PIPE_ENABLED(val, pipe),
+	WARN(adpa_pipe_enabled(dev_priv, val, pipe),
 	     "PCH VGA enabled on transcoder %c, should be disabled\n",
 	     pipe_name(pipe));
 
 	reg = PCH_LVDS;
 	val = I915_READ(reg);
-	WARN(LVDS_PIPE_ENABLED(val, pipe),
+	WARN(lvds_pipe_enabled(dev_priv, val, pipe),
 	     "PCH LVDS enabled on transcoder %c, should be disabled\n",
 	     pipe_name(pipe));
 
@@ -1370,7 +1417,7 @@ static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
 			     enum pipe pipe, int reg)
 {
 	u32 val = I915_READ(reg);
-	if (HDMI_PIPE_ENABLED(val, pipe)) {
+	if (hdmi_pipe_enabled(dev_priv, val, pipe)) {
 		DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n",
 			      reg, pipe);
 		I915_WRITE(reg, val & ~PORT_ENABLE);
@@ -1392,12 +1439,13 @@ static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
 
 	reg = PCH_ADPA;
 	val = I915_READ(reg);
-	if (ADPA_PIPE_ENABLED(val, pipe))
+	if (adpa_pipe_enabled(dev_priv, val, pipe))
 		I915_WRITE(reg, val & ~ADPA_DAC_ENABLE);
 
 	reg = PCH_LVDS;
 	val = I915_READ(reg);
-	if (LVDS_PIPE_ENABLED(val, pipe)) {
+	if (lvds_pipe_enabled(dev_priv, val, pipe)) {
+		DRM_DEBUG_KMS("disable lvds on pipe %d val 0x%08x\n", pipe, val);
 		I915_WRITE(reg, val & ~LVDS_PORT_EN);
 		POSTING_READ(reg);
 		udelay(100);
-- 
1.7.5.4

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Fix PCH port pipe select in CPT disable paths

[Jesse Barnes]

On Sat,  6 Aug 2011 10:54:07 -0700
Keith Packard <keithp@keithp.com> wrote:

> CPT pipe select is different from previous generations (using two bits
> instead of one). All of the paths from intel_disable_pch_ports were
> not making this distinction.
> 
> Mode setting with pipe A turned off would then also force all outputs
> on pipe B to get turned off as the disable code would mistakenly
> decide that all of these outputs were on pipe A and turn them off.
> 
> This is an extension of the CPT DP disable fix (why didn't I fix this then?)
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   13 ++-----
>  drivers/gpu/drm/i915/intel_display.c |   60 ++++++++++++++++++++++++++++++---
>  2 files changed, 58 insertions(+), 15 deletions(-)
> 

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 4/4] drm/i915: Remove unused 'reg' argument to dp_pipe_enabled

[Keith Packard]

Just an extra parameter which isn't actually needed.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4c4c903..f6f18c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -980,8 +980,8 @@ static void assert_transcoder_disabled(struct drm_i915_private *dev_priv,
 	     pipe_name(pipe));
 }
 
-static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
-			    int reg, u32 port_sel, u32 val)
+static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
+			    enum pipe pipe, u32 port_sel, u32 val)
 {
 	if ((val & DP_PORT_EN) == 0)
 		return false;
@@ -1049,7 +1049,7 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe, int reg, u32 port_sel)
 {
 	u32 val = I915_READ(reg);
-	WARN(dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val),
+	WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val),
 	     "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
 	     reg, pipe_name(pipe));
 }
@@ -1407,7 +1407,7 @@ static void disable_pch_dp(struct drm_i915_private *dev_priv,
 			   enum pipe pipe, int reg, u32 port_sel)
 {
 	u32 val = I915_READ(reg);
-	if (dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val)) {
+	if (dp_pipe_enabled(dev_priv, pipe, port_sel, val)) {
 		DRM_DEBUG_KMS("Disabling pch dp %x on pipe %d\n", reg, pipe);
 		I915_WRITE(reg, val & ~DP_PORT_EN);
 	}
-- 
1.7.5.4

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Remove unused 'reg' argument to dp_pipe_enabled

[Jesse Barnes]

On Sat,  6 Aug 2011 10:54:08 -0700
Keith Packard <keithp@keithp.com> wrote:

> Just an extra parameter which isn't actually needed.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---

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

-- 
Jesse Barnes, Intel Open Source Technology Center