* [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
@ 2016-01-14 15:56 Sagar Arun Kamble
2016-01-14 16:30 ` ✗ failure: Fi.CI.BAT Patchwork
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Sagar Arun Kamble @ 2016-01-14 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: shashidhar.hiremath
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
setup registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.
RC6 related instability can be avoided by disabling via BIOS settings
till driver fixes it.
v2: Had placed logic in gen8 function by mistake. Fixed it.
Ensuring RPM is not enabled in case BIOS disabled RC6.
v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
Runtime PM enabling happens before gen9_enable_rc6.
Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
v5: Caching reserved stolen base and size in the driver private data.
Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
Cc: Imre Deak <imre.deak@intel.com>
Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++++++++++++++++++----
drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 59 ++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_uncore.c | 4 +++
7 files changed, 109 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf7e0fc..0c8e61c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3223,6 +3223,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
u64 end);
void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
struct drm_mm_node *node);
+int i915_gem_init_stolen_reserved(struct drm_device *dev);
int i915_gem_init_stolen(struct drm_device *dev);
void i915_gem_cleanup_stolen(struct drm_device *dev);
struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad8..20ff6ca 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -343,6 +343,8 @@ struct i915_gtt {
size_t stolen_size; /* Total size of stolen memory */
size_t stolen_usable_size; /* Total size minus BIOS reserved */
+ size_t stolen_reserved_base;
+ size_t stolen_reserved_size;
u64 mappable_end; /* End offset that we can CPU map */
struct io_mapping *mappable; /* Mapping to our CPU mappable region */
phys_addr_t mappable_base; /* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 3476877..d5a65d9 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
*size = stolen_top - *base;
}
-int i915_gem_init_stolen(struct drm_device *dev)
+int i915_gem_init_stolen_reserved(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long reserved_total, reserved_base = 0, reserved_size;
+ unsigned long reserved_base = 0, reserved_size;
unsigned long stolen_top;
- mutex_init(&dev_priv->mm.stolen_lock);
-
#ifdef CONFIG_INTEL_IOMMU
if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
DRM_INFO("DMAR active, disabling use of stolen memory\n");
@@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device *dev)
return 0;
}
+ dev_priv->gtt.stolen_reserved_base = reserved_base;
+ dev_priv->gtt.stolen_reserved_size = reserved_size;
+
+ return 0;
+}
+
+int i915_gem_init_stolen(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reserved_total;
+ unsigned long stolen_top;
+
+ mutex_init(&dev_priv->mm.stolen_lock);
+
+#ifdef CONFIG_INTEL_IOMMU
+ if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
+ DRM_INFO("DMAR active, disabling use of stolen memory\n");
+ return 0;
+ }
+#endif
+
+ if (dev_priv->gtt.stolen_size == 0)
+ return 0;
+
+ if (dev_priv->mm.stolen_base == 0)
+ return 0;
+
+ stolen_top = dev_priv->mm.stolen_base + dev_priv->gtt.stolen_size;
+
/* It is possible for the reserved area to end before the end of stolen
- * memory, so just consider the start. */
- reserved_total = stolen_top - reserved_base;
+ * memory, so just consider the start. */
+ reserved_total = stolen_top - dev_priv->gtt.stolen_reserved_base;
DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK, usable: %luK\n",
dev_priv->gtt.stolen_size >> 10,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 007ae83..19ddf79 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6765,6 +6765,16 @@ enum skl_disp_power_wells {
#define VLV_PMWGICZ _MMIO(0x1300a4)
+#define RC6_LOCATION _MMIO(0xD40)
+#define RC6_CTX_IN_DRAM 1
+#define RC6_CTX_BASE _MMIO(0xD48)
+#define RC6_CTX_BASE_MASK 0xFFFFFFF0
+#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
+#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
+#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
+#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
+#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
+#define IDLE_TIME_MASK 0xFFFFF
#define FORCEWAKE _MMIO(0xA18C)
#define FORCEWAKE_VLV _MMIO(0x1300b0)
#define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
@@ -6903,6 +6913,7 @@ enum skl_disp_power_wells {
#define GEN6_RPDEUC _MMIO(0xA084)
#define GEN6_RPDEUCSW _MMIO(0xA088)
#define GEN6_RC_STATE _MMIO(0xA094)
+#define RC6_STATE (1<<18)
#define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
#define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
#define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0438b57..f22baef 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1564,6 +1564,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 02fe081..c9a32a4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4518,12 +4518,68 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
(mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
}
-static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
+static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ bool enable_rc6 = true;
+ unsigned long rc6_ctx_base;
+ bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
+
+ bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+ (GEN6_RC_CTL_RC6_ENABLE |
+ GEN6_RC_CTL_HW_ENABLE);
+ bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
+ GEN6_RC_CTL_HW_ENABLE) &&
+ (I915_READ(GEN6_RC_STATE) & RC6_STATE);
+
+ if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
+ DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
+ if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
+ (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
+ dev_priv->gtt.stolen_reserved_size)))) {
+ DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
+ enable_rc6 = false;
+ }
+
+ if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
+ DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ if (HAS_BSD2(dev) &&
+ ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
+ DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
+ DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
+ enable_rc6 = false;
+ }
+
+ return enable_rc6;
+}
+
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
{
/* No RC6 before Ironlake and code is gone for ilk. */
if (INTEL_INFO(dev)->gen < 6)
return 0;
+ if (IS_BROXTON(dev)) {
+ if (!bxt_check_bios_rc6_setup(dev)) {
+ DRM_INFO("RC6 disabled by BIOS\n");
+ return 0;
+ }
+ }
+
/* Respect the kernel parameter if it is set */
if (enable_rc6 >= 0) {
int mask;
@@ -6014,7 +6070,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
/*
* RPM depends on RC6 to save restore the GT HW context, so make RC6 a
* requirement.
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 277e60a..43fc3e8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -359,6 +359,10 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
void intel_uncore_sanitize(struct drm_device *dev)
{
+ i915_gem_init_stolen_reserved(dev);
+
+ i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
+
/* BIOS often leaves RC6 enabled, but disable it for hw init */
intel_disable_gt_powersave(dev);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✗ failure: Fi.CI.BAT
2016-01-14 15:56 [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Sagar Arun Kamble
@ 2016-01-14 16:30 ` Patchwork
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2016-01-14 16:30 UTC (permalink / raw)
To: sagar.a.kamble; +Cc: intel-gfx
== Summary ==
HEAD is now at 8fb2fee drm-intel-nightly: 2016y-01m-14d-13h-06m-44s UTC integration manifest
Applying: drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_drv.h
M drivers/gpu/drm/i915/i915_gem_stolen.c
M drivers/gpu/drm/i915/i915_reg.h
M drivers/gpu/drm/i915/intel_drv.h
M drivers/gpu/drm/i915/intel_pm.c
M drivers/gpu/drm/i915/intel_uncore.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_uncore.c
Auto-merging drivers/gpu/drm/i915/intel_pm.c
Auto-merging drivers/gpu/drm/i915/intel_drv.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_drv.h
Auto-merging drivers/gpu/drm/i915/i915_reg.h
Auto-merging drivers/gpu/drm/i915/i915_gem_stolen.c
Auto-merging drivers/gpu/drm/i915/i915_drv.h
Patch failed at 0001 drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-01-14 15:56 [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Sagar Arun Kamble
2016-01-14 16:30 ` ✗ failure: Fi.CI.BAT Patchwork
@ 2016-01-15 14:44 ` Imre Deak
2016-01-15 14:49 ` Imre Deak
` (2 more replies)
2016-02-01 8:30 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev3) Patchwork
2016-02-03 8:43 ` ✓ Fi.CI.BAT: success for series starting with [v7,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev4) Patchwork
3 siblings, 3 replies; 11+ messages in thread
From: Imre Deak @ 2016-01-15 14:44 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx, Chris Wilson; +Cc: shashidhar.hiremath
Adding Chris. We would need stolen_base and size early, could you check
if moving i915_gem_init_stolen() earlier based on the diff at the end
is ok?
On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> setup registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> RC6 related instability can be avoided by disabling via BIOS settings
> till driver fixes it.
>
> v2: Had placed logic in gen8 function by mistake. Fixed it.
> Ensuring RPM is not enabled in case BIOS disabled RC6.
>
> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
> Runtime PM enabling happens before gen9_enable_rc6.
> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
>
> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
>
> v5: Caching reserved stolen base and size in the driver private data.
> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
> drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++++++++++++++++++----
> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 59 ++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_uncore.c | 4 +++
> 7 files changed, 109 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cf7e0fc..0c8e61c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3223,6 +3223,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> u64 end);
> void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
> struct drm_mm_node *node);
> +int i915_gem_init_stolen_reserved(struct drm_device *dev);
> int i915_gem_init_stolen(struct drm_device *dev);
> void i915_gem_cleanup_stolen(struct drm_device *dev);
> struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index b448ad8..20ff6ca 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -343,6 +343,8 @@ struct i915_gtt {
>
> size_t stolen_size; /* Total size of stolen memory */
> size_t stolen_usable_size; /* Total size minus BIOS reserved */
> + size_t stolen_reserved_base;
> + size_t stolen_reserved_size;
> u64 mappable_end; /* End offset that we can CPU map */
> struct io_mapping *mappable; /* Mapping to our CPU mappable region */
> phys_addr_t mappable_base; /* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 3476877..d5a65d9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
> *size = stolen_top - *base;
> }
>
> -int i915_gem_init_stolen(struct drm_device *dev)
> +int i915_gem_init_stolen_reserved(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - unsigned long reserved_total, reserved_base = 0, reserved_size;
> + unsigned long reserved_base = 0, reserved_size;
> unsigned long stolen_top;
>
> - mutex_init(&dev_priv->mm.stolen_lock);
> -
> #ifdef CONFIG_INTEL_IOMMU
> if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> DRM_INFO("DMAR active, disabling use of stolen memory\n");
> @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device *dev)
> return 0;
> }
>
> + dev_priv->gtt.stolen_reserved_base = reserved_base;
> + dev_priv->gtt.stolen_reserved_size = reserved_size;
> +
> + return 0;
> +}
> +
> +int i915_gem_init_stolen(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned long reserved_total;
> + unsigned long stolen_top;
> +
> + mutex_init(&dev_priv->mm.stolen_lock);
> +
> +#ifdef CONFIG_INTEL_IOMMU
> + if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> + DRM_INFO("DMAR active, disabling use of stolen memory\n");
> + return 0;
> + }
> +#endif
> +
> + if (dev_priv->gtt.stolen_size == 0)
> + return 0;
> +
> + if (dev_priv->mm.stolen_base == 0)
> + return 0;
> +
> + stolen_top = dev_priv->mm.stolen_base + dev_priv->gtt.stolen_size;
> +
> /* It is possible for the reserved area to end before the end of stolen
> - * memory, so just consider the start. */
> - reserved_total = stolen_top - reserved_base;
> + * memory, so just consider the start. */
> + reserved_total = stolen_top - dev_priv->gtt.stolen_reserved_base;
>
> DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK, usable: %luK\n",
> dev_priv->gtt.stolen_size >> 10,
Hm, I don't see a reason to split i915_gem_init_stolen()
and i915_gem_init_stolen_reserved(). We could just bring
i915_gem_init_stolen() earlier (in a separate patch), see what I meant
at the end.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 007ae83..19ddf79 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6765,6 +6765,16 @@ enum skl_disp_power_wells {
>
> #define VLV_PMWGICZ _MMIO(0x1300a4)
>
> +#define RC6_LOCATION _MMIO(0xD40)
> +#define RC6_CTX_IN_DRAM 1
> +#define RC6_CTX_BASE _MMIO(0xD48)
> +#define RC6_CTX_BASE_MASK 0xFFFFFFF0
> +#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
> +#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
> +#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
> +#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
> +#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
> +#define IDLE_TIME_MASK 0xFFFFF
> #define FORCEWAKE _MMIO(0xA18C)
> #define FORCEWAKE_VLV _MMIO(0x1300b0)
> #define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
> @@ -6903,6 +6913,7 @@ enum skl_disp_power_wells {
> #define GEN6_RPDEUC _MMIO(0xA084)
> #define GEN6_RPDEUCSW _MMIO(0xA088)
> #define GEN6_RC_STATE _MMIO(0xA094)
> +#define RC6_STATE (1<<18)
> #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
> #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
> #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0438b57..f22baef 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1564,6 +1564,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb /* out */);
> uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>
> /* intel_sdvo.c */
> bool intel_sdvo_init(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 02fe081..c9a32a4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4518,12 +4518,68 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
> (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
> }
>
> -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + bool enable_rc6 = true;
> + unsigned long rc6_ctx_base;
> + bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
> +
> + bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> + (GEN6_RC_CTL_RC6_ENABLE |
> + GEN6_RC_CTL_HW_ENABLE);
> + bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
> + GEN6_RC_CTL_HW_ENABLE) &&
> + (I915_READ(GEN6_RC_STATE) & RC6_STATE);
> +
> + if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
> + DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
> + enable_rc6 = false;
> + }
> +
> + rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
> + if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
> + (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
> + dev_priv->gtt.stolen_reserved_size)))) {
> + DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
> + enable_rc6 = false;
> + }
> +
> + if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
> + DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
> + enable_rc6 = false;
> + }
> +
> + if (HAS_BSD2(dev) &&
> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
> + DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
> + enable_rc6 = false;
> + }
> +
> + if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
> + DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
> + enable_rc6 = false;
> + }
> +
> + return enable_rc6;
> +}
> +
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> {
> /* No RC6 before Ironlake and code is gone for ilk. */
> if (INTEL_INFO(dev)->gen < 6)
> return 0;
Please also add here an early return if enable_rc6 is already 0, so we
don't get redundant "RC6 disabled" messages in the log.
>
> + if (IS_BROXTON(dev)) {
> + if (!bxt_check_bios_rc6_setup(dev)) {
Nitpick, could be a single if.
> + DRM_INFO("RC6 disabled by BIOS\n");
> + return 0;
> + }
> + }
> +
> /* Respect the kernel parameter if it is set */
> if (enable_rc6 >= 0) {
> int mask;
> @@ -6014,7 +6070,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
> /*
> * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
> * requirement.
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 277e60a..43fc3e8 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -359,6 +359,10 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
>
> void intel_uncore_sanitize(struct drm_device *dev)
> {
> + i915_gem_init_stolen_reserved(dev);
This would now be called both during driver loading and resume which is
redundant. Also it would run before intel_setup_mchbar() which isn't
good. So I would suggest instead moving i915_gem_init_stolen() earlier
according to the following and calling intel_uncore_sanitize() right
after it, provided that Chris doesn't see any problem with that:
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a0f5659..f8cc66e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -391,20 +391,13 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_client;
- /* Initialise stolen first so that we may reserve preallocated
- * objects for the BIOS to KMS transition.
- */
- ret = i915_gem_init_stolen(dev);
- if (ret)
- goto cleanup_vga_switcheroo;
-
intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
ret = intel_irq_install(dev_priv);
if (ret)
- goto cleanup_gem_stolen;
+ goto cleanup_vga_switcheroo;
intel_setup_gmbus(dev);
@@ -458,8 +451,6 @@ cleanup_irq:
intel_guc_ucode_fini(dev);
drm_irq_uninstall(dev);
intel_teardown_gmbus(dev);
-cleanup_gem_stolen:
- i915_gem_cleanup_stolen(dev);
cleanup_vga_switcheroo:
vga_switcheroo_unregister_client(dev->pdev);
cleanup_vga_client:
@@ -1032,6 +1023,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
+ /* Initialise stolen first so that we may reserve preallocated
+ * objects for the BIOS to KMS transition.
+ */
+ ret = i915_gem_init_stolen(dev);
+ if (ret)
+ goto out_free_hangcheckwq;
intel_opregion_setup(dev);
i915_gem_load(dev);
@@ -1104,8 +1101,10 @@ out_gem_unload:
if (dev->pdev->msi_enabled)
pci_disable_msi(dev->pdev);
+ i915_gem_cleanup_stolen(dev);
intel_teardown_mchbar(dev);
pm_qos_remove_request(&dev_priv->pm_qos);
+out_free_hangcheckwq:
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
destroy_workqueue(dev_priv->hotplug.dp_wq);
> +
> + i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
> +
> /* BIOS often leaves RC6 enabled, but disable it for hw init */
> intel_disable_gt_powersave(dev);
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
@ 2016-01-15 14:49 ` Imre Deak
2016-01-15 15:29 ` Chris Wilson
2016-01-29 17:52 ` [PATCH v6 " Sagar Arun Kamble
2 siblings, 0 replies; 11+ messages in thread
From: Imre Deak @ 2016-01-15 14:49 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx, Chris Wilson; +Cc: shashidhar.hiremath
On pe, 2016-01-15 at 16:44 +0200, Imre Deak wrote:
> Adding Chris. We would need stolen_base and size early, could you
> check
> if moving i915_gem_init_stolen() earlier based on the diff at the end
> is ok?
Oops, I meant stolen reserved_base and reserved_size.
> On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote:
> > RC6 setup is shared between BIOS and Driver. BIOS sets up subset of
> > RC6
> > setup registers. If those are not setup Driver should not enable
> > RC6.
> > For implementing this, driver can check RC_CTRL0 and RC_CTRL1
> > values
> > to know if BIOS has enabled HW/SW RC6.
> > This will also enable user to control RC6 using BIOS settings
> > alone.
> > RC6 related instability can be avoided by disabling via BIOS
> > settings
> > till driver fixes it.
> >
> > v2: Had placed logic in gen8 function by mistake. Fixed it.
> > Ensuring RPM is not enabled in case BIOS disabled RC6.
> >
> > v3: Need to disable RPM if RC6 is disabled due to BIOS settings.
> > (Daniel)
> > Runtime PM enabling happens before gen9_enable_rc6.
> > Moved the updation of enable_rc6 parameter in
> > intel_uncore_sanitize.
> >
> > v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx
> > for bxt. (Imre)
> >
> > v5: Caching reserved stolen base and size in the driver private
> > data.
> > Reorganized RC6 setup check. Moved from gen9_enable_rc6 to
> > intel_uncore_sanitize. (Imre)
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++++++++++++++++++----
> > drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 59
> > ++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_uncore.c | 4 +++
> > 7 files changed, 109 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index cf7e0fc..0c8e61c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3223,6 +3223,7 @@ int
> > i915_gem_stolen_insert_node_in_range(struct drm_i915_private
> > *dev_priv,
> > u64 end);
> > void i915_gem_stolen_remove_node(struct drm_i915_private
> > *dev_priv,
> > struct drm_mm_node *node);
> > +int i915_gem_init_stolen_reserved(struct drm_device *dev);
> > int i915_gem_init_stolen(struct drm_device *dev);
> > void i915_gem_cleanup_stolen(struct drm_device *dev);
> > struct drm_i915_gem_object *
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index b448ad8..20ff6ca 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -343,6 +343,8 @@ struct i915_gtt {
> >
> > size_t stolen_size; /* Total size of stolen
> > memory */
> > size_t stolen_usable_size; /* Total size minus BIOS
> > reserved */
> > + size_t stolen_reserved_base;
> > + size_t stolen_reserved_size;
> > u64 mappable_end; /* End offset that we can
> > CPU map */
> > struct io_mapping *mappable; /* Mapping to our CPU
> > mappable region */
> > phys_addr_t mappable_base; /* PA of our GMADR */
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 3476877..d5a65d9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct
> > drm_i915_private *dev_priv,
> > *size = stolen_top - *base;
> > }
> >
> > -int i915_gem_init_stolen(struct drm_device *dev)
> > +int i915_gem_init_stolen_reserved(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - unsigned long reserved_total, reserved_base = 0,
> > reserved_size;
> > + unsigned long reserved_base = 0, reserved_size;
> > unsigned long stolen_top;
> >
> > - mutex_init(&dev_priv->mm.stolen_lock);
> > -
> > #ifdef CONFIG_INTEL_IOMMU
> > if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> > DRM_INFO("DMAR active, disabling use of stolen
> > memory\n");
> > @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device
> > *dev)
> > return 0;
> > }
> >
> > + dev_priv->gtt.stolen_reserved_base = reserved_base;
> > + dev_priv->gtt.stolen_reserved_size = reserved_size;
> > +
> > + return 0;
> > +}
> > +
> > +int i915_gem_init_stolen(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + unsigned long reserved_total;
> > + unsigned long stolen_top;
> > +
> > + mutex_init(&dev_priv->mm.stolen_lock);
> > +
> > +#ifdef CONFIG_INTEL_IOMMU
> > + if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> > + DRM_INFO("DMAR active, disabling use of stolen
> > memory\n");
> > + return 0;
> > + }
> > +#endif
> > +
> > + if (dev_priv->gtt.stolen_size == 0)
> > + return 0;
> > +
> > + if (dev_priv->mm.stolen_base == 0)
> > + return 0;
> > +
> > + stolen_top = dev_priv->mm.stolen_base + dev_priv-
> > >gtt.stolen_size;
> > +
> > /* It is possible for the reserved area to end before the
> > end of stolen
> > - * memory, so just consider the start. */
> > - reserved_total = stolen_top - reserved_base;
> > + * memory, so just consider the start. */
> > + reserved_total = stolen_top - dev_priv-
> > >gtt.stolen_reserved_base;
> >
> > DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK,
> > usable: %luK\n",
> > dev_priv->gtt.stolen_size >> 10,
>
> Hm, I don't see a reason to split i915_gem_init_stolen()
> and i915_gem_init_stolen_reserved(). We could just bring
> i915_gem_init_stolen() earlier (in a separate patch), see what I
> meant
> at the end.
>
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 007ae83..19ddf79 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6765,6 +6765,16 @@ enum skl_disp_power_wells {
> >
> > #define VLV_PMWGICZ _MMIO(0x1300a4
> > )
> >
> > +#define RC6_LOCATION _MMIO(0xD40)
> > +#define RC6_CTX_IN_DRAM 1
> > +#define RC6_CTX_BASE _MMIO(0xD48)
> > +#define RC6_CTX_BASE_MASK 0xFFFFFFF0
> > +#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054
> > )
> > +#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x120
> > 54)
> > +#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x2205
> > 4)
> > +#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A0
> > 54)
> > +#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C0
> > 54)
> > +#define IDLE_TIME_MASK 0xFFFFF
> > #define FORCEWAKE _MMIO(0xA18C)
> > #define FORCEWAKE_VLV _MMIO(0x1300
> > b0)
> > #define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
> > @@ -6903,6 +6913,7 @@ enum skl_disp_power_wells {
> > #define GEN6_RPDEUC _MMIO(0xA084)
> > #define GEN6_RPDEUCSW _MMIO(0xA088)
> > #define GEN6_RC_STATE _MMIO(0xA094)
> > +#define RC6_STATE (1<<18)
> > #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
> > #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
> > #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 0438b57..f22baef 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1564,6 +1564,7 @@ void skl_wm_get_hw_state(struct drm_device
> > *dev);
> > void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> > struct skl_ddb_allocation *ddb /* out
> > */);
> > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state
> > *pipe_config);
> > +int sanitize_rc6_option(const struct drm_device *dev, int
> > enable_rc6);
> >
> > /* intel_sdvo.c */
> > bool intel_sdvo_init(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 02fe081..c9a32a4 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4518,12 +4518,68 @@ static void intel_print_rc6_info(struct
> > drm_device *dev, u32 mode)
> > (mode & GEN6_RC_CTL_RC6_ENABLE) ?
> > "on" : "off");
> > }
> >
> > -static int sanitize_rc6_option(const struct drm_device *dev, int
> > enable_rc6)
> > +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + bool enable_rc6 = true;
> > + unsigned long rc6_ctx_base;
> > + bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
> > +
> > + bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> > + (GEN6_RC_CTL_RC6_ENABLE |
> > + GEN6_RC_CTL_HW_ENABLE);
> > + bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
> > + GEN6_RC_CTL_HW_ENABLE) &&
> > + (I915_READ(GEN6_RC_STATE) &
> > RC6_STATE);
> > +
> > + if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
> > + DRM_DEBUG_KMS("RC6 Base location not set
> > properly.\n");
> > + enable_rc6 = false;
> > + }
> > +
> > + rc6_ctx_base = I915_READ(RC6_CTX_BASE) &
> > RC6_CTX_BASE_MASK;
> > + if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base)
> > &&
> > + (rc6_ctx_base <= (dev_priv-
> > >gtt.stolen_reserved_base +
> > + dev_priv-
> > >gtt.stolen_reserved_size)))) {
> > + DRM_DEBUG_KMS("RC6 Base address not as
> > expected.\n");
> > + enable_rc6 = false;
> > + }
> > +
> > + if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK)
> > > 1) &&
> > + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) &
> > IDLE_TIME_MASK) > 1) &&
> > + ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK)
> > > 1) &&
> > + ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) &
> > IDLE_TIME_MASK) > 1))) {
> > + DRM_DEBUG_KMS("Engine Idle wait time not set
> > properly.\n");
> > + enable_rc6 = false;
> > + }
> > +
> > + if (HAS_BSD2(dev) &&
> > + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) &
> > IDLE_TIME_MASK) > 1)) {
> > + DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set
> > properly.\n");
> > + enable_rc6 = false;
> > + }
> > +
> > + if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
> > + DRM_DEBUG_KMS("HW/SW RC6 is not enabled by
> > BIOS.\n");
> > + enable_rc6 = false;
> > + }
> > +
> > + return enable_rc6;
> > +}
> > +
> > +int sanitize_rc6_option(const struct drm_device *dev, int
> > enable_rc6)
> > {
> > /* No RC6 before Ironlake and code is gone for ilk. */
> > if (INTEL_INFO(dev)->gen < 6)
> > return 0;
>
> Please also add here an early return if enable_rc6 is already 0, so
> we
> don't get redundant "RC6 disabled" messages in the log.
>
> >
> > + if (IS_BROXTON(dev)) {
> > + if (!bxt_check_bios_rc6_setup(dev)) {
>
> Nitpick, could be a single if.
>
> > + DRM_INFO("RC6 disabled by BIOS\n");
> > + return 0;
> > + }
> > + }
> > +
> > /* Respect the kernel parameter if it is set */
> > if (enable_rc6 >= 0) {
> > int mask;
> > @@ -6014,7 +6070,6 @@ void intel_init_gt_powersave(struct
> > drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - i915.enable_rc6 = sanitize_rc6_option(dev,
> > i915.enable_rc6);
> > /*
> > * RPM depends on RC6 to save restore the GT HW context,
> > so make RC6 a
> > * requirement.
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index 277e60a..43fc3e8 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -359,6 +359,10 @@ void intel_uncore_early_sanitize(struct
> > drm_device *dev, bool restore_forcewake)
> >
> > void intel_uncore_sanitize(struct drm_device *dev)
> > {
> > + i915_gem_init_stolen_reserved(dev);
>
> This would now be called both during driver loading and resume which
> is
> redundant. Also it would run before intel_setup_mchbar() which isn't
> good. So I would suggest instead moving i915_gem_init_stolen()
> earlier
> according to the following and calling intel_uncore_sanitize() right
> after it, provided that Chris doesn't see any problem with that:
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index a0f5659..f8cc66e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -391,20 +391,13 @@ static int i915_load_modeset_init(struct
> drm_device *dev)
> if (ret)
> goto cleanup_vga_client;
>
> - /* Initialise stolen first so that we may reserve
> preallocated
> - * objects for the BIOS to KMS transition.
> - */
> - ret = i915_gem_init_stolen(dev);
> - if (ret)
> - goto cleanup_vga_switcheroo;
> -
> intel_power_domains_init_hw(dev_priv, false);
>
> intel_csr_ucode_init(dev_priv);
>
> ret = intel_irq_install(dev_priv);
> if (ret)
> - goto cleanup_gem_stolen;
> + goto cleanup_vga_switcheroo;
>
> intel_setup_gmbus(dev);
>
> @@ -458,8 +451,6 @@ cleanup_irq:
> intel_guc_ucode_fini(dev);
> drm_irq_uninstall(dev);
> intel_teardown_gmbus(dev);
> -cleanup_gem_stolen:
> - i915_gem_cleanup_stolen(dev);
> cleanup_vga_switcheroo:
> vga_switcheroo_unregister_client(dev->pdev);
> cleanup_vga_client:
> @@ -1032,6 +1023,12 @@ int i915_driver_load(struct drm_device *dev,
> unsigned long flags)
>
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev);
> + /* Initialise stolen first so that we may reserve
> preallocated
> + * objects for the BIOS to KMS transition.
> + */
> + ret = i915_gem_init_stolen(dev);
> + if (ret)
> + goto out_free_hangcheckwq;
> intel_opregion_setup(dev);
>
> i915_gem_load(dev);
> @@ -1104,8 +1101,10 @@ out_gem_unload:
> if (dev->pdev->msi_enabled)
> pci_disable_msi(dev->pdev);
>
> + i915_gem_cleanup_stolen(dev);
> intel_teardown_mchbar(dev);
> pm_qos_remove_request(&dev_priv->pm_qos);
> +out_free_hangcheckwq:
> destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
> out_freedpwq:
> destroy_workqueue(dev_priv->hotplug.dp_wq);
>
>
> > +
> > + i915.enable_rc6 = sanitize_rc6_option(dev,
> > i915.enable_rc6);
> > +
> > /* BIOS often leaves RC6 enabled, but disable it for hw
> > init */
> > intel_disable_gt_powersave(dev);
> > }
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
2016-01-15 14:49 ` Imre Deak
@ 2016-01-15 15:29 ` Chris Wilson
2016-01-29 17:52 ` [PATCH v6 " Sagar Arun Kamble
2 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2016-01-15 15:29 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx, shashidhar.hiremath
On Fri, Jan 15, 2016 at 04:44:46PM +0200, Imre Deak wrote:
> Adding Chris. We would need stolen_base and size early, could you check
> if moving i915_gem_init_stolen() earlier based on the diff at the end
> is ok?
Had a quick discussion with Imre on irc, and yes this is fine. Though we
could move gem_init_stolen even earlier, into i915_gem_gtt_init() or
just after. I favour gtt_init since the stolen arena is closely coupled
with the gtt probing in i915_gem_gtt_init().
Also intel_setup_mchbar() is semantically related to our
dev_priv->regs = pci_iomap(), and I would suggest a new
intel_setup_mmio() to do both, nice and early.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
2016-01-15 14:49 ` Imre Deak
2016-01-15 15:29 ` Chris Wilson
@ 2016-01-29 17:52 ` Sagar Arun Kamble
2016-02-02 16:18 ` Imre Deak
2 siblings, 1 reply; 11+ messages in thread
From: Sagar Arun Kamble @ 2016-01-29 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: shashidhar.hiremath
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
setup registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.
RC6 related instability can be avoided by disabling via BIOS settings
till driver fixes it.
v2: Had placed logic in gen8 function by mistake. Fixed it.
Ensuring RPM is not enabled in case BIOS disabled RC6.
v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
Runtime PM enabling happens before gen9_enable_rc6.
Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
v5: Caching reserved stolen base and size in the driver private data.
Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
Cc: Imre Deak <imre.deak@intel.com>
Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++
drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_uncore.c | 2 ++
6 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f520c90..66a6da2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -342,6 +342,8 @@ struct i915_gtt {
size_t stolen_size; /* Total size of stolen memory */
size_t stolen_usable_size; /* Total size minus BIOS reserved */
+ size_t stolen_reserved_base;
+ size_t stolen_reserved_size;
u64 mappable_end; /* End offset that we can CPU map */
struct io_mapping *mappable; /* Mapping to our CPU mappable region */
phys_addr_t mappable_base; /* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index c384dc9..ba1a00d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
return 0;
}
+ dev_priv->gtt.stolen_reserved_base = reserved_base;
+ dev_priv->gtt.stolen_reserved_size = reserved_size;
+
/* It is possible for the reserved area to end before the end of stolen
* memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 65e32a3..00aac28 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
#define VLV_PMWGICZ _MMIO(0x1300a4)
+#define RC6_LOCATION _MMIO(0xD40)
+#define RC6_CTX_IN_DRAM 1
+#define RC6_CTX_BASE _MMIO(0xD48)
+#define RC6_CTX_BASE_MASK 0xFFFFFFF0
+#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
+#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
+#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
+#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
+#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
+#define IDLE_TIME_MASK 0xFFFFF
#define FORCEWAKE _MMIO(0xA18C)
#define FORCEWAKE_VLV _MMIO(0x1300b0)
#define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
@@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
#define GEN6_RPDEUC _MMIO(0xA084)
#define GEN6_RPDEUCSW _MMIO(0xA088)
#define GEN6_RC_STATE _MMIO(0xA094)
+#define RC6_STATE (1<<18)
#define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
#define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
#define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f620023..aaa2051a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..8929f69 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4558,12 +4558,69 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
}
-static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
+static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ bool enable_rc6 = true;
+ unsigned long rc6_ctx_base;
+ bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
+
+ bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+ (GEN6_RC_CTL_RC6_ENABLE |
+ GEN6_RC_CTL_HW_ENABLE);
+ bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
+ GEN6_RC_CTL_HW_ENABLE) &&
+ (I915_READ(GEN6_RC_STATE) & RC6_STATE);
+
+ if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
+ DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
+ if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
+ (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
+ dev_priv->gtt.stolen_reserved_size)))) {
+ DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
+ enable_rc6 = false;
+ }
+
+ if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
+ DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ if (HAS_BSD2(dev) &&
+ ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
+ DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
+ DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
+ enable_rc6 = false;
+ }
+
+ return enable_rc6;
+}
+
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
{
/* No RC6 before Ironlake and code is gone for ilk. */
if (INTEL_INFO(dev)->gen < 6)
return 0;
+ if (!enable_rc6)
+ return 0;
+
+ if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
+ DRM_INFO("RC6 disabled by BIOS\n");
+ return 0;
+ }
+
/* Respect the kernel parameter if it is set */
if (enable_rc6 >= 0) {
int mask;
@@ -6053,7 +6110,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
/*
* RPM depends on RC6 to save restore the GT HW context, so make RC6 a
* requirement.
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bfa79e5..436d8f2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
void intel_uncore_sanitize(struct drm_device *dev)
{
+ i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
+
/* BIOS often leaves RC6 enabled, but disable it for hw init */
intel_disable_gt_powersave(dev);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v6,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev3)
2016-01-14 15:56 [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Sagar Arun Kamble
2016-01-14 16:30 ` ✗ failure: Fi.CI.BAT Patchwork
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
@ 2016-02-01 8:30 ` Patchwork
2016-02-03 8:43 ` ✓ Fi.CI.BAT: success for series starting with [v7,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev4) Patchwork
3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2016-02-01 8:30 UTC (permalink / raw)
To: sagar.a.kamble; +Cc: intel-gfx
== Summary ==
Series 2497v3 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/2497/revisions/3/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
incomplete -> PASS (hsw-gt2)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> PASS (bsw-nuc-2)
bdw-nuci7 total:156 pass:147 dwarn:0 dfail:0 fail:0 skip:9
bdw-ultra total:159 pass:147 dwarn:0 dfail:0 fail:0 skip:12
bsw-nuc-2 total:159 pass:129 dwarn:0 dfail:0 fail:0 skip:30
byt-nuc total:159 pass:136 dwarn:0 dfail:0 fail:0 skip:23
hsw-brixbox total:159 pass:146 dwarn:0 dfail:0 fail:0 skip:13
hsw-gt2 total:159 pass:149 dwarn:0 dfail:0 fail:0 skip:10
ilk-hp8440p total:159 pass:111 dwarn:0 dfail:0 fail:0 skip:48
ivb-t430s total:159 pass:145 dwarn:0 dfail:0 fail:0 skip:14
skl-i5k-2 total:159 pass:144 dwarn:1 dfail:0 fail:0 skip:14
snb-dellxps total:159 pass:137 dwarn:0 dfail:0 fail:0 skip:22
Results at /archive/results/CI_IGT_test/Patchwork_1329/
6b1049b84dcd979f631d15b2ada325d8e5b2c4e1 drm-intel-nightly: 2016y-01m-29d-22h-50m-57s UTC integration manifest
70ec2e0c3023206bd1dd46aa9c1d720cd14bc4bb drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-01-29 17:52 ` [PATCH v6 " Sagar Arun Kamble
@ 2016-02-02 16:18 ` Imre Deak
2016-02-02 18:06 ` Kamble, Sagar A
2016-02-02 18:13 ` [PATCH v7 " Sagar Arun Kamble
0 siblings, 2 replies; 11+ messages in thread
From: Imre Deak @ 2016-02-02 16:18 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx; +Cc: shashidhar.hiremath
On pe, 2016-01-29 at 23:22 +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> setup registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> RC6 related instability can be avoided by disabling via BIOS settings
> till driver fixes it.
>
> v2: Had placed logic in gen8 function by mistake. Fixed it.
> Ensuring RPM is not enabled in case BIOS disabled RC6.
>
> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
> Runtime PM enabling happens before gen9_enable_rc6.
> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
>
> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
>
> v5: Caching reserved stolen base and size in the driver private data.
> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
>
> v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
> drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++
> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_uncore.c | 2 ++
> 6 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f520c90..66a6da2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -342,6 +342,8 @@ struct i915_gtt {
>
> size_t stolen_size; /* Total size of stolen memory */
> size_t stolen_usable_size; /* Total size minus BIOS reserved */
> + size_t stolen_reserved_base;
> + size_t stolen_reserved_size;
> u64 mappable_end; /* End offset that we can CPU map */
> struct io_mapping *mappable; /* Mapping to our CPU mappable region */
> phys_addr_t mappable_base; /* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index c384dc9..ba1a00d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
> return 0;
> }
>
> + dev_priv->gtt.stolen_reserved_base = reserved_base;
> + dev_priv->gtt.stolen_reserved_size = reserved_size;
> +
> /* It is possible for the reserved area to end before the end of stolen
> * memory, so just consider the start. */
> reserved_total = stolen_top - reserved_base;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 65e32a3..00aac28 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
>
> #define VLV_PMWGICZ _MMIO(0x1300a4)
>
> +#define RC6_LOCATION _MMIO(0xD40)
> +#define RC6_CTX_IN_DRAM 1
> +#define RC6_CTX_BASE _MMIO(0xD48)
> +#define RC6_CTX_BASE_MASK 0xFFFFFFF0
> +#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
> +#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
> +#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
> +#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
> +#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
> +#define IDLE_TIME_MASK 0xFFFFF
> #define FORCEWAKE _MMIO(0xA18C)
> #define FORCEWAKE_VLV _MMIO(0x1300b0)
> #define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
> @@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
> #define GEN6_RPDEUC _MMIO(0xA084)
> #define GEN6_RPDEUCSW _MMIO(0xA088)
> #define GEN6_RC_STATE _MMIO(0xA094)
> +#define RC6_STATE (1<<18)
> #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
> #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
> #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f620023..aaa2051a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb /* out */);
> uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>
> /* intel_sdvo.c */
> bool intel_sdvo_init(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31bc4ea..8929f69 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4558,12 +4558,69 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
> onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
> }
>
> -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + bool enable_rc6 = true;
> + unsigned long rc6_ctx_base;
> + bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
> +
> + bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> + (GEN6_RC_CTL_RC6_ENABLE |
> + GEN6_RC_CTL_HW_ENABLE);
> + bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
> + GEN6_RC_CTL_HW_ENABLE) &&
> + (I915_READ(GEN6_RC_STATE) & RC6_STATE);
> +
> + if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
> + DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
> + enable_rc6 = false;
> + }
> +
> + rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
> + if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
> + (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
> + dev_priv->gtt.stolen_reserved_size)))) {
> + DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
> + enable_rc6 = false;
> + }
> +
> + if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
> + ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
> + DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
> + enable_rc6 = false;
> + }
> +
> + if (HAS_BSD2(dev) &&
> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
> + DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
> + enable_rc6 = false;
> + }
I missed this from the previous review: this check would make sense on
SKL, but since the function is BXT specific it shouldn't be here, so
please remove checking PWRCTX_MAXCNT_VCSUNIT1 for now. With that this
is:
Reviewed-by: Imre Deak <imre.deak@intel.com>
> +
> + if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
> + DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
> + enable_rc6 = false;
> + }
> +
> + return enable_rc6;
> +}
> +
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> {
> /* No RC6 before Ironlake and code is gone for ilk. */
> if (INTEL_INFO(dev)->gen < 6)
> return 0;
>
> + if (!enable_rc6)
> + return 0;
> +
> + if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
> + DRM_INFO("RC6 disabled by BIOS\n");
> + return 0;
> + }
> +
> /* Respect the kernel parameter if it is set */
> if (enable_rc6 >= 0) {
> int mask;
> @@ -6053,7 +6110,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
> /*
> * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
> * requirement.
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index bfa79e5..436d8f2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
>
> void intel_uncore_sanitize(struct drm_device *dev)
> {
> + i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
> +
> /* BIOS often leaves RC6 enabled, but disable it for hw init */
> intel_disable_gt_powersave(dev);
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-02-02 16:18 ` Imre Deak
@ 2016-02-02 18:06 ` Kamble, Sagar A
2016-02-02 18:13 ` [PATCH v7 " Sagar Arun Kamble
1 sibling, 0 replies; 11+ messages in thread
From: Kamble, Sagar A @ 2016-02-02 18:06 UTC (permalink / raw)
To: imre.deak, intel-gfx; +Cc: shashidhar.hiremath
Thanks for the review Imre.
I Kept that BSD2 check thinking some BXT revision might support it :)
Will remove the check and send the patch.
Thanks
Sagar
On 2/2/2016 9:48 PM, Imre Deak wrote:
> On pe, 2016-01-29 at 23:22 +0530, Sagar Arun Kamble wrote:
>> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
>> setup registers. If those are not setup Driver should not enable RC6.
>> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
>> to know if BIOS has enabled HW/SW RC6.
>> This will also enable user to control RC6 using BIOS settings alone.
>> RC6 related instability can be avoided by disabling via BIOS settings
>> till driver fixes it.
>>
>> v2: Had placed logic in gen8 function by mistake. Fixed it.
>> Ensuring RPM is not enabled in case BIOS disabled RC6.
>>
>> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
>> Runtime PM enabling happens before gen9_enable_rc6.
>> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
>>
>> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
>>
>> v5: Caching reserved stolen base and size in the driver private data.
>> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
>>
>> v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
>> drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++
>> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++++++++++++++--
>> drivers/gpu/drm/i915/intel_uncore.c | 2 ++
>> 6 files changed, 77 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> index f520c90..66a6da2 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> @@ -342,6 +342,8 @@ struct i915_gtt {
>>
>> size_t stolen_size; /* Total size of stolen memory */
>> size_t stolen_usable_size; /* Total size minus BIOS reserved */
>> + size_t stolen_reserved_base;
>> + size_t stolen_reserved_size;
>> u64 mappable_end; /* End offset that we can CPU map */
>> struct io_mapping *mappable; /* Mapping to our CPU mappable region */
>> phys_addr_t mappable_base; /* PA of our GMADR */
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index c384dc9..ba1a00d 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
>> return 0;
>> }
>>
>> + dev_priv->gtt.stolen_reserved_base = reserved_base;
>> + dev_priv->gtt.stolen_reserved_size = reserved_size;
>> +
>> /* It is possible for the reserved area to end before the end of stolen
>> * memory, so just consider the start. */
>> reserved_total = stolen_top - reserved_base;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 65e32a3..00aac28 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
>>
>> #define VLV_PMWGICZ _MMIO(0x1300a4)
>>
>> +#define RC6_LOCATION _MMIO(0xD40)
>> +#define RC6_CTX_IN_DRAM 1
>> +#define RC6_CTX_BASE _MMIO(0xD48)
>> +#define RC6_CTX_BASE_MASK 0xFFFFFFF0
>> +#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
>> +#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
>> +#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
>> +#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
>> +#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
>> +#define IDLE_TIME_MASK 0xFFFFF
>> #define FORCEWAKE _MMIO(0xA18C)
>> #define FORCEWAKE_VLV _MMIO(0x1300b0)
>> #define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
>> @@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
>> #define GEN6_RPDEUC _MMIO(0xA084)
>> #define GEN6_RPDEUCSW _MMIO(0xA088)
>> #define GEN6_RC_STATE _MMIO(0xA094)
>> +#define RC6_STATE (1<<18)
>> #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
>> #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
>> #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index f620023..aaa2051a 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
>> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>> struct skl_ddb_allocation *ddb /* out */);
>> uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
>> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>>
>> /* intel_sdvo.c */
>> bool intel_sdvo_init(struct drm_device *dev,
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 31bc4ea..8929f69 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4558,12 +4558,69 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
>> onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
>> }
>>
>> -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>> +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
>> +{
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + bool enable_rc6 = true;
>> + unsigned long rc6_ctx_base;
>> + bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
>> +
>> + bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
>> + (GEN6_RC_CTL_RC6_ENABLE |
>> + GEN6_RC_CTL_HW_ENABLE);
>> + bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
>> + GEN6_RC_CTL_HW_ENABLE) &&
>> + (I915_READ(GEN6_RC_STATE) & RC6_STATE);
>> +
>> + if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
>> + DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
>> + enable_rc6 = false;
>> + }
>> +
>> + rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
>> + if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
>> + (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
>> + dev_priv->gtt.stolen_reserved_size)))) {
>> + DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
>> + enable_rc6 = false;
>> + }
>> +
>> + if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
>> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
>> + ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
>> + ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
>> + DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
>> + enable_rc6 = false;
>> + }
>> +
>> + if (HAS_BSD2(dev) &&
>> + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
>> + DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
>> + enable_rc6 = false;
>> + }
> I missed this from the previous review: this check would make sense on
> SKL, but since the function is BXT specific it shouldn't be here, so
> please remove checking PWRCTX_MAXCNT_VCSUNIT1 for now. With that this
> is:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
>
>> +
>> + if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
>> + DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
>> + enable_rc6 = false;
>> + }
>> +
>> + return enable_rc6;
>> +}
>> +
>> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>> {
>> /* No RC6 before Ironlake and code is gone for ilk. */
>> if (INTEL_INFO(dev)->gen < 6)
>> return 0;
>>
>> + if (!enable_rc6)
>> + return 0;
>> +
>> + if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
>> + DRM_INFO("RC6 disabled by BIOS\n");
>> + return 0;
>> + }
>> +
>> /* Respect the kernel parameter if it is set */
>> if (enable_rc6 >= 0) {
>> int mask;
>> @@ -6053,7 +6110,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> - i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
>> /*
>> * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
>> * requirement.
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index bfa79e5..436d8f2 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
>>
>> void intel_uncore_sanitize(struct drm_device *dev)
>> {
>> + i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
>> +
>> /* BIOS often leaves RC6 enabled, but disable it for hw init */
>> intel_disable_gt_powersave(dev);
>> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
2016-02-02 16:18 ` Imre Deak
2016-02-02 18:06 ` Kamble, Sagar A
@ 2016-02-02 18:13 ` Sagar Arun Kamble
1 sibling, 0 replies; 11+ messages in thread
From: Sagar Arun Kamble @ 2016-02-02 18:13 UTC (permalink / raw)
To: intel-gfx
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
setup registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.
RC6 related instability can be avoided by disabling via BIOS settings
till driver fixes it.
v2: Had placed logic in gen8 function by mistake. Fixed it.
Ensuring RPM is not enabled in case BIOS disabled RC6.
v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
Runtime PM enabling happens before gen9_enable_rc6.
Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
v5: Caching reserved stolen base and size in the driver private data.
Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
v7: Removed PWRCTX_MAXCNT_VCSUNIT1 check as it applies to SKL. (Imre)
Cc: Imre Deak <imre.deak@intel.com>
Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++
drivers/gpu/drm/i915/i915_reg.h | 11 +++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 54 ++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_uncore.c | 2 ++
6 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f520c90..66a6da2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -342,6 +342,8 @@ struct i915_gtt {
size_t stolen_size; /* Total size of stolen memory */
size_t stolen_usable_size; /* Total size minus BIOS reserved */
+ size_t stolen_reserved_base;
+ size_t stolen_reserved_size;
u64 mappable_end; /* End offset that we can CPU map */
struct io_mapping *mappable; /* Mapping to our CPU mappable region */
phys_addr_t mappable_base; /* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index c384dc9..ba1a00d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
return 0;
}
+ dev_priv->gtt.stolen_reserved_base = reserved_base;
+ dev_priv->gtt.stolen_reserved_size = reserved_size;
+
/* It is possible for the reserved area to end before the end of stolen
* memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 65e32a3..00aac28 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
#define VLV_PMWGICZ _MMIO(0x1300a4)
+#define RC6_LOCATION _MMIO(0xD40)
+#define RC6_CTX_IN_DRAM 1
+#define RC6_CTX_BASE _MMIO(0xD48)
+#define RC6_CTX_BASE_MASK 0xFFFFFFF0
+#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
+#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054)
+#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
+#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054)
+#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054)
+#define IDLE_TIME_MASK 0xFFFFF
#define FORCEWAKE _MMIO(0xA18C)
#define FORCEWAKE_VLV _MMIO(0x1300b0)
#define FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
@@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
#define GEN6_RPDEUC _MMIO(0xA084)
#define GEN6_RPDEUCSW _MMIO(0xA088)
#define GEN6_RC_STATE _MMIO(0xA094)
+#define RC6_STATE (1<<18)
#define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
#define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
#define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f620023..aaa2051a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..3178913 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4558,12 +4558,63 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
}
-static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
+static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ bool enable_rc6 = true;
+ unsigned long rc6_ctx_base;
+ bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
+
+ bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+ (GEN6_RC_CTL_RC6_ENABLE |
+ GEN6_RC_CTL_HW_ENABLE);
+ bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
+ GEN6_RC_CTL_HW_ENABLE) &&
+ (I915_READ(GEN6_RC_STATE) & RC6_STATE);
+
+ if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
+ DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
+ if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
+ (rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
+ dev_priv->gtt.stolen_reserved_size)))) {
+ DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
+ enable_rc6 = false;
+ }
+
+ if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
+ ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
+ DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
+ enable_rc6 = false;
+ }
+
+ if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
+ DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
+ enable_rc6 = false;
+ }
+
+ return enable_rc6;
+}
+
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
{
/* No RC6 before Ironlake and code is gone for ilk. */
if (INTEL_INFO(dev)->gen < 6)
return 0;
+ if (!enable_rc6)
+ return 0;
+
+ if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
+ DRM_INFO("RC6 disabled by BIOS\n");
+ return 0;
+ }
+
/* Respect the kernel parameter if it is set */
if (enable_rc6 >= 0) {
int mask;
@@ -6053,7 +6104,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
/*
* RPM depends on RC6 to save restore the GT HW context, so make RC6 a
* requirement.
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bfa79e5..436d8f2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
void intel_uncore_sanitize(struct drm_device *dev)
{
+ i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
+
/* BIOS often leaves RC6 enabled, but disable it for hw init */
intel_disable_gt_powersave(dev);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v7,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev4)
2016-01-14 15:56 [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Sagar Arun Kamble
` (2 preceding siblings ...)
2016-02-01 8:30 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev3) Patchwork
@ 2016-02-03 8:43 ` Patchwork
3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2016-02-03 8:43 UTC (permalink / raw)
To: sagar.a.kamble; +Cc: intel-gfx
== Summary ==
Series 2497v4 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/2497/revisions/4/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
incomplete -> PASS (hsw-gt2)
bdw-nuci7 total:156 pass:147 dwarn:0 dfail:0 fail:0 skip:9
bdw-ultra total:159 pass:147 dwarn:0 dfail:0 fail:0 skip:12
byt-nuc total:159 pass:136 dwarn:0 dfail:0 fail:0 skip:23
hsw-brixbox total:159 pass:146 dwarn:0 dfail:0 fail:0 skip:13
hsw-gt2 total:159 pass:149 dwarn:0 dfail:0 fail:0 skip:10
ilk-hp8440p total:159 pass:111 dwarn:0 dfail:0 fail:0 skip:48
ivb-t430s total:159 pass:145 dwarn:0 dfail:0 fail:0 skip:14
skl-i5k-2 total:159 pass:144 dwarn:1 dfail:0 fail:0 skip:14
snb-dellxps total:159 pass:137 dwarn:0 dfail:0 fail:0 skip:22
snb-x220t total:159 pass:137 dwarn:0 dfail:0 fail:1 skip:21
Results at /archive/results/CI_IGT_test/Patchwork_1346/
02932377a975a59ccd83095816d5b23183107d79 drm-intel-nightly: 2016y-02m-03d-01h-54m-27s UTC integration manifest
7d9a3ad5ecc8e68c7901ca9d027e7504cb84516a drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-02-03 8:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-14 15:56 [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Sagar Arun Kamble
2016-01-14 16:30 ` ✗ failure: Fi.CI.BAT Patchwork
2016-01-15 14:44 ` [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 Imre Deak
2016-01-15 14:49 ` Imre Deak
2016-01-15 15:29 ` Chris Wilson
2016-01-29 17:52 ` [PATCH v6 " Sagar Arun Kamble
2016-02-02 16:18 ` Imre Deak
2016-02-02 18:06 ` Kamble, Sagar A
2016-02-02 18:13 ` [PATCH v7 " Sagar Arun Kamble
2016-02-01 8:30 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev3) Patchwork
2016-02-03 8:43 ` ✓ Fi.CI.BAT: success for series starting with [v7,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 (rev4) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.