[PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[Mika Kuoppala]

We added this WARN_ON to guard against using uninitialized
forcewake domains. But forgot blissfully that not all
gens have forcewake domains in the first place.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911
Tested-by: Ding Heng <hengx.ding@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 76b60a3..bbee962 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -166,6 +166,9 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_do
 	struct intel_uncore_forcewake_domain *d;
 	enum forcewake_domain_id id;
 
+	if (INTEL_INFO(dev_priv->dev)->gen <= 5)
+		return;
+
 	WARN_ON(dev_priv->uncore.fw_domains == 0);
 
 	for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[Chris Wilson]

On Thu, Feb 05, 2015 at 12:21:11PM +0200, Mika Kuoppala wrote:
> We added this WARN_ON to guard against using uninitialized
> forcewake domains. But forgot blissfully that not all
> gens have forcewake domains in the first place.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911
> Tested-by: Ding Heng <hengx.ding@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Imo, here we should just

if (dev_priv->uncore.fw_domains == 0)
  return;

then it clarifies that we are allowed to call this code everywhere. To
address the issue you raise we should move the WARN_ON into the
uncore_init. If we have more counter-examples in the future, we may
regret adding the WARN, but in the meantime it will help remind us to
add the forcewake handling code.
-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

[PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[Mika Kuoppala]

We added this WARN_ON to guard against using uninitialized
forcewake domains. But forgot blissfully that not all
gens have forcewake domains in the first place.

v2: Move WARN_ON to fw_domains_init (Chris)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911
Tested-by: Ding Heng <hengx.ding@intel.com> (v1)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 76b60a3..e10bcfc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -166,7 +166,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_do
 	struct intel_uncore_forcewake_domain *d;
 	enum forcewake_domain_id id;
 
-	WARN_ON(dev_priv->uncore.fw_domains == 0);
+	if (dev_priv->uncore.fw_domains == 0)
+		return;
 
 	for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
 		fw_domain_reset(d);
@@ -997,6 +998,9 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (INTEL_INFO(dev_priv->dev)->gen <= 5)
+		return;
+
 	if (IS_GEN9(dev)) {
 		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
 		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
@@ -1069,6 +1073,8 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
 		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE, FORCEWAKE_ACK);
 	}
+
+	WARN_ON(dev_priv->uncore.fw_domains == 0);
 }
 
 void intel_uncore_init(struct drm_device *dev)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[Chris Wilson]

On Thu, Feb 05, 2015 at 05:45:42PM +0200, Mika Kuoppala wrote:
> We added this WARN_ON to guard against using uninitialized
> forcewake domains. But forgot blissfully that not all
> gens have forcewake domains in the first place.
> 
> v2: Move WARN_ON to fw_domains_init (Chris)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911
> Tested-by: Ding Heng <hengx.ding@intel.com> (v1)
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 76b60a3..e10bcfc 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -166,7 +166,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_do
>  	struct intel_uncore_forcewake_domain *d;
>  	enum forcewake_domain_id id;
>  
> -	WARN_ON(dev_priv->uncore.fw_domains == 0);
> +	if (dev_priv->uncore.fw_domains == 0)
> +		return;
>  
>  	for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
>  		fw_domain_reset(d);
> @@ -997,6 +998,9 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	if (INTEL_INFO(dev_priv->dev)->gen <= 5)
> +		return;
> +
>  	if (IS_GEN9(dev)) {
>  		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
>  		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> @@ -1069,6 +1073,8 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>  		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
>  			       FORCEWAKE, FORCEWAKE_ACK);
>  	}
> +
Maybe: /* All future platforms are expected to require complex power gating */
> +	WARN_ON(dev_priv->uncore.fw_domains == 0);
>  }

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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

