diff mbox series

drm/vmwgfx: Add tracepoints

Message ID 20240905181710.154732-1-ian.forbes@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Add tracepoints | expand

Commit Message

Ian Forbes Sept. 5, 2024, 6:17 p.m. UTC
Adds the necessary files to create tracepoints for the vmwgfx driver.

Adds a single tracepoint for command buffer completion. This tracepoint
can be used to time command buffer execution time and to decode command
buffer errors. The submission time is also now recorded when the command
buffer is submitted to hardware.

Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/Makefile        |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  9 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_trace.c  | 32 +++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_trace.h  | 62 ++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_trace.c
 create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_trace.h

Comments

Zack Rusin Sept. 6, 2024, 3:59 a.m. UTC | #1
On Thu, Sep 5, 2024 at 2:17 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> Adds the necessary files to create tracepoints for the vmwgfx driver.
>
> Adds a single tracepoint for command buffer completion. This tracepoint
> can be used to time command buffer execution time and to decode command
> buffer errors. The submission time is also now recorded when the command
> buffer is submitted to hardware.
>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/Makefile        |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  9 +++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_trace.c  | 32 +++++++++++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_trace.h  | 62 ++++++++++++++++++++++++++
>  4 files changed, 102 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_trace.c
>  create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_trace.h
>
> diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
> index 46a4ab688a7f..482c1935bde6 100644
> --- a/drivers/gpu/drm/vmwgfx/Makefile
> +++ b/drivers/gpu/drm/vmwgfx/Makefile
> @@ -10,6 +10,6 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>             vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
>             vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
>             vmwgfx_devcaps.o ttm_object.o vmwgfx_system_manager.o \
> -           vmwgfx_gem.o vmwgfx_vkms.o
> +           vmwgfx_gem.o vmwgfx_vkms.o vmwgfx_trace.o
>
>  obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> index 94e8982f5616..1ac7f382cdb1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> @@ -27,6 +27,7 @@
>
>  #include "vmwgfx_bo.h"
>  #include "vmwgfx_drv.h"
> +#include "vmwgfx_trace.h"
>
>  #include <drm/ttm/ttm_bo.h>
>
> @@ -141,6 +142,7 @@ struct vmw_cmdbuf_man {
>   * @man: The command buffer manager.
>   * @cb_header: Device command buffer header, allocated from a DMA pool.
>   * @cb_context: The device command buffer context.
> + * @inline_space: Whether inline command buffer space is used.
>   * @list: List head for attaching to the manager lists.
>   * @node: The range manager node.
>   * @handle: The DMA address of @cb_header. Handed to the device on command
> @@ -148,19 +150,20 @@ struct vmw_cmdbuf_man {
>   * @cmd: Pointer to the command buffer space of this buffer.
>   * @size: Size of the command buffer space of this buffer.
>   * @reserved: Reserved space of this buffer.
> - * @inline_space: Whether inline command buffer space is used.
> + * @submit_time: When the CB was submitted to hardware in jiffies.

In general it looks good but what's the reason for the submit_time?

z
Ian Forbes Sept. 6, 2024, 3:26 p.m. UTC | #2
On Thu, Sep 5, 2024 at 10:59 PM Zack Rusin <zack.rusin@broadcom.com> wrote:
>
>
> In general it looks good but what's the reason for the submit_time?
>
> z

So you can get an approximate time of how long each command buffer takes.
You can then use it to construct a histogram or look for outliers
using bpftrace.
Useful when doing performance analysis to determine if slowdowns are being
caused by the host or the guest driver.

$ sudo bpftrace -e 'tracepoint:vmwgfx:vmwgfx_cmdbuf_done{
if(args->status == 1) { $elapsed =(jiffies -
args->header->submit_time); @exec_times = hist($elapsed);  } }'
Attaching 1 probe...
^C

@exec_times:
[0]                   59 |@@@                                                 |
[1]                  334 |@@@@@@@@@@@@@@@@@@                                  |
[2, 4)               431 |@@@@@@@@@@@@@@@@@@@@@@@                             |
[4, 8)               961 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[8, 16)              596 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@                    |
[16, 32)             139 |@@@@@@@                                             |
[32, 64)              67 |@@@                                                 |
[64, 128)             36 |@                                                   |
[128, 256)            30 |@                                                   |
[256, 512)            17 |                                                    |
Zack Rusin Sept. 6, 2024, 3:52 p.m. UTC | #3
On Fri, Sep 6, 2024 at 11:26 AM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> On Thu, Sep 5, 2024 at 10:59 PM Zack Rusin <zack.rusin@broadcom.com> wrote:
> >
> >
> > In general it looks good but what's the reason for the submit_time?
> >
> > z
>
> So you can get an approximate time of how long each command buffer takes.
> You can then use it to construct a histogram or look for outliers
> using bpftrace.
> Useful when doing performance analysis to determine if slowdowns are being
> caused by the host or the guest driver.
>
> $ sudo bpftrace -e 'tracepoint:vmwgfx:vmwgfx_cmdbuf_done{
> if(args->status == 1) { $elapsed =(jiffies -
> args->header->submit_time); @exec_times = hist($elapsed);  } }'
> Attaching 1 probe...

