[PATCH v2 1/3] drm/i915/error: remove unused gen8_engine_sync_index

[PATCH v2 1/3] drm/i915/error: remove unused gen8_engine_sync_index

[Daniele Ceraolo Spurio]

Leftover from Gen8 ringbuffer support removal

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index a7933c9b5562..ef29fb48d6d9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1102,27 +1102,6 @@ static void i915_gem_record_fences(struct drm_i915_private *dev_priv,
 	error->nfence = i;
 }
 
-static inline u32
-gen8_engine_sync_index(struct intel_engine_cs *engine,
-		       struct intel_engine_cs *other)
-{
-	int idx;
-
-	/*
-	 * rcs -> 0 = vcs, 1 = bcs, 2 = vecs, 3 = vcs2;
-	 * vcs -> 0 = bcs, 1 = vecs, 2 = vcs2, 3 = rcs;
-	 * bcs -> 0 = vecs, 1 = vcs2. 2 = rcs, 3 = vcs;
-	 * vecs -> 0 = vcs2, 1 = rcs, 2 = vcs, 3 = bcs;
-	 * vcs2 -> 0 = rcs, 1 = vcs, 2 = bcs, 3 = vecs;
-	 */
-
-	idx = (other - engine) - 1;
-	if (idx < 0)
-		idx += I915_NUM_ENGINES;
-
-	return idx;
-}
-
 static void gen6_record_semaphore_state(struct intel_engine_cs *engine,
 					struct drm_i915_error_engine *ee)
 {
-- 
2.16.2

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

[PATCH v2 2/3] drm/i915/error: standardize function style in error capture

[Daniele Ceraolo Spurio]

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 by getting i915 from
error->i915. While at it, s/dev_priv/i915 in functions that don't
perform register reads.

v2: take i915 from error->i915 (Michal), s/dev_priv/i915,
    update commit message

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 84 ++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index ef29fb48d6d9..9afb1b9674c0 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1084,9 +1084,9 @@ 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 i915_gpu_state *error)
 {
+	struct drm_i915_private *dev_priv = error->i915;
 	int i;
 
 	if (INTEL_GEN(dev_priv) >= 6) {
@@ -1424,14 +1424,14 @@ 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 i915_gpu_state *error)
 {
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
+	struct drm_i915_private *i915 = error->i915;
+	struct i915_ggtt *ggtt = &i915->ggtt;
 	int i;
 
 	for (i = 0; i < I915_NUM_ENGINES; i++) {
-		struct intel_engine_cs *engine = dev_priv->engine[i];
+		struct intel_engine_cs *engine = i915->engine[i];
 		struct drm_i915_error_engine *ee = &error->engine[i];
 		struct i915_request *request;
 
@@ -1460,17 +1460,16 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
 			 * by userspace.
 			 */
 			ee->batchbuffer =
-				i915_error_object_create(dev_priv,
-							 request->batch);
+				i915_error_object_create(i915, request->batch);
 
-			if (HAS_BROKEN_CS_TLB(dev_priv))
+			if (HAS_BROKEN_CS_TLB(i915))
 				ee->wa_batchbuffer =
-					i915_error_object_create(dev_priv,
+					i915_error_object_create(i915,
 								 engine->scratch);
 			request_record_user_bo(request, ee);
 
 			ee->ctx =
-				i915_error_object_create(dev_priv,
+				i915_error_object_create(i915,
 							 request->ctx->engine[i].state);
 
 			error->simulated |=
@@ -1484,27 +1483,24 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
 			ee->cpu_ring_head = ring->head;
 			ee->cpu_ring_tail = ring->tail;
 			ee->ringbuffer =
-				i915_error_object_create(dev_priv, ring->vma);
+				i915_error_object_create(i915, ring->vma);
 
 			engine_record_requests(engine, request, ee);
 		}
 
 		ee->hws_page =
-			i915_error_object_create(dev_priv,
+			i915_error_object_create(i915,
 						 engine->status_page.vma);
 
-		ee->wa_ctx =
-			i915_error_object_create(dev_priv, engine->wa_ctx.vma);
+		ee->wa_ctx = i915_error_object_create(i915, engine->wa_ctx.vma);
 
-		ee->default_state =
-			capture_object(dev_priv, engine->default_state);
+		ee->default_state = capture_object(i915, engine->default_state);
 	}
 }
 
-static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
-				struct i915_gpu_state *error,
-				struct i915_address_space *vm,
-				int idx)
+static void gem_capture_vm(struct i915_gpu_state *error,
+			   struct i915_address_space *vm,
+			   int idx)
 {
 	struct drm_i915_error_buffer *active_bo;
 	struct i915_vma *vma;
@@ -1527,8 +1523,7 @@ 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 i915_gpu_state *error)
 {
 	int cnt = 0, i, j;
 
@@ -1548,14 +1543,13 @@ static void i915_capture_active_buffers(struct drm_i915_private *dev_priv,
 		for (j = 0; j < i && !found; j++)
 			found = error->engine[j].vm == ee->vm;
 		if (!found)
-			i915_gem_capture_vm(dev_priv, error, ee->vm, cnt++);
+			gem_capture_vm(error, ee->vm, cnt++);
 	}
 }
 
-static void i915_capture_pinned_buffers(struct drm_i915_private *dev_priv,
-					struct i915_gpu_state *error)
+static void capture_pinned_buffers(struct i915_gpu_state *error)
 {
-	struct i915_address_space *vm = &dev_priv->ggtt.base;
+	struct i915_address_space *vm = &error->i915->ggtt.base;
 	struct drm_i915_error_buffer *bo;
 	struct i915_vma *vma;
 	int count_inactive, count_active;
@@ -1605,9 +1599,9 @@ 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 i915_gpu_state *error)
 {
+	struct drm_i915_private *dev_priv = error->i915;
 	int i;
 
 	/* General organization
@@ -1704,24 +1698,25 @@ 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 i915_gpu_state *error)
 {
-	error->awake = dev_priv->gt.awake;
-	error->wakelock = atomic_read(&dev_priv->runtime_pm.wakeref_count);
-	error->suspended = dev_priv->runtime_pm.suspended;
+	struct drm_i915_private *i915 = error->i915;
+
+	error->awake = i915->gt.awake;
+	error->wakelock = atomic_read(&i915->runtime_pm.wakeref_count);
+	error->suspended = i915->runtime_pm.suspended;
 
 	error->iommu = -1;
 #ifdef CONFIG_INTEL_IOMMU
 	error->iommu = intel_iommu_gfx_mapped;
 #endif
-	error->reset_count = i915_reset_count(&dev_priv->gpu_error);
-	error->suspend_count = dev_priv->suspend_count;
+	error->reset_count = i915_reset_count(&i915->gpu_error);
+	error->suspend_count = i915->suspend_count;
 
 	memcpy(&error->device_info,
-	       INTEL_INFO(dev_priv),
+	       INTEL_INFO(i915),
 	       sizeof(error->device_info));
-	error->driver_caps = dev_priv->caps;
+	error->driver_caps = i915->caps;
 }
 
 static __always_inline void dup_param(const char *type, void *x)
@@ -1749,13 +1744,12 @@ static int capture(void *data)
 
 	capture_params(error);
 	capture_uc_state(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(error);
+	capture_reg_state(error);
+	gem_record_fences(error);
+	gem_record_rings(error);
+	capture_active_buffers(error);
+	capture_pinned_buffers(error);
 
 	error->overlay = intel_overlay_capture_error_state(error->i915);
 	error->display = intel_display_capture_error_state(error->i915);
-- 
2.16.2

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

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

[Michel Thierry]

On 3/5/2018 2:21 PM, Daniele Ceraolo Spurio 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 by getting i915 from
> error->i915.

Following the fist part of this logic, i915_error_object_free could 
probably be just error_object_free, but this would never end...

Changes look OK to me, feel free to add my r-b if you need it.

> While at it, s/dev_priv/i915 in functions that don't
> perform register reads.
> 
> v2: take i915 from error->i915 (Michal), s/dev_priv/i915,
>      update commit message
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

> ---
>   drivers/gpu/drm/i915/i915_gpu_error.c | 84 ++++++++++++++++-------------------
>   1 file changed, 39 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index ef29fb48d6d9..9afb1b9674c0 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1084,9 +1084,9 @@ 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 i915_gpu_state *error)
>   {
> +	struct drm_i915_private *dev_priv = error->i915;
>   	int i;
>   
>   	if (INTEL_GEN(dev_priv) >= 6) {
> @@ -1424,14 +1424,14 @@ 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 i915_gpu_state *error)
>   {
> -	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> +	struct drm_i915_private *i915 = error->i915;
> +	struct i915_ggtt *ggtt = &i915->ggtt;
>   	int i;
>   
>   	for (i = 0; i < I915_NUM_ENGINES; i++) {
> -		struct intel_engine_cs *engine = dev_priv->engine[i];
> +		struct intel_engine_cs *engine = i915->engine[i];
>   		struct drm_i915_error_engine *ee = &error->engine[i];
>   		struct i915_request *request;
>   
> @@ -1460,17 +1460,16 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
>   			 * by userspace.
>   			 */
>   			ee->batchbuffer =
> -				i915_error_object_create(dev_priv,
> -							 request->batch);
> +				i915_error_object_create(i915, request->batch);
>   
> -			if (HAS_BROKEN_CS_TLB(dev_priv))
> +			if (HAS_BROKEN_CS_TLB(i915))
>   				ee->wa_batchbuffer =
> -					i915_error_object_create(dev_priv,
> +					i915_error_object_create(i915,
>   								 engine->scratch);
>   			request_record_user_bo(request, ee);
>   
>   			ee->ctx =
> -				i915_error_object_create(dev_priv,
> +				i915_error_object_create(i915,
>   							 request->ctx->engine[i].state);
>   
>   			error->simulated |=
> @@ -1484,27 +1483,24 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
>   			ee->cpu_ring_head = ring->head;
>   			ee->cpu_ring_tail = ring->tail;
>   			ee->ringbuffer =
> -				i915_error_object_create(dev_priv, ring->vma);
> +				i915_error_object_create(i915, ring->vma);
>   
>   			engine_record_requests(engine, request, ee);
>   		}
>   
>   		ee->hws_page =
> -			i915_error_object_create(dev_priv,
> +			i915_error_object_create(i915,
>   						 engine->status_page.vma);
>   
> -		ee->wa_ctx =
> -			i915_error_object_create(dev_priv, engine->wa_ctx.vma);
> +		ee->wa_ctx = i915_error_object_create(i915, engine->wa_ctx.vma);
>   
> -		ee->default_state =
> -			capture_object(dev_priv, engine->default_state);
> +		ee->default_state = capture_object(i915, engine->default_state);
>   	}
>   }
>   
> -static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
> -				struct i915_gpu_state *error,
> -				struct i915_address_space *vm,
> -				int idx)
> +static void gem_capture_vm(struct i915_gpu_state *error,
> +			   struct i915_address_space *vm,
> +			   int idx)
>   {
>   	struct drm_i915_error_buffer *active_bo;
>   	struct i915_vma *vma;
> @@ -1527,8 +1523,7 @@ 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 i915_gpu_state *error)
>   {
>   	int cnt = 0, i, j;
>   
> @@ -1548,14 +1543,13 @@ static void i915_capture_active_buffers(struct drm_i915_private *dev_priv,
>   		for (j = 0; j < i && !found; j++)
>   			found = error->engine[j].vm == ee->vm;
>   		if (!found)
> -			i915_gem_capture_vm(dev_priv, error, ee->vm, cnt++);
> +			gem_capture_vm(error, ee->vm, cnt++);
>   	}
>   }
>   
> -static void i915_capture_pinned_buffers(struct drm_i915_private *dev_priv,
> -					struct i915_gpu_state *error)
> +static void capture_pinned_buffers(struct i915_gpu_state *error)
>   {
> -	struct i915_address_space *vm = &dev_priv->ggtt.base;
> +	struct i915_address_space *vm = &error->i915->ggtt.base;
>   	struct drm_i915_error_buffer *bo;
>   	struct i915_vma *vma;
>   	int count_inactive, count_active;
> @@ -1605,9 +1599,9 @@ 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 i915_gpu_state *error)
>   {
> +	struct drm_i915_private *dev_priv = error->i915;
>   	int i;
>   
>   	/* General organization
> @@ -1704,24 +1698,25 @@ 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 i915_gpu_state *error)
>   {
> -	error->awake = dev_priv->gt.awake;
> -	error->wakelock = atomic_read(&dev_priv->runtime_pm.wakeref_count);
> -	error->suspended = dev_priv->runtime_pm.suspended;
> +	struct drm_i915_private *i915 = error->i915;
> +
> +	error->awake = i915->gt.awake;
> +	error->wakelock = atomic_read(&i915->runtime_pm.wakeref_count);
> +	error->suspended = i915->runtime_pm.suspended;
>   
>   	error->iommu = -1;
>   #ifdef CONFIG_INTEL_IOMMU
>   	error->iommu = intel_iommu_gfx_mapped;
>   #endif
> -	error->reset_count = i915_reset_count(&dev_priv->gpu_error);
> -	error->suspend_count = dev_priv->suspend_count;
> +	error->reset_count = i915_reset_count(&i915->gpu_error);
> +	error->suspend_count = i915->suspend_count;
>   
>   	memcpy(&error->device_info,
> -	       INTEL_INFO(dev_priv),
> +	       INTEL_INFO(i915),
>   	       sizeof(error->device_info));
> -	error->driver_caps = dev_priv->caps;
> +	error->driver_caps = i915->caps;
>   }
>   
>   static __always_inline void dup_param(const char *type, void *x)
> @@ -1749,13 +1744,12 @@ static int capture(void *data)
>   
>   	capture_params(error);
>   	capture_uc_state(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(error);
> +	capture_reg_state(error);
> +	gem_record_fences(error);
> +	gem_record_rings(error);
> +	capture_active_buffers(error);
> +	capture_pinned_buffers(error);
>   
>   	error->overlay = intel_overlay_capture_error_state(error->i915);
>   	error->display = intel_display_capture_error_state(error->i915);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-05 22:21:21)
> 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 by getting i915 from
> error->i915. While at it, s/dev_priv/i915 in functions that don't
> perform register reads.
> 
> v2: take i915 from error->i915 (Michal), s/dev_priv/i915,
>     update commit message
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Michal Wajdeczko]

On Mon, 05 Mar 2018 23:21:21 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@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 by getting i915 from
> error->i915. While at it, s/dev_priv/i915 in functions that don't
> perform register reads.
>
> v2: take i915 from error->i915 (Michal), s/dev_priv/i915,
>     update commit message
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 84  
> ++++++++++++++++-------------------
>  1 file changed, 39 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c  
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index ef29fb48d6d9..9afb1b9674c0 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1084,9 +1084,9 @@ 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 i915_gpu_state *error)

hmm, as we drop i915 prefix then maybe we should start all static functions
with the verb, so s/gem_record_fences/record_gem_fences ?

>  {
> +	struct drm_i915_private *dev_priv = error->i915;
>  	int i;
> 	if (INTEL_GEN(dev_priv) >= 6) {
> @@ -1424,14 +1424,14 @@ 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 i915_gpu_state *error)
>  {
> -	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> +	struct drm_i915_private *i915 = error->i915;
> +	struct i915_ggtt *ggtt = &i915->ggtt;
>  	int i;
> 	for (i = 0; i < I915_NUM_ENGINES; i++) {
> -		struct intel_engine_cs *engine = dev_priv->engine[i];
> +		struct intel_engine_cs *engine = i915->engine[i];
>  		struct drm_i915_error_engine *ee = &error->engine[i];
>  		struct i915_request *request;
> @@ -1460,17 +1460,16 @@ static void i915_gem_record_rings(struct  
> drm_i915_private *dev_priv,
>  			 * by userspace.
>  			 */
>  			ee->batchbuffer =
> -				i915_error_object_create(dev_priv,
> -							 request->batch);
> +				i915_error_object_create(i915, request->batch);
> -			if (HAS_BROKEN_CS_TLB(dev_priv))
> +			if (HAS_BROKEN_CS_TLB(i915))
>  				ee->wa_batchbuffer =
> -					i915_error_object_create(dev_priv,
> +					i915_error_object_create(i915,
>  								 engine->scratch);
>  			request_record_user_bo(request, ee);
> 			ee->ctx =
> -				i915_error_object_create(dev_priv,
> +				i915_error_object_create(i915,
>  							 request->ctx->engine[i].state);
> 			error->simulated |=
> @@ -1484,27 +1483,24 @@ static void i915_gem_record_rings(struct  
> drm_i915_private *dev_priv,
>  			ee->cpu_ring_head = ring->head;
>  			ee->cpu_ring_tail = ring->tail;
>  			ee->ringbuffer =
> -				i915_error_object_create(dev_priv, ring->vma);
> +				i915_error_object_create(i915, ring->vma);
> 			engine_record_requests(engine, request, ee);
>  		}
> 		ee->hws_page =
> -			i915_error_object_create(dev_priv,
> +			i915_error_object_create(i915,
>  						 engine->status_page.vma);
> -		ee->wa_ctx =
> -			i915_error_object_create(dev_priv, engine->wa_ctx.vma);
> +		ee->wa_ctx = i915_error_object_create(i915, engine->wa_ctx.vma);
> -		ee->default_state =
> -			capture_object(dev_priv, engine->default_state);
> +		ee->default_state = capture_object(i915, engine->default_state);
>  	}
>  }
> -static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
> -				struct i915_gpu_state *error,
> -				struct i915_address_space *vm,
> -				int idx)
> +static void gem_capture_vm(struct i915_gpu_state *error,
> +			   struct i915_address_space *vm,
> +			   int idx)

same here, s/gem_capture_vm/capture_gem_vm ?

>  {
>  	struct drm_i915_error_buffer *active_bo;
>  	struct i915_vma *vma;
> @@ -1527,8 +1523,7 @@ 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 i915_gpu_state *error)
>  {
>  	int cnt = 0, i, j;
> @@ -1548,14 +1543,13 @@ static void i915_capture_active_buffers(struct  
> drm_i915_private *dev_priv,
>  		for (j = 0; j < i && !found; j++)
>  			found = error->engine[j].vm == ee->vm;
>  		if (!found)
> -			i915_gem_capture_vm(dev_priv, error, ee->vm, cnt++);
> +			gem_capture_vm(error, ee->vm, cnt++);
>  	}
>  }
> -static void i915_capture_pinned_buffers(struct drm_i915_private  
> *dev_priv,
> -					struct i915_gpu_state *error)
> +static void capture_pinned_buffers(struct i915_gpu_state *error)
>  {
> -	struct i915_address_space *vm = &dev_priv->ggtt.base;
> +	struct i915_address_space *vm = &error->i915->ggtt.base;
>  	struct drm_i915_error_buffer *bo;
>  	struct i915_vma *vma;
>  	int count_inactive, count_active;
> @@ -1605,9 +1599,9 @@ 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 i915_gpu_state *error)
>  {
> +	struct drm_i915_private *dev_priv = error->i915;
>  	int i;
> 	/* General organization
> @@ -1704,24 +1698,25 @@ 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 i915_gpu_state *error)
>  {
> -	error->awake = dev_priv->gt.awake;
> -	error->wakelock = atomic_read(&dev_priv->runtime_pm.wakeref_count);
> -	error->suspended = dev_priv->runtime_pm.suspended;
> +	struct drm_i915_private *i915 = error->i915;
> +
> +	error->awake = i915->gt.awake;
> +	error->wakelock = atomic_read(&i915->runtime_pm.wakeref_count);
> +	error->suspended = i915->runtime_pm.suspended;
> 	error->iommu = -1;
>  #ifdef CONFIG_INTEL_IOMMU
>  	error->iommu = intel_iommu_gfx_mapped;
>  #endif
> -	error->reset_count = i915_reset_count(&dev_priv->gpu_error);
> -	error->suspend_count = dev_priv->suspend_count;
> +	error->reset_count = i915_reset_count(&i915->gpu_error);
> +	error->suspend_count = i915->suspend_count;
> 	memcpy(&error->device_info,
> -	       INTEL_INFO(dev_priv),
> +	       INTEL_INFO(i915),
>  	       sizeof(error->device_info));
> -	error->driver_caps = dev_priv->caps;
> +	error->driver_caps = i915->caps;
>  }
> static __always_inline void dup_param(const char *type, void *x)
> @@ -1749,13 +1744,12 @@ static int capture(void *data)
> 	capture_params(error);
>  	capture_uc_state(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(error);
> +	capture_reg_state(error);
> +	gem_record_fences(error);
> +	gem_record_rings(error);
> +	capture_active_buffers(error);
> +	capture_pinned_buffers(error);
> 	error->overlay = intel_overlay_capture_error_state(error->i915);
>  	error->display = intel_display_capture_error_state(error->i915);

also, it looks that we are not consistent in using capture vs. record  
verb...
but cleanup of all above nitpicks can be continued in another spin, so

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

Quoting Michal Wajdeczko (2018-03-06 09:48:14)
> On Mon, 05 Mar 2018 23:21:21 +0100, Daniele Ceraolo Spurio  
> > static __always_inline void dup_param(const char *type, void *x)
> > @@ -1749,13 +1744,12 @@ static int capture(void *data)
> >       capture_params(error);
> >       capture_uc_state(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(error);
> > +     capture_reg_state(error);
> > +     gem_record_fences(error);
> > +     gem_record_rings(error);
> > +     capture_active_buffers(error);
> > +     capture_pinned_buffers(error);
> >       error->overlay = intel_overlay_capture_error_state(error->i915);
> >       error->display = intel_display_capture_error_state(error->i915);
> 
> also, it looks that we are not consistent in using capture vs. record  
> verb...

Once upon a time, I fancied using it for different effects. Record
HW register state, capture user buffers (capture requires allocation and
whatnot). Not that I have ever been consistent.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] drm/i915/error: capture uc_state after gen_state

[Daniele Ceraolo Spurio]

error->device_info.has_guc, which we check in capture_uc_state, is set
in capture_gen_state, so the latter needs to be performed first.

v2: rebased

Reported-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Fixes: 7d41ef3479a6 (drm/i915: Add Guc/HuC firmware details to error state)
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 9afb1b9674c0..9e5e9547adb2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1743,8 +1743,8 @@ static int capture(void *data)
 				  error->i915->gt.last_init_time);
 
 	capture_params(error);
-	capture_uc_state(error);
 	capture_gen_state(error);
+	capture_uc_state(error);
 	capture_reg_state(error);
 	gem_record_fences(error);
 	gem_record_rings(error);
-- 
2.16.2

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

Re: [PATCH v2 3/3] drm/i915/error: capture uc_state after gen_state

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-05 22:21:22)
> error->device_info.has_guc, which we check in capture_uc_state, is set
> in capture_gen_state, so the latter needs to be performed first.
> 
> v2: rebased
> 
> Reported-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Fixes: 7d41ef3479a6 (drm/i915: Add Guc/HuC firmware details to error state)
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/error: capture uc_state after gen_state

[Chris Wilson]

Quoting Chris Wilson (2018-03-06 09:23:21)
> Quoting Daniele Ceraolo Spurio (2018-03-05 22:21:22)
> > error->device_info.has_guc, which we check in capture_uc_state, is set
> > in capture_gen_state, so the latter needs to be performed first.
> > 
> > v2: rebased
> > 
> > Reported-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Fixes: 7d41ef3479a6 (drm/i915: Add Guc/HuC firmware details to error state)
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Picked up for dinq. Bah, should have applied this patch first for the
bugfix before the refactoring. My fault for remembering too late.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/error: remove unused gen8_engine_sync_index

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/error: remove unused gen8_engine_sync_index
URL   : https://patchwork.freedesktop.org/series/39403/
State : success

== Summary ==

Series 39403v1 series starting with [v2,1/3] drm/i915/error: remove unused gen8_engine_sync_index
https://patchwork.freedesktop.org/api/1.0/series/39403/revisions/1/mbox/

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:429s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:428s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:371s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:497s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:477s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:465s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:405s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:583s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:408s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:285s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:517s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:408s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:467s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:419s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:472s
fi-kbl-7560u     total:108  pass:104  dwarn:0   dfail:0   fail:0   skip:3  
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:516s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:580s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:437s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:520s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:534s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:500s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:482s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:424s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:521s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:391s

82e049a0d7e6bcaae89a1e0774c8dc0d60b081a1 drm-tip: 2018y-03m-05d-18h-55m-22s UTC integration manifest
b7bf31cffa00 drm/i915/error: capture uc_state after gen_state
72d9c6214db9 drm/i915/error: standardize function style in error capture
1b3f3ad9c402 drm/i915/error: remove unused gen8_engine_sync_index

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8236/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [v2,1/3] drm/i915/error: remove unused gen8_engine_sync_index

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/error: remove unused gen8_engine_sync_index
URL   : https://patchwork.freedesktop.org/series/39403/
State : failure

== Summary ==

---- Possible new issues:

Test gem_eio:
        Subgroup in-flight-external:
                pass       -> INCOMPLETE (shard-snb)

---- Known issues:

Test gem_eio:
        Subgroup in-flight-external:
                pass       -> INCOMPLETE (shard-apl) fdo#104945
Test kms_chv_cursor_fail:
        Subgroup pipe-b-128x128-top-edge:
                dmesg-warn -> PASS       (shard-snb) fdo#105185 +6
Test kms_flip:
        Subgroup dpms-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060
        Subgroup flip-vs-expired-vblank:
                fail       -> PASS       (shard-snb) fdo#102887
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-mmap-cpu:
                fail       -> PASS       (shard-apl) fdo#101623
Test kms_sysfs_edid_timing:
                warn       -> PASS       (shard-apl) fdo#100047

fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3453 pass:1818 dwarn:1   dfail:0   fail:7   skip:1626 time:11878s
shard-hsw        total:3468 pass:1773 dwarn:1   dfail:0   fail:2   skip:1691 time:11869s
shard-snb        total:3453 pass:1354 dwarn:3   dfail:0   fail:2   skip:2093 time:6853s
Blacklisted hosts:
shard-kbl        total:3274 pass:1852 dwarn:2   dfail:0   fail:5   skip:1411 time:8213s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8236/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx