Re: [PATCH 3/7] drm/xe/oa: Add input fence dependencies

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 18 Sep 2024 04:59:59 -0700, Souza, Jose wrote:
>

Hi Jose,

> On Fri, 2024-08-30 at 15:16 -0700, Ashutosh Dixit wrote:
> > Add input fence dependencies which will make OA configuration wait till
> > these dependencies are met (till input fences signal).
> >
> > v2: Change add_deps arg to xe_oa_submit_bb from bool to enum (Matt Brost)
> >
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_oa.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> > index 8ada74da3da19..b4b68019d35b7 100644
> > --- a/drivers/gpu/drm/xe/xe_oa.c
> > +++ b/drivers/gpu/drm/xe/xe_oa.c
> > @@ -42,6 +42,11 @@
> >  #define DEFAULT_POLL_PERIOD_NS (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY_HZ)
> >  #define XE_OA_UNIT_INVALID U32_MAX
> >
> > +enum xe_oa_submit_deps {
> > +	XE_OA_SUBMIT_NO_DEPS,
> > +	XE_OA_SUBMIT_ADD_DEPS,
> > +};
> > +
> >  struct xe_oa_reg {
> >	struct xe_reg addr;
> >	u32 value;
> > @@ -568,7 +573,8 @@ static __poll_t xe_oa_poll(struct file *file, poll_table *wait)
> >	return ret;
> >  }
> >
> > -static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_bb *bb)
> > +static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, enum xe_oa_submit_deps deps,
> > +					 struct xe_bb *bb)
> >  {
> >	struct xe_sched_job *job;
> >	struct dma_fence *fence;
> > @@ -581,11 +587,22 @@ static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_
> >		goto exit;
> >	}
> >
> > +	if (deps == XE_OA_SUBMIT_ADD_DEPS) {
> > +		for (int i = 0; i < stream->num_syncs && !err; i++)
> > +			err = xe_sync_entry_add_deps(&stream->syncs[i], job);
>
> err needs to be initialize to 0 otherwise it could take the error path
> below when there is no syncs set.

Somehow I missed this because I never see the drm_dbg print below. Anyway,
I have added the initialization, though the change is in Patch 1 of the
series since the initialization was there and it was removed in Patch 1.

Thanks for catching this,
Ashutosh

>
> > +		if (err) {
> > +			drm_dbg(&stream->oa->xe->drm, "xe_sync_entry_add_deps err %d\n", err);
> > +			goto err_put_job;
> > +		}
> > +	}
> > +
> >	xe_sched_job_arm(job);
> >	fence = dma_fence_get(&job->drm.s_fence->finished);
> >	xe_sched_job_push(job);
> >
> >	return fence;
> > +err_put_job:
> > +	xe_sched_job_put(job);
> >  exit:
> >	return ERR_PTR(err);
> >  }
> > @@ -663,7 +680,7 @@ static int xe_oa_modify_ctx_image(struct xe_oa_stream *stream, struct xe_lrc *lr
> >
> >	xe_oa_store_flex(stream, lrc, bb, flex, count);
> >
> > -	fence = xe_oa_submit_bb(stream, bb);
> > +	fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_NO_DEPS, bb);
> >	if (IS_ERR(fence)) {
> >		err = PTR_ERR(fence);
> >		goto free_bb;
> > @@ -692,7 +709,7 @@ static int xe_oa_load_with_lri(struct xe_oa_stream *stream, struct xe_oa_reg *re
> >
> >	write_cs_mi_lri(bb, reg_lri, 1);
> >
> > -	fence = xe_oa_submit_bb(stream, bb);
> > +	fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_NO_DEPS, bb);
> >	if (IS_ERR(fence)) {
> >		err = PTR_ERR(fence);
> >		goto free_bb;
> > @@ -943,7 +960,7 @@ static int xe_oa_emit_oa_config(struct xe_oa_stream *stream, struct xe_oa_config
> >		goto exit;
> >	}
> >
> > -	fence = xe_oa_submit_bb(stream, oa_bo->bb);
> > +	fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_ADD_DEPS, oa_bo->bb);
> >	if (IS_ERR(fence)) {
> >		err = PTR_ERR(fence);
> >		goto exit;
>