On Sat, May 28, 2016 at 01:12:54AM +0530, akash.goel@xxxxxxxxx wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
