Re: [Intel-gfx] [PATCH 02/11] drm/i915/guc: Move GuC interrupts related functions from i915_irq.c to intel_guc.c
On 10/18/2017 6:16 PM, Tvrtko Ursulin wrote: On 18/10/2017 07:46, Sagar Arun Kamble wrote: GuC interrupts handling is core GuC functionality. Better to keep it with other core functions in intel_guc.c. Since they are used from uC functions, GuC logging, i915 irq handling keeping them grouped in intel_guc.c instead of intel_uc.c. Suggested-by: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble Cc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Tvrtko Ursulin Cc: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_irq.c | 70 +-- drivers/gpu/drm/i915/intel_drv.h | 3 -- drivers/gpu/drm/i915/intel_guc.c | 71 +++- drivers/gpu/drm/i915/intel_guc.h | 4 ++ drivers/gpu/drm/i915/intel_guc_log.c | 6 +-- drivers/gpu/drm/i915/intel_uc.c | 8 ++-- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index caa6283..84a4bf3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -203,7 +203,6 @@ static void gen2_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 @@ -450,38 +449,6 @@ 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); - - gen9_reset_guc_interrupts(dev_priv); -} - /** * bdw_update_port_irq - update DE port interrupt * @dev_priv: driver private @@ -1474,7 +1441,7 @@ static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, 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]); + intel_guc_irq_handler(dev_priv, gt_iir[2]); Slight downside that it cannot be inlined any longer, should the compiler decide to do so. But it is probably not a relevant consideration for the frequency of these. Ok. If needed we can force by making these static inline from header. Thanks for review. } static bool bxt_port_hotplug_long_detect(enum port port, u32 val) @@ -1751,41 +1718,6 @@ 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) { - /* 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, flush; - - msg = I915_READ(SOFT_SCRATCH(15)); - flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED | - INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER); - if (flush) { - /* Clear the message bits that are handled */ - I915_WRITE(SOFT_SCRATCH(15), msg & ~flush); - - /* Handle flush interrupt in bottom half */ - queue_work(dev_priv->guc.log.runtime.flush_wq, - &dev_priv->guc.log.runtime.flush_work); - - dev_priv->guc.log.flush_interrupt_count++; - } else { - /* Not clearing of unhandled event bits won't result in - * re-triggering of the interrupt. - */ - } - } -} - static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_pr
Re: [Intel-gfx] [PATCH 02/11] drm/i915/guc: Move GuC interrupts related functions from i915_irq.c to intel_guc.c
On 18/10/2017 07:46, Sagar Arun Kamble wrote: GuC interrupts handling is core GuC functionality. Better to keep it with other core functions in intel_guc.c. Since they are used from uC functions, GuC logging, i915 irq handling keeping them grouped in intel_guc.c instead of intel_uc.c. Suggested-by: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble Cc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Tvrtko Ursulin Cc: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_irq.c | 70 +-- drivers/gpu/drm/i915/intel_drv.h | 3 -- drivers/gpu/drm/i915/intel_guc.c | 71 +++- drivers/gpu/drm/i915/intel_guc.h | 4 ++ drivers/gpu/drm/i915/intel_guc_log.c | 6 +-- drivers/gpu/drm/i915/intel_uc.c | 8 ++-- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index caa6283..84a4bf3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -203,7 +203,6 @@ static void gen2_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 @@ -450,38 +449,6 @@ 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); - - gen9_reset_guc_interrupts(dev_priv); -} - /** * bdw_update_port_irq - update DE port interrupt * @dev_priv: driver private @@ -1474,7 +1441,7 @@ static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, 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]); + intel_guc_irq_handler(dev_priv, gt_iir[2]); Slight downside that it cannot be inlined any longer, should the compiler decide to do so. But it is probably not a relevant consideration for the frequency of these. } static bool bxt_port_hotplug_long_detect(enum port port, u32 val) @@ -1751,41 +1718,6 @@ 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) { - /* 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, flush; - - msg = I915_READ(SOFT_SCRATCH(15)); - flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED | - INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER); - if (flush) { - /* Clear the message bits that are handled */ - I915_WRITE(SOFT_SCRATCH(15), msg & ~flush); - - /* Handle flush interrupt in bottom half */ - queue_work(dev_priv->guc.log.runtime.flush_wq, - &dev_priv->guc.log.runtime.flush_work); - - dev_priv->guc.log.flush_interrupt_count++; - } else { - /* Not clearing of unhandled event bits won
[Intel-gfx] [PATCH 02/11] drm/i915/guc: Move GuC interrupts related functions from i915_irq.c to intel_guc.c
GuC interrupts handling is core GuC functionality. Better to keep it with other core functions in intel_guc.c. Since they are used from uC functions, GuC logging, i915 irq handling keeping them grouped in intel_guc.c instead of intel_uc.c. Suggested-by: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble Cc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Tvrtko Ursulin Cc: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_irq.c | 70 +-- drivers/gpu/drm/i915/intel_drv.h | 3 -- drivers/gpu/drm/i915/intel_guc.c | 71 +++- drivers/gpu/drm/i915/intel_guc.h | 4 ++ drivers/gpu/drm/i915/intel_guc_log.c | 6 +-- drivers/gpu/drm/i915/intel_uc.c | 8 ++-- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index caa6283..84a4bf3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -203,7 +203,6 @@ static void gen2_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 @@ -450,38 +449,6 @@ 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); - - gen9_reset_guc_interrupts(dev_priv); -} - /** * bdw_update_port_irq - update DE port interrupt * @dev_priv: driver private @@ -1474,7 +1441,7 @@ static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, 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]); + intel_guc_irq_handler(dev_priv, gt_iir[2]); } static bool bxt_port_hotplug_long_detect(enum port port, u32 val) @@ -1751,41 +1718,6 @@ 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) { - /* 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, flush; - - msg = I915_READ(SOFT_SCRATCH(15)); - flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED | - INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER); - if (flush) { - /* Clear the message bits that are handled */ - I915_WRITE(SOFT_SCRATCH(15), msg & ~flush); - - /* Handle flush interrupt in bottom half */ - queue_work(dev_priv->guc.log.runtime.flush_wq, - &dev_priv->guc.log.runtime.flush_work); - - dev_priv->guc.log.flush_interrupt_count++; - } else { - /* Not clearing of unhandled event bits won't result in -* re-triggering of the interrupt. -*/ - } - } -} - static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv) { enum pipe pipe; diff --git a/dr