On Fri, Jan 22, 2016 at 11:10:10PM +0000, Dave Gordon wrote:
> In LRC mode, the HWSP is part of the default context object, and
> therefore does not exist independently. Worse, it doesn't contribute
> to the refcount on the default context object either.
>
> Currently, the default context is deallocated in intel_lr_context_free(),
> but the HWSP kmapping is not torn down until the subsequent call to
> intel_logical_ring_cleanup(). Between these calls, ring->status_page.obj
> continues to point to the (now non-existent) default context object,
> and the kernel mapping likewise points to a page which is now free.
>
> The solution is to dispose of the HWSP kmapping and pointer before the
> object itself is freed, so this patch moves the kmap teardown code from
> intel_lr_context_free() to intel_logical_ring_cleanup().
>
> This code was introduced in
>
> 48d8238 drm/i915/bdw: Generic logical ring init and cleanup
>
> i.e. it has been there ever since LRC mode was first implemented.
>
> v2:
> Split from "handle teardown of HWSP when context is freed".
>
> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index a3ab2b4..c702fc2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1986,13 +1986,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
> i915_cmd_parser_fini_ring(ring);
> i915_gem_batch_pool_fini(&ring->batch_pool);
>
> - if (ring->status_page.obj) {
> - struct page *page;
> -
> - page = i915_gem_object_get_page(ring->status_page.obj, LRC_PPHWSP_PN);
> - kunmap(page);
> - ring->status_page.obj = NULL;
> - }
> + /* Status page should have gone already */
> + WARN_ON(ring->status_page.page_addr);
> + WARN_ON(ring->status_page.obj);
>
> ring->disable_lite_restore_wa = false;
> ring->ctx_desc_template = 0;
> @@ -2431,6 +2427,22 @@ void intel_lr_context_free(struct intel_context *ctx)
> continue;
>
> if (ctx == ctx->i915->kernel_context) {
> + struct intel_engine_cs *ring = ringbuf->ring;
> +
> + /*
> + * The HWSP is part of the kernel context
> + * object in LRC mode, so tear it down now.
> + */
> + if (ring->status_page.obj) {
> + struct page *page;
> +
> + page = i915_gem_object_get_page(
> + ring->status_page.obj,
> + LRC_PPHWSP_PN);
> + kunmap(page);
> + ring->status_page.obj = NULL;
> + }
No. I see no reason why we need to add special code to the context-free.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
