* [PATCH v2] drm/i915: Kill check_power_well() calls
@ 2014-12-18 9:44 ville.syrjala
2014-12-18 10:07 ` [Intel-gfx] " Jani Nikula
2014-12-18 14:06 ` shuang.he
0 siblings, 2 replies; 4+ messages in thread
From: ville.syrjala @ 2014-12-18 9:44 UTC (permalink / raw)
To: intel-gfx; +Cc: stable, Egbert Eich
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
pps_{lock,unlock}() call intel_display_power_{get,put}() outside
pps_mutes to avoid deadlocks with the power_domain mutex. In theory
during aux transfers we should usually have the relevant power domain
references already held by some higher level code, so this should not
result in much overhead (exception being userspace i2c-dev access).
However thanks to the check_power_well() calls in
intel_display_power_{get/put}() we end up doing a few Punit reads for
each aux transfer. Obviously doing this for each byte transferred via
i2c-over-aux is not a good idea.
I can't think of a good way to keep check_power_well() while eliminating
the overhead, so let's just remove check_power_well() entirely.
Fixes a driver init time regression introduced by:
commit 773538e86081d146e0020435d614f4b96996c1f9
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date: Thu Sep 4 14:54:56 2014 +0300
drm/i915: Reset power sequencer pipe tracking when disp2d is off
Credit goes to Jani for figuring this out.
v2: Add the regression note in the commit message.
Cc: stable@vger.kernel.org
Cc: Egbert Eich <eich@suse.de>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86201
Tested-by: Wendy Wang <wendy.wang@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 ---------------------------
1 file changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6aa3a81..39e1b07 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -615,29 +615,6 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
vlv_power_sequencer_reset(dev_priv);
}
-static void check_power_well_state(struct drm_i915_private *dev_priv,
- struct i915_power_well *power_well)
-{
- bool enabled = power_well->ops->is_enabled(dev_priv, power_well);
-
- if (power_well->always_on || !i915.disable_power_well) {
- if (!enabled)
- goto mismatch;
-
- return;
- }
-
- if (enabled != (power_well->count > 0))
- goto mismatch;
-
- return;
-
-mismatch:
- I915_STATE_WARN(1, "state mismatch for '%s' (always_on %d hw state %d use-count %d disable_power_well %d\n",
- power_well->name, power_well->always_on, enabled,
- power_well->count, i915.disable_power_well);
-}
-
/**
* intel_display_power_get - grab a power domain reference
* @dev_priv: i915 device instance
@@ -669,8 +646,6 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
power_well->ops->enable(dev_priv, power_well);
power_well->hw_enabled = true;
}
-
- check_power_well_state(dev_priv, power_well);
}
power_domains->domain_use_count[domain]++;
@@ -709,8 +684,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
power_well->hw_enabled = false;
power_well->ops->disable(dev_priv, power_well);
}
-
- check_power_well_state(dev_priv, power_well);
}
mutex_unlock(&power_domains->lock);
--
2.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Kill check_power_well() calls
2014-12-18 9:44 [PATCH v2] drm/i915: Kill check_power_well() calls ville.syrjala
@ 2014-12-18 10:07 ` Jani Nikula
2014-12-18 14:06 ` shuang.he
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2014-12-18 10:07 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Egbert Eich, stable
On Thu, 18 Dec 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> pps_{lock,unlock}() call intel_display_power_{get,put}() outside
> pps_mutes to avoid deadlocks with the power_domain mutex. In theory
> during aux transfers we should usually have the relevant power domain
> references already held by some higher level code, so this should not
> result in much overhead (exception being userspace i2c-dev access).
> However thanks to the check_power_well() calls in
> intel_display_power_{get/put}() we end up doing a few Punit reads for
> each aux transfer. Obviously doing this for each byte transferred via
> i2c-over-aux is not a good idea.
>
> I can't think of a good way to keep check_power_well() while eliminating
> the overhead, so let's just remove check_power_well() entirely.
>
> Fixes a driver init time regression introduced by:
> commit 773538e86081d146e0020435d614f4b96996c1f9
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date: Thu Sep 4 14:54:56 2014 +0300
>
> drm/i915: Reset power sequencer pipe tracking when disp2d is off
>
> Credit goes to Jani for figuring this out.
>
> v2: Add the regression note in the commit message.
>
> Cc: stable@vger.kernel.org
> Cc: Egbert Eich <eich@suse.de>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86201
> Tested-by: Wendy Wang <wendy.wang@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-next-fixes, thanks for the patch.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 ---------------------------
> 1 file changed, 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6aa3a81..39e1b07 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -615,29 +615,6 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
> vlv_power_sequencer_reset(dev_priv);
> }
>
> -static void check_power_well_state(struct drm_i915_private *dev_priv,
> - struct i915_power_well *power_well)
> -{
> - bool enabled = power_well->ops->is_enabled(dev_priv, power_well);
> -
> - if (power_well->always_on || !i915.disable_power_well) {
> - if (!enabled)
> - goto mismatch;
> -
> - return;
> - }
> -
> - if (enabled != (power_well->count > 0))
> - goto mismatch;
> -
> - return;
> -
> -mismatch:
> - I915_STATE_WARN(1, "state mismatch for '%s' (always_on %d hw state %d use-count %d disable_power_well %d\n",
> - power_well->name, power_well->always_on, enabled,
> - power_well->count, i915.disable_power_well);
> -}
> -
> /**
> * intel_display_power_get - grab a power domain reference
> * @dev_priv: i915 device instance
> @@ -669,8 +646,6 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
> power_well->ops->enable(dev_priv, power_well);
> power_well->hw_enabled = true;
> }
> -
> - check_power_well_state(dev_priv, power_well);
> }
>
> power_domains->domain_use_count[domain]++;
> @@ -709,8 +684,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> power_well->hw_enabled = false;
> power_well->ops->disable(dev_priv, power_well);
> }
> -
> - check_power_well_state(dev_priv, power_well);
> }
>
> mutex_unlock(&power_domains->lock);
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/i915: Kill check_power_well() calls
2014-12-18 9:44 [PATCH v2] drm/i915: Kill check_power_well() calls ville.syrjala
2014-12-18 10:07 ` [Intel-gfx] " Jani Nikula
@ 2014-12-18 14:06 ` shuang.he
2014-12-19 8:01 ` Jani Nikula
1 sibling, 1 reply; 4+ messages in thread
From: shuang.he @ 2014-12-18 14:06 UTC (permalink / raw)
To: shuang.he, intel-gfx, ville.syrjala
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -2 364/364 362/364
ILK +1-1 364/366 364/366
SNB -1 448/450 447/450
IVB -1 497/498 496/498
BYT -1 289/289 288/289
HSW -1 563/564 562/564
BDW -3 416/417 413/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
PNV igt_gem_exec_blt NRUN(1, M23)PASS(1, M25) NRUN(1, M23)
*PNV igt_kms_sysfs_edid_timing PASS(2, M25M23) FAIL(1, M23)
ILK igt_gem_exec_blt NRUN(1, M37)PASS(1, M26) NRUN(1, M37)
ILK igt_kms_flip_bcs-flip-vs-modeset-interruptible DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
SNB igt_gem_exec_blt NRUN(3, M35M22)PASS(1, M35) NRUN(1, M35)
IVB igt_gem_exec_blt NRUN(3, M34M21M4)PASS(1, M34) NRUN(1, M34)
BYT igt_gem_exec_blt NRUN(1, M48)PASS(1, M48) NRUN(1, M48)
HSW igt_gem_exec_blt NRUN(2, M40M20)PASS(1, M40) NRUN(1, M40)
BDW igt_gem_exec_blt NRUN(1, M30)PASS(1, M28) NRUN(1, M30)
BDW igt_gem_render_linear_blits TIMEOUT(1, M30)PASS(1, M28) TIMEOUT(1, M30)
*BDW igt_gem_render_tiled_blits PASS(2, M28M30) TIMEOUT(1, M30)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/i915: Kill check_power_well() calls
2014-12-18 14:06 ` shuang.he
@ 2014-12-19 8:01 ` Jani Nikula
0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2014-12-19 8:01 UTC (permalink / raw)
To: shuang.he
On Thu, 18 Dec 2014, shuang.he@intel.com wrote:
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
> -------------------------------------Summary-------------------------------------
> Platform Delta drm-intel-nightly Series Applied
> PNV -2 364/364 362/364
> ILK +1-1 364/366 364/366
> SNB -1 448/450 447/450
> IVB -1 497/498 496/498
> BYT -1 289/289 288/289
> HSW -1 563/564 562/564
> BDW -3 416/417 413/417
> -------------------------------------Detailed-------------------------------------
> Platform Test drm-intel-nightly Series Applied
> PNV igt_gem_exec_blt NRUN(1, M23)PASS(1, M25) NRUN(1, M23)
> *PNV igt_kms_sysfs_edid_timing PASS(2, M25M23) FAIL(1, M23)
> ILK igt_gem_exec_blt NRUN(1, M37)PASS(1, M26) NRUN(1, M37)
> ILK igt_kms_flip_bcs-flip-vs-modeset-interruptible DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
> SNB igt_gem_exec_blt NRUN(3, M35M22)PASS(1, M35) NRUN(1, M35)
> IVB igt_gem_exec_blt NRUN(3, M34M21M4)PASS(1, M34) NRUN(1, M34)
> BYT igt_gem_exec_blt NRUN(1, M48)PASS(1, M48) NRUN(1, M48)
> HSW igt_gem_exec_blt NRUN(2, M40M20)PASS(1, M40) NRUN(1, M40)
> BDW igt_gem_exec_blt NRUN(1, M30)PASS(1, M28) NRUN(1, M30)
> BDW igt_gem_render_linear_blits TIMEOUT(1, M30)PASS(1, M28) TIMEOUT(1, M30)
> *BDW igt_gem_render_tiled_blits PASS(2, M28M30) TIMEOUT(1, M30)
> Note: You need to pay more attention to line start with '*'
The patch should not affect any of these.
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-19 8:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-18 9:44 [PATCH v2] drm/i915: Kill check_power_well() calls ville.syrjala
2014-12-18 10:07 ` [Intel-gfx] " Jani Nikula
2014-12-18 14:06 ` shuang.he
2014-12-19 8:01 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox