[Intel-gfx] [PATCH 04/18] drm/i915/gen9: block disable call for pw1 if dmc firmware is present.

[Daniel Vetter]

On Sun, Jul 26, 2015 at 12:30:25AM +0530, Animesh Manna wrote:
> Grabbing a runtime pm reference with intel_runtime_pm_get will only
> prevent device D3. But dmc firmware is required even earlier (namely
> for the skl power well 1). DMC is responsible to save the status of
> power well 1 and shut off the power well when panel is self refresh
> mode of display is completely off.
> 
> Another interesting criteria to work dmc as expected is pw1 to be
> enabled by driver and dmc will shut it off in its execution
> sequence. If already disabled by driver dmc will get confuse and
> behave differently than expected found during pc10 entry issue
> for skl.
> 
> So berfore we disable power-well 1, added check if dmc firmware is
> present and driver will not disable power well 1, but for any reason
> if firmware is not present of failed to load we can shut off the
> power well 1 which will save some power.
> 
> As skl is currently fully dependent on dmc to go in lowest possible
> power state (dc6) but the same is not applicable for bxt. Display
> engine can enter into dc9 without dmc, hence unblocking disable call.
> 
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Damien Lespiau <damien.lespiau at intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Sunil Kamath <sunil.kamath at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju at intel.com>

Please use the approach I've laid out in my original patch series with
"drm/i915: use correct power domain for csr loading" and "drm/i915: Only
allow rpm on gen9+ with dmc loaded". Your approach here requires a
flush_work in the runtime pm callbacks which can deadlock.

If you want to make dmc optional on bxt then you need to adjust the 2nd
patch a bit to no longer leak the runtime pm reference. Your approach here
is an inversion of control and that doesn't work well since control
inversions very easily lead to locking inversions.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6393b76..e6156d5 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1141,8 +1141,10 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>  
>  	for_each_power_well_rev(i, power_well, BIT(domain), power_domains) {
>  		WARN_ON(!power_well->count);
> -
> -		if (!--power_well->count && i915.disable_power_well) {
> +		if (IS_SKYLAKE(dev_priv) && (power_well->data == SKL_DISP_PW_1) &&
> +			(intel_csr_load_status_get(dev_priv) == FW_LOADED))
> +			DRM_DEBUG_KMS("dmc will disable pw1");
> +		else if (!--power_well->count && i915.disable_power_well) {
>  			DRM_DEBUG_KMS("disabling %s\n", power_well->name);
>  			power_well->hw_enabled = false;
>  			power_well->ops->disable(dev_priv, power_well);
> -- 
> 2.0.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch