Re: [PATCH] drm/i915: dynamic haswell display power well support

[Ben Widawsky <ben@bwidawsk.net>]

On Thu, 29 Nov 2012 16:19:54 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We can disable (almost) all the display hw if we only use pipe A, with
> the integrated edp transcoder on port A. Because we don't set the cpu
> transcoder that earyl (yet), we need to help us with a trick to simply
> check for any edp encoders.
> 
> And wrt the old code: Can anyone explain what that struct mutex
> grabbing was supposed to protect?
> 
> v2: Paulo Zanoni pointed out that we also need to configure the eDP
> cpu transcoder correctly.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

/me waits for tested-by on this one... you may want to ping Joe, since
he's one of the few I know with a working edp panel + HSW atm. (Also,
maybe run this by Art)

Power numbers and how it was tested would also be cool.

> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |  8 ++++-
>  drivers/gpu/drm/i915/intel_display.c | 66
> +++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h     |  1 -
> drivers/gpu/drm/i915/intel_pm.c      | 31 ----------------- 4 files
> changed, 68 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c index ad936c6..1a73a70 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -960,7 +960,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc
> *crtc) if (cpu_transcoder == TRANSCODER_EDP) {
>  		switch (pipe) {
>  		case PIPE_A:
> -			temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> +			/* Can only use the always-on power well for
> eDP when
> +			 * not using the panel fitter, and when not
> using motion
> +			 * blur mitigation (which we don't support).
> */
> +			if (dev_priv->pch_pf_size)
> +				temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> +			else
> +				temp |= TRANS_DDI_EDP_INPUT_A_ON;
>  			break;
>  		case PIPE_B:
>  			temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 52b6b0e..ea8e57c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5448,6 +5448,65 @@ static int ironlake_crtc_mode_set(struct
> drm_crtc *crtc, return fdi_config_ok ? ret : -EINVAL;
>  }
>  
> +/* Starting with Haswell, we have different power wells for
> + * different parts of the GPU. This attempts to enable them all.
> + */
> +static void haswell_set_power_well(struct drm_device *dev,
> +				   bool enable)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned long power_wells[] = {
> +		HSW_PWR_WELL_CTL1,
> +		HSW_PWR_WELL_CTL2,
> +		HSW_PWR_WELL_CTL4
> +	};
> +	int i;
> +
> +	if (!IS_HASWELL(dev))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(power_wells); i++) {
> +		int well = I915_READ(power_wells[i]);
> +
> +		if (enable) {
> +			I915_WRITE(power_wells[i], well &
> HSW_PWR_WELL_ENABLE);
> +			if (wait_for(I915_READ(power_wells[i]) &
> HSW_PWR_WELL_STATE, 20))
> +				DRM_ERROR("Error enabling power well
> %lx\n", power_wells[i]);
> +		} else {
> +			I915_WRITE(power_wells[i], well &
> ~HSW_PWR_WELL_ENABLE);
> +			if (wait_for((I915_READ(power_wells[i]) &
> HSW_PWR_WELL_STATE) == 0, 20))
> +				DRM_ERROR("Error disabling power
> well %lx\n", power_wells[i]);
> +		}
> +	}
> +}
> +
> +static void haswell_modeset_global_resources(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool enable = false;
> +	struct intel_crtc *crtc;
> +	struct intel_encoder *encoder;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> base.head) {
> +		if (crtc->pipe != PIPE_A && crtc->base.enabled)
> +			enable = true;
> +		/* XXX: Should check for edp transcoder here, but
> thanks to init
> +		 * sequence that's not yet availble. Just in case
> desktop eDP on
> +		 * PORT D is possible on haswell, too. */
> +	}
> +
> +	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> base.head) {
> +		if (encoder->type != INTEL_OUTPUT_EDP)
> +			enable = true;
> +	}
> +
> +	/* Even the eDP panel fitter is outside the always-on well.
> */
> +	if (dev_priv->pch_pf_size)
> +		enable = true;
> +
> +	haswell_set_power_well(dev, enable);
> +}
> +
>  static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  				 struct drm_display_mode *mode,
>  				 struct drm_display_mode
> *adjusted_mode, @@ -8421,6 +8480,8 @@ static void
> intel_init_display(struct drm_device *dev) } else if
> (IS_HASWELL(dev)) { dev_priv->display.fdi_link_train =
> hsw_fdi_link_train; dev_priv->display.write_eld = haswell_write_eld;
> +			dev_priv->display.modeset_global_resources =
> +				haswell_modeset_global_resources;
>  		} else
>  			dev_priv->display.update_wm = NULL;
>  	} else if (IS_G4X(dev)) {
> @@ -8592,11 +8653,6 @@ static void i915_disable_vga(struct drm_device
> *dev) 
>  void intel_modeset_init_hw(struct drm_device *dev)
>  {
> -	/* We attempt to init the necessary power wells early in the
> initialization
> -	 * time, so the subsystems that expect power to be enabled
> can work.
> -	 */
> -	intel_init_power_wells(dev);
> -
>  	intel_prepare_ddi(dev);
>  
>  	intel_init_clock_gating(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index 7ca7772..d20c603 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -646,7 +646,6 @@ extern void intel_update_fbc(struct drm_device
> *dev); extern void intel_gpu_ips_init(struct drm_i915_private
> *dev_priv); extern void intel_gpu_ips_teardown(void);
>  
> -extern void intel_init_power_wells(struct drm_device *dev);
>  extern void intel_enable_gt_powersave(struct drm_device *dev);
>  extern void intel_disable_gt_powersave(struct drm_device *dev);
>  extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c index f595b8d..8886130 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3916,37 +3916,6 @@ void intel_init_clock_gating(struct drm_device
> *dev) dev_priv->display.init_clock_gating(dev);
>  }
>  
> -/* Starting with Haswell, we have different power wells for
> - * different parts of the GPU. This attempts to enable them all.
> - */
> -void intel_init_power_wells(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned long power_wells[] = {
> -		HSW_PWR_WELL_CTL1,
> -		HSW_PWR_WELL_CTL2,
> -		HSW_PWR_WELL_CTL4
> -	};
> -	int i;
> -
> -	if (!IS_HASWELL(dev))
> -		return;
> -
> -	mutex_lock(&dev->struct_mutex);
> -
> -	for (i = 0; i < ARRAY_SIZE(power_wells); i++) {
> -		int well = I915_READ(power_wells[i]);
> -
> -		if ((well & HSW_PWR_WELL_STATE) == 0) {
> -			I915_WRITE(power_wells[i], well &
> HSW_PWR_WELL_ENABLE);
> -			if (wait_for((I915_READ(power_wells[i]) &
> HSW_PWR_WELL_STATE), 20))
> -				DRM_ERROR("Error enabling power well
> %lx\n", power_wells[i]);
> -		}
> -	}
> -
> -	mutex_unlock(&dev->struct_mutex);
> -}
> -
>  /* Set up chip specific power management-related functions */
>  void intel_init_pm(struct drm_device *dev)
>  {