From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC] Move BDW workarounds to ring init fn Date: Wed, 30 Jul 2014 12:36:28 +0300 Message-ID: <20140730093628.GD4193@intel.com> References: <1406565106-27522-1-git-send-email-arun.siluvery@linux.intel.com> <20140728172645.GO27580@intel.com> <53D81FEB.5060808@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4E9CB6E5B4 for ; Wed, 30 Jul 2014 02:36:32 -0700 (PDT) Content-Disposition: inline In-Reply-To: <53D81FEB.5060808@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Siluvery, Arun" Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Jul 29, 2014 at 11:27:55PM +0100, Siluvery, Arun wrote: > On 28/07/2014 18:26, Ville Syrj=E4l=E4 wrote: > > On Mon, Jul 28, 2014 at 05:31:45PM +0100, arun.siluvery@linux.intel.com= wrote: > >> From: Arun Siluvery > >> > >> This patch moves BDW workarounds from init_clock_gating() to render ri= ng > >> init fn otherwise they are lost when gpu is reset. > >> In case of execlists, some of the workarounds modify registers that are > >> part of register state context which doesn't get initialized until > >> init_clock_gating(); this results in default context with incorrect va= lues > >> as it is restored and saved before updated by workarounds. > > > > I don't think it has to do with execlists. Many of the registers are > > part of the context image even in ring buffer mode AFAIK. > > > >> > >> Open issue: > >> For Wa4x4STCOptimizationDisable, we set CACHE_MODE_1[6:6] =3D 1 > >> At the time when HW contexts are enabled after rings are initialized w= ith > >> default context this workaround is valid but followed by a context swi= tch > >> this is getting reset, please see below log snippet. > > > > This is a bit weird. The default context should have restore inhibit=3D= =3D1 > > so it shouldn't clobber the CACHE_MODE_1 register. There was a specific= magic > > dance you're supposed to do when accessing such registers with mmio, bu= t here > > we do the write even before the first context switch. > > > > Apparently there was some kind of problem with CACHE_MODE_0 on snb too: > > commit 3a69ddd6f872180b6f61fda87152b37202118fbc > > Author: Kenneth Graunke > > Date: Fri Apr 27 12:44:41 2012 -0700 > > > > drm/i915: Set the Stencil Cache eviction policy to non-LRA mode. > > > > but IIRC I wasn't able to reproduce it when I tried. > = > Similar to this register I am also applying this in render ring init fn. > = > > > > Maybe we need to delay these register writes until we've switched to th= e default > > context? > > > In its current state (WAs applied in init_clock_gating()) we are writing = > these registers after switching to default context. > = > When a new hw context is created does all the registers part of context = > start with default values or they sample the current state? and at what = > point this sampling takes place? We load each uninitialized context with restore inhibit=3Dtrue so AFAIK the current register values should stay intact. > = > As a test I have updated CACHE_MODE_1 after mi_set_context() then the = > workaround was valid with every context switch but I think it may not be = > the right way otherwise we will have to update other WA registers also = > at this point with every context switch. Maybe there's something special about the very first context switch? Though I don't see why that would be the case. > = > regards > Arun > = > >> > >> ... > >> [ 5.978209] [drm:i915_pages_create_for_stolen] offset=3D0x0, size= =3D8294400 > >> [ 5.978213] [drm:intel_alloc_plane_obj] plane fb obj ffff8801472e00= 00 > >> [ 5.978215] [drm:i915_gem_setup_global_gtt] reserving preallocated = space: 0 + 7e9000 > >> [ 5.978216] [drm:i915_gem_setup_global_gtt] clearing unused GTT spa= ce: [7e9000, fffff000] > >> [ 5.979613] [drm:i915_gem_init] CACHE_MODE_1: 0x00000180 > >> [ 5.981372] [drm:gen8_ppgtt_init] Allocated 4 pages for page direct= ories (0 wasted) > >> [ 5.981373] [drm:gen8_ppgtt_init] Allocated 2048 pages for page tab= les (0 wasted) > >> [ 5.981376] [drm:i915_gem_context_init] HW context support initiali= zed > >> [ 5.981462] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x00000180 > >> [ 5.981467] [drm:i915_gem_init_rings] CACHE_MODE_1: 0x00000180 > >> [ 5.981704] [drm:bdw_init_workarounds] CACHE_MODE_1: 0x000001C0 > >> [ 5.981716] [drm:init_status_page] bsd ring hws offset: 0x0081e000 > >> [ 5.981792] [drm:init_status_page] blitter ring hws offset: 0x0083f= 000 > >> [ 5.981910] [drm:init_status_page] video enhancement ring hws offse= t: 0x00860000 > >> [ 5.982001] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x000001C0 > >> [ 5.982104] [drm:i915_gem_context_enable] Switch render ring to def= ault_context > >> [ 5.982106] [drm:i915_gem_render_state_init] render ring: Render st= ate init > >> [ 5.982120] [drm:do_switch] render ring, CACHE_MODE_1: 0x000001C0, = uninitialized: 1 > >> [ 5.982121] [drm:i915_gem_context_enable] Switch bsd ring to defaul= t_context > >> [ 5.982122] [drm:do_switch] bsd ring, CACHE_MODE_1: 0x000001C0, uni= nitialized: 0 > >> [ 5.982123] [drm:i915_gem_context_enable] Switch blitter ring to de= fault_context > >> [ 5.982126] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x000001C0,= uninitialized: 0 > >> [ 5.982126] [drm:i915_gem_context_enable] Switch video enhancement = ring to default_context > >> [ 5.982128] [drm:do_switch] video enhancement ring, CACHE_MODE_1: 0= x000001C0, uninitialized: 0 > >> [ 5.982133] [drm:i915_gem_init] CACHE_MODE_1: 0x000001C0 > >> [ 5.982258] [drm:intel_init_clock_gating] > >> ... > >> [ 10.037019] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x00000180,= uninitialized: 0 > >> ... > >> [ 10.488145] [drm:do_switch] render ring, CACHE_MODE_1: 0x00000180, = uninitialized: 0 > >> ... > >> > >> I am currently testing this with an igt which triggers a gpu reset and= compares > >> WA register contents before and after reset but the test fails because= of > >> this register hence not sending it now. > >> Please let me know how to keep this WA valid after a context switch. > >> > >> > >> Arun Siluvery (1): > >> drm/i915/bdw: Initialize BDW workarounds in render ring init fn > >> > >> drivers/gpu/drm/i915/i915_debugfs.c | 46 ++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_pm.c | 59 -----------------------= ----- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 68 +++++++++++++++++++++++= ++++++++++ > >> 3 files changed, 114 insertions(+), 59 deletions(-) > >> > >> -- > >> 1.9.2 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- = Ville Syrj=E4l=E4 Intel OTC