[AMD Official Use Only]
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
> Sent: Tuesday, December 14, 2021 4:00 PM
> To: Huang, Ray <Ray.Huang@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> Daniel Vetter <daniel.vetter@xxxxxxxx>; Sumit Semwal
> <sumit.semwal@xxxxxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; Deucher,
> Alexander <Alexander.Deucher@xxxxxxx>; Liu, Monk
> <Monk.Liu@xxxxxxx>
> Subject: Re: [PATCH 1/3] dma-buf: make the flags can be configured during
> dma fence init
>
> Am 13.12.21 um 07:34 schrieb Huang Rui:
> > In some user scenarios, the get_timeline_name callback uses the flags
> > to decide which way to return the timeline string name. Once the
> > trace_dma_fence_init event is enabled, it will call get_timeline_name
> > callback to dump the fence structure. However, at this moment, the
> > flags are always 0, and it might trigger some issues in
> > get_timeline_name callback implementation of different gpu driver. So
> > make a member to initialize the flags in dma_fence_init().
>
> Well that doesn't make much sense to me.
>
> None of the dma_fence callbacks is called from the dma_fence_init function
> (or at least shouldn't). So drivers always have the opportunity to to adjust
> the flags.
>
> So please explain the rational again?
Once we enable trace_dma_fence_init event, we will call get_driver_name and get_timeline_name callback function to dump the names in dma_fence_init().
At that time, the flags are always 0. However, in amdgpu_fence_get_timeline_name(), it will check the flags (AMDGPU_FENCE_FLAG_EMBED_IN_JOB_BIT) to select which way to get the ring.
If the fence should be actually embedded in the job (will be set after that), it still will trigger a kernel panic (please see patch2) because it go with a wrong way. Because we cannot set the flags at the start of dma_fence_init. That is the problem.
Thanks,
Ray
>
> Christian.
>
> >
> > Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> > ---
> > drivers/dma-buf/dma-fence.c | 2 +-
> > include/linux/dma-fence.h | 7 +++++++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 066400ed8841..3e0622bf385f 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -952,7 +952,7 @@ dma_fence_init(struct dma_fence *fence, const
> struct dma_fence_ops *ops,
> > fence->lock = lock;
> > fence->context = context;
> > fence->seqno = seqno;
> > - fence->flags = 0UL;
> > + fence->flags = ops->init_flags;
> > fence->error = 0;
> >
> > trace_dma_fence_init(fence);
> > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > index 1ea691753bd3..f9270c5bc07a 100644
> > --- a/include/linux/dma-fence.h
> > +++ b/include/linux/dma-fence.h
> > @@ -131,6 +131,13 @@ struct dma_fence_ops {
> > */
> > bool use_64bit_seqno;
> >
> > + /**
> > + * @init_flags:
> > + *
> > + * The initial value of fence flags (A mask of DMA_FENCE_FLAG_*
> defined).
> > + */
> > + unsigned long init_flags;
> > +
> > /**
> > * @get_driver_name:
> > *
