[Intel-gfx] [PATCH 08/11] drm/i915/guc: Add client support to enable/disable GuC interrupts
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Oct 19 10:19:06 UTC 2017
On 18/10/2017 07:46, Sagar Arun Kamble wrote:
> This patch adds support to enable/disable GuC interrupts for different
> features without impacting others needs. Currently GuC log capture and
> CT buffer receive mechanisms use the GuC interrupts. GuC interrupts are
> currently enabled and disabled in different Logging scenarios all gated
> by log level.
>
> v2: Rebase with all GuC interrupt handlers moved to intel_guc.c. Handling
> multiple clients for GuC interrupts enable/disable.
> (Michal Wajdeczko)
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++++++
> drivers/gpu/drm/i915/intel_guc.c | 21 ++++++++++++++-------
> drivers/gpu/drm/i915/intel_guc.h | 11 ++++++++---
> drivers/gpu/drm/i915/intel_guc_log.c | 6 +++---
> drivers/gpu/drm/i915/intel_uc.c | 6 +++---
> 5 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0bb6e01..bd421da 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2531,6 +2531,14 @@ static int i915_guc_info(struct seq_file *m, void *data)
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> const struct intel_guc *guc = &dev_priv->guc;
>
> + seq_puts(m, "GuC Interrupt Clients: ");
> + spin_lock_irq(&dev_priv->irq_lock);
> + if (guc->interrupt_clients & BIT(GUC_INTR_CLIENT_LOG))
Could use test_bit.
Also, spinlock not required here apart from documentation purposes?
> + seq_puts(m, "GuC Logging\n");
> + else
> + seq_puts(m, "None\n");
> + spin_unlock_irq(&dev_priv->irq_lock);
> +
> if (!check_guc_submission(m))
> return 0;
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 959057a..fbd27ea 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -275,7 +275,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
> return 0;
>
> if (i915_modparams.guc_log_level >= 0)
> - intel_disable_guc_interrupts(guc);
> + intel_put_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>
> ctx = dev_priv->kernel_context;
>
> @@ -303,7 +303,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
> return 0;
>
> if (i915_modparams.guc_log_level >= 0)
> - intel_enable_guc_interrupts(guc);
> + intel_get_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>
> ctx = dev_priv->kernel_context;
>
> @@ -378,26 +378,33 @@ void intel_reset_guc_interrupts(struct intel_guc *guc)
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> -void intel_enable_guc_interrupts(struct intel_guc *guc)
> +void intel_get_guc_interrupts(struct intel_guc *guc, enum guc_intr_client id)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>
> spin_lock_irq(&dev_priv->irq_lock);
> - if (!dev_priv->guc.interrupts_enabled) {
> + if (!guc->interrupt_clients) {
> WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) &
> dev_priv->pm_guc_events);
> - dev_priv->guc.interrupts_enabled = true;
> gen6_enable_pm_irq(dev_priv, dev_priv->pm_guc_events);
> }
> + set_bit(id, &guc->interrupt_clients);
Can use __set_bit to save one atomic since it is already under the spinlock.
> +
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> -void intel_disable_guc_interrupts(struct intel_guc *guc)
> +void intel_put_guc_interrupts(struct intel_guc *guc, enum guc_intr_client id)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>
> spin_lock_irq(&dev_priv->irq_lock);
> - dev_priv->guc.interrupts_enabled = false;
> +
> + clear_bit(id, &guc->interrupt_clients);
Equally as above __clear_bit would work.
> +
> + if (guc->interrupt_clients) {
> + spin_unlock_irq(&dev_priv->irq_lock);
> + return;
> + }
>
> gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events);
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index e89b4ae..4d58bf7 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -34,6 +34,11 @@
> #include "i915_guc_reg.h"
> #include "i915_vma.h"
>
> +enum guc_intr_client {
> + GUC_INTR_CLIENT_LOG = 0,
> + GUC_INTR_CLIENT_COUNT
> +};
> +
> /*
> * Top level structure of GuC. It handles firmware loading and manages client
> * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
> @@ -48,7 +53,7 @@ struct intel_guc {
> struct drm_i915_gem_object *load_err_log;
>
> /* intel_guc_recv interrupt related state */
> - bool interrupts_enabled;
> + unsigned long interrupt_clients;
>
> struct i915_vma *ads_vma;
> struct i915_vma *stage_desc_pool;
> @@ -117,8 +122,8 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
> u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> void intel_reset_guc_interrupts(struct intel_guc *guc);
> -void intel_enable_guc_interrupts(struct intel_guc *guc);
> -void intel_disable_guc_interrupts(struct intel_guc *guc);
> +void intel_get_guc_interrupts(struct intel_guc *guc, enum guc_intr_client id);
> +void intel_put_guc_interrupts(struct intel_guc *guc, enum guc_intr_client id);
> void intel_guc_irq_handler(struct intel_guc *guc, u32 pm_iir);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index ed239cb..8c41d9a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -508,7 +508,7 @@ static void guc_flush_logs(struct intel_guc *guc)
> return;
>
> /* First disable the interrupts, will be renabled afterwards */
> - intel_disable_guc_interrupts(guc);
> + intel_put_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>
> /* Before initiating the forceful flush, wait for any pending/ongoing
> * flush to complete otherwise forceful flush may not actually happen.
> @@ -626,7 +626,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
> }
>
> /* GuC logging is currently the only user of Guc2Host interrupts */
> - intel_enable_guc_interrupts(guc);
> + intel_get_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
> } else {
> /* Once logging is disabled, GuC won't generate logs & send an
> * interrupt. But there could be some data in the log buffer
> @@ -658,7 +658,7 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
> /* GuC logging is currently the only user of Guc2Host interrupts */
> if (i915_modparams.guc_log_level >= 0) {
> intel_runtime_pm_get(dev_priv);
> - intel_disable_guc_interrupts(&dev_priv->guc);
> + intel_put_guc_interrupts(&dev_priv->guc, GUC_INTR_CLIENT_LOG);
> intel_runtime_pm_put(dev_priv);
> }
> /*
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 95c5ec4..d96c847 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -213,7 +213,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> goto err_log_capture;
>
> if (i915_modparams.guc_log_level >= 0)
> - intel_enable_guc_interrupts(guc);
> + intel_get_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>
> ret = guc_enable_communication(guc);
> if (ret)
> @@ -247,7 +247,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> guc_disable_communication(guc);
> err_interrupts:
> if (i915_modparams.guc_log_level >= 0)
> - intel_disable_guc_interrupts(guc);
> + intel_put_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
> err_log_capture:
> guc_capture_load_err_log(guc);
> err_submission:
> @@ -288,7 +288,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> guc_disable_communication(&dev_priv->guc);
>
> if (i915_modparams.guc_log_level >= 0)
> - intel_disable_guc_interrupts(&dev_priv->guc);
> + intel_put_guc_interrupts(&dev_priv->guc, GUC_INTR_CLIENT_LOG);
>
> if (i915_modparams.enable_guc_submission)
> i915_guc_submission_fini(dev_priv);
>
With the bit ops tidy:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list