On 8/12/2016 9:52 PM, Tvrtko Ursulin wrote:
On 12/08/16 07:25, akash.goel@xxxxxxxxx wrote:
From: Akash Goel <akash.goel@xxxxxxxxx>
As per the current i915 Driver load sequence, debugfs registration is
done
at the end and so the relay channel debugfs file is also created after
that
but the GuC firmware is loaded much earlier in the sequence.
As a result Driver could miss capturing the boot-time logs of GuC
firmware
if there are flush interrupts from the GuC side.
Relay has a provision to support early logging where initially only relay
channel can be created, to have buffers for storing logs, and later on
channel can be associated with a debugfs file at appropriate time.
Have availed that, which allows Driver to capture boot time logs also,
which can be collected once Userspace comes up.
Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 61
+++++++++++++++++++++---------
1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index af48f62..1c287d7 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1099,25 +1099,12 @@ static void guc_remove_log_relay_file(struct
intel_guc *guc)
relay_close(guc->log.relay_chan);
}
-static int guc_create_log_relay_file(struct intel_guc *guc)
+static int guc_create_relay_channel(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct rchan *guc_log_relay_chan;
- struct dentry *log_dir;
size_t n_subbufs, subbuf_size;
- /* For now create the log file in /sys/kernel/debug/dri/0 dir */
- log_dir = dev_priv->drm.primary->debugfs_root;
-
- /* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
- * not mounted and so can't create the relay file.
- * The relay API seems to fit well with debugfs only.
It only needs a dentry, I don't see that it has to be a debugfs one.
Besides dentry, there are other requirements for using relay, which can
be met only for a debugfs file.
debugfs wasn't the preferred choice to place the log file, but had no
other option, as relay API is compatible with debugfs only.
Also retrieving dentry of a file is not so straight forward, as it might
seem (spent considerable time on this initially).
- */
- if (!log_dir) {
- DRM_DEBUG_DRIVER("Parent debugfs directory not available
yet\n");
- return -ENODEV;
- }
-
/* Keep the size of sub buffers same as shared log buffer */
subbuf_size = guc->log.obj->base.size;
/* Store up to 8 snaphosts, which is large enough to buffer
sufficient
@@ -1127,7 +1114,7 @@ static int guc_create_log_relay_file(struct
intel_guc *guc)
*/
n_subbufs = 8;
- guc_log_relay_chan = relay_open("guc_log", log_dir,
+ guc_log_relay_chan = relay_open(NULL, NULL,
subbuf_size, n_subbufs, &relay_callbacks, dev_priv);
if (!guc_log_relay_chan) {
@@ -1140,6 +1127,33 @@ static int guc_create_log_relay_file(struct
intel_guc *guc)
return 0;
}
+static int guc_create_log_relay_file(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct dentry *log_dir;
+ int ret;
+
+ /* For now create the log file in /sys/kernel/debug/dri/0 dir */
+ log_dir = dev_priv->drm.primary->debugfs_root;
+
+ /* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
+ * not mounted and so can't create the relay file.
+ * The relay API seems to fit well with debugfs only.
+ */
+ if (!log_dir) {
+ DRM_DEBUG_DRIVER("Parent debugfs directory not available
yet\n");
+ return -ENODEV;
+ }
+
+ ret = relay_late_setup_files(guc->log.relay_chan, "guc_log",
log_dir);
+ if (ret) {
+ DRM_DEBUG_DRIVER("Couldn't associate the channel with file
%d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static void guc_log_cleanup(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1167,7 +1181,7 @@ static int guc_create_log_extras(struct
intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
void *vaddr;
- int ret;
+ int ret = 0;
lockdep_assert_held(&dev_priv->drm.struct_mutex);
@@ -1190,7 +1204,15 @@ static int guc_create_log_extras(struct
intel_guc *guc)
guc->log.buf_addr = vaddr;
}
- return 0;
+ if (!guc->log.relay_chan) {
+ /* Create a relay channel, so that we have buffers for storing
+ * the GuC firmware logs, the channel will be linked with a file
+ * later on when debugfs is registered.
+ */
+ ret = guc_create_relay_channel(guc);
+ }
+
+ return ret;
}
static void guc_create_log(struct intel_guc *guc)
@@ -1231,6 +1253,7 @@ static void guc_create_log(struct intel_guc *guc)
guc->log.obj = obj;
if (guc_create_log_extras(guc)) {
+ guc_log_cleanup(guc);
gem_release_guc_obj(guc->log.obj);
guc->log.obj = NULL;
i915.guc_log_level = -1;
@@ -1265,6 +1288,10 @@ static int guc_log_late_setup(struct intel_guc
*guc)
if (ret)
goto err;
+ /* Parent debugfs dir should be available by now, associate the
already
+ * opened relay channel with a debugfs file, which will then
allow User
+ * to pull the logs.
+ */
ret = guc_create_log_relay_file(guc);
if (ret)
goto err;
Can't spot any problems.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
I suppose you will also have to add some IGTs to exercise the whole
thing. Stopping/starting the logging, reading back, capturing some
commands etc. Or contract someone from validation to do it. :) Are there
any such plans?
Sure will seek help from Validation folks on this.
Best regards
Akash
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
