On Tue, 26 Jun 2012 23:08:50 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Having this early check in intel_ring_begin doesn't buy us anything,
> since we'll be calling into wait_request in the usual case already
> anyway. In the corner case of not waiting for free space using the
> last_retired_head we simply need to do the same check, too.
>
> With these changes we'll only ever get an -EIO from intel_ring_begin
> if the gpu has truely been declared dead.
>
> v2: Don't conflate the change to prevent intel_ring_begin from returning
> a spurious -EIO with the refactor to use i915_gem_check_wedge, which is
> just prep work to avoid returning -EAGAIN in callsites that can't handle
> syscall restarting.
I'm really scared by this change. It's diffuclt to review because there
are SO many callers of intel_ring_begin, and figuring out if they all
work in the wedged case is simply too difficult. I've yet to review the
rest of the series, but it may make more sense to put this change last
perhaps?
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index f30a53a..501546e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1230,13 +1230,9 @@ int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n)
> int intel_ring_begin(struct intel_ring_buffer *ring,
> int num_dwords)
> {
> - struct drm_i915_private *dev_priv = ring->dev->dev_private;
> int n = 4*num_dwords;
> int ret;
>
> - if (unlikely(atomic_read(&dev_priv->mm.wedged)))
> - return -EIO;
> -
> if (unlikely(ring->tail + n > ring->effective_size)) {
> ret = intel_wrap_ring_buffer(ring);
> if (unlikely(ret))
--
Ben Widawsky, Intel Open Source Technology Center
