Re: [PATCH 15/18] drm/i915: Debugfs support for GuC logging control

Tvrtko Ursulin
2016-08-15T16:03:38Z

On 15/08/16 15:49, akash.goel@intel.com wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> 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.
>
> v3: Besides minor cleanup, implement read method for the debugfs file and
> set the guc_log_level to -1 when logging is disabled. (Tvrtko)
>
> v4: Minor cleanup & rebase. (Tvrtko)
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 44 ++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_guc_submission.c | 62 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc.h | 1 +
> 3 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index aaa51bc..bfc22bf 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2626,6 +2626,47 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
> return 0;
> }
>
> +static int i915_guc_log_control_get(void *data, u64 *val)
> +{
> + struct drm_device *dev = data;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!dev_priv->guc.log.vma)
> + return -EINVAL;
> +
> + *val = i915.guc_log_level;
> +
> + return 0;
> +}
> +
> +static int i915_guc_log_control_set(void *data, u64 val)
> +{
> + struct drm_device *dev = data;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + int ret;
> +
> + ret = mutex_lock_interruptible(&dev->struct_mutex);
> + if (ret)
> + return ret;
> +
> + if (!dev_priv->guc.log.vma) {
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + intel_runtime_pm_get(dev_priv);
> + ret = i915_guc_log_control(dev_priv, val);
> + intel_runtime_pm_put(dev_priv);
> +
> +end:
> + mutex_unlock(&dev->struct_mutex);
> + return ret;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
> + i915_guc_log_control_get, i915_guc_log_control_set,
> + "%lld\n");
> +
> static int i915_edp_psr_status(struct seq_file *m, void *data)
> {
> struct drm_info_node *node = m->private;
> @@ -5436,7 +5477,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 85df2f3..be6c727 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -195,6 +195,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
> *
> @@ -1585,3 +1595,55 @@ void i915_guc_register(struct drm_i915_private *dev_priv)
> guc_log_late_setup(&dev_priv->guc);
> mutex_unlock(&dev_priv->drm.struct_mutex);
> }
> +
> +int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
> +{
> + union guc_log_control log_param;
> + int ret;
> +
> + log_param.value = control_val;
> +
> + 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 0;
> +
> + ret = host2guc_logging_control(&dev_priv->guc, log_param.value);
> + if (ret < 0) {
> + DRM_DEBUG_DRIVER("host2guc action failed %d\n", ret);
> + 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_priv->guc);
> + if (!ret)
> + gen9_enable_guc_interrupts(dev_priv);
> + } 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 then collect the left over logs.
> + */
> + i915_guc_flush_logs(dev_priv, true);
> +
> + /* GuC would have updated log buffer by now, so capture it */
> + i915_guc_capture_logs(dev_priv);
> +
> + /* As logging is disabled, update log level to reflect that */
> + i915.guc_log_level = -1;
> + } else {
> + /* In case interrupts were disabled, enable them now */
> + gen9_enable_guc_interrupts(dev_priv);
> + }
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index d7eda42..b56fe24 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -185,5 +185,6 @@ void i915_guc_capture_logs(struct drm_i915_private *dev_priv);
> void i915_guc_flush_logs(struct drm_i915_private *dev_priv, bool can_wait);
> void i915_guc_register(struct drm_i915_private *dev_priv);
> void i915_guc_unregister(struct drm_i915_private *dev_priv);
> +int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
>
> #endif
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko