Re: [PATCH v2 2/4] drm/i915: Ironlake mobile GPU with VT-d fix

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Sep 25, 2011 at 04:42:57PM -0700, Ben Widawsky wrote:
> > >  static int sandybridge_write_fence_reg(struct drm_i915_gem_object
> > > *obj, diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > b/drivers/gpu/drm/i915/i915_gem_gtt.c index 7a709cd..0c6226b 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -49,10 +49,39 @@ static unsigned int
> > > cache_level_to_agp_type(struct drm_device *dev, }
> > >  }
> > >  
> > > +static bool do_idling(struct drm_i915_private *dev_priv)
> > > +{
> > > +	bool ret = dev_priv->mm.interruptible;
> > > +
> > > +	if (dev_priv->mm.gtt->do_idle_maps) {
> > > +		dev_priv->mm.interruptible = false;
> > > +		if (i915_gpu_idle(dev_priv->dev))
> > > +			DRM_ERROR("couldn't idle GPU %d\n", ret);
> > > +	}
> > 
> > I don't like the uninterruptible sleep in core gem - this is only for
> > modesetting where backtracking is just not really feasible. I've
> > quickly checked the code and I haven't seen any problems in handing
> > back the -ERESTARTSYS to gem_object_unbind.
> > 
> 
> While I agree that we could probably return at idle time, we have to
> guarantee that no GPU activity occurs until the unmap has completed. I
> checked the code and don't really understand how it can occur, but I
> believe it *can* occur as I've seen it happen in the idle function. Any
> other proposal how to fix this, or any proof that the GPU can't wake up
> to do display stuff while doing the unmap?

Hm, looks like some misunderstanding. I understand why the gpu_idle needs
to be here, my gripes are solely with the uninterruptible sleep that you
use. I think a normal interruptible sleep with passing back the return
value to gem_unbind_object should work: By the time we're calling
gtt_unbind_object in gem_unbind_object we have not yet committed to the
unbound state, so can just bail out if the mappings are still there. And
gtt_unbind_object idles the gpu before touching the mappings, so when we
have to bail out with -ERESTARTSYS the mapping is untouched and everything
safe.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48