On Thu, May 21, 2015 at 01:18:44PM -0700, Jesse Barnes wrote:
> On 05/19/2015 10:32 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > GTT caching was disabled by default on gen8 due to not working with
> > big pages. Some information suggests that it got fixed, but still
> > GTT caching has been left disabled by default. Or could be it just
> > meant that the default was changed to off, and hence the problem
> > got solved.
> >
> > Enable GTT caching in the hopes of some performance increase.
> > Whether or not the big pages issue has been fixed is irrelevant
> > at this stage since we don't use big pages.
> >
> > This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
> > BDW, but supposedly it has larger TLBs so might not benefit as much.
> > On HSW GTT caching is enabled by default.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
> > 2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 84af255..90640d5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
> > #define RING_HWS_PGA(base) ((base)+0x80)
> > #define RING_HWS_PGA_GEN6(base) ((base)+0x2080)
> >
> > +#define HSW_GTT_CACHE_EN 0x4024
> > +#define GTT_CACHE_EN_ALL 0xF0007FFF
> > #define GEN7_WR_WATERMARK 0x4028
> > #define GEN7_GFX_PRIO_CTRL 0x402C
> > #define ARB_MODE 0x4030
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 5ec56b6..58517a50 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
> > I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
> > I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> >
> > + /*
> > + * WaGttCachingOffByDefault:bdw
> > + * GTT cache may not work with big pages, so if those
> > + * are ever enabled GTT cache may need to be disabled.
> > + */
> > + I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> > +
> > lpt_init_clock_gating(dev);
> > }
> >
> > @@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
> > /* WaDisableSDEUnitClockGating:chv */
> > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> > GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> > +
> > + /*
> > + * GTT cache may not work with big pages, so if those
> > + * are ever enabled GTT cache may need to be disabled.
> > + */
> > + I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> > }
> >
> > static void g4x_init_clock_gating(struct drm_device *dev)
> >
>
> Looks ok to me; I guess testing will be the real review here.
Just aside: If you think the real test for a patch is the real world imo
an important part of the review work is to make sure we do have that
testing coverage. We have a few igts that specifically exercise gtt tlb
issues (from previous generations), so I think we're covered here.
> Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
Merged all three, thanks for patches&review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
