Re: [PATCH 02/12] drm/i915: Only allow rpm on gen9+ with dmc loaded

[Animesh Manna <animesh.manna@intel.com>]

On 7/10/2015 1:34 AM, Daniel Vetter wrote:
> Instead of trying to deal with this complexity we'll simply require
> that the dmc firmware is available for runtime pm support. We do that
> by not releasing the rpm reference we acquire when starting the
> firmware loader work. Note that since we hold a rpm reference (and rpm
> get/put is synchronized with its own locking already) there's no need
> for any additional synchronization between the dmc loader and the rpm
> entry/exit code.
>
> Hence we can remove all dmc_load_status_get calls, they don't do
> anything any more.
>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

For BXT, without DMC firmware display engine can goto DC9, so if firmware failed to load then as we are holding rpm reference - it will block dc9 entry.
Do we really want to block dc9 if firmware is not present or failed to load?

Regards,
Animesh

> ---
>   drivers/gpu/drm/i915/intel_csr.c        |  9 +++++----
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 17 +++--------------
>   2 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 16cd9dae1c1b..03d83892cdb0 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -393,8 +393,11 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>   out:
>   	if (fw_loaded)
>   		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> -	else
> -		intel_csr_load_status_set(dev_priv, FW_FAILED);
> +
> +	/*
> +	 * We require the dmc firmware for runtime pm on gen9+ - leak the rpm
> +	 * reference in case this failed to disable rpm on.
> +	 */
>   
>   	release_firmware(fw);
>   }
> @@ -462,8 +465,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 2628b21ff2c0..ed8c0cee738f 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);
>   			}
>   		}
>   	}

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx