Re: [Intel-gfx] [PATCH] drm/i915/uc: Start preparing GuC/HuC for reset

2018-02-22 Thread Daniele Ceraolo Spurio 



On 22/02/18 11:28, Daniele Ceraolo Spurio wrote:



On 22/02/18 10:45, Michal Wajdeczko wrote:

Right after GPU reset there will be a small window of time during which
some of GuC/HuC fields will still show state before reset. Let's start
to fix that by sanitizing firmware status as we will use it shortly.

Suggested-by: Daniele Ceraolo Spurio 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c    |  1 +
  drivers/gpu/drm/i915/intel_guc.h   |  5 +
  drivers/gpu/drm/i915/intel_huc.h   |  5 +
  drivers/gpu/drm/i915/intel_uc.c    | 11 +++
  drivers/gpu/drm/i915/intel_uc.h    |  1 +
  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
  6 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c 
b/drivers/gpu/drm/i915/i915_gem.c

index 14c855b..9f91625 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct 
drm_i915_private *dev_priv)

  }
  i915_gem_revoke_fences(dev_priv);
+    intel_uc_reset_prepare(dev_priv);
  return err;
  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h 
b/drivers/gpu/drm/i915/intel_guc.h

index 52856a9..ffa528f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
*vma)
  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 
size);

  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
+static inline void intel_guc_reset_prepare(struct intel_guc *guc)
+{


We could also move guc_disable_communication and reset_guc_interrupts 
here and remove them from intel_uc_init_hw (I'm assuming there is no 
need to do either of the 2 the first time GuC is loaded).


Actually after looking at the flow I think we do still need those in 
intel_uc_init_hw for when we come out of suspension. we call 
intel_gpu_reset and not the full i915_reset in gem_sanitize, so 
reset_prepare is not called. for the same reason uc_fw->load_status will 
probably be incorrect after resume as well.


Daniele



Patch looks good to me apart from the above note.

Daniele


+    intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h 
b/drivers/gpu/drm/i915/intel_huc.h

index 40039db..c1ee5b5 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,9 @@ struct intel_huc {
  int intel_huc_init_hw(struct intel_huc *huc);
  int intel_huc_auth(struct intel_huc *huc);
+static inline void intel_huc_reset_prepare(struct intel_huc *huc)
+{
+    intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c 
b/drivers/gpu/drm/i915/intel_uc.c

index 9f1bac6..64ae0a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private 
*dev_priv)

  if (USES_GUC_SUBMISSION(dev_priv))
  gen9_disable_guc_interrupts(dev_priv);
  }
+
+void intel_uc_reset_prepare(struct drm_i915_private *i915)
+{
+    if (!USES_GUC(i915))
+    return;
+
+    GEM_BUG_ON(!HAS_GUC(i915));
+
+    intel_huc_reset_prepare(>huc);
+    intel_guc_reset_prepare(>guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h 
b/drivers/gpu/drm/i915/intel_uc.h

index f2984e0..5c10253 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,7 @@
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
  void intel_uc_fini(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
  static inline bool intel_uc_is_using_guc(void)
  {
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h

index d5fd460..68bc304 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
intel_uc_fw *uc_fw)

  return uc_fw->path != NULL;
  }
+static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw *uc_fw)
+{
+    if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+    uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Start preparing GuC/HuC for reset

2018-02-22 Thread Michel Thierry 

On 22/02/18 13:21, Michal Wajdeczko wrote:
On Thu, 22 Feb 2018 21:52:39 +0100, Michel Thierry 
 wrote:



On 22/02/18 10:45, Michal Wajdeczko wrote:

Right after GPU reset there will be a small window of time during which
some of GuC/HuC fields will still show state before reset. Let's start
to fix that by sanitizing firmware status as we will use it shortly.
 Suggested-by: Daniele Ceraolo Spurio 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c    |  1 +
  drivers/gpu/drm/i915/intel_guc.h   |  5 +
  drivers/gpu/drm/i915/intel_huc.h   |  5 +
  drivers/gpu/drm/i915/intel_uc.c    | 11 +++
  drivers/gpu/drm/i915/intel_uc.h    |  1 +
  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
  6 files changed, 29 insertions(+)
 diff --git a/drivers/gpu/drm/i915/i915_gem.c 
b/drivers/gpu/drm/i915/i915_gem.c

index 14c855b..9f91625 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct 
drm_i915_private *dev_priv)

  }
    i915_gem_revoke_fences(dev_priv);
+    intel_uc_reset_prepare(dev_priv);
    return err;
  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h 
b/drivers/gpu/drm/i915/intel_guc.h

index 52856a9..ffa528f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
*vma)
  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 
size);

  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
  +static inline void intel_guc_reset_prepare(struct intel_guc *guc)


Hi,

Would intel_reset_guc_prepare be better?


I was following existing naming from "i915_gem_reset_prepare"



Yes, guc is probably the only special one that can be reset or reset 
something itself...



But if we want to diverge and still follow obj-verb-subject pattern
then maybe something like this:

intel_uc_prepare_to_reset(i915)
{
   intel_huc_prepare_to_reset(huc)
   intel_guc_prepare_to_reset(guc)
}



This looks clearer to me.

Thanks


/Michal



Here you're preparing the guc for reset, not asking guc to prepare a 
reset.


For example, we have:
intel_guc_reset_engine  -> h2g requesting an engine reset
intel_reset_guc -> i915 will reset guc
gen9_reset_guc_interrupts -> i915 is resetting the iir reg

-Michel


+{
+    intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h 
b/drivers/gpu/drm/i915/intel_huc.h

index 40039db..c1ee5b5 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,9 @@ struct intel_huc {
  int intel_huc_init_hw(struct intel_huc *huc);
  int intel_huc_auth(struct intel_huc *huc);
  +static inline void intel_huc_reset_prepare(struct intel_huc *huc)


same here (for consistency).


+{
+    intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c 
b/drivers/gpu/drm/i915/intel_uc.c

index 9f1bac6..64ae0a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private 
*dev_priv)

  if (USES_GUC_SUBMISSION(dev_priv))
  gen9_disable_guc_interrupts(dev_priv);
  }
+
+void intel_uc_reset_prepare(struct drm_i915_private *i915)
+{
+    if (!USES_GUC(i915))
+    return;
+
+    GEM_BUG_ON(!HAS_GUC(i915));
+
+    intel_huc_reset_prepare(>huc);
+    intel_guc_reset_prepare(>guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h 
b/drivers/gpu/drm/i915/intel_uc.h

index f2984e0..5c10253 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,7 @@
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
  void intel_uc_fini(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
    static inline bool intel_uc_is_using_guc(void)
  {
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h

index d5fd460..68bc304 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool 
intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)

  return uc_fw->path != NULL;
  }
  +static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw 
*uc_fw)

+{
+    if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+    uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,

___
Intel-gfx mailing list

Re: [Intel-gfx] [PATCH] drm/i915/uc: Start preparing GuC/HuC for reset

2018-02-22 Thread Michal Wajdeczko 
On Thu, 22 Feb 2018 21:52:39 +0100, Michel Thierry  
 wrote:



On 22/02/18 10:45, Michal Wajdeczko wrote:

Right after GPU reset there will be a small window of time during which
some of GuC/HuC fields will still show state before reset. Let's start
to fix that by sanitizing firmware status as we will use it shortly.
 Suggested-by: Daniele Ceraolo Spurio 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c|  1 +
  drivers/gpu/drm/i915/intel_guc.h   |  5 +
  drivers/gpu/drm/i915/intel_huc.h   |  5 +
  drivers/gpu/drm/i915/intel_uc.c| 11 +++
  drivers/gpu/drm/i915/intel_uc.h|  1 +
  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
  6 files changed, 29 insertions(+)
 diff --git a/drivers/gpu/drm/i915/i915_gem.c  
b/drivers/gpu/drm/i915/i915_gem.c

index 14c855b..9f91625 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct  
drm_i915_private *dev_priv)

}
i915_gem_revoke_fences(dev_priv);
+   intel_uc_reset_prepare(dev_priv);
return err;
  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h  
b/drivers/gpu/drm/i915/intel_guc.h

index 52856a9..ffa528f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma  
*vma)
  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
size);

  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
  +static inline void intel_guc_reset_prepare(struct intel_guc *guc)


Hi,

Would intel_reset_guc_prepare be better?


I was following existing naming from "i915_gem_reset_prepare"

But if we want to diverge and still follow obj-verb-subject pattern
then maybe something like this:

intel_uc_prepare_to_reset(i915)
{
  intel_huc_prepare_to_reset(huc)
  intel_guc_prepare_to_reset(guc)
}

/Michal



Here you're preparing the guc for reset, not asking guc to prepare a  
reset.


For example, we have:
intel_guc_reset_engine  -> h2g requesting an engine reset
intel_reset_guc -> i915 will reset guc
gen9_reset_guc_interrupts -> i915 is resetting the iir reg

-Michel


+{
+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h  
b/drivers/gpu/drm/i915/intel_huc.h

index 40039db..c1ee5b5 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,9 @@ struct intel_huc {
  int intel_huc_init_hw(struct intel_huc *huc);
  int intel_huc_auth(struct intel_huc *huc);
  +static inline void intel_huc_reset_prepare(struct intel_huc *huc)


same here (for consistency).


+{
+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c  
b/drivers/gpu/drm/i915/intel_uc.c

index 9f1bac6..64ae0a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private  
*dev_priv)

if (USES_GUC_SUBMISSION(dev_priv))
gen9_disable_guc_interrupts(dev_priv);
  }
+
+void intel_uc_reset_prepare(struct drm_i915_private *i915)
+{
+   if (!USES_GUC(i915))
+   return;
+
+   GEM_BUG_ON(!HAS_GUC(i915));
+
+   intel_huc_reset_prepare(>huc);
+   intel_guc_reset_prepare(>guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h  
b/drivers/gpu/drm/i915/intel_uc.h

index f2984e0..5c10253 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,7 @@
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
  void intel_uc_fini(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
static inline bool intel_uc_is_using_guc(void)
  {
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h  
b/drivers/gpu/drm/i915/intel_uc_fw.h

index d5fd460..68bc304 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct  
intel_uc_fw *uc_fw)

return uc_fw->path != NULL;
  }
  +static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw  
*uc_fw)

+{
+   if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+   uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
   struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Start preparing GuC/HuC for reset

2018-02-22 Thread Michel Thierry 

On 22/02/18 10:45, Michal Wajdeczko wrote:

Right after GPU reset there will be a small window of time during which
some of GuC/HuC fields will still show state before reset. Let's start
to fix that by sanitizing firmware status as we will use it shortly.

Suggested-by: Daniele Ceraolo Spurio 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c|  1 +
  drivers/gpu/drm/i915/intel_guc.h   |  5 +
  drivers/gpu/drm/i915/intel_huc.h   |  5 +
  drivers/gpu/drm/i915/intel_uc.c| 11 +++
  drivers/gpu/drm/i915/intel_uc.h|  1 +
  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
  6 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 14c855b..9f91625 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct drm_i915_private 
*dev_priv)
}
  
  	i915_gem_revoke_fences(dev_priv);

+   intel_uc_reset_prepare(dev_priv);
  
  	return err;

  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 52856a9..ffa528f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
  
+static inline void intel_guc_reset_prepare(struct intel_guc *guc)


Hi,

Would intel_reset_guc_prepare be better?

Here you're preparing the guc for reset, not asking guc to prepare a reset.

For example, we have:
intel_guc_reset_engine  -> h2g requesting an engine reset
intel_reset_guc -> i915 will reset guc
gen9_reset_guc_interrupts -> i915 is resetting the iir reg

-Michel


+{
+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 40039db..c1ee5b5 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,9 @@ struct intel_huc {
  int intel_huc_init_hw(struct intel_huc *huc);
  int intel_huc_auth(struct intel_huc *huc);
  
+static inline void intel_huc_reset_prepare(struct intel_huc *huc)


same here (for consistency).


+{
+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6..64ae0a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
if (USES_GUC_SUBMISSION(dev_priv))
gen9_disable_guc_interrupts(dev_priv);
  }
+
+void intel_uc_reset_prepare(struct drm_i915_private *i915)
+{
+   if (!USES_GUC(i915))
+   return;
+
+   GEM_BUG_ON(!HAS_GUC(i915));
+
+   intel_huc_reset_prepare(>huc);
+   intel_guc_reset_prepare(>guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f2984e0..5c10253 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,7 @@
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
  void intel_uc_fini(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
  
  static inline bool intel_uc_is_using_guc(void)

  {
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h
index d5fd460..68bc304 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
intel_uc_fw *uc_fw)
return uc_fw->path != NULL;
  }
  
+static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw *uc_fw)

+{
+   if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+   uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
   struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Start preparing GuC/HuC for reset

2018-02-22 Thread Daniele Ceraolo Spurio 



On 22/02/18 10:45, Michal Wajdeczko wrote:

Right after GPU reset there will be a small window of time during which
some of GuC/HuC fields will still show state before reset. Let's start
to fix that by sanitizing firmware status as we will use it shortly.

Suggested-by: Daniele Ceraolo Spurio 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c|  1 +
  drivers/gpu/drm/i915/intel_guc.h   |  5 +
  drivers/gpu/drm/i915/intel_huc.h   |  5 +
  drivers/gpu/drm/i915/intel_uc.c| 11 +++
  drivers/gpu/drm/i915/intel_uc.h|  1 +
  drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
  6 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 14c855b..9f91625 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2981,6 +2981,7 @@ int i915_gem_reset_prepare(struct drm_i915_private 
*dev_priv)
}
  
  	i915_gem_revoke_fences(dev_priv);

+   intel_uc_reset_prepare(dev_priv);
  
  	return err;

  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 52856a9..ffa528f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
  u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
  
+static inline void intel_guc_reset_prepare(struct intel_guc *guc)

+{


We could also move guc_disable_communication and reset_guc_interrupts 
here and remove them from intel_uc_init_hw (I'm assuming there is no 
need to do either of the 2 the first time GuC is loaded).


Patch looks good to me apart from the above note.

Daniele


+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 40039db..c1ee5b5 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,9 @@ struct intel_huc {
  int intel_huc_init_hw(struct intel_huc *huc);
  int intel_huc_auth(struct intel_huc *huc);
  
+static inline void intel_huc_reset_prepare(struct intel_huc *huc)

+{
+   intel_uc_fw_reset_prepare(>fw);
+}
+
  #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6..64ae0a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,14 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
if (USES_GUC_SUBMISSION(dev_priv))
gen9_disable_guc_interrupts(dev_priv);
  }
+
+void intel_uc_reset_prepare(struct drm_i915_private *i915)
+{
+   if (!USES_GUC(i915))
+   return;
+
+   GEM_BUG_ON(!HAS_GUC(i915));
+
+   intel_huc_reset_prepare(>huc);
+   intel_guc_reset_prepare(>guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f2984e0..5c10253 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,7 @@
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
  void intel_uc_fini(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
  
  static inline bool intel_uc_is_using_guc(void)

  {
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h
index d5fd460..68bc304 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
intel_uc_fw *uc_fw)
return uc_fw->path != NULL;
  }
  
+static inline void intel_uc_fw_reset_prepare(struct intel_uc_fw *uc_fw)

+{
+   if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+   uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
   struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx