[PATCH 1/4] drm/i915: pnv has a backlight polarity control bit, too

[PATCH 1/4] drm/i915: pnv has a backlight polarity control bit, too

[Daniel Vetter]

We already correctly ignore bit0 on gen < 4, now we also now why ;-)
I've decided that losing that single bit of precision isn't worth the
trouble to sprinkle IS_PINEVIEW checks all over the backlight control
code - that code is way too fragile imo.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dcc04f..20244b9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1828,6 +1828,8 @@
  */
 #define   BACKLIGHT_DUTY_CYCLE_SHIFT		(0)
 #define   BACKLIGHT_DUTY_CYCLE_MASK		(0xffff)
+#define   BACKLIGHT_DUTY_CYCLE_MASK_PNV		(0xfffe)
+#define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
 
 #define BLC_HIST_CTL		0x61260
 
-- 
1.7.7.6

[PATCH 2/4] drm/i915: clear up backlight #define confusion on gen4+

[Daniel Vetter]

- Regroup definitions for BLC_PWM_CTL so that they're all together and
  and ordered according to the bitfields.

- Add all missing defintions for BLC_PWM_CTL2.

- Use the BLM_ (for backlight modulation) prefix consistently.

- Note that combination mode (i.e. also taking the legacy backlight
  control value from pci config space into account) is gen4 only.

- Move the new registers for PCH-split machines up, they're an almost
  match for the gen4 defitions.  Prefix the special PCH-only bits with
  BLM_PCH_. Also add the pipe C select bit for ivb.

- Rip out the second pair of PCH polarity definitions - they're only
  valid on early (pre-production) ilk silicon.

- Adapt the existing code to use the new definitions. This has the
  nice benefit of killing a magic (1 << 30) left behind be Jesse
  Barnes.

No functional changes in this patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |   55 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_display.c |    4 +-
 drivers/gpu/drm/i915/intel_lvds.c    |   12 ++++----
 3 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 20244b9..c4b1a2b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1807,18 +1807,35 @@
 #define PFIT_AUTO_RATIOS 0x61238
 
 /* Backlight control */
-#define BLC_PWM_CTL		0x61254
-#define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
-#define   BLM_COMBINATION_MODE (1 << 30)
+#define   BLM_PWM_ENABLE		(1 << 31)
+#define   BLM_COMBINATION_MODE		(1 << 30) /* gen4 only */
+#define   BLM_PIPE_SELECT		(1 << 29)
+#define   BLM_PIPE_SELECT_IVB		(3 << 29)
+#define   BLM_PIPE_A			(0 << 29)
+#define   BLM_PIPE_B			(1 << 29)
+#define   BLM_PIPE_C			(2 << 29) /* ivb + */
+#define   BLM_PIPE(pipe)		((pipe) << 29)
+#define   BLM_POLARITY_I965		(1 << 28) /* gen4 only */
+#define   BLM_PHASE_IN_INTERUPT_STATUS	(1 << 26)
+#define   BLM_PHASE_IN_ENABLE		(1 << 25)
+#define   BLM_PHASE_IN_INTERUPT_ENABL	(1 << 24)
+#define   BLM_PHASE_IN_TIME_BASE_SHIFT	(16)
+#define   BLM_PHASE_IN_TIME_BASE_MASK	(0xff << 16)
+#define   BLM_PHASE_IN_COUNT_SHIFT	(8)
+#define   BLM_PHASE_IN_COUNT_MASK	(0xff << 8)
+#define   BLM_PHASE_IN_INCR_SHIFT	(0)
+#define   BLM_PHASE_IN_INCR_MASK	(0xff << 0)
+#define BLC_PWM_CTL		0x61254
 /*
  * This is the most significant 15 bits of the number of backlight cycles in a
  * complete cycle of the modulated backlight control.
  *
  * The actual value is this field multiplied by two.
  */
