Re: [SKL-DMC-BUGFIX 4/5] drm/i915/skl: Block disable call for pw1 if dmc firmware is present.

[Imre Deak <imre.deak@intel.com>]

On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> 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.
> 
> v1: Initial version.
> 
> v2: Based on revire commnents from Sunil,
> - condition check for pw1 is moved in skl_set_power_well.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index c660245..00cd4ff 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -636,9 +636,15 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  		}
>  	} else {
>  		if (enable_requested) {
> -			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 (IS_SKYLAKE(dev) &&
> +				(power_well->data == SKL_DISP_PW_1) &&
> +				(intel_csr_load_status_get(dev_priv) == FW_LOADED))

We'll only ever get here with the firmware loaded, so no need to check
for it. 

> +				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
> +			else {
> +				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
> +				POSTING_READ(HSW_PWR_WELL_DRIVER);
> +				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
> +			}

Not disabling PW1 here is a good solution for now imo, but ideally we
should move both PW1 enabling and disabling out from this code and do
these as part of the display init/uninit sequence. This could be done as
a follow-up to this patchset.

>  
>  			if (GEN9_ENABLE_DC5(dev) &&
>  				power_well->data == SKL_DISP_PW_2)


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