Re: [PATCH 07/19] drm/i915: add port power domains

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 18 Feb 2014 00:02:08 +0200
Imre Deak <imre.deak@intel.com> wrote:

> Parts that poke port specific HW blocks like the encoder HW state
> readout or connector hotplug detect code need a way to check whether
> required power domains are on or enable/disable these. For this purpose
> add a set of power domains that refer to the port HW blocks. Get the
> proper port power domains during modeset.
> 
> For now when requesting the power domain for a DDI port get it for a 4
> lane configuration. This can be optimized later to request only the 2
> lane power domain, when proper support is added on the VLV PHY side for
> this. Atm, the PHY setup code assumes a 4 lane config in all cases.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 22 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h      | 11 ++++++++
>  drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c      |  9 +++++++
>  5 files changed, 90 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d90a707..14c7730 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2030,6 +2030,28 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>  		return "TRANSCODER_C";
>  	case POWER_DOMAIN_TRANSCODER_EDP:
>  		return "TRANSCODER_EDP";
> +	case POWER_DOMAIN_PORT_DDI_A_2_LANES:
> +		return "PORT_DDI_A_2_LANES";
> +	case POWER_DOMAIN_PORT_DDI_A_4_LANES:
> +		return "PORT_DDI_A_4_LANES";
> +	case POWER_DOMAIN_PORT_DDI_B_2_LANES:
> +		return "PORT_DDI_B_2_LANES";
> +	case POWER_DOMAIN_PORT_DDI_B_4_LANES:
> +		return "PORT_DDI_B_4_LANES";
> +	case POWER_DOMAIN_PORT_DDI_C_2_LANES:
> +		return "PORT_DDI_C_2_LANES";
> +	case POWER_DOMAIN_PORT_DDI_C_4_LANES:
> +		return "PORT_DDI_C_4_LANES";
> +	case POWER_DOMAIN_PORT_DDI_D_2_LANES:
> +		return "PORT_DDI_D_2_LANES";
> +	case POWER_DOMAIN_PORT_DDI_D_4_LANES:
> +		return "PORT_DDI_D_4_LANES";
> +	case POWER_DOMAIN_PORT_DSI:
> +		return "PORT_DSI";
> +	case POWER_DOMAIN_PORT_CRT:
> +		return "PORT_CRT";
> +	case POWER_DOMAIN_PORT_OTHER:
> +		return "PORT_OTHER";
>  	case POWER_DOMAIN_VGA:
>  		return "VGA";
>  	case POWER_DOMAIN_AUDIO:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eaa9210..76bd03a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -114,6 +114,17 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_TRANSCODER_B,
>  	POWER_DOMAIN_TRANSCODER_C,
>  	POWER_DOMAIN_TRANSCODER_EDP,
> +	POWER_DOMAIN_PORT_DDI_A_2_LANES,
> +	POWER_DOMAIN_PORT_DDI_A_4_LANES,
> +	POWER_DOMAIN_PORT_DDI_B_2_LANES,
> +	POWER_DOMAIN_PORT_DDI_B_4_LANES,
> +	POWER_DOMAIN_PORT_DDI_C_2_LANES,
> +	POWER_DOMAIN_PORT_DDI_C_4_LANES,
> +	POWER_DOMAIN_PORT_DDI_D_2_LANES,
> +	POWER_DOMAIN_PORT_DDI_D_4_LANES,
> +	POWER_DOMAIN_PORT_DSI,
> +	POWER_DOMAIN_PORT_CRT,
> +	POWER_DOMAIN_PORT_OTHER,
>  	POWER_DOMAIN_VGA,
>  	POWER_DOMAIN_AUDIO,
>  	POWER_DOMAIN_INIT,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fa50f6e..7ef06fa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3957,9 +3957,49 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
>  	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
>  		if ((1 << (domain)) & (mask))
>  
> -static unsigned long get_pipe_power_domains(struct drm_device *dev,
> -					    enum pipe pipe, bool pfit_enabled)
> +enum intel_display_power_domain
> +intel_display_port_power_domain(struct intel_encoder *intel_encoder)
>  {
> +	struct drm_device *dev = intel_encoder->base.dev;
> +	struct intel_digital_port *intel_dig_port;
> +
> +	switch (intel_encoder->type) {
> +	case INTEL_OUTPUT_UNKNOWN:
> +		/* Only DDI platforms should ever use this output type */
> +		WARN_ON_ONCE(!HAS_DDI(dev));
> +	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_HDMI:
> +	case INTEL_OUTPUT_EDP:
> +		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> +		switch (intel_dig_port->port) {
> +		case PORT_A:
> +			return POWER_DOMAIN_PORT_DDI_A_4_LANES;
> +		case PORT_B:
> +			return POWER_DOMAIN_PORT_DDI_B_4_LANES;
> +		case PORT_C:
> +			return POWER_DOMAIN_PORT_DDI_C_4_LANES;
> +		case PORT_D:
> +			return POWER_DOMAIN_PORT_DDI_D_4_LANES;
> +		default:
> +			WARN_ON_ONCE(1);
> +			return POWER_DOMAIN_PORT_OTHER;
> +		}
> +	case INTEL_OUTPUT_ANALOG:
> +		return POWER_DOMAIN_PORT_CRT;
> +	case INTEL_OUTPUT_DSI:
> +		return POWER_DOMAIN_PORT_DSI;
> +	default:
> +		return POWER_DOMAIN_PORT_OTHER;
> +	}
> +}
> +
> +static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
> +	bool pfit_enabled = intel_crtc->config.pch_pfit.enabled;
>  	unsigned long mask;
>  	enum transcoder transcoder;
>  
> @@ -3970,6 +4010,9 @@ static unsigned long get_pipe_power_domains(struct drm_device *dev,
>  	if (pfit_enabled)
>  		mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe));
>  
> +	for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> +		mask |= BIT(intel_display_port_power_domain(intel_encoder));
> +
>  	return mask;
>  }
>  
> @@ -4003,9 +4046,7 @@ static void modeset_update_power_wells(struct drm_device *dev)
>  		if (!crtc->base.enabled)
>  			continue;
>  
> -		pipe_domains[crtc->pipe] = get_pipe_power_domains(dev,
> -						crtc->pipe,
> -						crtc->config.pch_pfit.enabled);
> +		pipe_domains[crtc->pipe] = get_crtc_power_domains(&crtc->base);
>  
>  		for_each_power_domain(domain, pipe_domains[crtc->pipe])
>  			intel_display_power_get(dev_priv, domain);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6042797..e31eb1e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -733,6 +733,8 @@ bool intel_crtc_active(struct drm_crtc *crtc);
>  void hsw_enable_ips(struct intel_crtc *crtc);
>  void hsw_disable_ips(struct intel_crtc *crtc);
>  void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
> +enum intel_display_power_domain
> +intel_display_port_power_domain(struct intel_encoder *intel_encoder);
>  int valleyview_get_vco(struct drm_i915_private *dev_priv);
>  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_config *pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index faaa4ec..9cb7ed6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5412,6 +5412,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
>  #define HSW_ALWAYS_ON_POWER_DOMAINS (			\
>  	BIT(POWER_DOMAIN_PIPE_A) |			\
>  	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_CRT) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  #define HSW_DISPLAY_POWER_DOMAINS (				\
>  	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

I wonder if we want a way to parameterize the lane count instead, in
case we end up doing 1 lane for example in the future, or have some
other power domain that can provide multiple levels of power.

But I suppose we can deal with that if/when we come to it.

-- 
Jesse Barnes, Intel Open Source Technology Center