I agree Daniel. We need two patches here, 1) moving the enabling of the interrupts early on. 2) split the WA initialization into GT & Display and move GT workarounds before ring init. Thanks Deepak -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter Sent: Monday, May 4, 2015 8:25 PM To: Chris Wilson; Antoine, Peter; Deepak S; S, Deepak; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Tian, YeX Subject: Re: [PATCH v2] drm/i915: Avoid GPU hang when coming out of S3 or S4 On Wed, Apr 29, 2015 at 12:39:17PM +0100, Chris Wilson wrote: > > On Wed, Apr 29, 2015 at 02:07:19PM +0300, David Weinehall wrote: > > On Tue, Apr 28, 2015 at 03:46:46PM +0100, Chris Wilson wrote: > > > On Tue, Apr 28, 2015 at 02:38:25PM +0000, Antoine, Peter wrote: > > > > So is the plan to push these patches and have follow-on work to cover the other paths? > > > > As this fixes the Bugzilla issue that has been raised. > > > > > > You've identified an issue, but I think your patch is incomplete. > > > > I've tried my best to go through the remaining similar-looking code, > > but the rest seems fine (I might've missed something though). > > > > The only thing I reacted on was that in intel_runtime_resume() the > > call to intel_init_pch_refclk() is conditional on IS_GEN6(), but > > none of the other invocations of intel_init_pch_refclk() are. The > > commit message doesn't seem to provide a sufficient explanation for why this is so. > > The explanation for moving init_hw() was that it setups clock_gating. > If any in that path are required for enabling the rings, those should > be move to > init_render_ring() or the init_context. Yeah we've had this fun before. init_clock_gating is _not_ for GT workarounds, only for modeset workarounds. We might need to rename that hook to avoid getting bitten for eternity, but moving init_hw because of clock_gating to get the rings up and running is bogus. As Chris said we need to identify which bits need to get moved to the ring init or w/a bb code and do that (and in a separate patch from enabling the interrupts early enough like this one here does). -Daniel -- 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
