[Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts

Goel, Akash akash.goel at intel.com
Mon Jul 11 13:15:50 UTC 2016



On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote:
>
> On 10/07/16 14:41, 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.
>>
>> v2:
>> - Use common low level routines for PM IER/IIR programming (Chris)
>> - Rename interrupt functions to gen9_xxx from gen8_xxx (Chris)
>> - Replace disabling of wake ref asserts with rpm get/put (Chris)
>>
>> v3:
>> - Update comments for more clarity. (Tvrtko)
>> - Remove the masking of GuC interrupt, which was kept masked till the
>>    start of bottom half, its not really needed as there is only a
>>    single instance of work item & wq is ordered. (Tvrtko)
>>
>> v4:
>> - Rebase.
>> - Rename guc_events to pm_guc_events so as to be indicative of the
>>    register/control block it is associated with. (Chris)
>> - Add handling for back to back log buffer flush interrupts.
>>
>> 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 |  5 ++
>>   drivers/gpu/drm/i915/i915_irq.c            | 98
>> ++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_reg.h            | 11 ++++
>>   drivers/gpu/drm/i915/intel_drv.h           |  3 +
>>   drivers/gpu/drm/i915/intel_guc.h           |  4 ++
>>   drivers/gpu/drm/i915/intel_guc_loader.c    |  4 ++
>>   7 files changed, 122 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index c3a579f..6e2ddfa 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1794,6 +1794,7 @@ struct drm_i915_private {
>>       u32 pm_imr;
>>       u32 pm_ier;
>>       u32 pm_rps_events;
>> +    u32 pm_guc_events;
>>       u32 pipestat_irq_mask[I915_MAX_PIPES];
>>
>>       struct i915_hotplug hotplug;
>>
>> +
>>   /**
>>    * bdw_update_port_irq - update DE port interrupt
>>    * @dev_priv: driver private
>> @@ -1174,6 +1208,21 @@ static void gen6_pm_rps_work(struct work_struct
>> *work)
>>       mutex_unlock(&dev_priv->rps.hw_lock);
>>   }
>>
>> +static void gen9_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 cancellation during disabling guc interrupts. */
>> +    if (!dev_priv->guc.interrupts_enabled) {
>> +        spin_unlock_irq(&dev_priv->irq_lock);
>> +        return;
>> +    }
>> +    spin_unlock_irq(&dev_priv->irq_lock);
>> +
>> +    /* TODO: Handle the events for which GuC interrupted host */
>> +}
>>
>>   /**
>>    * ivybridge_parity_work - Workqueue called when a parity error
>> interrupt
>> @@ -1346,11 +1395,13 @@ static irqreturn_t gen8_gt_irq_ack(struct
>> drm_i915_private *dev_priv,
>>               DRM_ERROR("The master control interrupt lied (GT3)!\n");
>>       }
>>
>> -    if (master_ctl & GEN8_GT_PM_IRQ) {
>> +    if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
>>           gt_iir[2] = I915_READ_FW(GEN8_GT_IIR(2));
>> -        if (gt_iir[2] & dev_priv->pm_rps_events) {
>> +        if (gt_iir[2] & (dev_priv->pm_rps_events |
>> +                 dev_priv->pm_guc_events)) {
>>               I915_WRITE_FW(GEN8_GT_IIR(2),
>> -                      gt_iir[2] & dev_priv->pm_rps_events);
>> +                      gt_iir[2] & (dev_priv->pm_rps_events |
>> +                           dev_priv->pm_guc_events));
>>               ret = IRQ_HANDLED;
>>           } else
>>               DRM_ERROR("The master control interrupt lied (PM)!\n");
>> @@ -1382,6 +1433,9 @@ static void gen8_gt_irq_handler(struct
>> drm_i915_private *dev_priv,
>>
>>       if (gt_iir[2] & dev_priv->pm_rps_events)
>>           gen6_rps_irq_handler(dev_priv, gt_iir[2]);
>> +
>> +    if (gt_iir[2] & dev_priv->pm_guc_events)
>> +        gen9_guc_irq_handler(dev_priv, gt_iir[2]);
>>   }
>>
>>   static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
>> @@ -1628,6 +1682,38 @@ static void gen6_rps_irq_handler(struct
>> drm_i915_private *dev_priv, u32 pm_iir)
>>       }
>>   }
>>
>> +static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv,
>> u32 gt_iir)
>> +{
>> +    if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
>> +        spin_lock(&dev_priv->irq_lock);
>> +        if (dev_priv->guc.interrupts_enabled) {
>> +            /* Sample the log buffer flush related bits & clear them
>> +             * out now itself from the message identity register to
>> +             * minimize the probability of losing a flush interrupt,
>> +             * when there are back to back flush interrupts.
>> +             * There can be a new flush interrupt, for different log
>> +             * buffer type (like for ISR), whilst Host is handling
>> +             * one (for DPC). Since same bit is used in message
>> +             * register for ISR & DPC, it could happen that GuC
>> +             * sets the bit for 2nd interrupt but Host clears out
>> +             * the bit on handling the 1st interrupt.
>> +             */
>> +            u32 msg = I915_READ(SOFT_SCRATCH(15)) &
>> +                    (GUC2HOST_MSG_CRASH_DUMP_POSTED |
>> +                     GUC2HOST_MSG_FLUSH_LOG_BUFFER);
>> +            if (msg) {
>> +                /* Clear the message bits that are handled */
>> +                I915_WRITE(SOFT_SCRATCH(15),
>> +                    I915_READ(SOFT_SCRATCH(15)) & ~msg);
>> +
>> +                /* Handle flush interrupt event in bottom half */
>> +                queue_work(dev_priv->wq, &dev_priv->guc.events_work);
>
> Since the later patch is changing this to use a thread, since you have
> established worker is too slow - especially the shared one - I would
> really recommend you start with the kthread straight away. Not have the
> worker for a while in the same series and then later change it to a thread.
>
Actually it won't be appropriate to say that shared worker thread is too 
slow, but having a dedicated kthread definitely helps.

I kept the kthread patch at the last so that as per the response,
review comments can drop it also.

>> +            }
>> +        }
>> +        spin_unlock(&dev_priv->irq_lock);
>
> Why does the above needs to be done under the irq_lock ?
>
Using the irq_lock for 'guc.interrupts_enabled', especially useful
while disabling the interrupt.

Best regards
Akash

>> +    }
>> +}
>> +
>>   static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
>>                        enum pipe pipe)
>>   {
>> @@ -3754,7 +3840,7 @@ static void gen8_gt_irq_postinstall(struct
>> drm_i915_private *dev_priv)
>>       GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
>>       /*
>>        * RPS interrupts will get enabled/disabled on demand when RPS
>> itself
>> -     * is enabled/disabled.
>> +     * is enabled/disabled. Same wil be the case for GuC interrupts.
>>        */
>>       GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
>>       GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
>> @@ -4539,6 +4625,10 @@ void intel_irq_init(struct drm_i915_private
>> *dev_priv)
>>
>>       INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
>>       INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
>> +    INIT_WORK(&dev_priv->guc.events_work, gen9_guc2host_events_work);
>> +
>> +    if (HAS_GUC_UCODE(dev))
>> +        dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
>>
>>       /* Let's track the enabled rps events */
>>       if (IS_VALLEYVIEW(dev_priv))
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 8bfde75..d0d13a2 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5963,6 +5963,7 @@ enum {
>>   #define  GEN8_DE_PIPE_A_IRQ        (1<<16)
>>   #define  GEN8_DE_PIPE_IRQ(pipe)        (1<<(16+(pipe)))
>>   #define  GEN8_GT_VECS_IRQ        (1<<6)
>> +#define  GEN8_GT_GUC_IRQ        (1<<5)
>>   #define  GEN8_GT_PM_IRQ            (1<<4)
>>   #define  GEN8_GT_VCS2_IRQ        (1<<3)
>>   #define  GEN8_GT_VCS1_IRQ        (1<<2)
>> @@ -5974,6 +5975,16 @@ enum {
>>   #define GEN8_GT_IIR(which) _MMIO(0x44308 + (0x10 * (which)))
>>   #define GEN8_GT_IER(which) _MMIO(0x4430c + (0x10 * (which)))
>>
>> +#define GEN9_GUC_TO_HOST_INT_EVENT    (1<<31)
>> +#define GEN9_GUC_EXEC_ERROR_EVENT    (1<<30)
>> +#define GEN9_GUC_DISPLAY_EVENT        (1<<29)
>> +#define GEN9_GUC_SEMA_SIGNAL_EVENT    (1<<28)
>> +#define GEN9_GUC_IOMMU_MSG_EVENT    (1<<27)
>> +#define GEN9_GUC_DB_RING_EVENT        (1<<26)
>> +#define GEN9_GUC_DMA_DONE_EVENT        (1<<25)
>> +#define GEN9_GUC_FATAL_ERROR_EVENT    (1<<24)
>> +#define GEN9_GUC_NOTIFICATION_EVENT    (1<<23)
>> +
>>   #define GEN8_RCS_IRQ_SHIFT 0
>>   #define GEN8_BCS_IRQ_SHIFT 16
>>   #define GEN8_VCS1_IRQ_SHIFT 0
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index ae6c535..6868f31 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1102,6 +1102,9 @@ void gen8_irq_power_well_post_enable(struct
>> drm_i915_private *dev_priv,
>>                        unsigned int pipe_mask);
>>   void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
>>                        unsigned int pipe_mask);
>> +void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv);
>> +void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv);
>> +void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>>
>>   /* intel_crt.c */
>>   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 d52eca3..2663b41 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -131,6 +131,10 @@ struct intel_guc {
>>       struct intel_guc_fw guc_fw;
>>       struct intel_guc_log log;
>>
>> +    /* GuC2Host interrupt related state */
>> +    struct work_struct events_work;
>> +    bool interrupts_enabled;
>> +
>>       struct drm_i915_gem_object *ads_obj;
>>
>>       struct drm_i915_gem_object *ctx_pool_obj;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 4882f17..d74d7a5 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -450,6 +450,7 @@ int intel_guc_setup(struct drm_device *dev)
>>       }
>>
>>       direct_interrupts_to_host(dev_priv);
>> +    gen9_reset_guc_interrupts(dev_priv);
>>
>>       guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
>>
>> @@ -496,6 +497,9 @@ int intel_guc_setup(struct drm_device *dev)
>>           intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>>
>>       if (i915.enable_guc_submission) {
>> +        if (i915.guc_log_level >= 0)
>> +            gen9_enable_guc_interrupts(dev_priv);
>> +
>>           err = i915_guc_submission_enable(dev_priv);
>>           if (err)
>>               goto fail;
>>
>
> Regards,
>
> Tvrtko
>


More information about the Intel-gfx mailing list