On Tue, Apr 26, 2022 at 10:20 AM Christian König <christian.koenig@xxxxxxx> wrote: > > Am 26.04.22 um 19:16 schrieb Rob Clark: > > On Tue, Apr 26, 2022 at 10:08 AM Christian König > > <christian.koenig@xxxxxxx> wrote: > >> Am 26.04.22 um 19:05 schrieb Rob Clark: > >>> On Tue, Apr 26, 2022 at 9:42 AM Christian König > >>> <christian.koenig@xxxxxxx> wrote: > >>>> Am 26.04.22 um 18:32 schrieb Chia-I Wu: > >>>>> On Tue, Apr 12, 2022 at 2:26 PM Chia-I Wu <olvaffe@xxxxxxxxx> wrote: > >>>>>> In practice, trace_dma_fence_init called from dma_fence_init is good > >>>>>> enough and almost no driver calls trace_dma_fence_emit. But drm_sched > >>>>>> and virtio both have cases where trace_dma_fence_init and > >>>>>> trace_dma_fence_emit can be apart. It is easier for visualization tools > >>>>>> to always use the more correct trace_dma_fence_emit when visualizing > >>>>>> fence timelines. > >>>>>> > >>>>>> v2: improve commit message (Dmitry) > >>>>>> > >>>>>> Signed-off-by: Chia-I Wu <olvaffe@xxxxxxxxx> > >>>>>> Cc: Rob Clark <robdclark@xxxxxxxxxxxx> > >>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>>>> This has been reviewed. Should we land it? > >>>> No, there are still open discussions about it. > >>> I think if it is needed for trace visualization, that is justification > >>> enough for me > >>> > >>> I don't really see otherwise how a generic trace visualization tool > >>> like perfetto would handle the case that some fence timelines have > >>> separate events but others do not. > >> Well I just send a patch to completely remove the trace point. > >> > >> As I said it absolutely doesn't make sense to use this for > >> visualization, that's what the trace_dma_fence_init trace point is good for. I am a bit confused by this. _emit and _signaled are a great way to see how many fences are pending from cpu's point of view. How does _emit make no sense and _init is good instead? Or is this just that _init is good enough most of the time? (More below) > >> > >> The only use case is for debugging the GPU scheduler and we should > >> probably introduce a separate GPU scheduler specific trace point for > >> this instead if we should ever need it. > > Hmm, AFAIU from Chia-I, virtgpu has a separation of init and emit.. > > OTOH if using separate events in these special cases is better, then > > I'm ok with that and can revert this patch. Chia-I is more familiar > > with the visualization end of it, so I'll let him comment on whether > > that is a workable approach. > > Interesting, I wasn't aware of the virtgpu separation of init and emit. > > But yes if there is really an use case for tracing this time stamp as > well then we should probably have that use case specific. > > I just looked into the scheduler case a bit and found that even there we > already have a different trace point for it, which is probably the > reason why we never used trace_dma_fence_emit there. Yeah, I am using drm_sched tracepoints in that case. > > So yes, there really isn't much reason I can see two have two separate > trace points for every driver. That sounds fair. In any tool, it should be easy to see if a fence timeline has _emit in addition to _init, and adapt accordingly. We can drop this patch. A clarification that _emit is optional/redundant for most fence timelines should be nicer than removing it though. > > Christian. > > > > > BR, > > -R > > > >> Regards, > >> Christian. > >> > >>> BR, > >>> -R > >>> > >>>> Regards, > >>>> Christian. > >>>> > >>>>> (Or, how do I check if it has landed?) >
