Re: [Intel-gfx] [PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup

[Chris Wilson]

Quoting José Roberto de Souza (2018-07-16 23:38:38)
> @@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
> goto out_cleanup_hw;
> }
>  
> +   ret = intel_irq_install(dev_priv);
> +   if (ret)
> +   goto out_cleanup_hw;
> +
> +   /* i915_gem_init() call chain will call
> +* intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
> +*/
> +   intel_power_domains_init_hw(dev_priv, false);
> +
> +   ret = i915_gem_init(dev_priv);
> +   if (ret)
> +   goto cleanup_irq;
> +
> ret = i915_load_modeset_init(_priv->drm);
> if (ret < 0)
> -   goto out_cleanup_hw;
> +   goto cleanup_gem;

Bzzt. Order is extremely important. e.g. modeset init needs to reserve
portions of the GTT still in use by the BIOS before we wipe it.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup

[José Roberto de Souza]

i915_load_modeset_init() and intel_modeset_cleanup() was initializing
and cleaning up things that is not modeset only.
This will make easy initialize drive without display part.

Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_drv.c  | 56 ++--
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 16 +++-
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 99792039176f..402ed9b4f29e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -665,25 +665,15 @@ static int i915_load_modeset_init(struct drm_device *dev)
/* 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;
-
-   ret = i915_gem_init(dev_priv);
-   if (ret)
-   goto cleanup_modeset;
+   goto cleanup_gmbus;
 
intel_setup_overlay(dev_priv);
 
@@ -692,23 +682,17 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
ret = intel_fbdev_init(dev);
if (ret)
-   goto cleanup_gem;
+   goto cleanup_modeset;
 
/* Only enable hotplug handling once the fbdev is fully set up. */
intel_hpd_init(dev_priv);
 
return 0;
 
-cleanup_gem:
-   if (i915_gem_suspend(dev_priv))
-   DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-   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(dev_priv);
vga_switcheroo_unregister_client(pdev);
@@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const struct 
pci_device_id *ent)
goto out_cleanup_hw;
}
 
+   ret = intel_irq_install(dev_priv);
+   if (ret)
+   goto out_cleanup_hw;
+
+   /* i915_gem_init() call chain will call
+* intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
+*/
+   intel_power_domains_init_hw(dev_priv, false);
+
+   ret = i915_gem_init(dev_priv);
+   if (ret)
+   goto cleanup_irq;
+
ret = i915_load_modeset_init(_priv->drm);
if (ret < 0)
-   goto out_cleanup_hw;
+   goto cleanup_gem;
 
i915_driver_register(dev_priv);
 
@@ -1411,6 +1408,12 @@ int i915_driver_load(struct pci_dev *pdev, const struct 
pci_device_id *ent)
 
return 0;
 
+cleanup_gem:
+   if (i915_gem_suspend(dev_priv))
+   DRM_ERROR("failed to idle hardware; continuing to unload!\n");
+   i915_gem_fini(dev_priv);
+cleanup_irq:
+   drm_irq_uninstall(_priv->drm);
 out_cleanup_hw:
i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
@@ -1445,8 +1448,21 @@ void i915_driver_unload(struct drm_device *dev)
 
intel_gvt_cleanup(dev_priv);
 
+   intel_modeset_cleanup_prepare(dev);
+
+   intel_disable_gt_powersave(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);
+
intel_modeset_cleanup(dev);
 
+   intel_cleanup_gt_powersave(dev_priv);
+
intel_bios_cleanup(dev_priv);
 
vga_switcheroo_unregister_client(pdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4fb937399440..51eb48f6b57a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3419,6 +3419,7 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern int intel_modeset_init(struct drm_device *dev);
+extern void intel_modeset_cleanup_prepare(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_connector_register(struct drm_connector *);
 extern void intel_connector_unregister(struct drm_connector *);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bbf63741ae80..136fb8d51967 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15905,7 +15905,7 @@ static void intel_hpd_poll_fini(struct drm_device