| Message ID | 1468158084-22028-6-git-send-email-akash.goel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/07/16 14:41, akash.goel@intel.com wrote: > From: Sagar Arun Kamble <sagar.a.kamble@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@intel.com> > Signed-off-by: Akash Goel <akash.goel@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; > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 0fb00ab..0bac172 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -1044,6 +1044,8 @@ int intel_guc_suspend(struct drm_device *dev) > if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) > return 0; > > + gen9_disable_guc_interrupts(dev_priv); > + > ctx = dev_priv->kernel_context; > > data[0] = HOST2GUC_ACTION_ENTER_S_STATE; > @@ -1070,6 +1072,9 @@ int intel_guc_resume(struct drm_device *dev) > if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) > return 0; > > + if (i915.guc_log_level >= 0) > + gen9_enable_guc_interrupts(dev_priv); > + > ctx = dev_priv->kernel_context; > > data[0] = HOST2GUC_ACTION_EXIT_S_STATE; > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 24bbaf7..fd73c94 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 gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); > > /* For display hotplug interrupt */ > static inline void > @@ -411,6 +412,39 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv) > gen6_reset_rps_interrupts(dev_priv); > } > > +void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv) > +{ > + spin_lock_irq(&dev_priv->irq_lock); > + gen6_reset_pm_iir(dev_priv, dev_priv->pm_guc_events); > + spin_unlock_irq(&dev_priv->irq_lock); > +} > + > +void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv) > +{ > + spin_lock_irq(&dev_priv->irq_lock); > + if (!dev_priv->guc.interrupts_enabled) { > + 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); > + } > + spin_unlock_irq(&dev_priv->irq_lock); > +} > + > +void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv) > +{ > + spin_lock_irq(&dev_priv->irq_lock); > + dev_priv->guc.interrupts_enabled = false; > + > + gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events); > + > + spin_unlock_irq(&dev_priv->irq_lock); > + synchronize_irq(dev_priv->drm.irq); > + > + cancel_work_sync(&dev_priv->guc.events_work); > + gen9_reset_guc_interrupts(dev_priv); > +} > + > /** > * 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. > + } > + } > + spin_unlock(&dev_priv->irq_lock); Why does the above needs to be done under the irq_lock ? > + } > +} > + > 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
On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: > > On 10/07/16 14:41, akash.goel@intel.com wrote: >> From: Sagar Arun Kamble <sagar.a.kamble@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@intel.com> >> Signed-off-by: Akash Goel <akash.goel@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 >
On 11/07/16 14:15, Goel, Akash wrote: > On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: >> >> On 10/07/16 14:41, akash.goel@intel.com wrote: >>> From: Sagar Arun Kamble <sagar.a.kamble@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@intel.com> >>> Signed-off-by: Akash Goel <akash.goel@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. I think it should only be one implementation in the patch series. If we agreed on a kthread make it so from the start. And describe in the commit message why it was selected etc. >>> + } >>> + } >>> + 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. Why? I don't see how it gains you anything and so it seems preferable not to hold it over mmio accesses. Regards, Tvrtko
On 7/11/2016 6:53 PM, Tvrtko Ursulin wrote: > > On 11/07/16 14:15, Goel, Akash wrote: >> On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: >>> >>>> +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. > > I think it should only be one implementation in the patch series. If we > agreed on a kthread make it so from the start. > Agree but actually right now, added the kthread patch more as a RFC and presumed this won't be the final version of the series. Will do the needful, as per the review comments, in the next version. > And describe in the commit message why it was selected etc. > >>>> + } >>>> + } >>>> + 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. > > Why? I don't see how it gains you anything and so it seems preferable > not to hold it over mmio accesses. > Yes not needed for the mmio access part. Just needed for the inspection of 'guc.interrupts_enabled' value. Will reorder the code. Best regards Akash > Regards, > > Tvrtko > >
On 11/07/16 14:38, Goel, Akash wrote: > On 7/11/2016 6:53 PM, Tvrtko Ursulin wrote: >> >> On 11/07/16 14:15, Goel, Akash wrote: >>> On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: >>>> > >>>>> +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. >> >> I think it should only be one implementation in the patch series. If we >> agreed on a kthread make it so from the start. >> > Agree but actually right now, added the kthread patch more as a RFC and > presumed this won't be the final version of the series. > Will do the needful, as per the review comments, in the next version. Ack. >> And describe in the commit message why it was selected etc. >> >>>>> + } >>>>> + } >>>>> + 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. >> >> Why? I don't see how it gains you anything and so it seems preferable >> not to hold it over mmio accesses. >> > Yes not needed for the mmio access part. > Just needed for the inspection of 'guc.interrupts_enabled' value. > Will reorder the code. You don't need it just for reading that value, you can just drop it. Regards, Tvrtko
On 7/11/2016 7:13 PM, Tvrtko Ursulin wrote: > > On 11/07/16 14:38, Goel, Akash wrote: >> On 7/11/2016 6:53 PM, Tvrtko Ursulin wrote: >>> >>> On 11/07/16 14:15, Goel, Akash wrote: >>>> On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: >>>>> >> >>>>>> +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. >>> >>> I think it should only be one implementation in the patch series. If we >>> agreed on a kthread make it so from the start. >>> >> Agree but actually right now, added the kthread patch more as a RFC and >> presumed this won't be the final version of the series. >> Will do the needful, as per the review comments, in the next version. > > Ack. > >>> And describe in the commit message why it was selected etc. >>> >>>>>> + } >>>>>> + } >>>>>> + 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. >>> >>> Why? I don't see how it gains you anything and so it seems preferable >>> not to hold it over mmio accesses. >>> >> Yes not needed for the mmio access part. >> Just needed for the inspection of 'guc.interrupts_enabled' value. >> Will reorder the code. > > You don't need it just for reading that value, you can just drop it. > Its not strictly needed as its a mere read. But as per my limited understanding, without the spinlock (which provides an implicit barrier also) ISR might miss the reset of 'interrupts_enabled' flag, from a thread on other CPU, and queue the new work. The update will be visible eventually though. And same applies to the case when 'interrupts_enabled' flag is set from other CPU. Good practice to use locks for accessing shared variables ?. Best regards Akash > Regards, > > Tvrtko > >
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; diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 0fb00ab..0bac172 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -1044,6 +1044,8 @@ int intel_guc_suspend(struct drm_device *dev) if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; + gen9_disable_guc_interrupts(dev_priv); + ctx = dev_priv->kernel_context; data[0] = HOST2GUC_ACTION_ENTER_S_STATE; @@ -1070,6 +1072,9 @@ int intel_guc_resume(struct drm_device *dev) if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; + if (i915.guc_log_level >= 0) + gen9_enable_guc_interrupts(dev_priv); + ctx = dev_priv->kernel_context; data[0] = HOST2GUC_ACTION_EXIT_S_STATE; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 24bbaf7..fd73c94 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 gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); /* For display hotplug interrupt */ static inline void @@ -411,6 +412,39 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv) gen6_reset_rps_interrupts(dev_priv); } +void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv) +{ + spin_lock_irq(&dev_priv->irq_lock); + gen6_reset_pm_iir(dev_priv, dev_priv->pm_guc_events); + spin_unlock_irq(&dev_priv->irq_lock); +} + +void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv) +{ + spin_lock_irq(&dev_priv->irq_lock); + if (!dev_priv->guc.interrupts_enabled) { + 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); + } + spin_unlock_irq(&dev_priv->irq_lock); +} + +void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv) +{ + spin_lock_irq(&dev_priv->irq_lock); + dev_priv->guc.interrupts_enabled = false; + + gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events); + + spin_unlock_irq(&dev_priv->irq_lock); + synchronize_irq(dev_priv->drm.irq); + + cancel_work_sync(&dev_priv->guc.events_work); + gen9_reset_guc_interrupts(dev_priv); +} + /** * 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); + } + } + spin_unlock(&dev_priv->irq_lock); + } +} + 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;