[PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[Chris Wilson]

These are identical, so let's just use the same vfunc.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0bb18b88dc7a..fd249f5a82f0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1683,17 +1683,6 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
 	return 0;
 }
 
-static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt,
-			  struct drm_i915_gem_request *req)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
-
-	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
-	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
-	return 0;
-}
-
 static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
 			  struct drm_i915_gem_request *req)
 {
@@ -1731,15 +1720,10 @@ static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
 			  struct drm_i915_gem_request *req)
 {
 	struct intel_engine_cs *engine = req->engine;
-	struct drm_device *dev = ppgtt->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
+	struct drm_i915_private *dev_priv = req->i915;
 
 	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
 	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
-
-	POSTING_READ(RING_PP_DIR_DCLV(engine));
-
 	return 0;
 }
 
@@ -2074,18 +2058,15 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	int ret;
 
 	ppgtt->base.pte_encode = ggtt->base.pte_encode;
-	if (IS_GEN6(dev)) {
+	if (intel_vgpu_active(dev_priv) || IS_GEN6(dev))
 		ppgtt->switch_mm = gen6_mm_switch;
-	} else if (IS_HASWELL(dev)) {
+	else if (IS_HASWELL(dev))
 		ppgtt->switch_mm = hsw_mm_switch;
-	} else if (IS_GEN7(dev)) {
+	else if (IS_GEN7(dev))
 		ppgtt->switch_mm = gen7_mm_switch;
-	} else
+	else
 		BUG();
 
-	if (intel_vgpu_active(dev_priv))
-		ppgtt->switch_mm = vgpu_mm_switch;
-
 	ret = gen6_ppgtt_alloc(ppgtt);
 	if (ret)
 		return ret;
-- 
2.8.1

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

[PATCH 2/3] drm/i915: Clean up GPU hang message

[Chris Wilson]

Remove some redundant kernel messages as we deduce a hung GPU and
capture the error state.

v2: Fix "hang" vs "no progress" message whilst I was there
v3: s/snprintf/scnprintf/

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f0535df41dfd..0e0710acf7f1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3084,9 +3084,8 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
 		container_of(work, typeof(*dev_priv),
 			     gpu_error.hangcheck_work.work);
 	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	int busy_count = 0, rings_hung = 0;
-	bool stuck[I915_NUM_ENGINES] = { 0 };
+	unsigned int hung = 0, stuck = 0;
+	int busy_count = 0;
 #define BUSY 1
 #define KICK 5
 #define HUNG 20
@@ -3104,7 +3103,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
 	 */
 	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
 
-	for_each_engine_id(engine, dev_priv, id) {
+	for_each_engine(engine, dev_priv) {
 		bool busy = intel_engine_has_waiter(engine);
 		u64 acthd;
 		u32 seqno;
@@ -3167,10 +3166,15 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
 					break;
 				case HANGCHECK_HUNG:
 					engine->hangcheck.score += HUNG;
-					stuck[id] = true;
 					break;
 				}
 			}
+
+			if (engine->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG) {
+				hung |= intel_engine_flag(engine);
+				if (engine->hangcheck.action != HANGCHECK_HUNG)
+					stuck |= intel_engine_flag(engine);
+			}
 		} else {
 			engine->hangcheck.action = HANGCHECK_ACTIVE;
 
@@ -3195,17 +3199,24 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
 		busy_count += busy;
 	}
 
-	for_each_engine_id(engine, dev_priv, id) {
-		if (engine->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG) {
-			DRM_INFO("%s on %s\n",
-				 stuck[id] ? "stuck" : "no progress",
-				 engine->name);
-			rings_hung |= intel_engine_flag(engine);
-		}
-	}
+	if (hung) {
+		char msg[80];
+		int len;
 
-	if (rings_hung)
-		i915_handle_error(dev_priv, rings_hung, "Engine(s) hung");
+		/* If some rings hung but others were still busy, only
+		 * blame the hanging rings in the synopsis.
+		 */
+		if (stuck != hung)
+			hung &= ~stuck;
+		len = scnprintf(msg, sizeof(msg),
+				"%s on ", stuck == hung ? "No progress" : "Hang");
+		for_each_engine_masked(engine, dev_priv, hung)
+			len += scnprintf(msg + len, sizeof(msg) - len,
+					 "%s, ", engine->name);
+		msg[len-2] = '\0';
+
+		return i915_handle_error(dev_priv, hung, msg);
+	}
 
 	/* Reset timer in case GPU hangs without another request being added */
 	if (busy_count)
-- 
2.8.1

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

[PATCH 3/3] drm/i915: Skip capturing an error state if we already have one

[Chris Wilson]

As we only ever keep the first error state around, we can avoid some
work that can be quite intrusive if we don't record the error the second
time around. This does move the race whereby the user could discard one
error state as the second is being captured, but that race exists in the
current code and we hope that recapturing error state is only done for
debugging.

Note that as we discard the error state for simulated errors, igt that
exercise error capture continue to function.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index c6e05cccbedf..3a43b434c92e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1401,6 +1401,9 @@ void i915_capture_error_state(struct drm_i915_private *dev_priv,
 	struct drm_i915_error_state *error;
 	unsigned long flags;
 
+	if (READ_ONCE(dev_priv->gpu_error.first_error))
+		return;
+
 	/* Account for pipe specific data like PIPE*STAT */
 	error = kzalloc(sizeof(*error), GFP_ATOMIC);
 	if (!error) {
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()
URL   : https://patchwork.freedesktop.org/series/9449/
State : failure

== Summary ==

Series 9449v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/9449/revisions/1/mbox

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (ro-byt-n2820)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
        Subgroup basic-batch-kernel-default-uc:
                dmesg-fail -> PASS       (fi-skl-i7-6700k)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> DMESG-FAIL (ro-bdw-i7-5557U)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i5-5250u)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b:
                fail       -> PASS       (fi-skl-i5-6260u)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> INCOMPLETE (fi-skl-i5-6260u)

fi-kbl-qkkr      total:231  pass:160  dwarn:29  dfail:0   fail:2   skip:40 
fi-skl-i5-6260u  total:196  pass:184  dwarn:0   dfail:1   fail:0   skip:10 
fi-skl-i7-6700k  total:231  pass:190  dwarn:0   dfail:0   fail:2   skip:39 
fi-snb-i7-2600   total:231  pass:176  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:204  dwarn:2   dfail:1   fail:0   skip:22 
ro-bdw-i7-5557U  total:229  pass:203  dwarn:1   dfail:2   fail:0   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-byt-n2820     total:229  pass:181  dwarn:0   dfail:1   fail:2   skip:45 
ro-hsw-i3-4010u  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-hsw-i7-4770r  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-ilk-i7-620lm  total:229  pass:157  dwarn:0   dfail:1   fail:1   skip:70 
ro-ilk1-i5-650   total:224  pass:157  dwarn:0   dfail:1   fail:1   skip:65 
ro-ivb-i7-3770   total:229  pass:188  dwarn:0   dfail:1   fail:0   skip:40 
ro-skl3-i5-6260u total:229  pass:208  dwarn:1   dfail:1   fail:0   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1393/

49943b2 drm-intel-nightly: 2016y-07m-04d-07h-35m-36s UTC integration manifest
22fb51c drm/i915: Skip capturing an error state if we already have one
3898455 drm/i915: Clean up GPU hang message
5bf2752 drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

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

Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[Mika Kuoppala]

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

> These are identical, so let's just use the same vfunc.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 29 +++++------------------------
>  1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0bb18b88dc7a..fd249f5a82f0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1683,17 +1683,6 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  	return 0;
>  }
>  
> -static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt,
> -			  struct drm_i915_gem_request *req)
> -{
> -	struct intel_engine_cs *engine = req->engine;
> -	struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
> -
> -	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
> -	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> -	return 0;
> -}
> -
>  static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  			  struct drm_i915_gem_request *req)
>  {
> @@ -1731,15 +1720,10 @@ static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  			  struct drm_i915_gem_request *req)
>  {
>  	struct intel_engine_cs *engine = req->engine;
> -	struct drm_device *dev = ppgtt->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> +	struct drm_i915_private *dev_priv = req->i915;
>  
>  	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
>  	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> -
> -	POSTING_READ(RING_PP_DIR_DCLV(engine));
> -

Why is the posting not needed anymore?

-Mika


>  	return 0;
>  }
>  
> @@ -2074,18 +2058,15 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  	int ret;
>  
>  	ppgtt->base.pte_encode = ggtt->base.pte_encode;
> -	if (IS_GEN6(dev)) {
> +	if (intel_vgpu_active(dev_priv) || IS_GEN6(dev))
>  		ppgtt->switch_mm = gen6_mm_switch;
> -	} else if (IS_HASWELL(dev)) {
> +	else if (IS_HASWELL(dev))
>  		ppgtt->switch_mm = hsw_mm_switch;
> -	} else if (IS_GEN7(dev)) {
> +	else if (IS_GEN7(dev))
>  		ppgtt->switch_mm = gen7_mm_switch;
> -	} else
> +	else
>  		BUG();
>  
> -	if (intel_vgpu_active(dev_priv))
> -		ppgtt->switch_mm = vgpu_mm_switch;
> -
>  	ret = gen6_ppgtt_alloc(ppgtt);
>  	if (ret)
>  		return ret;
> -- 
> 2.8.1
>
> _______________________________________________
> 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: [PATCH 2/3] drm/i915: Clean up GPU hang message

