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 <[email protected]>
> Cc: Damien Lespiau <[email protected]>
> Cc: Imre Deak <[email protected]>
> Cc: Sunil Kamath <[email protected]>
> Signed-off-by: Animesh Manna <[email protected]>
> Signed-off-by: Vathsala Nagaraju <[email protected]>
> ---
>  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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to