[Intel-gfx] [PATCH 08/15] drm/i915/guc: Split relay control and GuC log level
Sagar Arun Kamble
sagar.a.kamble at intel.com
Tue Mar 6 07:16:04 UTC 2018
On 2/27/2018 6:22 PM, Michał Winiarski wrote:
> Those two concepts are really separate. Since GuC is writing data into
> its own buffer and we even provide a way for userspace to read directly
> from it using i915_guc_log_dump debugfs, there's no real reason to tie
> log level with relay creation.
> Let's create a separate debugfs, giving userspace a way to create a
> relay on demand, when it wants to read a continuous log rather than a
> snapshot.
>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 56 ++++++++++++++++++++++----
> drivers/gpu/drm/i915/i915_drv.c | 2 -
> drivers/gpu/drm/i915/intel_guc_log.c | 76 +++++++++++++++---------------------
> drivers/gpu/drm/i915/intel_guc_log.h | 9 +++--
> drivers/gpu/drm/i915/intel_uc.c | 22 -----------
> drivers/gpu/drm/i915/intel_uc.h | 2 -
> 6 files changed, 86 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 58983cafaece..4bd24bbe7966 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2496,32 +2496,73 @@ 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)
> +static int i915_guc_log_level_get(void *data, u64 *val)
> {
> struct drm_i915_private *dev_priv = data;
s/dev_priv/i915 here and at other places
>
> if (!USES_GUC(dev_priv))
> return -ENODEV;
>
> - *val = intel_guc_log_control_get(&dev_priv->guc);
> + *val = intel_guc_log_level_get(&dev_priv->guc);
>
> return 0;
> }
>
> -static int i915_guc_log_control_set(void *data, u64 val)
> +static int i915_guc_log_level_set(void *data, u64 val)
> {
> struct drm_i915_private *dev_priv = data;
>
> if (!USES_GUC(dev_priv))
> return -ENODEV;
>
> - return intel_guc_log_control_set(&dev_priv->guc, val);
> + return intel_guc_log_level_set(&dev_priv->guc, val);
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
> - i915_guc_log_control_get, i915_guc_log_control_set,
> +DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_level_fops,
> + i915_guc_log_level_get, i915_guc_log_level_set,
> "%lld\n");
>
> +static int i915_guc_log_relay_open(struct inode *inode, struct file *file)
> +{
> + struct drm_i915_private *dev_priv = inode->i_private;
> +
> + if (!USES_GUC(dev_priv))
> + return -ENODEV;
> +
> + file->private_data = dev_priv;
> +
write is passing guc struct so should we just set private_data to
&dev_priv->guc?
> + return intel_guc_log_relay_open(&dev_priv->guc);
> +}
> +
> +static ssize_t
> +i915_guc_log_relay_write(struct file *filp,
> + const char __user *ubuf,
> + size_t cnt,
> + loff_t *ppos)
> +{
> + struct drm_i915_private *dev_priv = filp->private_data;
> +
> + intel_guc_log_relay_flush(&dev_priv->guc);
> +
> + return cnt;
> +}
<snip>
> +void intel_guc_log_relay_close(struct intel_guc *guc)
> +{
> GEM_BUG_ON(!guc_log_has_runtime(guc));
>
guc_log_has_runtime() check has to be with runtime.lock mutex locked.
With comments addressed:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> + guc_log_flush_irq_disable(guc);
> + flush_work(&guc->log.runtime.flush_work);
> +
> + mutex_lock(&guc->log.runtime.lock);
> guc_log_unmap(guc);
> guc_log_relay_destroy(guc);
> -
> mutex_unlock(&guc->log.runtime.lock);
> }
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
> index 8c26cce77a98..df91f12a36ed 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/intel_guc_log.h
> @@ -61,9 +61,10 @@ struct intel_guc_log {
> int intel_guc_log_create(struct intel_guc *guc);
> void intel_guc_log_destroy(struct intel_guc *guc);
> void intel_guc_log_init_early(struct intel_guc *guc);
> -int intel_guc_log_control_get(struct intel_guc *guc);
> -int intel_guc_log_control_set(struct intel_guc *guc, u64 control_val);
> -int intel_guc_log_register(struct intel_guc *guc);
> -void intel_guc_log_unregister(struct intel_guc *guc);
> +int intel_guc_log_level_get(struct intel_guc *guc);
> +int intel_guc_log_level_set(struct intel_guc *guc, u64 control_val);
> +int intel_guc_log_relay_open(struct intel_guc *guc);
> +void intel_guc_log_relay_close(struct intel_guc *guc);
> +void intel_guc_log_relay_flush(struct intel_guc *guc);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 55a9b5b673e0..27e1f4c43b7b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -219,28 +219,6 @@ static void guc_free_load_err_log(struct intel_guc *guc)
> i915_gem_object_put(guc->load_err_log);
> }
>
> -int intel_uc_log_register(struct drm_i915_private *dev_priv)
> -{
> - int ret = 0;
> -
> - if (!USES_GUC(dev_priv))
> - return 0;
> -
> - if (i915_modparams.guc_log_level)
> - ret = intel_guc_log_register(&dev_priv->guc);
> -
> - return ret;
> -}
> -
> -void intel_uc_log_unregister(struct drm_i915_private *dev_priv)
> -{
> - if (!USES_GUC(dev_priv))
> - return;
> -
> - if (i915_modparams.guc_log_level)
> - intel_guc_log_unregister(&dev_priv->guc);
> -}
> -
> static int guc_enable_communication(struct intel_guc *guc)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 9bc22103396d..8a7249722ef1 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -31,8 +31,6 @@
> void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
> void intel_uc_init_early(struct drm_i915_private *dev_priv);
> void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
> -int intel_uc_log_register(struct drm_i915_private *dev_priv);
> -void intel_uc_log_unregister(struct drm_i915_private *dev_priv);
> void intel_uc_init_fw(struct drm_i915_private *dev_priv);
> void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
> int intel_uc_init_wq(struct drm_i915_private *dev_priv);
--
Thanks,
Sagar
More information about the Intel-gfx
mailing list