diff mbox

[2/2] drm/i915: a couple more uses for i915_gem_object_mark_dirty()

Message ID 1461860781-4504-2-git-send-email-david.s.gordon@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Gordon April 28, 2016, 4:26 p.m. UTC
intel_pin_and_map_ringbuffer_obj() should have been setting the dirty
flag, but wasn't; and the LRC code in intel_lr_context_do_pin() is also
updated to use the new function.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 10 +++++-----
 drivers/gpu/drm/i915/intel_ringbuffer.c |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Chris Wilson April 28, 2016, 4:36 p.m. UTC | #1
On Thu, Apr 28, 2016 at 05:26:21PM +0100, Dave Gordon wrote:
> intel_pin_and_map_ringbuffer_obj() should have been setting the dirty
> flag, but wasn't; and the LRC code in intel_lr_context_do_pin() is also
> updated to use the new function.

Technically it doesn't need to. Whilst there is any valid content in the
ringbuffer, it is pinned for use by requests and the hadware. Unlike the
context object whose content does need to persist when inactive.
-Chris
Dave Gordon April 28, 2016, 5:20 p.m. UTC | #2
On 28/04/16 17:36, Chris Wilson wrote:
> On Thu, Apr 28, 2016 at 05:26:21PM +0100, Dave Gordon wrote:
>> intel_pin_and_map_ringbuffer_obj() should have been setting the dirty
>> flag, but wasn't; and the LRC code in intel_lr_context_do_pin() is also
>> updated to use the new function.
>
> Technically it doesn't need to. Whilst there is any valid content in the
> ringbuffer, it is pinned for use by requests and the hadware. Unlike the
> context object whose content does need to persist when inactive.
> -Chris

What about across suspend and/or hibernate? Any chance of there being 
valid (new) content in a ringbuffer that gets thrown away? Though 
presumably we insist that all pending work is completed before allowing 
hibernation/suspend anyway?

.Dave.
Chris Wilson April 28, 2016, 5:27 p.m. UTC | #3
On Thu, Apr 28, 2016 at 06:20:30PM +0100, Dave Gordon wrote:
> On 28/04/16 17:36, Chris Wilson wrote:
> >On Thu, Apr 28, 2016 at 05:26:21PM +0100, Dave Gordon wrote:
> >>intel_pin_and_map_ringbuffer_obj() should have been setting the dirty
> >>flag, but wasn't; and the LRC code in intel_lr_context_do_pin() is also
> >>updated to use the new function.
> >
> >Technically it doesn't need to. Whilst there is any valid content in the
> >ringbuffer, it is pinned for use by requests and the hadware. Unlike the
> >context object whose content does need to persist when inactive.
> >-Chris
> 
> What about across suspend and/or hibernate? Any chance of there
> being valid (new) content in a ringbuffer that gets thrown away?
> Though presumably we insist that all pending work is completed
> before allowing hibernation/suspend anyway?

Correct, before suspend we wait for all GPU work to be completed. We
don't want the GPU writing into memory after the suspend/hibernation
thinks we are ready. The pages are pinned by our activity as well.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 15d341e..707aec0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1098,17 +1098,17 @@  static int intel_lr_context_do_pin(struct intel_context *ctx,
 		goto unpin_ctx_obj;
 	}
 
-	lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
-
 	ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf);
 	if (ret)
 		goto unpin_map;
 
-	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
-	intel_lr_context_descriptor_update(ctx, engine);
+	i915_gem_object_mark_dirty(ctx_obj);
+
+	lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
 	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
 	ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
-	ctx_obj->dirty = true;
+	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
+	intel_lr_context_descriptor_update(ctx, engine);
 
 	/* Invalidate GuC TLB. */
 	if (i915.enable_guc_submission)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 66f69cd..22a703e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2145,6 +2145,7 @@  int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 
 	ringbuf->virtual_start = addr;
 	ringbuf->vma = i915_gem_obj_to_ggtt(obj);
+	i915_gem_object_mark_dirty(obj);
 	return 0;
 
 err_unpin: