From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Re-enable rc6 w/fix Date: Tue, 15 Mar 2011 09:58:40 +0000 Message-ID: <1bdc18$jseofn@fmsmga002.fm.intel.com> References: <1300164901-4937-1-git-send-email-ben@bwidawsk.net> <20110315050020.GA3864@lundgren.kumite> <20110315071251.GC7406@lundgren.kumite> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 26DBC9E7FD for ; Tue, 15 Mar 2011 02:58:44 -0700 (PDT) In-Reply-To: <20110315071251.GC7406@lundgren.kumite> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 15 Mar 2011 00:12:51 -0700, Ben Widawsky wrote: > On Mon, Mar 14, 2011 at 10:00:20PM -0700, Ben Widawsky wrote: > > On Mon, Mar 14, 2011 at 09:55:01PM -0700, Ben Widawsky wrote: > > > This fixes a race condition with MI_SET_CONTEXT and setting of the > > > PWRCTXA register. If PWRCTXA ends up getting set before MI_SET_CONTEXT > > > completes, it's possible that the system will enter rc6, and try to > > > return to the default render context, which if unset, could cause a GPU > > > hang > > > > > > Resolve https://bugzilla.kernel.org/show_bug.cgi?id=28582 > > > > I'm still waiting for feedback on bugzilla if this patch works like the > > previous. Just submitting it here for review while we wait... > > It appears that I've jumped the gun on this fix. I can sort of reason > that the LOAD_REGISTER_IMM doesn't work, perhaps because somehow the > MI_SET_CONTEXT is really slow, and the GPU executes the register load > out of order, thus invoking the original potential race. If in doubt add a second MI_FLUSH, that is guaranteed to block on the retirement of the first. But I suspect the actual bug is the misuse of LOAD_REGISTER_IMM. > But I followed up with the i915_gpu_idle(), and that too did not work... Yeah, because the rings were idle (no outstanding requests and no active buffers) it was a no-op. So there is no shortcut, you have to add a request and then wait (or then idle). The alternative is to add the HEAD!=TAIL polling to i915_gpu_idle as well, but i915_gpu_idle() is also used during normal ops via evict_everything, so I'd rather keep it slim. -Chris -- Chris Wilson, Intel Open Source Technology Centre