On Wed, 02 Sep 2020, Ville Syrjälä <[email protected]> wrote: > On Wed, Sep 02, 2020 at 05:30:23PM +0300, Jani Nikula wrote: >> Streamline the modeset init by removing the extra init layer. >> >> No functional changes, which means the cleanup path looks hideous. >> >> Signed-off-by: Jani Nikula <[email protected]> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 63 +++++++++++---------------------- >> 1 file changed, 20 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c >> b/drivers/gpu/drm/i915/i915_drv.c >> index e332b6fd701d..4d9b61b1a115 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -215,46 +215,6 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv) >> release_resource(&dev_priv->mch_res); >> } >> >> -/* part #1: call before irq install */ >> -static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915) >> -{ >> - return intel_modeset_init_noirq(i915); >> -} >> - >> -/* part #2: call after irq install */ >> -static int i915_driver_modeset_probe(struct drm_i915_private *i915) >> -{ >> - int ret; >> - >> - /* Important: The output setup functions called by modeset_init need >> - * working irqs for e.g. gmbus and dp aux transfers. */ >> - ret = intel_modeset_init_nogem(i915); >> - if (ret) >> - goto out; >> - >> - ret = i915_gem_init(i915); >> - if (ret) >> - goto cleanup_modeset; >> - >> - ret = intel_modeset_init(i915); >> - if (ret) >> - goto cleanup_gem; >> - >> - return 0; >> - >> -cleanup_gem: >> - i915_gem_suspend(i915); >> - i915_gem_driver_remove(i915); >> - i915_gem_driver_release(i915); >> -cleanup_modeset: >> - /* FIXME */ >> - intel_modeset_driver_remove(i915); >> - intel_irq_uninstall(i915); >> - intel_modeset_driver_remove_noirq(i915); >> -out: >> - return ret; >> -} >> - >> static void intel_init_dpio(struct drm_i915_private *dev_priv) >> { >> /* >> @@ -923,7 +883,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct >> pci_device_id *ent) >> if (ret < 0) >> goto out_cleanup_mmio; >> >> - ret = i915_driver_modeset_probe_noirq(i915); >> + ret = intel_modeset_init_noirq(i915); >> if (ret < 0) >> goto out_cleanup_hw; >> >> @@ -931,10 +891,18 @@ int i915_driver_probe(struct pci_dev *pdev, const >> struct pci_device_id *ent) >> if (ret) >> goto out_cleanup_modeset; >> >> - ret = i915_driver_modeset_probe(i915); >> - if (ret < 0) >> + ret = intel_modeset_init_nogem(i915); >> + if (ret) >> goto out_cleanup_irq; >> >> + ret = i915_gem_init(i915); >> + if (ret) >> + goto out_cleanup_modeset2; >> + >> + ret = intel_modeset_init(i915); >> + if (ret) >> + goto out_cleanup_gem; >> + >> i915_driver_register(i915); >> >> enable_rpm_wakeref_asserts(&i915->runtime_pm); >> @@ -945,6 +913,15 @@ int i915_driver_probe(struct pci_dev *pdev, const >> struct pci_device_id *ent) >> >> return 0; >> >> +out_cleanup_gem: >> + i915_gem_suspend(i915); >> + i915_gem_driver_remove(i915); >> + i915_gem_driver_release(i915); >> +out_cleanup_modeset2: >> + intel_modeset_driver_remove(i915); >> + intel_irq_uninstall(i915); >> + intel_modeset_driver_remove_noirq(i915); >> + goto out_cleanup_modeset; > > Looks like we used to do the intel_irq_uninstall() twice? We even > have a FIXME in there stating as much. With this goto we only do > it the once I guess. So seems like a slight change in behaviour. > Though the comment says it gets called twice during driver remove > as well, which does not seem to be true (at least anymore). > > Anyways, fixing that properly likely requires some axctual thought > wrt. hpd vs. irq vs. other stuff. > > Series is > Reviewed-by: Ville Syrjälä <[email protected]>
Thanks. Pushed the lot, with a simple FIXME comment added about cleaning up the error path. BR, Jani. > >> out_cleanup_irq: >> intel_irq_uninstall(i915); >> out_cleanup_modeset: >> -- >> 2.20.1 -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
