* [PATCH] drm/i915: Report an error when i915.reset prevents a reset @ 2015-06-18 10:42 Chris Wilson 2015-06-22 13:44 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2015-06-18 10:42 UTC (permalink / raw) To: intel-gfx If the user disables the GPU reset using the i915.reset parameter and one occurs, report that we failed to reset the GPU. If we return early, as we currently do, then we leave all state intact (with a hung GPU) and clients block forever waiting for their requests to complete. Testcase: igt/gem_eio Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_dma.c | 1 - drivers/gpu/drm/i915/i915_drv.c | 3 --- drivers/gpu/drm/i915/intel_uncore.c | 3 +++ 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 88795d2f1819..c5349fa3fcce 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -165,7 +165,6 @@ static int i915_getparam(struct drm_device *dev, void *data, break; case I915_PARAM_HAS_GPU_RESET: value = i915.enable_hangcheck && - i915.reset && intel_has_gpu_reset(dev); break; default: diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 78ef0bb53c36..25ffe8afe744 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -863,9 +863,6 @@ int i915_reset(struct drm_device *dev) bool simulated; int ret; - if (!i915.reset) - return 0; - intel_reset_gt_powersave(dev); mutex_lock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 4a86cf007aa0..f8e75def1a1d 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1457,6 +1457,9 @@ static int gen6_do_reset(struct drm_device *dev) static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) { + if (!i915.reset) + return NULL; + if (INTEL_INFO(dev)->gen >= 6) return gen6_do_reset; else if (IS_GEN5(dev)) -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Report an error when i915.reset prevents a reset 2015-06-18 10:42 [PATCH] drm/i915: Report an error when i915.reset prevents a reset Chris Wilson @ 2015-06-22 13:44 ` Daniel Vetter 2015-06-22 13:53 ` Chris Wilson 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2015-06-22 13:44 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Thu, Jun 18, 2015 at 11:42:08AM +0100, Chris Wilson wrote: > If the user disables the GPU reset using the i915.reset parameter and > one occurs, report that we failed to reset the GPU. If we return early, > as we currently do, then we leave all state intact (with a hung GPU) > and clients block forever waiting for their requests to complete. > > Testcase: igt/gem_eio > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_dma.c | 1 - > drivers/gpu/drm/i915/i915_drv.c | 3 --- > drivers/gpu/drm/i915/intel_uncore.c | 3 +++ > 3 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 88795d2f1819..c5349fa3fcce 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -165,7 +165,6 @@ static int i915_getparam(struct drm_device *dev, void *data, > break; > case I915_PARAM_HAS_GPU_RESET: > value = i915.enable_hangcheck && > - i915.reset && > intel_has_gpu_reset(dev); > break; > default: > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 78ef0bb53c36..25ffe8afe744 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -863,9 +863,6 @@ int i915_reset(struct drm_device *dev) > bool simulated; > int ret; > > - if (!i915.reset) > - return 0; > - > intel_reset_gt_powersave(dev); > > mutex_lock(&dev->struct_mutex); > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 4a86cf007aa0..f8e75def1a1d 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1457,6 +1457,9 @@ static int gen6_do_reset(struct drm_device *dev) > > static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) > { > + if (!i915.reset) > + return NULL; Maybe a special reset function which always returns -EIO and prints something to illuminate the situation to dmesg? Otherwise even more wtf material in bugzilla ... -Daniel > + > if (INTEL_INFO(dev)->gen >= 6) > return gen6_do_reset; > else if (IS_GEN5(dev)) > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Report an error when i915.reset prevents a reset 2015-06-22 13:44 ` Daniel Vetter @ 2015-06-22 13:53 ` Chris Wilson 2015-06-22 14:48 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2015-06-22 13:53 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, Jun 22, 2015 at 03:44:51PM +0200, Daniel Vetter wrote: > On Thu, Jun 18, 2015 at 11:42:08AM +0100, Chris Wilson wrote: > > If the user disables the GPU reset using the i915.reset parameter and > > one occurs, report that we failed to reset the GPU. If we return early, > > as we currently do, then we leave all state intact (with a hung GPU) > > and clients block forever waiting for their requests to complete. > > > > Testcase: igt/gem_eio > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 1 - > > drivers/gpu/drm/i915/i915_drv.c | 3 --- > > drivers/gpu/drm/i915/intel_uncore.c | 3 +++ > > 3 files changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index 88795d2f1819..c5349fa3fcce 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -165,7 +165,6 @@ static int i915_getparam(struct drm_device *dev, void *data, > > break; > > case I915_PARAM_HAS_GPU_RESET: > > value = i915.enable_hangcheck && > > - i915.reset && > > intel_has_gpu_reset(dev); > > break; > > default: > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 78ef0bb53c36..25ffe8afe744 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -863,9 +863,6 @@ int i915_reset(struct drm_device *dev) > > bool simulated; > > int ret; > > > > - if (!i915.reset) > > - return 0; > > - > > intel_reset_gt_powersave(dev); > > > > mutex_lock(&dev->struct_mutex); > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index 4a86cf007aa0..f8e75def1a1d 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -1457,6 +1457,9 @@ static int gen6_do_reset(struct drm_device *dev) > > > > static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) > > { > > + if (!i915.reset) > > + return NULL; > > Maybe a special reset function which always returns -EIO and prints > something to illuminate the situation to dmesg? Otherwise even more wtf > material in bugzilla ... It does report a failure (different to reset() itself failing), and reset should be yet another do not touch module option. -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] 5+ messages in thread
* Re: [PATCH] drm/i915: Report an error when i915.reset prevents a reset 2015-06-22 13:53 ` Chris Wilson @ 2015-06-22 14:48 ` Daniel Vetter 2015-06-22 14:53 ` Chris Wilson 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2015-06-22 14:48 UTC (permalink / raw) To: Chris Wilson, Daniel Vetter, intel-gfx On Mon, Jun 22, 2015 at 02:53:39PM +0100, Chris Wilson wrote: > On Mon, Jun 22, 2015 at 03:44:51PM +0200, Daniel Vetter wrote: > > On Thu, Jun 18, 2015 at 11:42:08AM +0100, Chris Wilson wrote: > > > If the user disables the GPU reset using the i915.reset parameter and > > > one occurs, report that we failed to reset the GPU. If we return early, > > > as we currently do, then we leave all state intact (with a hung GPU) > > > and clients block forever waiting for their requests to complete. > > > > > > Testcase: igt/gem_eio > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 1 - > > > drivers/gpu/drm/i915/i915_drv.c | 3 --- > > > drivers/gpu/drm/i915/intel_uncore.c | 3 +++ > > > 3 files changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > index 88795d2f1819..c5349fa3fcce 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -165,7 +165,6 @@ static int i915_getparam(struct drm_device *dev, void *data, > > > break; > > > case I915_PARAM_HAS_GPU_RESET: > > > value = i915.enable_hangcheck && > > > - i915.reset && > > > intel_has_gpu_reset(dev); > > > break; > > > default: > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index 78ef0bb53c36..25ffe8afe744 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -863,9 +863,6 @@ int i915_reset(struct drm_device *dev) > > > bool simulated; > > > int ret; > > > > > > - if (!i915.reset) > > > - return 0; > > > - > > > intel_reset_gt_powersave(dev); > > > > > > mutex_lock(&dev->struct_mutex); > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > > index 4a86cf007aa0..f8e75def1a1d 100644 > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > @@ -1457,6 +1457,9 @@ static int gen6_do_reset(struct drm_device *dev) > > > > > > static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) > > > { > > > + if (!i915.reset) > > > + return NULL; > > > > Maybe a special reset function which always returns -EIO and prints > > something to illuminate the situation to dmesg? Otherwise even more wtf > > material in bugzilla ... > > It does report a failure (different to reset() itself failing), and reset > should be yet another do not touch module option. The only one I could spot is conditional upon the hang being a simulated one. But just marking i915.reset as unsafe works for me too, I'll amend your patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Report an error when i915.reset prevents a reset 2015-06-22 14:48 ` Daniel Vetter @ 2015-06-22 14:53 ` Chris Wilson 0 siblings, 0 replies; 5+ messages in thread From: Chris Wilson @ 2015-06-22 14:53 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, Jun 22, 2015 at 04:48:26PM +0200, Daniel Vetter wrote: > On Mon, Jun 22, 2015 at 02:53:39PM +0100, Chris Wilson wrote: > > On Mon, Jun 22, 2015 at 03:44:51PM +0200, Daniel Vetter wrote: > > > On Thu, Jun 18, 2015 at 11:42:08AM +0100, Chris Wilson wrote: > > > > If the user disables the GPU reset using the i915.reset parameter and > > > > one occurs, report that we failed to reset the GPU. If we return early, > > > > as we currently do, then we leave all state intact (with a hung GPU) > > > > and clients block forever waiting for their requests to complete. > > > > > > > > Testcase: igt/gem_eio > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > --- > > > > drivers/gpu/drm/i915/i915_dma.c | 1 - > > > > drivers/gpu/drm/i915/i915_drv.c | 3 --- > > > > drivers/gpu/drm/i915/intel_uncore.c | 3 +++ > > > > 3 files changed, 3 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > > index 88795d2f1819..c5349fa3fcce 100644 > > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > > @@ -165,7 +165,6 @@ static int i915_getparam(struct drm_device *dev, void *data, > > > > break; > > > > case I915_PARAM_HAS_GPU_RESET: > > > > value = i915.enable_hangcheck && > > > > - i915.reset && > > > > intel_has_gpu_reset(dev); > > > > break; > > > > default: > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > > index 78ef0bb53c36..25ffe8afe744 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -863,9 +863,6 @@ int i915_reset(struct drm_device *dev) > > > > bool simulated; > > > > int ret; > > > > > > > > - if (!i915.reset) > > > > - return 0; > > > > - > > > > intel_reset_gt_powersave(dev); > > > > > > > > mutex_lock(&dev->struct_mutex); > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > > > index 4a86cf007aa0..f8e75def1a1d 100644 > > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > > @@ -1457,6 +1457,9 @@ static int gen6_do_reset(struct drm_device *dev) > > > > > > > > static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) > > > > { > > > > + if (!i915.reset) > > > > + return NULL; > > > > > > Maybe a special reset function which always returns -EIO and prints > > > something to illuminate the situation to dmesg? Otherwise even more wtf > > > material in bugzilla ... > > > > It does report a failure (different to reset() itself failing), and reset > > should be yet another do not touch module option. > > The only one I could spot is conditional upon the hang being a simulated > one. But just marking i915.reset as unsafe works for me too, I'll amend > your patch. if (ret) { DRM_ERROR("Failed to reset chip: %i\n", ret); goto error; } ENODEV for anything > gen2 is a giveaway. -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] 5+ messages in thread
end of thread, other threads:[~2015-06-22 14:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-18 10:42 [PATCH] drm/i915: Report an error when i915.reset prevents a reset Chris Wilson 2015-06-22 13:44 ` Daniel Vetter 2015-06-22 13:53 ` Chris Wilson 2015-06-22 14:48 ` Daniel Vetter 2015-06-22 14:53 ` Chris Wilson
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.