[Intel-gfx] [PATCH] Always mark GEM objects as dirty when written by the CPU
Dave Gordon
david.s.gordon at intel.com
Mon Dec 7 04:51:49 PST 2015
On 01/12/15 12:42, Dave Gordon wrote:
> In various places, one or more pages of a GEM object are mapped into CPU
> address space and updated. In each such case, the object should be
> marked dirty, to ensure that the modifications are not discarded if the
> object is evicted under memory pressure.
>
> This is similar to commit
> commit 51bc140431e233284660b1d22c47dec9ecdb521e
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date: Mon Aug 31 15:10:39 2015 +0100
> drm/i915: Always mark the object as dirty when used by the GPU
>
> in which Chris ensured that updates by the GPU were not lost due to
> eviction, but this patch applies instead to the multiple places where
> object content is updated by the host CPU.
>
> It also incorporates and supercedes Alex Dai's earlier patch
> [PATCH v1] drm/i915/guc: Fix a fw content lost issue after it is evicted
>
> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Alex Dai <yu.dai at intel.com>
> ---
> drivers/gpu/drm/i915/i915_cmd_parser.c | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 1 +
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_render_state.c | 1 +
> drivers/gpu/drm/i915/i915_guc_submission.c | 1 +
> drivers/gpu/drm/i915/intel_lrc.c | 6 +++++-
> 7 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 814d894..292bd5d 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -945,6 +945,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
> drm_clflush_virt_range(src, batch_len);
>
> memcpy(dst, src, batch_len);
> + dest_obj->dirty = 1;
>
> unmap_src:
> vunmap(src_base);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 33adc8f..76bacba 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5201,6 +5201,7 @@ i915_gem_object_create_from_data(struct drm_device *dev,
> i915_gem_object_pin_pages(obj);
> sg = obj->pages;
> bytes = sg_copy_from_buffer(sg->sgl, sg->nents, (void *)data, size);
> + obj->dirty = 1;
> i915_gem_object_unpin_pages(obj);
>
> if (WARN_ON(bytes != size)) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e9c2bfd..49a74c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -208,6 +208,8 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
> return ret;
>
> ret = i915_gem_object_set_to_cpu_domain(obj, write);
> + if (write)
> + obj->dirty = 1;
> mutex_unlock(&dev->struct_mutex);
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index a4c243c..bc28a10 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -281,6 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
> }
>
> kunmap_atomic(vaddr);
> + obj->dirty = 1;
>
> return 0;
> }
> @@ -372,6 +373,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
> }
>
> kunmap_atomic(vaddr);
> + obj->dirty = 1;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index 5026a62..dd1976c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -144,6 +144,7 @@ static int render_state_setup(struct render_state *so)
> so->aux_batch_size = ALIGN(so->aux_batch_size, 8);
>
> kunmap(page);
> + so->obj->dirty = 1;
>
> ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a057cbd..b4a99a2 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -583,6 +583,7 @@ static void lr_context_update(struct drm_i915_gem_request *rq)
> reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(rb_obj);
>
> kunmap_atomic(reg_state);
> + ctx_obj->dirty = 1;
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4ebafab..bc77794 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -391,6 +391,7 @@ static int execlists_update_context(struct drm_i915_gem_request *rq)
> }
>
> kunmap_atomic(reg_state);
> + ctx_obj->dirty = 1;
>
> return 0;
> }
> @@ -1030,7 +1031,7 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
> if (ret)
> goto unpin_ctx_obj;
>
> - ctx_obj->dirty = true;
> + ctx_obj->dirty = 1;
>
> /* Invalidate GuC TLB. */
> if (i915.enable_guc_submission)
> @@ -1461,6 +1462,8 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
>
> out:
> kunmap_atomic(batch);
> + wa_ctx->obj->dirty = 1;
> +
> if (ret)
> lrc_destroy_wa_ctx_obj(ring);
>
> @@ -2536,6 +2539,7 @@ void intel_lr_context_reset(struct drm_device *dev,
> reg_state[CTX_RING_TAIL+1] = 0;
>
> kunmap_atomic(reg_state);
> + ctx_obj->dirty = 1;
>
> ringbuf->head = 0;
> ringbuf->tail = 0;
>
I think I missed i915_gem_phys_pwrite().
i915_gem_gtt_pwrite_fast() marks the object dirty for most cases (vit
set_to_gtt_domain(), but isn't called for all cases (or can return
before the set_domain). Then we try i915_gem_shmem_pwrite() for non-phys
objects (no check for stolen!) and that already marks the object dirty
[aside: we might be able to change that to page-by-page?], but
i915_gem_phys_pwrite() doesn't mark the object dirty, so we might lose
updates there?
Or maybe we should move the marking up into i915_gem_pwrite_ioctl()
instead. The target object is surely going to be dirtied, whatever type
it is.
.Dave.
More information about the Intel-gfx
mailing list