[v5,14/18] drm/i915: Watchdog timeout: Pass GuC shared data structure during param load

Submitted by Michel Thierry on March 25, 2017, 1:30 a.m.

Details

Message ID 20170325013010.36244-15-michel.thierry@intel.com
State New
Headers show
Series "Gen8+ engine-reset" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Michel Thierry March 25, 2017, 1:30 a.m.
For watchdog / media reset, the firmware must know the address of the shared
data page (the first page of the default context).

This information should be in DWORD 9 of the GUC_CTL structure.

Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index b627206b8f56..5db3def5f74e 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -135,7 +135,7 @@ 
 #define   GUC_ADS_ADDR_SHIFT		11
 #define   GUC_ADS_ADDR_MASK		0xfffff800
 
-#define GUC_CTL_RSRVD			9
+#define GUC_CTL_SHARED_DATA		9
 
 #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 7d92321f8731..afa584864cb5 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -119,6 +119,7 @@  static void guc_params_init(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 	u32 params[GUC_CTL_MAX_DWORDS];
+	struct i915_gem_context *ctx;
 	int i;
 
 	memset(&params, 0, sizeof(params));
@@ -167,6 +168,13 @@  static void guc_params_init(struct drm_i915_private *dev_priv)
 		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
 	}
 
+	/*
+	 * For watchdog / media reset, GuC must know the address of the shared
+	 * data page, which is the first page of the default context.
+	 */
+	ctx = dev_priv->kernel_context;
+	params[GUC_CTL_SHARED_DATA] = i915_ggtt_offset(ctx->engine[RCS].state);
+
 	I915_WRITE(SOFT_SCRATCH(0), 0);
 
 	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)

Comments

On Fri, Mar 24, 2017 at 06:30:06PM -0700, Michel Thierry wrote:
> For watchdog / media reset, the firmware must know the address of the shared
> data page (the first page of the default context).
> 
> This information should be in DWORD 9 of the GUC_CTL structure.
> 
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 2 +-
>  drivers/gpu/drm/i915/intel_guc_loader.c | 8 ++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index b627206b8f56..5db3def5f74e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -135,7 +135,7 @@
>  #define   GUC_ADS_ADDR_SHIFT		11
>  #define   GUC_ADS_ADDR_MASK		0xfffff800
>  
> -#define GUC_CTL_RSRVD			9
> +#define GUC_CTL_SHARED_DATA		9
>  
>  #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>  
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 7d92321f8731..afa584864cb5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -119,6 +119,7 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_guc *guc = &dev_priv->guc;
>  	u32 params[GUC_CTL_MAX_DWORDS];
> +	struct i915_gem_context *ctx;
>  	int i;
>  
>  	memset(&params, 0, sizeof(params));
> @@ -167,6 +168,13 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
>  		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
>  	}
>  
> +	/*
> +	 * For watchdog / media reset, GuC must know the address of the shared
> +	 * data page, which is the first page of the default context.
> +	 */
> +	ctx = dev_priv->kernel_context;
> +	params[GUC_CTL_SHARED_DATA] = i915_ggtt_offset(ctx->engine[RCS].state);

guc_ggtt_offset(), it's the same as i915_ggtt_offset() but with a couple
of extra guc specific checks.
-Chris
On 24/03/17 18:30, Michel Thierry wrote:
> For watchdog / media reset, the firmware must know the address of the shared
> data page (the first page of the default context).
>
> This information should be in DWORD 9 of the GUC_CTL structure.
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 2 +-
>  drivers/gpu/drm/i915/intel_guc_loader.c | 8 ++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index b627206b8f56..5db3def5f74e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -135,7 +135,7 @@
>  #define   GUC_ADS_ADDR_SHIFT		11
>  #define   GUC_ADS_ADDR_MASK		0xfffff800
>
> -#define GUC_CTL_RSRVD			9
> +#define GUC_CTL_SHARED_DATA		9
>
>  #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 7d92321f8731..afa584864cb5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -119,6 +119,7 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_guc *guc = &dev_priv->guc;
>  	u32 params[GUC_CTL_MAX_DWORDS];
> +	struct i915_gem_context *ctx;
>  	int i;
>
>  	memset(&params, 0, sizeof(params));
> @@ -167,6 +168,13 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
>  		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
>  	}
>
> +	/*
> +	 * For watchdog / media reset, GuC must know the address of the shared
> +	 * data page, which is the first page of the default context.
> +	 */
> +	ctx = dev_priv->kernel_context;
> +	params[GUC_CTL_SHARED_DATA] = i915_ggtt_offset(ctx->engine[RCS].state);

Since we use this page in several places (here, in the engine reset h2g 
and if I'm not mistaken also in the preemption h2g), is it worth saving 
its ggtt offset inside the guc struct to make things cleaner?

Regards,
Daniele

> +
>  	I915_WRITE(SOFT_SCRATCH(0), 0);
>
>  	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
>
On 17/04/17 14:28, Daniele Ceraolo Spurio wrote:
>
>
> On 24/03/17 18:30, Michel Thierry wrote:
>> For watchdog / media reset, the firmware must know the address of the
>> shared
>> data page (the first page of the default context).
>>
>> This information should be in DWORD 9 of the GUC_CTL structure.
>>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 2 +-
>>  drivers/gpu/drm/i915/intel_guc_loader.c | 8 ++++++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index b627206b8f56..5db3def5f74e 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -135,7 +135,7 @@
>>  #define   GUC_ADS_ADDR_SHIFT        11
>>  #define   GUC_ADS_ADDR_MASK        0xfffff800
>>
>> -#define GUC_CTL_RSRVD            9
>> +#define GUC_CTL_SHARED_DATA        9
>>
>>  #define GUC_CTL_MAX_DWORDS        (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 7d92321f8731..afa584864cb5 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -119,6 +119,7 @@ static void guc_params_init(struct
>> drm_i915_private *dev_priv)
>>  {
>>      struct intel_guc *guc = &dev_priv->guc;
>>      u32 params[GUC_CTL_MAX_DWORDS];
>> +    struct i915_gem_context *ctx;
>>      int i;
>>
>>      memset(&params, 0, sizeof(params));
>> @@ -167,6 +168,13 @@ static void guc_params_init(struct
>> drm_i915_private *dev_priv)
>>          params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
>>      }
>>
>> +    /*
>> +     * For watchdog / media reset, GuC must know the address of the
>> shared
>> +     * data page, which is the first page of the default context.
>> +     */
>> +    ctx = dev_priv->kernel_context;
>> +    params[GUC_CTL_SHARED_DATA] =
>> i915_ggtt_offset(ctx->engine[RCS].state);
>
> Since we use this page in several places (here, in the engine reset h2g
> and if I'm not mistaken also in the preemption h2g), is it worth saving
> its ggtt offset inside the guc struct to make things cleaner?
>

In suspend/resume too.

>
>> +
>>      I915_WRITE(SOFT_SCRATCH(0), 0);
>>
>>      for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
>>