| Message ID | 20180308154707.21716-3-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3/8/2018 9:16 PM, Michał Winiarski wrote: > From: Michal Wajdeczko <michal.wajdeczko@intel.com> > > To allow future code reuse. While here, fix comment style. > > v2: Notifications are a separate thing - rename the handler (Sagar) > > Suggested-by: Oscar Mateo <oscar.mateo@intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> I am suggesting update to the comments below since it is not clear the reason for clearing. Please check and incorporate in this patch itself if you feel it is right. Otherwise change looks good to me. Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 33 ++------------------------------- > drivers/gpu/drm/i915/intel_guc.c | 37 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_guc.h | 1 + > 3 files changed, 40 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 633c18785c1e..6b0cd6bc83f8 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1766,37 +1766,8 @@ 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. > - */ > - } > - } > + if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) > + intel_guc_to_host_event_handler(&dev_priv->guc); > } > > static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv) > diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c > index ff08ea0ebf49..25f92291fd40 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -364,6 +364,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len) > return ret; > } > > +void intel_guc_to_host_event_handler(struct intel_guc *guc) > +{ > + struct drm_i915_private *dev_priv = guc_to_i915(guc); > + u32 msg, flush; > + > + /* > + * 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. This paragraph is updated as: /* * Note: Logging protocol is, GuC will set the bit in message identity register and raise the GuC to Host * interrupt and then Host will read the bit, clear it, copy the logs and send ack. * If bits are cleared after sending ack, i915 might lose next log flush interrupt if Host clear operation * happens post GuC's update to message identity register corresponding to next interrupt. * Hence, sample the log buffer flush related bits and clear them out now * itself from the message identity register before sending ack to GuC about * flush completion through guc_log_flush_complete. */ and may be we can additionally include below paragraph to explain the scenario. > + * 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. > + */ > + > + 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(guc->log.runtime.flush_wq, > + &guc->log.runtime.flush_work); > + > + guc->log.flush_interrupt_count++; > + } else { > + /* > + * Not clearing of unhandled event bits won't result in > + * re-triggering of the interrupt. > + */ > + } > +} > + > int intel_guc_sample_forcewake(struct intel_guc *guc) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h > index b9424ac644ac..6d5aebe55039 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -125,6 +125,7 @@ int intel_guc_init(struct intel_guc *guc); > void intel_guc_fini(struct intel_guc *guc); > int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len); > int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len); > +void intel_guc_to_host_event_handler(struct intel_guc *guc); > int intel_guc_sample_forcewake(struct intel_guc *guc); > int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset); > int intel_guc_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 633c18785c1e..6b0cd6bc83f8 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1766,37 +1766,8 @@ 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. - */ - } - } + if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) + intel_guc_to_host_event_handler(&dev_priv->guc); } static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index ff08ea0ebf49..25f92291fd40 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -364,6 +364,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len) return ret; } +void intel_guc_to_host_event_handler(struct intel_guc *guc) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + u32 msg, flush; + + /* + * 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. + */ + + 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(guc->log.runtime.flush_wq, + &guc->log.runtime.flush_work); + + guc->log.flush_interrupt_count++; + } else { + /* + * Not clearing of unhandled event bits won't result in + * re-triggering of the interrupt. + */ + } +} + int intel_guc_sample_forcewake(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index b9424ac644ac..6d5aebe55039 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -125,6 +125,7 @@ int intel_guc_init(struct intel_guc *guc); void intel_guc_fini(struct intel_guc *guc); int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len); +void intel_guc_to_host_event_handler(struct intel_guc *guc); int intel_guc_sample_forcewake(struct intel_guc *guc); int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset); int intel_guc_suspend(struct intel_guc *guc);