Thanks for reviewing. Will fix all and re-rev.
On Thu, 2023-01-12 at 09:28 -0800, Ceraolo Spurio, Daniele wrote:
>
> On 1/11/2023 4:37 PM, Alan Previn wrote:
> > During suspend flow, i915 currently achors' on the
> > pm_suspend_prepare
> > callback as the location where we quiesce the entire GPU and
> > perform
> > all necessary cleanup in order to go into suspend. PXP is also
> > called
> > during this time to perform the arbitration session teardown (with
> > the assurance no additional GEM IOCTLs will come after that could
> > restart the session).
> >
> > However, if other devices or drivers fail their suspend_prepare,
> > the
> > system will not go into suspend and i915 will be expected to resume
> > operation. In this case, we need to re-initialize the PXP hardware
> > and this really should be done within the pm_resume_complete
> > callback
> > which is the correct opposing function in the resume sequence to
> > match pm_suspend_prepare of the suspend sequence.
> >
Alan:[snip]
> >
> > +static void i915_pm_complete(struct device *kdev)
>
> nit: this function should probably be moved to after pm_resume to
> keep
> the order of the PM functions (currently they're in the order they're
> called in a full suspend flow)
>
Alan: Will do.
> > +{
> > + struct drm_i915_private *i915 = kdev_to_i915(kdev);
> > +
> > + if (!i915)
> > + dev_err(kdev, "DRM not initialized, aborting
> > suspend.\n");
>
> This is a resume call, so you're not aborting suspend. The other 2
> resume calls don't check for i915, any reason you have to do so here?
> Also, any reason not to check for DRM_SWITCH_POWER_OFF ?
>
Alan: Will correct both above: remove the check and add the POWER_OFF)
as check.
> Daniele
>
> > +
> > + i915_drm_complete(&i915->drm);
> > +}
> > +
> > static int i915_pm_prepare(struct device *kdev)
> > {
> > struct drm_i915_private *i915 = kdev_to_i915(kdev);
> > @@ -1779,6 +1794,7 @@ const struct dev_pm_ops i915_pm_ops = {
> > * PMSG_RESUME]
> > */
> > .prepare = i915_pm_prepare,
> > + .complete = i915_pm_complete,
>
> Same as above, I'd move this to after .resume to keep the call order.
>
Alan: Will do.
> Daniele
>
