[Intel-gfx] [RFC 03/12] drm/i915: Support for GuC interrupts

[Chris Wilson]

On Sat, May 28, 2016 at 01:12:54AM +0530, akash.goel at intel.com wrote:
> From: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> 
> There are certain types of interrupts which Host can recieve from GuC.
> GuC ukernel sends an interrupt to Host for certain events, like for
> example retrieve/consume the logs generated by ukernel.
> This patch adds support to receive interrupts from GuC but currently
> enables & partially handles only the interrupt sent by GuC ukernel.
> Future patches will add support for handling other interrupt types.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Signed-off-by: Akash Goel <akash.goel at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |   1 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |   2 +
>  drivers/gpu/drm/i915/i915_irq.c            | 100 ++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h            |  11 ++++
>  drivers/gpu/drm/i915/intel_drv.h           |   3 +
>  drivers/gpu/drm/i915/intel_guc.h           |   5 ++
>  drivers/gpu/drm/i915/intel_guc_loader.c    |   1 +
>  7 files changed, 120 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4c8e34..7aae033 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1790,6 +1790,7 @@ struct drm_i915_private {
>  	u32 gt_irq_mask;
>  	u32 pm_irq_mask;
>  	u32 pm_rps_events;
> +	u32 guc_events;
>  	u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
>  	struct i915_hotplug hotplug;
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index da7c242..c2f3a67 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -990,6 +990,8 @@ int intel_guc_suspend(struct drm_device *dev)
>  	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>  		return 0;
>  
> +	gen8_disable_guc_interrupts(dev);
> +
>  	ctx = dev_priv->kernel_context;
>  
>  	data[0] = HOST2GUC_ACTION_ENTER_S_STATE;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index caaf1e2..b4294a8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -170,6 +170,7 @@ static void gen5_assert_iir_is_zero(struct drm_i915_private *dev_priv,
>  } while (0)
>  
>  static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> +static void gen8_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
>  
>  /* For display hotplug interrupt */
>  static inline void
> @@ -400,6 +401,55 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
>  	synchronize_irq(dev_priv->dev->irq);
>  }
>  
> +void gen8_reset_guc_interrupts(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	i915_reg_t reg = gen6_pm_iir(dev_priv);

>From the looks of this we have multiple shadows for the same register.
That's very bad.

Now the platforms might be mutually exclusive, but it is still a mistake
that will catch us out.

> +	spin_lock_irq(&dev_priv->irq_lock);
> +	I915_WRITE(reg, dev_priv->guc_events);
> +	I915_WRITE(reg, dev_priv->guc_events);

What? Not even the tiniest of comments to explain?

> +	POSTING_READ(reg);

Again. Not even the tiniest of comments to explain?

> +	spin_unlock_irq(&dev_priv->irq_lock);
> +}
> +
> +void gen8_enable_guc_interrupts(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	if (!dev_priv->guc.interrupts_enabled) {
> +		WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
> +					dev_priv->guc_events);
> +		dev_priv->guc.interrupts_enabled = true;
> +		I915_WRITE(gen6_pm_ier(dev_priv),
> +		      I915_READ(gen6_pm_ier(dev_priv)) | dev_priv->guc_events);

ier should be known, rmw on the reg should not be required.

> +		gen6_enable_pm_irq(dev_priv, dev_priv->guc_events);
> +	}
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +}
> +
> +void gen8_disable_guc_interrupts(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	dev_priv->guc.interrupts_enabled = false;
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
> +	cancel_work_sync(&dev_priv->guc.events_work);
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +
> +	__gen6_disable_pm_irq(dev_priv, dev_priv->guc_events);
> +	I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
> +				~dev_priv->guc_events);
> +
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
> +	synchronize_irq(dev->irq);
> +}
> +
>  /**
>   * bdw_update_port_irq - update DE port interrupt
>   * @dev_priv: driver private
> @@ -1174,6 +1224,27 @@ out:
>  	ENABLE_RPM_WAKEREF_ASSERTS(dev_priv);
>  }
>  
> +static void gen8_guc2host_events_work(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, struct drm_i915_private, guc.events_work);
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	/* Speed up work cancelation during disabling guc interrupts. */
> +	if (!dev_priv->guc.interrupts_enabled) {
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +		return;
> +	}
> +
> +	DISABLE_RPM_WAKEREF_ASSERTS(dev_priv);

This just shouts that the code is broken.


>  void intel_crt_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 41601c7..e20792d 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -125,6 +125,11 @@ struct intel_guc {
>  	struct intel_guc_fw guc_fw;
>  	uint32_t log_flags;
>  	struct drm_i915_gem_object *log_obj;
> +	/*
> +	 * work, interrupts_enabled are protected by dev_priv->irq_lock
> +	 */
> +	struct work_struct events_work;

The work gets added here, yet bugs are fixed for the worker in later
patches. Squash in the bug fixes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre