On Tue, May 7, 2019 at 9:23 PM Karol Herbst <kherbst@xxxxxxxxxx> wrote:
>
> On Tue, May 7, 2019 at 9:16 PM Lyude Paul <lyude@xxxxxxxxxx> wrote:
> >
> > On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:
> > > Signed-off-by: Karol Herbst <kherbst@xxxxxxxxxx>
> > > ---
> > > drm/nouveau/nouveau_acpi.c | 6 +++---
> > > drm/nouveau/nouveau_acpi.h | 4 ++--
> > > drm/nouveau/nouveau_drm.c | 15 +++++++++++----
> > > drm/nouveau/nouveau_drv.h | 2 ++
> > > 4 files changed, 18 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> > > index ffb19585..49b11d22 100644
> > > --- a/drm/nouveau/nouveau_acpi.c
> > > +++ b/drm/nouveau/nouveau_acpi.c
> > > @@ -359,11 +359,11 @@ void nouveau_register_dsm_handler(void)
> > > }
> > >
> > > /* Must be called for Optimus models before the card can be turned off */
> > > -void nouveau_switcheroo_optimus_dsm(void)
> > > +bool nouveau_switcheroo_optimus_dsm(void)
> > > {
> > > u32 result = 0;
> > > if (!nouveau_dsm_priv.optimus_detected ||
> > > nouveau_dsm_priv.optimus_skip_dsm)
> > > - return;
> > > + return false;
> > >
> > > if (nouveau_dsm_priv.optimus_flags_detected)
> > > nouveau_optimus_dsm(nouveau_dsm_priv.dhandle,
> > > NOUVEAU_DSM_OPTIMUS_FLAGS,
> > > @@ -371,7 +371,7 @@ void nouveau_switcheroo_optimus_dsm(void)
> > >
> > > nouveau_optimus_dsm(nouveau_dsm_priv.dhandle,
> > > NOUVEAU_DSM_OPTIMUS_CAPS,
> > > NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, &result);
> > > -
> > > + return true;
> > > }
> > >
> > > void nouveau_unregister_dsm_handler(void)
> > > diff --git a/drm/nouveau/nouveau_acpi.h b/drm/nouveau/nouveau_acpi.h
> > > index b86294fc..09b2a82d 100644
> > > --- a/drm/nouveau/nouveau_acpi.h
> > > +++ b/drm/nouveau/nouveau_acpi.h
> > > @@ -9,7 +9,7 @@ bool nouveau_is_optimus(void);
> > > bool nouveau_is_v1_dsm(void);
> > > void nouveau_register_dsm_handler(void);
> > > void nouveau_unregister_dsm_handler(void);
> > > -void nouveau_switcheroo_optimus_dsm(void);
> > > +bool nouveau_switcheroo_optimus_dsm(void);
> > > int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset, int len);
> > > bool nouveau_acpi_rom_supported(struct device *);
> > > void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
> > > @@ -18,7 +18,7 @@ static inline bool nouveau_is_optimus(void) { return
> > > false; };
> > > static inline bool nouveau_is_v1_dsm(void) { return false; };
> > > static inline void nouveau_register_dsm_handler(void) {}
> > > static inline void nouveau_unregister_dsm_handler(void) {}
> > > -static inline void nouveau_switcheroo_optimus_dsm(void) {}
> > > +static inline bool nouveau_switcheroo_optimus_dsm(void) { return false; }
> > > static inline bool nouveau_acpi_rom_supported(struct device *dev) { return
> > > false; }
> > > static inline int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset,
> > > int len) { return -EINVAL; }
> > > static inline void *nouveau_acpi_edid(struct drm_device *dev, struct
> > > drm_connector *connector) { return NULL; }
> > > diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> > > index 5020265b..405efda2 100644
> > > --- a/drm/nouveau/nouveau_drm.c
> > > +++ b/drm/nouveau/nouveau_drm.c
> > > @@ -903,6 +903,7 @@ nouveau_pmops_runtime_suspend(struct device *dev)
> > > {
> > > struct pci_dev *pdev = to_pci_dev(dev);
> > > struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > > + struct nouveau_drm *drm = nouveau_drm(drm_dev);
> > > int ret;
> > >
> > > if (!nouveau_pmops_runtime()) {
> > > @@ -910,12 +911,15 @@ nouveau_pmops_runtime_suspend(struct device *dev)
> > > return -EBUSY;
> > > }
> > >
> > > - nouveau_switcheroo_optimus_dsm();
> > > + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> > > + drm->runpm_dsm = nouveau_switcheroo_optimus_dsm();
> >
> > It seems like all nouveau_switcheroo_optimus_dsm() does here is check
> > nouveau_dsm_priv.optimus_detected/optimus_skip_dsm. Why not just make
> > add a function to check these that we call when setting up the DRM
> > device, then store the result of that into drm->runpm_dsm instead of
> > changing it every time we runtime suspend?
> >
>
> heh? and I ignore anybody telling me to use u8 instead of bools :p
>
uff.. wrong place
> > > ret = nouveau_do_suspend(drm_dev, true);
> > > pci_save_state(pdev);
> > > pci_disable_device(pdev);
> > > pci_ignore_hotplug(pdev);
> > > - pci_set_power_state(pdev, PCI_D3cold);
> > > + if (drm->runpm_dsm)
> > > + pci_set_power_state(pdev, PCI_D3cold);
> > > +
> > > drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
> > > return ret;
> > > }
> > > @@ -925,7 +929,8 @@ nouveau_pmops_runtime_resume(struct device *dev)
> > > {
> > > struct pci_dev *pdev = to_pci_dev(dev);
> > > struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > > - struct nvif_device *device = &nouveau_drm(drm_dev)->client.device;
> > > + struct nouveau_drm *drm = nouveau_drm(drm_dev);
> > > + struct nvif_device *device = &drm->client.device;
> > > int ret;
> > >
> > > if (!nouveau_pmops_runtime()) {
> > > @@ -933,7 +938,9 @@ nouveau_pmops_runtime_resume(struct device *dev)
> > > return -EBUSY;
> > > }
> > >
> > > - pci_set_power_state(pdev, PCI_D0);
> > > + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> > > + if (drm->runpm_dsm)
> > > + pci_set_power_state(pdev, PCI_D0);
> > > pci_restore_state(pdev);
> > > ret = pci_enable_device(pdev);
> > > if (ret)
> > > diff --git a/drm/nouveau/nouveau_drv.h b/drm/nouveau/nouveau_drv.h
> > > index da847244..941600e9 100644
> > > --- a/drm/nouveau/nouveau_drv.h
> > > +++ b/drm/nouveau/nouveau_drv.h
> > > @@ -214,6 +214,8 @@ struct nouveau_drm {
> > > struct nouveau_svm *svm;
> > >
> > > struct nouveau_dmem *dmem;
> > > +
> > > + bool runpm_dsm;
> >
> > I think this is semi-recent, but upstream/checkpatch now prefers we use
> > u8 in structs instead of plain bools.
> >
> > > };
> > >
> > > static inline struct nouveau_drm *
> > --
> > Cheers,
> > Lyude Paul
> >
_______________________________________________
Nouveau mailing list
Nouveau@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
