diff mbox

drm/i915: Init power domains early in driver load

Message ID 1452174665-13025-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Jan. 7, 2016, 1:51 p.m. UTC
Since

commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Fri Nov 27 18:55:26 2015 +0200

    drm/i915: Introduce a gmbus power domain

gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.

v2: Adjust cleanup paths too (Chris).

v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).

v4: Ville instead suggested to move gmbus setup later in the sequence,
since it's only needed by the modeset code.

v5: Move even close to the actual user, right next to the comment that
states where we really need gmbus (and interrupts!).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Meelis Roos <mroos@linux.ee>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---

Meelis, can you pls retest this one?

Thanks, Daniel
---
 drivers/gpu/drm/i915/i915_dma.c      | 6 +++---
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Ville Syrjälä Jan. 7, 2016, 2:32 p.m. UTC | #1
On Thu, Jan 07, 2016 at 02:51:05PM +0100, Daniel Vetter wrote:
> Since
> 
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Fri Nov 27 18:55:26 2015 +0200
> 
>     drm/i915: Introduce a gmbus power domain
> 
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
> 
> v2: Adjust cleanup paths too (Chris).
> 
> v3: Rebase onto -nightly (totally bogus tree I had lying around) and
> also move dpio init head (Ville).
> 
> v4: Ville instead suggested to move gmbus setup later in the sequence,
> since it's only needed by the modeset code.
> 
> v5: Move even close to the actual user, right next to the comment that
> states where we really need gmbus (and interrupts!).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Meelis Roos <mroos@linux.ee>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: stable@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> 
> Meelis, can you pls retest this one?
> 
> Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_dma.c      | 6 +++---
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 988a3806512a..d70d96fe553b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	if (ret)
>  		goto cleanup_gem_stolen;
>  
> +	intel_setup_gmbus(dev);
> +
>  	/* Important: The output setup functions called by modeset_init need
>  	 * working irqs for e.g. gmbus and dp aux transfers. */
>  	intel_modeset_init(dev);
> @@ -455,6 +457,7 @@ cleanup_gem:
>  cleanup_irq:
>  	intel_guc_ucode_fini(dev);
>  	drm_irq_uninstall(dev);
> +	intel_teardown_gmbus(dev);
>  cleanup_gem_stolen:
>  	i915_gem_cleanup_stolen(dev);
>  cleanup_vga_switcheroo:
> @@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev);
> -	intel_setup_gmbus(dev);
>  	intel_opregion_setup(dev);
>  
>  	i915_gem_load(dev);
> @@ -1101,7 +1103,6 @@ out_gem_unload:
>  	if (dev->pdev->msi_enabled)
>  		pci_disable_msi(dev->pdev);
>  
> -	intel_teardown_gmbus(dev);
>  	intel_teardown_mchbar(dev);
>  	pm_qos_remove_request(&dev_priv->pm_qos);
>  	destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
> @@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	intel_csr_ucode_fini(dev_priv);
>  
> -	intel_teardown_gmbus(dev);
>  	intel_teardown_mchbar(dev);
>  
>  	destroy_workqueue(dev_priv->hotplug.dp_wq);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 37945ddb4dad..ac0038bf4fbf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	mutex_lock(&dev->struct_mutex);
>  	intel_cleanup_gt_powersave(dev);
>  	mutex_unlock(&dev->struct_mutex);
> +
> +	intel_teardown_gmbus(dev);

The cleanup path is where things might still be a bit wonky. Should we be
doing this before turning off the interrupts? But then that might mean
that the hpd cleanup needs to be rehashed somewhat to make sure we shut
down hpd before unregistering gmbus. The whole init/cleanup sequence is
a bit of a mess tbh, so a major overhaul might be required to make it
actually sane.

In any case, I'm thinking this is at least no worse that what we had, so 
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  /*
> -- 
> 2.6.4
Meelis Roos Jan. 7, 2016, 3:49 p.m. UTC | #2
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Fri Nov 27 18:55:26 2015 +0200
> 
>     drm/i915: Introduce a gmbus power domain
> 
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
> 
> v2: Adjust cleanup paths too (Chris).
> 
> v3: Rebase onto -nightly (totally bogus tree I had lying around) and
> also move dpio init head (Ville).
> 
> v4: Ville instead suggested to move gmbus setup later in the sequence,
> since it's only needed by the modeset code.
> 
> v5: Move even close to the actual user, right next to the comment that
> states where we really need gmbus (and interrupts!).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Meelis Roos <mroos@linux.ee>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: stable@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> 
> Meelis, can you pls retest this one?

Tested successfully on SNB computer.

> 
> Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_dma.c      | 6 +++---
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 988a3806512a..d70d96fe553b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	if (ret)
>  		goto cleanup_gem_stolen;
>  
> +	intel_setup_gmbus(dev);
> +
>  	/* Important: The output setup functions called by modeset_init need
>  	 * working irqs for e.g. gmbus and dp aux transfers. */
>  	intel_modeset_init(dev);
> @@ -455,6 +457,7 @@ cleanup_gem:
>  cleanup_irq:
>  	intel_guc_ucode_fini(dev);
>  	drm_irq_uninstall(dev);
> +	intel_teardown_gmbus(dev);
>  cleanup_gem_stolen:
>  	i915_gem_cleanup_stolen(dev);
>  cleanup_vga_switcheroo:
> @@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev);
> -	intel_setup_gmbus(dev);
>  	intel_opregion_setup(dev);
>  
>  	i915_gem_load(dev);
> @@ -1101,7 +1103,6 @@ out_gem_unload:
>  	if (dev->pdev->msi_enabled)
>  		pci_disable_msi(dev->pdev);
>  
> -	intel_teardown_gmbus(dev);
>  	intel_teardown_mchbar(dev);
>  	pm_qos_remove_request(&dev_priv->pm_qos);
>  	destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
> @@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	intel_csr_ucode_fini(dev_priv);
>  
> -	intel_teardown_gmbus(dev);
>  	intel_teardown_mchbar(dev);
>  
>  	destroy_workqueue(dev_priv->hotplug.dp_wq);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 37945ddb4dad..ac0038bf4fbf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	mutex_lock(&dev->struct_mutex);
>  	intel_cleanup_gt_powersave(dev);
>  	mutex_unlock(&dev->struct_mutex);
> +
> +	intel_teardown_gmbus(dev);
>  }
>  
>  /*
>
Meelis Roos Jan. 7, 2016, 7:41 p.m. UTC | #3
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Fri Nov 27 18:55:26 2015 +0200
> 
>     drm/i915: Introduce a gmbus power domain
> 
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
> 
> v2: Adjust cleanup paths too (Chris).
> 
> v3: Rebase onto -nightly (totally bogus tree I had lying around) and
> also move dpio init head (Ville).
> 
> v4: Ville instead suggested to move gmbus setup later in the sequence,
> since it's only needed by the modeset code.
> 
> v5: Move even close to the actual user, right next to the comment that
> states where we really need gmbus (and interrupts!).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Meelis Roos <mroos@linux.ee>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: stable@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> 
> Meelis, can you pls retest this one?

I also confirmed that my 865G chipset computer was suffering from the 
same problem and the patch also helps on D865GLC mainboard with my 
userspace that autoloads eeprom driver.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..d70d96fe553b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -406,6 +406,8 @@  static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_gem_stolen;
 
+	intel_setup_gmbus(dev);
+
 	/* Important: The output setup functions called by modeset_init need
 	 * working irqs for e.g. gmbus and dp aux transfers. */
 	intel_modeset_init(dev);
@@ -455,6 +457,7 @@  cleanup_gem:
 cleanup_irq:
 	intel_guc_ucode_fini(dev);
 	drm_irq_uninstall(dev);
+	intel_teardown_gmbus(dev);
 cleanup_gem_stolen:
 	i915_gem_cleanup_stolen(dev);
 cleanup_vga_switcheroo:
@@ -1028,7 +1031,6 @@  int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
-	intel_setup_gmbus(dev);
 	intel_opregion_setup(dev);
 
 	i915_gem_load(dev);
@@ -1101,7 +1103,6 @@  out_gem_unload:
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
-	intel_teardown_gmbus(dev);
 	intel_teardown_mchbar(dev);
 	pm_qos_remove_request(&dev_priv->pm_qos);
 	destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
@@ -1203,7 +1204,6 @@  int i915_driver_unload(struct drm_device *dev)
 
 	intel_csr_ucode_fini(dev_priv);
 
-	intel_teardown_gmbus(dev);
 	intel_teardown_mchbar(dev);
 
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 37945ddb4dad..ac0038bf4fbf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15971,6 +15971,8 @@  void intel_modeset_cleanup(struct drm_device *dev)
 	mutex_lock(&dev->struct_mutex);
 	intel_cleanup_gt_powersave(dev);
 	mutex_unlock(&dev->struct_mutex);
+
+	intel_teardown_gmbus(dev);
 }
 
 /*