Re: [PATCH v2 1/4] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

From: Lyude Paul <lyude@redhat.com>
To: Karol Herbst <kherbst@redhat.com>, nouveau@lists.freedesktop.org
Cc: linux-pci@vger.kernel.org, Bjorn Helgaas <helgaas@kernel.org>
Subject: Re: [PATCH v2 1/4] drm: don't set the pci power state if the pci subsystem handles the ACPI bits
Date: Wed, 08 May 2019 15:10:40 -0400	[thread overview]
Message-ID: <70bd607ac14d19b023834853a44af40d9dee1942.camel@redhat.com> (raw)
In-Reply-To: <20190507201245.9295-2-kherbst@redhat.com>

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2019-05-07 at 22:12 +0200, Karol Herbst wrote:
> v2: rework detection of if Nouveau calling a DSM method or not
> 
> Signed-off-by: Karol Herbst <kherbst@redhat.com>
> ---
>  drm/nouveau/nouveau_acpi.c |  7 ++++++-
>  drm/nouveau/nouveau_acpi.h |  2 ++
>  drm/nouveau/nouveau_drm.c  | 14 +++++++++++---
>  drm/nouveau/nouveau_drv.h  |  2 ++
>  4 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> index ffb19585..92dfc900 100644
> --- a/drm/nouveau/nouveau_acpi.c
> +++ b/drm/nouveau/nouveau_acpi.c
> @@ -358,6 +358,12 @@ void nouveau_register_dsm_handler(void)
>  	vga_switcheroo_register_handler(&nouveau_dsm_handler, 0);
>  }
>  
> +bool nouveau_runpm_calls_dsm(void)
> +{
> +	return nouveau_dsm_priv.optimus_detected &&
> +		!nouveau_dsm_priv.optimus_skip_dsm;
> +}
> +
>  /* Must be called for Optimus models before the card can be turned off */
>  void nouveau_switcheroo_optimus_dsm(void)
>  {
> @@ -371,7 +377,6 @@ void nouveau_switcheroo_optimus_dsm(void)
>  
>  	nouveau_optimus_dsm(nouveau_dsm_priv.dhandle,
> NOUVEAU_DSM_OPTIMUS_CAPS,
>  		NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, &result);
> -
>  }
>  
>  void nouveau_unregister_dsm_handler(void)
> diff --git a/drm/nouveau/nouveau_acpi.h b/drm/nouveau/nouveau_acpi.h
> index b86294fc..0f5d7aa0 100644
> --- a/drm/nouveau/nouveau_acpi.h
> +++ b/drm/nouveau/nouveau_acpi.h
> @@ -13,6 +13,7 @@ void 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 *);
> +bool nouveau_runpm_calls_dsm(void);
>  #else
>  static inline bool nouveau_is_optimus(void) { return false; };
>  static inline bool nouveau_is_v1_dsm(void) { return false; };
> @@ -22,6 +23,7 @@ static inline void nouveau_switcheroo_optimus_dsm(void) {}
>  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; }
> +static inline bool nouveau_runpm_calls_dsm(void) { return false; }
>  #endif
>  
>  #endif
> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> index 5020265b..09e68e61 100644
> --- a/drm/nouveau/nouveau_drm.c
> +++ b/drm/nouveau/nouveau_drm.c
> @@ -556,6 +556,7 @@ nouveau_drm_device_init(struct drm_device *dev)
>  	nouveau_fbcon_init(dev);
>  	nouveau_led_init(dev);
>  
> +	drm->runpm_dsm = nouveau_runpm_calls_dsm();
>  	if (nouveau_pmops_runtime()) {
>  		pm_runtime_use_autosuspend(dev->dev);
>  		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
> @@ -903,6 +904,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 +912,15 @@ nouveau_pmops_runtime_suspend(struct device *dev)
>  		return -EBUSY;
>  	}
>  
> +	drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>  	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 +930,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 +939,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 *
-- 
Cheers,
	Lyude Paul