Re: [Intel-gfx] [PATCH 02/11] drm/i915/guc: Move GuC interrupts related functions from i915_irq.c to intel_guc.c

2017-10-18 Thread Sagar Arun Kamble



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

2017-10-18 Thread Tvrtko Ursulin


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

2017-10-17 Thread Sagar Arun Kamble
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