From: Ville Syrjälä <[email protected]>

Currently we register to vgaarb way too early. Thus it is
possible that the entire VGA decode logic gets nuked via
GMCH_CTRL before intel_vga_disable() has even disabled the
VGA plane. This could even cause a system hang because
we'll be unable to turn off the VGA plane gracefully.

Move the vgaarb registration into intel_display_driver_register().
I suppose we could do it a bit earlier (after intel_vga_disable()),
but not convinced there's any point.

Also the error handling here is pointless since the
registration can't fail (unless the device isn't a VGA class
in which case all VGA decode logic should aleady be disabled
by the BIOS via GMCH_CTRL). But let's toss in a WARN to catch
any future breakage of vga_client_register().

Signed-off-by: Ville Syrjälä <[email protected]>
---
 .../drm/i915/display/intel_display_driver.c    | 18 +++++++-----------
 drivers/gpu/drm/i915/display/intel_vga.c       |  7 ++-----
 drivers/gpu/drm/i915/display/intel_vga.h       |  2 +-
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 7e000ba3e08b..b149976f527b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -214,16 +214,12 @@ int intel_display_driver_probe_noirq(struct intel_display 
*display)
 
        intel_bios_init(display);
 
-       ret = intel_vga_register(display);
-       if (ret)
-               goto cleanup_bios;
-
        intel_psr_dc5_dc6_wa_init(display);
 
        /* FIXME: completely on the wrong abstraction layer */
        ret = intel_power_domains_init(display);
        if (ret < 0)
-               goto cleanup_vga;
+               goto cleanup_bios;
 
        intel_pmdemand_init_early(display);
 
@@ -235,7 +231,7 @@ int intel_display_driver_probe_noirq(struct intel_display 
*display)
        display->hotplug.dp_wq = alloc_ordered_workqueue("intel-dp", 0);
        if (!display->hotplug.dp_wq) {
                ret = -ENOMEM;
-               goto cleanup_vga_client_pw_domain_dmc;
+               goto cleanup_pw_domain_dmc;
        }
 
        display->wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
@@ -307,11 +303,9 @@ int intel_display_driver_probe_noirq(struct intel_display 
*display)
        destroy_workqueue(display->wq.modeset);
 cleanup_wq_dp:
        destroy_workqueue(display->hotplug.dp_wq);
-cleanup_vga_client_pw_domain_dmc:
+cleanup_pw_domain_dmc:
        intel_dmc_fini(display);
        intel_power_domains_driver_remove(display);
-cleanup_vga:
-       intel_vga_unregister(display);
 cleanup_bios:
        intel_bios_driver_remove(display);
 
@@ -566,6 +560,8 @@ void intel_display_driver_register(struct intel_display 
*display)
        if (!HAS_DISPLAY(display))
                return;
 
+       intel_vga_register(display);
+
        /* Must be done after probing outputs */
        intel_opregion_register(display);
        intel_acpi_video_register(display);
@@ -658,8 +654,6 @@ void intel_display_driver_remove_nogem(struct intel_display 
*display)
 
        intel_power_domains_driver_remove(display);
 
-       intel_vga_unregister(display);
-
        intel_bios_driver_remove(display);
 }
 
@@ -687,6 +681,8 @@ void intel_display_driver_unregister(struct intel_display 
*display)
 
        acpi_video_unregister();
        intel_opregion_unregister(display);
+
+       intel_vga_unregister(display);
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c 
b/drivers/gpu/drm/i915/display/intel_vga.c
index c45c4bbc3f95..f13734cfd904 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -135,7 +135,7 @@ static unsigned int intel_gmch_vga_set_decode(struct 
pci_dev *pdev, bool enable_
                return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
 }
 
-int intel_vga_register(struct intel_display *display)
+void intel_vga_register(struct intel_display *display)
 {
 
        struct pci_dev *pdev = to_pci_dev(display->drm->dev);
@@ -150,10 +150,7 @@ int intel_vga_register(struct intel_display *display)
         * vga_client_register() fails with -ENODEV.
         */
        ret = vga_client_register(pdev, intel_gmch_vga_set_decode);
-       if (ret && ret != -ENODEV)
-               return ret;
-
-       return 0;
+       drm_WARN_ON(display->drm, ret && ret != -ENODEV);
 }
 
 void intel_vga_unregister(struct intel_display *display)
diff --git a/drivers/gpu/drm/i915/display/intel_vga.h 
b/drivers/gpu/drm/i915/display/intel_vga.h
index 16d699f3b641..80084265c6cd 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.h
+++ b/drivers/gpu/drm/i915/display/intel_vga.h
@@ -10,7 +10,7 @@ struct intel_display;
 
 void intel_vga_reset_io_mem(struct intel_display *display);
 void intel_vga_disable(struct intel_display *display);
-int intel_vga_register(struct intel_display *display);
+void intel_vga_register(struct intel_display *display);
 void intel_vga_unregister(struct intel_display *display);
 
 #endif /* __INTEL_VGA_H__ */
-- 
2.51.2

Reply via email to