drm/i915/gem: Clear read/write domains for GPU clear

Submitted by Chris Wilson on June 24, 2019, 2:16 p.m.

Details

Message ID 20190624141630.11015-1-chris@chris-wilson.co.uk
State Accepted
Commit 871918dffefc594e765cc7e885a36a7fd3f38da7
Headers show
Series "drm/i915/gem: Clear read/write domains for GPU clear" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Chris Wilson June 24, 2019, 2:16 p.m.
Update the domains for the write via the GPU so that we do not
shortcircuit any set-domain wait afterwards.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110978
Fixes: b2dbf8d982a4 ("drm/i915/blt: Remove recursive vma->lock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_client_blt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
index 9b01c3b5b31d..6f537e8e4dea 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
@@ -162,11 +162,12 @@  static void clear_pages_worker(struct work_struct *work)
 		goto out_signal;
 
 	if (obj->cache_dirty) {
-		obj->write_domain = 0;
 		if (i915_gem_object_has_struct_page(obj))
 			drm_clflush_sg(w->sleeve->pages);
 		obj->cache_dirty = false;
 	}
+	obj->read_domains = I915_GEM_GPU_DOMAINS;
+	obj->write_domain = 0;
 
 	/* XXX: we need to kill this */
 	mutex_lock(&i915->drm.struct_mutex);

Comments

On 24/06/2019 15:16, Chris Wilson wrote:
> Update the domains for the write via the GPU so that we do not
> shortcircuit any set-domain wait afterwards.

I'm lost. How do we short-circuit the set-domain wait?

> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110978
> Fixes: b2dbf8d982a4 ("drm/i915/blt: Remove recursive vma->lock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_client_blt.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> index 9b01c3b5b31d..6f537e8e4dea 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> @@ -162,11 +162,12 @@ static void clear_pages_worker(struct work_struct *work)
>   		goto out_signal;
>   
>   	if (obj->cache_dirty) {
> -		obj->write_domain = 0;
>   		if (i915_gem_object_has_struct_page(obj))
>   			drm_clflush_sg(w->sleeve->pages);
>   		obj->cache_dirty = false;
>   	}
> +	obj->read_domains = I915_GEM_GPU_DOMAINS;
> +	obj->write_domain = 0;
>   
>   	/* XXX: we need to kill this */
>   	mutex_lock(&i915->drm.struct_mutex);
>
Quoting Matthew Auld (2019-06-24 16:50:48)
> On 24/06/2019 15:16, Chris Wilson wrote:
> > Update the domains for the write via the GPU so that we do not
> > shortcircuit any set-domain wait afterwards.
> 
> I'm lost. How do we short-circuit the set-domain wait?

If we never change the read_domain, then we may skip the clflush on
set_cpu_domain thus missing the updated contents on !llc.

So s/wait/clflush/ to be precise
-Chris
On 24/06/2019 17:00, Chris Wilson wrote:
> Quoting Matthew Auld (2019-06-24 16:50:48)
>> On 24/06/2019 15:16, Chris Wilson wrote:
>>> Update the domains for the write via the GPU so that we do not
>>> shortcircuit any set-domain wait afterwards.
>>
>> I'm lost. How do we short-circuit the set-domain wait?
> 
> If we never change the read_domain, then we may skip the clflush on
> set_cpu_domain thus missing the updated contents on !llc.
> 
> So s/wait/clflush/ to be precise

Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> -Chris
>