On 7/19/2016 4:54 PM, Tvrtko Ursulin wrote:
On 10/07/16 14:41, akash.goel@xxxxxxxxx wrote:
From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
This patch provides debugfs interface i915_guc_output_control for
on the fly enabling/disabling of logging in GuC firmware and controlling
the verbosity level of logs.
The value written to the file, should have bit 0 set to enable logging
and
bits 4-7 should contain the verbosity info.
v2: Add a forceful flush, to collect left over logs, on disabling
logging.
Useful for Validation.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++-
drivers/gpu/drm/i915/i915_guc_submission.c | 57
++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc.h | 1 +
3 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 5e35565..3c9c7f7 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2644,6 +2644,35 @@ static int i915_guc_log_dump(struct seq_file
*m, void *data)
return 0;
}
+static int
+i915_guc_log_control_set(void *data, u64 val)
+{
+ struct drm_device *dev = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
to_i915 should be used.
Sorry for missing this, need to use this at other places also.
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+
+ if (!i915.enable_guc_submission || !dev_priv->guc.log.obj) {
Wouldn't guc.log.obj be enough?
Actually failure in allocation of log buffer, at boot time, is not
considered fatal and submission through GuC is still done.
So i915.enable_guc_submission could be 1 with guc.log.obj as NULL.
+ ret = -EINVAL;
+ goto end;
+ }
+
+ intel_runtime_pm_get(dev_priv);
+ ret = i915_guc_log_control(dev, val);
+ intel_runtime_pm_put(dev_priv);
+
+end:
+ mutex_unlock(&dev->struct_mutex);
+ return ret;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
+ NULL, i915_guc_log_control_set,
+ "0x%08llx\n");
Does the readback still work with no get method?
readback will give a 'Permission denied' error
+
static int i915_edp_psr_status(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -5464,7 +5493,8 @@ static const struct i915_debugfs_files {
{"i915_fbc_false_color", &i915_fbc_fc_fops},
{"i915_dp_test_data", &i915_displayport_test_data_fops},
{"i915_dp_test_type", &i915_displayport_test_type_fops},
- {"i915_dp_test_active", &i915_displayport_test_active_fops}
+ {"i915_dp_test_active", &i915_displayport_test_active_fops},
+ {"i915_guc_log_control", &i915_guc_log_control_fops}
};
void intel_display_crc_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8cc31c6..2e3b723 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -193,6 +193,16 @@ static int host2guc_force_logbuffer_flush(struct
intel_guc *guc)
return host2guc_action(guc, data, 2);
}
+static int host2guc_logging_control(struct intel_guc *guc, u32
control_val)
+{
+ u32 data[2];
+
+ data[0] = HOST2GUC_ACTION_UK_LOG_ENABLE_LOGGING;
+ data[1] = control_val;
+
+ return host2guc_action(guc, data, 2);
+}
+
/*
* Initialise, update, or clear doorbell data shared with the GuC
*
@@ -1455,3 +1465,50 @@ void i915_guc_register(struct drm_device *dev)
guc_log_late_setup(dev);
mutex_unlock(&dev->struct_mutex);
}
+
+int i915_guc_log_control(struct drm_device *dev, uint64_t control_val)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
to_i915
Actually, function should take dev_priv if not even guc depending on the
established convention in the file.
Ok for all the new logging related exported functions, will use dev_priv.
+ union guc_log_control log_param;
+ int ret;
+
+ log_param.logging_enabled = control_val & 0x1;
+ log_param.verbosity = (control_val >> 4) & 0xF;
+
+ if (log_param.verbosity < GUC_LOG_VERBOSITY_MIN ||
+ log_param.verbosity > GUC_LOG_VERBOSITY_MAX)
+ return -EINVAL;
+
+ /* This combination doesn't make sense & won't have any effect */
+ if (!log_param.logging_enabled && (i915.guc_log_level < 0))
+ return -EINVAL;
Hm, disabling while already disabled - why should that return an error?
Might be annoying in scripts.
Just to make the User aware. Ok will suppress this and return 0.
+
+ ret = host2guc_logging_control(&dev_priv->guc, log_param.value);
+ if (ret < 0) {
+ DRM_DEBUG_DRIVER("host2guc action failed\n");
Add ret to the log since it is easy?
fine will do that.
+ return ret;
+ }
+
+ i915.guc_log_level = log_param.verbosity;
+
+ /* If log_level was set as -1 at boot time, then the relay
channel file
+ * wouldn't have been created by now and interrupts also would
not have
+ * been enabled.
+ */
+ if (!dev_priv->guc.log.relay_chan) {
+ ret = guc_log_late_setup(dev);
+ if (!ret)
+ gen9_enable_guc_interrupts(dev_priv);
Hm, look at the above and below, do we need to create the relay channel
if logging_enabled == false ?
Can come here only if logging is enabled, by the virtue of above check,
/* This combination doesn't make sense & won't have any effect */
if (!log_param.logging_enabled && (i915.guc_log_level < 0))
return -EINVAL;
When guc_log_level < 0, first write on this file by User should be to
enable logging.
Best regards
Akash
+ } else if (!log_param.logging_enabled) {
+ /* Once logging is disabled, GuC won't generate logs & send an
+ * interrupt. But there could be some data in the log buffer
+ * which is yet to be captured. So request GuC to update the log
+ * buffer state and send the flush interrupt so that Host can
+ * collect the left over logs also.
+ */
+ flush_work(&dev_priv->guc.events_work);
+ host2guc_force_logbuffer_flush(&dev_priv->guc);
+ }
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h
b/drivers/gpu/drm/i915/intel_guc.h
index ed773b5..d56bde6 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -178,5 +178,6 @@ void i915_guc_capture_logs(struct drm_device *dev);
void i915_guc_capture_logs_on_reset(struct drm_device *dev);
void i915_guc_register(struct drm_device *dev);
void i915_guc_unregister(struct drm_device *dev);
+int i915_guc_log_control(struct drm_device *dev, uint64_t control_val);
#endif
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
