drm/i915: Init power domains early in driver load
diff mbox

Message ID 1452167061-27252-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Jan. 7, 2016, 11:44 a.m. UTC
Since

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

    drm/i915: Introduce a gmbus power domain

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

v2: Adjust cleanup paths too (Chris).

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

Comments

Chris Wilson Jan. 7, 2016, 11:52 a.m. UTC | #1
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
kernel test robot Jan. 7, 2016, 11:52 a.m. UTC | #2
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
Jani Nikula Jan. 7, 2016, 12:14 p.m. UTC | #3
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:
Ville Syrjälä Jan. 7, 2016, 12:52 p.m. UTC | #4
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
Daniel Vetter Jan. 7, 2016, 1:15 p.m. UTC | #5
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
Jani Nikula Jan. 7, 2016, 1:25 p.m. UTC | #6
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
Ville Syrjälä Jan. 7, 2016, 1:26 p.m. UTC | #7
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.

Patch
diff mbox

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: