diff mbox series

[4/6] drm/i915: Dynamically allocate s0ix struct for VLV

Message ID 20190816012343.36433-5-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series Some more display uncore prep work | expand

Commit Message

Daniele Ceraolo Spurio Aug. 16, 2019, 1:23 a.m. UTC 
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 <daniele.ceraolospurio@intel.com>
Cc: Imre Deak <imre.deak@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(-)

Comments

Lucas De Marchi Aug. 16, 2019, 5:09 a.m. UTC | #1 
On Thu, Aug 15, 2019 at 6:24 PM Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> 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 <daniele.ceraolospurio@intel.com>
> Cc: Imre Deak <imre.deak@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_state;
> +       struct vlv_s0ix_state *s = dev_priv->s0ix_state;
>         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));
>
>         /* GT SA CZ domain, 0x100000-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;
>         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++)

why?

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, 0x100000-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
Jani Nikula Aug. 16, 2019, 9:35 a.m. UTC | #2 
On Thu, 15 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> 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 <daniele.ceraolospurio@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>

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.nikula@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_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.


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

>  
>  	/* GT SA CZ domain, 0x100000-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.

>  	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, 0x100000-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;

I'd keep the vlv_ prefix in the member name.

BR,
Jani.


>  
>  	enum {
>  		I915_SAGV_UNKNOWN = 0,
Chris Wilson Aug. 16, 2019, 9:40 a.m. UTC | #3 
Quoting Jani Nikula (2019-08-16 10:35:08)
> On Thu, 15 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> 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
Jani Nikula Aug. 16, 2019, 10:32 a.m. UTC | #4 
On Fri, 16 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2019-08-16 10:35:08)
>> On Thu, 15 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> 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.
Daniele Ceraolo Spurio Aug. 16, 2019, 3:30 p.m. UTC | #5 
<snip>

>>
>> @@ -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, 0x100000-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
> 
> 
>
Daniele Ceraolo Spurio Aug. 16, 2019, 3:33 p.m. UTC | #6 
On 8/16/19 2:35 AM, Jani Nikula wrote:
> On Thu, 15 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> 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 <daniele.ceraolospurio@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
> 
> 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.nikula@intel.com
> 
> 

<snip>


>> @@ -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, 0x100000-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, 0x100000-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;
> 
> I'd keep the vlv_ prefix in the member name.

ok (including the fwd decl mentioned in the other reply)

Daniele

> 
> BR,
> Jani.
> 
> 
>>   
>>   	enum {
>>   		I915_SAGV_UNKNOWN = 0,
>
diff mbox series

Patch

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_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));
 
 	/* GT SA CZ domain, 0x100000-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;
 	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]);
 
 	/* GT SA CZ domain, 0x100000-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,