-#define   BACKLIGHT_MODULATION_FREQ_MASK		(0x7fff << 17)
-#define   BLM_LEGACY_MODE				(1 << 16)
+#define   BACKLIGHT_MODULATION_FREQ_SHIFT	(17)
+#define   BACKLIGHT_MODULATION_FREQ_MASK	(0x7fff << 17)
+#define   BLM_LEGACY_MODE			(1 << 16) /* gen2 only */
 /*
  * This is the number of cycles out of the backlight modulation cycle for which
  * the backlight is on.
@@ -1833,6 +1850,19 @@
 
 #define BLC_HIST_CTL		0x61260
 
+/* New registers for PCH-split platforms. Safe where new bits show up, the
+ * register layout machtes with gen4 BLC_PWM_CTL[12]. */
+#define BLC_PWM_CPU_CTL2	0x48250
+#define BLC_PWM_CPU_CTL		0x48254
+
+/* PCH CTL1 is totally different, all but the below bits are reserved. CTL2 is
+ * like the normal CTL from gen4 and earlier. Hooray for confusing naming. */
+#define BLC_PWM_PCH_CTL1	0xc8250
+#define   BLM_PCH_PWM_ENABLE			(1 << 30)
+#define   BLM_PCH_OVERRIDE_ENABLE		(1 << 30)
+#define   BLM_PCH_POLARITY			(1 << 29)
+#define BLC_PWM_PCH_CTL2	0xc8254
+
 /* TV port control */
 #define TV_CTL			0x68000
 /** Enables the TV encoder */
@@ -3840,21 +3870,6 @@
 #define PCH_LVDS	0xe1180
 #define  LVDS_DETECTED	(1 << 1)
 
-#define BLC_PWM_CPU_CTL2	0x48250
-#define  PWM_ENABLE		(1 << 31)
-#define  PWM_PIPE_A		(0 << 29)
-#define  PWM_PIPE_B		(1 << 29)
-#define BLC_PWM_CPU_CTL		0x48254
-
-#define BLC_PWM_PCH_CTL1	0xc8250
-#define  PWM_PCH_ENABLE		(1 << 31)
-#define  PWM_POLARITY_ACTIVE_LOW	(1 << 29)
-#define  PWM_POLARITY_ACTIVE_HIGH	(0 << 29)
-#define  PWM_POLARITY_ACTIVE_LOW2	(1 << 28)
-#define  PWM_POLARITY_ACTIVE_HIGH2	(0 << 28)
-
-#define BLC_PWM_PCH_CTL2	0xc8254
-
 #define PCH_PP_STATUS		0xc7200
 #define PCH_PP_CONTROL		0xc7204
 #define  PANEL_UNLOCK_REGS	(0xabcd << 16)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f5148ac..5cc1e9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6889,9 +6889,9 @@ static void ivb_pch_pwm_override(struct drm_device *dev)
 	 * IVB has CPU eDP backlight regs too, set things up to let the
 	 * PCH regs control the backlight
 	 */
-	I915_WRITE(BLC_PWM_CPU_CTL2, PWM_ENABLE);
+	I915_WRITE(BLC_PWM_CPU_CTL2, BLM_PWM_ENABLE);
 	I915_WRITE(BLC_PWM_CPU_CTL, 0);
-	I915_WRITE(BLC_PWM_PCH_CTL1, PWM_ENABLE | (1<<30));
+	I915_WRITE(BLC_PWM_PCH_CTL1, BLM_PCH_PWM_ENABLE | BLM_PCH_OVERRIDE_ENABLE);
 }
 
 void intel_modeset_init_hw(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 08eb04c..a7269e6 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1081,16 +1081,16 @@ out:
 
 		/* make sure PWM is enabled and locked to the LVDS pipe */
 		pwm = I915_READ(BLC_PWM_CPU_CTL2);
-		if (pipe == 0 && (pwm & PWM_PIPE_B))
-			I915_WRITE(BLC_PWM_CPU_CTL2, pwm & ~PWM_ENABLE);
+		if (pipe == 0 && (pwm & BLM_PIPE_B))
+			I915_WRITE(BLC_PWM_CPU_CTL2, pwm & ~BLM_PWM_ENABLE);
 		if (pipe)
-			pwm |= PWM_PIPE_B;
+			pwm |= BLM_PIPE_B;
 		else
-			pwm &= ~PWM_PIPE_B;
-		I915_WRITE(BLC_PWM_CPU_CTL2, pwm | PWM_ENABLE);
+			pwm &= ~BLM_PIPE_B;
+		I915_WRITE(BLC_PWM_CPU_CTL2, pwm | BLM_PWM_ENABLE);
 
 		pwm = I915_READ(BLC_PWM_PCH_CTL1);
-		pwm |= PWM_PCH_ENABLE;
+		pwm |= BLM_PCH_PWM_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
 		/*
 		 * Unlock registers and just
-- 
1.7.7.6

[PATCH 3/4] drm/i915: properly enable the blc controller on the right pipe

[Daniel Vetter]

On gen4+ we have a bitfield to specify from which pipe the backlight
controller should take it's clock. For PCH split platforms we've
already set these up, but only at initialization time. And without
taking into account the 3rd pipe added with ivb.

For gen4, we've completely ignored these. Although we do restrict lvds
to the 2nd pipe, so this is only a problem on machines where we boot
up with the lvds on the first pipe.

So restructure the code to enable the backlight on the right pipe at
modeset time.

v2: For odd reasons panel_enable_backlight gets called twice in a
modeset, so we can't WARN_ON in there if the backlight controller is
switched on already.

Tested-By: Kamal Mostafa <kamal@canonical.com>
Bugzilla: https://bugs.launchpad.net/bugs/954661
Cc: Carsten Emde <C.Emde@osadl.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_drv.h   |    3 +-
 drivers/gpu/drm/i915/intel_lvds.c  |   32 +++++------------------------
 drivers/gpu/drm/i915/intel_panel.c |   38 +++++++++++++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39d7b07..2fc2623 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -377,7 +377,8 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern u32 intel_panel_get_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
 extern int intel_panel_setup_backlight(struct drm_device *dev);
-extern void intel_panel_enable_backlight(struct drm_device *dev);
+extern void intel_panel_enable_backlight(struct drm_device *dev,
+					 enum pipe pipe);
 extern void intel_panel_disable_backlight(struct drm_device *dev);
 extern void intel_panel_destroy_backlight(struct drm_device *dev);
 extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index a7269e6..492db77 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -71,6 +71,7 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
 static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds->base.base.crtc);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg, stat_reg;
 
@@ -107,7 +108,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 	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);
+	intel_panel_enable_backlight(dev, intel_crtc->pipe);
 }
 
 static void intel_lvds_disable(struct intel_lvds *intel_lvds)
@@ -1074,35 +1075,14 @@ bool intel_lvds_init(struct drm_device *dev)
 		goto failed;
 
 out:
+	/*
+	 * Unlock registers and just
+	 * leave them unlocked
+	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		u32 pwm;
-
-		pipe = (I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT) ? 1 : 0;
-
-		/* make sure PWM is enabled and locked to the LVDS pipe */
-		pwm = I915_READ(BLC_PWM_CPU_CTL2);
-		if (pipe == 0 && (pwm & BLM_PIPE_B))
-			I915_WRITE(BLC_PWM_CPU_CTL2, pwm & ~BLM_PWM_ENABLE);
-		if (pipe)
-			pwm |= BLM_PIPE_B;
-		else
-			pwm &= ~BLM_PIPE_B;
-		I915_WRITE(BLC_PWM_CPU_CTL2, pwm | BLM_PWM_ENABLE);
-
-		pwm = I915_READ(BLC_PWM_PCH_CTL1);
-		pwm |= BLM_PCH_PWM_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);
 	}
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 2a1625d..5e52639 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -287,9 +287,18 @@ void intel_panel_disable_backlight(struct drm_device *dev)
 
 	dev_priv->backlight_enabled = false;
 	intel_panel_actually_set_backlight(dev, 0);
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		uint32_t reg;
+
+		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
+
+		I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE);
+	}
 }
 
-void intel_panel_enable_backlight(struct drm_device *dev)
+void intel_panel_enable_backlight(struct drm_device *dev,
+				  enum pipe pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -298,6 +307,33 @@ void intel_panel_enable_backlight(struct drm_device *dev)
 
 	dev_priv->backlight_enabled = true;
 	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		uint32_t reg, tmp;
+
+		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
+
+
+		tmp = I915_READ(reg);
+
+		/* While modesetting this code gets called twice, hence don't
+		 * adjust the backlight pipe if the backlight pwm is enabled
+		 * again already. */
+		if (tmp & BLM_PWM_ENABLE)
+			return;
+
+		if (dev_priv->num_pipe == 3)
+			tmp &= ~BLM_PIPE_SELECT_IVB;
+		else
+			tmp &= ~BLM_PIPE_SELECT;
+
+		tmp |= BLM_PIPE(pipe);
+		tmp &= ~BLM_PWM_ENABLE;
+
+		I915_WRITE(reg, tmp);
+		POSTING_READ(reg);
+		I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
+	}
 }
 
 static void intel_panel_init_backlight(struct drm_device *dev)
-- 
1.7.7.6

[PATCH 4/4] drm/i915: allow pipe A for lvds on gen4

[Daniel Vetter]

Given the havoc the missing backlight pipe select code might have
caused, let's try to re-enabled pipe A support for lvds on gen4 hw.
Just to see what all blows up ...

Note though that

commit 4add75c43f39573edc884d46b7c2b7414f01171a
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Dec 4 17:49:46 2010 +0000

    drm/i915: Allow LVDS to be on pipe A for Ironlake+

claims that this caused tons of spurious wakeups somehow.

More details can be found in the old revert:

commit 12e8ba25ef52f19e7a42e61aecb3c1fef83b2a82
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Sep 7 23:39:28 2010 +0100

    Revert "drm/i915: Allow LVDS on pipe A on gen4+"

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16307

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_lvds.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 492db77..ab4d647 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -968,6 +968,8 @@ bool intel_lvds_init(struct drm_device *dev)
 	intel_encoder->clone_mask = (1 << INTEL_LVDS_CLONE_BIT);
 	if (HAS_PCH_SPLIT(dev))
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
+	else if (IS_GEN4(dev))
+		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 	else
 		intel_encoder->crtc_mask = (1 << 1);
 
-- 
1.7.7.6

Re: [PATCH 1/4] drm/i915: pnv has a backlight polarity control bit, too

[Paul Menzel]

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

Dear Daniel,


Am Dienstag, den 05.06.2012, 10:07 +0200 schrieb Daniel Vetter:
> We already correctly ignore bit0 on gen < 4, now we also now why ;-)

s,also now,also know,

Maybe also do not use abbreviations in commit summaries.

> I've decided that losing that single bit of precision isn't worth the
> trouble to sprinkle IS_PINEVIEW checks all over the backlight control
> code - that code is way too fragile imo.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dcc04f..20244b9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1828,6 +1828,8 @@
>   */
>  #define   BACKLIGHT_DUTY_CYCLE_SHIFT		(0)
>  #define   BACKLIGHT_DUTY_CYCLE_MASK		(0xffff)
> +#define   BACKLIGHT_DUTY_CYCLE_MASK_PNV		(0xfffe)
> +#define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
>  
>  #define BLC_HIST_CTL		0x61260


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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

[PATCH] drm/i915: properly enable the blc controller on the right pipe

[Daniel Vetter]

On gen4+ we have a bitfield to specify from which pipe the backlight
controller should take it's clock. For PCH split platforms we've
already set these up, but only at initialization time. And without
taking into account the 3rd pipe added with ivb.

For gen4, we've completely ignored these. Although we do restrict lvds
to the 2nd pipe, so this is only a problem on machines where we boot
up with the lvds on the first pipe.

So restructure the code to enable the backlight on the right pipe at
modeset time.

v2: For odd reasons panel_enable_backlight gets called twice in a
modeset, so we can't WARN_ON in there if the backlight controller is
switched on already.

v3: backlight enable can also be called through dpms on, so the check
in there is legit. Update the comment to reflect that.

Tested-By: Kamal Mostafa <kamal@canonical.com>
Bugzilla: https://bugs.launchpad.net/bugs/954661
Cc: Carsten Emde <C.Emde@osadl.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_drv.h   |    3 +-
 drivers/gpu/drm/i915/intel_lvds.c  |   32 +++++------------------------
 drivers/gpu/drm/i915/intel_panel.c |   38 +++++++++++++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39d7b07..2fc2623 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -377,7 +377,8 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern u32 intel_panel_get_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
 extern int intel_panel_setup_backlight(struct drm_device *dev);
-extern void intel_panel_enable_backlight(struct drm_device *dev);
+extern void intel_panel_enable_backlight(struct drm_device *dev,
+					 enum pipe pipe);
 extern void intel_panel_disable_backlight(struct drm_device *dev);
 extern void intel_panel_destroy_backlight(struct drm_device *dev);
 extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index a7269e6..492db77 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -71,6 +71,7 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
 static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds->base.base.crtc);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg, stat_reg;
 
@@ -107,7 +108,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 	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);
+	intel_panel_enable_backlight(dev, intel_crtc->pipe);
 }
 
 static void intel_lvds_disable(struct intel_lvds *intel_lvds)
@@ -1074,35 +1075,14 @@ bool intel_lvds_init(struct drm_device *dev)
 		goto failed;
 
 out:
+	/*
+	 * Unlock registers and just
+	 * leave them unlocked
+	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		u32 pwm;
-
-		pipe = (I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT) ? 1 : 0;
-
-		/* make sure PWM is enabled and locked to the LVDS pipe */
-		pwm = I915_READ(BLC_PWM_CPU_CTL2);
-		if (pipe == 0 && (pwm & BLM_PIPE_B))
-			I915_WRITE(BLC_PWM_CPU_CTL2, pwm & ~BLM_PWM_ENABLE);
-		if (pipe)
-			pwm |= BLM_PIPE_B;
-		else
-			pwm &= ~BLM_PIPE_B;
-		I915_WRITE(BLC_PWM_CPU_CTL2, pwm | BLM_PWM_ENABLE);
-
-		pwm = I915_READ(BLC_PWM_PCH_CTL1);
-		pwm |= BLM_PCH_PWM_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);
 	}
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 2a1625d..7191908 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -287,9 +287,18 @@ void intel_panel_disable_backlight(struct drm_device *dev)
 
 	dev_priv->backlight_enabled = false;
 	intel_panel_actually_set_backlight(dev, 0);
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		uint32_t reg;
+
+		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
+
+		I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE);
+	}
 }
 
-void intel_panel_enable_backlight(struct drm_device *dev)
+void intel_panel_enable_backlight(struct drm_device *dev,
+				  enum pipe pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -298,6 +307,33 @@ void intel_panel_enable_backlight(struct drm_device *dev)
 
 	dev_priv->backlight_enabled = true;
 	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		uint32_t reg, tmp;
+
+		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
+
+
+		tmp = I915_READ(reg);
+
+		/* Note that this can also get called through dpms changes. And
+		 * we don't track the backlight dpms state, hence check whether
+		 * we have to do anything first. */
+		if (tmp & BLM_PWM_ENABLE)
+			return;
+
+		if (dev_priv->num_pipe == 3)
+			tmp &= ~BLM_PIPE_SELECT_IVB;
+		else
+			tmp &= ~BLM_PIPE_SELECT;
+
+		tmp |= BLM_PIPE(pipe);
+		tmp &= ~BLM_PWM_ENABLE;
+
+		I915_WRITE(reg, tmp);
+		POSTING_READ(reg);
+		I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
+	}
 }
 
 static void intel_panel_init_backlight(struct drm_device *dev)
-- 
1.7.7.6

Re: [PATCH 3/4] drm/i915: properly enable the blc controller on the right pipe

[Daniel Vetter]

On Tue, Jun 05, 2012 at 10:07:10AM +0200, Daniel Vetter wrote:
> On gen4+ we have a bitfield to specify from which pipe the backlight
> controller should take it's clock. For PCH split platforms we've
> already set these up, but only at initialization time. And without
> taking into account the 3rd pipe added with ivb.
> 
> For gen4, we've completely ignored these. Although we do restrict lvds
> to the 2nd pipe, so this is only a problem on machines where we boot
> up with the lvds on the first pipe.
> 
> So restructure the code to enable the backlight on the right pipe at
> modeset time.
> 
> v2: For odd reasons panel_enable_backlight gets called twice in a
> modeset, so we can't WARN_ON in there if the backlight controller is
> switched on already.

Note that further inspections showed that nothing odd is going on, these
are just dpms on/off calls from Xorg. I've simply forgotten to update the
commit message and the comment in the code ...
-Daniel

> 
> Tested-By: Kamal Mostafa <kamal@canonical.com>
> Bugzilla: https://bugs.launchpad.net/bugs/954661
> Cc: Carsten Emde <C.Emde@osadl.org>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |    3 +-
>  drivers/gpu/drm/i915/intel_lvds.c  |   32 +++++------------------------
>  drivers/gpu/drm/i915/intel_panel.c |   38 +++++++++++++++++++++++++++++++++++-
>  3 files changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 39d7b07..2fc2623 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -377,7 +377,8 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
>  extern u32 intel_panel_get_backlight(struct drm_device *dev);
>  extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
>  extern int intel_panel_setup_backlight(struct drm_device *dev);
> -extern void intel_panel_enable_backlight(struct drm_device *dev);
> +extern void intel_panel_enable_backlight(struct drm_device *dev,
> +					 enum pipe pipe);
>  extern void intel_panel_disable_backlight(struct drm_device *dev);
>  extern void intel_panel_destroy_backlight(struct drm_device *dev);
>  extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index a7269e6..492db77 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -71,6 +71,7 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
>  static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  {
>  	struct drm_device *dev = intel_lvds->base.base.dev;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds->base.base.crtc);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 ctl_reg, lvds_reg, stat_reg;
>  
> @@ -107,7 +108,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  	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);
> +	intel_panel_enable_backlight(dev, intel_crtc->pipe);
>  }
>  
>  static void intel_lvds_disable(struct intel_lvds *intel_lvds)
> @@ -1074,35 +1075,14 @@ bool intel_lvds_init(struct drm_device *dev)
>  		goto failed;
>  
>  out:
> +	/*
> +	 * Unlock registers and just
> +	 * leave them unlocked
> +	 */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		u32 pwm;
> -
> -		pipe = (I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT) ? 1 : 0;
> -
> -		/* make sure PWM is enabled and locked to the LVDS pipe */
> -		pwm = I915_READ(BLC_PWM_CPU_CTL2);
> -		if (pipe == 0 && (pwm & BLM_PIPE_B))
> -			I915_WRITE(BLC_PWM_CPU_CTL2, pwm & ~BLM_PWM_ENABLE);
> -		if (pipe)
> -			pwm |= BLM_PIPE_B;
> -		else
> -			pwm &= ~BLM_PIPE_B;
> -		I915_WRITE(BLC_PWM_CPU_CTL2, pwm | BLM_PWM_ENABLE);
> -
> -		pwm = I915_READ(BLC_PWM_PCH_CTL1);
> -		pwm |= BLM_PCH_PWM_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);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 2a1625d..5e52639 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -287,9 +287,18 @@ void intel_panel_disable_backlight(struct drm_device *dev)
>  
>  	dev_priv->backlight_enabled = false;
>  	intel_panel_actually_set_backlight(dev, 0);
> +
> +	if (INTEL_INFO(dev)->gen >= 4) {
> +		uint32_t reg;
> +
> +		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
> +
> +		I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE);
> +	}
>  }
>  
> -void intel_panel_enable_backlight(struct drm_device *dev)
> +void intel_panel_enable_backlight(struct drm_device *dev,
> +				  enum pipe pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> @@ -298,6 +307,33 @@ void intel_panel_enable_backlight(struct drm_device *dev)
>  
>  	dev_priv->backlight_enabled = true;
>  	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> +
> +	if (INTEL_INFO(dev)->gen >= 4) {
> +		uint32_t reg, tmp;
> +
> +		reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2;
> +
> +
> +		tmp = I915_READ(reg);
> +
> +		/* While modesetting this code gets called twice, hence don't
> +		 * adjust the backlight pipe if the backlight pwm is enabled
> +		 * again already. */
> +		if (tmp & BLM_PWM_ENABLE)
> +			return;
> +
> +		if (dev_priv->num_pipe == 3)
> +			tmp &= ~BLM_PIPE_SELECT_IVB;
> +		else
> +			tmp &= ~BLM_PIPE_SELECT;
> +
> +		tmp |= BLM_PIPE(pipe);
> +		tmp &= ~BLM_PWM_ENABLE;
> +
> +		I915_WRITE(reg, tmp);
> +		POSTING_READ(reg);
> +		I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
> +	}
>  }
>  
>  static void intel_panel_init_backlight(struct drm_device *dev)
> -- 
> 1.7.7.6
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/4] drm/i915: pnv has a backlight polarity control bit, too

[Eugeni Dodonov]

On 06/05/2012 05:07 AM, Daniel Vetter wrote:
> We already correctly ignore bit0 on gen < 4, now we also now why ;-)

s/now/know

> I've decided that losing that single bit of precision isn't worth the
> trouble to sprinkle IS_PINEVIEW checks all over the backlight control
> code - that code is way too fragile imo.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Eugeni

Re: [PATCH 2/4] drm/i915: clear up backlight #define confusion on gen4+

[Eugeni Dodonov]

On 06/05/2012 05:07 AM, Daniel Vetter wrote:
> - Regroup definitions for BLC_PWM_CTL so that they're all together and
>   and ordered according to the bitfields.
> 
> - Add all missing defintions for BLC_PWM_CTL2.

s/defintions/definitions

> 
> - Use the BLM_ (for backlight modulation) prefix consistently.
> 
> - Note that combination mode (i.e. also taking the legacy backlight
>   control value from pci config space into account) is gen4 only.
> 
> - Move the new registers for PCH-split machines up, they're an almost
>   match for the gen4 defitions.  Prefix the special PCH-only bits with
>   BLM_PCH_. Also add the pipe C select bit for ivb.
> 
> - Rip out the second pair of PCH polarity definitions - they're only
>   valid on early (pre-production) ilk silicon.
> 
> - Adapt the existing code to use the new definitions. This has the
>   nice benefit of killing a magic (1 << 30) left behind be Jesse
>   Barnes.
> 
> No functional changes in this patch.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

The bits look correct, but I think it would make it easier to follow up
if you'd split it into 2 patches to simplify things:
1. Add/regroup BLC_PWM_CTL2 and BLC_PWM_CTL stuff (and all the other
pre-PCH stuff)
2. Add/move the PCH registers

So each patch would correspond to one set of documents.

But if you think that the current way is good the way it is, for the
functional changes, as far as I can see:

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Eugeni

Re: [PATCH] drm/i915: properly enable the blc controller on the right pipe

[Eugeni Dodonov]

On 06/05/2012 07:14 AM, Daniel Vetter wrote:
> +		/* Note that this can also get called through dpms changes. And
> +		 * we don't track the backlight dpms state, hence check whether
> +		 * we have to do anything first. */
> +		if (tmp & BLM_PWM_ENABLE)
> +			return;
> +

I am tempted to add a WARN_ON(pipe > dev_priv->num_pipe) here, now that
we should handle 3 pipes.

> +		if (dev_priv->num_pipe == 3)
> +			tmp &= ~BLM_PIPE_SELECT_IVB;
> +		else
> +			tmp &= ~BLM_PIPE_SELECT;
> +
> +		tmp |= BLM_PIPE(pipe);
> +		tmp &= ~BLM_PWM_ENABLE;
> +
> +		I915_WRITE(reg, tmp);
> +		POSTING_READ(reg);
> +		I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
> +	}

But besides that:
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Eugeni

Re: [PATCH 4/4] drm/i915: allow pipe A for lvds on gen4

[Eugeni Dodonov]

On 06/05/2012 05:07 AM, Daniel Vetter wrote:
> Given the havoc the missing backlight pipe select code might have
> caused, let's try to re-enabled pipe A support for lvds on gen4 hw.
> Just to see what all blows up ...
> 
> Note though that
> 
> commit 4add75c43f39573edc884d46b7c2b7414f01171a
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Sat Dec 4 17:49:46 2010 +0000
> 
>     drm/i915: Allow LVDS to be on pipe A for Ironlake+
> 
> claims that this caused tons of spurious wakeups somehow.
> 
> More details can be found in the old revert:
> 
> commit 12e8ba25ef52f19e7a42e61aecb3c1fef83b2a82
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue Sep 7 23:39:28 2010 +0100
> 
>     Revert "drm/i915: Allow LVDS on pipe A on gen4+"
> 
>     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16307
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I think we'll notice soon enough if it breaks something, so why not
giving it a try?

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Eugeni

Re: [PATCH 4/4] drm/i915: allow pipe A for lvds on gen4

[Daniel Vetter]

On Tue, Jun 12, 2012 at 12:35:17PM -0300, Eugeni Dodonov wrote:
> On 06/05/2012 05:07 AM, Daniel Vetter wrote:
> > Given the havoc the missing backlight pipe select code might have
> > caused, let's try to re-enabled pipe A support for lvds on gen4 hw.
> > Just to see what all blows up ...
> > 
> > Note though that
> > 
> > commit 4add75c43f39573edc884d46b7c2b7414f01171a
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Sat Dec 4 17:49:46 2010 +0000
> > 
> >     drm/i915: Allow LVDS to be on pipe A for Ironlake+
> > 
> > claims that this caused tons of spurious wakeups somehow.
> > 
> > More details can be found in the old revert:
> > 
> > commit 12e8ba25ef52f19e7a42e61aecb3c1fef83b2a82
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue Sep 7 23:39:28 2010 +0100
> > 
> >     Revert "drm/i915: Allow LVDS on pipe A on gen4+"
> > 
> >     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16307
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I think we'll notice soon enough if it breaks something, so why not
> giving it a try?
> 
> Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Thanks for reviewing these, queud all four to -next.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48