[v4] drm/i915/hsw: Change default LLC age to 3

Commit Message

Ben Widawsky Aug. 5, 2013, 6:47 a.m. UTC

The default LLC age was changed:
commit 0d8ff15e9a15f2b393e53337a107b7a1e5919b6d
Author: Ben Widawsky <[e-mail]>
Date:   Thu Jul 4 11:02:03 2013 -0700

drm/i915/hsw: Set correct Haswell PTE encodings.

On the surface it would seem setting a default age wouldn't matter
because all GEM BOs are aged similarly, so the order in which objects
are evicted would not be subject to aging. The current working theory as
to why this caused a regression though is that LLC is a bit special in
that it is shared with the CPU. Presumably (not verified) the CPU
fetches cachelines with age 3, and therefore recently cached GPU objects
would be evicted before similar CPU object first when the LLC is full.
It stands to reason therefore that this would negatively impact CPU
bound benchmarks - but those seem to be low on the priority list.

eLLC OTOH does not have this same property as LLC. It should be used
entirely for the GPU, and so the age really shouldn't matter.
Furthermore, we have no evidence to suggest one is better than another
on eLLC. Since we've never properly supported eLLC before no, there
should be no regression. If the GPU client really wants "younger"
objects, they should use MOCS.

v2: Drop the extra #define (Chad)

v3: Actually git add

v4: Pimped commit message

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67062
Signed-off-by: Ben Widawsky <[e-mail]>
Reviewed-by: Chad Versace <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Aug. 5, 2013, 8:21 a.m. UTC | #1

On Sun, Aug 04, 2013 at 11:47:29PM -0700, Ben Widawsky wrote:
> The default LLC age was changed:
> commit 0d8ff15e9a15f2b393e53337a107b7a1e5919b6d
> Author: Ben Widawsky <[e-mail]>
> Date:   Thu Jul 4 11:02:03 2013 -0700
> 
> drm/i915/hsw: Set correct Haswell PTE encodings.
> 
> On the surface it would seem setting a default age wouldn't matter
> because all GEM BOs are aged similarly, so the order in which objects
> are evicted would not be subject to aging. The current working theory as
> to why this caused a regression though is that LLC is a bit special in
> that it is shared with the CPU. Presumably (not verified) the CPU
> fetches cachelines with age 3, and therefore recently cached GPU objects
> would be evicted before similar CPU object first when the LLC is full.
> It stands to reason therefore that this would negatively impact CPU
> bound benchmarks - but those seem to be low on the priority list.
> 
> eLLC OTOH does not have this same property as LLC. It should be used
> entirely for the GPU, and so the age really shouldn't matter.
> Furthermore, we have no evidence to suggest one is better than another
> on eLLC. Since we've never properly supported eLLC before no, there
> should be no regression. If the GPU client really wants "younger"
> objects, they should use MOCS.
> 
> v2: Drop the extra #define (Chad)
> 
> v3: Actually git add
> 
> v4: Pimped commit message
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67062
> Signed-off-by: Ben Widawsky <[e-mail]>
> Reviewed-by: Chad Versace <[e-mail]>
Queued for -next, thanks for the patch.
-Daniel

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e7b4204..3e7f124 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -52,6 +52,7 @@ 
  */
 #define HSW_CACHEABILITY_CONTROL(bits)	((((bits) & 0x7) << 1) | \
 					 (((bits) & 0x8) << (11 - 3)))
+#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)
 
@@ -105,7 +106,7 @@  static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
 	pte |= HSW_PTE_ADDR_ENCODE(addr);
 
 	if (level != I915_CACHE_NONE)
-		pte |= HSW_WB_LLC_AGE0;
+		pte |= HSW_WB_LLC_AGE3;
 
 	return pte;
 }