On Wed, Dec 16, 2015 at 11:06:20AM +0000, Chris Wilson wrote:
> 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 */
lgtm.
> > 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().
Yeah, r-b confirmed ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
