Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-16 Thread Daniele Ceraolo Spurio



On 8/16/19 2:35 AM, Jani Nikula wrote:

On Thu, 15 Aug 2019, Daniele Ceraolo Spurio  
wrote:

This is only required for a single platform so no need to reserve the
memory on all of them.

This removes the last direct dependency of i915_drv.h on i915_reg.h
(apart from the i915_reg_t definition).

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Imre Deak 


Heh, I've already sent a version of this [1], but I don't mind you
finishing the job. Especially because I think it's better to handle the
alloc/free explicitly instead of the way I do it.


D'oh! I completely missed that



I do have some nitpicks on this one though, inline.


[1] 
http://patchwork.freedesktop.org/patch/msgid/20190807144939.32123-1-jani.nik...@intel.com








@@ -2137,7 +2228,7 @@ static int i915_pm_restore(struct device *kdev)
   */
  static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
  {
-   struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
+   struct vlv_s0ix_state *s = dev_priv->s0ix_state;


I think I'd now call this function unconditionally, and return early if
(!s). This puts the decision to do this or not in one place only, in
vlv_alloc_s0ix_state(), instead of duplicating the conditions.


ack





int i;
  
  	/* GAM 0x4000-0x4770 */

@@ -2147,7 +2238,7 @@ static void vlv_save_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
s->gfx_pend_tlb0 = I915_READ(GEN7_GFX_PEND_TLB0);
s->gfx_pend_tlb1 = I915_READ(GEN7_GFX_PEND_TLB1);
  
-	for (i = 0; i < ARRAY_SIZE(s->lra_limits); i++)

+   for (i = 0; i < GEN7_LRA_LIMITS_REG_NUM; i++)
s->lra_limits[i] = I915_READ(GEN7_LRA_LIMITS(i));
  
  	s->media_max_req_count	= I915_READ(GEN7_MEDIA_MAX_REQ_COUNT);

@@ -2191,7 +2282,7 @@ static void vlv_save_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
s->pm_imr= I915_READ(GEN6_PMIMR);
s->pm_ier= I915_READ(GEN6_PMIER);
  
-	for (i = 0; i < ARRAY_SIZE(s->gt_scratch); i++)

+   for (i = 0; i < GEN7_GT_SCRATCH_REG_NUM; i++)
s->gt_scratch[i] = I915_READ(GEN7_GT_SCRATCH(i));


The above two hunks are in the wrong patch.



Yup, leftovers from a previous version.

  
  	/* GT SA CZ domain, 0x10-0x138124 */

@@ -2218,7 +2309,7 @@ static void vlv_save_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
  
  static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)

  {
-   struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
+   struct vlv_s0ix_state *s = dev_priv->s0ix_state;


Early return on !s here as well, and call the function unconditionally.



ok


u32 val;
int i;
  
@@ -2229,7 +2320,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)

I915_WRITE(GEN7_GFX_PEND_TLB0,  s->gfx_pend_tlb0);
I915_WRITE(GEN7_GFX_PEND_TLB1,  s->gfx_pend_tlb1);
  
-	for (i = 0; i < ARRAY_SIZE(s->lra_limits); i++)

+   for (i = 0; i < GEN7_LRA_LIMITS_REG_NUM; i++)
I915_WRITE(GEN7_LRA_LIMITS(i), s->lra_limits[i]);
  
  	I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->media_max_req_count);

@@ -2273,7 +2364,7 @@ static void vlv_restore_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
I915_WRITE(GEN6_PMIMR,  s->pm_imr);
I915_WRITE(GEN6_PMIER,  s->pm_ier);
  
-	for (i = 0; i < ARRAY_SIZE(s->gt_scratch); i++)

+   for (i = 0; i < GEN7_GT_SCRATCH_REG_NUM; i++)
I915_WRITE(GEN7_GT_SCRATCH(i), s->gt_scratch[i]);


The above two hunks are in the wrong patch.

  
  	/* GT SA CZ domain, 0x10-0x138124 */

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6722d54ccd5..9b41f2209b69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -527,68 +527,6 @@ struct i915_suspend_saved_registers {
u16 saveGCDGMBUS;
  };
  
-struct vlv_s0ix_state {

-   /* GAM */
-   u32 wr_watermark;
-   u32 gfx_prio_ctrl;
-   u32 arb_mode;
-   u32 gfx_pend_tlb0;
-   u32 gfx_pend_tlb1;
-   u32 lra_limits[GEN7_LRA_LIMITS_REG_NUM];
-   u32 media_max_req_count;
-   u32 gfx_max_req_count;
-   u32 render_hwsp;
-   u32 ecochk;
-   u32 bsd_hwsp;
-   u32 blt_hwsp;
-   u32 tlb_rd_addr;
-
-   /* MBC */
-   u32 g3dctl;
-   u32 gsckgctl;
-   u32 mbctl;
-
-   /* GCP */
-   u32 ucgctl1;
-   u32 ucgctl3;
-   u32 rcgctl1;
-   u32 rcgctl2;
-   u32 rstctl;
-   u32 misccpctl;
-
-   /* GPM */
-   u32 gfxpause;
-   u32 rpdeuhwtc;
-   u32 rpdeuc;
-   u32 ecobus;
-   u32 pwrdwnupctl;
-   u32 rp_down_timeout;
-   u32 rp_deucsw;
-   u32 rcubmabdtmr;
-   u32 rcedata;
-   u32 spare2gh;
-
-   /* Display 1 CZ domain */
-   u32 gt_imr;
-   u32 gt_ier;
-   u32 pm_imr;
-   u32 pm_ier;
-   u32 gt_scratch[GEN7_GT_SCRATCH_REG_NUM];
-
-   /* GT SA CZ domain */
-   u32 tilectl;
-   u32

Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-16 Thread Daniele Ceraolo Spurio





@@ -2229,7 +2320,7 @@ static void vlv_restore_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
 I915_WRITE(GEN7_GFX_PEND_TLB0,  s->gfx_pend_tlb0);
 I915_WRITE(GEN7_GFX_PEND_TLB1,  s->gfx_pend_tlb1);

-   for (i = 0; i < ARRAY_SIZE(s->lra_limits); i++)
+   for (i = 0; i < GEN7_LRA_LIMITS_REG_NUM; i++)


why?


Leftover from the first version I had locally where I was trying to 
dynamically allocate just the arrays instead of the whole thing. Will 
get rid of it.


Daniele



Lucas De Marchi


 I915_WRITE(GEN7_LRA_LIMITS(i), s->lra_limits[i]);

 I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->media_max_req_count);
@@ -2273,7 +2364,7 @@ static void vlv_restore_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
 I915_WRITE(GEN6_PMIMR,  s->pm_imr);
 I915_WRITE(GEN6_PMIER,  s->pm_ier);

-   for (i = 0; i < ARRAY_SIZE(s->gt_scratch); i++)
+   for (i = 0; i < GEN7_GT_SCRATCH_REG_NUM; i++)
 I915_WRITE(GEN7_GT_SCRATCH(i), s->gt_scratch[i]);

 /* GT SA CZ domain, 0x10-0x138124 */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6722d54ccd5..9b41f2209b69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -527,68 +527,6 @@ struct i915_suspend_saved_registers {
 u16 saveGCDGMBUS;
  };

-struct vlv_s0ix_state {
-   /* GAM */
-   u32 wr_watermark;
-   u32 gfx_prio_ctrl;
-   u32 arb_mode;
-   u32 gfx_pend_tlb0;
-   u32 gfx_pend_tlb1;
-   u32 lra_limits[GEN7_LRA_LIMITS_REG_NUM];
-   u32 media_max_req_count;
-   u32 gfx_max_req_count;
-   u32 render_hwsp;
-   u32 ecochk;
-   u32 bsd_hwsp;
-   u32 blt_hwsp;
-   u32 tlb_rd_addr;
-
-   /* MBC */
-   u32 g3dctl;
-   u32 gsckgctl;
-   u32 mbctl;
-
-   /* GCP */
-   u32 ucgctl1;
-   u32 ucgctl3;
-   u32 rcgctl1;
-   u32 rcgctl2;
-   u32 rstctl;
-   u32 misccpctl;
-
-   /* GPM */
-   u32 gfxpause;
-   u32 rpdeuhwtc;
-   u32 rpdeuc;
-   u32 ecobus;
-   u32 pwrdwnupctl;
-   u32 rp_down_timeout;
-   u32 rp_deucsw;
-   u32 rcubmabdtmr;
-   u32 rcedata;
-   u32 spare2gh;
-
-   /* Display 1 CZ domain */
-   u32 gt_imr;
-   u32 gt_ier;
-   u32 pm_imr;
-   u32 pm_ier;
-   u32 gt_scratch[GEN7_GT_SCRATCH_REG_NUM];
-
-   /* GT SA CZ domain */
-   u32 tilectl;
-   u32 gt_fifoctl;
-   u32 gtlc_wake_ctrl;
-   u32 gtlc_survive;
-   u32 pmwgicz;
-
-   /* Display 2 CZ domain */
-   u32 gu_ctl0;
-   u32 gu_ctl1;
-   u32 pcbr;
-   u32 clock_gate_dis2;
-};
-
  struct intel_rps_ei {
 ktime_t ktime;
 u32 render_c0;
@@ -1622,7 +1560,7 @@ struct drm_i915_private {
 u32 suspend_count;
 bool power_domains_suspended;
 struct i915_suspend_saved_registers regfile;
-   struct vlv_s0ix_state vlv_s0ix_state;
+   void *s0ix_state;

 enum {
 I915_SAGV_UNKNOWN = 0,
--
2.22.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx





___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-16 Thread Jani Nikula
On Fri, 16 Aug 2019, Chris Wilson  wrote:
> Quoting Jani Nikula (2019-08-16 10:35:08)
>> On Thu, 15 Aug 2019, Daniele Ceraolo Spurio 
>>  wrote:
>> > @@ -1622,7 +1560,7 @@ struct drm_i915_private {
>> >   u32 suspend_count;
>> >   bool power_domains_suspended;
>> >   struct i915_suspend_saved_registers regfile;
>> > - struct vlv_s0ix_state vlv_s0ix_state;
>> > + void *s0ix_state;
>> 
>> I'd keep the vlv_ prefix in the member name.
>
> And forward decl to avoid a naked void *

Oh yes absolutely, I had that in my version of the patch, and didn't
even spot it here!

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-16 Thread Chris Wilson
Quoting Jani Nikula (2019-08-16 10:35:08)
> On Thu, 15 Aug 2019, Daniele Ceraolo Spurio  
> wrote:
> > @@ -1622,7 +1560,7 @@ struct drm_i915_private {
> >   u32 suspend_count;
> >   bool power_domains_suspended;
> >   struct i915_suspend_saved_registers regfile;
> > - struct vlv_s0ix_state vlv_s0ix_state;
> > + void *s0ix_state;
> 
> I'd keep the vlv_ prefix in the member name.

And forward decl to avoid a naked void *
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-16 Thread Jani Nikula
On Thu, 15 Aug 2019, Daniele Ceraolo Spurio  
wrote:
> This is only required for a single platform so no need to reserve the
> memory on all of them.
>
> This removes the last direct dependency of i915_drv.h on i915_reg.h
> (apart from the i915_reg_t definition).
>
> Signed-off-by: Daniele Ceraolo Spurio 
> Cc: Imre Deak 

Heh, I've already sent a version of this [1], but I don't mind you
finishing the job. Especially because I think it's better to handle the
alloc/free explicitly instead of the way I do it.

I do have some nitpicks on this one though, inline.


[1] 
http://patchwork.freedesktop.org/patch/msgid/20190807144939.32123-1-jani.nik...@intel.com


> ---
>  drivers/gpu/drm/i915/i915_drv.c | 107 +---
>  drivers/gpu/drm/i915/i915_drv.h |  64 +--
>  2 files changed, 100 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2541a3a1c229..1723b2ddfccd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -80,6 +80,68 @@
>  
>  static struct drm_driver driver;
>  
> +struct vlv_s0ix_state {
> + /* GAM */
> + u32 wr_watermark;
> + u32 gfx_prio_ctrl;
> + u32 arb_mode;
> + u32 gfx_pend_tlb0;
> + u32 gfx_pend_tlb1;
> + u32 lra_limits[GEN7_LRA_LIMITS_REG_NUM];
> + u32 media_max_req_count;
> + u32 gfx_max_req_count;
> + u32 render_hwsp;
> + u32 ecochk;
> + u32 bsd_hwsp;
> + u32 blt_hwsp;
> + u32 tlb_rd_addr;
> +
> + /* MBC */
> + u32 g3dctl;
> + u32 gsckgctl;
> + u32 mbctl;
> +
> + /* GCP */
> + u32 ucgctl1;
> + u32 ucgctl3;
> + u32 rcgctl1;
> + u32 rcgctl2;
> + u32 rstctl;
> + u32 misccpctl;
> +
> + /* GPM */
> + u32 gfxpause;
> + u32 rpdeuhwtc;
> + u32 rpdeuc;
> + u32 ecobus;
> + u32 pwrdwnupctl;
> + u32 rp_down_timeout;
> + u32 rp_deucsw;
> + u32 rcubmabdtmr;
> + u32 rcedata;
> + u32 spare2gh;
> +
> + /* Display 1 CZ domain */
> + u32 gt_imr;
> + u32 gt_ier;
> + u32 pm_imr;
> + u32 pm_ier;
> + u32 gt_scratch[GEN7_GT_SCRATCH_REG_NUM];
> +
> + /* GT SA CZ domain */
> + u32 tilectl;
> + u32 gt_fifoctl;
> + u32 gtlc_wake_ctrl;
> + u32 gtlc_survive;
> + u32 pmwgicz;
> +
> + /* Display 2 CZ domain */
> + u32 gu_ctl0;
> + u32 gu_ctl1;
> + u32 pcbr;
> + u32 clock_gate_dis2;
> +};
> +
>  static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>  {
>   int domain = pci_domain_nr(dev_priv->drm.pdev->bus);
> @@ -466,6 +528,28 @@ static void intel_detect_preproduction_hw(struct 
> drm_i915_private *dev_priv)
>   }
>  }
>  
> +static int vlv_alloc_s0ix_state(struct drm_i915_private *i915)
> +{
> + if (!IS_VALLEYVIEW(i915))
> + return 0;
> +
> + /* we write all the values in the structure, so no need to zero it out 
> */
> + i915->s0ix_state = kmalloc(sizeof(struct vlv_s0ix_state), GFP_KERNEL);
> + if (!i915->s0ix_state)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static void vlv_free_s0ix_state(struct drm_i915_private *i915)
> +{
> + if (!i915->s0ix_state)
> + return;
> +
> + kfree(i915->s0ix_state);
> + i915->s0ix_state = NULL;
> +}
> +
>  /**
>   * i915_driver_early_probe - setup state not requiring device access
>   * @dev_priv: device private
> @@ -508,13 +592,17 @@ static int i915_driver_early_probe(struct 
> drm_i915_private *dev_priv)
>   if (ret < 0)
>   return ret;
>  
> + ret = vlv_alloc_s0ix_state(dev_priv);
> + if (ret < 0)
> + goto err_workqueues;
> +
>   intel_wopcm_init_early(&dev_priv->wopcm);
>  
>   intel_gt_init_early(&dev_priv->gt, dev_priv);
>  
>   ret = i915_gem_init_early(dev_priv);
>   if (ret < 0)
> - goto err_workqueues;
> + goto err_gt;
>  
>   /* This must be called before any calls to HAS_PCH_* */
>   intel_detect_pch(dev_priv);
> @@ -536,8 +624,10 @@ static int i915_driver_early_probe(struct 
> drm_i915_private *dev_priv)
>  
>  err_gem:
>   i915_gem_cleanup_early(dev_priv);
> -err_workqueues:
> +err_gt:
>   intel_gt_driver_late_release(&dev_priv->gt);
> + vlv_free_s0ix_state(dev_priv);
> +err_workqueues:
>   i915_workqueues_cleanup(dev_priv);
>   return ret;
>  }
> @@ -553,6 +643,7 @@ static void i915_driver_late_release(struct 
> drm_i915_private *dev_priv)
>   intel_power_domains_cleanup(dev_priv);
>   i915_gem_cleanup_early(dev_priv);
>   intel_gt_driver_late_release(&dev_priv->gt);
> + vlv_free_s0ix_state(dev_priv);
>   i915_workqueues_cleanup(dev_priv);
>  
>   pm_qos_remove_request(&dev_priv->sb_qos);
> @@ -2137,7 +2228,7 @@ static int i915_pm_restore(struct device *kdev)
>   */
>  static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  {
> - struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix

Re: [Intel-gfx] [PATCH 4/6] drm/i915: Dynamically allocate s0ix struct for VLV

2019-08-15 Thread Lucas De Marchi
On Thu, Aug 15, 2019 at 6:24 PM Daniele Ceraolo Spurio
 wrote:
>
> This is only required for a single platform so no need to reserve the
> memory on all of them.
>
> This removes the last direct dependency of i915_drv.h on i915_reg.h
> (apart from the i915_reg_t definition).
>
> Signed-off-by: Daniele Ceraolo Spurio 
> Cc: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 107 +---
>  drivers/gpu/drm/i915/i915_drv.h |  64 +--
>  2 files changed, 100 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2541a3a1c229..1723b2ddfccd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -80,6 +80,68 @@
>
>  static struct drm_driver driver;
>
> +struct vlv_s0ix_state {
> +   /* GAM */
> +   u32 wr_watermark;
> +   u32 gfx_prio_ctrl;
> +   u32 arb_mode;
> +   u32 gfx_pend_tlb0;
> +   u32 gfx_pend_tlb1;
> +   u32 lra_limits[GEN7_LRA_LIMITS_REG_NUM];
> +   u32 media_max_req_count;
> +   u32 gfx_max_req_count;
> +   u32 render_hwsp;
> +   u32 ecochk;
> +   u32 bsd_hwsp;
> +   u32 blt_hwsp;
> +   u32 tlb_rd_addr;
> +
> +   /* MBC */
> +   u32 g3dctl;
> +   u32 gsckgctl;
> +   u32 mbctl;
> +
> +   /* GCP */
> +   u32 ucgctl1;
> +   u32 ucgctl3;
> +   u32 rcgctl1;
> +   u32 rcgctl2;
> +   u32 rstctl;
> +   u32 misccpctl;
> +
> +   /* GPM */
> +   u32 gfxpause;
> +   u32 rpdeuhwtc;
> +   u32 rpdeuc;
> +   u32 ecobus;
> +   u32 pwrdwnupctl;
> +   u32 rp_down_timeout;
> +   u32 rp_deucsw;
> +   u32 rcubmabdtmr;
> +   u32 rcedata;
> +   u32 spare2gh;
> +
> +   /* Display 1 CZ domain */
> +   u32 gt_imr;
> +   u32 gt_ier;
> +   u32 pm_imr;
> +   u32 pm_ier;
> +   u32 gt_scratch[GEN7_GT_SCRATCH_REG_NUM];
> +
> +   /* GT SA CZ domain */
> +   u32 tilectl;
> +   u32 gt_fifoctl;
> +   u32 gtlc_wake_ctrl;
> +   u32 gtlc_survive;
> +   u32 pmwgicz;
> +
> +   /* Display 2 CZ domain */
> +   u32 gu_ctl0;
> +   u32 gu_ctl1;
> +   u32 pcbr;
> +   u32 clock_gate_dis2;
> +};
> +
>  static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>  {
> int domain = pci_domain_nr(dev_priv->drm.pdev->bus);
> @@ -466,6 +528,28 @@ static void intel_detect_preproduction_hw(struct 
> drm_i915_private *dev_priv)
> }
>  }
>
> +static int vlv_alloc_s0ix_state(struct drm_i915_private *i915)
> +{
> +   if (!IS_VALLEYVIEW(i915))
> +   return 0;
> +
> +   /* we write all the values in the structure, so no need to zero it 
> out */
> +   i915->s0ix_state = kmalloc(sizeof(struct vlv_s0ix_state), GFP_KERNEL);
> +   if (!i915->s0ix_state)
> +   return -ENOMEM;
> +
> +   return 0;
> +}
> +
> +static void vlv_free_s0ix_state(struct drm_i915_private *i915)
> +{
> +   if (!i915->s0ix_state)
> +   return;
> +
> +   kfree(i915->s0ix_state);
> +   i915->s0ix_state = NULL;
> +}
> +
>  /**
>   * i915_driver_early_probe - setup state not requiring device access
>   * @dev_priv: device private
> @@ -508,13 +592,17 @@ static int i915_driver_early_probe(struct 
> drm_i915_private *dev_priv)
> if (ret < 0)
> return ret;
>
> +   ret = vlv_alloc_s0ix_state(dev_priv);
> +   if (ret < 0)
> +   goto err_workqueues;
> +
> intel_wopcm_init_early(&dev_priv->wopcm);
>
> intel_gt_init_early(&dev_priv->gt, dev_priv);
>
> ret = i915_gem_init_early(dev_priv);
> if (ret < 0)
> -   goto err_workqueues;
> +   goto err_gt;
>
> /* This must be called before any calls to HAS_PCH_* */
> intel_detect_pch(dev_priv);
> @@ -536,8 +624,10 @@ static int i915_driver_early_probe(struct 
> drm_i915_private *dev_priv)
>
>  err_gem:
> i915_gem_cleanup_early(dev_priv);
> -err_workqueues:
> +err_gt:
> intel_gt_driver_late_release(&dev_priv->gt);
> +   vlv_free_s0ix_state(dev_priv);
> +err_workqueues:
> i915_workqueues_cleanup(dev_priv);
> return ret;
>  }
> @@ -553,6 +643,7 @@ static void i915_driver_late_release(struct 
> drm_i915_private *dev_priv)
> intel_power_domains_cleanup(dev_priv);
> i915_gem_cleanup_early(dev_priv);
> intel_gt_driver_late_release(&dev_priv->gt);
> +   vlv_free_s0ix_state(dev_priv);
> i915_workqueues_cleanup(dev_priv);
>
> pm_qos_remove_request(&dev_priv->sb_qos);
> @@ -2137,7 +2228,7 @@ static int i915_pm_restore(struct device *kdev)
>   */
>  static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  {
> -   struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
> +   struct vlv_s0ix_state *s = dev_priv->s0ix_state;
> int i;
>
> /* GAM 0x4000-0x4770 */
> @@ -2147,7 +2238,7 @@ static void vlv_save_g