| Message ID | 1449596332-23470-1-git-send-email-wayne.boyer@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 08, 2015 at 09:38:52AM -0800, Wayne Boyer wrote: > Do some further clean up based on the initial review of > drm/i915: Separate cherryview from valleyview. > > In this case, in i915_gem_alloc_context_obj() only call > i915_gem_object_set_cache_level() for Ivy Bridge devices > since later platforms don't have L3 control bits in the PTE. > > v2: Expand comment to mention snooping requirement. (Ville, Imre) > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Wayne Boyer <wayne.boyer@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 4b1161d..900ffd0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -189,8 +189,15 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) > * shouldn't touch the cache level, especially as that > * would make the object snooped which might have a > * negative performance impact. > + * > + * Snooping is required on non-llc platforms in execlist > + * mode, but since all GGTT accesses use PAT entry 0 we > + * get snooping anyway regardless of cache_level. > + * > + * This is only applicable for Ivy Bridge devices since > + * later platforms don't have L3 control bits in the PTE. > */ > - if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) { > + if (IS_IVYBRIDGE(dev)) { > ret = i915_gem_object_set_cache_level(obj, I915_CACHE_L3_LLC); > /* Failure shouldn't ever happen this early */ > if (WARN_ON(ret)) { > -- > 2.6.3
On Tue, Dec 08, 2015 at 07:45:50PM +0200, Ville Syrjälä wrote: > On Tue, Dec 08, 2015 at 09:38:52AM -0800, Wayne Boyer wrote: > > Do some further clean up based on the initial review of > > drm/i915: Separate cherryview from valleyview. > > > > In this case, in i915_gem_alloc_context_obj() only call > > i915_gem_object_set_cache_level() for Ivy Bridge devices > > since later platforms don't have L3 control bits in the PTE. > > > > v2: Expand comment to mention snooping requirement. (Ville, Imre) > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Wayne Boyer <wayne.boyer@intel.com> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Loses Haswell. -Chris
On Tue, Dec 08, 2015 at 08:50:48PM +0000, Chris Wilson wrote: > On Tue, Dec 08, 2015 at 07:45:50PM +0200, Ville Syrjälä wrote: > > On Tue, Dec 08, 2015 at 09:38:52AM -0800, Wayne Boyer wrote: > > > Do some further clean up based on the initial review of > > > drm/i915: Separate cherryview from valleyview. > > > > > > In this case, in i915_gem_alloc_context_obj() only call > > > i915_gem_object_set_cache_level() for Ivy Bridge devices > > > since later platforms don't have L3 control bits in the PTE. > > > > > > v2: Expand comment to mention snooping requirement. (Ville, Imre) > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: Wayne Boyer <wayne.boyer@intel.com> > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Loses Haswell. Nope. HSW doesn't do L3 via PTEs.
On Tue, Dec 08, 2015 at 11:07:01PM +0200, Ville Syrjälä wrote: > On Tue, Dec 08, 2015 at 08:50:48PM +0000, Chris Wilson wrote: > > On Tue, Dec 08, 2015 at 07:45:50PM +0200, Ville Syrjälä wrote: > > > On Tue, Dec 08, 2015 at 09:38:52AM -0800, Wayne Boyer wrote: > > > > Do some further clean up based on the initial review of > > > > drm/i915: Separate cherryview from valleyview. > > > > > > > > In this case, in i915_gem_alloc_context_obj() only call > > > > i915_gem_object_set_cache_level() for Ivy Bridge devices > > > > since later platforms don't have L3 control bits in the PTE. > > > > > > > > v2: Expand comment to mention snooping requirement. (Ville, Imre) > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Signed-off-by: Wayne Boyer <wayne.boyer@intel.com> > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Loses Haswell. > > Nope. HSW doesn't do L3 via PTEs. I was actually thinking of eLLC, but we never differentiate that bit in our cache levels. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 4b1161d..900ffd0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -189,8 +189,15 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) * shouldn't touch the cache level, especially as that * would make the object snooped which might have a * negative performance impact. + * + * Snooping is required on non-llc platforms in execlist + * mode, but since all GGTT accesses use PAT entry 0 we + * get snooping anyway regardless of cache_level. + * + * This is only applicable for Ivy Bridge devices since + * later platforms don't have L3 control bits in the PTE. */ - if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) { + if (IS_IVYBRIDGE(dev)) { ret = i915_gem_object_set_cache_level(obj, I915_CACHE_L3_LLC); /* Failure shouldn't ever happen this early */ if (WARN_ON(ret)) {
Do some further clean up based on the initial review of drm/i915: Separate cherryview from valleyview. In this case, in i915_gem_alloc_context_obj() only call i915_gem_object_set_cache_level() for Ivy Bridge devices since later platforms don't have L3 control bits in the PTE. v2: Expand comment to mention snooping requirement. (Ville, Imre) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Wayne Boyer <wayne.boyer@intel.com> --- drivers/gpu/drm/i915/i915_gem_context.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)