* [PATCH 2/3] drm/i915: Clean up GPU hang message
2016-07-04 7:48 [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Chris Wilson
@ 2016-07-04 7:48 ` Chris Wilson
2016-07-05 10:01 ` Mika Kuoppala
2016-07-04 7:48 ` [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one Chris Wilson
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-07-04 7:48 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] drm/i915: Clean up GPU hang message
2016-07-04 7:48 ` [PATCH 2/3] drm/i915: Clean up GPU hang message Chris Wilson
@ 2016-07-05 10:01 ` Mika Kuoppala
0 siblings, 0 replies; 10+ messages in thread
From: Mika Kuoppala @ 2016-07-05 10:01 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one
2016-07-04 7:48 [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Chris Wilson
2016-07-04 7:48 ` [PATCH 2/3] drm/i915: Clean up GPU hang message Chris Wilson
@ 2016-07-04 7:48 ` Chris Wilson
2016-07-05 10:10 ` Mika Kuoppala
2016-07-04 9:01 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-07-04 7:48 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one
2016-07-04 7:48 ` [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one Chris Wilson
@ 2016-07-05 10:10 ` Mika Kuoppala
2016-07-05 10:28 ` Chris Wilson
0 siblings, 1 reply; 10+ messages in thread
From: Mika Kuoppala @ 2016-07-05 10:10 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one
2016-07-05 10:10 ` Mika Kuoppala
@ 2016-07-05 10:28 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-07-05 10:28 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()
2016-07-04 7:48 [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Chris Wilson
2016-07-04 7:48 ` [PATCH 2/3] drm/i915: Clean up GPU hang message Chris Wilson
2016-07-04 7:48 ` [PATCH 3/3] drm/i915: Skip capturing an error state if we already have one Chris Wilson
@ 2016-07-04 9:01 ` Patchwork
2016-07-05 9:59 ` [PATCH 1/3] " Mika Kuoppala
2016-07-05 10:07 ` Mika Kuoppala
4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-07-04 9:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()
2016-07-04 7:48 [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Chris Wilson
` (2 preceding siblings ...)
2016-07-04 9:01 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Patchwork
@ 2016-07-05 9:59 ` Mika Kuoppala
2016-07-05 10:04 ` Chris Wilson
2016-07-05 10:07 ` Mika Kuoppala
4 siblings, 1 reply; 10+ messages in thread
From: Mika Kuoppala @ 2016-07-05 9:59 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()
2016-07-05 9:59 ` [PATCH 1/3] " Mika Kuoppala
@ 2016-07-05 10:04 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-07-05 10:04 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch()
2016-07-04 7:48 [PATCH 1/3] drm/i915: Amalgamate gen6_mm_switch() and vgpu_mm_switch() Chris Wilson
` (3 preceding siblings ...)
2016-07-05 9:59 ` [PATCH 1/3] " Mika Kuoppala
@ 2016-07-05 10:07 ` Mika Kuoppala
4 siblings, 0 replies; 10+ messages in thread
From: Mika Kuoppala @ 2016-07-05 10:07 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 10+ messages in thread