| Message ID | 1351271318-3148-3-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 26 Oct 2012 10:08:38 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > The BIOS shouldn't be touching this memory across suspend/resume, so > just leave it alone. This saves us ~50ms on resume on my T420. > > v2: change gtt restore default on pre-gen4 (Chris) > move needs_gtt_restore flag into dev_priv I'm confident that it will work on gen3 now as well, just gen2 remains doubtful. Hmm, I still have the pnv box that required the GTT rewrite after resume on my desk... Perhaps a more explicit test for a "sane" BIOS would be one that provides OpRegion. -Chris
On Fri, Oct 26, 2012 at 10:08:38AM -0700, Jesse Barnes wrote: > The BIOS shouldn't be touching this memory across suspend/resume, so > just leave it alone. This saves us ~50ms on resume on my T420. > > v2: change gtt restore default on pre-gen4 (Chris) > move needs_gtt_restore flag into dev_priv > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> I've just realized: GGTT PTEs are stored in stolen mem, and hence not restored accross S4. -Daniel > --- > drivers/gpu/drm/i915/i915_dma.c | 4 ++++ > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index b5977b4..c027266 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1339,6 +1339,10 @@ static int i915_load_modeset_init(struct drm_device *dev) > /* FIXME: do pre/post-mode set stuff in core KMS code */ > dev->vblank_disable_allowed = 1; > > + /* Gen4+ should have saner BIOSes (we hope) */ > + if (INTEL_INFO(dev)->gen < 4) > + dev_priv->needs_gtt_restore = true; > + > ret = intel_fbdev_init(dev); > if (ret) > goto cleanup_irq; > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 4d858a9..be9f47d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -540,7 +540,8 @@ static int i915_drm_thaw(struct drm_device *dev) > > intel_gt_reset(dev); > > - if (drm_core_check_feature(dev, DRIVER_MODESET)) { > + if (drm_core_check_feature(dev, DRIVER_MODESET) && > + dev_priv->needs_gtt_restore) { > mutex_lock(&dev->struct_mutex); > i915_gem_restore_gtt_mappings(dev); > mutex_unlock(&dev->struct_mutex); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1e84a59..a38eba8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -873,6 +873,8 @@ typedef struct drm_i915_private { > > struct delayed_work gen6_power_work; > > + bool needs_gtt_restore; > + > enum no_fbc_reason no_fbc_reason; > > struct drm_mm_node *compressed_fb; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 30 Oct 2012 18:59:31 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Oct 26, 2012 at 10:08:38AM -0700, Jesse Barnes wrote: > > The BIOS shouldn't be touching this memory across suspend/resume, so > > just leave it alone. This saves us ~50ms on resume on my T420. > > > > v2: change gtt restore default on pre-gen4 (Chris) > > move needs_gtt_restore flag into dev_priv > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > I've just realized: GGTT PTEs are stored in stolen mem, and hence not > restored accross S4. How to ruin the day. So we may as just evict everything upon suspend and rebuild as needed? -Chris
On Tue, Oct 30, 2012 at 10:32 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, 30 Oct 2012 18:59:31 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Fri, Oct 26, 2012 at 10:08:38AM -0700, Jesse Barnes wrote: >> > The BIOS shouldn't be touching this memory across suspend/resume, so >> > just leave it alone. This saves us ~50ms on resume on my T420. >> > >> > v2: change gtt restore default on pre-gen4 (Chris) >> > move needs_gtt_restore flag into dev_priv >> > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> I've just realized: GGTT PTEs are stored in stolen mem, and hence not >> restored accross S4. > > How to ruin the day. So we may as just evict everything upon suspend and > rebuild as needed? I think we already do that for pretty much all objects (I might have been confused a bit). The problem is to correctly sprinkle the entire gtt with scratch page entries ... I don't see an easy way to avoid that. -Daniel
On Tue, 30 Oct 2012 21:32:17 +0000 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, 30 Oct 2012 18:59:31 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, Oct 26, 2012 at 10:08:38AM -0700, Jesse Barnes wrote: > > > The BIOS shouldn't be touching this memory across suspend/resume, so > > > just leave it alone. This saves us ~50ms on resume on my T420. > > > > > > v2: change gtt restore default on pre-gen4 (Chris) > > > move needs_gtt_restore flag into dev_priv > > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > > I've just realized: GGTT PTEs are stored in stolen mem, and hence not > > restored accross S4. > > How to ruin the day. So we may as just evict everything upon suspend and > rebuild as needed? Yeah Daniel is a party pooper. So I need to measure this again anyway now that we write combine things, it may not be worth it anymore (though 2M of writes is still a fairly large amount). If it's still a long delay, I'll just apply it to the S3 paths instead of S4.
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b5977b4..c027266 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1339,6 +1339,10 @@ static int i915_load_modeset_init(struct drm_device *dev) /* FIXME: do pre/post-mode set stuff in core KMS code */ dev->vblank_disable_allowed = 1; + /* Gen4+ should have saner BIOSes (we hope) */ + if (INTEL_INFO(dev)->gen < 4) + dev_priv->needs_gtt_restore = true; + ret = intel_fbdev_init(dev); if (ret) goto cleanup_irq; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 4d858a9..be9f47d 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -540,7 +540,8 @@ static int i915_drm_thaw(struct drm_device *dev) intel_gt_reset(dev); - if (drm_core_check_feature(dev, DRIVER_MODESET)) { + if (drm_core_check_feature(dev, DRIVER_MODESET) && + dev_priv->needs_gtt_restore) { mutex_lock(&dev->struct_mutex); i915_gem_restore_gtt_mappings(dev); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1e84a59..a38eba8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -873,6 +873,8 @@ typedef struct drm_i915_private { struct delayed_work gen6_power_work; + bool needs_gtt_restore; + enum no_fbc_reason no_fbc_reason; struct drm_mm_node *compressed_fb;
The BIOS shouldn't be touching this memory across suspend/resume, so just leave it alone. This saves us ~50ms on resume on my T420. v2: change gtt restore default on pre-gen4 (Chris) move needs_gtt_restore flag into dev_priv Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_dma.c | 4 ++++ drivers/gpu/drm/i915/i915_drv.c | 3 ++- drivers/gpu/drm/i915/i915_drv.h | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-)