From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris Date: Tue, 30 Jul 2013 21:01:22 +0300 Message-ID: <20130730180122.GW5004@intel.com> References: <1375203516-8023-1-git-send-email-chris@chris-wilson.co.uk> <1375203516-8023-2-git-send-email-chris@chris-wilson.co.uk> <20130730171928.GV5004@intel.com> <20130730173615.GA6785@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 9790EE6CC4 for ; Tue, 30 Jul 2013 11:01:49 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130730173615.GA6785@cantiga.alporthouse.com> 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: Chris Wilson , intel-gfx@lists.freedesktop.org, Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Tue, Jul 30, 2013 at 06:36:15PM +0100, Chris Wilson wrote: > On Tue, Jul 30, 2013 at 08:19:28PM +0300, Ville Syrj=E4l=E4 wrote: > > On Tue, Jul 30, 2013 at 05:58:36PM +0100, Chris Wilson wrote: > > > Haswell GT3e has the unique feature of supporting Write-Through cache= ing > > > of objects within the eLLC. The purpose of this is to enable the disp= lay > > > plane to remain coherent whilst objects lie resident in the eLLC - so > > > that we in theory get the best of both worlds, perfect display and fa= st > > > access. > > = > > The description here talks about eLLC only, but you set the PTE for > > WT in LLC/eLLC both. > = > s/eLLC/LLC/ > = > For some reason, I keep telling myself that it is a magic property of > the eLLC otherwise why wouldn't they do it for all LLC! > = > > > - ret =3D i915_gem_object_set_cache_level(obj, I915_CACHE_NONE); > > > + ret =3D i915_gem_object_set_cache_level(obj, > > > + HAS_WT(obj->base.dev) ? I915_CACHE_WT : I915_CACHE_NONE); > > = > > Don't we need to tweak the write domain like we do for UC to make sure > > already dirty lines get flushed from caches? > = > You need to explicitly do the flush, which gets ugly. Ah right, because i915_gem_clflush_object() checks the cache_level. > I choose to ignore > the problem as unlike the LLC -> UC transition, I haven't spotted any > dirt. However, it doesn't look too bad if we replace it like so: Hmm, and what about CPU access in general? CPU doesn't know about the WT policy, so I'm assuming we'd need to flush after all CPU writes. > = > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_= gem.c > index ac1b9cd..0e089e2 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3362,27 +3362,32 @@ int i915_gem_object_set_cache_level(struct drm_i9= 15_gem_object *obj, > i915_gem_obj_ggtt_set_color(obj, cache_level); > } > = > - if (cache_level =3D=3D I915_CACHE_NONE) { > - u32 old_read_domains, old_write_domain; > - > + if (cache_level =3D=3D I915_CACHE_NONE || > + cache_level =3D=3D I915_CACHE_WT) { > /* If we're coming from LLC cached, then we haven't > * actually been tracking whether the data is in the > * CPU cache or not, since we only allow one bit set > * in obj->write_domain and have been skipping the clflushes. > - * Just set it to the CPU cache for now. > + * Do them now. > */ > - WARN_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU); > - WARN_ON(obj->base.read_domains & ~I915_GEM_DOMAIN_CPU); > + if (obj->base.read_domains & I915_GEM_DOMAIN_CPU) { > + u32 old_read_domains, old_write_domain; > = > - old_read_domains =3D obj->base.read_domains; > - old_write_domain =3D obj->base.write_domain; > + BUG_ON(obj->pages =3D=3D NULL); > = > - obj->base.read_domains =3D I915_GEM_DOMAIN_CPU; > - obj->base.write_domain =3D I915_GEM_DOMAIN_CPU; > + trace_i915_gem_object_clflush(obj); > + drm_clflush_sg(obj->pages); > = > - trace_i915_gem_object_change_domain(obj, > - old_read_domains, > - old_write_domain); > + old_read_domains =3D obj->base.read_domains; > + old_write_domain =3D obj->base.write_domain; > + > + obj->base.read_domains &=3D ~I915_GEM_DOMAIN_CPU; > + obj->base.write_domain &=3D ~I915_GEM_DOMAIN_CPU; > + > + trace_i915_gem_object_change_domain(obj, > + old_read_domains, > + old_write_domain); > + } > } > = > obj->cache_level =3D cache_level; > = > -- = > Chris Wilson, Intel Open Source Technology Centre -- = Ville Syrj=E4l=E4 Intel OTC