mbox series

[0/3] drm/msm: More GPU tracepoints

Message ID 20200901154200.2451899-1-robdclark@gmail.com
Headers show
Series drm/msm: More GPU tracepoints | expand

Message

Rob Clark Sept. 1, 2020, 3:41 p.m. UTC
From: Rob Clark <robdclark@chromium.org>

Various extra tracepoints that I've been collecting.

Rob Clark (3):
  drm/msm/gpu: Add GPU freq_change traces
  drm/msm: Convert shrinker msgs to tracepoints
  drm/msm/gpu: Add suspend/resume tracepoints

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c  |  3 +
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c  |  4 ++
 drivers/gpu/drm/msm/msm_gem_shrinker.c |  5 +-
 drivers/gpu/drm/msm/msm_gpu.c          |  4 ++
 drivers/gpu/drm/msm/msm_gpu_trace.h    | 83 ++++++++++++++++++++++++++
 5 files changed, 97 insertions(+), 2 deletions(-)

Comments

Rob Clark Sept. 2, 2020, 2:45 p.m. UTC | #1
The cat is somewhat out of the bag already.. so I took the approach of
making the more useful of the traces for visualization (freq_change
trace) identical to the i915 one in units and format, so userspace
just has to add another event name to a list, and not have to add more
parsing code.

But the bigger problem is that it doesn't seem possible to #include
multiple foo_trace.h's in a single C file, so I'm not seeing how it is
possible to have both generic and driver specific traces.

BR,
-R

On Tue, Sep 1, 2020 at 11:52 PM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>
> Hi Rob,
>
> Do you think we could make all these generic? Visualization tools will need to do some processing so these can be neatly presented and it could be far more convenient if people wouldn't need to add code for each GPU driver.
>
> Maybe we could put all these tracepoints in DRM core as they seem useful to all drivers?
>
> Thanks,
>
> Tomeu
>
> On Tue, 1 Sep 2020 at 17:41, Rob Clark <robdclark@gmail.com> wrote:
>>
>> From: Rob Clark <robdclark@chromium.org>
>>
>> Various extra tracepoints that I've been collecting.
>>
>> Rob Clark (3):
>>   drm/msm/gpu: Add GPU freq_change traces
>>   drm/msm: Convert shrinker msgs to tracepoints
>>   drm/msm/gpu: Add suspend/resume tracepoints
>>
>>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c  |  3 +
>>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c  |  4 ++
>>  drivers/gpu/drm/msm/msm_gem_shrinker.c |  5 +-
>>  drivers/gpu/drm/msm/msm_gpu.c          |  4 ++
>>  drivers/gpu/drm/msm/msm_gpu_trace.h    | 83 ++++++++++++++++++++++++++
>>  5 files changed, 97 insertions(+), 2 deletions(-)
>>
>> --
>> 2.26.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jordan Crouse Sept. 2, 2020, 8:40 p.m. UTC | #2
On Tue, Sep 01, 2020 at 08:41:54AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Technically the GMU specific one is a bit redundant, but it was useful
> to track down a bug.

Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c |  3 +++
>  drivers/gpu/drm/msm/msm_gpu.c         |  2 ++
>  drivers/gpu/drm/msm/msm_gpu_trace.h   | 31 +++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 46a29e383bfd..ab1e9eb619e0 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -11,6 +11,7 @@
>  #include "a6xx_gpu.h"
>  #include "a6xx_gmu.xml.h"
>  #include "msm_gem.h"
> +#include "msm_gpu_trace.h"
>  #include "msm_mmu.h"
>  
>  static void a6xx_gmu_fault(struct a6xx_gmu *gmu)
> @@ -124,6 +125,8 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
>  	gmu->current_perf_index = perf_index;
>  	gmu->freq = gmu->gpu_freqs[perf_index];
>  
> +	trace_msm_gmu_freq_change(gmu->freq, perf_index);
> +
>  	/*
>  	 * This can get called from devfreq while the hardware is idle. Don't
>  	 * bring up the power if it isn't already active
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index d5645472b25d..b02866527386 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -32,6 +32,8 @@ static int msm_devfreq_target(struct device *dev, unsigned long *freq,
>  	if (IS_ERR(opp))
>  		return PTR_ERR(opp);
>  
> +	trace_msm_gpu_freq_change(dev_pm_opp_get_freq(opp));
> +
>  	if (gpu->funcs->gpu_set_freq)
>  		gpu->funcs->gpu_set_freq(gpu, opp);
>  	else
> diff --git a/drivers/gpu/drm/msm/msm_gpu_trace.h b/drivers/gpu/drm/msm/msm_gpu_trace.h
> index 122b84789238..07572ab179fa 100644
> --- a/drivers/gpu/drm/msm/msm_gpu_trace.h
> +++ b/drivers/gpu/drm/msm/msm_gpu_trace.h
> @@ -83,6 +83,37 @@ TRACE_EVENT(msm_gpu_submit_retired,
>  		    __entry->start_ticks, __entry->end_ticks)
>  );
>  
> +
> +TRACE_EVENT(msm_gpu_freq_change,
> +		TP_PROTO(u32 freq),
> +		TP_ARGS(freq),
> +		TP_STRUCT__entry(
> +			__field(u32, freq)
> +			),
> +		TP_fast_assign(
> +			/* trace freq in MHz to match intel_gpu_freq_change, to make life easier
> +			 * for userspace
> +			 */
> +			__entry->freq = DIV_ROUND_UP(freq, 1000000);
> +			),
> +		TP_printk("new_freq=%u", __entry->freq)
> +);
> +
> +
> +TRACE_EVENT(msm_gmu_freq_change,
> +		TP_PROTO(u32 freq, u32 perf_index),
> +		TP_ARGS(freq, perf_index),
> +		TP_STRUCT__entry(
> +			__field(u32, freq)
> +			__field(u32, perf_index)
> +			),
> +		TP_fast_assign(
> +			__entry->freq = freq;
> +			__entry->perf_index = perf_index;
> +			),
> +		TP_printk("freq=%u, perf_index=%u", __entry->freq, __entry->perf_index)
> +);
> +
>  #endif
>  
>  #undef TRACE_INCLUDE_PATH
> -- 
> 2.26.2
>