Re: [Intel-gfx] [PATCH RESEND] drm/i915: register vga switcheroo later, unregister earlier

2019-10-06 Thread Lukas Wunner
On Sun, Oct 06, 2019 at 12:46:43PM +0300, Jani Nikula wrote:
> Move vga switcheroo and dsm handler register later in
> i915_driver_register(), and unregister in i915_driver_unregister(). The
> dsm handler unregister is a nop, and is only added for completeness.
> 
> My unsubstantiated suspicion is that the vga switcheroo state change
> would not work as early as we register the hooks currently. In any case
> exposing the interfaces to the world only after we've got everything set
> up seems prudent.
> 
> Also replace the error handling in vga switcheroo register with a simple
> error message. This is done at the same time due to lack of error
> propagation from i915_driver_register().
> 
> Signed-off-by: Jani Nikula 

Acked-by: Lukas Wunner 

Seems like a good idea to limp on instead of bailing out if vga_switcheroo
registration fails.

Taking a brief look at intel_acpi.c, I notice intel_dsm_detect() checks
for presence of exactly 2 devices with class PCI_CLASS_DISPLAY_VGA.
This breaks if a third GPU is attached via Thunderbolt or CardBus.
Not sure if that can happen in reality, I suspect the DSM is only
present on older laptops?

I'm also confused that intel_acpi.c doesn't register a vga_switcheroo
handler.  Is the mux controlled by a separate ACPI method and the one
checked for in intel_acpi.c is only used to gather system information?

Thanks,

Lukas

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9354924576c4..63d47d699305 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -289,12 +289,6 @@ static int i915_driver_modeset_probe(struct 
> drm_i915_private *i915)
>   if (ret)
>   goto out;
>  
> - intel_register_dsm_handler();
> -
> - ret = i915_switcheroo_register(i915);
> - if (ret)
> - goto cleanup_vga_client;
> -
>   /* must happen before intel_power_domains_init_hw() on VLV/CHV */
>   intel_update_rawclk(i915);
>  
> @@ -343,8 +337,6 @@ static int i915_driver_modeset_probe(struct 
> drm_i915_private *i915)
>  cleanup_csr:
>   intel_csr_ucode_fini(i915);
>   intel_power_domains_driver_remove(i915);
> - i915_switcheroo_unregister(i915);
> -cleanup_vga_client:
>   intel_vga_unregister(i915);
>  out:
>   return ret;
> @@ -356,8 +348,6 @@ static void i915_driver_modeset_remove(struct 
> drm_i915_private *i915)
>  
>   intel_bios_driver_remove(i915);
>  
> - i915_switcheroo_unregister(i915);
> -
>   intel_vga_unregister(i915);
>  
>   intel_csr_ucode_fini(i915);
> @@ -1344,6 +1334,11 @@ static void i915_driver_register(struct 
> drm_i915_private *dev_priv)
>  
>   intel_power_domains_enable(dev_priv);
>   intel_runtime_pm_enable(_priv->runtime_pm);
> +
> + intel_register_dsm_handler();
> +
> + if (i915_switcheroo_register(dev_priv))
> + DRM_ERROR("Failed to register vga switcheroo!\n");
>  }
>  
>  /**
> @@ -1352,6 +1347,10 @@ static void i915_driver_register(struct 
> drm_i915_private *dev_priv)
>   */
>  static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  {
> + i915_switcheroo_unregister(dev_priv);
> +
> + intel_unregister_dsm_handler();
> +
>   intel_runtime_pm_disable(_priv->runtime_pm);
>   intel_power_domains_disable(dev_priv);
>  
> -- 
> 2.20.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH RESEND] drm/i915: register vga switcheroo later, unregister earlier

2019-10-06 Thread Jani Nikula
Move vga switcheroo and dsm handler register later in
i915_driver_register(), and unregister in i915_driver_unregister(). The
dsm handler unregister is a nop, and is only added for completeness.

My unsubstantiated suspicion is that the vga switcheroo state change
would not work as early as we register the hooks currently. In any case
exposing the interfaces to the world only after we've got everything set
up seems prudent.

Also replace the error handling in vga switcheroo register with a simple
error message. This is done at the same time due to lack of error
propagation from i915_driver_register().

Cc: Lukas Wunner 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9354924576c4..63d47d699305 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -289,12 +289,6 @@ static int i915_driver_modeset_probe(struct 
drm_i915_private *i915)
if (ret)
goto out;
 
-   intel_register_dsm_handler();
-
-   ret = i915_switcheroo_register(i915);
-   if (ret)
-   goto cleanup_vga_client;
-
/* must happen before intel_power_domains_init_hw() on VLV/CHV */
intel_update_rawclk(i915);
 
@@ -343,8 +337,6 @@ static int i915_driver_modeset_probe(struct 
drm_i915_private *i915)
 cleanup_csr:
intel_csr_ucode_fini(i915);
intel_power_domains_driver_remove(i915);
-   i915_switcheroo_unregister(i915);
-cleanup_vga_client:
intel_vga_unregister(i915);
 out:
return ret;
@@ -356,8 +348,6 @@ static void i915_driver_modeset_remove(struct 
drm_i915_private *i915)
 
intel_bios_driver_remove(i915);
 
-   i915_switcheroo_unregister(i915);
-
intel_vga_unregister(i915);
 
intel_csr_ucode_fini(i915);
@@ -1344,6 +1334,11 @@ static void i915_driver_register(struct drm_i915_private 
*dev_priv)
 
intel_power_domains_enable(dev_priv);
intel_runtime_pm_enable(_priv->runtime_pm);
+
+   intel_register_dsm_handler();
+
+   if (i915_switcheroo_register(dev_priv))
+   DRM_ERROR("Failed to register vga switcheroo!\n");
 }
 
 /**
@@ -1352,6 +1347,10 @@ static void i915_driver_register(struct drm_i915_private 
*dev_priv)
  */
 static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 {
+   i915_switcheroo_unregister(dev_priv);
+
+   intel_unregister_dsm_handler();
+
intel_runtime_pm_disable(_priv->runtime_pm);
intel_power_domains_disable(dev_priv);
 
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx