On Tue, 12 Jan 2016 01:37:39 +0200
Sviatoslav Chagaev <sviatoslav.chag...@gmail.com> wrote:
> On Sun, 10 Jan 2016 21:28:31 +0100
> Joerg Jung <m...@umaxx.net> wrote:
> > On Sun, Jan 10, 2016 at 04:59:22AM +0200, Sviatoslav Chagaev wrote:
> >
> > > An Intel developer claims [2] that it's not a bug in Intel KMS driver and
> > > people
> > > claim [3] the problem goes away when booting in legacy BIOS mode.
> >
> > That makes no sense to me. If the problem goes aways when booting
> > legacy BIOS, so it must be a bug when not, right?
>
> Judging by the code at e.g. sys/dev/pci/drm/i915/intel_panel.c:984, it
> seems Intel KMS driver expects BIOS or EFI to configure backlight,
> then reads these values and uses them itself.
>
> I found documentation for Intel Haswell [1] and glanced over
> descriptions of relevant registers (BLC_PWM_CTL, BLC_PWM2_CTL,
> BLC_PWM_DATA, BLC_MISC_CTL, BLM_HIST_CTL, BLM_HIST_BIN, BLM_HIST_GUARD,
> UTIL_PIN_CTL, SBLC_PWM_CTL1, SBLC_PWM_CTL2), they are confusing...
>
> Based on these, I agree, it might be a bug in Intel KMS.
>
> I'll add debug prints and play with these registers, maybe I'll spot
> some incorrect values.
>
> [1]
> https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-commandreference-registers_0_0.pdf
>
Had no luck with Intel KMS:
Added register dump to every backlight operation, see first diff.
State of registers I get from EFI:
pch_dump_backlight_registers(): pch_setup_backlight
MY_BLC_PWM_CTL = 0x00000000; MY_BLC_PWM_DATA = 0x00000000
MY_BLC_PWM_DATA2 = 0x00000000; MY_BLM_HIST_CTL = 0x00000000
MY_BLM_HIST_BIN = 0x00000000; MY_BLM_HIST_GUARD = 0x00000000
MY_BLC_PWM2_CTL = 0x60000000; MY_BLC_MISC_CTL = 0x00000000
MY_UTIL_PIN_CTL = 0x00000000; MY_SBLC_PWM_CTL1 = 0xc0000000
MY_SBLC_PWM_CTL2 = 0x0ad901c3;
State of registers after Intel KMS configures them:
pch_dump_backlight_registers(): pch_set_backlight
MY_BLC_PWM_CTL = 0xe0000000; MY_BLC_PWM_DATA = 0x00000ad9
MY_BLC_PWM_DATA2 = 0x00000000; MY_BLM_HIST_CTL = 0x00000006
MY_BLM_HIST_BIN = 0x00000000; MY_BLM_HIST_GUARD = 0x00000000
MY_BLC_PWM2_CTL = 0x60000000; MY_BLC_MISC_CTL = 0x00000000
MY_UTIL_PIN_CTL = 0x00000000; MY_SBLC_PWM_CTL1 = 0x80000000
MY_SBLC_PWM_CTL2 = 0x0ad90000;
BLC_PWM_CTL change seems strange.
Modified pch_*_backlight() functions so they do not change the
configuration set by EFI, see second diff, but observed no change in
behaviour.
Index: sys/dev/pci/drm/i915/i915_reg.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_reg.h,v
retrieving revision 1.11
diff -u -p -r1.11 i915_reg.h
--- sys/dev/pci/drm/i915/i915_reg.h 25 Sep 2015 16:15:19 -0000 1.11
+++ sys/dev/pci/drm/i915/i915_reg.h 12 Jan 2016 02:54:49 -0000
@@ -2493,6 +2493,18 @@
#define BACKLIGHT_DUTY_CYCLE_MASK_PNV (0xfffe)
#define BLM_POLARITY_PNV (1 << 0) /* pnv only */
+#define MY_BLC_PWM_CTL 0x48250
+#define MY_BLC_PWM_DATA 0x48254
+#define MY_BLC_PWM_DATA2 0x48354
+#define MY_BLM_HIST_CTL 0x48260
+#define MY_BLM_HIST_BIN 0x48264
+#define MY_BLM_HIST_GUARD 0x48268
+#define MY_BLC_PWM2_CTL 0x48350
+#define MY_BLC_MISC_CTL 0x48360
+#define MY_UTIL_PIN_CTL 0x48400
+#define MY_SBLC_PWM_CTL1 0xc8250
+#define MY_SBLC_PWM_CTL2 0xc8254
+
#define BLC_HIST_CTL (dev_priv->info->display_mmio_offset + 0x61260)
/* New registers for PCH-split platforms. Safe where new bits show up, the
Index: sys/dev/pci/drm/i915/intel_panel.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_panel.c,v
retrieving revision 1.11
diff -u -p -r1.11 intel_panel.c
--- sys/dev/pci/drm/i915/intel_panel.c 23 Sep 2015 23:12:12 -0000 1.11
+++ sys/dev/pci/drm/i915/intel_panel.c 12 Jan 2016 02:54:49 -0000
@@ -36,6 +36,38 @@
#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
+void pch_dump_backlight_registers(struct intel_connector *connector, const
char *msg)
+{
+ struct drm_device *dev = connector->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 i = 0;
+ u32 value;
+
+ printf("%s(): %s\n", __func__, msg);
+
+#define Y(register) \
+ value = I915_READ(register); \
+ printf("%-17s = 0x%08x%s", \
+ #register, value, (i++ & 1) ? "\n" : "; ");
+
+ Y(MY_BLC_PWM_CTL);
+ Y(MY_BLC_PWM_DATA);
+ Y(MY_BLC_PWM_DATA2);
+ Y(MY_BLM_HIST_CTL);
+ Y(MY_BLM_HIST_BIN);
+ Y(MY_BLM_HIST_GUARD);
+ Y(MY_BLC_PWM2_CTL);
+ Y(MY_BLC_MISC_CTL);
+ Y(MY_UTIL_PIN_CTL);
+ Y(MY_SBLC_PWM_CTL1);
+ Y(MY_SBLC_PWM_CTL2);
+
+ if (i & 1)
+ printf("\n");
+
+#undef Y
+}
+
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
@@ -366,6 +398,8 @@ static u32 pch_get_backlight(struct inte
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ pch_dump_backlight_registers(connector, __func__);
+
return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
}
@@ -439,6 +473,8 @@ static void pch_set_backlight(struct int
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
+ pch_dump_backlight_registers(connector, __func__);
+
tmp = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
}
@@ -535,6 +571,8 @@ static void pch_disable_backlight(struct
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
+ pch_dump_backlight_registers(connector, __func__);
+
intel_panel_actually_set_backlight(connector, 0);
tmp = I915_READ(BLC_PWM_CPU_CTL2);
@@ -648,6 +686,8 @@ static void pch_enable_backlight(struct
intel_pipe_to_cpu_transcoder(dev_priv, pipe);
u32 cpu_ctl2, pch_ctl1, pch_ctl2;
+ pch_dump_backlight_registers(connector, __func__);
+
cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
if (cpu_ctl2 & BLM_PWM_ENABLE) {
DRM_DEBUG_KMS("cpu backlight already enabled\n");
@@ -980,6 +1020,8 @@ static int pch_setup_backlight(struct in
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;
+
+ pch_dump_backlight_registers(connector, __func__);
pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
Index: sys/dev/pci/drm/i915/i915_reg.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_reg.h,v
retrieving revision 1.11
diff -u -p -r1.11 i915_reg.h
--- sys/dev/pci/drm/i915/i915_reg.h 25 Sep 2015 16:15:19 -0000 1.11
+++ sys/dev/pci/drm/i915/i915_reg.h 12 Jan 2016 23:07:04 -0000
@@ -2493,6 +2493,18 @@
#define BACKLIGHT_DUTY_CYCLE_MASK_PNV (0xfffe)
#define BLM_POLARITY_PNV (1 << 0) /* pnv only */
+#define MY_BLC_PWM_CTL 0x48250
+#define MY_BLC_PWM_DATA 0x48254
+#define MY_BLC_PWM_DATA2 0x48354
+#define MY_BLM_HIST_CTL 0x48260
+#define MY_BLM_HIST_BIN 0x48264
+#define MY_BLM_HIST_GUARD 0x48268
+#define MY_BLC_PWM2_CTL 0x48350
+#define MY_BLC_MISC_CTL 0x48360
+#define MY_UTIL_PIN_CTL 0x48400
+#define MY_SBLC_PWM_CTL1 0xc8250
+#define MY_SBLC_PWM_CTL2 0xc8254
+
#define BLC_HIST_CTL (dev_priv->info->display_mmio_offset + 0x61260)
/* New registers for PCH-split platforms. Safe where new bits show up, the
Index: sys/dev/pci/drm/i915/intel_panel.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_panel.c,v
retrieving revision 1.11
diff -u -p -r1.11 intel_panel.c
--- sys/dev/pci/drm/i915/intel_panel.c 23 Sep 2015 23:12:12 -0000 1.11
+++ sys/dev/pci/drm/i915/intel_panel.c 12 Jan 2016 23:07:05 -0000
@@ -36,6 +36,38 @@
#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
+void pch_dump_backlight_registers(struct intel_connector *connector, const
char *msg)
+{
+ struct drm_device *dev = connector->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 i = 0;
+ u32 value;
+
+ printf("%s(): %s\n", __func__, msg);
+
+#define Y(register) \
+ value = I915_READ(register); \
+ printf("%-17s = 0x%08x%s", \
+ #register, value, (i++ & 1) ? "\n" : "; ");
+
+ Y(MY_BLC_PWM_CTL);
+ Y(MY_BLC_PWM_DATA);
+ Y(MY_BLC_PWM_DATA2);
+ Y(MY_BLM_HIST_CTL);
+ Y(MY_BLM_HIST_BIN);
+ Y(MY_BLM_HIST_GUARD);
+ Y(MY_BLC_PWM2_CTL);
+ Y(MY_BLC_MISC_CTL);
+ Y(MY_UTIL_PIN_CTL);
+ Y(MY_SBLC_PWM_CTL1);
+ Y(MY_SBLC_PWM_CTL2);
+
+ if (i & 1)
+ printf("\n");
+
+#undef Y
+}
+
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
@@ -366,7 +398,9 @@ static u32 pch_get_backlight(struct inte
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
+ pch_dump_backlight_registers(connector, __func__);
+
+ return I915_READ(MY_SBLC_PWM_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK;
}
static u32 i9xx_get_backlight(struct intel_connector *connector)
@@ -439,8 +473,10 @@ static void pch_set_backlight(struct int
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
- tmp = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
- I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
+ pch_dump_backlight_registers(connector, __func__);
+
+ tmp = I915_READ(MY_SBLC_PWM_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp | level);
}
static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
@@ -535,13 +571,10 @@ static void pch_disable_backlight(struct
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
- intel_panel_actually_set_backlight(connector, 0);
-
- tmp = I915_READ(BLC_PWM_CPU_CTL2);
- I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
+ pch_dump_backlight_registers(connector, __func__);
- tmp = I915_READ(BLC_PWM_PCH_CTL1);
- I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
+ tmp = I915_READ(MY_SBLC_PWM_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp);
}
static void i9xx_disable_backlight(struct intel_connector *connector)
@@ -642,51 +675,23 @@ static void pch_enable_backlight(struct
{
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_panel *panel = &connector->panel;
- enum pipe pipe = intel_get_pipe_from_connector(connector);
- enum transcoder cpu_transcoder =
- intel_pipe_to_cpu_transcoder(dev_priv, pipe);
- u32 cpu_ctl2, pch_ctl1, pch_ctl2;
-
- cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
- if (cpu_ctl2 & BLM_PWM_ENABLE) {
- DRM_DEBUG_KMS("cpu backlight already enabled\n");
- cpu_ctl2 &= ~BLM_PWM_ENABLE;
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
- }
-
- pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
- if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
- DRM_DEBUG_KMS("pch backlight already enabled\n");
- pch_ctl1 &= ~BLM_PCH_PWM_ENABLE;
- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
- }
+ u32 tmp;
- if (cpu_transcoder == TRANSCODER_EDP)
- cpu_ctl2 = BLM_TRANSCODER_EDP;
- else
- cpu_ctl2 = BLM_PIPE(cpu_transcoder);
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
- POSTING_READ(BLC_PWM_CPU_CTL2);
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
+ pch_dump_backlight_registers(connector, __func__);
- /* This won't stick until the above enable. */
- intel_panel_actually_set_backlight(connector, panel->backlight.level);
+ if (I915_READ(MY_BLC_PWM_CTL) != 0)
+ I915_WRITE(MY_BLC_PWM_CTL, 0);
- pch_ctl2 = panel->backlight.max << 16;
- I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
-
- /* XXX: transitional */
- if (dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)
- return;
+ if (I915_READ(MY_SBLC_PWM_CTL2) == 0)
+ I915_WRITE(MY_SBLC_PWM_CTL2, 0x0ad90ad9);
- pch_ctl1 = 0;
- if (panel->backlight.active_low_pwm)
- pch_ctl1 |= BLM_PCH_POLARITY;
+ /* SBLC_PWM_CTL1 must be programmed *after* SBLC_PWM_CTL2 */
+ if (I915_READ(MY_SBLC_PWM_CTL1) != 0xc0000000)
+ I915_WRITE(MY_SBLC_PWM_CTL1, 0xc0000000);
- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
- POSTING_READ(BLC_PWM_PCH_CTL1);
- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
+ tmp = I915_READ(MY_SBLC_PWM_CTL2);
+ tmp |= tmp >> 16;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp);
}
static void i9xx_enable_backlight(struct intel_connector *connector)
@@ -979,22 +984,23 @@ static int pch_setup_backlight(struct in
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
- u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;
- pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
- panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
+ pch_dump_backlight_registers(connector, __func__);
- pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
- panel->backlight.max = pch_ctl2 >> 16;
- if (!panel->backlight.max)
- return -ENODEV;
+ if (I915_READ(MY_BLC_PWM_CTL) != 0)
+ I915_WRITE(MY_BLC_PWM_CTL, 0);
- val = pch_get_backlight(connector);
- panel->backlight.level = intel_panel_compute_brightness(connector, val);
+ if (I915_READ(MY_SBLC_PWM_CTL2) == 0)
+ I915_WRITE(MY_SBLC_PWM_CTL2, 0x0ad90ad9);
- cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
- panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) &&
- (pch_ctl1 & BLM_PCH_PWM_ENABLE) && panel->backlight.level != 0;
+ /* SBLC_PWM_CTL1 must be programmed *after* SBLC_PWM_CTL2 */
+ if (I915_READ(MY_SBLC_PWM_CTL1) != 0xc0000000)
+ I915_WRITE(MY_SBLC_PWM_CTL1, 0xc0000000);
+
+ panel->backlight.active_low_pwm = false;
+ panel->backlight.max = I915_READ(MY_SBLC_PWM_CTL2) >> 16;
+ panel->backlight.level = intel_panel_compute_brightness(connector,
+ I915_READ(MY_SBLC_PWM_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK);
return 0;
}
