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@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Alex Dai <yu.dai@xxxxxxxxx>
---
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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
