[1/3] drm/i915/guc: Don't open log relay if GuC is not running

Submitted by Michal Wajdeczko on Aug. 18, 2019, 9:52 a.m.

Details

Message ID 20190818095204.31568-2-michal.wajdeczko@intel.com
State Accepted
Commit 1ce5ba970cb713b8336ef4074c7a42a21c4e82e1
Headers show
Series "Don't fail on GuC failure" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Michal Wajdeczko Aug. 18, 2019, 9:52 a.m.
As we plan to continue driver load after GuC initialization
failure, we can't assume that GuC log data will be available
just because GuC was initially enabled. We must check that
GuC is still running instead.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  4 ++++
 drivers/gpu/drm/i915/i915_debugfs.c        | 17 +++++++++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index 59702ebc68f6..36332064de9c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -372,6 +372,7 @@  static int guc_log_relay_create(struct intel_guc_log *log)
 	int ret;
 
 	lockdep_assert_held(&log->relay.lock);
+	GEM_BUG_ON(!log->vma);
 
 	 /* Keep the size of sub buffers same as shared log buffer */
 	subbuf_size = log->vma->size;
@@ -554,6 +555,9 @@  int intel_guc_log_relay_open(struct intel_guc_log *log)
 {
 	int ret;
 
+	if (!log->vma)
+		return -ENODEV;
+
 	mutex_lock(&log->relay.lock);
 
 	if (intel_guc_log_relay_enabled(log)) {
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e7ce739fe545..b39226d7f8d2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2028,14 +2028,16 @@  DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_level_fops,
 
 static int i915_guc_log_relay_open(struct inode *inode, struct file *file)
 {
-	struct drm_i915_private *dev_priv = inode->i_private;
+	struct drm_i915_private *i915 = inode->i_private;
+	struct intel_guc *guc = &i915->gt.uc.guc;
+	struct intel_guc_log *log = &guc->log;
 
-	if (!USES_GUC(dev_priv))
+	if (!intel_guc_is_running(guc))
 		return -ENODEV;
 
-	file->private_data = &dev_priv->gt.uc.guc.log;
+	file->private_data = log;
 
-	return intel_guc_log_relay_open(&dev_priv->gt.uc.guc.log);
+	return intel_guc_log_relay_open(log);
 }
 
 static ssize_t
@@ -2047,16 +2049,15 @@  i915_guc_log_relay_write(struct file *filp,
 	struct intel_guc_log *log = filp->private_data;
 
 	intel_guc_log_relay_flush(log);
-
 	return cnt;
 }
 
 static int i915_guc_log_relay_release(struct inode *inode, struct file *file)
 {
-	struct drm_i915_private *dev_priv = inode->i_private;
-
-	intel_guc_log_relay_close(&dev_priv->gt.uc.guc.log);
+	struct drm_i915_private *i915 = inode->i_private;
+	struct intel_guc *guc = &i915->gt.uc.guc;
 
+	intel_guc_log_relay_close(&guc->log);
 	return 0;
 }
 

Comments

Quoting Michal Wajdeczko (2019-08-18 10:52:02)
> As we plan to continue driver load after GuC initialization
> failure, we can't assume that GuC log data will be available
> just because GuC was initially enabled. We must check that
> GuC is still running instead.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  4 ++++
>  drivers/gpu/drm/i915/i915_debugfs.c        | 17 +++++++++--------
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index 59702ebc68f6..36332064de9c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -372,6 +372,7 @@ static int guc_log_relay_create(struct intel_guc_log *log)
>         int ret;
>  
>         lockdep_assert_held(&log->relay.lock);
> +       GEM_BUG_ON(!log->vma);
>  
>          /* Keep the size of sub buffers same as shared log buffer */
>         subbuf_size = log->vma->size;
> @@ -554,6 +555,9 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
>  {
>         int ret;
>  
> +       if (!log->vma)
> +               return -ENODEV;

Ok, this is called from debugfs so after registration when the presence
or not of the log->vma is static.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris