On Mon, 2023-02-27 at 18:21 -0800, Teres Alexis, Alan Previn wrote:
> Add helper functions into a new file for heci-packet-submission.
> The helpers will handle generating the MTL GSC-CS Memory-Header
> and submission of the Heci-Cmd-Packet instructions to the engine.
>
>
alan:snip
> +int
> +intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc *gsc,
> + struct intel_context *ce,
> + struct intel_gsc_heci_non_priv_pkt *pkt,
> + u32 *cmd, int timeout_ms)
> +{
> + struct intel_engine_cs *eng;
> + struct i915_request *rq;
> + int err;
> +
> + rq = intel_context_create_request(ce);
> + if (IS_ERR(rq))
> + return PTR_ERR(rq);
> +
> + emit_gsc_heci_pkt_nonpriv(cmd, pkt);
> +
> + i915_vma_lock(pkt->bb_vma);
> + err = i915_vma_move_to_active(pkt->bb_vma, rq, EXEC_OBJECT_WRITE);
> + i915_vma_unlock(pkt->bb_vma);
> + if (err)
> + goto out_rq;
> +
alan:
depending on timing (appears to be a racy trigger event), in <5% of the time when I tested this internally,
I am seeing lockdep issues when running live_selftests(gt_timelines) followed by a PXP teardown at pxp-fini.
The lockdep kernel logs point to the sequence of calling "intel_context_create_request" followed by
i915_vma_lock/move_to_active/i915_vma_unlock for the objects (and how the internal ww-locks vs timeline-locks are taken)
Internal discussions realize that i really shouldnt be using these function call sequences and should instead
follow what our workaround batch buffers do:
(the following is the current fix proposal from internal discussions, but i still need to do more testing +
debug before i re-rev but i wanted to put this review comment first so the follow-up action is not lost)
i915_gem_ww_ctx_init(&ww, false);
i915_gem_object_lock(pkt->bb_vma->obj, &ww);
intel_context_pin_ww(ce, &ww);
i915_request_create(ce);
i915_vma_move_to_active(pkt->bb_vma, rq, EXEC_OBJECT_WRITE);
submit (breadcrumbs, init-bb, flush...)
i915_request_get/add/put(rq);
intel_context_unpin(ce);
i915_gem_ww_ctx_fini(&ww);
alan:snip
