On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> - if (!i915_gem_obj_ggtt_bound(ctx_obj))
> - seq_puts(m, "\tNot bound in GGTT\n");
> - else
> - ggtt_offset = i915_gem_obj_ggtt_offset(ctx_obj);
> + if (vma->flags & I915_VMA_GLOBAL_BIND)
> + seq_printf(m, "\tBound in GGTT at 0x%x\n",
0x%04x?
> + lower_32_bits(vma->node.start));
>
> - if (i915_gem_object_get_pages(ctx_obj)) {
> - seq_puts(m, "\tFailed to get pages for context object\n");
> + if (i915_gem_object_get_pages(vma->obj)) {
> + seq_puts(m, "\tFailed to get pages for context object\n\n");
> return;
> }
>
> - page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
> - if (!WARN_ON(page == NULL)) {
> - reg_state = kmap_atomic(page);
> + page = i915_gem_object_get_page(vma->obj, LRC_STATE_PN);
> + if (page) {
Dropped the WARN_ON? No mention in the commit message.
> @@ -620,9 +631,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32
> hw_flags)
>
> intel_ring_emit(ring, MI_NOOP);
> intel_ring_emit(ring, MI_SET_CONTEXT);
> - intel_ring_emit(ring,
> - i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
> - flags);
> + intel_ring_emit(ring, req->ctx->engine[RCS].state->node.start | flags);
Do we somewhere make sure flags do not collide with address? Not
related to this patch, though.
> @@ -778,16 +788,12 @@ static int do_rcs_switch(struct drm_i915_gem_request
> *req)
> from = engine->last_context;
>
> /*
> - * Clear this page out of any CPU caches for coherent swap-in/out. Note
> - * that thanks to write = false in this call and us not setting any gpu
> - * write domains when putting a context object onto the active list
> - * (when switching away from it), this won't block.
> - *
> - * XXX: We need a real interface to do this instead of trickery.
What is changed to make this comment obsolete or should have it been
removed earlier?
>
> return 0;
>
> -unpin_out:
> - i915_gem_object_ggtt_unpin(to->engine[RCS].state);
> +unpin_vma:
sole error path; "err"
> + i915_vma_unpin(to->engine[RCS].state);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c621fa23cd28..21a4d0220c17 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1077,9 +1077,10 @@ static void i915_gem_record_rings(struct
> drm_i915_private *dev_priv,
> i915_error_ggtt_object_create(dev_priv,
>
> engine->scratch.obj);
>
> - ee->ctx =
> - i915_error_ggtt_object_create(dev_priv,
> -
> request->ctx->engine[i].state);
> + if (request->ctx->engine[i].state) {
> + ee->ctx =
> i915_error_ggtt_object_create(dev_priv,
> +
> request->ctx->engine[i].state->obj);
> + }
Why conditional now?
>
> i915_gem_context_get(ctx);
> return 0;
>
> unpin_map:
> - i915_gem_object_unpin_map(ce->state);
> -unpin_ctx_obj:
> - i915_gem_object_ggtt_unpin(ce->state);
> + i915_gem_object_unpin_map(ce->state->obj);
> +unpin_vma:
> + __i915_vma_unpin(ce->state);
err_vma while at it?
> @@ -2161,7 +2162,7 @@ static int execlists_context_deferred_alloc(struct
> i915_gem_context *ctx,
> }
>
> ce->ring = ring;
> - ce->state = ctx_obj;
> + ce->state = vma;
Maybe the member name could be just ce->vma too?
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
