diff mbox

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

Message ID 20170325013010.36244-15-michel.thierry@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michel Thierry March 25, 2017, 1:30 a.m. UTC 
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(-)

Comments

Chris Wilson March 25, 2017, 9:15 a.m. UTC | #1 
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
Daniele Ceraolo Spurio April 17, 2017, 9:28 p.m. UTC | #2 
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++)
>
Michel Thierry April 17, 2017, 10:31 p.m. UTC | #3 
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++)
>>
diff mbox

Patch

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++)