On Wed, 2014-06-11 at 15:24 -0700, Jesse Barnes wrote: > On Wed, 11 Jun 2014 15:21:16 -0700 > Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote: > > > On Tue, 10 Jun 2014 17:26:45 +0200 > > Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > > On Tue, Jun 10, 2014 at 05:41:49PM +0300, Imre Deak wrote: > > > > On Tue, 2014-06-10 at 15:57 +0200, Daniel Vetter wrote: > > > > > On Tue, Jun 10, 2014 at 04:42:50PM +0300, Imre Deak wrote: > > > > > > On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote: > > > > > > > This allows the system to enter the lowest power mode during system freeze. > > > > > > > > > > > > > > v2: delete force wake timer at suspend (Imre) > > > > > > > v3: add GT work suspend function (Imre) > > > > > > > v4: use uncore forcewake reset (Daniel) > > > > > > > > > > > > > > Signed-off-by: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx> > > > > > > > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > > > > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > > > > > drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++++++++ > > > > > > > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > > > > > > > 5 files changed, 25 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > > > > > index 66c6ffb..7148eac 100644 > > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > > > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev) > > > > > > > drm_irq_uninstall(dev); > > > > > > > dev_priv->enable_hotplug_processing = false; > > > > > > > > > > > > > > - intel_disable_gt_powersave(dev); > > > > > > > + intel_suspend_gt_powersave(dev); > > > > > > > > > > > > I realized now that we actually do need to enable RC6 explicitly. If we > > > > > > get here right after runtime resume, the deferred RC6 enabling might be > > > > > > still pending and so RC6 might not be enabled. > > > > Seems like we could just flush the enable if it was outstanding? > > Oh and I see we already have the flush in v4... Right, I was just blind and I guess also thinking in terms of timers that don't have a similar flush API. Anyway this is fine and my comment about it above can be ignored. > do you think we might not actually arm the deferred enable work before > we get here? I haven't thought about this. But now that you mentioned I looked into the device and dpm core and can't see what would prevent our system suspend handler to be called while we are still running our driver load function. But this is a separate issue (if true at all) that exists regardless of this patch and also not a really real life scenario. So I suggest we investigate this as a follow-up. > I think the runtime_resume and suspend calls should be serialized, but > if not we'd need some locking I guess... Yes, this is taken care of the RPM framework. Similarly system suspend/resume handlers are serialized wrt. the runtime suspend/resume handlers. This patch is: Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> Btw, now all 4 patches in the patchset are reviewed.
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
