Attachment 93568 Details for Bug 74628 – Patch with debugging option

[patch] Patch with debugging option

0001-drm-i915-add-i915.edp_delays-option.patch (text/plain), 7.23 KB, created by Paulo Zanoni on 2014-02-06 21:43:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Paulo Zanoni

Created: 2014-02-06 21:43:07 UTC

Size: 7.23 KB

patch

obsolete

>From 83c4ac2695302e5bca0e885537d8390647e3713a Mon Sep 17 00:00:00 2001
>From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>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 <paulo.r.zanoni@intel.com>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 74628: 93565 | 93568 | 93769 | 94966 | 94967 | 94968 | 95007

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.