On Thu, Sep 26, 2024 at 06:36:13PM +0300, Ville Syrjälä wrote: > On Thu, Sep 26, 2024 at 10:48:41AM -0400, Rodrigo Vivi wrote: > > On Wed, Sep 25, 2024 at 05:45:25PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > Since this switcheroo stuff bypasses all the core pm we > > > have to manually manage the pci state. To that end add the > > > missing pci_restore_state() to the switcheroo resume hook. > > > We already have the pci_save_state() counterpart on the > > > suspend side. > > > > > > I suppose this might not matter in practice as the > > > integrated GPU probably won't lose any state in D3, > > > and I presume there are no machines where this code > > > would come into play with an Intel discrete GPU. > > > > > > Arguably none of this code should exist in the driver > > > in the first place, and instead the entire switcheroo > > > mechanism should be rewritten and properly integrated into > > > core pm code... > > > > > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx> > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > Cc: linux-pci@xxxxxxxxxxxxxxx > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/i915_driver.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > > > index fe7c34045794..c3e7225ea1ba 100644 > > > --- a/drivers/gpu/drm/i915/i915_driver.c > > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > > @@ -1311,6 +1311,8 @@ int i915_driver_resume_switcheroo(struct drm_i915_private *i915) > > > if (ret) > > > return ret; > > > > > > + pci_restore_state(pdev); > > > > then why not simply call that inside the resume, for a better alignment > > with the save counterpart? > > This is switcheroo resume. And the counterpart is in switcheroo suspend. > > For the core pm hooks I'm getting rid of both save and restore. With this I totally agree. I probably missed something when just reading the patches... I had to apply them all to see the final version. So, Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > > > > + > > > ret = i915_drm_resume_early(&i915->drm); > > > if (ret) > > > return ret; > > > -- > > > 2.44.2 > > > > > -- > Ville Syrjälä > Intel
