On 01/03/16 17:14, Chris Wilson wrote:
On Tue, Mar 01, 2016 at 04:29:35PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
commit e5756c10d841ddb448293c849392f3d6b809561f
Author: Imre Deak <imre.deak@xxxxxxxxx>
Date: Fri Aug 14 18:43:30 2015 +0300
drm/i915/bxt: don't allow cached GEM mappings on A stepping
Added an exception of disallowing snooping for Broxton A
stepping hardware but userptr was still enabling it regardless.
Move the check to HAS_SNOOP now that it is used from multiple
call sites and use it.
Idea is that userspace which relies on userptr snooping, for
example for fine grained buffered SVM, can query the caching
mode on a created userptr object to determine whether coherency
is supported or not.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Imre Deak <imre.deak@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 671295523317..73f0db17b990 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2629,6 +2629,7 @@ struct drm_i915_cmd_table {
#define HAS_LLC(dev) (INTEL_INFO(dev)->has_llc)
#define HAS_WT(dev) ((IS_HASWELL(dev) || IS_BROADWELL(dev)) && \
__I915__(dev)->ellc_size)
+#define HAS_SNOOP(dev) (!IS_BXT_REVID(dev, 0, BXT_REVID_A1))
#define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
#define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3d31d3ac589e..1c6e8fb9d392 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3949,7 +3949,7 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
* cacheline, whereas normally such cachelines would get
* invalidated.
*/
- if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
+ if (!HAS_SNOOP(dev))
if (!HAS_LLC && !HAS_SNOOP)
and HAS_SNOOP should be true for everything that is not llc, with the
exception above. So make a dev_info->has_snoop feature bit (or
uses_snoop?) and then clear the copy inside the struct for the special
case.
Sure I can do that, but isn't it the same as this patch? Unless
something with LLC and no snooping appears?
return -ENODEV;
level = I915_CACHE_LLC;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 4b09c840d493..48aaff019b42 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -782,7 +782,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file
drm_gem_private_object_init(dev, &obj->base, args->user_size);
i915_gem_object_init(obj, &i915_gem_userptr_ops);
- obj->cache_level = I915_CACHE_LLC;
+ obj->cache_level = HAS_SNOOP(dev) ? I915_CACHE_LLC : I915_CACHE_NONE;
This should be
if (!HAS_SNOOP && !HAS_LLC)
return -ENODEV;
as we can not support a coherent pointer shared between memory and the GPU
in this case.
Hm interesting, why not? I mean, what is the difference compared to
normal BOs which will be uncached on !LLC?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
