| Message ID | 20210119144912.12653-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915/gem: Almagamate clflushes on suspend | expand |
On Tue, 19 Jan 2021 at 14:49, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > When flushing objects larger than the CPU cache it is preferrable to use > a single wbinvd() rather than overlapping clflush(). At runtime, we > avoid wbinvd() due to its system-wide latencies, but during > singlethreaded suspend, no one will observe the imposed latency and we > can opt for the faster wbinvd to clear all objects in a single hit. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
On Tue, Jan 19, 2021 at 02:49:08PM +0000, Chris Wilson wrote: > When flushing objects larger than the CPU cache it is preferrable to use > a single wbinvd() rather than overlapping clflush(). At runtime, we > avoid wbinvd() due to its system-wide latencies, but during > singlethreaded suspend, no one will observe the imposed latency and we > can opt for the faster wbinvd to clear all objects in a single hit. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Reviewed-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 16 +++++----------- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c013148835e6..d3a287bf56c5 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1175,19 +1175,13 @@ int i915_gem_freeze_late(struct drm_i915_private *i915) > * the objects as well, see i915_gem_freeze() > */ > > - wakeref = intel_runtime_pm_get(&i915->runtime_pm); > - > - i915_gem_shrink(i915, -1UL, NULL, ~0); > + with_intel_runtime_pm(&i915->runtime_pm, wakeref) > + i915_gem_shrink(i915, -1UL, NULL, ~0); > i915_gem_drain_freed_objects(i915); > > - list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) { > - i915_gem_object_lock(obj, NULL); > - drm_WARN_ON(&i915->drm, > - i915_gem_object_set_to_cpu_domain(obj, true)); > - i915_gem_object_unlock(obj); > - } > - > - intel_runtime_pm_put(&i915->runtime_pm, wakeref); > + wbinvd_on_all_cpus(); with CONFIG_SMP=n, this results in: drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_freeze_late': drivers/gpu/drm/i915/i915_gem.c:1182:2: error: implicit declaration of function 'wbinvd_on_all_cpus' Other drivers calling this function include <asm/smp.h>. Guenter > + list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) > + __start_cpu_write(obj); > > return 0; > }
Quoting Guenter Roeck (2021-01-23 14:46:33) > On Tue, Jan 19, 2021 at 02:49:08PM +0000, Chris Wilson wrote: > > When flushing objects larger than the CPU cache it is preferrable to use > > a single wbinvd() rather than overlapping clflush(). At runtime, we > > avoid wbinvd() due to its system-wide latencies, but during > > singlethreaded suspend, no one will observe the imposed latency and we > > can opt for the faster wbinvd to clear all objects in a single hit. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Matthew Auld <matthew.auld@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 16 +++++----------- > > 1 file changed, 5 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index c013148835e6..d3a287bf56c5 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -1175,19 +1175,13 @@ int i915_gem_freeze_late(struct drm_i915_private *i915) > > * the objects as well, see i915_gem_freeze() > > */ > > > > - wakeref = intel_runtime_pm_get(&i915->runtime_pm); > > - > > - i915_gem_shrink(i915, -1UL, NULL, ~0); > > + with_intel_runtime_pm(&i915->runtime_pm, wakeref) > > + i915_gem_shrink(i915, -1UL, NULL, ~0); > > i915_gem_drain_freed_objects(i915); > > > > - list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) { > > - i915_gem_object_lock(obj, NULL); > > - drm_WARN_ON(&i915->drm, > > - i915_gem_object_set_to_cpu_domain(obj, true)); > > - i915_gem_object_unlock(obj); > > - } > > - > > - intel_runtime_pm_put(&i915->runtime_pm, wakeref); > > + wbinvd_on_all_cpus(); > > with CONFIG_SMP=n, this results in: > > drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_freeze_late': > drivers/gpu/drm/i915/i915_gem.c:1182:2: error: implicit declaration of function 'wbinvd_on_all_cpus' > > Other drivers calling this function include <asm/smp.h>. I mistakenly thought this was next to i915_gem_suspend... -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c013148835e6..d3a287bf56c5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1175,19 +1175,13 @@ int i915_gem_freeze_late(struct drm_i915_private *i915) * the objects as well, see i915_gem_freeze() */ - wakeref = intel_runtime_pm_get(&i915->runtime_pm); - - i915_gem_shrink(i915, -1UL, NULL, ~0); + with_intel_runtime_pm(&i915->runtime_pm, wakeref) + i915_gem_shrink(i915, -1UL, NULL, ~0); i915_gem_drain_freed_objects(i915); - list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) { - i915_gem_object_lock(obj, NULL); - drm_WARN_ON(&i915->drm, - i915_gem_object_set_to_cpu_domain(obj, true)); - i915_gem_object_unlock(obj); - } - - intel_runtime_pm_put(&i915->runtime_pm, wakeref); + wbinvd_on_all_cpus(); + list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) + __start_cpu_write(obj); return 0; }
When flushing objects larger than the CPU cache it is preferrable to use a single wbinvd() rather than overlapping clflush(). At runtime, we avoid wbinvd() due to its system-wide latencies, but during singlethreaded suspend, no one will observe the imposed latency and we can opt for the faster wbinvd to clear all objects in a single hit. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)