All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC 0/8] Per client engine busyness
@ 2020-01-10 13:30 Tvrtko Ursulin
  2020-01-10 13:30 ` [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Another re-spin of the per-client engine busyness series. Highlights from this
version:

 * Two patches added on top of the series to provide the data in GuC mode.
   (For warnings and limitations please read the respective commit messages.)
 * Refactor to introduce a notion of i915_drm_client and move to separate source
   file.

Internally we track time spent on engines for each struct intel_context. This
can serve as a building block for several features from the want list:
smarter scheduler decisions, getrusage(2)-like per-GEM-context functionality
wanted by some customers, cgroups controller, dynamic SSEU tuning,...

Externally, in sysfs, we expose time spent on GPU per client and per engine
class.

Sysfs interface enables us to implement a "top-like" tool for GPU tasks. Or with
a "screenshot":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
intel-gpu-top -  906/ 955 MHz;    0% RC6;  5.30 Watts;      933 irqs/s

      IMC reads:     4414 MiB/s
     IMC writes:     3805 MiB/s

          ENGINE      BUSY                                      MI_SEMA MI_WAIT
     Render/3D/0   93.46% |████████████████████████████████▋  |      0%      0%
       Blitter/0    0.00% |                                   |      0%      0%
         Video/0    0.00% |                                   |      0%      0%
  VideoEnhance/0    0.00% |                                   |      0%      0%

  PID            NAME  Render/3D      Blitter        Video      VideoEnhance
 2733       neverball |██████▌     ||            ||            ||            |
 2047            Xorg |███▊        ||            ||            ||            |
 2737        glxgears |█▍          ||            ||            ||            |
 2128           xfwm4 |            ||            ||            ||            |
 2047            Xorg |            ||            ||            ||            |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Implementation wise we add a a bunch of files in sysfs like:

	# cd /sys/class/drm/card0/clients/
	# tree
	.
	├── 7
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	├── 8
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	└── 9
	    ├── busy
	    │   ├── 0
	    │   ├── 1
	    │   ├── 2
	    │   └── 3
	    ├── name
	    └── pid


Files in 'busy' directories are numbered using the engine class ABI values and
they contain accumulated nanoseconds each client spent on engines of a
respective class.

Tvrtko Ursulin (8):
  drm/i915: Expose list of clients in sysfs
  drm/i915: Update client name on context create
  drm/i915: Track per-context engine busyness
  drm/i915: Track all user contexts per client
  drm/i915: Contexts can use struct pid stored in the client
  drm/i915: Expose per-engine client busyness
  drm/i915: Track hw reported context runtime
  drm/i915: Fallback to hw context runtime when sw tracking is not
    available

 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  60 +++-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  16 +-
 drivers/gpu/drm/i915/gt/intel_context.c       |  20 ++
 drivers/gpu/drm/i915/gt/intel_context.h       |  18 ++
 drivers/gpu/drm/i915/gt/intel_context_types.h |  14 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  16 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  55 +++-
 drivers/gpu/drm/i915/i915_debugfs.c           |   7 +-
 drivers/gpu/drm/i915/i915_drm_client.c        | 298 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_drm_client.h        |  83 +++++
 drivers/gpu/drm/i915/i915_drv.h               |   5 +
 drivers/gpu/drm/i915/i915_gem.c               |  35 +-
 drivers/gpu/drm/i915/i915_gpu_error.c         |   8 +-
 drivers/gpu/drm/i915/i915_sysfs.c             |   8 +
 drivers/gpu/drm/i915/intel_device_info.c      |   2 +
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 17 files changed, 606 insertions(+), 43 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:36   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create Tvrtko Ursulin
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Expose a list of clients with open file handles in sysfs.

This will be a basis for a top-like utility showing per-client and per-
engine GPU load.

Currently we only expose each client's pid and name under opaque numbered
directories in /sys/class/drm/card0/clients/.

For instance:

/sys/class/drm/card0/clients/3/name: Xorg
/sys/class/drm/card0/clients/3/pid: 5664

v2:
 Chris Wilson:
 * Enclose new members into dedicated structs.
 * Protect against failed sysfs registration.

v3:
 * sysfs_attr_init.

v4:
 * Fix for internal clients.

v5:
 * Use cyclic ida for client id. (Chris)
 * Do not leak pid reference. (Chris)
 * Tidy code with some locals.

v6:
 * Use xa_alloc_cyclic to simplify locking. (Chris)
 * No need to unregister individial sysfs files. (Chris)
 * Rebase on top of fpriv kref.
 * Track client closed status and reflect in sysfs.

v7:
 * Make drm_client more standalone concept.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/Makefile          |   3 +-
 drivers/gpu/drm/i915/i915_drm_client.c | 150 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drm_client.h |  61 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h        |   5 +
 drivers/gpu/drm/i915/i915_gem.c        |  35 ++++--
 drivers/gpu/drm/i915/i915_sysfs.c      |   8 ++
 6 files changed, 254 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b8c5f8934dbd..3a677a20a709 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -36,7 +36,8 @@ subdir-ccflags-y += -I$(srctree)/$(src)
 # Please keep these build lists sorted!
 
 # core driver code
-i915-y += i915_drv.o \
+i915-y += i915_drm_client.o \
+	  i915_drv.o \
 	  i915_irq.o \
 	  i915_getparam.o \
 	  i915_params.o \
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
new file mode 100644
index 000000000000..02356f48d85b
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -0,0 +1,150 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "i915_drm_client.h"
+#include "i915_gem.h"
+#include "i915_utils.h"
+
+static ssize_t
+show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct i915_drm_client *client =
+		container_of(attr, typeof(*client), attr.name);
+
+	return snprintf(buf, PAGE_SIZE, "%s%s%s",
+			client->closed ? "<" : "",
+			client->name,
+			client->closed ? ">" : "");
+}
+
+static ssize_t
+show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct i915_drm_client *client =
+		container_of(attr, typeof(*client), attr.pid);
+
+	return snprintf(buf, PAGE_SIZE, "%s%u%s",
+			client->closed ? "<" : "",
+			pid_nr(client->pid),
+			client->closed ? ">" : "");
+}
+
+static int
+__i915_drm_client_register(struct i915_drm_client *client,
+			   struct task_struct *task)
+{
+	struct i915_drm_clients *clients = client->clients;
+	struct device_attribute *attr;
+	int ret = -ENOMEM;
+	char idstr[32];
+
+	if (!clients->root)
+		return 0; /* intel_fbdev_init registers a client before sysfs */
+
+	client->name = kstrdup(task->comm, GFP_KERNEL);
+	if (!client->name)
+		goto err_name;
+
+	snprintf(idstr, sizeof(idstr), "%u", client->id);
+	client->root = kobject_create_and_add(idstr, clients->root);
+	if (!client->root)
+		goto err_client;
+
+	attr = &client->attr.name;
+	sysfs_attr_init(&attr->attr);
+	attr->attr.name = "name";
+	attr->attr.mode = 0444;
+	attr->show = show_client_name;
+
+	ret = sysfs_create_file(client->root, (struct attribute *)attr);
+	if (ret)
+		goto err_attr;
+
+	attr = &client->attr.pid;
+	sysfs_attr_init(&attr->attr);
+	attr->attr.name = "pid";
+	attr->attr.mode = 0444;
+	attr->show = show_client_pid;
+
+	ret = sysfs_create_file(client->root, (struct attribute *)attr);
+	if (ret)
+		goto err_attr;
+
+	client->pid = get_task_pid(task, PIDTYPE_PID);
+
+	return 0;
+
+err_attr:
+	kobject_put(client->root);
+err_client:
+	kfree(client->name);
+err_name:
+	return ret;
+}
+
+static void __i915_drm_client_unregister(struct i915_drm_client *client)
+{
+	if (!client->name)
+		return; /* fbdev client or error during drm open */
+
+	kobject_put(fetch_and_zero(&client->root));
+	put_pid(fetch_and_zero(&client->pid));
+	kfree(fetch_and_zero(&client->name));
+}
+
+struct i915_drm_client *
+i915_drm_client_add(struct i915_drm_clients *clients, struct task_struct *task)
+{
+	struct i915_drm_client *client;
+	u32 next = 0;
+	int ret;
+
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return ERR_PTR(-ENOMEM);
+
+	kref_init(&client->kref);
+	client->clients = clients;
+
+	ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
+			      xa_limit_32b, &next, GFP_KERNEL);
+	if (ret)
+		goto err_id;
+
+	ret = __i915_drm_client_register(client, task);
+	if (ret)
+		goto err_register;
+
+	return client;
+
+err_register:
+	xa_erase(&clients->xarray, client->id);
+err_id:
+	kfree(client);
+
+	return ERR_PTR(ret);
+}
+
+void __i915_drm_client_free(struct kref *kref)
+{
+	struct i915_drm_client *client =
+		container_of(kref, typeof(*client), kref);
+
+	__i915_drm_client_unregister(client);
+	xa_erase(&client->clients->xarray, client->id);
+	kfree_rcu(client, rcu);
+}
+
+void i915_drm_client_close(struct i915_drm_client *client)
+{
+	GEM_BUG_ON(client->closed);
+	client->closed = true;
+	i915_drm_client_put(client);
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
new file mode 100644
index 000000000000..8b261dc4a1f3
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -0,0 +1,61 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef __I915_DRM_CLIENT_H__
+#define __I915_DRM_CLIENT_H__
+
+#include <linux/device.h>
+#include <linux/kobject.h>
+#include <linux/kref.h>
+#include <linux/pid.h>
+#include <linux/rcupdate.h>
+#include <linux/sched.h>
+#include <linux/xarray.h>
+
+struct i915_drm_clients {
+	struct xarray xarray;
+	struct kobject *root;
+};
+
+struct i915_drm_client {
+	struct kref kref;
+
+	struct rcu_head rcu;
+
+	unsigned int id;
+	struct pid *pid;
+	char *name;
+	bool closed;
+
+	struct i915_drm_clients *clients;
+
+	struct kobject *root;
+	struct {
+		struct device_attribute pid;
+		struct device_attribute name;
+	} attr;
+};
+
+static inline struct i915_drm_client *
+i915_drm_client_get(struct i915_drm_client *client)
+{
+	kref_get(&client->kref);
+	return client;
+}
+
+void __i915_drm_client_free(struct kref *kref);
+
+static inline void i915_drm_client_put(struct i915_drm_client *client)
+{
+	kref_put(&client->kref, __i915_drm_client_free);
+}
+
+void i915_drm_client_close(struct i915_drm_client *client);
+
+struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients,
+					    struct task_struct *task);
+
+#endif /* !__I915_DRM_CLIENT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1025d783f494..b04c868ef9b5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -91,6 +91,7 @@
 #include "intel_wakeref.h"
 #include "intel_wopcm.h"
 
+#include "i915_drm_client.h"
 #include "i915_gem.h"
 #include "i915_gem_fence_reg.h"
 #include "i915_gem_gtt.h"
@@ -224,6 +225,8 @@ struct drm_i915_file_private {
 	/** ban_score: Accumulated score of all ctx bans and fast hangs. */
 	atomic_t ban_score;
 	unsigned long hang_timestamp;
+
+	struct i915_drm_client *client;
 };
 
 /* Interface history:
@@ -1286,6 +1289,8 @@ struct drm_i915_private {
 
 	struct i915_pmu pmu;
 
+	struct i915_drm_clients clients;
+
 	struct i915_hdcp_comp_master *hdcp_master;
 	bool hdcp_comp_added;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 94f993e4c12f..74007ce8f81f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1213,12 +1213,14 @@ static void i915_gem_init__mm(struct drm_i915_private *i915)
 	i915_gem_init__objects(i915);
 }
 
-void i915_gem_init_early(struct drm_i915_private *dev_priv)
+void i915_gem_init_early(struct drm_i915_private *i915)
 {
-	i915_gem_init__mm(dev_priv);
-	i915_gem_init__contexts(dev_priv);
+	i915_gem_init__mm(i915);
+	i915_gem_init__contexts(i915);
 
-	spin_lock_init(&dev_priv->fb_tracking.lock);
+	spin_lock_init(&i915->fb_tracking.lock);
+
+	xa_init_flags(&i915->clients.xarray, XA_FLAGS_ALLOC);
 }
 
 void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
@@ -1227,6 +1229,8 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
 	GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
 	GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
 	WARN_ON(dev_priv->mm.shrink_count);
+	WARN_ON(!xa_empty(&dev_priv->clients.xarray));
+	xa_destroy(&dev_priv->clients.xarray);
 }
 
 int i915_gem_freeze(struct drm_i915_private *dev_priv)
@@ -1280,6 +1284,8 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct i915_request *request;
 
+	i915_drm_client_close(file_priv->client);
+
 	/* Clean up our request list when the client is going away, so that
 	 * later retire_requests won't dereference our soon-to-be-gone
 	 * file_priv.
@@ -1293,17 +1299,25 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv;
-	int ret;
+	struct i915_drm_client *client;
+	int ret = -ENOMEM;
 
 	DRM_DEBUG("\n");
 
 	file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
 	if (!file_priv)
-		return -ENOMEM;
+		goto err_alloc;
+
+	client = i915_drm_client_add(&i915->clients, current);
+	if (IS_ERR(client)) {
+		ret = PTR_ERR(client);
+		goto err_client;
+	}
 
 	file->driver_priv = file_priv;
 	file_priv->dev_priv = i915;
 	file_priv->file = file;
+	file_priv->client = client;
 
 	spin_lock_init(&file_priv->mm.lock);
 	INIT_LIST_HEAD(&file_priv->mm.request_list);
@@ -1313,8 +1327,15 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
 
 	ret = i915_gem_context_open(i915, file);
 	if (ret)
-		kfree(file_priv);
+		goto err_context;
+
+	return 0;
 
+err_context:
+	i915_drm_client_close(client);
+err_client:
+	kfree(file_priv);
+err_alloc:
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index ad2b1b833d7b..3ab50e29fddf 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -559,6 +559,11 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
 	struct device *kdev = dev_priv->drm.primary->kdev;
 	int ret;
 
+	dev_priv->clients.root =
+		kobject_create_and_add("clients", &kdev->kobj);
+	if (!dev_priv->clients.root)
+		DRM_ERROR("Per-client sysfs setup failed\n");
+
 #ifdef CONFIG_PM
 	if (HAS_RC6(dev_priv)) {
 		ret = sysfs_merge_group(&kdev->kobj,
@@ -619,4 +624,7 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
 	sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group);
 	sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group);
 #endif
+
+	if (dev_priv->clients.root)
+		kobject_put(dev_priv->clients.root);
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
  2020-01-10 13:30 ` [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:39   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness Tvrtko Ursulin
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Some clients have the DRM fd passed to them over a socket by the X server.

Grab the real client and pid when they create their first context and
update the exposed data for more useful enumeration.

v2:
 * Do not leak the pid reference and borrow context idr_lock. (Chris)

v3:
 * More avoiding leaks. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 36 ++++++++++++++++++---
 drivers/gpu/drm/i915/i915_drm_client.c      |  4 +--
 drivers/gpu/drm/i915/i915_drm_client.h      |  4 +++
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index a2e57e62af30..ba3c29a01535 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,7 @@
 #include "gt/intel_lrc_reg.h"
 #include "gt/intel_ring.h"
 
+#include "i915_drm_client.h"
 #include "i915_gem_context.h"
 #include "i915_globals.h"
 #include "i915_trace.h"
@@ -2181,7 +2182,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct drm_i915_gem_context_create_ext *args = data;
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct i915_drm_client *client = file_priv->client;
 	struct create_ext ext_data;
+	struct pid *pid;
 	int ret;
 	u32 id;
 
@@ -2195,16 +2199,35 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	ext_data.fpriv = file->driver_priv;
+	pid = get_task_pid(current, PIDTYPE_PID);
+
+	ext_data.fpriv = file_priv;
 	if (client_is_banned(ext_data.fpriv)) {
 		DRM_DEBUG("client %s[%d] banned from creating ctx\n",
-			  current->comm, task_pid_nr(current));
-		return -EIO;
+			  current->comm, pid_nr(pid));
+		ret = -EIO;
+		goto err_pid;
+	}
+
+	/*
+	 * Borrow struct_mutex to protect the client remove-add cycle.
+	 */
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		goto err_pid;
+	if (client->pid != pid) {
+		__i915_drm_client_unregister(client);
+		ret = __i915_drm_client_register(client, current);
 	}
+	mutex_unlock(&dev->struct_mutex);
+	if (ret)
+		goto err_pid;
 
 	ext_data.ctx = i915_gem_create_context(i915, args->flags);
-	if (IS_ERR(ext_data.ctx))
-		return PTR_ERR(ext_data.ctx);
+	if (IS_ERR(ext_data.ctx)) {
+		ret = PTR_ERR(ext_data.ctx);
+		goto err_pid;
+	}
 
 	if (args->flags & I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS) {
 		ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
@@ -2226,6 +2249,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 
 err_ctx:
 	context_close(ext_data.ctx);
+err_pid:
+	put_pid(pid);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 02356f48d85b..666ec67c77e9 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -36,7 +36,7 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
 			client->closed ? ">" : "");
 }
 
-static int
+int
 __i915_drm_client_register(struct i915_drm_client *client,
 			   struct task_struct *task)
 {
@@ -89,7 +89,7 @@ __i915_drm_client_register(struct i915_drm_client *client,
 	return ret;
 }
 
-static void __i915_drm_client_unregister(struct i915_drm_client *client)
+void __i915_drm_client_unregister(struct i915_drm_client *client)
 {
 	if (!client->name)
 		return; /* fbdev client or error during drm open */
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 8b261dc4a1f3..2c692345bc4e 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -58,4 +58,8 @@ void i915_drm_client_close(struct i915_drm_client *client);
 struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients,
 					    struct task_struct *task);
 
+int __i915_drm_client_register(struct i915_drm_client *client,
+			       struct task_struct *task);
+void __i915_drm_client_unregister(struct i915_drm_client *client);
+
 #endif /* !__I915_DRM_CLIENT_H__ */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
  2020-01-10 13:30 ` [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
  2020-01-10 13:30 ` [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:46   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 4/8] drm/i915: Track all user contexts per client Tvrtko Ursulin
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Some customers want to know how much of the GPU time are their clients
using in order to make dynamic load balancing decisions.

With the hooks already in place which track the overall engine busyness,
we can extend that slightly to split that time between contexts.

v2: Fix accounting for tail updates.
v3: Rebase.
v4: Mark currently running contexts as active on stats enable.
v5: Include some headers to fix the build.
v6: Added fine grained lock.
v7: Convert to seqlock. (Chris Wilson)
v8: Rebase and tidy with helpers.
v9: Refactor.
v10: Move recording start to promotion. (Chris)
v11: Consolidate duplicated code. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.c       | 20 ++++++++
 drivers/gpu/drm/i915/gt/intel_context.h       | 11 +++++
 drivers/gpu/drm/i915/gt/intel_context_types.h |  9 ++++
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 16 ++++++-
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 46 ++++++++++++++++---
 5 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 9796a54b4f47..93894460f008 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -248,6 +248,7 @@ intel_context_init(struct intel_context *ce,
 	INIT_LIST_HEAD(&ce->signals);
 
 	mutex_init(&ce->pin_mutex);
+	seqlock_init(&ce->stats.lock);
 
 	i915_active_init(&ce->active,
 			 __intel_context_active, __intel_context_retire);
@@ -342,6 +343,25 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
 	return rq;
 }
 
+ktime_t intel_context_get_busy_time(struct intel_context *ce)
+{
+	unsigned int seq;
+	ktime_t total;
+
+	do {
+		seq = read_seqbegin(&ce->stats.lock);
+
+		total = ce->stats.total;
+
+		if (ce->stats.active)
+			total = ktime_add(total,
+					  ktime_sub(ktime_get(),
+						    ce->stats.start));
+	} while (read_seqretry(&ce->stats.lock, seq));
+
+	return total;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftest_context.c"
 #endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 30bd248827d8..30f0268fcc9a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -224,4 +224,15 @@ intel_context_clear_nopreempt(struct intel_context *ce)
 	clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
 }
 
+static inline void
+__intel_context_stats_start(struct intel_context_stats *stats, ktime_t now)
+{
+	if (!stats->active) {
+		stats->start = now;
+		stats->active = true;
+	}
+}
+
+ktime_t intel_context_get_busy_time(struct intel_context *ce);
+
 #endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index ca1420fb8b53..963d33dc5289 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -11,6 +11,7 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/types.h>
+#include <linux/seqlock.h>
 
 #include "i915_active_types.h"
 #include "i915_utils.h"
@@ -83,6 +84,14 @@ struct intel_context {
 
 	/** sseu: Control eu/slice partitioning */
 	struct intel_sseu sseu;
+
+	/** stats: Context GPU engine busyness tracking. */
+	struct intel_context_stats {
+		seqlock_t lock;
+		bool active;
+		ktime_t start;
+		ktime_t total;
+	} stats;
 };
 
 #endif /* __INTEL_CONTEXT_TYPES__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 825c94e7ca0b..9a346c060469 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1543,8 +1543,20 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 
 		engine->stats.enabled_at = ktime_get();
 
-		/* XXX submission method oblivious? */
-		for (port = execlists->active; (rq = *port); port++)
+		/*
+		 * Mark currently running context as active.
+		 * XXX submission method oblivious?
+		 */
+
+		rq = NULL;
+		port = execlists->active;
+		if (port)
+			rq = *port;
+		if (rq)
+			__intel_context_stats_start(&rq->context->stats,
+						    engine->stats.enabled_at);
+
+		for (; (rq = *port); port++)
 			engine->stats.active++;
 
 		for (port = execlists->pending; (rq = *port); port++) {
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 9af1b2b493f4..dd559547500f 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1195,6 +1195,32 @@ static void reset_active(struct i915_request *rq,
 	ce->lrc_desc |= CTX_DESC_FORCE_RESTORE;
 }
 
+static void
+intel_context_stats_start(struct intel_context_stats *stats)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&stats->lock, flags);
+	__intel_context_stats_start(stats, ktime_get());
+	write_sequnlock_irqrestore(&stats->lock, flags);
+}
+
+static void
+intel_context_stats_stop(struct intel_context_stats *stats)
+{
+	unsigned long flags;
+
+	if (!READ_ONCE(stats->active))
+		return;
+
+	write_seqlock_irqsave(&stats->lock, flags);
+	GEM_BUG_ON(!READ_ONCE(stats->active));
+	stats->total = ktime_add(stats->total,
+				 ktime_sub(ktime_get(), stats->start));
+	stats->active = false;
+	write_sequnlock_irqrestore(&stats->lock, flags);
+}
+
 static inline struct intel_engine_cs *
 __execlists_schedule_in(struct i915_request *rq)
 {
@@ -1262,7 +1288,7 @@ static inline void
 __execlists_schedule_out(struct i915_request *rq,
 			 struct intel_engine_cs * const engine)
 {
-	struct intel_context * const ce = rq->context;
+	struct intel_context *ce = rq->context;
 
 	/*
 	 * NB process_csb() is not under the engine->active.lock and hence
@@ -1279,6 +1305,7 @@ __execlists_schedule_out(struct i915_request *rq,
 		intel_engine_add_retire(engine, ce->timeline);
 
 	intel_engine_context_out(engine);
+	intel_context_stats_stop(&ce->stats);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 	intel_gt_pm_put_async(engine->gt);
 
@@ -2250,6 +2277,7 @@ static void process_csb(struct intel_engine_cs *engine)
 	rmb();
 
 	do {
+		struct i915_request *rq;
 		bool promote;
 
 		if (++head == num_entries)
@@ -2305,7 +2333,11 @@ static void process_csb(struct intel_engine_cs *engine)
 
 			WRITE_ONCE(execlists->pending[0], NULL);
 		} else {
-			GEM_BUG_ON(!*execlists->active);
+			rq = *execlists->active++;
+			GEM_BUG_ON(!rq);
+
+			GEM_BUG_ON(execlists->active - execlists->inflight >
+				   execlists_num_ports(execlists));
 
 			/* port0 completed, advanced to port1 */
 			trace_ports(execlists, "completed", execlists->active);
@@ -2316,13 +2348,15 @@ static void process_csb(struct intel_engine_cs *engine)
 			 * coherent (visible from the CPU) before the
 			 * user interrupt and CSB is processed.
 			 */
-			GEM_BUG_ON(!i915_request_completed(*execlists->active) &&
+			GEM_BUG_ON(!i915_request_completed(rq) &&
 				   !reset_in_progress(execlists));
-			execlists_schedule_out(*execlists->active++);
 
-			GEM_BUG_ON(execlists->active - execlists->inflight >
-				   execlists_num_ports(execlists));
+			execlists_schedule_out(rq);
 		}
+
+		rq = *execlists->active;
+		if (rq)
+			intel_context_stats_start(&rq->context->stats);
 	} while (head != tail);
 
 	execlists->csb_head = head;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 4/8] drm/i915: Track all user contexts per client
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:30 ` [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client Tvrtko Ursulin
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We soon want to start answering questions like how much GPU time is the
context belonging to a client which exited still using.

To enable this we start tracking all context belonging to a client on a
separate list, plus we make contexts take a reference on their clients
file_priv.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 21 ++++++++++++++++++-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  6 ++++++
 drivers/gpu/drm/i915/i915_drm_client.c        |  3 +++
 drivers/gpu/drm/i915/i915_drm_client.h        |  5 +++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index ba3c29a01535..ba8ccc754f20 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -301,8 +301,18 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
 
 static void i915_gem_context_free(struct i915_gem_context *ctx)
 {
+	struct i915_drm_client *client = ctx->client;
+
 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
 
+	if (client) {
+		spin_lock(&client->ctx_lock);
+		list_del_rcu(&ctx->client_link);
+		spin_unlock(&client->ctx_lock);
+
+		i915_drm_client_put(client);
+	}
+
 	spin_lock(&ctx->i915->gem.contexts.lock);
 	list_del(&ctx->link);
 	spin_unlock(&ctx->i915->gem.contexts.lock);
@@ -772,6 +782,7 @@ static int gem_context_register(struct i915_gem_context *ctx,
 				struct drm_i915_file_private *fpriv,
 				u32 *id)
 {
+	struct i915_drm_client *client;
 	struct i915_address_space *vm;
 	int ret;
 
@@ -789,9 +800,17 @@ static int gem_context_register(struct i915_gem_context *ctx,
 
 	/* And finally expose ourselves to userspace via the idr */
 	ret = xa_alloc(&fpriv->context_xa, id, ctx, xa_limit_32b, GFP_KERNEL);
-	if (ret)
+	if (ret) {
 		put_pid(fetch_and_zero(&ctx->pid));
+		goto out;
+	}
 
+	ctx->client = client = i915_drm_client_get(fpriv->client);
+	spin_lock(&client->ctx_lock);
+	list_add_tail_rcu(&ctx->client_link, &client->ctx_list);
+	spin_unlock(&client->ctx_lock);
+
+out:
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 017ca803ab47..879824159646 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -104,6 +104,12 @@ struct i915_gem_context {
 	struct list_head link;
 	struct llist_node free_link;
 
+	/** client: struct i915_drm_client */
+	struct i915_drm_client *client;
+
+	/** link: &fpriv.context_list */
+	struct list_head client_link;
+
 	/**
 	 * @ref: reference count
 	 *
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 666ec67c77e9..195777b95891 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -111,6 +111,9 @@ i915_drm_client_add(struct i915_drm_clients *clients, struct task_struct *task)
 		return ERR_PTR(-ENOMEM);
 
 	kref_init(&client->kref);
+	spin_lock_init(&client->ctx_lock);
+	INIT_LIST_HEAD(&client->ctx_list);
+
 	client->clients = clients;
 
 	ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 2c692345bc4e..16d8db075a7d 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -10,9 +10,11 @@
 #include <linux/device.h>
 #include <linux/kobject.h>
 #include <linux/kref.h>
+#include <linux/list.h>
 #include <linux/pid.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
+#include <linux/spinlock.h>
 #include <linux/xarray.h>
 
 struct i915_drm_clients {
@@ -30,6 +32,9 @@ struct i915_drm_client {
 	char *name;
 	bool closed;
 
+	spinlock_t ctx_lock;
+	struct list_head ctx_list;
+
 	struct i915_drm_clients *clients;
 
 	struct kobject *root;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 4/8] drm/i915: Track all user contexts per client Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:42   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Now that contexts keep their parent client reference counted, we can
remove separate struct pid reference owned by contexts in favour of the
one already held by the client.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 ++++---------
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 10 ----------
 drivers/gpu/drm/i915/i915_debugfs.c               |  7 ++++---
 drivers/gpu/drm/i915/i915_gpu_error.c             |  8 ++++----
 4 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index ba8ccc754f20..758cebb99ba4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -323,7 +323,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
 	if (ctx->timeline)
 		intel_timeline_put(ctx->timeline);
 
-	put_pid(ctx->pid);
 	mutex_destroy(&ctx->mutex);
 
 	kfree_rcu(ctx, rcu);
@@ -794,24 +793,20 @@ static int gem_context_register(struct i915_gem_context *ctx,
 		WRITE_ONCE(vm->file, fpriv); /* XXX */
 	mutex_unlock(&ctx->mutex);
 
-	ctx->pid = get_task_pid(current, PIDTYPE_PID);
 	snprintf(ctx->name, sizeof(ctx->name), "%s[%d]",
-		 current->comm, pid_nr(ctx->pid));
+		 current->comm, pid_nr(client->pid));
 
 	/* And finally expose ourselves to userspace via the idr */
 	ret = xa_alloc(&fpriv->context_xa, id, ctx, xa_limit_32b, GFP_KERNEL);
-	if (ret) {
-		put_pid(fetch_and_zero(&ctx->pid));
-		goto out;
-	}
+	if (ret)
+		return ret;
 
 	ctx->client = client = i915_drm_client_get(fpriv->client);
 	spin_lock(&client->ctx_lock);
 	list_add_tail_rcu(&ctx->client_link, &client->ctx_list);
 	spin_unlock(&client->ctx_lock);
 
-out:
-	return ret;
+	return 0;
 }
 
 int i915_gem_context_open(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 879824159646..23421377a43d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -90,16 +90,6 @@ struct i915_gem_context {
 	 */
 	struct i915_address_space __rcu *vm;
 
-	/**
-	 * @pid: process id of creator
-	 *
-	 * Note that who created the context may not be the principle user,
-	 * as the context may be shared across a local socket. However,
-	 * that should only affect the default context, all contexts created
-	 * explicitly by the client are expected to be isolated.
-	 */
-	struct pid *pid;
-
 	/** link: place with &drm_i915_private.context_list */
 	struct list_head link;
 	struct llist_node free_link;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8f01c2bc7355..bc533501b4e0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -346,7 +346,8 @@ static void print_context_stats(struct seq_file *m,
 			rcu_read_unlock();
 
 			rcu_read_lock();
-			task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID);
+			task = pid_task(ctx->client->pid ?: file->pid,
+					PIDTYPE_PID);
 			snprintf(name, sizeof(name), "%s",
 				 task ? task->comm : "<unknown>");
 			rcu_read_unlock();
@@ -1492,10 +1493,10 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		spin_unlock(&i915->gem.contexts.lock);
 
 		seq_puts(m, "HW context ");
-		if (ctx->pid) {
+		if (ctx->client->pid) {
 			struct task_struct *task;
 
-			task = get_pid_task(ctx->pid, PIDTYPE_PID);
+			task = get_pid_task(ctx->client->pid, PIDTYPE_PID);
 			if (task) {
 				seq_printf(m, "(%s [%d]) ",
 					   task->comm, task->pid);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index fda0977d2059..9240327bdb7d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1235,8 +1235,8 @@ static void record_request(const struct i915_request *request,
 	erq->pid = 0;
 	rcu_read_lock();
 	ctx = rcu_dereference(request->context->gem_context);
-	if (ctx)
-		erq->pid = pid_nr(ctx->pid);
+	if (ctx && ctx->client->pid)
+		erq->pid = pid_nr(ctx->client->pid);
 	rcu_read_unlock();
 }
 
@@ -1313,11 +1313,11 @@ static bool record_context(struct drm_i915_error_context *e,
 	if (ctx && !kref_get_unless_zero(&ctx->ref))
 		ctx = NULL;
 	rcu_read_unlock();
-	if (!ctx)
+	if (!ctx || !ctx->client->pid)
 		return false;
 
 	rcu_read_lock();
-	task = pid_task(ctx->pid, PIDTYPE_PID);
+	task = pid_task(ctx->client->pid, PIDTYPE_PID);
 	if (task) {
 		strcpy(e->comm, task->comm);
 		e->pid = task->pid;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 13:58   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime Tvrtko Ursulin
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Expose per-client and per-engine busyness under the previously added sysfs
client root.

The new files are one per-engine instance and located under the 'busy'
directory. Each contains a monotonically increasing nano-second resolution
times each client's jobs were executing on the GPU.

This enables userspace to create a top-like tool for GPU utilization:

==========================================================================
intel-gpu-top -  935/ 935 MHz;    0% RC6; 14.73 Watts;     1097 irqs/s

      IMC reads:     1401 MiB/s
     IMC writes:        4 MiB/s

          ENGINE      BUSY                                 MI_SEMA MI_WAIT
     Render/3D/0   63.73% |███████████████████           |      3%      0%
       Blitter/0    9.53% |██▊                           |      6%      0%
         Video/0   39.32% |███████████▊                  |     16%      0%
         Video/1   15.62% |████▋                         |      0%      0%
  VideoEnhance/0    0.00% |                              |      0%      0%

  PID            NAME     RCS          BCS          VCS         VECS
 4084        gem_wsim |█████▌     ||█          ||           ||           |
 4086        gem_wsim |█▌         ||           ||███        ||           |
==========================================================================

v2: Use intel_context_engine_get_busy_time.
v3: New directory structure.
v4: Rebase.
v5: sysfs_attr_init.
v6: Small tidy in i915_gem_add_client.
v7: Rebase to be engine class based.
v8:
 * Always enable stats.
 * Walk all client contexts.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drm_client.c | 127 ++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drm_client.h |  13 +++
 2 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 195777b95891..55b2f86cc4c1 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -8,7 +8,11 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include <uapi/drm/i915_drm.h>
+
 #include "i915_drm_client.h"
+#include "gem/i915_gem_context.h"
+#include "i915_drv.h"
 #include "i915_gem.h"
 #include "i915_utils.h"
 
@@ -36,13 +40,62 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
 			client->closed ? ">" : "");
 }
 
+static u64
+sw_busy_add(struct i915_gem_context *ctx, unsigned int engine_class)
+{
+	struct i915_gem_engines *engines = rcu_dereference(ctx->engines);
+	struct i915_gem_engines_iter it;
+	struct intel_context *ce;
+	u64 total = 0;
+
+	for_each_gem_engine(ce, engines, it) {
+		if (ce->engine->uabi_class != engine_class)
+			continue;
+
+		total += ktime_to_ns(intel_context_get_busy_time(ce));
+	}
+
+	return total;
+}
+
+static ssize_t
+show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct i915_engine_busy_attribute *i915_attr =
+		container_of(attr, typeof(*i915_attr), attr);
+	struct list_head *list = &i915_attr->client->ctx_list;
+	unsigned int engine_class = i915_attr->engine_class;
+	struct i915_gem_context *ctx;
+	u64 total = 0;
+
+	if (i915_attr->no_busy_stats)
+		return -ENODEV;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(ctx, list, client_link)
+		total += sw_busy_add(ctx, engine_class);
+	rcu_read_unlock();
+
+	return snprintf(buf, PAGE_SIZE, "%llu\n", total);
+}
+
+static const char *uabi_class_names[] = {
+	[I915_ENGINE_CLASS_RENDER] = "0",
+	[I915_ENGINE_CLASS_COPY] = "1",
+	[I915_ENGINE_CLASS_VIDEO] = "2",
+	[I915_ENGINE_CLASS_VIDEO_ENHANCE] = "3",
+};
+
 int
 __i915_drm_client_register(struct i915_drm_client *client,
 			   struct task_struct *task)
 {
 	struct i915_drm_clients *clients = client->clients;
+	struct drm_i915_private *i915 =
+		container_of(clients, typeof(*i915), clients);
+	struct intel_engine_cs *engine;
 	struct device_attribute *attr;
-	int ret = -ENOMEM;
+	int i, ret = -ENOMEM;
 	char idstr[32];
 
 	if (!clients->root)
@@ -77,10 +130,71 @@ __i915_drm_client_register(struct i915_drm_client *client,
 	if (ret)
 		goto err_attr;
 
+       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
+		client->busy_root =
+			kobject_create_and_add("busy", client->root);
+		if (!client->busy_root)
+			goto err_attr;
+
+		for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++) {
+			struct i915_engine_busy_attribute *i915_attr =
+				&client->attr.busy[i];
+
+			i915_attr->client = client;
+			i915_attr->engine_class = i;
+
+			attr = &i915_attr->attr;
+
+			sysfs_attr_init(&attr->attr);
+
+			attr->attr.name = uabi_class_names[i];
+			attr->attr.mode = 0444;
+			attr->show = show_client_busy;
+
+			ret = sysfs_create_file(client->busy_root,
+						(struct attribute *)attr);
+			if (ret)
+				goto err_busy;
+		}
+
+		/* Enable busy stats on all engines. */
+		i = 0;
+		for_each_uabi_engine(engine, i915) {
+			ret = intel_enable_engine_stats(engine);
+			if (ret) {
+				int j, k;
+
+				/* Unwind if not available. */
+				j = 0;
+				for_each_uabi_engine(engine, i915) {
+					if (j++ == i)
+						break;
+
+					intel_disable_engine_stats(engine);
+				}
+
+				for (k = 0;
+				     k < ARRAY_SIZE(uabi_class_names);
+				     k++) {
+					GEM_WARN_ON(client->attr.busy[k].no_busy_stats);
+					client->attr.busy[k].no_busy_stats = true;
+				}
+
+				dev_notice_once(i915->drm.dev,
+						"Engine busy stats not available! (%d)",
+						ret);
+				break;
+			}
+			i++;
+		}
+       }
+
 	client->pid = get_task_pid(task, PIDTYPE_PID);
 
 	return 0;
 
+err_busy:
+	kobject_put(client->busy_root);
 err_attr:
 	kobject_put(client->root);
 err_client:
@@ -91,9 +205,20 @@ __i915_drm_client_register(struct i915_drm_client *client,
 
 void __i915_drm_client_unregister(struct i915_drm_client *client)
 {
+	struct i915_drm_clients *clients = client->clients;
+	struct drm_i915_private *i915 =
+		container_of(clients, typeof(*i915), clients);
+	struct intel_engine_cs *engine;
+
 	if (!client->name)
 		return; /* fbdev client or error during drm open */
 
+	if (client->busy_root && !client->attr.busy[0].no_busy_stats) {
+		for_each_uabi_engine(engine, i915)
+			intel_disable_engine_stats(engine);
+	}
+
+	kobject_put(fetch_and_zero(&client->busy_root));
 	kobject_put(fetch_and_zero(&client->root));
 	put_pid(fetch_and_zero(&client->pid));
 	kfree(fetch_and_zero(&client->name));
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 16d8db075a7d..4b4b9ea0abdf 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -17,11 +17,22 @@
 #include <linux/spinlock.h>
 #include <linux/xarray.h>
 
+#include "gt/intel_engine_types.h"
+
 struct i915_drm_clients {
 	struct xarray xarray;
 	struct kobject *root;
 };
 
+struct i915_drm_client;
+
+struct i915_engine_busy_attribute {
+	struct device_attribute attr;
+	struct i915_drm_client *client;
+	unsigned int engine_class;
+	bool no_busy_stats;
+};
+
 struct i915_drm_client {
 	struct kref kref;
 
@@ -38,9 +49,11 @@ struct i915_drm_client {
 	struct i915_drm_clients *clients;
 
 	struct kobject *root;
+	struct kobject *busy_root;
 	struct {
 		struct device_attribute pid;
 		struct device_attribute name;
+		struct i915_engine_busy_attribute busy[MAX_ENGINE_CLASS];
 	} attr;
 };
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 14:03   ` Chris Wilson
  2020-01-10 13:30 ` [Intel-gfx] [RFC 8/8] drm/i915: Fallback to hw context runtime when sw tracking is not available Tvrtko Ursulin
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

GPU saves accumulated context runtime (in CS timestamp units) in PPHWSP
which will be useful for us in cases when we are not able to track context
busyness ourselves (like with GuC). Keep a copy of this in struct
intel_context from where it can be easily read even if the context is not
pinned.

QQQ: Do we want to make this accounting conditional / able to turn on/off?

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.h       | 7 +++++++
 drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++++
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 9 +++++++++
 drivers/gpu/drm/i915/intel_device_info.c      | 2 ++
 drivers/gpu/drm/i915/intel_device_info.h      | 1 +
 5 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 30f0268fcc9a..389a05736fc7 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 
 #include "i915_active.h"
+#include "i915_drv.h"
 #include "intel_context_types.h"
 #include "intel_engine_types.h"
 #include "intel_ring_types.h"
@@ -235,4 +236,10 @@ __intel_context_stats_start(struct intel_context_stats *stats, ktime_t now)
 
 ktime_t intel_context_get_busy_time(struct intel_context *ce);
 
+static inline u64 intel_context_get_hw_runtime_ns(struct intel_context *ce)
+{
+	return ce->total_runtime *
+	       RUNTIME_INFO(ce->engine->i915)->cs_timestamp_period_ns;
+}
+
 #endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 963d33dc5289..7b08bf87fb82 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -69,6 +69,11 @@ struct intel_context {
 	u64 lrc_desc;
 	u32 tag; /* cookie passed to HW to track this context on submission */
 
+	/* Time on GPU as tracked by the hw. */
+	u32 last_runtime;
+	u64 total_runtime;
+	u32 *pphwsp;
+
 	unsigned int active_count; /* protected by timeline->mutex */
 
 	atomic_t pin_count;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index dd559547500f..26999b43e5a1 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1289,6 +1289,7 @@ __execlists_schedule_out(struct i915_request *rq,
 			 struct intel_engine_cs * const engine)
 {
 	struct intel_context *ce = rq->context;
+	u32 old, new;
 
 	/*
 	 * NB process_csb() is not under the engine->active.lock and hence
@@ -1309,6 +1310,13 @@ __execlists_schedule_out(struct i915_request *rq,
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 	intel_gt_pm_put_async(engine->gt);
 
+	old = ce->last_runtime;
+	ce->last_runtime = new = ce->pphwsp[16];
+	if (new > old)
+		ce->total_runtime += new - old;
+	else
+		ce->total_runtime += (~0UL - old) + new + 1;
+
 	/*
 	 * If this is part of a virtual engine, its next request may
 	 * have been blocked waiting for access to the active context.
@@ -2608,6 +2616,7 @@ __execlists_context_pin(struct intel_context *ce,
 
 	ce->lrc_desc = lrc_descriptor(ce, engine) | CTX_DESC_FORCE_RESTORE;
 	ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
+	ce->pphwsp = vaddr + LRC_PPHWSP_PN * PAGE_SIZE;
 	__execlists_update_reg_state(ce, engine);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 6670a0763be2..7732748e1939 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -1042,6 +1042,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 
 	/* Initialize command stream timestamp frequency */
 	runtime->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
+	runtime->cs_timestamp_period_ns =
+		div_u64(1e6, runtime->cs_timestamp_frequency_khz);
 }
 
 void intel_driver_caps_print(const struct intel_driver_caps *caps,
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 2725cb7fc169..9ec816dbc418 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -216,6 +216,7 @@ struct intel_runtime_info {
 	struct sseu_dev_info sseu;
 
 	u32 cs_timestamp_frequency_khz;
+	u32 cs_timestamp_period_ns;
 
 	/* Media engine access to SFC per instance */
 	u8 vdbox_sfc_access;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 8/8] drm/i915: Fallback to hw context runtime when sw tracking is not available
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime Tvrtko Ursulin
@ 2020-01-10 13:30 ` Tvrtko Ursulin
  2020-01-10 16:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Per client engine busyness (rev3) Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 13:30 UTC (permalink / raw)
  To: Intel-gfx; +Cc: kui.wen

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In GuC mode we are not receiving the context switch interrupts to be able
to accurately track context runtimes.

We can fallback to using PPHWSP counter updated by the GPU on context save.

QQQ
Downsides are: 1) we do not see currently executing batch and 2) with a
12MHz command streamer timestamp timer frequency the 32-bit counter wraps
every ~358 seconds. This makes endless OpenCL batches with hearbeats
turned off also a problem.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drm_client.c | 34 ++++++++++++++++++++------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 55b2f86cc4c1..0b84ae528dcc 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -58,6 +58,24 @@ sw_busy_add(struct i915_gem_context *ctx, unsigned int engine_class)
 	return total;
 }
 
+static u64
+hw_busy_add(struct i915_gem_context *ctx, unsigned int engine_class)
+{
+	struct i915_gem_engines *engines = rcu_dereference(ctx->engines);
+	struct i915_gem_engines_iter it;
+	struct intel_context *ce;
+	u64 total = 0;
+
+	for_each_gem_engine(ce, engines, it) {
+		if (ce->engine->uabi_class != engine_class)
+			continue;
+
+		total += intel_context_get_hw_runtime_ns(ce);
+	}
+
+	return total;
+}
+
 static ssize_t
 show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
 {
@@ -68,12 +86,14 @@ show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
 	struct i915_gem_context *ctx;
 	u64 total = 0;
 
-	if (i915_attr->no_busy_stats)
-		return -ENODEV;
-
 	rcu_read_lock();
-	list_for_each_entry_rcu(ctx, list, client_link)
-		total += sw_busy_add(ctx, engine_class);
+	if (i915_attr->no_busy_stats) {
+		list_for_each_entry_rcu(ctx, list, client_link)
+			total += hw_busy_add(ctx, engine_class);
+	} else {
+		list_for_each_entry_rcu(ctx, list, client_link)
+			total += sw_busy_add(ctx, engine_class);
+	}
 	rcu_read_unlock();
 
 	return snprintf(buf, PAGE_SIZE, "%llu\n", total);
@@ -164,7 +184,7 @@ __i915_drm_client_register(struct i915_drm_client *client,
 			if (ret) {
 				int j, k;
 
-				/* Unwind if not available. */
+				/* Unwind and fallback if not available. */
 				j = 0;
 				for_each_uabi_engine(engine, i915) {
 					if (j++ == i)
@@ -181,7 +201,7 @@ __i915_drm_client_register(struct i915_drm_client *client,
 				}
 
 				dev_notice_once(i915->drm.dev,
-						"Engine busy stats not available! (%d)",
+						"Reduced accuracy context runtime mode (%d)",
 						ret);
 				break;
 			}
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs
  2020-01-10 13:30 ` [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
@ 2020-01-10 13:36   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 13:36 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:42)
> +static ssize_t
> +show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> +       struct i915_drm_client *client =
> +               container_of(attr, typeof(*client), attr.name);
> +
> +       return snprintf(buf, PAGE_SIZE, "%s%s%s",
> +                       client->closed ? "<" : "",
> +                       client->name,
> +                       client->closed ? ">" : "");

client->closed ? "<%s>" : "%s", unspeakably evil?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create
  2020-01-10 13:30 ` [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create Tvrtko Ursulin
@ 2020-01-10 13:39   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 13:39 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:43)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Some clients have the DRM fd passed to them over a socket by the X server.
> 
> Grab the real client and pid when they create their first context and
> update the exposed data for more useful enumeration.
> 
> v2:
>  * Do not leak the pid reference and borrow context idr_lock. (Chris)
> 
> v3:
>  * More avoiding leaks. (Chris)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 36 ++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_drm_client.c      |  4 +--
>  drivers/gpu/drm/i915/i915_drm_client.h      |  4 +++
>  3 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index a2e57e62af30..ba3c29a01535 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -77,6 +77,7 @@
>  #include "gt/intel_lrc_reg.h"
>  #include "gt/intel_ring.h"
>  
> +#include "i915_drm_client.h"
>  #include "i915_gem_context.h"
>  #include "i915_globals.h"
>  #include "i915_trace.h"
> @@ -2181,7 +2182,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  {
>         struct drm_i915_private *i915 = to_i915(dev);
>         struct drm_i915_gem_context_create_ext *args = data;
> +       struct drm_i915_file_private *file_priv = file->driver_priv;
> +       struct i915_drm_client *client = file_priv->client;
>         struct create_ext ext_data;
> +       struct pid *pid;
>         int ret;
>         u32 id;
>  
> @@ -2195,16 +2199,35 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>         if (ret)
>                 return ret;
>  
> -       ext_data.fpriv = file->driver_priv;
> +       pid = get_task_pid(current, PIDTYPE_PID);
> +
> +       ext_data.fpriv = file_priv;

Might as well do this in the declaration ?

struct create_ext ext_data {
	.fpriv = file->driver_priv,
};
struct i915_drm_client *client = ext_data.fpriv->client;

?

>         if (client_is_banned(ext_data.fpriv)) {
>                 DRM_DEBUG("client %s[%d] banned from creating ctx\n",
> -                         current->comm, task_pid_nr(current));
> -               return -EIO;
> +                         current->comm, pid_nr(pid));
> +               ret = -EIO;
> +               goto err_pid;
> +       }
> +
> +       /*
> +        * Borrow struct_mutex to protect the client remove-add cycle.
> +        */
> +       ret = mutex_lock_interruptible(&dev->struct_mutex);
> +       if (ret)
> +               goto err_pid;
> +       if (client->pid != pid) {
> +               __i915_drm_client_unregister(client);
> +               ret = __i915_drm_client_register(client, current);
>         }
> +       mutex_unlock(&dev->struct_mutex);
> +       if (ret)
> +               goto err_pid;

-> i915_drm_client_update();

And let it manage the pid locally?

>  
>         ext_data.ctx = i915_gem_create_context(i915, args->flags);
> -       if (IS_ERR(ext_data.ctx))
> -               return PTR_ERR(ext_data.ctx);
> +       if (IS_ERR(ext_data.ctx)) {
> +               ret = PTR_ERR(ext_data.ctx);
> +               goto err_pid;
> +       }
>  
>         if (args->flags & I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS) {
>                 ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
> @@ -2226,6 +2249,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  
>  err_ctx:
>         context_close(ext_data.ctx);
> +err_pid:
> +       put_pid(pid);
> +
>         return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> index 02356f48d85b..666ec67c77e9 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -36,7 +36,7 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
>                         client->closed ? ">" : "");
>  }
>  
> -static int
> +int
>  __i915_drm_client_register(struct i915_drm_client *client,
>                            struct task_struct *task)
>  {
> @@ -89,7 +89,7 @@ __i915_drm_client_register(struct i915_drm_client *client,
>         return ret;
>  }
>  
> -static void __i915_drm_client_unregister(struct i915_drm_client *client)
> +void __i915_drm_client_unregister(struct i915_drm_client *client)
>  {
>         if (!client->name)
>                 return; /* fbdev client or error during drm open */
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
> index 8b261dc4a1f3..2c692345bc4e 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.h
> +++ b/drivers/gpu/drm/i915/i915_drm_client.h
> @@ -58,4 +58,8 @@ void i915_drm_client_close(struct i915_drm_client *client);
>  struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients,
>                                             struct task_struct *task);
>  
> +int __i915_drm_client_register(struct i915_drm_client *client,
> +                              struct task_struct *task);
> +void __i915_drm_client_unregister(struct i915_drm_client *client);
> +
>  #endif /* !__I915_DRM_CLIENT_H__ */
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client
  2020-01-10 13:30 ` [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client Tvrtko Ursulin
@ 2020-01-10 13:42   ` Chris Wilson
  2020-01-30 16:11     ` Tvrtko Ursulin
  0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 13:42 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:46)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Now that contexts keep their parent client reference counted, we can
> remove separate struct pid reference owned by contexts in favour of the
> one already held by the client.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 ++++---------
>  drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 10 ----------
>  drivers/gpu/drm/i915/i915_debugfs.c               |  7 ++++---
>  drivers/gpu/drm/i915/i915_gpu_error.c             |  8 ++++----
>  4 files changed, 12 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index ba8ccc754f20..758cebb99ba4 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -323,7 +323,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
>         if (ctx->timeline)
>                 intel_timeline_put(ctx->timeline);
>  
> -       put_pid(ctx->pid);
>         mutex_destroy(&ctx->mutex);
>  
>         kfree_rcu(ctx, rcu);
> @@ -794,24 +793,20 @@ static int gem_context_register(struct i915_gem_context *ctx,
>                 WRITE_ONCE(vm->file, fpriv); /* XXX */
>         mutex_unlock(&ctx->mutex);
>  
> -       ctx->pid = get_task_pid(current, PIDTYPE_PID);
>         snprintf(ctx->name, sizeof(ctx->name), "%s[%d]",
> -                current->comm, pid_nr(ctx->pid));
> +                current->comm, pid_nr(client->pid));
>  
>         /* And finally expose ourselves to userspace via the idr */
>         ret = xa_alloc(&fpriv->context_xa, id, ctx, xa_limit_32b, GFP_KERNEL);
> -       if (ret) {
> -               put_pid(fetch_and_zero(&ctx->pid));
> -               goto out;
> -       }
> +       if (ret)
> +               return ret;
>  
>         ctx->client = client = i915_drm_client_get(fpriv->client);
>         spin_lock(&client->ctx_lock);
>         list_add_tail_rcu(&ctx->client_link, &client->ctx_list);
>         spin_unlock(&client->ctx_lock);
>  
> -out:
> -       return ret;
> +       return 0;
>  }
>  
>  int i915_gem_context_open(struct drm_i915_private *i915,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 879824159646..23421377a43d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -90,16 +90,6 @@ struct i915_gem_context {
>          */
>         struct i915_address_space __rcu *vm;
>  
> -       /**
> -        * @pid: process id of creator
> -        *
> -        * Note that who created the context may not be the principle user,
> -        * as the context may be shared across a local socket. However,
> -        * that should only affect the default context, all contexts created
> -        * explicitly by the client are expected to be isolated.
> -        */
> -       struct pid *pid;
> -
>         /** link: place with &drm_i915_private.context_list */
>         struct list_head link;
>         struct llist_node free_link;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8f01c2bc7355..bc533501b4e0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -346,7 +346,8 @@ static void print_context_stats(struct seq_file *m,
>                         rcu_read_unlock();
>  
>                         rcu_read_lock();
> -                       task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID);
> +                       task = pid_task(ctx->client->pid ?: file->pid,
> +                                       PIDTYPE_PID);
>                         snprintf(name, sizeof(name), "%s",
>                                  task ? task->comm : "<unknown>");
>                         rcu_read_unlock();
> @@ -1492,10 +1493,10 @@ static int i915_context_status(struct seq_file *m, void *unused)
>                 spin_unlock(&i915->gem.contexts.lock);
>  
>                 seq_puts(m, "HW context ");
> -               if (ctx->pid) {
> +               if (ctx->client->pid) {
>                         struct task_struct *task;
>  
> -                       task = get_pid_task(ctx->pid, PIDTYPE_PID);
> +                       task = get_pid_task(ctx->client->pid, PIDTYPE_PID);
>                         if (task) {
>                                 seq_printf(m, "(%s [%d]) ",
>                                            task->comm, task->pid);
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index fda0977d2059..9240327bdb7d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1235,8 +1235,8 @@ static void record_request(const struct i915_request *request,
>         erq->pid = 0;
>         rcu_read_lock();
>         ctx = rcu_dereference(request->context->gem_context);
> -       if (ctx)
> -               erq->pid = pid_nr(ctx->pid);
> +       if (ctx && ctx->client->pid)
> +               erq->pid = pid_nr(ctx->client->pid);
>         rcu_read_unlock();
>  }
>  
> @@ -1313,11 +1313,11 @@ static bool record_context(struct drm_i915_error_context *e,
>         if (ctx && !kref_get_unless_zero(&ctx->ref))
>                 ctx = NULL;
>         rcu_read_unlock();
> -       if (!ctx)
> +       if (!ctx || !ctx->client->pid)
>                 return false;

Can we have a client without a pid?

Holding a reference to the ctx is sufficent to protect the
i915_drm_client, ok.

>         rcu_read_lock();
> -       task = pid_task(ctx->pid, PIDTYPE_PID);
> +       task = pid_task(ctx->client->pid, PIDTYPE_PID);
>         if (task) {
>                 strcpy(e->comm, task->comm);
>                 e->pid = task->pid;

Ok, if you move this ahead to patch 3, we can use this as justification
for i915_drm_client all by itself.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness
  2020-01-10 13:30 ` [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness Tvrtko Ursulin
@ 2020-01-10 13:46   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 13:46 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:44)
>  #endif /* __INTEL_CONTEXT_TYPES__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 825c94e7ca0b..9a346c060469 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1543,8 +1543,20 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
>  
>                 engine->stats.enabled_at = ktime_get();
>  
> -               /* XXX submission method oblivious? */
> -               for (port = execlists->active; (rq = *port); port++)
> +               /*
> +                * Mark currently running context as active.
> +                * XXX submission method oblivious?
> +                */
> +
> +               rq = NULL;
> +               port = execlists->active;

execlists->active is never NULL (it always points at one of the arrays).
*execlists->active may be NULL.

> +               if (port)
> +                       rq = *port;
> +               if (rq)
> +                       __intel_context_stats_start(&rq->context->stats,
> +                                                   engine->stats.enabled_at);
> +
> +               for (; (rq = *port); port++)
>                         engine->stats.active++;
>  
> @@ -2250,6 +2277,7 @@ static void process_csb(struct intel_engine_cs *engine)
>         rmb();
>  
>         do {
> +               struct i915_request *rq;
>                 bool promote;
>  
>                 if (++head == num_entries)
> @@ -2305,7 +2333,11 @@ static void process_csb(struct intel_engine_cs *engine)
>  
>                         WRITE_ONCE(execlists->pending[0], NULL);
>                 } else {
> -                       GEM_BUG_ON(!*execlists->active);
> +                       rq = *execlists->active++;
> +                       GEM_BUG_ON(!rq);
> +
> +                       GEM_BUG_ON(execlists->active - execlists->inflight >
> +                                  execlists_num_ports(execlists));
>  
>                         /* port0 completed, advanced to port1 */
>                         trace_ports(execlists, "completed", execlists->active);
> @@ -2316,13 +2348,15 @@ static void process_csb(struct intel_engine_cs *engine)
>                          * coherent (visible from the CPU) before the
>                          * user interrupt and CSB is processed.
>                          */
> -                       GEM_BUG_ON(!i915_request_completed(*execlists->active) &&
> +                       GEM_BUG_ON(!i915_request_completed(rq) &&
>                                    !reset_in_progress(execlists));
> -                       execlists_schedule_out(*execlists->active++);
>  
> -                       GEM_BUG_ON(execlists->active - execlists->inflight >
> -                                  execlists_num_ports(execlists));
> +                       execlists_schedule_out(rq);
>                 }
> +
> +               rq = *execlists->active;
> +               if (rq)
> +                       intel_context_stats_start(&rq->context->stats);
>         } while (head != tail);

Actually, we can do this after processing the entire event buf.

if (execlists_active(execlists))
	intel_context_stats_start((*execlists->active)->context->stats);

Once we apply the fix in
https://patchwork.freedesktop.org/patch/347934/?series=71809&rev=1

We can in fact do this as a part of set_timeslice() which means we have
all the time-related updates in the same spot.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness
  2020-01-10 13:30 ` [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
@ 2020-01-10 13:58   ` Chris Wilson
  2020-01-10 14:09     ` Tvrtko Ursulin
  0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 13:58 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:47)
> +static ssize_t
> +show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> +       struct i915_engine_busy_attribute *i915_attr =
> +               container_of(attr, typeof(*i915_attr), attr);
> +       struct list_head *list = &i915_attr->client->ctx_list;
> +       unsigned int engine_class = i915_attr->engine_class;
> +       struct i915_gem_context *ctx;
> +       u64 total = 0;
> +
> +       if (i915_attr->no_busy_stats)
> +               return -ENODEV;
> +
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(ctx, list, client_link)
> +               total += sw_busy_add(ctx, engine_class);
> +       rcu_read_unlock();
> +
> +       return snprintf(buf, PAGE_SIZE, "%llu\n", total);
> +}
> +
> +static const char *uabi_class_names[] = {
> +       [I915_ENGINE_CLASS_RENDER] = "0",
> +       [I915_ENGINE_CLASS_COPY] = "1",
> +       [I915_ENGINE_CLASS_VIDEO] = "2",
> +       [I915_ENGINE_CLASS_VIDEO_ENHANCE] = "3",
> +};

Hmm. /sys/class/drm/card0/clients/0/busy/0

Ok. I was worried this was 0/0 and so very bland and liable to clash
later.

> +
>  int
>  __i915_drm_client_register(struct i915_drm_client *client,
>                            struct task_struct *task)
>  {
>         struct i915_drm_clients *clients = client->clients;
> +       struct drm_i915_private *i915 =
> +               container_of(clients, typeof(*i915), clients);
> +       struct intel_engine_cs *engine;
>         struct device_attribute *attr;
> -       int ret = -ENOMEM;
> +       int i, ret = -ENOMEM;
>         char idstr[32];
>  
>         if (!clients->root)
> @@ -77,10 +130,71 @@ __i915_drm_client_register(struct i915_drm_client *client,
>         if (ret)
>                 goto err_attr;
>  
> +       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
> +               client->busy_root =
> +                       kobject_create_and_add("busy", client->root);
> +               if (!client->busy_root)
> +                       goto err_attr;
> +
> +               for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++) {
> +                       struct i915_engine_busy_attribute *i915_attr =
> +                               &client->attr.busy[i];


if (!intel_engine_lookup_user(i915, i, 0))
	continue;

i.e. skip if we don't have any engines of that class in the system.

> +
> +                       i915_attr->client = client;
> +                       i915_attr->engine_class = i;
> +
> +                       attr = &i915_attr->attr;
> +
> +                       sysfs_attr_init(&attr->attr);
> +
> +                       attr->attr.name = uabi_class_names[i];
> +                       attr->attr.mode = 0444;
> +                       attr->show = show_client_busy;
> +
> +                       ret = sysfs_create_file(client->busy_root,
> +                                               (struct attribute *)attr);
> +                       if (ret)
> +                               goto err_busy;
> +               }
> +
> +               /* Enable busy stats on all engines. */
> +               i = 0;
> +               for_each_uabi_engine(engine, i915) {
> +                       ret = intel_enable_engine_stats(engine);

Hmm. We gave it a global bit in 

	i915->caps.scheduler & I915_SCHEDULER_CAP_ENABLED.

That'll avoid having to do the individual checking and rollback.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime
  2020-01-10 13:30 ` [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime Tvrtko Ursulin
@ 2020-01-10 14:03   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 14:03 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 13:30:48)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> GPU saves accumulated context runtime (in CS timestamp units) in PPHWSP
> which will be useful for us in cases when we are not able to track context
> busyness ourselves (like with GuC). Keep a copy of this in struct
> intel_context from where it can be easily read even if the context is not
> pinned.
> 
> QQQ: Do we want to make this accounting conditional / able to turn on/off?
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_context.h       | 7 +++++++
>  drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++++
>  drivers/gpu/drm/i915/gt/intel_lrc.c           | 9 +++++++++
>  drivers/gpu/drm/i915/intel_device_info.c      | 2 ++
>  drivers/gpu/drm/i915/intel_device_info.h      | 1 +
>  5 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 30f0268fcc9a..389a05736fc7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -12,6 +12,7 @@
>  #include <linux/types.h>
>  
>  #include "i915_active.h"
> +#include "i915_drv.h"
>  #include "intel_context_types.h"
>  #include "intel_engine_types.h"
>  #include "intel_ring_types.h"
> @@ -235,4 +236,10 @@ __intel_context_stats_start(struct intel_context_stats *stats, ktime_t now)
>  
>  ktime_t intel_context_get_busy_time(struct intel_context *ce);
>  
> +static inline u64 intel_context_get_hw_runtime_ns(struct intel_context *ce)
> +{
> +       return ce->total_runtime *
> +              RUNTIME_INFO(ce->engine->i915)->cs_timestamp_period_ns;
> +}
> +
>  #endif /* __INTEL_CONTEXT_H__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index 963d33dc5289..7b08bf87fb82 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -69,6 +69,11 @@ struct intel_context {
>         u64 lrc_desc;
>         u32 tag; /* cookie passed to HW to track this context on submission */
>  
> +       /* Time on GPU as tracked by the hw. */
> +       u32 last_runtime;
> +       u64 total_runtime;
> +       u32 *pphwsp;

I wouldn't bother with keeping pphwsp, we know it's the page before the
reg state. At least for the foreseeable future.

>         unsigned int active_count; /* protected by timeline->mutex */
>  
>         atomic_t pin_count;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index dd559547500f..26999b43e5a1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1289,6 +1289,7 @@ __execlists_schedule_out(struct i915_request *rq,
>                          struct intel_engine_cs * const engine)
>  {
>         struct intel_context *ce = rq->context;
> +       u32 old, new;
>  
>         /*
>          * NB process_csb() is not under the engine->active.lock and hence
> @@ -1309,6 +1310,13 @@ __execlists_schedule_out(struct i915_request *rq,
>         execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
>         intel_gt_pm_put_async(engine->gt);
>  
> +       old = ce->last_runtime;
> +       ce->last_runtime = new = ce->pphwsp[16];
> +       if (new > old)
> +               ce->total_runtime += new - old;
> +       else
> +               ce->total_runtime += (~0UL - old) + new + 1;

It's u32, unsigned wrap-around arithmetic is defined, so just
ce->total_runtime += new - old;

> +
>         /*
>          * If this is part of a virtual engine, its next request may
>          * have been blocked waiting for access to the active context.
> @@ -2608,6 +2616,7 @@ __execlists_context_pin(struct intel_context *ce,
>  
>         ce->lrc_desc = lrc_descriptor(ce, engine) | CTX_DESC_FORCE_RESTORE;
>         ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
> +       ce->pphwsp = vaddr + LRC_PPHWSP_PN * PAGE_SIZE;
>         __execlists_update_reg_state(ce, engine);
>  
>         return 0;
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 6670a0763be2..7732748e1939 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -1042,6 +1042,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  
>         /* Initialize command stream timestamp frequency */
>         runtime->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
> +       runtime->cs_timestamp_period_ns =
> +               div_u64(1e6, runtime->cs_timestamp_frequency_khz);

drm_debug(&dev_priv->drm, "CS timestamp wraparound in %lld\n",
div_u64(U32_MAX * runtime->cs_timestamp_period_ns, NSEC_PER_SEC);
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness
  2020-01-10 13:58   ` Chris Wilson
@ 2020-01-10 14:09     ` Tvrtko Ursulin
  2020-01-10 14:12       ` Chris Wilson
  0 siblings, 1 reply; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-10 14:09 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx; +Cc: kui.wen


On 10/01/2020 13:58, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-01-10 13:30:47)
>> +static ssize_t
>> +show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
>> +{
>> +       struct i915_engine_busy_attribute *i915_attr =
>> +               container_of(attr, typeof(*i915_attr), attr);
>> +       struct list_head *list = &i915_attr->client->ctx_list;
>> +       unsigned int engine_class = i915_attr->engine_class;
>> +       struct i915_gem_context *ctx;
>> +       u64 total = 0;
>> +
>> +       if (i915_attr->no_busy_stats)
>> +               return -ENODEV;
>> +
>> +       rcu_read_lock();
>> +       list_for_each_entry_rcu(ctx, list, client_link)
>> +               total += sw_busy_add(ctx, engine_class);
>> +       rcu_read_unlock();
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%llu\n", total);
>> +}
>> +
>> +static const char *uabi_class_names[] = {
>> +       [I915_ENGINE_CLASS_RENDER] = "0",
>> +       [I915_ENGINE_CLASS_COPY] = "1",
>> +       [I915_ENGINE_CLASS_VIDEO] = "2",
>> +       [I915_ENGINE_CLASS_VIDEO_ENHANCE] = "3",
>> +};
> 
> Hmm. /sys/class/drm/card0/clients/0/busy/0
> 
> Ok. I was worried this was 0/0 and so very bland and liable to clash
> later.
> 
>> +
>>   int
>>   __i915_drm_client_register(struct i915_drm_client *client,
>>                             struct task_struct *task)
>>   {
>>          struct i915_drm_clients *clients = client->clients;
>> +       struct drm_i915_private *i915 =
>> +               container_of(clients, typeof(*i915), clients);
>> +       struct intel_engine_cs *engine;
>>          struct device_attribute *attr;
>> -       int ret = -ENOMEM;
>> +       int i, ret = -ENOMEM;
>>          char idstr[32];
>>   
>>          if (!clients->root)
>> @@ -77,10 +130,71 @@ __i915_drm_client_register(struct i915_drm_client *client,
>>          if (ret)
>>                  goto err_attr;
>>   
>> +       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>> +               client->busy_root =
>> +                       kobject_create_and_add("busy", client->root);
>> +               if (!client->busy_root)
>> +                       goto err_attr;
>> +
>> +               for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++) {
>> +                       struct i915_engine_busy_attribute *i915_attr =
>> +                               &client->attr.busy[i];
> 
> 
> if (!intel_engine_lookup_user(i915, i, 0))
> 	continue;
> 
> i.e. skip if we don't have any engines of that class in the system.

Yes, thanks.

>> +
>> +                       i915_attr->client = client;
>> +                       i915_attr->engine_class = i;
>> +
>> +                       attr = &i915_attr->attr;
>> +
>> +                       sysfs_attr_init(&attr->attr);
>> +
>> +                       attr->attr.name = uabi_class_names[i];
>> +                       attr->attr.mode = 0444;
>> +                       attr->show = show_client_busy;
>> +
>> +                       ret = sysfs_create_file(client->busy_root,
>> +                                               (struct attribute *)attr);
>> +                       if (ret)
>> +                               goto err_busy;
>> +               }
>> +
>> +               /* Enable busy stats on all engines. */
>> +               i = 0;
>> +               for_each_uabi_engine(engine, i915) {
>> +                       ret = intel_enable_engine_stats(engine);
> 
> Hmm. We gave it a global bit in
> 
> 	i915->caps.scheduler & I915_SCHEDULER_CAP_ENABLED.
> 
> That'll avoid having to do the individual checking and rollback.

I could add a top level check as a short circuit, but I prefer to check 
return code from intel_enable_engine_stats since it returns one.

Also if new GuC will have I915_SCHEDULER_CAP_ENABLED it will still fail 
to enable engine stats and then fallback to pphwsp has to happen.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness
  2020-01-10 14:09     ` Tvrtko Ursulin
@ 2020-01-10 14:12       ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2020-01-10 14:12 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: kui.wen

Quoting Tvrtko Ursulin (2020-01-10 14:09:09)
> 
> On 10/01/2020 13:58, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2020-01-10 13:30:47)
> >> +static ssize_t
> >> +show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
> >> +{
> >> +       struct i915_engine_busy_attribute *i915_attr =
> >> +               container_of(attr, typeof(*i915_attr), attr);
> >> +       struct list_head *list = &i915_attr->client->ctx_list;
> >> +       unsigned int engine_class = i915_attr->engine_class;
> >> +       struct i915_gem_context *ctx;
> >> +       u64 total = 0;
> >> +
> >> +       if (i915_attr->no_busy_stats)
> >> +               return -ENODEV;
> >> +
> >> +       rcu_read_lock();
> >> +       list_for_each_entry_rcu(ctx, list, client_link)
> >> +               total += sw_busy_add(ctx, engine_class);
> >> +       rcu_read_unlock();
> >> +
> >> +       return snprintf(buf, PAGE_SIZE, "%llu\n", total);
> >> +}
> >> +
> >> +static const char *uabi_class_names[] = {
> >> +       [I915_ENGINE_CLASS_RENDER] = "0",
> >> +       [I915_ENGINE_CLASS_COPY] = "1",
> >> +       [I915_ENGINE_CLASS_VIDEO] = "2",
> >> +       [I915_ENGINE_CLASS_VIDEO_ENHANCE] = "3",
> >> +};
> > 
> > Hmm. /sys/class/drm/card0/clients/0/busy/0
> > 
> > Ok. I was worried this was 0/0 and so very bland and liable to clash
> > later.
> > 
> >> +
> >>   int
> >>   __i915_drm_client_register(struct i915_drm_client *client,
> >>                             struct task_struct *task)
> >>   {
> >>          struct i915_drm_clients *clients = client->clients;
> >> +       struct drm_i915_private *i915 =
> >> +               container_of(clients, typeof(*i915), clients);
> >> +       struct intel_engine_cs *engine;
> >>          struct device_attribute *attr;
> >> -       int ret = -ENOMEM;
> >> +       int i, ret = -ENOMEM;
> >>          char idstr[32];
> >>   
> >>          if (!clients->root)
> >> @@ -77,10 +130,71 @@ __i915_drm_client_register(struct i915_drm_client *client,
> >>          if (ret)
> >>                  goto err_attr;
> >>   
> >> +       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
> >> +               client->busy_root =
> >> +                       kobject_create_and_add("busy", client->root);
> >> +               if (!client->busy_root)
> >> +                       goto err_attr;
> >> +
> >> +               for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++) {
> >> +                       struct i915_engine_busy_attribute *i915_attr =
> >> +                               &client->attr.busy[i];
> > 
> > 
> > if (!intel_engine_lookup_user(i915, i, 0))
> >       continue;
> > 
> > i.e. skip if we don't have any engines of that class in the system.
> 
> Yes, thanks.
> 
> >> +
> >> +                       i915_attr->client = client;
> >> +                       i915_attr->engine_class = i;
> >> +
> >> +                       attr = &i915_attr->attr;
> >> +
> >> +                       sysfs_attr_init(&attr->attr);
> >> +
> >> +                       attr->attr.name = uabi_class_names[i];
> >> +                       attr->attr.mode = 0444;
> >> +                       attr->show = show_client_busy;
> >> +
> >> +                       ret = sysfs_create_file(client->busy_root,
> >> +                                               (struct attribute *)attr);
> >> +                       if (ret)
> >> +                               goto err_busy;
> >> +               }
> >> +
> >> +               /* Enable busy stats on all engines. */
> >> +               i = 0;
> >> +               for_each_uabi_engine(engine, i915) {
> >> +                       ret = intel_enable_engine_stats(engine);
> > 
> > Hmm. We gave it a global bit in
> > 
> >       i915->caps.scheduler & I915_SCHEDULER_CAP_ENABLED.
> > 
> > That'll avoid having to do the individual checking and rollback.
> 
> I could add a top level check as a short circuit, but I prefer to check 
> return code from intel_enable_engine_stats since it returns one.

My suggestion was to remove the return code and make it bug out, as we
[can] check before use in i915_pmu.c as well.

> Also if new GuC will have I915_SCHEDULER_CAP_ENABLED it will still fail 
> to enable engine stats and then fallback to pphwsp has to happen.

Brainfart, CAP_SUPPORTS_STATS. 
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Per client engine busyness (rev3)
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (7 preceding siblings ...)
  2020-01-10 13:30 ` [Intel-gfx] [RFC 8/8] drm/i915: Fallback to hw context runtime when sw tracking is not available Tvrtko Ursulin
@ 2020-01-10 16:26 ` Patchwork
  2020-01-10 16:51 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-01-14  4:39 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2020-01-10 16:26 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Per client engine busyness (rev3)
URL   : https://patchwork.freedesktop.org/series/70977/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f2a2ab0fb90e drm/i915: Expose list of clients in sysfs
-:61: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#61: 
new file mode 100644

-:66: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#66: FILE: drivers/gpu/drm/i915/i915_drm_client.c:1:
+/*

-:67: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#67: FILE: drivers/gpu/drm/i915/i915_drm_client.c:2:
+ * SPDX-License-Identifier: MIT

-:222: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#222: FILE: drivers/gpu/drm/i915/i915_drm_client.h:1:
+/*

-:223: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#223: FILE: drivers/gpu/drm/i915/i915_drm_client.h:2:
+ * SPDX-License-Identifier: MIT

total: 0 errors, 5 warnings, 0 checks, 338 lines checked
4cd0960d1b2d drm/i915: Update client name on context create
b69ea9c5500b drm/i915: Track per-context engine busyness
3878342bf2cc drm/i915: Track all user contexts per client
-:56: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#56: FILE: drivers/gpu/drm/i915/gem/i915_gem_context.c:808:
+	ctx->client = client = i915_drm_client_get(fpriv->client);

-:116: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#116: FILE: drivers/gpu/drm/i915/i915_drm_client.h:35:
+	spinlock_t ctx_lock;

total: 0 errors, 0 warnings, 2 checks, 84 lines checked
342d49e806c4 drm/i915: Contexts can use struct pid stored in the client
eb3f14196571 drm/i915: Expose per-engine client busyness
-:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#25: 
     Render/3D/0   63.73% |███████████████████           |      3%      0%

-:107: WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
#107: FILE: drivers/gpu/drm/i915/i915_drm_client.c:82:
+static const char *uabi_class_names[] = {

-:132: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#132: FILE: drivers/gpu/drm/i915/i915_drm_client.c:133:
+       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {$

-:132: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (7, 16)
#132: FILE: drivers/gpu/drm/i915/i915_drm_client.c:133:
+       if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
+		client->busy_root =

-:189: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#189: FILE: drivers/gpu/drm/i915/i915_drm_client.c:190:
+       }$

total: 0 errors, 5 warnings, 0 checks, 198 lines checked
0f5c8f1a3375 drm/i915: Track hw reported context runtime
-:72: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#72: FILE: drivers/gpu/drm/i915/gt/intel_lrc.c:1314:
+	ce->last_runtime = new = ce->pphwsp[16];

total: 0 errors, 0 warnings, 1 checks, 70 lines checked
82e4aff984e7 drm/i915: Fallback to hw context runtime when sw tracking is not available

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for Per client engine busyness (rev3)
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (8 preceding siblings ...)
  2020-01-10 16:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Per client engine busyness (rev3) Patchwork
@ 2020-01-10 16:51 ` Patchwork
  2020-01-14  4:39 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2020-01-10 16:51 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Per client engine busyness (rev3)
URL   : https://patchwork.freedesktop.org/series/70977/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7718 -> Patchwork_16052
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/index.html

Known issues
------------

  Here are the changes found in Patchwork_16052 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_sync@basic-all:
    - fi-tgl-y:           [PASS][1] -> [INCOMPLETE][2] ([i915#470] / [i915#472])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-tgl-y/igt@gem_sync@basic-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-tgl-y/igt@gem_sync@basic-all.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-lmem:        [PASS][3] -> [INCOMPLETE][4] ([i915#671])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [PASS][5] -> [FAIL][6] ([i915#579])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-j1900:       [TIMEOUT][7] ([i915#816]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-byt-j1900/igt@gem_close_race@basic-threads.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-byt-j1900/igt@gem_close_race@basic-threads.html

  * {igt@gem_exec_basic@basic@bcs0}:
    - fi-byt-n2820:       [FAIL][9] ([i915#694]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-byt-n2820/igt@gem_exec_basic@basic@bcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-byt-n2820/igt@gem_exec_basic@basic@bcs0.html

  * {igt@gem_exec_basic@basic@vcs0}:
    - fi-byt-n2820:       [WARN][11] -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-byt-n2820/igt@gem_exec_basic@basic@vcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-byt-n2820/igt@gem_exec_basic@basic@vcs0.html

  * igt@gem_exec_suspend@basic-s0:
    - {fi-ehl-1}:         [INCOMPLETE][13] ([i915#937]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-ehl-1/igt@gem_exec_suspend@basic-s0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-ehl-1/igt@gem_exec_suspend@basic-s0.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-x1275:       [INCOMPLETE][15] ([i915#879]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-kbl-x1275/igt@i915_module_load@reload-with-fault-injection.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-kbl-x1275/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][17] ([i915#178]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][19] ([i915#725]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_execlists:
    - fi-kbl-soraka:      [DMESG-FAIL][21] ([i915#656]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-kbl-soraka/igt@i915_selftest@live_execlists.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-kbl-soraka/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [DMESG-FAIL][23] ([i915#722]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][25] ([i915#563]) -> [DMESG-FAIL][26] ([i915#725])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/fi-hsw-4770/igt@i915_selftest@live_blt.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
  [i915#470]: https://gitlab.freedesktop.org/drm/intel/issues/470
  [i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
  [i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#722]: https://gitlab.freedesktop.org/drm/intel/issues/722
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816
  [i915#879]: https://gitlab.freedesktop.org/drm/intel/issues/879
  [i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937


Participating hosts (46 -> 37)
------------------------------

  Additional (2): fi-bsw-nick fi-bsw-n3050 
  Missing    (11): fi-hsw-4200u fi-icl-u2 fi-bwr-2160 fi-bsw-cyan fi-ctg-p8600 fi-elk-e7500 fi-cfl-8109u fi-pnv-d510 fi-blb-e6850 fi-byt-clapper fi-skl-6600u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7718 -> Patchwork_16052

  CI-20190529: 20190529
  CI_DRM_7718: 37be537ac03a8299982f5fd177418aef86fdcc9e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5362: c2843f8e06a2cf7d372cd154310bf0e3b7722ab8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16052: 82e4aff984e7327f89675ebc5e5769039a398147 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

82e4aff984e7 drm/i915: Fallback to hw context runtime when sw tracking is not available
0f5c8f1a3375 drm/i915: Track hw reported context runtime
eb3f14196571 drm/i915: Expose per-engine client busyness
342d49e806c4 drm/i915: Contexts can use struct pid stored in the client
3878342bf2cc drm/i915: Track all user contexts per client
b69ea9c5500b drm/i915: Track per-context engine busyness
4cd0960d1b2d drm/i915: Update client name on context create
f2a2ab0fb90e drm/i915: Expose list of clients in sysfs

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] ✗ Fi.CI.IGT: failure for Per client engine busyness (rev3)
  2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
                   ` (9 preceding siblings ...)
  2020-01-10 16:51 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-01-14  4:39 ` Patchwork
  10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2020-01-14  4:39 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Per client engine busyness (rev3)
URL   : https://patchwork.freedesktop.org/series/70977/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7718_full -> Patchwork_16052_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16052_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16052_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16052_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_busy@close-race:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb7/igt@gem_busy@close-race.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb6/igt@gem_busy@close-race.html

  
Known issues
------------

  Here are the changes found in Patchwork_16052_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276] / [fdo#112080])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@gem_ctx_persistence@vcs1-queued.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_shared@q-smoketest-blt:
    - shard-tglb:         [PASS][5] -> [INCOMPLETE][6] ([fdo#111735])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb4/igt@gem_ctx_shared@q-smoketest-blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb3/igt@gem_ctx_shared@q-smoketest-blt.html

  * igt@gem_exec_await@wide-contexts:
    - shard-tglb:         [PASS][7] -> [INCOMPLETE][8] ([fdo#111736] / [i915#472])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb5/igt@gem_exec_await@wide-contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb3/igt@gem_exec_await@wide-contexts.html

  * igt@gem_exec_create@forked:
    - shard-glk:          [PASS][9] -> [TIMEOUT][10] ([fdo#112271] / [i915#940])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-glk2/igt@gem_exec_create@forked.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-glk9/igt@gem_exec_create@forked.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112080]) +7 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109276]) +19 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@gem_exec_schedule@independent-bsd2.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_exec_schedule@pi-distinct-iova-bsd:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([i915#677]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb3/igt@gem_exec_schedule@pi-distinct-iova-bsd.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb2/igt@gem_exec_schedule@pi-distinct-iova-bsd.html

  * igt@gem_exec_schedule@preempt-queue-contexts-blt:
    - shard-tglb:         [PASS][17] -> [INCOMPLETE][18] ([fdo#111606] / [fdo#111677] / [i915#472])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb7/igt@gem_exec_schedule@preempt-queue-contexts-blt.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb6/igt@gem_exec_schedule@preempt-queue-contexts-blt.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#112146]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb7/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_exec_schedule@smoketest-all:
    - shard-tglb:         [PASS][21] -> [INCOMPLETE][22] ([i915#463] / [i915#472])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb5/igt@gem_exec_schedule@smoketest-all.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb2/igt@gem_exec_schedule@smoketest-all.html

  * igt@gem_exec_schedule@smoketest-vebox:
    - shard-tglb:         [PASS][23] -> [INCOMPLETE][24] ([i915#472] / [i915#707])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb7/igt@gem_exec_schedule@smoketest-vebox.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb4/igt@gem_exec_schedule@smoketest-vebox.html

  * igt@gem_fenced_exec_thrash@2-spare-fences:
    - shard-snb:          [PASS][25] -> [INCOMPLETE][26] ([i915#82])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-snb6/igt@gem_fenced_exec_thrash@2-spare-fences.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-snb5/igt@gem_fenced_exec_thrash@2-spare-fences.html

  * igt@gem_persistent_relocs@forked-interruptible-thrash-inactive:
    - shard-snb:          [PASS][27] -> [TIMEOUT][28] ([fdo#112271] / [i915#530])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-tglb:         [PASS][29] -> [TIMEOUT][30] ([fdo#112126] / [fdo#112271] / [i915#530])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb4/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb7/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_pipe_control_store_loop@reused-buffer:
    - shard-tglb:         [PASS][31] -> [INCOMPLETE][32] ([i915#707] / [i915#796])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb2/igt@gem_pipe_control_store_loop@reused-buffer.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb2/igt@gem_pipe_control_store_loop@reused-buffer.html

  * igt@gem_sync@basic-all:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([i915#470] / [i915#472])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb7/igt@gem_sync@basic-all.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb5/igt@gem_sync@basic-all.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          [PASS][35] -> [DMESG-WARN][36] ([i915#180]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-kbl7/igt@gem_workarounds@suspend-resume-fd.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-kbl1/igt@gem_workarounds@suspend-resume-fd.html

  * igt@i915_selftest@live_execlists:
    - shard-glk:          [PASS][37] -> [TIMEOUT][38] ([fdo#112271] / [i915#529])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-glk1/igt@i915_selftest@live_execlists.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-glk6/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gt_timelines:
    - shard-tglb:         [PASS][39] -> [INCOMPLETE][40] ([i915#455])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb4/igt@i915_selftest@live_gt_timelines.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb5/igt@i915_selftest@live_gt_timelines.html

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          [PASS][41] -> [INCOMPLETE][42] ([i915#69])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl4/igt@i915_suspend@sysfs-reader.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl10/igt@i915_suspend@sysfs-reader.html

  * igt@kms_color@pipe-a-ctm-0-5:
    - shard-skl:          [PASS][43] -> [DMESG-WARN][44] ([i915#109]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl8/igt@kms_color@pipe-a-ctm-0-5.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl6/igt@kms_color@pipe-a-ctm-0-5.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][45] -> [FAIL][46] ([i915#79])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][47] -> [INCOMPLETE][48] ([fdo#103665])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt:
    - shard-tglb:         [PASS][49] -> [FAIL][50] ([i915#49]) +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-apl:          [PASS][51] -> [DMESG-WARN][52] ([i915#180])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][53] -> [FAIL][54] ([fdo#108145])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][55] -> [SKIP][56] ([fdo#109441]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@kms_psr@psr2_cursor_render.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][57] ([fdo#109276] / [fdo#112080]) -> [PASS][58] +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb7/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb1/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][59] ([fdo#110841]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_ctx_shared@q-smoketest-all:
    - shard-tglb:         [INCOMPLETE][61] ([fdo#111735]) -> [PASS][62] +2 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb2/igt@gem_ctx_shared@q-smoketest-all.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb4/igt@gem_ctx_shared@q-smoketest-all.html

  * igt@gem_exec_parallel@contexts:
    - shard-tglb:         [INCOMPLETE][63] ([i915#470] / [i915#472]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb6/igt@gem_exec_parallel@contexts.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb5/igt@gem_exec_parallel@contexts.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][65] ([fdo#112146]) -> [PASS][66] +6 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb8/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_exec_schedule@preempt-queue-blt:
    - shard-tglb:         [INCOMPLETE][67] ([fdo#111677] / [i915#472]) -> [PASS][68] +2 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb3/igt@gem_exec_schedule@preempt-queue-blt.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb5/igt@gem_exec_schedule@preempt-queue-blt.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [SKIP][69] ([fdo#109276]) -> [PASS][70] +24 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_render_copy@yf-tiled-to-vebox-x-tiled:
    - shard-tglb:         [INCOMPLETE][71] -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb2/igt@gem_render_copy@yf-tiled-to-vebox-x-tiled.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb7/igt@gem_render_copy@yf-tiled-to-vebox-x-tiled.html

  * igt@gem_sync@basic-store-all:
    - shard-tglb:         [INCOMPLETE][73] ([i915#472]) -> [PASS][74] +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-tglb6/igt@gem_sync@basic-store-all.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-tglb1/igt@gem_sync@basic-store-all.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][75] ([i915#454]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb2/igt@i915_pm_dc@dc6-psr.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb6/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_color@pipe-b-ctm-0-75:
    - shard-skl:          [DMESG-WARN][77] ([i915#109]) -> [PASS][78] +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl6/igt@kms_color@pipe-b-ctm-0-75.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl3/igt@kms_color@pipe-b-ctm-0-75.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [DMESG-WARN][79] ([i915#180]) -> [PASS][80] +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-apl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-iclb:         [INCOMPLETE][81] ([i915#123] / [i915#140]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb3/igt@kms_frontbuffer_tracking@psr-suspend.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb1/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [DMESG-WARN][83] ([i915#180]) -> [PASS][84] +3 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][85] ([fdo#108145]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][87] ([fdo#108145] / [i915#265]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-skl:          [DMESG-WARN][89] ([IGT#6]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-skl1/igt@kms_plane_multiple@atomic-pipe-c-tiling-y.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-skl1/igt@kms_plane_multiple@atomic-pipe-c-tiling-y.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][91] ([fdo#109642] / [fdo#111068]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb5/igt@kms_psr2_su@frontbuffer.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][93] ([fdo#109441]) -> [PASS][94] +2 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb5/igt@kms_psr@psr2_primary_mmap_cpu.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@perf@short-reads:
    - shard-glk:          [TIMEOUT][95] ([fdo#112271] / [i915#51]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-glk5/igt@perf@short-reads.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-glk3/igt@perf@short-reads.html

  * igt@perf_pmu@busy-vcs1:
    - shard-iclb:         [SKIP][97] ([fdo#112080]) -> [PASS][98] +11 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-iclb3/igt@perf_pmu@busy-vcs1.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-iclb2/igt@perf_pmu@busy-vcs1.html

  
#### Warnings ####

  * igt@gem_tiled_blits@normal:
    - shard-hsw:          [FAIL][99] ([i915#818]) -> [FAIL][100] ([i915#694])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7718/shard-hsw1/igt@gem_tiled_blits@normal.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/shard-hsw2/igt@gem_tiled_blits@normal.html

  
  [IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111606]: https://bugs.freedesktop.org/show_bug.cgi?id=111606
  [fdo#111677]: https://bugs.freedesktop.org/show_bug.cgi?id=111677
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112126]: https://bugs.freedesktop.org/show_bug.cgi?id=112126
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#455]: https://gitlab.freedesktop.org/drm/intel/issues/455
  [i915#463]: https://gitlab.freedesktop.org/drm/intel/issues/463
  [i915#470]: https://gitlab.freedesktop.org/drm/intel/issues/470
  [i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#51]: https://gitlab.freedesktop.org/drm/intel/issues/51
  [i915#529]: https://gitlab.freedesktop.org/drm/intel/issues/529
  [i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#707]: https://gitlab.freedesktop.org/drm/intel/issues/707
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#796]: https://gitlab.freedesktop.org/drm/intel/issues/796
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#940]: https://gitlab.freedesktop.org/drm/intel/issues/940


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7718 -> Patchwork_16052

  CI-20190529: 20190529
  CI_DRM_7718: 37be537ac03a8299982f5fd177418aef86fdcc9e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5362: c2843f8e06a2cf7d372cd154310bf0e3b7722ab8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16052: 82e4aff984e7327f89675ebc5e5769039a398147 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16052/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client
  2020-01-10 13:42   ` Chris Wilson
@ 2020-01-30 16:11     ` Tvrtko Ursulin
  0 siblings, 0 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-01-30 16:11 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 10/01/2020 13:42, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-01-10 13:30:46)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Now that contexts keep their parent client reference counted, we can
>> remove separate struct pid reference owned by contexts in favour of the
>> one already held by the client.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 ++++---------
>>   drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 10 ----------
>>   drivers/gpu/drm/i915/i915_debugfs.c               |  7 ++++---
>>   drivers/gpu/drm/i915/i915_gpu_error.c             |  8 ++++----
>>   4 files changed, 12 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> index ba8ccc754f20..758cebb99ba4 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> @@ -323,7 +323,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
>>          if (ctx->timeline)
>>                  intel_timeline_put(ctx->timeline);
>>   
>> -       put_pid(ctx->pid);
>>          mutex_destroy(&ctx->mutex);
>>   
>>          kfree_rcu(ctx, rcu);
>> @@ -794,24 +793,20 @@ static int gem_context_register(struct i915_gem_context *ctx,
>>                  WRITE_ONCE(vm->file, fpriv); /* XXX */
>>          mutex_unlock(&ctx->mutex);
>>   
>> -       ctx->pid = get_task_pid(current, PIDTYPE_PID);
>>          snprintf(ctx->name, sizeof(ctx->name), "%s[%d]",
>> -                current->comm, pid_nr(ctx->pid));
>> +                current->comm, pid_nr(client->pid));
>>   
>>          /* And finally expose ourselves to userspace via the idr */
>>          ret = xa_alloc(&fpriv->context_xa, id, ctx, xa_limit_32b, GFP_KERNEL);
>> -       if (ret) {
>> -               put_pid(fetch_and_zero(&ctx->pid));
>> -               goto out;
>> -       }
>> +       if (ret)
>> +               return ret;
>>   
>>          ctx->client = client = i915_drm_client_get(fpriv->client);
>>          spin_lock(&client->ctx_lock);
>>          list_add_tail_rcu(&ctx->client_link, &client->ctx_list);
>>          spin_unlock(&client->ctx_lock);
>>   
>> -out:
>> -       return ret;
>> +       return 0;
>>   }
>>   
>>   int i915_gem_context_open(struct drm_i915_private *i915,
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>> index 879824159646..23421377a43d 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>> @@ -90,16 +90,6 @@ struct i915_gem_context {
>>           */
>>          struct i915_address_space __rcu *vm;
>>   
>> -       /**
>> -        * @pid: process id of creator
>> -        *
>> -        * Note that who created the context may not be the principle user,
>> -        * as the context may be shared across a local socket. However,
>> -        * that should only affect the default context, all contexts created
>> -        * explicitly by the client are expected to be isolated.
>> -        */
>> -       struct pid *pid;
>> -
>>          /** link: place with &drm_i915_private.context_list */
>>          struct list_head link;
>>          struct llist_node free_link;
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 8f01c2bc7355..bc533501b4e0 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -346,7 +346,8 @@ static void print_context_stats(struct seq_file *m,
>>                          rcu_read_unlock();
>>   
>>                          rcu_read_lock();
>> -                       task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID);
>> +                       task = pid_task(ctx->client->pid ?: file->pid,
>> +                                       PIDTYPE_PID);
>>                          snprintf(name, sizeof(name), "%s",
>>                                   task ? task->comm : "<unknown>");
>>                          rcu_read_unlock();
>> @@ -1492,10 +1493,10 @@ static int i915_context_status(struct seq_file *m, void *unused)
>>                  spin_unlock(&i915->gem.contexts.lock);
>>   
>>                  seq_puts(m, "HW context ");
>> -               if (ctx->pid) {
>> +               if (ctx->client->pid) {
>>                          struct task_struct *task;
>>   
>> -                       task = get_pid_task(ctx->pid, PIDTYPE_PID);
>> +                       task = get_pid_task(ctx->client->pid, PIDTYPE_PID);
>>                          if (task) {
>>                                  seq_printf(m, "(%s [%d]) ",
>>                                             task->comm, task->pid);
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index fda0977d2059..9240327bdb7d 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -1235,8 +1235,8 @@ static void record_request(const struct i915_request *request,
>>          erq->pid = 0;
>>          rcu_read_lock();
>>          ctx = rcu_dereference(request->context->gem_context);
>> -       if (ctx)
>> -               erq->pid = pid_nr(ctx->pid);
>> +       if (ctx && ctx->client->pid)
>> +               erq->pid = pid_nr(ctx->client->pid);
>>          rcu_read_unlock();
>>   }
>>   
>> @@ -1313,11 +1313,11 @@ static bool record_context(struct drm_i915_error_context *e,
>>          if (ctx && !kref_get_unless_zero(&ctx->ref))
>>                  ctx = NULL;
>>          rcu_read_unlock();
>> -       if (!ctx)
>> +       if (!ctx || !ctx->client->pid)
>>                  return false;
> 
> Can we have a client without a pid?

Yes, when fbdev registers I ignore it as a client because sysfs is not 
there yet. I contemplated stashing it somewhere and registering later 
but did not yet decide to actually do it. This was actually because 
without sysfs I had no use for pid and name. Perhaps now that I have 
moved this patch earlier I can change this to always have those two.. 
okay I'll see.

> 
> Holding a reference to the ctx is sufficent to protect the
> i915_drm_client, ok.
> 
>>          rcu_read_lock();
>> -       task = pid_task(ctx->pid, PIDTYPE_PID);
>> +       task = pid_task(ctx->client->pid, PIDTYPE_PID);
>>          if (task) {
>>                  strcpy(e->comm, task->comm);
>>                  e->pid = task->pid;
> 
> Ok, if you move this ahead to patch 3, we can use this as justification
> for i915_drm_client all by itself.

Yes mostly done but will see the above, plus a small regression in 
busyness tracking crept in after rebase, somehow. Will post the series 
when I resolve that.

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 0/8] Per client engine busyness
@ 2020-02-07 16:13 Tvrtko Ursulin
  0 siblings, 0 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2020-02-07 16:13 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Another re-spin of the per-client engine busyness series. Highlights from this
version:

 * Refactor of the i915_drm_client concept based on feedback from Chris.
 * Tracking contexts belonging to a client had a flaw where reaped contexts
   would stop contributing to the busyness making the counters go backwards.
   I was simply broken. In this version I had to track per-client stats at the
   same call-sites where per-context is tracked. It's a little bit more runtime
   cost but not too bad.
 * GuC support is out - needs more time than I have at the moment to properly
   wire it up with the latest changes. Plus there is a monotonicity issue with
   either the value stored in pphwsp by the GPU or a bug in my patch which also
   needs to be debugged.

Internally we track time spent on engines for each struct intel_context. This
can serve as a building block for several features from the want list:
smarter scheduler decisions, getrusage(2)-like per-GEM-context functionality
wanted by some customers, cgroups controller, dynamic SSEU tuning,...

Externally, in sysfs, we expose time spent on GPU per client and per engine
class.

Sysfs interface enables us to implement a "top-like" tool for GPU tasks. Or with
a "screenshot":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
intel-gpu-top -  906/ 955 MHz;    0% RC6;  5.30 Watts;      933 irqs/s

      IMC reads:     4414 MiB/s
     IMC writes:     3805 MiB/s

          ENGINE      BUSY                                      MI_SEMA MI_WAIT
     Render/3D/0   93.46% |████████████████████████████████▋  |      0%      0%
       Blitter/0    0.00% |                                   |      0%      0%
         Video/0    0.00% |                                   |      0%      0%
  VideoEnhance/0    0.00% |                                   |      0%      0%

  PID            NAME  Render/3D      Blitter        Video      VideoEnhance
 2733       neverball |██████▌     ||            ||            ||            |
 2047            Xorg |███▊        ||            ||            ||            |
 2737        glxgears |█▍          ||            ||            ||            |
 2128           xfwm4 |            ||            ||            ||            |
 2047            Xorg |            ||            ||            ||            |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Implementation wise we add a a bunch of files in sysfs like:

	# cd /sys/class/drm/card0/clients/
	# tree
	.
	├── 7
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	├── 8
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	└── 9
	    ├── busy
	    │   ├── 0
	    │   ├── 1
	    │   ├── 2
	    │   └── 3
	    ├── name
	    └── pid


Files in 'busy' directories are numbered using the engine class ABI values and
they contain accumulated nanoseconds each client spent on engines of a
respective class.

It is stil a RFC since it misses dedicated test cases to ensure things really
work as advertised.

Tvrtko Ursulin (6):
  drm/i915: Expose list of clients in sysfs
  drm/i915: Update client name on context create
  drm/i915: Make GEM contexts track DRM clients
  drm/i915: Track per-context engine busyness
  drm/i915: Track per drm client engine class busyness
  drm/i915: Expose per-engine client busyness

 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  31 +-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  13 +-
 drivers/gpu/drm/i915/gt/intel_context.c       |  20 +
 drivers/gpu/drm/i915/gt/intel_context.h       |  35 ++
 drivers/gpu/drm/i915/gt/intel_context_types.h |   9 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  16 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  65 +++-
 drivers/gpu/drm/i915/i915_debugfs.c           |  31 +-
 drivers/gpu/drm/i915/i915_drm_client.c        | 350 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_drm_client.h        |  88 +++++
 drivers/gpu/drm/i915/i915_drv.h               |   5 +
 drivers/gpu/drm/i915/i915_gem.c               |  35 +-
 drivers/gpu/drm/i915/i915_gpu_error.c         |  21 +-
 drivers/gpu/drm/i915/i915_sysfs.c             |   8 +
 15 files changed, 673 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c
 create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] [RFC 0/8] Per client engine busyness
@ 2019-12-19 18:00 Tvrtko Ursulin
  0 siblings, 0 replies; 23+ messages in thread
From: Tvrtko Ursulin @ 2019-12-19 18:00 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Another re-spin of the per-client engine busyness series. Highlights from this
version:

 * Now tracks GPU time for clients who exit with GPU work left running.
 * No more global toggle - it is now constantly on.

Internally we track time spent on engines for each struct intel_context. This
can serve as a building block for several features from the want list:
smarter scheduler decisions, getrusage(2)-like per-GEM-context functionality
wanted by some customers, cgroups controller, dynamic SSEU tuning,...

Externally, in sysfs, we expose time spent on GPU per client and per engine
class.

Sysfs interface enables us to implement a "top-like" tool for GPU tasks. Or with
a "screenshot":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
intel-gpu-top -  906/ 955 MHz;    0% RC6;  5.30 Watts;      933 irqs/s

      IMC reads:     4414 MiB/s
     IMC writes:     3805 MiB/s

          ENGINE      BUSY                                      MI_SEMA MI_WAIT
     Render/3D/0   93.46% |████████████████████████████████▋  |      0%      0%
       Blitter/0    0.00% |                                   |      0%      0%
         Video/0    0.00% |                                   |      0%      0%
  VideoEnhance/0    0.00% |                                   |      0%      0%

  PID            NAME  Render/3D      Blitter        Video      VideoEnhance
 2733       neverball |██████▌     ||            ||            ||            |
 2047            Xorg |███▊        ||            ||            ||            |
 2737        glxgears |█▍          ||            ||            ||            |
 2128           xfwm4 |            ||            ||            ||            |
 2047            Xorg |            ||            ||            ||            |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Implementation wise we add a a bunch of files in sysfs like:

	# cd /sys/class/drm/card0/clients/
	# tree
	.
	├── 7
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	├── 8
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	├── 9
	│   ├── busy
	│   │   ├── 0
	│   │   ├── 1
	│   │   ├── 2
	│   │   └── 3
	│   ├── name
	│   └── pid
	└── enable_stats

Files in 'busy' directories are numbered using the engine class ABI values and
they contain accumulated nanoseconds each client spent on engines of a
respective class.

I will post the corresponding patch to intel_gpu_top for reference as well.

Tvrtko Ursulin (8):
  drm/i915: Switch context id allocation directoy to xarray
  drm/i915: Reference count struct drm_i915_file_private
  drm/i915: Expose list of clients in sysfs
  drm/i915: Update client name on context create
  drm/i915: Track per-context engine busyness
  drm/i915: Track all user contexts per client
  drm/i915: Contexts can use struct pid stored in the client
  drm/i915: Expose per-engine client busyness

 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 113 ++++++---
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  16 +-
 .../gpu/drm/i915/gem/selftests/mock_context.c |   3 +-
 drivers/gpu/drm/i915/gt/intel_context.c       |  20 ++
 drivers/gpu/drm/i915/gt/intel_context.h       |  11 +
 drivers/gpu/drm/i915/gt/intel_context_types.h |   9 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  16 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  52 +++-
 drivers/gpu/drm/i915/i915_debugfs.c           |   7 +-
 drivers/gpu/drm/i915/i915_drv.c               |   4 -
 drivers/gpu/drm/i915/i915_drv.h               |  66 ++++-
 drivers/gpu/drm/i915/i915_gem.c               | 236 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c         |   6 +-
 drivers/gpu/drm/i915/i915_sysfs.c             |   8 +
 14 files changed, 486 insertions(+), 81 deletions(-)

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-02-07 16:13 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 13:30 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
2020-01-10 13:30 ` [Intel-gfx] [RFC 1/8] drm/i915: Expose list of clients in sysfs Tvrtko Ursulin
2020-01-10 13:36   ` Chris Wilson
2020-01-10 13:30 ` [Intel-gfx] [RFC 2/8] drm/i915: Update client name on context create Tvrtko Ursulin
2020-01-10 13:39   ` Chris Wilson
2020-01-10 13:30 ` [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness Tvrtko Ursulin
2020-01-10 13:46   ` Chris Wilson
2020-01-10 13:30 ` [Intel-gfx] [RFC 4/8] drm/i915: Track all user contexts per client Tvrtko Ursulin
2020-01-10 13:30 ` [Intel-gfx] [RFC 5/8] drm/i915: Contexts can use struct pid stored in the client Tvrtko Ursulin
2020-01-10 13:42   ` Chris Wilson
2020-01-30 16:11     ` Tvrtko Ursulin
2020-01-10 13:30 ` [Intel-gfx] [RFC 6/8] drm/i915: Expose per-engine client busyness Tvrtko Ursulin
2020-01-10 13:58   ` Chris Wilson
2020-01-10 14:09     ` Tvrtko Ursulin
2020-01-10 14:12       ` Chris Wilson
2020-01-10 13:30 ` [Intel-gfx] [RFC 7/8] drm/i915: Track hw reported context runtime Tvrtko Ursulin
2020-01-10 14:03   ` Chris Wilson
2020-01-10 13:30 ` [Intel-gfx] [RFC 8/8] drm/i915: Fallback to hw context runtime when sw tracking is not available Tvrtko Ursulin
2020-01-10 16:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Per client engine busyness (rev3) Patchwork
2020-01-10 16:51 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-01-14  4:39 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2020-02-07 16:13 [Intel-gfx] [RFC 0/8] Per client engine busyness Tvrtko Ursulin
2019-12-19 18:00 Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.