[Intel-gfx] [PATCH] drm/i915: Remove unsafe i915.enable_rc6
Chris Wilson
chris at chris-wilson.co.uk
Sat Mar 25 14:23:44 UTC 2017
It has been many years since the last confirmed sighting (and fix) of an
RC6 related bug (usually a system hang). Remove the parameter to stop
users from setting dangerous values.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula at intel.com>
Cc: Imre Deak <imre.deak at intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_params.c | 9 ---
drivers/gpu/drm/i915/i915_params.h | 1 -
drivers/gpu/drm/i915/i915_pci.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 5 --
drivers/gpu/drm/i915/intel_pm.c | 126 ++++++++----------------------------
drivers/gpu/drm/i915/intel_uc.c | 2 +-
drivers/gpu/drm/i915/intel_uncore.c | 2 -
9 files changed, 33 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6020ef8bd3b2..1223c677cbf9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2367,7 +2367,7 @@ static int intel_runtime_suspend(struct device *kdev)
struct drm_i915_private *dev_priv = to_i915(dev);
int ret;
- if (WARN_ON_ONCE(!(dev_priv->rps.enabled && intel_enable_rc6())))
+ if (WARN_ON_ONCE(!(dev_priv->rps.enabled && HAS_RC6(dev_priv))))
return -ENODEV;
if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 15a0c31a713d..7ac115de62e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2961,6 +2961,7 @@ intel_info(const struct drm_i915_private *dev_priv)
#define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
#define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
#define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
+#define HAS_RC6pp(dev_priv) (false)
#define HAS_CSR(dev_priv) ((dev_priv)->info.has_csr)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8241b673940d..3137572752e3 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -32,7 +32,6 @@ struct i915_params i915 __read_mostly = {
.lvds_channel_mode = 0,
.panel_use_ssc = -1,
.vbt_sdvo_panel_type = -1,
- .enable_rc6 = -1,
.enable_dc = -1,
.enable_fbc = -1,
.enable_hangcheck = true,
@@ -81,14 +80,6 @@ MODULE_PARM_DESC(semaphores,
"Use semaphores for inter-ring sync "
"(default: -1 (use per-chip defaults))");
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400);
-MODULE_PARM_DESC(enable_rc6,
- "Enable power-saving render C-state 6. "
- "Different stages can be selected via bitmask values "
- "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). "
- "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
- "default: -1 (use per-chip default)");
-
module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
MODULE_PARM_DESC(enable_dc,
"Enable power-saving display C-states. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index b7bf1578f3eb..b6686a5450fa 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -34,7 +34,6 @@
func(int, lvds_channel_mode); \
func(int, panel_use_ssc); \
func(int, vbt_sdvo_panel_type); \
- func(int, enable_rc6); \
func(int, enable_dc); \
func(int, enable_fbc); \
func(int, enable_ppgtt); \
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f87b0c4e564d..e0846c6c33ae 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -200,6 +200,8 @@ static const struct intel_device_info intel_gm45_info = {
GEN_DEFAULT_PIPEOFFSETS, \
CURSOR_OFFSETS
+/* Ironlake does support rc6, but we do not implement the power contexts */
+
static const struct intel_device_info intel_ironlake_d_info = {
GEN5_FEATURES,
.platform = INTEL_IRONLAKE,
@@ -218,7 +220,6 @@ static const struct intel_device_info intel_ironlake_m_info = {
.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
.has_llc = 1, \
.has_rc6 = 1, \
- .has_rc6p = 1, \
.has_gmbus_irq = 1, \
.has_hw_contexts = 1, \
.has_aliasing_ppgtt = 1, \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0b3c813336d5..05e5f3b6f2c8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1831,11 +1831,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry **entries,
const struct skl_ddb_entry *ddb,
int ignore);
bool ilk_disable_lp_wm(struct drm_device *dev);
-int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
-static inline int intel_enable_rc6(void)
-{
- return i915.enable_rc6;
-}
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ab6fff8369ac..9c92b908530f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5460,26 +5460,6 @@ static void valleyview_disable_rps(struct drm_i915_private *dev_priv)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
}
-static void intel_print_rc6_info(struct drm_i915_private *dev_priv, u32 mode)
-{
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- if (mode & (GEN7_RC_CTL_TO_MODE | GEN6_RC_CTL_EI_MODE(1)))
- mode = GEN6_RC_CTL_RC6_ENABLE;
- else
- mode = 0;
- }
- if (HAS_RC6p(dev_priv))
- DRM_DEBUG_DRIVER("Enabling RC6 states: "
- "RC6 %s RC6p %s RC6pp %s\n",
- onoff(mode & GEN6_RC_CTL_RC6_ENABLE),
- onoff(mode & GEN6_RC_CTL_RC6p_ENABLE),
- onoff(mode & GEN6_RC_CTL_RC6pp_ENABLE));
-
- else
- DRM_DEBUG_DRIVER("Enabling RC6 states: RC6 %s\n",
- onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
-}
-
static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
{
struct i915_ggtt *ggtt = &dev_priv->ggtt;
@@ -5542,42 +5522,16 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
return enable_rc6;
}
-int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
+static bool sanitize_rc6(struct drm_i915_private *i915)
{
- /* No RC6 before Ironlake and code is gone for ilk. */
- if (INTEL_INFO(dev_priv)->gen < 6)
- return 0;
-
- if (!enable_rc6)
- return 0;
+ struct intel_device_info *info = mkwrite_device_info(i915);
- if (IS_GEN9_LP(dev_priv) && !bxt_check_bios_rc6_setup(dev_priv)) {
+ if (IS_GEN9_LP(i915) && !bxt_check_bios_rc6_setup(i915)) {
DRM_INFO("RC6 disabled by BIOS\n");
- return 0;
- }
-
- /* Respect the kernel parameter if it is set */
- if (enable_rc6 >= 0) {
- int mask;
-
- if (HAS_RC6p(dev_priv))
- mask = INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE |
- INTEL_RC6pp_ENABLE;
- else
- mask = INTEL_RC6_ENABLE;
-
- if ((enable_rc6 & mask) != enable_rc6)
- DRM_DEBUG_DRIVER("Adjusting RC6 mask to %d "
- "(requested %d, valid %d)\n",
- enable_rc6 & mask, enable_rc6, mask);
-
- return enable_rc6 & mask;
+ info->has_rc6 = 0;
}
- if (IS_IVYBRIDGE(dev_priv))
- return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
-
- return INTEL_RC6_ENABLE;
+ return info->has_rc6;
}
static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
@@ -5666,7 +5620,6 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- uint32_t rc6_mask = 0;
/* 1a: Software RC state - RC0 */
I915_WRITE(GEN6_RC_STATE, 0);
@@ -5700,22 +5653,19 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
/* 3a: Enable RC6 */
- if (intel_enable_rc6() & INTEL_RC6_ENABLE)
- rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
- DRM_INFO("RC6 %s\n", onoff(rc6_mask & GEN6_RC_CTL_RC6_ENABLE));
I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
I915_WRITE(GEN6_RC_CONTROL,
- GEN6_RC_CTL_HW_ENABLE | GEN6_RC_CTL_EI_MODE(1) | rc6_mask);
+ GEN6_RC_CTL_HW_ENABLE |
+ GEN6_RC_CTL_EI_MODE(1) |
+ GEN6_RC_CTL_RC6_ENABLE);
/*
* 3b: Enable Coarse Power Gating only when RC6 is enabled.
* WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be disabled with RC6.
*/
- if (NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
- I915_WRITE(GEN9_PG_ENABLE, 0);
- else
- I915_WRITE(GEN9_PG_ENABLE, (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
- (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0);
+ I915_WRITE(GEN9_PG_ENABLE,
+ NEEDS_WaRsDisableCoarsePowerGating(dev_priv) ?
+ (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0);
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
}
@@ -5724,7 +5674,6 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- uint32_t rc6_mask = 0;
/* 1a: Software RC state - RC0 */
I915_WRITE(GEN6_RC_STATE, 0);
@@ -5749,17 +5698,14 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
/* 3: Enable RC6 */
- if (intel_enable_rc6() & INTEL_RC6_ENABLE)
- rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
- intel_print_rc6_info(dev_priv, rc6_mask);
if (IS_BROADWELL(dev_priv))
I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
GEN7_RC_CTL_TO_MODE |
- rc6_mask);
+ GEN6_RC_CTL_RC6_ENABLE);
else
I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
GEN6_RC_CTL_EI_MODE(1) |
- rc6_mask);
+ GEN6_RC_CTL_RC6_ENABLE);
/* 4 Program defaults and thresholds for RPS*/
I915_WRITE(GEN6_RPNSWREQ,
@@ -5801,9 +5747,8 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- u32 rc6vids, rc6_mask = 0;
+ u32 rc6vids, rc6_mask;
u32 gtfifodbg;
- int rc6_mode;
int ret;
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
@@ -5846,22 +5791,12 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_RC6p_THRESHOLD, 150000);
I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
- /* Check if we are enabling RC6 */
- rc6_mode = intel_enable_rc6();
- if (rc6_mode & INTEL_RC6_ENABLE)
- rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
-
/* We don't use those on Haswell */
- if (!IS_HASWELL(dev_priv)) {
- if (rc6_mode & INTEL_RC6p_ENABLE)
- rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
-
- if (rc6_mode & INTEL_RC6pp_ENABLE)
- rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
- }
-
- intel_print_rc6_info(dev_priv, rc6_mask);
-
+ rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
+ if (HAS_RC6p(dev_priv))
+ rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
+ if (HAS_RC6pp(dev_priv))
+ rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
I915_WRITE(GEN6_RC_CONTROL,
rc6_mask |
GEN6_RC_CTL_EI_MODE(1) |
@@ -6290,7 +6225,7 @@ static void cherryview_enable_rps(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- u32 gtfifodbg, val, rc6_mode = 0, pcbr;
+ u32 gtfifodbg, val, rc6_mode, pcbr;
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
@@ -6333,10 +6268,9 @@ static void cherryview_enable_rps(struct drm_i915_private *dev_priv)
pcbr = I915_READ(VLV_PCBR);
/* 3: Enable RC6 */
- if ((intel_enable_rc6() & INTEL_RC6_ENABLE) &&
- (pcbr >> VLV_PCBR_ADDR_SHIFT))
+ rc6_mode = 0;
+ if (pcbr >> VLV_PCBR_ADDR_SHIFT)
rc6_mode = GEN7_RC_CTL_TO_MODE;
-
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
/* 4 Program defaults and thresholds for RPS*/
@@ -6379,7 +6313,7 @@ static void valleyview_enable_rps(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- u32 gtfifodbg, val, rc6_mode = 0;
+ u32 gtfifodbg, val;
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
@@ -6431,12 +6365,8 @@ static void valleyview_enable_rps(struct drm_i915_private *dev_priv)
VLV_MEDIA_RC6_COUNT_EN |
VLV_RENDER_RC6_COUNT_EN));
- if (intel_enable_rc6() & INTEL_RC6_ENABLE)
- rc6_mode = GEN7_RC_CTL_TO_MODE | VLV_RC_CTL_CTX_RST_PARALLEL;
-
- intel_print_rc6_info(dev_priv, rc6_mode);
-
- I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ I915_WRITE(GEN6_RC_CONTROL,
+ GEN7_RC_CTL_TO_MODE | VLV_RC_CTL_CTX_RST_PARALLEL);
/* Setting Fixed Bias */
val = VLV_OVERRIDE_EN |
@@ -6936,7 +6866,7 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
* RPM depends on RC6 to save restore the GT HW context, so make RC6 a
* requirement.
*/
- if (!i915.enable_rc6) {
+ if (!sanitize_rc6(dev_priv)) {
DRM_INFO("RC6 disabled, disabling runtime PM support\n");
intel_runtime_pm_get(dev_priv);
}
@@ -6993,7 +6923,7 @@ void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
if (IS_VALLEYVIEW(dev_priv))
valleyview_cleanup_gt_powersave(dev_priv);
- if (!i915.enable_rc6)
+ if (!HAS_RC6(dev_priv))
intel_runtime_pm_put(dev_priv);
}
@@ -8433,7 +8363,7 @@ u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
{
u64 time_hw, units, div;
- if (!intel_enable_rc6())
+ if (!HAS_RC6(dev_priv))
return 0;
intel_runtime_pm_get(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index e01622757b84..9d08f7fd5497 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -296,7 +296,7 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
action[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
/* WaRsDisableCoarsePowerGating:skl,bxt */
- if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
+ if (NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
action[1] = 0;
else
/* bit 0 and 1 are for Render and Media domain separately */
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b036a2d5c2d4..06483227b594 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -432,8 +432,6 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv)
void intel_uncore_sanitize(struct drm_i915_private *dev_priv)
{
- i915.enable_rc6 = sanitize_rc6_option(dev_priv, i915.enable_rc6);
-
/* BIOS often leaves RC6 enabled, but disable it for hw init */
intel_sanitize_gt_powersave(dev_priv);
}
--
2.11.0
More information about the Intel-gfx
mailing list