Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Thu, Apr 23, 2015 at 08:56:40PM +0200, Daniel Vetter wrote: On Thu, Apr 23, 2015 at 04:43:52PM +0100, Chris Wilson wrote: On Fri, Apr 17, 2015 at 04:49:18PM +0300, Mika Kuoppala wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote: On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. Yeah I agree that there's more room for cleanup. I've done this minimal patch purely to shut up the bogus WARN_ONs when I tried to unify the gen6/7 pt alloc code in the next patch. Since it's bogus. How about: Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx switch logic and failed, and I wasn't fully convinced that the gen7 one won't WARN if we actually enable full ppgtt. Given all that I decided to go with the most minimal patch and just removed the offending bogus WARN. But Mika/Michel promised to hang around for eventual cleanups? Yes. There is more to come after this series. I can include Chris's suggestion. Looking at this WARN that fires continually on gen6/gen7, (it's nice to have such a blatant WARN to explain the perf decrease), don't we need diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a1a9f5dad541..006d4a2610f7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1562,6 +1562,8 @@ int i915_ppgtt_init_hw(struct drm_device *dev) ret = ppgtt-switch_mm(ppgtt, ring); if (ret != 0) return ret; + + ppgtt-pd_dirty_rings = ~(1 i); } } Yeah, but I didn't really see the point in the debug infrastructure for aliasing ppgtt. The pde loading is done somewhere completely else than for full ppgtt. And if we forget to load the pdes the effects are obvious, whereas failing to reload when changes happen with full ppgtt is much harder to spot quickly. So I opted to undo those checks again, they have been defunct before my changes anyway. This performance regression is still unfixed. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Fri, Jul 31, 2015 at 05:26:24PM +0100, Chris Wilson wrote: This performance regression is still unfixed. Hah. Just found out that adding the git id to the version also changes the kernel name - so everytime I was booting the wrong kernel and the bisect ended up at the wrong patch. So whilst we still could benefit from clearing the dirty flags after setting them, I've just wasted a day. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Fri, Apr 17, 2015 at 04:49:18PM +0300, Mika Kuoppala wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote: On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. Yeah I agree that there's more room for cleanup. I've done this minimal patch purely to shut up the bogus WARN_ONs when I tried to unify the gen6/7 pt alloc code in the next patch. Since it's bogus. How about: Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx switch logic and failed, and I wasn't fully convinced that the gen7 one won't WARN if we actually enable full ppgtt. Given all that I decided to go with the most minimal patch and just removed the offending bogus WARN. But Mika/Michel promised to hang around for eventual cleanups? Yes. There is more to come after this series. I can include Chris's suggestion. Looking at this WARN that fires continually on gen6/gen7, (it's nice to have such a blatant WARN to explain the perf decrease), don't we need diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a1a9f5dad541..006d4a2610f7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1562,6 +1562,8 @@ int i915_ppgtt_init_hw(struct drm_device *dev) ret = ppgtt-switch_mm(ppgtt, ring); if (ret != 0) return ret; + + ppgtt-pd_dirty_rings = ~(1 i); } } ? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Thu, Apr 23, 2015 at 08:56:40PM +0200, Daniel Vetter wrote: Yeah, but I didn't really see the point in the debug infrastructure for aliasing ppgtt. The pde loading is done somewhere completely else than for full ppgtt. And if we forget to load the pdes the effects are obvious, whereas failing to reload when changes happen with full ppgtt is much harder to spot quickly. So I opted to undo those checks again, they have been defunct before my changes anyway. But they did just serve a purpose. They said we were not skipping the no-op context switches. I disagree though and that we want to reduce the differences in handling the two different types of mm. There is no good reason to bake in knowledge of full-ppgtt vs aliasing-ppgtt here as the decision whether to do the mm switch is purely decided on the validity of the current/next PD. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Thu, Apr 23, 2015 at 04:43:52PM +0100, Chris Wilson wrote: On Fri, Apr 17, 2015 at 04:49:18PM +0300, Mika Kuoppala wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote: On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. Yeah I agree that there's more room for cleanup. I've done this minimal patch purely to shut up the bogus WARN_ONs when I tried to unify the gen6/7 pt alloc code in the next patch. Since it's bogus. How about: Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx switch logic and failed, and I wasn't fully convinced that the gen7 one won't WARN if we actually enable full ppgtt. Given all that I decided to go with the most minimal patch and just removed the offending bogus WARN. But Mika/Michel promised to hang around for eventual cleanups? Yes. There is more to come after this series. I can include Chris's suggestion. Looking at this WARN that fires continually on gen6/gen7, (it's nice to have such a blatant WARN to explain the perf decrease), don't we need diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a1a9f5dad541..006d4a2610f7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1562,6 +1562,8 @@ int i915_ppgtt_init_hw(struct drm_device *dev) ret = ppgtt-switch_mm(ppgtt, ring); if (ret != 0) return ret; + + ppgtt-pd_dirty_rings = ~(1 i); } } Yeah, but I didn't really see the point in the debug infrastructure for aliasing ppgtt. The pde loading is done somewhere completely else than for full ppgtt. And if we forget to load the pdes the effects are obvious, whereas failing to reload when changes happen with full ppgtt is much harder to spot quickly. So I opted to undo those checks again, they have been defunct before my changes anyway. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Thu, Apr 23, 2015 at 08:56:40PM +0200, Daniel Vetter wrote: Yeah, but I didn't really see the point in the debug infrastructure for aliasing ppgtt. The pde loading is done somewhere completely else than for full ppgtt. And if we forget to load the pdes the effects are obvious, whereas failing to reload when changes happen with full ppgtt is much harder to spot quickly. So I opted to undo those checks again, they have been defunct before my changes anyway. You removed the WARN, but didn't fix the broken code the WARN was telling you about. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Fri, Apr 17, 2015 at 04:49:18PM +0300, Mika Kuoppala wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote: On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. Yeah I agree that there's more room for cleanup. I've done this minimal patch purely to shut up the bogus WARN_ONs when I tried to unify the gen6/7 pt alloc code in the next patch. Since it's bogus. How about: Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx switch logic and failed, and I wasn't fully convinced that the gen7 one won't WARN if we actually enable full ppgtt. Given all that I decided to go with the most minimal patch and just removed the offending bogus WARN. But Mika/Michel promised to hang around for eventual cleanups? Yes. There is more to come after this series. I can include Chris's suggestion. No r-b as an interim solution to move this patch series forward? Without this we'll WARN. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Tue, Apr 14, 2015 at 06:53:41PM +0100, Chris Wilson wrote:
On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote:
On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote:
On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote:
We load the ppgtt ptes once per gpu reset/driver load/resume and
that's all that's needed. Note that this only blows up when we're
using the allocate_va_range funcs and not the special-purpose ones
used. With this change we can get rid of that duplication.
Honestly, I would prefer the test to be rewritten so that the
information on which vm was being used was passed along and not that
magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly
what vm it bound the objects into, and yet towards the end we decide to
guess again. Also, I would rather the execbuffer test be moved to
i915_gem_context since it is scattering internal knowledge about.
Yeah I agree that there's more room for cleanup. I've done this minimal
patch purely to shut up the bogus WARN_ONs when I tried to unify the
gen6/7 pt alloc code in the next patch. Since it's bogus.
How about:
Yeah, but imo there's also more. I tried to understand the gen8 legacy ctx
switch logic and failed, and I wasn't fully convinced that the gen7 one
won't WARN if we actually enable full ppgtt. Given all that I decided to
go with the most minimal patch and just removed the offending bogus WARN.
But Mika/Michel promised to hang around for eventual cleanups?
-Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index ed9232126644..be0f475ee1e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -638,22 +638,14 @@ mi_set_context(struct intel_engine_cs *ring,
static inline bool should_skip_switch(struct intel_engine_cs *ring,
struct intel_context *from,
- struct intel_context *to)
+ struct intel_context *to,
+ struct i915_hw_ppgtt *ppgtt)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (to-remap_slice)
return false;
- if (to-ppgtt) {
- if (from == to !test_bit(ring-id,
- to-ppgtt-pd_dirty_rings))
- return true;
- } else if (dev_priv-mm.aliasing_ppgtt) {
- if (from == to !test_bit(ring-id,
- dev_priv-mm.aliasing_ppgtt-pd_dirty_rings))
- return true;
- }
+ if (from == to !test_bit(ring-id, ppgtt-pd_dirty_rings))
+ return true;
return false;
}
@@ -661,15 +653,13 @@ static inline bool should_skip_switch(struct
intel_engine_cs *ring,
static bool
needs_pd_load_pre(struct intel_engine_cs *ring, struct intel_context *to)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!to-ppgtt)
return false;
if (INTEL_INFO(ring-dev)-gen 8)
return true;
- if (ring != dev_priv-ring[RCS])
+ if (ring-id != RCS)
return true;
return false;
@@ -679,15 +669,13 @@ static bool
needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to,
u32 hw_flags)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!to-ppgtt)
return false;
if (!IS_GEN8(ring-dev))
return false;
- if (ring != dev_priv-ring[RCS])
+ if (ring-id != RCS)
return false;
if (hw_flags MI_RESTORE_INHIBIT)
@@ -701,14 +689,15 @@ static int do_switch(struct intel_engine_cs *ring,
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
struct intel_context *from = ring-last_context;
+ struct i915_hw_ppgtt *ppgtt = to-ppgtt ?:
dev_priv-mm.aliasing_ppgtt;
u32 hw_flags = 0;
bool uninitialized = false;
int ret, i;
- if (from != NULL ring == dev_priv-ring[RCS])
+ if (from != NULL ring-id == RCS)
BUG_ON(from-legacy_hw_ctx.rcs_vma == NULL);
- if (should_skip_switch(ring, from, to))
+ if (should_skip_switch(ring, from, to, ppgtt))
return 0;
/* Trying to pin first makes error handling easier. */
@@ -851,6 +840,9 @@ done:
i915_gem_context_reference(to);
ring-last_context = to;
+ WARN(ppgtt-pd_dirty_rings (1ring-id),
+%s didn't clear reload\n, ring-name);
+
if (uninitialized) {
if (ring-init_context) {
ret = ring-init_context(ring, to);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. Yeah I agree that there's more room for cleanup. I've done this minimal patch purely to shut up the bogus WARN_ONs when I tried to unify the gen6/7 pt alloc code in the next patch. Since it's bogus. But I agree that the current pd reloading is hard to understand, and might even be the reason why gen7 full ppgtt doesn't quite work. Who knows, but otoh the code isnt' too harmful either (until we decide to can gen7 full ppgtt for real and remove it all). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
We load the ppgtt ptes once per gpu reset/driver load/resume and
that's all that's needed. Note that this only blows up when we're
using the allocate_va_range funcs and not the special-purpose ones
used. With this change we can get rid of that duplication.
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
drivers/gpu/drm/i915/i915_gem_context.c| 6 --
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ---
2 files changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index dd5bff657c9c..6b0962db2cf7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -575,8 +575,6 @@ static inline bool should_skip_switch(struct
intel_engine_cs *ring,
struct intel_context *from,
struct intel_context *to)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (to-remap_slice)
return false;
@@ -584,10 +582,6 @@ static inline bool should_skip_switch(struct
intel_engine_cs *ring,
if (from == to !test_bit(ring-id,
to-ppgtt-pd_dirty_rings))
return true;
- } else if (dev_priv-mm.aliasing_ppgtt) {
- if (from == to !test_bit(ring-id,
- dev_priv-mm.aliasing_ppgtt-pd_dirty_rings))
- return true;
}
return false;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a7ed9f695586..1d5badf1b887 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1250,9 +1250,6 @@ i915_gem_ringbuffer_submission(struct drm_device *dev,
struct drm_file *file,
if (ctx-ppgtt)
WARN(ctx-ppgtt-pd_dirty_rings (1ring-id),
%s didn't clear reload\n, ring-name);
- else if (dev_priv-mm.aliasing_ppgtt)
- WARN(dev_priv-mm.aliasing_ppgtt-pd_dirty_rings
- (1ring-id), %s didn't clear reload\n, ring-name);
instp_mode = args-flags I915_EXEC_CONSTANTS_MASK;
instp_mask = I915_EXEC_CONSTANTS_MASK;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Tue, Apr 14, 2015 at 07:11:25PM +0200, Daniel Vetter wrote:
On Tue, Apr 14, 2015 at 05:06:36PM +0100, Chris Wilson wrote:
On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote:
We load the ppgtt ptes once per gpu reset/driver load/resume and
that's all that's needed. Note that this only blows up when we're
using the allocate_va_range funcs and not the special-purpose ones
used. With this change we can get rid of that duplication.
Honestly, I would prefer the test to be rewritten so that the
information on which vm was being used was passed along and not that
magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly
what vm it bound the objects into, and yet towards the end we decide to
guess again. Also, I would rather the execbuffer test be moved to
i915_gem_context since it is scattering internal knowledge about.
Yeah I agree that there's more room for cleanup. I've done this minimal
patch purely to shut up the bogus WARN_ONs when I tried to unify the
gen6/7 pt alloc code in the next patch. Since it's bogus.
How about:
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index ed9232126644..be0f475ee1e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -638,22 +638,14 @@ mi_set_context(struct intel_engine_cs *ring,
static inline bool should_skip_switch(struct intel_engine_cs *ring,
struct intel_context *from,
- struct intel_context *to)
+ struct intel_context *to,
+ struct i915_hw_ppgtt *ppgtt)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (to-remap_slice)
return false;
- if (to-ppgtt) {
- if (from == to !test_bit(ring-id,
- to-ppgtt-pd_dirty_rings))
- return true;
- } else if (dev_priv-mm.aliasing_ppgtt) {
- if (from == to !test_bit(ring-id,
- dev_priv-mm.aliasing_ppgtt-pd_dirty_rings))
- return true;
- }
+ if (from == to !test_bit(ring-id, ppgtt-pd_dirty_rings))
+ return true;
return false;
}
@@ -661,15 +653,13 @@ static inline bool should_skip_switch(struct
intel_engine_cs *ring,
static bool
needs_pd_load_pre(struct intel_engine_cs *ring, struct intel_context *to)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!to-ppgtt)
return false;
if (INTEL_INFO(ring-dev)-gen 8)
return true;
- if (ring != dev_priv-ring[RCS])
+ if (ring-id != RCS)
return true;
return false;
@@ -679,15 +669,13 @@ static bool
needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to,
u32 hw_flags)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!to-ppgtt)
return false;
if (!IS_GEN8(ring-dev))
return false;
- if (ring != dev_priv-ring[RCS])
+ if (ring-id != RCS)
return false;
if (hw_flags MI_RESTORE_INHIBIT)
@@ -701,14 +689,15 @@ static int do_switch(struct intel_engine_cs *ring,
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
struct intel_context *from = ring-last_context;
+ struct i915_hw_ppgtt *ppgtt = to-ppgtt ?: dev_priv-mm.aliasing_ppgtt;
u32 hw_flags = 0;
bool uninitialized = false;
int ret, i;
- if (from != NULL ring == dev_priv-ring[RCS])
+ if (from != NULL ring-id == RCS)
BUG_ON(from-legacy_hw_ctx.rcs_vma == NULL);
- if (should_skip_switch(ring, from, to))
+ if (should_skip_switch(ring, from, to, ppgtt))
return 0;
/* Trying to pin first makes error handling easier. */
@@ -851,6 +840,9 @@ done:
i915_gem_context_reference(to);
ring-last_context = to;
+ WARN(ppgtt-pd_dirty_rings (1ring-id),
+%s didn't clear reload\n, ring-name);
+
if (uninitialized) {
if (ring-init_context) {
ret = ring-init_context(ring, to);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ef644e7eaac0..595daecb3283 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1385,13 +1385,6 @@ i915_gem_ringbuffer_submission(struct drm_device *dev,
struct drm_file *file,
if (ret)
goto error;
- if (ctx-ppgtt)
- WARN(ctx-ppgtt-pd_dirty_rings (1ring-id),
- %s didn't clear reload\n, ring-name);
- else if (dev_priv-mm.aliasing_ppgtt)
-
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Don't look at pg_dirty_rings for aliasing ppgtt
On Tue, Apr 14, 2015 at 05:35:18PM +0200, Daniel Vetter wrote: We load the ppgtt ptes once per gpu reset/driver load/resume and that's all that's needed. Note that this only blows up when we're using the allocate_va_range funcs and not the special-purpose ones used. With this change we can get rid of that duplication. Honestly, I would prefer the test to be rewritten so that the information on which vm was being used was passed along and not that magic sprinkled in the middle of nowhere. e.g. execbuffer knows exactly what vm it bound the objects into, and yet towards the end we decide to guess again. Also, I would rather the execbuffer test be moved to i915_gem_context since it is scattering internal knowledge about. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
