| Message ID | 1471272599-14586-19-git-send-email-akash.goel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/08/16 15:49, akash.goel@intel.com wrote: > From: Akash Goel <akash.goel@intel.com> > > 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. > > v2: > - Remove the couple of FIXMEs, as now the relay channel will be created > early before enabling the flush interrupts, so no possibility of relay > channel pointer being modified & read at the same time from 2 different > execution contexts. > - Rebase. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Akash Goel <akash.goel@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 66 +++++++++++++++++++++--------- > 1 file changed, 46 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 8733c19..34e4335 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -937,13 +937,39 @@ 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; > > + /* Keep the size of sub buffers same as shared log buffer */ > + subbuf_size = guc->log.vma->obj->base.size; > + > + /* Store up to 8 snapshots, which is large enough to buffer sufficient > + * boot time logs and provides enough leeway to User, in terms of > + * latency, for consuming the logs from relay. Also doesn't take > + * up too much memory. > + */ > + n_subbufs = 8; > + > + guc_log_relay_chan = relay_open(NULL, NULL, subbuf_size, > + n_subbufs, &relay_callbacks, dev_priv); > + if (!guc_log_relay_chan) { > + DRM_ERROR("Couldn't create relay chan for GuC logging\n"); > + return -ENOMEM; > + } > + > + guc->log.relay_chan = guc_log_relay_chan; > + 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; > > @@ -963,25 +989,12 @@ static int guc_create_log_relay_file(struct intel_guc *guc) > return -ENODEV; > } > > - /* Keep the size of sub buffers same as shared log buffer */ > - subbuf_size = guc->log.vma->obj->base.size; > - > - /* Store up to 8 snapshots, which is large enough to buffer sufficient > - * boot time logs and provides enough leeway to User, in terms of > - * latency, for consuming the logs from relay. Also doesn't take > - * up too much memory. > - */ > - n_subbufs = 8; > - > - guc_log_relay_chan = relay_open("guc_log", log_dir, subbuf_size, > - n_subbufs, &relay_callbacks, dev_priv); > - if (!guc_log_relay_chan) { > - DRM_ERROR("Couldn't create relay chan for GuC logging\n"); > - return -ENOMEM; > + ret = relay_late_setup_files(guc->log.relay_chan, "guc_log", log_dir); > + if (ret) { > + DRM_ERROR("Couldn't associate relay chan with file %d\n", ret); > + return ret; > } > > - /* FIXME: Cover the update under a lock ? */ > - guc->log.relay_chan = guc_log_relay_chan; > return 0; > } > > @@ -1001,7 +1014,6 @@ static void guc_move_to_next_buf(struct intel_guc *guc) > > static void *guc_get_write_buffer(struct intel_guc *guc) > { > - /* FIXME: Cover the check under a lock ? */ > if (!guc->log.relay_chan) > return NULL; > > @@ -1227,6 +1239,16 @@ static int guc_create_log_extras(struct intel_guc *guc) > guc->log.buf_addr = vaddr; > } > > + 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); > + if (ret) > + return ret; > + } > + > if (!guc->log.flush_wq) { > INIT_WORK(&guc->log.flush_work, guc_capture_logs_work); > > @@ -1314,6 +1336,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; > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 8733c19..34e4335 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -937,13 +937,39 @@ 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; + /* Keep the size of sub buffers same as shared log buffer */ + subbuf_size = guc->log.vma->obj->base.size; + + /* Store up to 8 snapshots, which is large enough to buffer sufficient + * boot time logs and provides enough leeway to User, in terms of + * latency, for consuming the logs from relay. Also doesn't take + * up too much memory. + */ + n_subbufs = 8; + + guc_log_relay_chan = relay_open(NULL, NULL, subbuf_size, + n_subbufs, &relay_callbacks, dev_priv); + if (!guc_log_relay_chan) { + DRM_ERROR("Couldn't create relay chan for GuC logging\n"); + return -ENOMEM; + } + + guc->log.relay_chan = guc_log_relay_chan; + 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; @@ -963,25 +989,12 @@ static int guc_create_log_relay_file(struct intel_guc *guc) return -ENODEV; } - /* Keep the size of sub buffers same as shared log buffer */ - subbuf_size = guc->log.vma->obj->base.size; - - /* Store up to 8 snapshots, which is large enough to buffer sufficient - * boot time logs and provides enough leeway to User, in terms of - * latency, for consuming the logs from relay. Also doesn't take - * up too much memory. - */ - n_subbufs = 8; - - guc_log_relay_chan = relay_open("guc_log", log_dir, subbuf_size, - n_subbufs, &relay_callbacks, dev_priv); - if (!guc_log_relay_chan) { - DRM_ERROR("Couldn't create relay chan for GuC logging\n"); - return -ENOMEM; + ret = relay_late_setup_files(guc->log.relay_chan, "guc_log", log_dir); + if (ret) { + DRM_ERROR("Couldn't associate relay chan with file %d\n", ret); + return ret; } - /* FIXME: Cover the update under a lock ? */ - guc->log.relay_chan = guc_log_relay_chan; return 0; } @@ -1001,7 +1014,6 @@ static void guc_move_to_next_buf(struct intel_guc *guc) static void *guc_get_write_buffer(struct intel_guc *guc) { - /* FIXME: Cover the check under a lock ? */ if (!guc->log.relay_chan) return NULL; @@ -1227,6 +1239,16 @@ static int guc_create_log_extras(struct intel_guc *guc) guc->log.buf_addr = vaddr; } + 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); + if (ret) + return ret; + } + if (!guc->log.flush_wq) { INIT_WORK(&guc->log.flush_work, guc_capture_logs_work); @@ -1314,6 +1336,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;