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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jul 10, 2015 at 01:50:23PM +0530, Animesh Manna wrote:
> 
> 
> 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?

I guess for bxt we can lift the restriction and check
dev_priv->csr.dmc_payload in the bxt power well code and drop the power
well reference here by adjusting the condition.

This really was just to showcase what I had in mind with grabbing the
right power well reference: We use get/put to make sure that any of the
platform power well code is guaranteed to never run, instead of perhaps
adding another mutex or something similar (which usually results in more
design problems than just a get/put pair).

In short: This is just to demonstrate the flow, details can of course be
adjusted.
-Daniel

> 
> 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);
> >  			}
> >  		}
> >  	}
> 

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