[Mika Kuoppala]

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

> Remove some redundant kernel messages as we deduce a hung GPU and
> capture the error state.
>
> v2: Fix "hang" vs "no progress" message whilst I was there
> v3: s/snprintf/scnprintf/
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  drivers/gpu/drm/i915/i915_irq.c | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f0535df41dfd..0e0710acf7f1 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3084,9 +3084,8 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
>  		container_of(work, typeof(*dev_priv),
>  			     gpu_error.hangcheck_work.work);
>  	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int busy_count = 0, rings_hung = 0;
> -	bool stuck[I915_NUM_ENGINES] = { 0 };
> +	unsigned int hung = 0, stuck = 0;
> +	int busy_count = 0;
>  #define BUSY 1
>  #define KICK 5
>  #define HUNG 20
> @@ -3104,7 +3103,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
>  	 */
>  	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>  
> -	for_each_engine_id(engine, dev_priv, id) {
> +	for_each_engine(engine, dev_priv) {
>  		bool busy = intel_engine_has_waiter(engine);
>  		u64 acthd;
>  		u32 seqno;
> @@ -3167,10 +3166,15 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
>  					break;
>  				case HANGCHECK_HUNG:
>  					engine->hangcheck.score += HUNG;
> -					stuck[id] = true;
>  					break;
>  				}
>  			}
> +
> +			if (engine->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG) {
> +				hung |= intel_engine_flag(engine);
> +				if (engine->hangcheck.action != HANGCHECK_HUNG)
> +					stuck |= intel_engine_flag(engine);
> +			}
>  		} else {
>  			engine->hangcheck.action = HANGCHECK_ACTIVE;
>  
> @@ -3195,17 +3199,24 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
>  		busy_count += busy;
>  	}
>  
> -	for_each_engine_id(engine, dev_priv, id) {
> -		if (engine->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG) {
> -			DRM_INFO("%s on %s\n",
> -				 stuck[id] ? "stuck" : "no progress",
> -				 engine->name);
> -			rings_hung |= intel_engine_flag(engine);
> -		}
> -	}
> +	if (hung) {
> +		char msg[80];
> +		int len;
>  
> -	if (rings_hung)
> -		i915_handle_error(dev_priv, rings_hung, "Engine(s) hung");
> +		/* If some rings hung but others were still busy, only
> +		 * blame the hanging rings in the synopsis.
> +		 */
> +		if (stuck != hung)
> +			hung &= ~stuck;
> +		len = scnprintf(msg, sizeof(msg),
> +				"%s on ", stuck == hung ? "No progress" : "Hang");
> +		for_each_engine_masked(engine, dev_priv, hung)
> +			len += scnprintf(msg + len, sizeof(msg) - len,
> +					 "%s, ", engine->name);
> +		msg[len-2] = '\0';
> +
> +		return i915_handle_error(dev_priv, hung, msg);
> +	}
>  
>  	/* Reset timer in case GPU hangs without another request being added */
>  	if (busy_count)
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[Chris Wilson]

