Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
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.g...@intel.com wrote: From: Sagar Arun Kamble 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 Signed-off-by: Akash Goel --- 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
Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
On 7/11/2016 4:00 PM, Tvrtko Ursulin wrote: On 10/07/16 14:41, akash.g...@intel.com wrote: From: Sagar Arun Kamble 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 Signed-off-by: Akash Goel --- 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
Re: [Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
On 10/07/16 14:41, akash.g...@intel.com wrote: From: Sagar Arun Kamble 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 Signed-off-by: Akash Goel --- 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 w
[Intel-gfx] [PATCH 05/17] drm/i915: Support for GuC interrupts
From: Sagar Arun Kamble 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 Signed-off-by: Akash Goel --- 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_pri