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 <jani.nikula@xxxxxxxxx>
Acked-by: Lukas Wunner <lukas@xxxxxxxxx>
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(&dev_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(&dev_priv->runtime_pm);
> intel_power_domains_disable(dev_priv);
>
> --
> 2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
