[PATCH 2/2] drm/amdgpu: add amdgpu runpm usage trace for separate funcs

[Liang, Prike]

[Public]

> -----Original Message-----
> From: Deucher, Alexander <Alexander.Deucher at amd.com>
> Sent: Friday, November 10, 2023 5:49 AM
> To: Liang, Prike <Prike.Liang at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: RE: [PATCH 2/2] drm/amdgpu: add amdgpu runpm usage trace for
> separate funcs
>
> [Public]
>
> > -----Original Message-----
> > From: Liang, Prike <Prike.Liang at amd.com>
> > Sent: Thursday, November 9, 2023 2:37 AM
> > To: amd-gfx at lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Liang, Prike
> > <Prike.Liang at amd.com>
> > Subject: [PATCH 2/2] drm/amdgpu: add amdgpu runpm usage trace for
> > separate funcs
> >
> > Add trace for amdgpu runpm separate funcs usage and this will help
> > debugging on the case of runpm usage missed to dereference.
> > In the normal case the runpm usage count referred by one kind of
> > functionality pairwise and usage should be changed from 1 to 0,
> > otherwise there will be an issue in the amdgpu runpm usage dereference.
> >
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
>
> Looks good.  Not sure if you want to add tracepoints to the other call sites as
> well.  These are probably the trickiest however.
>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>
[Prike] Thanks for the review, now the trace points only added to the amdgpu functions which are referencing and dereferencing amdgpu runpm usage separately and from the checking that seems no more separate functions need the trace point at the moment.

> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |  4 ++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c   |  7 ++++++-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h   | 15 +++++++++++++++
> >  3 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > index e7e87a3b2601..decbbe3d4f06 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > @@ -42,6 +42,7 @@
> >  #include <linux/dma-fence-array.h>
> >  #include <linux/pci-p2pdma.h>
> >  #include <linux/pm_runtime.h>
> > +#include "amdgpu_trace.h"
> >
> >  /**
> >   * amdgpu_dma_buf_attach - &dma_buf_ops.attach implementation @@
> -
> > 63,6 +64,7 @@ static int amdgpu_dma_buf_attach(struct dma_buf
> *dmabuf,
> >               attach->peer2peer = false;
> >
> >       r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> > +     trace_amdgpu_runpm_reference_dumps(1, __func__);
> >       if (r < 0)
> >               goto out;
> >
> > @@ -70,6 +72,7 @@ static int amdgpu_dma_buf_attach(struct dma_buf
> > *dmabuf,
> >
> >  out:
> >       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > +     trace_amdgpu_runpm_reference_dumps(0, __func__);
> >       return r;
> >  }
> >
> > @@ -90,6 +93,7 @@ static void amdgpu_dma_buf_detach(struct dma_buf
> > *dmabuf,
> >
> >       pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> >       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > +     trace_amdgpu_runpm_reference_dumps(0, __func__);
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > index 709a2c1b9d63..1026a9fa0c0f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > @@ -183,6 +183,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring,
> > struct dma_fence **f, struct amd
> >       amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
> >                              seq, flags | AMDGPU_FENCE_FLAG_INT);
> >       pm_runtime_get_noresume(adev_to_drm(adev)->dev);
> > +     trace_amdgpu_runpm_reference_dumps(1, __func__);
> >       ptr = &ring->fence_drv.fences[seq & ring-
> > >fence_drv.num_fences_mask];
> >       if (unlikely(rcu_dereference_protected(*ptr, 1))) {
> >               struct dma_fence *old;
> > @@ -286,8 +287,11 @@ bool amdgpu_fence_process(struct amdgpu_ring
> > *ring)
> >           seq != ring->fence_drv.sync_seq)
> >               amdgpu_fence_schedule_fallback(ring);
> >
> > -     if (unlikely(seq == last_seq))
> > +     if (unlikely(seq == last_seq)) {
> > +             pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > +             trace_amdgpu_runpm_reference_dumps(0, __func__);
> >               return false;
> > +     }
> >
> >       last_seq &= drv->num_fences_mask;
> >       seq &= drv->num_fences_mask;
> > @@ -310,6 +314,7 @@ bool amdgpu_fence_process(struct amdgpu_ring
> > *ring)
> >               dma_fence_put(fence);
> >               pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> >               pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > +             trace_amdgpu_runpm_reference_dumps(0, __func__);
> >       } while (last_seq != seq);
> >
> >       return true;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > index 2fd1bfb35916..5d4792645540 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > @@ -554,6 +554,21 @@ TRACE_EVENT(amdgpu_reset_reg_dumps,
> >                     __entry->value)
> >  );
> >
> > +TRACE_EVENT(amdgpu_runpm_reference_dumps,
> > +         TP_PROTO(uint32_t index, const char *func),
> > +         TP_ARGS(index, func),
> > +         TP_STRUCT__entry(
> > +                          __field(uint32_t, index)
> > +                          __string(func, func)
> > +                          ),
> > +         TP_fast_assign(
> > +                        __entry->index = index;
> > +                        __assign_str(func, func);
> > +                        ),
> > +         TP_printk("amdgpu runpm reference dump 0x%d: 0x%s\n",
> > +                   __entry->index,
> > +                   __get_str(func))
> > +);
> >  #undef AMDGPU_JOB_GET_TIMELINE_NAME
> >  #endif
> >
> > --
> > 2.34.1
>