From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC] drm/i915: Move the STC LRA eviction policy workaround after ring init. Date: Fri, 27 Apr 2012 10:34:17 +0200 Message-ID: <20120427083417.GA4841@phenom.ffwll.local> References: <1335509150-1516-1-git-send-email-kenneth@whitecape.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id F0CBA9F024 for ; Fri, 27 Apr 2012 01:33:21 -0700 (PDT) Received: by wgbds1 with SMTP id ds1so289829wgb.0 for ; Fri, 27 Apr 2012 01:33:21 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1335509150-1516-1-git-send-email-kenneth@whitecape.org> 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: Kenneth Graunke Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Apr 26, 2012 at 11:45:50PM -0700, Kenneth Graunke wrote: > Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in > OpenGL programs such as Google MapsGL, Google Earth, and gzdoom. > > While commit 09be664ecc77d58 introduced the workaround, the register > write didn't actually stick: a printk and I915_READ immediately after > the write would return the new value, but a second one would show that > it had reverted to the original value...even with no intervening code. > > The hardware documentation mentions that the ring must be idle before > writing CACHE_MODE_0. This provided a clue that it might be necessary > to initialize the rings before attempting to program the register. Sure > enough, moving the write after ring initialization makes it stick. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535 > Signed-off-by: Kenneth Graunke > --- > drivers/gpu/drm/i915/i915_dma.c | 7 +++++++ > drivers/gpu/drm/i915/intel_pm.c | 4 ---- > 2 files changed, 7 insertions(+), 4 deletions(-) > > Here's a horrible patch---putting that one workaround bit directly inside > i915_load_modeset_init is clearly a terrible idea. There will obviously > be many more, and per-generation. I suspect that many more of the > workaround bits we're setting in init_clock_gating may need to be moved > later in the init process too...but I haven't checked to see which ones > are failing to stick. > > So I guess there's a couple questions: > > * Does it make sense to move ALL the init_clock_gating stuff to this > point in time? Or are there some registers that need to be done early, > where they are today? (If we can move them all, we could just move the > call to init_clock_gating and be done with it...) > > Apparently CACHE_MODE_0 is a context-specific register, while some others > are not. I like having all the workaround bits in one place, but that > may or may not be feasible... > > * Do we want to make an intel_apply_workarounds() function or such? > Perhaps use a function pointer that gets filled in on a per-chipset basis? > > * Is this the best time to set it? Later? Elsewhere? > > * What should the code look like long-term, and what would be easiest for > backporting to stable kernels? I'm working on a real fix for -next that cleans up our gem init handling and workaround setting, but for -fixes I guess we just need to add yet another cludge to init_render_ring ... That way it should get called at all the right places (driver load, resume and gpu reset). -Daniel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48