Re: [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5716
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV              +1                 282/283              283/283
ILK              +3                 316/319              319/319
SNB                                  322/346              322/346
IVB                 -1              382/384              381/384
BYT                                  296/296              296/296
HSW                                  425/428              425/428
BDW                                  318/333              318/333
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gen3_render_linear_blits      FAIL(2, M7)CRASH(1, M23)PASS(4, M25M23)      PASS(1, M25)
*ILK  igt_drv_suspend_debugfs-reader      DMESG_WARN(2, M26M37)      PASS(1, M37)
*ILK  igt_drv_suspend_forcewake      DMESG_WARN(4, M26M37)      PASS(1, M37)
*ILK  igt_gem_workarounds_suspend-resume      DMESG_WARN(2, M26M37)      PASS(1, M37)
 IVB  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      DMESG_WARN(2, M34)PASS(5, M21M34)      DMESG_WARN(1, M21)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5722
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  282/283              282/283
ILK              +3                 316/319              319/319
SNB              +2-3              322/346              321/346
IVB                 -2              382/384              380/384
BYT                                  296/296              296/296
HSW                 -1              351/354              350/354
BDW                                  318/333              318/333
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt_drv_suspend_debugfs-reader      DMESG_WARN(3, M26M37)      PASS(1, M37)
*ILK  igt_drv_suspend_forcewake      DMESG_WARN(6, M26M37)      PASS(1, M37)
*ILK  igt_gem_workarounds_suspend-resume      DMESG_WARN(3, M26M37)      PASS(1, M37)
 SNB  igt_kms_cursor_crc_cursor-size-change      NSPT(2, M22M35)PASS(2, M35)DMESG_FAIL(1, M35)      PASS(1, M35)
 SNB  igt_kms_flip_dpms-vs-vblank-race-interruptible      DMESG_WARN(2, M35)PASS(3, M22M35)      DMESG_WARN(1, M35)
 SNB  igt_kms_flip_event_leak      NSPT(2, M22M35)PASS(3, M35)      PASS(1, M35)
 SNB  igt_kms_flip_modeset-vs-vblank-race      DMESG_WARN(1, M35)PASS(2, M22M35)      DMESG_WARN(1, M35)
*SNB  igt_kms_flip_tiling_flip-changes-tiling      PASS(2, M22M35)      FAIL(1, M35)
 IVB  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      DMESG_WARN(2, M34)PASS(6, M21M34)      DMESG_WARN(1, M34)
*IVB  igt_gem_storedw_batches_loop_normal      PASS(2, M21M34)      DMESG_WARN(1, M34)
*HSW  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      PASS(3, M20)      DMESG_WARN(1, M40)
*HSW  igt_pm_rpm_debugfs-read      DMESG_WARN(3, M20)      INIT(1, M40)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON

[Jani Nikula]

On Thu, 05 Feb 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Feb 05, 2015 at 05:45:42PM +0200, Mika Kuoppala wrote:
>> We added this WARN_ON to guard against using uninitialized
>> forcewake domains. But forgot blissfully that not all
>> gens have forcewake domains in the first place.
>> 
>> v2: Move WARN_ON to fw_domains_init (Chris)
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911
>> Tested-by: Ding Heng <hengx.ding@intel.com> (v1)
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_uncore.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 76b60a3..e10bcfc 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -166,7 +166,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_do
>>  	struct intel_uncore_forcewake_domain *d;
>>  	enum forcewake_domain_id id;
>>  
>> -	WARN_ON(dev_priv->uncore.fw_domains == 0);
>> +	if (dev_priv->uncore.fw_domains == 0)
>> +		return;
>>  
>>  	for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
>>  		fw_domain_reset(d);
>> @@ -997,6 +998,9 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  
>> +	if (INTEL_INFO(dev_priv->dev)->gen <= 5)
>> +		return;
>> +
>>  	if (IS_GEN9(dev)) {
>>  		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
>>  		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
>> @@ -1069,6 +1073,8 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
>>  		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
>>  			       FORCEWAKE, FORCEWAKE_ACK);
>>  	}
>> +
> Maybe: /* All future platforms are expected to require complex power gating */
>> +	WARN_ON(dev_priv->uncore.fw_domains == 0);
>>  }
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.


> -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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx