Re: [Intel-gfx] [PATCH v6 3/8] drm/i915/pxp: Add MTL helpers to submit Heci-Cmd-Packet to GSC

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

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