On Wed, Dec 16, 2015 at 10:52:06AM +0100, Daniel Vetter wrote:
> On Fri, Dec 11, 2015 at 11:33:05AM +0000, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index d7bbd015de35..875bdf814d73 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13405,10 +13405,6 @@ static int intel_atomic_prepare_commit(struct
> > drm_device *dev,
> >
> > ret = __i915_wait_request(intel_plane_state->wait_req,
> > true, NULL, NULL);
> > -
> > - /* Swallow -EIO errors to allow updates during hw
> > lockup. */
> > - if (ret == -EIO)
>
> I'd like to keep a WARN_ON here since it's ABI relevant, and consistent
> with the other places we've had a WARN_ON(-EIO).
>
> > - ret = 0;
> > if (ret) {
> > mutex_lock(&dev->struct_mutex);
> > drm_atomic_helper_cleanup_planes(dev, state);
> > @@ -13744,9 +13740,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> > */
> > if (needs_modeset(crtc_state))
> > ret = i915_gem_object_wait_rendering(old_obj, true);
> > -
> > - /* Swallow -EIO errors to allow updates during hw lockup. */
> > - if (ret && ret != -EIO)
> > + if (ret)
>
> Same here really.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 875bdf814d73..1586d9107cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13406,6 +13406,8 @@ static int intel_atomic_prepare_commit(struct
drm_device *dev,
ret = __i915_wait_request(intel_plane_state->wait_req,
true, NULL, NULL);
if (ret) {
+ /* Any hang should be swallowed by the wait */
+ WARN_ON(ret == -EIO);
mutex_lock(&dev->struct_mutex);
drm_atomic_helper_cleanup_planes(dev, state);
mutex_unlock(&dev->struct_mutex);
@@ -13740,8 +13742,11 @@ intel_prepare_plane_fb(struct drm_plane *plane,
*/
if (needs_modeset(crtc_state))
ret = i915_gem_object_wait_rendering(old_obj, true);
- if (ret)
+ if (ret) {
+ /* GPU hangs should have been swallowed by the wait */
+ WARN_ON(ret == -EIO);
return ret;
+ }
}
/* For framebuffer backed by dmabuf, wait for fence */
> We already have a WARN_ON in the mmio_flip, so are covered there already.
>
> Otherwise looks good, and with the above addressed and under the
> assumption that the last caller of check_wedge is in request_alloc (which
> I was too lazy to check perfectly):
Yes, later it is moved to the new i915_gem_request.c and made static
alongside it's only caller request_alloc().
-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
