On Tue, Sep 11, 2018 at 05:56:01PM -0700, Manasi Navare wrote:
> On Icelake, a separate power well PG2 is created for
> VDSC engine used for eDP/MIPI DSI. This patch adds a new
> display power domain for Power well 2.
> 
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: Imre Deak <imre.d...@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.h    |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.h 
> b/drivers/gpu/drm/i915/intel_display.h
> index 3fe52788b4cf..bef71d27cdfe 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -256,6 +256,7 @@ enum intel_display_power_domain {
>       POWER_DOMAIN_MODESET,
>       POWER_DOMAIN_GT_IRQ,
>       POWER_DOMAIN_INIT,
> +     POWER_DOMAIN_VDSC_EDP_MIPI,

This is better named VDSC_PIPE_A. The other pipes have also VDSC
functionality which could be on separate power wells in the future.

>  
>       POWER_DOMAIN_NUM,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 480dadb1047b..146e2d6cf954 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum 
> intel_display_power_domain domain)
>               return "MODESET";
>       case POWER_DOMAIN_GT_IRQ:
>               return "GT_IRQ";
> +     case POWER_DOMAIN_VDSC_EDP_MIPI:
> +             return "VDSC_EDP_MIPI";
>       default:
>               MISSING_CASE(domain);
>               return "?";
> @@ -1966,18 +1968,16 @@ void intel_display_power_put(struct drm_i915_private 
> *dev_priv,
>       BIT_ULL(POWER_DOMAIN_AUDIO) |                   \
>       BIT_ULL(POWER_DOMAIN_INIT))
>       /*
> -      * - transcoder WD
> -      * - KVMR (HW control)
> +      * - eDP/MIPI DSI VDSC

We're not changing anything in the PW3 domains list, so why changing
the above?

>        */
>  #define ICL_PW_2_POWER_DOMAINS (                     \
> -     ICL_PW_3_POWER_DOMAINS |                        \
> -     BIT_ULL(POWER_DOMAIN_INIT))

The above is bogus, both the PW3 domains and the INIT domain should
stay included in the list of PW2 domains.

> +     BIT_ULL(POWER_DOMAIN_VDSC_EDP_MIPI))
>       /*
> -      * - eDP/DSI VDSC
> +      * - transcoder WD

Why adding here the transcoder WD?

>        * - KVMR (HW control)
>        */
>  #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS (           \
> -     ICL_PW_2_POWER_DOMAINS |                        \
> +     ICL_PW_3_POWER_DOMAINS |                        \

The PW2 domain list should stay included in the DC_OFF domain list (and
so no need to add the PW3 domain list either).

>       BIT_ULL(POWER_DOMAIN_MODESET) |                 \
>       BIT_ULL(POWER_DOMAIN_AUX_A) |                   \
>       BIT_ULL(POWER_DOMAIN_INIT))
> -- 
> 2.18.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to