Re: [PATCH 18/18] drm/i915: Notify GuC when RC6 state is changed

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Mar 26, 2015 at 12:41:25PM -0700, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
> 
> Whenever RC6 state (0xA210) is changed, driver needs to notify GuC
> via guc_action.
> 
> Issue: VIZ-4884
> Change-Id: I15c661a915c670691d020471ecaccb00f7afb624
> Signed-off-by: Alex Dai <yu.dai@intel.com>

What exactly is the interaction here? Please elaborate in the commit
message, since as-is this looks extremely racy: If the guc can fall over
if we change rc6 behind it's back it can still fall over right between
when we change rc6 mode and when we tell the guc.

Also note that rc6 setup is delayed in resume/driver load (because it
takes forever) and we expect the gpu to be fully working while we enable
rc6. If the guc falls over on resume because of this there's a problem. We
might need specific igt testcases which race resume against gpu
submissions.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_guc.h           |  1 +
>  drivers/gpu/drm/i915/intel_guc_api.h       |  1 +
>  drivers/gpu/drm/i915/intel_guc_client.c    | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc_scheduler.c |  3 +++
>  drivers/gpu/drm/i915/intel_pm.c            |  4 ++++
>  5 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 3c41483..baa2446 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -181,5 +181,6 @@ void i915_guc_client_free(struct drm_device *dev,
>  int i915_guc_client_submit(struct i915_guc_client *client,
>  			   struct intel_context *ctx,
>  			   struct intel_engine_cs *ring);
> +int host2guc_sample_forcewake(struct intel_guc *guc, bool enable);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_api.h b/drivers/gpu/drm/i915/intel_guc_api.h
> index 3d4f74a..94c914e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_api.h
> +++ b/drivers/gpu/drm/i915/intel_guc_api.h
> @@ -190,6 +190,7 @@ struct guc_context_desc {
>  /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
>  enum host2guc_action {
>  	HOST2GUC_ACTION_DEFAULT = 0x0,
> +	HOST2GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
>  	HOST2GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
>  	HOST2GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
>  	HOST2GUC_ACTION_LIMIT
> diff --git a/drivers/gpu/drm/i915/intel_guc_client.c b/drivers/gpu/drm/i915/intel_guc_client.c
> index 264e0ab..29450d2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_client.c
> +++ b/drivers/gpu/drm/i915/intel_guc_client.c
> @@ -262,6 +262,22 @@ static int host2guc_release_doorbell(struct intel_guc *guc,
>  	return intel_guc_action(guc, data, 2);
>  }
>  
> +int host2guc_sample_forcewake(struct intel_guc *guc, bool enable)
> +{
> +	u32 data[2];
> +
> +	if (guc->guc_fw.uc_fw_load_status != INTEL_UC_FIRMWARE_SUCCESS)
> +		return -ENODEV;
> +
> +	data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
> +	if (enable)
> +		data[1] = 3; /* Bit0: Render, Bit1: Media */
> +	else
> +		data[1] = 0;
> +
> +	return intel_guc_action(guc, data, 2);
> +}
> +
>  static void init_doorbell(struct intel_guc *guc,
>  			  struct i915_guc_client *client)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_guc_scheduler.c b/drivers/gpu/drm/i915/intel_guc_scheduler.c
> index ea1ff28..bd7ea0e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_scheduler.c
> +++ b/drivers/gpu/drm/i915/intel_guc_scheduler.c
> @@ -139,6 +139,9 @@ int guc_scheduler_enable(struct drm_device *dev)
>  
>  	direct_interrupts_to_guc(dev_priv);
>  
> +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> +	host2guc_sample_forcewake(guc, dev_priv->rps.enabled);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fa4ccb3..dfde98d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4113,6 +4113,8 @@ static void gen9_disable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  	I915_WRITE(GEN9_PG_ENABLE, 0);
> +
> +	host2guc_sample_forcewake(&dev_priv->guc, 0);
>  }
>  
>  static void gen6_disable_rps(struct drm_device *dev)
> @@ -4330,6 +4332,8 @@ static void gen9_enable_rc6(struct drm_device *dev)
>  
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
> +	host2guc_sample_forcewake(&dev_priv->guc,
> +			rc6_mask & GEN6_RC_CTL_RC6_ENABLE);
>  }
>  
>  static void gen8_enable_rps(struct drm_device *dev)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx