Re: [Intel-gfx] [PATCH 2/3] drm/i915/error: standardize function style in error capture

[Michal Wajdeczko]

On Fri, 02 Mar 2018 20:19:29 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospu...@intel.com> wrote:

some of the static functions used from capture() have the "i915_"
prefix while other don't; most of them take i915 as a parameter, but one
of them derives it internally from error->i915. Let's be consistent by
avoiding prefix for static functions and always providing i915 as a
parameter.

Maybe this one static function that derived i915 from error->i915 is the
one that did it correctly? I see no point in passing dev_priv directly
as extra param as it is already attached to passed gpu error state.

/Michal


Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 47  
++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c  
b/drivers/gpu/drm/i915/i915_gpu_error.c
index ef29fb48d6d9..d1f96e6a723a 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1084,8 +1084,8 @@ static uint32_t i915_error_generate_code(struct  
drm_i915_private *dev_priv,
        return error_code;
 }
-static void i915_gem_record_fences(struct drm_i915_private *dev_priv,
-                                  struct i915_gpu_state *error)
+static void gem_record_fences(struct drm_i915_private *dev_priv,
+                             struct i915_gpu_state *error)
 {
        int i;
@@ -1424,8 +1424,8 @@ capture_object(struct drm_i915_private *dev_priv,
        }
 }
-static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
-                                 struct i915_gpu_state *error)
+static void gem_record_rings(struct drm_i915_private *dev_priv,
+                            struct i915_gpu_state *error)
 {
        struct i915_ggtt *ggtt = &dev_priv->ggtt;
        int i;
@@ -1527,8 +1527,8 @@ static void i915_gem_capture_vm(struct  
drm_i915_private *dev_priv,
        error->active_bo_count[idx] = count;
 }
-static void i915_capture_active_buffers(struct drm_i915_private  
*dev_priv,
-                                       struct i915_gpu_state *error)
+static void capture_active_buffers(struct drm_i915_private *dev_priv,
+                                  struct i915_gpu_state *error)
 {
        int cnt = 0, i, j;
@@ -1552,8 +1552,8 @@ static void i915_capture_active_buffers(struct  
drm_i915_private *dev_priv,
        }
 }
-static void i915_capture_pinned_buffers(struct drm_i915_private  
*dev_priv,
-                                       struct i915_gpu_state *error)
+static void capture_pinned_buffers(struct drm_i915_private *dev_priv,
+                                  struct i915_gpu_state *error)
 {
        struct i915_address_space *vm = &dev_priv->ggtt.base;
        struct drm_i915_error_buffer *bo;
@@ -1583,9 +1583,9 @@ static void i915_capture_pinned_buffers(struct  
drm_i915_private *dev_priv,
        error->pinned_bo = bo;
 }
-static void capture_uc_state(struct i915_gpu_state *error)
+static void capture_uc_state(struct drm_i915_private *i915,
+                            struct i915_gpu_state *error)
 {
-       struct drm_i915_private *i915 = error->i915;
        struct i915_error_uc *error_uc = &error->uc;
        /* Capturing uC state won't be useful if there is no GuC */
@@ -1605,8 +1605,8 @@ static void capture_uc_state(struct i915_gpu_state  
*error)
 }
/* Capture all registers which don't fit into another category. */
-static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
-                                  struct i915_gpu_state *error)
+static void capture_reg_state(struct drm_i915_private *dev_priv,
+                             struct i915_gpu_state *error)
 {
        int i;
@@ -1704,8 +1704,8 @@ static void i915_error_capture_msg(struct  
drm_i915_private *dev_priv,
                  engine_mask ? "reset" : "continue");
 }
-static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
-                                  struct i915_gpu_state *error)
+static void capture_gen_state(struct drm_i915_private *dev_priv,
+                             struct i915_gpu_state *error)
 {
        error->awake = dev_priv->gt.awake;
        error->wakelock = atomic_read(&dev_priv->runtime_pm.wakeref_count);
@@ -1741,6 +1741,7 @@ static void capture_params(struct i915_gpu_state  
*error)
 static int capture(void *data)
 {
        struct i915_gpu_state *error = data;
+       struct drm_i915_private *i915 = error->i915;
        error->time = ktime_get_real();
        error->boottime = ktime_get_boottime();
@@ -1748,17 +1749,17 @@ static int capture(void *data)
                                  error->i915->gt.last_init_time);
        capture_params(error);
-       capture_uc_state(error);
+       capture_uc_state(i915, error);
-       i915_capture_gen_state(error->i915, error);
-       i915_capture_reg_state(error->i915, error);
-       i915_gem_record_fences(error->i915, error);
-       i915_gem_record_rings(error->i915, error);
-       i915_capture_active_buffers(error->i915, error);
-       i915_capture_pinned_buffers(error->i915, error);
+       capture_gen_state(i915, error);
+       capture_reg_state(i915, error);
+       gem_record_fences(i915, error);
+       gem_record_rings(i915, error);
+       capture_active_buffers(i915, error);
+       capture_pinned_buffers(i915, error);
-       error->overlay = intel_overlay_capture_error_state(error->i915);
-       error->display = intel_display_capture_error_state(error->i915);
+       error->overlay = intel_overlay_capture_error_state(i915);
+       error->display = intel_display_capture_error_state(i915);
        return 0;
 }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx