[Intel-gfx] [PATCH v2 03/15] drm/i915/guc: Move GuC notification handling to separate function
Sagar Arun Kamble
sagar.a.kamble at intel.com
Fri Mar 9 07:25:09 UTC 2018
On 3/8/2018 9:16 PM, Michał Winiarski wrote:
> From: Michal Wajdeczko <michal.wajdeczko at 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 at intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at 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 at 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);
--
Thanks,
Sagar
More information about the Intel-gfx
mailing list