On Mon, Apr 13, 2015 at 02:33:50PM +0300, Imre Deak wrote:
> > @@ -424,6 +434,25 @@ static void skl_set_power_well(struct drm_i915_private
> > *dev_priv,
> > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask);
> > POSTING_READ(HSW_PWR_WELL_DRIVER);
> > DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
> > +
> > + if (GEN9_ENABLE_DC5(dev) &&
> > + power_well->data == SKL_DISP_PW_2) {
> > + if (dev_priv->csr.states <= FW_LOADING) {
> > + /*
> > + * TODO: wait for a completion event or
> > + * similar here instead of busy
> > + * waiting using wait_for function.
> > + */
> > + if (wait_for(
> > + intel_csr_load_status_get(
> > + dev_priv), 1000))
> > + DRM_ERROR("Timed out waiting
> > for CSR to be loaded!");
>
> This waits until the state is set to FW_LOADING or FW_FAILED, whereas it
> should wait until it's FW_LOADED. I think the above block becomes
> clearer by returning the state from the helper:
>
> if (GEN9_ENABLE_DC5(dev) && power_well->data == SKL_DISP_PW_2) {
> enum csr_state state;
>
> wait_for((state = intel_csr_state(dev_priv)) != FW_UNINITIALIZED, 1000);
> if (state != FW_LOADED)
> DRM_ERROR("CSR firmware not ready (%d)\n", state);
> else
> gen9_enable_dc5(dev_priv);
> }
Also, this level of indentation is becoming unmanageable. Maybe we
should move this code to skl_power_well_post_enable()?
--
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
