diff mbox

[v2] drm/i915/gen9: Fix DMC firmware initialization

Message ID 1457121461-16729-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak March 4, 2016, 7:57 p.m. UTC
In commit 1e657ad7 we moved the last step of firmware initialization to
skl_display_core_init(), where it will be run only during system resume,
but not during driver loading. Since this init step needs to be done
whenever we program the firmware fix this by moving the initialization
to the end of intel_csr_load_program().

While at it simplify a bit csr_load_work_fn().

This issue prevented DC5/6 transitions, this change will re-enable those.

v2:
- remove debugging left-over and redundant comment in csr_load_work_fn()

Fixes: 1e657ad7a48f ("drm/i915/gen9: Write dc state debugmask bits only once")
CC: Mika Kuoppala <mika.kuoppala@intel.com>
CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c        | 40 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h        |  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++----------------
 3 files changed, 29 insertions(+), 35 deletions(-)

Comments

Mika Kuoppala March 7, 2016, 11:58 a.m. UTC | #1
Imre Deak <imre.deak@intel.com> writes:

> [ text/plain ]
> In commit 1e657ad7 we moved the last step of firmware initialization to
> skl_display_core_init(), where it will be run only during system resume,
> but not during driver loading. Since this init step needs to be done
> whenever we program the firmware fix this by moving the initialization
> to the end of intel_csr_load_program().
>
> While at it simplify a bit csr_load_work_fn().
>
> This issue prevented DC5/6 transitions, this change will re-enable those.
>
> v2:
> - remove debugging left-over and redundant comment in csr_load_work_fn()
>
> Fixes: 1e657ad7a48f ("drm/i915/gen9: Write dc state debugmask bits only once")
> CC: Mika Kuoppala <mika.kuoppala@intel.com>
> CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c        | 40 +++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_drv.h        |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++----------------
>  3 files changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 902054e..d417d9a 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -212,6 +212,24 @@ static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
>  	return NULL;
>  }
>  
> +static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
> +{
> +	uint32_t val, mask;
> +
> +	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> +
> +	if (IS_BROXTON(dev_priv))
> +		mask |= DC_STATE_DEBUG_MASK_CORES;
> +
> +	/* The below bit doesn't need to be cleared ever afterwards */
> +	val = I915_READ(DC_STATE_DEBUG);
> +	if ((val & mask) != mask) {
> +		val |= mask;
> +		I915_WRITE(DC_STATE_DEBUG, val);
> +		POSTING_READ(DC_STATE_DEBUG);
> +	}
> +}
> +
>  /**
>   * intel_csr_load_program() - write the firmware from memory to register.
>   * @dev_priv: i915 drm device.
> @@ -220,19 +238,19 @@ static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
>   * Everytime display comes back from low power state this function is called to
>   * copy the firmware from internal memory to registers.
>   */
> -bool intel_csr_load_program(struct drm_i915_private *dev_priv)
> +void intel_csr_load_program(struct drm_i915_private *dev_priv)
>  {
>  	u32 *payload = dev_priv->csr.dmc_payload;
>  	uint32_t i, fw_size;
>  
>  	if (!IS_GEN9(dev_priv)) {
>  		DRM_ERROR("No CSR support available for this platform\n");
> -		return false;
> +		return;
>  	}
>  
>  	if (!dev_priv->csr.dmc_payload) {
>  		DRM_ERROR("Tried to program CSR with empty payload\n");
> -		return false;
> +		return;
>  	}
>  
>  	fw_size = dev_priv->csr.dmc_fw_size;
> @@ -246,7 +264,7 @@ bool intel_csr_load_program(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->csr.dc_state = 0;
>  
> -	return true;
> +	gen9_set_dc_state_debugmask(dev_priv);
>  }
>  
>  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> @@ -388,18 +406,12 @@ static void csr_load_work_fn(struct work_struct *work)
>  
>  	ret = request_firmware(&fw, dev_priv->csr.fw_path,
>  			       &dev_priv->dev->pdev->dev);
> -	if (!fw)
> -		goto out;
> +	if (fw)
> +		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
>  

Why we check the fw pointer here and not the return value?

-Mika


> -	dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
> -	if (!dev_priv->csr.dmc_payload)
> -		goto out;
> -
> -	/* load csr program during system boot, as needed for DC states */
> -	intel_csr_load_program(dev_priv);
> -
> -out:
>  	if (dev_priv->csr.dmc_payload) {
> +		intel_csr_load_program(dev_priv);
> +
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>  
>  		DRM_INFO("Finished loading %s (v%u.%u)\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd0b4ea..3daf1e3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1279,7 +1279,7 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
>  
>  /* intel_csr.c */
>  void intel_csr_ucode_init(struct drm_i915_private *);
> -bool intel_csr_load_program(struct drm_i915_private *);
> +void intel_csr_load_program(struct drm_i915_private *);
>  void intel_csr_ucode_fini(struct drm_i915_private *);
>  
>  /* intel_dp.c */
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 09c52b1..5adf4b3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -470,24 +470,6 @@ static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
>  	  */
>  }
>  
> -static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
> -{
> -	uint32_t val, mask;
> -
> -	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> -
> -	if (IS_BROXTON(dev_priv))
> -		mask |= DC_STATE_DEBUG_MASK_CORES;
> -
> -	/* The below bit doesn't need to be cleared ever afterwards */
> -	val = I915_READ(DC_STATE_DEBUG);
> -	if ((val & mask) != mask) {
> -		val |= mask;
> -		I915_WRITE(DC_STATE_DEBUG, val);
> -		POSTING_READ(DC_STATE_DEBUG);
> -	}
> -}
> -
>  static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
>  				u32 state)
>  {
> @@ -2141,8 +2123,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  
>  	skl_init_cdclk(dev_priv);
>  
> -	if (dev_priv->csr.dmc_payload && intel_csr_load_program(dev_priv))
> -		gen9_set_dc_state_debugmask(dev_priv);
> +	if (dev_priv->csr.dmc_payload)
> +		intel_csr_load_program(dev_priv);
>  }
>  
>  static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
> -- 
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak March 7, 2016, 12:06 p.m. UTC | #2
On ma, 2016-03-07 at 13:58 +0200, Mika Kuoppala wrote:
> Imre Deak <imre.deak@intel.com> writes:
> 
> > [ text/plain ]
> > In commit 1e657ad7 we moved the last step of firmware
> > initialization to
> > skl_display_core_init(), where it will be run only during system
> > resume,
> > but not during driver loading. Since this init step needs to be
> > done
> > whenever we program the firmware fix this by moving the
> > initialization
> > to the end of intel_csr_load_program().
> > 
> > While at it simplify a bit csr_load_work_fn().
> > 
> > This issue prevented DC5/6 transitions, this change will re-enable
> > those.
> > 
> > v2:
> > - remove debugging left-over and redundant comment in
> > csr_load_work_fn()
> > 
> > Fixes: 1e657ad7a48f ("drm/i915/gen9: Write dc state debugmask bits
> > only once")
> > CC: Mika Kuoppala <mika.kuoppala@intel.com>
> > CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_csr.c        | 40
> > +++++++++++++++++++++------------
> >  drivers/gpu/drm/i915/intel_drv.h        |  2 +-
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++----------------
> >  3 files changed, 29 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > b/drivers/gpu/drm/i915/intel_csr.c
> > index 902054e..d417d9a 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -212,6 +212,24 @@ static const struct stepping_info
> > *intel_get_stepping_info(struct drm_device *de
> >  	return NULL;
> >  }
> >  
> > +static void gen9_set_dc_state_debugmask(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	uint32_t val, mask;
> > +
> > +	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> > +
> > +	if (IS_BROXTON(dev_priv))
> > +		mask |= DC_STATE_DEBUG_MASK_CORES;
> > +
> > +	/* The below bit doesn't need to be cleared ever
> > afterwards */
> > +	val = I915_READ(DC_STATE_DEBUG);
> > +	if ((val & mask) != mask) {
> > +		val |= mask;
> > +		I915_WRITE(DC_STATE_DEBUG, val);
> > +		POSTING_READ(DC_STATE_DEBUG);
> > +	}
> > +}
> > +
> >  /**
> >   * intel_csr_load_program() - write the firmware from memory to
> > register.
> >   * @dev_priv: i915 drm device.
> > @@ -220,19 +238,19 @@ static const struct stepping_info
> > *intel_get_stepping_info(struct drm_device *de
> >   * Everytime display comes back from low power state this function
> > is called to
> >   * copy the firmware from internal memory to registers.
> >   */
> > -bool intel_csr_load_program(struct drm_i915_private *dev_priv)
> > +void intel_csr_load_program(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 *payload = dev_priv->csr.dmc_payload;
> >  	uint32_t i, fw_size;
> >  
> >  	if (!IS_GEN9(dev_priv)) {
> >  		DRM_ERROR("No CSR support available for this
> > platform\n");
> > -		return false;
> > +		return;
> >  	}
> >  
> >  	if (!dev_priv->csr.dmc_payload) {
> >  		DRM_ERROR("Tried to program CSR with empty
> > payload\n");
> > -		return false;
> > +		return;
> >  	}
> >  
> >  	fw_size = dev_priv->csr.dmc_fw_size;
> > @@ -246,7 +264,7 @@ bool intel_csr_load_program(struct
> > drm_i915_private *dev_priv)
> >  
> >  	dev_priv->csr.dc_state = 0;
> >  
> > -	return true;
> > +	gen9_set_dc_state_debugmask(dev_priv);
> >  }
> >  
> >  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> > @@ -388,18 +406,12 @@ static void csr_load_work_fn(struct
> > work_struct *work)
> >  
> >  	ret = request_firmware(&fw, dev_priv->csr.fw_path,
> >  			       &dev_priv->dev->pdev->dev);
> > -	if (!fw)
> > -		goto out;
> > +	if (fw)
> > +		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv,
> > fw);
> >  
> 
> Why we check the fw pointer here and not the return value?

Yep, I noticed this too, it was an overlook in the the original code.
Since it happens to work I didn't want to change it in this patch. But
we should fix this as a follow-up.

--Imre

> 
> -Mika
> 
> 
> > -	dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
> > -	if (!dev_priv->csr.dmc_payload)
> > -		goto out;
> > -
> > -	/* load csr program during system boot, as needed for DC
> > states */
> > -	intel_csr_load_program(dev_priv);
> > -
> > -out:
> >  	if (dev_priv->csr.dmc_payload) {
> > +		intel_csr_load_program(dev_priv);
> > +
> >  		intel_display_power_put(dev_priv,
> > POWER_DOMAIN_INIT);
> >  
> >  		DRM_INFO("Finished loading %s (v%u.%u)\n",
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index cd0b4ea..3daf1e3 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1279,7 +1279,7 @@ u32 skl_plane_ctl_rotation(unsigned int
> > rotation);
> >  
> >  /* intel_csr.c */
> >  void intel_csr_ucode_init(struct drm_i915_private *);
> > -bool intel_csr_load_program(struct drm_i915_private *);
> > +void intel_csr_load_program(struct drm_i915_private *);
> >  void intel_csr_ucode_fini(struct drm_i915_private *);
> >  
> >  /* intel_dp.c */
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 09c52b1..5adf4b3 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -470,24 +470,6 @@ static void assert_can_disable_dc9(struct
> > drm_i915_private *dev_priv)
> >  	  */
> >  }
> >  
> > -static void gen9_set_dc_state_debugmask(struct drm_i915_private
> > *dev_priv)
> > -{
> > -	uint32_t val, mask;
> > -
> > -	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> > -
> > -	if (IS_BROXTON(dev_priv))
> > -		mask |= DC_STATE_DEBUG_MASK_CORES;
> > -
> > -	/* The below bit doesn't need to be cleared ever
> > afterwards */
> > -	val = I915_READ(DC_STATE_DEBUG);
> > -	if ((val & mask) != mask) {
> > -		val |= mask;
> > -		I915_WRITE(DC_STATE_DEBUG, val);
> > -		POSTING_READ(DC_STATE_DEBUG);
> > -	}
> > -}
> > -
> >  static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
> >  				u32 state)
> >  {
> > @@ -2141,8 +2123,8 @@ static void skl_display_core_init(struct
> > drm_i915_private *dev_priv,
> >  
> >  	skl_init_cdclk(dev_priv);
> >  
> > -	if (dev_priv->csr.dmc_payload &&
> > intel_csr_load_program(dev_priv))
> > -		gen9_set_dc_state_debugmask(dev_priv);
> > +	if (dev_priv->csr.dmc_payload)
> > +		intel_csr_load_program(dev_priv);
> >  }
> >  
> >  static void skl_display_core_uninit(struct drm_i915_private
> > *dev_priv)
Mika Kuoppala March 7, 2016, 12:36 p.m. UTC | #3
Imre Deak <imre.deak@intel.com> writes:

> [ text/plain ]
> In commit 1e657ad7 we moved the last step of firmware initialization to
> skl_display_core_init(), where it will be run only during system resume,
> but not during driver loading. Since this init step needs to be done
> whenever we program the firmware fix this by moving the initialization
> to the end of intel_csr_load_program().
>
> While at it simplify a bit csr_load_work_fn().
>
> This issue prevented DC5/6 transitions, this change will re-enable those.
>
> v2:
> - remove debugging left-over and redundant comment in csr_load_work_fn()
>
> Fixes: 1e657ad7a48f ("drm/i915/gen9: Write dc state debugmask bits only once")
> CC: Mika Kuoppala <mika.kuoppala@intel.com>
> CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_csr.c        | 40 +++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_drv.h        |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++----------------
>  3 files changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 902054e..d417d9a 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -212,6 +212,24 @@ static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
>  	return NULL;
>  }
>  
> +static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
> +{
> +	uint32_t val, mask;
> +
> +	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> +
> +	if (IS_BROXTON(dev_priv))
> +		mask |= DC_STATE_DEBUG_MASK_CORES;
> +
> +	/* The below bit doesn't need to be cleared ever afterwards */
> +	val = I915_READ(DC_STATE_DEBUG);
> +	if ((val & mask) != mask) {
> +		val |= mask;
> +		I915_WRITE(DC_STATE_DEBUG, val);
> +		POSTING_READ(DC_STATE_DEBUG);
> +	}
> +}
> +
>  /**
>   * intel_csr_load_program() - write the firmware from memory to register.
>   * @dev_priv: i915 drm device.
> @@ -220,19 +238,19 @@ static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
>   * Everytime display comes back from low power state this function is called to
>   * copy the firmware from internal memory to registers.
>   */
> -bool intel_csr_load_program(struct drm_i915_private *dev_priv)
> +void intel_csr_load_program(struct drm_i915_private *dev_priv)
>  {
>  	u32 *payload = dev_priv->csr.dmc_payload;
>  	uint32_t i, fw_size;
>  
>  	if (!IS_GEN9(dev_priv)) {
>  		DRM_ERROR("No CSR support available for this platform\n");
> -		return false;
> +		return;
>  	}
>  
>  	if (!dev_priv->csr.dmc_payload) {
>  		DRM_ERROR("Tried to program CSR with empty payload\n");
> -		return false;
> +		return;
>  	}
>  
>  	fw_size = dev_priv->csr.dmc_fw_size;
> @@ -246,7 +264,7 @@ bool intel_csr_load_program(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->csr.dc_state = 0;
>  
> -	return true;
> +	gen9_set_dc_state_debugmask(dev_priv);
>  }
>  
>  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> @@ -388,18 +406,12 @@ static void csr_load_work_fn(struct work_struct *work)
>  
>  	ret = request_firmware(&fw, dev_priv->csr.fw_path,
>  			       &dev_priv->dev->pdev->dev);
> -	if (!fw)
> -		goto out;
> +	if (fw)
> +		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
>  
> -	dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
> -	if (!dev_priv->csr.dmc_payload)
> -		goto out;
> -
> -	/* load csr program during system boot, as needed for DC states */
> -	intel_csr_load_program(dev_priv);
> -
> -out:
>  	if (dev_priv->csr.dmc_payload) {
> +		intel_csr_load_program(dev_priv);
> +
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>  
>  		DRM_INFO("Finished loading %s (v%u.%u)\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd0b4ea..3daf1e3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1279,7 +1279,7 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
>  
>  /* intel_csr.c */
>  void intel_csr_ucode_init(struct drm_i915_private *);
> -bool intel_csr_load_program(struct drm_i915_private *);
> +void intel_csr_load_program(struct drm_i915_private *);
>  void intel_csr_ucode_fini(struct drm_i915_private *);
>  
>  /* intel_dp.c */
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 09c52b1..5adf4b3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -470,24 +470,6 @@ static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
>  	  */
>  }
>  
> -static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
> -{
> -	uint32_t val, mask;
> -
> -	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> -
> -	if (IS_BROXTON(dev_priv))
> -		mask |= DC_STATE_DEBUG_MASK_CORES;
> -
> -	/* The below bit doesn't need to be cleared ever afterwards */
> -	val = I915_READ(DC_STATE_DEBUG);
> -	if ((val & mask) != mask) {
> -		val |= mask;
> -		I915_WRITE(DC_STATE_DEBUG, val);
> -		POSTING_READ(DC_STATE_DEBUG);
> -	}
> -}
> -
>  static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
>  				u32 state)
>  {
> @@ -2141,8 +2123,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  
>  	skl_init_cdclk(dev_priv);
>  
> -	if (dev_priv->csr.dmc_payload && intel_csr_load_program(dev_priv))
> -		gen9_set_dc_state_debugmask(dev_priv);
> +	if (dev_priv->csr.dmc_payload)
> +		intel_csr_load_program(dev_priv);
>  }
>  
>  static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
> -- 
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 902054e..d417d9a 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -212,6 +212,24 @@  static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
 	return NULL;
 }
 
+static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
+{
+	uint32_t val, mask;
+
+	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
+
+	if (IS_BROXTON(dev_priv))
+		mask |= DC_STATE_DEBUG_MASK_CORES;
+
+	/* The below bit doesn't need to be cleared ever afterwards */
+	val = I915_READ(DC_STATE_DEBUG);
+	if ((val & mask) != mask) {
+		val |= mask;
+		I915_WRITE(DC_STATE_DEBUG, val);
+		POSTING_READ(DC_STATE_DEBUG);
+	}
+}
+
 /**
  * intel_csr_load_program() - write the firmware from memory to register.
  * @dev_priv: i915 drm device.
@@ -220,19 +238,19 @@  static const struct stepping_info *intel_get_stepping_info(struct drm_device *de
  * Everytime display comes back from low power state this function is called to
  * copy the firmware from internal memory to registers.
  */
-bool intel_csr_load_program(struct drm_i915_private *dev_priv)
+void intel_csr_load_program(struct drm_i915_private *dev_priv)
 {
 	u32 *payload = dev_priv->csr.dmc_payload;
 	uint32_t i, fw_size;
 
 	if (!IS_GEN9(dev_priv)) {
 		DRM_ERROR("No CSR support available for this platform\n");
-		return false;
+		return;
 	}
 
 	if (!dev_priv->csr.dmc_payload) {
 		DRM_ERROR("Tried to program CSR with empty payload\n");
-		return false;
+		return;
 	}
 
 	fw_size = dev_priv->csr.dmc_fw_size;
@@ -246,7 +264,7 @@  bool intel_csr_load_program(struct drm_i915_private *dev_priv)
 
 	dev_priv->csr.dc_state = 0;
 
-	return true;
+	gen9_set_dc_state_debugmask(dev_priv);
 }
 
 static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
@@ -388,18 +406,12 @@  static void csr_load_work_fn(struct work_struct *work)
 
 	ret = request_firmware(&fw, dev_priv->csr.fw_path,
 			       &dev_priv->dev->pdev->dev);
-	if (!fw)
-		goto out;
+	if (fw)
+		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
 
-	dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
-	if (!dev_priv->csr.dmc_payload)
-		goto out;
-
-	/* load csr program during system boot, as needed for DC states */
-	intel_csr_load_program(dev_priv);
-
-out:
 	if (dev_priv->csr.dmc_payload) {
+		intel_csr_load_program(dev_priv);
+
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
 		DRM_INFO("Finished loading %s (v%u.%u)\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cd0b4ea..3daf1e3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1279,7 +1279,7 @@  u32 skl_plane_ctl_rotation(unsigned int rotation);
 
 /* intel_csr.c */
 void intel_csr_ucode_init(struct drm_i915_private *);
-bool intel_csr_load_program(struct drm_i915_private *);
+void intel_csr_load_program(struct drm_i915_private *);
 void intel_csr_ucode_fini(struct drm_i915_private *);
 
 /* intel_dp.c */
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 09c52b1..5adf4b3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -470,24 +470,6 @@  static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
 	  */
 }
 
-static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
-{
-	uint32_t val, mask;
-
-	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
-
-	if (IS_BROXTON(dev_priv))
-		mask |= DC_STATE_DEBUG_MASK_CORES;
-
-	/* The below bit doesn't need to be cleared ever afterwards */
-	val = I915_READ(DC_STATE_DEBUG);
-	if ((val & mask) != mask) {
-		val |= mask;
-		I915_WRITE(DC_STATE_DEBUG, val);
-		POSTING_READ(DC_STATE_DEBUG);
-	}
-}
-
 static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
 				u32 state)
 {
@@ -2141,8 +2123,8 @@  static void skl_display_core_init(struct drm_i915_private *dev_priv,
 
 	skl_init_cdclk(dev_priv);
 
-	if (dev_priv->csr.dmc_payload && intel_csr_load_program(dev_priv))
-		gen9_set_dc_state_debugmask(dev_priv);
+	if (dev_priv->csr.dmc_payload)
+		intel_csr_load_program(dev_priv);
 }
 
 static void skl_display_core_uninit(struct drm_i915_private *dev_priv)