[1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

Submitted by Karol Herbst on May 4, 2019, 4:32 p.m.

Details

Message ID 20190504163219.8349-2-kherbst@redhat.com
State New
Headers show
Series "Potential fix for runpm issues on various laptops" ( rev: 1 ) in Nouveau

Not browsing as part of any series.

Commit Message

Karol Herbst May 4, 2019, 4:32 p.m.
Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 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(-)

Patch hide | download patch | download mbox

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();
 	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;
 };
 
 static inline struct nouveau_drm *

Comments

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:
> Signed-off-by: Karol Herbst <kherbst@redhat.com>
> ---
>  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?

>  	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 *
On Tue, May 7, 2019 at 9:16 PM Lyude Paul <lyude@redhat.com> wrote:
>
> On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:
> > Signed-off-by: Karol Herbst <kherbst@redhat.com>
> > ---
> >  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

> >       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
>
On Tue, May 7, 2019 at 9:23 PM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Tue, May 7, 2019 at 9:16 PM Lyude Paul <lyude@redhat.com> wrote:
> >
> > On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:
> > > Signed-off-by: Karol Herbst <kherbst@redhat.com>
> > > ---
> > >  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
> >