[PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't do CPU access to GPU contexts so making the GPU access snoop
the CPU caches seems silly, and potentially expensive.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 6043062..746fb23 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -240,7 +240,7 @@ __create_hw_context(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	if (INTEL_INFO(dev)->gen >= 7) {
+	if (INTEL_INFO(dev)->gen >= 7 && HAS_LLC(dev)) {
 		ret = i915_gem_object_set_cache_level(ctx->obj,
 						      I915_CACHE_L3_LLC);
 		/* Failure shouldn't ever happen this early */
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[Chris Wilson]

On Mon, Mar 31, 2014 at 06:09:49PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We don't do CPU access to GPU contexts so making the GPU access snoop
> the CPU caches seems silly, and potentially expensive.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Maybe define a macro to be HAS_L3_CACHE?

The root cause is that our singular enum is not flexible enough to
account for all platform variations.

Otherwise, lgtm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[Ville Syrjälä]

On Mon, Mar 31, 2014 at 04:39:37PM +0100, Chris Wilson wrote:
> On Mon, Mar 31, 2014 at 06:09:49PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We don't do CPU access to GPU contexts so making the GPU access snoop
> > the CPU caches seems silly, and potentially expensive.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Maybe define a macro to be HAS_L3_CACHE?

What should I do with such a macro?

> 
> The root cause is that our singular enum is not flexible enough to
> account for all platform variations.
> 
> Otherwise, lgtm.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[Chris Wilson]

On Tue, Apr 01, 2014 at 01:11:02PM +0300, Ville Syrjälä wrote:
> On Mon, Mar 31, 2014 at 04:39:37PM +0100, Chris Wilson wrote:
> > On Mon, Mar 31, 2014 at 06:09:49PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We don't do CPU access to GPU contexts so making the GPU access snoop
> > > the CPU caches seems silly, and potentially expensive.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Maybe define a macro to be HAS_L3_CACHE?
> 
> What should I do with such a macro?

I am trying to express what exactly we are testing for here. It is not
exactly LLC we care about, but L3 to hide the context switch latency.
Even though Ben thinks that's a waste of our limited resources.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Make contexts non-snooped on non-LLC platforms

[Ville Syrjälä]

On Tue, Apr 01, 2014 at 11:19:19AM +0100, Chris Wilson wrote:
> On Tue, Apr 01, 2014 at 01:11:02PM +0300, Ville Syrjälä wrote:
> > On Mon, Mar 31, 2014 at 04:39:37PM +0100, Chris Wilson wrote:
> > > On Mon, Mar 31, 2014 at 06:09:49PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We don't do CPU access to GPU contexts so making the GPU access snoop
> > > > the CPU caches seems silly, and potentially expensive.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Maybe define a macro to be HAS_L3_CACHE?
> > 
> > What should I do with such a macro?
> 
> I am trying to express what exactly we are testing for here. It is not
> exactly LLC we care about, but L3 to hide the context switch latency.
> Even though Ben thinks that's a waste of our limited resources.

VLV has L3 too, but we can't control it via the PTEs.

-- 
Ville Syrjälä
Intel OTC

[PATCH v2] drm/i915: Make contexts non-snooped on non-LLC platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't do CPU access to GPU contexts so making the GPU access snoop
the CPU caches seems silly, and potentially expensive.

v2: Use !IS_VALLEYVIEW instead of HAS_LLC as this is really
    about what the PTEs can represent.
    Add a comment clarifying the situation.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8827892..30b355a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -240,7 +240,15 @@ __create_hw_context(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	if (INTEL_INFO(dev)->gen >= 7) {
+	/*
+	 * Try to make the context utilize L3 as well as LLC.
+	 *
+	 * On VLV we don't have L3 controls in the PTEs so we
+	 * shouldn't touch the cache level, especially as that
+	 * would make the object snooped which might have a
+	 * negative performance impact.
+	 */
+	if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev)) {
 		ret = i915_gem_object_set_cache_level(ctx->obj,
 						      I915_CACHE_L3_LLC);
 		/* Failure shouldn't ever happen this early */
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Make contexts non-snooped on non-LLC platforms

[Chris Wilson]

On Fri, Apr 04, 2014 at 04:36:10PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We don't do CPU access to GPU contexts so making the GPU access snoop
> the CPU caches seems silly, and potentially expensive.
> 
> v2: Use !IS_VALLEYVIEW instead of HAS_LLC as this is really
>     about what the PTEs can represent.
>     Add a comment clarifying the situation.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2] drm/i915: Make contexts non-snooped on non-LLC platforms

[Daniel Vetter]

On Fri, Apr 04, 2014 at 02:41:00PM +0100, Chris Wilson wrote:
> On Fri, Apr 04, 2014 at 04:36:10PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We don't do CPU access to GPU contexts so making the GPU access snoop
> > the CPU caches seems silly, and potentially expensive.
> > 
> > v2: Use !IS_VALLEYVIEW instead of HAS_LLC as this is really
> >     about what the PTEs can represent.
> >     Add a comment clarifying the situation.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch