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.vet...@intel.com>
> Cc: Damien Lespiau <damien.lesp...@intel.com>
> Cc: Imre Deak <imre.d...@intel.com>
> Cc: Sunil Kamath <sunil.kam...@intel.com>
> Signed-off-by: Animesh Manna <animesh.ma...@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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to