On Tue, Jul 05, 2016 at 12:59:25PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > These are identical, so let's just use the same vfunc.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 29 +++++------------------------
> >  1 file changed, 5 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 0bb18b88dc7a..fd249f5a82f0 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1683,17 +1683,6 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
> >  	return 0;
> >  }
> >  
> > -static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt,
> > -			  struct drm_i915_gem_request *req)
> > -{
> > -	struct intel_engine_cs *engine = req->engine;
> > -	struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
> > -
> > -	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
> > -	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> > -	return 0;
> > -}
> > -
> >  static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
> >  			  struct drm_i915_gem_request *req)
> >  {
> > @@ -1731,15 +1720,10 @@ static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
> >  			  struct drm_i915_gem_request *req)
> >  {
> >  	struct intel_engine_cs *engine = req->engine;
> > -	struct drm_device *dev = ppgtt->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > +	struct drm_i915_private *dev_priv = req->i915;
> >  
> >  	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
> >  	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> > -
> > -	POSTING_READ(RING_PP_DIR_DCLV(engine));
> > -
> 
> Why is the posting not needed anymore?

It was never needed. Before the GPU can use these values (from a batch
executed on the command stream) the register writes are flushed, and
these are only ever used by the GPU.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()

[Mika Kuoppala]

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

> These are identical, so let's just use the same vfunc.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 29 +++++------------------------
>  1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0bb18b88dc7a..fd249f5a82f0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1683,17 +1683,6 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  	return 0;
>  }
>  
> -static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt,
> -			  struct drm_i915_gem_request *req)
> -{
> -	struct intel_engine_cs *engine = req->engine;
> -	struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
> -
> -	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
> -	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> -	return 0;
> -}
> -
>  static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  			  struct drm_i915_gem_request *req)
>  {
> @@ -1731,15 +1720,10 @@ static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  			  struct drm_i915_gem_request *req)
>  {
>  	struct intel_engine_cs *engine = req->engine;
> -	struct drm_device *dev = ppgtt->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> +	struct drm_i915_private *dev_priv = req->i915;
>  
>  	I915_WRITE(RING_PP_DIR_DCLV(engine), PP_DIR_DCLV_2G);
>  	I915_WRITE(RING_PP_DIR_BASE(engine), get_pd_offset(ppgtt));
> -
> -	POSTING_READ(RING_PP_DIR_DCLV(engine));
> -
>  	return 0;
>  }
>  
> @@ -2074,18 +2058,15 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  	int ret;
>  
>  	ppgtt->base.pte_encode = ggtt->base.pte_encode;
> -	if (IS_GEN6(dev)) {
> +	if (intel_vgpu_active(dev_priv) || IS_GEN6(dev))
>  		ppgtt->switch_mm = gen6_mm_switch;
> -	} else if (IS_HASWELL(dev)) {
> +	else if (IS_HASWELL(dev))
>  		ppgtt->switch_mm = hsw_mm_switch;
> -	} else if (IS_GEN7(dev)) {
> +	else if (IS_GEN7(dev))
>  		ppgtt->switch_mm = gen7_mm_switch;
> -	} else
> +	else
>  		BUG();
>  
> -	if (intel_vgpu_active(dev_priv))
> -		ppgtt->switch_mm = vgpu_mm_switch;
> -
>  	ret = gen6_ppgtt_alloc(ppgtt);
>  	if (ret)
>  		return ret;
> -- 
> 2.8.1
>
> _______________________________________________
> 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: [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one

[Mika Kuoppala]

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

> As we only ever keep the first error state around, we can avoid some
> work that can be quite intrusive if we don't record the error the second
> time around. This does move the race whereby the user could discard one
> error state as the second is being captured, but that race exists in the
> current code and we hope that recapturing error state is only done for
> debugging.
>
> Note that as we discard the error state for simulated errors, igt that
> exercise error capture continue to function.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Reading through how the simulated works, we could record rings early and
as such bail out early if context doesn't want error capture.

But this patch makes sense to not provoke our racy capturing for no
gain,

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c6e05cccbedf..3a43b434c92e 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1401,6 +1401,9 @@ void i915_capture_error_state(struct drm_i915_private *dev_priv,
>  	struct drm_i915_error_state *error;
>  	unsigned long flags;
>  
> +	if (READ_ONCE(dev_priv->gpu_error.first_error))
> +		return;
> +
>  	/* Account for pipe specific data like PIPE*STAT */
>  	error = kzalloc(sizeof(*error), GFP_ATOMIC);
>  	if (!error) {
> -- 
> 2.8.1
>
> _______________________________________________
> 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: [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one

[Chris Wilson]

On Tue, Jul 05, 2016 at 01:10:29PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > As we only ever keep the first error state around, we can avoid some
> > work that can be quite intrusive if we don't record the error the second
> > time around. This does move the race whereby the user could discard one
> > error state as the second is being captured, but that race exists in the
> > current code and we hope that recapturing error state is only done for
> > debugging.
> >
> > Note that as we discard the error state for simulated errors, igt that
> > exercise error capture continue to function.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> 
> Reading through how the simulated works, we could record rings early and
> as such bail out early if context doesn't want error capture.

Hmm, not really. We do want to do the full error capture even for
simulated hangs so that we do exercise the raciness of the code in igt.

Most of the time we don't want the risk exposure or the overhead,
especially in normal runtime where we only see the first error the user
hits.

Thanks for the review
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx