From 83c4ac2695302e5bca0e885537d8390647e3713a Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Thu, 6 Feb 2014 18:57:10 -0200 Subject: [PATCH] drm/i915: add i915.edp_delays option We recently discovered that the amount of time we were actually waiting when doing the eDP panel power operations was much bigger than the amount of time we thought we were waiting. So we fixed this, and got regression reports. So this commit adds the i915.edp_delays option that can be used to debug the possible regressions, including an option to simulate the old code behavior. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 10 +++++ drivers/gpu/drm/i915/i915_params.c | 6 +++ drivers/gpu/drm/i915/intel_dp.c | 82 +++++++++++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 728b9c3..c3c7521 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -151,6 +151,15 @@ enum hpd_pin { HPD_NUM_PINS }; +enum edp_delays { + EDP_DELAYS_AUTO = -1, + EDP_DELAYS_VBT = 0, + EDP_DELAYS_BIOS = 1, + EDP_DELAYS_SPEC = 2, + EDP_DELAYS_I915 = 3, + EDP_DELAYS_I915_OLD = 4, +}; + #define I915_GEM_GPU_DOMAINS \ (I915_GEM_DOMAIN_RENDER | \ I915_GEM_DOMAIN_SAMPLER | \ @@ -1957,6 +1966,7 @@ struct i915_params { bool prefault_disable; bool reset; int invert_brightness; + int edp_delays; }; extern struct i915_params i915 __read_mostly; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index c743057..f6dd68a0 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -47,6 +47,7 @@ struct i915_params i915 __read_mostly = { .prefault_disable = 0, .reset = true, .invert_brightness = 0, + .edp_delays = -1, }; module_param_named(modeset, i915.modeset, int, 0400); @@ -153,3 +154,8 @@ MODULE_PARM_DESC(invert_brightness, "report PCI device ID, subsystem vendor and subsystem device ID " "to dri-devel@lists.freedesktop.org, if your machine needs it. " "It will then be included in an upcoming module version."); + +module_param_named(edp_delays, i915.edp_delays, int, 0600); +MODULE_PARM_DESC(edp_delays, + "Which set of eDP delays to use. " + "(-1=auto [default], 0=VBT, 1=BIOS, 2=spec, 3=i915 4=i915-old)"); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f1ef3d4..60adfe7 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1091,8 +1091,18 @@ static void wait_panel_on(struct intel_dp *intel_dp) static void wait_panel_off(struct intel_dp *intel_dp) { + u32 mask, value; + DRM_DEBUG_KMS("Wait for panel power off time\n"); - wait_panel_status(intel_dp, IDLE_OFF_MASK, IDLE_OFF_VALUE); + + mask = IDLE_OFF_MASK; + value = IDLE_OFF_VALUE; + if (i915.edp_delays == EDP_DELAYS_I915_OLD) { + mask |= PP_SEQUENCE_STATE_MASK; + value |= PP_SEQUENCE_STATE_OFF_IDLE; + } + + wait_panel_status(intel_dp, mask, value); } static void wait_panel_power_cycle(struct intel_dp *intel_dp) @@ -1204,7 +1214,9 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp) DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n", I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg)); - if ((pp & POWER_TARGET_ON) == 0) + if (i915.edp_delays == EDP_DELAYS_I915_OLD) + msleep(intel_dp->panel_power_cycle_delay); + else if ((pp & POWER_TARGET_ON) == 0) intel_dp->last_power_cycle = jiffies; intel_runtime_pm_put(dev_priv); @@ -1301,7 +1313,8 @@ void intel_edp_panel_off(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Turn eDP power off\n"); - edp_wait_backlight_off(intel_dp); + if (i915.edp_delays != EDP_DELAYS_I915_OLD) + edp_wait_backlight_off(intel_dp); pp = ironlake_get_pp_control(intel_dp); /* We need to switch off panel power _and_ force vdd, for otherwise some @@ -1335,7 +1348,10 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp) * link. So delay a bit to make sure the image is solid before * allowing it to appear. */ - wait_backlight_on(intel_dp); + if (i915.edp_delays == EDP_DELAYS_I915_OLD) + msleep(intel_dp->backlight_on_delay); + else + wait_backlight_on(intel_dp); pp = ironlake_get_pp_control(intel_dp); pp |= EDP_BLC_ENABLE; @@ -1367,6 +1383,9 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); + + if (i915.edp_delays == EDP_DELAYS_I915_OLD) + msleep(intel_dp->backlight_off_delay); intel_dp->last_backlight_off = jiffies; } @@ -3574,11 +3593,44 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, #define assign_final(field) final.field = (max(cur.field, vbt.field) == 0 ? \ spec.field : \ max(cur.field, vbt.field)) - assign_final(t1_t3); - assign_final(t8); - assign_final(t9); - assign_final(t10); - assign_final(t11_t12); + + switch (i915.edp_delays) { + case EDP_DELAYS_VBT: + /* Just use the VBT values. In some machines, the VBT values are + * very different from the values set by the BIOS! */ + final = vbt; + break; + case EDP_DELAYS_BIOS: + /* Just use whatever is set by the BIOS. The problem is that the + * spec suggests the backlight registers should be set to 1 and + * the wait should be done in software, so this option may not + * be too safe. */ + final = cur; + break; + case EDP_DELAYS_SPEC: + /* Just try the values our driver believes are the maximum + * values defined by the spec. */ + final = spec; + break; + case EDP_DELAYS_I915: + /* The good old policy we've always used: use the maximum value + * between BIOS and VBT if not zero, otherwise use the spec + * value. */ + case EDP_DELAYS_I915_OLD: + /* At some point our code did the equivalent of this option, so + * when we get regression reports, we should ask the users to + * try this one too. */ + case EDP_DELAYS_AUTO: + /* The default. */ + default: + /* Bad user! */ + assign_final(t1_t3); + assign_final(t8); + assign_final(t9); + assign_final(t10); + assign_final(t11_t12); + } + #undef assign_final #define get_delay(field) (DIV_ROUND_UP(final.field, 10)) @@ -3609,6 +3661,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, u32 pp_on, pp_off, pp_div, port_sel = 0; int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev); int pp_on_reg, pp_off_reg, pp_div_reg; + u32 backlight_on_val, backlight_off_val; if (HAS_PCH_SPLIT(dev)) { pp_on_reg = PCH_PP_ON_DELAYS; @@ -3630,9 +3683,16 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, * do the manual sleep, and once when we disable the panel and wait for * the PP_STATUS bit to become zero. */ + if (i915.edp_delays == EDP_DELAYS_I915_OLD) { + backlight_on_val = seq->t8; + backlight_off_val = seq->t10; + } else { + backlight_on_val = 1; + backlight_off_val = 1; + } pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | - (1 << PANEL_LIGHT_ON_DELAY_SHIFT); - pp_off = (1 << PANEL_LIGHT_OFF_DELAY_SHIFT) | + (backlight_on_val << PANEL_LIGHT_ON_DELAY_SHIFT); + pp_off = (backlight_off_val << PANEL_LIGHT_OFF_DELAY_SHIFT) | (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); /* Compute the divisor for the pp clock, simply match the Bspec * formula. */ -- 1.8.5.3