diff mbox

[08/11] drm/i915/guc: Add client support to enable/disable GuC interrupts

Message ID 1508309222-26406-9-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Oct. 18, 2017, 6:46 a.m. UTC
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@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@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(-)

Comments

Tvrtko Ursulin Oct. 19, 2017, 10:19 a.m. UTC | #1
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@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@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@intel.com>

Regards,

Tvrtko
sagar.a.kamble@intel.com Oct. 21, 2017, 4:38 p.m. UTC | #2
On 10/19/2017 3:49 PM, Tvrtko Ursulin wrote:
>
> 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@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@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.
Yes. will update
>
> Also, spinlock not required here apart from documentation purposes?
Yes. Given the frequency of enable/disable of interrupts this spin_lock 
is not so important.
>
>> +        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.
Yes.
>
>> +
>>       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.
Yes.
>
>> +
>> +    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@intel.com>
>
> Regards,
>
> Tvrtko
diff mbox

Patch

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))
+		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);
+
 	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);
+
+	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);