[Intel-gfx] [PATCH 4/7] drm/i915: properly set ppgtt cacheability on snb

[Ben Widawsky]

On Wed, Apr 11, 2012 at 12:27:25AM +0200, Daniel Vetter wrote:
> On Tue, Apr 10, 2012 at 03:11:07PM -0700, Ben Widawsky wrote:
> > On Sat, Mar 31, 2012 at 11:22:00AM +0200, Daniel Vetter wrote:
> > > For some reason snb has 2 fields to set ppgtt cacheability. This one
> > > here does not exist on gen7.
> > > 
> > > This might explain why ppgtt wasn't a win on snb like on ivb - not
> > > enough pte caching.
> > 
> > So, is it a win now?
> 
> I've later noticed that the register is part of the bsd block, so I've
> figured I'm not gonna benchmark vaapi. I'm a lazy bastard.
> 
> > > Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c |    3 +++
> > >  drivers/gpu/drm/i915/i915_reg.h |    4 ++++
> > >  2 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 5dc5f42..cab70ef 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3766,6 +3766,9 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> > >  	if (INTEL_INFO(dev)->gen == 6) {
> > >  		uint32_t ecochk, gab_ctl, ecobits;
> > >  
> > > +		ecobits = I915_READ(GAC_ECO_BITS); 
> > > +		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
> > > +
> > >  		gab_ctl = I915_READ(GAB_CTL);
> > >  		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
> > >  
> > 
> > Is this a bad rebase? This doesn't apply to whatever I am using.
> 
> Mucks around in the same area as the previous patch and hence conflicts
> without that applied.

Rebase fail was from previous patch, as discussed on IRC> ecobits was
defined before its time.

> 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index e72c251..5457980 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -125,6 +125,10 @@
> > >  #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
> > >  #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
> > >  
> > > +#define GAC_ECO_BITS			0x14090
> > > +#define   ECOBITS_PPGTT_CACHE64B	(3<<8)
> > > +#define   ECOBITS_PPGTT_CACHE4B		(0<<8)
> > > +
> > >  #define GAB_CTL				0x24000
> > >  #define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
> > >  
> > 
> > According to bspec, but 13 must be set to a 1 also.
> 
> I've checked and bit13 seems to be set already on my snb, so I've just
> or'ed in the missing bits.

I think if we're going to be pedantic about things, which I approve of,
and this series is doing, you may as well set 13 also... but whatever
you want. I don't care too much.

Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

> -Daniel
> -- 
> Daniel Vetter
> Mail: daniel at ffwll.ch
> Mobile: +41 (0)79 365 57 48
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx