All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07  9:10 ` Daniel Vetter
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07  9:10 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos, stable,
	Daniel Vetter

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");
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07  9:10 ` Daniel Vetter
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07  9:10 UTC (permalink / raw)
  To: DRI Development
  Cc: Meelis Roos, Jani Nikula, Daniel Vetter,
	Intel Graphics Development, stable, Daniel Vetter

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");
-- 
2.6.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07  9:10 ` Daniel Vetter
@ 2016-01-07  9:22   ` Chris Wilson
  -1 siblings, 0 replies; 29+ messages in thread
From: Chris Wilson @ 2016-01-07  9:22 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Meelis Roos, Jani Nikula,
	Intel Graphics Development, stable, Daniel Vetter

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 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);

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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07  9:22   ` Chris Wilson
  0 siblings, 0 replies; 29+ messages in thread
From: Chris Wilson @ 2016-01-07  9:22 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Meelis Roos, Jani Nikula,
	Intel Graphics Development, stable, Daniel Vetter

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 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);

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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07  9:10 ` Daniel Vetter
@ 2016-01-07 10:23   ` Meelis Roos
  -1 siblings, 0 replies; 29+ messages in thread
From: Meelis Roos @ 2016-01-07 10:23 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, stable, Daniel Vetter

> 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>

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");
> 

-- 
Meelis Roos (mroos@ut.ee)      http://www.cs.ut.ee/~mroos/

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 10:23   ` Meelis Roos
  0 siblings, 0 replies; 29+ messages in thread
From: Meelis Roos @ 2016-01-07 10:23 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Jani Nikula, DRI Development, stable, Daniel Vetter,
	Intel Graphics Development

> 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>

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");
> 

-- 
Meelis Roos (mroos@ut.ee)      http://www.cs.ut.ee/~mroos/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07  9:10 ` Daniel Vetter
                   ` (2 preceding siblings ...)
  (?)
@ 2016-01-07 11:44 ` Daniel Vetter
  2016-01-07 11:52     ` Chris Wilson
                     ` (3 more replies)
  -1 siblings, 4 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07 11:44 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos,
	Chris Wilson, stable, Daniel Vetter

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:
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 11:44 ` Daniel Vetter
@ 2016-01-07 11:52     ` Chris Wilson
  2016-01-07 11:52     ` kbuild test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Chris Wilson @ 2016-01-07 11:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos, stable,
	Daniel Vetter

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 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>

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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 11:52     ` Chris Wilson
  0 siblings, 0 replies; 29+ messages in thread
From: Chris Wilson @ 2016-01-07 11:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos, stable,
	Daniel Vetter

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 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>

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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 11:44 ` Daniel Vetter
@ 2016-01-07 11:52     ` kbuild test robot
  2016-01-07 11:52     ` kbuild test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: kbuild test robot @ 2016-01-07 11:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: kbuild-all, DRI Development, Meelis Roos, Jani Nikula,
	Daniel Vetter, Intel Graphics Development, stable, Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

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-domains-early-in-driver-load/20160107-194542
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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24308 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 11:52     ` kbuild test robot
  0 siblings, 0 replies; 29+ messages in thread
From: kbuild test robot @ 2016-01-07 11:52 UTC (permalink / raw)
  Cc: Jani Nikula, Daniel Vetter, Meelis Roos, DRI Development,
	kbuild-all, Daniel Vetter, stable, Intel Graphics Development

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

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-domains-early-in-driver-load/20160107-194542
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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24308 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 11:44 ` Daniel Vetter
@ 2016-01-07 12:14     ` Jani Nikula
  2016-01-07 11:52     ` kbuild test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-01-07 12:14 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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 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>

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:

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 12:14     ` Jani Nikula
  0 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-01-07 12:14 UTC (permalink / raw)
  To: DRI Development
  Cc: Meelis Roos, Daniel Vetter, Intel Graphics Development, stable,
	Daniel Vetter

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 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>

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:

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 11:44 ` Daniel Vetter
@ 2016-01-07 12:52     ` Ville Syrjälä
  2016-01-07 11:52     ` kbuild test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 12:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Jani Nikula, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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 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);

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

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 12:52     ` Ville Syrjälä
  0 siblings, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 12:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Meelis Roos, Jani Nikula, Intel Graphics Development,
	DRI Development, stable, Daniel Vetter

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 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);

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 12:52     ` Ville Syrjälä
  (?)
@ 2016-01-07 13:15     ` Daniel Vetter
  2016-01-07 13:25         ` Jani Nikula
  2016-01-07 13:26         ` Ville Syrjälä
  -1 siblings, 2 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07 13:15 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Jani Nikula, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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 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);
>
> 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07  9:10 ` Daniel Vetter
                   ` (3 preceding siblings ...)
  (?)
@ 2016-01-07 13:16 ` Daniel Vetter
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07 13:16 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos,
	Chris Wilson, stable, Daniel Vetter

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:
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 13:15     ` Daniel Vetter
@ 2016-01-07 13:25         ` Jani Nikula
  2016-01-07 13:26         ` Ville Syrjälä
  1 sibling, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-01-07 13:25 UTC (permalink / raw)
  To: Daniel Vetter, Ville Syrjälä
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Meelis Roos, Chris Wilson, stable, Daniel Vetter

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 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);
>>
>> 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


-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 13:25         ` Jani Nikula
  0 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-01-07 13:25 UTC (permalink / raw)
  To: Daniel Vetter, Ville Syrjälä
  Cc: Meelis Roos, Intel Graphics Development, DRI Development, stable,
	Daniel Vetter

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 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);
>>
>> 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


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 13:15     ` Daniel Vetter
@ 2016-01-07 13:26         ` Ville Syrjälä
  2016-01-07 13:26         ` Ville Syrjälä
  1 sibling, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 13:26 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Jani Nikula, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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 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);
> >
> > 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.

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 13:26         ` Ville Syrjälä
  0 siblings, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 13:26 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Jani Nikula, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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 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);
> >
> > 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.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07  9:10 ` Daniel Vetter
                   ` (4 preceding siblings ...)
  (?)
@ 2016-01-07 13:51 ` Daniel Vetter
  2016-01-07 14:32     ` Ville Syrjälä
                     ` (2 more replies)
  -1 siblings, 3 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-07 13:51 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos,
	Chris Wilson, stable, Daniel Vetter

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);
 }
 
 /*
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 13:51 ` Daniel Vetter
@ 2016-01-07 14:32     ` Ville Syrjälä
  2016-01-07 15:49   ` Meelis Roos
  2016-01-07 19:41     ` Meelis Roos
  2 siblings, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 14:32 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Patrik Jakobsson,
	Imre Deak, Jani Nikula, Meelis Roos, Chris Wilson, stable,
	Daniel Vetter

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

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 14:32     ` Ville Syrjälä
  0 siblings, 0 replies; 29+ messages in thread
From: Ville Syrjälä @ 2016-01-07 14:32 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Meelis Roos, Jani Nikula, Intel Graphics Development,
	DRI Development, stable, Daniel Vetter

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 13:51 ` Daniel Vetter
  2016-01-07 14:32     ` Ville Syrjälä
@ 2016-01-07 15:49   ` Meelis Roos
  2016-01-07 19:41     ` Meelis Roos
  2 siblings, 0 replies; 29+ messages in thread
From: Meelis Roos @ 2016-01-07 15:49 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Chris Wilson, stable,
	Daniel Vetter

> 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 (mroos@ut.ee)      http://www.cs.ut.ee/~mroos/

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
  2016-01-07 13:51 ` Daniel Vetter
@ 2016-01-07 19:41     ` Meelis Roos
  2016-01-07 15:49   ` Meelis Roos
  2016-01-07 19:41     ` Meelis Roos
  2 siblings, 0 replies; 29+ messages in thread
From: Meelis Roos @ 2016-01-07 19:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development,
	Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Chris Wilson, stable,
	Daniel Vetter

> 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.

-- 
Meelis Roos (mroos@linux.ee)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-07 19:41     ` Meelis Roos
  0 siblings, 0 replies; 29+ messages in thread
From: Meelis Roos @ 2016-01-07 19:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Jani Nikula, DRI Development, stable, Daniel Vetter,
	Intel Graphics Development

> 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.

-- 
Meelis Roos (mroos@linux.ee)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* ✗ failure: Fi.CI.BAT
  2016-01-07  9:10 ` Daniel Vetter
                   ` (5 preceding siblings ...)
  (?)
@ 2016-01-11  9:12 ` Patchwork
  -1 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2016-01-11  9:12 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Summary ==

Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
                dmesg-warn -> PASS       (bdw-ultra)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (ilk-hp8440p)
                dmesg-warn -> PASS       (byt-nuc)

bdw-ultra        total:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:114  dwarn:3   dfail:0   fail:0   skip:24 
byt-nuc          total:141  pass:119  dwarn:7   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
hsw-xps12        total:138  pass:133  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:100  dwarn:4   dfail:0   fail:0   skip:37 
skl-i5k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

HANGED ivb-t430s in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b

Results at /archive/results/CI_IGT_test/Patchwork_1112/

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] drm/i915: Init power domains early in driver load
@ 2016-01-13 10:55 Daniel Vetter
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Vetter @ 2016-01-13 10:55 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Daniel Vetter, Ville Syrjälä,
	Patrik Jakobsson, Imre Deak, Jani Nikula, Meelis Roos,
	Chris Wilson, stable, Daniel Vetter

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>
---

Resending since our CI seems to have missed it.
-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 44a896ce32e6..a0f5659032fc 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:
@@ -1029,7 +1032,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);
@@ -1102,7 +1104,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);
@@ -1204,7 +1205,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 20a73d9e19d7..86fce41281fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16125,6 +16125,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);
 }
 
 /*
-- 
2.7.0.rc3


^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2016-01-13 10:55 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07  9:10 [PATCH] drm/i915: Init power domains early in driver load Daniel Vetter
2016-01-07  9:10 ` Daniel Vetter
2016-01-07  9:22 ` [Intel-gfx] " Chris Wilson
2016-01-07  9:22   ` Chris Wilson
2016-01-07 10:23 ` Meelis Roos
2016-01-07 10:23   ` Meelis Roos
2016-01-07 11:44 ` Daniel Vetter
2016-01-07 11:52   ` Chris Wilson
2016-01-07 11:52     ` Chris Wilson
2016-01-07 11:52   ` [Intel-gfx] " kbuild test robot
2016-01-07 11:52     ` kbuild test robot
2016-01-07 12:14   ` Jani Nikula
2016-01-07 12:14     ` Jani Nikula
2016-01-07 12:52   ` Ville Syrjälä
2016-01-07 12:52     ` Ville Syrjälä
2016-01-07 13:15     ` Daniel Vetter
2016-01-07 13:25       ` Jani Nikula
2016-01-07 13:25         ` Jani Nikula
2016-01-07 13:26       ` Ville Syrjälä
2016-01-07 13:26         ` Ville Syrjälä
2016-01-07 13:16 ` Daniel Vetter
2016-01-07 13:51 ` Daniel Vetter
2016-01-07 14:32   ` Ville Syrjälä
2016-01-07 14:32     ` Ville Syrjälä
2016-01-07 15:49   ` Meelis Roos
2016-01-07 19:41   ` Meelis Roos
2016-01-07 19:41     ` Meelis Roos
2016-01-11  9:12 ` ✗ failure: Fi.CI.BAT Patchwork
2016-01-13 10:55 [PATCH] drm/i915: Init power domains early in driver load Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.