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

Reply via email to