Can't you do the same with just:
bpftrace -e 'kprobe:vmw_cmdbuf_header_submit { @start[tid] = nsecs; }
kretprobe:vmw_cmdbuf_header_submit /@start[tid]/ { @ns[comm] =
hist(nsecs - @start[tid]); delete(@start[tid]); }'
Or kfunc/kretfunc if you want to condition it based on args->status?

z
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 46a4ab688a7f..482c1935bde6 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -10,6 +10,6 @@  vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
 	    vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
 	    vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
 	    vmwgfx_devcaps.o ttm_object.o vmwgfx_system_manager.o \
-	    vmwgfx_gem.o vmwgfx_vkms.o
+	    vmwgfx_gem.o vmwgfx_vkms.o vmwgfx_trace.o
 
 obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 94e8982f5616..1ac7f382cdb1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -27,6 +27,7 @@ 
 
 #include "vmwgfx_bo.h"
 #include "vmwgfx_drv.h"
+#include "vmwgfx_trace.h"
 
 #include <drm/ttm/ttm_bo.h>
 
@@ -141,6 +142,7 @@  struct vmw_cmdbuf_man {
  * @man: The command buffer manager.
  * @cb_header: Device command buffer header, allocated from a DMA pool.
  * @cb_context: The device command buffer context.
+ * @inline_space: Whether inline command buffer space is used.
  * @list: List head for attaching to the manager lists.
  * @node: The range manager node.
  * @handle: The DMA address of @cb_header. Handed to the device on command
@@ -148,19 +150,20 @@  struct vmw_cmdbuf_man {
  * @cmd: Pointer to the command buffer space of this buffer.
  * @size: Size of the command buffer space of this buffer.
  * @reserved: Reserved space of this buffer.
- * @inline_space: Whether inline command buffer space is used.
+ * @submit_time: When the CB was submitted to hardware in jiffies.
  */
 struct vmw_cmdbuf_header {
 	struct vmw_cmdbuf_man *man;
 	SVGACBHeader *cb_header;
 	SVGACBContext cb_context;
+	bool inline_space;
 	struct list_head list;
 	struct drm_mm_node node;
 	dma_addr_t handle;
 	u8 *cmd;
 	size_t size;
 	size_t reserved;
-	bool inline_space;
+	u64 submit_time;
 };
 
 /**
@@ -303,6 +306,7 @@  static int vmw_cmdbuf_header_submit(struct vmw_cmdbuf_header *header)
 	struct vmw_cmdbuf_man *man = header->man;
 	u32 val;
 
+	header->submit_time = get_jiffies_64();
 	val = upper_32_bits(header->handle);
 	vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH, val);
 
@@ -391,6 +395,7 @@  static void vmw_cmdbuf_ctx_process(struct vmw_cmdbuf_man *man,
 		list_del(&entry->list);
 		wake_up_all(&man->idle_queue);
 		ctx->num_hw_submitted--;
+		trace_vmwgfx_cmdbuf_done(status, entry);
 		switch (status) {
 		case SVGA_CB_STATUS_COMPLETED:
 			__vmw_cmdbuf_header_free(entry);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_trace.c b/drivers/gpu/drm/vmwgfx/vmwgfx_trace.c
new file mode 100644
index 000000000000..065365fb147f
--- /dev/null
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_trace.c
@@ -0,0 +1,32 @@ 
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/**************************************************************************
+ *
+ * Copyright (c) 2024 Broadcom. All Rights Reserved. The term
+ * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **************************************************************************/
+
+#ifndef __CHECKER__
+#define CREATE_TRACE_POINTS
+#include "vmwgfx_trace.h"
+#endif
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_trace.h b/drivers/gpu/drm/vmwgfx/vmwgfx_trace.h
new file mode 100644
index 000000000000..808829321cf7
--- /dev/null
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_trace.h
@@ -0,0 +1,62 @@ 
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/**************************************************************************
+ *
+ * Copyright (c) 2024 Broadcom. All Rights Reserved. The term
+ * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **************************************************************************/
+
+#if !defined(_TRACE_VMWGFX_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_VMWGFX_H_
+
+#include <linux/stringify.h>
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM vmwgfx
+
+struct vmw_cmdbuf_header;
+
+TRACE_EVENT(vmwgfx_cmdbuf_done,
+
+	TP_PROTO(int status, struct vmw_cmdbuf_header *header),
+	TP_ARGS(status, header),
+	TP_STRUCT__entry(
+		__field(int, status)
+		__field(struct vmw_cmdbuf_header*, header)
+	),
+	TP_fast_assign(
+		__entry->status = status;
+		__entry->header = header;
+	),
+	TP_printk("status=%d", __entry->status)
+);
+
+#endif /* _TRACE_VMWGFX_H_ */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/vmwgfx
+#define TRACE_INCLUDE_FILE vmwgfx_trace
+#include <trace/define_trace.h>