Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Wed, 2015-11-18 at 09:55 +0100, Daniel Vetter wrote: > On Wed, Nov 18, 2015 at 09:36:46AM +0100, Maarten Lankhorst wrote: > > Op 17-11-15 om 18:44 schreef 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.syrj...@linux.intel.com: > > > > > > From: Ville Syrjälä> > > > > > > > > > > > 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 on the revert. > > > > > > Cheers, Daniel > > > > > Reset is already fixed upstream so reverting wont help. > > Sorry, I missed that the patch landed already. > > > What igt is used for load detection then? > > We're missing it it seems. Iirc Ander was working on it, but only the > debugfs part to force-enable load-detect logic was merged into the kernel. That wasn't me. Perhaps it was Matt? Ander ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Wed, Nov 18, 2015 at 09:36:46AM +0100, Maarten Lankhorst wrote: > Op 17-11-15 om 18:44 schreef 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.syrj...@linux.intel.com: > From: Ville Syrjälä> > 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 on the revert. > > > > Cheers, Daniel > > > Reset is already fixed upstream so reverting wont help. Sorry, I missed that the patch landed already. > What igt is used for load detection then? We're missing it it seems. Iirc Ander was working on it, but only the debugfs part to force-enable load-detect logic was merged into the kernel. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Op 17-11-15 om 18:44 schreef 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.syrj...@linux.intel.com: From: Ville Syrjälä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 on the revert. > > Cheers, Daniel > Reset is already fixed upstream so reverting wont help. What igt is used for load detection then? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
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.syrj...@linux.intel.com: > > > From: Ville Syrjälä> > > > > > 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 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(>struct_mutex); > > if (ret) > > return ret; > > > > ret = drm_atomic_helper_prepare_planes(dev, state); > > - if (!ret && !async) { > > + if (!ret && !async && !i915_reset_in_progress(_priv->gpu_error)) { > > u32 reset_counter; > > > > reset_counter = atomic_read(_priv->gpu_error.reset_counter); > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote: > Hey, > > Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > > From: Ville Syrjälä> > > > 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. > 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(>struct_mutex); > if (ret) > return ret; > > ret = drm_atomic_helper_prepare_planes(dev, state); > - if (!ret && !async) { > + if (!ret && !async && !i915_reset_in_progress(_priv->gpu_error)) { > u32 reset_counter; > > reset_counter = atomic_read(_priv->gpu_error.reset_counter); -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Hey, Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > From: Ville Syrjälä> > 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. More context please? I don't think reverting is the way to go here. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Hey, Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > From: Ville Syrjälä> > 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. 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(>struct_mutex); if (ret) return ret; ret = drm_atomic_helper_prepare_planes(dev, state); - if (!ret && !async) { + if (!ret && !async && !i915_reset_in_progress(_priv->gpu_error)) { u32 reset_counter; reset_counter = atomic_read(_priv->gpu_error.reset_counter); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx