Re: [PATCH] drm/i915: Prefer setting PTE cache age to 3

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 22, 2013 at 09:53:23AM -0800, Jesse Barnes wrote:
> On Fri, 22 Nov 2013 10:37:53 +0000
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > We have conflicting benchmark data that suggest either age 0 or age 3 is
> > better. However, the earlier benchmark on which we based the switch to
> > age 0
> > 
> > (commit 0d8ff15e9a15f2b393e53337a107b7a1e5919b6d
> > Author: Ben Widawsky <benjamin.widawsky@intel.com>
> > Date:   Thu Jul 4 11:02:03 2013 -0700
> > 
> >     drm/i915/hsw: Set correct Haswell PTE encodings)
> > 
> > actually seems to prefer the default PTE encoding as age 3. Presumably,
> > this is in part due to the use of MOCS to override the PTE encodings
> > when appropriate.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69870
> > Tested-by: mengmeng.meng@intel.com
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index efb5dab61c81..c4a3f9ae0d43 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -57,7 +57,9 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
> >  #define HSW_WB_LLC_AGE3			HSW_CACHEABILITY_CONTROL(0x2)
> >  #define HSW_WB_LLC_AGE0			HSW_CACHEABILITY_CONTROL(0x3)
> >  #define HSW_WB_ELLC_LLC_AGE0		HSW_CACHEABILITY_CONTROL(0xb)
> > +#define HSW_WB_ELLC_LLC_AGE3		HSW_CACHEABILITY_CONTROL(0x8)
> >  #define HSW_WT_ELLC_LLC_AGE0		HSW_CACHEABILITY_CONTROL(0x6)
> > +#define HSW_WT_ELLC_LLC_AGE3		HSW_CACHEABILITY_CONTROL(0x7)
> >  
> >  #define GEN8_PTES_PER_PAGE		(PAGE_SIZE / sizeof(gen8_gtt_pte_t))
> >  #define GEN8_PDES_PER_PAGE		(PAGE_SIZE / sizeof(gen8_ppgtt_pde_t))
> > @@ -185,10 +187,10 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
> >  	case I915_CACHE_NONE:
> >  		break;
> >  	case I915_CACHE_WT:
> > -		pte |= HSW_WT_ELLC_LLC_AGE0;
> > +		pte |= HSW_WT_ELLC_LLC_AGE3;
> >  		break;
> >  	default:
> > -		pte |= HSW_WB_ELLC_LLC_AGE0;
> > +		pte |= HSW_WB_ELLC_LLC_AGE3;
> >  		break;
> >  	}
> >  
> 
> Yeah I guess as long as userspace can override the default with MOCS,
> this is fine.

IIRC MOCS can only set the age to 3. So this change does limit what
userspace might achieve w/ MOCS usage a little bit.

I guess it might be better to change userspace to set everything to
LLC(+eLLC) at age 3 via MOCS (except potential scanout buffers), and
leave the PTE default at age 0.

But then again we need to also remember that not everything has a MOCS
field. So I'm not sure there's any truly correct answer for this.

-- 
Ville Syrjälä
Intel OTC