diff mbox

[05/18] drm/i915/gen9: csr_init after runtime pm enable

Message ID 1437850839-16782-6-git-send-email-animesh.manna@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Manna, Animesh July 25, 2015, 7 p.m. UTC
As skl is fully dependent on dmc to go to low power state (dc5/dc6)
which requires a trigger from rpm and to ensure the dmc firmware
is available for runtime pm support rpm-reference-count is used
by not releasing the rpm reference acquire when starting the
firmware loader work.

So moved the intel_csr_ucode_init call after runtime pm enable.

Since have introduced a async work in next patches for loading
firmware and flush_work() will be used while disabling pw2. So
there's no need for any additional synchronization between the
dmc loader and trigger for low power state.

Note that for bxt without dmc, display engine can go to lowest
possible state (dc9), so releasing the rpm reference.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         |  6 +++---
 drivers/gpu/drm/i915/intel_csr.c        | 11 ++++++-----
 drivers/gpu/drm/i915/intel_runtime_pm.c | 17 +++--------------
 3 files changed, 12 insertions(+), 22 deletions(-)

Comments

Daniel Vetter July 28, 2015, 7:56 a.m. UTC | #1
On Sun, Jul 26, 2015 at 12:30:26AM +0530, Animesh Manna wrote:
> As skl is fully dependent on dmc to go to low power state (dc5/dc6)
> which requires a trigger from rpm and to ensure the dmc firmware
> is available for runtime pm support rpm-reference-count is used
> by not releasing the rpm reference acquire when starting the
> firmware loader work.
> 
> So moved the intel_csr_ucode_init call after runtime pm enable.
> 
> Since have introduced a async work in next patches for loading
> firmware and flush_work() will be used while disabling pw2. So
> there's no need for any additional synchronization between the
> dmc loader and trigger for low power state.
> 
> Note that for bxt without dmc, display engine can go to lowest
> possible state (dc9), so releasing the rpm reference.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c         |  6 +++---
>  drivers/gpu/drm/i915/intel_csr.c        | 11 ++++++-----
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 17 +++--------------
>  3 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b1f9e55..cdd3fbd 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -877,9 +877,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	intel_uncore_init(dev);
>  
> -	/* Load CSR Firmware for SKL */
> -	intel_csr_ucode_init(dev);
> -
>  	ret = i915_gem_gtt_init(dev);
>  	if (ret)
>  		goto out_freecsr;
> @@ -1025,6 +1022,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	intel_runtime_pm_enable(dev_priv);
>  
> +	/* Load CSR Firmware for SKL */
> +	intel_csr_ucode_init(dev);
> +
>  	i915_audio_component_init(dev_priv);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index f440299..e759190 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -404,10 +404,13 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  
>  	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
>  out:
> -	if (fw_loaded)
> +	if (fw_loaded || IS_BROXTON(dev))

Imo the IS_BXT should be separate patch.

>  		intel_runtime_pm_put(dev_priv);
> -	else
> -		intel_csr_load_status_set(dev_priv, FW_FAILED);
> +
> +	/*
> +	 * We require the dmc firmware for runtime pm on skl - leak the rpm
> +	 * reference in case this failed to disable rpm on.
> +	 */

Looks like part of my "drm/i915: Only allow rpm on gen9+ with dmc loaded"
patch snuck in here, should be split out again.
-Daniel

>  
>  	release_firmware(fw);
>  }
> @@ -477,8 +480,6 @@ void intel_csr_ucode_fini(struct drm_device *dev)
>  
>  void assert_csr_loaded(struct drm_i915_private *dev_priv)
>  {
> -	WARN(intel_csr_load_status_get(dev_priv) != FW_LOADED,
> -	     "CSR is not loaded.\n");
>  	WARN(!I915_READ(CSR_PROGRAM_BASE),
>  				"CSR program storage start is NULL\n");
>  	WARN(!I915_READ(CSR_SSP_BASE), "CSR SSP Base Not fine\n");
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e6156d5..a9bb299 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -644,21 +644,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  
>  			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
>  				power_well->data == SKL_DISP_PW_2) {
> -				enum csr_state state;
> -				/* TODO: wait for a completion event or
> -				 * similar here instead of busy
> -				 * waiting using wait_for function.
> -				 */
> -				wait_for((state = intel_csr_load_status_get(dev_priv)) !=
> -						FW_UNINITIALIZED, 1000);
> -				if (state != FW_LOADED)
> -					DRM_ERROR("CSR firmware not ready (%d)\n",
> -							state);
> +				if (SKL_ENABLE_DC6(dev))
> +					skl_enable_dc6(dev_priv);
>  				else
> -					if (SKL_ENABLE_DC6(dev))
> -						skl_enable_dc6(dev_priv);
> -					else
> -						gen9_enable_dc5(dev_priv);
> +					gen9_enable_dc5(dev_priv);
>  			}
>  		}
>  	}
> -- 
> 2.0.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b1f9e55..cdd3fbd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -877,9 +877,6 @@  int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_uncore_init(dev);
 
-	/* Load CSR Firmware for SKL */
-	intel_csr_ucode_init(dev);
-
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
 		goto out_freecsr;
@@ -1025,6 +1022,9 @@  int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_runtime_pm_enable(dev_priv);
 
+	/* Load CSR Firmware for SKL */
+	intel_csr_ucode_init(dev);
+
 	i915_audio_component_init(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index f440299..e759190 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -404,10 +404,13 @@  static void finish_csr_load(const struct firmware *fw, void *context)
 
 	DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
 out:
-	if (fw_loaded)
+	if (fw_loaded || IS_BROXTON(dev))
 		intel_runtime_pm_put(dev_priv);
-	else
-		intel_csr_load_status_set(dev_priv, FW_FAILED);
+
+	/*
+	 * We require the dmc firmware for runtime pm on skl - leak the rpm
+	 * reference in case this failed to disable rpm on.
+	 */
 
 	release_firmware(fw);
 }
@@ -477,8 +480,6 @@  void intel_csr_ucode_fini(struct drm_device *dev)
 
 void assert_csr_loaded(struct drm_i915_private *dev_priv)
 {
-	WARN(intel_csr_load_status_get(dev_priv) != FW_LOADED,
-	     "CSR is not loaded.\n");
 	WARN(!I915_READ(CSR_PROGRAM_BASE),
 				"CSR program storage start is NULL\n");
 	WARN(!I915_READ(CSR_SSP_BASE), "CSR SSP Base Not fine\n");
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e6156d5..a9bb299 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -644,21 +644,10 @@  static void skl_set_power_well(struct drm_i915_private *dev_priv,
 
 			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
 				power_well->data == SKL_DISP_PW_2) {
-				enum csr_state state;
-				/* TODO: wait for a completion event or
-				 * similar here instead of busy
-				 * waiting using wait_for function.
-				 */
-				wait_for((state = intel_csr_load_status_get(dev_priv)) !=
-						FW_UNINITIALIZED, 1000);
-				if (state != FW_LOADED)
-					DRM_ERROR("CSR firmware not ready (%d)\n",
-							state);
+				if (SKL_ENABLE_DC6(dev))
+					skl_enable_dc6(dev_priv);
 				else
-					if (SKL_ENABLE_DC6(dev))
-						skl_enable_dc6(dev_priv);
-					else
-						gen9_enable_dc5(dev_priv);
+					gen9_enable_dc5(dev_priv);
 			}
 		}
 	}