[PATCH v2] drm/msm: add trace_dma_fence_emit to msm_gpu_submit

[Christian König]

Am 26.04.22 um 19:16 schrieb Rob Clark:
> On Tue, Apr 26, 2022 at 10:08 AM Christian König
> <christian.koenig at amd.com> 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 at amd.com> 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 at gmail.com> 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 at gmail.com>
>>>>>> Cc: Rob Clark <robdclark at chromium.org>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>>> 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.
>>
>> 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.

So yes, there really isn't much reason I can see two have two separate 
trace points for every driver.

Christian.

>
> BR,
> -R
>
>> Regards,
>> Christian.
>>
>>> BR,
>>> -R
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> (Or, how do I check if it has landed?)