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@xxxxxxxxxxxxxxx:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
