diff mbox series

[v2,5/6] drm/i915: dynamically allocate forcewake domains

Message ID 20190620010021.20637-6-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series Display uncore prep patches | expand

Commit Message

Daniele Ceraolo Spurio June 20, 2019, 1 a.m. UTC
We'd like to introduce a display uncore with no forcewake domains, so
let's avoid wasting memory and be ready to allocate only what we need.
Even without multiple uncore, we still don't need all the domains on all
gens.

v2: avoid hidden control flow, improve checks (Tvrtko), fix IVB special
case, add failure injection point

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 101 ++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_uncore.h |  13 ++--
 2 files changed, 77 insertions(+), 37 deletions(-)

Comments

Chris Wilson June 20, 2019, 7:55 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2019-06-20 02:00:20)
> We'd like to introduce a display uncore with no forcewake domains, so
> let's avoid wasting memory and be ready to allocate only what we need.
> Even without multiple uncore, we still don't need all the domains on all
> gens.
> 
> v2: avoid hidden control flow, improve checks (Tvrtko), fix IVB special
> case, add failure injection point
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 101 ++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_uncore.h |  13 ++--
>  2 files changed, 77 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 00bf5e085a2c..2bd602a41bb7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -344,7 +344,7 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
>  {
>         struct intel_uncore_forcewake_domain *domain =
>                container_of(timer, struct intel_uncore_forcewake_domain, timer);
> -       struct intel_uncore *uncore = forcewake_domain_to_uncore(domain);
> +       struct intel_uncore *uncore = domain->uncore;
>         unsigned long irqflags;
>  
>         assert_rpm_device_not_suspended(uncore->rpm);
> @@ -1293,23 +1293,27 @@ do { \
>         (uncore)->funcs.read_fw_domains = x##_reg_read_fw_domains; \
>  } while (0)
>  
> -static void fw_domain_init(struct intel_uncore *uncore,
> -                          enum forcewake_domain_id domain_id,
> -                          i915_reg_t reg_set,
> -                          i915_reg_t reg_ack)
> +static int __fw_domain_init(struct intel_uncore *uncore,
> +                           enum forcewake_domain_id domain_id,
> +                           i915_reg_t reg_set,
> +                           i915_reg_t reg_ack)
>  {
>         struct intel_uncore_forcewake_domain *d;
>  
> -       if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
> -               return;
> +       GEM_BUG_ON(domain_id >= FW_DOMAIN_ID_COUNT);
> +       GEM_BUG_ON(uncore->fw_domain[domain_id]);
>  
> -       d = &uncore->fw_domain[domain_id];
> +       if (i915_inject_load_failure())
> +               return -ENOMEM;

Will be an interesting test for the test (well dmesg parser to see if it
is still getting upset over nothing).

> -       WARN_ON(d->wake_count);
> +       d = kzalloc(sizeof(*d), GFP_KERNEL);
> +       if (!d)
> +               return -ENOMEM;
>  
>         WARN_ON(!i915_mmio_reg_valid(reg_set));
>         WARN_ON(!i915_mmio_reg_valid(reg_ack));
>  
> +       d->uncore = uncore;
>         d->wake_count = 0;
>         d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
>         d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack);
> @@ -1326,7 +1330,6 @@ static void fw_domain_init(struct intel_uncore *uncore,
>         BUILD_BUG_ON(FORCEWAKE_MEDIA_VEBOX0 != (1 << FW_DOMAIN_ID_MEDIA_VEBOX0));
>         BUILD_BUG_ON(FORCEWAKE_MEDIA_VEBOX1 != (1 << FW_DOMAIN_ID_MEDIA_VEBOX1));
>  
> -
>         d->mask = BIT(domain_id);
>  
>         hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> @@ -1335,6 +1338,10 @@ static void fw_domain_init(struct intel_uncore *uncore,
>         uncore->fw_domains |= BIT(domain_id);
>  
>         fw_domain_reset(d);
> +
> +       uncore->fw_domain[domain_id] = d;

Fwiw, I would pair this with setting the mask in uncore->fw_domains.

> +
> +       return 0;
>  }
>  
>  static void fw_domain_fini(struct intel_uncore *uncore,
> @@ -1342,29 +1349,41 @@ static void fw_domain_fini(struct intel_uncore *uncore,
>  {
>         struct intel_uncore_forcewake_domain *d;
>  
> -       if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
> -               return;
> +       GEM_BUG_ON(domain_id >= FW_DOMAIN_ID_COUNT);
>  
> -       d = &uncore->fw_domain[domain_id];
> +       d = fetch_and_zero(&uncore->fw_domain[domain_id]);
> +       if (!d)
> +               return;
>  
> +       uncore->fw_domains &= ~BIT(domain_id);
>         WARN_ON(d->wake_count);
>         WARN_ON(hrtimer_cancel(&d->timer));
> -       memset(d, 0, sizeof(*d));
> +       kfree(d);
> +}
>  
> -       uncore->fw_domains &= ~BIT(domain_id);
> +static void intel_uncore_fw_domains_fini(struct intel_uncore *uncore)
> +{
> +       struct intel_uncore_forcewake_domain *d;
> +       int tmp;
> +
> +       for_each_fw_domain(d, uncore, tmp)
> +               fw_domain_fini(uncore, d->id);
>  }
>  
> -static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
> +static int intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>  {
>         struct drm_i915_private *i915 = uncore->i915;
> +       int ret = 0;
>  
>         GEM_BUG_ON(!intel_uncore_has_forcewake(uncore));
>  
> +#define fw_domain_init(uncore__, id__, set__, ack__) \
> +       (ret ?: (ret = __fw_domain_init((uncore__), (id__), (set__), (ack__))))

Seems a reasonable compromise, that I'm sure we'll live to regret. :)

>         if (INTEL_GEN(i915) >= 11) {
>                 int i;
>  
> -               uncore->funcs.force_wake_get =
> -                       fw_domains_get_with_fallback;
> +               uncore->funcs.force_wake_get = fw_domains_get_with_fallback;
>                 uncore->funcs.force_wake_put = fw_domains_put;
>                 fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
>                                FORCEWAKE_RENDER_GEN9,
> @@ -1372,6 +1391,7 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>                 fw_domain_init(uncore, FW_DOMAIN_ID_BLITTER,
>                                FORCEWAKE_BLITTER_GEN9,
>                                FORCEWAKE_ACK_BLITTER_GEN9);
> +
>                 for (i = 0; i < I915_MAX_VCS; i++) {
>                         if (!HAS_ENGINE(i915, _VCS(i)))
>                                 continue;
> @@ -1389,8 +1409,7 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>                                        FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(i));
>                 }
>         } else if (IS_GEN_RANGE(i915, 9, 10)) {
> -               uncore->funcs.force_wake_get =
> -                       fw_domains_get_with_fallback;
> +               uncore->funcs.force_wake_get = fw_domains_get_with_fallback;
>                 uncore->funcs.force_wake_put = fw_domains_put;
>                 fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
>                                FORCEWAKE_RENDER_GEN9,
> @@ -1439,8 +1458,10 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>                 __raw_uncore_write32(uncore, FORCEWAKE, 0);
>                 __raw_posting_read(uncore, ECOBUS);
>  
> -               fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
> -                              FORCEWAKE_MT, FORCEWAKE_MT_ACK);
> +               ret = __fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
> +                                      FORCEWAKE_MT, FORCEWAKE_MT_ACK);
> +               if (ret)
> +                       goto out;
>  
>                 spin_lock_irq(&uncore->lock);
>                 fw_domains_get_with_thread_status(uncore, FORCEWAKE_RENDER);
> @@ -1451,6 +1472,7 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>                 if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
>                         DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
>                         DRM_INFO("when using vblank-synced partial screen updates.\n");
> +                       fw_domain_fini(uncore, FW_DOMAIN_ID_RENDER);
>                         fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
>                                        FORCEWAKE, FORCEWAKE_ACK);
>                 }
> @@ -1462,8 +1484,16 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>                                FORCEWAKE, FORCEWAKE_ACK);
>         }
>  
> +#undef fw_domain_init
> +
>         /* All future platforms are expected to require complex power gating */
> -       WARN_ON(uncore->fw_domains == 0);
> +       WARN_ON(!ret && uncore->fw_domains == 0);
> +
> +out:
> +       if (ret)
> +               intel_uncore_fw_domains_fini(uncore);
> +
> +       return ret;
>  }
>  
>  #define ASSIGN_FW_DOMAINS_TABLE(uncore, d) \
> @@ -1564,13 +1594,17 @@ static void uncore_raw_init(struct intel_uncore *uncore)
>         }
>  }
>  
> -static void uncore_forcewake_init(struct intel_uncore *uncore)
> +static int uncore_forcewake_init(struct intel_uncore *uncore)
>  {
>         struct drm_i915_private *i915 = uncore->i915;
> +       int ret;
>  
>         GEM_BUG_ON(!intel_uncore_has_forcewake(uncore));
>  
> -       intel_uncore_fw_domains_init(uncore);
> +       ret = intel_uncore_fw_domains_init(uncore);
> +       if (ret)
> +               return ret;
> +
>         forcewake_early_sanitize(uncore, 0);
>  
>         if (IS_GEN_RANGE(i915, 6, 7)) {
> @@ -1603,6 +1637,8 @@ static void uncore_forcewake_init(struct intel_uncore *uncore)
>  
>         uncore->pmic_bus_access_nb.notifier_call = i915_pmic_bus_access_notifier;
>         iosf_mbi_register_pmic_bus_access_notifier(&uncore->pmic_bus_access_nb);
> +
> +       return 0;
>  }
>  
>  int intel_uncore_init_mmio(struct intel_uncore *uncore)
> @@ -1621,10 +1657,13 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  
>         uncore->unclaimed_mmio_check = 1;
>  
> -       if (!intel_uncore_has_forcewake(uncore))
> +       if (!intel_uncore_has_forcewake(uncore)) {
>                 uncore_raw_init(uncore);
> -       else
> -               uncore_forcewake_init(uncore);
> +       } else {
> +               ret = uncore_forcewake_init(uncore);
> +               if (ret)
> +                       goto out_mmio_cleanup;
> +       }
>  
>         /* make sure fw funcs are set if and only if we have fw*/
>         GEM_BUG_ON(intel_uncore_has_forcewake(uncore) != !!uncore->funcs.force_wake_get);
> @@ -1646,6 +1685,11 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>                 DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
>  
>         return 0;
> +
> +out_mmio_cleanup:
> +       uncore_mmio_cleanup(uncore);
> +
> +       return ret;
>  }
>  
>  /*
> @@ -1691,6 +1735,7 @@ void intel_uncore_fini_mmio(struct intel_uncore *uncore)
>                 iosf_mbi_unregister_pmic_bus_access_notifier_unlocked(
>                         &uncore->pmic_bus_access_nb);
>                 intel_uncore_forcewake_reset(uncore);
> +               intel_uncore_fw_domains_fini(uncore);

Ok, looks like this is paired correctly. I suppose we can't do the
allocations any earlier, and it does make sense that we can't detect the
domains until we can probe the HW so mmio.

>                 iosf_mbi_punit_release();
>         }
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 59505a2f9097..7108475d9b24 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -126,6 +126,7 @@ struct intel_uncore {
>         enum forcewake_domains fw_domains_saved; /* user domains saved for S3 */
>  
>         struct intel_uncore_forcewake_domain {
> +               struct intel_uncore *uncore;
>                 enum forcewake_domain_id id;
>                 enum forcewake_domains mask;
>                 unsigned int wake_count;
> @@ -133,7 +134,7 @@ struct intel_uncore {
>                 struct hrtimer timer;
>                 u32 __iomem *reg_set;
>                 u32 __iomem *reg_ack;
> -       } fw_domain[FW_DOMAIN_ID_COUNT];
> +       } *fw_domain[FW_DOMAIN_ID_COUNT];

How long before we start using a tree for the countless domains :)

>  
>         struct {
>                 unsigned int count;
> @@ -147,18 +148,12 @@ struct intel_uncore {
>  
>  /* Iterate over initialised fw domains */
>  #define for_each_fw_domain_masked(domain__, mask__, uncore__, tmp__) \
> -       for (tmp__ = (mask__); \
> -            tmp__ ? (domain__ = &(uncore__)->fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
> +       for (tmp__ = (mask__); tmp__ ;) \
> +               for_each_if(domain__ = (uncore__)->fw_domain[__mask_next_bit(tmp__)])
>  
>  #define for_each_fw_domain(domain__, uncore__, tmp__) \
>         for_each_fw_domain_masked(domain__, (uncore__)->fw_domains, uncore__, tmp__)
>  
> -static inline struct intel_uncore *
> -forcewake_domain_to_uncore(const struct intel_uncore_forcewake_domain *d)
> -{
> -       return container_of(d, struct intel_uncore, fw_domain[d->id]);
> -}
> -
>  static inline bool
>  intel_uncore_has_forcewake(const struct intel_uncore *uncore)
>  {

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 00bf5e085a2c..2bd602a41bb7 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -344,7 +344,7 @@  intel_uncore_fw_release_timer(struct hrtimer *timer)
 {
 	struct intel_uncore_forcewake_domain *domain =
 	       container_of(timer, struct intel_uncore_forcewake_domain, timer);
-	struct intel_uncore *uncore = forcewake_domain_to_uncore(domain);
+	struct intel_uncore *uncore = domain->uncore;
 	unsigned long irqflags;
 
 	assert_rpm_device_not_suspended(uncore->rpm);
@@ -1293,23 +1293,27 @@  do { \
 	(uncore)->funcs.read_fw_domains = x##_reg_read_fw_domains; \
 } while (0)
 
-static void fw_domain_init(struct intel_uncore *uncore,
-			   enum forcewake_domain_id domain_id,
-			   i915_reg_t reg_set,
-			   i915_reg_t reg_ack)
+static int __fw_domain_init(struct intel_uncore *uncore,
+			    enum forcewake_domain_id domain_id,
+			    i915_reg_t reg_set,
+			    i915_reg_t reg_ack)
 {
 	struct intel_uncore_forcewake_domain *d;
 
-	if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
-		return;
+	GEM_BUG_ON(domain_id >= FW_DOMAIN_ID_COUNT);
+	GEM_BUG_ON(uncore->fw_domain[domain_id]);
 
-	d = &uncore->fw_domain[domain_id];
+	if (i915_inject_load_failure())
+		return -ENOMEM;
 
-	WARN_ON(d->wake_count);
+	d = kzalloc(sizeof(*d), GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
 
 	WARN_ON(!i915_mmio_reg_valid(reg_set));
 	WARN_ON(!i915_mmio_reg_valid(reg_ack));
 
+	d->uncore = uncore;
 	d->wake_count = 0;
 	d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
 	d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack);
@@ -1326,7 +1330,6 @@  static void fw_domain_init(struct intel_uncore *uncore,
 	BUILD_BUG_ON(FORCEWAKE_MEDIA_VEBOX0 != (1 << FW_DOMAIN_ID_MEDIA_VEBOX0));
 	BUILD_BUG_ON(FORCEWAKE_MEDIA_VEBOX1 != (1 << FW_DOMAIN_ID_MEDIA_VEBOX1));
 
-
 	d->mask = BIT(domain_id);
 
 	hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
@@ -1335,6 +1338,10 @@  static void fw_domain_init(struct intel_uncore *uncore,
 	uncore->fw_domains |= BIT(domain_id);
 
 	fw_domain_reset(d);
+
+	uncore->fw_domain[domain_id] = d;
+
+	return 0;
 }
 
 static void fw_domain_fini(struct intel_uncore *uncore,
@@ -1342,29 +1349,41 @@  static void fw_domain_fini(struct intel_uncore *uncore,
 {
 	struct intel_uncore_forcewake_domain *d;
 
-	if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
-		return;
+	GEM_BUG_ON(domain_id >= FW_DOMAIN_ID_COUNT);
 
-	d = &uncore->fw_domain[domain_id];
+	d = fetch_and_zero(&uncore->fw_domain[domain_id]);
+	if (!d)
+		return;
 
+	uncore->fw_domains &= ~BIT(domain_id);
 	WARN_ON(d->wake_count);
 	WARN_ON(hrtimer_cancel(&d->timer));
-	memset(d, 0, sizeof(*d));
+	kfree(d);
+}
 
-	uncore->fw_domains &= ~BIT(domain_id);
+static void intel_uncore_fw_domains_fini(struct intel_uncore *uncore)
+{
+	struct intel_uncore_forcewake_domain *d;
+	int tmp;
+
+	for_each_fw_domain(d, uncore, tmp)
+		fw_domain_fini(uncore, d->id);
 }
 
-static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
+static int intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 {
 	struct drm_i915_private *i915 = uncore->i915;
+	int ret = 0;
 
 	GEM_BUG_ON(!intel_uncore_has_forcewake(uncore));
 
+#define fw_domain_init(uncore__, id__, set__, ack__) \
+	(ret ?: (ret = __fw_domain_init((uncore__), (id__), (set__), (ack__))))
+
 	if (INTEL_GEN(i915) >= 11) {
 		int i;
 
-		uncore->funcs.force_wake_get =
-			fw_domains_get_with_fallback;
+		uncore->funcs.force_wake_get = fw_domains_get_with_fallback;
 		uncore->funcs.force_wake_put = fw_domains_put;
 		fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE_RENDER_GEN9,
@@ -1372,6 +1391,7 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 		fw_domain_init(uncore, FW_DOMAIN_ID_BLITTER,
 			       FORCEWAKE_BLITTER_GEN9,
 			       FORCEWAKE_ACK_BLITTER_GEN9);
+
 		for (i = 0; i < I915_MAX_VCS; i++) {
 			if (!HAS_ENGINE(i915, _VCS(i)))
 				continue;
@@ -1389,8 +1409,7 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 				       FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(i));
 		}
 	} else if (IS_GEN_RANGE(i915, 9, 10)) {
-		uncore->funcs.force_wake_get =
-			fw_domains_get_with_fallback;
+		uncore->funcs.force_wake_get = fw_domains_get_with_fallback;
 		uncore->funcs.force_wake_put = fw_domains_put;
 		fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE_RENDER_GEN9,
@@ -1439,8 +1458,10 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 		__raw_uncore_write32(uncore, FORCEWAKE, 0);
 		__raw_posting_read(uncore, ECOBUS);
 
-		fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
-			       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
+		ret = __fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
+				       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
+		if (ret)
+			goto out;
 
 		spin_lock_irq(&uncore->lock);
 		fw_domains_get_with_thread_status(uncore, FORCEWAKE_RENDER);
@@ -1451,6 +1472,7 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
 			DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
 			DRM_INFO("when using vblank-synced partial screen updates.\n");
+			fw_domain_fini(uncore, FW_DOMAIN_ID_RENDER);
 			fw_domain_init(uncore, FW_DOMAIN_ID_RENDER,
 				       FORCEWAKE, FORCEWAKE_ACK);
 		}
@@ -1462,8 +1484,16 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 			       FORCEWAKE, FORCEWAKE_ACK);
 	}
 
+#undef fw_domain_init
+
 	/* All future platforms are expected to require complex power gating */
-	WARN_ON(uncore->fw_domains == 0);
+	WARN_ON(!ret && uncore->fw_domains == 0);
+
+out:
+	if (ret)
+		intel_uncore_fw_domains_fini(uncore);
+
+	return ret;
 }
 
 #define ASSIGN_FW_DOMAINS_TABLE(uncore, d) \
@@ -1564,13 +1594,17 @@  static void uncore_raw_init(struct intel_uncore *uncore)
 	}
 }
 
-static void uncore_forcewake_init(struct intel_uncore *uncore)
+static int uncore_forcewake_init(struct intel_uncore *uncore)
 {
 	struct drm_i915_private *i915 = uncore->i915;
+	int ret;
 
 	GEM_BUG_ON(!intel_uncore_has_forcewake(uncore));
 
-	intel_uncore_fw_domains_init(uncore);
+	ret = intel_uncore_fw_domains_init(uncore);
+	if (ret)
+		return ret;
+
 	forcewake_early_sanitize(uncore, 0);
 
 	if (IS_GEN_RANGE(i915, 6, 7)) {
@@ -1603,6 +1637,8 @@  static void uncore_forcewake_init(struct intel_uncore *uncore)
 
 	uncore->pmic_bus_access_nb.notifier_call = i915_pmic_bus_access_notifier;
 	iosf_mbi_register_pmic_bus_access_notifier(&uncore->pmic_bus_access_nb);
+
+	return 0;
 }
 
 int intel_uncore_init_mmio(struct intel_uncore *uncore)
@@ -1621,10 +1657,13 @@  int intel_uncore_init_mmio(struct intel_uncore *uncore)
 
 	uncore->unclaimed_mmio_check = 1;
 
-	if (!intel_uncore_has_forcewake(uncore))
+	if (!intel_uncore_has_forcewake(uncore)) {
 		uncore_raw_init(uncore);
-	else
-		uncore_forcewake_init(uncore);
+	} else {
+		ret = uncore_forcewake_init(uncore);
+		if (ret)
+			goto out_mmio_cleanup;
+	}
 
 	/* make sure fw funcs are set if and only if we have fw*/
 	GEM_BUG_ON(intel_uncore_has_forcewake(uncore) != !!uncore->funcs.force_wake_get);
@@ -1646,6 +1685,11 @@  int intel_uncore_init_mmio(struct intel_uncore *uncore)
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
 
 	return 0;
+
+out_mmio_cleanup:
+	uncore_mmio_cleanup(uncore);
+
+	return ret;
 }
 
 /*
@@ -1691,6 +1735,7 @@  void intel_uncore_fini_mmio(struct intel_uncore *uncore)
 		iosf_mbi_unregister_pmic_bus_access_notifier_unlocked(
 			&uncore->pmic_bus_access_nb);
 		intel_uncore_forcewake_reset(uncore);
+		intel_uncore_fw_domains_fini(uncore);
 		iosf_mbi_punit_release();
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 59505a2f9097..7108475d9b24 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -126,6 +126,7 @@  struct intel_uncore {
 	enum forcewake_domains fw_domains_saved; /* user domains saved for S3 */
 
 	struct intel_uncore_forcewake_domain {
+		struct intel_uncore *uncore;
 		enum forcewake_domain_id id;
 		enum forcewake_domains mask;
 		unsigned int wake_count;
@@ -133,7 +134,7 @@  struct intel_uncore {
 		struct hrtimer timer;
 		u32 __iomem *reg_set;
 		u32 __iomem *reg_ack;
-	} fw_domain[FW_DOMAIN_ID_COUNT];
+	} *fw_domain[FW_DOMAIN_ID_COUNT];
 
 	struct {
 		unsigned int count;
@@ -147,18 +148,12 @@  struct intel_uncore {
 
 /* Iterate over initialised fw domains */
 #define for_each_fw_domain_masked(domain__, mask__, uncore__, tmp__) \
-	for (tmp__ = (mask__); \
-	     tmp__ ? (domain__ = &(uncore__)->fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
+	for (tmp__ = (mask__); tmp__ ;) \
+		for_each_if(domain__ = (uncore__)->fw_domain[__mask_next_bit(tmp__)])
 
 #define for_each_fw_domain(domain__, uncore__, tmp__) \
 	for_each_fw_domain_masked(domain__, (uncore__)->fw_domains, uncore__, tmp__)
 
-static inline struct intel_uncore *
-forcewake_domain_to_uncore(const struct intel_uncore_forcewake_domain *d)
-{
-	return container_of(d, struct intel_uncore, fw_domain[d->id]);
-}
-
 static inline bool
 intel_uncore_has_forcewake(const struct intel_uncore *uncore)
 {