[PATCH 00/17] drm/i915: Split driver init step to phases

[PATCH 00/17] drm/i915: Split driver init step to phases

[Imre Deak]

This is v2 of [1]. I addressed the comments from Chris and Jani. The
bulk of the change compared to v1 stems from Chris' idea to split the
different init steps into separate functions. This required some further
code shuffling as a preparation, hence the additional patches.

In the end driver load error path and dirver unload function became much
clearer. The init steps on the modesetting path would still need
to be moved to one of the new init phases functions according to the
role/effect of the given init step, but I left this for later.

Smoke tested on Gen4, SNB, IVB, BXT.

[1]
https://lists.freedesktop.org/archives/intel-gfx/2016-March/089328.html

CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Jani Nikula <jani.nikula@intel.com>
CC: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Imre Deak (17):
  Fix MCHBAR cleanup on the driver init error path
  drm/i915: Move load time PCH detect, DPIO, power domain SW init
    earlier
  drm/i915: Move load time IRQ SW init earlier
  drm/i915: Move load time init of display/audio hooks earlier
  drm/i915: Move load time init of clock gating hooks earlier
  drm/i915: Move load time runtime device info init earlier
  drm/i915: Move load time gem_load_init earlier
  drm/i915: Move load time runtime PM get later
  drm/i915: Move load time shrinker registration later
  drm/i915: Move load time audio component registration earlier
  drm/i915: Move unload time display power domain uninit later
  drm/i915: Move unload time GTT, MSI IRQ cleanup later
  drm/i915: Move unload time opregion unregistration earlier
  drm/i915: Split out load time early initialization
  drm/i915: Split out load time MMIO initialization
  drm/i915: Split out load time HW initialization
  drm/i915: Split out load time interface registration

 drivers/gpu/drm/i915/i915_dma.c      | 372 +++++++++++++++++++++++------------
 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_gem.c      |  34 ++--
 drivers/gpu/drm/i915/i915_irq.c      |   2 -
 drivers/gpu/drm/i915/intel_audio.c   |  16 +-
 drivers/gpu/drm/i915/intel_display.c |  82 ++++----
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 drivers/gpu/drm/i915/intel_pm.c      |  93 +++++----
 8 files changed, 374 insertions(+), 230 deletions(-)

-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 01/17] Fix MCHBAR cleanup on the driver init error path

[Imre Deak]

MCHBAR is cleaned up in i915_mmio_cleanup(), so the separate call in
i915_driver_load() is incorrect.

CC: David Weinehall <david.weinehall@intel.com>
Fixes: ad5c3d3ffbb2 ("drm/i915: Move MCHBAR setup earlier during init")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4aa3db6..fbb9815 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1175,7 +1175,6 @@ out_gem_unload:
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
-	intel_teardown_mchbar(dev);
 	pm_qos_remove_request(&dev_priv->pm_qos);
 	arch_phys_wc_del(dev_priv->gtt.mtrr);
 	io_mapping_free(dev_priv->gtt.mappable);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 01/17] Fix MCHBAR cleanup on the driver init error path

[David Weinehall]

On Fri, Mar 11, 2016 at 06:31:26PM +0200, Imre Deak wrote:
> MCHBAR is cleaned up in i915_mmio_cleanup(), so the separate call in
> i915_driver_load() is incorrect.
> 
> CC: David Weinehall <david.weinehall@intel.com>
> Fixes: ad5c3d3ffbb2 ("drm/i915: Move MCHBAR setup earlier during init")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4aa3db6..fbb9815 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1175,7 +1175,6 @@ out_gem_unload:
>  	if (dev->pdev->msi_enabled)
>  		pci_disable_msi(dev->pdev);
>  
> -	intel_teardown_mchbar(dev);
>  	pm_qos_remove_request(&dev_priv->pm_qos);
>  	arch_phys_wc_del(dev_priv->gtt.mtrr);
>  	io_mapping_free(dev_priv->gtt.mappable);

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 02/17] drm/i915: Move load time PCH detect, DPIO, power domain SW init earlier

[Imre Deak]

These are all SW only init steps not accessing the device and they only
need the platform identification macros to work, which are already
available earlier, so move these init steps earlier.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index fbb9815..a7bda8a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1020,7 +1020,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret < 0)
 		goto out_free_priv;
 
+	/* This must be called before any calls to HAS_PCH_* */
+	intel_detect_pch(dev);
+
 	intel_pm_setup(dev);
+	intel_init_dpio(dev_priv);
+	intel_power_domains_init(dev_priv);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -1045,9 +1050,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret < 0)
 		goto put_bridge;
 
-	/* This must be called before any calls to HAS_PCH_* */
-	intel_detect_pch(dev);
-
 	intel_uncore_init(dev);
 
 	ret = i915_gem_gtt_init(dev);
@@ -1124,16 +1126,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_device_info_runtime_init(dev);
 
-	intel_init_dpio(dev_priv);
-
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
 			goto out_gem_unload;
 	}
 
-	intel_power_domains_init(dev_priv);
-
 	ret = i915_load_modeset_init(dev);
 	if (ret < 0) {
 		DRM_ERROR("failed to init modeset\n");
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 03/17] drm/i915: Move load time IRQ SW init earlier

[Imre Deak]

Most of the IRQ init is setting up hooks so move that part earlier.
Leave the pm_qos_add_request() call in place.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 5 ++++-
 drivers/gpu/drm/i915/i915_irq.c | 2 --
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a7bda8a..4cf7bd4 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1026,6 +1026,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_pm_setup(dev);
 	intel_init_dpio(dev_priv);
 	intel_power_domains_init(dev_priv);
+	intel_irq_init(dev_priv);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -1100,7 +1101,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	dev_priv->gtt.mtrr = arch_phys_wc_add(dev_priv->gtt.mappable_base,
 					      aperture_size);
 
-	intel_irq_init(dev_priv);
+	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY,
+			   PM_QOS_DEFAULT_VALUE);
+
 	intel_uncore_sanitize(dev);
 
 	intel_opregion_setup(dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 53e5104..8c7f730 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4564,8 +4564,6 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	INIT_DELAYED_WORK(&dev_priv->gpu_error.hangcheck_work,
 			  i915_hangcheck_elapsed);
 
-	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
-
 	if (IS_GEN2(dev_priv)) {
 		dev->max_vblank_count = 0;
 		dev->driver->get_vblank_counter = i8xx_get_vblank_counter;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 04/17] drm/i915: Move load time init of display/audio hooks earlier

[Imre Deak]

All of this is SW only initialization so we can move them earlier. Move
the mutex init where the rest of the locks are inited. While at it also
convert dev to dev_priv.

v2:
- use the term hook instead of callback for these functions (Jani)

CC: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  3 ++
 drivers/gpu/drm/i915/intel_audio.c   | 16 +++----
 drivers/gpu/drm/i915/intel_display.c | 82 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 4 files changed, 51 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4cf7bd4..44f15de 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1015,6 +1015,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	mutex_init(&dev_priv->modeset_restore_lock);
 	mutex_init(&dev_priv->av_mutex);
 	mutex_init(&dev_priv->wm.wm_mutex);
+	mutex_init(&dev_priv->pps_mutex);
 
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
@@ -1027,6 +1028,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_init_dpio(dev_priv);
 	intel_power_domains_init(dev_priv);
 	intel_irq_init(dev_priv);
+	intel_init_display_hooks(dev_priv);
+	intel_init_audio_hooks(dev_priv);
 
 	intel_runtime_pm_get(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 30f9214..fdc8b2a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -564,23 +564,21 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 }
 
 /**
- * intel_init_audio - Set up chip specific audio functions
- * @dev: drm device
+ * intel_init_audio_hooks - Set up chip specific audio hooks
+ * @dev_priv: device private
  */
-void intel_init_audio(struct drm_device *dev)
+void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (IS_G4X(dev)) {
+	if (IS_G4X(dev_priv)) {
 		dev_priv->display.audio_codec_enable = g4x_audio_codec_enable;
 		dev_priv->display.audio_codec_disable = g4x_audio_codec_disable;
-	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.audio_codec_enable = ilk_audio_codec_enable;
 		dev_priv->display.audio_codec_disable = ilk_audio_codec_disable;
-	} else if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) {
+	} else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8) {
 		dev_priv->display.audio_codec_enable = hsw_audio_codec_enable;
 		dev_priv->display.audio_codec_disable = hsw_audio_codec_disable;
-	} else if (HAS_PCH_SPLIT(dev)) {
+	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		dev_priv->display.audio_codec_enable = ilk_audio_codec_enable;
 		dev_priv->display.audio_codec_disable = ilk_audio_codec_disable;
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2d151ad..b7d145c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14829,23 +14829,24 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
 	.atomic_state_clear = intel_atomic_state_clear,
 };
 
-/* Set up chip specific display functions */
-static void intel_init_display(struct drm_device *dev)
+/**
+ * intel_init_display_hooks - initialize the display modesetting hooks
+ * @dev_priv: device private
+ */
+void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (HAS_PCH_SPLIT(dev) || IS_G4X(dev))
+	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
 		dev_priv->display.find_dpll = g4x_find_best_dpll;
-	else if (IS_CHERRYVIEW(dev))
+	else if (IS_CHERRYVIEW(dev_priv))
 		dev_priv->display.find_dpll = chv_find_best_dpll;
-	else if (IS_VALLEYVIEW(dev))
+	else if (IS_VALLEYVIEW(dev_priv))
 		dev_priv->display.find_dpll = vlv_find_best_dpll;
-	else if (IS_PINEVIEW(dev))
+	else if (IS_PINEVIEW(dev_priv))
 		dev_priv->display.find_dpll = pnv_find_best_dpll;
 	else
 		dev_priv->display.find_dpll = i9xx_find_best_dpll;
 
-	if (INTEL_INFO(dev)->gen >= 9) {
+	if (INTEL_INFO(dev_priv)->gen >= 9) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			skylake_get_initial_plane_config;
@@ -14853,7 +14854,7 @@ static void intel_init_display(struct drm_device *dev)
 			haswell_crtc_compute_clock;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
-	} else if (HAS_DDI(dev)) {
+	} else if (HAS_DDI(dev_priv)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			ironlake_get_initial_plane_config;
@@ -14861,7 +14862,7 @@ static void intel_init_display(struct drm_device *dev)
 			haswell_crtc_compute_clock;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
-	} else if (HAS_PCH_SPLIT(dev)) {
+	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			ironlake_get_initial_plane_config;
@@ -14869,7 +14870,7 @@ static void intel_init_display(struct drm_device *dev)
 			ironlake_crtc_compute_clock;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
-	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			i9xx_get_initial_plane_config;
@@ -14886,89 +14887,89 @@ static void intel_init_display(struct drm_device *dev)
 	}
 
 	/* Returns the core display clock speed */
-	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
+	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			skylake_get_display_clock_speed;
-	else if (IS_BROXTON(dev))
+	else if (IS_BROXTON(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			broxton_get_display_clock_speed;
-	else if (IS_BROADWELL(dev))
+	else if (IS_BROADWELL(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			broadwell_get_display_clock_speed;
-	else if (IS_HASWELL(dev))
+	else if (IS_HASWELL(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			haswell_get_display_clock_speed;
-	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
+	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			valleyview_get_display_clock_speed;
-	else if (IS_GEN5(dev))
+	else if (IS_GEN5(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			ilk_get_display_clock_speed;
-	else if (IS_I945G(dev) || IS_BROADWATER(dev) ||
-		 IS_GEN6(dev) || IS_IVYBRIDGE(dev))
+	else if (IS_I945G(dev_priv) || IS_BROADWATER(dev_priv) ||
+		 IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i945_get_display_clock_speed;
-	else if (IS_GM45(dev))
+	else if (IS_GM45(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			gm45_get_display_clock_speed;
-	else if (IS_CRESTLINE(dev))
+	else if (IS_CRESTLINE(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i965gm_get_display_clock_speed;
-	else if (IS_PINEVIEW(dev))
+	else if (IS_PINEVIEW(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			pnv_get_display_clock_speed;
-	else if (IS_G33(dev) || IS_G4X(dev))
+	else if (IS_G33(dev_priv) || IS_G4X(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			g33_get_display_clock_speed;
-	else if (IS_I915G(dev))
+	else if (IS_I915G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915_get_display_clock_speed;
-	else if (IS_I945GM(dev) || IS_845G(dev))
+	else if (IS_I945GM(dev_priv) || IS_845G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i9xx_misc_get_display_clock_speed;
-	else if (IS_I915GM(dev))
+	else if (IS_I915GM(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915gm_get_display_clock_speed;
-	else if (IS_I865G(dev))
+	else if (IS_I865G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i865_get_display_clock_speed;
-	else if (IS_I85X(dev))
+	else if (IS_I85X(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i85x_get_display_clock_speed;
 	else { /* 830 */
-		WARN(!IS_I830(dev), "Unknown platform. Assuming 133 MHz CDCLK\n");
+		WARN(!IS_I830(dev_priv), "Unknown platform. Assuming 133 MHz CDCLK\n");
 		dev_priv->display.get_display_clock_speed =
 			i830_get_display_clock_speed;
 	}
 
-	if (IS_GEN5(dev)) {
+	if (IS_GEN5(dev_priv)) {
 		dev_priv->display.fdi_link_train = ironlake_fdi_link_train;
-	} else if (IS_GEN6(dev)) {
+	} else if (IS_GEN6(dev_priv)) {
 		dev_priv->display.fdi_link_train = gen6_fdi_link_train;
-	} else if (IS_IVYBRIDGE(dev)) {
+	} else if (IS_IVYBRIDGE(dev_priv)) {
 		/* FIXME: detect B0+ stepping and use auto training */
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
-	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
-		if (IS_BROADWELL(dev)) {
+		if (IS_BROADWELL(dev_priv)) {
 			dev_priv->display.modeset_commit_cdclk =
 				broadwell_modeset_commit_cdclk;
 			dev_priv->display.modeset_calc_cdclk =
 				broadwell_modeset_calc_cdclk;
 		}
-	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.modeset_commit_cdclk =
 			valleyview_modeset_commit_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			valleyview_modeset_calc_cdclk;
-	} else if (IS_BROXTON(dev)) {
+	} else if (IS_BROXTON(dev_priv)) {
 		dev_priv->display.modeset_commit_cdclk =
 			broxton_modeset_commit_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			broxton_modeset_calc_cdclk;
 	}
 
-	switch (INTEL_INFO(dev)->gen) {
+	switch (INTEL_INFO(dev_priv)->gen) {
 	case 2:
 		dev_priv->display.queue_flip = intel_gen2_queue_flip;
 		break;
@@ -14995,8 +14996,6 @@ static void intel_init_display(struct drm_device *dev)
 		/* Default just returns -ENODEV to indicate unsupported */
 		dev_priv->display.queue_flip = intel_default_queue_flip;
 	}
-
-	mutex_init(&dev_priv->pps_mutex);
 }
 
 /*
@@ -15323,9 +15322,6 @@ void intel_modeset_init(struct drm_device *dev)
 		}
 	}
 
-	intel_init_display(dev);
-	intel_init_audio(dev);
-
 	if (IS_GEN2(dev)) {
 		dev->mode_config.max_width = 2048;
 		dev->mode_config.max_height = 2048;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 02b3d22..d0f05a8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1109,7 +1109,7 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
 			      uint64_t fb_modifier, uint32_t pixel_format);
 
 /* intel_audio.c */
-void intel_init_audio(struct drm_device *dev);
+void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
 void intel_audio_codec_enable(struct intel_encoder *encoder);
 void intel_audio_codec_disable(struct intel_encoder *encoder);
 void i915_audio_component_init(struct drm_i915_private *dev_priv);
@@ -1117,6 +1117,7 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
 
 /* intel_display.c */
 extern const struct drm_plane_funcs intel_plane_funcs;
+void intel_init_display_hooks(struct drm_i915_private *dev_priv);
 unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 05/17] drm/i915: Move load time init of clock gating hooks earlier

[Imre Deak]

Split out the part initing the clock gating hooks and move it earlier.
Add a new NOP hook for platforms without the need to apply clockgating
or workaround settings, so that the hook can be called unconditionally.
Also add a WARN for future platforms that forget to add a hook.

The rest of the hooks in intel_init_pm() should be inited in the same
way, but atm some of the hooks are set only conditionally, so before
doing this we need to make the setup unconditional and use instead some
flags.

v2:
- add a NOP hook and WARN if no hook is set for the platform (Chris)
- use the term hook instead of callback for these functions (Jani)

CC: Jani Nikula <jani.nikula@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 93 +++++++++++++++++++++++++---------------
 3 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44f15de..60e8fe6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1029,6 +1029,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_power_domains_init(dev_priv);
 	intel_irq_init(dev_priv);
 	intel_init_display_hooks(dev_priv);
+	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
 
 	intel_runtime_pm_get(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0f05a8..7a642ae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1587,6 +1587,7 @@ void intel_suspend_hw(struct drm_device *dev);
 int ilk_wm_max_level(const struct drm_device *dev);
 void intel_update_watermarks(struct drm_crtc *crtc);
 void intel_init_pm(struct drm_device *dev);
+void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv);
 void intel_pm_setup(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d7aef17..49366da 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7097,8 +7097,7 @@ void intel_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->display.init_clock_gating)
-		dev_priv->display.init_clock_gating(dev);
+	dev_priv->display.init_clock_gating(dev);
 }
 
 void intel_suspend_hw(struct drm_device *dev)
@@ -7107,6 +7106,63 @@ void intel_suspend_hw(struct drm_device *dev)
 		lpt_suspend_hw(dev);
 }
 
+static void nop_init_clock_gating(struct drm_device *dev)
+{
+	DRM_DEBUG_KMS("No clock gating settings or workarounds applied.\n");
+}
+
+/**
+ * intel_init_clock_gating_hooks - setup the clock gating hooks
+ * @dev_priv: device private
+ *
+ * Setup the hooks that configure which clocks of a given platform can be
+ * gated and also apply various GT and display specific workarounds for these
+ * platforms. Note that some GT specific workarounds are applied separately
+ * when GPU contexts or batchbuffers start their execution.
+ */
+void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
+{
+	if (IS_SKYLAKE(dev_priv))
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	else if (IS_KABYLAKE(dev_priv))
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	else if (IS_BROXTON(dev_priv))
+		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
+	else if (IS_BROADWELL(dev_priv))
+		dev_priv->display.init_clock_gating = broadwell_init_clock_gating;
+	else if (IS_CHERRYVIEW(dev_priv))
+		dev_priv->display.init_clock_gating = cherryview_init_clock_gating;
+	else if (IS_HASWELL(dev_priv))
+		dev_priv->display.init_clock_gating = haswell_init_clock_gating;
+	else if (IS_IVYBRIDGE(dev_priv))
+		dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
+	else if (IS_VALLEYVIEW(dev_priv))
+		dev_priv->display.init_clock_gating = valleyview_init_clock_gating;
+	else if (IS_GEN6(dev_priv))
+		dev_priv->display.init_clock_gating = gen6_init_clock_gating;
+	else if (IS_GEN5(dev_priv))
+		dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
+	else if (IS_G4X(dev_priv))
+		dev_priv->display.init_clock_gating = g4x_init_clock_gating;
+	else if (IS_CRESTLINE(dev_priv))
+		dev_priv->display.init_clock_gating = crestline_init_clock_gating;
+	else if (IS_BROADWATER(dev_priv))
+		dev_priv->display.init_clock_gating = broadwater_init_clock_gating;
+	else if (IS_GEN4(dev_priv))
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	else if (IS_GEN3(dev_priv))
+		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
+	else if (IS_I85X(dev_priv) || IS_I865G(dev_priv))
+		dev_priv->display.init_clock_gating = i85x_init_clock_gating;
+	else if (IS_GEN2(dev_priv))
+		dev_priv->display.init_clock_gating = i830_init_clock_gating;
+	else {
+		MISSING_CASE(INTEL_DEVID(dev_priv));
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	}
+
+}
+
 /* Set up chip specific power management-related functions */
 void intel_init_pm(struct drm_device *dev)
 {
@@ -7123,10 +7179,6 @@ void intel_init_pm(struct drm_device *dev)
 	/* For FIFO watermark updates */
 	if (INTEL_INFO(dev)->gen >= 9) {
 		skl_setup_wm_latency(dev);
-
-		if (IS_BROXTON(dev))
-			dev_priv->display.init_clock_gating =
-				bxt_init_clock_gating;
 		dev_priv->display.update_wm = skl_update_wm;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		ilk_setup_wm_latency(dev);
@@ -7146,29 +7198,12 @@ void intel_init_pm(struct drm_device *dev)
 			DRM_DEBUG_KMS("Failed to read display plane latency. "
 				      "Disable CxSR\n");
 		}
-
-		if (IS_GEN5(dev))
-			dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
-		else if (IS_GEN6(dev))
-			dev_priv->display.init_clock_gating = gen6_init_clock_gating;
-		else if (IS_IVYBRIDGE(dev))
-			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
-		else if (IS_HASWELL(dev))
-			dev_priv->display.init_clock_gating = haswell_init_clock_gating;
-		else if (INTEL_INFO(dev)->gen == 8)
-			dev_priv->display.init_clock_gating = broadwell_init_clock_gating;
 	} else if (IS_CHERRYVIEW(dev)) {
 		vlv_setup_wm_latency(dev);
-
 		dev_priv->display.update_wm = vlv_update_wm;
-		dev_priv->display.init_clock_gating =
-			cherryview_init_clock_gating;
 	} else if (IS_VALLEYVIEW(dev)) {
 		vlv_setup_wm_latency(dev);
-
 		dev_priv->display.update_wm = vlv_update_wm;
-		dev_priv->display.init_clock_gating =
-			valleyview_init_clock_gating;
 	} else if (IS_PINEVIEW(dev)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
 					    dev_priv->is_ddr3,
@@ -7184,20 +7219,13 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.update_wm = NULL;
 		} else
 			dev_priv->display.update_wm = pineview_update_wm;
-		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.update_wm = g4x_update_wm;
-		dev_priv->display.init_clock_gating = g4x_init_clock_gating;
 	} else if (IS_GEN4(dev)) {
 		dev_priv->display.update_wm = i965_update_wm;
-		if (IS_CRESTLINE(dev))
-			dev_priv->display.init_clock_gating = crestline_init_clock_gating;
-		else if (IS_BROADWATER(dev))
-			dev_priv->display.init_clock_gating = broadwater_init_clock_gating;
 	} else if (IS_GEN3(dev)) {
 		dev_priv->display.update_wm = i9xx_update_wm;
 		dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
-		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
 	} else if (IS_GEN2(dev)) {
 		if (INTEL_INFO(dev)->num_pipes == 1) {
 			dev_priv->display.update_wm = i845_update_wm;
@@ -7206,11 +7234,6 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.update_wm = i9xx_update_wm;
 			dev_priv->display.get_fifo_size = i830_get_fifo_size;
 		}
-
-		if (IS_I85X(dev) || IS_I865G(dev))
-			dev_priv->display.init_clock_gating = i85x_init_clock_gating;
-		else
-			dev_priv->display.init_clock_gating = i830_init_clock_gating;
 	} else {
 		DRM_ERROR("unexpected fall-through in intel_init_pm\n");
 	}
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/17] drm/i915: Move load time init of clock gating hooks earlier

[Chris Wilson]

On Fri, Mar 11, 2016 at 06:31:30PM +0200, Imre Deak wrote:
> +	else if (IS_G4X(dev_priv))
> +		dev_priv->display.init_clock_gating = g4x_init_clock_gating;
> +	else if (IS_CRESTLINE(dev_priv))
> +		dev_priv->display.init_clock_gating = crestline_init_clock_gating;
> +	else if (IS_BROADWATER(dev_priv))
> +		dev_priv->display.init_clock_gating = broadwater_init_clock_gating;
> +	else if (IS_GEN4(dev_priv))
> +		dev_priv->display.init_clock_gating = nop_init_clock_gating;

G4X || CRESTLINE || BROADWATER should cover all of GEN4.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/17] drm/i915: Move load time init of clock gating hooks earlier

[Imre Deak]

On Fri, 2016-03-11 at 19:57 +0000, Chris Wilson wrote:
> On Fri, Mar 11, 2016 at 06:31:30PM +0200, Imre Deak wrote:
> > +	else if (IS_G4X(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > g4x_init_clock_gating;
> > +	else if (IS_CRESTLINE(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > crestline_init_clock_gating;
> > +	else if (IS_BROADWATER(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > broadwater_init_clock_gating;
> > +	else if (IS_GEN4(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > nop_init_clock_gating;
> 
> G4X || CRESTLINE || BROADWATER should cover all of GEN4.

Yep, I missed this. Will remove the GEN4 check.

> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915: Move load time init of clock gating hooks earlier

[Imre Deak]

Split out the part initing the clock gating hooks and move it earlier.
Add a new NOP hook for platforms without the need to apply clockgating
or workaround settings, so that the hook can be called unconditionally.
Also add a WARN for future platforms that forget to add a hook.

The rest of the hooks in intel_init_pm() should be inited in the same
way, but atm some of the hooks are set only conditionally, so before
doing this we need to make the setup unconditional and use instead some
flags.

v2:
- add a NOP hook and WARN if no hook is set for the platform (Chris)
- use the term hook instead of callback for these functions (Jani)
v3:
- remove the GEN4() check it's already covered by earlier platform
  checks (Chris)

CC: Jani Nikula <jani.nikula@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 91 ++++++++++++++++++++++++----------------
 3 files changed, 58 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44f15de..60e8fe6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1029,6 +1029,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_power_domains_init(dev_priv);
 	intel_irq_init(dev_priv);
 	intel_init_display_hooks(dev_priv);
+	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
 
 	intel_runtime_pm_get(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0f05a8..7a642ae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1587,6 +1587,7 @@ void intel_suspend_hw(struct drm_device *dev);
 int ilk_wm_max_level(const struct drm_device *dev);
 void intel_update_watermarks(struct drm_crtc *crtc);
 void intel_init_pm(struct drm_device *dev);
+void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv);
 void intel_pm_setup(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d7aef17..4f4b402 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7097,8 +7097,7 @@ void intel_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->display.init_clock_gating)
-		dev_priv->display.init_clock_gating(dev);
+	dev_priv->display.init_clock_gating(dev);
 }
 
 void intel_suspend_hw(struct drm_device *dev)
@@ -7107,6 +7106,61 @@ void intel_suspend_hw(struct drm_device *dev)
 		lpt_suspend_hw(dev);
 }
 
+static void nop_init_clock_gating(struct drm_device *dev)
+{
+	DRM_DEBUG_KMS("No clock gating settings or workarounds applied.\n");
+}
+
+/**
+ * intel_init_clock_gating_hooks - setup the clock gating hooks
+ * @dev_priv: device private
+ *
+ * Setup the hooks that configure which clocks of a given platform can be
+ * gated and also apply various GT and display specific workarounds for these
+ * platforms. Note that some GT specific workarounds are applied separately
+ * when GPU contexts or batchbuffers start their execution.
+ */
+void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
+{
+	if (IS_SKYLAKE(dev_priv))
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	else if (IS_KABYLAKE(dev_priv))
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	else if (IS_BROXTON(dev_priv))
+		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
+	else if (IS_BROADWELL(dev_priv))
+		dev_priv->display.init_clock_gating = broadwell_init_clock_gating;
+	else if (IS_CHERRYVIEW(dev_priv))
+		dev_priv->display.init_clock_gating = cherryview_init_clock_gating;
+	else if (IS_HASWELL(dev_priv))
+		dev_priv->display.init_clock_gating = haswell_init_clock_gating;
+	else if (IS_IVYBRIDGE(dev_priv))
+		dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
+	else if (IS_VALLEYVIEW(dev_priv))
+		dev_priv->display.init_clock_gating = valleyview_init_clock_gating;
+	else if (IS_GEN6(dev_priv))
+		dev_priv->display.init_clock_gating = gen6_init_clock_gating;
+	else if (IS_GEN5(dev_priv))
+		dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
+	else if (IS_G4X(dev_priv))
+		dev_priv->display.init_clock_gating = g4x_init_clock_gating;
+	else if (IS_CRESTLINE(dev_priv))
+		dev_priv->display.init_clock_gating = crestline_init_clock_gating;
+	else if (IS_BROADWATER(dev_priv))
+		dev_priv->display.init_clock_gating = broadwater_init_clock_gating;
+	else if (IS_GEN3(dev_priv))
+		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
+	else if (IS_I85X(dev_priv) || IS_I865G(dev_priv))
+		dev_priv->display.init_clock_gating = i85x_init_clock_gating;
+	else if (IS_GEN2(dev_priv))
+		dev_priv->display.init_clock_gating = i830_init_clock_gating;
+	else {
+		MISSING_CASE(INTEL_DEVID(dev_priv));
+		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+	}
+
+}
+
 /* Set up chip specific power management-related functions */
 void intel_init_pm(struct drm_device *dev)
 {
@@ -7123,10 +7177,6 @@ void intel_init_pm(struct drm_device *dev)
 	/* For FIFO watermark updates */
 	if (INTEL_INFO(dev)->gen >= 9) {
 		skl_setup_wm_latency(dev);
-
-		if (IS_BROXTON(dev))
-			dev_priv->display.init_clock_gating =
-				bxt_init_clock_gating;
 		dev_priv->display.update_wm = skl_update_wm;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		ilk_setup_wm_latency(dev);
@@ -7146,29 +7196,12 @@ void intel_init_pm(struct drm_device *dev)
 			DRM_DEBUG_KMS("Failed to read display plane latency. "
 				      "Disable CxSR\n");
 		}
-
-		if (IS_GEN5(dev))
-			dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
-		else if (IS_GEN6(dev))
-			dev_priv->display.init_clock_gating = gen6_init_clock_gating;
-		else if (IS_IVYBRIDGE(dev))
-			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
-		else if (IS_HASWELL(dev))
-			dev_priv->display.init_clock_gating = haswell_init_clock_gating;
-		else if (INTEL_INFO(dev)->gen == 8)
-			dev_priv->display.init_clock_gating = broadwell_init_clock_gating;
 	} else if (IS_CHERRYVIEW(dev)) {
 		vlv_setup_wm_latency(dev);
-
 		dev_priv->display.update_wm = vlv_update_wm;
-		dev_priv->display.init_clock_gating =
-			cherryview_init_clock_gating;
 	} else if (IS_VALLEYVIEW(dev)) {
 		vlv_setup_wm_latency(dev);
-
 		dev_priv->display.update_wm = vlv_update_wm;
-		dev_priv->display.init_clock_gating =
-			valleyview_init_clock_gating;
 	} else if (IS_PINEVIEW(dev)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
 					    dev_priv->is_ddr3,
@@ -7184,20 +7217,13 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.update_wm = NULL;
 		} else
 			dev_priv->display.update_wm = pineview_update_wm;
-		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.update_wm = g4x_update_wm;
-		dev_priv->display.init_clock_gating = g4x_init_clock_gating;
 	} else if (IS_GEN4(dev)) {
 		dev_priv->display.update_wm = i965_update_wm;
-		if (IS_CRESTLINE(dev))
-			dev_priv->display.init_clock_gating = crestline_init_clock_gating;
-		else if (IS_BROADWATER(dev))
-			dev_priv->display.init_clock_gating = broadwater_init_clock_gating;
 	} else if (IS_GEN3(dev)) {
 		dev_priv->display.update_wm = i9xx_update_wm;
 		dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
-		dev_priv->display.init_clock_gating = gen3_init_clock_gating;
 	} else if (IS_GEN2(dev)) {
 		if (INTEL_INFO(dev)->num_pipes == 1) {
 			dev_priv->display.update_wm = i845_update_wm;
@@ -7206,11 +7232,6 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.update_wm = i9xx_update_wm;
 			dev_priv->display.get_fifo_size = i830_get_fifo_size;
 		}
-
-		if (IS_I85X(dev) || IS_I865G(dev))
-			dev_priv->display.init_clock_gating = i85x_init_clock_gating;
-		else
-			dev_priv->display.init_clock_gating = i830_init_clock_gating;
 	} else {
 		DRM_ERROR("unexpected fall-through in intel_init_pm\n");
 	}
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 06/17] drm/i915: Move load time runtime device info init earlier

[Imre Deak]

This init step accesses the device, but doesn't have any device
specific side effect. It also sets up some platform specific
attributes that may be required early, so move it earlier.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 60e8fe6..da96ccd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1057,6 +1057,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_uncore_init(dev);
 
+	intel_device_info_runtime_init(dev);
+
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
 		goto out_uncore_fini;
@@ -1131,8 +1133,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 			DRM_DEBUG_DRIVER("can't enable MSI");
 	}
 
-	intel_device_info_runtime_init(dev);
-
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 07/17] drm/i915: Move load time gem_load_init earlier

[Imre Deak]

The only steps requiring device access is the fence and swizzling
initialization, so split these out keeping them in their current place
and move the rest of init steps earlier.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 13 ++++++++-----
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++++++--------------
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index da96ccd..9ada500 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1031,6 +1031,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
+	i915_gem_load_init(dev);
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -1114,7 +1115,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_opregion_setup(dev);
 
-	i915_gem_load_init(dev);
+	i915_gem_load_init_fences(dev_priv);
+	i915_gem_detect_bit_6_swizzle(dev);
+
 	i915_gem_shrinker_init(dev_priv);
 
 	/* On the 945G/GM, the chipset reports the MSI capability on the
@@ -1136,7 +1139,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
-			goto out_gem_unload;
+			goto out_cleanup_shrinker;
 	}
 
 	ret = i915_load_modeset_init(dev);
@@ -1174,7 +1177,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 out_power_well:
 	intel_power_domains_fini(dev_priv);
 	drm_vblank_cleanup(dev);
-out_gem_unload:
+out_cleanup_shrinker:
 	i915_gem_shrinker_cleanup(dev_priv);
 
 	if (dev->pdev->msi_enabled)
@@ -1190,9 +1193,9 @@ out_uncore_fini:
 	i915_mmio_cleanup(dev);
 put_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
-	i915_gem_load_cleanup(dev);
 out_runtime_pm_put:
 	intel_runtime_pm_put(dev_priv);
+	i915_gem_load_cleanup(dev);
 	i915_workqueues_cleanup(dev_priv);
 out_free_priv:
 	kfree(dev_priv);
@@ -1277,8 +1280,8 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_uncore_fini(dev);
 	i915_mmio_cleanup(dev);
 
-	i915_gem_load_cleanup(dev);
 	pci_dev_put(dev_priv->bridge_dev);
+	i915_gem_load_cleanup(dev);
 	i915_workqueues_cleanup(dev_priv);
 	kfree(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1557d65..f62b6d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2836,6 +2836,7 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
+void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
 void *i915_gem_object_alloc(struct drm_device *dev);
 void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b854af2..3dab0d6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5029,6 +5029,26 @@ init_ring_lists(struct intel_engine_cs *ring)
 }
 
 void
+i915_gem_load_init_fences(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv) &&
+	    !IS_CHERRYVIEW(dev_priv))
+		dev_priv->num_fence_regs = 32;
+	else if (INTEL_INFO(dev_priv)->gen >= 4 || IS_I945G(dev_priv) ||
+		 IS_I945GM(dev_priv) || IS_G33(dev_priv))
+		dev_priv->num_fence_regs = 16;
+	else
+		dev_priv->num_fence_regs = 8;
+
+	if (intel_vgpu_active(dev_priv->dev))
+		dev_priv->num_fence_regs =
+				I915_READ(vgtif_reg(avail_rs.fence_num));
+
+	/* Initialize fence registers to zero */
+	i915_gem_restore_fences(dev_priv->dev);
+}
+
+void
 i915_gem_load_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5067,17 +5087,6 @@ i915_gem_load_init(struct drm_device *dev)
 
 	dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL;
 
-	if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev))
-		dev_priv->num_fence_regs = 32;
-	else if (INTEL_INFO(dev)->gen >= 4 || IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
-		dev_priv->num_fence_regs = 16;
-	else
-		dev_priv->num_fence_regs = 8;
-
-	if (intel_vgpu_active(dev))
-		dev_priv->num_fence_regs =
-				I915_READ(vgtif_reg(avail_rs.fence_num));
-
 	/*
 	 * Set initial sequence number for requests.
 	 * Using this number allows the wraparound to happen early,
@@ -5086,11 +5095,8 @@ i915_gem_load_init(struct drm_device *dev)
 	dev_priv->next_seqno = ((u32)~0 - 0x1100);
 	dev_priv->last_seqno = ((u32)~0 - 0x1101);
 
-	/* Initialize fence registers to zero */
 	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
-	i915_gem_restore_fences(dev);
 
-	i915_gem_detect_bit_6_swizzle(dev);
 	init_waitqueue_head(&dev_priv->pending_flip_queue);
 
 	dev_priv->mm.interruptible = true;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 08/17] drm/i915: Move load time runtime PM get later

[Imre Deak]

We require the device to be powered only before accessing it, so we can
move this call later.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9ada500..93b9839 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1033,8 +1033,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_init_audio_hooks(dev_priv);
 	i915_gem_load_init(dev);
 
-	intel_runtime_pm_get(dev_priv);
-
 	intel_display_crc_init(dev);
 
 	i915_dump_device_info(dev_priv);
@@ -1047,6 +1045,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		DRM_INFO("This is an early pre-production Haswell machine. "
 			 "It may not be fully functional.\n");
 
+	intel_runtime_pm_get(dev_priv);
+
 	if (i915_get_bridge_dev(dev)) {
 		ret = -EIO;
 		goto out_runtime_pm_put;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 09/17] drm/i915: Move load time shrinker registration later

[Imre Deak]

According to the new init phases scheme we should register the driver
with frameworks/userspace only one the device is setup fully. So move
the shrinker registration later accordingly.

Also fix the shrinker unregistration order wrt. the acpi unregistration
to fix the corresponding init order.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 93b9839..346ed8e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1118,8 +1118,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	i915_gem_load_init_fences(dev_priv);
 	i915_gem_detect_bit_6_swizzle(dev);
 
-	i915_gem_shrinker_init(dev_priv);
-
 	/* On the 945G/GM, the chipset reports the MSI capability on the
 	 * integrated graphics even though the support isn't actually there
 	 * according to the published specs.  It doesn't appear to function
@@ -1139,7 +1137,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
-			goto out_cleanup_shrinker;
+			goto out_disable_msi;
 	}
 
 	ret = i915_load_modeset_init(dev);
@@ -1148,6 +1146,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto out_power_well;
 	}
 
+	i915_gem_shrinker_init(dev_priv);
 	/*
 	 * Notify a valid surface after modesetting,
 	 * when running inside a VM.
@@ -1177,9 +1176,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 out_power_well:
 	intel_power_domains_fini(dev_priv);
 	drm_vblank_cleanup(dev);
-out_cleanup_shrinker:
-	i915_gem_shrinker_cleanup(dev_priv);
-
+out_disable_msi:
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
@@ -1224,12 +1221,11 @@ int i915_driver_unload(struct drm_device *dev)
 
 	i915_teardown_sysfs(dev);
 
-	i915_gem_shrinker_cleanup(dev_priv);
-
 	io_mapping_free(dev_priv->gtt.mappable);
 	arch_phys_wc_del(dev_priv->gtt.mtrr);
 
 	acpi_video_unregister();
+	i915_gem_shrinker_cleanup(dev_priv);
 
 	drm_vblank_cleanup(dev);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 10/17] drm/i915: Move load time audio component registration earlier

[Imre Deak]

We should register all the interfaces before we enable runtime PM.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 346ed8e..da1bea8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1165,10 +1165,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (IS_GEN5(dev))
 		intel_gpu_ips_init(dev_priv);
 
-	intel_runtime_pm_enable(dev_priv);
-
 	i915_audio_component_init(dev_priv);
 
+	intel_runtime_pm_enable(dev_priv);
+
 	intel_runtime_pm_put(dev_priv);
 
 	return 0;
@@ -1207,8 +1207,6 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_fbdev_fini(dev);
 
-	i915_audio_component_cleanup(dev_priv);
-
 	ret = i915_gem_suspend(dev);
 	if (ret) {
 		DRM_ERROR("failed to idle hardware: %d\n", ret);
@@ -1217,6 +1215,8 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_power_domains_fini(dev_priv);
 
+	i915_audio_component_cleanup(dev_priv);
+
 	intel_gpu_ips_teardown();
 
 	i915_teardown_sysfs(dev);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 11/17] drm/i915: Move unload time display power domain uninit later

[Imre Deak]

Move the power domain uninitialization later so that it matches its
corresponding init order. Since we access the HW during the later
unitialization steps keep a wake reference until after the last such
step.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index da1bea8..b50d111 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1213,7 +1213,7 @@ int i915_driver_unload(struct drm_device *dev)
 		return ret;
 	}
 
-	intel_power_domains_fini(dev_priv);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
 	i915_audio_component_cleanup(dev_priv);
 
@@ -1269,6 +1269,8 @@ int i915_driver_unload(struct drm_device *dev)
 	mutex_unlock(&dev->struct_mutex);
 	intel_fbc_cleanup_cfb(dev_priv);
 
+	intel_power_domains_fini(dev_priv);
+
 	pm_qos_remove_request(&dev_priv->pm_qos);
 
 	i915_global_gtt_cleanup(dev);
@@ -1277,6 +1279,9 @@ int i915_driver_unload(struct drm_device *dev)
 	i915_mmio_cleanup(dev);
 
 	pci_dev_put(dev_priv->bridge_dev);
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
 	i915_gem_load_cleanup(dev);
 	i915_workqueues_cleanup(dev_priv);
 	kfree(dev_priv);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 12/17] drm/i915: Move unload time GTT, MSI IRQ cleanup later

[Imre Deak]

Move the GTT,MSI IRQ cleanup later so that it matches their
corresponding init order. Also fix the order of these calls wrt. each
other to match their corresponding init order.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b50d111..7ab8ac5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1221,9 +1221,6 @@ int i915_driver_unload(struct drm_device *dev)
 
 	i915_teardown_sysfs(dev);
 
-	io_mapping_free(dev_priv->gtt.mappable);
-	arch_phys_wc_del(dev_priv->gtt.mtrr);
-
 	acpi_video_unregister();
 	i915_gem_shrinker_cleanup(dev_priv);
 
@@ -1254,9 +1251,6 @@ int i915_driver_unload(struct drm_device *dev)
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	i915_destroy_error_state(dev);
 
-	if (dev->pdev->msi_enabled)
-		pci_disable_msi(dev->pdev);
-
 	intel_opregion_fini(dev);
 
 	/* Flush any outstanding unpin_work. */
@@ -1271,8 +1265,11 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_power_domains_fini(dev_priv);
 
+	if (dev->pdev->msi_enabled)
+		pci_disable_msi(dev->pdev);
 	pm_qos_remove_request(&dev_priv->pm_qos);
-
+	arch_phys_wc_del(dev_priv->gtt.mtrr);
+	io_mapping_free(dev_priv->gtt.mappable);
 	i915_global_gtt_cleanup(dev);
 
 	intel_uncore_fini(dev);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 13/17] drm/i915: Move unload time opregion unregistration earlier

[Imre Deak]

Move the opregion unregistration earlier to match its corresponding
registration order.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7ab8ac5..e2465c6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1222,6 +1222,7 @@ int i915_driver_unload(struct drm_device *dev)
 	i915_teardown_sysfs(dev);
 
 	acpi_video_unregister();
+	intel_opregion_fini(dev);
 	i915_gem_shrinker_cleanup(dev_priv);
 
 	drm_vblank_cleanup(dev);
@@ -1251,8 +1252,6 @@ int i915_driver_unload(struct drm_device *dev)
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	i915_destroy_error_state(dev);
 
-	intel_opregion_fini(dev);
-
 	/* Flush any outstanding unpin_work. */
 	flush_workqueue(dev_priv->wq);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 14/17] drm/i915: Split out load time early initialization

[Imre Deak]

According to the new init phases scheme we should initialize "SW-only"
state not requiring accessing the device as the very first step, so that
the reasoning about dependencies of later steps becomes easier. So move
these init steps into a separate function. This also has the benefit of
making the error path cleaner both in the new function and int
i915_driver_load()/unload().

No functional change.

Suggested by Chris.

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 130 +++++++++++++++++++++++++---------------
 1 file changed, 81 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e2465c6..93e7951 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -933,6 +933,83 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
 	destroy_workqueue(dev_priv->wq);
 }
 
+/**
+ * i915_driver_init_early - setup state not requiring device access
+ * @dev_priv: device private
+ *
+ * Initialize everything that is a "SW-only" state, that is state not
+ * requiring accessing the device or exposing the driver via kernel internal
+ * or userspace interfaces. Example steps belonging here: lock initialization,
+ * system memory allocation, setting up device specific attributes and
+ * function hooks not requiring accessing the device.
+ */
+static int i915_driver_init_early(struct drm_i915_private *dev_priv,
+				  struct drm_device *dev,
+				  struct intel_device_info *info)
+{
+	struct intel_device_info *device_info;
+	int ret = 0;
+
+	dev_priv->dev = dev;
+
+	/* Setup the write-once "constant" device info */
+	device_info = (struct intel_device_info *)&dev_priv->info;
+	memcpy(device_info, info, sizeof(dev_priv->info));
+	device_info->device_id = dev->pdev->device;
+
+	spin_lock_init(&dev_priv->irq_lock);
+	spin_lock_init(&dev_priv->gpu_error.lock);
+	mutex_init(&dev_priv->backlight_lock);
+	spin_lock_init(&dev_priv->uncore.lock);
+	spin_lock_init(&dev_priv->mm.object_stat_lock);
+	spin_lock_init(&dev_priv->mmio_flip_lock);
+	mutex_init(&dev_priv->sb_lock);
+	mutex_init(&dev_priv->modeset_restore_lock);
+	mutex_init(&dev_priv->av_mutex);
+	mutex_init(&dev_priv->wm.wm_mutex);
+	mutex_init(&dev_priv->pps_mutex);
+
+	ret = i915_workqueues_init(dev_priv);
+	if (ret < 0)
+		return ret;
+
+	/* This must be called before any calls to HAS_PCH_* */
+	intel_detect_pch(dev);
+
+	intel_pm_setup(dev);
+	intel_init_dpio(dev_priv);
+	intel_power_domains_init(dev_priv);
+	intel_irq_init(dev_priv);
+	intel_init_display_hooks(dev_priv);
+	intel_init_clock_gating_hooks(dev_priv);
+	intel_init_audio_hooks(dev_priv);
+	i915_gem_load_init(dev);
+
+	intel_display_crc_init(dev);
+
+	i915_dump_device_info(dev_priv);
+
+	/* Not all pre-production machines fall into this category, only the
+	 * very first ones. Almost everything should work, except for maybe
+	 * suspend/resume. And we don't implement workarounds that affect only
+	 * pre-production machines. */
+	if (IS_HSW_EARLY_SDV(dev))
+		DRM_INFO("This is an early pre-production Haswell machine. "
+			 "It may not be fully functional.\n");
+
+	return 0;
+}
+
+/**
+ * i915_driver_cleanup_early - cleanup the setup done in i915_driver_init_early()
+ * @dev_priv: device private
+ */
+static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
+{
+	i915_gem_load_cleanup(dev_priv->dev);
+	i915_workqueues_cleanup(dev_priv);
+}
+
 static int i915_mmio_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -987,64 +1064,21 @@ static void i915_mmio_cleanup(struct drm_device *dev)
 int i915_driver_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_i915_private *dev_priv;
-	struct intel_device_info *info, *device_info;
 	int ret = 0;
 	uint32_t aperture_size;
 
-	info = (struct intel_device_info *) flags;
-
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
 	if (dev_priv == NULL)
 		return -ENOMEM;
 
 	dev->dev_private = dev_priv;
-	dev_priv->dev = dev;
 
-	/* Setup the write-once "constant" device info */
-	device_info = (struct intel_device_info *)&dev_priv->info;
-	memcpy(device_info, info, sizeof(dev_priv->info));
-	device_info->device_id = dev->pdev->device;
+	ret = i915_driver_init_early(dev_priv, dev,
+				     (struct intel_device_info *)flags);
 
-	spin_lock_init(&dev_priv->irq_lock);
-	spin_lock_init(&dev_priv->gpu_error.lock);
-	mutex_init(&dev_priv->backlight_lock);
-	spin_lock_init(&dev_priv->uncore.lock);
-	spin_lock_init(&dev_priv->mm.object_stat_lock);
-	spin_lock_init(&dev_priv->mmio_flip_lock);
-	mutex_init(&dev_priv->sb_lock);
-	mutex_init(&dev_priv->modeset_restore_lock);
-	mutex_init(&dev_priv->av_mutex);
-	mutex_init(&dev_priv->wm.wm_mutex);
-	mutex_init(&dev_priv->pps_mutex);
-
-	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
 		goto out_free_priv;
 
-	/* This must be called before any calls to HAS_PCH_* */
-	intel_detect_pch(dev);
-
-	intel_pm_setup(dev);
-	intel_init_dpio(dev_priv);
-	intel_power_domains_init(dev_priv);
-	intel_irq_init(dev_priv);
-	intel_init_display_hooks(dev_priv);
-	intel_init_clock_gating_hooks(dev_priv);
-	intel_init_audio_hooks(dev_priv);
-	i915_gem_load_init(dev);
-
-	intel_display_crc_init(dev);
-
-	i915_dump_device_info(dev_priv);
-
-	/* Not all pre-production machines fall into this category, only the
-	 * very first ones. Almost everything should work, except for maybe
-	 * suspend/resume. And we don't implement workarounds that affect only
-	 * pre-production machines. */
-	if (IS_HSW_EARLY_SDV(dev))
-		DRM_INFO("This is an early pre-production Haswell machine. "
-			 "It may not be fully functional.\n");
-
 	intel_runtime_pm_get(dev_priv);
 
 	if (i915_get_bridge_dev(dev)) {
@@ -1192,8 +1226,7 @@ put_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
 out_runtime_pm_put:
 	intel_runtime_pm_put(dev_priv);
-	i915_gem_load_cleanup(dev);
-	i915_workqueues_cleanup(dev_priv);
+	i915_driver_cleanup_early(dev_priv);
 out_free_priv:
 	kfree(dev_priv);
 
@@ -1278,8 +1311,7 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
-	i915_gem_load_cleanup(dev);
-	i915_workqueues_cleanup(dev_priv);
+	i915_driver_cleanup_early(dev_priv);
 	kfree(dev_priv);
 
 	return 0;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 15/17] drm/i915: Split out load time MMIO initialization

[Imre Deak]

According to the new init phases scheme we should have a definite step
in the init sequence where MMIO access is setup, so move the
corresponding code to a separate function. This also has the benefit of
making the error path cleaner both in the new function and in
i915_driver_load()/unload().

No functional change.

Suggested by Chris.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 69 +++++++++++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 93e7951..ab97404 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1051,6 +1051,50 @@ static void i915_mmio_cleanup(struct drm_device *dev)
 }
 
 /**
+ * i915_driver_init_mmio - setup device MMIO
+ * @dev_priv: device private
+ *
+ * Setup minimal device state necessary for MMIO accesses later in the
+ * initialization sequence. The setup here should avoid any other device-wide
+ * side effects or exposing the driver via kernel internal or user space
+ * interfaces.
+ */
+static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	int ret;
+
+	if (i915_get_bridge_dev(dev))
+		return -EIO;
+
+	ret = i915_mmio_setup(dev);
+	if (ret < 0)
+		goto put_bridge;
+
+	intel_uncore_init(dev);
+
+	return 0;
+
+put_bridge:
+	pci_dev_put(dev_priv->bridge_dev);
+
+	return ret;
+}
+
+/**
+ * i915_driver_cleanup_mmio - cleanup the setup done in i915_driver_init_mmio()
+ * @dev_priv: device private
+ */
+static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	intel_uncore_fini(dev);
+	i915_mmio_cleanup(dev);
+	pci_dev_put(dev_priv->bridge_dev);
+}
+
+/**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
  * @flags: startup flags
@@ -1081,22 +1125,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_runtime_pm_get(dev_priv);
 
-	if (i915_get_bridge_dev(dev)) {
-		ret = -EIO;
-		goto out_runtime_pm_put;
-	}
-
-	ret = i915_mmio_setup(dev);
+	ret = i915_driver_init_mmio(dev_priv);
 	if (ret < 0)
-		goto put_bridge;
-
-	intel_uncore_init(dev);
+		goto out_runtime_pm_put;
 
 	intel_device_info_runtime_init(dev);
 
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
-		goto out_uncore_fini;
+		goto out_cleanup_mmio;
 
 	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over. */
@@ -1219,11 +1256,8 @@ out_disable_msi:
 	io_mapping_free(dev_priv->gtt.mappable);
 out_gtt:
 	i915_global_gtt_cleanup(dev);
-out_uncore_fini:
-	intel_uncore_fini(dev);
-	i915_mmio_cleanup(dev);
-put_bridge:
-	pci_dev_put(dev_priv->bridge_dev);
+out_cleanup_mmio:
+	i915_driver_cleanup_mmio(dev_priv);
 out_runtime_pm_put:
 	intel_runtime_pm_put(dev_priv);
 	i915_driver_cleanup_early(dev_priv);
@@ -1304,10 +1338,7 @@ int i915_driver_unload(struct drm_device *dev)
 	io_mapping_free(dev_priv->gtt.mappable);
 	i915_global_gtt_cleanup(dev);
 
-	intel_uncore_fini(dev);
-	i915_mmio_cleanup(dev);
-
-	pci_dev_put(dev_priv->bridge_dev);
+	i915_driver_cleanup_mmio(dev_priv);
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 16/17] drm/i915: Split out load time HW initialization

[Imre Deak]

According to the new init phases scheme we should have a definite step
in the init sequence where we setup things requiring accessing the
device, so move the corresponding code to separate function. The steps
in this init phase should avoid exposing the driver via some interface,
which is done in the last registration init phase. This changae also
has the benefit of making the error path cleaner both in the new
function and i915_driver_load()/unload().

No functional change.

Suggested by Chris.

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 122 ++++++++++++++++++++++++----------------
 1 file changed, 75 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab97404..aaf1b17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1095,45 +1095,23 @@ static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
 }
 
 /**
- * i915_driver_load - setup chip and create an initial config
- * @dev: DRM device
- * @flags: startup flags
+ * i915_driver_init_hw - setup state requiring device access
+ * @dev_priv: device private
  *
- * The driver load routine has to do several things:
- *   - drive output discovery via intel_modeset_init()
- *   - initialize the memory manager
- *   - allocate initial config memory
- *   - setup the DRM framebuffer with the allocated memory
+ * Setup state that requires accessing the device, but doesn't require
+ * exposing the driver via kernel internal or userspace interfaces.
  */
-int i915_driver_load(struct drm_device *dev, unsigned long flags)
+static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv;
-	int ret = 0;
+	struct drm_device *dev = dev_priv->dev;
 	uint32_t aperture_size;
-
-	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (dev_priv == NULL)
-		return -ENOMEM;
-
-	dev->dev_private = dev_priv;
-
-	ret = i915_driver_init_early(dev_priv, dev,
-				     (struct intel_device_info *)flags);
-
-	if (ret < 0)
-		goto out_free_priv;
-
-	intel_runtime_pm_get(dev_priv);
-
-	ret = i915_driver_init_mmio(dev_priv);
-	if (ret < 0)
-		goto out_runtime_pm_put;
+	int ret;
 
 	intel_device_info_runtime_init(dev);
 
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
-		goto out_cleanup_mmio;
+		return ret;
 
 	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over. */
@@ -1205,10 +1183,73 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 			DRM_DEBUG_DRIVER("can't enable MSI");
 	}
 
+	return 0;
+
+out_gtt:
+	i915_global_gtt_cleanup(dev);
+
+	return ret;
+}
+
+/**
+ * i915_driver_cleanup_hw - cleanup the setup done in i915_driver_init_hw()
+ * @dev_priv: device private
+ */
+static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	if (dev->pdev->msi_enabled)
+		pci_disable_msi(dev->pdev);
+
+	pm_qos_remove_request(&dev_priv->pm_qos);
+	arch_phys_wc_del(dev_priv->gtt.mtrr);
+	io_mapping_free(dev_priv->gtt.mappable);
+	i915_global_gtt_cleanup(dev);
+}
+
+/**
+ * i915_driver_load - setup chip and create an initial config
+ * @dev: DRM device
+ * @flags: startup flags
+ *
+ * The driver load routine has to do several things:
+ *   - drive output discovery via intel_modeset_init()
+ *   - initialize the memory manager
+ *   - allocate initial config memory
+ *   - setup the DRM framebuffer with the allocated memory
+ */
+int i915_driver_load(struct drm_device *dev, unsigned long flags)
+{
+	struct drm_i915_private *dev_priv;
+	int ret = 0;
+
+	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
+	if (dev_priv == NULL)
+		return -ENOMEM;
+
+	dev->dev_private = dev_priv;
+
+	ret = i915_driver_init_early(dev_priv, dev,
+				     (struct intel_device_info *)flags);
+
+	if (ret < 0)
+		goto out_free_priv;
+
+	intel_runtime_pm_get(dev_priv);
+
+	ret = i915_driver_init_mmio(dev_priv);
+	if (ret < 0)
+		goto out_runtime_pm_put;
+
+	ret = i915_driver_init_hw(dev_priv);
+	if (ret < 0)
+		goto out_cleanup_mmio;
+
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
-			goto out_disable_msi;
+			goto out_cleanup_hw;
 	}
 
 	ret = i915_load_modeset_init(dev);
@@ -1247,15 +1288,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 out_power_well:
 	intel_power_domains_fini(dev_priv);
 	drm_vblank_cleanup(dev);
-out_disable_msi:
-	if (dev->pdev->msi_enabled)
-		pci_disable_msi(dev->pdev);
-
-	pm_qos_remove_request(&dev_priv->pm_qos);
-	arch_phys_wc_del(dev_priv->gtt.mtrr);
-	io_mapping_free(dev_priv->gtt.mappable);
-out_gtt:
-	i915_global_gtt_cleanup(dev);
+out_cleanup_hw:
+	i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
 	i915_driver_cleanup_mmio(dev_priv);
 out_runtime_pm_put:
@@ -1331,13 +1365,7 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_power_domains_fini(dev_priv);
 
-	if (dev->pdev->msi_enabled)
-		pci_disable_msi(dev->pdev);
-	pm_qos_remove_request(&dev_priv->pm_qos);
-	arch_phys_wc_del(dev_priv->gtt.mtrr);
-	io_mapping_free(dev_priv->gtt.mappable);
-	i915_global_gtt_cleanup(dev);
-
+	i915_driver_cleanup_hw(dev_priv);
 	i915_driver_cleanup_mmio(dev_priv);
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 17/17] drm/i915: Split out load time interface registration

[Imre Deak]

According to the new init phases scheme we should register the device
making it available via some kernel internal or user space interface as
the last step in the init sequence, so move the corresponding code to a
separate function.

Also add a TODO comment about code that still needs to be moved around
to one of the init phases functions depending on what the role and effect
of that code is.

No functional change, except for the reordering of the unload time
unregistration steps of sysfs wrt. acpi and opregion.

Suggested by Chris.

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index aaf1b17..43dcb5a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * i915_driver_init_register - register the driver with the rest of the system
+ * @dev_priv: device private
+ *
+ * Perform any steps necessary to make the driver available via kernel
+ * internal or userspace interfaces.
+ */
+static void i915_driver_init_register(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	i915_gem_shrinker_init(dev_priv);
+	/*
+	 * Notify a valid surface after modesetting,
+	 * when running inside a VM.
+	 */
+	if (intel_vgpu_active(dev))
+		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
+
+	i915_setup_sysfs(dev);
+
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		/* Must be done after probing outputs */
+		intel_opregion_init(dev);
+		acpi_video_register();
+	}
+
+	if (IS_GEN5(dev_priv))
+		intel_gpu_ips_init(dev_priv);
+
+	i915_audio_component_init(dev_priv);
+}
+
+/**
+ * i915_driver_cleanup_register - undo registration done in i915_driver_init_register()
+ * @dev_priv: device private
+ */
+static void i915_driver_cleanup_register(struct drm_i915_private *dev_priv)
+{
+	i915_audio_component_cleanup(dev_priv);
+	intel_gpu_ips_teardown();
+	acpi_video_unregister();
+	intel_opregion_fini(dev_priv->dev);
+	i915_teardown_sysfs(dev_priv->dev);
+	i915_gem_shrinker_cleanup(dev_priv);
+}
+
+/**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
  * @flags: startup flags
@@ -1246,6 +1293,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
+	/*
+	 * TODO: move the vblank init and parts of modeset init steps into one
+	 * of the i915_driver_init_* functions according to the role/effect of
+	 * the given init step.
+	 */
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
@@ -1258,26 +1310,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto out_power_well;
 	}
 
-	i915_gem_shrinker_init(dev_priv);
-	/*
-	 * Notify a valid surface after modesetting,
-	 * when running inside a VM.
-	 */
-	if (intel_vgpu_active(dev))
-		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
-
-	i915_setup_sysfs(dev);
-
-	if (INTEL_INFO(dev)->num_pipes) {
-		/* Must be done after probing outputs */
-		intel_opregion_init(dev);
-		acpi_video_register();
-	}
-
-	if (IS_GEN5(dev))
-		intel_gpu_ips_init(dev_priv);
-
-	i915_audio_component_init(dev_priv);
+	i915_driver_init_register(dev_priv);
 
 	intel_runtime_pm_enable(dev_priv);
 
@@ -1316,15 +1349,7 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
-	i915_audio_component_cleanup(dev_priv);
-
-	intel_gpu_ips_teardown();
-
-	i915_teardown_sysfs(dev);
-
-	acpi_video_unregister();
-	intel_opregion_fini(dev);
-	i915_gem_shrinker_cleanup(dev_priv);
+	i915_driver_cleanup_register(dev_priv);
 
 	drm_vblank_cleanup(dev);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 17/17] drm/i915: Split out load time interface registration

[Chris Wilson]

On Fri, Mar 11, 2016 at 06:31:42PM +0200, Imre Deak wrote:
> According to the new init phases scheme we should register the device
> making it available via some kernel internal or user space interface as
> the last step in the init sequence, so move the corresponding code to a
> separate function.
> 
> Also add a TODO comment about code that still needs to be moved around
> to one of the init phases functions depending on what the role and effect
> of that code is.
> 
> No functional change, except for the reordering of the unload time
> unregistration steps of sysfs wrt. acpi and opregion.
> 
> Suggested by Chris.
> 
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index aaf1b17..43dcb5a 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
>  }
>  
>  /**
> + * i915_driver_init_register - register the driver with the rest of the system
> + * @dev_priv: device private
> + *
> + * Perform any steps necessary to make the driver available via kernel
> + * internal or userspace interfaces.
> + */
> +static void i915_driver_init_register(struct drm_i915_private *dev_priv)

This is the only one I didn't like. The problem with _register is that
it makes me think of mmio register (yes, I know that's too
driver-centric and the use of register_callback_interface is
widespread.) A compromise would be

i915_driver_init_frameworks()

Other than I got to here without spotting anything obnoxious or
troublematic wrt to mmio/gem. The only thing we lack is fault injection
into igt/drv_module_reload, maybe we can do
	i915.inject_fault = LOAD_MMIO | LOAD_FRAMEWORK etc
The other tricky part is deciding on what the failure should be. For
critical faults we just expect the module to fail to load, but for
aspects like GEM, we just want to the GPU to be disabled but modesetting
still work.

Anyway the above should give us better coverage of these error paths and
so hopefully prevent a panic-on-boot for some unsuspecting users.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 17/17] drm/i915: Split out load time interface registration

[Imre Deak]

On Fri, 2016-03-11 at 19:55 +0000, Chris Wilson wrote:
> On Fri, Mar 11, 2016 at 06:31:42PM +0200, Imre Deak wrote:
> > According to the new init phases scheme we should register the
> > device
> > making it available via some kernel internal or user space
> > interface as
> > the last step in the init sequence, so move the corresponding code
> > to a
> > separate function.
> > 
> > Also add a TODO comment about code that still needs to be moved
> > around
> > to one of the init phases functions depending on what the role and
> > effect
> > of that code is.
> > 
> > No functional change, except for the reordering of the unload time
> > unregistration steps of sysfs wrt. acpi and opregion.
> > 
> > Suggested by Chris.
> > 
> > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++
> > --------------
> >  1 file changed, 54 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index aaf1b17..43dcb5a 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct
> > drm_i915_private *dev_priv)
> >  }
> >  
> >  /**
> > + * i915_driver_init_register - register the driver with the rest
> > of the system
> > + * @dev_priv: device private
> > + *
> > + * Perform any steps necessary to make the driver available via
> > kernel
> > + * internal or userspace interfaces.
> > + */
> > +static void i915_driver_init_register(struct drm_i915_private
> > *dev_priv)
> 
> This is the only one I didn't like. The problem with _register is
> that
> it makes me think of mmio register (yes, I know that's too
> driver-centric and the use of register_callback_interface is
> widespread.) A compromise would be
> 
> i915_driver_init_frameworks()

Ok, can rename it.

> Other than I got to here without spotting anything obnoxious or
> troublematic wrt to mmio/gem. The only thing we lack is fault
> injection
> into igt/drv_module_reload, maybe we can do
> 	i915.inject_fault = LOAD_MMIO | LOAD_FRAMEWORK etc
> The other tricky part is deciding on what the failure should be. For
> critical faults we just expect the module to fail to load, but for
> aspects like GEM, we just want to the GPU to be disabled but
> modesetting
> still work.

Yep sounds useful, but can we do it as a follow-up?

--Imre

> Anyway the above should give us better coverage of these error paths
> and
> so hopefully prevent a panic-on-boot for some unsuspecting users.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 17/17] drm/i915: Split out load time interface registration

[Chris Wilson]

On Fri, Mar 11, 2016 at 10:11:20PM +0200, Imre Deak wrote:
> On Fri, 2016-03-11 at 19:55 +0000, Chris Wilson wrote:
> > On Fri, Mar 11, 2016 at 06:31:42PM +0200, Imre Deak wrote:
> > > According to the new init phases scheme we should register the
> > > device
> > > making it available via some kernel internal or user space
> > > interface as
> > > the last step in the init sequence, so move the corresponding code
> > > to a
> > > separate function.
> > > 
> > > Also add a TODO comment about code that still needs to be moved
> > > around
> > > to one of the init phases functions depending on what the role and
> > > effect
> > > of that code is.
> > > 
> > > No functional change, except for the reordering of the unload time
> > > unregistration steps of sysfs wrt. acpi and opregion.
> > > 
> > > Suggested by Chris.
> > > 
> > > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++
> > > --------------
> > >  1 file changed, 54 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > b/drivers/gpu/drm/i915/i915_dma.c
> > > index aaf1b17..43dcb5a 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct
> > > drm_i915_private *dev_priv)
> > >  }
> > >  
> > >  /**
> > > + * i915_driver_init_register - register the driver with the rest
> > > of the system
> > > + * @dev_priv: device private
> > > + *
> > > + * Perform any steps necessary to make the driver available via
> > > kernel
> > > + * internal or userspace interfaces.
> > > + */
> > > +static void i915_driver_init_register(struct drm_i915_private
> > > *dev_priv)
> > 
> > This is the only one I didn't like. The problem with _register is
> > that
> > it makes me think of mmio register (yes, I know that's too
> > driver-centric and the use of register_callback_interface is
> > widespread.) A compromise would be
> > 
> > i915_driver_init_frameworks()
> 
> Ok, can rename it.
> 
> > Other than I got to here without spotting anything obnoxious or
> > troublematic wrt to mmio/gem. The only thing we lack is fault
> > injection
> > into igt/drv_module_reload, maybe we can do
> > 	i915.inject_fault = LOAD_MMIO | LOAD_FRAMEWORK etc
> > The other tricky part is deciding on what the failure should be. For
> > critical faults we just expect the module to fail to load, but for
> > aspects like GEM, we just want to the GPU to be disabled but
> > modesetting
> > still work.
> 
> Yep sounds useful, but can we do it as a follow-up?

I'd rather not. The question is how much of this churn is covered by
igt? I think the answer is scarily low, since half of this is error path
during init. Adding a module paramter and then checking bits in each of
the new phase functions is going to be a relatively simple job and lets
us have a little more confidence that the changes + fixes are solid.

The tricker part would be adding the loop over insmod i915.ko into
drv_module_reload_basic, but well worth the bugs it is likely to
uncover.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 17/17] drm/i915: Split out load time interface registration

[Imre Deak]

On Fri, 2016-03-11 at 20:38 +0000, Chris Wilson wrote:
> On Fri, Mar 11, 2016 at 10:11:20PM +0200, Imre Deak wrote:
> > On Fri, 2016-03-11 at 19:55 +0000, Chris Wilson wrote:
> > > On Fri, Mar 11, 2016 at 06:31:42PM +0200, Imre Deak wrote:
> > > > According to the new init phases scheme we should register the
> > > > device
> > > > making it available via some kernel internal or user space
> > > > interface as
> > > > the last step in the init sequence, so move the corresponding
> > > > code
> > > > to a
> > > > separate function.
> > > > 
> > > > Also add a TODO comment about code that still needs to be moved
> > > > around
> > > > to one of the init phases functions depending on what the role
> > > > and
> > > > effect
> > > > of that code is.
> > > > 
> > > > No functional change, except for the reordering of the unload
> > > > time
> > > > unregistration steps of sysfs wrt. acpi and opregion.
> > > > 
> > > > Suggested by Chris.
> > > > 
> > > > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_dma.c | 83
> > > > +++++++++++++++++++++++++++
> > > > --------------
> > > >  1 file changed, 54 insertions(+), 29 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > > b/drivers/gpu/drm/i915/i915_dma.c
> > > > index aaf1b17..43dcb5a 100644
> > > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > > @@ -1209,6 +1209,53 @@ static void
> > > > i915_driver_cleanup_hw(struct
> > > > drm_i915_private *dev_priv)
> > > >  }
> > > >  
> > > >  /**
> > > > + * i915_driver_init_register - register the driver with the
> > > > rest
> > > > of the system
> > > > + * @dev_priv: device private
> > > > + *
> > > > + * Perform any steps necessary to make the driver available
> > > > via
> > > > kernel
> > > > + * internal or userspace interfaces.
> > > > + */
> > > > +static void i915_driver_init_register(struct drm_i915_private
> > > > *dev_priv)
> > > 
> > > This is the only one I didn't like. The problem with _register is
> > > that
> > > it makes me think of mmio register (yes, I know that's too
> > > driver-centric and the use of register_callback_interface is
> > > widespread.) A compromise would be
> > > 
> > > i915_driver_init_frameworks()
> > 
> > Ok, can rename it.
> > 
> > > Other than I got to here without spotting anything obnoxious or
> > > troublematic wrt to mmio/gem. The only thing we lack is fault
> > > injection
> > > into igt/drv_module_reload, maybe we can do
> > > 	i915.inject_fault = LOAD_MMIO | LOAD_FRAMEWORK etc
> > > The other tricky part is deciding on what the failure should be.
> > > For
> > > critical faults we just expect the module to fail to load, but
> > > for
> > > aspects like GEM, we just want to the GPU to be disabled but
> > > modesetting
> > > still work.
> > 
> > Yep sounds useful, but can we do it as a follow-up?
> 
> I'd rather not. The question is how much of this churn is covered by
> igt? I think the answer is scarily low, since half of this is error
> path
> during init. Adding a module paramter and then checking bits in each
> of
> the new phase functions is going to be a relatively simple job and
> lets
> us have a little more confidence that the changes + fixes are solid.

Yes not much coverage, but that means that probably there are already
existing issues. I specifically avoided touching any of the problematic
parts (in the modesetting init function) and I don't see anything in
this patchset that would make things worse (as you also pointed out).
Realistically, fixing up all the error path handling in the modesetting
path would be quite a bit of work. Otoh, this patchset is a dependency
on other PM related stuff that is quite late already, so I'd like to
progress with that. That's why I suggested to fix up the other parts
later.

--Imre

> The tricker part would be adding the loop over insmod i915.ko into
> drv_module_reload_basic, but well worth the bugs it is likely to
> uncover.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 17/17] drm/i915: Split out load time interface registration

[Imre Deak]

According to the new init phases scheme we should register the device
making it available via some kernel internal or user space interface as
the last step in the init sequence, so move the corresponding code to a
separate function.

Also add a TODO comment about code that still needs to be moved around
to one of the init phases functions depending on what the role and effect
of that code is.

No functional change, except for the reordering of the unload time
unregistration steps of sysfs wrt. acpi and opregion.

Suggested by Chris.

v3:
- rename i915_driver_init_register to i915_driver_init_frameworks
  (Chris)

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index aaf1b17..b1dbf1b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * i915_driver_init_frameworks - register the driver with the rest of the system
+ * @dev_priv: device private
+ *
+ * Perform any steps necessary to make the driver available via kernel
+ * internal or userspace interfaces.
+ */
+static void i915_driver_init_frameworks(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	i915_gem_shrinker_init(dev_priv);
+	/*
+	 * Notify a valid surface after modesetting,
+	 * when running inside a VM.
+	 */
+	if (intel_vgpu_active(dev))
+		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
+
+	i915_setup_sysfs(dev);
+
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		/* Must be done after probing outputs */
+		intel_opregion_init(dev);
+		acpi_video_register();
+	}
+
+	if (IS_GEN5(dev_priv))
+		intel_gpu_ips_init(dev_priv);
+
+	i915_audio_component_init(dev_priv);
+}
+
+/**
+ * i915_driver_cleanup_frameworks - cleanup the setup done in i915_driver_init_frameworks()
+ * @dev_priv: device private
+ */
+static void i915_driver_cleanup_frameworks(struct drm_i915_private *dev_priv)
+{
+	i915_audio_component_cleanup(dev_priv);
+	intel_gpu_ips_teardown();
+	acpi_video_unregister();
+	intel_opregion_fini(dev_priv->dev);
+	i915_teardown_sysfs(dev_priv->dev);
+	i915_gem_shrinker_cleanup(dev_priv);
+}
+
+/**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
  * @flags: startup flags
@@ -1246,6 +1293,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
+	/*
+	 * TODO: move the vblank init and parts of modeset init steps into one
+	 * of the i915_driver_init_* functions according to the role/effect of
+	 * the given init step.
+	 */
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
@@ -1258,26 +1310,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto out_power_well;
 	}
 
-	i915_gem_shrinker_init(dev_priv);
-	/*
-	 * Notify a valid surface after modesetting,
-	 * when running inside a VM.
-	 */
-	if (intel_vgpu_active(dev))
-		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
-
-	i915_setup_sysfs(dev);
-
-	if (INTEL_INFO(dev)->num_pipes) {
-		/* Must be done after probing outputs */
-		intel_opregion_init(dev);
-		acpi_video_register();
-	}
-
-	if (IS_GEN5(dev))
-		intel_gpu_ips_init(dev_priv);
-
-	i915_audio_component_init(dev_priv);
+	i915_driver_init_frameworks(dev_priv);
 
 	intel_runtime_pm_enable(dev_priv);
 
@@ -1316,15 +1349,7 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
-	i915_audio_component_cleanup(dev_priv);
-
-	intel_gpu_ips_teardown();
-
-	i915_teardown_sysfs(dev);
-
-	acpi_video_unregister();
-	intel_opregion_fini(dev);
-	i915_gem_shrinker_cleanup(dev_priv);
+	i915_driver_cleanup_frameworks(dev_priv);
 
 	drm_vblank_cleanup(dev);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 17/17] drm/i915: Split out load time interface registration

[Imre Deak]

According to the new init phases scheme we should register the device
making it available via some kernel internal or user space interface as
the last step in the init sequence, so move the corresponding code to a
separate function.

Also add a TODO comment about code that still needs to be moved around
to one of the init phases functions depending on what the role and effect
of that code is.

No functional change, except for the reordering of the unload time
unregistration steps of sysfs wrt. acpi and opregion.

Suggested by Chris.

v3:
- rename i915_driver_init_register to i915_driver_init_frameworks
  (Chris)
v4:
- rename i915_driver_init_frameworks to i915_driver_register (Daniel)

CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index aaf1b17..a5121cd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * i915_driver_register - register the driver with the rest of the system
+ * @dev_priv: device private
+ *
+ * Perform any steps necessary to make the driver available via kernel
+ * internal or userspace interfaces.
+ */
+static void i915_driver_register(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	i915_gem_shrinker_init(dev_priv);
+	/*
+	 * Notify a valid surface after modesetting,
+	 * when running inside a VM.
+	 */
+	if (intel_vgpu_active(dev))
+		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
+
+	i915_setup_sysfs(dev);
+
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		/* Must be done after probing outputs */
+		intel_opregion_init(dev);
+		acpi_video_register();
+	}
+
+	if (IS_GEN5(dev_priv))
+		intel_gpu_ips_init(dev_priv);
+
+	i915_audio_component_init(dev_priv);
+}
+
+/**
+ * i915_driver_unregister - cleanup the registration done in i915_driver_regiser()
+ * @dev_priv: device private
+ */
+static void i915_driver_unregister(struct drm_i915_private *dev_priv)
+{
+	i915_audio_component_cleanup(dev_priv);
+	intel_gpu_ips_teardown();
+	acpi_video_unregister();
+	intel_opregion_fini(dev_priv->dev);
+	i915_teardown_sysfs(dev_priv->dev);
+	i915_gem_shrinker_cleanup(dev_priv);
+}
+
+/**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
  * @flags: startup flags
@@ -1246,6 +1293,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
+	/*
+	 * TODO: move the vblank init and parts of modeset init steps into one
+	 * of the i915_driver_init_/i915_driver_register functions according
+	 * to the role/effect of the given init step.
+	 */
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
 		if (ret)
@@ -1258,26 +1310,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto out_power_well;
 	}
 
-	i915_gem_shrinker_init(dev_priv);
-	/*
-	 * Notify a valid surface after modesetting,
-	 * when running inside a VM.
-	 */
-	if (intel_vgpu_active(dev))
-		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
-
-	i915_setup_sysfs(dev);
-
-	if (INTEL_INFO(dev)->num_pipes) {
-		/* Must be done after probing outputs */
-		intel_opregion_init(dev);
-		acpi_video_register();
-	}
-
-	if (IS_GEN5(dev))
-		intel_gpu_ips_init(dev_priv);
-
-	i915_audio_component_init(dev_priv);
+	i915_driver_register(dev_priv);
 
 	intel_runtime_pm_enable(dev_priv);
 
@@ -1316,15 +1349,7 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
-	i915_audio_component_cleanup(dev_priv);
-
-	intel_gpu_ips_teardown();
-
-	i915_teardown_sysfs(dev);
-
-	acpi_video_unregister();
-	intel_opregion_fini(dev);
-	i915_gem_shrinker_cleanup(dev_priv);
+	i915_driver_unregister(dev_priv);
 
 	drm_vblank_cleanup(dev);
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 17/17] drm/i915: Split out load time interface registration

[Daniel Vetter]

On Mon, Mar 14, 2016 at 01:00:41PM +0200, Imre Deak wrote:
> According to the new init phases scheme we should register the device
> making it available via some kernel internal or user space interface as
> the last step in the init sequence, so move the corresponding code to a
> separate function.
> 
> Also add a TODO comment about code that still needs to be moved around
> to one of the init phases functions depending on what the role and effect
> of that code is.
> 
> No functional change, except for the reordering of the unload time
> unregistration steps of sysfs wrt. acpi and opregion.
> 
> Suggested by Chris.
> 
> v3:
> - rename i915_driver_init_register to i915_driver_init_frameworks
>   (Chris)
> v4:
> - rename i915_driver_init_frameworks to i915_driver_register (Daniel)
> 
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Yeah, this brings us a large step towards de-midlayering i915 load/unload,
I like.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_dma.c | 83 +++++++++++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index aaf1b17..a5121cd 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1209,6 +1209,53 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
>  }
>  
>  /**
> + * i915_driver_register - register the driver with the rest of the system
> + * @dev_priv: device private
> + *
> + * Perform any steps necessary to make the driver available via kernel
> + * internal or userspace interfaces.
> + */
> +static void i915_driver_register(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	i915_gem_shrinker_init(dev_priv);
> +	/*
> +	 * Notify a valid surface after modesetting,
> +	 * when running inside a VM.
> +	 */
> +	if (intel_vgpu_active(dev))
> +		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
> +
> +	i915_setup_sysfs(dev);
> +
> +	if (INTEL_INFO(dev_priv)->num_pipes) {
> +		/* Must be done after probing outputs */
> +		intel_opregion_init(dev);
> +		acpi_video_register();
> +	}
> +
> +	if (IS_GEN5(dev_priv))
> +		intel_gpu_ips_init(dev_priv);
> +
> +	i915_audio_component_init(dev_priv);
> +}
> +
> +/**
> + * i915_driver_unregister - cleanup the registration done in i915_driver_regiser()
> + * @dev_priv: device private
> + */
> +static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> +{
> +	i915_audio_component_cleanup(dev_priv);
> +	intel_gpu_ips_teardown();
> +	acpi_video_unregister();
> +	intel_opregion_fini(dev_priv->dev);
> +	i915_teardown_sysfs(dev_priv->dev);
> +	i915_gem_shrinker_cleanup(dev_priv);
> +}
> +
> +/**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
>   * @flags: startup flags
> @@ -1246,6 +1293,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	if (ret < 0)
>  		goto out_cleanup_mmio;
>  
> +	/*
> +	 * TODO: move the vblank init and parts of modeset init steps into one
> +	 * of the i915_driver_init_/i915_driver_register functions according
> +	 * to the role/effect of the given init step.
> +	 */
>  	if (INTEL_INFO(dev)->num_pipes) {
>  		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
>  		if (ret)
> @@ -1258,26 +1310,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  		goto out_power_well;
>  	}
>  
> -	i915_gem_shrinker_init(dev_priv);
> -	/*
> -	 * Notify a valid surface after modesetting,
> -	 * when running inside a VM.
> -	 */
> -	if (intel_vgpu_active(dev))
> -		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
> -
> -	i915_setup_sysfs(dev);
> -
> -	if (INTEL_INFO(dev)->num_pipes) {
> -		/* Must be done after probing outputs */
> -		intel_opregion_init(dev);
> -		acpi_video_register();
> -	}
> -
> -	if (IS_GEN5(dev))
> -		intel_gpu_ips_init(dev_priv);
> -
> -	i915_audio_component_init(dev_priv);
> +	i915_driver_register(dev_priv);
>  
>  	intel_runtime_pm_enable(dev_priv);
>  
> @@ -1316,15 +1349,7 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
>  
> -	i915_audio_component_cleanup(dev_priv);
> -
> -	intel_gpu_ips_teardown();
> -
> -	i915_teardown_sysfs(dev);
> -
> -	acpi_video_unregister();
> -	intel_opregion_fini(dev);
> -	i915_gem_shrinker_cleanup(dev_priv);
> +	i915_driver_unregister(dev_priv);
>  
>  	drm_vblank_cleanup(dev);
>  
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Split driver init step to phases (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Split driver init step to phases (rev3)
URL   : https://patchwork.freedesktop.org/series/4374/
State : failure

== Summary ==

Series 4374v3 drm/i915: Split driver init step to phases
http://patchwork.freedesktop.org/api/1.0/series/4374/revisions/3/mbox/

Test core_prop_blob:
        Subgroup basic:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_ctx_param_basic:
        Subgroup invalid-size-set:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_exec_basic:
        Subgroup gtt-bsd1:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup readonly-bsd:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_flink_basic:
        Subgroup flink-lifetime:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_mmap_gtt:
        Subgroup basic-small-copy-xy:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_ringfill:
        Subgroup basic-default-hang:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup basic-default-s3:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_storedw_loop:
        Subgroup basic-blt:
                pass       -> INCOMPLETE (bsw-nuc-2)
        Subgroup basic-bsd1:
                incomplete -> SKIP       (bsw-nuc-2)
Test gem_sync:
        Subgroup basic-blt:
                incomplete -> PASS       (bsw-nuc-2)
Test kms_addfb_basic:
        Subgroup basic:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup basic-x-tiled:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup bo-too-small-due-to-tiling:
                incomplete -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (hsw-gt2)
                pass       -> DMESG-WARN (bdw-ultra)
Test kms_pipe_crc_basic:
        Subgroup bad-pipe:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup nonblocking-crc-pipe-a:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup nonblocking-crc-pipe-b:
                incomplete -> SKIP       (bsw-nuc-2)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (snb-x220t)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-fail -> FAIL       (snb-x220t)
                incomplete -> PASS       (bsw-nuc-2)
Test prime_self_import:
        Subgroup basic-llseek-size:
                incomplete -> PASS       (bsw-nuc-2)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:172  dwarn:1   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:166  pass:134  dwarn:1   dfail:0   fail:0   skip:30 
hsw-brixbox      total:194  pass:171  dwarn:1   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:176  dwarn:1   dfail:0   fail:0   skip:17 
ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34 
snb-x220t        total:194  pass:159  dwarn:1   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1585/

571147d0be8b04cbbe99db761e82ef105c6f82ad drm-intel-nightly: 2016y-03m-11d-13h-31m-03s UTC integration manifest
3eb9877d5555e15e99f3252935ba9cb1852ec617 drm/i915: Split out load time interface registration
13a2a2bf523582c47d2afdec624e8df6e6c50f74 drm/i915: Split out load time HW initialization
b9c09ad27b367046b80bdb3ae554f28870d0794e drm/i915: Split out load time MMIO initialization
edd82d6aae0493cde3bef60269729c5a11a2a2ed drm/i915: Split out load time early initialization
70136efb1965480ce29d4a805b9f5e9b7c6f6ff5 drm/i915: Move unload time opregion unregistration earlier
693a514b0b4d1a6f3265ebf8e0ca49ae61c378b9 drm/i915: Move unload time GTT, MSI IRQ cleanup later
603290cde6c9ca550b6dde956f5d36912d56837f drm/i915: Move unload time display power domain uninit later
1c9269b0169b268a5ba7442727a0212bfd3bd1e4 drm/i915: Move load time audio component registration earlier
56ad4f879e09c2ce0fa6346b4ce35985188f76f6 drm/i915: Move load time shrinker registration later
dc107972d66c23ef52e98aabbf6bfced059a662b drm/i915: Move load time runtime PM get later
be8977a41fc98b76a5b6ed04638051c65528e14c drm/i915: Move load time gem_load_init earlier
9265f656297e420336f00a9d645910daa116cd6d drm/i915: Move load time runtime device info init earlier
3d36c048c27191cce6e607d598dafe9eae046798 drm/i915: Move load time init of clock gating hooks earlier
c35b56b2d3a18c385c38073c0b7c10d24c4d951d drm/i915: Move load time init of display/audio hooks earlier
20551b94911e2555b31f527bd3a19df07c2edd8e drm/i915: Move load time IRQ SW init earlier
4dcff93e20c38b2c5cb303c58319007af2485d8e drm/i915: Move load time PCH detect, DPIO, power domain SW init earlier
42a40e3e50de10c7ca8972bdea9918e71684c998 Fix MCHBAR cleanup on the driver init error path

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Split driver init step to phases (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Split driver init step to phases (rev4)
URL   : https://patchwork.freedesktop.org/series/4374/
State : failure

== Summary ==

Series 4374v4 drm/i915: Split driver init step to phases
http://patchwork.freedesktop.org/api/1.0/series/4374/revisions/4/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> INCOMPLETE (snb-dellxps)
Test gem_exec_basic:
        Subgroup readonly-render:
                incomplete -> PASS       (ilk-hp8440p)
Test gem_mmap_gtt:
        Subgroup basic-write-no-prefault:
                incomplete -> PASS       (ilk-hp8440p)
Test gem_ringfill:
        Subgroup basic-default-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
                incomplete -> DMESG-WARN (ilk-hp8440p)
Test gem_tiled_pread_basic:
                incomplete -> PASS       (byt-nuc)
Test kms_addfb_basic:
        Subgroup addfb25-yf-tiled:
                incomplete -> PASS       (ilk-hp8440p)
        Subgroup bad-pitch-1024:
                incomplete -> PASS       (ilk-hp8440p)
        Subgroup size-max:
                incomplete -> PASS       (ilk-hp8440p)
        Subgroup small-bo:
                incomplete -> PASS       (ilk-hp8440p)
        Subgroup unused-offsets:
                incomplete -> PASS       (ilk-hp8440p)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (bdw-ultra)
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-modeset:
                incomplete -> PASS       (bsw-nuc-2)
                pass       -> DMESG-WARN (hsw-brixbox)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (hsw-brixbox)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (ivb-t430s)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (hsw-gt2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test prime_self_import:
        Subgroup basic-llseek-size:
                incomplete -> PASS       (ilk-hp8440p)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:172  dwarn:1   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:194  pass:157  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:194  pass:154  dwarn:4   dfail:0   fail:1   skip:35 
hsw-brixbox      total:194  pass:170  dwarn:2   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:177  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:194  pass:125  dwarn:5   dfail:0   fail:1   skip:63 
ivb-t430s        total:194  pass:168  dwarn:0   dfail:0   fail:0   skip:26 
skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
snb-dellxps      total:27   pass:20   dwarn:0   dfail:0   fail:0   skip:6  
snb-x220t        total:194  pass:159  dwarn:1   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1591/

3e5ecc8c5ff80cb1fb635ce1cf16b7cd4cfb1979 drm-intel-nightly: 2016y-03m-14d-09h-06m-00s UTC integration manifest
9b7c99c102fcc948ed5e593f4f0474629d039ea9 drm/i915: Split out load time interface registration
677f47e901d2914a1ebb84d2f1254a6dc05e3ff9 drm/i915: Split out load time HW initialization
4bd8f04f778f8e4d0addcdd6e691c45625e3561a drm/i915: Split out load time MMIO initialization
9ba20b3995619a371f3c81e2a4cd0ba65ad81425 drm/i915: Split out load time early initialization
58739ffb77f1ab2742cee9074ffcfbb90699d66b drm/i915: Move unload time opregion unregistration earlier
83e35d8bbb77e3e604477ee35c742adcbbe37a3d drm/i915: Move unload time GTT, MSI IRQ cleanup later
d89be579d478859f52b5e03807d39b8258eaeca6 drm/i915: Move unload time display power domain uninit later
a9178ed2ecd73c994444a2aad7b3fe675c469e3c drm/i915: Move load time audio component registration earlier
4acc396227838b4ae9740e9e15d992cab2b46d8b drm/i915: Move load time shrinker registration later
d6e96bba9320ebfd684aab032a68854b15e97024 drm/i915: Move load time runtime PM get later
8eff8bdd26eccbed7bc90bdee8d83bcd749601e0 drm/i915: Move load time gem_load_init earlier
9d3a1bfcab8df0eab58d319c712eec5021730915 drm/i915: Move load time runtime device info init earlier
e9a0b7bf2667192cd0bbe43db3a723279ebb00d9 drm/i915: Move load time init of clock gating hooks earlier
e666353fb978f8c1db531264d85a9bce5a768161 drm/i915: Move load time init of display/audio hooks earlier
c06d4cfa3680256ee8cd1d487206482d469da646 drm/i915: Move load time IRQ SW init earlier
be54dcc7a88765fd8518a62f739a7404766736be drm/i915: Move load time PCH detect, DPIO, power domain SW init earlier
06325f6fc613e924cab65916c8a479d9d6a8d962 Fix MCHBAR cleanup on the driver init error path

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx