Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.

[Caz Yokoyama <Caz.Yokoyama@intel.com>]

On Tue, 2019-03-12 at 16:09 +0000, Chris Wilson wrote:
> Quoting Caz Yokoyama (2019-03-12 16:11:14)
> > On Tue, 2019-03-12 at 15:51 +0000, Chris Wilson wrote:
> > > Quoting Caz Yokoyama (2019-03-12 15:52:09)
> > > > On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > > > > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > > > > +       if (val & DROP_SUSPEND) {
> > > > > > +               struct device *dev = i915->drm.dev;
> > > > > > +
> > > > > > +               dev_info(dev, "%s() %d: runtime_status:
> > > > > > %d\n",
> > > > > > __func__, __LINE__,
> > > > > > +                        dev->power.runtime_status);
> > > > > 
> > > > > 
> > > > > > +               pm_runtime_disable(dev);
> > > > > > +               if (!pm_runtime_status_suspended(dev)) {
> > > > > > +                       dev_warn(dev, "%s() %d:
> > > > > > runtime_status:
> > > > > > %d\n", __func__, __LINE__,
> > > > > > +                                dev-
> > > > > > >power.runtime_status);
> > > > > > +               }
> > > > > > +               pm_runtime_enable(dev);
> > > > > 
> > > > > ret = pm_runtime_force_suspend(i915->drm.dev);
> > > > > 
> > > > 
> > > > Yes, this works. However, pm_runtime_force_suspend() always
> > > > exits
> > > > on 
> > > >         if (pm_runtime_status_suspended(dev))
> > > >                 return 0;
> > > > Therefore, this patch is meaningless after suspended state.
> > > 
> > > Which would be fine for DROP_SUSPEND. Ah, so your intent is to do
> > > a
> > > DROP_WAKE as well :)
> > 
> > My intention is to remove gem-execbuf-stress-extra-wait subtest
> > because it does same as gem-execbuf-stress except for meaningless
> > (10 x
> > 5 sec) delay.
> 
> Then why complain that pm_runtime_force_suspend() is a no-op starting
> from a suspended state?
No, I am not complaining. The patch above is to confirm the patch is
meaningless on suspended state.

> 
> WAIT_EXTRA is after WAIT_STATUS, so it will always be in a suspended
> state. If your intent then is to bolster the test by forcing a fresh
> suspend cycle, make that explicit and have both DROP_WAKE |
> DROP_SUSPEND. As it stands we can use DROP_SUSPEND as a faster
> WAIT_STATUS.
Let me update the patch.
-caz

> -Chris

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev