From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 17/25] drm/i915: ValleyView cacheability is different Date: Wed, 21 Mar 2012 14:35:30 -0700 Message-ID: <20120321143530.4013fd17@jbarnes-desktop> References: <1332359326-15051-1-git-send-email-jbarnes@virtuousgeek.org> <1332359326-15051-18-git-send-email-jbarnes@virtuousgeek.org> <20120321211936.GI9913@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0135888250==" Return-path: Received: from oproxy1-pub.bluehost.com (oproxy1-pub.bluehost.com [66.147.249.253]) by gabe.freedesktop.org (Postfix) with SMTP id DE4BD9E761 for ; Wed, 21 Mar 2012 14:35:33 -0700 (PDT) In-Reply-To: <20120321211936.GI9913@phenom.ffwll.local> 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: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0135888250== Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/leRdx+m9PmoSYnLGXAvYqrS"; protocol="application/pgp-signature" --Sig_/leRdx+m9PmoSYnLGXAvYqrS Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Wed, 21 Mar 2012 22:19:36 +0100 Daniel Vetter wrote: > On Wed, Mar 21, 2012 at 12:48:38PM -0700, Jesse Barnes wrote: > > The GT can snoop CPU writes, but doesn't snoop into the CPU cache when > > it does writes, so we can't use the cache bits the same way. > >=20 > > So map the status and pipe control pages as uncached on ValleyView, and > > only set the pages to cached if we're on a supported platform. > >=20 > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_drv.c | 2 + > > drivers/gpu/drm/i915/intel_ringbuffer.c | 35 +++++++++++++++++++++++= +------ > > 2 files changed, 30 insertions(+), 7 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i91= 5_drv.c > > index e4fa294..a636703 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -255,6 +255,7 @@ static const struct intel_device_info intel_valleyv= iew_m_info =3D { > > .has_bsd_ring =3D 1, > > .has_blt_ring =3D 1, > > .is_valleyview =3D 1, > > + .has_llc =3D 0, > > }; > > =20 > > static const struct intel_device_info intel_valleyview_d_info =3D { > > @@ -264,6 +265,7 @@ static const struct intel_device_info intel_valleyv= iew_d_info =3D { > > .has_bsd_ring =3D 1, > > .has_blt_ring =3D 1, > > .is_valleyview =3D 1, > > + .has_llc =3D 0, >=20 > Usually we don't set feature bits to 0, please drop these 2 hunks. >=20 > > }; > > =20 > > static const struct pci_device_id pciidlist[] =3D { /* aka */ > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/= i915/intel_ringbuffer.c > > index ca3972f..f52abc4 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -319,6 +319,8 @@ init_pipe_control(struct intel_ring_buffer *ring) > > { > > struct pipe_control *pc; > > struct drm_i915_gem_object *obj; > > + int cache_level =3D HAS_LLC(ring->dev) ? I915_CACHE_LLC : I915_CACHE_= NONE; > > + struct drm_device *dev; >=20 > Nope, that's not gonna work. Afaik we have 3 kinds of snoopable bus access > from the gpu: > - llc, i.e. snoopable access for all render operations, support if > intel_info->has_llc =3D=3D 1 > - snoopable access to untiled mem with the blitter, supported on all > generations (down to mighty old i81x) > - snoopable access to the hw status page >=20 > Please clear up the confusion here. Below you also use the IS_VLV macro, > that seems more appropriate. Also I'm wondering whether this is supposed > to be fixed in future silicon revisions, if so please mark this as a w/a > that can be reaped as soon as we don't use this early silicon for testing > any more. Yeah I don't like this either. I have a meeting with the hw guys on Friday and will try to clear this up. It may be best to just check for IS_VALLEYVIEW here instead (this is what I had before, then I decided to get clever so I could squash in the HAS_LLC change in i915_gem.c). You're right there are multiple types of cacheability we need to accommodate. So maybe HAS_LLC for i915_gem.c and HWS_CACHEABLE or something to easily identify the retro chips... --=20 Jesse Barnes, Intel Open Source Technology Center --Sig_/leRdx+m9PmoSYnLGXAvYqrS Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJPakmiAAoJEIEoDkX4Qk9h5JkQAKsLklrnYXFVGTecgiDIkogm mLdN2DIqWTxpNjfrtN4C6WKX0WCbMeLTDXpXBFv2ffjCuZAlpy0KRh/l2jpmoCo5 q15WyrJTzr/7RHUNKy/6cdBxoffzqC1giVD8O+YEItHHuO32d5AaLqN43DKOIxTy Zmd3YSBU0NwVLeUYQ/Dp50JSEKbMKCoWc/7nxb+TEyCznkCurR3R9Vk8luTyPvMC ou1pofop10UIIxz15kEfkF9KmRYuEEvBxk4w/Hl8f2oQ/oN5g0Wwz+VljKINtPtt 7JhHb7ZGMf589sLS3u69xzIq+iL83c0IsGE4xr4bIqVRimQ+Hl1NY/pIexQpcW7d WfiIPuQmatWDmSdtu8sXxjg9GOyTARtogwZ8E0mtsSZer/v55t2tRq4aK+WShL3u x4hyNrP8elpoivNkJ/1PTZ5j+lUIr8k1vf15TDpRNqJlqJmeuD9Wc4p7t4xVVqMa u+VcNz5chG/3CjoA936e5v5H546+OviExptBAgtcBsoe0TccrK3ykgF0VgyAWQLw mTfG6+3a2uvslOqh84nbrjRDu9J2wLZcjYQwDsbDuO6K0AD2BNcE2sqJswxarEYE 054h5YtMiD0TbPuAfd2FhAq75+G1It+JccHid6zYup+Uprl7+O6Rh3VA9lQviMbJ WDrYqMyMrep9OwJ0pmoy =138V -----END PGP SIGNATURE----- --Sig_/leRdx+m9PmoSYnLGXAvYqrS-- --===============0135888250== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0135888250==--