[PATCH] drm/i915: Don't ban default context when stop_rings!=0

[PATCH] drm/i915: Don't ban default context when stop_rings!=0

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we've explicitly stopped the rings for testing purposes, don't ban
the default context. Fixes kms_flip hang tests.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
I'm not sure this is what we want in general, but it's what kms_flip expects
currently.

 drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3618bb0..52f9ea7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2259,14 +2259,13 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
 		return true;
 
 	if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
-		if (dev_priv->gpu_error.stop_rings == 0 &&
-		    i915_gem_context_is_default(ctx)) {
-			DRM_ERROR("gpu hanging too fast, banning!\n");
-		} else {
+		if (!i915_gem_context_is_default(ctx)) {
 			DRM_DEBUG("context hanging too fast, banning!\n");
+			return true;
+		} else if (dev_priv->gpu_error.stop_rings == 0) {
+			DRM_ERROR("gpu hanging too fast, banning!\n");
+			return true;
 		}
-
-		return true;
 	}
 
 	return false;
-- 
1.8.3.2

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

Re: [PATCH] drm/i915: Don't ban default context when stop_rings!=0

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we've explicitly stopped the rings for testing purposes, don't ban
> the default context. Fixes kms_flip hang tests.
>

To keep logs clean from 'unnecessary' hang messages we set stop_rings also on
all the gem_reset_stats hanging tests. I think this breaks all the ban
tests on gem_reset_stats.

So we have dig this nice hole, for avoiding dmesg output on cases
where dmesg output should be valid and justificed.

-Mika

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> I'm not sure this is what we want in general, but it's what kms_flip expects
> currently.
>
>  drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3618bb0..52f9ea7 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2259,14 +2259,13 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
>  		return true;
>  
>  	if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
> -		if (dev_priv->gpu_error.stop_rings == 0 &&
> -		    i915_gem_context_is_default(ctx)) {
> -			DRM_ERROR("gpu hanging too fast, banning!\n");
> -		} else {
> +		if (!i915_gem_context_is_default(ctx)) {
>  			DRM_DEBUG("context hanging too fast, banning!\n");
> +			return true;
> +		} else if (dev_priv->gpu_error.stop_rings == 0) {
> +			DRM_ERROR("gpu hanging too fast, banning!\n");
> +			return true;
>  		}
> -
> -		return true;
>  	}
>  
>  	return false;
> -- 
> 1.8.3.2
>
> _______________________________________________
> 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

Re: [PATCH] drm/i915: Don't ban default context when stop_rings!=0

[Mika Kuoppala]

Mika Kuoppala <mika.kuoppala@linux.intel.com> writes:

> ville.syrjala@linux.intel.com writes:
>
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> If we've explicitly stopped the rings for testing purposes, don't ban
>> the default context. Fixes kms_flip hang tests.
>>
>
> To keep logs clean from 'unnecessary' hang messages we set stop_rings also on
> all the gem_reset_stats hanging tests. I think this breaks all the ban
> tests on gem_reset_stats.
>
> So we have dig this nice hole, for avoiding dmesg output on cases
> where dmesg output should be valid and justificed.

I think the correct solution is to have white lists for dmesg output
in piglit test runner, so that we can remove this overloading of
stop_rings.

Injecting proper hang should output a proper log message and this
should be testable.

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

Re: [PATCH] drm/i915: Don't ban default context when stop_rings!=0

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we've explicitly stopped the rings for testing purposes, don't ban
> the default context. Fixes kms_flip hang tests.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> I'm not sure this is what we want in general, but it's what kms_flip expects
> currently.

Acked-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3618bb0..52f9ea7 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2259,14 +2259,13 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
>  		return true;
>  
>  	if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
> -		if (dev_priv->gpu_error.stop_rings == 0 &&
> -		    i915_gem_context_is_default(ctx)) {
> -			DRM_ERROR("gpu hanging too fast, banning!\n");
> -		} else {
> +		if (!i915_gem_context_is_default(ctx)) {
>  			DRM_DEBUG("context hanging too fast, banning!\n");
> +			return true;
> +		} else if (dev_priv->gpu_error.stop_rings == 0) {
> +			DRM_ERROR("gpu hanging too fast, banning!\n");
> +			return true;
>  		}
> -
> -		return true;
>  	}
>  
>  	return false;
> -- 
> 1.8.3.2
>
> _______________________________________________
> 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

Re: [PATCH] drm/i915: Don't ban default context when stop_rings!=0

[Daniel Vetter]

On Fri, Feb 21, 2014 at 06:13:51PM +0200, Mika Kuoppala wrote:
> Mika Kuoppala <mika.kuoppala@linux.intel.com> writes:
> 
> > ville.syrjala@linux.intel.com writes:
> >
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> If we've explicitly stopped the rings for testing purposes, don't ban
> >> the default context. Fixes kms_flip hang tests.
> >>
> >
> > To keep logs clean from 'unnecessary' hang messages we set stop_rings also on
> > all the gem_reset_stats hanging tests. I think this breaks all the ban
> > tests on gem_reset_stats.
> >
> > So we have dig this nice hole, for avoiding dmesg output on cases
> > where dmesg output should be valid and justificed.
> 
> I think the correct solution is to have white lists for dmesg output
> in piglit test runner, so that we can remove this overloading of
> stop_rings.
> 
> Injecting proper hang should output a proper log message and this
> should be testable.

The important part is that tests dont hit a DRM_ERROR, since that will
both upset piglit and also QA's testing infrastructure. Ville's patch
seems to dtrt by using DRM_DEBUG for non-default context banning.

Also with hw contexts and the reset stats stuff we expect some userspace
apps to hang the gpu often (malicious webgl shaders with inifite loops),
so debug level is the right thing anyway - usespace should not be allowed
to spam dmesg.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch