On Fri, Mar 01, 2019 at 04:49:34PM -0800, José Roberto de Souza wrote: > The initialization of those componentes is required by the GEM/GT not > only display so lets move then to a more the appropriate place. > > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 39 ++++++++++++++++------------ > drivers/gpu/drm/i915/intel_display.c | 7 ----- > 2 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index cc07259ec946..2b5ce764e694 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -691,24 +691,15 @@ static int i915_modeset_load(struct drm_device *dev) > if (ret) > goto cleanup_vga_client; > > - /* must happen before intel_power_domains_init_hw() on VLV/CHV */ > - intel_update_rawclk(dev_priv); > - > - intel_power_domains_init_hw(dev_priv, false); > - > intel_csr_ucode_init(dev_priv); > > - ret = intel_irq_install(dev_priv); > - if (ret) > - goto cleanup_csr; > - > intel_setup_gmbus(dev_priv); > > /* Important: The output setup functions called by modeset_init need > * working irqs for e.g. gmbus and dp aux transfers. */ > ret = intel_modeset_init(dev); > if (ret) > - goto cleanup_irq; > + goto cleanup_gmbus; > > ret = i915_gem_init(dev_priv); > if (ret) > @@ -736,12 +727,9 @@ static int i915_modeset_load(struct drm_device *dev) > i915_gem_fini(dev_priv); > cleanup_modeset: > intel_modeset_cleanup(dev); > -cleanup_irq: > - drm_irq_uninstall(dev); > +cleanup_gmbus: > intel_teardown_gmbus(dev_priv); > -cleanup_csr: > intel_csr_ucode_fini(dev_priv); > - intel_power_domains_fini_hw(dev_priv); > vga_switcheroo_unregister_client(pdev); > cleanup_vga_client: > vga_client_register(pdev, NULL, NULL, NULL); > @@ -1765,9 +1753,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret < 0) > goto out_cleanup_mmio; > > + /* must happen before intel_power_domains_init_hw() on VLV/CHV */ > + intel_update_rawclk(dev_priv); > + > + intel_power_domains_init_hw(dev_priv, false); > + > + ret = intel_irq_install(dev_priv); > + if (ret) > + goto out_cleanup_power; What are the steps we're reordering wrt. irq enable and why is it OK to reorder them? > + > ret = i915_modeset_load(&dev_priv->drm); > if (ret < 0) > - goto out_cleanup_hw; > + goto out_cleanup_irq; > > i915_driver_register(dev_priv); > > @@ -1777,7 +1774,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) > > return 0; > > -out_cleanup_hw: > +out_cleanup_irq: > + drm_irq_uninstall(&dev_priv->drm); > +out_cleanup_power: > + intel_power_domains_fini_hw(dev_priv); > i915_driver_cleanup_hw(dev_priv); > out_cleanup_mmio: > i915_driver_cleanup_mmio(dev_priv); > @@ -1810,6 +1810,13 @@ void i915_driver_unload(struct drm_device *dev) > > intel_gvt_cleanup(dev_priv); > > + /* > + * Interrupts and polling as the first thing to avoid creating havoc. > + * Too much stuff here (turning of connectors, ...) would > + * experience fancy races otherwise. > + */ > + intel_irq_uninstall(dev_priv); This too seems slightly questionable considering the flush_workqueue() etc. in intel_modeset_cleanup(). Can we be sure all modeset activity has in fact finished sufficiently to no require interrupts? > + > i915_modeset_unload(dev); > > /* Free error state after interrupts are fully disabled. */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7963348f1c64..5158e8ecb9ed 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -16364,13 +16364,6 @@ void intel_modeset_cleanup(struct drm_device *dev) > flush_work(&dev_priv->atomic_helper.free_work); > WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list)); > > - /* > - * Interrupts and polling as the first thing to avoid creating havoc. > - * Too much stuff here (turning of connectors, ...) would > - * experience fancy races otherwise. > - */ > - intel_irq_uninstall(dev_priv); > - > /* > * Due to the hpd irq storm handling the hotplug work can re-arm the > * poll handlers. Hence disable polling after hpd handling is shut down. > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx