* [PATCH 1/6] drm/i915: Remove the useless flush_chipset
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-10 9:18 ` Chris Wilson
2015-02-09 21:54 ` [PATCH 2/6] drm/i915: Pass eb_vmas to execbuffer implementations Ben Widawsky
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
flush_chipset makes no sense with execlists because the former is for strictly
prior to gen6, while the latter is for gen >= 8
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/intel_lrc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 70e449b..e727217 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -576,7 +576,6 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
struct intel_engine_cs *ring = ringbuf->ring;
struct i915_vma *vma;
uint32_t flush_domains = 0;
- bool flush_chipset = false;
int ret;
list_for_each_entry(vma, vmas, exec_list) {
@@ -587,7 +586,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
return ret;
if (obj->base.write_domain & I915_GEM_DOMAIN_CPU)
- flush_chipset |= i915_gem_clflush_object(obj, false);
+ i915_gem_clflush_object(obj, false);
flush_domains |= obj->base.write_domain;
}
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/6] drm/i915: Pass eb_vmas to execbuffer implementations
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
2015-02-09 21:54 ` [PATCH 1/6] drm/i915: Remove the useless flush_chipset Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-10 9:19 ` Chris Wilson
2015-02-09 21:54 ` [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf Ben Widawsky
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
The role of eb_vmas continues to grow here as it becomes the proper
encapsulation for the data passed to the various execution function.
Next patch makes use of it... This patch was initially part of the next patch,
but got split out after I had found a bug that convinced me the two should be
separate.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 13 +++++++++++--
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 26 ++++++++++----------------
drivers/gpu/drm/i915/intel_lrc.c | 8 +++++---
drivers/gpu/drm/i915/intel_lrc.h | 3 ++-
4 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4179b90..90ff6aa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1641,6 +1641,15 @@ struct i915_workarounds {
u32 count;
};
+struct eb_vmas {
+ struct list_head vmas;
+ int and;
+ union {
+ struct i915_vma *lut[0];
+ struct hlist_head buckets[0];
+ };
+};
+
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1896,7 +1905,7 @@ struct drm_i915_private {
struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas,
+ struct eb_vmas *eb,
struct drm_i915_gem_object *batch_obj,
u64 exec_start, u32 flags);
int (*init_rings)(struct drm_device *dev);
@@ -2626,7 +2635,7 @@ int i915_gem_ringbuffer_submission(struct drm_device *dev,
struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas,
+ struct eb_vmas *eb,
struct drm_i915_gem_object *batch_obj,
u64 exec_start, u32 flags);
int i915_gem_execbuffer(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b773368..13ed13e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -41,15 +41,6 @@
#define BATCH_OFFSET_BIAS (256*1024)
-struct eb_vmas {
- struct list_head vmas;
- int and;
- union {
- struct i915_vma *lut[0];
- struct hlist_head buckets[0];
- };
-};
-
static struct eb_vmas *
eb_create(struct drm_i915_gem_execbuffer2 *args)
{
@@ -617,10 +608,11 @@ eb_vma_misplaced(struct i915_vma *vma)
static int
i915_gem_execbuffer_reserve(struct intel_engine_cs *ring,
- struct list_head *vmas,
+ struct eb_vmas *eb,
bool *need_relocs)
{
struct drm_i915_gem_object *obj;
+ struct list_head *vmas = &eb->vmas;
struct i915_vma *vma;
struct i915_address_space *vm;
struct list_head ordered_vmas;
@@ -803,7 +795,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
goto err;
need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
- ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs);
+ ret = i915_gem_execbuffer_reserve(ring, eb, &need_relocs);
if (ret)
goto err;
@@ -829,8 +821,9 @@ err:
static int
i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring,
- struct list_head *vmas)
+ struct eb_vmas *eb)
{
+ struct list_head *vmas = &eb->vmas;
struct i915_vma *vma;
uint32_t flush_domains = 0;
bool flush_chipset = false;
@@ -1136,12 +1129,13 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas,
+ struct eb_vmas *eb,
struct drm_i915_gem_object *batch_obj,
u64 exec_start, u32 flags)
{
struct drm_clip_rect *cliprects = NULL;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct list_head *vmas = &eb->vmas;
u64 exec_len;
int instp_mode;
u32 instp_mask;
@@ -1190,7 +1184,7 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
}
}
- ret = i915_gem_execbuffer_move_to_gpu(ring, vmas);
+ ret = i915_gem_execbuffer_move_to_gpu(ring, eb);
if (ret)
goto error;
@@ -1463,7 +1457,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
/* Move the objects en-masse into the GTT, evicting if necessary. */
need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
- ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs);
+ ret = i915_gem_execbuffer_reserve(ring, eb, &need_relocs);
if (ret)
goto err;
@@ -1527,7 +1521,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
exec_start += i915_gem_obj_offset(batch_obj, vm);
ret = dev_priv->gt.do_execbuf(dev, file, ring, ctx, args,
- &eb->vmas, batch_obj, exec_start, flags);
+ eb, batch_obj, exec_start, flags);
/*
* FIXME: We crucially rely upon the active tracking for the (ppgtt)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e727217..03741f9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -571,9 +571,10 @@ static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
struct intel_context *ctx,
- struct list_head *vmas)
+ struct eb_vmas *eb)
{
struct intel_engine_cs *ring = ringbuf->ring;
+ struct list_head *vmas = &eb->vmas;
struct i915_vma *vma;
uint32_t flush_domains = 0;
int ret;
@@ -621,12 +622,13 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas,
+ struct eb_vmas *eb,
struct drm_i915_gem_object *batch_obj,
u64 exec_start, u32 flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+ struct list_head *vmas = &eb->vmas;
int instp_mode;
u32 instp_mask;
int ret;
@@ -677,7 +679,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
return -EINVAL;
}
- ret = execlists_move_to_gpu(ringbuf, ctx, vmas);
+ ret = execlists_move_to_gpu(ringbuf, ctx, eb);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 6f2d7da..661080bc 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -80,11 +80,12 @@ void intel_lr_context_unpin(struct intel_engine_cs *ring,
/* Execlists */
int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists);
+struct eb_vmas;
int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas,
+ struct eb_vmas *eb,
struct drm_i915_gem_object *batch_obj,
u64 exec_start, u32 flags);
u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
2015-02-09 21:54 ` [PATCH 1/6] drm/i915: Remove the useless flush_chipset Ben Widawsky
2015-02-09 21:54 ` [PATCH 2/6] drm/i915: Pass eb_vmas to execbuffer implementations Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-10 9:21 ` Chris Wilson
2015-02-09 21:54 ` [PATCH 4/6] drm/i915: Add debugfs knobs for wbinvd threshold Ben Widawsky
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
If we're moving a bunch of buffers from the CPU domain to the GPU domain, and
we've already blown out the entire cache via a wbinvd, there is nothing more to
do.
With this and the previous patches, I am seeing a 3x FPS increase on a certain
benchmark which uses a giant 2d array texture. Unless I missed something in the
code, it should only effect non-LLC i915 platforms.
I haven't yet run any numbers for other benchmarks, nor have I attempted to
check if various conformance tests still pass.
v2: Rewrite the patch to be i915 only
Obtain whether or not we wbinvd up front.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 ++++++++++++++++----
drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++++--
4 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 90ff6aa..5d2f62d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1643,6 +1643,7 @@ struct i915_workarounds {
struct eb_vmas {
struct list_head vmas;
+ bool do_wbinvd;
int and;
union {
struct i915_vma *lut[0];
@@ -1913,6 +1914,8 @@ struct drm_i915_private {
void (*stop_ring)(struct intel_engine_cs *ring);
} gt;
+ size_t wbinvd_threshold;
+
uint32_t request_uniq;
/*
@@ -2810,6 +2813,11 @@ static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv)
void i915_gem_reset(struct drm_device *dev);
bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
+static inline bool cpu_cache_is_coherent(struct drm_device *dev,
+ enum i915_cache_level level)
+{
+ return HAS_LLC(dev) || level != I915_CACHE_NONE;
+}
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init(struct drm_device *dev);
int i915_gem_init_rings(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fc81889..5bfb332 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -61,12 +61,6 @@ static int i915_gem_shrinker_oom(struct notifier_block *nb,
void *ptr);
static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
-static bool cpu_cache_is_coherent(struct drm_device *dev,
- enum i915_cache_level level)
-{
- return HAS_LLC(dev) || level != I915_CACHE_NONE;
-}
-
static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
{
if (!cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
@@ -4878,6 +4872,11 @@ int i915_gem_init(struct drm_device *dev)
dev_priv->gt.stop_ring = intel_logical_ring_stop;
}
+ dev_priv->wbinvd_threshold = boot_cpu_data.x86_cache_size << 10;
+ /* Pick a high default in the unlikely case we got nothing */
+ if (!dev_priv->wbinvd_threshold)
+ dev_priv->wbinvd_threshold = (8 << 20);
+
ret = i915_gem_init_userptr(dev);
if (ret)
goto out_unlock;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 13ed13e..56f9268 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -50,7 +50,7 @@ eb_create(struct drm_i915_gem_execbuffer2 *args)
unsigned size = args->buffer_count;
size *= sizeof(struct i915_vma *);
size += sizeof(struct eb_vmas);
- eb = kmalloc(size, GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
+ eb = kzalloc(size, GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
}
if (eb == NULL) {
@@ -78,6 +78,7 @@ eb_reset(struct eb_vmas *eb)
{
if (eb->and >= 0)
memset(eb->buckets, 0, (eb->and+1)*sizeof(struct hlist_head));
+ eb->do_wbinvd = false;
}
static int
@@ -154,6 +155,11 @@ eb_lookup_vmas(struct eb_vmas *eb,
hlist_add_head(&vma->exec_node,
&eb->buckets[handle & eb->and]);
}
+
+ if (vma->node.size >= to_i915(obj->base.dev)->wbinvd_threshold &&
+ obj->base.write_domain & I915_GEM_DOMAIN_CPU &&
+ !cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
+ eb->do_wbinvd = true;
++i;
}
@@ -826,7 +832,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring,
struct list_head *vmas = &eb->vmas;
struct i915_vma *vma;
uint32_t flush_domains = 0;
- bool flush_chipset = false;
+ bool flush_chipset = eb->do_wbinvd;
int ret;
list_for_each_entry(vma, vmas, exec_list) {
@@ -835,12 +841,18 @@ i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring,
if (ret)
return ret;
+ flush_domains |= obj->base.write_domain;
+
+ if (eb->do_wbinvd)
+ continue;
+
if (obj->base.write_domain & I915_GEM_DOMAIN_CPU)
flush_chipset |= i915_gem_clflush_object(obj, false);
-
- flush_domains |= obj->base.write_domain;
}
+ if (eb->do_wbinvd)
+ wbinvd();
+
if (flush_chipset)
i915_gem_chipset_flush(ring->dev);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 03741f9..16ca4a2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -586,12 +586,18 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
if (ret)
return ret;
+ flush_domains |= obj->base.write_domain;
+
+ if (eb->do_wbinvd)
+ continue;
+
if (obj->base.write_domain & I915_GEM_DOMAIN_CPU)
i915_gem_clflush_object(obj, false);
-
- flush_domains |= obj->base.write_domain;
}
+ if (eb->do_wbinvd)
+ wbinvd();
+
if (flush_domains & I915_GEM_DOMAIN_GTT)
wmb();
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf
2015-02-09 21:54 ` [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf Ben Widawsky
@ 2015-02-10 9:21 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2015-02-10 9:21 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky
On Mon, Feb 09, 2015 at 01:54:16PM -0800, Ben Widawsky wrote:
> If we're moving a bunch of buffers from the CPU domain to the GPU domain, and
> we've already blown out the entire cache via a wbinvd, there is nothing more to
> do.
>
> With this and the previous patches, I am seeing a 3x FPS increase on a certain
> benchmark which uses a giant 2d array texture. Unless I missed something in the
> code, it should only effect non-LLC i915 platforms.
Out of curiosity, have you compared with how this performs with an
improved userspace? There are several techniques which userspace can do
that are much higher performance than either clflush or wbinvd.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/6] drm/i915: Add debugfs knobs for wbinvd threshold
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
` (2 preceding siblings ...)
2015-02-09 21:54 ` [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-09 21:54 ` [PATCH 5/6] drm/i915: Extract checking the necessity of flush Ben Widawsky
2015-02-09 21:54 ` [PATCH 6/6] drm/i915: obey wbinvd threshold in more places Ben Widawsky
5 siblings, 0 replies; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_debugfs.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b315f01..e0fd3ba 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4316,6 +4316,39 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
i915_cache_sharing_get, i915_cache_sharing_set,
"%llu\n");
+static int i915_wbinvd_threshold_get(void *data, u64 *val)
+{
+ struct drm_device *dev = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+ *val = dev_priv->wbinvd_threshold;
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ return 0;
+}
+
+static int i915_wbinvd_threshold_set(void *data, u64 val)
+{
+ struct drm_device *dev = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+ dev_priv->wbinvd_threshold = val;
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_wbinvd_threshold_fops,
+ i915_wbinvd_threshold_get, i915_wbinvd_threshold_set,
+ "%llu\n");
static int i915_forcewake_open(struct inode *inode, struct file *file)
{
struct drm_device *dev = inode->i_private;
@@ -4450,6 +4483,7 @@ static const struct i915_debugfs_files {
{"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
{"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
{"i915_fbc_false_color", &i915_fbc_fc_fops},
+ {"i915_wbinvd_threshold", &i915_wbinvd_threshold_fops},
};
void intel_display_crc_init(struct drm_device *dev)
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/6] drm/i915: Extract checking the necessity of flush
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
` (3 preceding siblings ...)
2015-02-09 21:54 ` [PATCH 4/6] drm/i915: Add debugfs knobs for wbinvd threshold Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-09 21:54 ` [PATCH 6/6] drm/i915: obey wbinvd threshold in more places Ben Widawsky
5 siblings, 0 replies; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5bfb332..4d5a69d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -69,6 +69,35 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
return obj->pin_display;
}
+static bool
+is_cpu_flush_required(struct drm_i915_gem_object *obj)
+{
+ /* If we don't have a page list set up, then we're not pinned
+ * to GPU, and we can ignore the cache flush because it'll happen
+ * again at bind time.
+ */
+ if (obj->pages == NULL)
+ return false;
+
+ /*
+ * Stolen memory is always coherent with the GPU as it is explicitly
+ * marked as wc by the system, or the system is cache-coherent.
+ */
+ if (obj->stolen || obj->phys_handle)
+ return false;
+
+ /* If the GPU is snooping the contents of the CPU cache,
+ * we do not need to manually clear the CPU cache lines. However,
+ * the caches are only snooped when the render cache is
+ * flushed/invalidated. As we always have to emit invalidations
+ * and flushes when moving into and out of the RENDER domain, correct
+ * snooping behaviour occurs naturally as the result of our domain
+ * tracking.
+ */
+ return !cpu_cache_is_coherent(obj->base.dev, obj->cache_level);
+}
+
+
static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
{
if (obj->tiling_mode)
@@ -3615,29 +3644,7 @@ bool
i915_gem_clflush_object(struct drm_i915_gem_object *obj,
bool force)
{
- /* If we don't have a page list set up, then we're not pinned
- * to GPU, and we can ignore the cache flush because it'll happen
- * again at bind time.
- */
- if (obj->pages == NULL)
- return false;
-
- /*
- * Stolen memory is always coherent with the GPU as it is explicitly
- * marked as wc by the system, or the system is cache-coherent.
- */
- if (obj->stolen || obj->phys_handle)
- return false;
-
- /* If the GPU is snooping the contents of the CPU cache,
- * we do not need to manually clear the CPU cache lines. However,
- * the caches are only snooped when the render cache is
- * flushed/invalidated. As we always have to emit invalidations
- * and flushes when moving into and out of the RENDER domain, correct
- * snooping behaviour occurs naturally as the result of our domain
- * tracking.
- */
- if (!force && cpu_cache_is_coherent(obj->base.dev, obj->cache_level)) {
+ if (!force && !is_cpu_flush_required(obj)) {
obj->cache_dirty = true;
return false;
}
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/6] drm/i915: obey wbinvd threshold in more places
2015-02-09 21:54 [PATCH 0/6] Prefer wbinvd() where appropriate Ben Widawsky
` (4 preceding siblings ...)
2015-02-09 21:54 ` [PATCH 5/6] drm/i915: Extract checking the necessity of flush Ben Widawsky
@ 2015-02-09 21:54 ` Ben Widawsky
2015-02-10 9:28 ` Chris Wilson
2015-02-10 20:49 ` shuang.he
5 siblings, 2 replies; 12+ messages in thread
From: Ben Widawsky @ 2015-02-09 21:54 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++++
drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++++++++++---
3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5d2f62d..dfecdfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2818,6 +2818,10 @@ static inline bool cpu_cache_is_coherent(struct drm_device *dev,
{
return HAS_LLC(dev) || level != I915_CACHE_NONE;
}
+static inline bool i915_gem_obj_should_clflush(struct drm_i915_gem_object *obj)
+{
+ return obj->base.size >= to_i915(obj->base.dev)->wbinvd_threshold;
+}
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init(struct drm_device *dev);
int i915_gem_init_rings(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4d5a69d..59be709 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -204,6 +204,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
char *vaddr = obj->phys_handle->vaddr;
struct sg_table *st;
struct scatterlist *sg;
+ const bool do_wbinvd = i915_gem_obj_should_clflush(obj);
int i;
if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
@@ -219,12 +220,15 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
src = kmap_atomic(page);
memcpy(vaddr, src, PAGE_SIZE);
- drm_clflush_virt_range(vaddr, PAGE_SIZE);
+ if (!do_wbinvd)
+ drm_clflush_virt_range(vaddr, PAGE_SIZE);
kunmap_atomic(src);
page_cache_release(page);
vaddr += PAGE_SIZE;
}
+ if (do_wbinvd)
+ wbinvd();
i915_gem_chipset_flush(obj->base.dev);
@@ -252,6 +256,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
static void
i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj)
{
+ const bool do_wbinvd = i915_gem_obj_should_clflush(obj);
int ret;
BUG_ON(obj->madv == __I915_MADV_PURGED);
@@ -282,7 +287,8 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj)
continue;
dst = kmap_atomic(page);
- drm_clflush_virt_range(vaddr, PAGE_SIZE);
+ if (!do_wbinvd)
+ drm_clflush_virt_range(vaddr, PAGE_SIZE);
memcpy(dst, vaddr, PAGE_SIZE);
kunmap_atomic(dst);
@@ -295,6 +301,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj)
obj->dirty = 0;
}
+ if (do_wbinvd && !ret)
+ wbinvd();
+
sg_free_table(obj->pages);
kfree(obj->pages);
@@ -396,7 +405,10 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
return -EFAULT;
}
- drm_clflush_virt_range(vaddr, args->size);
+ if (args->size >= to_i915(obj->base.dev)->wbinvd_threshold)
+ wbinvd();
+ else
+ drm_clflush_virt_range(vaddr, args->size);
i915_gem_chipset_flush(dev);
return 0;
}
@@ -647,6 +659,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
int obj_do_bit17_swizzling, page_do_bit17_swizzling;
int prefaulted = 0;
int needs_clflush = 0;
+ bool do_wbinvd = false;
struct sg_page_iter sg_iter;
user_data = to_user_ptr(args->data_ptr);
@@ -658,6 +671,9 @@ i915_gem_shmem_pread(struct drm_device *dev,
if (ret)
return ret;
+ if (needs_clflush && i915_gem_obj_should_clflush(obj))
+ do_wbinvd = true;
+
offset = args->offset;
for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,
@@ -714,6 +730,9 @@ next_page:
}
out:
+ if (do_wbinvd && !ret)
+ wbinvd();
+
i915_gem_object_unpin_pages(obj);
return ret;
@@ -4061,7 +4080,12 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
/* Flush the CPU cache if it's still invalid. */
if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0) {
- i915_gem_clflush_object(obj, false);
+ struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+ if (is_cpu_flush_required(obj) &&
+ obj->base.size >= dev_priv->wbinvd_threshold)
+ wbinvd();
+ else
+ i915_gem_clflush_object(obj, false);
obj->base.read_domains |= I915_GEM_DOMAIN_CPU;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 746f77f..13cc493 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -371,6 +371,9 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
unsigned pte = start >> GEN8_PTE_SHIFT & GEN8_PTE_MASK;
struct sg_page_iter sg_iter;
+ const bool needs_flush = !HAS_LLC(ppgtt->base.dev);
+ bool do_wbinvd = needs_flush &&
+ pages->nents * PAGE_SIZE >= to_i915(vm->dev)->wbinvd_threshold;
pt_vaddr = NULL;
@@ -385,7 +388,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
cache_level, true);
if (++pte == GEN8_PTES_PER_PAGE) {
- if (!HAS_LLC(ppgtt->base.dev))
+ if (needs_flush && !do_wbinvd)
drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
kunmap_atomic(pt_vaddr);
pt_vaddr = NULL;
@@ -401,6 +404,9 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
kunmap_atomic(pt_vaddr);
}
+
+ if (do_wbinvd)
+ wbinvd();
}
static void gen8_free_page_tables(struct page **pt_pages)
@@ -660,11 +666,12 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
pd_vaddr[j] = gen8_pde_encode(ppgtt->base.dev, addr,
I915_CACHE_LLC);
}
- if (!HAS_LLC(ppgtt->base.dev))
- drm_clflush_virt_range(pd_vaddr, PAGE_SIZE);
kunmap_atomic(pd_vaddr);
}
+ if (!HAS_LLC(ppgtt->base.dev))
+ wbinvd();
+
ppgtt->switch_mm = gen8_mm_switch;
ppgtt->base.clear_range = gen8_ppgtt_clear_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
--
2.3.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] drm/i915: obey wbinvd threshold in more places
2015-02-09 21:54 ` [PATCH 6/6] drm/i915: obey wbinvd threshold in more places Ben Widawsky
@ 2015-02-10 9:28 ` Chris Wilson
2015-02-10 20:49 ` shuang.he
1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2015-02-10 9:28 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky
On Mon, Feb 09, 2015 at 01:54:19PM -0800, Ben Widawsky wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 ++++
> drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++++++++++---
> 3 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5d2f62d..dfecdfd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2818,6 +2818,10 @@ static inline bool cpu_cache_is_coherent(struct drm_device *dev,
> {
> return HAS_LLC(dev) || level != I915_CACHE_NONE;
> }
> +static inline bool i915_gem_obj_should_clflush(struct drm_i915_gem_object *obj)
> +{
> + return obj->base.size >= to_i915(obj->base.dev)->wbinvd_threshold;
> +}
if (i915_gem_obj_should_clflush(obj)) wbinvd()?
Does wbinvd always have the same characteristic threshold, even coupled
with a second access (read or write) inside the TLB flushing of
kunmap_atomic. I would imagine that these workloads are dramatically
different to the replacement in execbuffer.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] drm/i915: obey wbinvd threshold in more places
2015-02-09 21:54 ` [PATCH 6/6] drm/i915: obey wbinvd threshold in more places Ben Widawsky
2015-02-10 9:28 ` Chris Wilson
@ 2015-02-10 20:49 ` shuang.he
1 sibling, 0 replies; 12+ messages in thread
From: shuang.he @ 2015-02-10 20:49 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, benjamin.widawsky
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5739
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 282/283 282/283
ILK 271/278 271/278
SNB +2-22 340/346 320/346
IVB +1-2 378/384 377/384
BYT 296/296 296/296
HSW +4 421/428 425/428
BDW 318/333 318/333
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt_drv_suspend_debugfs-reader DMESG_WARN(1, M37) NO_RESULT(1, M37)
*ILK igt_drv_suspend_fence-restore-tiled2untiled DMESG_WARN(1, M37) NO_RESULT(1, M37)
*ILK igt_drv_suspend_fence-restore-untiled DMESG_WARN(1, M37) NO_RESULT(1, M37)
*ILK igt_drv_suspend_forcewake DMESG_WARN(1, M37) NO_RESULT(1, M37)
*ILK igt_gem_workarounds_suspend-resume DMESG_WARN(1, M37) INIT(1, M37)
SNB igt_kms_cursor_crc_cursor-size-change NSPT(1, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_kms_flip_event_leak NSPT(1, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_kms_flip_modeset-vs-vblank-race DMESG_WARN(1, M22)PASS(1, M22) PASS(1, M22)
SNB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_kms_pipe_crc_basic_read-crc-pipe-A DMESG_WARN(1, M22)PASS(6, M22) PASS(1, M22)
SNB igt_kms_rotation_crc_primary-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_kms_rotation_crc_sprite-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_cursor NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_cursor-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_dpms-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_drm-resources-equal NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_fences NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_fences-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_gem-execbuf NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_gem-mmap-cpu NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_gem-mmap-gtt NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_gem-pread NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_i2c NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_modeset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_pci-d3-state NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
SNB igt_pm_rpm_rte NSPT(2, M22)PASS(1, M22) NSPT(1, M22)
IVB igt_gem_pwrite_pread_snooped-copy-performance DMESG_WARN(1, M34)PASS(5, M34) DMESG_WARN(1, M34)
IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(2, M34)PASS(2, M34) PASS(1, M34)
IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(3, M34) DMESG_WARN(1, M34)
HSW igt_gem_storedw_loop_blt DMESG_WARN(3, M20)PASS(3, M20) PASS(1, M20)
HSW igt_gem_storedw_loop_vebox DMESG_WARN(3, M20)PASS(2, M20) PASS(1, M20)
*HSW igt_kms_flip_bo-too-big BLACKLIST(1, M20) PASS(1, M20)
*HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M20) PASS(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread