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.
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 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 --- drivers/gpu/drm/i915/i915_dma.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b4741d121a74..405aba2ca736 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv); - ret = intel_irq_install(dev_priv); if (ret) goto cleanup_gem_stolen; @@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev); + intel_power_domains_init(dev_priv); + intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) 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");
On Thu, Jan 07, 2016 at 10:10:56AM +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 domaingmbus also needs the power domain infrastructure right from the start, since as soon as we register the i2c controllers someone can use them.
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 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
drivers/gpu/drm/i915/i915_dma.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b4741d121a74..405aba2ca736 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv);
- ret = intel_irq_install(dev_priv); if (ret) goto cleanup_gem_stolen;
@@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
- intel_power_domains_init(dev_priv);
- intel_power_domains_init_hw(dev_priv);
A long time ago you wished for static/runtime analysis to check the ordering constraints, so do I :|
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; }
- intel_power_domains_init(dev_priv);
Wouldn't you also have to update the unwind-on-error paths? -Chris
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 domaingmbus also needs the power domain infrastructure right from the start, since as soon as we register the i2c controllers someone can use them.
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 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
Tested-by: Meelis Roos mroos@linux.ee
Worked fine on my SNB computer.
drivers/gpu/drm/i915/i915_dma.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b4741d121a74..405aba2ca736 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv);
- ret = intel_irq_install(dev_priv); if (ret) goto cleanup_gem_stolen;
@@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
intel_power_domains_init(dev_priv);
intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev);
@@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) 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");
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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev); + intel_power_domains_init(dev_priv); + intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) 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"); - goto out_power_well; + goto out_vblank; }
/* @@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
return 0;
-out_power_well: - intel_power_domains_fini(dev_priv); +out_vblank: drm_vblank_cleanup(dev); out_gem_unload: WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier)); @@ -1103,6 +1101,7 @@ out_gem_unload:
intel_teardown_gmbus(dev); intel_teardown_mchbar(dev); + intel_power_domains_fini(dev_priv); pm_qos_remove_request(&dev_priv->pm_qos); destroy_workqueue(dev_priv->gpu_error.hangcheck_wq); out_freedpwq:
On Thu, Jan 07, 2016 at 12:44:21PM +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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Onion looks fine to me,
Acked-by: Chris Wilson chris@chris-wilson.co.uk
I don't feel comfortable enough with the power domains dependencies to know if the init sequence is correct. -Chris
Hi Daniel,
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on next-20160106] [cannot apply to v4.4-rc8] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Init-power-d... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-x007-01060743 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
drivers/gpu/drm/i915/i915_dma.c:1028:2: error: too few arguments to function 'intel_power_domains_init_hw'
intel_power_domains_init_hw(dev_priv); ^ In file included from drivers/gpu/drm/i915/i915_dma.c:35:0: drivers/gpu/drm/i915/intel_drv.h:1417:6: note: declared here void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume); ^
vim +/intel_power_domains_init_hw +1028 drivers/gpu/drm/i915/i915_dma.c
1022 goto out_freedpwq; 1023 } 1024 1025 intel_irq_init(dev_priv); 1026 intel_uncore_sanitize(dev); 1027 intel_power_domains_init(dev_priv);
1028 intel_power_domains_init_hw(dev_priv);
1029 1030 /* Try to make sure MCHBAR is enabled before poking at it */ 1031 intel_setup_mchbar(dev);
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, 07 Jan 2016, Daniel Vetter daniel.vetter@ffwll.ch 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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93608
drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
intel_power_domains_init(dev_priv);
intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev);
@@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) 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");
goto out_power_well;
goto out_vblank;}
/*
@@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank: drm_vblank_cleanup(dev); out_gem_unload: WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier)); @@ -1103,6 +1101,7 @@ out_gem_unload:
intel_teardown_gmbus(dev); intel_teardown_mchbar(dev);
- intel_power_domains_fini(dev_priv); pm_qos_remove_request(&dev_priv->pm_qos); destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
On Thu, Jan 07, 2016 at 12:44:21PM +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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
- intel_power_domains_init(dev_priv);
- intel_power_domains_init_hw(dev_priv);
I think intel_init_dpio() needs to be moved too. We need to know the DPIO IOSF ports before attempting to talk to the PHY (which can happen from intel_power_domains_init_hw()).
I'm also wondering why we're doing gmbus init this early. We shouldn't need it until modeset init.
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) 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");
goto out_power_well;
goto out_vblank;}
/*
@@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank: drm_vblank_cleanup(dev); out_gem_unload: WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier)); @@ -1103,6 +1101,7 @@ out_gem_unload:
intel_teardown_gmbus(dev); intel_teardown_mchbar(dev);
- intel_power_domains_fini(dev_priv); pm_qos_remove_request(&dev_priv->pm_qos); destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
2.6.4
On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Thu, Jan 07, 2016 at 12:44:21PM +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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
intel_power_domains_init_hw(dev_priv, false); intel_csr_ucode_init(dev_priv);@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
intel_power_domains_init(dev_priv);intel_power_domains_init_hw(dev_priv);I think intel_init_dpio() needs to be moved too. We need to know the DPIO IOSF ports before attempting to talk to the PHY (which can happen from intel_power_domains_init_hw()).
Ugh, will change.
I'm also wondering why we're doing gmbus init this early. We shouldn't need it until modeset init.
Anyone can access the gmbus controller once we register it. Userspace can (like what seems to happen on Meelis' box), but also the i2c core has some auto-probed stuff in some configs afaik. -Daniel
On Thu, 07 Jan 2016, Daniel Vetter daniel.vetter@ffwll.ch wrote:
On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Thu, Jan 07, 2016 at 12:44:21PM +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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
intel_power_domains_init_hw(dev_priv, false); intel_csr_ucode_init(dev_priv);@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
intel_power_domains_init(dev_priv);intel_power_domains_init_hw(dev_priv);I think intel_init_dpio() needs to be moved too. We need to know the DPIO IOSF ports before attempting to talk to the PHY (which can happen from intel_power_domains_init_hw()).
Ugh, will change.
I'm also wondering why we're doing gmbus init this early. We shouldn't need it until modeset init.
Anyone can access the gmbus controller once we register it. Userspace can (like what seems to happen on Meelis' box), but also the i2c core has some auto-probed stuff in some configs afaik.
Ville's question was why we register the i2c adapters this early.
As I explained in [1], the auto-probing happens when there's an i2c driver with matching class (I2C_CLASS_DDC). That's what happens on Meelis' box. But yes, we need to take userspace into account as well.
In short, when we call intel_gmbus_setup(), we need to be ready for i2c communication.
BR, Jani.
[1] http://mid.gmane.org/87vb75znpz.fsf@intel.com
On Thu, Jan 07, 2016 at 02:15:50PM +0100, Daniel Vetter wrote:
On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Thu, Jan 07, 2016 at 12:44:21PM +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 domaingmbus 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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..490d8b0d931e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
intel_power_domains_init_hw(dev_priv, false); intel_csr_ucode_init(dev_priv);@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev);
intel_power_domains_init(dev_priv);intel_power_domains_init_hw(dev_priv);I think intel_init_dpio() needs to be moved too. We need to know the DPIO IOSF ports before attempting to talk to the PHY (which can happen from intel_power_domains_init_hw()).
Ugh, will change.
I think I placed the dpio init in the current place so that it sits next to intel_device_info_runtime_init(). Doing a lot of hw bashing before all this runtime device info stuff has been set up seems rather wrong to me.
I'm also wondering why we're doing gmbus init this early. We shouldn't need it until modeset init.
Anyone can access the gmbus controller once we register it. Userspace can (like what seems to happen on Meelis' box), but also the i2c core has some auto-probed stuff in some configs afaik.
Sure, but I don't see any reason why we'd need to init it that early. The only requirement is that we need to init before we ourselves use it, which I think means we don't actually need it until output setup. And gmbus being a component of the display engine means the init should really be part of the modeset init.
So I tend to think the better fix would be to move gmbus init to happen later.
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).
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 Tested-by: Meelis Roos mroos@linux.ee Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 988a3806512a..fcefd3beef50 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_irq_init(dev_priv); intel_uncore_sanitize(dev); + intel_power_domains_init(dev_priv); + intel_init_dpio(dev_priv); + intel_power_domains_init_hw(dev_priv, false);
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1049,20 +1051,16 @@ 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"); - goto out_power_well; + goto out_vblank; }
/* @@ -1091,8 +1089,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
return 0;
-out_power_well: - intel_power_domains_fini(dev_priv); +out_vblank: drm_vblank_cleanup(dev); out_gem_unload: WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier)); @@ -1103,6 +1100,7 @@ out_gem_unload:
intel_teardown_gmbus(dev); intel_teardown_mchbar(dev); + intel_power_domains_fini(dev_priv); pm_qos_remove_request(&dev_priv->pm_qos); destroy_workqueue(dev_priv->gpu_error.hangcheck_wq); out_freedpwq:
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); }
/*
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 domaingmbus 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
intel_modeset_init(dev);
- working irqs for e.g. gmbus and dp aux transfers. */
@@ -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
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 domaingmbus 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
intel_modeset_init(dev);
- working irqs for e.g. gmbus and dp aux transfers. */
@@ -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);
}
/*
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 domaingmbus 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.
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter -
Jani Nikula -
kbuild test robot -
Meelis Roos -
Ville Syrjälä