Nouveau - Mar 2014 - [PATCH] drm: compute runpm on load, don't register autosuspend for non-runpm

There was a proliferation of duplicated checks for runpm == -1 &&
optimus. Instead of continuing that tradition, get rid of all of them,
only doing the optimus computation once on load.

This should hopefully fix secondary cards suspending and then being
unable to come back in non-optimus setups.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: <stable at vger.kernel.org> # 3.12+
---

This is as yet untested, but I wanted to send this out to the list
since I think a few people are still running into these annoying
issues.

I don't have access to an actual optimus configuration, so if someone
could give this a shot on such a setup and confirm that things
autosuspend as expected without any runpm parameters, that'd be great.

 drm/nouveau_drm.c | 18 +++++-------------
 drm/nouveau_vga.c |  4 +---
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
index 8f811a5..a6225ee 100644
--- a/drm/nouveau_drm.c
+++ b/drm/nouveau_drm.c
@@ -352,6 +352,10 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
 	struct nouveau_drm *drm;
 	int ret;
 
+	if (nouveau_runtime_pm == -1)
+		nouveau_runtime_pm +			nouveau_is_optimus() || nouveau_is_v1_dsm();
+
 	ret = nouveau_cli_create(nouveau_name(dev), "DRM", sizeof(*drm),
 				 (void **)&drm);
 	if (ret)
@@ -443,7 +447,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
 	nouveau_accel_init(drm);
 	nouveau_fbcon_init(dev);
 
-	if (nouveau_runtime_pm != 0) {
+	if (nouveau_runtime_pm) {
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
@@ -894,12 +898,6 @@ static int nouveau_pmops_runtime_suspend(struct device
*dev)
 	if (nouveau_runtime_pm == 0)
 		return -EINVAL;
 
-	/* are we optimus enabled? */
-	if (nouveau_runtime_pm == -1 && !nouveau_is_optimus() &&
!nouveau_is_v1_dsm()) {
-		DRM_DEBUG_DRIVER("failing to power off - not optimus\n");
-		return -EINVAL;
-	}
-
 	nv_debug_level(SILENT);
 	drm_kms_helper_poll_disable(drm_dev);
 	vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_OFF);
@@ -951,12 +949,6 @@ static int nouveau_pmops_runtime_idle(struct device *dev)
 	if (nouveau_runtime_pm == 0)
 		return -EBUSY;
 
-	/* are we optimus enabled? */
-	if (nouveau_runtime_pm == -1 && !nouveau_is_optimus() &&
!nouveau_is_v1_dsm()) {
-		DRM_DEBUG_DRIVER("failing to power off - not optimus\n");
-		return -EBUSY;
-	}
-
 	/* if we have a hdmi audio device - make sure it has a driver loaded */
 	if (drm->hdmi_device) {
 		if (!drm->hdmi_device->driver) {
diff --git a/drm/nouveau_vga.c b/drm/nouveau_vga.c
index fb84da3..b00b05a 100644
--- a/drm/nouveau_vga.c
+++ b/drm/nouveau_vga.c
@@ -91,9 +91,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
 
 	vga_client_register(dev->pdev, dev, NULL, nouveau_vga_set_decode);
 
-	if (nouveau_runtime_pm == 1)
-		runtime = true;
-	if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() ||
nouveau_is_v1_dsm()))
+	if (nouveau_runtime_pm)
 		runtime = true;
 	vga_switcheroo_register_client(dev->pdev, &nouveau_switcheroo_ops,
runtime);
 
-- 
1.8.3.2

On Thu, Mar 20, 2014 at 1:28 AM, Ilia Mirkin <imirkin at alum.mit.edu>
wrote:
> There was a proliferation of duplicated checks for runpm == -1 &&
> optimus. Instead of continuing that tradition, get rid of all of them,
> only doing the optimus computation once on load.
>
> This should hopefully fix secondary cards suspending and then being
> unable to come back in non-optimus setups.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: <stable at vger.kernel.org> # 3.12+
Also untested, but looks like a saner idea to me.  Dave?
> ---
>
> This is as yet untested, but I wanted to send this out to the list
> since I think a few people are still running into these annoying
> issues.
>
> I don't have access to an actual optimus configuration, so if someone
> could give this a shot on such a setup and confirm that things
> autosuspend as expected without any runpm parameters, that'd be great.
>
>  drm/nouveau_drm.c | 18 +++++-------------
>  drm/nouveau_vga.c |  4 +---
>  2 files changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
> index 8f811a5..a6225ee 100644
> --- a/drm/nouveau_drm.c
> +++ b/drm/nouveau_drm.c
> @@ -352,6 +352,10 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
>         struct nouveau_drm *drm;
>         int ret;
>
> +       if (nouveau_runtime_pm == -1)
> +               nouveau_runtime_pm > +                      
nouveau_is_optimus() || nouveau_is_v1_dsm();
> +
>         ret = nouveau_cli_create(nouveau_name(dev), "DRM",
sizeof(*drm),
>                                  (void **)&drm);
>         if (ret)
> @@ -443,7 +447,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
>         nouveau_accel_init(drm);
>         nouveau_fbcon_init(dev);
>
> -       if (nouveau_runtime_pm != 0) {
> +       if (nouveau_runtime_pm) {
>                 pm_runtime_use_autosuspend(dev->dev);
>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>                 pm_runtime_set_active(dev->dev);
> @@ -894,12 +898,6 @@ static int nouveau_pmops_runtime_suspend(struct device
*dev)
>         if (nouveau_runtime_pm == 0)
>                 return -EINVAL;
>
> -       /* are we optimus enabled? */
> -       if (nouveau_runtime_pm == -1 && !nouveau_is_optimus()
&& !nouveau_is_v1_dsm()) {
> -               DRM_DEBUG_DRIVER("failing to power off - not
optimus\n");
> -               return -EINVAL;
> -       }
> -
>         nv_debug_level(SILENT);
>         drm_kms_helper_poll_disable(drm_dev);
>         vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_OFF);
> @@ -951,12 +949,6 @@ static int nouveau_pmops_runtime_idle(struct device
*dev)
>         if (nouveau_runtime_pm == 0)
>                 return -EBUSY;
>
> -       /* are we optimus enabled? */
> -       if (nouveau_runtime_pm == -1 && !nouveau_is_optimus()
&& !nouveau_is_v1_dsm()) {
> -               DRM_DEBUG_DRIVER("failing to power off - not
optimus\n");
> -               return -EBUSY;
> -       }
> -
>         /* if we have a hdmi audio device - make sure it has a driver
loaded */
>         if (drm->hdmi_device) {
>                 if (!drm->hdmi_device->driver) {
> diff --git a/drm/nouveau_vga.c b/drm/nouveau_vga.c
> index fb84da3..b00b05a 100644
> --- a/drm/nouveau_vga.c
> +++ b/drm/nouveau_vga.c
> @@ -91,9 +91,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
>
>         vga_client_register(dev->pdev, dev, NULL,
nouveau_vga_set_decode);
>
> -       if (nouveau_runtime_pm == 1)
> -               runtime = true;
> -       if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() ||
nouveau_is_v1_dsm()))
> +       if (nouveau_runtime_pm)
>                 runtime = true;
>         vga_switcheroo_register_client(dev->pdev,
&nouveau_switcheroo_ops, runtime);
>
> --
> 1.8.3.2
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

> On Thu, Mar 20, 2014 at 1:28 AM, Ilia Mirkin <imirkin at
alum.mit.edu> wrote:
> > There was a proliferation of duplicated checks for runpm == -1
&&
> > optimus. Instead of continuing that tradition, get rid of all of them,
> > only doing the optimus computation once on load.
> >
> > This should hopefully fix secondary cards suspending and then being
> > unable to come back in non-optimus setups.
> >
> > Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> > Cc: <stable at vger.kernel.org> # 3.12+
> Also untested, but looks like a saner idea to me.  Dave?
Unfortunately not sane, we are looking at adding apple-gmux support, so the load
ordering is indeterminate, so doing this will mean we just have to undo it
later.

Dave.