[Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."

[Daniel Vetter]

On Mon, Nov 02, 2015 at 05:38:07PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote:
> > Hey,
> > 
> > Op 30-10-15 om 22:06 schreef ville.syrjala at linux.intel.com:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9.
> > >
> > > commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully interruptible.")
> > > breaks GPU reset on gen3/4 machines. Go back to to non-interruptible.
> > >
> > I've done some digging and by forcing an unconditional modeset during reset I was able to trigger it on my system.
> 
> Hmm, maybe we should add some kind of debug knob to do just that. That
> way we could test most of the gen3/4 reset path with gen5+.
> 
> I thought we already had that for load detection, but I may have
> imagined it considering that load detection seems to have been 
> broken now for a while.

We still have it for load detect, including an igt. No one looks at igt
results though :(

Maarten, can you please take care of both of these before pushing more
atomic work? And I guess that means that we should apply the revert first.
So Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch> on the revert.

Cheers, Daniel

> 
> > It should be fixed by applying the rest of the interruptible
> > series so we can mask EIO, followed by replacing i915_mutex_lock_interruptible with mutex_lock_interruptible so there will be no waiting for gpu reset.
> > This is what's causing the deadlock here. :)
> > 
> > ~Maarten
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 22c0f8a54053..df6dbbc85855 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3207,9 +3207,11 @@ void intel_prepare_reset(struct drm_device *dev)
> >  	if (IS_GEN2(dev))
> >  		return;
> >  
> > +#if 0
> >  	/* reset doesn't touch the display */
> >  	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> >  		return;
> > +#endif
> >  
> >  	drm_modeset_lock_all(dev);
> >  	/*
> > @@ -3245,7 +3247,12 @@ void intel_finish_reset(struct drm_device *dev)
> >  		 * FIXME: Atomic will make this obsolete since we won't schedule
> >  		 * CS-based flips (which might get lost in gpu resets) any more.
> >  		 */
> > +#if 0
> >  		intel_update_primary_planes(dev);
> > +#else
> > +		intel_display_resume(dev);
> > +		drm_modeset_unlock_all(dev);
> > +#endif
> >  		return;
> >  	}
> >  
> > @@ -13174,12 +13181,12 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> >  			flush_workqueue(dev_priv->wq);
> >  	}
> >  
> > -	ret = i915_mutex_lock_interruptible(dev);
> > +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> >  	if (ret)
> >  		return ret;
> >  
> >  	ret = drm_atomic_helper_prepare_planes(dev, state);
> > -	if (!ret && !async) {
> > +	if (!ret && !async && !i915_reset_in_progress(&dev_priv->gpu_error)) {
> >  		u32 reset_counter;
> >  
> >  		reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch