Re: [Intel-gfx] [PATCH v2] drm/i915: Kill check_power_well() calls

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 18 Dec 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> pps_{lock,unlock}() call intel_display_power_{get,put}() outside
> pps_mutes to avoid deadlocks with the power_domain mutex. In theory
> during aux transfers we should usually have the relevant power domain
> references already held by some higher level code, so this should not
> result in much overhead (exception being userspace i2c-dev access).
> However thanks to the check_power_well() calls in
> intel_display_power_{get/put}() we end up doing a few Punit reads for
> each aux transfer. Obviously doing this for each byte transferred via
> i2c-over-aux is not a good idea.
>
> I can't think of a good way to keep check_power_well() while eliminating
> the overhead, so let's just remove check_power_well() entirely.
>
> Fixes a driver init time regression introduced by:
>  commit 773538e86081d146e0020435d614f4b96996c1f9
>  Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>  Date:   Thu Sep 4 14:54:56 2014 +0300
>
>     drm/i915: Reset power sequencer pipe tracking when disp2d is off
>
> Credit goes to Jani for figuring this out.
>
> v2: Add the regression note in the commit message.
>
> Cc: stable@vger.kernel.org
> Cc: Egbert Eich <eich@suse.de>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86201
> Tested-by: Wendy Wang <wendy.wang@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 27 ---------------------------
>  1 file changed, 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6aa3a81..39e1b07 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -615,29 +615,6 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
>  		vlv_power_sequencer_reset(dev_priv);
>  }
>  
> -static void check_power_well_state(struct drm_i915_private *dev_priv,
> -				   struct i915_power_well *power_well)
> -{
> -	bool enabled = power_well->ops->is_enabled(dev_priv, power_well);
> -
> -	if (power_well->always_on || !i915.disable_power_well) {
> -		if (!enabled)
> -			goto mismatch;
> -
> -		return;
> -	}
> -
> -	if (enabled != (power_well->count > 0))
> -		goto mismatch;
> -
> -	return;
> -
> -mismatch:
> -	I915_STATE_WARN(1, "state mismatch for '%s' (always_on %d hw state %d use-count %d disable_power_well %d\n",
> -		  power_well->name, power_well->always_on, enabled,
> -		  power_well->count, i915.disable_power_well);
> -}
> -
>  /**
>   * intel_display_power_get - grab a power domain reference
>   * @dev_priv: i915 device instance
> @@ -669,8 +646,6 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
>  			power_well->ops->enable(dev_priv, power_well);
>  			power_well->hw_enabled = true;
>  		}
> -
> -		check_power_well_state(dev_priv, power_well);
>  	}
>  
>  	power_domains->domain_use_count[domain]++;
> @@ -709,8 +684,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>  			power_well->hw_enabled = false;
>  			power_well->ops->disable(dev_priv, power_well);
>  		}
> -
> -		check_power_well_state(dev_priv, power_well);
>  	}
>  
>  	mutex_unlock(&power_domains->lock);
> -- 
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center