Re: [PATCH 3/3] drm/msm: Expose client engine utilization via fdinfo

2022-03-01 Thread Tvrtko Ursulin



On 28/02/2022 16:01, Rob Clark wrote:

On Mon, Feb 28, 2022 at 6:33 AM Tvrtko Ursulin
 wrote:



On 25/02/2022 22:14, Rob Clark wrote:

On Fri, Feb 25, 2022 at 12:25 PM Rob Clark  wrote:


From: Rob Clark 

Similar to AMD commit
874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
infrastructure added in previous patches, we add basic client info
and GPU engine utilisation for msm.

Example output:

  # cat /proc/`pgrep glmark2`/fdinfo/6
  pos:0
  flags:  0242
  mnt_id: 21
  ino:162
  drm-driver: msm
  drm-client-id:  7
  drm-engine-gpu: 1734371319 ns
  drm-cycles-gpu: 1153645024


Nice, so my vendor agnostic actually worked (with that single fixup of
accounting for the fact pdev tag is optional)?


Note that it might be useful to have a standardized way to report # of
cycles and max freq, so userspace tool can derive %utilization in
addition to just %busy


How do you define %utilisation vs %busy - I don't exactly follow since I
see the two as same?


so, say you are running at 50% of max clk, and gpu is busy 70% of the
time.  The utilization is only 35% because the gpu could scale up the
clk to get more work done.


Got it, thanks. I don't think we have the equivalent on the i915 side 
(we do have global frequency reporting via perf/PMU). In general things 
like these I imagined would be defined as driver specific tags. If you 
look at the drm-usage-stats.rst in my series, there is a "Driver 
specific implementations" section in there which links to the i915 doc 
section.


I've also put a "big fat" comment into the i915 fdinfo fops vfunc 
pointing back to drm-usage-stats.rst which I think is useful when large 
teams work on a driver. Not sure if that applies to msm so just mentioning.


Since this all works for you, would you mind applying your ack against 
20220222140422.1121163-9-tvrtko.ursu...@linux.intel.com?


I need to get some updates r-b's for my series and then I submit it 
again to Dave and Daniel for final acks.


After that, for a 2nd/follow-up phase, I plan to re-surrect the amdgpu 
patch I had to make it compliant to common format, plus document the 
option of engine utilisation tag being in percentage units as exposed 
from that driver. And extending gputop to support that as well.


Regards,

Tvrtko


Looking at your patch I guess I don't understand the difference between
'elapsed' and 'cycles' inside your retire_submit(). Both are scoped to a
single context and are not global? If 'elapsed' is time context has
spent on the GPU, cycles isn't the same just in a different unit?


Correct, we capture (from GPU cmdstream) two counters both before and
after a submit (aka execbuf) runs, one is a fixed-rate counter, which
gives us elapsed time.  The second is a counter that increments every
clk cycle, which gives us the # of cycles.  With the two values, we
can calculate GPU frequency.

BR,
-R


Regards,

Tvrtko



Re: [PATCH 3/3] drm/msm: Expose client engine utilization via fdinfo

2022-02-28 Thread Rob Clark
On Mon, Feb 28, 2022 at 6:33 AM Tvrtko Ursulin
 wrote:
>
>
> On 25/02/2022 22:14, Rob Clark wrote:
> > On Fri, Feb 25, 2022 at 12:25 PM Rob Clark  wrote:
> >>
> >> From: Rob Clark 
> >>
> >> Similar to AMD commit
> >> 874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
> >> infrastructure added in previous patches, we add basic client info
> >> and GPU engine utilisation for msm.
> >>
> >> Example output:
> >>
> >>  # cat /proc/`pgrep glmark2`/fdinfo/6
> >>  pos:0
> >>  flags:  0242
> >>  mnt_id: 21
> >>  ino:162
> >>  drm-driver: msm
> >>  drm-client-id:  7
> >>  drm-engine-gpu: 1734371319 ns
> >>  drm-cycles-gpu: 1153645024
>
> Nice, so my vendor agnostic actually worked (with that single fixup of
> accounting for the fact pdev tag is optional)?
>
> > Note that it might be useful to have a standardized way to report # of
> > cycles and max freq, so userspace tool can derive %utilization in
> > addition to just %busy
>
> How do you define %utilisation vs %busy - I don't exactly follow since I
> see the two as same?

so, say you are running at 50% of max clk, and gpu is busy 70% of the
time.  The utilization is only 35% because the gpu could scale up the
clk to get more work done.

> Looking at your patch I guess I don't understand the difference between
> 'elapsed' and 'cycles' inside your retire_submit(). Both are scoped to a
> single context and are not global? If 'elapsed' is time context has
> spent on the GPU, cycles isn't the same just in a different unit?

Correct, we capture (from GPU cmdstream) two counters both before and
after a submit (aka execbuf) runs, one is a fixed-rate counter, which
gives us elapsed time.  The second is a counter that increments every
clk cycle, which gives us the # of cycles.  With the two values, we
can calculate GPU frequency.

BR,
-R

> Regards,
>
> Tvrtko
>


Re: [PATCH 3/3] drm/msm: Expose client engine utilization via fdinfo

2022-02-28 Thread Tvrtko Ursulin



On 25/02/2022 22:14, Rob Clark wrote:

On Fri, Feb 25, 2022 at 12:25 PM Rob Clark  wrote:


From: Rob Clark 

Similar to AMD commit
874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
infrastructure added in previous patches, we add basic client info
and GPU engine utilisation for msm.

Example output:

 # cat /proc/`pgrep glmark2`/fdinfo/6
 pos:0
 flags:  0242
 mnt_id: 21
 ino:162
 drm-driver: msm
 drm-client-id:  7
 drm-engine-gpu: 1734371319 ns
 drm-cycles-gpu: 1153645024


Nice, so my vendor agnostic actually worked (with that single fixup of 
accounting for the fact pdev tag is optional)?



Note that it might be useful to have a standardized way to report # of
cycles and max freq, so userspace tool can derive %utilization in
addition to just %busy


How do you define %utilisation vs %busy - I don't exactly follow since I 
see the two as same?


Looking at your patch I guess I don't understand the difference between 
'elapsed' and 'cycles' inside your retire_submit(). Both are scoped to a 
single context and are not global? If 'elapsed' is time context has 
spent on the GPU, cycles isn't the same just in a different unit?


Regards,

Tvrtko


BR,
-R



See also: https://patchwork.freedesktop.org/patch/468505/

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/msm/msm_drv.c | 17 -
  drivers/gpu/drm/msm/msm_gpu.c | 20 ++--
  drivers/gpu/drm/msm/msm_gpu.h | 19 +++
  3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 16f37f3d9061..fdf401e6f09e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -911,7 +911,22 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
 DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, 
DRM_RENDER_ALLOW),
  };

-DEFINE_DRM_GEM_FOPS(fops);
+static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f)
+{
+   struct drm_file *file = f->private_data;
+   struct drm_device *dev = file->minor->dev;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct drm_printer p = drm_seq_file_printer(m);
+
+   if (!priv->gpu)
+   return;
+
+   msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, );
+}
+
+DEFINE_DRM_GEM_FOPS(fops,
+   .show_fdinfo = msm_fop_show_fdinfo,
+);

  static const struct drm_driver msm_driver = {
 .driver_features= DRIVER_GEM |
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 173ebd449f2f..6302f3fe564b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -4,6 +4,8 @@
   * Author: Rob Clark 
   */

+#include "drm/drm_drv.h"
+
  #include "msm_gpu.h"
  #include "msm_gem.h"
  #include "msm_mmu.h"
@@ -146,6 +148,15 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
 return 0;
  }

+void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
+struct drm_printer *p)
+{
+   drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name);
+   drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno);
+   drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
+   drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
+}
+
  int msm_gpu_hw_init(struct msm_gpu *gpu)
  {
 int ret;
@@ -643,7 +654,7 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
  {
 int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
 volatile struct msm_gpu_submit_stats *stats;
-   u64 elapsed, clock = 0;
+   u64 elapsed, clock = 0, cycles;
 unsigned long flags;

 stats = >memptrs->stats[index];
@@ -651,12 +662,17 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
 elapsed = (stats->alwayson_end - stats->alwayson_start) * 1;
 do_div(elapsed, 192);

+   cycles = stats->cpcycles_end - stats->cpcycles_start;
+
 /* Calculate the clock frequency from the number of CP cycles */
 if (elapsed) {
-   clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
+   clock = cycles * 1000;
 do_div(clock, elapsed);
 }

+   submit->queue->ctx->elapsed_ns += elapsed;
+   submit->queue->ctx->cycles += cycles;
+
 trace_msm_gpu_submit_retired(submit, elapsed, clock,
 stats->alwayson_start, stats->alwayson_end);

diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 696e2ed8a236..ad4fe05dee03 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -328,6 +328,22 @@ struct msm_file_private {
 struct kref ref;
 int seqno;

+   /**
+* elapsed:
+*
+* The total (cumulative) elapsed time GPU was busy with rendering
+* from this context in 

Re: [PATCH 3/3] drm/msm: Expose client engine utilization via fdinfo

2022-02-25 Thread Rob Clark
On Fri, Feb 25, 2022 at 12:25 PM Rob Clark  wrote:
>
> From: Rob Clark 
>
> Similar to AMD commit
> 874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
> infrastructure added in previous patches, we add basic client info
> and GPU engine utilisation for msm.
>
> Example output:
>
> # cat /proc/`pgrep glmark2`/fdinfo/6
> pos:0
> flags:  0242
> mnt_id: 21
> ino:162
> drm-driver: msm
> drm-client-id:  7
> drm-engine-gpu: 1734371319 ns
> drm-cycles-gpu: 1153645024

Note that it might be useful to have a standardized way to report # of
cycles and max freq, so userspace tool can derive %utilization in
addition to just %busy

BR,
-R

>
> See also: https://patchwork.freedesktop.org/patch/468505/
>
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/msm_drv.c | 17 -
>  drivers/gpu/drm/msm/msm_gpu.c | 20 ++--
>  drivers/gpu/drm/msm/msm_gpu.h | 19 +++
>  3 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 16f37f3d9061..fdf401e6f09e 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -911,7 +911,22 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
> DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, 
> DRM_RENDER_ALLOW),
>  };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> +   struct drm_file *file = f->private_data;
> +   struct drm_device *dev = file->minor->dev;
> +   struct msm_drm_private *priv = dev->dev_private;
> +   struct drm_printer p = drm_seq_file_printer(m);
> +
> +   if (!priv->gpu)
> +   return;
> +
> +   msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, );
> +}
> +
> +DEFINE_DRM_GEM_FOPS(fops,
> +   .show_fdinfo = msm_fop_show_fdinfo,
> +);
>
>  static const struct drm_driver msm_driver = {
> .driver_features= DRIVER_GEM |
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 173ebd449f2f..6302f3fe564b 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -4,6 +4,8 @@
>   * Author: Rob Clark 
>   */
>
> +#include "drm/drm_drv.h"
> +
>  #include "msm_gpu.h"
>  #include "msm_gem.h"
>  #include "msm_mmu.h"
> @@ -146,6 +148,15 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
> return 0;
>  }
>
> +void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
> +struct drm_printer *p)
> +{
> +   drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name);
> +   drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno);
> +   drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
> +   drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
> +}
> +
>  int msm_gpu_hw_init(struct msm_gpu *gpu)
>  {
> int ret;
> @@ -643,7 +654,7 @@ static void retire_submit(struct msm_gpu *gpu, struct 
> msm_ringbuffer *ring,
>  {
> int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
> volatile struct msm_gpu_submit_stats *stats;
> -   u64 elapsed, clock = 0;
> +   u64 elapsed, clock = 0, cycles;
> unsigned long flags;
>
> stats = >memptrs->stats[index];
> @@ -651,12 +662,17 @@ static void retire_submit(struct msm_gpu *gpu, struct 
> msm_ringbuffer *ring,
> elapsed = (stats->alwayson_end - stats->alwayson_start) * 1;
> do_div(elapsed, 192);
>
> +   cycles = stats->cpcycles_end - stats->cpcycles_start;
> +
> /* Calculate the clock frequency from the number of CP cycles */
> if (elapsed) {
> -   clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
> +   clock = cycles * 1000;
> do_div(clock, elapsed);
> }
>
> +   submit->queue->ctx->elapsed_ns += elapsed;
> +   submit->queue->ctx->cycles += cycles;
> +
> trace_msm_gpu_submit_retired(submit, elapsed, clock,
> stats->alwayson_start, stats->alwayson_end);
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 696e2ed8a236..ad4fe05dee03 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -328,6 +328,22 @@ struct msm_file_private {
> struct kref ref;
> int seqno;
>
> +   /**
> +* elapsed:
> +*
> +* The total (cumulative) elapsed time GPU was busy with rendering
> +* from this context in ns.
> +*/
> +   uint64_t elapsed_ns;
> +
> +   /**
> +* cycles:
> +*
> +* The total (cumulative) GPU cycles elapsed attributed to this
> +* context.
> +*/
> +   uint64_t cycles;
> +
> /**
>  * entities:
>  *
> @@ -511,6 +527,9 @@ static inline void gpu_write64(struct msm_gpu *gpu, 

[PATCH 3/3] drm/msm: Expose client engine utilization via fdinfo

2022-02-25 Thread Rob Clark
From: Rob Clark 

Similar to AMD commit
874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
infrastructure added in previous patches, we add basic client info
and GPU engine utilisation for msm.

Example output:

# cat /proc/`pgrep glmark2`/fdinfo/6
pos:0
flags:  0242
mnt_id: 21
ino:162
drm-driver: msm
drm-client-id:  7
drm-engine-gpu: 1734371319 ns
drm-cycles-gpu: 1153645024

See also: https://patchwork.freedesktop.org/patch/468505/

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/msm_drv.c | 17 -
 drivers/gpu/drm/msm/msm_gpu.c | 20 ++--
 drivers/gpu/drm/msm/msm_gpu.h | 19 +++
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 16f37f3d9061..fdf401e6f09e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -911,7 +911,22 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, 
DRM_RENDER_ALLOW),
 };
 
-DEFINE_DRM_GEM_FOPS(fops);
+static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f)
+{
+   struct drm_file *file = f->private_data;
+   struct drm_device *dev = file->minor->dev;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct drm_printer p = drm_seq_file_printer(m);
+
+   if (!priv->gpu)
+   return;
+
+   msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, );
+}
+
+DEFINE_DRM_GEM_FOPS(fops,
+   .show_fdinfo = msm_fop_show_fdinfo,
+);
 
 static const struct drm_driver msm_driver = {
.driver_features= DRIVER_GEM |
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 173ebd449f2f..6302f3fe564b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -4,6 +4,8 @@
  * Author: Rob Clark 
  */
 
+#include "drm/drm_drv.h"
+
 #include "msm_gpu.h"
 #include "msm_gem.h"
 #include "msm_mmu.h"
@@ -146,6 +148,15 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
return 0;
 }
 
+void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
+struct drm_printer *p)
+{
+   drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name);
+   drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno);
+   drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
+   drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
+}
+
 int msm_gpu_hw_init(struct msm_gpu *gpu)
 {
int ret;
@@ -643,7 +654,7 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
 {
int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
volatile struct msm_gpu_submit_stats *stats;
-   u64 elapsed, clock = 0;
+   u64 elapsed, clock = 0, cycles;
unsigned long flags;
 
stats = >memptrs->stats[index];
@@ -651,12 +662,17 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
elapsed = (stats->alwayson_end - stats->alwayson_start) * 1;
do_div(elapsed, 192);
 
+   cycles = stats->cpcycles_end - stats->cpcycles_start;
+
/* Calculate the clock frequency from the number of CP cycles */
if (elapsed) {
-   clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
+   clock = cycles * 1000;
do_div(clock, elapsed);
}
 
+   submit->queue->ctx->elapsed_ns += elapsed;
+   submit->queue->ctx->cycles += cycles;
+
trace_msm_gpu_submit_retired(submit, elapsed, clock,
stats->alwayson_start, stats->alwayson_end);
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 696e2ed8a236..ad4fe05dee03 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -328,6 +328,22 @@ struct msm_file_private {
struct kref ref;
int seqno;
 
+   /**
+* elapsed:
+*
+* The total (cumulative) elapsed time GPU was busy with rendering
+* from this context in ns.
+*/
+   uint64_t elapsed_ns;
+
+   /**
+* cycles:
+*
+* The total (cumulative) GPU cycles elapsed attributed to this
+* context.
+*/
+   uint64_t cycles;
+
/**
 * entities:
 *
@@ -511,6 +527,9 @@ static inline void gpu_write64(struct msm_gpu *gpu, u32 lo, 
u32 hi, u64 val)
 int msm_gpu_pm_suspend(struct msm_gpu *gpu);
 int msm_gpu_pm_resume(struct msm_gpu *gpu);
 
+void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
+struct drm_printer *p);
+
 int msm_submitqueue_init(struct drm_device *drm, struct msm_file_private *ctx);
 struct msm_gpu_submitqueue *msm_submitqueue_get(struct msm_file_private *ctx,
u32 id);
-- 
2.35.1